On 3/23/26 10:39 PM, Omar Elghoul wrote:
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


Hello Omar. Thank you for the bisect, apologies for breaking the
functionality. Thank you Michael for the patch, I will be careful next
time about DMA sync issues. Looks like the bug is fixed, is there
anything else needed from my end?


Reply via email to