Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation

[Omar Elghoul <oelghoul@linux.ibm.com>]

On 3/23/26 12:58 PM, Michael S. Tsirkin wrote:

> On Mon, Mar 23, 2026 at 11:52:34AM -0400, Michael S. Tsirkin wrote:
>> On Mon, Mar 23, 2026 at 11:01:31AM -0400, Omar Elghoul wrote:
>>> [...]
>> 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:
>>
>> 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.
>>
>>
>> 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)
>>
>>
>>
>>
>> -- 
>> MST
> or maybe like this:
Both of these patches resolve the issue on my end.
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 97035b49bae7..835f52651006 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1956,13 +1956,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
>   	 */
>   	buf -= VIRTNET_RX_PAD + xdp_headroom;
>   
> -	if (rq->use_page_pool_dma) {
> -		int offset = buf - page_address(page) +
> -			     VIRTNET_RX_PAD + xdp_headroom;
> -
> -		page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> -	}
> -
>   	len -= vi->hdr_len;
>   	u64_stats_add(&stats->bytes, len);
>   
> @@ -2398,9 +2391,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>   
>   	head_skb = NULL;
>   
> -	if (rq->use_page_pool_dma)
> -		page_pool_dma_sync_for_cpu(rq->page_pool, page, offset, len);
> -
>   	u64_stats_add(&stats->bytes, len - vi->hdr_len);
>   
>   	if (check_mergeable_len(dev, ctx, len))
> @@ -2563,6 +2553,13 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   		return;
>   	}
>   
> +	if (rq->use_page_pool_dma) {
> +		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);
> +	}
> +
>   	/* 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
>