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

[Nikolay Aleksandrov <razor@blackwall.org>]

On 19/08/2026 13:02, Nikolay Aleksandrov wrote:
> On 19/08/2026 12:51, Hangbin Liu wrote:
>> On Wed, Aug 19, 2026 at 11:35:29AM +0300, Nikolay Aleksandrov wrote:
>>> hmm why don't you change the way the reset is done? *untested* but in theory
>>> you could just record the values at a reset "moment" in reset unbalanced and
>>> just use the delta, so it becomes a reader and there is only 1 writer left (tx).
>>> Keep the counters only increasing (important), only record a snapshot at a reset
>>> moment, count current total bytes (sum all per-cpu data), decrement the previous
>>> total from it and use that as the "interval bytes" to div.
>>
>> Oh, you mean add another variable to track the total unbalanced load? e.g.
>>
> 
> right, but without any locking because...
> 

one more minor nit below

>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index 659a77323444..a65be54049d3 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -1546,10 +1546,10 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>       return bond_do_alb_xmit(skb, bond, tx_slave);
>>   }
>> -static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
>> +static u64 reset_unbalanced_load(struct bonding *bond, struct alb_bond_info *bond_info)
>>   {
>>       struct unbalanced_load_stats *p;
>> -    u64 tx_bytes, total_bytes = 0;
>> +    u64 delta, tx_bytes, total_bytes = 0;
>>       unsigned int start;
>>       int i;
>> @@ -1560,14 +1560,15 @@ static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
>>               tx_bytes = u64_stats_read(&p->tx_bytes);
>>           } while (u64_stats_fetch_retry(&p->syncp, start));
>> -        u64_stats_update_begin(&p->syncp);
>> -        u64_stats_set(&p->tx_bytes, 0);
>> -        u64_stats_update_end(&p->syncp);
>> -
>>           total_bytes += tx_bytes;
>>       }
>> -    return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL);
>> +    spin_lock_bh(&bond->mode_lock);
>> +    delta = total_bytes - bond_info->total_unbalanced;
>> +    bond_info->total_unbalanced = total_bytes;
>> +    spin_unlock_bh(&bond->mode_lock);
>> +
> 
> ... there should be only 1 alb monitor running, no need to lock to keep it up-to-date
>      also this is its only user, so remove the spinlock
> 
>> +    return div_u64(delta, BOND_TLB_REBALANCE_INTERVAL);
>>   }
>>   void bond_alb_monitor(struct work_struct *work)
>> @@ -1612,7 +1613,7 @@ void bond_alb_monitor(struct work_struct *work)
>>           bond_for_each_slave_rcu(bond, slave, iter) {
>>               tlb_clear_slave(bond, slave, 1);
>>               if (slave == rcu_access_pointer(bond->curr_active_slave))
>> -                SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond_info);
>> +                SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond, bond_info);
>>           }
>>           atomic_set(&bond_info->tx_rebalance_counter, 0);
>>       }
>> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
>> index 51c083c76115..9d3877644286 100644
>> --- a/include/net/bond_alb.h
>> +++ b/include/net/bond_alb.h
>> @@ -131,6 +131,7 @@ struct unbalanced_load_stats {
>>   struct alb_bond_info {
>>       struct tlb_client_info    *tx_hashtbl; /* Dynamically allocated */
>>       struct unbalanced_load_stats __percpu    *unbalanced_load;
>> +    u64            total_unbalanced;

I'd name this prev_total_unbalanced or something similar since it is
the previous recorded value

>>       atomic_t        tx_rebalance_counter;
>>       int            lp_counter;
>>       /* -------- rlb parameters -------- */
>>
>> This looks like an easy update :) Hope I didn't miss anything.
>>
>> Thanks
>> Hangbin
>