[PATCH net v2 0/3] fixes two virtio-net related bugs.

[PATCH net v2 0/3] fixes two virtio-net related bugs.

[Xuan Zhuo]

As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
small amount of code duplication.

Commit #3 is new to fix the hdr len in tunnel gso feature.

Hi @Paolo Abenchi,
Could you please test commit #3? I don't have a suitable test environment, as
QEMU doesn't currently support the tunnel GSO feature.

Thanks.

Xuan Zhuo (3):
  virtio-net: fix incorrect flags recording in big mode
  virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  virtio-net: correct hdr_len handling for tunnel gso

 drivers/net/virtio_net.c   | 16 ++++++++-----
 include/linux/virtio_net.h | 46 ++++++++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 12 deletions(-)

--
2.32.0.3.g01195cf9f

[PATCH net v2 1/3] virtio-net: fix incorrect flags recording in big mode

[Xuan Zhuo]

The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
checksummed packets handling") is to record the flags in advance, as
their value may be overwritten in the XDP case. However, the flags
recorded under big mode are incorrect, because in big mode, the passed
buf does not point to the rx buffer, but rather to the page of the
submitted buffer. This commit fixes this issue.

For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
buf address when unmapping for small packets") fixed it.

Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7da5a37917e9..22f7725798e3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2620,22 +2620,28 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 		return;
 	}
 
-	/* 1. Save the flags early, as the XDP program might overwrite them.
+	/* 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.
 	 */
-	flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
 
-	if (vi->mergeable_rx_bufs)
+	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)
+	} 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
+	} else {
+		flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
 		skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
+	}
 
 	if (unlikely(!skb))
 		return;
-- 
2.32.0.3.g01195cf9f

[PATCH net v2 2/3] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

The commit be50da3e9d4a ("net: virtio_net: implement exact header length
guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
feature in virtio-net.

This feature requires virtio-net to set hdr_len to the actual header
length of the packet when transmitting, the number of
bytes from the start of the packet to the beginning of the
transport-layer payload.

However, in practice, hdr_len was being set using skb_headlen(skb),
which is clearly incorrect. This commit fixes that issue.

Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/linux/virtio_net.h | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 20e0584db1dd..e059e9c57937 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -217,20 +217,34 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
 
 	if (skb_is_gso(skb)) {
 		struct skb_shared_info *sinfo = skb_shinfo(skb);
+		u16 hdr_len = 0;
+
+		/* In certain code paths (such as the af_packet.c receive path),
+		 * this function may be called without a transport header.
+		 */
+		if (skb_transport_header_was_set(skb))
+			hdr_len = skb_transport_offset(skb);
 
-		/* This is a hint as to how much should be linear. */
-		hdr->hdr_len = __cpu_to_virtio16(little_endian,
-						 skb_headlen(skb));
 		hdr->gso_size = __cpu_to_virtio16(little_endian,
 						  sinfo->gso_size);
-		if (sinfo->gso_type & SKB_GSO_TCPV4)
+		if (sinfo->gso_type & SKB_GSO_TCPV4) {
 			hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
-		else if (sinfo->gso_type & SKB_GSO_TCPV6)
+			if (hdr_len)
+				hdr_len += tcp_hdrlen(skb);
+		} else if (sinfo->gso_type & SKB_GSO_TCPV6) {
 			hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
-		else if (sinfo->gso_type & SKB_GSO_UDP_L4)
+			if (hdr_len)
+				hdr_len += tcp_hdrlen(skb);
+		} else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
 			hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
-		else
+			if (hdr_len)
+				hdr_len += sizeof(struct udphdr);
+		} else {
 			return -EINVAL;
+		}
+
+		hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
+
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
 			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
 	} else
-- 
2.32.0.3.g01195cf9f

[PATCH net v2 3/3] virtio-net: correct hdr_len handling for tunnel gso

[Xuan Zhuo]

The commit a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP
GSO tunneling.") introduces support for the UDP GSO tunnel feature in
virtio-net.

The virtio spec says:

    If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
    VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
    all the headers up to and including the inner transport.

The commit did not update the hdr_len to include the inner transport.

Fixes: a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP GSO tunneling.")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/linux/virtio_net.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index e059e9c57937..765fd5f471a4 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -403,6 +403,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
 	unsigned int inner_nh, outer_th;
 	int tnl_gso_type;
+	u16 hdr_len;
 	int ret;
 
 	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
@@ -434,6 +435,23 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 	outer_th = skb->transport_header - skb_headroom(skb);
 	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
 	vhdr->outer_th_offset = cpu_to_le16(outer_th);
