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

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

[Wesley Atwell]

Hi,

this v2 addresses the review on the earlier quantization series.

Simon was right that the original 3/3 only showed the explicit
rcv_ssthresh-limited ALIGN-up behavior. For v2, 3/3 is 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 only the part with a clear fail-before/pass-after
story today:

  - 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:
- git diff --check
- checkpatch on the touched diff
- local vmksft targeted run of
  net/packetdrill:tcp_rcv_quantization_credit.pkt passes with this
  series applied for ipv4, ipv6, and ipv4-mapped-ipv6
- the same packetdrill fails on HEAD without 2/3 with:

    expected: win 84
      actual: win 85

Changes in v2
=============

- leave 1/3 unchanged
- rename gran to granularity in 2/3
- clarify in 2/3 why representable tp->rcv_wnd state is required across
  later no-shrink transitions
- clarify in 2/3 that the smaller longstanding non-zero case remains
  intentionally unchanged in this series
- replace 3/3 with the proven OOO-memory reproducer for the raw
  free_space case
- drop the IPv4-only restriction in 3/3 after validating the test on
  the default packetdrill protocol set

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 |  8 +++++---
 .../packetdrill/tcp_rcv_quantization_credit.pkt   | 62 ++++++++++++++++++++++
 3 files changed, 78 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH net-next v2 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>
---
 tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt | 8 +++++---
 1 file changed, 5 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 7e6bc5fb0c8d..0b19de9f9307 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,13 @@ 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>
+// This test cares about the OOO SACK state and the resulting tcpi_rcv_mss.
+// Keep the ACK/SACK shape exact, but do not pin the precise advertised
+// receive window here because unrelated rwnd accounting changes can adjust it.
+   +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 v2 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>
---
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 35c3b0ab5a0c..e5c4c09101be 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;
+		u32 granularity = 1U << 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 v2 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 000000000000..bb4f63772326
--- /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 v2 2/3] tcp: keep scaled no-shrink window representable

[Eric Dumazet]

On Mon, Mar 23, 2026 at 11:04 PM Wesley Atwell <atwellwea@gmail.com> wrote:
>
> 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>
> ---
> 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 35c3b0ab5a0c..e5c4c09101be 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;
> +               u32 granularity = 1U << 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)

@free_space is a signed integer, and @granularity is unsigned.
If @free_space is negative, this first condition will always be true,
because the comparison will promote @free_space to a very large
(unsigned) value.

> +                       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

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

[Eric Dumazet]

On Mon, Mar 23, 2026 at 11:04 PM Wesley Atwell <atwellwea@gmail.com> wrote:
>
> 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>
> ---
>  tools/testing/selftests/net/packetdrill/tcp_ooo_rcv_mss.pkt | 8 +++++---
>  1 file changed, 5 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 7e6bc5fb0c8d..0b19de9f9307 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,13 @@ 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>
> +// This test cares about the OOO SACK state and the resulting tcpi_rcv_mss.
> +// Keep the ACK/SACK shape exact, but do not pin the precise advertised
> +// receive window here because unrelated rwnd accounting changes can adjust it.

I do not think we need a comment, the change is captured in the
changelog just fine.

Most packetdrill tests do not have 'win xxx' in their xmit packets, we
do not need
to explain this.

> +   +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