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

[Nikolay Aleksandrov <razor@blackwall.org>]

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...

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