From: "Théo Lebrun" <theo.lebrun@bootlin.com>
To: Nicolas Ferre <nicolas.ferre@microchip.com>,
Claudiu Beznea <claudiu.beznea@tuxon.dev>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>, Jeff Garzik <jeff@garzik.org>,
Conor Dooley <conor.dooley@microchip.com>
Cc: "Paolo Valerio" <pvalerio@redhat.com>,
"Nicolai Buchwitz" <nb@tipi-net.de>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
"Vladimir Kondratiev" <vladimir.kondratiev@mobileye.com>,
"Gregory CLEMENT" <gregory.clement@bootlin.com>,
"Benoît Monin" <benoit.monin@bootlin.com>,
"Tawfik Bayouk" <tawfik.bayouk@mobileye.com>,
"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
"Maxime Chevallier" <maxime.chevallier@bootlin.com>,
stable@vger.kernel.org, "Théo Lebrun" <theo.lebrun@bootlin.com>
Subject: [PATCH net v4] net: macb: drop in-flight Tx SKBs on close
Date: Thu, 02 Jul 2026 17:37:02 +0200 [thread overview]
Message-ID: <20260702-macb-drop-tx-v4-1-1c833eebdbc8@bootlin.com> (raw)
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>
reply other threads:[~2026-07-02 15:37 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260702-macb-drop-tx-v4-1-1c833eebdbc8@bootlin.com \
--to=theo.lebrun@bootlin.com \
--cc=andrew+netdev@lunn.ch \
--cc=benoit.monin@bootlin.com \
--cc=claudiu.beznea@tuxon.dev \
--cc=conor.dooley@microchip.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gregory.clement@bootlin.com \
--cc=jeff@garzik.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.chevallier@bootlin.com \
--cc=nb@tipi-net.de \
--cc=netdev@vger.kernel.org \
--cc=nicolas.ferre@microchip.com \
--cc=pabeni@redhat.com \
--cc=pvalerio@redhat.com \
--cc=stable@vger.kernel.org \
--cc=tawfik.bayouk@mobileye.com \
--cc=thomas.petazzoni@bootlin.com \
--cc=vladimir.kondratiev@mobileye.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox