[PATCH net 0/2] virtio-net: xsk: rx: fix the frame's length check

[PATCH net 0/2] virtio-net: xsk: rx: fix the frame's length check

[Bui Quang Minh]

Hi everyone,

This series contains 2 patches for the zerocopy XDP receive path in virtio
net
- Patch 1: there is a difference between first buffer and the following
buffers in this receive path. While the first buffer contains virtio
header, the following ones do not. So the length of the remaining region
for frame data is also different in 2 cases. The current maximum frame's
length check is only correct for the following buffers not the first one.
- Patch 2: no functional change. The tricky xdp->data adjustment due to
the above difference is moved to buf_to_xdp() so that this helper contains
all logic to build xdp_buff and the tricky adjustment does not scatter
over different functions.

Thanks,
Quang Minh.

Bui Quang Minh (2):
  virtio-net: xsk: rx: fix the frame's length check
  virtio-net: xsk: rx: move the xdp->data adjustment to buf_to_xdp()

 drivers/net/virtio_net.c | 46 ++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net 1/2] virtio-net: xsk: rx: fix the frame's length check

[Bui Quang Minh]

When calling buf_to_xdp, the len argument is the frame data's length
without virtio header's length (vi->hdr_len). We check that len with

	xsk_pool_get_rx_frame_size() + vi->hdr_len

to ensure the provided len does not larger than the allocated chunk
size. The additional vi->hdr_len is because in virtnet_add_recvbuf_xsk,
we use part of XDP_PACKET_HEADROOM for virtio header and ask the vhost
to start placing data from

	hard_start + XDP_PACKET_HEADROOM - vi->hdr_len
not
	hard_start + XDP_PACKET_HEADROOM

But the first buffer has virtio_header, so the maximum frame's length in
the first buffer can only be

	xsk_pool_get_rx_frame_size()
not
	xsk_pool_get_rx_frame_size() + vi->hdr_len

like in the current check.

This commit adds an additional argument to buf_to_xdp differentiate
between the first buffer and other ones to correctly calculate the maximum
frame's length.

Fixes: a4e7ba702701 ("virtio_net: xsk: rx: support recv small mode")
Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/net/virtio_net.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e53ba600605a..7c9cf5ed1827 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1127,15 +1127,37 @@ static void check_sq_full_and_disable(struct virtnet_info *vi,
 	}
 }
 
+/**
+ * buf_to_xdp() - convert the @buf context to xdp_buff
+ * @vi: virtnet_info struct
+ * @rq: the receive queue struct
+ * @buf: the xdp_buff pointer that is passed to virtqueue_add_inbuf_premapped in
+ *       virtnet_add_recvbuf_xsk
+ * @len: the length of received data without virtio header's length
+ * @first_buf: this buffer is the first one or not
+ */
 static struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
