[PATCH net 0/2] net: macb: fix TXUBR interrupt storm on link flapping

[PATCH net 0/2] net: macb: fix TXUBR interrupt storm on link flapping

[Christian Taedcke via B4 Relay]

We observed a hard interrupt storm in the Cadence GEM (macb) driver on
a Xilinx ZynqMP based platform running a PREEMPT_RT kernel (6.6.142).

After several Ethernet link up/down transitions, the CPU that the MAC
IRQ is pinned to is pegged at 100% in the threaded MAC interrupt
handler and the kernel reports "sched: RT throttling activated",
killing the network interface. The MAC ISR keeps refiring with the
level-triggered TXUBR (TX used bit read) set, because the transmitter
is left pointing at a descriptor whose used bit is set. See the
individual commit messages for the full analysis.

Patch 1 fixes the root cause: gem_shuffle_tx_one_ring() resets tx_tail to
the ring base on link-up but never reprograms the hardware TBQP pointer.

Patch 2 fixes a second, independent bug: macb_interrupt() masks only
TCOMP (not TXUBR) when scheduling the TX NAPI, and macb_tx_poll()
re-enables only TCOMP. Because TXUBR is level-triggered, a persistent
used-descriptor condition keeps it asserted and re-fires immediately,
storming the MAC interrupt.

Both patches are required: on the affected platform the interrupt storm
still reproduces with patch 1 applied alone, so patch 2 is needed as
well to stop it.

The relevant code is essentially identical in mainline, so the bug is not
RT-specific. PREEMPT_RT merely turns the storm into a fatal failure via
RT throttling.

Tested on ZynqMP with PREEMPT_RT by repeatedly flapping the Ethernet
link. The interrupt storm and RT throttling no longer occur.

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

 drivers/net/ethernet/cadence/macb_main.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260703-upstreaming-macb-irq-storm-ebd290b1e832

Best regards,
--  
Christian Taedcke <christian.taedcke@weidmueller.com>

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

[Christian Taedcke via B4 Relay]

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 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;
 	}
 
 	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);
 }

-- 
2.54.0

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

[Sebastian Andrzej Siewior]

On 2026-07-06 16:02:14 [+0200], Christian Taedcke via B4 Relay wrote:
> 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.

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?

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

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

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

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

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

[Sebastian Andrzej Siewior]

On 2026-07-07 15:36:24 [+0200], Taedcke, Christian wrote:
> Thank you for the quick review! This is my first Linux kernel
> contribution, so I appreciate your feedback here.

You are doing good.

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

It makes a difference if the problem you are facing is limited to
PREEMPT_RT (and so does not trigger on !PREEMPT_RT due to $REASON),
or also effects !PREEMPT_RT but may or may not trigger easily on
PREEMPT_RT.

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

It is usually good to reproduce the issue on vanilla ensuring that the
problem was not introduced by a backport or was solved differently
upstream.

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

I see.

> [1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm pg. 1040
> 
> Christian

Sebastian

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

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 3787 bytes --]

On Mon, Jul 06, 2026 at 04:02:14PM +0200, Christian Taedcke via B4 Relay wrote:
> 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 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;
>  	}
>  
>  	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).
> +	 */

Could you elaborate on why we need to reprogram the TBQP here? Based on my
understanding, the transmit-buffer queue pointer automatically resets to the
value of TBQP when TX is disabled. The following is quoted from the Zynq
UltraScale TRM [1]:
  While transmit is disabled, bit [3] of the network control is
  set Low, the transmit-buffer queue pointer resets to point to the address indicated by the
  transmit-buffer queue base address register. Disabling receive does not have the same
  effect on the receive-buffer queue pointer.

[1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm

Thanks,
Kevin

> +	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>  unlock:
>  	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
>  }
> 
> -- 
> 2.54.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Taedcke, Christian]

On 7/7/2026 11:13 AM, Kevin Hao wrote:
> On Mon, Jul 06, 2026 at 04:02:14PM +0200, Christian Taedcke via B4 Relay wrote:
>> 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 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;
>>  	}
>>  
>>  	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).
>> +	 */
> 
> Could you elaborate on why we need to reprogram the TBQP here? Based on my
> understanding, the transmit-buffer queue pointer automatically resets to the
> value of TBQP when TX is disabled. The following is quoted from the Zynq
> UltraScale TRM [1]:
>   While transmit is disabled, bit [3] of the network control is
>   set Low, the transmit-buffer queue pointer resets to point to the address indicated by the
>   transmit-buffer queue base address register. Disabling receive does not have the same
>   effect on the receive-buffer queue pointer.
> 
> [1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm

Thanks for the review and the TRM pointer.

I agree that the TRM says the transmit pointer is reset while TE is low. My
question is whether this describes an internal pointer being reloaded from TBQP,
or whether TBQP itself is restored to the original ring base.

I will instrument this on my board and check how TBQP behaves across the link
down/up path.

> 
> Thanks,
> Kevin
> 

Christian

>> +	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>>  unlock:
>>  	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
>>  }
>>
>> -- 
>> 2.54.0
>>
>>

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

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

