[PATCH net-next 0/5] ixgbe: xsk: a couple of changes for zerocopy

[PATCH net-next 0/5] ixgbe: xsk: a couple of changes for zerocopy

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

The series mostly follows the development of i40e/ice to improve the
performance for zerocopy mode in the tx path.

Jason Xing (5):
  ixgbe: xsk: remove budget from ixgbe_clean_xdp_tx_irq
  ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc
  ixgbe: xsk: use ixgbe_desc_unused as the budget in ixgbe_xmit_zc
  ixgbe: xsk: support batched xsk Tx interfaces to increase performance
  ixgbe: xsk: add TX multi-buffer support

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   6 +-
 .../ethernet/intel/ixgbe/ixgbe_txrx_common.h  |   2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 121 ++++++++++++------
 3 files changed, 86 insertions(+), 43 deletions(-)

-- 
2.41.3

[PATCH net-next 1/5] ixgbe: xsk: remove budget from ixgbe_clean_xdp_tx_irq

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Since 'budget' parameter in ixgbe_clean_xdp_tx_irq() takes no effect,
the patch removes it. No functional change here.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c        | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 6122a0abb41f..a59fd8f74b5e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3591,7 +3591,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
 
 	ixgbe_for_each_ring(ring, q_vector->tx) {
 		bool wd = ring->xsk_pool ?
-			  ixgbe_clean_xdp_tx_irq(q_vector, ring, budget) :
+			  ixgbe_clean_xdp_tx_irq(q_vector, ring) :
 			  ixgbe_clean_tx_irq(q_vector, ring, budget);
 
 		if (!wd)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
index 78deea5ec536..788722fe527a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
@@ -42,7 +42,7 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
 			  const int budget);
 void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring);
 bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
-			    struct ixgbe_ring *tx_ring, int napi_budget);
+			    struct ixgbe_ring *tx_ring);
 int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
 void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index ac58964b2f08..0ade15058d98 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -454,7 +454,7 @@ static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
 }
 
 bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
-			    struct ixgbe_ring *tx_ring, int napi_budget)
+			    struct ixgbe_ring *tx_ring)
 {
 	u16 ntc = tx_ring->next_to_clean, ntu = tx_ring->next_to_use;
 	unsigned int total_packets = 0, total_bytes = 0;
-- 
2.41.3

[PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Resolve the budget underflow 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 underflow
never happens.

Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 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 0ade15058d98..a463c5ac9c7c 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

[PATCH net-next 3/5] ixgbe: xsk: use ixgbe_desc_unused as the budget in ixgbe_xmit_zc

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

- Adjust ixgbe_desc_unused as the budget value.
- Avoid checking desc_unused over and over again in the loop.

The patch makes ixgbe follow i40e driver that was done in commit
1fd972ebe523 ("i40e: move check of full Tx ring to outside of send loop").
[ Note that the above i40e patch has problem when ixgbe_desc_unused(tx_ring)
returns zero. The zero value as the budget value means we don't have any
possible descs to be sent, so it should return true instead to tell the
napi poll not to launch another poll to handle tx packets. Even though
that patch behaves correctly by returning true in this case, it happens
because of the unexpected underflow of the budget. Taking the current
version of i40e_xmit_zc() as an example, it returns true as expected. ]
Hence, this patch adds a standalone if statement of zero budget in front
of ixgbe_xmit_zc() as explained before.

Use ixgbe_desc_unused to replace the original fixed budget with the number
of available slots in the Tx ring. It can gain some performance.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index a463c5ac9c7c..f3d3f5c1cdc7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -393,17 +393,14 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
 	struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
 	union ixgbe_adv_tx_desc *tx_desc = NULL;
 	struct ixgbe_tx_buffer *tx_bi;
-	bool work_done = true;
 	struct xdp_desc desc;
 	dma_addr_t dma;
 	u32 cmd_type;
 
-	while (likely(budget)) {
-		if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
-			work_done = false;
-			break;
-		}
+	if (!budget)
+		return true;
 
+	while (likely(budget)) {
 		if (!netif_carrier_ok(xdp_ring->netdev))
 			break;
 
@@ -442,7 +439,7 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
 		xsk_tx_release(pool);
 	}
 
-	return !!budget && work_done;
+	return !!budget;
 }
 
 static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
@@ -505,7 +502,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 	if (xsk_uses_need_wakeup(pool))
 		xsk_set_tx_need_wakeup(pool);
 
-	return ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
+	return ixgbe_xmit_zc(tx_ring, ixgbe_desc_unused(tx_ring));
 }
 
 int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
-- 
2.41.3

[PATCH net-next 4/5] ixgbe: xsk: support batched xsk Tx interfaces to increase performance

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Like what i40e driver initially did in commit 3106c580fb7cf
("i40e: Use batched xsk Tx interfaces to increase performance"), use
the batched xsk feature to transmit packets.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 106 +++++++++++++------
 1 file changed, 72 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index f3d3f5c1cdc7..9fe2c4bf8bc5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -2,12 +2,15 @@
 /* Copyright(c) 2018 Intel Corporation. */
 
 #include <linux/bpf_trace.h>
+#include <linux/unroll.h>
 #include <net/xdp_sock_drv.h>
 #include <net/xdp.h>
 
 #include "ixgbe.h"
 #include "ixgbe_txrx_common.h"
 
+#define PKTS_PER_BATCH 4
+
 struct xsk_buff_pool *ixgbe_xsk_pool(struct ixgbe_adapter *adapter,
 				     struct ixgbe_ring *ring)
 {
@@ -388,58 +391,93 @@ void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring)
 	}
 }
 
