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

[PATCH net-next v4 0/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"), SO_RCVBUF set before connect() or on a listener leaves a non-zero
window_clamp behind. The SYN, SYN-ACK and SYN cookie paths take that for
a TCP_WINDOW_CLAMP set by the user, so the route "window" metric
(RTAX_WINDOW) is skipped, and a real TCP_WINDOW_CLAMP set before
SO_RCVBUF is overwritten.

Patch 1 records the clamp the user set with TCP_WINDOW_CLAMP in a new
field and keys the SYN, SYN-ACK and SYN cookie paths off it. Patch 2
adds nine packetdrill tests (active open, passive open, SYN cookies over
IPv4 and IPv6, TCP_WINDOW_CLAMP reset, SO_RCVBUF raise on an established
socket). All nine fail on net-next without patch 1 and pass with it.

As Jakub asked, the commit message of patch 1 now says how the problem
was found and how it was tested.

Changes since v3, all from the AI review of v3 (each point was checked
against net-next before acting on it):

 - SYN cookies: cookie_v4_check() and cookie_v6_check() still used
   "window_clamp ?: RTAX_WINDOW". With SO_RCVBUF on the listener and a
   route window metric, v3 sent a SYN-ACK with wscale 0 while the cookie
   child used rcv_wscale 5 (reproduced on the wire with v3, and by the
   new cookie tests). The SYN-ACK path and both cookie paths now use
   one helper, tcp_req_window_clamp().

 - A single bit could leave window_clamp too low: v3's tcp_set_rcvbuf()
   returned early once TCP_WINDOW_CLAMP was set, so after
   tcp_init_buffer_space() lowered window_clamp, a larger SO_RCVBUF
   could not raise it again. v4 stores the requested clamp (u32
   window_clamp_user) and lets SO_RCVBUF move window_clamp up to it.

 - Lockless read of a bitfield: the listener side now reads a separate
   u32 with READ_ONCE(), written with WRITE_ONCE(), instead of a bit in
   a byte shared with nonagle and rate_app_limited.

 - window_clamp_user is listed in net_cachelines/tcp_sock.rst. It sits
   in the cold part of struct tcp_sock; pahole shows the size (2432
   bytes on x86_64, with and without MPTCP) and the fast path groups
   unchanged.

 - Selftests: symbolic TCP_FASTOPEN_CONNECT and TCP_FASTOPEN_NO_COOKIE
   (TCP_WINDOW_CLAMP stays numeric because packetdrill has no symbol for
   it, and the comment no longer refers to a local build); new listener
   tests for SO_RCVBUF alone and for TCP_WINDOW_CLAMP then SO_RCVBUF;
   SYN cookie tests for IPv4 and IPv6 that check tcpi_rcv_wscale of the
   child; a TCP_WINDOW_CLAMP=0 reset test; a test that SO_RCVBUF raises
   window_clamp up to the user's clamp and not above it.

 - Patch 1 changelog: how the bug was found and tested; the cookie paths
   are named.

 - Rebased on current net-next.

Link: https://lore.kernel.org/all/20261012132622.48550-1-jain.sm@gmail.com/ (v3)

Changes since v2:
 - track a user TCP_WINDOW_CLAMP instead of checking the socket state,
   so the TCP_SYN_SENT paths (TCP_FASTOPEN_CONNECT, sock_ops) are fixed
   too; corrected the changelog; two more selftests.

Changes since v1 (single [PATCH net]):
 - retargeted from net to net-next, added the packetdrill selftest, and
   rewrote the changelog.

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

 .../networking/net_cachelines/tcp_sock.rst    |  1 +
 include/linux/tcp.h                           |  1 +
 include/net/tcp.h                             | 14 ++++++++++
 net/ipv4/syncookies.c                         |  3 +--
 net/ipv4/tcp.c                                | 14 +++++++++-
 net/ipv4/tcp_minisocks.c                      |  4 +--
 net/ipv4/tcp_output.c                         |  2 +-
 net/ipv6/syncookies.c                         |  2 +-
 .../tcp_fastopen_connect_rcvbuf_window.pkt    | 21 +++++++++++++++
 .../tcp_rcvbuf_before_connect_window.pkt      | 27 +++++++++++++++++++
 .../tcp_rcvbuf_listener_window.pkt            | 20 ++++++++++++++
 .../tcp_syncookies_rcvbuf_window_ip4.pkt      | 22 +++++++++++++++
 .../tcp_syncookies_rcvbuf_window_ip6.pkt      | 22 +++++++++++++++
 .../tcp_window_clamp_rcvbuf_raise.pkt         | 27 +++++++++++++++++++
 .../tcp_window_clamp_reset_rcvbuf.pkt         | 22 +++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf.pkt            | 23 ++++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf_listener.pkt   | 25 +++++++++++++++++
 17 files changed, 242 insertions(+), 8 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_rcvbuf_listener_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip4.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip6.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_rcvbuf_raise.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_reset_rcvbuf.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf_listener.pkt


base-commit: 279d5c8afcd38bd841832a39bfe1aa4f8518d98e
--
2.43.0

[PATCH net-next v4 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(), tcp_openreq_init_rwin() and the SYN cookie paths
(cookie_v4_check(), cookie_v6_check()) 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 and without TCP_WINDOW_CLAMP nothing
changes: before and after this patch the advertised window and scale
are derived from sk_rcvbuf (tcp_full_space()).

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 - still lands before
tcp_connect_init().

Record instead the clamp the user set with TCP_WINDOW_CLAMP, in a new
tp->window_clamp_user field (0 when there is none; TCP_WINDOW_CLAMP=0
and tcp_disconnect() reset it), and use it as follows:

 - tcp_connect_init() applies the route metric whenever there is no
   user clamp, in any socket state.

 - tcp_openreq_init_rwin(), cookie_v4_check() and cookie_v6_check() all
   pick the request's clamp with the same new helper,
   tcp_req_window_clamp(): the listener's window_clamp if the user set
   one, else the route metric. The SYN-ACK and the SYN cookie path must
   agree, or the child of a cookie would use a different receive window
   scale than the one advertised in the SYN-ACK. The listener is not
   locked on these paths, so the field is written with WRITE_ONCE() and
   read with READ_ONCE().

 - tcp_set_rcvbuf() keeps updating window_clamp from the buffer size,
   but caps it at the user's clamp. SO_RCVBUF on an established socket
   thus still grows window_clamp, which is what the commit above was
   for, also after tcp_init_buffer_space() lowered it below the user's
   clamp, but no longer above a clamp set with TCP_WINDOW_CLAMP.

A value is stored, not a flag, so that SO_RCVBUF can raise window_clamp
back up to the user's clamp. The new u32 sits in the cold part of
struct tcp_sock; on x86_64 the size of struct tcp_sock (2432 bytes, with
and without MPTCP) and the fast path cacheline groups do not change.

Example: two netns joined by a veth pair, "ip route ... window 4096" on
the routes between them, SO_RCVBUF set to 1 MiB:

  before:  SYN win 65535, wscale 5      (SO_RCVBUF before connect())
           SYN-ACK win 65535, wscale 5  (SO_RCVBUF on the listener)
  after:   SYN win 2920, wscale 0
           SYN-ACK win 2896, wscale 0

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

The problem was found by code inspection, assisted by an LLM (Claude
Code, Claude Opus 5.5), while reviewing the commit above. It was
confirmed with a reproducer in a qemu guest (two netns joined by a veth
pair, "ip route ... window 4096", tcpdump of the SYN and SYN-ACK, and
TCP_INFO of both ends, including a SYN cookie case), and with the
packetdrill tests added by the next patch, which fail without this
patch and pass with it.

Fixes: b025461303d8 ("tcp: update window_clamp when SO_RCVBUF is set")
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
v4:
 - also convert cookie_v4_check() and cookie_v6_check(); the SYN-ACK and
   cookie paths now share tcp_req_window_clamp()
 - store the user's clamp (u32 window_clamp_user, READ_ONCE/WRITE_ONCE)
   instead of a bit, and let SO_RCVBUF raise window_clamp up to it
 - add the field to net_cachelines/tcp_sock.rst
 - say in the changelog how the bug was found and tested

 .../networking/net_cachelines/tcp_sock.rst         |  1 +
 include/linux/tcp.h                                |  1 +
 include/net/tcp.h                                  | 14 ++++++++++++++
 net/ipv4/syncookies.c                              |  3 +--
 net/ipv4/tcp.c                                     | 14 +++++++++++++-
 net/ipv4/tcp_minisocks.c                           |  4 +---
 net/ipv4/tcp_output.c                              |  2 +-
 net/ipv6/syncookies.c                              |  2 +-
 8 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/net_cachelines/tcp_sock.rst b/Documentation/networking/net_cachelines/tcp_sock.rst
index 103328dc409..f317c7154d2 100644
--- a/Documentation/networking/net_cachelines/tcp_sock.rst
+++ b/Documentation/networking/net_cachelines/tcp_sock.rst
@@ -152,6 +152,7 @@ u32                           urg_seq
 unsigned_int                  keepalive_time
 unsigned_int                  keepalive_intvl
 int                           linger2
+u32                           window_clamp_user
 u8                            bpf_sock_ops_cb_flags
 u8:1                          bpf_chg_cc_inprogress
 u16                           timeout_rehash
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index e4d1720f49f..4f28fa6c4f4 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -474,6 +474,7 @@ struct tcp_sock {
 	unsigned int		keepalive_intvl;  /* time interval between keep alive probes */
 
 	int			linger2;
+	u32	window_clamp_user; /* TCP_WINDOW_CLAMP set by the user, 0 if none */
 
 
 /* Sock_ops bpf program related variables */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0265c27a5fd..35b64fffd74 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1879,6 +1879,20 @@ extern void tcp_openreq_init_rwin(struct request_sock *req,
 				  const struct sock *sk_listener,
 				  const struct dst_entry *dst);
 
+/* Window clamp for a request socket of listener @tp: the listener's clamp
+ * if the user set one with TCP_WINDOW_CLAMP, else the route window metric.
+ * The SYN-ACK and the SYN cookie paths must make the same choice, or the
+ * child's window scale would not match the one sent in the SYN-ACK.
+ * The listener is not locked here.
+ */
+static inline u32 tcp_req_window_clamp(const struct tcp_sock *tp,
+				       const struct dst_entry *dst)
+{
+	if (READ_ONCE(tp->window_clamp_user))
+		return READ_ONCE(tp->window_clamp);
+	return dst_metric(dst, RTAX_WINDOW);
+}
+
 void tcp_enter_memory_pressure(struct sock *sk);
 void tcp_leave_memory_pressure(struct sock *sk);
 
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 73e12976818..6e78efbdc31 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -472,8 +472,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	}
 
 	/* Try to redo what tcp_v4_send_synack did. */
-	req->rsk_window_clamp = READ_ONCE(tp->window_clamp) ? :
-				dst_metric(&rt->dst, RTAX_WINDOW);
+	req->rsk_window_clamp = tcp_req_window_clamp(tp, &rt->dst);
 	/* limit the window selection if the user enforce a smaller rx buffer */
 	full_space = tcp_full_space(sk);
 	if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 99d7237a623..64465bc257d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1859,7 +1859,15 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
 
 void tcp_set_rcvbuf(struct sock *sk, int val)
 {
-	tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
+	u32 user_clamp = tcp_sk(sk)->window_clamp_user;
+	int win = tcp_win_from_space(sk, val);
+
+	/* window_clamp follows the receive buffer, but never goes above a
+	 * clamp the user set with TCP_WINDOW_CLAMP.
+	 */
+	if (user_clamp)
+		win = min_t(u32, win, user_clamp);
+	tcp_set_window_clamp(sk, win);
 }
 
 #ifdef CONFIG_MMU
@@ -3429,6 +3437,7 @@ int tcp_disconnect(struct sock *sk, int flags)
 	tp->is_cwnd_limited = 0;
 	tp->max_packets_out = 0;
 	tp->window_clamp = 0;
+	WRITE_ONCE(tp->window_clamp_user, 0);
 	tp->delivered = 0;
 	tp->delivered_ce = 0;
 	tp->accecn_fail_mode = 0;
@@ -4058,6 +4067,9 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 
 	case TCP_WINDOW_CLAMP:
 		err = tcp_set_window_clamp(sk, val);
+		if (!err)
+			WRITE_ONCE(tp->window_clamp_user,
+				   val ? tp->window_clamp : 0);
 		break;
 
 	case TCP_QUICKACK:
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 0ddfd5af6e5..c17dc5c4648 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -435,15 +435,13 @@ void tcp_openreq_init_rwin(struct request_sock *req,
 	struct inet_request_sock *ireq = inet_rsk(req);
 	const struct tcp_sock *tp = tcp_sk(sk_listener);
 	int full_space = tcp_full_space(sk_listener);
-	u32 window_clamp;
 	__u8 rcv_wscale;
 	u32 rcv_wnd;
 	int mss;
 
 	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);
+	req->rsk_window_clamp = tcp_req_window_clamp(tp, dst);
 
 	/* 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 c8865205cdc..21b4ae303dd 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));
 
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index b581cb1ee2e..d7d61f8d25e 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -254,7 +254,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 		}
 	}
 
-	req->rsk_window_clamp = READ_ONCE(tp->window_clamp) ? :dst_metric(dst, RTAX_WINDOW);
+	req->rsk_window_clamp = tcp_req_window_clamp(tp, dst);
 	/* limit the window selection if the user enforce a smaller rx buffer */
 	full_space = tcp_full_space(sk);
 	if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
-- 
2.43.0

[PATCH net-next v4 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).

Active open, checked on the SYN:

 - tcp_rcvbuf_before_connect_window.pkt: SO_RCVBUF set before connect()
   must not defeat the route metric.

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

 - tcp_window_clamp_reset_rcvbuf.pkt: TCP_WINDOW_CLAMP=0 removes the
   user's clamp, so a later SO_RCVBUF must again not defeat the route
   metric.

 - 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 either.

Passive open, checked on the SYN-ACK and on the child:

 - tcp_rcvbuf_listener_window.pkt: SO_RCVBUF on the listener must not
   defeat the route metric.

 - tcp_window_clamp_vs_rcvbuf_listener.pkt: TCP_WINDOW_CLAMP and then
   SO_RCVBUF on the listener; the SYN-ACK must use the user's clamp.

 - tcp_syncookies_rcvbuf_window_ip4.pkt and its _ip6 twin: with
   net.ipv4.tcp_syncookies=2, the child created from the cookie must use
   the window scale the SYN-ACK advertised (tcpi_rcv_wscale).

Established socket:

 - tcp_window_clamp_rcvbuf_raise.pkt: after tcp_init_buffer_space()
   lowered window_clamp, a larger SO_RCVBUF must raise it again, up to
   the value set with TCP_WINDOW_CLAMP and not beyond.

All nine tests fail without the previous patch and pass with it. They
are picked up by the packetdrill selftest Makefile
(TEST_PROGS := $(wildcard *.pkt)).

Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
v4:
 - symbolic TCP_FASTOPEN_CONNECT / TCP_FASTOPEN_NO_COOKIE
 - new: listener (SYN-ACK) tests, SYN cookie tests for IPv4 and IPv6,
   TCP_WINDOW_CLAMP=0 reset test, SO_RCVBUF raise test

 .../tcp_fastopen_connect_rcvbuf_window.pkt    | 21 +++++++++++++++
 .../tcp_rcvbuf_before_connect_window.pkt      | 27 +++++++++++++++++++
 .../tcp_rcvbuf_listener_window.pkt            | 20 ++++++++++++++
 .../tcp_syncookies_rcvbuf_window_ip4.pkt      | 22 +++++++++++++++
 .../tcp_syncookies_rcvbuf_window_ip6.pkt      | 22 +++++++++++++++
 .../tcp_window_clamp_rcvbuf_raise.pkt         | 27 +++++++++++++++++++
 .../tcp_window_clamp_reset_rcvbuf.pkt         | 22 +++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf.pkt            | 23 ++++++++++++++++
 .../tcp_window_clamp_vs_rcvbuf_listener.pkt   | 25 +++++++++++++++++
 9 files changed, 209 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_rcvbuf_listener_window.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip4.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip6.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_rcvbuf_raise.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_reset_rcvbuf.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf_listener.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 00000000000..b297294ed16
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_fastopen_connect_rcvbuf_window.pkt
@@ -0,0 +1,21 @@
+// 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.
+--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, TCP_FASTOPEN_CONNECT, [1], 4) = 0
+   +0 setsockopt(3, SOL_TCP, TCP_FASTOPEN_NO_COOKIE, [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 00000000000..7ec7f14d8e8
--- /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_rcvbuf_listener_window.pkt b/tools/testing/selftests/net/packetdrill/tcp_rcvbuf_listener_window.pkt
new file mode 100644
index 00000000000..ed20a75fa9a
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_rcvbuf_listener_window.pkt
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+// Passive side: SO_RCVBUF set on a listener must not defeat the route
+// "window" metric on the SYN-ACK, and the child must use the window scale
+// that the SYN-ACK advertised.
+--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_REUSEADDR, [1], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 win 2896 <mss 1460,sackOK,TS val 100 ecr 100,nop,wscale 0>
+ +.01 < . 1:1(0) ack 1 win 514 <nop,nop,TS val 110 ecr 100>
+   +0 accept(3, ..., ...) = 4
+   +0 %{ assert tcpi_rcv_wscale == 0, tcpi_rcv_wscale }%
diff --git a/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip4.pkt b/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip4.pkt
new file mode 100644
index 00000000000..e26f78704ab
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip4.pkt
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// SYN cookies: the child built by cookie_v4_check() must use the same
+// window clamp rule as the SYN-ACK. With SO_RCVBUF set on the listener and
+// a route "window" metric, the SYN-ACK advertises window scale 0, so the
+// child must use a receive window scale of 0 too.
+--ip_version=ipv4
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_syncookies=2
+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_REUSEADDR, [1], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 10) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 win 2896 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0>
+ +.01 < . 1:1(0) ack 1 win 514 <nop,nop,TS val 110 ecr 4000>
+   +0 accept(3, ..., ...) = 4
+   +0 %{ assert tcpi_rcv_wscale == 0, tcpi_rcv_wscale }%
diff --git a/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip6.pkt b/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip6.pkt
new file mode 100644
index 00000000000..8b5dd71d765
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_syncookies_rcvbuf_window_ip6.pkt
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// SYN cookies: the child built by cookie_v6_check() must use the same
+// window clamp rule as the SYN-ACK. With SO_RCVBUF set on the listener and
+// a route "window" metric, the SYN-ACK advertises window scale 0, so the
+// child must use a receive window scale of 0 too.
+--ip_version=ipv6
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_syncookies=2
+ip -6 route replace fd3d:fa7b:d17d::1 via fd3d:0a0b:17d6:8888::1 dev tun0 window 4096`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 10) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 win 2896 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0>
+ +.01 < . 1:1(0) ack 1 win 514 <nop,nop,TS val 110 ecr 4000>
+   +0 accept(3, ..., ...) = 4
+   +0 %{ assert tcpi_rcv_wscale == 0, tcpi_rcv_wscale }%
diff --git a/tools/testing/selftests/net/packetdrill/tcp_window_clamp_rcvbuf_raise.pkt b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_rcvbuf_raise.pkt
new file mode 100644
index 00000000000..08ef005bf99
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_rcvbuf_raise.pkt
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+// After the handshake, tcp_init_buffer_space() lowers window_clamp to the
+// receive space. A later, larger SO_RCVBUF must raise window_clamp again,
+// but only up to the clamp the user set with TCP_WINDOW_CLAMP.
+//
+// packetdrill has no symbol for TCP_WINDOW_CLAMP, so optname 10 is used.
+--ip_version=ipv4
+
+`./defaults.sh`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_TCP, 10, [40000], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [8192], 4) = 0
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
+   +0 > S 0:0(0) <...>
+  +.1 < S. 0:0(0) ack 1 win 65535 <mss 1460,nop,wscale 7>
+   +0 > . 1:1(0) ack 1
+
+// The small receive buffer keeps window_clamp below the user's clamp:
+// 8192 (tcp_full_space()) minus the one segment (advmss 1460) that
+// tcp_init_buffer_space() reserves.
+   +0 getsockopt(3, SOL_TCP, 10, [6732], [4]) = 0
+
+// A larger SO_RCVBUF raises it again, up to the user's clamp.
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [212992], 4) = 0
+   +0 getsockopt(3, SOL_TCP, 10, [40000], [4]) = 0
diff --git a/tools/testing/selftests/net/packetdrill/tcp_window_clamp_reset_rcvbuf.pkt b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_reset_rcvbuf.pkt
new file mode 100644
index 00000000000..458f2f313c7
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_reset_rcvbuf.pkt
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// TCP_WINDOW_CLAMP set back to 0 on an unconnected socket removes the
+// user's clamp: a later SO_RCVBUF must then not stop the route "window"
+// metric from being applied on the SYN.
+//
+// packetdrill has no symbol for TCP_WINDOW_CLAMP, so optname 10 is used.
+--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_TCP, 10, [0], 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)
+
+// No user clamp is left, so the SYN advertises the route window.
+   +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 00000000000..69a05959e42
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf.pkt
@@ -0,0 +1,23 @@
+// 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 raise window_clamp above the
+// user's clamp, and the route "window" metric must not replace it either.
+//
+// packetdrill has no symbol for TCP_WINDOW_CLAMP, so optname 10 is used.
+--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
diff --git a/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf_listener.pkt b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf_listener.pkt
new file mode 100644
index 00000000000..f7e7ddb5000
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_window_clamp_vs_rcvbuf_listener.pkt
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+// Passive side: a TCP_WINDOW_CLAMP set on a listener must be honoured on
+// the SYN-ACK even when SO_RCVBUF is set afterwards on the listener, and
+// the route "window" metric must not replace it.
+//
+// packetdrill has no symbol for TCP_WINDOW_CLAMP, so optname 10 is used.
+--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_REUSEADDR, [1], 4) = 0
+   +0 setsockopt(3, SOL_TCP, 10, [16384], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1048576], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+// The SYN-ACK advertises the user's clamp, not the route window and not
+// the SO_RCVBUF-derived full window.
+   +0 < S 0:0(0) win 65535 <mss 1460,sackOK,TS val 100 ecr 0,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 win 15928 <mss 1460,sackOK,TS val 100 ecr 100,nop,wscale 0>
+ +.01 < . 1:1(0) ack 1 win 514 <nop,nop,TS val 110 ecr 100>
+   +0 accept(3, ..., ...) = 4
+   +0 %{ assert tcpi_rcv_wscale == 0, tcpi_rcv_wscale }%
-- 
2.43.0

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

[Jakub Kicinski]

On Wed, 21 Oct 2026 21:27:14 +0530 Shashank Mohan Jain wrote:
> The problem was found by code inspection, assisted by an LLM (Claude
> Code, Claude Opus 5.5), while reviewing the commit above. It was
> confirmed with a reproducer in a qemu guest (two netns joined by a veth
> pair, "ip route ... window 4096", tcpdump of the SYN and SYN-ACK, and
> TCP_INFO of both ends, including a SYN cookie case), and with the
> packetdrill tests added by the next patch, which fail without this
> patch and pass with it.

Sounds like a genuine net-next material. Please drop the Fixes tag
completely, and repost after net-next reopens

If someone disagrees please speak up.


## Form letter - net-next-closed

The merge window for Linux v7.4 has begun, and the net-next
tree is now closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens. There is some uncertainty
about how long this merge window will last, so please check
https://netdev.bots.linux.dev/net-next.html
before reposting. net-next _may_ reopen on Nov 2nd.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer

Re: [PATCH net-next v4 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 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · 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).

- [Medium] tcp: unordered lockless reads of window_clamp in
  tcp_req_window_clamp()
- [Low] The changelog says: "Without an RTAX_WINDOW metric and without
  TCP_WINDOW_CLAMP nothing changes".

Pre-existing issues:
- [Medium] The patch adds tp->window_clamp_user and comments that
  window_clamp "never goes above a clamp the user set with…

--

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

This patch adds tp->window_clamp_user to record the clamp set with
TCP_WINDOW_CLAMP. tcp_connect_init(), tcp_openreq_init_rwin() and the SYN
cookie paths now use it, instead of any non-zero window_clamp, to decide
whether to apply the RTAX_WINDOW route metric. tcp_set_rcvbuf() now caps
window_clamp at the user's clamp.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 99d7237a623d8..64465bc257d18 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1859,7 +1859,15 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
>  
>  void tcp_set_rcvbuf(struct sock *sk, int val)
>  {
> -	tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
> +	u32 user_clamp = tcp_sk(sk)->window_clamp_user;
> +	int win = tcp_win_from_space(sk, val);
> +
> +	/* window_clamp follows the receive buffer, but never goes above a
> +	 * clamp the user set with TCP_WINDOW_CLAMP.
> +	 */
> +	if (user_clamp)
> +		win = min_t(u32, win, user_clamp);
> +	tcp_set_window_clamp(sk, win);
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but the new comment says
window_clamp never goes above a clamp set with TCP_WINDOW_CLAMP. Only
tcp_set_rcvbuf() enforces that. The other writers of tp->window_clamp
still ignore window_clamp_user.

tcp_measure_rcv_mss() resets window_clamp from sk_rcvbuf every time
scaling_ratio changes, even with SOCK_RCVBUF_LOCK set:

net/ipv4/tcp_input.c:tcp_measure_rcv_mss() {
    ...
			if (old_ratio != tcp_sk(sk)->scaling_ratio) {
				struct tcp_sock *tp = tcp_sk(sk);

				val = tcp_win_from_space(sk, sk->sk_rcvbuf);
				WRITE_ONCE(tp->window_clamp, val);
    ...
}

tcp_rcvbuf_grow() raises it during DRS autotuning:

net/ipv4/tcp_input.c:tcp_rcvbuf_grow() {
    ...
		WRITE_ONCE(tp->window_clamp,
			   tcp_win_from_space(sk, rcvbuf));
    ...
}

tcp_set_rcvlowat() raises a non-zero clamp in any state, including
TCP_LISTEN:

net/ipv4/tcp.c:tcp_set_rcvlowat() {
    ...
		if (tp->window_clamp && tp->window_clamp < val)
			WRITE_ONCE(tp->window_clamp, val);
    ...
}

mptcp_set_rcvlowat() also writes the subflow window_clamp directly.

Take an established socket that sets TCP_WINDOW_CLAMP=X and then a large
SO_RCVBUF. window_clamp stays capped at X only until the next
scaling_ratio change, when tcp_measure_rcv_mss() sets it from the larger
sk_rcvbuf.

Is the changelog statement that SO_RCVBUF no longer grows window_clamp
"above a clamp set with TCP_WINDOW_CLAMP" accurate beyond that short
window?

Now take a listener that sets TCP_WINDOW_CLAMP=X and then SO_RCVLOWAT=Y,
with Y > X and the rcvbuf not locked. This leaves window_clamp=Y and
window_clamp_user=X. tcp_req_window_clamp() then gives Y to request
sockets and SYN cookies as if it were the user's clamp:

	if (READ_ONCE(tp->window_clamp_user))
		return READ_ONCE(tp->window_clamp);

The same things happened before this patch, so this is not a
regression. Should these writers also be capped at window_clamp_user? If
not, could the new comment and the changelog be reworded to cover only
the tcp_set_rcvbuf() path?

[ ... ]

> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index c8865205cdc8b..21b4ae303dd63 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));

[Severity: Low]
The changelog says:

  Without an RTAX_WINDOW metric and without TCP_WINDOW_CLAMP nothing
  changes: before and after this patch the advertised window and scale
  are derived from sk_rcvbuf (tcp_full_space()).

Does that hold for MPTCP subflows? mptcp_set_rcvlowat() writes the
subflow window_clamp directly and never sets window_clamp_user:

net/mptcp/sockopt.c:mptcp_set_rcvlowat() {
    ...
	mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
		...
		WRITE_ONCE(ssk->sk_rcvbuf, space);
		WRITE_ONCE(tcp_sk(ssk)->window_clamp, val);
    ...
}

__mptcp_socket_create() adds msk->first to conn_list as soon as it is
created. That happens on bind(), on listen(), or on any setsockopt that
goes through __mptcp_nmpc_sk(). So SO_RCVLOWAT can reach a subflow that
is still in TCP_CLOSE, or the listener subflow. The write happens when
the rcvbuf is not locked and mptcp_space_from_win(val) > sk_rcvbuf.

Before this patch, and before b025461303d8, that non-zero value was kept
as the clamp. tcp_connect_init() checked "if (!tp->window_clamp)" and
tcp_openreq_init_rwin() used "window_clamp ? : dst_metric(...)".
tcp_select_initial_window() then derived rcv_wscale from it, and the
final window_clamp was capped at val.

After this patch, window_clamp_user is 0 on these subflows.
tcp_connect_init() overwrites window_clamp with
dst_metric(dst, RTAX_WINDOW), which is usually 0, and
tcp_req_window_clamp() ignores the listener subflow's clamp.

With default sysctls and val=1MB, the wscale in the SYN/SYN-ACK seems to
go from about 5 to about 7. The initial window_clamp is also no longer
limited to val.

The new behaviour matches plain TCP, because tcp_set_rcvlowat() only
raises a window_clamp that is already non-zero. This patch has a Fixes:
tag, and this MPTCP change goes back further than b025461303d8. Could
the changelog mention it?

[ ... ]

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1879,6 +1879,20 @@ extern void tcp_openreq_init_rwin(struct request_sock *req,
>  				  const struct sock *sk_listener,
>  				  const struct dst_entry *dst);
>  
> +/* Window clamp for a request socket of listener @tp: the listener's clamp
> + * if the user set one with TCP_WINDOW_CLAMP, else the route window metric.
> + * The SYN-ACK and the SYN cookie paths must make the same choice, or the
> + * child's window scale would not match the one sent in the SYN-ACK.
> + * The listener is not locked here.
> + */
> +static inline u32 tcp_req_window_clamp(const struct tcp_sock *tp,
> +				       const struct dst_entry *dst)
> +{
> +	if (READ_ONCE(tp->window_clamp_user))
> +		return READ_ONCE(tp->window_clamp);
> +	return dst_metric(dst, RTAX_WINDOW);
> +}
> +
>  void tcp_enter_memory_pressure(struct sock *sk);
>  void tcp_leave_memory_pressure(struct sock *sk);
>  

[Severity: Medium]
tcp_req_window_clamp() runs on the listener without the socket lock,
from tcp_openreq_init_rwin() and from cookie_v4_check() and
cookie_v6_check(). It loads window_clamp_user and then window_clamp.
Nothing orders those two loads against the two stores that
setsockopt(TCP_WINDOW_CLAMP) does on the listener.

In do_tcp_setsockopt(), tcp_set_window_clamp() stores the new
window_clamp first, and window_clamp_user is written afterwards with
WRITE_ONCE(). READ_ONCE() and WRITE_ONCE() prevent tearing. They do not
order the two fields against each other. On a weakly ordered
architecture, a SYN handled at the same time on another CPU can see the
new, non-zero window_clamp_user together with the old window_clamp.

If SO_RCVBUF was never set on the listener, the old window_clamp is 0.
tcp_req_window_clamp() then returns 0. Unless SOCK_RCVBUF_LOCK is set,
tcp_select_initial_window() treats a 0 clamp as no clamp at all, so the
request gets neither the user's clamp nor the RTAX_WINDOW metric. If
SO_RCVBUF was set earlier, the reader can instead pick up the old,
larger window_clamp derived from SO_RCVBUF in place of the user's clamp.
The SYN-ACK and the cookie ACK make this choice at different times, so
they can end up with different rcv_wscale values. Avoiding that mismatch
is the point of the new helper.

tcp_disconnect() has the same problem in reverse. It zeroes
window_clamp with a plain store and only then clears window_clamp_user.
A reader that already passed the TCP_LISTEN check can see the old
window_clamp_user and window_clamp == 0. The plain store was there
before this patch. Should it become WRITE_ONCE() now that
tcp_req_window_clamp() reads the field locklessly?

The window is narrow, because it needs a setsockopt() or a disconnect
on a live listener while SYNs arrive. Still, the changelog presents
READ_ONCE()/WRITE_ONCE() as what makes the unlocked listener path safe.

Pairing smp_store_release() on window_clamp_user with smp_load_acquire()
in tcp_req_window_clamp() would cover the setsockopt case. It would not
cover tcp_disconnect(), where the reader can still see the newer
window_clamp of 0. Would it be simpler to make the result robust to a
stale or zero window_clamp? For example, return window_clamp_user
itself, or the minimum of the two with a fallback to window_clamp_user
when window_clamp reads 0. Or is there a reason the listener's
window_clamp has to be used here rather than the stored user value?

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

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

[shashank Jain]

> [Medium] tcp: unordered lockless reads of window_clamp in tcp_req_window_clamp()

Right, nothing orders the two loads against the two stores in
setsockopt(TCP_WINDOW_CLAMP) or tcp_disconnect(). The case that matters
is the one where window_clamp reads 0: the request then gets no clamp at
all. Any other stale value is a clamp the listener had just before, the
same as a SYN that arrives just before the setsockopt(). And a
setsockopt() between the SYN-ACK and the cookie ACK changes the clamp
whatever the ordering.

So in v5 tcp_req_window_clamp() falls back to the user's clamp when
window_clamp reads 0, without barriers:

u32 user_clamp = READ_ONCE(tp->window_clamp_user);

if (!user_clamp)
return dst_metric(dst, RTAX_WINDOW);
return READ_ONCE(tp->window_clamp) ? : user_clamp;

I kept window_clamp rather than the stored user value, because a listener's
window_clamp can legitimately differ from it: SO_RCVBUF lowers it, and
SO_RCVLOWAT raises it so that the window can hold rcvlowat bytes. Capping
that at the user's clamp could leave a reader waiting for data the window
does not let in. The plain store in tcp_disconnect() becomes WRITE_ONCE(),
and the changelog no longer says that READ_ONCE()/WRITE_ONCE() make this
path safe.

> [Low] The changelog says: "Without an RTAX_WINDOW metric and without
> TCP_WINDOW_CLAMP nothing changes".

Correct for MPTCP subflows. mptcp_set_rcvlowat() writes the subflow's
window_clamp without setting window_clamp_user, so tcp_connect_init() now
replaces it with the route metric, as for plain TCP, where
tcp_set_rcvlowat() only raises a clamp that is already non-zero. v5 says
this in the changelog and Cc's the MPTCP maintainers.

> Pre-existing issues:
> - [Medium] The patch adds tp->window_clamp_user and comments that
>   window_clamp "never goes above a clamp the user set with...

Agreed, the comment and the changelog claim more than the patch does.
Only tcp_set_rcvbuf() is capped. tcp_measure_rcv_mss(), tcp_rcvbuf_grow()
and SO_RCVLOWAT still set window_clamp without looking at the user's
clamp, as before this patch. v5 rewords both to cover only the SO_RCVBUF
path. Whether autotuning should respect TCP_WINDOW_CLAMP is a separate
question that this patch doesn't change. I can look at it as a follow-up
once window_clamp_user is in.

v5 will go out when net-next reopens, without the Fixes: tag as Jakub
asked.

pw-bot: cr

On Thu, Oct 22, 2026 at 2:21 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 21 Oct 2026 21:27:14 +0530 Shashank Mohan Jain wrote:
> > The problem was found by code inspection, assisted by an LLM (Claude
> > Code, Claude Opus 5.5), while reviewing the commit above. It was
> > confirmed with a reproducer in a qemu guest (two netns joined by a veth
> > pair, "ip route ... window 4096", tcpdump of the SYN and SYN-ACK, and
> > TCP_INFO of both ends, including a SYN cookie case), and with the
> > packetdrill tests added by the next patch, which fail without this
> > patch and pass with it.
>
> Sounds like a genuine net-next material. Please drop the Fixes tag
> completely, and repost after net-next reopens
>
> If someone disagrees please speak up.
>
>
> ## Form letter - net-next-closed
>
> The merge window for Linux v7.4 has begun, and the net-next
> tree is now closed for new drivers, features, code refactoring
> and optimizations. We are currently accepting bug fixes only.
>
> Please repost when net-next reopens. There is some uncertainty
> about how long this merge window will last, so please check
> https://netdev.bots.linux.dev/net-next.html
> before reposting. net-next _may_ reopen on Nov 2nd.
>
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
> --
> pw-bot: defer