[PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[Wesley Atwell]

Hi,

This v3 addresses the follow-up review on v2.

Eric pointed out that 1/3 does not need the added packetdrill comment
and that 2/3 compared signed free_space against an unsigned
granularity.

This revision drops the extra in-file comment from 1/3 and keeps
the scaled-window granularity in int space in 2/3 so the comparison
stays type-safe. The overall approach and reproducer remain unchanged
from v2.

Simon was right that the original 3/3 only showed the explicit
rcv_ssthresh-limited ALIGN-up behavior. For v2, 3/3 was replaced with
an OOO-memory-based reproducer that first grows rcv_ssthresh with
in-order data and then drives raw backed free_space below
rcv_ssthresh without advancing rcv_nxt. In the instrumented
old-behavior run that shaped this test, the critical ACK reached
free_space=86190, rcv_ssthresh=86286, and still advertised 87040
(85 << 10). With 2/3 applied, the same ACK stays at 84.

That follow-up also clarified why the broader 2/3 change is required.
A narrower variant that preserved the old rcv_ssthresh-limited ALIGN-up
behavior was not sufficient: earlier ACKs still stored 85 in tp->rcv_wnd,
and tcp_select_window() later preserved that extra unit because shrinking
was disallowed. Keeping tp->rcv_wnd representable across the scaled
no-shrink path is what lets later ACKs settle at the correct
wire-visible edge.

Problem
=======

In the scaled no-shrink path, __tcp_select_window() rounds free_space up
to the receive-window scale quantum:

  window = ALIGN(free_space, 1 << tp->rx_opt.rcv_wscale);

When raw backed free_space sits just below the next quantum, that can
expose fresh sender-visible credit that is not actually backed by the
current receive-memory state.

Approach
========

This repost keeps the part with a clear fail-before/pass-after case:

  - relax one unrelated packetdrill test which was pinning an
    incidental advertised window
  - keep tp->rcv_wnd representable in scaled units by rounding larger
    windows down to the scale quantum
  - preserve only the small non-zero case that would otherwise scale
    away to zero; changing that longstanding non-zero-to-zero behavior
    would be a separate change from the bug proven here
  - prove the actual raw-free_space case with a packetdrill sequence
    that reaches free_space < rcv_ssthresh without changing SO_RCVBUF
    after the handshake

Tests
=====

Local validation included:
- git diff --check
- checkpatch on the touched diff
- /home/wes/nipa/local/vmksft dirty --tests
  'net/packetdrill:tcp_ooo_rcv_mss.pkt
   net/packetdrill:tcp_rcv_quantization_credit.pkt'
  passes in run 20260324-202158-4929 for ipv4, ipv6, and
  ipv4-mapped-ipv6
- the same quantization packetdrill fails on HEAD without 2/3 with:

    expected: win 84
      actual: win 85

Changes in v3
=============

- drop the unnecessary explanatory packetdrill comment from 1/3
- keep 2/3 granularity in signed int space to avoid the free_space
  signed/unsigned comparison bug Eric pointed out
- keep 3/3 unchanged

Series layout
=============

  1/3 selftests: packetdrill: stop pinning rwnd in tcp_ooo_rcv_mss
  2/3 tcp: keep scaled no-shrink window representable
  3/3 selftests: packetdrill: cover scaled rwnd quantization slack

Thanks,
Wesley Atwell

---
 net/ipv4/tcp_output.c                              | 16 +++++++++++-----
 .../selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt |  5 ++---
 .../packetdrill/tcp_rcv_quantization_credit.pkt   | 62 ++++++++++++++++++++++
 3 files changed, 75 insertions(+), 8 deletions(-)
 base-commit: 5446b8691eb8278f10deca92048fad84ffd1e4d5

-- 
2.43.0

[PATCH net-next v3 1/3] selftests: packetdrill: stop pinning rwnd in tcp_ooo_rcv_mss

[Wesley Atwell]

tcp_ooo_rcv_mss.pkt cares about the OOO SACK state and the resulting
tcpi_rcv_mss update.

Its exact advertised receive-window value is incidental to that test and
can legitimately move when unrelated rwnd accounting changes adjust the
ACK window.

Drop the hard-coded win 81 checks and keep only the ACK/SACK shape and
the tcpi_rcv_mss assertion.

Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
---
v3:
- drop the explanatory packetdrill comment per review

 tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt b/tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt
index 7e6bc5fb0c8d78f36dc3d18842ff11d938c4e41b..0116abd86093e1920f14d027d34ff5ad30ccaeef 100644
--- a/tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt
+++ b/tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt
@@ -17,11 +17,10 @@ sysctl -q net.ipv4.tcp_rmem="4096 131072 $((32*1024*1024))"`
    +0 accept(3, ..., ...) = 4
 
    +0 < . 2001:11001(9000) ack 1 win 257
-   +0 > . 1:1(0) ack 1 win 81 <nop,nop,sack 2001:11001>
+   +0 > . 1:1(0) ack 1 <nop,nop,sack 2001:11001>
 
 // check that ooo packet properly updates tcpi_rcv_mss
    +0 %{ assert tcpi_rcv_mss == 1000, tcpi_rcv_mss }%
 
    +0 < . 11001:21001(10000) ack 1 win 257
-   +0 > . 1:1(0) ack 1 win 81 <nop,nop,sack 2001:21001>
-
+   +0 > . 1:1(0) ack 1 <nop,nop,sack 2001:21001>
-- 
2.43.0

[PATCH net-next v3 2/3] tcp: keep scaled no-shrink window representable

[Wesley Atwell]

In the scaled no-shrink path, __tcp_select_window() currently rounds the
raw free-space value up to the receive-window scale quantum.

When raw backed free_space sits just below the next quantum, that can
expose fresh sender-visible credit beyond the currently backed receive
space.

Fix this by keeping tp->rcv_wnd representable in scaled units: round
larger windows down to the scale quantum and preserve only the small
non-zero case that would otherwise scale away to zero.

This series intentionally leaves that smaller longstanding non-zero case
unchanged. The proven bug and the new reproducer are both in the
larger-window path where free_space is at least one scale quantum, so
changing 0 < free_space < granularity into zero would be a separate
behavior change.

That representability matters across ACK transitions too, not only on
the immediate raw-free_space-limited ACK. tcp_select_window() preserves
the currently offered window when shrinking is disallowed, so if an
earlier ACK stores a rounded-up value in tp->rcv_wnd, a later
raw-free_space-limited ACK can keep inheriting that extra unit.

Keeping tp->rcv_wnd representable throughout the scaled no-shrink path
prevents that carry-forward and makes later no-shrink decisions reason
from a right edge the peer could actually have seen on the wire.

This removes the larger-window quantization slack while preserving the
small non-zero case needed to avoid scaling away to zero.

Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
---
v3:
- keep granularity in signed int space so the free_space comparison
  stays type-safe

v2:
- rename gran to granularity
- clarify why representable tp->rcv_wnd state is required across later
  no-shrink transitions
- clarify that this series still intentionally leaves the smaller
  longstanding non-zero case unchanged

 net/ipv4/tcp_output.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 35c3b0ab5a0cb714155d5720fe56888f71aecced..5fc0e0d22f10bf56ece1be536b75013768112acf 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3375,13 +3375,19 @@ u32 __tcp_select_window(struct sock *sk)
 	 * scaled window will not line up with the MSS boundary anyway.
 	 */
 	if (tp->rx_opt.rcv_wscale) {
-		window = free_space;
+		int granularity = 1 << tp->rx_opt.rcv_wscale;
 
-		/* Advertise enough space so that it won't get scaled away.
-		 * Import case: prevent zero window announcement if
-		 * 1<<rcv_wscale > mss.
+		/* Keep tp->rcv_wnd representable in scaled units so later
+		 * no-shrink decisions reason about the same right edge we
+		 * can advertise on the wire. Preserve only a small non-zero
+		 * offer that would otherwise get scaled away to zero.
 		 */
-		window = ALIGN(window, (1 << tp->rx_opt.rcv_wscale));
+		if (free_space >= granularity)
+			window = round_down(free_space, granularity);
+		else if (free_space > 0)
+			window = granularity;
+		else
+			window = 0;
 	} else {
 		window = tp->rcv_wnd;
 		/* Get the largest window that is a nice multiple of mss.
-- 
2.43.0

[PATCH net-next v3 3/3] selftests: packetdrill: cover scaled rwnd quantization slack

[Wesley Atwell]

Add a packetdrill reproducer for the free_space-limited scaled
no-shrink quantization case.

Grow rcv_ssthresh with in-order data, then queue tiny OOO skbs so
receive memory drives raw free_space just below rcv_ssthresh without
advancing rcv_nxt. The final ACK reaches the case where raw free_space
sits just above 84 scaled units while rcv_ssthresh stays slightly
larger.

Old code rounds that final free_space value up and advertises 85. With
the fix, the ACK stays at 84.

This gives fail-before/pass-after coverage for the actual raw
free_space bug rather than the separate rcv_ssthresh-limited ALIGN-up
behavior.

Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
---
v2:
- replace the old rcv_ssthresh-limited sequence with an OOO-memory
  reproducer for the raw free_space case
- drop the follow-on ACK transition and keep the final free_space
  failure case that proves the bug
- drop the IPv4-only restriction after verifying the test passes for
  ipv4, ipv6, and ipv4-mapped-ipv6
- reword the in-file comments to describe the observed behavior without
  referring to a removed path as "buggy"

 .../packetdrill/tcp_rcv_quantization_credit.pkt    | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rcv_quantization_credit.pkt

diff --git a/tools/testing/selftests/net/packetdrill/tcp_rcv_quantization_credit.pkt b/tools/testing/selftests/net/packetdrill/tcp_rcv_quantization_credit.pkt
new file mode 100644
index 0000000000000000000000000000000000000000..bb4f63772326fb1146574ad3f43d59082ee49c18
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_rcv_quantization_credit.pkt
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+
+--mss=1000
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_moderate_rcvbuf=0
+sysctl -q net.ipv4.tcp_shrink_window=0
+sysctl -q net.ipv4.tcp_rmem="4096 131072 $((32*1024*1024))"`
+
+// Exercise the scaled no-shrink path when raw backed free_space, not
+// rcv_ssthresh, limits the post-ACK window in __tcp_select_window().
+//
+// Grow rcv_ssthresh with in-order data, then queue tiny OOO skbs so receive
+// memory drops raw free_space to just over 84 scaled units while the current
+// rcv_ssthresh stays slightly larger. The final OOO ACK should keep the
+// sender-visible window at 84 rather than reopening it to 85.
+   +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 1000,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 10>
+   +0 < . 1:1(0) ack 1 win 257
+
+   +0 accept(3, ..., ...) = 4
+
+   +0 < P. 1:10001(10000) ack 1 win 257
+   * > .  1:1(0) ack 10001
+
+   +0 < P. 10001:11024(1023) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 12024:12025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 13024:13025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 14024:14025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 15024:15025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 16024:16025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 17024:17025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 18024:18025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+   +0 < P. 19024:19025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024
+
+// The last tiny OOO skb pushes raw free_space just below rcv_ssthresh
+// without crossing the next lower scaled unit.
+   +0 < P. 20024:20025(1) ack 1 win 257
+   * > .  1:1(0) ack 11024 win 84
+   +0 %{ assert (tcpi_rcv_wnd >> 10) == 84, tcpi_rcv_wnd }%
-- 
2.43.0

Re: [PATCH net-next v3 3/3] selftests: packetdrill: cover scaled rwnd quantization slack

[Simon Baatz]

Hi Wesley,

On Tue, Mar 24, 2026 at 02:53:01PM -0600, Wesley Atwell wrote:
> Add a packetdrill reproducer for the free_space-limited scaled
> no-shrink quantization case.
> 
> Grow rcv_ssthresh with in-order data, then queue tiny OOO skbs so
> receive memory drives raw free_space just below rcv_ssthresh without
> advancing rcv_nxt. The final ACK reaches the case where raw free_space
> sits just above 84 scaled units while rcv_ssthresh stays slightly
> larger.

While this may be the intended behavior, this test case does not
actually demonstrate this:

After the second in-order packet rcv_nxt == rcv_wup == 11024 and
rcv_wnd is 84*1024 (with your patch).  Even if __tcp_select_window()
returns smaller values as more and more OOO packets get queued, the
externally visible advertised window can't go below 84, as the
advertised window can't shrink (as you point out in your cover
letter).
 
