From 7af191794bc51044c6a4cf074ea1a7e610cc33cd Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 7 Nov 2026 21:58:47 +0100 Subject: [PATCH 1/8] net: macb: credit unsent frames to BQL in macb_tx_error_task() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 7b42f4a88e76df58b90760cd5292bd90c26293eb upstream. The frames dropped on a TX error are never returned to BQL. Each error shrinks the BQL budget for good until the queue stops with an empty ring. netdev_tx_reset_queue() is not safe here as the task runs without tx_ptr_lock, so macb_start_xmit() may still be queueing. Cc: stable+noautosel@kernel.org # not seen in real life Fixes: e6a532185daa ("net: cadence: macb: Implement BQL") Signed-off-by: Nicolai Buchwitz Reviewed-by: Théo Lebrun Link: https://patch.msgid.link/20261107-nb-macb-tx-error-task-net-v1-1-a2aac5d64d52@tipi-net.de Signed-off-by: Jakub Kicinski Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 4a5c3bd74a0a0..a4e7b93287809 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -1357,10 +1357,8 @@ static void macb_tx_error_task(struct work_struct *work) skb->data); bp->dev->stats.tx_packets++; queue->stats.tx_packets++; - packets++; bp->dev->stats.tx_bytes += skb->len; queue->stats.tx_bytes += skb->len; - bytes += skb->len; } } else { /* "Buffers exhausted mid-frame" errors may only happen @@ -1374,6 +1372,12 @@ static void macb_tx_error_task(struct work_struct *work) desc->ctrl = ctrl | MACB_BIT(TX_USED); } + /* BQL has to see every frame leaving the ring, sent or not */ + if (skb) { + packets++; + bytes += skb->len; + } + macb_tx_unmap(bp, tx_skb, 0); } From f9e83403c69e17707b7c9714ddaac60ef9983273 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 7 Nov 2026 21:58:48 +0100 Subject: [PATCH 2/8] net: macb: take tx_ptr_lock in macb_tx_error_task() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit c45ceb26e0dc71bb048bda7e50cdab609ef5b5af upstream. The error task resets the TX ring under bp->lock while macb_start_xmit() updates it under tx_ptr_lock. Stopping the queues does not wait for an xmit already running on another CPU, so a frame mapped during the reset is lost and its skb and DMA mapping leak. Take tx_ptr_lock like macb_start_xmit() does. Cc: stable+noautosel@kernel.org # not seen in real life Fixes: 138badbc21a0 ("net: macb: use NAPI for TX completion path") Signed-off-by: Nicolai Buchwitz Reviewed-by: Théo Lebrun Link: https://patch.msgid.link/20261107-nb-macb-tx-error-task-net-v1-2-a2aac5d64d52@tipi-net.de Signed-off-by: Jakub Kicinski Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index a4e7b93287809..3114fb095854b 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -1313,7 +1313,8 @@ static void macb_tx_error_task(struct work_struct *work) * network engine about the macb/gem being halted. */ napi_disable(&queue->napi_tx); - spin_lock_irqsave(&bp->lock, flags); + spin_lock_irqsave(&queue->tx_ptr_lock, flags); + spin_lock(&bp->lock); /* Make sure nobody is trying to queue up new packets */ netif_tx_stop_all_queues(bp->dev); @@ -1409,7 +1410,9 @@ static void macb_tx_error_task(struct work_struct *work) netif_tx_start_all_queues(bp->dev); macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART)); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); + spin_unlock_irqrestore(&queue->tx_ptr_lock, flags); + napi_enable(&queue->napi_tx); } From b93ddb72cacd0ae9c6fdbf9d1ff39af6f7c27afe Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sun, 18 Oct 2026 11:41:56 +0200 Subject: [PATCH 3/8] net: macb: flush the IER write before re-checking the ring The IER write at the end of macb_tx_poll() is posted. The ring check right after it can run before TCOMP is unmasked, and a frame that completes in that window never raises an interrupt. With TSO this happens when TXUBR triggers a poll while the frame is still on the wire. The completed frame is never reaped and BQL keeps the queue stopped. Read IMR back after the write so the check runs with TCOMP unmasked. macb_rx_poll() has the same window. Fixes: 138badbc21a0 ("net: macb: use NAPI for TX completion path") Fixes: 0bf476fc3624 ("net: macb: Fix lost RX packet wakeup race in NAPI receive") Link: https://lore.kernel.org/netdev/20260514215459.36109-3-lukasz@raczylo.com/ Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 3114fb095854b..f9516397967ee 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -1924,7 +1924,11 @@ static int macb_rx_poll(struct napi_struct *napi, int budget) * potentially race with the interrupt handler doing the same * actions if an interrupt is raised just after enabling them, * but this should be harmless. + * + * The IER write is posted. Flush it before checking the ring, + * a completion in between would raise no interrupt. */ + queue_readl(queue, IMR); if (macb_rx_pending(queue)) { queue_writel(queue, IDR, bp->rx_intr_mask); if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) @@ -2019,7 +2023,11 @@ static int macb_tx_poll(struct napi_struct *napi, int budget) * potentially race with the interrupt handler doing the same * actions if an interrupt is raised just after enabling them, * but this should be harmless. + * + * The IER write is posted. Flush it before checking the ring, + * a completion in between would raise no interrupt. */ + queue_readl(queue, IMR); if (macb_tx_complete_pending(queue)) { queue_writel(queue, IDR, MACB_BIT(TCOMP)); if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) From 9de39c616b294847d50633806f35adb5708495f9 Mon Sep 17 00:00:00 2001 From: Christian Taedcke Date: Thu, 22 Oct 2026 11:38:47 +0200 Subject: [PATCH 4/8] 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. 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 Link: https://lore.kernel.org/netdev/20261022-std-macb-v14-send-v1-2-9187bc545bf6@beckhoff.com/ Signed-off-by: Christian Taedcke Signed-off-by: Nicolai Buchwitz --- 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 f9516397967ee..91e3dc592ffaf 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -854,7 +854,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; @@ -903,6 +903,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); } From 9fd1569cdc4b4e0790708d8bef075770ec829952 Mon Sep 17 00:00:00 2001 From: Steffen Dirkwinkel Date: Sat, 7 Nov 2026 14:55:04 +0100 Subject: [PATCH 5/8] net: macb: rotate unreaped TX frames behind the ring base on link-up 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 Link: https://lore.kernel.org/netdev/20261022-std-macb-v14-send-v1-4-9187bc545bf6@beckhoff.com/ Assisted-by: LLM Signed-off-by: Steffen Dirkwinkel [nb: context adjusted for 6.18, the NCR write is not under bp->lock there] Signed-off-by: Nicolai Buchwitz --- 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 91e3dc592ffaf..436dbd7e8db25 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -829,7 +829,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; @@ -848,8 +848,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; @@ -857,7 +868,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++) { @@ -897,24 +908,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; @@ -987,6 +997,15 @@ static void macb_mac_link_up(struct phylink_config *config, macb_writel(bp, NCR, ctrl | MACB_BIT(RE) | MACB_BIT(TE)); + 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(ndev); } From 4f89183e6c472390032fa2876b282d147512df42 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 7 Nov 2026 14:44:42 +0100 Subject: [PATCH 6/8] net: macb: compare the TBQP index relative to the ring base macb_tx_restart() treats the raw TBQP value divided by the descriptor size as a ring index. With 24-byte descriptors the ring base is never a multiple of the descriptor size, so the index is off and the comparison with the head index means nothing. The restart then kicks a running DMA most of the time and skips the one case where it should kick. Take the index relative to the ring base. Fixes: 5ad7f18cd82c ("net: macb: Restart tx only if queue pointer is lagging") Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 436dbd7e8db25..55ce08bae30ad 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -1980,9 +1980,9 @@ static void macb_tx_restart(struct macb_queue *queue) if (queue->tx_head == queue->tx_tail) goto out_tx_ptr_unlock; - tbqp = queue_readl(queue, TBQP) / macb_dma_desc_get_size(bp); - tbqp = macb_adj_dma_desc_idx(bp, macb_tx_ring_wrap(bp, tbqp)); - head_idx = macb_adj_dma_desc_idx(bp, macb_tx_ring_wrap(bp, queue->tx_head)); + tbqp = queue_readl(queue, TBQP) - lower_32_bits(queue->tx_ring_dma); + tbqp /= macb_dma_desc_get_size(bp); + head_idx = macb_tx_ring_wrap(bp, queue->tx_head); if (tbqp == head_idx) goto out_tx_ptr_unlock; From 6455d7ab4244357d28c7b01bea60779fdc595493 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 7 Nov 2026 14:44:42 +0100 Subject: [PATCH 7/8] net: macb: restart the transmitter from every TX poll Raspberry Pi 5 users (BCM2712 with the GEM on the RP1 south bridge) reported TX queue stalls of five seconds: macb 1f00100000.ethernet eth0: NETDEV WATCHDOG: CPU: 0: transmit queue 0 timed out 5508 ms Further inspection of the registers showed TBQP on the first descriptor of a valid TSO frame with TGO and the used bit clear. So the DMA never fetched the frame. The GEM reads the descriptor behind the frame it fetches. If the next frame lands there a few microseconds later, the DMA has already seen the used bit and halts after the current frame. The TSTART of that xmit is ignored while the transmitter is busy and no TXUBR follows. The TXUBR and tx_pending flags do not help either, the interrupt that clears them often comes before the halt. Restart the transmitter from every TX poll. The completion of the previous frame always comes after the halt. Fixes: 4298388574da ("net: macb: restart tx after tx used bit read") Link: https://github.com/raspberrypi/linux/issues/7661 Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb.h | 2 -- drivers/net/ethernet/cadence/macb_main.c | 25 ++++++------------------ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index d046a922a1a4e..5515b3b2ab8a6 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -1296,8 +1296,6 @@ struct macb_queue { struct macb_tx_skb *tx_skb; dma_addr_t tx_ring_dma; struct work_struct tx_error_task; - bool txubr_pending; - bool tx_pending; struct napi_struct napi_tx; dma_addr_t rx_ring_dma; diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 55ce08bae30ad..01064137fc18f 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2027,12 +2027,10 @@ static int macb_tx_poll(struct napi_struct *napi, int budget) work_done = macb_tx_complete(queue, budget); - rmb(); // ensure txubr_pending is up to date - if (queue->txubr_pending) { - queue->txubr_pending = false; - netdev_vdbg(bp->dev, "poll: tx restart\n"); - macb_tx_restart(queue); - } + /* The DMA can park in front of a queued frame without raising + * TXUBR, so check the ring position on every poll. + */ + macb_tx_restart(queue); netdev_vdbg(bp->dev, "TX poll: queue = %u, work_done = %d, budget = %d\n", (unsigned int)(queue - bp->queues), work_done, budget); @@ -2211,12 +2209,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id) queue_writel(queue, ISR, MACB_BIT(TCOMP) | MACB_BIT(TXUBR)); - if (status & MACB_BIT(TXUBR) || queue->tx_pending) { - queue->txubr_pending = true; - queue->tx_pending = 0; - wmb(); // ensure softirq can see update - } - napi_schedule(&queue->napi_tx); } @@ -2670,14 +2662,9 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev) spin_lock(&bp->lock); macb_tx_lpi_wake(bp); - /* TSTART write might get dropped, so make the IRQ retrigger a buffer read */ - if (macb_readl(bp, TSR) & MACB_BIT(TGO)) - queue->tx_pending = 1; - macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART)); - /* Flush PCIe posted-write queue; see comment in macb_tx_restart(). - * Also flushes the preceding macb_tx_lpi_wake() NCR write and the - * TSR-read tx_pending breadcrumb above. + /* Flush the PCIe posted-write queue, see macb_tx_restart(). Also + * flushes the preceding macb_tx_lpi_wake() NCR write. */ if (bp->caps & MACB_CAPS_PCIE_POSTED_WRITES) (void)macb_readl(bp, NCR); From 4290afe6f2cf5659e54acb654b8d7d29a66ace53 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Sat, 7 Nov 2026 14:59:15 +0100 Subject: [PATCH 8/8] net: macb: run the TX poll from the timeout handler macb_tx_timeout() only restarts the DMA. When the frames went out but their completion was lost, TBQP already matches the head and the restart does nothing. Nothing reaps the frames and the queue stays stopped for good. Schedule the TX poll instead, it reaps what completed and restarts the DMA. Fixes: e438ec3e9e95 ("net: macb: add TX stall timeout callback to recover from lost TSTART write") Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 01064137fc18f..613f88785ec3a 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -4737,8 +4737,12 @@ static int macb_setup_tc(struct net_device *dev, enum tc_setup_type type, static void macb_tx_timeout(struct net_device *dev, unsigned int q) { struct macb *bp = netdev_priv(dev); + struct macb_queue *queue = &bp->queues[q]; - macb_tx_restart(&bp->queues[q]); + queue_writel(queue, IDR, MACB_BIT(TCOMP)); + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) + queue_writel(queue, ISR, MACB_BIT(TCOMP)); + napi_schedule(&queue->napi_tx); } static const struct net_device_ops macb_netdev_ops = {