[PATCH net v3 0/2] bonding: fix TLB load-tracking overflow on high-speed NICs

[PATCH net v3 0/2] bonding: fix TLB load-tracking overflow on high-speed NICs

[Hangbin Liu]

The bonding TLB (Transmit Load Balancing) mode tracks per-slave and
per-client transmit byte counts in u32 fields. At sustained throughput
above ~3.2 Gbit/s over the 10-second rebalance interval these counters
wrap, causing compute_gap() to produce incorrect gap values and
mis-select transmit slaves. Such speeds are routine on modern NICs
under heavy traffic.

This two-patch series fixes the overflow by widening the relevant
fields to u64.

Patch 1 converts the unbalanced_load counter to per-cpu state as a
preparatory step. The counter sits in the transmit hot path, so
converting it to per-cpu before widening avoids introducing cross-CPU
synchronization overhead for a u64.

Patch 2 widens tx_bytes, load_history, load, and the per-cpu
unbalanced_load tx_bytes from u32 to u64. It adds u64_stats_sync
protection for the per-cpu counter to prevent tearing on 32-bit
architectures, and reworks compute_gap() to use u64 arithmetic with
READ_ONCE() on slave->speed.

Note: In patch 2, I return 0 directly when the speed is unknown, because
I believe the comparison becomes meaningless if the NIC speed cannot be
determined. For example: a 1 G NIC and a 10 G NIC are bonded together,
yet both are marked as unknown. If you would prefer to retain the existing
logic, we could instead set the speed to `ULLONG_MAX` when speed is unknown.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
Changes in v3:
- Add a preparatory patch to convert unbalanced_load to per-cpu first
- widens tlb counters to u64 and add helpers to prevent tearing on 32-bit
- Link to v2: https://lore.kernel.org/r/20260814-bond_overflow-v2-1-d3fe588ad167@kylinos.cn

Changes in v2:
- update comment description, including AI-detected info.
- fix tx_bytes/load type detected by sashiko
- cast SPEED_UNKNOWN to 0 before shift, detected by sashiko
- Link to v1: https://lore.kernel.org/r/20260810-bond_overflow-v1-1-c9ff29d76770@kylinos.cn

---
Hangbin Liu (2):
      bonding: convert unbalanced_load to per-cpu state
      bonding: fix u32 overflow in compute_gap()

 drivers/net/bonding/bond_alb.c  | 69 +++++++++++++++++++++++++++++++----------
 drivers/net/bonding/bond_main.c |  9 ++++++
 include/net/bond_alb.h          | 13 +++++---
 3 files changed, 71 insertions(+), 20 deletions(-)
---
base-commit: 21040c7f931502070dcc66bb0f1aeed07dec032b
change-id: 20260806-bond_overflow-ac6a6a78d6a0

Best regards,
-- 
Hangbin Liu <liuhangbin@kylinos.cn>

[PATCH net v3 1/2] bonding: convert unbalanced_load to per-cpu state

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

A later patch widens the bonding TLB tx counters from u32 to u64. The
unbalanced_load counter sits in the transmit hot path, and cross-CPU
synchronization of a u64 would introduce measurable overhead. Convert
unbalanced_load to a per-cpu counter first so that the subsequent
widening only touches per-cpu data local to each CPU.

Introduce struct unbalanced_load_stats to hold the per-cpu counter,
and move the aggregation into a helper, reset_unbalanced_load(), which
sums and clears all per-cpu instances.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_alb.c  | 25 ++++++++++++++++++-------
 drivers/net/bonding/bond_main.c |  9 +++++++++
 include/net/bond_alb.h          |  6 +++++-
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 839f7482dc18..d54d834cf72b 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1345,7 +1345,7 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
 		/* unbalanced or unassigned, send through primary */
 		tx_slave = rcu_dereference(bond->curr_active_slave);
 		if (bond->params.tlb_dynamic_lb)
-			bond_info->unbalanced_load += skb->len;
+			this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
 	}
 
 	if (tx_slave && bond_slave_can_tx(tx_slave)) {
@@ -1529,6 +1529,21 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
+{
+	struct unbalanced_load_stats *p;
+	u32 total_bytes = 0;
+	int i;
+
+	for_each_possible_cpu(i) {
+		p = per_cpu_ptr(bond_info->unbalanced_load, i);
+		total_bytes += READ_ONCE(p->tx_bytes);
+		WRITE_ONCE(p->tx_bytes, 0);
+	}
+
+	return total_bytes / BOND_TLB_REBALANCE_INTERVAL;
+}
+
 void bond_alb_monitor(struct work_struct *work)
 {
 	struct bonding *bond = container_of(work, struct bonding,
@@ -1570,12 +1585,8 @@ void bond_alb_monitor(struct work_struct *work)
 	if (atomic_read(&bond_info->tx_rebalance_counter) >= BOND_TLB_REBALANCE_TICKS) {
 		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 =
-					bond_info->unbalanced_load /
-						BOND_TLB_REBALANCE_INTERVAL;
-				bond_info->unbalanced_load = 0;
-			}
+			if (slave == rcu_access_pointer(bond->curr_active_slave))
+				SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond_info);
 		}
 		atomic_set(&bond_info->tx_rebalance_counter, 0);
 	}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 522eab060f9e..9fb44e0031c8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5995,6 +5995,7 @@ static void bond_destructor(struct net_device *bond_dev)
 		destroy_workqueue(bond->wq);
 
 	free_percpu(bond->rr_tx_counter);
+	free_percpu(bond->alb_info.unbalanced_load);
 }
 
 void bond_setup(struct net_device *bond_dev)
@@ -6494,6 +6495,10 @@ static int bond_init(struct net_device *bond_dev)
 	if (!bond->wq)
 		return -ENOMEM;
 
+	bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
+	if (!bond->alb_info.unbalanced_load)
+		goto wq_out;
+
 	bond->notifier_ctx = false;
 
 	spin_lock_init(&bond->stats_lock);
@@ -6511,6 +6516,10 @@ static int bond_init(struct net_device *bond_dev)
 		eth_hw_addr_random(bond_dev);
 
 	return 0;
