Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/2] tcp: exclude old ACKs from fast path
@ 2026-09-14  9:04 Inbal Schussheim
  2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Inbal Schussheim @ 2026-09-14  9:04 UTC (permalink / raw)
  To: netdev
  Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
	linux-kselftest, amit.klein, Inbal Schussheim

Exclude ACKs outside [SND.UNA, SND.NXT] from TCP header prediction so
that they fall through to the slow path, where ACK
validation is applied.

Add a packetdrill test for a data segment carrying an
excessively old ACK. The test fails on the unpatched kernel and passes
with the fix.

---

Changes in v3:
- Add a Fixes tag and stable Cc.
- Clarify the visible impact of accepting payload carried by an
  excessively old ACK (RCV.NXT is advanced before the ACK is rejected).
- Document ACK sequence validity as a TCP fast-path condition.
- Initialize the packetdrill test with the suite defaults and disable
  invalid-packet rate limiting.
- Add an SPDX license identifier to the packetdrill test.

Changes in v2:

- Exclude all ACKs before SND.UNA from header prediction instead of
  duplicating old-ACK validation in the fast path.
- Cover pure ACKs as well as data-carrying segments.
- Let the existing slow path handle validation and challenge ACKs.
- Remove the packetdrill reproducer from the commit message; add the
  packetdrill test separately.

v2: https://lore.kernel.org/netdev/20260909075644.1408171-1-inbal.lipshtat@mail.huji.ac.il/
v1: https://lore.kernel.org/netdev/20260906123151.1391349-1-inbal.lipshtat@mail.huji.ac.il/T/#u

Inbal Schussheim (2):
  tcp: exclude old ACKs from tcp fast path
  selftests: net: packetdrill: test exclusion of old ACK from TCP fast
    path

 net/ipv4/tcp_input.c                          |  3 +-
 .../tcp_rfc5961_reject-old-ack.pkt            | 29 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net v3 1/2] tcp: exclude old ACKs from tcp fast path
  2026-09-14  9:04 [PATCH net v3 0/2] tcp: exclude old ACKs from fast path Inbal Schussheim
