Re: [PATCH net-next v7 2/3] net: airoha: fix ETS QoS stats counter underflow and cross-channel corruption
From: Lorenzo Bianconi <lorenzo@kernel.org>
Date: 2026-07-02 13:31:46
Also in:
linux-arm-kernel, linux-mediatek
airoha_qdma_get_tx_ets_stats() has two bugs:
- The hardware counters read via airoha_qdma_rr() are 32-bit values
but are stored in u64 locals and subtracted from u64 baselines. When
a 32-bit hardware counter wraps around, the subtraction produces a
large underflow value passed to _bstats_update().
- The baseline counters (cpu_tx_packets, fwd_tx_packets) are stored as
single per-device fields, but airoha_qdma_get_tx_ets_stats() is
called with different channel values (0-3). Each call reads a
different channel's hardware counter but overwrites the same
baseline, corrupting the delta computation for other channels.
Fix both by:
- Narrowing the counter locals and baselines to u32 so that 32-bit
unsigned subtraction handles wrap-around naturally.
- Grouping the baselines into a per-channel qos_stats array so each
channel tracks its own previous counter value independently.
Fixes: 20bf7d07c956 ("net: airoha: Add sched ETS offload support")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>commenting on sashiko's report: https://sashiko.dev/#/patchset/20260701-airoha-ethtool-priv_flags-v7-0-b4153bd44428%40kernel.org
quoted hunk ↗ jump to hunk
--- drivers/net/ethernet/airoha/airoha_eth.c | 18 +++++++++++------- drivers/net/ethernet/airoha/airoha_eth.h | 7 ++++--- 2 files changed, 15 insertions(+), 10 deletions(-)diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c index 8bba54ebcf07..2c9ceb9f16f8 100644 --- a/drivers/net/ethernet/airoha/airoha_eth.c +++ b/drivers/net/ethernet/airoha/airoha_eth.c@@ -2491,16 +2491,20 @@ static int airoha_qdma_get_tx_ets_stats(struct net_device *netdev, int channel, { struct airoha_gdm_dev *dev = netdev_priv(netdev); struct airoha_qdma *qdma = dev->qdma; + u32 cpu_tx_packets, fwd_tx_packets; + u64 tx_packets; - u64 cpu_tx_packets = airoha_qdma_rr(qdma, REG_CNTR_VAL(channel << 1)); - u64 fwd_tx_packets = airoha_qdma_rr(qdma, - REG_CNTR_VAL((channel << 1) + 1)); - u64 tx_packets = (cpu_tx_packets - dev->cpu_tx_packets) + - (fwd_tx_packets - dev->fwd_tx_packets); + cpu_tx_packets = airoha_qdma_rr(qdma, REG_CNTR_VAL(channel << 1)); + fwd_tx_packets = airoha_qdma_rr(qdma, + REG_CNTR_VAL((channel << 1) + 1)); + tx_packets = (u32)(cpu_tx_packets - + dev->qos_stats[channel].cpu_tx_packets) + + (u32)(fwd_tx_packets - + dev->qos_stats[channel].fwd_tx_packets);
- Will this addition overflow in 32-bit space before the result is assigned to
the 64-bit tx_packets?
- I do not think this is a problem since we are just considering the delta
betwen cpu_tx_packets/fwd_tx_packets and the previous value. Moreover, the
u32 cast will take care of possible wrap-around.
_bstats_update(opt->stats.bstats, 0, tx_packets);
- This isn't a bug introduced by this patch, but does calling _bstats_update()
here directly from process context race with the software datapath?
- Sashiko is right here. This is a pre-existing (theoretical) issue not
introduced by this patch. However, since the Airoha EN7581/EN7583 is
ARM64-only, u64_stats_update_begin/end are NOPs on this platform and
there is no actual race. IIUC the seqcount corruption scenario only
applies to 32-bit architectures. I guess we can
Regards,
Lorenzo
quoted hunk ↗ jump to hunk
- dev->cpu_tx_packets = cpu_tx_packets; - dev->fwd_tx_packets = fwd_tx_packets; + dev->qos_stats[channel].cpu_tx_packets = cpu_tx_packets; + dev->qos_stats[channel].fwd_tx_packets = fwd_tx_packets; return 0; }diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h index 87ab3ea10664..ac5f571f3e53 100644 --- a/drivers/net/ethernet/airoha/airoha_eth.h +++ b/drivers/net/ethernet/airoha/airoha_eth.h@@ -545,9 +545,10 @@ struct airoha_gdm_dev { struct airoha_eth *eth; DECLARE_BITMAP(qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS); - /* qos stats counters */ - u64 cpu_tx_packets; - u64 fwd_tx_packets; + struct { + u32 cpu_tx_packets; + u32 fwd_tx_packets; + } qos_stats[AIROHA_NUM_QOS_CHANNELS]; u32 flags; int nbq;-- 2.54.0
Attachments
- signature.asc [application/pgp-signature] 228 bytes