> I agree that the TRM says the transmit pointer is reset while TE is low. My
> question is whether this describes an internal pointer being reloaded from TBQP,
> or whether TBQP itself is restored to the original ring base.

The Zynq UltraScale TRM [1] describes the receive-buffer queue pointer as follows:

  An internal counter represents the receive-buffer queue pointer and it is not
  visible through the CPU interface.

I could not find a similar description for the transmit-buffer queue pointer,
but I believe it behaves the same way. From a software perspective, it should
be safe to assume that the TBQP is reset to point to the start of the transmit
descriptor list upon reset. This assumption is supported by the description
of the transmit_q_ptr (GEM) Register [2]:

  Reading this register returns the location of the descriptor currently being accessed.
  Since the DMA handles two frames at once, this may not necessarily be pointing to the
  current frame being transmitted.

[1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm
[2] https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/transmit_q_ptr-GEM-Register

Thanks,
Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Théo Lebrun]

Hello Kevin & Christian,

On Wed Jul 8, 2026 at 5:05 AM CEST, Kevin Hao wrote:
>> I agree that the TRM says the transmit pointer is reset while TE is low. My
>> question is whether this describes an internal pointer being reloaded from TBQP,
>> or whether TBQP itself is restored to the original ring base.
>
> The Zynq UltraScale TRM [1] describes the receive-buffer queue pointer as follows:
>
>   An internal counter represents the receive-buffer queue pointer and it is not
>   visible through the CPU interface.
>
> I could not find a similar description for the transmit-buffer queue pointer,
> but I believe it behaves the same way. From a software perspective, it should
> be safe to assume that the TBQP is reset to point to the start of the transmit
> descriptor list upon reset. This assumption is supported by the description
> of the transmit_q_ptr (GEM) Register [2]:
>
>   Reading this register returns the location of the descriptor currently being accessed.
>   Since the DMA handles two frames at once, this may not necessarily be pointing to the
>   current frame being transmitted.
>
> [1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm
> [2] https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/transmit_q_ptr-GEM-Register

For what it's worth, I agree with Kevin.

It should be rather easy to detect if the patch is needed, with more
logging. Dump TBQP before link-down & dump it at link-up. The code
expects TBQP to reset to the ring start automatically whereas this
commit message says the TBQP after link-up is some offset into the ring.

Lastly, the cover letter mentions that [PATCH 1/2] alone isn't enough.
But it doesn't mention that [PATCH 2/2] alone doesn't solve the issue.
This would be a useful test as well.

On Tue Jul 7, 2026 at 3:36 PM CEST, Taedcke, Christian wrote:
> Thank you for the quick review! This is my first Linux kernel
> contribution, so I appreciate your feedback here.

Welcome!

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] macb_hresp_error_task() reinitializes all TX and RX rings without holding any locks and without synchronizing with NAPI.
- [High] macb_tx_error_task() accesses and modifies TX queue state without holding queue->tx_ptr_lock, causing data races with concurrent macb_start_xmit() and gem_shuffle_tx_rings() operations.
--

commit f087c1d0ffc5f4dd5ef2d3cbe0d1c4868c7a17b2
Author: Christian Taedcke <christian.taedcke@weidmueller.com>

net: macb: reprogram TBQP after shuffling the TX ring on link-up

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.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index fd282a1700fb9..b11cb8f068b73 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;
>  	}

[Severity: High]
This is a pre-existing issue, but does macb_tx_error_task() modify TX queue
state without holding queue->tx_ptr_lock?

Looking at macb_tx_error_task(), it resets queue->tx_head and queue->tx_tail
to 0 without holding queue->tx_ptr_lock (it only takes bp->lock):

drivers/net/ethernet/cadence/macb_main.c:macb_tx_error_task() {
    ...
    queue->tx_head = 0;
    queue->tx_tail = 0;
    ...
}

Concurrently, macb_start_xmit() can be executing, which holds
queue->tx_ptr_lock but not bp->lock.

Also, macb_mac_link_up() releases bp->lock before calling
gem_shuffle_tx_rings(), enabling a race.

