[PATCH RFC net 0/6] net: macb: fix TX stalls, IRQ storms and a lost NCR update on link-up

[PATCH RFC net 0/6] net: macb: fix TX stalls, IRQ storms and a lost NCR update on link-up

[Steffen Dirkwinkel]

Hello Théo, Christian, all,

I've been looking at RCU and TX stalls on macb devices (Xilinx ZynqMP on
Beckhoff CX9240) for a while and have been carrying downstream patches
similar to Christian's. A while ago I let Claude try to reproduce this on my
devices, it was able to reproduce and found some fixes but I wasn't
happy with the quality of the patches and my understanding of them. I
briefly talked to Théo in Prague and he suggested I send what I have as
either the patches or the reproducers may be useful for ideas.
I've finally gotten around to it and also ran some more testing and improved
the patches in the meantime. The series includes Christian's patches in the
original form (should also at least be reworded) and I also have Patch 1 in here
since the bench seems to have reproduced the issue on hardware (see below).

If anyone has other ideas / patches for these issues I'm also happy to
run tests on devices.

Claude output from here:

This series fixes the TX stalls, TXUBR interrupt storms and RCU stalls we
reproduce on ZynqMP (GEM rev 0x50070106, two TX queues, PREEMPT_RT).
Patch 1 is Théo's NCR locking patch [1], unchanged; patches 2 and 3 are
Christian's v1 [2], unchanged; patches 4 to 6 are ours. Patch 6 is most
likely what Théo reported in [3].

Reproducers (all on the same ZynqMP board, RT config, IRQs on CPU 0,
rcu_cpu_stall_timeout=8, iperf3 -P10 against a host on the same link):

 A. the link speed is toggled between 100 and 1000 Mbit/s every ~12 s
    with "ethtool -s end0 speed {100,1000} duplex full autoneg on" while
    the load runs; after every link-up the transmit counter, the netdev
    watchdog counters and the interrupt rate are checked. A storm is
    left running so the RCU stall detector can report it.
 B. a managed switch cycles the DUT's two ports (one port only, the other
    port only at 10 Mbit/s, back at autoneg), ~140 s per cycle; a hit is
    an RCU stall on the serial console.

Results, vanilla net/main 78731b46967e, PREEMPT_RT:

  driver                       A: failures/toggles          B: hits/cycles
  net/main                     watchdog at link-up 2,       1 TX stall episode
                               RCU stall at link-up 3       in 74 cycles
                               (idle mode: stall at 3)
  patches 2-6                  0/300 (idle mode 0/50)       0/74 (with load)

Same on a v7.2.7-rt tree with the net/main driver files (same bench):

  net/main                     8/9 toggles failed, RCU stall after 9 link-ups
  patches 2-6                  1/300 (one dead transmitter), 0 stalls in ~1300

Per patch, one patch removed at a time from the series (method A):

  patch 2 (TBQP) alone off        0/100; TBQP reads the stale pre-link-down
                                  index in 176/295 link-ups without the write,
                                  0/555 with it (trace)
  patch 3 (TXUBR mask) alone off  0/100
  patches 2+3 off                 RCU stall at toggle 85 (1/153)
  patches 2+3+4 off               RCU stall at toggle 42 (1/41, p = 0.03)
  patch 4 (rotate) alone off      1/97: TBQP stopped at index 0 on the used
                                  descriptor of an unreaped frame, watchdog
  patch 5 (kick) alone off        8/11 toggles: watchdogs, p = 7e-13
  patch 6 (queue 0) alone off     1/100 watchdog; per-stream share at
                                  100 Mbit/s 6.9 to 11.7 Mbit/s over both
                                  queues vs 9.3 Mbit/s x10 with it
  patches 5+6 off (2-4 only)      8/16 toggles: watchdogs on queue 0

So patches 2, 3 and 4 together prevent the storm and the RCU stall, patch
5 fixes the TX stall after link-up, patch 6 fixes the strict-priority
unfairness (no starvation-induced stall reproduced on this device).

Patch 1: once in ~300 toggles the transmitter stayed dead after a link-up
with patches 2-6. Tracing every NCR writer showed the cause:
macb_mac_link_down() clears RE|TE with an unlocked read-modify-write,
and macb_tx_restart() in the TX NAPI poll, which had read NCR with TE set
just before, writes its image back with TSTART 40 us to 900 us later. TE
is set again while the link is down, the next link-up starts with TE set,
the TBQP write of patch 2 is dropped and the ring is rotated under a
running transmitter. With the read-to-write gap of the TSTART path
widened to 1 ms to amplify the race: without patch 1, 3 dead transmitters
and an RCU stall in 792 toggles, 6 lost updates in 408 traced link-downs;
with patch 1, 0 in 1196 toggles (p = 0.025) and 0 lost updates in 1206
link-downs. The
traces (link_down clear, the stale write from irq/22-eth%d, TE set at
the next link-up) are available on request.

Answering the review of [2]: with the rest of the series in place,
removing patch 2 or patch 3 alone gives 0/100 each, removing both brings
the RCU stall back (1 in 153 toggles), removing 2, 3 and 4 gives 1 in 41;
TBQP of queue 0 keeps its
value while TE is low in about half of the link-ups on this GEM, as in
Christian's trace, while the fetch still starts at the base after TSTART.
Both reproducers were run on vanilla net/main; only PREEMPT_RT was tested.

Théo's steady-state case ("iperf3 -P10" for 30 min without transitions)
gave no watchdog on this device with or without the series.

[1] https://lore.kernel.org/netdev/20260918-macb-ncr-rmw-v1-1-30c3494f93cd@bootlin.com/
[2] https://lore.kernel.org/netdev/20260706-upstreaming-macb-irq-storm-v1-1-ab3115b5a13a@weidmueller.com/
[3] https://lore.kernel.org/netdev/DLIJN4CDS11K.RVJGK3UALMJE@bootlin.com/

---
Christian Taedcke (2):
      net: macb: reprogram TBQP after shuffling the TX ring on link-up
      net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

Steffen Dirkwinkel (3):
      net: macb: rotate unreaped TX frames behind the ring base on link-up
      net: macb: kick the transmitter on link-up
      net: macb: keep best-effort traffic on TX queue 0

Théo Lebrun (1):
      net: macb: take bp->lock around NCR read-modify-writes

 drivers/net/ethernet/cadence/macb_main.c | 80 ++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 13 deletions(-)
---
base-commit: 78731b46967e3e7102b5cd5062bef185f3404b5c
change-id: 20261022-std-macb-v14-send-6c5fd939b4e1

Best regards,
--  
Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

[PATCH RFC net 1/6] net: macb: take bp->lock around NCR read-modify-writes

[Steffen Dirkwinkel]

From: Théo Lebrun <theo.lebrun@bootlin.com>

NCR is read-modify-written from many contexts:

 - macb_mac_link_down() clears RE|TE,
 - macb_mac_link_up() sets RE|TE|PTPUNI,
 - macb_hresp_error_task() clears then re-sets RE|TE,
 - macb_start_xmit() / macb_tx_restart() / macb_tx_error_task() set
   TSTART (already under bp->lock),
 - macb_interrupt() might toggle RE (also under bp->lock).

The first three risk concurrent RMW with anyone from the list as they
don't grab bp->lock.

Theoretical bugfix only, it has never reproduced on hardware. Also note
it does *not* take the lock hoping to protect against full races
inbetween BH scheduling concurrently. We only want to ensure consistent
NCR RMW operations.

Fixes: 7897b071ac3b ("net: macb: convert to phylink")
Cc: stable@vger.kernel.org
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 261a7e87520a..4b5037867537 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -777,6 +777,7 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
 	struct net_device *netdev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(netdev);
 	struct macb_queue *queue;
+	unsigned long flags;
 	unsigned int q;
 	u32 ctrl;
 
@@ -786,8 +787,10 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
 				     bp->rx_intr_mask | MACB_TX_INT_FLAGS | MACB_BIT(HRESP));
 
 	/* Disable Rx and Tx */
+	spin_lock_irqsave(&bp->lock, flags);
 	ctrl = macb_readl(bp, NCR) & ~(MACB_BIT(RE) | MACB_BIT(TE));
 	macb_writel(bp, NCR, ctrl);
+	spin_unlock_irqrestore(&bp->lock, flags);
 
 	netif_tx_stop_all_queues(netdev);
 }
@@ -940,11 +943,13 @@ static void macb_mac_link_up(struct phylink_config *config,
 	}
 
 	/* Enable Rx and Tx; Enable PTP unicast */
+	spin_lock_irqsave(&bp->lock, flags);
 	ctrl = macb_readl(bp, NCR);
 	if (gem_has_ptp(bp))
 		ctrl |= MACB_BIT(PTPUNI);
 
 	macb_writel(bp, NCR, ctrl | MACB_BIT(RE) | MACB_BIT(TE));
