Re: [PATCH net v4 2/2] bonding: fix u32 overflow in compute_gap()

Hangbin Liu <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <aouffvAgf9fK-Sfd@fedora>
Hi Nikolay,
On Fri, Aug 21, 2026 at 04:12:31PM +0300, Nikolay Aleksandrov wrote:
> Completely untested, but something like:
> 
> -static u64 compute_gap(struct slave *slave)
> +static s64 compute_gap(struct slave *slave)
>  {
>         u64 slave_load = SLAVE_TLB_INFO(slave).load << 3;
>         u32 raw_speed = READ_ONCE(slave->speed);
>         u64 speed = (u64)raw_speed << 20;
> 
>         if (raw_speed == (u32)SPEED_UNKNOWN)
> -               return 0;
> -
> -       if (speed <= slave_load)
> -               return 0;
> +               return S64_MIN;
> 
> -       return speed - slave_load;
> +       return (s64)speed - (s64)slave_load;
>  }

I get what you mean now. I think this is a boundary‑choice problem.

I reject all unknown‑speed and overloaded NICs, while you believe we should
keep overloaded NICs. What if someone argues we ought to retain unknown‑speed
but non‑overloaded NICs (though I do not see how we could compute that)?

In any case, I am fine with your design. If there are no other objections,
I will resend the patch following your plan.

Thanks
Hangbin

> 
> Then change the selection variables and comparison:
> 
> -       u64 max_gap = 0;
> +       s64 max_gap = S64_MIN;
> 
> ...
> 
> -                       u64 gap = compute_gap(slave);
> +                       s64 gap = compute_gap(slave);
> 
> -                       if (max_gap <= gap) {
> +                       if (!least_loaded || max_gap < gap) {
> 
> This should choose the slave with smallest gap and put the unknown speed behind all
> slaves with known speeds.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.