From d2a9484fb86fab1fc816473e41606ab162f607cb Mon Sep 17 00:00:00 2001 From: git-hulk Date: Wed, 2 Sep 2026 18:28:09 +0800 Subject: [PATCH] Write the saved capture from the handle it was captured on dumper_create opened a *second* live capture on the same interface, compiled the filter onto it again and ran its own pcap_loop in its own thread. So `-w` doubled the kernel capture cost and, because the two handles are independent, saved a different set of packets from the ones being analysed. It was also skipped entirely when reading a file, so `-r in.pcap -w out.pcap` silently wrote nothing. The sniffer now owns the dump file and writes each frame from its own handler, before anything is parsed, so the saved capture is exactly what libpcap delivered. That drops the second handle, the dumper thread and its loop, and makes -w work with -r. Assistant By Opus 5 Co-Authored-By: Claude Opus 5 (1M context) --- src/dumper.c | 56 +++++++++++++++--------------------------------- src/dumper.h | 9 ++++---- src/server.c | 38 -------------------------------- src/server.h | 2 -- src/sniffer.c | 9 ++++++++ src/sniffer.h | 2 ++ tests/e2e/run.sh | 20 +++++++++++++++++ 7 files changed, 52 insertions(+), 84 deletions(-) diff --git a/src/dumper.c b/src/dumper.c index dff51a1..d38d4f2 100644 --- a/src/dumper.c +++ b/src/dumper.c @@ -9,57 +9,35 @@ * **/ +#include #include #include "dumper.h" -#include "sniffer.h" -#include "server.h" -void dump_packet_handler(u_char *file, const struct pcap_pkthdr *header, const u_char *pkt_data) { - pcap_dump(file, header, pkt_data); -} - -struct dumper *dumper_create(struct options *opts, char *err) { +struct dumper *dumper_create(pcap_t *pcap, const char *path, char *err) { struct dumper *d; - pcap_t *pcap; - pcap_dumper_t *file; - struct bpf_program *bpf; - - d = malloc(sizeof(*d)); - d->pcap = NULL; - d->file = NULL; - d->bpf = NULL; - pcap = sniffer_online(opts->dev, opts->snaplen, opts->buf_size, err); - if (!pcap) goto error; - d->pcap = pcap; - bpf = sniffer_compile(pcap, opts->filter, err); - if (!bpf) goto error; - d->bpf = bpf; - file = pcap_dump_open(pcap, opts->save_file); - if (!file) goto error; - d->file = file; + d = calloc(1, sizeof(*d)); + if (!d) { + snprintf(err, MAX_ERR_BUFF_SIZE, "out of memory"); + return NULL; + } + d->file = pcap_dump_open(pcap, path); + if (!d->file) { + snprintf(err, MAX_ERR_BUFF_SIZE, "%s", pcap_geterr(pcap)); + free(d); + return NULL; + } return d; - -error: - dumper_destroy(d); - return NULL; -} - -void dumper_terminate(struct dumper *d) { - pcap_breakloop(d->pcap); } -int dumper_run(struct dumper *d) { - return pcap_loop(d->pcap, 0, dump_packet_handler, (unsigned char *)d->file); +void dumper_write(struct dumper *d, const struct pcap_pkthdr *header, + const unsigned char *packet) { + pcap_dump((unsigned char *)d->file, header, packet); } void dumper_destroy(struct dumper *d) { - if (d->pcap) pcap_close(d->pcap); - if (d->bpf) { - pcap_freecode(d->bpf); - free(d->bpf); - } + if (!d) return; if (d->file) pcap_dump_close(d->file); free(d); } diff --git a/src/dumper.h b/src/dumper.h index 58d0258..054f8f6 100644 --- a/src/dumper.h +++ b/src/dumper.h @@ -16,14 +16,13 @@ #include "tcpkit.h" struct dumper { - struct bpf_program *bpf; pcap_dumper_t *file; - pcap_t *pcap; }; -struct dumper *dumper_create(struct options *opts, char *err); -void dumper_terminate(struct dumper *d); -int dumper_run(struct dumper *d); +/* Writes to `path` through `pcap`, the handle the packets are captured on. */ +struct dumper *dumper_create(pcap_t *pcap, const char *path, char *err); +void dumper_write(struct dumper *d, const struct pcap_pkthdr *header, + const unsigned char *packet); void dumper_destroy(struct dumper *d); #endif diff --git a/src/server.c b/src/server.c index bd699cd..e6a4b05 100644 --- a/src/server.c +++ b/src/server.c @@ -20,19 +20,16 @@ #include "tcpkit.h" #include "server.h" #include "sniffer.h" -#include "dumper.h" #include "log.h" #include "stats.h" #include "cJSON.h" #include "hashtable.h" -static int server_spwan_dumper_thread(struct server *srv); static void *server_stats_loop(void *arg); static int server_spwan_stats_thread(struct server *srv); struct server *server_create(struct options *opts, char *err) { struct server *srv; - struct dumper *d; srv = calloc(1, sizeof(*srv)); if (!srv) { @@ -43,14 +40,6 @@ struct server *server_create(struct options *opts, char *err) { srv->sniffer = sniffer_create(opts, err); if (!srv->sniffer) goto error; - if (!opts->offline_file && opts->save_file) { - if (!(d = dumper_create(opts, err))) goto error; - srv->dumper = d; - if (server_spwan_dumper_thread(srv) != 0) { - snprintf(err, MAX_ERR_BUFF_SIZE, "create the dumper thread: %s", strerror(errno)); - goto error; - } - } if (opts->protocol != ProtocolRaw) { if (server_spwan_stats_thread(srv) != 0) { snprintf(err, MAX_ERR_BUFF_SIZE, "create the stats thread: %s", strerror(errno)); @@ -61,7 +50,6 @@ struct server *server_create(struct options *opts, char *err) { error: server_terminate(srv); - if (srv->dumper_tid) pthread_join(srv->dumper_tid, NULL); if (srv->stats_tid) pthread_join(srv->stats_tid, NULL); server_destroy(srv); return NULL; @@ -76,7 +64,6 @@ int server_run(struct server *srv, char *err) { return -1; } server_terminate(srv); - if (srv->dumper_tid) pthread_join(srv->dumper_tid, NULL); if (srv->stats_tid) pthread_join(srv->stats_tid, NULL); if (!srv->opts->offline_file) { if (pcap_stats(srv->sniffer->pcap, &stat) == 0) { @@ -98,39 +85,14 @@ void server_terminate(struct server *srv) { if (!srv || TK_LOAD(&srv->stopped)) return; TK_STORE(&srv->stopped, 1); sniffer_terminate(srv->sniffer); - if (srv->dumper) dumper_terminate(srv->dumper); } void server_destroy(struct server *srv) { if (!srv) return; sniffer_destroy(srv->sniffer); - if (srv->dumper) dumper_destroy(srv->dumper); free(srv); } -static void* server_dump_loop(void *arg) { - struct dumper *d; - char err[MAX_ERR_BUFF_SIZE]; - - d = (struct dumper *) arg; - if (dumper_run(d) == -1) { - snprintf(err, MAX_ERR_BUFF_SIZE, "%s", pcap_geterr(d->pcap)); - } - return NULL; -} - -static int server_spwan_dumper_thread(struct server *srv) { - int ret; - pthread_t tid; - - ret = pthread_create(&tid, NULL, server_dump_loop, srv->dumper); - if (ret != 0) { - return ret; - } - srv->dumper_tid = tid; - return 0; -} - static int server_spwan_stats_thread(struct server *srv) { int ret; pthread_t tid; diff --git a/src/server.h b/src/server.h index d0bdd5d..41df1d4 100644 --- a/src/server.h +++ b/src/server.h @@ -27,8 +27,6 @@ struct server { struct options *opts; struct sniffer *sniffer; - struct dumper* dumper; - pthread_t dumper_tid; pthread_t stats_tid; /* Set by server_terminate, which also runs from the signal handler, and * read by the stats thread. */ diff --git a/src/sniffer.c b/src/sniffer.c index 600fd68..0a35029 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -111,6 +111,11 @@ struct sniffer *sniffer_create(struct options *opts, char *err) { sniffer->bpf = bpf; } + if (opts->save_file) { + sniffer->dumper = dumper_create(pcap, opts->save_file, err); + if (!sniffer->dumper) goto error; + } + if (opts->script) { lua_state = lua_state_create(opts->script, err); if (!lua_state) goto error; @@ -136,6 +141,7 @@ void sniffer_destroy(struct sniffer *sniffer) { free(sniffer->bpf); } if (sniffer->lua_state) lua_close(sniffer->lua_state); + dumper_destroy(sniffer->dumper); free(sniffer); } @@ -190,6 +196,9 @@ static void packet_handler(unsigned char *user, struct user_packet upacket; struct sniffer *sniffer = (struct sniffer*) user; + /* Save the frame exactly as it was captured, before anything is parsed. */ + if (sniffer->dumper) dumper_write(sniffer->dumper, header, packet); + switch(pcap_datalink((pcap_t*)sniffer->pcap)) { case DLT_NULL: linkhdr_size = NULL_HDRLEN; diff --git a/src/sniffer.h b/src/sniffer.h index c92d800..e6e7308 100644 --- a/src/sniffer.h +++ b/src/sniffer.h @@ -19,6 +19,7 @@ #include "tcpkit.h" #include "stats.h" #include "hashtable.h" +#include "dumper.h" struct sniffer { pcap_t *pcap; @@ -39,6 +40,7 @@ struct sniffer { int expire_primed; lua_State *lua_state; struct bpf_program *bpf; + struct dumper *dumper; }; static inline void sniffer_stats_lock(struct sniffer *sniffer) { diff --git a/tests/e2e/run.sh b/tests/e2e/run.sh index b73d115..3678e5a 100755 --- a/tests/e2e/run.sh +++ b/tests/e2e/run.sh @@ -151,6 +151,26 @@ expect_lines "a udp filter suppresses the redis latency lines" "$out" 0 replay redis-session.pcap -p redis "tcp port 6379" expect_lines "a matching filter keeps the redis latency lines" "$out" 2 +echo "== e2e: writing a capture" +dump="$tmpdir/dumped.pcap" +tcpkit_run -r fixtures/redis-session.pcap -P "$STATS_PORT" -p raw -w "$dump" +expect_status "-w alongside -r exits cleanly" "$status" 0 +if [ -s "$dump" ]; then + ok "-w writes a capture" +else + fail "-w writes a capture" +fi + +tcpkit_run -r "$dump" -P "$STATS_PORT" -p raw +expect_lines "the written capture replays identically" "$out" 8 +expect_contains "the written capture keeps the addresses" "$out" \ + "10.0.0.1.51137 > 10.0.0.2.6379" + +tcpkit_run -r fixtures/redis-session.pcap -P "$STATS_PORT" -p raw \ + -w "$tmpdir/filtered.pcap" udp +tcpkit_run -r "$tmpdir/filtered.pcap" -P "$STATS_PORT" -p raw +expect_lines "only the filtered packets are written" "$out" 1 + echo "== e2e: error handling" replay does-not-exist.pcap -p redis expect_status "a missing capture file exits with an error" "$status" 1