Netdev List
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jay Vosburgh <jv@jvosburgh.net>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hangbin Liu <liuhangbin@kylinos.cn>
Subject: Re: [PATCH net] bonding: fix u32 overflow in compute_gap()
Date: Thu, 13 Aug 2026 10:28:47 +0800	[thread overview]
Message-ID: <an0r33hYV6sJlfJX@fedora> (raw)
In-Reply-To: <20260812180111.76d0cece@kernel.org>

On Wed, Aug 12, 2026 at 06:01:11PM -0700, Jakub Kicinski wrote:
> On Mon, 10 Aug 2026 10:38:23 +0800 Hangbin Liu wrote:
> > From: Hangbin Liu <liuhangbin@kylinos.cn>
> > 
> > compute_gap() computes the gap between a slave's link capacity and its
> > current TLB load. Both terms use u32 left-shifts that overflow on modern
> > hardware:
> > 
> >   - slave->speed is u32 in Mbps; speed << 20 overflows at > 4Gbps.
> >   - SLAVE_TLB_INFO(slave).load is u32; load << 3 overflows at > 512M.
> > 
> > Cast both operands to s64 before shifting so the arithmetic is performed
> > in 64 bits. Also update the comment to make it more clear.
> 
> Could you clarify the impact in the commit message a little more
> explicitly? If the links are the same speed -- does this fix still
> matter?

No, with same high speed NICs. If one save has lower load, e.g. 3Gbit/s,
Another has higher load, e.g. 5Gbit/s. The higher load will overflow and
became a smaller value.

> 
> AI over here says:
> 
>   The fix is incomplete for very high loads: each hash bucket’s u32 tx_bytes
>   already wraps above roughly 3.44 Gbit/s sustained over the 10-second interval,
>   and aggregate u32 load wraps above roughly 34.4 Gbit/s.
> 
> While these are not exactly the same lines of code - I think it'd be
> worth to address them all in one series. 

Ah, yes. The load is already overflow here. Not in the gap compute.
So the gap compute fix is mainly for different speed NICs.

> 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> Oh, you work at KylinOS now 

Right.

> -- please clearly state in the commit
> message if the issue was seen in real life or AI-detected.

OK, I will.

Thanks
Hangbin

      reply	other threads:[~2026-08-13  2:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10  2:38 [PATCH net] bonding: fix u32 overflow in compute_gap() Hangbin Liu
2026-08-13  1:01 ` Jakub Kicinski
2026-08-13  2:28   ` Hangbin Liu [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=an0r33hYV6sJlfJX@fedora \
    --to=liuhangbin@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jv@jvosburgh.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuhangbin@kylinos.cn \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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