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

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

[Sagi Maimon]

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.  The DMA engine
has been stopped by then, so the hardware no longer references the
buffers.  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 an AXI Ethernet MAC behind a PCIe endpoint: 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.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Assisted-by: LLM sparse
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---

Notes:
    Found by the Sashiko review of v2 of "net: axienet: bound TX completion
    cleanup by the NAPI budget":
    https://lore.kernel.org/netdev/20260917115657.20697-1-maimon.sagi@gmail.com/
    It is independent of that patch and applies on its own.

 .../net/ethernet/xilinx/xilinx_axienet_main.c  | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 1722b7038f34..02bcb89d1bbe 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -187,6 +187,24 @@ static void axienet_dma_bd_release(struct net_device *ndev)
 	struct axienet_local *lp = netdev_priv(ndev);
 
 	/* If we end up here, tx_bd_v must have been DMA allocated. */
+	for (i = 0; i < lp->tx_bd_num; i++) {
+		struct axidma_bd *cur_p = &lp->tx_bd_v[i];
+
+		/* axienet_free_tx_chain() clears cntrl when it reclaims a
+		 * descriptor, so a non-zero value means the mapping is live.
+		 */
+		if (cur_p->cntrl) {
+			dma_addr_t addr = desc_get_phys_addr(lp, cur_p);
+
+			dma_unmap_single(lp->dev, addr,
+					 (cur_p->cntrl &
+					  XAXIDMA_BD_CTRL_LENGTH_MASK),
+					 DMA_TO_DEVICE);
+		}
+		if (cur_p->skb)
+			dev_kfree_skb(cur_p->skb);
+	}
+
 	dma_free_coherent(lp->dev,
 			  sizeof(*lp->tx_bd_v) * lp->tx_bd_num,
 			  lp->tx_bd_v,

base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.47.0

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

[Jacob Keller]

On 9/27/2026 1:10 AM, 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.  The DMA engine
> has been stopped by then, so the hardware no longer references the
> buffers.  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 an AXI Ethernet MAC behind a PCIe endpoint: 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.
> 
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
> Assisted-by: LLM sparse
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
> ---
> 
> Notes:
>     Found by the Sashiko review of v2 of "net: axienet: bound TX completion
>     cleanup by the NAPI budget":
>     https://lore.kernel.org/netdev/20260917115657.20697-1-maimon.sagi@gmail.com/
>     It is independent of that patch and applies on its own.
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  .../net/ethernet/xilinx/xilinx_axienet_main.c  | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 1722b7038f34..02bcb89d1bbe 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -187,6 +187,24 @@ static void axienet_dma_bd_release(struct net_device *ndev)
>  	struct axienet_local *lp = netdev_priv(ndev);
>  
>  	/* If we end up here, tx_bd_v must have been DMA allocated. */
> +	for (i = 0; i < lp->tx_bd_num; i++) {
> +		struct axidma_bd *cur_p = &lp->tx_bd_v[i];
> +
> +		/* axienet_free_tx_chain() clears cntrl when it reclaims a
> +		 * descriptor, so a non-zero value means the mapping is live.
> +		 */
> +		if (cur_p->cntrl) {
> +			dma_addr_t addr = desc_get_phys_addr(lp, cur_p);
> +
> +			dma_unmap_single(lp->dev, addr,
> +					 (cur_p->cntrl &
> +					  XAXIDMA_BD_CTRL_LENGTH_MASK),
> +					 DMA_TO_DEVICE);
> +		}
> +		if (cur_p->skb)
> +			dev_kfree_skb(cur_p->skb);
> +	}
> +
>  	dma_free_coherent(lp->dev,
>  			  sizeof(*lp->tx_bd_v) * lp->tx_bd_num,
>  			  lp->tx_bd_v,
> 
> base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7

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

[Joe Damato]

On Sun, Sep 27, 2026 at 11:10:34AM +0300, Sagi Maimon wrote:

[...]
 
> 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.

The code added in this patch and in axienet_dma_err_handler are almost
identical (as your commit message suggests). I am wondering if it's possible
to factor this code out into a helper and instead call it from both places
instead of repeating the same code?

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

[Sagi Maimon]

On Tue, Sep 29, 2026 at 3:12 AM Joe Damato <joe@dama.to> wrote:
>
> On Sun, Sep 27, 2026 at 11:10:34AM +0300, Sagi Maimon wrote:
>
> [...]
>
> > 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.
>
> The code added in this patch and in axienet_dma_err_handler are almost
> identical (as your commit message suggests). I am wondering if it's possible
> to factor this code out into a helper and instead call it from both places
> instead of repeating the same code?

Yes, the unmap-and-free part is the same; the error handler also clears
each descriptor for reuse, and frees with dev_kfree_skb_irq() where this
uses dev_kfree_skb(), so a shared helper would use dev_kfree_skb_any().
Since this goes to net with a Fixes: tag I kept it to the minimal
change.  I will send the helper to net-next once this fix has reached
it, if that works for you.
Thanks,
Sagi