[PATCH net-next 0/2] xsk: improvement performance in copy mode

[PATCH net-next 0/2] xsk: improvement performance in copy mode

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Like in VM using virtio_net, there are not that many machines supporting
advanced function like multi-buffer and zerocopy. Using xsk copy mode
becomes a default choice to support bypass kernel feature instead of
resorting to DPDK.

Prior to this series, zerocopy mode has a better performance than copy
mode. But now, the copy mode outperforms zc mode by 12.9%, which was
tested on ixgbe driver by means of xdpsock.

The thought behind this series is to aggregate packets in a certain
small group like GSO/GRO and then send them at one time by only grabbing
the tx queue and disable bh once.

Jason Xing (2):
  xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt
  xsk: support generic batch xmit in copy mode

 Documentation/networking/af_xdp.rst |   9 ++
 include/linux/netdevice.h           |   2 +
 include/net/xdp_sock.h              |   2 +
 include/uapi/linux/if_xdp.h         |   1 +
 net/core/dev.c                      |  18 ++++
 net/xdp/xsk.c                       | 135 +++++++++++++++++++++++++++-
 tools/include/uapi/linux/if_xdp.h   |   1 +
 7 files changed, 165 insertions(+), 3 deletions(-)

-- 
2.41.3

[PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

This patch is to prepare for later batch xmit in generic path. Add a new
socket option to provide an alternative to achieve a higher overall
throughput.

skb_batch will be used to store newly allocated skb at one time in the
xmit path.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 Documentation/networking/af_xdp.rst |  9 ++++++++
 include/net/xdp_sock.h              |  2 ++
 include/uapi/linux/if_xdp.h         |  1 +
 net/xdp/xsk.c                       | 32 +++++++++++++++++++++++++++++
 tools/include/uapi/linux/if_xdp.h   |  1 +
 5 files changed, 45 insertions(+)

diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
index 50d92084a49c..1194bdfaf61e 100644
--- a/Documentation/networking/af_xdp.rst
+++ b/Documentation/networking/af_xdp.rst
@@ -447,6 +447,15 @@ mode to allow application to tune the per-socket maximum iteration for
 better throughput and less frequency of send syscall.
 Allowed range is [32, xs->tx->nentries].
 
+XDP_GENERIC_XMIT_BATCH
+----------------------
+
+It provides an option that allows application to use batch xmit in the copy
+mode. Batch process minimizes the number of grabbing/releasing queue lock
+without redundant actions compared to before to gain the overall performance
+improvement whereas it might increase the latency of per packet. The maximum
+value shouldn't be larger than xs->max_tx_budget.
+
 XDP_STATISTICS getsockopt
 -------------------------
 
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index ce587a225661..b5a3e37da8db 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -61,6 +61,7 @@ struct xdp_sock {
 		XSK_BOUND,
 		XSK_UNBOUND,
 	} state;
+	struct sk_buff **skb_batch;
 
 	struct xsk_queue *tx ____cacheline_aligned_in_smp;
 	struct list_head tx_list;
@@ -70,6 +71,7 @@ struct xdp_sock {
 	 * preventing other XSKs from being starved.
 	 */
 	u32 tx_budget_spent;
+	u32 generic_xmit_batch;
 
 	/* Statistics */
 	u64 rx_dropped;
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
index 23a062781468..44cb72cd328e 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
 #define XDP_STATISTICS			7
 #define XDP_OPTIONS			8
 #define XDP_MAX_TX_SKB_BUDGET		9
+#define XDP_GENERIC_XMIT_BATCH		10
 
 struct xdp_umem_reg {
 	__u64 addr; /* Start of packet data area */
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 9c3acecc14b1..7a149f4ac273 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1122,6 +1122,7 @@ static int xsk_release(struct socket *sock)
 	xskq_destroy(xs->tx);
 	xskq_destroy(xs->fq_tmp);
 	xskq_destroy(xs->cq_tmp);
+	kfree(xs->skb_batch);
 
 	sock_orphan(sk);
 	sock->sk = NULL;
@@ -1456,6 +1457,37 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
 		WRITE_ONCE(xs->max_tx_budget, budget);
 		return 0;
 	}
+	case XDP_GENERIC_XMIT_BATCH:
+	{
+		unsigned int batch, batch_alloc_len;
+		struct sk_buff **new;
+
+		if (optlen != sizeof(batch))
+			return -EINVAL;
+		if (copy_from_sockptr(&batch, optval, sizeof(batch)))
+			return -EFAULT;
+		if (batch > xs->max_tx_budget)
+			return -EACCES;
+
+		mutex_lock(&xs->mutex);
+		if (!batch) {
+			kfree(xs->skb_batch);
+			xs->generic_xmit_batch = 0;
+			goto out;
+		}
+		batch_alloc_len = sizeof(struct sk_buff *) * batch;
+		new = kmalloc(batch_alloc_len, GFP_KERNEL);
+		if (!new)
+			return -ENOMEM;
+		if (xs->skb_batch)
+			kfree(xs->skb_batch);
+
+		xs->skb_batch = new;
+		xs->generic_xmit_batch = batch;
+out:
+		mutex_unlock(&xs->mutex);
+		return 0;
+	}
 	default:
 		break;
 	}
diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index 23a062781468..44cb72cd328e 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
 #define XDP_STATISTICS			7
 #define XDP_OPTIONS			8
 #define XDP_MAX_TX_SKB_BUDGET		9
+#define XDP_GENERIC_XMIT_BATCH		10
 
 struct xdp_umem_reg {
 	__u64 addr; /* Start of packet data area */
-- 
2.41.3

Re: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[Maciej Fijalkowski]

On Mon, Aug 11, 2025 at 09:12:35PM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> This patch is to prepare for later batch xmit in generic path. Add a new
> socket option to provide an alternative to achieve a higher overall
> throughput.
> 
> skb_batch will be used to store newly allocated skb at one time in the
> xmit path.

I don't think we need yet another setsockopt. You previously added a knob
for manipulating max tx budget on generic xmit and that should be enough.
I think that we should strive for making the batching approach a default
path in xsk generic xmit.

> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  Documentation/networking/af_xdp.rst |  9 ++++++++
>  include/net/xdp_sock.h              |  2 ++
>  include/uapi/linux/if_xdp.h         |  1 +
>  net/xdp/xsk.c                       | 32 +++++++++++++++++++++++++++++
>  tools/include/uapi/linux/if_xdp.h   |  1 +
>  5 files changed, 45 insertions(+)
> 
> diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
> index 50d92084a49c..1194bdfaf61e 100644
> --- a/Documentation/networking/af_xdp.rst
> +++ b/Documentation/networking/af_xdp.rst
> @@ -447,6 +447,15 @@ mode to allow application to tune the per-socket maximum iteration for
>  better throughput and less frequency of send syscall.
>  Allowed range is [32, xs->tx->nentries].
>  
> +XDP_GENERIC_XMIT_BATCH
> +----------------------
> +
> +It provides an option that allows application to use batch xmit in the copy
> +mode. Batch process minimizes the number of grabbing/releasing queue lock
> +without redundant actions compared to before to gain the overall performance
> +improvement whereas it might increase the latency of per packet. The maximum
> +value shouldn't be larger than xs->max_tx_budget.
> +
>  XDP_STATISTICS getsockopt
>  -------------------------
>  
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index ce587a225661..b5a3e37da8db 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -61,6 +61,7 @@ struct xdp_sock {
>  		XSK_BOUND,
>  		XSK_UNBOUND,
>  	} state;
> +	struct sk_buff **skb_batch;
>  
>  	struct xsk_queue *tx ____cacheline_aligned_in_smp;
>  	struct list_head tx_list;
> @@ -70,6 +71,7 @@ struct xdp_sock {
>  	 * preventing other XSKs from being starved.
>  	 */
>  	u32 tx_budget_spent;
> +	u32 generic_xmit_batch;
>  
>  	/* Statistics */
>  	u64 rx_dropped;
> diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
> index 23a062781468..44cb72cd328e 100644
> --- a/include/uapi/linux/if_xdp.h
> +++ b/include/uapi/linux/if_xdp.h
> @@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
>  #define XDP_STATISTICS			7
>  #define XDP_OPTIONS			8
>  #define XDP_MAX_TX_SKB_BUDGET		9
> +#define XDP_GENERIC_XMIT_BATCH		10
>  
>  struct xdp_umem_reg {
>  	__u64 addr; /* Start of packet data area */
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 9c3acecc14b1..7a149f4ac273 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1122,6 +1122,7 @@ static int xsk_release(struct socket *sock)
>  	xskq_destroy(xs->tx);
>  	xskq_destroy(xs->fq_tmp);
>  	xskq_destroy(xs->cq_tmp);
> +	kfree(xs->skb_batch);
>  
>  	sock_orphan(sk);
>  	sock->sk = NULL;
> @@ -1456,6 +1457,37 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
>  		WRITE_ONCE(xs->max_tx_budget, budget);
>  		return 0;
>  	}
> +	case XDP_GENERIC_XMIT_BATCH:
> +	{
> +		unsigned int batch, batch_alloc_len;
> +		struct sk_buff **new;
> +
> +		if (optlen != sizeof(batch))
> +			return -EINVAL;
> +		if (copy_from_sockptr(&batch, optval, sizeof(batch)))
> +			return -EFAULT;
> +		if (batch > xs->max_tx_budget)
> +			return -EACCES;
> +
> +		mutex_lock(&xs->mutex);
> +		if (!batch) {
> +			kfree(xs->skb_batch);
> +			xs->generic_xmit_batch = 0;
> +			goto out;
> +		}
> +		batch_alloc_len = sizeof(struct sk_buff *) * batch;
> +		new = kmalloc(batch_alloc_len, GFP_KERNEL);
> +		if (!new)
> +			return -ENOMEM;
> +		if (xs->skb_batch)
> +			kfree(xs->skb_batch);
> +
> +		xs->skb_batch = new;
> +		xs->generic_xmit_batch = batch;
> +out:
> +		mutex_unlock(&xs->mutex);
> +		return 0;
> +	}
>  	default:
>  		break;
>  	}
> diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
> index 23a062781468..44cb72cd328e 100644
> --- a/tools/include/uapi/linux/if_xdp.h
> +++ b/tools/include/uapi/linux/if_xdp.h
> @@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
>  #define XDP_STATISTICS			7
>  #define XDP_OPTIONS			8
>  #define XDP_MAX_TX_SKB_BUDGET		9
> +#define XDP_GENERIC_XMIT_BATCH		10
>  
>  struct xdp_umem_reg {
>  	__u64 addr; /* Start of packet data area */
> -- 
> 2.41.3
> 

Re: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[Jason Xing]

On Wed, Aug 13, 2025 at 12:40 AM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Mon, Aug 11, 2025 at 09:12:35PM +0800, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > This patch is to prepare for later batch xmit in generic path. Add a new
> > socket option to provide an alternative to achieve a higher overall
> > throughput.
> >
> > skb_batch will be used to store newly allocated skb at one time in the
> > xmit path.
>
> I don't think we need yet another setsockopt. You previously added a knob
> for manipulating max tx budget on generic xmit and that should be enough.
> I think that we should strive for making the batching approach a default
> path in xsk generic xmit.

You're right, it‘s the right direction that we should take. But I
considered this as well before cooking the series and then gave up, my
experiments show that in some real cases (not xdpsock) the batch
process might increase latency. It's a side effect. At that time I
thought many years ago the invention of GRO didn't become the default.

Thanks,
Jason

>
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  Documentation/networking/af_xdp.rst |  9 ++++++++
> >  include/net/xdp_sock.h              |  2 ++
> >  include/uapi/linux/if_xdp.h         |  1 +
> >  net/xdp/xsk.c                       | 32 +++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/if_xdp.h   |  1 +
> >  5 files changed, 45 insertions(+)
> >
> > diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
> > index 50d92084a49c..1194bdfaf61e 100644
> > --- a/Documentation/networking/af_xdp.rst
> > +++ b/Documentation/networking/af_xdp.rst
> > @@ -447,6 +447,15 @@ mode to allow application to tune the per-socket maximum iteration for
> >  better throughput and less frequency of send syscall.
> >  Allowed range is [32, xs->tx->nentries].
> >
> > +XDP_GENERIC_XMIT_BATCH
> > +----------------------
> > +
> > +It provides an option that allows application to use batch xmit in the copy
> > +mode. Batch process minimizes the number of grabbing/releasing queue lock
> > +without redundant actions compared to before to gain the overall performance
> > +improvement whereas it might increase the latency of per packet. The maximum
> > +value shouldn't be larger than xs->max_tx_budget.
> > +
> >  XDP_STATISTICS getsockopt
> >  -------------------------
> >
> > diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> > index ce587a225661..b5a3e37da8db 100644
> > --- a/include/net/xdp_sock.h
> > +++ b/include/net/xdp_sock.h
> > @@ -61,6 +61,7 @@ struct xdp_sock {
> >               XSK_BOUND,
> >               XSK_UNBOUND,
> >       } state;
> > +     struct sk_buff **skb_batch;
> >
> >       struct xsk_queue *tx ____cacheline_aligned_in_smp;
> >       struct list_head tx_list;
> > @@ -70,6 +71,7 @@ struct xdp_sock {
> >        * preventing other XSKs from being starved.
> >        */
> >       u32 tx_budget_spent;
> > +     u32 generic_xmit_batch;
> >
> >       /* Statistics */
> >       u64 rx_dropped;
> > diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
> > index 23a062781468..44cb72cd328e 100644
> > --- a/include/uapi/linux/if_xdp.h
> > +++ b/include/uapi/linux/if_xdp.h
> > @@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
> >  #define XDP_STATISTICS                       7
> >  #define XDP_OPTIONS                  8
> >  #define XDP_MAX_TX_SKB_BUDGET                9
> > +#define XDP_GENERIC_XMIT_BATCH               10
> >
> >  struct xdp_umem_reg {
> >       __u64 addr; /* Start of packet data area */
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 9c3acecc14b1..7a149f4ac273 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -1122,6 +1122,7 @@ static int xsk_release(struct socket *sock)
> >       xskq_destroy(xs->tx);
> >       xskq_destroy(xs->fq_tmp);
> >       xskq_destroy(xs->cq_tmp);
> > +     kfree(xs->skb_batch);
> >
> >       sock_orphan(sk);
> >       sock->sk = NULL;
> > @@ -1456,6 +1457,37 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
> >               WRITE_ONCE(xs->max_tx_budget, budget);
> >               return 0;
> >       }
> > +     case XDP_GENERIC_XMIT_BATCH:
> > +     {
> > +             unsigned int batch, batch_alloc_len;
> > +             struct sk_buff **new;
> > +
> > +             if (optlen != sizeof(batch))
> > +                     return -EINVAL;
> > +             if (copy_from_sockptr(&batch, optval, sizeof(batch)))
> > +                     return -EFAULT;
> > +             if (batch > xs->max_tx_budget)
> > +                     return -EACCES;
> > +
> > +             mutex_lock(&xs->mutex);
> > +             if (!batch) {
> > +                     kfree(xs->skb_batch);
> > +                     xs->generic_xmit_batch = 0;
> > +                     goto out;
> > +             }
> > +             batch_alloc_len = sizeof(struct sk_buff *) * batch;
> > +             new = kmalloc(batch_alloc_len, GFP_KERNEL);
> > +             if (!new)
> > +                     return -ENOMEM;
> > +             if (xs->skb_batch)
> > +                     kfree(xs->skb_batch);
> > +
> > +             xs->skb_batch = new;
> > +             xs->generic_xmit_batch = batch;
> > +out:
> > +             mutex_unlock(&xs->mutex);
> > +             return 0;
> > +     }
> >       default:
> >               break;
> >       }
> > diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
> > index 23a062781468..44cb72cd328e 100644
> > --- a/tools/include/uapi/linux/if_xdp.h
> > +++ b/tools/include/uapi/linux/if_xdp.h
> > @@ -80,6 +80,7 @@ struct xdp_mmap_offsets {
> >  #define XDP_STATISTICS                       7
> >  #define XDP_OPTIONS                  8
> >  #define XDP_MAX_TX_SKB_BUDGET                9
> > +#define XDP_GENERIC_XMIT_BATCH               10
> >
> >  struct xdp_umem_reg {
> >       __u64 addr; /* Start of packet data area */
> > --
> > 2.41.3
> >

[PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Zerocopy mode has a good feature named multi buffer while copy mode has
to transmit skb one by one like normal flows. The latter might lose the
bypass power to some extent because of grabbing/releasing the same tx
queue lock and disabling/enabling bh and stuff on a packet basis.
Contending the same queue lock will bring a worse result.

This patch supports batch feature by permitting owning the queue lock to
send the generic_xmit_batch number of packets at one time. To further
achieve a better result, some codes[1] are removed on purpose from
xsk_direct_xmit_batch() as referred to __dev_direct_xmit().

[1]
1. advance the device check to granularity of sendto syscall.
2. remove validating packets because of its uselessness.
3. remove operation of softnet_data.xmit.recursion because it's not
   necessary.
4. remove BQL flow control. We don't need to do BQL control because it
   probably limit the speed. An ideal scenario is to use a standalone and
   clean tx queue to send packets only for xsk. Less competition shows
   better performance results.

Experiments:
1) Tested on virtio_net:
With this patch series applied, the performance number of xdpsock[2] goes
up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
If we test with another thread competing the same queue, a 28% increase
(from 405466 pps to 521076 pps) can be observed.
2) Tested on ixgbe:
The results of zerocopy and copy mode are respectively 1303277 pps and
1187347 pps. After this socket option took effect, copy mode reaches
1472367 which was higher than zerocopy mode impressively.

[2]: ./xdpsock -i eth1 -t  -S -s 64

It's worth mentioning batch process might bring high latency in certain
cases. The recommended value is 32.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 include/linux/netdevice.h |   2 +
 net/core/dev.c            |  18 +++++++
 net/xdp/xsk.c             | 103 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5e5de4b0a433..27738894daa7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3352,6 +3352,8 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
 
 int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
 int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
+int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
+			  struct netdev_queue *txq, u32 max_batch, u32 *cur);
 
 static inline int dev_queue_xmit(struct sk_buff *skb)
 {
diff --git a/net/core/dev.c b/net/core/dev.c
index 68dc47d7e700..7a512bd38806 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4742,6 +4742,24 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 }
 EXPORT_SYMBOL(__dev_queue_xmit);
 
+int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
+			  struct netdev_queue *txq, u32 max_batch, u32 *cur)
+{
+	int ret = NETDEV_TX_BUSY;
+
+	local_bh_disable();
+	HARD_TX_LOCK(dev, txq, smp_processor_id());
+	for (; *cur < max_batch; (*cur)++) {
+		ret = netdev_start_xmit(skb[*cur], dev, txq, false);
+		if (ret != NETDEV_TX_OK)
+			break;
+	}
+	HARD_TX_UNLOCK(dev, txq);
+	local_bh_enable();
+
+	return ret;
+}
+
 int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
 {
 	struct net_device *dev = skb->dev;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 7a149f4ac273..92ad82472776 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -780,9 +780,102 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
 	return ERR_PTR(err);
 }
 
-static int __xsk_generic_xmit(struct sock *sk)
+static int __xsk_generic_xmit_batch(struct xdp_sock *xs)
+{
+	u32 max_batch = READ_ONCE(xs->generic_xmit_batch);
+	struct sk_buff **skb = xs->skb_batch;
+	struct net_device *dev = xs->dev;
+	struct netdev_queue *txq;
+	bool sent_frame = false;
+	struct xdp_desc desc;
+	u32 i = 0, j = 0;
+	u32 max_budget;
+	int err = 0;
+
+	mutex_lock(&xs->mutex);
+
+	/* Since we dropped the RCU read lock, the socket state might have changed. */
+	if (unlikely(!xsk_is_bound(xs))) {
+		err = -ENXIO;
+		goto out;
+	}
+
+	if (xs->queue_id >= dev->real_num_tx_queues)
+		goto out;
+
+	if (unlikely(!netif_running(dev) ||
+		     !netif_carrier_ok(dev)))
+		goto out;
+
+	max_budget = READ_ONCE(xs->max_tx_budget);
+	txq = netdev_get_tx_queue(dev, xs->queue_id);
+	do {
+		for (; i < max_batch && xskq_cons_peek_desc(xs->tx, &desc, xs->pool); i++) {
+			if (max_budget-- == 0) {
+				err = -EAGAIN;
+				break;
+			}
+			/* This is the backpressure mechanism for the Tx path.
+			 * Reserve space in the completion queue and only proceed
+			 * if there is space in it. This avoids having to implement
+			 * any buffering in the Tx path.
+			 */
+			err = xsk_cq_reserve_addr_locked(xs->pool, desc.addr);
+			if (err) {
+				err = -EAGAIN;
+				break;
+			}
+
+			skb[i] = xsk_build_skb(xs, &desc);
+			if (IS_ERR(skb[i])) {
+				err = PTR_ERR(skb[i]);
+				break;
+			}
+
+			xskq_cons_release(xs->tx);
+
+			if (xp_mb_desc(&desc))
+				xs->skb = skb[i];
+		}
+
+		if (i) {
+			err = xsk_direct_xmit_batch(skb, dev, txq, i, &j);
+			if  (err == NETDEV_TX_BUSY) {
+				err = -EAGAIN;
+			} else if (err == NET_XMIT_DROP) {
+				j++;
+				err = -EBUSY;
+			}
+
+			sent_frame = true;
+			xs->skb = NULL;
+		}
+
+		if (err)
+			goto out;
+		i = j = 0;
+	} while (xskq_cons_peek_desc(xs->tx, &desc, xs->pool));
+
+	if (xskq_has_descs(xs->tx)) {
+		if (xs->skb)
+			xsk_drop_skb(xs->skb);
+		xskq_cons_release(xs->tx);
+	}
+
+out:
+	for (; j < i; j++) {
+		xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb[j]));
+		xsk_consume_skb(skb[j]);
+	}
+	if (sent_frame)
+		__xsk_tx_release(xs);
+
+	mutex_unlock(&xs->mutex);
+	return err;
+}
+
+static int __xsk_generic_xmit(struct xdp_sock *xs)
 {
-	struct xdp_sock *xs = xdp_sk(sk);
 	bool sent_frame = false;
 	struct xdp_desc desc;
 	struct sk_buff *skb;
@@ -871,11 +964,15 @@ static int __xsk_generic_xmit(struct sock *sk)
 
 static int xsk_generic_xmit(struct sock *sk)
 {
+	struct xdp_sock *xs = xdp_sk(sk);
 	int ret;
 
 	/* Drop the RCU lock since the SKB path might sleep. */
 	rcu_read_unlock();
-	ret = __xsk_generic_xmit(sk);
+	if (READ_ONCE(xs->generic_xmit_batch))
+		ret = __xsk_generic_xmit_batch(xs);
+	else
+		ret = __xsk_generic_xmit(xs);
 	/* Reaquire RCU lock before going into common code. */
 	rcu_read_lock();
 
-- 
2.41.3

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jesper Dangaard Brouer]

On 11/08/2025 15.12, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Zerocopy mode has a good feature named multi buffer while copy mode has
> to transmit skb one by one like normal flows. The latter might lose the
> bypass power to some extent because of grabbing/releasing the same tx
> queue lock and disabling/enabling bh and stuff on a packet basis.
> Contending the same queue lock will bring a worse result.
> 

I actually think that it is worth optimizing the non-zerocopy mode for
AF_XDP.  My use-case was virtual net_devices like veth.


> This patch supports batch feature by permitting owning the queue lock to
> send the generic_xmit_batch number of packets at one time. To further
> achieve a better result, some codes[1] are removed on purpose from
> xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> 
> [1]
> 1. advance the device check to granularity of sendto syscall.
> 2. remove validating packets because of its uselessness.
> 3. remove operation of softnet_data.xmit.recursion because it's not
>     necessary.
> 4. remove BQL flow control. We don't need to do BQL control because it
>     probably limit the speed. An ideal scenario is to use a standalone and
>     clean tx queue to send packets only for xsk. Less competition shows
>     better performance results.
> 
> Experiments:
> 1) Tested on virtio_net:

