[PATCH net v4] net: macb: drop in-flight Tx SKBs on close

[PATCH net v4] net: macb: drop in-flight Tx SKBs on close

[Théo Lebrun]

The MACB driver has since forever leaked the outgoing SKBs that
have not yet been marked as completed. They live in queue->tx_skb
which gets freed without remorse nor checking.

macb_free_consistent() gets called in a few codepaths, but only close will
trigger the added expressions. In macb_open() and macb_alloc_consistent()
failure cases, queues' tx_skb just got allocated and are empty.

Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
Cc: stable@vger.kernel.org
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v4:
- Drop the skb_drop_reason code. No other Ethernet driver does that and
  the reasoning (because our stats are broken) is a bad one.
- Take Rb trailer from Nicolai.
- Drop <hskinnemoen@atmel.com> email that gets rejected.
- Rebase upon latest net/main (d8e8b85a85fe).
- Link to v3: https://patch.msgid.link/20260617-macb-drop-tx-v3-0-d4c7e57d890b@bootlin.com

Changes in v3:
- Drop stats fixing. A proper fix deserves its own net-next refactoring
  series to migrate to netdev_stat_ops (ynltool uAPI), which will come
  in later. We keep the tx_dropped++ because they are safe as every
  other context is disabled when macb_free_consistent() is called.
- Rebased to latest net/main (406e8a651a7b), nothing to report.
- Link to v2: https://patch.msgid.link/20260428-macb-drop-tx-v2-0-647f5199d8df@bootlin.com

Changes in v2:
- Increment tx_dropped stat once per SKB, not once per frame.
- Reset tx_head & tx_tail to avoid keeping stalled cursors.
- Fix SKB dropped reasons throughout by adding the reason as parameter
  to macb_tx_unmap(). This is a new patch. Then the drop-all-on-close
  fix can use this ability to report we are not consuming SKBs.
- Add increment to stats->tx_dropped on DMA mapping failure and
  tx_error_task. Done as separate patches (3 and 4).
- Rebase upon net/main @ 46f74a3f7d57, nothing to report.
- Link to v1: https://patch.msgid.link/20260424-macb-drop-tx-v1-1-b3ecb787d84d@bootlin.com
---
 drivers/net/ethernet/cadence/macb_main.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index fd282a1700fb..d394f1f43b68 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2668,8 +2668,25 @@ static void macb_free_consistent(struct macb *bp)
 	dma_free_coherent(dev, size, bp->queues[0].rx_ring, bp->queues[0].rx_ring_dma);
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-		kfree(queue->tx_skb);
-		queue->tx_skb = NULL;
+		if (queue->tx_skb) {
+			unsigned int dropped = 0, tail;
+
+			for (tail = queue->tx_tail; tail != queue->tx_head;
+			     tail++) {
+				if (macb_tx_skb(queue, tail)->skb)
+					dropped++;
+				macb_tx_unmap(bp, macb_tx_skb(queue, tail), 0);
+			}
+
+			queue->stats.tx_dropped += dropped;
+			bp->dev->stats.tx_dropped += dropped;
+
+			kfree(queue->tx_skb);
+			queue->tx_skb = NULL;
+		}
+
+		queue->tx_head = 0;
+		queue->tx_tail = 0;
 		queue->tx_ring = NULL;
 		queue->rx_ring = NULL;
 	}

---
base-commit: d8e8b85a85fe21954d303db68034aac4639df88d
change-id: 20260423-macb-drop-tx-f9ce72720d05

Best regards,
--  
Théo Lebrun <theo.lebrun@bootlin.com>

Re: [PATCH net v4] net: macb: drop in-flight Tx SKBs on close

[Paolo Abeni]

