Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation
From: Omar Elghoul <hidden>
Date: 2026-03-23 16:55:37
Also in:
lkml, virtualization
On 3/23/26 11:52 AM, Michael S. Tsirkin wrote:
On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:quoted
[...]Well... I am not sure how I missed it. Obvious in hindsight: static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq, void *buf, unsigned int len, void **ctx, unsigned int *xdp_xmit, struct virtnet_rq_stats *stats) { struct net_device *dev = vi->dev; struct sk_buff *skb; u8 flags; if (unlikely(len < vi->hdr_len + ETH_HLEN)) { pr_debug("%s: short packet %i\n", dev->name, len); DEV_STATS_INC(dev, rx_length_errors); virtnet_rq_free_buf(vi, rq, buf); return; } /* About the flags below: * 1. Save the flags early, as the XDP program might overwrite them. * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID * stay valid after XDP processing. * 2. XDP doesn't work with partially checksummed packets (refer to * virtnet_xdp_set()), so packets marked as * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing. */ if (vi->mergeable_rx_bufs) { flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags; skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit, stats); } else if (vi->big_packets) { void *p = page_address((struct page *)buf); flags = ((struct virtio_net_common_hdr *)p)->hdr.flags; skb = receive_big(dev, vi, rq, buf, len, stats); } else { flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags; skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats); } So we are reading the header, before dma sync, which is within receive_mergeable and friends:
Thank you for your analysis and explanation.
static struct sk_buff *receive_mergeable(struct net_device *dev,
struct virtnet_info *vi,
struct receive_queue *rq,
void *buf,
void *ctx,
unsigned int len,
unsigned int *xdp_xmit,
struct virtnet_rq_stats *stats)
{
struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
int num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
struct page *page = virt_to_head_page(buf);
int offset = buf - page_address(page);
struct sk_buff *head_skb, *curr_skb;
unsigned int truesize = mergeable_ctx_to_truesize(ctx);
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
head_skb = NULL;
if (rq->use_page_pool_dma)
page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
Just as a test, the below should fix it (compiled only), but the real
fix is more complex since we need to be careful to avoid expensive syncing
twice.I applied your patch and tested it on my system. With this change, I could not reproduce the same error anymore. I would be happy to test a proper fix once you have one.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 97035b49bae7..57b4f5954bed 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c@@ -931,9 +931,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx) { + void *buf; + BUG_ON(!rq->page_pool); - return virtqueue_get_buf_ctx(rq->vq, len, ctx); + buf = virtqueue_get_buf_ctx(rq->vq, len, ctx); + if (buf && rq->use_page_pool_dma && *len) { + struct page *page = virt_to_head_page(buf); + int offset = buf - page_address(page); + + page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, *len); + } + + return buf; } static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)