If you also want to test on veth, then an optimization is to increase
dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
AF_XDP packets getting reallocated by veth driver. I never completed
upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)

  [1] 
https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org 


(more below...)

> With this patch series applied, the performance number of xdpsock[2] goes
> up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> If we test with another thread competing the same queue, a 28% increase
> (from 405466 pps to 521076 pps) can be observed.
> 2) Tested on ixgbe:
> The results of zerocopy and copy mode are respectively 1303277 pps and
> 1187347 pps. After this socket option took effect, copy mode reaches
> 1472367 which was higher than zerocopy mode impressively.
> 
> [2]: ./xdpsock -i eth1 -t  -S -s 64
> 
> It's worth mentioning batch process might bring high latency in certain
> cases. The recommended value is 32.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>   include/linux/netdevice.h |   2 +
>   net/core/dev.c            |  18 +++++++
>   net/xdp/xsk.c             | 103 ++++++++++++++++++++++++++++++++++++--
>   3 files changed, 120 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 5e5de4b0a433..27738894daa7 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3352,6 +3352,8 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
>   
>   int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
>   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
> +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> +			  struct netdev_queue *txq, u32 max_batch, u32 *cur);
>   
>   static inline int dev_queue_xmit(struct sk_buff *skb)
>   {
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 68dc47d7e700..7a512bd38806 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4742,6 +4742,24 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>   }
>   EXPORT_SYMBOL(__dev_queue_xmit);
>   
> +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> +			  struct netdev_queue *txq, u32 max_batch, u32 *cur)
> +{
> +	int ret = NETDEV_TX_BUSY;
> +
> +	local_bh_disable();
> +	HARD_TX_LOCK(dev, txq, smp_processor_id());
> +	for (; *cur < max_batch; (*cur)++) {
> +		ret = netdev_start_xmit(skb[*cur], dev, txq, false);

The last argument ('false') to netdev_start_xmit() indicate if there are
'more' packets to be sent. This allows the NIC driver to postpone
writing the tail-pointer/doorbell. For physical hardware this is a large
performance gain.

If index have not reached 'max_batch' then we know 'more' packets are true.

   bool more = !!(*cur != max_batch);

Can I ask you to do a test with netdev_start_xmit() using the 'more' 
boolian ?


> +		if (ret != NETDEV_TX_OK)
> +			break;
> +	}
> +	HARD_TX_UNLOCK(dev, txq);
> +	local_bh_enable();
> +
> +	return ret;
> +}
> +
>   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
>   {
>   	struct net_device *dev = skb->dev;
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7a149f4ac273..92ad82472776 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -780,9 +780,102 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
>   	return ERR_PTR(err);
>   }
>   
> -static int __xsk_generic_xmit(struct sock *sk)
> +static int __xsk_generic_xmit_batch(struct xdp_sock *xs)
> +{
> +	u32 max_batch = READ_ONCE(xs->generic_xmit_batch);
> +	struct sk_buff **skb = xs->skb_batch;
> +	struct net_device *dev = xs->dev;
> +	struct netdev_queue *txq;
> +	bool sent_frame = false;
> +	struct xdp_desc desc;
> +	u32 i = 0, j = 0;
> +	u32 max_budget;
> +	int err = 0;
> +
> +	mutex_lock(&xs->mutex);
> +
> +	/* Since we dropped the RCU read lock, the socket state might have changed. */
> +	if (unlikely(!xsk_is_bound(xs))) {
> +		err = -ENXIO;
> +		goto out;
> +	}
> +
> +	if (xs->queue_id >= dev->real_num_tx_queues)
> +		goto out;
> +
> +	if (unlikely(!netif_running(dev) ||
> +		     !netif_carrier_ok(dev)))
> +		goto out;
> +
> +	max_budget = READ_ONCE(xs->max_tx_budget);
> +	txq = netdev_get_tx_queue(dev, xs->queue_id);
> +	do {
> +		for (; i < max_batch && xskq_cons_peek_desc(xs->tx, &desc, xs->pool); i++) {
> +			if (max_budget-- == 0) {
> +				err = -EAGAIN;
> +				break;
> +			}
> +			/* This is the backpressure mechanism for the Tx path.
> +			 * Reserve space in the completion queue and only proceed
> +			 * if there is space in it. This avoids having to implement
> +			 * any buffering in the Tx path.
> +			 */
> +			err = xsk_cq_reserve_addr_locked(xs->pool, desc.addr);
> +			if (err) {
> +				err = -EAGAIN;
> +				break;
> +			}
> +
> +			skb[i] = xsk_build_skb(xs, &desc);

There is a missed opportunity for bulk allocating the SKBs here
(via kmem_cache_alloc_bulk).

But this also requires changing the SKB alloc function used by
xsk_build_skb(). As a seperate patch, I recommend that you change the
sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
I expect this will be a large performance improvement on it's own.
Can I ask you to benchmark this change before the batch xmit change?

Opinions needed from other maintainers please (I might be wrong!):
I don't think the socket level accounting done in sock_alloc_send_skb()
is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
code comment above.

--Jesper

> +			if (IS_ERR(skb[i])) {
> +				err = PTR_ERR(skb[i]);
> +				break;
> +			}
> +
> +			xskq_cons_release(xs->tx);
> +
> +			if (xp_mb_desc(&desc))
> +				xs->skb = skb[i];
> +		}
> +
> +		if (i) {
> +			err = xsk_direct_xmit_batch(skb, dev, txq, i, &j);
> +			if  (err == NETDEV_TX_BUSY) {
> +				err = -EAGAIN;
> +			} else if (err == NET_XMIT_DROP) {
> +				j++;
> +				err = -EBUSY;
> +			}
> +
> +			sent_frame = true;
> +			xs->skb = NULL;
> +		}
> +
> +		if (err)
> +			goto out;
> +		i = j = 0;
> +	} while (xskq_cons_peek_desc(xs->tx, &desc, xs->pool));
> +
> +	if (xskq_has_descs(xs->tx)) {
> +		if (xs->skb)
> +			xsk_drop_skb(xs->skb);
> +		xskq_cons_release(xs->tx);
> +	}
> +
> +out:
> +	for (; j < i; j++) {
> +		xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb[j]));
> +		xsk_consume_skb(skb[j]);
> +	}
> +	if (sent_frame)
> +		__xsk_tx_release(xs);
> +
> +	mutex_unlock(&xs->mutex);
> +	return err;
> +}
> +
> +static int __xsk_generic_xmit(struct xdp_sock *xs)
>   {
> -	struct xdp_sock *xs = xdp_sk(sk);
>   	bool sent_frame = false;
>   	struct xdp_desc desc;
>   	struct sk_buff *skb;
> @@ -871,11 +964,15 @@ static int __xsk_generic_xmit(struct sock *sk)
>   
>   static int xsk_generic_xmit(struct sock *sk)
>   {
> +	struct xdp_sock *xs = xdp_sk(sk);
>   	int ret;
>   
>   	/* Drop the RCU lock since the SKB path might sleep. */
>   	rcu_read_unlock();
> -	ret = __xsk_generic_xmit(sk);
> +	if (READ_ONCE(xs->generic_xmit_batch))
> +		ret = __xsk_generic_xmit_batch(xs);
> +	else
> +		ret = __xsk_generic_xmit(xs);
>   	/* Reaquire RCU lock before going into common code. */
>   	rcu_read_lock();
>   

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Maciej Fijalkowski]

On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
> 
> 
> On 11/08/2025 15.12, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> > 
> > Zerocopy mode has a good feature named multi buffer while copy mode has
> > to transmit skb one by one like normal flows. The latter might lose the
> > bypass power to some extent because of grabbing/releasing the same tx
> > queue lock and disabling/enabling bh and stuff on a packet basis.
> > Contending the same queue lock will bring a worse result.
> > 
> 
> I actually think that it is worth optimizing the non-zerocopy mode for
> AF_XDP.  My use-case was virtual net_devices like veth.
> 
> 
> > This patch supports batch feature by permitting owning the queue lock to
> > send the generic_xmit_batch number of packets at one time. To further
> > achieve a better result, some codes[1] are removed on purpose from
> > xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> > 
> > [1]
> > 1. advance the device check to granularity of sendto syscall.
> > 2. remove validating packets because of its uselessness.
> > 3. remove operation of softnet_data.xmit.recursion because it's not
> >     necessary.
> > 4. remove BQL flow control. We don't need to do BQL control because it
> >     probably limit the speed. An ideal scenario is to use a standalone and
> >     clean tx queue to send packets only for xsk. Less competition shows
> >     better performance results.
> > 
> > Experiments:
> > 1) Tested on virtio_net:
> 
> If you also want to test on veth, then an optimization is to increase
> dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> AF_XDP packets getting reallocated by veth driver. I never completed
> upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
> 
>  [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
> 
> 
> (more below...)
> 
> > With this patch series applied, the performance number of xdpsock[2] goes
> > up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> > If we test with another thread competing the same queue, a 28% increase
> > (from 405466 pps to 521076 pps) can be observed.
> > 2) Tested on ixgbe:
> > The results of zerocopy and copy mode are respectively 1303277 pps and
> > 1187347 pps. After this socket option took effect, copy mode reaches
> > 1472367 which was higher than zerocopy mode impressively.
> > 
> > [2]: ./xdpsock -i eth1 -t  -S -s 64
> > 
> > It's worth mentioning batch process might bring high latency in certain
> > cases. The recommended value is 32.