-static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
+static void ixgbe_set_rs_bit(struct ixgbe_ring *xdp_ring)
+{
+	u16 ntu = xdp_ring->next_to_use ? xdp_ring->next_to_use - 1 : xdp_ring->count - 1;
+	union ixgbe_adv_tx_desc *tx_desc;
+
+	tx_desc = IXGBE_TX_DESC(xdp_ring, ntu);
+	tx_desc->read.cmd_type_len |= cpu_to_le32(IXGBE_TXD_CMD_RS);
+}
+
+static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
+			   int i)
+
 {
 	struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
 	union ixgbe_adv_tx_desc *tx_desc = NULL;
 	struct ixgbe_tx_buffer *tx_bi;
-	struct xdp_desc desc;
 	dma_addr_t dma;
 	u32 cmd_type;
 
-	if (!budget)
-		return true;
+	dma = xsk_buff_raw_get_dma(pool, desc[i].addr);
+	xsk_buff_raw_dma_sync_for_device(pool, dma, desc[i].len);
 
-	while (likely(budget)) {
-		if (!netif_carrier_ok(xdp_ring->netdev))
-			break;
+	tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
+	tx_bi->bytecount = desc[i].len;
+	tx_bi->xdpf = NULL;
+	tx_bi->gso_segs = 1;
 
-		if (!xsk_tx_peek_desc(pool, &desc))
-			break;
+	tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
+	tx_desc->read.buffer_addr = cpu_to_le64(dma);
 
-		dma = xsk_buff_raw_get_dma(pool, desc.addr);
-		xsk_buff_raw_dma_sync_for_device(pool, dma, desc.len);
+	cmd_type = IXGBE_ADVTXD_DTYP_DATA |
+		   IXGBE_ADVTXD_DCMD_DEXT |
+		   IXGBE_ADVTXD_DCMD_IFCS;
+	cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
+	tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
+	tx_desc->read.olinfo_status =
+		cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
 
-		tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
-		tx_bi->bytecount = desc.len;
-		tx_bi->xdpf = NULL;
-		tx_bi->gso_segs = 1;
+	xdp_ring->next_to_use++;
+}
 
-		tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
-		tx_desc->read.buffer_addr = cpu_to_le64(dma);
+static void ixgbe_xmit_pkt_batch(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc)
+{
+	u32 i;
 
-		/* put descriptor type bits */
-		cmd_type = IXGBE_ADVTXD_DTYP_DATA |
-			   IXGBE_ADVTXD_DCMD_DEXT |
-			   IXGBE_ADVTXD_DCMD_IFCS;
-		cmd_type |= desc.len | IXGBE_TXD_CMD;
-		tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
-		tx_desc->read.olinfo_status =
-			cpu_to_le32(desc.len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+	unrolled_count(PKTS_PER_BATCH)
+	for (i = 0; i < PKTS_PER_BATCH; i++)
+		ixgbe_xmit_pkt(xdp_ring, desc, i);
+}
 
-		xdp_ring->next_to_use++;
-		if (xdp_ring->next_to_use == xdp_ring->count)
-			xdp_ring->next_to_use = 0;
+static void ixgbe_fill_tx_hw_ring(struct ixgbe_ring *xdp_ring,
+				  struct xdp_desc *descs, u32 nb_pkts)
+{
+	u32 batched, leftover, i;
+
+	batched = nb_pkts & ~(PKTS_PER_BATCH - 1);
+	leftover = nb_pkts & (PKTS_PER_BATCH - 1);
+	for (i = 0; i < batched; i += PKTS_PER_BATCH)
+		ixgbe_xmit_pkt_batch(xdp_ring, &descs[i]);
+	for (i = batched; i < batched + leftover; i++)
+		ixgbe_xmit_pkt(xdp_ring, &descs[i], 0);
+}
 
-		budget--;
-	}
+static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
+{
+	struct xdp_desc *descs = xdp_ring->xsk_pool->tx_descs;
+	u32 nb_pkts, nb_processed = 0;
 
-	if (tx_desc) {
-		ixgbe_xdp_ring_update_tail(xdp_ring);
-		xsk_tx_release(pool);
+	if (!netif_carrier_ok(xdp_ring->netdev))
+		return true;
+
+	nb_pkts = xsk_tx_peek_release_desc_batch(xdp_ring->xsk_pool, budget);
+	if (!nb_pkts)
+		return true;
+
+	if (xdp_ring->next_to_use + nb_pkts >= xdp_ring->count) {
+		nb_processed = xdp_ring->count - xdp_ring->next_to_use;
+		ixgbe_fill_tx_hw_ring(xdp_ring, descs, nb_processed);
+		xdp_ring->next_to_use = 0;
 	}
 
-	return !!budget;
+	ixgbe_fill_tx_hw_ring(xdp_ring, &descs[nb_processed], nb_pkts - nb_processed);
+
+	ixgbe_set_rs_bit(xdp_ring);
+	ixgbe_xdp_ring_update_tail(xdp_ring);
+
+	return nb_pkts < budget;
 }
 
 static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
-- 
2.41.3

[PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Use the common interface to see if the desc is the end of packets. If
so, set IXGBE_TXD_CMD_EOP bit instead of setting for all preceding
descriptors. This is also how i40e driver did in commit a92b96c4ae10
("i40e: xsk: add TX multi-buffer support").

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a59fd8f74b5e..c34737065f9e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -52,6 +52,8 @@
 #include "ixgbe_txrx_common.h"
 #include "devlink/devlink.h"
 
+#define IXGBE_MAX_BUFFER_TXD 4
+
 char ixgbe_driver_name[] = "ixgbe";
 static const char ixgbe_driver_string[] =
 			      "Intel(R) 10 Gigabit PCI Express Network Driver";
@@ -11805,6 +11807,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 			       NETDEV_XDP_ACT_XSK_ZEROCOPY;
 
+	netdev->xdp_zc_max_segs = IXGBE_MAX_BUFFER_TXD;
+
 	/* MTU range: 68 - 9710 */
 	netdev->min_mtu = ETH_MIN_MTU;
 	netdev->max_mtu = IXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 9fe2c4bf8bc5..3d9fa4f2403e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -424,7 +424,9 @@ static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
 	cmd_type = IXGBE_ADVTXD_DTYP_DATA |
 		   IXGBE_ADVTXD_DCMD_DEXT |
 		   IXGBE_ADVTXD_DCMD_IFCS;
-	cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
+	cmd_type |= desc[i].len;
+	if (xsk_is_eop_desc(&desc[i]))
+		cmd_type |= IXGBE_TXD_CMD_EOP;
 	tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
 	tx_desc->read.olinfo_status =
 		cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
-- 
2.41.3

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Tony Nguyen]

On 7/20/2025 2:11 AM, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Resolve the budget underflow 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 underflow
> never happens.
> 
> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")

Hi Jason,

Seems like this one should be split off and go to iwl-net/net like the 
other similar ones [1]? Also, some of the updates made to the other 
series apply here as well?

Thanks,
Tony

[1] 
https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/

> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>   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 0ade15058d98..a463c5ac9c7c 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) {

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Jason Xing]

Hi Tony,

On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
>
>
> On 7/20/2025 2:11 AM, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Resolve the budget underflow 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 underflow
> > never happens.
> >
> > Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
>
> Hi Jason,
>
> Seems like this one should be split off and go to iwl-net/net like the
> other similar ones [1]? Also, some of the updates made to the other
> series apply here as well?

The other three patches are built on top of this one. If without the
patch, the whole series will be warned because of build failure. I was
thinking we could backport this patch that will be backported to the
net branch after the whole series goes into the net-next branch.

Or you expect me to cook four patches without this one first so that
you could easily cherry pick this one then without building conflict?

>
> Thanks,
> Tony
>
> [1]
> https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/

Regarding this one, should I send a v4 version with the current patch
included? And target [iwl-net/net] explicitly as well?

I'm not sure if I follow you. Could you instruct me on how to proceed
next in detail?

Thanks,
Jason

Re: [PATCH net-next 1/5] ixgbe: xsk: remove budget from ixgbe_clean_xdp_tx_irq

[Larysa Zaremba]

On Sun, Jul 20, 2025 at 05:11:19PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Since 'budget' parameter in ixgbe_clean_xdp_tx_irq() takes no effect,
> the patch removes it. No functional change here.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>

Should target iwl-next, but otherwise fine.

Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c        | 2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c         | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 6122a0abb41f..a59fd8f74b5e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -3591,7 +3591,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
>  
>  	ixgbe_for_each_ring(ring, q_vector->tx) {
>  		bool wd = ring->xsk_pool ?
> -			  ixgbe_clean_xdp_tx_irq(q_vector, ring, budget) :
> +			  ixgbe_clean_xdp_tx_irq(q_vector, ring) :
>  			  ixgbe_clean_tx_irq(q_vector, ring, budget);
>  
>  		if (!wd)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
> index 78deea5ec536..788722fe527a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
> @@ -42,7 +42,7 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
>  			  const int budget);
>  void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring);
>  bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
> -			    struct ixgbe_ring *tx_ring, int napi_budget);
> +			    struct ixgbe_ring *tx_ring);
>  int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
>  void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);
>  
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index ac58964b2f08..0ade15058d98 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -454,7 +454,7 @@ static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
>  }
>  
>  bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
> -			    struct ixgbe_ring *tx_ring, int napi_budget)
> +			    struct ixgbe_ring *tx_ring)
>  {
>  	u16 ntc = tx_ring->next_to_clean, ntu = tx_ring->next_to_use;
>  	unsigned int total_packets = 0, total_bytes = 0;
> -- 
> 2.41.3
> 
> 

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Larysa Zaremba]

On Sun, Jul 20, 2025 at 05:11:20PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Resolve the budget underflow 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 underflow
> never happens.
> 
> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>

Right, it is possible to decrement a zero in the current state of code.

Should target iwl-net, otherwise fine. You could include info why do you add 
likely, as such change is questionable in something that goes into a stable 
tree, but should be fine, as the standard budget is 256, so we rarely would not 
hit the loop codition.

Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>

> ---
>  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 0ade15058d98..a463c5ac9c7c 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
> 
> 

Re: [PATCH net-next 3/5] ixgbe: xsk: use ixgbe_desc_unused as the budget in ixgbe_xmit_zc

[Larysa Zaremba]

On Sun, Jul 20, 2025 at 05:11:21PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> - Adjust ixgbe_desc_unused as the budget value.
> - Avoid checking desc_unused over and over again in the loop.
> 
> The patch makes ixgbe follow i40e driver that was done in commit
> 1fd972ebe523 ("i40e: move check of full Tx ring to outside of send loop").
> [ Note that the above i40e patch has problem when ixgbe_desc_unused(tx_ring)
> returns zero. The zero value as the budget value means we don't have any
> possible descs to be sent, so it should return true instead to tell the
> napi poll not to launch another poll to handle tx packets.

I do not think such reasoning is correct. If you look at the current mature 
implementation in i40e and ice, it always returns (nb_pkts < budget), so when 
the budget is `0`, the napi will always be rescheduled. Zero unused descriptors 
means that the entire ring is held by HW, so it makes sense to retry to 
reclaim some resources ASAP. Also, zero unused normal descriptors does not mean 
there is no UMEM descriptors to process.

Please, remove the following lines and the patch should be fine:

+     if (!budget)
+             return true;

> Even though
> that patch behaves correctly by returning true in this case, it happens
> because of the unexpected underflow of the budget. Taking the current
> version of i40e_xmit_zc() as an example, it returns true as expected. ]
> Hence, this patch adds a standalone if statement of zero budget in front
> of ixgbe_xmit_zc() as explained before.
> 
> Use ixgbe_desc_unused to replace the original fixed budget with the number
> of available slots in the Tx ring. It can gain some performance.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index a463c5ac9c7c..f3d3f5c1cdc7 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -393,17 +393,14 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
>  	struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
>  	union ixgbe_adv_tx_desc *tx_desc = NULL;
>  	struct ixgbe_tx_buffer *tx_bi;
> -	bool work_done = true;
>  	struct xdp_desc desc;
>  	dma_addr_t dma;
>  	u32 cmd_type;
>  
> -	while (likely(budget)) {
> -		if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
> -			work_done = false;
> -			break;
> -		}
> +	if (!budget)
> +		return true;
>  
> +	while (likely(budget)) {
>  		if (!netif_carrier_ok(xdp_ring->netdev))
>  			break;
>  
> @@ -442,7 +439,7 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
>  		xsk_tx_release(pool);
>  	}
>  
> -	return !!budget && work_done;
> +	return !!budget;
>  }
>  
>  static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
> @@ -505,7 +502,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
>  	if (xsk_uses_need_wakeup(pool))
>  		xsk_set_tx_need_wakeup(pool);
>  
> -	return ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
> +	return ixgbe_xmit_zc(tx_ring, ixgbe_desc_unused(tx_ring));
>  }
>  
>  int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
> -- 
> 2.41.3
> 
> 

Re: [PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

[Maciej Fijalkowski]

On Sun, Jul 20, 2025 at 05:11:23PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Use the common interface to see if the desc is the end of packets. If
> so, set IXGBE_TXD_CMD_EOP bit instead of setting for all preceding
> descriptors. This is also how i40e driver did in commit a92b96c4ae10
> ("i40e: xsk: add TX multi-buffer support").
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 4 +++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index a59fd8f74b5e..c34737065f9e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -52,6 +52,8 @@
>  #include "ixgbe_txrx_common.h"
>  #include "devlink/devlink.h"
>  
> +#define IXGBE_MAX_BUFFER_TXD 4
> +
>  char ixgbe_driver_name[] = "ixgbe";
>  static const char ixgbe_driver_string[] =
>  			      "Intel(R) 10 Gigabit PCI Express Network Driver";
> @@ -11805,6 +11807,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
>  			       NETDEV_XDP_ACT_XSK_ZEROCOPY;
>  
> +	netdev->xdp_zc_max_segs = IXGBE_MAX_BUFFER_TXD;

Hi Jason,

nack to this as you would allow fragmented frames on Rx side which is not
supported even with your patchset.

Generally ixgbe needs some love, i have several patches in my backlog plus
I think Larysa will be focusing on this driver.

please stick to enabling xsk batching on tx side.

> +
>  	/* MTU range: 68 - 9710 */
>  	netdev->min_mtu = ETH_MIN_MTU;
>  	netdev->max_mtu = IXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index 9fe2c4bf8bc5..3d9fa4f2403e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -424,7 +424,9 @@ static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
>  	cmd_type = IXGBE_ADVTXD_DTYP_DATA |
>  		   IXGBE_ADVTXD_DCMD_DEXT |
>  		   IXGBE_ADVTXD_DCMD_IFCS;
> -	cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> +	cmd_type |= desc[i].len;
> +	if (xsk_is_eop_desc(&desc[i]))
> +		cmd_type |= IXGBE_TXD_CMD_EOP;
>  	tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
>  	tx_desc->read.olinfo_status =
>  		cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> -- 
> 2.41.3
> 

Re: [PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

[Jason Xing]

Hi Maciej,

On Fri, Jul 25, 2025 at 6:00 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Sun, Jul 20, 2025 at 05:11:23PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Use the common interface to see if the desc is the end of packets. If
> > so, set IXGBE_TXD_CMD_EOP bit instead of setting for all preceding
> > descriptors. This is also how i40e driver did in commit a92b96c4ae10
> > ("i40e: xsk: add TX multi-buffer support").
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 4 +++-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index a59fd8f74b5e..c34737065f9e 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -52,6 +52,8 @@
> >  #include "ixgbe_txrx_common.h"
> >  #include "devlink/devlink.h"
> >
> > +#define IXGBE_MAX_BUFFER_TXD 4
> > +
> >  char ixgbe_driver_name[] = "ixgbe";
> >  static const char ixgbe_driver_string[] =
> >                             "Intel(R) 10 Gigabit PCI Express Network Driver";
> > @@ -11805,6 +11807,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >       netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
> >                              NETDEV_XDP_ACT_XSK_ZEROCOPY;
> >
> > +     netdev->xdp_zc_max_segs = IXGBE_MAX_BUFFER_TXD;
>
> Hi Jason,
>
> nack to this as you would allow fragmented frames on Rx side which is not
> supported even with your patchset.

I'm not sure about this one, to be honest when I observed no
performance impact with this patch. How could we support the idea of
this patch, I wonder? Do we need to correspondingly adjust the
hardware? Sorry that I wasn't able to find such information in the
datasheet :(

>
> Generally ixgbe needs some love, i have several patches in my backlog plus
> I think Larysa will be focusing on this driver.

Though ixgbe is an old driver, we still have thousands of machines
running with this driver. Looking forward to your patch then.

Thanks,
Jason

>
> please stick to enabling xsk batching on tx side.
>
> > +
> >       /* MTU range: 68 - 9710 */
> >       netdev->min_mtu = ETH_MIN_MTU;
> >       netdev->max_mtu = IXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > index 9fe2c4bf8bc5..3d9fa4f2403e 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > @@ -424,7 +424,9 @@ static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
> >       cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> >                  IXGBE_ADVTXD_DCMD_DEXT |
> >                  IXGBE_ADVTXD_DCMD_IFCS;
> > -     cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> > +     cmd_type |= desc[i].len;
> > +     if (xsk_is_eop_desc(&desc[i]))
> > +             cmd_type |= IXGBE_TXD_CMD_EOP;
> >       tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> >       tx_desc->read.olinfo_status =
> >               cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > --
> > 2.41.3
> >

Re: [PATCH net-next 4/5] ixgbe: xsk: support batched xsk Tx interfaces to increase performance

[Larysa Zaremba]

On Sun, Jul 20, 2025 at 05:11:22PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Like what i40e driver initially did in commit 3106c580fb7cf
> ("i40e: Use batched xsk Tx interfaces to increase performance"), use
> the batched xsk feature to transmit packets.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 106 +++++++++++++------
>  1 file changed, 72 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index f3d3f5c1cdc7..9fe2c4bf8bc5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -2,12 +2,15 @@
>  /* Copyright(c) 2018 Intel Corporation. */
>  
>  #include <linux/bpf_trace.h>
> +#include <linux/unroll.h>
>  #include <net/xdp_sock_drv.h>
>  #include <net/xdp.h>
>  
>  #include "ixgbe.h"
>  #include "ixgbe_txrx_common.h"
>  
> +#define PKTS_PER_BATCH 4
> +
>  struct xsk_buff_pool *ixgbe_xsk_pool(struct ixgbe_adapter *adapter,
>  				     struct ixgbe_ring *ring)
>  {
> @@ -388,58 +391,93 @@ void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring)
>  	}
>  }
>  
> -static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> +static void ixgbe_set_rs_bit(struct ixgbe_ring *xdp_ring)
> +{
> +	u16 ntu = xdp_ring->next_to_use ? xdp_ring->next_to_use - 1 : xdp_ring->count - 1;
> +	union ixgbe_adv_tx_desc *tx_desc;
> +
> +	tx_desc = IXGBE_TX_DESC(xdp_ring, ntu);
> +	tx_desc->read.cmd_type_len |= cpu_to_le32(IXGBE_TXD_CMD_RS);
> +}
> +
> +static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
> +			   int i)
> +

`i` parameter seems redundant here, why not just pass desc + i as a parameter?