@ 2026-09-14  9:04 ` Inbal Schussheim
  2026-09-14 10:40   ` Eric Dumazet
  2026-09-16 21:04   ` netdev-bot+sashiko
  2026-09-14  9:04 ` [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP " Inbal Schussheim
  2026-09-17 13:30 ` [PATCH net v3 0/2] tcp: exclude old ACKs from " patchwork-bot+netdevbpf
  2 siblings, 2 replies; 9+ messages in thread
From: Inbal Schussheim @ 2026-09-14  9:04 UTC (permalink / raw)
  To: netdev
  Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
	linux-kselftest, amit.klein, Inbal Schussheim, Tamir Shahar,
	stable

Exclude old ACKs before SND.UNA from the tcp fast path
as well as ACKs after SND.NXT.

Such ACKs will fall through to the slow path, where tcp_ack()
performs the appropriate validation and challenge ACK handling
according to RFC5961 and Commit 3d501dd326fb1c7 ("tcp: do not
accept ACK of bytes we never sent").

This prevents old ACKs from being accepted
or modifying connection state as part of the fast path before
appropriate ACK validation is applied.
In particular, this prevents payload carried by a segment with
an excessively old ACK from advancing RCV.NXT before the ACK
is rejected.

Fixes: 31770e34e43d ("tcp: Revert "tcp: remove header prediction"")
Reported-by: Amit Klein <amit.klein@mail.huji.ac.il>
Reported-by: Tamir Shahar <tamir.shahar1@mail.huji.ac.il>
Reported-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
Suggested-by: Eric Dumazet <edumazet@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
---
 net/ipv4/tcp_input.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index daff93d51342..03d317a58132 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6490,6 +6490,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
  *	  or pure receivers (this means either the sequence number or the ack
  *	  value must stay constant)
  *	- Unexpected TCP option.
+ *	- ACK sequence number is outside [SND.UNA, SND.NXT].
  *
  *	When these conditions are not satisfied it drops into a standard
  *	receive procedure patterned after RFC793 to handle all cases.
@@ -6539,7 +6540,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
 
 	if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
 	    TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
-	    !after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
+	    between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
 		int tcp_header_len = tp->tcp_header_len;
 		s32 delta = 0;
 		int flag = 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
  2026-09-14  9:04 [PATCH net v3 0/2] tcp: exclude old ACKs from fast path Inbal Schussheim
  2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
@ 2026-09-14  9:04 ` Inbal Schussheim
  2026-09-14 10:41   ` Eric Dumazet
  2026-09-16 21:04   ` netdev-bot+sashiko
  2026-09-17 13:30 ` [PATCH net v3 0/2] tcp: exclude old ACKs from " patchwork-bot+netdevbpf
  2 siblings, 2 replies; 9+ messages in thread
From: Inbal Schussheim @ 2026-09-14  9:04 UTC (permalink / raw)
  To: netdev
  Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
	linux-kselftest, amit.klein, Inbal Schussheim

Add a packetdrill test for an in-sequence data segment carrying an
excessively old ACK.

Verify that the segment falls through from the TCP fast path to the slow
path, where the existing ACK validation rejects it and sends a challenge
ACK. The payload is not accepted and RCV.NXT remains unchanged.

Based on the reproducer from Commit 3d501dd326fb
("tcp: do not accept ACK of bytes we never sent").

Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
---
 .../tcp_rfc5961_reject-old-ack.pkt            | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt

diff --git a/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
new file mode 100644
index 000000000000..32dd9de1d366
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_invalid_ratelimit=0
+`
+
+// Test rejection of data segments carrying excessively old ACKs
+
+0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
++0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
++0 bind(3, ..., ...) = 0
++0 listen(3, 1024) = 0
+
+// ---------------- Handshake ------------------- //
++0 < S 0:0(0) win 65535
++0 > S. 0:0(0) ack 1 <...>
++0 < . 1:1(0) ack 1 win 65535
++0 accept(3, ..., ...) = 4
+
+// Populate receive memory so the following segment can use
+// header prediction.
++0 < P. 1:501(500) ack 1 win 65535
++0 > . 1:1(0) ack 501
+
+// Send an in-sequence data segment carrying an excessively old ACK.
++0 < P. 501:1501(1000) ack 2794967397 win 65535
+
+// Challenge ACK; RCV.NXT must remain 501.
++0 > . 1:1(0) ack 501
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 1/2] tcp: exclude old ACKs from tcp fast path
  2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
@ 2026-09-14 10:40   ` Eric Dumazet
  2026-09-16 21:04   ` netdev-bot+sashiko
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-09-14 10:40 UTC (permalink / raw)
  To: Inbal Schussheim
  Cc: netdev, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
	linux-kselftest, amit.klein, Tamir Shahar, stable

