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

[Jason Wang]

On Tue, Mar 24, 2026 at 12:58 AM Michael S. Tsirkin <mst@xxxxxxxxxx> 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:
> > > Hi,
> > >
> > > I've been testing linux-next (tags later than 03/17) and hit new issues in
> > > virtio-net on s390x. I bisected the issue, and I found this patch to be the
> > > first buggy commit.
> > >
> > > The issue seems to only be reproducible when running in Secure Execution.
> > > Tested in a KVM guest, the virtio-net performance appears greatly reduced,
> > > and the dmesg output shows many instances of the following error messages.
> > >
> > > Partial relevant logs
> > > =====================
> > > [   49.332028] macvtap0: bad gso: type: 0, size: 0, flags 1 tunnel 0 tnl csum 0
> > > [   74.365668] macvtap0: bad gso: type: 2e, size: 27948, flags 0 tunnel 0 tnl csum 0
> > > [  403.302168] macvtap0: bad csum: flags: 2, gso_type: 23 rx_tnl_csum 0
> > > [  403.302271] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> > > [  403.302279] macvtap0: bad csum: flags: 2, gso_type: e1 rx_tnl_csum 0
> > > [  403.309492] macvtap0: bad csum: flags: 2, gso_type: 4c rx_tnl_csum 0
> > > [  403.317029] macvtap0: bad csum: flags: 2, gso_type: e0 rx_tnl_csum 0
> > >
> > > Steps to reproduce
> > > ==================
> > > 1. Boot a Linux guest implementing this patch under QEMU/KVM (*) with SE
> > >    enabled and a virtio-net-ccw device attached.
> > > 2. Run dmesg. The error message is usually already present at boot time,
> > >    but if not, it can be reproduced by creating any network traffic.
> > >
> > > (*) This patch was not tested in a non-KVM hypervisor environment.
> > >
> > > I've further confirmed that reverting this patch onto its parent commit
> > > resolves the issue. Please let me know if you'd like me to test a fix or if
> > > you would need more information.
> > >
> > > Thanks in advance.
> > >
> > > Best,
> > > Omar
> >
> > 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:
>
> 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
>

This looks better.

Acked-by: Jason Wang <jasowang@xxxxxxxxxx>

Thanks