>  {
>  	struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
>  	union ixgbe_adv_tx_desc *tx_desc = NULL;
>  	struct ixgbe_tx_buffer *tx_bi;
> -	struct xdp_desc desc;
>  	dma_addr_t dma;
>  	u32 cmd_type;
>  
> -	if (!budget)
> -		return true;
> +	dma = xsk_buff_raw_get_dma(pool, desc[i].addr);
> +	xsk_buff_raw_dma_sync_for_device(pool, dma, desc[i].len);
>  
> -	while (likely(budget)) {
> -		if (!netif_carrier_ok(xdp_ring->netdev))
> -			break;
> +	tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
> +	tx_bi->bytecount = desc[i].len;
> +	tx_bi->xdpf = NULL;
> +	tx_bi->gso_segs = 1;
>  
> -		if (!xsk_tx_peek_desc(pool, &desc))
> -			break;
> +	tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
> +	tx_desc->read.buffer_addr = cpu_to_le64(dma);
>  
> -		dma = xsk_buff_raw_get_dma(pool, desc.addr);
> -		xsk_buff_raw_dma_sync_for_device(pool, dma, desc.len);
> +	cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> +		   IXGBE_ADVTXD_DCMD_DEXT |
> +		   IXGBE_ADVTXD_DCMD_IFCS;
> +	cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> +	tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> +	tx_desc->read.olinfo_status =
> +		cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
>  
> -		tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
> -		tx_bi->bytecount = desc.len;
> -		tx_bi->xdpf = NULL;
> -		tx_bi->gso_segs = 1;
> +	xdp_ring->next_to_use++;
> +}
>  
> -		tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
> -		tx_desc->read.buffer_addr = cpu_to_le64(dma);
> +static void ixgbe_xmit_pkt_batch(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc)
> +{
> +	u32 i;
>  
> -		/* put descriptor type bits */
> -		cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> -			   IXGBE_ADVTXD_DCMD_DEXT |
> -			   IXGBE_ADVTXD_DCMD_IFCS;
> -		cmd_type |= desc.len | IXGBE_TXD_CMD;
> -		tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> -		tx_desc->read.olinfo_status =
> -			cpu_to_le32(desc.len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +	unrolled_count(PKTS_PER_BATCH)
> +	for (i = 0; i < PKTS_PER_BATCH; i++)
> +		ixgbe_xmit_pkt(xdp_ring, desc, i);
> +}
>  
> -		xdp_ring->next_to_use++;
> -		if (xdp_ring->next_to_use == xdp_ring->count)
> -			xdp_ring->next_to_use = 0;
> +static void ixgbe_fill_tx_hw_ring(struct ixgbe_ring *xdp_ring,
> +				  struct xdp_desc *descs, u32 nb_pkts)
> +{
> +	u32 batched, leftover, i;
> +
> +	batched = nb_pkts & ~(PKTS_PER_BATCH - 1);
> +	leftover = nb_pkts & (PKTS_PER_BATCH - 1);
> +	for (i = 0; i < batched; i += PKTS_PER_BATCH)
> +		ixgbe_xmit_pkt_batch(xdp_ring, &descs[i]);
> +	for (i = batched; i < batched + leftover; i++)
> +		ixgbe_xmit_pkt(xdp_ring, &descs[i], 0);
> +}
>  
> -		budget--;
> -	}
> +static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> +{
> +	struct xdp_desc *descs = xdp_ring->xsk_pool->tx_descs;
> +	u32 nb_pkts, nb_processed = 0;
>  
> -	if (tx_desc) {
> -		ixgbe_xdp_ring_update_tail(xdp_ring);
> -		xsk_tx_release(pool);
> +	if (!netif_carrier_ok(xdp_ring->netdev))
> +		return true;
> +
> +	nb_pkts = xsk_tx_peek_release_desc_batch(xdp_ring->xsk_pool, budget);
> +	if (!nb_pkts)
> +		return true;
> +
> +	if (xdp_ring->next_to_use + nb_pkts >= xdp_ring->count) {
> +		nb_processed = xdp_ring->count - xdp_ring->next_to_use;
> +		ixgbe_fill_tx_hw_ring(xdp_ring, descs, nb_processed);
> +		xdp_ring->next_to_use = 0;
>  	}
>  
> -	return !!budget;
> +	ixgbe_fill_tx_hw_ring(xdp_ring, &descs[nb_processed], nb_pkts - nb_processed);
> +
> +	ixgbe_set_rs_bit(xdp_ring);
> +	ixgbe_xdp_ring_update_tail(xdp_ring);
> +
> +	return nb_pkts < budget;
>  }
>  
>  static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
> -- 
> 2.41.3
> 
> 

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Larysa Zaremba]

On Fri, Jul 25, 2025 at 07:18:11AM +0800, Jason Xing wrote:
> Hi Tony,
> 
> On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
> >
> >
> >
> > On 7/20/2025 2:11 AM, Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > Resolve the budget underflow 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 underflow
> > > never happens.
> > >
> > > Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
> >
> > Hi Jason,
> >
> > Seems like this one should be split off and go to iwl-net/net like the
> > other similar ones [1]? Also, some of the updates made to the other
> > series apply here as well?
> 
> The other three patches are built on top of this one. If without the
> patch, the whole series will be warned because of build failure. I was
> thinking we could backport this patch that will be backported to the
> net branch after the whole series goes into the net-next branch.
> 
> Or you expect me to cook four patches without this one first so that
> you could easily cherry pick this one then without building conflict?
> 
> >
> > Thanks,
> > Tony
> >
> > [1]
> > https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/
> 
> Regarding this one, should I send a v4 version with the current patch
> included? And target [iwl-net/net] explicitly as well?
> 
> I'm not sure if I follow you. Could you instruct me on how to proceed
> next in detail?
>

What I usually do is send the fix as soon as I have it. While I prepare and test 
the series, the fix usually manages to get into the tree. Advise you do the 
same, given you have things to change in v2, but the fix can be resent almost 
as it is now (just change the tree).

Tony can have a different opinion though.
 
> Thanks,
> Jason
> 

Re: [PATCH net-next 5/5] ixgbe: xsk: add TX multi-buffer support

[Maciej Fijalkowski]

On Fri, Jul 25, 2025 at 06:09:09PM +0800, Jason Xing wrote:
> Hi Maciej,
> 
> On Fri, Jul 25, 2025 at 6:00 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Sun, Jul 20, 2025 at 05:11:23PM +0800, Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > Use the common interface to see if the desc is the end of packets. If
> > > so, set IXGBE_TXD_CMD_EOP bit instead of setting for all preceding
> > > descriptors. This is also how i40e driver did in commit a92b96c4ae10
> > > ("i40e: xsk: add TX multi-buffer support").
> > >
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 4 +++-
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > index a59fd8f74b5e..c34737065f9e 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > @@ -52,6 +52,8 @@
> > >  #include "ixgbe_txrx_common.h"
> > >  #include "devlink/devlink.h"
> > >
> > > +#define IXGBE_MAX_BUFFER_TXD 4
> > > +
> > >  char ixgbe_driver_name[] = "ixgbe";
> > >  static const char ixgbe_driver_string[] =
> > >                             "Intel(R) 10 Gigabit PCI Express Network Driver";
> > > @@ -11805,6 +11807,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >       netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
> > >                              NETDEV_XDP_ACT_XSK_ZEROCOPY;
> > >
> > > +     netdev->xdp_zc_max_segs = IXGBE_MAX_BUFFER_TXD;
> >
> > Hi Jason,
> >
> > nack to this as you would allow fragmented frames on Rx side which is not
> > supported even with your patchset.
> 
> I'm not sure about this one, to be honest when I observed no
> performance impact with this patch. How could we support the idea of
> this patch, I wonder? Do we need to correspondingly adjust the
> hardware? Sorry that I wasn't able to find such information in the
> datasheet :(

setting xdp_zc_max_segs will cause xsk control path to allow multi-buffer
traffic for zero-copy driver. ixgbe_clean_rx_irq_zc() is not adjusted for
that.

> 
> >
> > Generally ixgbe needs some love, i have several patches in my backlog plus
> > I think Larysa will be focusing on this driver.
> 
> Though ixgbe is an old driver, we still have thousands of machines
> running with this driver. Looking forward to your patch then.

I am taking a note here and will try to raise the priority around ixgbe
work, thanks!

> 
> Thanks,
> Jason
> 
> >
> > please stick to enabling xsk batching on tx side.
> >
> > > +
> > >       /* MTU range: 68 - 9710 */
> > >       netdev->min_mtu = ETH_MIN_MTU;
> > >       netdev->max_mtu = IXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > > index 9fe2c4bf8bc5..3d9fa4f2403e 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > > @@ -424,7 +424,9 @@ static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
> > >       cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> > >                  IXGBE_ADVTXD_DCMD_DEXT |
> > >                  IXGBE_ADVTXD_DCMD_IFCS;
> > > -     cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> > > +     cmd_type |= desc[i].len;
> > > +     if (xsk_is_eop_desc(&desc[i]))
> > > +             cmd_type |= IXGBE_TXD_CMD_EOP;
> > >       tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> > >       tx_desc->read.olinfo_status =
> > >               cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > > --
> > > 2.41.3
> > >

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Jason Xing]

On Fri, Jul 25, 2025 at 6:58 PM Larysa Zaremba <larysa.zaremba@intel.com> wrote:
>
> On Fri, Jul 25, 2025 at 07:18:11AM +0800, Jason Xing wrote:
> > Hi Tony,
> >
> > On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
> > >
> > >
> > >
> > > On 7/20/2025 2:11 AM, Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > Resolve the budget underflow 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 underflow
> > > > never happens.
> > > >
> > > > Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
> > >
> > > Hi Jason,
> > >
> > > Seems like this one should be split off and go to iwl-net/net like the
> > > other similar ones [1]? Also, some of the updates made to the other
> > > series apply here as well?
> >
> > The other three patches are built on top of this one. If without the
> > patch, the whole series will be warned because of build failure. I was
> > thinking we could backport this patch that will be backported to the
> > net branch after the whole series goes into the net-next branch.
> >
> > Or you expect me to cook four patches without this one first so that
> > you could easily cherry pick this one then without building conflict?
> >
> > >
> > > Thanks,
> > > Tony
> > >
> > > [1]
> > > https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/
> >
> > Regarding this one, should I send a v4 version with the current patch
> > included? And target [iwl-net/net] explicitly as well?
> >
> > I'm not sure if I follow you. Could you instruct me on how to proceed
> > next in detail?
> >
>
> What I usually do is send the fix as soon as I have it. While I prepare and test
> the series, the fix usually manages to get into the tree. Advise you do the

I see, but this series is built on top of this patch, so in V2 I
should still cook these three patches based on the current patch?

> same, given you have things to change in v2, but the fix can be resent almost
> as it is now (just change the tree).

Got it, I will send it soon as a standalone patch.

Thanks,
Jason

>
> Tony can have a different opinion though.
>
> > Thanks,
> > Jason
> >

Re: [PATCH net-next 4/5] ixgbe: xsk: support batched xsk Tx interfaces to increase performance

[Jason Xing]

On Fri, Jul 25, 2025 at 6:52 PM Larysa Zaremba <larysa.zaremba@intel.com> wrote:
>
> On Sun, Jul 20, 2025 at 05:11:22PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Like what i40e driver initially did in commit 3106c580fb7cf
> > ("i40e: Use batched xsk Tx interfaces to increase performance"), use
> > the batched xsk feature to transmit packets.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 106 +++++++++++++------
> >  1 file changed, 72 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > index f3d3f5c1cdc7..9fe2c4bf8bc5 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > @@ -2,12 +2,15 @@
> >  /* Copyright(c) 2018 Intel Corporation. */
> >
> >  #include <linux/bpf_trace.h>
> > +#include <linux/unroll.h>
> >  #include <net/xdp_sock_drv.h>
> >  #include <net/xdp.h>
> >
> >  #include "ixgbe.h"
> >  #include "ixgbe_txrx_common.h"
> >
> > +#define PKTS_PER_BATCH 4
> > +
> >  struct xsk_buff_pool *ixgbe_xsk_pool(struct ixgbe_adapter *adapter,
> >                                    struct ixgbe_ring *ring)
> >  {
> > @@ -388,58 +391,93 @@ void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring)
> >       }
> >  }
> >
> > -static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> > +static void ixgbe_set_rs_bit(struct ixgbe_ring *xdp_ring)
> > +{
> > +     u16 ntu = xdp_ring->next_to_use ? xdp_ring->next_to_use - 1 : xdp_ring->count - 1;
> > +     union ixgbe_adv_tx_desc *tx_desc;
> > +
> > +     tx_desc = IXGBE_TX_DESC(xdp_ring, ntu);
> > +     tx_desc->read.cmd_type_len |= cpu_to_le32(IXGBE_TXD_CMD_RS);
> > +}
> > +
> > +static void ixgbe_xmit_pkt(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc,
> > +                        int i)
> > +
>
> `i` parameter seems redundant here, why not just pass desc + i as a parameter?