Given the issue I spotted on your ixgbe batching patch, the comparison
against zc performance is probably not reliable.

> > 
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >   include/linux/netdevice.h |   2 +
> >   net/core/dev.c            |  18 +++++++
> >   net/xdp/xsk.c             | 103 ++++++++++++++++++++++++++++++++++++--
> >   3 files changed, 120 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 5e5de4b0a433..27738894daa7 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3352,6 +3352,8 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
> >   int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
> >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
> > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > +			  struct netdev_queue *txq, u32 max_batch, u32 *cur);
> >   static inline int dev_queue_xmit(struct sk_buff *skb)
> >   {
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 68dc47d7e700..7a512bd38806 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4742,6 +4742,24 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >   }
> >   EXPORT_SYMBOL(__dev_queue_xmit);
> > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > +			  struct netdev_queue *txq, u32 max_batch, u32 *cur)
> > +{
> > +	int ret = NETDEV_TX_BUSY;
> > +
> > +	local_bh_disable();
> > +	HARD_TX_LOCK(dev, txq, smp_processor_id());
> > +	for (; *cur < max_batch; (*cur)++) {
> > +		ret = netdev_start_xmit(skb[*cur], dev, txq, false);
> 
> The last argument ('false') to netdev_start_xmit() indicate if there are
> 'more' packets to be sent. This allows the NIC driver to postpone
> writing the tail-pointer/doorbell. For physical hardware this is a large
> performance gain.
> 
> If index have not reached 'max_batch' then we know 'more' packets are true.
> 
>   bool more = !!(*cur != max_batch);
> 
> Can I ask you to do a test with netdev_start_xmit() using the 'more' boolian
> ?
> 
> 
> > +		if (ret != NETDEV_TX_OK)
> > +			break;
> > +	}
> > +	HARD_TX_UNLOCK(dev, txq);
> > +	local_bh_enable();
> > +
> > +	return ret;
> > +}
> > +
> >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
> >   {
> >   	struct net_device *dev = skb->dev;
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 7a149f4ac273..92ad82472776 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -780,9 +780,102 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> >   	return ERR_PTR(err);
> >   }
> > -static int __xsk_generic_xmit(struct sock *sk)
> > +static int __xsk_generic_xmit_batch(struct xdp_sock *xs)
> > +{
> > +	u32 max_batch = READ_ONCE(xs->generic_xmit_batch);
> > +	struct sk_buff **skb = xs->skb_batch;
> > +	struct net_device *dev = xs->dev;
> > +	struct netdev_queue *txq;
> > +	bool sent_frame = false;
> > +	struct xdp_desc desc;
> > +	u32 i = 0, j = 0;
> > +	u32 max_budget;
> > +	int err = 0;
> > +
> > +	mutex_lock(&xs->mutex);
> > +
> > +	/* Since we dropped the RCU read lock, the socket state might have changed. */
> > +	if (unlikely(!xsk_is_bound(xs))) {
> > +		err = -ENXIO;
> > +		goto out;
> > +	}
> > +
> > +	if (xs->queue_id >= dev->real_num_tx_queues)
> > +		goto out;
> > +
> > +	if (unlikely(!netif_running(dev) ||
> > +		     !netif_carrier_ok(dev)))
> > +		goto out;
> > +
> > +	max_budget = READ_ONCE(xs->max_tx_budget);
> > +	txq = netdev_get_tx_queue(dev, xs->queue_id);
> > +	do {
> > +		for (; i < max_batch && xskq_cons_peek_desc(xs->tx, &desc, xs->pool); i++) {

here we should think how to come up with slightly modified version of
xsk_tx_peek_release_desc_batch() for generic xmit needs, or what could we
borrow from this approach that will be applicable here.

> > +			if (max_budget-- == 0) {
> > +				err = -EAGAIN;
> > +				break;
> > +			}
> > +			/* This is the backpressure mechanism for the Tx path.
> > +			 * Reserve space in the completion queue and only proceed
> > +			 * if there is space in it. This avoids having to implement
> > +			 * any buffering in the Tx path.
> > +			 */
> > +			err = xsk_cq_reserve_addr_locked(xs->pool, desc.addr);
> > +			if (err) {
> > +				err = -EAGAIN;
> > +				break;
> > +			}
> > +
> > +			skb[i] = xsk_build_skb(xs, &desc);
> 
> There is a missed opportunity for bulk allocating the SKBs here
> (via kmem_cache_alloc_bulk).

+1

> 
> But this also requires changing the SKB alloc function used by
> xsk_build_skb(). As a seperate patch, I recommend that you change the
> sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
> I expect this will be a large performance improvement on it's own.
> Can I ask you to benchmark this change before the batch xmit change?
> 
> Opinions needed from other maintainers please (I might be wrong!):
> I don't think the socket level accounting done in sock_alloc_send_skb()
> is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
> code comment above.

Thanks for bringing this up, I had the same feeling.

> 
> --Jesper
> 
> > +			if (IS_ERR(skb[i])) {
> > +				err = PTR_ERR(skb[i]);
> > +				break;
> > +			}
> > +
> > +			xskq_cons_release(xs->tx);
> > +
> > +			if (xp_mb_desc(&desc))
> > +				xs->skb = skb[i];
> > +		}
> > +
> > +		if (i) {
> > +			err = xsk_direct_xmit_batch(skb, dev, txq, i, &j);
> > +			if  (err == NETDEV_TX_BUSY) {
> > +				err = -EAGAIN;
> > +			} else if (err == NET_XMIT_DROP) {
> > +				j++;
> > +				err = -EBUSY;
> > +			}
> > +
> > +			sent_frame = true;
> > +			xs->skb = NULL;
> > +		}
> > +
> > +		if (err)
> > +			goto out;
> > +		i = j = 0;
> > +	} while (xskq_cons_peek_desc(xs->tx, &desc, xs->pool));

from the quick glance i don't follow why you have this call here whilst
having it above in the while loop.

BTW do we have something bulk skb freeing in the kernel? given we're gonna
eventually do kmem_cache_alloc_bulk for skbs then could we do
kmem_cache_free_bulk() as well?

> > +
> > +	if (xskq_has_descs(xs->tx)) {
> > +		if (xs->skb)
> > +			xsk_drop_skb(xs->skb);
> > +		xskq_cons_release(xs->tx);
> > +	}
> > +
> > +out:
> > +	for (; j < i; j++) {
> > +		xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb[j]));
> > +		xsk_consume_skb(skb[j]);
> > +	}
> > +	if (sent_frame)
> > +		__xsk_tx_release(xs);
> > +
> > +	mutex_unlock(&xs->mutex);
> > +	return err;
> > +}
> > +
> > +static int __xsk_generic_xmit(struct xdp_sock *xs)
> >   {
> > -	struct xdp_sock *xs = xdp_sk(sk);
> >   	bool sent_frame = false;
> >   	struct xdp_desc desc;
> >   	struct sk_buff *skb;
> > @@ -871,11 +964,15 @@ static int __xsk_generic_xmit(struct sock *sk)
> >   static int xsk_generic_xmit(struct sock *sk)
> >   {
> > +	struct xdp_sock *xs = xdp_sk(sk);
> >   	int ret;
> >   	/* Drop the RCU lock since the SKB path might sleep. */
> >   	rcu_read_unlock();
> > -	ret = __xsk_generic_xmit(sk);
> > +	if (READ_ONCE(xs->generic_xmit_batch))
> > +		ret = __xsk_generic_xmit_batch(xs);
> > +	else
> > +		ret = __xsk_generic_xmit(xs);
> >   	/* Reaquire RCU lock before going into common code. */
> >   	rcu_read_lock();
> 
> 

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Wed, Aug 13, 2025 at 1:49 AM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
> >
> >
> > On 11/08/2025 15.12, Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > Zerocopy mode has a good feature named multi buffer while copy mode has
> > > to transmit skb one by one like normal flows. The latter might lose the
> > > bypass power to some extent because of grabbing/releasing the same tx
> > > queue lock and disabling/enabling bh and stuff on a packet basis.
> > > Contending the same queue lock will bring a worse result.
> > >
> >
> > I actually think that it is worth optimizing the non-zerocopy mode for
> > AF_XDP.  My use-case was virtual net_devices like veth.
> >
> >
> > > This patch supports batch feature by permitting owning the queue lock to
> > > send the generic_xmit_batch number of packets at one time. To further
> > > achieve a better result, some codes[1] are removed on purpose from
> > > xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> > >
> > > [1]
> > > 1. advance the device check to granularity of sendto syscall.
> > > 2. remove validating packets because of its uselessness.
> > > 3. remove operation of softnet_data.xmit.recursion because it's not
> > >     necessary.
> > > 4. remove BQL flow control. We don't need to do BQL control because it
> > >     probably limit the speed. An ideal scenario is to use a standalone and
> > >     clean tx queue to send packets only for xsk. Less competition shows
> > >     better performance results.
> > >
> > > Experiments:
> > > 1) Tested on virtio_net:
> >
> > If you also want to test on veth, then an optimization is to increase
> > dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> > AF_XDP packets getting reallocated by veth driver. I never completed
> > upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
> >
> >  [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
> >
> >
> > (more below...)
> >
> > > With this patch series applied, the performance number of xdpsock[2] goes
> > > up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> > > If we test with another thread competing the same queue, a 28% increase
> > > (from 405466 pps to 521076 pps) can be observed.
> > > 2) Tested on ixgbe:
> > > The results of zerocopy and copy mode are respectively 1303277 pps and
> > > 1187347 pps. After this socket option took effect, copy mode reaches
> > > 1472367 which was higher than zerocopy mode impressively.
> > >
> > > [2]: ./xdpsock -i eth1 -t  -S -s 64
> > >
> > > It's worth mentioning batch process might bring high latency in certain
> > > cases. The recommended value is 32.
>
> Given the issue I spotted on your ixgbe batching patch, the comparison
> against zc performance is probably not reliable.

I have to clarify the thing: zc performance was tested without that
series applied. That means without that series, the number is 1303277
pps. What I used is './xdpsock -i enp2s0f0np0 -t -q 11  -z -s 64'.

>
> > >
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > >   include/linux/netdevice.h |   2 +
> > >   net/core/dev.c            |  18 +++++++
> > >   net/xdp/xsk.c             | 103 ++++++++++++++++++++++++++++++++++++--
> > >   3 files changed, 120 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 5e5de4b0a433..27738894daa7 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -3352,6 +3352,8 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
> > >   int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
> > >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
> > > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > > +                     struct netdev_queue *txq, u32 max_batch, u32 *cur);
> > >   static inline int dev_queue_xmit(struct sk_buff *skb)
> > >   {
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 68dc47d7e700..7a512bd38806 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -4742,6 +4742,24 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > >   }
> > >   EXPORT_SYMBOL(__dev_queue_xmit);
> > > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > > +                     struct netdev_queue *txq, u32 max_batch, u32 *cur)
> > > +{
> > > +   int ret = NETDEV_TX_BUSY;
> > > +
> > > +   local_bh_disable();
> > > +   HARD_TX_LOCK(dev, txq, smp_processor_id());
> > > +   for (; *cur < max_batch; (*cur)++) {
> > > +           ret = netdev_start_xmit(skb[*cur], dev, txq, false);
> >
> > The last argument ('false') to netdev_start_xmit() indicate if there are
> > 'more' packets to be sent. This allows the NIC driver to postpone
> > writing the tail-pointer/doorbell. For physical hardware this is a large
> > performance gain.
> >
> > If index have not reached 'max_batch' then we know 'more' packets are true.
> >
> >   bool more = !!(*cur != max_batch);
> >
> > Can I ask you to do a test with netdev_start_xmit() using the 'more' boolian
> > ?
> >
> >
> > > +           if (ret != NETDEV_TX_OK)
> > > +                   break;
> > > +   }
> > > +   HARD_TX_UNLOCK(dev, txq);
> > > +   local_bh_enable();
> > > +
> > > +   return ret;
> > > +}
> > > +
> > >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
> > >   {
> > >     struct net_device *dev = skb->dev;
> > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > index 7a149f4ac273..92ad82472776 100644
> > > --- a/net/xdp/xsk.c
> > > +++ b/net/xdp/xsk.c
> > > @@ -780,9 +780,102 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> > >     return ERR_PTR(err);
> > >   }
> > > -static int __xsk_generic_xmit(struct sock *sk)
> > > +static int __xsk_generic_xmit_batch(struct xdp_sock *xs)
> > > +{
> > > +   u32 max_batch = READ_ONCE(xs->generic_xmit_batch);
> > > +   struct sk_buff **skb = xs->skb_batch;
> > > +   struct net_device *dev = xs->dev;
> > > +   struct netdev_queue *txq;
> > > +   bool sent_frame = false;
> > > +   struct xdp_desc desc;
> > > +   u32 i = 0, j = 0;
> > > +   u32 max_budget;
> > > +   int err = 0;
> > > +
> > > +   mutex_lock(&xs->mutex);
> > > +
> > > +   /* Since we dropped the RCU read lock, the socket state might have changed. */
> > > +   if (unlikely(!xsk_is_bound(xs))) {
> > > +           err = -ENXIO;
> > > +           goto out;
> > > +   }
> > > +
> > > +   if (xs->queue_id >= dev->real_num_tx_queues)
> > > +           goto out;
> > > +
> > > +   if (unlikely(!netif_running(dev) ||
> > > +                !netif_carrier_ok(dev)))
> > > +           goto out;
> > > +
> > > +   max_budget = READ_ONCE(xs->max_tx_budget);
> > > +   txq = netdev_get_tx_queue(dev, xs->queue_id);
> > > +   do {
> > > +           for (; i < max_batch && xskq_cons_peek_desc(xs->tx, &desc, xs->pool); i++) {
>
> here we should think how to come up with slightly modified version of
> xsk_tx_peek_release_desc_batch() for generic xmit needs, or what could we
> borrow from this approach that will be applicable here.

Okay, I will dig into it more. Thanks.

>
> > > +                   if (max_budget-- == 0) {
> > > +                           err = -EAGAIN;
> > > +                           break;
> > > +                   }
> > > +                   /* This is the backpressure mechanism for the Tx path.
> > > +                    * Reserve space in the completion queue and only proceed
> > > +                    * if there is space in it. This avoids having to implement
> > > +                    * any buffering in the Tx path.
> > > +                    */
> > > +                   err = xsk_cq_reserve_addr_locked(xs->pool, desc.addr);
> > > +                   if (err) {
> > > +                           err = -EAGAIN;
> > > +                           break;
> > > +                   }
> > > +
> > > +                   skb[i] = xsk_build_skb(xs, &desc);
> >
> > There is a missed opportunity for bulk allocating the SKBs here
> > (via kmem_cache_alloc_bulk).
>
> +1
>
> >
> > But this also requires changing the SKB alloc function used by
> > xsk_build_skb(). As a seperate patch, I recommend that you change the
> > sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
> > I expect this will be a large performance improvement on it's own.
> > Can I ask you to benchmark this change before the batch xmit change?
> >
> > Opinions needed from other maintainers please (I might be wrong!):
> > I don't think the socket level accounting done in sock_alloc_send_skb()
> > is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
> > code comment above.
>
> Thanks for bringing this up, I had the same feeling.
>
> >
> > --Jesper
> >
> > > +                   if (IS_ERR(skb[i])) {
> > > +                           err = PTR_ERR(skb[i]);
> > > +                           break;
> > > +                   }
> > > +
> > > +                   xskq_cons_release(xs->tx);
> > > +
> > > +                   if (xp_mb_desc(&desc))
> > > +                           xs->skb = skb[i];
> > > +           }
> > > +
> > > +           if (i) {
> > > +                   err = xsk_direct_xmit_batch(skb, dev, txq, i, &j);
> > > +                   if  (err == NETDEV_TX_BUSY) {
> > > +                           err = -EAGAIN;
> > > +                   } else if (err == NET_XMIT_DROP) {
> > > +                           j++;
> > > +                           err = -EBUSY;
> > > +                   }
> > > +
> > > +                   sent_frame = true;
> > > +                   xs->skb = NULL;
> > > +           }
> > > +
> > > +           if (err)
> > > +                   goto out;
> > > +           i = j = 0;
> > > +   } while (xskq_cons_peek_desc(xs->tx, &desc, xs->pool));
>
> from the quick glance i don't follow why you have this call here whilst
> having it above in the while loop.

Because it avoids the first unnecessary xskq_cons_peek_desc() check.

>
> BTW do we have something bulk skb freeing in the kernel? given we're gonna
> eventually do kmem_cache_alloc_bulk for skbs then could we do
> kmem_cache_free_bulk() as well?

Good point. Let me deal with it :)