+
+	switch (skb->inner_ipproto) {
+	case IPPROTO_TCP:
+		hdr_len = inner_tcp_hdrlen(skb);
+		break;
+
+	case IPPROTO_UDP:
+		hdr_len = sizeof(struct udphdr);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	hdr_len += skb_inner_transport_offset(skb);
+	hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
+
 	return 0;
 }
 
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net v2 0/3] fixes two virtio-net related bugs.

[Jason Wang]

On Mon, Oct 13, 2025 at 10:06 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
> Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
> small amount of code duplication.
>
> Commit #3 is new to fix the hdr len in tunnel gso feature.
>
> Hi @Paolo Abenchi,
> Could you please test commit #3? I don't have a suitable test environment, as
> QEMU doesn't currently support the tunnel GSO feature.

It has been there.

Thanks

>
> Thanks.
>
> Xuan Zhuo (3):
>   virtio-net: fix incorrect flags recording in big mode
>   virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
>   virtio-net: correct hdr_len handling for tunnel gso
>
>  drivers/net/virtio_net.c   | 16 ++++++++-----
>  include/linux/virtio_net.h | 46 ++++++++++++++++++++++++++++++++------
>  2 files changed, 50 insertions(+), 12 deletions(-)
>
> --
> 2.32.0.3.g01195cf9f
>

Re: [PATCH net v2 0/3] fixes two virtio-net related bugs.

[Michael S. Tsirkin]

On Mon, Oct 13, 2025 at 10:06:26AM +0800, Xuan Zhuo wrote:
> As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
> Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
> small amount of code duplication.


How were commits 1 and 2 tested?


> Commit #3 is new to fix the hdr len in tunnel gso feature.
> 
> Hi @Paolo Abenchi,
> Could you please test commit #3? I don't have a suitable test environment, as
> QEMU doesn't currently support the tunnel GSO feature.
> 
> Thanks.


AFAIK host_tunnel and guest_tunnel flags enable exactly that.


> Xuan Zhuo (3):
>   virtio-net: fix incorrect flags recording in big mode
>   virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
>   virtio-net: correct hdr_len handling for tunnel gso
> 
>  drivers/net/virtio_net.c   | 16 ++++++++-----
>  include/linux/virtio_net.h | 46 ++++++++++++++++++++++++++++++++------
>  2 files changed, 50 insertions(+), 12 deletions(-)
> 
> --
> 2.32.0.3.g01195cf9f

Re: [PATCH net v2 3/3] virtio-net: correct hdr_len handling for tunnel gso

[Paolo Abeni]

On 10/13/25 4:06 AM, Xuan Zhuo wrote:
> The commit a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP
> GSO tunneling.") introduces support for the UDP GSO tunnel feature in
> virtio-net.
> 
> The virtio spec says:
> 
>     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
>     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
>     all the headers up to and including the inner transport.
> 
> The commit did not update the hdr_len to include the inner transport.
> 
> Fixes: a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Side note: qemu support for UDP GSO tunneling is available in qemu since
commit a5289563ad.

> ---
>  include/linux/virtio_net.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index e059e9c57937..765fd5f471a4 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -403,6 +403,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
>  	unsigned int inner_nh, outer_th;
>  	int tnl_gso_type;
> +	u16 hdr_len;
>  	int ret;
>  
>  	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> @@ -434,6 +435,23 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  	outer_th = skb->transport_header - skb_headroom(skb);
>  	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
>  	vhdr->outer_th_offset = cpu_to_le16(outer_th);
> +
> +	switch (skb->inner_ipproto) {
> +	case IPPROTO_TCP:
> +		hdr_len = inner_tcp_hdrlen(skb);
> +		break;
> +
> +	case IPPROTO_UDP:
> +		hdr_len = sizeof(struct udphdr);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	hdr_len += skb_inner_transport_offset(skb);
> +	hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);

I'm not sure this is the correct fix.

virtio_net_hdr_tnl_from_skb() just called virtio_net_hdr_from_skb() on
the inner header. The virtio spec also specifies:

"""
 If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
 \field{hdr_len} indicates the header length that needs to be replicated
 for each packet. It's the number of bytes from the beginning of the packet
 to the beginning of the transport payload.
"""

If `hdr_len` is currently wrong for UDP GSO packets, it's also wrong for
 plain GSO packets (without UDP tunnel) and the its value should be
possibly fixed in virtio_net_hdr_from_skb().

Thanks,

Paolo

Re: [PATCH net v2 0/3] fixes two virtio-net related bugs.

[Xuan Zhuo]

On Mon, 13 Oct 2025 03:56:35 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Oct 13, 2025 at 10:06:26AM +0800, Xuan Zhuo wrote:
> > As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
> > Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
> > small amount of code duplication.
>
>
> How were commits 1 and 2 tested?


