Re: [net-next,v9,2/2] bpf: make tcp_tso_autosize() available to BPF congestion controls

[netdev-bot+sashiko@kernel.org]

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

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

- [Low] New kfunc `tcp_tso_autosize()` is registered in the TCP
  congestion-control kfunc set (net/ipv4/bpf_tcp_ca.c:197) without any…

--

bpf: make tcp_tso_autosize() available to BPF congestion controls

This exposes tcp_tso_autosize() as a BPF kfunc and registers it in the TCP
congestion-control kfunc set.  It also sanitizes min_tso_segs to at least 1
and returns that sanitized minimum when mss_now is 0.

> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index ed4fea98dfde1..9deed2244c2de 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -194,6 +194,7 @@ BTF_ID_FLAGS(func, tcp_reno_cong_avoid)
>  BTF_ID_FLAGS(func, tcp_reno_undo_cwnd)
>  BTF_ID_FLAGS(func, tcp_slow_start)
>  BTF_ID_FLAGS(func, tcp_cong_avoid_ai)
> +BTF_ID_FLAGS(func, tcp_tso_autosize)
>  BTF_KFUNCS_END(bpf_tcp_ca_check_kfunc_ids)

[Severity: Low]
Is there a selftest planned for this new kfunc?  A grep for
tcp_tso_autosize at this commit matches only:

    Documentation/networking/net_cachelines/netns_ipv4_sysctl.rst
    include/net/tcp.h
    net/ipv4/bpf_tcp_ca.c
    net/ipv4/tcp.c
    net/ipv4/tcp_bbr.c
    net/ipv4/tcp_output.c

so nothing under tools/testing/selftests/bpf references it.

The other members of bpf_tcp_ca_check_kfunc_ids do have a consumer, for
example in tools/testing/selftests/bpf/progs/bpf_cubic.c:

    extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
    extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;

and the tso_segs struct_ops program in
tools/testing/selftests/bpf/progs/tcp_ca_kfunc.c still calls
bbr_tso_segs(sk, mss_now) rather than the newly exposed kfunc.

Would adding a struct_ops program that declares

    extern __u32 tcp_tso_autosize(struct sock *sk, __u32 mss_now, int min_tso_segs) __ksym;

and calls it be worthwhile, so that the load-time BTF/argument match for
the promoted kfunc is checked?

> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index b1b493d46b9cf..b7f2ee5e7a6b1 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2252,13 +2252,21 @@ static bool tcp_nagle_check(bool partial, const struct tcp_sock *tp,
>   * in bigger TSO bursts. We we cut the RTT-based allowance in half
>   * for every 2^9 usec (aka 512 us) of RTT, so that the RTT-based allowance
>   * is below 1500 bytes after 6 * ~500 usec = 3ms.
> + *
> + * The min_tso_segs is floored to 1 to avoid surprising conversion. Also,
> + * BPF callers may pass mss_now == 0. In that case the function returns the
> + * sanitized min_tso_segs value and skips autosizing.
>   */
> -u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> -		     int min_tso_segs)
> +__bpf_kfunc u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> +				 int min_tso_segs)
>  {
> +	u32 min_tso = max(min_tso_segs, 1);
>  	unsigned long bytes;
>  	u32 r;
>  
> +	if (unlikely(!mss_now))
> +		return min_tso;
> +

[Severity: Low]
Can either of these two new branches be reached by anything in the tree
today?

tcp_tso_segs() passes sysctl_tcp_min_tso_segs, whose minimum is 1, together
with a non-zero tcp_current_mss() value, and bbr_tso_segs() already filters
out !mss_now before passing 1 or 2:

    net/ipv4/tcp_bbr.c:bbr_tso_segs() {
        ...
        if (mss_now)
            segs = tcp_tso_autosize(sk, mss_now, min_tso_segs);
        ...
    }

That leaves a BPF congestion control as the only caller that can pass
mss_now == 0 or min_tso_segs < 1, and no such program exists in
tools/testing/selftests/bpf.  Would a test that exercises these two paths
be worth adding alongside the kfunc registration?

>  	bytes = READ_ONCE(sk->sk_pacing_rate) >> READ_ONCE(sk->sk_pacing_shift);
>  
>  	r = tcp_min_rtt(tcp_sk(sk)) >> READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_tso_rtt_log);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831165230.2696893-1-chia-yu.chang%40nokia-bell-labs.com