[PATCH net v3 0/2] net: airoha: fix silent RX loss on the shared CPU ring

Vitaliy Sochnev posted 2 patches 3 weeks, 3 days ago
drivers/net/ethernet/airoha/airoha_eth.c  | 16 +++++++++++-----
drivers/net/ethernet/airoha/airoha_eth.h  |  3 ++-
drivers/net/ethernet/airoha/airoha_regs.h |  2 ++
3 files changed, 15 insertions(+), 6 deletions(-)
[PATCH net v3 0/2] net: airoha: fix silent RX loss on the shared CPU ring
Posted by Vitaliy Sochnev 3 weeks, 3 days ago
v3: dropped the RX ring stall recovery (v2 2/3) as asked [1]. At 128 the
stall does not occur - 500 forced PPPoE reconnects over 20 h with the
detector compiled in and armed, zero triggers - and it did not fix the
bug on its own anyway. I will resend it if the stall turns up at the
larger ring.

2/2 keeps its code and its Acked-by; the commit message changed. It now
cites the register capture taken with no recovery in the tree instead of
numbers from builds carrying it, states that 1/2 does not cover this
failure, and quantifies what the bigger rings cost in memory.

v2 [2] answered the v1 review: DONE-bit overwrite hypothesis disproven,
QDMA_DESC_DROP_MASK never set, RX_DSCP_NUM default raised to the vendor
SDK's 32.

Still open for the airoha folks: in 2/2 hw set DONE on descriptor 15
while 0-14 were untouched and the driver's consumer sat at 0. Is
out-of-order completion within an RX ring expected, or is the driver
violating a constraint on RX_CPU_IDX by leaving one descriptor unposted?
Growing the ring avoids the symptom; the rule behind it is still unknown.

Tested on Nokia XG-040G-MF (AN7583) on a live PPPoE line. These two
patches without the recovery are what ran longest here: 508 forced
reconnects over 20 h 32 min, zero rx_dropped/rx_errors across 39158
samples.

[1] https://lore.kernel.org/netdev/apaA5jDYH71F0JaS@lore-desk/
[2] https://lore.kernel.org/netdev/20260831234701.206021-1-sochnev.v.74@gmail.com/

Vitaliy Sochnev (2):
  net: airoha: handle RX_NO_CPU_DSCP interrupt, not just RX_DONE
  net: airoha: grow the small RX rings

 drivers/net/ethernet/airoha/airoha_eth.c  | 16 +++++++++++-----
 drivers/net/ethernet/airoha/airoha_eth.h  |  3 ++-
 drivers/net/ethernet/airoha/airoha_regs.h |  2 ++
 3 files changed, 15 insertions(+), 6 deletions(-)

-- 
2.55.0
net: airoha: RX rings below 32 descriptors let hw DMA past the ring
Posted by Vitaliy Sochnev 2 weeks, 6 days ago
e84b89f17a12 ("net: airoha: grow the small RX rings") is queued for
net-next as an RX loss fix. It also stops hw from writing descriptors past
the end of the ring into unrelated kernel memory, which its commit message
does not mention. Given that, it may be worth stable.

Nokia XG-040G-MF (AN7583), 512 MiB, 6.18.44, OpenWrt snapshot. Images
below differ only in RX_DSCP_NUM(); no other patches, no instrumentation,
verified against vmlinux. RX_DSCP_NUM() is shared, so other variants with
a small ring are presumably affected too - not tested here.

Reproducer: raw UDP frames with source port 67, so REG_FE_VIP_PATN(8)
forces them to ring 4, ~900 pps to the board's MAC. With ring 4 at 16 the
box panics within a minute.

Mechanism
---------

At 16 descriptors ring 4 occupies 512 bytes; dma_alloc_coherent() rounds
to a page. After the ring stopped advancing, the remaining 3584 bytes of
that page contained 560 non-zero words repeating with a 32-byte period -
sizeof(struct airoha_qdma_desc):

  +4  ctrl  0xC0000000   QDMA_DESC_DONE_MASK | QDMA_DESC_DROP_MASK, len 0
  +16 msg0  0x00008000
  +20 msg1  0x2A5E0000
  +24 msg2  0x007F000E
  +28 msg3  0x0000FFFF

msg1 equals the value in the last descriptor the driver did see, so these
come from the same engine. REG_RX_RING_SIZE(4) reads 0x00020010 - the size
field is programmed correctly as 16. Writes staying inside the page are
invisible; past it they hit whatever follows.

Panics
------

Three on the 16-descriptor build, all garbage pointers in subsystems
unrelated to networking:

  __queue_work+0xa4 <- dbs_irq_work (cpufreq), x21 = 2d9ce5fd003cae80
  sched_balance_rq+0x84 <- sched_balance_domains, addr 0040000034124819
  Kernel panic - not syncing: corrupted stack end detected inside scheduler

The third occurred with no synthetic load: ordinary DHCP traffic after a
network restart, 4 minutes in.

Ring size is the only variable
------------------------------

  ring   frames fed   page tail after run   panic
  16     27 000       560 words, signature  yes, 3x
  32     271 909      0 of 768              no
  128    269 845      0 of 1024             no

