Re: [PATCH net v3] net: axienet: free outstanding TX buffers in axienet_dma_bd_release()
flat view
From: Joe Damato <hidden>
Date: 2026-10-05 23:41:44
Also in:
linux-arm-kernel, lkml
On Sun, Oct 04, 2026 at 11:37:59AM +0300, Sagi Maimon wrote:
axienet_dma_bd_release() walks the RX ring to unmap and free every receive buffer before releasing it, but frees the TX descriptor ring with dma_free_coherent() alone. Any descriptor that axienet_free_tx_chain() had not yet reclaimed still holds its skb and its streaming DMA mapping, and both are lost. axienet_stop() disables TX NAPI and stops the DMA engine before calling it, so nothing reclaims those descriptors afterwards. Bringing the interface down while frames are in flight therefore leaks up to lp->tx_bd_num skbs and mappings each time. Walk the TX ring the way axienet_dma_err_handler() already does: unmap every descriptor whose cntrl is still set - axienet_free_tx_chain() clears it on reclaim - and free any skb still attached, as a drop. This relies on axienet_stop() having stopped the DMA engine first, as the RX walk in the same function already does.
[...]
Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <redacted>
---
Notes:
Changes in v3:
- Clear tx_bd_v and rx_bd_v after freeing the rings. v2 only caught a
NULL tx_bd_v from a first open; after a close followed by a failed
reset the walk would have read the freed ring (Sashiko).
- Reword the comment on the skb free: a descriptor can complete after
TX NAPI was disabled, so "never transmitted" was not always true
(Sashiko).
- Say in the commit message which hardware the test ran on.
- Kept Jacob's Reviewed-by, as the changes are small; please say if
that is not OK.
- The hardware test is v1's. The changes since only affect the
failed-reset paths, which it did not exercise, and how the freed skbs
are accounted.
- v2: https://lore.kernel.org/netdev/20260930133851.663023-1-maimon.sagi@gmail.com/ (local)[...]
quoted hunk ↗ jump to hunk
.../net/ethernet/xilinx/xilinx_axienet_main.c | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 09443623a3e2..c88c671f8b2d 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c@@ -186,11 +186,34 @@ static void axienet_dma_bd_release(struct net_device *ndev)
[...]
quoted hunk ↗ jump to hunk
+ lp->tx_bd_v = NULL; if (!lp->rx_bd_v) return;@@ -221,6 +244,7 @@ static void axienet_dma_bd_release(struct net_device *ndev) sizeof(*lp->rx_bd_v) * lp->rx_bd_num, lp->rx_bd_v, lp->rx_bd_p); + lp->rx_bd_v = NULL; }
The added null writes makes me think that centralizing this code and using it from both axienet_dma_err_handler and axienet_dma_bd_release (instead of repeating it) is a good idea like I mentioned in the last post. The code seems right tho even tho I don't like duplicating the logic. Reviewed-by: Joe Damato <redacted>