Thanks,
Jason

>
> > > +
> > > +   if (xskq_has_descs(xs->tx)) {
> > > +           if (xs->skb)
> > > +                   xsk_drop_skb(xs->skb);
> > > +           xskq_cons_release(xs->tx);
> > > +   }
> > > +
> > > +out:
> > > +   for (; j < i; j++) {
> > > +           xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb[j]));
> > > +           xsk_consume_skb(skb[j]);
> > > +   }
> > > +   if (sent_frame)
> > > +           __xsk_tx_release(xs);
> > > +
> > > +   mutex_unlock(&xs->mutex);
> > > +   return err;
> > > +}
> > > +
> > > +static int __xsk_generic_xmit(struct xdp_sock *xs)
> > >   {
> > > -   struct xdp_sock *xs = xdp_sk(sk);
> > >     bool sent_frame = false;
> > >     struct xdp_desc desc;
> > >     struct sk_buff *skb;
> > > @@ -871,11 +964,15 @@ static int __xsk_generic_xmit(struct sock *sk)
> > >   static int xsk_generic_xmit(struct sock *sk)
> > >   {
> > > +   struct xdp_sock *xs = xdp_sk(sk);
> > >     int ret;
> > >     /* Drop the RCU lock since the SKB path might sleep. */
> > >     rcu_read_unlock();
> > > -   ret = __xsk_generic_xmit(sk);
> > > +   if (READ_ONCE(xs->generic_xmit_batch))
> > > +           ret = __xsk_generic_xmit_batch(xs);
> > > +   else
> > > +           ret = __xsk_generic_xmit(xs);
> > >     /* Reaquire RCU lock before going into common code. */
> > >     rcu_read_lock();
> >
> >

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Wed, Aug 13, 2025 at 9:02 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Wed, Aug 13, 2025 at 1:49 AM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
> > >
> > >
> > > On 11/08/2025 15.12, Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > Zerocopy mode has a good feature named multi buffer while copy mode has
> > > > to transmit skb one by one like normal flows. The latter might lose the
> > > > bypass power to some extent because of grabbing/releasing the same tx
> > > > queue lock and disabling/enabling bh and stuff on a packet basis.
> > > > Contending the same queue lock will bring a worse result.
> > > >
> > >
> > > I actually think that it is worth optimizing the non-zerocopy mode for
> > > AF_XDP.  My use-case was virtual net_devices like veth.
> > >
> > >
> > > > This patch supports batch feature by permitting owning the queue lock to
> > > > send the generic_xmit_batch number of packets at one time. To further
> > > > achieve a better result, some codes[1] are removed on purpose from
> > > > xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> > > >
> > > > [1]
> > > > 1. advance the device check to granularity of sendto syscall.
> > > > 2. remove validating packets because of its uselessness.
> > > > 3. remove operation of softnet_data.xmit.recursion because it's not
> > > >     necessary.
> > > > 4. remove BQL flow control. We don't need to do BQL control because it
> > > >     probably limit the speed. An ideal scenario is to use a standalone and
> > > >     clean tx queue to send packets only for xsk. Less competition shows
> > > >     better performance results.
> > > >
> > > > Experiments:
> > > > 1) Tested on virtio_net:
> > >
> > > If you also want to test on veth, then an optimization is to increase
> > > dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> > > AF_XDP packets getting reallocated by veth driver. I never completed
> > > upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
> > >
> > >  [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
> > >
> > >
> > > (more below...)
> > >
> > > > With this patch series applied, the performance number of xdpsock[2] goes
> > > > up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> > > > If we test with another thread competing the same queue, a 28% increase
> > > > (from 405466 pps to 521076 pps) can be observed.
> > > > 2) Tested on ixgbe:
> > > > The results of zerocopy and copy mode are respectively 1303277 pps and
> > > > 1187347 pps. After this socket option took effect, copy mode reaches
> > > > 1472367 which was higher than zerocopy mode impressively.
> > > >
> > > > [2]: ./xdpsock -i eth1 -t  -S -s 64
> > > >
> > > > It's worth mentioning batch process might bring high latency in certain
> > > > cases. The recommended value is 32.
> >
> > Given the issue I spotted on your ixgbe batching patch, the comparison
> > against zc performance is probably not reliable.
>
> I have to clarify the thing: zc performance was tested without that
> series applied. That means without that series, the number is 1303277
> pps. What I used is './xdpsock -i enp2s0f0np0 -t -q 11  -z -s 64'.

------
@Maciej Fijalkowski
Interesting thing is that copy mode is way worse than zerocopy if the
nic is _i40e_.

With ixgbe, even copy mode reaches nearly 50-60% of the full speed
(which is only 1Gb/sec) while either zc or batch version copy mode
reaches 70%.
With i40e, copy mode only reaches nearly 9% while zc mode 70%. i40e
has a much faster speed (10Gb/sec) comparatively.

Here are some summaries (budget 32, batch 32):
                 copy      batch             zc
i40e      1777859  2102579     14880643
ixgbe    1187347  1472367     1303277

For i40e, here are numbers around the batch copy mode (budget is 128):
no batch       batch 64
1825027      2228328.
Side note: 2228328 seems to be the max limit in copy mode with this
series applied after testing with different settings.

It turns out that testing on i40e is definitely needed because the
xdpsock test hits the bottleneck on ixgbe.

-----
@Jesper Dangaard Brouer
In terms of the 'more' boolean as Jesper said, related drivers might
need changes like this because in the 'for' loop of the batch process
in xsk_direct_xmit_batch(), drivers may fail to send and then break
the 'for' loop, which leads to no chance to kick the hardware. Or we
can keep trying to send in xsk_direct_xmit_batch() instead of breaking
immediately even if the driver becomes busy right now.

As to ixgbe, the performance doesn't improve as I analyzed (because it
already reaches 70% of full speed).

As to i40e, only adding 'more' logic, the number goes from 2102579 to
2585224 with the 32 batch and 32 budget settings. The number goes from
2200013 to 2697313 with the  batch and 64 budget settings. See! 22+%
improvement!

As to virtio_net, no obvious change here probably because the hardirq
logic doesn't have a huge effect.

Thanks,
Jason

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jesper Dangaard Brouer]

On 13/08/2025 15.06, Jason Xing wrote:
> On Wed, Aug 13, 2025 at 9:02 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>>
>> On Wed, Aug 13, 2025 at 1:49 AM Maciej Fijalkowski
>> <maciej.fijalkowski@intel.com> wrote:
>>>
>>> On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
>>>>
>>>>
>>>> On 11/08/2025 15.12, Jason Xing wrote:
>>>>> From: Jason Xing <kernelxing@tencent.com>
>>>>>
>>>>> Zerocopy mode has a good feature named multi buffer while copy mode has
>>>>> to transmit skb one by one like normal flows. The latter might lose the
>>>>> bypass power to some extent because of grabbing/releasing the same tx
>>>>> queue lock and disabling/enabling bh and stuff on a packet basis.
>>>>> Contending the same queue lock will bring a worse result.
>>>>>
>>>>
>>>> I actually think that it is worth optimizing the non-zerocopy mode for
>>>> AF_XDP.  My use-case was virtual net_devices like veth.
>>>>
>>>>
>>>>> This patch supports batch feature by permitting owning the queue lock to
>>>>> send the generic_xmit_batch number of packets at one time. To further
>>>>> achieve a better result, some codes[1] are removed on purpose from
>>>>> xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
>>>>>
>>>>> [1]
>>>>> 1. advance the device check to granularity of sendto syscall.
>>>>> 2. remove validating packets because of its uselessness.
>>>>> 3. remove operation of softnet_data.xmit.recursion because it's not
>>>>>      necessary.
>>>>> 4. remove BQL flow control. We don't need to do BQL control because it
>>>>>      probably limit the speed. An ideal scenario is to use a standalone and
>>>>>      clean tx queue to send packets only for xsk. Less competition shows
>>>>>      better performance results.
>>>>>
>>>>> Experiments:
>>>>> 1) Tested on virtio_net:
>>>>
>>>> If you also want to test on veth, then an optimization is to increase
>>>> dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
>>>> AF_XDP packets getting reallocated by veth driver. I never completed
>>>> upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
>>>>
>>>>   [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
>>>>
>>>>
>>>> (more below...)
>>>>
>>>>> With this patch series applied, the performance number of xdpsock[2] goes
>>>>> up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
>>>>> If we test with another thread competing the same queue, a 28% increase
>>>>> (from 405466 pps to 521076 pps) can be observed.
>>>>> 2) Tested on ixgbe:
>>>>> The results of zerocopy and copy mode are respectively 1303277 pps and
>>>>> 1187347 pps. After this socket option took effect, copy mode reaches
>>>>> 1472367 which was higher than zerocopy mode impressively.
>>>>>
>>>>> [2]: ./xdpsock -i eth1 -t  -S -s 64
>>>>>
>>>>> It's worth mentioning batch process might bring high latency in certain
>>>>> cases. The recommended value is 32.
>>>
>>> Given the issue I spotted on your ixgbe batching patch, the comparison
>>> against zc performance is probably not reliable.
>>
>> I have to clarify the thing: zc performance was tested without that
>> series applied. That means without that series, the number is 1303277
>> pps. What I used is './xdpsock -i enp2s0f0np0 -t -q 11  -z -s 64'.
> 

My i40e device is running at 40Gbit/s.
I see significantly higher packet per sec (pps) that you are reporting:

$ sudo ./xdpsock -i i40e2 --txonly -q 2 -z -s 64

  sock0@i40e2:2 txonly xdp-drv
                    pps            pkts           1.00
rx                 0              0
tx                 21,546,859     21,552,896


The "copy" mode (-c/--copy) looks like this:

$ sudo ./xdpsock -i i40e2 --txonly -q 2 --copy -s 64

  sock0@i40e2:2 txonly xdp-drv
                    pps            pkts           1.00
rx                 0              0
tx                 2,135,424      2,136,192


The skb-mode (-S, --xdp-skb) looks like this:

$ sudo ./xdpsock -i i40e2 --txonly -q 2 --xdp-skb -s 64

  sock0@i40e2:2 txonly xdp-skb
                    pps            pkts           1.00
rx                 0              0
tx                 2,187,992      2,188,800


The HUGE performance gap to "xdp-drv" zero-copy mode, tells me that
there is a huge potential for improving the performance for the copy
mode, both "native" xdp-drv and xdp-skb, cases.
Thus, the work your are doing here is important.


> ------
> @Maciej Fijalkowski
> Interesting thing is that copy mode is way worse than zerocopy if the
> nic is _i40e_.
> 
> With ixgbe, even copy mode reaches nearly 50-60% of the full speed
> (which is only 1Gb/sec) while either zc or batch version copy mode
> reaches 70%.
> With i40e, copy mode only reaches nearly 9% while zc mode 70%. i40e
> has a much faster speed (10Gb/sec) comparatively.
> 
> Here are some summaries (budget 32, batch 32):
>                 copy       batch         zc
> i40e       1,777,859   2,102,579   14,880,643
> ixgbe      1,187,347   1,472,367    1,303,277
> 
(added thousands separators to make above readable)

Those number only make sense if
  i40e runs at link speed 10Git/s and
  ixgbe runs at link speed 1Gbit/s


> For i40e, here are numbers around the batch copy mode (budget is 128):
> no batch       batch 64
> 1825027      2228328.
> Side note: 2228328 seems to be the max limit in copy mode with this
> series applied after testing with different settings.
> 
> It turns out that testing on i40e is definitely needed because the
> xdpsock test hits the bottleneck on ixgbe.
> 
> -----
> @Jesper Dangaard Brouer
> In terms of the 'more' boolean as Jesper said, related drivers might
> need changes like this because in the 'for' loop of the batch process
> in xsk_direct_xmit_batch(), drivers may fail to send and then break
> the 'for' loop, which leads to no chance to kick the hardware.

If sending with 'more' indicator and driver fail to send, then it is the
responsibility of the driver to update the tail-ptr/doorbell.
Example for ixgbe driver:
  https://elixir.bootlin.com/linux/v6.16/source/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c#L8879-L8880

> Or we
> can keep trying to send in xsk_direct_xmit_batch() instead of breaking
> immediately even if the driver becomes busy right now.
> 
> As to ixgbe, the performance doesn't improve as I analyzed (because it
> already reaches 70% of full speed).
> 

If ixgbe runs at 1Gbit/s then remember Ethernet wire-speed is 1.488
Mpps. Thus, you are much closer than 70% to wire-speed.


> As to i40e, only adding 'more' logic, the number goes from 2102579 to
> 2585224 with the 32 batch and 32 budget settings. The number goes from
> 2200013 to 2697313 with the  batch and 64 budget settings. See! 22+%
> improvement!

That is a very big performance gain IMHO. Shows that avoiding the tail-
ptr/doorbell between each packet have a huge benefit.

