> The QDMA hardware raises a dedicated interrupt (NO_CPU_DSCP, one bit > per ring in QDMA_CSR_INT_ENABLE2/3) when an RX ring runs out of free > CPU descriptors. airoha_qdma_hw_init() already unmasks this interrupt > for every ring (INT_RX1_MASK()/INT_RX2_MASK() OR it together with the > RX_DONE bits before writing the enable register), but > airoha_irq_handler() only ever extracts the RX_DONE bits from the same > status word - the NO_CPU_DSCP bits are read and acknowledged (cleared) > along with everything else at the top of the handler, then silently > dropped. > > This matters because once a ring is genuinely drained to zero posted > descriptors, no further RX_DONE interrupt can fire for it: hardware > has nothing left to receive a frame into, so NAPI is never rescheduled > and airoha_qdma_fill_rx_queue() (which reposts descriptors) is never > called again. The ring is stuck until the interface is brought down > and back up. > > This is most visible on rings that carry low, bursty volumes of > protocol control traffic, in particular RX ring 4, to which > airoha_fe_vip_setup() force-routes ~15 unrelated VIP-classified > protocols (BOOTP, PPPoE Discovery, ISAKMP, DHCPv6, SIP, LLDP, PPP > LCP/IPCP/CHAP/PAP/IPv6CP, ...) via PATN_FCPU_EN_MASK, all sharing the > same RX_DSCP_NUM() default of 16 descriptors. A short burst on that > ring (e.g. a DHCP lease renewal exchange, or the LCP/IPCP/CHAP/PAP > negotiation that follows a PPPoE PADO) can drain it faster than the > CPU reposts descriptors, after which every one of those protocols > silently stops being received on that device until it is reconfigured > - with no error, warning, or netdev/ethtool counter indicating why. > > Fix airoha_irq_handler() to treat NO_CPU_DSCP the same as RX_DONE for > the purpose of scheduling NAPI: airoha_qdma_rx_process() already calls > airoha_qdma_fill_rx_queue() unconditionally at the end of every poll, > even when zero descriptors were reaped, so scheduling NAPI in response > to NO_CPU_DSCP is sufficient to make an emptied ring recover on its > own. airoha_qdma_rx_napi_poll() is updated to re-enable the > NO_CPU_DSCP bit alongside RX_DONE when napi_complete() runs, mirroring > the existing disable/enable dance so the interrupt isn't left masked > after its first use. > > One open question worth flagging explicitly: if the underlying > no-free-descriptor condition re-latches this bit immediately after the > ack write (rather than only on the next empty->non-empty transition), > a ring that airoha_qdma_fill_rx_queue() genuinely cannot repost into > (e.g. page_pool_dev_alloc_frag() returning NULL under memory pressure) > would turn this into a self-reasserting interrupt storm on the hard > IRQ path: mask -> napi_schedule() -> poll reaps 0, refills 0 -> > napi_complete() -> unmask -> NO_CPU_DSCP fires again immediately. I > don't have documentation confirming which behavior this bit actually > has. Regardless of the answer, masking NO_CPU_DSCP until a refill > actually succeeds - the natural-looking alternative - is worse: a > fully memory-starved ring can never fire RX_DONE either (nothing was > posted for hw to complete), so that would leave it permanently dead > once the memory pressure clears rather than self-healing. A bounded > storm tied to genuine memory pressure, if that's what this is, seems > preferable to a ring with no way back either way. Hi Vitaliy, I do not have info about it. Adding airoha folks in CC. > > Fixes: 23290c7bc190 ("net: airoha: Introduce Airoha NPU support") > Link: https://github.com/openwrt/openwrt/issues/24715 > Signed-off-by: Vitaliy Sochnev > --- > drivers/net/ethernet/airoha/airoha_eth.c | 28 +++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) thx for working on it. Just a couple of nit inline. Fixing them: Acked-by: Lorenzo Bianconi > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c > index 64619e9a704d..a3e5aaeb75b3 100644 > --- a/drivers/net/ethernet/airoha/airoha_eth.c > +++ b/drivers/net/ethernet/airoha/airoha_eth.c > @@ -784,13 +784,15 @@ static int airoha_qdma_rx_napi_poll(struct napi_struct *napi, int budget) > int i, qid = q - &qdma->q_rx[0]; > int intr_reg = qid < RX_DONE_HIGH_OFFSET ? QDMA_INT_REG_IDX1 > : QDMA_INT_REG_IDX2; > + u32 bit = qid % RX_DONE_HIGH_OFFSET; nit: what about doing something like: u32 irq_id = qid % RX_DONE_HIGH_OFFSET; u32 intr_mask = BIT(irq_id) | BIT(irq_id + RX_NO_CPU_DSCP_LOW_OFFSET); > > for (i = 0; i < ARRAY_SIZE(qdma->irq_banks); i++) { > if (!(BIT(qid) & RX_IRQ_BANK_PIN_MASK(i))) > continue; > > airoha_qdma_irq_enable(&qdma->irq_banks[i], intr_reg, > - BIT(qid % RX_DONE_HIGH_OFFSET)); > + BIT(bit) | > + BIT(bit + RX_NO_CPU_DSCP_LOW_OFFSET)); > } > } > > @@ -1468,16 +1470,32 @@ static irqreturn_t airoha_irq_handler(int irq, void *dev_instance) > if (!test_bit(DEV_STATE_INITIALIZED, &qdma->eth->state)) > return IRQ_NONE; > > - rx_intr1 = intr[1] & RX_DONE_LOW_INT_MASK; > + /* A ring can also raise NO_CPU_DSCP when it runs out of free RX > + * descriptors (e.g. a burst of VIP-classified control traffic > + * forced onto a small ring). Once a ring is fully drained no more > + * RX_DONE interrupts can fire for it, since there are no free > + * descriptors left for hardware to receive into, so without this > + * NAPI is never rescheduled and the ring never gets refilled again. > + * Treat NO_CPU_DSCP the same as RX_DONE for scheduling NAPI: > + * airoha_qdma_rx_process() unconditionally calls > + * airoha_qdma_fill_rx_queue() at the end of every poll, even when > + * zero descriptors were reaped, so this alone is enough to recover > + * the ring. > + */ I guess this comment (AI driven??) should go into the commit message, it seems too verbose for the code and it is quite obvious what you want to do. > + rx_intr1 = intr[1] & (RX_DONE_LOW_INT_MASK | RX_NO_CPU_DSCP_LOW_INT_MASK); > if (rx_intr1) { > airoha_qdma_irq_disable(irq_bank, QDMA_INT_REG_IDX1, rx_intr1); > - rx_intr_mask |= rx_intr1; > + rx_intr_mask |= (rx_intr1 & RX_DONE_LOW_INT_MASK) | > + ((rx_intr1 & RX_NO_CPU_DSCP_LOW_INT_MASK) >> > + RX_NO_CPU_DSCP_LOW_OFFSET); What about defining a macro in airoha_regs.h #define RX_NO_CPU_DSCP_INT_RX1_MASK(_n) (((_n) & RX_NO_CPU_DSCP_LOW_INT_MASK) >>\ RX_NO_CPU_DSCP_LOW_OFFSET) > } > > - rx_intr2 = intr[2] & RX_DONE_HIGH_INT_MASK; > + rx_intr2 = intr[2] & (RX_DONE_HIGH_INT_MASK | RX_NO_CPU_DSCP_HIGH_INT_MASK); > if (rx_intr2) { > airoha_qdma_irq_disable(irq_bank, QDMA_INT_REG_IDX2, rx_intr2); > - rx_intr_mask |= (rx_intr2 << 16); > + rx_intr_mask |= ((rx_intr2 & RX_DONE_HIGH_INT_MASK) | > + ((rx_intr2 & RX_NO_CPU_DSCP_HIGH_INT_MASK) >> > + RX_NO_CPU_DSCP_LOW_OFFSET)) << 16; I guess here you do not need to shift RX_NO_CPU_DSCP_HIGH_INT_MASK mask since it is already in the range [31:16], just mask like: (rx_intr2 & RX_NO_CPU_DSCP_HIGH_INT_MASK) > } > > for (i = 0; rx_intr_mask && i < ARRAY_SIZE(qdma->q_rx); i++) { > -- > 2.55.0 >