+	spin_unlock_irqrestore(&bp->lock, flags);
 
 	netif_tx_wake_all_queues(netdev);
 }
@@ -1995,6 +2000,7 @@ static void macb_hresp_error_task(struct work_struct *work)
 	struct macb *bp = from_work(bp, work, hresp_err_bh_work);
 	struct net_device *netdev = bp->netdev;
 	struct macb_queue *queue;
+	unsigned long flags;
 	unsigned int q;
 	u32 ctrl;
 
@@ -2003,9 +2009,11 @@ static void macb_hresp_error_task(struct work_struct *work)
 					 MACB_TX_INT_FLAGS |
 					 MACB_BIT(HRESP));
 	}
+	spin_lock_irqsave(&bp->lock, flags);
 	ctrl = macb_readl(bp, NCR);
 	ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
 	macb_writel(bp, NCR, ctrl);
+	spin_unlock_irqrestore(&bp->lock, flags);
 
 	netif_tx_stop_all_queues(netdev);
 	netif_carrier_off(netdev);
@@ -2022,8 +2030,10 @@ static void macb_hresp_error_task(struct work_struct *work)
 			     MACB_TX_INT_FLAGS |
 			     MACB_BIT(HRESP));
 
-	ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
-	macb_writel(bp, NCR, ctrl);
+	spin_lock_irqsave(&bp->lock, flags);
+	ctrl = MACB_BIT(RE) | MACB_BIT(TE);
+	macb_writel(bp, NCR, macb_readl(bp, NCR) | ctrl);
+	spin_unlock_irqrestore(&bp->lock, flags);
 
 	netif_carrier_on(netdev);
 	netif_tx_start_all_queues(netdev);

-- 
2.56.0

[PATCH RFC net 2/6] net: macb: reprogram TBQP after shuffling the TX ring on link-up

[Steffen Dirkwinkel]

From: Christian Taedcke <christian.taedcke@weidmueller.com>

gem_shuffle_tx_one_ring() rotates the software TX ring so that the
tail sits at index 0 and resets queue->tx_tail to 0, but it never
reprograms the hardware transmit buffer queue pointer (TBQP). Other
paths that reset tx_tail to the ring base (macb_init_buffers() and
macb_tx_error_task()) also reprogram TBQP to queue->tx_ring_dma; this
path does not, leaving TBQP pointing at a stale descriptor.

gem_shuffle_tx_rings() runs on every link-up from
macb_mac_link_up(). After a few link up/down flaps that leave
un-completed descriptors in the ring, the stale TBQP keeps pointing at
a descriptor whose used bit is set. When TX is re-enabled on link-up,
the GEM reads that used descriptor and raises TXUBR. macb_interrupt()
schedules the TX NAPI, macb_tx_poll() makes no progress (work_done ==
0) and macb_tx_restart() re-issues TSTART, which makes the controller
read the same used descriptor again and re-assert TXUBR. As the MAC
interrupt is level-triggered, it never deasserts and one CPU is pegged
at 100% in the threaded handler, eventually triggering "sched: RT
throttling activated" and a dead network interface.

Fix it by reprogramming TBQP to the ring base on every path of
gem_shuffle_tx_one_ring() that resets tx_tail to 0, mirroring
macb_tx_error_task(). The early return for an already-aligned tail is
left untouched as TBQP is already consistent there. This is safe
because the shuffle runs from macb_mac_link_up() while TE is still
disabled, so the transmitter is halted.

Fixes: 881a0263d502 ("net: macb: Shuffle the tx ring before enabling tx")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4b5037867537..97494933eb1e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -823,7 +823,7 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 	if (!count) {
 		queue->tx_head = 0;
 		queue->tx_tail = 0;
-		goto unlock;
+		goto reset_hw_ptr;
 	}
 
 	shift = tail % ring_size;
@@ -872,6 +872,13 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 	/* Make descriptor updates visible to hardware */
 	wmb();
 
+reset_hw_ptr:
+	/* tx_tail was reset to the ring base, so TBQP must be reprogrammed
+	 * to match; otherwise it keeps pointing at a stale descriptor. Safe
+	 * to write directly here as TX is still disabled (called from
+	 * macb_mac_link_up() before TE is set).
+	 */
+	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
 unlock:
 	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
 }

-- 
2.56.0

[PATCH RFC net 3/6] net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

[Steffen Dirkwinkel]