> As to virtio_net, no obvious change here probably because the hardirq
> logic doesn't have a huge effect.
> 

Perhaps virtio_net doesn't implement the SKB 'more' feature?

--Jesper

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Sat, Aug 16, 2025 at 12:40 AM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>
>
>
> On 13/08/2025 15.06, Jason Xing wrote:
> > On Wed, Aug 13, 2025 at 9:02 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >>
> >> On Wed, Aug 13, 2025 at 1:49 AM Maciej Fijalkowski
> >> <maciej.fijalkowski@intel.com> wrote:
> >>>
> >>> On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
> >>>>
> >>>>
> >>>> On 11/08/2025 15.12, Jason Xing wrote:
> >>>>> From: Jason Xing <kernelxing@tencent.com>
> >>>>>
> >>>>> Zerocopy mode has a good feature named multi buffer while copy mode has
> >>>>> to transmit skb one by one like normal flows. The latter might lose the
> >>>>> bypass power to some extent because of grabbing/releasing the same tx
> >>>>> queue lock and disabling/enabling bh and stuff on a packet basis.
> >>>>> Contending the same queue lock will bring a worse result.
> >>>>>
> >>>>
> >>>> I actually think that it is worth optimizing the non-zerocopy mode for
> >>>> AF_XDP.  My use-case was virtual net_devices like veth.
> >>>>
> >>>>
> >>>>> This patch supports batch feature by permitting owning the queue lock to
> >>>>> send the generic_xmit_batch number of packets at one time. To further
> >>>>> achieve a better result, some codes[1] are removed on purpose from
> >>>>> xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> >>>>>
> >>>>> [1]
> >>>>> 1. advance the device check to granularity of sendto syscall.
> >>>>> 2. remove validating packets because of its uselessness.
> >>>>> 3. remove operation of softnet_data.xmit.recursion because it's not
> >>>>>      necessary.
> >>>>> 4. remove BQL flow control. We don't need to do BQL control because it
> >>>>>      probably limit the speed. An ideal scenario is to use a standalone and
> >>>>>      clean tx queue to send packets only for xsk. Less competition shows
> >>>>>      better performance results.
> >>>>>
> >>>>> Experiments:
> >>>>> 1) Tested on virtio_net:
> >>>>
> >>>> If you also want to test on veth, then an optimization is to increase
> >>>> dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> >>>> AF_XDP packets getting reallocated by veth driver. I never completed
> >>>> upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
> >>>>
> >>>>   [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
> >>>>
> >>>>
> >>>> (more below...)
> >>>>
> >>>>> With this patch series applied, the performance number of xdpsock[2] goes
> >>>>> up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> >>>>> If we test with another thread competing the same queue, a 28% increase
> >>>>> (from 405466 pps to 521076 pps) can be observed.
> >>>>> 2) Tested on ixgbe:
> >>>>> The results of zerocopy and copy mode are respectively 1303277 pps and
> >>>>> 1187347 pps. After this socket option took effect, copy mode reaches
> >>>>> 1472367 which was higher than zerocopy mode impressively.
> >>>>>
> >>>>> [2]: ./xdpsock -i eth1 -t  -S -s 64
> >>>>>
> >>>>> It's worth mentioning batch process might bring high latency in certain
> >>>>> cases. The recommended value is 32.
> >>>
> >>> Given the issue I spotted on your ixgbe batching patch, the comparison
> >>> against zc performance is probably not reliable.
> >>
> >> I have to clarify the thing: zc performance was tested without that
> >> series applied. That means without that series, the number is 1303277
> >> pps. What I used is './xdpsock -i enp2s0f0np0 -t -q 11  -z -s 64'.
> >
>
> My i40e device is running at 40Gbit/s.
> I see significantly higher packet per sec (pps) that you are reporting:
>
> $ sudo ./xdpsock -i i40e2 --txonly -q 2 -z -s 64
>
>   sock0@i40e2:2 txonly xdp-drv
>                     pps            pkts           1.00
> rx                 0              0
> tx                 21,546,859     21,552,896
>
>
> The "copy" mode (-c/--copy) looks like this:
>
> $ sudo ./xdpsock -i i40e2 --txonly -q 2 --copy -s 64
>
>   sock0@i40e2:2 txonly xdp-drv
>                     pps            pkts           1.00
> rx                 0              0
> tx                 2,135,424      2,136,192
>
>
> The skb-mode (-S, --xdp-skb) looks like this:
>
> $ sudo ./xdpsock -i i40e2 --txonly -q 2 --xdp-skb -s 64
>
>   sock0@i40e2:2 txonly xdp-skb
>                     pps            pkts           1.00
> rx                 0              0
> tx                 2,187,992      2,188,800
>
>
> The HUGE performance gap to "xdp-drv" zero-copy mode, tells me that
> there is a huge potential for improving the performance for the copy
> mode, both "native" xdp-drv and xdp-skb, cases.
> Thus, the work your are doing here is important.

Great! Thanks for your affirmation.

>
>
> > ------
> > @Maciej Fijalkowski
> > Interesting thing is that copy mode is way worse than zerocopy if the
> > nic is _i40e_.
> >
> > With ixgbe, even copy mode reaches nearly 50-60% of the full speed
> > (which is only 1Gb/sec) while either zc or batch version copy mode
> > reaches 70%.
> > With i40e, copy mode only reaches nearly 9% while zc mode 70%. i40e
> > has a much faster speed (10Gb/sec) comparatively.
> >
> > Here are some summaries (budget 32, batch 32):
> >                 copy       batch         zc
> > i40e       1,777,859   2,102,579   14,880,643
> > ixgbe      1,187,347   1,472,367    1,303,277
> >
> (added thousands separators to make above readable)
>
> Those number only make sense if
>   i40e runs at link speed 10Git/s and
>   ixgbe runs at link speed 1Gbit/s
>
>
> > For i40e, here are numbers around the batch copy mode (budget is 128):
> > no batch       batch 64
> > 1825027      2228328.
> > Side note: 2228328 seems to be the max limit in copy mode with this
> > series applied after testing with different settings.
> >
> > It turns out that testing on i40e is definitely needed because the
> > xdpsock test hits the bottleneck on ixgbe.
> >
> > -----
> > @Jesper Dangaard Brouer
> > In terms of the 'more' boolean as Jesper said, related drivers might
> > need changes like this because in the 'for' loop of the batch process
> > in xsk_direct_xmit_batch(), drivers may fail to send and then break
> > the 'for' loop, which leads to no chance to kick the hardware.
>
> If sending with 'more' indicator and driver fail to send, then it is the
> responsibility of the driver to update the tail-ptr/doorbell.
> Example for ixgbe driver:
>   https://elixir.bootlin.com/linux/v6.16/source/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c#L8879-L8880

Thanks for sharing this. I'm learning :)

>
> > Or we
> > can keep trying to send in xsk_direct_xmit_batch() instead of breaking
> > immediately even if the driver becomes busy right now.
> >
> > As to ixgbe, the performance doesn't improve as I analyzed (because it
> > already reaches 70% of full speed).
> >
>
> If ixgbe runs at 1Gbit/s then remember Ethernet wire-speed is 1.488
> Mpps. Thus, you are much closer than 70% to wire-speed.

Right. 70% probably is the maximum number that xdpsock can reach.

>
>
> > As to i40e, only adding 'more' logic, the number goes from 2102579 to
> > 2585224 with the 32 batch and 32 budget settings. The number goes from
> > 2200013 to 2697313 with the  batch and 64 budget settings. See! 22+%
> > improvement!
>
> That is a very big performance gain IMHO. Shows that avoiding the tail-
> ptr/doorbell between each packet have a huge benefit.

That's right, and I will include it into the series as you suggested. Thanks!

>
> > As to virtio_net, no obvious change here probably because the hardirq
> > logic doesn't have a huge effect.
> >
>
> Perhaps virtio_net doesn't implement the SKB 'more' feature?

Oh, let me investigate it more deeply.

BTW, I'm not sure if you missed the previous last email, what do you
think of socket level accounting after reading that reply?

Thanks,
Jason

>
> --Jesper
>

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Sat, Aug 16, 2025 at 8:03 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Sat, Aug 16, 2025 at 12:40 AM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
> >
> >
> >
> > On 13/08/2025 15.06, Jason Xing wrote:
> > > On Wed, Aug 13, 2025 at 9:02 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> > >>
> > >> On Wed, Aug 13, 2025 at 1:49 AM Maciej Fijalkowski
> > >> <maciej.fijalkowski@intel.com> wrote:
> > >>>
> > >>> On Tue, Aug 12, 2025 at 04:30:03PM +0200, Jesper Dangaard Brouer wrote:
> > >>>>
> > >>>>
> > >>>> On 11/08/2025 15.12, Jason Xing wrote:
> > >>>>> From: Jason Xing <kernelxing@tencent.com>
> > >>>>>
> > >>>>> Zerocopy mode has a good feature named multi buffer while copy mode has
> > >>>>> to transmit skb one by one like normal flows. The latter might lose the
> > >>>>> bypass power to some extent because of grabbing/releasing the same tx
> > >>>>> queue lock and disabling/enabling bh and stuff on a packet basis.
> > >>>>> Contending the same queue lock will bring a worse result.
> > >>>>>
> > >>>>
> > >>>> I actually think that it is worth optimizing the non-zerocopy mode for
> > >>>> AF_XDP.  My use-case was virtual net_devices like veth.
> > >>>>
> > >>>>
> > >>>>> This patch supports batch feature by permitting owning the queue lock to
> > >>>>> send the generic_xmit_batch number of packets at one time. To further
> > >>>>> achieve a better result, some codes[1] are removed on purpose from
> > >>>>> xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> > >>>>>
> > >>>>> [1]
> > >>>>> 1. advance the device check to granularity of sendto syscall.
> > >>>>> 2. remove validating packets because of its uselessness.
> > >>>>> 3. remove operation of softnet_data.xmit.recursion because it's not
> > >>>>>      necessary.
> > >>>>> 4. remove BQL flow control. We don't need to do BQL control because it
> > >>>>>      probably limit the speed. An ideal scenario is to use a standalone and
> > >>>>>      clean tx queue to send packets only for xsk. Less competition shows
> > >>>>>      better performance results.
> > >>>>>
> > >>>>> Experiments:
> > >>>>> 1) Tested on virtio_net:
> > >>>>
> > >>>> If you also want to test on veth, then an optimization is to increase
> > >>>> dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> > >>>> AF_XDP packets getting reallocated by veth driver. I never completed
> > >>>> upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
> > >>>>
> > >>>>   [1] https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org
> > >>>>
> > >>>>
> > >>>> (more below...)
> > >>>>
> > >>>>> With this patch series applied, the performance number of xdpsock[2] goes
> > >>>>> up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> > >>>>> If we test with another thread competing the same queue, a 28% increase
> > >>>>> (from 405466 pps to 521076 pps) can be observed.
> > >>>>> 2) Tested on ixgbe:
> > >>>>> The results of zerocopy and copy mode are respectively 1303277 pps and
> > >>>>> 1187347 pps. After this socket option took effect, copy mode reaches
> > >>>>> 1472367 which was higher than zerocopy mode impressively.
> > >>>>>
> > >>>>> [2]: ./xdpsock -i eth1 -t  -S -s 64
> > >>>>>
> > >>>>> It's worth mentioning batch process might bring high latency in certain
> > >>>>> cases. The recommended value is 32.
> > >>>
> > >>> Given the issue I spotted on your ixgbe batching patch, the comparison
> > >>> against zc performance is probably not reliable.
> > >>
> > >> I have to clarify the thing: zc performance was tested without that
> > >> series applied. That means without that series, the number is 1303277
> > >> pps. What I used is './xdpsock -i enp2s0f0np0 -t -q 11  -z -s 64'.
> > >
> >
> > My i40e device is running at 40Gbit/s.
> > I see significantly higher packet per sec (pps) that you are reporting:
> >
> > $ sudo ./xdpsock -i i40e2 --txonly -q 2 -z -s 64
> >
> >   sock0@i40e2:2 txonly xdp-drv
> >                     pps            pkts           1.00
> > rx                 0              0
> > tx                 21,546,859     21,552,896
> >
> >
> > The "copy" mode (-c/--copy) looks like this:
> >
> > $ sudo ./xdpsock -i i40e2 --txonly -q 2 --copy -s 64
> >
> >   sock0@i40e2:2 txonly xdp-drv
> >                     pps            pkts           1.00
> > rx                 0              0
> > tx                 2,135,424      2,136,192
> >
> >
> > The skb-mode (-S, --xdp-skb) looks like this:
> >
> > $ sudo ./xdpsock -i i40e2 --txonly -q 2 --xdp-skb -s 64
> >
> >   sock0@i40e2:2 txonly xdp-skb
> >                     pps            pkts           1.00
> > rx                 0              0
> > tx                 2,187,992      2,188,800
> >
> >
> > The HUGE performance gap to "xdp-drv" zero-copy mode, tells me that
> > there is a huge potential for improving the performance for the copy
> > mode, both "native" xdp-drv and xdp-skb, cases.
> > Thus, the work your are doing here is important.
>
> Great! Thanks for your affirmation.
>
> >
> >
> > > ------
> > > @Maciej Fijalkowski
> > > Interesting thing is that copy mode is way worse than zerocopy if the
> > > nic is _i40e_.
> > >
> > > With ixgbe, even copy mode reaches nearly 50-60% of the full speed
> > > (which is only 1Gb/sec) while either zc or batch version copy mode
> > > reaches 70%.
> > > With i40e, copy mode only reaches nearly 9% while zc mode 70%. i40e
> > > has a much faster speed (10Gb/sec) comparatively.
> > >
> > > Here are some summaries (budget 32, batch 32):
> > >                 copy       batch         zc
> > > i40e       1,777,859   2,102,579   14,880,643
> > > ixgbe      1,187,347   1,472,367    1,303,277
> > >
> > (added thousands separators to make above readable)
> >
> > Those number only make sense if
> >   i40e runs at link speed 10Git/s and
> >   ixgbe runs at link speed 1Gbit/s
> >
> >
> > > For i40e, here are numbers around the batch copy mode (budget is 128):
> > > no batch       batch 64
> > > 1825027      2228328.
> > > Side note: 2228328 seems to be the max limit in copy mode with this
> > > series applied after testing with different settings.
> > >
> > > It turns out that testing on i40e is definitely needed because the
> > > xdpsock test hits the bottleneck on ixgbe.
> > >
> > > -----
> > > @Jesper Dangaard Brouer
> > > In terms of the 'more' boolean as Jesper said, related drivers might
> > > need changes like this because in the 'for' loop of the batch process
> > > in xsk_direct_xmit_batch(), drivers may fail to send and then break
> > > the 'for' loop, which leads to no chance to kick the hardware.
> >
> > If sending with 'more' indicator and driver fail to send, then it is the
> > responsibility of the driver to update the tail-ptr/doorbell.
> > Example for ixgbe driver:
> >   https://elixir.bootlin.com/linux/v6.16/source/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c#L8879-L8880
>
> Thanks for sharing this. I'm learning :)
>
> >
> > > Or we
> > > can keep trying to send in xsk_direct_xmit_batch() instead of breaking
> > > immediately even if the driver becomes busy right now.
> > >
> > > As to ixgbe, the performance doesn't improve as I analyzed (because it
> > > already reaches 70% of full speed).
> > >
> >
> > If ixgbe runs at 1Gbit/s then remember Ethernet wire-speed is 1.488
> > Mpps. Thus, you are much closer than 70% to wire-speed.
>
> Right. 70% probably is the maximum number that xdpsock can reach.
>
> >
> >
> > > As to i40e, only adding 'more' logic, the number goes from 2102579 to
> > > 2585224 with the 32 batch and 32 budget settings. The number goes from
> > > 2200013 to 2697313 with the  batch and 64 budget settings. See! 22+%
> > > improvement!
> >
> > That is a very big performance gain IMHO. Shows that avoiding the tail-
> > ptr/doorbell between each packet have a huge benefit.
>
> That's right, and I will include it into the series as you suggested. Thanks!
>
> >
> > > As to virtio_net, no obvious change here probably because the hardirq
> > > logic doesn't have a huge effect.
> > >
> >
> > Perhaps virtio_net doesn't implement the SKB 'more' feature?
>
> Oh, let me investigate it more deeply.

