Re: [PATCH net v4] net: axienet: free outstanding TX buffers in axienet_dma_bd_release()

[Simon Horman <horms@kernel.org>]

On Wed, Oct 07, 2026 at 06:46:20AM +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.
> 
> axienet_dma_err_handler() already walks the TX ring this way before it
> restarts the DMA engine: it unmaps every descriptor whose cntrl is still
> set - axienet_free_tx_chain() clears it on reclaim - frees any skb still
> attached, and clears the descriptor.  Move that loop into a helper,
> axienet_free_tx_bufs(), and call it from axienet_dma_bd_release() too.
> This relies on axienet_stop() having stopped the DMA engine first, as
> the RX walk in the same function already does.  The helper frees the
> skbs with dev_kfree_skb_any(), as drops: in the error handler, which
> runs from a workqueue, that frees them directly rather than deferring
> them to softirq as dev_kfree_skb_irq() did.
> 
> The walk must not run on a ring that is not there.  axienet_open() does
> not check the result of the reset that runs axienet_dma_bd_init(), so
> when that reset fails tx_bd_v is either still NULL or, after an earlier
> close, points at the ring that close freed.  Clear tx_bd_v and rx_bd_v
> once their rings are freed, and skip the walk when tx_bd_v is NULL.
> That also ends the second dma_free_coherent() of a stale ring which the
> same path already did.  On the axienet_dma_bd_init() error path the TX
> ring has just been allocated zeroed, so the walk does nothing.
> 
> This was reported by the Sashiko AI review bot.
> 
> Tested on the AXI Ethernet MAC of an ADVA TimeCard X2 (PCIe card, with
> the built-in AXI DMA): traffic passes, and after each of ten down/up
> cycles and five module reloads, all made with traffic running and each
> running axienet_dma_bd_release(), traffic resumes and nothing is logged.
> The leak itself was not measured, and neither the failed-reset paths nor
> axienet_dma_err_handler() were exercised.
> 
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
> Reviewed-by: Joe Damato <joe@dama.to>
> Assisted-by: LLM sparse
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>

In future please post new patch revisions as new email threads,
not as a reply to an earlier revision.

Thanks!