Let me resolve this :)

Thanks,
Jason

>
> >  {
> >       struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
> >       union ixgbe_adv_tx_desc *tx_desc = NULL;
> >       struct ixgbe_tx_buffer *tx_bi;
> > -     struct xdp_desc desc;
> >       dma_addr_t dma;
> >       u32 cmd_type;
> >
> > -     if (!budget)
> > -             return true;
> > +     dma = xsk_buff_raw_get_dma(pool, desc[i].addr);
> > +     xsk_buff_raw_dma_sync_for_device(pool, dma, desc[i].len);
> >
> > -     while (likely(budget)) {
> > -             if (!netif_carrier_ok(xdp_ring->netdev))
> > -                     break;
> > +     tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
> > +     tx_bi->bytecount = desc[i].len;
> > +     tx_bi->xdpf = NULL;
> > +     tx_bi->gso_segs = 1;
> >
> > -             if (!xsk_tx_peek_desc(pool, &desc))
> > -                     break;
> > +     tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
> > +     tx_desc->read.buffer_addr = cpu_to_le64(dma);
> >
> > -             dma = xsk_buff_raw_get_dma(pool, desc.addr);
> > -             xsk_buff_raw_dma_sync_for_device(pool, dma, desc.len);
> > +     cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> > +                IXGBE_ADVTXD_DCMD_DEXT |
> > +                IXGBE_ADVTXD_DCMD_IFCS;
> > +     cmd_type |= desc[i].len | IXGBE_TXD_CMD_EOP;
> > +     tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> > +     tx_desc->read.olinfo_status =
> > +             cpu_to_le32(desc[i].len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> >
> > -             tx_bi = &xdp_ring->tx_buffer_info[xdp_ring->next_to_use];
> > -             tx_bi->bytecount = desc.len;
> > -             tx_bi->xdpf = NULL;
> > -             tx_bi->gso_segs = 1;
> > +     xdp_ring->next_to_use++;
> > +}
> >
> > -             tx_desc = IXGBE_TX_DESC(xdp_ring, xdp_ring->next_to_use);
> > -             tx_desc->read.buffer_addr = cpu_to_le64(dma);
> > +static void ixgbe_xmit_pkt_batch(struct ixgbe_ring *xdp_ring, struct xdp_desc *desc)
> > +{
> > +     u32 i;
> >
> > -             /* put descriptor type bits */
> > -             cmd_type = IXGBE_ADVTXD_DTYP_DATA |
> > -                        IXGBE_ADVTXD_DCMD_DEXT |
> > -                        IXGBE_ADVTXD_DCMD_IFCS;
> > -             cmd_type |= desc.len | IXGBE_TXD_CMD;
> > -             tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
> > -             tx_desc->read.olinfo_status =
> > -                     cpu_to_le32(desc.len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +     unrolled_count(PKTS_PER_BATCH)
> > +     for (i = 0; i < PKTS_PER_BATCH; i++)
> > +             ixgbe_xmit_pkt(xdp_ring, desc, i);
> > +}
> >
> > -             xdp_ring->next_to_use++;
> > -             if (xdp_ring->next_to_use == xdp_ring->count)
> > -                     xdp_ring->next_to_use = 0;
> > +static void ixgbe_fill_tx_hw_ring(struct ixgbe_ring *xdp_ring,
> > +                               struct xdp_desc *descs, u32 nb_pkts)
> > +{
> > +     u32 batched, leftover, i;
> > +
> > +     batched = nb_pkts & ~(PKTS_PER_BATCH - 1);
> > +     leftover = nb_pkts & (PKTS_PER_BATCH - 1);
> > +     for (i = 0; i < batched; i += PKTS_PER_BATCH)
> > +             ixgbe_xmit_pkt_batch(xdp_ring, &descs[i]);
> > +     for (i = batched; i < batched + leftover; i++)
> > +             ixgbe_xmit_pkt(xdp_ring, &descs[i], 0);
> > +}
> >
> > -             budget--;
> > -     }
> > +static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> > +{
> > +     struct xdp_desc *descs = xdp_ring->xsk_pool->tx_descs;
> > +     u32 nb_pkts, nb_processed = 0;
> >
> > -     if (tx_desc) {
> > -             ixgbe_xdp_ring_update_tail(xdp_ring);
> > -             xsk_tx_release(pool);
> > +     if (!netif_carrier_ok(xdp_ring->netdev))
> > +             return true;
> > +
> > +     nb_pkts = xsk_tx_peek_release_desc_batch(xdp_ring->xsk_pool, budget);
> > +     if (!nb_pkts)
> > +             return true;
> > +
> > +     if (xdp_ring->next_to_use + nb_pkts >= xdp_ring->count) {
> > +             nb_processed = xdp_ring->count - xdp_ring->next_to_use;
> > +             ixgbe_fill_tx_hw_ring(xdp_ring, descs, nb_processed);
> > +             xdp_ring->next_to_use = 0;
> >       }
> >
> > -     return !!budget;
> > +     ixgbe_fill_tx_hw_ring(xdp_ring, &descs[nb_processed], nb_pkts - nb_processed);
> > +
> > +     ixgbe_set_rs_bit(xdp_ring);
> > +     ixgbe_xdp_ring_update_tail(xdp_ring);
> > +
> > +     return nb_pkts < budget;
> >  }
> >
> >  static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
> > --
> > 2.41.3
> >
> >

Re: [PATCH net-next 3/5] ixgbe: xsk: use ixgbe_desc_unused as the budget in ixgbe_xmit_zc

[Jason Xing]

On Fri, Jul 25, 2025 at 5:45 PM Larysa Zaremba <larysa.zaremba@intel.com> wrote:
>
> On Sun, Jul 20, 2025 at 05:11:21PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > - Adjust ixgbe_desc_unused as the budget value.
> > - Avoid checking desc_unused over and over again in the loop.
> >
> > The patch makes ixgbe follow i40e driver that was done in commit
> > 1fd972ebe523 ("i40e: move check of full Tx ring to outside of send loop").
> > [ Note that the above i40e patch has problem when ixgbe_desc_unused(tx_ring)
> > returns zero. The zero value as the budget value means we don't have any
> > possible descs to be sent, so it should return true instead to tell the
> > napi poll not to launch another poll to handle tx packets.
>
> I do not think such reasoning is correct. If you look at the current mature
> implementation in i40e and ice, it always returns (nb_pkts < budget), so when
> the budget is `0`, the napi will always be rescheduled. Zero unused descriptors

Sorry, I'm afraid I don't think so. In ice_xmit_zc(), if the budget is
zero, it will return true because of the following codes:
nb_pkts = xsk_tx_peek_release_desc_batch(xsk_pool, budget);
if (!nb_pkts)
        return true;

Supposing there is no single desc in the tx ring, the budget will
always be zero even when the napi poll is triggered.

Thanks,
Jason

> means that the entire ring is held by HW, so it makes sense to retry to
> reclaim some resources ASAP. Also, zero unused normal descriptors does not mean
> there is no UMEM descriptors to process.
>
> Please, remove the following lines and the patch should be fine:
>
> +     if (!budget)
> +             return true;
>
> > Even though
> > that patch behaves correctly by returning true in this case, it happens
> > because of the unexpected underflow of the budget. Taking the current
> > version of i40e_xmit_zc() as an example, it returns true as expected. ]
> > Hence, this patch adds a standalone if statement of zero budget in front
> > of ixgbe_xmit_zc() as explained before.
> >
> > Use ixgbe_desc_unused to replace the original fixed budget with the number
> > of available slots in the Tx ring. It can gain some performance.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > index a463c5ac9c7c..f3d3f5c1cdc7 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > @@ -393,17 +393,14 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> >       struct xsk_buff_pool *pool = xdp_ring->xsk_pool;
> >       union ixgbe_adv_tx_desc *tx_desc = NULL;
> >       struct ixgbe_tx_buffer *tx_bi;
> > -     bool work_done = true;
> >       struct xdp_desc desc;
> >       dma_addr_t dma;
> >       u32 cmd_type;
> >
> > -     while (likely(budget)) {
> > -             if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
> > -                     work_done = false;
> > -                     break;
> > -             }
> > +     if (!budget)
> > +             return true;
> >
> > +     while (likely(budget)) {
> >               if (!netif_carrier_ok(xdp_ring->netdev))
> >                       break;
> >
> > @@ -442,7 +439,7 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
> >               xsk_tx_release(pool);
> >       }
> >
> > -     return !!budget && work_done;
> > +     return !!budget;
> >  }
> >
> >  static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
> > @@ -505,7 +502,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
> >       if (xsk_uses_need_wakeup(pool))
> >               xsk_set_tx_need_wakeup(pool);
> >
> > -     return ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
> > +     return ixgbe_xmit_zc(tx_ring, ixgbe_desc_unused(tx_ring));
> >  }
> >
> >  int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
> > --
> > 2.41.3
> >
> >

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Tony Nguyen]