Here I'm going to update two things:

#1
Virtio_net driver does implement the more feature at the link:
https://elixir.bootlin.com/linux/v6.16/source/drivers/net/virtio_net.c#L3352

If there is 'more' indicator without setting __QUEUE_STATE_DRV_XOFF,
then driver will not notify the host.

#2
As to the previous question about xsk that might fail to sense the
BUSY error from the driver, I double checked and saw it can be
problematic:
i40e_xmit_frame_ring()
    -> if (i40e_maybe_stop_tx(tx_ring, count + 4 + 1)) {
              return NETDEV_TX_BUSY;
It means that at the very beginning of driver sending packets on i40e
driver, it might be stopped due to the above case in the middle of
sending a number of packets even in the normal sending process (see
dev_hard_start_xmit()). Then the qdisc will miss the opportunity to
ask the driver to kick. The same situation could be applied to this
xsk batch xmit.

Notice that the case is different from the BQL limit and check in
ixgbe_tx_map() (see the link you mentioned). We cannot prevent it
because DQL algo might not stop sending packets but the above code
snippet can. I checked mlx4 and mlx5 and didn't spot any chance where
the BUSY value could be returned in the xmit process.

So I wonder if we fix the problem from the driver side like this
regardless of the current batch xmit:
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 048c33039130..81cc2219632b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -3904,6 +3904,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct
sk_buff *skb,
         */
        if (i40e_maybe_stop_tx(tx_ring, count + 4 + 1)) {
                tx_ring->tx_stats.tx_busy++;
+               writel(tx_ring->next_to_use, tx_ring->tail);
                return NETDEV_TX_BUSY;
        }

If so, there are a few drivers needing to be adjusted... I'm not sure
if I'm moving in the right direction...

Thanks,
Jason

>
> BTW, I'm not sure if you missed the previous last email, what do you
> think of socket level accounting after reading that reply?
>
> Thanks,
> Jason
>
> >
> > --Jesper
> >

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Tue, Aug 12, 2025 at 10:30 PM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>
>
>
> On 11/08/2025 15.12, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Zerocopy mode has a good feature named multi buffer while copy mode has
> > to transmit skb one by one like normal flows. The latter might lose the
> > bypass power to some extent because of grabbing/releasing the same tx
> > queue lock and disabling/enabling bh and stuff on a packet basis.
> > Contending the same queue lock will bring a worse result.
> >
>
> I actually think that it is worth optimizing the non-zerocopy mode for
> AF_XDP.  My use-case was virtual net_devices like veth.
>
>
> > This patch supports batch feature by permitting owning the queue lock to
> > send the generic_xmit_batch number of packets at one time. To further
> > achieve a better result, some codes[1] are removed on purpose from
> > xsk_direct_xmit_batch() as referred to __dev_direct_xmit().
> >
> > [1]
> > 1. advance the device check to granularity of sendto syscall.
> > 2. remove validating packets because of its uselessness.
> > 3. remove operation of softnet_data.xmit.recursion because it's not
> >     necessary.
> > 4. remove BQL flow control. We don't need to do BQL control because it
> >     probably limit the speed. An ideal scenario is to use a standalone and
> >     clean tx queue to send packets only for xsk. Less competition shows
> >     better performance results.
> >
> > Experiments:
> > 1) Tested on virtio_net:
>
> If you also want to test on veth, then an optimization is to increase
> dev->needed_headroom to XDP_PACKET_HEADROOM (256), as this avoids non-zc
> AF_XDP packets getting reallocated by veth driver. I never completed
> upstreaming this[1] before I left Red Hat.  (virtio_net might also benefit)
>
>   [1]
> https://github.com/xdp-project/xdp-project/blob/main/areas/core/veth_benchmark04.org

Oh, even though I'm not that familiar with veth, I am willing to learn
it these days. Thanks for sharing this with me!

>
>
> (more below...)
>
> > With this patch series applied, the performance number of xdpsock[2] goes
> > up by 33%. Before, it was 767743 pps; while after it was 1021486 pps.
> > If we test with another thread competing the same queue, a 28% increase
> > (from 405466 pps to 521076 pps) can be observed.
> > 2) Tested on ixgbe:
> > The results of zerocopy and copy mode are respectively 1303277 pps and
> > 1187347 pps. After this socket option took effect, copy mode reaches
> > 1472367 which was higher than zerocopy mode impressively.
> >
> > [2]: ./xdpsock -i eth1 -t  -S -s 64
> >
> > It's worth mentioning batch process might bring high latency in certain
> > cases. The recommended value is 32.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >   include/linux/netdevice.h |   2 +
> >   net/core/dev.c            |  18 +++++++
> >   net/xdp/xsk.c             | 103 ++++++++++++++++++++++++++++++++++++--
> >   3 files changed, 120 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 5e5de4b0a433..27738894daa7 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3352,6 +3352,8 @@ u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
> >
> >   int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev);
> >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id);
> > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > +                       struct netdev_queue *txq, u32 max_batch, u32 *cur);
> >
> >   static inline int dev_queue_xmit(struct sk_buff *skb)
> >   {
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 68dc47d7e700..7a512bd38806 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4742,6 +4742,24 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >   }
> >   EXPORT_SYMBOL(__dev_queue_xmit);
> >
> > +int xsk_direct_xmit_batch(struct sk_buff **skb, struct net_device *dev,
> > +                       struct netdev_queue *txq, u32 max_batch, u32 *cur)
> > +{
> > +     int ret = NETDEV_TX_BUSY;
> > +
> > +     local_bh_disable();
> > +     HARD_TX_LOCK(dev, txq, smp_processor_id());
> > +     for (; *cur < max_batch; (*cur)++) {
> > +             ret = netdev_start_xmit(skb[*cur], dev, txq, false);
>
> The last argument ('false') to netdev_start_xmit() indicate if there are
> 'more' packets to be sent. This allows the NIC driver to postpone
> writing the tail-pointer/doorbell. For physical hardware this is a large
> performance gain.
>
> If index have not reached 'max_batch' then we know 'more' packets are true.
>
>    bool more = !!(*cur != max_batch);
>
> Can I ask you to do a test with netdev_start_xmit() using the 'more'
> boolian ?

Agreed, really insightful information. I'm taking note here. Will get
back more information here soon.

>
>
> > +             if (ret != NETDEV_TX_OK)
> > +                     break;
> > +     }
> > +     HARD_TX_UNLOCK(dev, txq);
> > +     local_bh_enable();
> > +
> > +     return ret;
> > +}
> > +
> >   int __dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
> >   {
> >       struct net_device *dev = skb->dev;
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 7a149f4ac273..92ad82472776 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -780,9 +780,102 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> >       return ERR_PTR(err);
> >   }
> >
> > -static int __xsk_generic_xmit(struct sock *sk)
> > +static int __xsk_generic_xmit_batch(struct xdp_sock *xs)
> > +{
> > +     u32 max_batch = READ_ONCE(xs->generic_xmit_batch);
> > +     struct sk_buff **skb = xs->skb_batch;
> > +     struct net_device *dev = xs->dev;
> > +     struct netdev_queue *txq;
> > +     bool sent_frame = false;
> > +     struct xdp_desc desc;
> > +     u32 i = 0, j = 0;
> > +     u32 max_budget;
> > +     int err = 0;
> > +
> > +     mutex_lock(&xs->mutex);
> > +
> > +     /* Since we dropped the RCU read lock, the socket state might have changed. */
> > +     if (unlikely(!xsk_is_bound(xs))) {
> > +             err = -ENXIO;
> > +             goto out;
> > +     }
> > +
> > +     if (xs->queue_id >= dev->real_num_tx_queues)
> > +             goto out;
> > +
> > +     if (unlikely(!netif_running(dev) ||
> > +                  !netif_carrier_ok(dev)))
> > +             goto out;
> > +
> > +     max_budget = READ_ONCE(xs->max_tx_budget);
> > +     txq = netdev_get_tx_queue(dev, xs->queue_id);
> > +     do {
> > +             for (; i < max_batch && xskq_cons_peek_desc(xs->tx, &desc, xs->pool); i++) {
> > +                     if (max_budget-- == 0) {
> > +                             err = -EAGAIN;
> > +                             break;
> > +                     }
> > +                     /* This is the backpressure mechanism for the Tx path.
> > +                      * Reserve space in the completion queue and only proceed
> > +                      * if there is space in it. This avoids having to implement
> > +                      * any buffering in the Tx path.
> > +                      */
> > +                     err = xsk_cq_reserve_addr_locked(xs->pool, desc.addr);
> > +                     if (err) {
> > +                             err = -EAGAIN;
> > +                             break;
> > +                     }
> > +
> > +                     skb[i] = xsk_build_skb(xs, &desc);
>
> There is a missed opportunity for bulk allocating the SKBs here
> (via kmem_cache_alloc_bulk).
>
> But this also requires changing the SKB alloc function used by
> xsk_build_skb(). As a seperate patch, I recommend that you change the
> sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
> I expect this will be a large performance improvement on it's own.
> Can I ask you to benchmark this change before the batch xmit change?

Sure, I will do that.

>
> Opinions needed from other maintainers please (I might be wrong!):
> I don't think the socket level accounting done in sock_alloc_send_skb()
> is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
> code comment above.

Point taken. It seems no chance to remove it in this common helper.
Let me give it more thoughts :)

Thanks,
Jason

>
> --Jesper
>
> > +                     if (IS_ERR(skb[i])) {
> > +                             err = PTR_ERR(skb[i]);
> > +                             break;
> > +                     }
> > +
> > +                     xskq_cons_release(xs->tx);
> > +
> > +                     if (xp_mb_desc(&desc))
> > +                             xs->skb = skb[i];
> > +             }
> > +
> > +             if (i) {
> > +                     err = xsk_direct_xmit_batch(skb, dev, txq, i, &j);
> > +                     if  (err == NETDEV_TX_BUSY) {
> > +                             err = -EAGAIN;
> > +                     } else if (err == NET_XMIT_DROP) {
> > +                             j++;
> > +                             err = -EBUSY;
> > +                     }
> > +
> > +                     sent_frame = true;
> > +                     xs->skb = NULL;
> > +             }
> > +
> > +             if (err)
> > +                     goto out;
> > +             i = j = 0;
> > +     } while (xskq_cons_peek_desc(xs->tx, &desc, xs->pool));
> > +
> > +     if (xskq_has_descs(xs->tx)) {
> > +             if (xs->skb)
> > +                     xsk_drop_skb(xs->skb);
> > +             xskq_cons_release(xs->tx);
> > +     }
> > +
> > +out:
> > +     for (; j < i; j++) {
> > +             xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb[j]));
> > +             xsk_consume_skb(skb[j]);
> > +     }
> > +     if (sent_frame)
> > +             __xsk_tx_release(xs);
> > +
> > +     mutex_unlock(&xs->mutex);
> > +     return err;
> > +}
> > +
> > +static int __xsk_generic_xmit(struct xdp_sock *xs)
> >   {
> > -     struct xdp_sock *xs = xdp_sk(sk);
> >       bool sent_frame = false;
> >       struct xdp_desc desc;
> >       struct sk_buff *skb;
> > @@ -871,11 +964,15 @@ static int __xsk_generic_xmit(struct sock *sk)
> >
> >   static int xsk_generic_xmit(struct sock *sk)
> >   {
> > +     struct xdp_sock *xs = xdp_sk(sk);
> >       int ret;
> >
> >       /* Drop the RCU lock since the SKB path might sleep. */
> >       rcu_read_unlock();
> > -     ret = __xsk_generic_xmit(sk);
> > +     if (READ_ONCE(xs->generic_xmit_batch))
> > +             ret = __xsk_generic_xmit_batch(xs);
> > +     else
> > +             ret = __xsk_generic_xmit(xs);
> >       /* Reaquire RCU lock before going into common code. */
> >       rcu_read_lock();
> >
>

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Tue, Aug 12, 2025 at 10:30 PM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>
...
>
> But this also requires changing the SKB alloc function used by
> xsk_build_skb(). As a seperate patch, I recommend that you change the
> sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
> I expect this will be a large performance improvement on it's own.
> Can I ask you to benchmark this change before the batch xmit change?
>
> Opinions needed from other maintainers please (I might be wrong!):
> I don't think the socket level accounting done in sock_alloc_send_skb()
> is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
> code comment above.

Here I'm bringing back the last test you expected to know :)

I use alloc_skb() to replace sock_alloc_send_skb() and introduce other
minor changes, say, removing sock_wfree() from xsk_destruct_skb(). It
turns out to be a stable 5% performance improvement on i40e driver.
slight improvement on virtio_net. That's good news.

Bad news is that the above logic has bugs like freeing skb in the napi
poll causes accessing skb->sk in xsk_destruct_skb() which triggers a
NULL pointer issue. How did I spot this one? I removed the BQL flow
control and started two xdpsock on different queues, then I saw a
panic[1]... To solve the problem like that, I'm afraid that we still
need to charge a certain length value into sk_wmem_alloc so that
sock_wfree(skb) can be the last one to free the socket finally.

So this socket level accounting mechanism keeps its safety in the above case.

IMHO, we can get rid of the limitation of sk_sndbuf but still use
skb_set_owner_w() that charges the len of skb. If we stick to removing
the whole accounting function, probably we have to adjust the position
of xsk_cq_submit_locked(), but I reckon for now it's not practical...

Any thoughts on this?

[1]
 997 [  133.528449] RIP: 0010:xsk_destruct_skb+0x6a/0x90
 998 [  133.528920] Code: 8b 6c 02 28 48 8b 43 18 4c 8b a0 68 03 00 00
49 8d 9c 24 e8 00 00 00 48 89 df e8 f1 eb 06 00 48 89 c6 49 8b 84 24
88 00 00 00 <48> 8b 50 10 03 2a 48      8b 40 10 48 89 df 89 28 5b 5d
41 5c e9 6e ec
 999 [  133.530526] RSP: 0018:ffffae71c06a0d08 EFLAGS: 00010046
1000 [  133.531005] RAX: 0000000000000000 RBX: ffff9f42c81c49e8 RCX:
00000000000002e7
1001 [  133.531631] RDX: 0000000000000001 RSI: 0000000000000286 RDI:
ffff9f42c81c49e8
1002 [  133.532249] RBP: 0000000000000001 R08: 0000000000000008 R09:
00000000000000001003 [  133.532867] R10: ffffffff978080c0 R11:
ffffae71c06a0ff8 R12: ffff9f42c81c4900
1004 [  133.533491] R13: ffffae71c06a0d88 R14: ffff9f42e0f1f900 R15:
ffff9f42ce850d801005 [  133.534123] FS:  0000000000000000(0000)
GS:ffff9f5227655000(0000) knlGS:00000000000000001006 [  133.534831]
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
1007 [  133.535366] CR2: 0000000000000010 CR3: 000000011c820000 CR4:
00000000003506f0
1008 [  133.536014] Call Trace:
1009 [  133.536313]  <IRQ>
1010 [  133.536583]  skb_release_head_state+0x20/0x90
1011 [  133.537021]  napi_consume_skb+0x42/0x120
1012 [  133.537429]  __free_old_xmit+0x76/0x170 [virtio_net]
1013 [  133.537923]  free_old_xmit+0x53/0xc0 [virtio_net]
1014 [  133.538395]  virtnet_poll+0xed/0x5d0 [virtio_net]
1015 [  133.538867]  ? blake2s_compress+0x52/0xa0
1016 [  133.539286]  __napi_poll+0x28/0x200
1017 [  133.539668]  net_rx_action+0x319/0x400
1018 [  133.540068]  ? sched_clock_cpu+0xb/0x190
1019 [  133.540482]  ? __run_timers+0x1d1/0x260
1020 [  133.540906]  ? __pfx_dl_task_timer+0x10/0x10
1021 [  133.541349]  ? lock_timer_base+0x72/0x90
1022 [  133.541767]  handle_softirqs+0xce/0x2e0
1023 [  133.542178]  __irq_exit_rcu+0xc6/0xf0
1024 [  133.542575]  common_interrupt+0x81/0xa0

Thanks,
Jason

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jesper Dangaard Brouer]

I need some help from Cc Magnus or Björn, to explain why you changes
fails in xsk_destruct_skb().


On 15/08/2025 08.44, Jason Xing wrote:
> On Tue, Aug 12, 2025 at 10:30 PM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>>
> ...
>>
>> But this also requires changing the SKB alloc function used by
>> xsk_build_skb(). As a seperate patch, I recommend that you change the
>> sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
>> I expect this will be a large performance improvement on it's own.
>> Can I ask you to benchmark this change before the batch xmit change?
>>
>> Opinions needed from other maintainers please (I might be wrong!):
>> I don't think the socket level accounting done in sock_alloc_send_skb()
>> is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
>> code comment above.
> 
> Here I'm bringing back the last test you expected to know :)
> 
> I use alloc_skb() to replace sock_alloc_send_skb() and introduce other
> minor changes, say, removing sock_wfree() from xsk_destruct_skb(). It
> turns out to be a stable 5% performance improvement on i40e driver.
> slight improvement on virtio_net. That's good news.
> 
> Bad news is that the above logic has bugs like freeing skb in the napi
> poll causes accessing skb->sk in xsk_destruct_skb() which triggers a
> NULL pointer issue. How did I spot this one? I removed the BQL flow
> control and started two xdpsock on different queues, then I saw a
> panic[1]... To solve the problem like that, I'm afraid that we still
> need to charge a certain length value into sk_wmem_alloc so that
> sock_wfree(skb) can be the last one to free the socket finally.
> 
> So this socket level accounting mechanism keeps its safety in the above case.
> 
> IMHO, we can get rid of the limitation of sk_sndbuf but still use
> skb_set_owner_w() that charges the len of skb. If we stick to removing
> the whole accounting function, probably we have to adjust the position
> of xsk_cq_submit_locked(), but I reckon for now it's not practical...
> 
> Any thoughts on this?
> 
> [1]
>   997 [  133.528449] RIP: 0010:xsk_destruct_skb+0x6a/0x90
>   998 [  133.528920] Code: 8b 6c 02 28 48 8b 43 18 4c 8b a0 68 03 00 00
> 49 8d 9c 24 e8 00 00 00 48 89 df e8 f1 eb 06 00 48 89 c6 49 8b 84 24
> 88 00 00 00 <48> 8b 50 10 03 2a 48      8b 40 10 48 89 df 89 28 5b 5d
> 41 5c e9 6e ec
>   999 [  133.530526] RSP: 0018:ffffae71c06a0d08 EFLAGS: 00010046
> 1000 [  133.531005] RAX: 0000000000000000 RBX: ffff9f42c81c49e8 RCX:
> 00000000000002e7
> 1001 [  133.531631] RDX: 0000000000000001 RSI: 0000000000000286 RDI:
> ffff9f42c81c49e8
> 1002 [  133.532249] RBP: 0000000000000001 R08: 0000000000000008 R09:
> 00000000000000001003 [  133.532867] R10: ffffffff978080c0 R11:
> ffffae71c06a0ff8 R12: ffff9f42c81c4900
> 1004 [  133.533491] R13: ffffae71c06a0d88 R14: ffff9f42e0f1f900 R15:
> ffff9f42ce850d801005 [  133.534123] FS:  0000000000000000(0000)
> GS:ffff9f5227655000(0000) knlGS:00000000000000001006 [  133.534831]
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> 1007 [  133.535366] CR2: 0000000000000010 CR3: 000000011c820000 CR4:
> 00000000003506f0
> 1008 [  133.536014] Call Trace:
> 1009 [  133.536313]  <IRQ>
> 1010 [  133.536583]  skb_release_head_state+0x20/0x90
> 1011 [  133.537021]  napi_consume_skb+0x42/0x120
> 1012 [  133.537429]  __free_old_xmit+0x76/0x170 [virtio_net]
> 1013 [  133.537923]  free_old_xmit+0x53/0xc0 [virtio_net]
> 1014 [  133.538395]  virtnet_poll+0xed/0x5d0 [virtio_net]
> 1015 [  133.538867]  ? blake2s_compress+0x52/0xa0
> 1016 [  133.539286]  __napi_poll+0x28/0x200
> 1017 [  133.539668]  net_rx_action+0x319/0x400
> 1018 [  133.540068]  ? sched_clock_cpu+0xb/0x190
> 1019 [  133.540482]  ? __run_timers+0x1d1/0x260
> 1020 [  133.540906]  ? __pfx_dl_task_timer+0x10/0x10
> 1021 [  133.541349]  ? lock_timer_base+0x72/0x90
> 1022 [  133.541767]  handle_softirqs+0xce/0x2e0
> 1023 [  133.542178]  __irq_exit_rcu+0xc6/0xf0
> 1024 [  133.542575]  common_interrupt+0x81/0xa0
> 
> Thanks,
> Jason

Re: [PATCH net-next 2/2] xsk: support generic batch xmit in copy mode

[Jason Xing]

On Fri, Aug 22, 2025 at 1:29 AM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>
> I need some help from Cc Magnus or Björn, to explain why you changes
> fails in xsk_destruct_skb().

Oh, I mean the reason for using socket level accounting is we need to
make sure of the safety in tx completion period. In xsk, that is,
xsk_destruct_skb needs to fetch the corresponding sk from skb and
manipulate its ring structure. Without accounting, the socket can be
released and destroyed before the driver calls skb->destructor(). Only
with the accounting protection, the socket is still alive because the
following code:
sock_wfree()
    -> if (refcount_sub_and_test(len, &sk->sk_wmem_alloc))
             __sk_free(sk);

It seems no way to rid the accounting feature for now without
refactoring the whole logic.

We can probably remove the sk_sndbuf limitation, but I still do more
investigation :)

