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

[Hangbin Liu]

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.