Controls: the same scan on idle 32-descriptor rings reads 0 of 768, so the
560 words are not pre-existing content; 273 292 frames of identical traffic
on a non-VIP source port (ring 0) caused no panic.

On the shipped configuration (ring 4 = 128, default 32) the board also
completed 1623 consecutive PPPoE dial-ups, 16 VIP frames each, and ran
9 h 52 min of DHCP with renewals every minute - 1178 samples, no lease
loss, no rx errors, no panic.

The boundary is between 16 and 32. The vendor SDK default is 32, which
looks like a hardware minimum rather than a tuning choice.

Question for airoha
-------------------

Is 32 the minimum RX ring size the QDMA accepts? If so the driver should
clamp or reject smaller values rather than depend on RX_DSCP_NUM() being
large enough.

Withdrawing the NO_CPU_DSCP patch
---------------------------------

Please drop

  [PATCH net v3 1/2] net: airoha: handle RX_NO_CPU_DSCP interrupt

acked by Lorenzo, not applied. Its rationale - ring drains to zero, the
dropped NO_CPU_DSCP interrupt leaves it dead - is contradicted by
measurement:

  - NO_CPU_DSCP never fires. Instrumented builds logged zero events across
    every run; REG_INT_ENABLE(bank0,1) reads 0x839F839F, so the bit is
    unmasked for ring 4.
  - page_pool_dev_alloc_frag() never failed and q->queued never reached
    zero, so the described path is unreachable.
  - A/B images with and without the patch failed identically.

airoha_irq_handler() does drop the interrupt, so handling it may still be
correct, but not for the reason I gave.

Correction
----------

In the v3 cover letter I asked whether out-of-order completion pointed at
a constraint on RX_CPU_IDX, having seen QDMA_DESC_DONE_MASK set in the
slot fill_rx_queue() leaves unposted. airoha_qdma_rx_process() never
clears ctrl, so that bit is the residue of the last consumed frame.
Disregard it.
Re: net: airoha: RX rings below 32 descriptors let hw DMA past the ring
Posted by Vitaliy Sochnev 2 weeks, 6 days ago
Correction to the platform description, and a stronger dump.

I wrote that the images "differ only in RX_DSCP_NUM(); no other patches".
That was about the difference between my own images and is misleading as a
description of the driver. The tree is OpenWrt's, and it carries an
out-of-tree HW GRO patch that touches exactly the ring under test:

  AIROHA_RXQ_LRO_EN_MASK = GENMASK(7, 0)   -> rings 0-7, including ring 4

For an LRO ring that patch sets buf_size to 16 KiB instead of PAGE_SIZE/2
and clears RX_RING_SG_EN_MASK, which mainline always sets. Both are
plausibly relevant to a DMA overrun, so the result needed rechecking with
that removed.

To be precise about the base: it is 6.18.44 with the airoha RX path
backported from mainline, including 269389ba5398 ("Set REG_RX_CPU_IDX()
once in airoha_qdma_fill_rx_queue()") and bbfb1983944f ("Reserve RX
headroom to avoid skb reallocation"), both in net today. The GRO patch is
the only out-of-tree piece touching this path.

Rechecked with its LRO mask zeroed, which restores page order 0,
buf_size = PAGE_SIZE/2 and RX_RING_SG_EN - confirmed on the board by the
posted buffer length dropping from 0x3E80 to 0x680. Everything
reproduces:

  ring   LRO   panics                        overrun signature
  16     on    3 (one with no load at all)   present, 560 words
  16     off   2 (one with no load at all)   present, see below
  32     on    none                          0 of 768
  32     off   none, 268 018 frames          0 of 768
  128    on    none                          0 of 1024
  128    off   none, 273 921 frames          0 of 1024

The clean dump, ring 4 = 16, LRO off, taken while the ring was stalled.
Descriptor 16 does not exist in a 16-entry ring:

  desc 15 (last real)      desc 16 (past the end)
  +4  ctrl 0x80000156      +4  ctrl 0xC0000000   DONE|DROP, len 0
      DONE, len 342        +8  addr 0x00000000   no buffer posted
  +20 msg1 0x2A5E0000      +16 msg0 0x00008000
                           +20 msg1 0x2A5E0000
                           +24 msg2 0x007F000E
                           +28 msg3 0x0000FFFF

msg0-msg3 are bit-identical to the dump taken with LRO on, so it is the
same engine either way. addr = 0 explains DROP: hw ran past the posted
descriptors, found no buffer in the next slot - a slot that is not part of
the ring - and marked the completion dropped, but wrote the structure
anyway.

The panic in the ring 4 = 16, LRO off run landed in yet another place:

  nf_conntrack_hash_check_insert+0x480 [nf_conntrack]
  nf_conntrack_in / nf_hook_slow / __ip_local_out / udp_send_skb
  Comm: ntpd

Five panics so far, in four distinct places, none of them networking:
cpufreq's deferred work (__queue_work), the scheduler's load balancer
(sched_balance_rq), the scheduler's stack-end check (twice), and
conntrack's hash insert.

Nothing else in the original mail changes.