Re: [PATCH net v3 2/2] bonding: fix u32 overflow in compute_gap()
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: 2026-08-19 10:02:31
Also in:
lkml
On 19/08/2026 12:51, Hangbin Liu wrote:
On Wed, Aug 19, 2026 at 11:35:29AM +0300, Nikolay Aleksandrov wrote:quoted
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...
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
+ 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