From: Christian Taedcke <christian.taedcke@weidmueller.com>

macb_interrupt() defers TX completion handling to NAPI, but when it
schedules the poll it only masks TCOMP, even though TXUBR is enabled
alongside it (both are part of MACB_TX_INT_FLAGS). macb_tx_poll() is
asymmetric in the same way and only re-enables TCOMP. TXUBR is thus
left unmasked while responsibility for handling it has been deferred
to NAPI.

Unlike an edge event, TXUBR is a persistent condition: the controller
keeps it asserted for as long as the transmitter reads a buffer
descriptor whose used bit is set. Leaving a level-triggered source
enabled while NAPI owns its processing means the interrupt refires
immediately after the handler returns, before the poll has had a
chance to clear the underlying condition. This turns into a hard
interrupt storm that pegs a CPU in the (threaded) MAC IRQ handler and,
on PREEMPT_RT, triggers RT throttling ("sched: RT throttling
activated"), taking the network interface down.

Several situations can keep the used-bit read asserted across a poll -
for example unreaped completed descriptors still sitting at tx_tail,
or a transmit restart racing with macb_start_xmit(). The specific
trigger does not matter: as long as the source stays unmasked, any
persistent assertion is enough to storm, so the interrupt handling
itself must be made self-limiting.

Mask TXUBR together with TCOMP in the IDR write when scheduling the TX
NAPI, and re-enable both from the napi_complete path in
macb_tx_poll(), making the TX interrupt mask/unmask symmetric and
consistent with how the driver already treats every other
NAPI-serviced source. The pending TXUBR is still recorded in
queue->txubr_pending before masking and acted on by macb_tx_restart(),
so no event is lost. A persistent TXUBR now degrades to NAPI-paced
polling instead of a CPU-pegging hard interrupt storm.

Fixes: 138badbc21a0 ("net: macb: use NAPI for TX completion path")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 97494933eb1e..14c8299cb7f3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1979,7 +1979,7 @@ static int macb_tx_poll(struct napi_struct *napi, int budget)
 		    (unsigned int)(queue - bp->queues), work_done, budget);
 
 	if (work_done < budget && napi_complete_done(napi, work_done)) {
-		queue_writel(queue, IER, MACB_BIT(TCOMP));
+		queue_writel(queue, IER, MACB_BIT(TCOMP) | MACB_BIT(TXUBR));
 
 		/* Packet completions only seem to propagate to raise
 		 * interrupts when interrupts are enabled at the time, so if
@@ -2174,7 +2174,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 
 		if (status & (MACB_BIT(TCOMP) |
 			      MACB_BIT(TXUBR))) {
-			queue_writel(queue, IDR, MACB_BIT(TCOMP));
+			queue_writel(queue, IDR, MACB_BIT(TCOMP) |
+						 MACB_BIT(TXUBR));
 			macb_queue_isr_clear(bp, queue, MACB_BIT(TCOMP) |
 							MACB_BIT(TXUBR));
 			if (status & MACB_BIT(TXUBR)) {

-- 
2.56.0

[PATCH RFC net 4/6] net: macb: rotate unreaped TX frames behind the ring base on link-up

[Steffen Dirkwinkel]

From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

Start the link-up rotation in gem_shuffle_tx_one_ring() at the first
frame that still has to be sent instead of at tx_tail, and schedule the
TX NAPI poll once transmit is enabled again.

macb_mac_link_down() masks TCOMP before disabling transmit. A frame
completed in that window keeps the used bit on its first descriptor and
is never reaped. The rotation moves it to index 0, where the controller
starts after link-up, reads the used bit and stops; macb_tx_restart()
then keeps writing TSTART onto the same descriptor, an interrupt storm
that on PREEMPT_RT ends in RCU stalls.

Keeping the completed frames behind the ring base leaves tx_tail on
them for NAPI to reap. Reaping in link-down instead would have to wait
for NAPI there, which deadlocks against macb_close() and the PM
callbacks disabling it.

Fixes: 881a0263d502 ("net: macb: Shuffle the tx ring before enabling tx")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 43 +++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 14c8299cb7f3..6e7ead23d0b7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -798,7 +798,7 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
 /* Use juggling algorithm to left rotate tx ring and tx skb array */
 static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 {
-	unsigned int head, tail, count, ring_size, desc_size;
+	unsigned int head, tail, count, ring_size, desc_size, start, done;
 	struct macb_tx_skb tx_skb, *skb_curr, *skb_next;
 	struct macb_dma_desc *desc_curr, *desc_next;
 	unsigned int i, cycles, shift, curr, next;
@@ -817,8 +817,19 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 	ring_size = bp->tx_ring_size;
 	count = CIRC_CNT(head, tail, ring_size);
 
-	if (!(tail % ring_size))
-		goto unlock;
+	/* Frames completed while link-down had TCOMP masked are still
+	 * unreaped and their used bit would stop the controller at the ring
+	 * base, so rotate the first frame still to be sent there instead.
+	 */
+	rmb();
+	for (start = tail; start != head &&
+	     (macb_tx_desc(queue, start)->ctrl & MACB_BIT(TX_USED)); start++)
+		while (start + 1 != head && !macb_tx_skb(queue, start)->skb)
+			start++;
+	done = start - tail;
+
+	if (!(start % ring_size))
+		goto reset_hw_ptr;
 
 	if (!count) {
 		queue->tx_head = 0;
@@ -826,7 +837,7 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 		goto reset_hw_ptr;
 	}
 
-	shift = tail % ring_size;
+	shift = start % ring_size;
 	cycles = gcd(ring_size, shift);
 
 	for (i = 0; i < cycles; i++) {
@@ -866,24 +877,23 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 		       sizeof(struct macb_tx_skb));
 	}
 
-	queue->tx_head = count;
-	queue->tx_tail = 0;
+	queue->tx_tail = done ? ring_size - done : 0;
+	queue->tx_head = queue->tx_tail + count;
 
 	/* Make descriptor updates visible to hardware */
 	wmb();
 
 reset_hw_ptr:
-	/* tx_tail was reset to the ring base, so TBQP must be reprogrammed
-	 * to match; otherwise it keeps pointing at a stale descriptor. Safe
-	 * to write directly here as TX is still disabled (called from
-	 * macb_mac_link_up() before TE is set).
+	/* The first frame to be sent is now at the ring base, so TBQP must
+	 * be reprogrammed to match; otherwise it keeps pointing at a stale
+	 * descriptor. Safe to write directly here as TX is still disabled
+	 * (called from macb_mac_link_up() before TE is set).
 	 */
 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
-unlock:
 	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
 }
 
