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

On Tue, Oct 6, 2026 at 2:41 AM Joe Damato [off-list ref] wrote:

On Sun, Oct 04, 2026 at 11:37:59AM +0300, Sagi Maimon wrote:

quoted

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.

[...]

quoted

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

 .../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

+     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>

Thanks for the review, Joe.
Agreed on the helper.  As on v2, I kept the fix minimal for net, and
will send the helper to net-next once this has reached it, with both
axienet_dma_bd_release() and axienet_dma_err_handler() using it.
Thanks,
Sagi