Re: [PATCH net-next v3] tcp: honor BPF_SOCK_OPS_RWND_INIT on the active connect path

On 8/1/26 12:01 AM, [email protected] wrote:
> 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 <[email protected]>
> 
> 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/[email protected]/
> Link: https://lore.kernel.org/netdev/[email protected]/
> 
>> 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), 

I read the above alike: 'a badly written bpf program could be
ineffective'; not much relevant.

This version LGTM; Eric do you have some comments?

Thanks,

Paolo