+
+wq_out:
+	destroy_workqueue(bond->wq);
+	return -ENOMEM;
 }
 
 unsigned int bond_get_num_tx_queues(void)
diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
index e5945427f38d..3fabf4714dec 100644
--- a/include/net/bond_alb.h
+++ b/include/net/bond_alb.h
@@ -123,9 +123,13 @@ struct tlb_slave_info {
 			 */
 };
 
+struct unbalanced_load_stats {
+	u32			tx_bytes;
+};
+
 struct alb_bond_info {
 	struct tlb_client_info	*tx_hashtbl; /* Dynamically allocated */
-	u32			unbalanced_load;
+	struct unbalanced_load_stats __percpu	*unbalanced_load;
 	atomic_t		tx_rebalance_counter;
 	int			lp_counter;
 	/* -------- rlb parameters -------- */

-- 
2.55.0

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

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

The TLB load-tracking fields tx_bytes, load_history, load, and
unbalanced_load are all u32. At sustained throughput above ~3.2 Gbit/s
over the 10-second rebalance interval the byte counters wrap, causing
compute_gap() to produce incorrect gap values and mis-select slaves.
Such speeds are common on modern NICs under heavy traffic.

Widen these fields to u64. Use u64_stats_sync to protect the per-cpu
unbalanced_load_stats against tearing on 32-bit architectures, and
div_u64() for the 64-bit divisions. The tx_bytes, load, and load_history
are protected in spin_lock.

Rework compute_gap() to use u64 arithmetic throughout. Return 0 when the
speed is unknown or the slave is already overloaded.

Detected by AI code review.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 drivers/net/bonding/bond_alb.c  | 56 ++++++++++++++++++++++++++++++-----------
 drivers/net/bonding/bond_main.c |  2 +-
 include/net/bond_alb.h          |  9 ++++---
 3 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index d54d834cf72b..659a77323444 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -6,6 +6,7 @@
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
 #include <linux/pkt_sched.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
@@ -74,8 +75,8 @@ static inline u8 _simple_hash(const u8 *hash_start, int hash_size)
 static inline void tlb_init_table_entry(struct tlb_client_info *entry, int save_load)
 {
 	if (save_load) {
-		entry->load_history = 1 + entry->tx_bytes /
-				      BOND_TLB_REBALANCE_INTERVAL;
+		entry->load_history = 1 + div_u64(entry->tx_bytes,
+				      BOND_TLB_REBALANCE_INTERVAL);
 		entry->tx_bytes = 0;
 	}
 
@@ -158,25 +159,35 @@ static void tlb_deinitialize(struct bonding *bond)
 	spin_unlock_bh(&bond->mode_lock);
 }
 
-static long long compute_gap(struct slave *slave)
+static u64 compute_gap(struct slave *slave)
 {
-	return (s64) (slave->speed << 20) - /* Convert to Megabit per sec */
-	       (s64) (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
+	u32 raw_speed = READ_ONCE(slave->speed);
+	u64 speed = (u64)raw_speed;
+
+	/* It's meaningless to compare gap on unknown speed NIC */
+	if (raw_speed == (u32)SPEED_UNKNOWN)
+		return 0;
+
+	/* skip slave which is over loaded */
+	if ((speed << 20) <= (SLAVE_TLB_INFO(slave).load << 3))
+		return 0;
+
+	return (speed << 20) - /* Convert to Megabit per sec */
+	       (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
 }
 
 static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
 {
 	struct slave *slave, *least_loaded;
 	struct list_head *iter;
-	long long max_gap;
+	u64 max_gap = 0;
 
 	least_loaded = NULL;
-	max_gap = LLONG_MIN;
 
 	/* Find the slave with the largest gap */
 	bond_for_each_slave_rcu(bond, slave, iter) {
 		if (bond_slave_can_tx(slave)) {
-			long long gap = compute_gap(slave);
+			u64 gap = compute_gap(slave);
 
 			if (max_gap < gap) {
 				least_loaded = slave;
@@ -1344,8 +1355,14 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
 	if (!tx_slave) {
 		/* unbalanced or unassigned, send through primary */
 		tx_slave = rcu_dereference(bond->curr_active_slave);
-		if (bond->params.tlb_dynamic_lb)
-			this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
+		if (bond->params.tlb_dynamic_lb) {
+			struct unbalanced_load_stats *pcpu_load;
+
+			pcpu_load = this_cpu_ptr(bond_info->unbalanced_load);
+			u64_stats_update_begin(&pcpu_load->syncp);
+			u64_stats_add(&pcpu_load->tx_bytes, skb->len);
+			u64_stats_update_end(&pcpu_load->syncp);
+		}
 	}
 
 	if (tx_slave && bond_slave_can_tx(tx_slave)) {
@@ -1529,19 +1546,28 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
+static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
 {
 	struct unbalanced_load_stats *p;
-	u32 total_bytes = 0;
+	u64 tx_bytes, total_bytes = 0;
+	unsigned int start;
 	int i;
 
 	for_each_possible_cpu(i) {
 		p = per_cpu_ptr(bond_info->unbalanced_load, i);
-		total_bytes += READ_ONCE(p->tx_bytes);
-		WRITE_ONCE(p->tx_bytes, 0);
+		do {
+			start = u64_stats_fetch_begin(&p->syncp);
+			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 total_bytes / BOND_TLB_REBALANCE_INTERVAL;
+	return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL);
 }
 
 void bond_alb_monitor(struct work_struct *work)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9fb44e0031c8..4c4d9bf71e0c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -6495,7 +6495,7 @@ static int bond_init(struct net_device *bond_dev)
 	if (!bond->wq)
 		return -ENOMEM;
 
-	bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
+	bond->alb_info.unbalanced_load = netdev_alloc_pcpu_stats(struct unbalanced_load_stats);
 	if (!bond->alb_info.unbalanced_load)
 		goto wq_out;
 
diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
index 3fabf4714dec..51c083c76115 100644
--- a/include/net/bond_alb.h
+++ b/include/net/bond_alb.h
@@ -57,12 +57,12 @@ struct tlb_client_info {
 				 * packets to a Client that the Hash function
 				 * gave this entry index.
 				 */
-	u32 tx_bytes;		/* Each Client accumulates the BytesTx that
+	u64 tx_bytes;		/* Each Client accumulates the BytesTx that
 				 * were transmitted to it, and after each
 				 * CallBack the LoadHistory is divided
 				 * by the balance interval
 				 */
-	u32 load_history;	/* This field contains the amount of Bytes
+	u64 load_history;	/* This field contains the amount of Bytes
 				 * that were transmitted to this client by
 				 * the server on the previous balance
 				 * interval in Bps.
@@ -118,13 +118,14 @@ struct tlb_slave_info {
 			 * are the entries that were assigned to use this
 			 * slave for transmit.
 			 */
-	u32 load;	/* Each slave sums the loadHistory of all clients
+	u64 load;	/* Each slave sums the loadHistory of all clients
 			 * assigned to it
 			 */
 };
 
 struct unbalanced_load_stats {
-	u32			tx_bytes;
+	u64_stats_t		tx_bytes;
+	struct u64_stats_sync	syncp;
 };
 
 struct alb_bond_info {

-- 
2.55.0

Re: [PATCH net v3 1/2] bonding: convert unbalanced_load to per-cpu state

[Nikolay Aleksandrov]

On 18/08/2026 11:47, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> A later patch widens the bonding TLB tx counters from u32 to u64. The
> unbalanced_load counter sits in the transmit hot path, and cross-CPU
> synchronization of a u64 would introduce measurable overhead. Convert
> unbalanced_load to a per-cpu counter first so that the subsequent
> widening only touches per-cpu data local to each CPU.
> 
> Introduce struct unbalanced_load_stats to hold the per-cpu counter,
> and move the aggregation into a helper, reset_unbalanced_load(), which
> sums and clears all per-cpu instances.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
> ---
>   drivers/net/bonding/bond_alb.c  | 25 ++++++++++++++++++-------
>   drivers/net/bonding/bond_main.c |  9 +++++++++
>   include/net/bond_alb.h          |  6 +++++-
>   3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 839f7482dc18..d54d834cf72b 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1345,7 +1345,7 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>   		/* unbalanced or unassigned, send through primary */
>   		tx_slave = rcu_dereference(bond->curr_active_slave);
>   		if (bond->params.tlb_dynamic_lb)
> -			bond_info->unbalanced_load += skb->len;
> +			this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
>   	}
>   
>   	if (tx_slave && bond_slave_can_tx(tx_slave)) {
> @@ -1529,6 +1529,21 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
> +{
> +	struct unbalanced_load_stats *p;
> +	u32 total_bytes = 0;
> +	int i;
> +
> +	for_each_possible_cpu(i) {
> +		p = per_cpu_ptr(bond_info->unbalanced_load, i);
> +		total_bytes += READ_ONCE(p->tx_bytes);
> +		WRITE_ONCE(p->tx_bytes, 0);
> +	}
> +
> +	return total_bytes / BOND_TLB_REBALANCE_INTERVAL;
> +}
> +
>   void bond_alb_monitor(struct work_struct *work)
>   {
>   	struct bonding *bond = container_of(work, struct bonding,
> @@ -1570,12 +1585,8 @@ void bond_alb_monitor(struct work_struct *work)
>   	if (atomic_read(&bond_info->tx_rebalance_counter) >= BOND_TLB_REBALANCE_TICKS) {
>   		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 =
> -					bond_info->unbalanced_load /
> -						BOND_TLB_REBALANCE_INTERVAL;
> -				bond_info->unbalanced_load = 0;
> -			}
> +			if (slave == rcu_access_pointer(bond->curr_active_slave))
> +				SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond_info);
>   		}
>   		atomic_set(&bond_info->tx_rebalance_counter, 0);
>   	}
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 522eab060f9e..9fb44e0031c8 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -5995,6 +5995,7 @@ static void bond_destructor(struct net_device *bond_dev)
>   		destroy_workqueue(bond->wq);
>   
>   	free_percpu(bond->rr_tx_counter);
> +	free_percpu(bond->alb_info.unbalanced_load);
>   }
>   
>   void bond_setup(struct net_device *bond_dev)
> @@ -6494,6 +6495,10 @@ static int bond_init(struct net_device *bond_dev)
>   	if (!bond->wq)
>   		return -ENOMEM;
>   
> +	bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
> +	if (!bond->alb_info.unbalanced_load)
> +		goto wq_out;
> +

The bond doesn't generally depend on this struct and shouldn't fail to init
if it fails to allocate and the user hasn't chosen alb mode. Also here is too
early to tell, the netlink mode hasn't been applied yet.
Maybe the allocation could be moved to bond_alb_initialize and the freeing to
bond_alb_deinitialize?

>   	bond->notifier_ctx = false;
>   
>   	spin_lock_init(&bond->stats_lock);
> @@ -6511,6 +6516,10 @@ static int bond_init(struct net_device *bond_dev)
>   		eth_hw_addr_random(bond_dev);
>   
>   	return 0;
> +
> +wq_out:
> +	destroy_workqueue(bond->wq);
> +	return -ENOMEM;
>   }
>   
>   unsigned int bond_get_num_tx_queues(void)
> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
> index e5945427f38d..3fabf4714dec 100644
> --- a/include/net/bond_alb.h
> +++ b/include/net/bond_alb.h
> @@ -123,9 +123,13 @@ struct tlb_slave_info {
>   			 */
>   };
>   
> +struct unbalanced_load_stats {
> +	u32			tx_bytes;
> +};
> +
>   struct alb_bond_info {
>   	struct tlb_client_info	*tx_hashtbl; /* Dynamically allocated */
> -	u32			unbalanced_load;
> +	struct unbalanced_load_stats __percpu	*unbalanced_load;
>   	atomic_t		tx_rebalance_counter;
>   	int			lp_counter;
>   	/* -------- rlb parameters -------- */
> 

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

[Nikolay Aleksandrov]

On 18/08/2026 11:47, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> The TLB load-tracking fields tx_bytes, load_history, load, and
> unbalanced_load are all u32. At sustained throughput above ~3.2 Gbit/s
> over the 10-second rebalance interval the byte counters wrap, causing
> compute_gap() to produce incorrect gap values and mis-select slaves.
> Such speeds are common on modern NICs under heavy traffic.
> 
> Widen these fields to u64. Use u64_stats_sync to protect the per-cpu
> unbalanced_load_stats against tearing on 32-bit architectures, and
> div_u64() for the 64-bit divisions. The tx_bytes, load, and load_history
> are protected in spin_lock.
> 
> Rework compute_gap() to use u64 arithmetic throughout. Return 0 when the
> speed is unknown or the slave is already overloaded.
> 
> Detected by AI code review.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
> ---
>   drivers/net/bonding/bond_alb.c  | 56 ++++++++++++++++++++++++++++++-----------
>   drivers/net/bonding/bond_main.c |  2 +-
>   include/net/bond_alb.h          |  9 ++++---
>   3 files changed, 47 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index d54d834cf72b..659a77323444 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -6,6 +6,7 @@
>   #include <linux/skbuff.h>
>   #include <linux/netdevice.h>
>   #include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
>   #include <linux/pkt_sched.h>
>   #include <linux/spinlock.h>
>   #include <linux/slab.h>
> @@ -74,8 +75,8 @@ static inline u8 _simple_hash(const u8 *hash_start, int hash_size)
>   static inline void tlb_init_table_entry(struct tlb_client_info *entry, int save_load)
>   {
>   	if (save_load) {
> -		entry->load_history = 1 + entry->tx_bytes /
> -				      BOND_TLB_REBALANCE_INTERVAL;
> +		entry->load_history = 1 + div_u64(entry->tx_bytes,
> +				      BOND_TLB_REBALANCE_INTERVAL);
>   		entry->tx_bytes = 0;
>   	}
>   
> @@ -158,25 +159,35 @@ static void tlb_deinitialize(struct bonding *bond)
>   	spin_unlock_bh(&bond->mode_lock);
>   }
>   
> -static long long compute_gap(struct slave *slave)
> +static u64 compute_gap(struct slave *slave)
>   {
> -	return (s64) (slave->speed << 20) - /* Convert to Megabit per sec */
> -	       (s64) (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
> +	u32 raw_speed = READ_ONCE(slave->speed);
> +	u64 speed = (u64)raw_speed;
> +
> +	/* It's meaningless to compare gap on unknown speed NIC */
> +	if (raw_speed == (u32)SPEED_UNKNOWN)
> +		return 0;
> +
> +	/* skip slave which is over loaded */
> +	if ((speed << 20) <= (SLAVE_TLB_INFO(slave).load << 3))
> +		return 0;
> +
> +	return (speed << 20) - /* Convert to Megabit per sec */
> +	       (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
>   }
>   
>   static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
>   {
>   	struct slave *slave, *least_loaded;
>   	struct list_head *iter;
> -	long long max_gap;
> +	u64 max_gap = 0;
>   
>   	least_loaded = NULL;
> -	max_gap = LLONG_MIN;
>   
>   	/* Find the slave with the largest gap */
>   	bond_for_each_slave_rcu(bond, slave, iter) {
>   		if (bond_slave_can_tx(slave)) {
> -			long long gap = compute_gap(slave);
> +			u64 gap = compute_gap(slave);
>   
>   			if (max_gap < gap) {
>   				least_loaded = slave;
> @@ -1344,8 +1355,14 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>   	if (!tx_slave) {
>   		/* unbalanced or unassigned, send through primary */
>   		tx_slave = rcu_dereference(bond->curr_active_slave);
> -		if (bond->params.tlb_dynamic_lb)
> -			this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
> +		if (bond->params.tlb_dynamic_lb) {
> +			struct unbalanced_load_stats *pcpu_load;
> +
> +			pcpu_load = this_cpu_ptr(bond_info->unbalanced_load);
> +			u64_stats_update_begin(&pcpu_load->syncp);
> +			u64_stats_add(&pcpu_load->tx_bytes, skb->len);
> +			u64_stats_update_end(&pcpu_load->syncp);

this still races with...

> +		}
>   	}
>   
>   	if (tx_slave && bond_slave_can_tx(tx_slave)) {
> @@ -1529,19 +1546,28 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
> +static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
>   {
>   	struct unbalanced_load_stats *p;
> -	u32 total_bytes = 0;
> +	u64 tx_bytes, total_bytes = 0;
> +	unsigned int start;
>   	int i;
>   
>   	for_each_possible_cpu(i) {
>   		p = per_cpu_ptr(bond_info->unbalanced_load, i);
> -		total_bytes += READ_ONCE(p->tx_bytes);
> -		WRITE_ONCE(p->tx_bytes, 0);
> +		do {
> +			start = u64_stats_fetch_begin(&p->syncp);
> +			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);

... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
must do that themselves, so you can't be sure what value will end up, the zeroing
might not work at all and can get overwritten

> +
> +		total_bytes += tx_bytes;
>   	}
>   
> -	return total_bytes / BOND_TLB_REBALANCE_INTERVAL;
> +	return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL);
>   }
>   
>   void bond_alb_monitor(struct work_struct *work)
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 9fb44e0031c8..4c4d9bf71e0c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -6495,7 +6495,7 @@ static int bond_init(struct net_device *bond_dev)
>   	if (!bond->wq)
>   		return -ENOMEM;
>   
> -	bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
> +	bond->alb_info.unbalanced_load = netdev_alloc_pcpu_stats(struct unbalanced_load_stats);
>   	if (!bond->alb_info.unbalanced_load)
>   		goto wq_out;
>   
> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
> index 3fabf4714dec..51c083c76115 100644
> --- a/include/net/bond_alb.h
> +++ b/include/net/bond_alb.h
> @@ -57,12 +57,12 @@ struct tlb_client_info {
>   				 * packets to a Client that the Hash function
>   				 * gave this entry index.
>   				 */
> -	u32 tx_bytes;		/* Each Client accumulates the BytesTx that
> +	u64 tx_bytes;		/* Each Client accumulates the BytesTx that
>   				 * were transmitted to it, and after each
>   				 * CallBack the LoadHistory is divided
>   				 * by the balance interval
>   				 */
> -	u32 load_history;	/* This field contains the amount of Bytes
> +	u64 load_history;	/* This field contains the amount of Bytes
>   				 * that were transmitted to this client by
>   				 * the server on the previous balance
>   				 * interval in Bps.
> @@ -118,13 +118,14 @@ struct tlb_slave_info {
>   			 * are the entries that were assigned to use this
>   			 * slave for transmit.
>   			 */
> -	u32 load;	/* Each slave sums the loadHistory of all clients
> +	u64 load;	/* Each slave sums the loadHistory of all clients
>   			 * assigned to it
>   			 */
>   };
>   
>   struct unbalanced_load_stats {
> -	u32			tx_bytes;
> +	u64_stats_t		tx_bytes;
> +	struct u64_stats_sync	syncp;
>   };
>   
>   struct alb_bond_info {
> 

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

[Nikolay Aleksandrov]

On 18/08/2026 12:44, Nikolay Aleksandrov wrote:
> On 18/08/2026 11:47, Hangbin Liu wrote:
>> From: Hangbin Liu <liuhangbin@kylinos.cn>
>>
>> The TLB load-tracking fields tx_bytes, load_history, load, and
>> unbalanced_load are all u32. At sustained throughput above ~3.2 Gbit/s
>> over the 10-second rebalance interval the byte counters wrap, causing
>> compute_gap() to produce incorrect gap values and mis-select slaves.
>> Such speeds are common on modern NICs under heavy traffic.
>>
>> Widen these fields to u64. Use u64_stats_sync to protect the per-cpu
>> unbalanced_load_stats against tearing on 32-bit architectures, and
>> div_u64() for the 64-bit divisions. The tx_bytes, load, and load_history
>> are protected in spin_lock.
>>
>> Rework compute_gap() to use u64 arithmetic throughout. Return 0 when the
>> speed is unknown or the slave is already overloaded.
>>
>> Detected by AI code review.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
>> ---
>>   drivers/net/bonding/bond_alb.c  | 56 ++++++++++++++++++++++++++++++-----------
>>   drivers/net/bonding/bond_main.c |  2 +-
>>   include/net/bond_alb.h          |  9 ++++---
>>   3 files changed, 47 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index d54d834cf72b..659a77323444 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/skbuff.h>
>>   #include <linux/netdevice.h>
>>   #include <linux/etherdevice.h>
>> +#include <linux/ethtool.h>
>>   #include <linux/pkt_sched.h>
>>   #include <linux/spinlock.h>
>>   #include <linux/slab.h>
>> @@ -74,8 +75,8 @@ static inline u8 _simple_hash(const u8 *hash_start, int hash_size)
>>   static inline void tlb_init_table_entry(struct tlb_client_info *entry, int save_load)
>>   {
>>       if (save_load) {
>> -        entry->load_history = 1 + entry->tx_bytes /
>> -                      BOND_TLB_REBALANCE_INTERVAL;
>> +        entry->load_history = 1 + div_u64(entry->tx_bytes,
>> +                      BOND_TLB_REBALANCE_INTERVAL);
>>           entry->tx_bytes = 0;
>>       }
>> @@ -158,25 +159,35 @@ static void tlb_deinitialize(struct bonding *bond)
>>       spin_unlock_bh(&bond->mode_lock);
>>   }
>> -static long long compute_gap(struct slave *slave)
>> +static u64 compute_gap(struct slave *slave)
>>   {
>> -    return (s64) (slave->speed << 20) - /* Convert to Megabit per sec */
>> -           (s64) (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
>> +    u32 raw_speed = READ_ONCE(slave->speed);
>> +    u64 speed = (u64)raw_speed;
>> +
>> +    /* It's meaningless to compare gap on unknown speed NIC */
>> +    if (raw_speed == (u32)SPEED_UNKNOWN)
>> +        return 0;
>> +
>> +    /* skip slave which is over loaded */
>> +    if ((speed << 20) <= (SLAVE_TLB_INFO(slave).load << 3))
>> +        return 0;
>> +
>> +    return (speed << 20) - /* Convert to Megabit per sec */
>> +           (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
>>   }
>>   static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
>>   {
>>       struct slave *slave, *least_loaded;
>>       struct list_head *iter;
>> -    long long max_gap;
>> +    u64 max_gap = 0;
>>       least_loaded = NULL;
>> -    max_gap = LLONG_MIN;
>>       /* Find the slave with the largest gap */
>>       bond_for_each_slave_rcu(bond, slave, iter) {
>>           if (bond_slave_can_tx(slave)) {
>> -            long long gap = compute_gap(slave);
>> +            u64 gap = compute_gap(slave);
>>               if (max_gap < gap) {
>>                   least_loaded = slave;
>> @@ -1344,8 +1355,14 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>>       if (!tx_slave) {
>>           /* unbalanced or unassigned, send through primary */
>>           tx_slave = rcu_dereference(bond->curr_active_slave);
>> -        if (bond->params.tlb_dynamic_lb)
>> -            this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
>> +        if (bond->params.tlb_dynamic_lb) {
>> +            struct unbalanced_load_stats *pcpu_load;
>> +
>> +            pcpu_load = this_cpu_ptr(bond_info->unbalanced_load);
>> +            u64_stats_update_begin(&pcpu_load->syncp);
>> +            u64_stats_add(&pcpu_load->tx_bytes, skb->len);
>> +            u64_stats_update_end(&pcpu_load->syncp);
> 
> this still races with...
> 
>> +        }
>>       }
>>       if (tx_slave && bond_slave_can_tx(tx_slave)) {
>> @@ -1529,19 +1546,28 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
>> +static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
>>   {
>>       struct unbalanced_load_stats *p;
>> -    u32 total_bytes = 0;
>> +    u64 tx_bytes, total_bytes = 0;
>> +    unsigned int start;
>>       int i;
>>       for_each_possible_cpu(i) {
>>           p = per_cpu_ptr(bond_info->unbalanced_load, i);
>> -        total_bytes += READ_ONCE(p->tx_bytes);
>> -        WRITE_ONCE(p->tx_bytes, 0);
>> +        do {
>> +            start = u64_stats_fetch_begin(&p->syncp);
>> +            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);
> 
> ... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
> must do that themselves, so you can't be sure what value will end up, the zeroing
> might not work at all and can get overwritten
> 

I meant - it doesn't improve on the current situation where it can also happen. :)

>> +
>> +        total_bytes += tx_bytes;
>>       }
>> -    return total_bytes / BOND_TLB_REBALANCE_INTERVAL;
>> +    return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL);
>>   }
>>   void bond_alb_monitor(struct work_struct *work)
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 9fb44e0031c8..4c4d9bf71e0c 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -6495,7 +6495,7 @@ static int bond_init(struct net_device *bond_dev)
>>       if (!bond->wq)
>>           return -ENOMEM;
>> -    bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
>> +    bond->alb_info.unbalanced_load = netdev_alloc_pcpu_stats(struct unbalanced_load_stats);
>>       if (!bond->alb_info.unbalanced_load)
>>           goto wq_out;
>> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
>> index 3fabf4714dec..51c083c76115 100644
>> --- a/include/net/bond_alb.h
>> +++ b/include/net/bond_alb.h
>> @@ -57,12 +57,12 @@ struct tlb_client_info {
>>                    * packets to a Client that the Hash function
>>                    * gave this entry index.
>>                    */
>> -    u32 tx_bytes;        /* Each Client accumulates the BytesTx that
>> +    u64 tx_bytes;        /* Each Client accumulates the BytesTx that
>>                    * were transmitted to it, and after each
>>                    * CallBack the LoadHistory is divided
>>                    * by the balance interval
>>                    */
>> -    u32 load_history;    /* This field contains the amount of Bytes
>> +    u64 load_history;    /* This field contains the amount of Bytes
>>                    * that were transmitted to this client by
>>                    * the server on the previous balance
>>                    * interval in Bps.
>> @@ -118,13 +118,14 @@ struct tlb_slave_info {
>>                * are the entries that were assigned to use this
>>                * slave for transmit.
>>                */
>> -    u32 load;    /* Each slave sums the loadHistory of all clients
>> +    u64 load;    /* Each slave sums the loadHistory of all clients
>>                * assigned to it
>>                */
>>   };
>>   struct unbalanced_load_stats {
>> -    u32            tx_bytes;
>> +    u64_stats_t        tx_bytes;
>> +    struct u64_stats_sync    syncp;
>>   };
>>   struct alb_bond_info {
>>
> 

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

[Nikolay Aleksandrov]

On 18/08/2026 14:06, Nikolay Aleksandrov wrote:
> On 18/08/2026 12:44, Nikolay Aleksandrov wrote:
>> On 18/08/2026 11:47, Hangbin Liu wrote:
>>> From: Hangbin Liu <liuhangbin@kylinos.cn>
>>>
>>> The TLB load-tracking fields tx_bytes, load_history, load, and
>>> unbalanced_load are all u32. At sustained throughput above ~3.2 Gbit/s
>>> over the 10-second rebalance interval the byte counters wrap, causing
>>> compute_gap() to produce incorrect gap values and mis-select slaves.
>>> Such speeds are common on modern NICs under heavy traffic.
>>>
>>> Widen these fields to u64. Use u64_stats_sync to protect the per-cpu
>>> unbalanced_load_stats against tearing on 32-bit architectures, and
>>> div_u64() for the 64-bit divisions. The tx_bytes, load, and load_history
>>> are protected in spin_lock.
>>>
>>> Rework compute_gap() to use u64 arithmetic throughout. Return 0 when the
>>> speed is unknown or the slave is already overloaded.
>>>
>>> Detected by AI code review.
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
>>> ---
>>>   drivers/net/bonding/bond_alb.c  | 56 ++++++++++++++++++++++++++++++-----------
>>>   drivers/net/bonding/bond_main.c |  2 +-
>>>   include/net/bond_alb.h          |  9 ++++---
>>>   3 files changed, 47 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>>> index d54d834cf72b..659a77323444 100644
>>> --- a/drivers/net/bonding/bond_alb.c
>>> +++ b/drivers/net/bonding/bond_alb.c
>>> @@ -6,6 +6,7 @@
>>>   #include <linux/skbuff.h>
>>>   #include <linux/netdevice.h>
>>>   #include <linux/etherdevice.h>
>>> +#include <linux/ethtool.h>
>>>   #include <linux/pkt_sched.h>
>>>   #include <linux/spinlock.h>
>>>   #include <linux/slab.h>
>>> @@ -74,8 +75,8 @@ static inline u8 _simple_hash(const u8 *hash_start, int hash_size)
>>>   static inline void tlb_init_table_entry(struct tlb_client_info *entry, int save_load)
>>>   {
>>>       if (save_load) {
>>> -        entry->load_history = 1 + entry->tx_bytes /
>>> -                      BOND_TLB_REBALANCE_INTERVAL;
>>> +        entry->load_history = 1 + div_u64(entry->tx_bytes,
>>> +                      BOND_TLB_REBALANCE_INTERVAL);
>>>           entry->tx_bytes = 0;
>>>       }
>>> @@ -158,25 +159,35 @@ static void tlb_deinitialize(struct bonding *bond)
>>>       spin_unlock_bh(&bond->mode_lock);
>>>   }
>>> -static long long compute_gap(struct slave *slave)
>>> +static u64 compute_gap(struct slave *slave)
>>>   {
>>> -    return (s64) (slave->speed << 20) - /* Convert to Megabit per sec */
>>> -           (s64) (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
>>> +    u32 raw_speed = READ_ONCE(slave->speed);
>>> +    u64 speed = (u64)raw_speed;
>>> +
>>> +    /* It's meaningless to compare gap on unknown speed NIC */
>>> +    if (raw_speed == (u32)SPEED_UNKNOWN)
>>> +        return 0;
>>> +
>>> +    /* skip slave which is over loaded */
>>> +    if ((speed << 20) <= (SLAVE_TLB_INFO(slave).load << 3))
>>> +        return 0;
>>> +
>>> +    return (speed << 20) - /* Convert to Megabit per sec */
>>> +           (SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */
>>>   }
>>>   static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
>>>   {
>>>       struct slave *slave, *least_loaded;
>>>       struct list_head *iter;
>>> -    long long max_gap;
>>> +    u64 max_gap = 0;
>>>       least_loaded = NULL;
>>> -    max_gap = LLONG_MIN;
>>>       /* Find the slave with the largest gap */
>>>       bond_for_each_slave_rcu(bond, slave, iter) {
>>>           if (bond_slave_can_tx(slave)) {
>>> -            long long gap = compute_gap(slave);
>>> +            u64 gap = compute_gap(slave);
>>>               if (max_gap < gap) {
>>>                   least_loaded = slave;
>>> @@ -1344,8 +1355,14 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
>>>       if (!tx_slave) {
>>>           /* unbalanced or unassigned, send through primary */
>>>           tx_slave = rcu_dereference(bond->curr_active_slave);
>>> -        if (bond->params.tlb_dynamic_lb)
>>> -            this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
>>> +        if (bond->params.tlb_dynamic_lb) {
>>> +            struct unbalanced_load_stats *pcpu_load;
>>> +
>>> +            pcpu_load = this_cpu_ptr(bond_info->unbalanced_load);
>>> +            u64_stats_update_begin(&pcpu_load->syncp);
>>> +            u64_stats_add(&pcpu_load->tx_bytes, skb->len);
>>> +            u64_stats_update_end(&pcpu_load->syncp);
>>
>> this still races with...
>>
>>> +        }
>>>       }
>>>       if (tx_slave && bond_slave_can_tx(tx_slave)) {
>>> @@ -1529,19 +1546,28 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
>>> +static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
>>>   {
>>>       struct unbalanced_load_stats *p;
>>> -    u32 total_bytes = 0;
>>> +    u64 tx_bytes, total_bytes = 0;
>>> +    unsigned int start;
>>>       int i;
>>>       for_each_possible_cpu(i) {
>>>           p = per_cpu_ptr(bond_info->unbalanced_load, i);
>>> -        total_bytes += READ_ONCE(p->tx_bytes);
>>> -        WRITE_ONCE(p->tx_bytes, 0);
>>> +        do {
>>> +            start = u64_stats_fetch_begin(&p->syncp);
>>> +            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);
>>
>> ... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
>> must do that themselves, so you can't be sure what value will end up, the zeroing
>> might not work at all and can get overwritten
>>
> 
> I meant - it doesn't improve on the current situation where it can also happen. :)
> 

Sorry for the multiple replies, but thinking about this - having multiple concurrent
writers could cause write tearing for 32-bit architectures (the monitor is a writer
and can write concurrently with tx) leading to invalid result.

>>> +
>>> +        total_bytes += tx_bytes;
>>>       }
>>> -    return total_bytes / BOND_TLB_REBALANCE_INTERVAL;
>>> +    return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL);
>>>   }
>>>   void bond_alb_monitor(struct work_struct *work)
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index 9fb44e0031c8..4c4d9bf71e0c 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -6495,7 +6495,7 @@ static int bond_init(struct net_device *bond_dev)
>>>       if (!bond->wq)
>>>           return -ENOMEM;
>>> -    bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
>>> +    bond->alb_info.unbalanced_load = netdev_alloc_pcpu_stats(struct unbalanced_load_stats);
>>>       if (!bond->alb_info.unbalanced_load)
>>>           goto wq_out;
>>> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h
>>> index 3fabf4714dec..51c083c76115 100644
>>> --- a/include/net/bond_alb.h
>>> +++ b/include/net/bond_alb.h
>>> @@ -57,12 +57,12 @@ struct tlb_client_info {
>>>                    * packets to a Client that the Hash function
>>>                    * gave this entry index.
>>>                    */
>>> -    u32 tx_bytes;        /* Each Client accumulates the BytesTx that
>>> +    u64 tx_bytes;        /* Each Client accumulates the BytesTx that
>>>                    * were transmitted to it, and after each
>>>                    * CallBack the LoadHistory is divided
>>>                    * by the balance interval
>>>                    */
>>> -    u32 load_history;    /* This field contains the amount of Bytes
>>> +    u64 load_history;    /* This field contains the amount of Bytes
>>>                    * that were transmitted to this client by
>>>                    * the server on the previous balance
>>>                    * interval in Bps.
>>> @@ -118,13 +118,14 @@ struct tlb_slave_info {
>>>                * are the entries that were assigned to use this
>>>                * slave for transmit.
>>>                */
>>> -    u32 load;    /* Each slave sums the loadHistory of all clients
>>> +    u64 load;    /* Each slave sums the loadHistory of all clients
>>>                * assigned to it
>>>                */
>>>   };
>>>   struct unbalanced_load_stats {
>>> -    u32            tx_bytes;
>>> +    u64_stats_t        tx_bytes;
>>> +    struct u64_stats_sync    syncp;
>>>   };
>>>   struct alb_bond_info {
>>>
>>
> 

Re: [PATCH net v3 1/2] bonding: convert unbalanced_load to per-cpu state

[Hangbin Liu]

On Tue, Aug 18, 2026 at 12:42:10PM +0300, Nikolay Aleksandrov wrote:
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 522eab060f9e..9fb44e0031c8 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -5995,6 +5995,7 @@ static void bond_destructor(struct net_device *bond_dev)
> >   		destroy_workqueue(bond->wq);
> >   	free_percpu(bond->rr_tx_counter);
> > +	free_percpu(bond->alb_info.unbalanced_load);
> >   }
> >   void bond_setup(struct net_device *bond_dev)
> > @@ -6494,6 +6495,10 @@ static int bond_init(struct net_device *bond_dev)
> >   	if (!bond->wq)
> >   		return -ENOMEM;
> > +	bond->alb_info.unbalanced_load = alloc_percpu(struct unbalanced_load_stats);
> > +	if (!bond->alb_info.unbalanced_load)
> > +		goto wq_out;
> > +
> 
> The bond doesn't generally depend on this struct and shouldn't fail to init
> if it fails to allocate and the user hasn't chosen alb mode. Also here is too
> early to tell, the netlink mode hasn't been applied yet.
> Maybe the allocation could be moved to bond_alb_initialize and the freeing to
> bond_alb_deinitialize?

Makes sense. Let me move it to tlb_initialize/tlb_deinitialize.

Thanks
Hangbin

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

[Hangbin Liu]

Hi Nikolay,
On Tue, Aug 18, 2026 at 02:51:31PM +0300, Nikolay Aleksandrov wrote:
> > > > @@ -1344,8 +1355,14 @@ static netdev_tx_t bond_do_alb_xmit(struct sk_buff *skb, struct bonding *bond,
> > > >       if (!tx_slave) {
> > > >           /* unbalanced or unassigned, send through primary */
> > > >           tx_slave = rcu_dereference(bond->curr_active_slave);
> > > > -        if (bond->params.tlb_dynamic_lb)
> > > > -            this_cpu_add(bond_info->unbalanced_load->tx_bytes, skb->len);
> > > > +        if (bond->params.tlb_dynamic_lb) {
> > > > +            struct unbalanced_load_stats *pcpu_load;
> > > > +
> > > > +            pcpu_load = this_cpu_ptr(bond_info->unbalanced_load);
> > > > +            u64_stats_update_begin(&pcpu_load->syncp);
> > > > +            u64_stats_add(&pcpu_load->tx_bytes, skb->len);
> > > > +            u64_stats_update_end(&pcpu_load->syncp);
> > > 
> > > this still races with...
> > > 
> > > > +        }
> > > >       }
> > > >       if (tx_slave && bond_slave_can_tx(tx_slave)) {
> > > > @@ -1529,19 +1546,28 @@ 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 u32 reset_unbalanced_load(struct alb_bond_info *bond_info)
> > > > +static u64 reset_unbalanced_load(struct alb_bond_info *bond_info)
> > > >   {
> > > >       struct unbalanced_load_stats *p;
> > > > -    u32 total_bytes = 0;
> > > > +    u64 tx_bytes, total_bytes = 0;
> > > > +    unsigned int start;
> > > >       int i;
> > > >       for_each_possible_cpu(i) {
> > > >           p = per_cpu_ptr(bond_info->unbalanced_load, i);
> > > > -        total_bytes += READ_ONCE(p->tx_bytes);
> > > > -        WRITE_ONCE(p->tx_bytes, 0);
> > > > +        do {
> > > > +            start = u64_stats_fetch_begin(&p->syncp);
> > > > +            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);
> > > 
> > > ... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
> > > must do that themselves, so you can't be sure what value will end up, the zeroing
> > > might not work at all and can get overwritten
> > > 
> > 
> > I meant - it doesn't improve on the current situation where it can also happen. :)

Ah, yes. I forgot this. The reset_unbalanced_load() could be called on any
CPU, which conflicts with other writers.

I re-checked the code. unbalanced_load is only called in two situations:
1. To rebalance the load in bond_alb_monitor(), which only executes once
   every 10 seconds.
2. !tx_slave in bond_do_alb_xmit(), which is only for multicast/broadcast
   traffic. This traffic shouldn't be significant.

So looks using spin_lock here is acceptable. What do you think?

> > 
> 
> Sorry for the multiple replies, but thinking about this - having multiple concurrent
> writers could cause write tearing for 32-bit architectures (the monitor is a writer
> and can write concurrently with tx) leading to invalid result.

Never mind. A detailed explanation is always welcome. I really appreciate
your review and comments.

Best Regards
Hangbin

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

[Hangbin Liu]

On Wed, Aug 19, 2026 at 10:07:22AM +0800, Hangbin Liu wrote:
> > > > ... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
> > > > must do that themselves, so you can't be sure what value will end up, the zeroing
> > > > might not work at all and can get overwritten
> > > > 
> > > 
> > > I meant - it doesn't improve on the current situation where it can also happen. :)
> 
> Ah, yes. I forgot this. The reset_unbalanced_load() could be called on any
> CPU, which conflicts with other writers.
> 
> I re-checked the code. unbalanced_load is only called in two situations:
> 1. To rebalance the load in bond_alb_monitor(), which only executes once
>    every 10 seconds.
> 2. !tx_slave in bond_do_alb_xmit(), which is only for multicast/broadcast
>    traffic. This traffic shouldn't be significant.
> 
> So looks using spin_lock here is acceptable. What do you think?

I mean, drop the per-CPU design directly and use spin_lock to protect the data.

Hangbin

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

[Nikolay Aleksandrov]

On 19/08/2026 05:09, Hangbin Liu wrote:
> On Wed, Aug 19, 2026 at 10:07:22AM +0800, Hangbin Liu wrote:
>>>>> ... this here, as u64_stats_update_begin doesn't provide exclusive access, so writers
>>>>> must do that themselves, so you can't be sure what value will end up, the zeroing
>>>>> might not work at all and can get overwritten
>>>>>
>>>>
>>>> I meant - it doesn't improve on the current situation where it can also happen. :)
>>
>> Ah, yes. I forgot this. The reset_unbalanced_load() could be called on any
>> CPU, which conflicts with other writers.
>>
>> I re-checked the code. unbalanced_load is only called in two situations:
>> 1. To rebalance the load in bond_alb_monitor(), which only executes once
>>     every 10 seconds.
>> 2. !tx_slave in bond_do_alb_xmit(), which is only for multicast/broadcast
>>     traffic. This traffic shouldn't be significant.
>>
>> So looks using spin_lock here is acceptable. What do you think?
> 
> I mean, drop the per-CPU design directly and use spin_lock to protect the data.
> 
> Hangbin

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.

Cheers,
  Nik

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

[Hangbin Liu]

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.

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);
+
+	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

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

[Nikolay Aleksandrov]

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

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

[Nikolay Aleksandrov]

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
>