mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: Tejas Birajdar <tejasbirajdar@meta.com>,
	Eric Dumazet <edumazet@google.com>,
	Neal Cardwell <ncardwell@google.com>,
	netdev@vger.kernel.org
Cc: Jakub Kicinski <kuba@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path
Date: Thu, 30 Jul 2026 10:45:23 +0200	[thread overview]
Message-ID: <049f7b53-8bbe-4ee8-a347-6c07ee11c1ac@redhat.com> (raw)
In-Reply-To: <20260723214208.3655474-1-tejasbirajdar@meta.com>

On 7/23/26 11:42 PM, Tejas Birajdar wrote:
> 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.
> 
> Fixes: 13d3b1ebe287 ("bpf: Support for setting initial receive window")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Tejas Birajdar <tejasbirajdar@meta.com>
> ---
> v2:
>   - Do the inflated full_space arithmetic in u64 and clamp to INT_MAX to
>     avoid overflow, and apply the same fix to the existing listener path
>     in tcp_openreq_init_rwin().
>   - Read full_space after tcp_rwnd_init_bpf() so a program that also raises
>     SO_RCVBUF via bpf_setsockopt() is reflected in the offered window.
>   - Re-ran the sockops BPF/RWND functional test and the full in-tree
>     packetdrill regression suite on the revised code (details below).
> v1: https://lore.kernel.org/netdev/20260722170033.2763794-1-tejasbirajdar@meta.com/
> 
> Functional test: a cgroup sockops BPF program returning
> skops->reply = N for BPF_SOCK_OPS_RWND_INIT was attached to the
> connecting socket and driven with packetdrill on the active-open
> (connect) path (advmss 1460, negotiated wscale 8). The offered window on
> the first post-handshake ACK now tracks the requested value:
> 
>   req_segs   offered window (bytes)   = req_segs * advmss
>   256        373760
>   1024       1495040  (~1.43 MB)
>   4096       5980160  (~5.7 MB)

The still present, sashiko reported, integer overflow in
tcp_select_initial_window() looks real to me, and the last value in the
above table is greater than the overflow threshold, so I'm wondering
what I'm missing here?

I *think* you should additionally take care of tcp_select_initial_window().

/P


      reply	other threads:[~2026-07-30  8:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23 21:42 Tejas Birajdar
2026-07-30  8:45 ` Paolo Abeni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=049f7b53-8bbe-4ee8-a347-6c07ee11c1ac@redhat.com \
    --to=pabeni@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=tejasbirajdar@meta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®