Thread (13 messages) 13 messages, 5 authors, 2d ago

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

From: Taedcke, Christian <hidden>
Date: 2026-07-07 13:36:29
Also in: linux-rt-devel, lkml, stable

Thank you for the quick review! This is my first Linux kernel
contribution, so I appreciate your feedback here.

On 7/6/2026 5:04 PM, Sebastian Andrzej Siewior wrote:
On 2026-07-06 16:02:14 [+0200], Christian Taedcke via B4 Relay wrote:
quoted
From: Christian Taedcke <redacted>

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.
But this should also happen with !RT at which point the interrupt runs
at 100% CPU and the softirq has hardly an chance to make progress, no?
Problably yes. I had issues reproducing the issue since it appeared only
on specific test setups when a lot packets where sent to another network
device and this device's power was cut. And even then on some test runs
the issue was not visible after a few hundred iterations. But after a
restart of the whole test setup (including cold reboot of all devices)
the issue sometimes appeared after 5 iterations.
I only metion RT here because it was the only thing i tested. I only ran
the RT kernel.
Should I change the description?
quoted
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")
This is v7.0-rc4. So that RT tree of yours has some backports or did you
run into this while trying to reproduce it upstream?
There were some backports. I ran this on the linux-yocto kernel
https://git.yoctoproject.org/linux-yocto branch
v6.6/standard/preempt-rt/base.
The "Fixes:" commit was backported as 0a47c3889fcd before their version
of 6.6.130.

The kernel i reproduced the issue on was linux-yocto branch
v6.6/standard/preempt-rt/base after 6.6.142 was merged into it.
quoted
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christian Taedcke <redacted>
---
 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 fd282a1700fb..b11cb8f068b7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -820,7 +820,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;
This update is even needed for count == 0 case? I kind of do understand
that you need to updated if you shuffled the descriptors around.
This was my understanding before researching more because of the email
from Kevin in this thread: count == 0 may happen anywhere within the ring
(e.g. when both the tail and the head point to the middle).
Resetting queue->tx_tail to 0 but not resetting TBQP results in them
being out-of-sync.
But as Kevin mentioned in his email TBQP is reset to the original
value when transmit is disabled (by setting bit 3 in NCR register).

I will investigate this further why my code change fixed the issue for
me, but according to the documentation in [1] it should be a no-op.

[1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm pg. 1040
quoted
 	}
 
 	shift = tail % ring_size;
@@ -869,6 +869,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);
 }
Sebastian
Christian
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help