[PATCH net v2] net: airoha: fix MIB stats collection to be lossless
From: Aniket Negi <hidden>
Date: 2026-07-01 17:40:20
Also in:
linux-mediatek, lkml, netdev
Subsystem:
airoha ethernet driver, networking drivers, the rest · Maintainers:
Lorenzo Bianconi, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
The current driver resets hardware MIB counters after every read via
REG_FE_GDM_MIB_CLEAR. This creates a race window: packets arriving
between the read and the clear are silently lost from statistics.
Fix this by removing the MIB clear and switching to a delta-based
software tracking approach:
- 64-bit H+L registers (tx/rx ok pkts, ok bytes, E64..L1023):
read the absolute hardware total directly each poll.
- 32-bit registers (drops, bc, mc, errors, runt, long, ...):
store the previous raw register value in mib_prev and accumulate
(u32)(curr - prev) into a 64-bit software counter. Unsigned
subtraction handles wrap-around transparently.
- tx_len[0]/rx_len[0] ({0,64} RMON bucket) combines RUNT_CNT
(32-bit, delta-tracked via mib_prev.tx_runt_cnt) and E64_CNT
(64-bit, absolute). A u64 accumulator tx_runt_accum64 holds the
running RUNT delta sum so that each poll sets:
tx_len[0] = tx_runt_accum64 + E64_abs
without double-counting the E64 value.
Merge airoha_dev_get_hw_stats() into airoha_update_hw_stats(),
moving the port spin_lock inside so callers do not need a separate
wrapper.
Signed-off-by: Aniket Negi <redacted>
---
Changes in v2:
- Store _CNT_L register reads in val before adding to stats, improving
readability (suggested by Lorenzo Bianconi)
- Fix double-counting bug in the RUNT+E64 combined bucket: previously
"+=" for E64 re-added the full absolute counter each poll; now a
dedicated tx_runt_accum64/rx_runt_accum64 accumulator holds the
running RUNT delta, and tx_len[0] is assigned (not accumulated) each
poll as runt_accum64 + E64_abs
- Replace 7-element tx_len[]/rx_len[] shadow arrays in mib_prev with
focused tx_runt_cnt/tx_long_cnt and rx_runt_cnt/rx_long_cnt fields;
only RUNT and LONG are 32-bit and need wrap-around tracking
- Rename inner struct hw_prev_stats to mib_prev; rename accumulator
fields to tx_runt_accum64/rx_runt_accum64 for clarity
- Fix comment alignment in mib_prev struct block
- Rename airoha_dev_get_hw_stats() to airoha_update_hw_stats() and
move the port spin_lock inside, removing the separate wrapper
drivers/net/ethernet/airoha/airoha_eth.c | 115 +++++++++++++----------
drivers/net/ethernet/airoha/airoha_eth.h | 27 ++++++
2 files changed, 92 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 59001fd4b6f7..4b7c547de165 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c@@ -1686,12 +1686,14 @@ static void airoha_qdma_stop_napi(struct airoha_qdma *qdma) } } -static void airoha_dev_get_hw_stats(struct airoha_gdm_dev *dev) +static void airoha_update_hw_stats(struct airoha_gdm_dev *dev) { struct airoha_gdm_port *port = dev->port; struct airoha_eth *eth = dev->eth; u32 val, i = 0; + spin_lock(&port->stats_lock); + /* Read relevant MIB for GDM with multiple port attached */ if (port->id == AIROHA_GDM3_IDX || port->id == AIROHA_GDM4_IDX) airoha_fe_rmw(eth, REG_FE_GDM_MIB_CFG(port->id),
@@ -1701,152 +1703,165 @@ static void airoha_dev_get_hw_stats(struct airoha_gdm_dev *dev) u64_stats_update_begin(&dev->stats.syncp); - /* TX */ + /* TX - 64-bit H+L registers: hw accumulates the total, read directly. */ val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id)); - dev->stats.tx_ok_pkts += ((u64)val << 32); + dev->stats.tx_ok_pkts = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id)); dev->stats.tx_ok_pkts += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_H(port->id)); - dev->stats.tx_ok_bytes += ((u64)val << 32); + dev->stats.tx_ok_bytes = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_L(port->id)); dev->stats.tx_ok_bytes += val; + /* TX - 32-bit registers: accumulate delta to handle wrap-around. */ val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_DROP_CNT(port->id)); - dev->stats.tx_drops += val; + dev->stats.tx_drops += (u32)(val - dev->stats.mib_prev.tx_drops); + dev->stats.mib_prev.tx_drops = val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_BC_CNT(port->id)); - dev->stats.tx_broadcast += val; + dev->stats.tx_broadcast += (u32)(val - dev->stats.mib_prev.tx_broadcast); + dev->stats.mib_prev.tx_broadcast = val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_MC_CNT(port->id)); - dev->stats.tx_multicast += val; + dev->stats.tx_multicast += (u32)(val - dev->stats.mib_prev.tx_multicast); + dev->stats.mib_prev.tx_multicast = val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_RUNT_CNT(port->id)); - dev->stats.tx_len[i] += val; + dev->stats.mib_prev.tx_runt_accum64 += + (u32)(val - dev->stats.mib_prev.tx_runt_cnt); + dev->stats.mib_prev.tx_runt_cnt = val; + + /* tx_len[0]: RUNT (32-bit, delta) + E64 (64-bit, absolute) → {0, 64} bucket. + * Accumulate RUNT delta in tx_runt_accum64, then assign tx_len[0] as + * accum + E64_abs so each call gives the correct combined total. + */ + + dev->stats.tx_len[i] = dev->stats.mib_prev.tx_runt_accum64; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] += (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_H(port->id)); - dev->stats.tx_len[i] += ((u64)val << 32); + dev->stats.tx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_L(port->id)); dev->stats.tx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_LONG_CNT(port->id)); - dev->stats.tx_len[i++] += val; + dev->stats.tx_len[i++] += (u32)(val - dev->stats.mib_prev.tx_long_cnt); + dev->stats.mib_prev.tx_long_cnt = val; /* RX */ val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_H(port->id)); - dev->stats.rx_ok_pkts += ((u64)val << 32); + dev->stats.rx_ok_pkts = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_L(port->id)); dev->stats.rx_ok_pkts += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_H(port->id)); - dev->stats.rx_ok_bytes += ((u64)val << 32); + dev->stats.rx_ok_bytes = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_L(port->id)); dev->stats.rx_ok_bytes += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_DROP_CNT(port->id)); - dev->stats.rx_drops += val; + dev->stats.rx_drops += (u32)(val - dev->stats.mib_prev.rx_drops); + dev->stats.mib_prev.rx_drops = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_BC_CNT(port->id)); - dev->stats.rx_broadcast += val; + dev->stats.rx_broadcast += (u32)(val - dev->stats.mib_prev.rx_broadcast); + dev->stats.mib_prev.rx_broadcast = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_MC_CNT(port->id)); - dev->stats.rx_multicast += val; + dev->stats.rx_multicast += (u32)(val - dev->stats.mib_prev.rx_multicast); + dev->stats.mib_prev.rx_multicast = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ERROR_DROP_CNT(port->id)); - dev->stats.rx_errors += val; + dev->stats.rx_errors += (u32)(val - dev->stats.mib_prev.rx_errors); + dev->stats.mib_prev.rx_errors = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_CRC_ERR_CNT(port->id)); - dev->stats.rx_crc_error += val; + dev->stats.rx_crc_error += (u32)(val - dev->stats.mib_prev.rx_crc_error); + dev->stats.mib_prev.rx_crc_error = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_OVERFLOW_DROP_CNT(port->id)); - dev->stats.rx_over_errors += val; + dev->stats.rx_over_errors += (u32)(val - dev->stats.mib_prev.rx_over_errors); + dev->stats.mib_prev.rx_over_errors = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_FRAG_CNT(port->id)); - dev->stats.rx_fragment += val; + dev->stats.rx_fragment += (u32)(val - dev->stats.mib_prev.rx_fragment); + dev->stats.mib_prev.rx_fragment = val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_JABBER_CNT(port->id)); - dev->stats.rx_jabber += val; + dev->stats.rx_jabber += (u32)(val - dev->stats.mib_prev.rx_jabber); + dev->stats.mib_prev.rx_jabber = val; i = 0; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_RUNT_CNT(port->id)); - dev->stats.rx_len[i] += val; + dev->stats.mib_prev.rx_runt_accum64 += + (u32)(val - dev->stats.mib_prev.rx_runt_cnt); + dev->stats.mib_prev.rx_runt_cnt = val; + + /* rx_len[0]: RUNT (32-bit, delta) + E64 (64-bit, absolute) → {0, 64} bucket. + * then assign rx_len[0] = rx_runt_accum64 + E64_abs. + */ + dev->stats.rx_len[i] = dev->stats.mib_prev.rx_runt_accum64; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] += (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_H(port->id)); - dev->stats.rx_len[i] += ((u64)val << 32); + dev->stats.rx_len[i] = (u64)val << 32; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_L(port->id)); dev->stats.rx_len[i++] += val; val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_LONG_CNT(port->id)); - dev->stats.rx_len[i++] += val; + dev->stats.rx_len[i] += (u32)(val - dev->stats.mib_prev.rx_long_cnt); + dev->stats.mib_prev.rx_long_cnt = val; u64_stats_update_end(&dev->stats.syncp); -} - -static void airoha_update_hw_stats(struct airoha_gdm_dev *dev) -{ - struct airoha_gdm_port *port = dev->port; - int i; - - spin_lock(&port->stats_lock); - - for (i = 0; i < ARRAY_SIZE(port->devs); i++) { - if (port->devs[i]) - airoha_dev_get_hw_stats(port->devs[i]); - } - - /* Reset MIB counters */ - airoha_fe_set(dev->eth, REG_FE_GDM_MIB_CLEAR(port->id), - FE_GDM_MIB_RX_CLEAR_MASK | FE_GDM_MIB_TX_CLEAR_MASK); spin_unlock(&port->stats_lock); }
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index f6d01a8e8da1..3af1c49dd62d 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h@@ -245,6 +245,33 @@ struct airoha_hw_stats { u64 rx_fragment; u64 rx_jabber; u64 rx_len[7]; + + struct { + /* Previous HW register values for 32-bit counter delta + * tracking. Storing the last seen value and accumulating + * (u32)(curr - prev) into the 64-bit software counter + * handles wrap-around transparently via unsigned arithmetic. + * tx_runt_accum64/rx_runt_accum64 hold the running sum of + * runt deltas. These fields are never reported to userspace. + */ + u32 tx_drops; + u32 tx_broadcast; + u32 tx_multicast; + u32 tx_runt_cnt; + u32 tx_long_cnt; + u64 tx_runt_accum64; + u32 rx_drops; + u32 rx_broadcast; + u32 rx_multicast; + u32 rx_errors; + u32 rx_crc_error; + u32 rx_over_errors; + u32 rx_fragment; + u32 rx_jabber; + u32 rx_runt_cnt; + u32 rx_long_cnt; + u64 rx_runt_accum64; + } mib_prev; }; enum {
base-commit: a225f8c20712713406ae47024b8df42deacddd4a -- 2.43.0