On Mon, Sep 14, 2026 at 2:04 AM Inbal Schussheim
<inbal.lipshtat@mail.huji.ac.il> wrote:
>
> Exclude old ACKs before SND.UNA from the tcp fast path
> as well as ACKs after SND.NXT.
>
> Such ACKs will fall through to the slow path, where tcp_ack()
> performs the appropriate validation and challenge ACK handling
> according to RFC5961 and Commit 3d501dd326fb1c7 ("tcp: do not
> accept ACK of bytes we never sent").
>
> This prevents old ACKs from being accepted
> or modifying connection state as part of the fast path before
> appropriate ACK validation is applied.
> In particular, this prevents payload carried by a segment with
> an excessively old ACK from advancing RCV.NXT before the ACK
> is rejected.
>
> Fixes: 31770e34e43d ("tcp: Revert "tcp: remove header prediction"")
> Reported-by: Amit Klein <amit.klein@mail.huji.ac.il>
> Reported-by: Tamir Shahar <tamir.shahar1@mail.huji.ac.il>
> Reported-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>

nit: (no need for a new version)
You are the patch author, the " Reported-by: Inbal Schussheim
<inbal.lipshtat@mail.huji.ac.il>"
is redundant with Signed-off-by from the same person.

Reviewed-by: Eric Dumazet <edumazet@google.com>

> Suggested-by: Eric Dumazet <edumazet@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
> ---
>  net/ipv4/tcp_input.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index daff93d51342..03d317a58132 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6490,6 +6490,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
>   *       or pure receivers (this means either the sequence number or the ack
>   *       value must stay constant)
>   *     - Unexpected TCP option.
> + *     - ACK sequence number is outside [SND.UNA, SND.NXT].
>   *
>   *     When these conditions are not satisfied it drops into a standard
>   *     receive procedure patterned after RFC793 to handle all cases.
> @@ -6539,7 +6540,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>
>         if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
>             TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
> -           !after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
> +           between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
>                 int tcp_header_len = tp->tcp_header_len;
>                 s32 delta = 0;
>                 int flag = 0;
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
  2026-09-14  9:04 ` [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP " Inbal Schussheim
@ 2026-09-14 10:41   ` Eric Dumazet
  2026-09-16 21:04   ` netdev-bot+sashiko
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-09-14 10:41 UTC (permalink / raw)
  To: Inbal Schussheim
  Cc: netdev, ncardwell, kuniyu, davem, kuba, pabeni, horms, shuah,
	linux-kselftest, amit.klein

On Mon, Sep 14, 2026 at 2:04 AM Inbal Schussheim
<inbal.lipshtat@mail.huji.ac.il> wrote:
>
> Add a packetdrill test for an in-sequence data segment carrying an
> excessively old ACK.
>
> Verify that the segment falls through from the TCP fast path to the slow
> path, where the existing ACK validation rejects it and sends a challenge
> ACK. The payload is not accepted and RCV.NXT remains unchanged.
>
> Based on the reproducer from Commit 3d501dd326fb
> ("tcp: do not accept ACK of bytes we never sent").
>
> Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 1/2] tcp: exclude old ACKs from tcp fast path
  2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
  2026-09-14 10:40   ` Eric Dumazet
@ 2026-09-16 21:04   ` netdev-bot+sashiko
  1 sibling, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 21:04 UTC (permalink / raw)
  To: inbal.lipshtat
  Cc: netdev, edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms,
	shuah, linux-kselftest, amit.klein

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Documentation/networking/snmp_counter.rst contains a verbatim
  paste of the tcp_rcv_established() "fast path is disabled when:" list…

--

tcp: exclude old ACKs from tcp fast path

This changes the header prediction test in tcp_rcv_established() so that
segments whose ack_seq is below SND.UNA (in addition to those above
SND.NXT) are sent to the slow path, where tcp_ack() applies RFC5961
validation. A matching bullet is added to the comment listing the
conditions that disable the fast path.

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 0f60a1dbf9274..92bc60716f33d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6490,6 +6490,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
>   *	  or pure receivers (this means either the sequence number or the ack
>   *	  value must stay constant)
>   *	- Unexpected TCP option.
> + *	- ACK sequence number is outside [SND.UNA, SND.NXT].

[Severity: Low]
This isn't a bug, but should Documentation/networking/snmp_counter.rst be
updated together with this comment?

That document reproduces this exact list and says it is a copy of the
kernel comment:

  When kernel receives a TCP packet, it has two paths to handler the
  packet, one is fast path, another is slow path. The comment in kernel
  code provides a good explanation of them, I pasted them below::

    It is split into a fast path and a slow path. The fast path is
    disabled when:
    ...
    - Data is sent in both directions. The fast path only supports pure senders
      or pure receivers (this means either the sequence number or the ack
      value must stay constant)
    - Unexpected TCP option.

The pasted copy still ends at "- Unexpected TCP option." and has no
bullet for the new ACK window condition, so after this patch a reader of
the documentation would conclude that a segment carrying an ACK below
SND.UNA is still eligible for header prediction (and therefore still
counted in TcpExtTCPHPAcks / TcpExtTCPHPHits rather than being handled in
the slow path).

>   *
>   *	When these conditions are not satisfied it drops into a standard
>   *	receive procedure patterned after RFC793 to handle all cases.
> @@ -6539,7 +6540,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>  
>  	if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
>  	    TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
> -	    !after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
> +	    between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
>  		int tcp_header_len = tp->tcp_header_len;
>  		s32 delta = 0;
>  		int flag = 0;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914090408.1435080-1-inbal.lipshtat%40mail.huji.ac.il

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
  2026-09-14  9:04 ` [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP " Inbal Schussheim
  2026-09-14 10:41   ` Eric Dumazet
@ 2026-09-16 21:04   ` netdev-bot+sashiko
  2026-09-17 13:17     ` Paolo Abeni
  1 sibling, 1 reply; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-16 21:04 UTC (permalink / raw)
  To: inbal.lipshtat
  Cc: netdev, edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms,
	shuah, linux-kselftest, amit.klein

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] The new packetdrill test
  tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt…

--

selftests: net: packetdrill: test exclusion of old ACK from TCP fast path

This adds a packetdrill script that primes the receive queue with a 500
byte segment and then sends an in-sequence 1000 byte data segment carrying
an excessively old ACK, expecting a challenge ACK with RCV.NXT unchanged.
It is based on the reproducer from commit 3d501dd326fb ("tcp: do not accept
ACK of bytes we never sent").

No Link: tags in the commit header.

> diff --git a/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
> new file mode 100644
> index 0000000000000..32dd9de1d3662
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +`./defaults.sh
> +sysctl -q net.ipv4.tcp_invalid_ratelimit=0
> +`
> +
> +// Test rejection of data segments carrying excessively old ACKs
> +

[ ... socket setup and handshake snipped ... ]

> +// Populate receive memory so the following segment can use
> +// header prediction.
> +0 < P. 1:501(500) ack 1 win 65535
> +0 > . 1:1(0) ack 501
> +
> +// Send an in-sequence data segment carrying an excessively old ACK.
> +0 < P. 501:1501(1000) ack 2794967397 win 65535
> +
> +// Challenge ACK; RCV.NXT must remain 501.
> +0 > . 1:1(0) ack 501

[Severity: Medium]

Can this script also pass on a kernel that does not have the fast path old
ACK exclusion?  In tcp_rcv_established() there appear to be two independent
routes to the exact output asserted here.

The route the commit message describes is the header prediction gate:

net/ipv4/tcp_input.c:tcp_rcv_established() {
	if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
	    TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
	    between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
	...
}

But if that gate is absent and the fast path is entered, the segment can
still leave it before being queued:

net/ipv4/tcp_input.c:tcp_rcv_established() {
	...
			if ((int)skb->truesize > sk->sk_forward_alloc)
				goto step5;
	...
}

Both routes end up in the same place:

net/ipv4/tcp_input.c:tcp_ack() {
	...
	if (before(ack, prior_snd_una - max_window)) {
		if (!(flag & FLAG_NO_CHALLENGE_ACK))
			tcp_send_challenge_ack(sk, false);
		return -SKB_DROP_REASON_TCP_TOO_OLD_ACK;
	}
	...
}

which emits one bare ACK with ack=501 and leaves rcv_nxt at 501, so the
single expectation "+0 > . 1:1(0) ack 501" is satisfied either way.

Given that, does the regression value of the test rest entirely on the
sk_forward_alloc head-room left by the 500 byte priming segment, which the
script does not assert?  Forward allocation is granted in whole pages:

net/core/sock.c:__sk_mem_schedule() {
	int ret, amt = sk_mem_pages(size);

	sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
	...
}

so the residual head-room after the priming segment is roughly PAGE_SIZE
minus the truesize of that skb.  With the default MAX_SKB_FRAGS of 17 the
500 byte skb lands in kmalloc-1024 (truesize around 1280), leaving enough
room for the 1000 byte segment (truesize around 2304), and an unfixed
kernel would queue the payload and fail the script.

With CONFIG_MAX_SKB_FRAGS=45 (BIG TCP), skb_shared_info grows by 28 * 16
bytes and the priming skb moves up a kmalloc bucket, leaving under 2048
bytes of forward allocation:

include/linux/skbuff.h:
	#ifndef CONFIG_MAX_SKB_FRAGS
	# define CONFIG_MAX_SKB_FRAGS 17
	#endif

	#define MAX_SKB_FRAGS CONFIG_MAX_SKB_FRAGS

In that configuration an unfixed kernel would take the truesize bail-out,
emit the same "ack 501" and report a pass while covering nothing.  Other
PAGE_SIZE, NET_SKB_PAD, kmalloc bucket or debug allocator combinations
look like they can have the same effect.

Would it be worth pinning the path with an nstat bracket, the way the
neighbouring tests do, for example on TcpExtTCPHPHits or
TcpExtTCPChallengeACK?  tcp_rcv_big_endseq.pkt uses:

    0 `nstat -n`
   ...
   +0 `nstat | grep TcpExtBeyondWindow | grep -q " 3 "`

That would make the script fail rather than silently pass if the segment
reaches the fast path.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914090408.1435080-1-inbal.lipshtat%40mail.huji.ac.il

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
  2026-09-16 21:04   ` netdev-bot+sashiko
@ 2026-09-17 13:17     ` Paolo Abeni
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2026-09-17 13:17 UTC (permalink / raw)
  To: netdev-bot+sashiko, inbal.lipshtat
  Cc: netdev, edumazet, ncardwell, kuniyu, davem, kuba, horms, shuah,
	linux-kselftest, amit.klein

On 9/16/26 23:04, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] The new packetdrill test
>    tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt…
> 
> --
> 
> selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
> 
> This adds a packetdrill script that primes the receive queue with a 500
> byte segment and then sends an in-sequence 1000 byte data segment carrying
> an excessively old ACK, expecting a challenge ACK with RCV.NXT unchanged.
> It is based on the reproducer from commit 3d501dd326fb ("tcp: do not accept
> ACK of bytes we never sent").
> 
> No Link: tags in the commit header.
> 
>> diff --git a/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
>> new file mode 100644
>> index 0000000000000..32dd9de1d3662
>> --- /dev/null
>> +++ b/tools/testing/selftests/net/packetdrill/tcp_rfc5961_reject-old-ack.pkt
>> @@ -0,0 +1,29 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +`./defaults.sh
>> +sysctl -q net.ipv4.tcp_invalid_ratelimit=0
>> +`
>> +
>> +// Test rejection of data segments carrying excessively old ACKs
>> +
> 
> [ ... socket setup and handshake snipped ... ]
> 
>> +// Populate receive memory so the following segment can use
>> +// header prediction.
>> +0 < P. 1:501(500) ack 1 win 65535
>> +0 > . 1:1(0) ack 501
>> +
>> +// Send an in-sequence data segment carrying an excessively old ACK.
>> +0 < P. 501:1501(1000) ack 2794967397 win 65535
>> +
>> +// Challenge ACK; RCV.NXT must remain 501.
>> +0 > . 1:1(0) ack 501
> 
> [Severity: Medium]
> 
> Can this script also pass on a kernel that does not have the fast path old
> ACK exclusion?  In tcp_rcv_established() there appear to be two independent
> routes to the exact output asserted here.
> 
> The route the commit message describes is the header prediction gate:
> 
> net/ipv4/tcp_input.c:tcp_rcv_established() {
> 	if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
> 	    TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
> 	    between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
> 	...
> }
> 
> But if that gate is absent and the fast path is entered, the segment can
> still leave it before being queued:
> 
> net/ipv4/tcp_input.c:tcp_rcv_established() {
> 	...
> 			if ((int)skb->truesize > sk->sk_forward_alloc)
> 				goto step5;
> 	...
> }
> 
> Both routes end up in the same place:
> 
> net/ipv4/tcp_input.c:tcp_ack() {
> 	...
> 	if (before(ack, prior_snd_una - max_window)) {
> 		if (!(flag & FLAG_NO_CHALLENGE_ACK))
> 			tcp_send_challenge_ack(sk, false);
> 		return -SKB_DROP_REASON_TCP_TOO_OLD_ACK;
> 	}
> 	...
> }
> 
> which emits one bare ACK with ack=501 and leaves rcv_nxt at 501, so the
> single expectation "+0 > . 1:1(0) ack 501" is satisfied either way.
> 
> Given that, does the regression value of the test rest entirely on the
> sk_forward_alloc head-room left by the 500 byte priming segment, which the
> script does not assert?  Forward allocation is granted in whole pages:
> 
> net/core/sock.c:__sk_mem_schedule() {
> 	int ret, amt = sk_mem_pages(size);
> 
> 	sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
> 	...
> }
> 
> so the residual head-room after the priming segment is roughly PAGE_SIZE
> minus the truesize of that skb.  With the default MAX_SKB_FRAGS of 17 the
> 500 byte skb lands in kmalloc-1024 (truesize around 1280), leaving enough
> room for the 1000 byte segment (truesize around 2304), and an unfixed
> kernel would queue the payload and fail the script.
> 
> With CONFIG_MAX_SKB_FRAGS=45 (BIG TCP), skb_shared_info grows by 28 * 16
> bytes and the priming skb moves up a kmalloc bucket, leaving under 2048
> bytes of forward allocation:
> 
> include/linux/skbuff.h:
> 	#ifndef CONFIG_MAX_SKB_FRAGS
> 	# define CONFIG_MAX_SKB_FRAGS 17
> 	#endif
> 
> 	#define MAX_SKB_FRAGS CONFIG_MAX_SKB_FRAGS
> 
> In that configuration an unfixed kernel would take the truesize bail-out,
> emit the same "ack 501" and report a pass while covering nothing.  Other
> PAGE_SIZE, NET_SKB_PAD, kmalloc bucket or debug allocator combinations
> look like they can have the same effect.
> 
> Would it be worth pinning the path with an nstat bracket, the way the
> neighbouring tests do, for example on TcpExtTCPHPHits or
> TcpExtTCPChallengeACK?  tcp_rcv_big_endseq.pkt uses:
> 
>      0 `nstat -n`
>     ...
>     +0 `nstat | grep TcpExtBeyondWindow | grep -q " 3 "`
> 
> That would make the script fail rather than silently pass if the segment
> reaches the fast path.

IIRC the nipa CI runs with CONFIG_MAX_SKB_FRAGS == 17. The above could be
a possible follow-up, not blocking.

/P


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net v3 0/2] tcp: exclude old ACKs from fast path
  2026-09-14  9:04 [PATCH net v3 0/2] tcp: exclude old ACKs from fast path Inbal Schussheim
  2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
  2026-09-14  9:04 ` [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP " Inbal Schussheim
@ 2026-09-17 13:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-17 13:30 UTC (permalink / raw)
  To: Inbal Schussheim
  Cc: netdev, edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms,
	shuah, linux-kselftest, amit.klein

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 14 Sep 2026 12:04:06 +0300 you wrote:
> Exclude ACKs outside [SND.UNA, SND.NXT] from TCP header prediction so
> that they fall through to the slow path, where ACK
> validation is applied.
> 
> Add a packetdrill test for a data segment carrying an
> excessively old ACK. The test fails on the unpatched kernel and passes
> with the fix.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] tcp: exclude old ACKs from tcp fast path
    https://git.kernel.org/netdev/net/c/f81e6c3fb063
  - [net,v3,2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP fast path
    https://git.kernel.org/netdev/net/c/d841cd7513f3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-17 13:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  9:04 [PATCH net v3 0/2] tcp: exclude old ACKs from fast path Inbal Schussheim
2026-09-14  9:04 ` [PATCH net v3 1/2] tcp: exclude old ACKs from tcp " Inbal Schussheim
2026-09-14 10:40   ` Eric Dumazet
2026-09-16 21:04   ` netdev-bot+sashiko
2026-09-14  9:04 ` [PATCH net v3 2/2] selftests: net: packetdrill: test exclusion of old ACK from TCP " Inbal Schussheim
2026-09-14 10:41   ` Eric Dumazet
2026-09-16 21:04   ` netdev-bot+sashiko
2026-09-17 13:17     ` Paolo Abeni
2026-09-17 13:30 ` [PATCH net v3 0/2] tcp: exclude old ACKs from " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox