RE: [Intel-wired-lan] [PATCH v2 iwl-net] ixgbe: xsk: resolve the negative overflow of budget in ixgbe_xmit_zc

-----Original Message-----
From: Intel-wired-lan <redacted> On Behalf Of
Jason Xing
Sent: Saturday, July 26, 2025 12:34 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
[off-list ref]; Zaremba, Larysa
[off-list ref]; andrew+netdev@lunn.ch;
davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
pabeni@redhat.com; bjorn@kernel.org; Fijalkowski, Maciej
[off-list ref]
Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Jason Xing
[off-list ref]
Subject: [Intel-wired-lan] [PATCH v2 iwl-net] ixgbe: xsk: resolve the negative
overflow of budget in ixgbe_xmit_zc

From: Jason Xing <kernelxing@tencent.com>

Resolve the budget negative overflow which leads to returning true in
ixgbe_xmit_zc even when the budget of descs are thoroughly consumed.

Before this patch, when the budget is decreased to zero and finishes
sending the last allowed desc in ixgbe_xmit_zc, it will always turn back and
enter into the while() statement to see if it should keep processing packets,
but in the meantime it unexpectedly decreases the value again to 'unsigned
int (0--)', namely, UINT_MAX. Finally, the ixgbe_xmit_zc returns true,
showing 'we complete cleaning the budget'. That also means
'clean_complete = true' in ixgbe_poll.

The true theory behind this is if that budget number of descs are
consumed, it implies that we might have more descs to be done. So we
should return false in ixgbe_xmit_zc to tell napi poll to find another chance
to start polling to handle the rest of descs. On the contrary, returning true
here means job done and we know we finish all the possible descs this time
and we don't intend to start a new napi poll.

It is apparently against our expectations. Please also see how
ixgbe_clean_tx_irq() handles the problem: it uses do..while() statement to
make sure the budget can be decreased to zero at most and the negative
overflow never happens.

The patch adds 'likely' because we rarely would not hit the loop codition
since the standard budget is 256.

Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Larysa Zaremba <redacted>
---
Link: https://lore.kernel.org/all/20250720091123.474-3-
kerneljasonxing@gmail.com/
1. use 'negative overflow' instead of 'underflow' (Willem) 2. add reviewed-by
tag (Larysa) 3. target iwl-net branch (Larysa) 4. add the reason why the patch
adds likely() (Larysa)
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c

b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index ac58964b2f08..7b941505a9d0 100644

--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c

@@ -398,7 +398,7 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring

*xdp_ring, unsigned int budget)
 	dma_addr_t dma;
 	u32 cmd_type;

-	while (budget-- > 0) {
+	while (likely(budget)) {
 		if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
 			work_done = false;
 			break;

@@ -433,6 +433,8 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring

*xdp_ring, unsigned int budget)
 		xdp_ring->next_to_use++;
 		if (xdp_ring->next_to_use == xdp_ring->count)
 			xdp_ring->next_to_use = 0;
+
+		budget--;
 	}

 	if (tx_desc) {
--
2.41.3