BPF List
 help / color / mirror / Atom feed
* [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path
@ 2026-07-30 22:00 Tejas Birajdar
  2026-07-31 22:01 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Tejas Birajdar @ 2026-07-30 22:00 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, netdev
  Cc: Jakub Kicinski, Paolo Abeni, David S . Miller, David Ahern, bpf,
	linux-kernel, Tejas Birajdar, Sashiko

BPF_SOCK_OPS_RWND_INIT lets a sockops BPF program pick the initial TCP
receive window, e.g. to advertise a larger window up front in environments
where that is known to be safe. Today it is only effective for the passive
(listener) side; on the active (connect) side the value is computed and
then silently discarded.

On the passive path tcp_openreq_init_rwin() inflates full_space when the
program returns a non-zero window, so tcp_select_initial_window() can offer
it:

	else if (full_space < (u64)rcv_wnd * mss)
		full_space = min_t(u64, (u64)rcv_wnd * mss, INT_MAX);

tcp_select_initial_window() only clamps the requested window *down* to the
available space, so without inflating the space first the BPF reply can
never raise the offered window above tcp_full_space(sk).

tcp_connect_init() calls tcp_rwnd_init_bpf() but never inflates full_space,
so on connect() the requested window is clamped back to tcp_full_space(sk)
(~64KB at the default rcvbuf) and the program's value is ignored.

Inflate full_space in tcp_connect_init() as well; tp->advmss is the mss the
listener path uses (both are tcp_mss_clamp(tp, dst_metric_advmss(dst))).
Read full_space after tcp_rwnd_init_bpf() so a program that also adjusts
SO_RCVBUF is still reflected. Compute the inflated value in u64 and clamp
to INT_MAX to avoid overflow (full_space is int, rcv_wnd is u32), and
apply the same overflow fix to the existing listener-side computation.

tcp_select_initial_window() itself also computes init_rcv_wnd * mss in
32-bit when clamping the offered window down to the requested value. A
large requested window (init_rcv_wnd greater than ~2.9M segments at
1460 mss) wraps this multiply and collapses the offered window to a tiny
value, so compute it in u64 as well.

Fixes: 13d3b1ebe287 ("bpf: Support for setting initial receive window")
Suggested-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Tejas Birajdar <tejasbirajdar@meta.com>
---
v3:
  - Also compute init_rcv_wnd * mss in u64 in tcp_select_initial_window();
    a large requested window otherwise overflows the 32-bit multiply and
    collapses the offered window.
v2: https://lore.kernel.org/netdev/20260723214208.3655474-1-tejasbirajdar@meta.com/
  - Compute the inflated full_space in u64 and clamp to INT_MAX in both the
    connect and listener paths; read full_space after tcp_rwnd_init_bpf().
v1: https://lore.kernel.org/netdev/20260722170033.2763794-1-tejasbirajdar@meta.com/

Verified on the connect path with packetdrill: the offered initial window
now tracks the BPF-requested value, and a large request that previously
overflowed no longer collapses it. No new failures in the in-tree
packetdrill selftests.

 net/ipv4/tcp_minisocks.c | 4 ++--
 net/ipv4/tcp_output.c    | 8 ++++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index ddc4b17a826b..f8c1123aba43 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -453,8 +453,8 @@ void tcp_openreq_init_rwin(struct request_sock *req,
 	rcv_wnd = tcp_rwnd_init_bpf((struct sock *)req);
 	if (rcv_wnd == 0)
 		rcv_wnd = dst_metric(dst, RTAX_INITRWND);
-	else if (full_space < rcv_wnd * mss)
-		full_space = rcv_wnd * mss;
+	else if (full_space < (u64)rcv_wnd * mss)
+		full_space = min_t(u64, (u64)rcv_wnd * mss, INT_MAX);
 
 	/* tcp_full_space because it is guaranteed to be the first packet */
 	tcp_select_initial_window(sk_listener, full_space,
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index d7c1444b5e30..fcaa04e65189 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -251,7 +251,7 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
 		(*rcv_wnd) = space;
 
 	if (init_rcv_wnd)
-		*rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
+		*rcv_wnd = min_t(u64, *rcv_wnd, (u64)init_rcv_wnd * mss);
 
 	*rcv_wscale = 0;
 	if (wscale_ok) {
@@ -4103,6 +4103,7 @@ static void tcp_connect_init(struct sock *sk)
 	const struct dst_entry *dst = __sk_dst_get(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 	__u8 rcv_wscale;
+	int full_space;
 	u16 user_mss;
 	u32 rcv_wnd;
 
@@ -4137,10 +4138,13 @@ static void tcp_connect_init(struct sock *sk)
 		WRITE_ONCE(tp->window_clamp, tcp_full_space(sk));
 
 	rcv_wnd = tcp_rwnd_init_bpf(sk);
+	full_space = tcp_full_space(sk);
 	if (rcv_wnd == 0)
 		rcv_wnd = dst_metric(dst, RTAX_INITRWND);
+	else if (full_space < (u64)rcv_wnd * tp->advmss)
+		full_space = min_t(u64, (u64)rcv_wnd * tp->advmss, INT_MAX);
 
-	tcp_select_initial_window(sk, tcp_full_space(sk),
+	tcp_select_initial_window(sk, full_space,
 				  tp->advmss - (tp->rx_opt.ts_recent_stamp ? tp->tcp_header_len - sizeof(struct tcphdr) : 0),
 				  &tp->rcv_wnd,
 				  &tp->window_clamp,
-- 
2.53.0-Meta


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

* Re: [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path
  2026-07-30 22:00 [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path Tejas Birajdar
@ 2026-07-31 22:01 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-31 22:01 UTC (permalink / raw)
  To: Tejas Birajdar; +Cc: bpf

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

Pre-existing issues:
- [Medium] The `SOCK_RCVBUF_LOCK` check executes before the BPF program runs and before `full_space` is inflated, silently defeating BPF-requested windows and bypassing dynamic `SO_RCVBUF` adjustments.
--

--- Patch [1]: [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path ---
commit 2cf5d1d17ae1c7488533ecaa0fcbb83ae4591ec8
Author: Tejas Birajdar <tejasbirajdar@meta.com>

tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path

This commit updates tcp_connect_init() to honor the initial TCP receive
window requested by a BPF_SOCK_OPS_RWND_INIT program on the active connect
path. It inflates full_space if the program returns a non-zero window,
matching the listener path's behavior, and uses u64 arithmetic to prevent
overflows.

Link: https://lore.kernel.org/netdev/20260723214208.3655474-1-tejasbirajdar@meta.com/
Link: https://lore.kernel.org/netdev/20260722170033.2763794-1-tejasbirajdar@meta.com/

> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index d7c1444b5e304..fcaa04e65189b 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c

[ ... ]

> @@ -4137,10 +4138,13 @@ static void tcp_connect_init(struct sock *sk)
>  		WRITE_ONCE(tp->window_clamp, tcp_full_space(sk));
>  
>  	rcv_wnd = tcp_rwnd_init_bpf(sk);
> +	full_space = tcp_full_space(sk);
>  	if (rcv_wnd == 0)
>  		rcv_wnd = dst_metric(dst, RTAX_INITRWND);
> +	else if (full_space < (u64)rcv_wnd * tp->advmss)
> +		full_space = min_t(u64, (u64)rcv_wnd * tp->advmss, INT_MAX);

[Severity: Medium]
This is a pre-existing issue, but does the SOCK_RCVBUF_LOCK check happening
before the BPF program runs defeat the purpose of allowing BPF to adjust
the receive window?

If an application sets SO_RCVBUF prior to connect(), SOCK_RCVBUF_LOCK is
set. In tcp_connect_init(), this caps tp->window_clamp to the old
tcp_full_space() before tcp_rwnd_init_bpf() is invoked.

When the BPF program returns a large requested window and full_space is
inflated, tcp_select_initial_window() bounds space by
min(window_clamp, space).

This caps it right back to the pre-BPF full_space and discards the BPF
request.

Conversely, if the application did not set SO_RCVBUF but the BPF program
adjusts it dynamically (which sets SOCK_RCVBUF_LOCK), the enforcement
check has already passed. This means window_clamp may remain 0 and bypass
the lock's constraints, resulting in an unconstrained window.

The same behavior appears to occur in the listener path in
tcp_openreq_init_rwin() in net/ipv4/tcp_minisocks.c, where
req->rsk_window_clamp is capped to the pre-BPF full_space before the BPF
program executes.

Should the SOCK_RCVBUF_LOCK checks be moved after tcp_rwnd_init_bpf() so
that dynamically adjusted SO_RCVBUF values are properly accounted for?

> -	tcp_select_initial_window(sk, tcp_full_space(sk),
> +	tcp_select_initial_window(sk, full_space,
>  				  tp->advmss - (tp->rx_opt.ts_recent_stamp ? tp->tcp_header_len - sizeof(struct tcphdr) : 0),
>  				  &tp->rcv_wnd,
>  				  &tp->window_clamp,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730220055.2946171-1-tejasbirajdar@meta.com?part=1

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

end of thread, other threads:[~2026-07-31 22:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 22:00 [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path Tejas Birajdar
2026-07-31 22:01 ` sashiko-bot

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