These two are not troublesome. I tested #1 in big mode. #2 has no special
requirements. I tested it on qemu and observed the hdr len.

Thanks.


>
>
> > Commit #3 is new to fix the hdr len in tunnel gso feature.
> >
> > Hi @Paolo Abenchi,
> > Could you please test commit #3? I don't have a suitable test environment, as
> > QEMU doesn't currently support the tunnel GSO feature.
> >
> > Thanks.
>
>
> AFAIK host_tunnel and guest_tunnel flags enable exactly that.
>
>
> > Xuan Zhuo (3):
> >   virtio-net: fix incorrect flags recording in big mode
> >   virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
> >   virtio-net: correct hdr_len handling for tunnel gso
> >
> >  drivers/net/virtio_net.c   | 16 ++++++++-----
> >  include/linux/virtio_net.h | 46 ++++++++++++++++++++++++++++++++------
> >  2 files changed, 50 insertions(+), 12 deletions(-)
> >
> > --
> > 2.32.0.3.g01195cf9f
>

Re: [PATCH net v2 0/3] fixes two virtio-net related bugs.

[Xuan Zhuo]

On Mon, 13 Oct 2025 13:56:58 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Mon, Oct 13, 2025 at 10:06 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
> > Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
> > small amount of code duplication.
> >
> > Commit #3 is new to fix the hdr len in tunnel gso feature.
> >
> > Hi @Paolo Abenchi,
> > Could you please test commit #3? I don't have a suitable test environment, as
> > QEMU doesn't currently support the tunnel GSO feature.
>
> It has been there.

Maybe I missed it.

Thanks.


>
> Thanks
>
> >
> > Thanks.
> >
> > Xuan Zhuo (3):
> >   virtio-net: fix incorrect flags recording in big mode
> >   virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
> >   virtio-net: correct hdr_len handling for tunnel gso
> >
> >  drivers/net/virtio_net.c   | 16 ++++++++-----
> >  include/linux/virtio_net.h | 46 ++++++++++++++++++++++++++++++++------
> >  2 files changed, 50 insertions(+), 12 deletions(-)
> >
> > --
> > 2.32.0.3.g01195cf9f
> >
>

Re: [PATCH net v2 3/3] virtio-net: correct hdr_len handling for tunnel gso

[Xuan Zhuo]

On Mon, 13 Oct 2025 10:11:31 +0200, Paolo Abeni <pabeni@redhat.com> wrote:
> On 10/13/25 4:06 AM, Xuan Zhuo wrote:
> > The commit a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP
> > GSO tunneling.") introduces support for the UDP GSO tunnel feature in
> > virtio-net.
> >
> > The virtio spec says:
> >
> >     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
> >     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
> >     all the headers up to and including the inner transport.
> >
> > The commit did not update the hdr_len to include the inner transport.
> >
> > Fixes: a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Side note: qemu support for UDP GSO tunneling is available in qemu since
> commit a5289563ad.
>
> > ---
> >  include/linux/virtio_net.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index e059e9c57937..765fd5f471a4 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -403,6 +403,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> >  	unsigned int inner_nh, outer_th;
> >  	int tnl_gso_type;
> > +	u16 hdr_len;
> >  	int ret;
> >
> >  	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> > @@ -434,6 +435,23 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >  	outer_th = skb->transport_header - skb_headroom(skb);
> >  	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
> >  	vhdr->outer_th_offset = cpu_to_le16(outer_th);
> > +
> > +	switch (skb->inner_ipproto) {
> > +	case IPPROTO_TCP:
> > +		hdr_len = inner_tcp_hdrlen(skb);
> > +		break;
> > +
> > +	case IPPROTO_UDP:
> > +		hdr_len = sizeof(struct udphdr);
> > +		break;
> > +
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	hdr_len += skb_inner_transport_offset(skb);
> > +	hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
>
> I'm not sure this is the correct fix.
>
> virtio_net_hdr_tnl_from_skb() just called virtio_net_hdr_from_skb() on
> the inner header. The virtio spec also specifies:
>
> """
>  If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>  \field{hdr_len} indicates the header length that needs to be replicated
>  for each packet. It's the number of bytes from the beginning of the packet
>  to the beginning of the transport payload.
> """
>
> If `hdr_len` is currently wrong for UDP GSO packets, it's also wrong for
>  plain GSO packets (without UDP tunnel) and the its value should be
> possibly fixed in virtio_net_hdr_from_skb().

YES. This is what the commit #2 does.

Thanks.


>
> Thanks,
>
> Paolo
>