[PATCH net v2] net: axienet: bound TX completion cleanup by the NAPI budget

[PATCH net v2] net: axienet: bound TX completion cleanup by the NAPI budget

[Sagi Maimon]

axienet_tx_poll() passes lp->tx_bd_num to axienet_free_tx_chain() as
@nr_bds, and @budget is only forwarded to napi_consume_skb() as its
bulk-free hint.  Nothing limits the cleanup loop to the NAPI budget, so
the number of packets returned is bounded by the TX ring size rather
than by the budget, and the poll can report more work than it was
given:

  eth0: NAPI poll function axienet_tx_poll+0x0/0x180 [xilinx_emac]
        returned 96, exceeding its budget of 64.

Returning more than the budget breaks the NAPI contract.  It also makes
the "packets < budget" test in axienet_tx_poll() false, so
napi_complete_done() is skipped and TX completion interrupts are not
re-enabled on that pass.  NAPI reschedules the poll, so this recovers,
but the accounting is wrong either way.

In steady state fewer descriptors complete per poll than the budget
allows, which is why this is rarely observed.  Triggering it needs more
than @budget completions outstanding at once - for example when TX
completion interrupts have not been taken for a while and a full ring is
reclaimed in one go.

Stop the loop once the budget is spent.  cur_p->skb is only set on a
packet's last descriptor, so breaking there never leaves a packet
half-freed.

A zero budget must not be treated as a spent budget.  netpoll calls
napi->poll() with a budget of 0 to reclaim the TX path only, and
axienet_tx_poll() forwards it unchanged with @force false; testing
packets >= budget alone would break out before examining a single
descriptor, leaving skbs unfreed and lp->tx_bd_ci unchanged.  Since
cur_p->cntrl is cleared only here, axienet_check_tx_bd_space() would
keep reporting the ring full and netconsole output could stall.  Zero
therefore means no limit, which also covers the @force callers that
clean up after a DMA mapping failure with a budget of 0.

Fixes: 9e2bc267e780 ("net: axienet: Use NAPI for TX completion path")
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
v2:
- Do not treat a zero budget as a spent budget.  netpoll polls with a
  budget of 0 to reclaim the TX path only, and v1 broke out of the loop
  before examining any descriptor, so nothing was reclaimed and the ring
  could stay full.  Reported by the Sashiko AI reviewer.
- Update the @budget kernel-doc: it bounds the cleanup when @force is
  false, and zero means no limit.  It previously read "use 0 when not
  called from NAPI poll", which after v1 would have meant "reclaim
  nothing".
- Compile-tested only; the board this was found on is not available to
  me at the moment.  v1 was verified on hardware with the napi:napi_poll
  tracepoint (8424 polls, max work 64 against a budget of 64).

v1: https://lore.kernel.org/netdev/20260914114821.55503-1-maimon.sagi@gmail.com/
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 782f903d318f..f643453261f1 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -772,7 +772,9 @@ static int axienet_device_reset(struct net_device *ndev)
  * @force:	Whether to clean descriptors even if not complete
  * @sizep:	Pointer to a u32 accumulating the total byte count of
  *		completed packets (using skb->len). Ignored if NULL.
- * @budget:	NAPI budget (use 0 when not called from NAPI poll)
+ * @budget:	NAPI budget. When @force is false, cleanup stops after this
+ *		many completed packets. Zero means no limit, as used by the
+ *		netpoll TX reclaim and by callers outside NAPI poll.
  *
  * Would either be called after a successful transmit operation, or after
  * there was an error when setting up the chain.
@@ -788,6 +790,15 @@ static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd,
 	dma_addr_t phys;
 
 	for (i = 0; i < nr_bds; i++) {
+		/* A NAPI poll must not return more than its budget.  Stop on a
+		 * packet boundary once it is spent - cur_p->skb is only set on
+		 * a packet's last descriptor, so no packet is left half-freed.
+		 * A zero budget means no limit: netpoll polls with a budget of
+		 * 0 to reclaim the TX path only, and must still clean the ring.
+		 */
+		if (!force && budget && packets >= budget)
+			break;
+
 		cur_p = &lp->tx_bd_v[(first_bd + i) % lp->tx_bd_num];
 		status = cur_p->status;
 
-- 
2.47.0