-				   struct receive_queue *rq, void *buf, u32 len)
+				   struct receive_queue *rq, void *buf,
+				   u32 len, bool first_buf)
 {
 	struct xdp_buff *xdp;
 	u32 bufsize;
 
 	xdp = (struct xdp_buff *)buf;
 
-	bufsize = xsk_pool_get_rx_frame_size(rq->xsk_pool) + vi->hdr_len;
+	/* In virtnet_add_recvbuf_xsk, we use part of XDP_PACKET_HEADROOM for
+	 * virtio header and ask the vhost to fill data from
+	 *         hard_start + XDP_PACKET_HEADROOM - vi->hdr_len
+	 * The first buffer has virtio header so the remaining region for frame
+	 * data is
+	 *         xsk_pool_get_rx_frame_size()
+	 * While other buffers than the first one do not have virtio header, so
+	 * the maximum frame data's length can be
+	 *         xsk_pool_get_rx_frame_size() + vi->hdr_len
+	 */
+	bufsize = xsk_pool_get_rx_frame_size(rq->xsk_pool);
+	if (!first_buf)
+		bufsize += vi->hdr_len;
 
 	if (unlikely(len > bufsize)) {
 		pr_debug("%s: rx error: len %u exceeds truesize %u\n",
@@ -1260,7 +1282,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
 
 		u64_stats_add(&stats->bytes, len);
 
-		xdp = buf_to_xdp(vi, rq, buf, len);
+		xdp = buf_to_xdp(vi, rq, buf, len, false);
 		if (!xdp)
 			goto err;
 
@@ -1358,7 +1380,7 @@ static void virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queu
 
 	u64_stats_add(&stats->bytes, len);
 
-	xdp = buf_to_xdp(vi, rq, buf, len);
+	xdp = buf_to_xdp(vi, rq, buf, len, true);
 	if (!xdp)
 		return;
 
-- 
2.43.0

Re: [PATCH net 1/2] virtio-net: xsk: rx: fix the frame's length check

[Jakub Kicinski]

On Sun, 15 Jun 2025 22:13:32 +0700 Bui Quang Minh wrote:
> +/**
> + * buf_to_xdp() - convert the @buf context to xdp_buff
> + * @vi: virtnet_info struct
> + * @rq: the receive queue struct
> + * @buf: the xdp_buff pointer that is passed to virtqueue_add_inbuf_premapped in
> + *       virtnet_add_recvbuf_xsk
> + * @len: the length of received data without virtio header's length
> + * @first_buf: this buffer is the first one or not
> + */
>  static struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
> -				   struct receive_queue *rq, void *buf, u32 len)
> +				   struct receive_queue *rq, void *buf,
> +				   u32 len, bool first_buf)

I think Michael mention he's AFK so while we wait could you fix this
kdoc? I'm not sure whether the kdoc is really necessary here, but if
you want to keep it you have to document the return value:

Warning: drivers/net/virtio_net.c:1141 No description found for return value of 'buf_to_xdp'
-- 
pw-bot: cr

Re: [PATCH net 1/2] virtio-net: xsk: rx: fix the frame's length check

[Bui Quang Minh]

On 6/19/25 09:11, Jakub Kicinski wrote:
> On Sun, 15 Jun 2025 22:13:32 +0700 Bui Quang Minh wrote:
>> +/**
>> + * buf_to_xdp() - convert the @buf context to xdp_buff
>> + * @vi: virtnet_info struct
>> + * @rq: the receive queue struct
>> + * @buf: the xdp_buff pointer that is passed to virtqueue_add_inbuf_premapped in
>> + *       virtnet_add_recvbuf_xsk
>> + * @len: the length of received data without virtio header's length
>> + * @first_buf: this buffer is the first one or not
>> + */
>>   static struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
>> -				   struct receive_queue *rq, void *buf, u32 len)
>> +				   struct receive_queue *rq, void *buf,
>> +				   u32 len, bool first_buf)
> I think Michael mention he's AFK so while we wait could you fix this
> kdoc? I'm not sure whether the kdoc is really necessary here, but if
> you want to keep it you have to document the return value:
>
> Warning: drivers/net/virtio_net.c:1141 No description found for return value of 'buf_to_xdp'

I want to add kdoc to clarify that the @len must be without virtio 
header's length. I'll fix it in the next version.

Thanks,
Quang Minh.

Re: [PATCH net 1/2] virtio-net: xsk: rx: fix the frame's length check

[Jakub Kicinski]

On Thu, 19 Jun 2025 21:17:03 +0700 Bui Quang Minh wrote:
> > I think Michael mention he's AFK so while we wait could you fix this
> > kdoc? I'm not sure whether the kdoc is really necessary here, but if
> > you want to keep it you have to document the return value:
> >
> > Warning: drivers/net/virtio_net.c:1141 No description found for return value of 'buf_to_xdp'  
> 
> I want to add kdoc to clarify that the @len must be without virtio 
> header's length. I'll fix it in the next version.

If that's the case I'd personally limit the doc to:

/* Note that @len is the length of received data without virtio header */

but up to you, it's quite subjective.

[PATCH net 2/2] virtio-net: xsk: rx: move the xdp->data adjustment to buf_to_xdp()

[Bui Quang Minh]

This commit does not do any functional changes. It moves xdp->data
adjustment for buffer other than first buffer to buf_to_xdp() helper so
that the xdp_buff adjustment does not scatter over different functions.

Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/net/virtio_net.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7c9cf5ed1827..678502aefddd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1167,7 +1167,19 @@ static struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
 		return NULL;
 	}
 
-	xsk_buff_set_size(xdp, len);
+	if (first_buf) {
+		xsk_buff_set_size(xdp, len);
+	} else {
+		/* This is the same as xsk_buff_set_size but with the adjusted
+		 * xdp->data.
+		 */
+		xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
+		xdp->data -= vi->hdr_len;
+		xdp->data_meta = xdp->data;
+		xdp->data_end = xdp->data + len;
+		xdp->flags = 0;
+	}
+
 	xsk_buff_dma_sync_for_cpu(xdp);
 
 	return xdp;
@@ -1292,7 +1304,7 @@ static int xsk_append_merge_buffer(struct virtnet_info *vi,
 			goto err;
 		}
 
-		memcpy(buf, xdp->data - vi->hdr_len, len);
+		memcpy(buf, xdp->data, len);
 
 		xsk_buff_free(xdp);
 
-- 
2.43.0