-/* Rotate the queue so that the tail is at index 0 */
+/* Rotate the queue so that the first frame to be sent is at index 0 */
 static void gem_shuffle_tx_rings(struct macb *bp)
 {
 	struct macb_queue *queue;
@@ -958,6 +968,15 @@ static void macb_mac_link_up(struct phylink_config *config,
 	macb_writel(bp, NCR, ctrl | MACB_BIT(RE) | MACB_BIT(TE));
 	spin_unlock_irqrestore(&bp->lock, flags);
 
+	if (!(bp->caps & MACB_CAPS_MACB_IS_EMAC)) {
+		/* Reap the frames completed while link-down had TCOMP masked */
+		local_bh_disable();
+		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
+			if (READ_ONCE(queue->tx_tail) != READ_ONCE(queue->tx_head))
+				napi_schedule(&queue->napi_tx);
+		local_bh_enable();
+	}
+
 	netif_tx_wake_all_queues(netdev);
 }
 

-- 
2.56.0

[PATCH RFC net 5/6] net: macb: kick the transmitter on link-up

[Steffen Dirkwinkel]

From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

Write TSTART in macb_mac_link_up() for every queue that still has
frames outstanding once transmit has been re-enabled.

Frames queued before a link-down stay in the ring, and setting NCR.TE
does not start the transmitter; the driver relies on the next
macb_start_xmit() to write TSTART. If BQL stopped the queue before the
link went down, that transmit never comes and the queue stays idle
until the netdev watchdog fires.

Like the rotation, the kick is skipped on the at91rm9200 EMAC, which
uses its own TX path.

Fixes: 881a0263d502 ("net: macb: Shuffle the tx ring before enabling tx")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6e7ead23d0b7..3dfc7ffdd598 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -903,6 +903,8 @@ static void gem_shuffle_tx_rings(struct macb *bp)
 		gem_shuffle_tx_one_ring(queue);
 }
 