On 7/2/26 5:37 PM, Théo Lebrun wrote:
> The MACB driver has since forever leaked the outgoing SKBs that
> have not yet been marked as completed. They live in queue->tx_skb
> which gets freed without remorse nor checking.
> 
> macb_free_consistent() gets called in a few codepaths, but only close will
> trigger the added expressions. In macb_open() and macb_alloc_consistent()
> failure cases, queues' tx_skb just got allocated and are empty.
> 
> Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> Changes in v4:
> - Drop the skb_drop_reason code. No other Ethernet driver does that and
>   the reasoning (because our stats are broken) is a bad one.
> - Take Rb trailer from Nicolai.
> - Drop <hskinnemoen@atmel.com> email that gets rejected.
> - Rebase upon latest net/main (d8e8b85a85fe).
> - Link to v3: https://patch.msgid.link/20260617-macb-drop-tx-v3-0-d4c7e57d890b@bootlin.com
> 
> Changes in v3:
> - Drop stats fixing. A proper fix deserves its own net-next refactoring
>   series to migrate to netdev_stat_ops (ynltool uAPI), which will come
>   in later. We keep the tx_dropped++ because they are safe as every
>   other context is disabled when macb_free_consistent() is called.
> - Rebased to latest net/main (406e8a651a7b), nothing to report.
> - Link to v2: https://patch.msgid.link/20260428-macb-drop-tx-v2-0-647f5199d8df@bootlin.com
> 
> Changes in v2:
> - Increment tx_dropped stat once per SKB, not once per frame.
> - Reset tx_head & tx_tail to avoid keeping stalled cursors.
> - Fix SKB dropped reasons throughout by adding the reason as parameter
>   to macb_tx_unmap(). This is a new patch. Then the drop-all-on-close
>   fix can use this ability to report we are not consuming SKBs.
> - Add increment to stats->tx_dropped on DMA mapping failure and
>   tx_error_task. Done as separate patches (3 and 4).
> - Rebase upon net/main @ 46f74a3f7d57, nothing to report.
> - Link to v1: https://patch.msgid.link/20260424-macb-drop-tx-v1-1-b3ecb787d84d@bootlin.com
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index fd282a1700fb..d394f1f43b68 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2668,8 +2668,25 @@ static void macb_free_consistent(struct macb *bp)
>  	dma_free_coherent(dev, size, bp->queues[0].rx_ring, bp->queues[0].rx_ring_dma);
>  
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> -		kfree(queue->tx_skb);
> -		queue->tx_skb = NULL;
> +		if (queue->tx_skb) {
> +			unsigned int dropped = 0, tail;
> +
> +			for (tail = queue->tx_tail; tail != queue->tx_head;
> +			     tail++) {
> +				if (macb_tx_skb(queue, tail)->skb)
> +					dropped++;
> +				macb_tx_unmap(bp, macb_tx_skb(queue, tail), 0);
> +			}
> +
> +			queue->stats.tx_dropped += dropped;
> +			bp->dev->stats.tx_dropped += dropped;
> +
> +			kfree(queue->tx_skb);
> +			queue->tx_skb = NULL;
> +		}
> +
> +		queue->tx_head = 0;
> +		queue->tx_tail = 0;

Both sashikos noted this could race vs tx_error_task, but it looks like
a pre-existing issue, and I think it should be addressed with a
follow-up patch.

/P

Re: [PATCH net v4] net: macb: drop in-flight Tx SKBs on close

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 02 Jul 2026 17:37:02 +0200 you wrote:
> The MACB driver has since forever leaked the outgoing SKBs that
> have not yet been marked as completed. They live in queue->tx_skb
> which gets freed without remorse nor checking.
> 
> macb_free_consistent() gets called in a few codepaths, but only close will
> trigger the added expressions. In macb_open() and macb_alloc_consistent()
> failure cases, queues' tx_skb just got allocated and are empty.
> 
> [...]

Here is the summary with links:
  - [net,v4] net: macb: drop in-flight Tx SKBs on close
    https://git.kernel.org/netdev/net/c/27f575836cfe

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html