Although netif_tx_stop_all_queues() is called, it only sets a state flag and
does not wait for a currently executing macb_start_xmit() to finish.

Could this lead to data corruption or double-frees?

[ ... ]

> @@ -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);
>  }

[Severity: Critical]
This is a pre-existing issue, but does macb_hresp_error_task() reinitialize
all TX and RX rings without holding any locks or synchronizing with NAPI?

Looking at macb_hresp_error_task(), it calls mog_init_rings() which
destructively resets tx_head, tx_tail, and wipes descriptors across all
queues back to 0:

drivers/net/ethernet/cadence/macb_main.c:macb_hresp_error_task() {
    ...
    bp->macbgem_ops.mog_init_rings(bp);
    ...
}

However, macb_hresp_error_task() does not acquire bp->lock or
queue->tx_ptr_lock, and it fails to call napi_disable() to synchronize with
RX/TX softirq paths.

Concurrently, NAPI polls (macb_tx_poll, macb_rx_poll) or macb_start_xmit()
might still be operating on the ring state.

Could this cause memory corruption or use-after-frees?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-upstreaming-macb-irq-storm-v1-0-ab3115b5a13a@weidmueller.com?part=1

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

[Christian Taedcke via B4 Relay]

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 b11cb8f068b7..f75cf2ffdf6f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1971,7 +1971,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
@@ -2161,7 +2161,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.54.0

Re: [PATCH net 2/2] net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

[Sebastian Andrzej Siewior]

On 2026-07-06 16:02:15 [+0200], Christian Taedcke via B4 Relay wrote:
> 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.

So this is not a race condition, this is always a failure?

Sebastian

Re: [PATCH net 2/2] net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

[Taedcke, Christian]

On 7/6/2026 5:05 PM, Sebastian Andrzej Siewior wrote:
> On 2026-07-06 16:02:15 [+0200], Christian Taedcke via B4 Relay wrote:
>> 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.
> 
> So this is not a race condition, this is always a failure?

As far as in understand it: yes

> 
> Sebastian

Christian

Re: [PATCH net 2/2] net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete interrupt masking in the NAPI reschedule path leaves TXUBR unmasked, causing unnecessary hard interrupts.
- [Medium] Ad-hoc cross-CPU synchronization flag with mismatched memory barriers causes a data race.
--

commit 8cade27ad39e331dd72cf96ffc689e581ab23335
Author: Christian Taedcke <christian.taedcke@weidmueller.com>

net: macb: mask TXUBR during TX NAPI poll to prevent IRQ storms

This commit attempts to fix an interrupt storm caused by TXUBR remaining
unmasked while NAPI handles completions. It masks TXUBR alongside TCOMP
during NAPI scheduling and uses a flag to track pending TXUBR events.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1971,7 +1971,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

[Severity: Medium]
In the reschedule path just below this change, does TXUBR also need to be
masked?

When packets are still pending, the code reschedules NAPI and masks TCOMP:

	if (macb_tx_complete_pending(queue)) {
		queue_writel(queue, IDR, MACB_BIT(TCOMP));
		macb_queue_isr_clear(bp, queue, MACB_BIT(TCOMP));
		...
		napi_schedule(napi);
	}

Since IER just unmasked both TCOMP and TXUBR in the lines above, failing to
mask TXUBR here leaves it unmasked while NAPI takes over. Under heavy TX load,
does this cause unnecessary hard interrupts?

[ ... ]

> @@ -2161,7 +2161,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)) {

[Severity: Medium]
Is this use of a plain boolean and memory barriers sufficient for cross-CPU
synchronization here?

The commit message notes that pending TXUBR is recorded in
queue->txubr_pending, and the interrupt handler updates it like this:

		if (status & MACB_BIT(TXUBR)) {
			queue->txubr_pending = true;
			wmb(); // ensure softirq can see update
		}

Concurrently, macb_tx_poll() locklessly reads and clears it:

	rmb(); // ensure txubr_pending is up to date
	if (queue->txubr_pending) {
		queue->txubr_pending = false;

Using wmb() and rmb() enforces instruction ordering but does not flush
caches, which creates a data race on the plain boolean under the kernel
memory model. Could this lead to undefined behavior or lost wakeups if the
compiler caches or reorders the flag?

Could this be replaced with atomic bit operations like set_bit() and
test_and_clear_bit() instead to safely synchronize between the hardirq
and softirq contexts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-upstreaming-macb-irq-storm-v1-0-ab3115b5a13a@weidmueller.com?part=2