BPF List
 help / color / mirror / Atom feed
* Re: [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague
       [not found] <20260614071756.239686-1-chia-yu.chang@nokia-bell-labs.com>
@ 2026-06-15  7:19 ` sashiko-bot
  2026-06-16  1:51 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-15  7:19 UTC (permalink / raw)
  To: chia-yu.chang; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] BPF programs can cause a kernel panic by passing `mss_now == 0` to the `bbr_tso_segs` kfunc, leading to a division by zero.
- [Medium] The `tcp_tso_autosize` function is not registered as a BPF kfunc, preventing BPF congestion control algorithms from reusing default TSO autosizing logic.
--

commit 39a9bf39a3860ba9dd1b2bed8669b97491df87c6
Author: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague

This patch replaces existing min_tso_segs() with tso_segs() CC callbak
for CC algorithm to provides explicit tso segment number of each data
burst and overrides tcp_tso_autosize().

> diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
> --- a/net/ipv4/tcp_bbr.c
> +++ b/net/ipv4/tcp_bbr.c
[ ... ]
> @@ -297,11 +297,19 @@ static void bbr_set_pacing_rate(struct sock *sk, u32 bw, int gain)
>  }
>  
>  /* override sysctl_tcp_min_tso_segs */
> -__bpf_kfunc static u32 bbr_min_tso_segs(struct sock *sk)
> +static u32 bbr_min_tso_segs(struct sock *sk)
>  {
>  	return READ_ONCE(sk->sk_pacing_rate) < (bbr_min_tso_rate >> 3) ? 1 : 2;
>  }
>  
> +__bpf_kfunc static u32 bbr_tso_segs(struct sock *sk, u32 mss_now)
> +{
> +	u32 min_tso;
> +
> +	min_tso = bbr_min_tso_segs(sk);
> +	return tcp_tso_autosize(sk, mss_now, min_tso);
> +}

[Severity: High]
Can a buggy BPF program trigger a divide-by-zero panic here?

Since bbr_tso_segs() is exposed as a BPF kfunc, the verifier does not
restrict the value ranges of scalar arguments like mss_now. If mss_now is 0,
it is passed directly to tcp_tso_autosize(), where it is used as a divisor:

tcp_tso_autosize()
    ...
    return max_t(u32, bytes / mss_now, min_tso_segs);

Does this need an explicit check for mss_now == 0 to prevent a local denial
of service?

> +
>  static u32 bbr_tso_segs_goal(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
[ ... ]
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
[ ... ]
> @@ -2269,6 +2269,7 @@ static u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
>  
>  	return max_t(u32, bytes / mss_now, min_tso_segs);
>  }
> +EXPORT_SYMBOL(tcp_tso_autosize);

[Severity: Medium]
Is there a way for out-of-tree BPF congestion controls to reuse the default
autosizing logic?

With the removal of min_tso_segs, BPF congestion control algorithms that
want a custom minimum TSO size must implement tso_segs and handle the
autosizing themselves. However, while tcp_tso_autosize() is exported to C
modules here, it does not appear to be registered as a BPF kfunc (e.g., via
BTF_KFUNCS_START).

Does this force BPF developers to either lose custom minimum TSO sizing
capabilities or duplicate the tcp_tso_autosize() logic within their BPF
programs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614071756.239686-1-chia-yu.chang@nokia-bell-labs.com?part=1

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

* Re: [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague
       [not found] <20260614071756.239686-1-chia-yu.chang@nokia-bell-labs.com>
  2026-06-15  7:19 ` [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague sashiko-bot
@ 2026-06-16  1:51 ` Jakub Kicinski
  2026-06-16  2:17   ` Jakub Kicinski
  1 sibling, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-06-16  1:51 UTC (permalink / raw)
  To: edumazet, ncardwell
  Cc: chia-yu.chang, jolsa, yonghong.song, song, linux-kselftest,
	memxor, shuah, martin.lau, ast, daniel, andrii, eddyz87, horms,
	dsahern, bpf, netdev, pabeni, jhs, stephen, davem, andrew+netdev,
	donald.hunter, kuniyu, ij, koen.de_schepper, g.white,
	ingemar.s.johansson, mirja.kuehlewind, cheshire, rs.ietf,
	Jason_Livingood, vidhi_goel

On Sun, 14 Jun 2026 09:17:56 +0200 chia-yu.chang@nokia-bell-labs.com
wrote:
> This patch replaces existing min_tso_segs() with tso_segs() CC callbak
> for CC algorithm to provides explicit tso segment number of each data
> burst and overrides tcp_tso_autosize().
> 
> No functional change.

Eric, Neal, looks good?

The min rtt thing in tcp_tso_autosize() helps a bit but if the sender
gets congested for a longer stretch min_rtts on new connections are
high and we're back to sending small TSO, keeping the sender overloaded.
Which is to say - I _hope_ this also solves some of Meta's problems :)

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

* Re: [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague
  2026-06-16  1:51 ` Jakub Kicinski
@ 2026-06-16  2:17   ` Jakub Kicinski
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-06-16  2:17 UTC (permalink / raw)
  To: edumazet, ncardwell
  Cc: chia-yu.chang, jolsa, yonghong.song, song, linux-kselftest,
	memxor, shuah, martin.lau, ast, daniel, andrii, eddyz87, horms,
	dsahern, bpf, netdev, pabeni, jhs, stephen, davem, andrew+netdev,
	donald.hunter, kuniyu, ij, koen.de_schepper, g.white,
	ingemar.s.johansson, mirja.kuehlewind, cheshire, rs.ietf,
	Jason_Livingood, vidhi_goel

On Mon, 15 Jun 2026 18:51:02 -0700 Jakub Kicinski wrote:
> On Sun, 14 Jun 2026 09:17:56 +0200 chia-yu.chang@nokia-bell-labs.com
> wrote:
> > This patch replaces existing min_tso_segs() with tso_segs() CC callbak
> > for CC algorithm to provides explicit tso segment number of each data
> > burst and overrides tcp_tso_autosize().
> > 
> > No functional change.  
> 
> Eric, Neal, looks good?
> 
> The min rtt thing in tcp_tso_autosize() helps a bit but if the sender
> gets congested for a longer stretch min_rtts on new connections are
> high and we're back to sending small TSO, keeping the sender overloaded.
> Which is to say - I _hope_ this also solves some of Meta's problems :)

Ugh, I didn't see the Sashiko report, it's only CCed to the author and
bpf@, not to netdev :/

The zero-check sounds legit. Let's revisit this after the merge window.

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

end of thread, other threads:[~2026-06-16  2:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260614071756.239686-1-chia-yu.chang@nokia-bell-labs.com>
2026-06-15  7:19 ` [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague sashiko-bot
2026-06-16  1:51 ` Jakub Kicinski
2026-06-16  2:17   ` Jakub Kicinski

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