On 7/25/2025 3:57 AM, Larysa Zaremba wrote:
> On Fri, Jul 25, 2025 at 07:18:11AM +0800, Jason Xing wrote:
>> Hi Tony,
>>
>> On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>>>
>>>
>>>
>>> On 7/20/2025 2:11 AM, Jason Xing wrote:
>>>> From: Jason Xing <kernelxing@tencent.com>
>>>>
>>>> Resolve the budget underflow 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 underflow
>>>> never happens.
>>>>
>>>> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
>>>
>>> Hi Jason,
>>>
>>> Seems like this one should be split off and go to iwl-net/net like the
>>> other similar ones [1]? Also, some of the updates made to the other
>>> series apply here as well?
>>
>> The other three patches are built on top of this one. If without the
>> patch, the whole series will be warned because of build failure. I was
>> thinking we could backport this patch that will be backported to the
>> net branch after the whole series goes into the net-next branch.
>>
>> Or you expect me to cook four patches without this one first so that
>> you could easily cherry pick this one then without building conflict?
>>
>>>
>>> Thanks,
>>> Tony
>>>
>>> [1]
>>> https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/
>>
>> Regarding this one, should I send a v4 version with the current patch
>> included? And target [iwl-net/net] explicitly as well?
>>
>> I'm not sure if I follow you. Could you instruct me on how to proceed
>> next in detail?
>>
> 
> What I usually do is send the fix as soon as I have it. While I prepare and test
> the series, the fix usually manages to get into the tree. Advise you do the
> same, given you have things to change in v2, but the fix can be resent almost
> as it is now (just change the tree).
> 
> Tony can have a different opinion though.

