Thread (12 messages) 12 messages, 2 authors, 2d ago

Re: [PATCH net-next v9 2/3] net: airoha: fix ETS QoS stats counter underflow and cross-channel corruption

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2026-07-21 21:35:31
Also in: linux-mediatek, netdev

On 7/21/2026 2:27 AM, Lorenzo Bianconi wrote:
quoted
On 7/20/2026 3:03 PM, Lorenzo Bianconi wrote:
quoted
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().
This issue would only be a problem during rollover, which depending on
how fast the counts increment may not be a big problem. I could see this
not being worth going to net since it could be rare enough that it isn't
considered a widespread issue...
quoted
- 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.
However, this issue seems like its going to cause a problem every time
you read because any time you use a mix of channels you will get
corrupted values?
Hi Jacob,

I agree this is a real bug (and it needs to be fixed). However, the real
use-case is having a single channel per net_device (a single HTB offloaded
qdisc) and multiple hw queues (connected to the ETS offloaded classes).
In this scenario we do not trigger this issue.
quoted
This targets a commit which merged in v6.14, but the patch is part of a
series aimed at net-next. Could you explain why this shouldn't be
separated out and put as a fix in net? It seems pretty obvious that
users can easily reproduce problems by requesting stats from each
channel? Or is this not really possible to trigger from userspace until
patch 3/3?
For the reason described above and to avoid any possible conflicts with patch
3/3 I decided to add this patch here (adding the proper Fixes tag for the
backport) but if you prefer I can remove patch 2/3 from this series and send
it to net. What do you prefer?
If users can't trigger the issue easily in the normal use cases, and the
result is only bad stat values (and not actually corrupt memory etc), I
have no objections to waiting. The scope of the fix is fairly minor and
avoiding the conflicts seems like a reasonable goal.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Regards,
Lorenzo
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help