> Old code rounds that final free_space value up and advertises 85. With
> the fix, the ACK stays at 84.

The old code is consistent as well, it simply uses 85 throughout:

   ...
   +0 < P. 19024:19025(1) ack 1 win 257
   * > .  1:1(0) ack 11024 win 85

// The last tiny OOO skb pushes raw free_space just below rcv_ssthresh
// without crossing the next lower scaled unit.
   +0 < P. 20024:20025(1) ack 1 win 257
   * > .  1:1(0) ack 11024 win 85

So, like the v1 version, this test demonstrates that the offered
window differs when constrained by rcv_ssthresh (old: 83, 85, 85,
... ; new: 82, 84, 84, ...).  But as this window cannot
shrink, OOO packets will not cause the peer-visible window to change
in this scenario (for either the old or the new code).
 
- Simon
-- 
Simon Baatz <gmbnomis@gmail.com>

Re: [PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[Simon Baatz]

Hi Wesley,

On Tue, Mar 24, 2026 at 02:52:58PM -0600, Wesley Atwell wrote:
> Hi,
> 
> This v3 addresses the follow-up review on v2.
> 
> Eric pointed out that 1/3 does not need the added packetdrill comment
> and that 2/3 compared signed free_space against an unsigned
> granularity.
> 
> This revision drops the extra in-file comment from 1/3 and keeps
> the scaled-window granularity in int space in 2/3 so the comparison
> stays type-safe. The overall approach and reproducer remain unchanged
> from v2.
> 
> Simon was right that the original 3/3 only showed the explicit
> rcv_ssthresh-limited ALIGN-up behavior. For v2, 3/3 was replaced with
> an OOO-memory-based reproducer that first grows rcv_ssthresh with
> in-order data and then drives raw backed free_space below
> rcv_ssthresh without advancing rcv_nxt. In the instrumented
> old-behavior run that shaped this test, the critical ACK reached
> free_space=86190, rcv_ssthresh=86286, and still advertised 87040
> (85 << 10). With 2/3 applied, the same ACK stays at 84.
> 
> That follow-up also clarified why the broader 2/3 change is required.
> A narrower variant that preserved the old rcv_ssthresh-limited ALIGN-up
> behavior was not sufficient: earlier ACKs still stored 85 in tp->rcv_wnd,
> and tcp_select_window() later preserved that extra unit because shrinking
> was disallowed. Keeping tp->rcv_wnd representable across the scaled
> no-shrink path is what lets later ACKs settle at the correct
> wire-visible edge.

So, you are saying that 84 defines the "correct
wire-visible edge"?  That's a strong claim.

The test in 3/3 adds OOO packets until the window calculated from
free_space is 84.  But why stop there?  If I added further OOO
packets until the calculated window drops to 83, I can claim, by the
same reasoning, that 83 is the correct value and the initial 84 is
wrong.

In other words, this is a very synthetic scenario that can be steered
to arbitrary values.  As stated in v1, I would really like to see a
packetdrill (or real-world scenario) where the old behavior actually
hurts (after all, this series claims that the current behavior needs
to be fixed).

 
- Simon
-- 
Simon Baatz <gmbnomis@gmail.com>

Re: [PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[Eric Dumazet]

On Wed, Mar 25, 2026 at 12:58 AM Simon Baatz <gmbnomis@gmail.com> wrote:
>
> Hi Wesley,
>
> On Tue, Mar 24, 2026 at 02:52:58PM -0600, Wesley Atwell wrote:
> > Hi,
> >
> > This v3 addresses the follow-up review on v2.
> >
> > Eric pointed out that 1/3 does not need the added packetdrill comment
> > and that 2/3 compared signed free_space against an unsigned
> > granularity.
> >
> > This revision drops the extra in-file comment from 1/3 and keeps
> > the scaled-window granularity in int space in 2/3 so the comparison
> > stays type-safe. The overall approach and reproducer remain unchanged
> > from v2.
> >
> > Simon was right that the original 3/3 only showed the explicit
> > rcv_ssthresh-limited ALIGN-up behavior. For v2, 3/3 was replaced with
> > an OOO-memory-based reproducer that first grows rcv_ssthresh with
> > in-order data and then drives raw backed free_space below
> > rcv_ssthresh without advancing rcv_nxt. In the instrumented
> > old-behavior run that shaped this test, the critical ACK reached
> > free_space=86190, rcv_ssthresh=86286, and still advertised 87040
> > (85 << 10). With 2/3 applied, the same ACK stays at 84.
> >
> > That follow-up also clarified why the broader 2/3 change is required.
> > A narrower variant that preserved the old rcv_ssthresh-limited ALIGN-up
> > behavior was not sufficient: earlier ACKs still stored 85 in tp->rcv_wnd,
> > and tcp_select_window() later preserved that extra unit because shrinking
> > was disallowed. Keeping tp->rcv_wnd representable across the scaled
> > no-shrink path is what lets later ACKs settle at the correct
> > wire-visible edge.
>
> So, you are saying that 84 defines the "correct
> wire-visible edge"?  That's a strong claim.
>
> The test in 3/3 adds OOO packets until the window calculated from
> free_space is 84.  But why stop there?  If I added further OOO
> packets until the calculated window drops to 83, I can claim, by the
> same reasoning, that 83 is the correct value and the initial 84 is
> wrong.
>
> In other words, this is a very synthetic scenario that can be steered
> to arbitrary values.  As stated in v1, I would really like to see a
> packetdrill (or real-world scenario) where the old behavior actually
> hurts (after all, this series claims that the current behavior needs
> to be fixed).
>

This series seems to be a social engineering experiment.

Could we have the prompts that were fed to an AI agent ?

I really do not see the point. RWIN is by essence advisory, and TCP
cannot accept
arbitrary bloated (small skb->len / skb->truesize ratio) anyway.

Re: [PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[Wesley Atwell]

Hi,

I still think 2/3 is a legitimate fix. To clarify, I was not trying to
claim that 84 is some magic number in the abstract, and I agree the
packetdrill is artificial.

My point was only that, in the constructed case, the old code can
preserve a scaled window that is larger than the currently backed
receive space, while 2/3 keeps the stored window representable in
scaled units.
That said, I am probably missing the reason why that is not a problem
according to the feedback you all have given.

So I am going to drop it here.

To be clear this has nothing to do with social engineering, just was
trying to fix something that doesn't need fixed I suppose.

Thanks,
Wesley Atwell

Re: [PATCH net-next v3 0/3] tcp: fix scaled no-shrink rwnd quantization slack

[Eric Dumazet]

On Wed, Mar 25, 2026 at 10:18 AM Wesley Atwell <atwellwea@gmail.com> wrote:
>
> Hi,
>
> I still think 2/3 is a legitimate fix. To clarify, I was not trying to
> claim that 84 is some magic number in the abstract, and I agree the
> packetdrill is artificial.
>
> My point was only that, in the constructed case, the old code can
> preserve a scaled window that is larger than the currently backed
> receive space, while 2/3 keeps the stored window representable in
> scaled units.
> That said, I am probably missing the reason why that is not a problem
> according to the feedback you all have given.
>
> So I am going to drop it here.
>
> To be clear this has nothing to do with social engineering, just was
> trying to fix something that doesn't need fixed I suppose.

We have limited time and many bugs to review.

Your series really lacks the signal explaining why it's needed and why
it's important.
What real workload suffers from the current behavior.

Thanks.