I agree. Normally in these situations, send the fix first and after that 
one is
applied, the other patches can be sent.
This patch would've fit in nice with the other series, however, as that 
one is already in process and this one can standalone. I would send this 
fix by itself.

Thanks,
Tony

Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit_zc

[Jason Xing]

On Sat, Jul 26, 2025 at 12:54 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
>
>
> On 7/25/2025 3:57 AM, Larysa Zaremba wrote:
> > On Fri, Jul 25, 2025 at 07:18:11AM +0800, Jason Xing wrote:
> >> Hi Tony,
> >>
> >> On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
> >>>
> >>>
> >>>
> >>> On 7/20/2025 2:11 AM, Jason Xing wrote:
> >>>> From: Jason Xing <kernelxing@tencent.com>
> >>>>
> >>>> Resolve the budget underflow 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 underflow
> >>>> never happens.
> >>>>
> >>>> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
> >>>
> >>> Hi Jason,
> >>>
> >>> Seems like this one should be split off and go to iwl-net/net like the
> >>> other similar ones [1]? Also, some of the updates made to the other
> >>> series apply here as well?
> >>
> >> The other three patches are built on top of this one. If without the
> >> patch, the whole series will be warned because of build failure. I was
> >> thinking we could backport this patch that will be backported to the
> >> net branch after the whole series goes into the net-next branch.
> >>
> >> Or you expect me to cook four patches without this one first so that
> >> you could easily cherry pick this one then without building conflict?
> >>
> >>>
> >>> Thanks,
> >>> Tony
> >>>
> >>> [1]
> >>> https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/
> >>
> >> Regarding this one, should I send a v4 version with the current patch
> >> included? And target [iwl-net/net] explicitly as well?
> >>
> >> I'm not sure if I follow you. Could you instruct me on how to proceed
> >> next in detail?
> >>
> >
> > What I usually do is send the fix as soon as I have it. While I prepare and test
> > the series, the fix usually manages to get into the tree. Advise you do the
> > same, given you have things to change in v2, but the fix can be resent almost
> > as it is now (just change the tree).
> >
> > Tony can have a different opinion though.
>
> I agree. Normally in these situations, send the fix first and after that
> one is
> applied, the other patches can be sent.
> This patch would've fit in nice with the other series, however, as that
> one is already in process and this one can standalone. I would send this
> fix by itself.

Got it. I will leave those two fixes as they are and send this one
targetting the right branch as soon as possible today.

Thanks,
Jason