| Message ID | 20250113135558.3180360-4-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d16b34479064fcb8dbb66a72308b5034be819161 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tcp: add a new PAWS_ACK drop reason | expand |
On Mon, Jan 13, 2025 at 8:56 AM Eric Dumazet <edumazet@google.com> wrote: > > Prior patch in the series added TCP_RFC7323_PAWS_ACK drop reason. > > This patch adds the corresponding SNMP counter, for folks > using nstat instead of tracing for TCP diagnostics. > > nstat -az | grep PAWSOldAck > > Suggested-by: Neal Cardwell <ncardwell@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > include/net/dropreason-core.h | 1 + > include/uapi/linux/snmp.h | 1 + > net/ipv4/proc.c | 1 + > net/ipv4/tcp_input.c | 7 ++++--- > 4 files changed, 7 insertions(+), 3 deletions(-) Looks great to me. Thanks, Eric! Reviewed-by: Neal Cardwell <ncardwell@google.com> neal
On Mon, Jan 13, 2025 at 9:56 PM Eric Dumazet <edumazet@google.com> wrote: > > Prior patch in the series added TCP_RFC7323_PAWS_ACK drop reason. > > This patch adds the corresponding SNMP counter, for folks > using nstat instead of tracing for TCP diagnostics. > > nstat -az | grep PAWSOldAck > > Suggested-by: Neal Cardwell <ncardwell@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Really good suggestion so that we'll not miss this one :) Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
On Mon, Jan 13, 2025 at 9:28 AM Neal Cardwell <ncardwell@google.com> wrote: > > On Mon, Jan 13, 2025 at 8:56 AM Eric Dumazet <edumazet@google.com> wrote: > > > > Prior patch in the series added TCP_RFC7323_PAWS_ACK drop reason. > > > > This patch adds the corresponding SNMP counter, for folks > > using nstat instead of tracing for TCP diagnostics. > > > > nstat -az | grep PAWSOldAck > > > > Suggested-by: Neal Cardwell <ncardwell@google.com> > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > --- > > include/net/dropreason-core.h | 1 + > > include/uapi/linux/snmp.h | 1 + > > net/ipv4/proc.c | 1 + > > net/ipv4/tcp_input.c | 7 ++++--- > > 4 files changed, 7 insertions(+), 3 deletions(-) > > Looks great to me. Thanks, Eric! > > Reviewed-by: Neal Cardwell <ncardwell@google.com> Wrote a little packetdrill test for this, and it passes, as expected... Tested-by: Neal Cardwell <ncardwell@google.com> neal ps: the packetdrill test, which I'm tentatively calling gtests/net/tcp/paws/paws-disordered-ack-old-seq-discard.pkt : --- // Test PAWS processing for reordered pure ACK packets with old sequence // numbers. These are common due to the following common case: // ACKs being generated on different CPUs than data segments, // causing ACKs to be transmitted on different tx queues than data segments // causing reordering of older ACKs behind newer data segments // if the ACKs end up in a longer queue than the data segments. // For these packets, we simply discard them, since this 2024 commit: // tcp: add TCP_RFC7323_PAWS_ACK drop reason // Check outgoing TCP timestamp values. --tcp_ts_tick_usecs=1000 // Set up config. `../common/defaults.sh` // Establish a connection. 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 +0 bind(3, ..., ...) = 0 +0 listen(3, 1) = 0 +0 < S 0:0(0) win 32792 <mss 1012,sackOK,TS val 2000 ecr 0,nop,wscale 7> +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 1000 ecr 100,nop,wscale 8> +.010 < . 1:1(0) ack 1 win 257 <nop, nop, TS val 2010 ecr 1000> +0 accept(3, ..., ...) = 4 +0 %{ TCP_INFINITE_SSTHRESH = 0x7fffffff }% // Send a request. +0 write(4, ..., 1000) = 1000 +0 > P. 1:1001(1000) ack 1 <nop, nop, TS val 1010 ecr 2010> +0 %{ assert tcpi_ca_state == TCP_CA_Open }% +0 %{ assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd }% +0 %{ assert tcpi_snd_ssthresh == TCP_INFINITE_SSTHRESH, tcpi_snd_ssthresh }% // The peer received our request and ACKed it immediately, and then // sent a data segment in reply. However, the ACK is reordered // behind the reply data segment. // First we receive a response packet. +.010 < . 1:1001(1000) ack 1001 win 257 <nop, nop, TS val 2022 ecr 1010> +0 > . 1001:1001(0) ack 1001 <nop, nop, TS val 1020 ecr 2022> // Reset nstat counters, in case we're not in a new network namespce: +0 `nstat -n` // Then we receive a disordered pure ACK packet with old sequence number. +0 < . 1:1(0) ack 1001 win 257 <nop, nop, TS val 2020 ecr 1010> // Because it fails PAWS but is an expected kind of pure ACK with // an old sequence number, we don't send a dupack. // But verify that we increment TcpExtPAWSOldAck. +0 `nstat | grep TcpExtPAWSOldAck | grep -q " 1 "` // Wait a while to verify we don't send a dupack for that disoordered ACK. // Expect another transmit so that packetdrill will sniff that // and see any erroneous dupack instead if we sent that. +.300 write(4, ..., 1000) = 1000 +0 > P. 1001:2001(1000) ack 1001 <nop, nop, TS val 1320 ecr 2021>
From: Eric Dumazet <edumazet@google.com> Date: Mon, 13 Jan 2025 13:55:58 +0000 > Prior patch in the series added TCP_RFC7323_PAWS_ACK drop reason. > > This patch adds the corresponding SNMP counter, for folks > using nstat instead of tracing for TCP diagnostics. > > nstat -az | grep PAWSOldAck > > Suggested-by: Neal Cardwell <ncardwell@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h index 28555109f9bdf883af2567f74dea86a327beba26..ed864934e20b11fcf91478afe2450b2eadce799f 100644 --- a/include/net/dropreason-core.h +++ b/include/net/dropreason-core.h @@ -262,6 +262,7 @@ enum skb_drop_reason { SKB_DROP_REASON_TCP_RFC7323_PAWS, /** * @SKB_DROP_REASON_TCP_RFC7323_PAWS_ACK: PAWS check, old ACK packet. + * Corresponds to LINUX_MIB_PAWS_OLD_ACK. */ SKB_DROP_REASON_TCP_RFC7323_PAWS_ACK, /** @SKB_DROP_REASON_TCP_OLD_SEQUENCE: Old SEQ field (duplicate packet) */ diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h index 2e75674e7d4f8f50f624ca454e6d7d1ed86f5c26..848c7784e684c03bdf743e42594317f3d889d83f 100644 --- a/include/uapi/linux/snmp.h +++ b/include/uapi/linux/snmp.h @@ -186,6 +186,7 @@ enum LINUX_MIB_TIMEWAITKILLED, /* TimeWaitKilled */ LINUX_MIB_PAWSACTIVEREJECTED, /* PAWSActiveRejected */ LINUX_MIB_PAWSESTABREJECTED, /* PAWSEstabRejected */ + LINUX_MIB_PAWS_OLD_ACK, /* PAWSOldAck */ LINUX_MIB_DELAYEDACKS, /* DelayedACKs */ LINUX_MIB_DELAYEDACKLOCKED, /* DelayedACKLocked */ LINUX_MIB_DELAYEDACKLOST, /* DelayedACKLost */ diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c index 40053a02bae103a9e1617ab0457da6398d75cd85..affd21a0f57281947f88c6563be3d99aae613baf 100644 --- a/net/ipv4/proc.c +++ b/net/ipv4/proc.c @@ -189,6 +189,7 @@ static const struct snmp_mib snmp4_net_list[] = { SNMP_MIB_ITEM("TWKilled", LINUX_MIB_TIMEWAITKILLED), SNMP_MIB_ITEM("PAWSActive", LINUX_MIB_PAWSACTIVEREJECTED), SNMP_MIB_ITEM("PAWSEstab", LINUX_MIB_PAWSESTABREJECTED), + SNMP_MIB_ITEM("PAWSOldAck", LINUX_MIB_PAWS_OLD_ACK), SNMP_MIB_ITEM("DelayedACKs", LINUX_MIB_DELAYEDACKS), SNMP_MIB_ITEM("DelayedACKLocked", LINUX_MIB_DELAYEDACKLOCKED), SNMP_MIB_ITEM("DelayedACKLost", LINUX_MIB_DELAYEDACKLOST), diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index dc0e88bcc5352dafee38143076f9e4feebdf8be3..eb82e01da911048b41ca380f913ef55566be79a7 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -5969,12 +5969,13 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb, if (unlikely(th->syn)) goto syn_challenge; - /* Old ACK are common, do not change PAWSESTABREJECTED + /* Old ACK are common, increment PAWS_OLD_ACK * and do not send a dupack. */ - if (reason == SKB_DROP_REASON_TCP_RFC7323_PAWS_ACK) + if (reason == SKB_DROP_REASON_TCP_RFC7323_PAWS_ACK) { + NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWS_OLD_ACK); goto discard; - + } NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED); if (!tcp_oow_rate_limited(sock_net(sk), skb, LINUX_MIB_TCPACKSKIPPEDPAWS,
Prior patch in the series added TCP_RFC7323_PAWS_ACK drop reason. This patch adds the corresponding SNMP counter, for folks using nstat instead of tracing for TCP diagnostics. nstat -az | grep PAWSOldAck Suggested-by: Neal Cardwell <ncardwell@google.com> Signed-off-by: Eric Dumazet <edumazet@google.com> --- include/net/dropreason-core.h | 1 + include/uapi/linux/snmp.h | 1 + net/ipv4/proc.c | 1 + net/ipv4/tcp_input.c | 7 ++++--- 4 files changed, 7 insertions(+), 3 deletions(-)