Thanks,
Jason

>
>
> On 15/08/2025 08.44, Jason Xing wrote:
> > On Tue, Aug 12, 2025 at 10:30 PM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
> >>
> > ...
> >>
> >> But this also requires changing the SKB alloc function used by
> >> xsk_build_skb(). As a seperate patch, I recommend that you change the
> >> sock_alloc_send_skb() to instead use build_skb (or build_skb_around).
> >> I expect this will be a large performance improvement on it's own.
> >> Can I ask you to benchmark this change before the batch xmit change?
> >>
> >> Opinions needed from other maintainers please (I might be wrong!):
> >> I don't think the socket level accounting done in sock_alloc_send_skb()
> >> is correct/relevant for AF_XDP/XSK, because the "backpressure mechanism"
> >> code comment above.
> >
> > Here I'm bringing back the last test you expected to know :)
> >
> > I use alloc_skb() to replace sock_alloc_send_skb() and introduce other
> > minor changes, say, removing sock_wfree() from xsk_destruct_skb(). It
> > turns out to be a stable 5% performance improvement on i40e driver.
> > slight improvement on virtio_net. That's good news.
> >
> > Bad news is that the above logic has bugs like freeing skb in the napi
> > poll causes accessing skb->sk in xsk_destruct_skb() which triggers a
> > NULL pointer issue. How did I spot this one? I removed the BQL flow
> > control and started two xdpsock on different queues, then I saw a
> > panic[1]... To solve the problem like that, I'm afraid that we still
> > need to charge a certain length value into sk_wmem_alloc so that
> > sock_wfree(skb) can be the last one to free the socket finally.
> >
> > So this socket level accounting mechanism keeps its safety in the above case.
> >
> > IMHO, we can get rid of the limitation of sk_sndbuf but still use
> > skb_set_owner_w() that charges the len of skb. If we stick to removing
> > the whole accounting function, probably we have to adjust the position
> > of xsk_cq_submit_locked(), but I reckon for now it's not practical...
> >
> > Any thoughts on this?
> >
> > [1]
> >   997 [  133.528449] RIP: 0010:xsk_destruct_skb+0x6a/0x90
> >   998 [  133.528920] Code: 8b 6c 02 28 48 8b 43 18 4c 8b a0 68 03 00 00
> > 49 8d 9c 24 e8 00 00 00 48 89 df e8 f1 eb 06 00 48 89 c6 49 8b 84 24
> > 88 00 00 00 <48> 8b 50 10 03 2a 48      8b 40 10 48 89 df 89 28 5b 5d
> > 41 5c e9 6e ec
> >   999 [  133.530526] RSP: 0018:ffffae71c06a0d08 EFLAGS: 00010046
> > 1000 [  133.531005] RAX: 0000000000000000 RBX: ffff9f42c81c49e8 RCX:
> > 00000000000002e7
> > 1001 [  133.531631] RDX: 0000000000000001 RSI: 0000000000000286 RDI:
> > ffff9f42c81c49e8
> > 1002 [  133.532249] RBP: 0000000000000001 R08: 0000000000000008 R09:
> > 00000000000000001003 [  133.532867] R10: ffffffff978080c0 R11:
> > ffffae71c06a0ff8 R12: ffff9f42c81c4900
> > 1004 [  133.533491] R13: ffffae71c06a0d88 R14: ffff9f42e0f1f900 R15:
> > ffff9f42ce850d801005 [  133.534123] FS:  0000000000000000(0000)
> > GS:ffff9f5227655000(0000) knlGS:00000000000000001006 [  133.534831]
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > 1007 [  133.535366] CR2: 0000000000000010 CR3: 000000011c820000 CR4:
> > 00000000003506f0
> > 1008 [  133.536014] Call Trace:
> > 1009 [  133.536313]  <IRQ>
> > 1010 [  133.536583]  skb_release_head_state+0x20/0x90
> > 1011 [  133.537021]  napi_consume_skb+0x42/0x120
> > 1012 [  133.537429]  __free_old_xmit+0x76/0x170 [virtio_net]
> > 1013 [  133.537923]  free_old_xmit+0x53/0xc0 [virtio_net]
> > 1014 [  133.538395]  virtnet_poll+0xed/0x5d0 [virtio_net]
> > 1015 [  133.538867]  ? blake2s_compress+0x52/0xa0
> > 1016 [  133.539286]  __napi_poll+0x28/0x200
> > 1017 [  133.539668]  net_rx_action+0x319/0x400
> > 1018 [  133.540068]  ? sched_clock_cpu+0xb/0x190
> > 1019 [  133.540482]  ? __run_timers+0x1d1/0x260
> > 1020 [  133.540906]  ? __pfx_dl_task_timer+0x10/0x10
> > 1021 [  133.541349]  ? lock_timer_base+0x72/0x90
> > 1022 [  133.541767]  handle_softirqs+0xce/0x2e0
> > 1023 [  133.542178]  __irq_exit_rcu+0xc6/0xf0
> > 1024 [  133.542575]  common_interrupt+0x81/0xa0
> >
> > Thanks,
> > Jason
>

Re: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20250811131236.56206-2-kerneljasonxing@gmail.com>
References: <20250811131236.56206-2-kerneljasonxing@gmail.com>
TO: Jason Xing <kerneljasonxing@gmail.com>
TO: davem@davemloft.net
TO: edumazet@google.com
TO: kuba@kernel.org
TO: pabeni@redhat.com
TO: bjorn@kernel.org
TO: magnus.karlsson@intel.com
TO: maciej.fijalkowski@intel.com
TO: jonathan.lemon@gmail.com
TO: sdf@fomichev.me
TO: ast@kernel.org
TO: daniel@iogearbox.net
TO: hawk@kernel.org
TO: john.fastabend@gmail.com
TO: horms@kernel.org
TO: andrew+netdev@lunn.ch
CC: bpf@vger.kernel.org
CC: netdev@vger.kernel.org
CC: Jason Xing <kernelxing@tencent.com>