+static void macb_tx_restart(struct macb_queue *queue);
+
 static void macb_mac_link_up(struct phylink_config *config,
 			     struct phy_device *phydev,
 			     unsigned int mode, phy_interface_t interface,
@@ -969,6 +971,10 @@ static void macb_mac_link_up(struct phylink_config *config,
 	spin_unlock_irqrestore(&bp->lock, flags);
 
 	if (!(bp->caps & MACB_CAPS_MACB_IS_EMAC)) {
+		/* With BQL holding a queue stopped no xmit would issue TSTART */
+		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
+			macb_tx_restart(queue);
+
 		/* Reap the frames completed while link-down had TCOMP masked */
 		local_bh_disable();
 		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)

-- 
2.56.0

[PATCH RFC net 6/6] net: macb: keep best-effort traffic on TX queue 0

[Steffen Dirkwinkel]

From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

Add an ndo_select_queue() that puts every frame on queue 0 unless
traffic classes are configured (taprio, or software mqprio).

The GEM serves its transmit queues in strict priority order, the
highest queue first, while the stack hashes flows over all of them. On
a saturated link the flows on the lower queues starve until the netdev
watchdog fires on them, and the timeout path cannot help as nothing is
wrong with the queue.

XPS settings, the socket queue cache and the queue choice of
PACKET_QDISC_BYPASS sockets no longer take effect on multi-queue GEMs
without traffic classes; queues 1 and up stay reachable through tc.

Fixes: 02c958dd3446 ("net/macb: add TX multiqueue support for gem")
Assisted-by: LLM
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 3dfc7ffdd598..eae43490c5f7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4596,10 +4596,21 @@ static void macb_tx_timeout(struct net_device *netdev, unsigned int q)
 	macb_tx_restart(&bp->queues[q]);
 }
 
+/* GEM serves TX queues in strict priority order, the highest queue first */
+static u16 macb_select_queue(struct net_device *netdev, struct sk_buff *skb,
+			     struct net_device *sb_dev)
+{
+	if (netdev_get_num_tc(netdev))
+		return netdev_pick_tx(netdev, skb, sb_dev);
+
+	return 0;
+}
+
 static const struct net_device_ops macb_netdev_ops = {
 	.ndo_open		= macb_open,
 	.ndo_stop		= macb_close,
 	.ndo_start_xmit		= macb_start_xmit,
+	.ndo_select_queue	= macb_select_queue,
 	.ndo_set_rx_mode	= macb_set_rx_mode,
 	.ndo_get_stats64	= macb_get_stats,
 	.ndo_eth_ioctl		= macb_ioctl,

-- 
2.56.0

Re: [PATCH RFC net 0/6] net: macb: fix TX stalls, IRQ storms and a lost NCR update on link-up

[Nicolai Buchwitz]

Hi Steffen

On 22.10.2026 11:38, Steffen Dirkwinkel wrote:
> Hello Théo, Christian, all,
> 
> I've been looking at RCU and TX stalls on macb devices (Xilinx ZynqMP 
> on
> Beckhoff CX9240) for a while and have been carrying downstream patches
> similar to Christian's. A while ago I let Claude try to reproduce this 
> on my
> devices, it was able to reproduce and found some fixes but I wasn't
> happy with the quality of the patches and my understanding of them. I
> briefly talked to Théo in Prague and he suggested I send what I have as
> either the patches or the reproducers may be useful for ideas.
> I've finally gotten around to it and also ran some more testing and 
> improved
> the patches in the meantime. The series includes Christian's patches in 
> the
> original form (should also at least be reworded) and I also have Patch 
> 1 in here
> since the bench seems to have reproduced the issue on hardware (see 
> below).
> 
> If anyone has other ideas / patches for these issues I'm also happy to
> run tests on devices.

Thanks for posting this RFC. I will go later through the details, but 
wanted to let you
know that I'm also investigating the macb watchdog issue on RPi RP1 [1].

Yesterday Théo also shared some findings in the thread.

Three things from that work which touch your series:

- Patch 3 changes the IER write in macb_tx_poll() that I need a readl 
after. The write
   is posted, a TCOMP landing between the ring check and the write 
reaching the MAC is
   lost. That seems to be the RP1 stall, masking TXUBR does not change 
it.
   The two patches need ordering.

- The watchdog only recovers a lost TSTART. With a missed completion 
TBQP == tx_head and
   macb_tx_restart() does nothing, the queue never comes back. I have a 
patch that
   schedules TX NAPI from macb_tx_timeout() when the tail descriptor has 
TX_USED set.

- RP1 has a single TX queue, so the starvation from patch 6 is not what 
we see.

I can run the series on an RP1 board with my reproducer if that helps.

[1] 
https://github.com/raspberrypi/linux/issues/7661#issuecomment-6322904343

> [...]

Thanks,
Nicolai