[PATCH net-next v3 0/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[PATCH net-next v3 0/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[Shashank Mohan Jain]

v2 was flagged by the netdev AI reviewer, and the review was right on both
points:

 - The TCP_CLOSE/TCP_LISTEN state check in v2 did not cover every path on
   which SO_RCVBUF is applied before tcp_connect_init() runs. The socket is
   already in TCP_SYN_SENT before tcp_connect_init(), so SO_RCVBUF set in
   that gap - via TCP_FASTOPEN_CONNECT before the first write(), or via
   bpf_setsockopt() in a BPF_SOCK_OPS_TCP_CONNECT_CB program - still wrote
   window_clamp and still overrode the route metric.

 - The v2 changelog was wrong about the "no route metric" case (there both
   the old and the patched kernel end at tcp_full_space(), so nothing
   changes) and overstated the impact.

v3 changes the mechanism: instead of keying off the socket state, it
records whether window_clamp was set by the user via
setsockopt(TCP_WINDOW_CLAMP) and keys tcp_connect_init() and
tcp_openreq_init_rwin() off that, so the route metric is applied on every
pre-connection path, the TCP_SYN_SENT ones included. The changelog now
states the real impact: (1) routes with an RTAX_WINDOW metric and (2) a
TCP_WINDOW_CLAMP overwritten by a later SO_RCVBUF on an unconnected socket
or listener.

Patch 1 is the fix. Patch 2 adds packetdrill tests for the plain
SO_RCVBUF-before-connect case, the TCP_WINDOW_CLAMP-vs-SO_RCVBUF case, and
the TCP_FASTOPEN_CONNECT (TCP_SYN_SENT gap) case; the last fails on both
unpatched net-next and the v2 patch and passes with patch 1.

Changes since v2:
 - fix: distinguish a user TCP_WINDOW_CLAMP from an sk_rcvbuf-derived clamp
   (new tp->window_clamp_user bit) instead of checking the socket state, so
   the route metric survives SO_RCVBUF on the TCP_SYN_SENT paths (TFO,
   sock_ops) too; a genuine TCP_WINDOW_CLAMP is no longer overwritten by a
   later SO_RCVBUF.
 - changelog: drop the inaccurate "no route metric" case and the "any
   application" overstatement; state the impact as RTAX_WINDOW routes and
   an overwritten TCP_WINDOW_CLAMP.
 - selftest: add the TCP_WINDOW_CLAMP-vs-SO_RCVBUF and TCP_FASTOPEN_CONNECT
   cases alongside the original one.

Changes since v1 (single [PATCH net]):
 - retargeted from net to net-next, added the packetdrill selftest, and
   rewrote the changelog (the window scale option is still sent, with a
   shift of 0; since a2cbb1603943 the clamp does not persist past the first
   data segments).

Shashank Mohan Jain (2):
  tcp: keep the route window metric when SO_RCVBUF is set before connect
  selftests/net/packetdrill: cover SO_RCVBUF vs the route window clamp

 include/linux/tcp.h                           |  3 ++-
 net/ipv4/tcp.c                                | 12 +++++++++
 net/ipv4/tcp_minisocks.c                      |  7 +++--
 net/ipv4/tcp_output.c                         |  2 +-
 .../tcp_fastopen_connect_rcvbuf_window.pkt    | 22 +++++++++++++++
 .../tcp_rcvbuf_before_connect_window.pkt      | 27 +++++++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf.pkt            | 24 +++++++++++++++++
 7 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt


base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
-- 
2.43.0

[PATCH net-next v3 1/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[Shashank Mohan Jain]

Since commit b025461303d8 ("tcp: update window_clamp when SO_RCVBUF is
set"), setsockopt(SO_RCVBUF) writes tp->window_clamp in every socket
state through tcp_set_window_clamp().

tcp_connect_init() and tcp_openreq_init_rwin() treat any non-zero
window_clamp as a clamp the user asked for and, in that case, skip the
route window metric (RTAX_WINDOW, "ip route ... window N"). A clamp left
behind by an SO_RCVBUF call therefore has two observable effects:

 (1) on a route that carries an RTAX_WINDOW metric, SO_RCVBUF set before
     connect() (or on a listener) changes the window and window scale
     advertised in the SYN / SYN-ACK, because the route metric is no
     longer applied;

 (2) a window clamp the application set with setsockopt(TCP_WINDOW_CLAMP)
     is overwritten by a later SO_RCVBUF on the still unconnected socket
     or on a listener.

Without an RTAX_WINDOW metric there is no change: both before and after
this patch the advertised window and scale are derived from sk_rcvbuf
(tcp_full_space()), so only (1) and (2) are affected.

Keying the decision off the socket state is not enough, because the
socket is already in TCP_SYN_SENT before tcp_connect_init() runs.
SO_RCVBUF applied in that gap - through TCP_FASTOPEN_CONNECT before the
first write(), or through bpf_setsockopt() in a
BPF_SOCK_OPS_TCP_CONNECT_CB program (as samples/bpf/tcp_clamp_kern.c and
friends do) - still lands before tcp_connect_init() and still overrides
the metric.

Record instead whether window_clamp was set explicitly by the user via
setsockopt(TCP_WINDOW_CLAMP), in a new tp->window_clamp_user bit, and key
tcp_connect_init() and tcp_openreq_init_rwin() off that bit rather than
off a non-zero window_clamp. The route metric is then applied on all
pre-connection paths, regardless of socket state, whenever the clamp was
not set by the user; a passive socket inherits the decision from its
listener. SO_RCVBUF no longer overwrites a genuine TCP_WINDOW_CLAMP
either. When SOCK_RCVBUF_LOCK is set both functions still clamp
window_clamp to tcp_full_space(), and without a user clamp SO_RCVBUF
keeps updating window_clamp, so SO_RCVBUF on an established socket still
grows the advertised window as b025461303d8 intended.

Example: two netns joined by a veth pair, "ip route ... window 4096" on
the route to the peer, SO_RCVBUF set to 1 MiB before connect():

  before:  SYN win 65535, wscale 5
  after:   SYN win 2920,  wscale 0

(the window scale option is still sent, with a shift of 0.)

Fixes: b025461303d8 ("tcp: update window_clamp when SO_RCVBUF is set")
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
 include/linux/tcp.h      |  3 ++-
 net/ipv4/tcp.c           | 12 ++++++++++++
 net/ipv4/tcp_minisocks.c |  7 +++++--
 net/ipv4/tcp_output.c    |  2 +-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index e4d1720f4..2324841e5 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -301,7 +301,8 @@ struct tcp_sock {
  *	0x5?10 << 16 + snd_wnd in net byte order
  */
 	u8	nonagle     : 4,/* Disable Nagle algorithm?             */
-		rate_app_limited:1;  /* rate_{delivered,interval_us} limited? */
+		rate_app_limited:1,  /* rate_{delivered,interval_us} limited? */
+		window_clamp_user:1; /* TCP_WINDOW_CLAMP set by the user */
 	u8	received_ce_pending:4, /* Not yet transmit cnt of received_ce */
 		accecn_opt_sent_w_dsack:1,/* Sent ACCECN opt in previous ACK w/ D-SACK */
 		unused2:3;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f4c9101e5..5f364928b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1858,6 +1858,15 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
 
 void tcp_set_rcvbuf(struct sock *sk, int val)
 {
+	/* A window clamp set explicitly via setsockopt(TCP_WINDOW_CLAMP)
+	 * takes precedence: SO_RCVBUF must not overwrite it.  Without such
+	 * a user clamp, keep updating window_clamp from the buffer size so
+	 * that SO_RCVBUF on an established socket still grows the advertised
+	 * window (commit b025461303d8).
+	 */
+	if (tcp_sk(sk)->window_clamp_user)
+		return;
+
 	tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
 }
 
@@ -3427,6 +3436,7 @@ int tcp_disconnect(struct sock *sk, int flags)
 	tp->is_cwnd_limited = 0;
 	tp->max_packets_out = 0;
 	tp->window_clamp = 0;
+	tp->window_clamp_user = 0;
 	tp->delivered = 0;
 	tp->delivered_ce = 0;
 	tp->accecn_fail_mode = 0;
@@ -4056,6 +4066,8 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 
 	case TCP_WINDOW_CLAMP:
 		err = tcp_set_window_clamp(sk, val);
+		if (!err)
+			tp->window_clamp_user = !!val;
 		break;
 
 	case TCP_QUICKACK:
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 0ddfd5af6..c24dc975a 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -442,8 +442,11 @@ void tcp_openreq_init_rwin(struct request_sock *req,
 
 	mss = tcp_mss_clamp(tp, tcp_dst_advmss(dst));
 	window_clamp = READ_ONCE(tp->window_clamp);
-	/* Set this up on the first call only */
-	req->rsk_window_clamp = window_clamp ? : dst_metric(dst, RTAX_WINDOW);
+	/* Set this up on the first call only.  Honour a window clamp the
+	 * listener set via TCP_WINDOW_CLAMP; otherwise use the route metric.
+	 */
+	req->rsk_window_clamp = tp->window_clamp_user ? window_clamp :
+				dst_metric(dst, RTAX_WINDOW);
 
 	/* limit the window selection if the user enforce a smaller rx buffer */
 	if (sk_listener->sk_userlocks & SOCK_RCVBUF_LOCK &&
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c8865205c..21b4ae303 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4137,7 +4137,7 @@ static void tcp_connect_init(struct sock *sk)
 
 	tcp_ca_dst_init(sk, dst);
 
-	if (!tp->window_clamp)
+	if (!tp->window_clamp_user)
 		WRITE_ONCE(tp->window_clamp, dst_metric(dst, RTAX_WINDOW));
 	tp->advmss = tcp_mss_clamp(tp, tcp_dst_advmss(dst));
 
-- 
2.43.0

Re: [PATCH net-next v3 1/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch changes only one of the three places that choose
  req->rsk_window_clamp, so SYN-cookie connections can end up with the…
- [Medium] The new early return in tcp_set_rcvbuf()
  (net/ipv4/tcp.c:1867-1868) is keyed on a single bit, window_clamp_user.
- [Medium] tcp_openreq_init_rwin() (net/ipv4/tcp_minisocks.c:448) reads
  the new bitfield tp->window_clamp_user with a plain load on a listener…
- [Low] The patch adds a new bitfield `window_clamp_user` to struct
  tcp_sock, in the tcp_sock_write_txrx cacheline group next to nonagle…

--

tcp: keep the route window metric when SO_RCVBUF is set before connect

This patch adds a window_clamp_user bit to struct tcp_sock. The bit records
whether TCP_WINDOW_CLAMP was set explicitly. tcp_connect_init() and
tcp_openreq_init_rwin() now apply the RTAX_WINDOW route metric whenever that
bit is clear, and tcp_set_rcvbuf() stops updating window_clamp once a user
clamp is set.

> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index e4d1720f49f08..2324841e53f27 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -301,7 +301,8 @@ struct tcp_sock {
>   *	0x5?10 << 16 + snd_wnd in net byte order
>   */
>  	u8	nonagle     : 4,/* Disable Nagle algorithm?             */
> -		rate_app_limited:1;  /* rate_{delivered,interval_us} limited? */
> +		rate_app_limited:1,  /* rate_{delivered,interval_us} limited? */
> +		window_clamp_user:1; /* TCP_WINDOW_CLAMP set by the user */

[Severity: Low]
This isn't a bug, but the comment at the top of struct tcp_sock says
"Please update the document when adding new fields." Should
window_clamp_user be added to
Documentation/networking/net_cachelines/tcp_sock.rst? That file already
lists the neighbouring nonagle and rate_app_limited fields.

>  	u8	received_ce_pending:4, /* Not yet transmit cnt of received_ce */
>  		accecn_opt_sent_w_dsack:1,/* Sent ACCECN opt in previous ACK w/ D-SACK */
>  		unused2:3;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index f4c9101e5996a..5f364928b72ba 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1858,6 +1858,15 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
>  
>  void tcp_set_rcvbuf(struct sock *sk, int val)
>  {
> +	/* A window clamp set explicitly via setsockopt(TCP_WINDOW_CLAMP)
> +	 * takes precedence: SO_RCVBUF must not overwrite it.  Without such
> +	 * a user clamp, keep updating window_clamp from the buffer size so
> +	 * that SO_RCVBUF on an established socket still grows the advertised
> +	 * window (commit b025461303d8).
> +	 */
> +	if (tcp_sk(sk)->window_clamp_user)
> +		return;

[Severity: Medium]
Can this leave window_clamp stuck below the value the user asked for? The
bit only records that TCP_WINDOW_CLAMP was set. The kernel can still lower
tp->window_clamp afterwards.

Take TCP_WINDOW_CLAMP of 1 MiB set before connect(), with the default
sk_rcvbuf. At establishment, tcp_init_buffer_space() lowers the clamp:

net/ipv4/tcp_input.c:tcp_init_buffer_space() {
    ...
	maxwin = tcp_full_space(sk);

	if (tp->window_clamp >= maxwin) {
		WRITE_ONCE(tp->window_clamp, maxwin);
    ...
}

Suppose the application then enlarges SO_RCVBUF. __sock_set_rcvbuf()
raises sk_rcvbuf and sets SOCK_RCVBUF_LOCK, but tcp_set_rcvbuf() now
returns early, so window_clamp stays near the initial tcp_full_space().

tcp_rcvbuf_grow() also returns early when SOCK_RCVBUF_LOCK is set, and
tcp_grow_window() limits growth to min(window_clamp, tcp_space()).

Doesn't that mean the advertised window can't grow into the larger buffer,
even though the new size is still under the user's 1 MiB clamp? This only
changes if tcp_measure_rcv_mss() happens to reset window_clamp after a
scaling_ratio change. Before this patch, SO_RCVBUF updated the effective
clamp here, as b025461303d8 intended. Passive children inherit
window_clamp_user from the listener, so they would behave the same way.

Would it work to store the requested clamp value, rather than a single bit,
and let SO_RCVBUF raise window_clamp up to that value?

>  	tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
>  }

[ ... ]

> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 0ddfd5af6e58f..c24dc975a9c43 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -442,8 +442,11 @@ void tcp_openreq_init_rwin(struct request_sock *req,
>  
>  	mss = tcp_mss_clamp(tp, tcp_dst_advmss(dst));
>  	window_clamp = READ_ONCE(tp->window_clamp);
> -	/* Set this up on the first call only */
> -	req->rsk_window_clamp = window_clamp ? : dst_metric(dst, RTAX_WINDOW);
> +	/* Set this up on the first call only.  Honour a window clamp the
> +	 * listener set via TCP_WINDOW_CLAMP; otherwise use the route metric.
> +	 */
> +	req->rsk_window_clamp = tp->window_clamp_user ? window_clamp :
> +				dst_metric(dst, RTAX_WINDOW);

[Severity: High]
Does this leave the SYN cookie paths on the old rule? cookie_v4_check()
and cookie_v6_check() are meant to redo what the SYN-ACK path did. They
still pick the clamp based on a non-zero window_clamp:

net/ipv4/syncookies.c:cookie_v4_check() {
    ...
	/* Try to redo what tcp_v4_send_synack did. */
	req->rsk_window_clamp = READ_ONCE(tp->window_clamp) ? :
				dst_metric(&rt->dst, RTAX_WINDOW);
    ...
	if (!req->syncookie)
		ireq->rcv_wscale = rcv_wscale;
    ...
}

net/ipv6/syncookies.c:cookie_v6_check() {
    ...
	req->rsk_window_clamp = READ_ONCE(tp->window_clamp) ? :dst_metric(dst, RTAX_WINDOW);
    ...
}

tcp_conn_request() also calls tcp_openreq_init_rwin() for want_cookie
requests, so the cookie SYN-ACK now follows the new rule. The cookie does
not encode our rcv_wscale, so it is recomputed when the ACK arrives.

Consider a listener with SO_RCVBUF set but no TCP_WINDOW_CLAMP, a route with
"window 4096", and SYN cookies in use (syncookies=2, or a full accept queue
during a SYN flood):

  SYN-ACK path:  rsk_window_clamp = 4096, wscale 0 advertised
  cookie path:   window_clamp left non-zero by tcp_set_rcvbuf(),
                 rcv_wscale recomputed as about 5

Wouldn't the peer apply a shift of 0 to the child's windows while the child
shifts right by 5? The peer would then see a window about 32 times smaller
for the whole connection. The child's window_clamp would also ignore
RTAX_WINDOW.

Before this patch both paths used the same window_clamp ?: metric
expression, so the advertised and recomputed scales matched.

This also affects the commit message, which says:

  The route metric is then applied on all
  pre-connection paths, regardless of socket state, whenever the clamp was
  not set by the user; a passive socket inherits the decision from its
  listener.

That doesn't hold for SYN cookie connections. Should cookie_v4_check() and
cookie_v6_check() be converted to use window_clamp_user as well?

[Severity: Medium]
Is window_clamp_user read here without the listener lock? window_clamp is
read with READ_ONCE() just above because the listener isn't locked, but
window_clamp_user is a plain load.

On the write side, do_tcp_setsockopt() holds only the listener's socket
lock:

	case TCP_WINDOW_CLAMP:
		err = tcp_set_window_clamp(sk, val);
		if (!err)
			tp->window_clamp_user = !!val;

That is a plain read-modify-write of the u8 that window_clamp_user shares
with nonagle and rate_app_limited. TCP_NODELAY and TCP_CORK also rewrite
that byte on a listener. A bitfield can't be annotated with
READ_ONCE()/WRITE_ONCE(), so won't KCSAN report this race against SYN
processing?

The clamp and the flag are also stored separately, so a concurrent SYN
could see a mix of old and new values:

- If it sees the old SO_RCVBUF-derived window_clamp with the new flag set,
  the request uses a clamp that is neither the route metric nor the
  user's value.

- If it sees the new clamp with the old flag clear, the route metric is
  used, but the child created by sk_clone() later copies
  window_clamp_user=1 from the listener. That child then ignores SO_RCVBUF
  even though no clamp was ever applied to it.

>  
>  	/* limit the window selection if the user enforce a smaller rx buffer */
>  	if (sk_listener->sk_userlocks & SOCK_RCVBUF_LOCK &&

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012132622.48550-1-jain.sm%40gmail.com

[PATCH net-next v3 2/2] selftests/net/packetdrill: cover SO_RCVBUF vs the route window clamp

[Shashank Mohan Jain]

Add packetdrill tests for the interaction between SO_RCVBUF, a user
TCP_WINDOW_CLAMP and the route "window" metric (RTAX_WINDOW) on the SYN:

 - tcp_rcvbuf_before_connect_window.pkt: SO_RCVBUF set before connect()
   must not defeat the route metric; the SYN must advertise the
   route-clamped window and window scale 0.

 - tcp_window_clamp_vs_rcvbuf.pkt: a clamp set with TCP_WINDOW_CLAMP must
   be honoured on the SYN and not be overwritten by a later SO_RCVBUF on
   the still unconnected socket.

 - tcp_fastopen_connect_rcvbuf_window.pkt: with TCP_FASTOPEN_CONNECT the
   SYN is deferred to the first write() while the socket sits in
   TCP_SYN_SENT; SO_RCVBUF applied in that gap must not defeat the route
   metric on the deferred SYN either.

Without the first patch of this series the SYN carried win 65535,
wscale 5 in the first and third cases (the route metric was skipped) and
the SO_RCVBUF-derived window in the second (the user clamp was lost).

The tests are picked up automatically by the packetdrill selftest
Makefile (TEST_PROGS := $(wildcard *.pkt)).

Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
 .../tcp_fastopen_connect_rcvbuf_window.pkt    | 22 +++++++++++++++
 .../tcp_rcvbuf_before_connect_window.pkt      | 27 +++++++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf.pkt            | 24 +++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt

diff --git a/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt b/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
new file mode 100644
index 000000000..7892ac566
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// TCP_FASTOPEN_CONNECT defers the SYN until the first write(); between
+// connect() and that write the socket is in TCP_SYN_SENT. SO_RCVBUF applied
+// in that gap must not defeat the route "window" metric on the deferred SYN.
+// optname 30 = TCP_FASTOPEN_CONNECT, 34 = TCP_FASTOPEN_NO_COOKIE on SOL_TCP.
+--ip_version=ipv4
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_fastopen=5
+ip route replace 192.0.2.1 via 192.168.0.1 dev tun0 window 4096`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_TCP, 30, [1], 4) = 0
+   +0 setsockopt(3, SOL_TCP, 34, [1], 4) = 0
+   +0 connect(3, ..., ...) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 write(3, ..., 1) = 1
+
+// The deferred SYN (carrying 1 byte of data) must advertise the route
+// clamped window and window scale 0 (not win 65535, wscale 5).
+   +0 > S 0:1(1) win 2920 <mss 1460,sackOK,TS val 0 ecr 0,nop,wscale 0>
diff --git a/tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt b/tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt
new file mode 100644
index 000000000..7ec7f14d8
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_rcvbuf_before_connect_window.pkt
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+// Setting SO_RCVBUF before connect() must not defeat the route "window"
+// metric on the SYN.
+//
+// The route to the peer carries "window 4096" (RTAX_WINDOW). With SO_RCVBUF
+// also set before connect(), the SYN the kernel sends must still advertise
+// the small, route-clamped receive window and a window scale of 0 (the
+// window scale option is sent, with a shift of 0).
+//
+// Since commit b025461303d8 ("tcp: update window_clamp when SO_RCVBUF is
+// set"), setsockopt(SO_RCVBUF) wrote tp->window_clamp while the socket was
+// still in TCP_CLOSE. tcp_connect_init() then treated that as a user clamp
+// and skipped RTAX_WINDOW, so the SYN carried win 65535, wscale 5 instead.
+--ip_version=ipv4
+
+`./defaults.sh
+ip route replace 192.0.2.1 via 192.168.0.1 dev tun0 window 4096`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+
+// The SYN must advertise the route-clamped window and window scale 0.
+   +0 > S 0:0(0) win 2920 <mss 1460,sackOK,TS val 0 ecr 0,nop,wscale 0>
+  +.1 < S. 0:0(0) ack 1 win 65535 <mss 1460,nop,wscale 7>
+   +0 > . 1:1(0) ack 1
diff --git a/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
new file mode 100644
index 000000000..3d73178df
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+// A window clamp the user set with setsockopt(TCP_WINDOW_CLAMP) must be
+// honoured on the SYN even when SO_RCVBUF is set afterwards on the still
+// unconnected socket: SO_RCVBUF must not overwrite the user's clamp, and
+// the route "window" metric must not replace it either.
+//
+// TCP_WINDOW_CLAMP is optname 10 on SOL_TCP (this packetdrill build has no
+// symbol for it).
+--ip_version=ipv4
+
+`./defaults.sh
+ip route replace 192.0.2.1 via 192.168.0.1 dev tun0 window 4096`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_TCP, 10, [16384], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+
+// The SYN advertises the user's clamp (win 16060), not the route window
+// (2920) and not the SO_RCVBUF-derived full window.
+   +0 > S 0:0(0) win 16060 <mss 1460,sackOK,TS val 0 ecr 0,nop,wscale 0>
+  +.1 < S. 0:0(0) ack 1 win 65535 <mss 1460,nop,wscale 7>
+   +0 > . 1:1(0) ack 1
-- 
2.43.0

Re: [PATCH net-next v3 2/2] selftests/net/packetdrill: cover SO_RCVBUF vs the route window clamp

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new tests only cover the active-open side.
- [Low] Two of the new scripts use raw socket option numbers instead of
  their names.

--

selftests/net/packetdrill: cover SO_RCVBUF vs the route window clamp

This adds three packetdrill scripts. They check the window and window scale
sent on an active-open SYN when SO_RCVBUF, TCP_WINDOW_CLAMP and the route
RTAX_WINDOW metric interact. One script covers the deferred SYN that
TCP_FASTOPEN_CONNECT sends.

> diff --git a/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt b/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
> new file mode 100644
> index 0000000000000..7892ac566e189
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
> @@ -0,0 +1,22 @@

[ ... ]

> +// optname 30 = TCP_FASTOPEN_CONNECT, 34 = TCP_FASTOPEN_NO_COOKIE on SOL_TCP.

[ ... ]

> +    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +   +0 setsockopt(3, SOL_TCP, 30, [1], 4) = 0
> +   +0 setsockopt(3, SOL_TCP, 34, [1], 4) = 0

[Severity: Low]
This isn't a bug, but could these lines use the symbolic names instead of
the raw optnames 30 and 34?

tcp_syscall_bad_arg_fastopen-invalid-buf-ptr.pkt in the same directory
already does this:

    +0 setsockopt(3, SOL_TCP, TCP_FASTOPEN_CONNECT, [1], 4) = 0

So the packetdrill these selftests run against knows at least that name.

The raw 10 in tcp_window_clamp_vs_rcvbuf.pkt has this comment:

    // TCP_WINDOW_CLAMP is optname 10 on SOL_TCP (this packetdrill build has no
    // symbol for it).

That comment describes a local packetdrill build, not something the
upstream selftest depends on. If the packetdrill used here really has no
TCP_WINDOW_CLAMP symbol, could the comment say that without referring to a
particular build?

The numeric values themselves are correct.

> +   +0 connect(3, ..., ...) = 0

[ ... ]

> diff --git a/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
> new file mode 100644
> index 0000000000000..3d73178df6433
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
> @@ -0,0 +1,24 @@

[ ... ]

> +    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +   +0 setsockopt(3, SOL_TCP, 10, [16384], 4) = 0
> +   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
> +   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> +   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)

[Severity: Low]
All three new scripts call connect() and check only the outgoing SYN. Is
the passive side of the fix tested anywhere?

The fix earlier in this series ("tcp: keep the route window metric when
SO_RCVBUF is set before connect") also changes tcp_openreq_init_rwin() in
net/ipv4/tcp_minisocks.c. That function now picks the clamp based on the
listener's user-clamp bit:

    req->rsk_window_clamp = tp->window_clamp_user ? window_clamp :
                            dst_metric(dst, RTAX_WINDOW);

The fix's commit message says the change applies "on a listener" and to
the "SYN / SYN-ACK". It also says "a passive socket inherits the decision
from its listener".

No script in tools/testing/selftests/net/packetdrill/ sets up a listener
behind a route with a window metric and then checks the SYN-ACK.

Would it make sense to add listener versions of two cases: SO_RCVBUF
alone, and TCP_WINDOW_CLAMP followed by SO_RCVBUF?

The new reset rules also look untested. In do_tcp_setsockopt(),
TCP_WINDOW_CLAMP=0 now clears the bit:

    tp->window_clamp_user = !!val;

tcp_disconnect() clears it as well.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012132622.48550-1-jain.sm%40gmail.com

Re: [PATCH net-next v3 0/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v3 0/2] tcp: keep the route window metric when SO_RCVBUF is set before connect

[shashank Jain]

On Mon, Oct 12, 2026, netdev-bot+sinfo@kernel.org wrote:
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>  - How the issue was discovered ...

Found by manual code inspection assisted by an LLM (Claude Code, Claude
Opus) while reviewing the networking changes that set window_clamp on
unconnected sockets, then confirmed before posting with the packetdrill
tests in patch 2/2 (SO_RCVBUF before connect with a route window metric,
TCP_WINDOW_CLAMP vs a later SO_RCVBUF, and the TCP_FASTOPEN_CONNECT
SYN_SENT case) plus a veth/netns reproducer. The patches carry an
"Assisted-by: LLM" tag for that reason.

Thanks,
Shashank

On Mon, Oct 12, 2026 at 7:00 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.