Hi Jason,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Xing/xsk-introduce-XDP_GENERIC_XMIT_BATCH-setsockopt/20250811-211509
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250811131236.56206-2-kerneljasonxing%40gmail.com
patch subject: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt
:::::: branch date: 6 days ago
:::::: commit date: 6 days ago
config: s390-randconfig-r073-20250817 (https://download.01.org/0day-ci/archive/20250817/202508171049.SGYNFbP3-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202508171049.SGYNFbP3-lkp@intel.com/

smatch warnings:
net/xdp/xsk.c:1495 xsk_setsockopt() warn: inconsistent returns '&xs->mutex'.

vim +1495 net/xdp/xsk.c

c05cd364581472 Kevin Laatz        2019-08-27  1343  
c0c77d8fb787cf Björn Töpel        2018-05-02  1344  static int xsk_setsockopt(struct socket *sock, int level, int optname,
a7b75c5a8c4144 Christoph Hellwig  2020-07-23  1345  			  sockptr_t optval, unsigned int optlen)
c0c77d8fb787cf Björn Töpel        2018-05-02  1346  {
c0c77d8fb787cf Björn Töpel        2018-05-02  1347  	struct sock *sk = sock->sk;
c0c77d8fb787cf Björn Töpel        2018-05-02  1348  	struct xdp_sock *xs = xdp_sk(sk);
c0c77d8fb787cf Björn Töpel        2018-05-02  1349  	int err;
c0c77d8fb787cf Björn Töpel        2018-05-02  1350  
c0c77d8fb787cf Björn Töpel        2018-05-02  1351  	if (level != SOL_XDP)
c0c77d8fb787cf Björn Töpel        2018-05-02  1352  		return -ENOPROTOOPT;
c0c77d8fb787cf Björn Töpel        2018-05-02  1353  
c0c77d8fb787cf Björn Töpel        2018-05-02  1354  	switch (optname) {
b9b6b68e8abd10 Björn Töpel        2018-05-02  1355  	case XDP_RX_RING:
f61459030ec7ff Magnus Karlsson    2018-05-02  1356  	case XDP_TX_RING:
b9b6b68e8abd10 Björn Töpel        2018-05-02  1357  	{
b9b6b68e8abd10 Björn Töpel        2018-05-02  1358  		struct xsk_queue **q;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1359  		int entries;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1360  
b9b6b68e8abd10 Björn Töpel        2018-05-02  1361  		if (optlen < sizeof(entries))
b9b6b68e8abd10 Björn Töpel        2018-05-02  1362  			return -EINVAL;
a7b75c5a8c4144 Christoph Hellwig  2020-07-23  1363  		if (copy_from_sockptr(&entries, optval, sizeof(entries)))
b9b6b68e8abd10 Björn Töpel        2018-05-02  1364  			return -EFAULT;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1365  
b9b6b68e8abd10 Björn Töpel        2018-05-02  1366  		mutex_lock(&xs->mutex);
455302d1c9ae93 Ilya Maximets      2019-06-28  1367  		if (xs->state != XSK_READY) {
455302d1c9ae93 Ilya Maximets      2019-06-28  1368  			mutex_unlock(&xs->mutex);
455302d1c9ae93 Ilya Maximets      2019-06-28  1369  			return -EBUSY;
455302d1c9ae93 Ilya Maximets      2019-06-28  1370  		}
f61459030ec7ff Magnus Karlsson    2018-05-02  1371  		q = (optname == XDP_TX_RING) ? &xs->tx : &xs->rx;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1372  		err = xsk_init_queue(entries, q, false);
77cd0d7b3f257f Magnus Karlsson    2019-08-14  1373  		if (!err && optname == XDP_TX_RING)
77cd0d7b3f257f Magnus Karlsson    2019-08-14  1374  			/* Tx needs to be explicitly woken up the first time */
77cd0d7b3f257f Magnus Karlsson    2019-08-14  1375  			xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1376  		mutex_unlock(&xs->mutex);
b9b6b68e8abd10 Björn Töpel        2018-05-02  1377  		return err;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1378  	}
c0c77d8fb787cf Björn Töpel        2018-05-02  1379  	case XDP_UMEM_REG:
c0c77d8fb787cf Björn Töpel        2018-05-02  1380  	{
c05cd364581472 Kevin Laatz        2019-08-27  1381  		size_t mr_size = sizeof(struct xdp_umem_reg);
c05cd364581472 Kevin Laatz        2019-08-27  1382  		struct xdp_umem_reg mr = {};
c0c77d8fb787cf Björn Töpel        2018-05-02  1383  		struct xdp_umem *umem;
c0c77d8fb787cf Björn Töpel        2018-05-02  1384  
c05cd364581472 Kevin Laatz        2019-08-27  1385  		if (optlen < sizeof(struct xdp_umem_reg_v1))
c05cd364581472 Kevin Laatz        2019-08-27  1386  			return -EINVAL;
341ac980eab90a Stanislav Fomichev 2023-11-27  1387  		else if (optlen < sizeof(mr))
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1388  			mr_size = sizeof(struct xdp_umem_reg_v1);
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1389  
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1390  		BUILD_BUG_ON(sizeof(struct xdp_umem_reg_v1) >= sizeof(struct xdp_umem_reg));
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1391  
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1392  		/* Make sure the last field of the struct doesn't have
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1393  		 * uninitialized padding. All padding has to be explicit
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1394  		 * and has to be set to zero by the userspace to make
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1395  		 * struct xdp_umem_reg extensible in the future.
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1396  		 */
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1397  		BUILD_BUG_ON(offsetof(struct xdp_umem_reg, tx_metadata_len) +
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1398  			     sizeof_field(struct xdp_umem_reg, tx_metadata_len) !=
32654bbd6313b4 Stanislav Fomichev 2024-07-26  1399  			     sizeof(struct xdp_umem_reg));
c05cd364581472 Kevin Laatz        2019-08-27  1400  
a7b75c5a8c4144 Christoph Hellwig  2020-07-23  1401  		if (copy_from_sockptr(&mr, optval, mr_size))
c0c77d8fb787cf Björn Töpel        2018-05-02  1402  			return -EFAULT;
c0c77d8fb787cf Björn Töpel        2018-05-02  1403  
c0c77d8fb787cf Björn Töpel        2018-05-02  1404  		mutex_lock(&xs->mutex);
455302d1c9ae93 Ilya Maximets      2019-06-28  1405  		if (xs->state != XSK_READY || xs->umem) {
a49049ea257656 Björn Töpel        2018-05-22  1406  			mutex_unlock(&xs->mutex);
a49049ea257656 Björn Töpel        2018-05-22  1407  			return -EBUSY;
a49049ea257656 Björn Töpel        2018-05-22  1408  		}
c0c77d8fb787cf Björn Töpel        2018-05-02  1409  
a49049ea257656 Björn Töpel        2018-05-22  1410  		umem = xdp_umem_create(&mr);
a49049ea257656 Björn Töpel        2018-05-22  1411  		if (IS_ERR(umem)) {
c0c77d8fb787cf Björn Töpel        2018-05-02  1412  			mutex_unlock(&xs->mutex);
a49049ea257656 Björn Töpel        2018-05-22  1413  			return PTR_ERR(umem);
c0c77d8fb787cf Björn Töpel        2018-05-02  1414  		}
c0c77d8fb787cf Björn Töpel        2018-05-02  1415  
c0c77d8fb787cf Björn Töpel        2018-05-02  1416  		/* Make sure umem is ready before it can be seen by others */
c0c77d8fb787cf Björn Töpel        2018-05-02  1417  		smp_wmb();
9764f4b301c3e7 Björn Töpel        2019-09-04  1418  		WRITE_ONCE(xs->umem, umem);
c0c77d8fb787cf Björn Töpel        2018-05-02  1419  		mutex_unlock(&xs->mutex);
c0c77d8fb787cf Björn Töpel        2018-05-02  1420  		return 0;
c0c77d8fb787cf Björn Töpel        2018-05-02  1421  	}
423f38329d2679 Magnus Karlsson    2018-05-02  1422  	case XDP_UMEM_FILL_RING:
fe2308328cd2f2 Magnus Karlsson    2018-05-02  1423  	case XDP_UMEM_COMPLETION_RING:
423f38329d2679 Magnus Karlsson    2018-05-02  1424  	{
423f38329d2679 Magnus Karlsson    2018-05-02  1425  		struct xsk_queue **q;
423f38329d2679 Magnus Karlsson    2018-05-02  1426  		int entries;
423f38329d2679 Magnus Karlsson    2018-05-02  1427  
237f3cf13b20db Eric Dumazet       2024-04-04  1428  		if (optlen < sizeof(entries))
237f3cf13b20db Eric Dumazet       2024-04-04  1429  			return -EINVAL;
a7b75c5a8c4144 Christoph Hellwig  2020-07-23  1430  		if (copy_from_sockptr(&entries, optval, sizeof(entries)))
423f38329d2679 Magnus Karlsson    2018-05-02  1431  			return -EFAULT;
423f38329d2679 Magnus Karlsson    2018-05-02  1432  
423f38329d2679 Magnus Karlsson    2018-05-02  1433  		mutex_lock(&xs->mutex);
455302d1c9ae93 Ilya Maximets      2019-06-28  1434  		if (xs->state != XSK_READY) {
455302d1c9ae93 Ilya Maximets      2019-06-28  1435  			mutex_unlock(&xs->mutex);
455302d1c9ae93 Ilya Maximets      2019-06-28  1436  			return -EBUSY;
455302d1c9ae93 Ilya Maximets      2019-06-28  1437  		}
a49049ea257656 Björn Töpel        2018-05-22  1438  
7361f9c3d71955 Magnus Karlsson    2020-08-28  1439  		q = (optname == XDP_UMEM_FILL_RING) ? &xs->fq_tmp :
7361f9c3d71955 Magnus Karlsson    2020-08-28  1440  			&xs->cq_tmp;
b9b6b68e8abd10 Björn Töpel        2018-05-02  1441  		err = xsk_init_queue(entries, q, true);
423f38329d2679 Magnus Karlsson    2018-05-02  1442  		mutex_unlock(&xs->mutex);
423f38329d2679 Magnus Karlsson    2018-05-02  1443  		return err;
423f38329d2679 Magnus Karlsson    2018-05-02  1444  	}
45e359be1ce88f Jason Xing         2025-07-05  1445  	case XDP_MAX_TX_SKB_BUDGET:
45e359be1ce88f Jason Xing         2025-07-05  1446  	{
45e359be1ce88f Jason Xing         2025-07-05  1447  		unsigned int budget;
45e359be1ce88f Jason Xing         2025-07-05  1448  
45e359be1ce88f Jason Xing         2025-07-05  1449  		if (optlen != sizeof(budget))
45e359be1ce88f Jason Xing         2025-07-05  1450  			return -EINVAL;
45e359be1ce88f Jason Xing         2025-07-05  1451  		if (copy_from_sockptr(&budget, optval, sizeof(budget)))
45e359be1ce88f Jason Xing         2025-07-05  1452  			return -EFAULT;
45e359be1ce88f Jason Xing         2025-07-05  1453  		if (!xs->tx ||
45e359be1ce88f Jason Xing         2025-07-05  1454  		    budget < TX_BATCH_SIZE || budget > xs->tx->nentries)
45e359be1ce88f Jason Xing         2025-07-05  1455  			return -EACCES;
45e359be1ce88f Jason Xing         2025-07-05  1456  
45e359be1ce88f Jason Xing         2025-07-05  1457  		WRITE_ONCE(xs->max_tx_budget, budget);
45e359be1ce88f Jason Xing         2025-07-05  1458  		return 0;
45e359be1ce88f Jason Xing         2025-07-05  1459  	}
48248366d9de2a Jason Xing         2025-08-11  1460  	case XDP_GENERIC_XMIT_BATCH:
48248366d9de2a Jason Xing         2025-08-11  1461  	{
48248366d9de2a Jason Xing         2025-08-11  1462  		unsigned int batch, batch_alloc_len;
48248366d9de2a Jason Xing         2025-08-11  1463  		struct sk_buff **new;
48248366d9de2a Jason Xing         2025-08-11  1464  
48248366d9de2a Jason Xing         2025-08-11  1465  		if (optlen != sizeof(batch))
48248366d9de2a Jason Xing         2025-08-11  1466  			return -EINVAL;
48248366d9de2a Jason Xing         2025-08-11  1467  		if (copy_from_sockptr(&batch, optval, sizeof(batch)))
48248366d9de2a Jason Xing         2025-08-11  1468  			return -EFAULT;
48248366d9de2a Jason Xing         2025-08-11  1469  		if (batch > xs->max_tx_budget)
48248366d9de2a Jason Xing         2025-08-11  1470  			return -EACCES;
48248366d9de2a Jason Xing         2025-08-11  1471  
48248366d9de2a Jason Xing         2025-08-11  1472  		mutex_lock(&xs->mutex);
48248366d9de2a Jason Xing         2025-08-11  1473  		if (!batch) {
48248366d9de2a Jason Xing         2025-08-11  1474  			kfree(xs->skb_batch);
48248366d9de2a Jason Xing         2025-08-11  1475  			xs->generic_xmit_batch = 0;
48248366d9de2a Jason Xing         2025-08-11  1476  			goto out;
48248366d9de2a Jason Xing         2025-08-11  1477  		}
48248366d9de2a Jason Xing         2025-08-11  1478  		batch_alloc_len = sizeof(struct sk_buff *) * batch;
48248366d9de2a Jason Xing         2025-08-11  1479  		new = kmalloc(batch_alloc_len, GFP_KERNEL);
48248366d9de2a Jason Xing         2025-08-11  1480  		if (!new)
48248366d9de2a Jason Xing         2025-08-11  1481  			return -ENOMEM;
48248366d9de2a Jason Xing         2025-08-11  1482  		if (xs->skb_batch)
48248366d9de2a Jason Xing         2025-08-11  1483  			kfree(xs->skb_batch);
48248366d9de2a Jason Xing         2025-08-11  1484  
48248366d9de2a Jason Xing         2025-08-11  1485  		xs->skb_batch = new;
48248366d9de2a Jason Xing         2025-08-11  1486  		xs->generic_xmit_batch = batch;
48248366d9de2a Jason Xing         2025-08-11  1487  out:
48248366d9de2a Jason Xing         2025-08-11  1488  		mutex_unlock(&xs->mutex);
48248366d9de2a Jason Xing         2025-08-11  1489  		return 0;
48248366d9de2a Jason Xing         2025-08-11  1490  	}
c0c77d8fb787cf Björn Töpel        2018-05-02  1491  	default:
c0c77d8fb787cf Björn Töpel        2018-05-02  1492  		break;
c0c77d8fb787cf Björn Töpel        2018-05-02  1493  	}
c0c77d8fb787cf Björn Töpel        2018-05-02  1494  
c0c77d8fb787cf Björn Töpel        2018-05-02 @1495  	return -ENOPROTOOPT;
c0c77d8fb787cf Björn Töpel        2018-05-02  1496  }
c0c77d8fb787cf Björn Töpel        2018-05-02  1497  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[Dan Carpenter]

Hi Jason,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Xing/xsk-introduce-XDP_GENERIC_XMIT_BATCH-setsockopt/20250811-211509
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250811131236.56206-2-kerneljasonxing%40gmail.com
patch subject: [PATCH net-next 1/2] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt
config: s390-randconfig-r073-20250817 (https://download.01.org/0day-ci/archive/20250817/202508171049.SGYNFbP3-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202508171049.SGYNFbP3-lkp@intel.com/

smatch warnings:
net/xdp/xsk.c:1495 xsk_setsockopt() warn: inconsistent returns '&xs->mutex'.

vim +1495 net/xdp/xsk.c

48248366d9de2a Jason Xing         2025-08-11  1460  	case XDP_GENERIC_XMIT_BATCH:
48248366d9de2a Jason Xing         2025-08-11  1461  	{
48248366d9de2a Jason Xing         2025-08-11  1462  		unsigned int batch, batch_alloc_len;
48248366d9de2a Jason Xing         2025-08-11  1463  		struct sk_buff **new;
48248366d9de2a Jason Xing         2025-08-11  1464  
48248366d9de2a Jason Xing         2025-08-11  1465  		if (optlen != sizeof(batch))
48248366d9de2a Jason Xing         2025-08-11  1466  			return -EINVAL;
48248366d9de2a Jason Xing         2025-08-11  1467  		if (copy_from_sockptr(&batch, optval, sizeof(batch)))
48248366d9de2a Jason Xing         2025-08-11  1468  			return -EFAULT;
48248366d9de2a Jason Xing         2025-08-11  1469  		if (batch > xs->max_tx_budget)
48248366d9de2a Jason Xing         2025-08-11  1470  			return -EACCES;
48248366d9de2a Jason Xing         2025-08-11  1471  
48248366d9de2a Jason Xing         2025-08-11  1472  		mutex_lock(&xs->mutex);
48248366d9de2a Jason Xing         2025-08-11  1473  		if (!batch) {
48248366d9de2a Jason Xing         2025-08-11  1474  			kfree(xs->skb_batch);
48248366d9de2a Jason Xing         2025-08-11  1475  			xs->generic_xmit_batch = 0;
48248366d9de2a Jason Xing         2025-08-11  1476  			goto out;
48248366d9de2a Jason Xing         2025-08-11  1477  		}
48248366d9de2a Jason Xing         2025-08-11  1478  		batch_alloc_len = sizeof(struct sk_buff *) * batch;
48248366d9de2a Jason Xing         2025-08-11  1479  		new = kmalloc(batch_alloc_len, GFP_KERNEL);
48248366d9de2a Jason Xing         2025-08-11  1480  		if (!new)
48248366d9de2a Jason Xing         2025-08-11  1481  			return -ENOMEM;

mutex_unlock(&xs->mutex); before returning

48248366d9de2a Jason Xing         2025-08-11  1482  		if (xs->skb_batch)
48248366d9de2a Jason Xing         2025-08-11  1483  			kfree(xs->skb_batch);
48248366d9de2a Jason Xing         2025-08-11  1484  
48248366d9de2a Jason Xing         2025-08-11  1485  		xs->skb_batch = new;
48248366d9de2a Jason Xing         2025-08-11  1486  		xs->generic_xmit_batch = batch;
48248366d9de2a Jason Xing         2025-08-11  1487  out:
48248366d9de2a Jason Xing         2025-08-11  1488  		mutex_unlock(&xs->mutex);
48248366d9de2a Jason Xing         2025-08-11  1489  		return 0;
48248366d9de2a Jason Xing         2025-08-11  1490  	}
c0c77d8fb787cf Björn Töpel        2018-05-02  1491  	default:
c0c77d8fb787cf Björn Töpel        2018-05-02  1492  		break;
c0c77d8fb787cf Björn Töpel        2018-05-02  1493  	}
c0c77d8fb787cf Björn Töpel        2018-05-02  1494  
c0c77d8fb787cf Björn Töpel        2018-05-02 @1495  	return -ENOPROTOOPT;
c0c77d8fb787cf Björn Töpel        2018-05-02  1496  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki