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

[Taedcke, Christian]

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:
>> From: Christian Taedcke <christian.taedcke@xxxxxxxxxxxxxxx>
>>
>> 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?

> 
>> 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.

> 
>> Cc: stable@xxxxxxxxxxxxxxx
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Christian Taedcke <christian.taedcke@xxxxxxxxxxxxxxx>
>> ---
>>  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
> 
>>  	}
>>  
>>  	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