From 9406c9ad06258611edbebb9b5e11351b48beea03 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Wed, 2 Sep 2026 18:30:38 +0800 Subject: [PATCH] Measure latency on connections that were already established The server side of a connection was only ever learned from a captured SYN, so a connection that existed before tcpkit started was invisible: packet_direction returned -1 for every one of its packets and they were dropped without being counted. That is the normal case for the tool's own documented use, since redis and memcached clients hold pooled connections open for the life of the process -- `tcpkit -i eth0 tcp port 6379 -p redis` against a busy server could sit there printing nothing at all until a client happened to reconnect. When neither end of a data-carrying packet is a known endpoint, take the lower of the two ports to be the service: clients draw from the ephemeral range above it. Unlike guessing from the direction of the first packet, that holds whether a request or a response is seen first, which matters because a capture starts mid-conversation. A captured handshake still wins, and the guess is only made once per endpoint. Replaying a capture with no handshake, whose first frame is a response to a request that was never captured, went from no output to correctly reporting the following request against 10.0.0.2:6379. Assistant By Opus 5 Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 6 +++ src/packet.c | 72 +++++++++++++++++++++----------- tests/e2e/run.sh | 8 ++++ tests/fixtures/established.pcap | Bin 0 -> 268 bytes tests/fixtures/gen_fixtures.py | 12 ++++++ 5 files changed, 74 insertions(+), 24 deletions(-) create mode 100644 tests/fixtures/established.pcap diff --git a/README.md b/README.md index 562c83e..c54265f 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,12 @@ The output was like below: Use the option `-t` would only show the request which the request latency was slower than threshold(in units of millisecond). +tcpkit works out which side of a connection is the server from the TCP +handshake when it captures one. For connections that were already established +when tcpkit started -- the usual case with pooled clients -- it assumes the +lower of the two ports is the service, since clients draw from the ephemeral +range above it. A captured handshake always takes precedence. + ## How to Use Lua Script diff --git a/src/packet.c b/src/packet.c index 223df48..9bfb6a0 100644 --- a/src/packet.c +++ b/src/packet.c @@ -326,12 +326,40 @@ void print_user_packet(struct sniffer *sniffer, struct user_packet *upacket) { } } -void process_user_packet(struct sniffer *sniffer, struct user_packet *upacket) { - int src; - uint8_t syn_mask = 0x02, ack_mask = 0x10; +/* Records `ip:port` as a server endpoint, which is what the latency and byte + * counters are keyed on. */ +static void register_endpoint(struct sniffer *sniffer, struct in_addr ip, uint16_t port) { char key[32]; struct query_stats *stats; + snprintf(key, sizeof(key), "%u:%d", ip.s_addr, port); + sniffer_stats_lock(sniffer); + if (!hashtable_get(sniffer->syn_tab, key)) { + stats = calloc(1, sizeof(*stats)); + if (stats) { + stats->ip = ip; + stats->port = port; + hashtable_add(sniffer->syn_tab, key, stats); + } + } + sniffer_stats_unlock(sniffer); +} + +/* With no handshake to learn from, the server side has to be guessed. Service + * ports sit below the ephemeral range clients draw from, so the lower port is + * the server -- which holds whichever direction happens to be seen first. */ +static void infer_server_endpoint(struct sniffer *sniffer, struct user_packet *upacket) { + if (upacket->port_dst <= upacket->port_src) { + register_endpoint(sniffer, upacket->ip_dst, upacket->port_dst); + } else { + register_endpoint(sniffer, upacket->ip_src, upacket->port_src); + } +} + +void process_user_packet(struct sniffer *sniffer, struct user_packet *upacket) { + int direction; + uint8_t syn_mask = 0x02, ack_mask = 0x10; + // push to lua state if script exists if (sniffer->lua_state) { push_packet_to_lua_state(sniffer->lua_state, upacket); @@ -343,29 +371,25 @@ void process_user_packet(struct sniffer *sniffer, struct user_packet *upacket) { expire_stale_requests(sniffer, upacket->tv); if (upacket->payload_size == 0) { if ((upacket->flags & syn_mask) != 0) { - src = (upacket->flags & ack_mask) != 0; - if (src) { - snprintf(key, sizeof(key), "%u:%d", upacket->ip_src.s_addr, upacket->port_src); + /* On a SYN the server is the destination, on its SYN+ACK the source. */ + if ((upacket->flags & ack_mask) != 0) { + register_endpoint(sniffer, upacket->ip_src, upacket->port_src); } else { - snprintf(key, sizeof(key), "%u:%d", upacket->ip_dst.s_addr, upacket->port_dst); - } - sniffer_stats_lock(sniffer); - if (!hashtable_get(sniffer->syn_tab, key)) { - stats = calloc(1, sizeof(*stats)); - if (stats) { - stats->ip = src ? upacket->ip_src : upacket->ip_dst; - stats->port = src ? upacket->port_src : upacket->port_dst; - hashtable_add(sniffer->syn_tab, key, stats); - } + register_endpoint(sniffer, upacket->ip_dst, upacket->port_dst); } - sniffer_stats_unlock(sniffer); - } - } else { - switch(packet_direction(sniffer, upacket)) { - case 0: - return process_response_packet(sniffer, upacket); - case 1: - return process_request_packet(sniffer, upacket); } + return; + } + + direction = packet_direction(sniffer, upacket); + if (direction == -1) { + /* The capture started after this connection was established. */ + infer_server_endpoint(sniffer, upacket); + direction = packet_direction(sniffer, upacket); + } + if (direction == 0) { + process_response_packet(sniffer, upacket); + } else if (direction == 1) { + process_request_packet(sniffer, upacket); } } diff --git a/tests/e2e/run.sh b/tests/e2e/run.sh index 3678e5a..24d898f 100755 --- a/tests/e2e/run.sh +++ b/tests/e2e/run.sh @@ -127,6 +127,14 @@ expect_lines "each request stays on one line" "$out" 3 expect_contains "a truncated resp array is summarised" "$out" '*2..$3..GET' expect_contains "a payload with no crlf is summarised" "$out" "GETNOCRLFATALL" +echo "== e2e: established connections" +replay established.pcap -p redis +expect_status "a capture with no handshake exits cleanly" "$status" 0 +expect_lines "the answered request is reported" "$out" 1 +expect_contains "the server side is identified without a syn" "$out" \ + "10.0.0.1:51137 => 10.0.0.2:6379" +expect_contains "the latency is measured" "$out" "1.500 ms" + echo "== e2e: stale requests" replay stale-request.pcap -p redis expect_status "a stale request exits cleanly" "$status" 0 diff --git a/tests/fixtures/established.pcap b/tests/fixtures/established.pcap new file mode 100644 index 0000000000000000000000000000000000000000..54c66ec0c355587c91ee87f7e96fab0d04a1962b GIT binary patch literal 268 zcmca|c+)~A1{MYwxWmf8zzF1kFbLQ}F%yWx2qu|07+e_`^o2|q7#!F_CUY?`Faa^6 z#Ovb+85o2+85o#f21xw>|1O$=LB)`ltBRKktPQB&3yLAeK#T+%W&<^>4P+3=z~cuc zUV{t*8aM%Dn6Cx{gO(95mx?hjm%D2SkO4F~5n?jP$s{{-0?_ooAd}e~A