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

[Jay Vosburgh <jv@jvosburgh.net>]

Hangbin Liu <hangbin.liu@linux.dev> wrote:

>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. Also use a prev_total_unbalanced
>to store the previous total load to avoid reset per-cpu data.
>
>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 s64 arithmetic with
>READ_ONCE() on slave->speed.
>
>Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>

	For the series, looks good to me.

	My only nit is that you don't state why you use READ_ONCE for
the access of ->speed, and I didn't see it in the prior discussion
(perhaps I missed it).  Regardless, I presume it's because the access
could race with a concurrent update in bond_update_speed_duplex.

Acked-by: Jay Vosburgh <jv@jvosburgh.net>

	-J


>---
>Changes in v6:
>- Do not return fixed value for unknown speed logic, we need load balance
>  even NIC speed is UNKNOWN (sashiko)
>- Link to v5: https://lore.kernel.org/r/20260825-bond_overflow-v5-0-7a800de133f1@kylinos.cn
>
>Changes in v5:
>- use s64 as the return value for compute_gap, so we can compute the
>  difference when NICs are overload. (Nikolay Aleksandrov)
>- Link to v4: https://lore.kernel.org/r/20260820-bond_overflow-v4-0-805ba0d3efb6@kylinos.cn
>
>Changes in v4:
>- move per-cpu allocation to tlb_initialize/tlb_deinitialize (Nikolay Aleksandrov)
>- use an extra prev_total_unbalanced to avoid reset per-cpu data (Nikolay Aleksandrov)
>- Link to v3: https://lore.kernel.org/r/20260818-bond_overflow-v3-0-e05d4dbc2fd8@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 | 63 +++++++++++++++++++++++++++++++++++-------
> include/net/bond_alb.h         | 14 +++++++---
> 2 files changed, 63 insertions(+), 14 deletions(-)
>---
>base-commit: a8455260b2e9c024d1872ac1c094793d55a7e537
>change-id: 20260806-bond_overflow-ac6a6a78d6a0
>
>Best regards,
>-- 
>Hangbin Liu <liuhangbin@kylinos.cn>
>

---
	-Jay Vosburgh, jv@jvosburgh.net