[PATCH net v8 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN

[PATCH net v8 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

v8:
  1. move changes of virtio_net_hdr_from_skb for udp tunnel to #2
  2. remove change for num_sg

v7:
  1. fix bug reported by robot
  2. Still splitting into two commits, as the issues fixed originate from
     different commits. This separation makes it easier to selectively revert to
     previous versions.

v6:
  1. rename to guest_hdrlen
  2. introduce a function virtio_net_set_hdrlen to set the hdrlen

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 path set fixes that issue.

As discussed in [0], this version checks the VIRTIO_NET_F_GUEST_HDRLEN is
negotiated.


[0]: http://lore.kernel.org/all/20251029030913.20423-1-xuanzhuo@linux.alibaba.com




Xuan Zhuo (2):
  virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  virtio-net: correct hdr_len handling for tunnel gso

 drivers/net/tun_vnet.h     |  2 +-
 drivers/net/virtio_net.c   |  6 ++-
 include/linux/virtio_net.h | 75 ++++++++++++++++++++++++++++++--------
 3 files changed, 66 insertions(+), 17 deletions(-)

--
2.32.0.3.g01195cf9f

[PATCH net v8 1/2] 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>
---
 drivers/net/tun_vnet.h     |  2 +-
 drivers/net/virtio_net.c   |  6 +++-
 include/linux/virtio_net.h | 58 ++++++++++++++++++++++++++++++--------
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index a5f93b6c4482..fa5cab9d3e55 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -244,7 +244,7 @@ tun_vnet_hdr_tnl_from_skb(unsigned int flags,
 
 	if (virtio_net_hdr_tnl_from_skb(skb, tnl_hdr, has_tnl_offload,
 					tun_vnet_is_little_endian(flags),
-					vlan_hlen, true)) {
+					vlan_hlen, true, false)) {
 		struct virtio_net_hdr_v1 *hdr = &tnl_hdr->hash_hdr.hdr;
 		struct skb_shared_info *sinfo = skb_shinfo(skb);
 
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 72d6a9c6a5a2..7106333ef904 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3267,8 +3267,12 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
 	struct virtio_net_hdr_v1_hash_tunnel *hdr;
 	int num_sg;
 	unsigned hdr_len = vi->hdr_len;
+	bool feature_hdrlen;
 	bool can_push;
 
+	feature_hdrlen = virtio_has_feature(vi->vdev,
+					    VIRTIO_NET_F_GUEST_HDRLEN);
+
 	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
 
 	/* Make sure it's safe to cast between formats */
@@ -3288,7 +3292,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
 
 	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
 					virtio_is_little_endian(vi->vdev), 0,
-					false))
+					false, feature_hdrlen))
 		return -EPROTO;
 
 	if (vi->mergeable_rx_bufs)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 75dabb763c65..23f42bb8ece0 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -207,20 +207,40 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 	return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type);
 }
 
-static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
-					  struct virtio_net_hdr *hdr,
-					  bool little_endian,
-					  bool has_data_valid,
-					  int vlan_hlen)
+static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
+					 struct virtio_net_hdr *hdr,
+					 bool little_endian,
+					 bool feature_hdrlen)
+{
+	u16 hdr_len;
+
+	if (feature_hdrlen) {
+		hdr_len = skb_transport_offset(skb);
+
+		if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
+			hdr_len += sizeof(struct udphdr);
+		else
+			hdr_len += tcp_hdrlen(skb);
+	} else {
+		/* This is a hint as to how much should be linear. */
+		hdr_len = skb_headlen(skb);
+	}
+
+	hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
+}
+
+static inline int __virtio_net_hdr_from_skb(const struct sk_buff *skb,
+					    struct virtio_net_hdr *hdr,
+					    bool little_endian,
+					    bool has_data_valid,
+					    bool feature_hdrlen,
+					    int vlan_hlen)
 {
 	memset(hdr, 0, sizeof(*hdr));   /* no info leak */
 
 	if (skb_is_gso(skb)) {
 		struct skb_shared_info *sinfo = skb_shinfo(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)
@@ -231,6 +251,10 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
 			hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
 		else
 			return -EINVAL;
+
+		virtio_net_set_hdrlen(skb, hdr, little_endian,
+				      feature_hdrlen);
+
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
 			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
 	} else
@@ -250,6 +274,16 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
 	return 0;
 }
 
+static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
+					  struct virtio_net_hdr *hdr,
+					  bool little_endian,
+					  bool has_data_valid,
+					  int vlan_hlen)
+{
+	return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
+					 has_data_valid, false, vlan_hlen);
+}
+
 static inline unsigned int virtio_l3min(bool is_ipv6)
 {
 	return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
@@ -385,7 +419,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 			    bool tnl_hdr_negotiated,
 			    bool little_endian,
 			    int vlan_hlen,
-			    bool has_data_valid)
+			    bool has_data_valid,
+			    bool feature_hdrlen)
 {
 	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
 	unsigned int inner_nh, outer_th;
@@ -395,8 +430,9 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
 						    SKB_GSO_UDP_TUNNEL_CSUM);
 	if (!tnl_gso_type)
-		return virtio_net_hdr_from_skb(skb, hdr, little_endian,
-					       has_data_valid, vlan_hlen);
+		return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
+						 has_data_valid,
+						 feature_hdrlen, vlan_hlen);
 
 	/* Tunnel support not negotiated but skb ask for it. */
 	if (!tnl_hdr_negotiated)
-- 
2.32.0.3.g01195cf9f

[PATCH net v8 2/2] 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.

I observed that the "hdr_len" is 116 for this packet:

    17:36:18.241105 52:55:00:d1:27:0a > 2e:2c:df:46:a9:e1, ethertype IPv4 (0x0800), length 2912: (tos 0x0, ttl 64, id 45197, offset 0, flags [none], proto UDP (17), length 2898)
        192.168.122.100.50613 > 192.168.122.1.4789: [bad udp cksum 0x8106 -> 0x26a0!] VXLAN, flags [I] (0x08), vni 1
    fa:c3:ba:82:05:ee > ce:85:0c:31:77:e5, ethertype IPv4 (0x0800), length 2862: (tos 0x0, ttl 64, id 14678, offset 0, flags [DF], proto TCP (6), length 2848)
        192.168.3.1.49880 > 192.168.3.2.9898: Flags [P.], cksum 0x9266 (incorrect -> 0xaa20), seq 515667:518463, ack 1, win 64, options [nop,nop,TS val 2990048824 ecr 2798801412], length 2796

116 = 14(mac) + 20(ip) + 8(udp) + 8(vxlan) + 14(inner mac) + 20(inner ip) + 32(innner tcp)

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 | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 23f42bb8ece0..906157779955 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -209,18 +209,29 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 
 static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
 					 struct virtio_net_hdr *hdr,
+					 struct skb_shared_info *sinfo,
 					 bool little_endian,
 					 bool feature_hdrlen)
 {
 	u16 hdr_len;
 
 	if (feature_hdrlen) {
-		hdr_len = skb_transport_offset(skb);
-
-		if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
-			hdr_len += sizeof(struct udphdr);
-		else
-			hdr_len += tcp_hdrlen(skb);
+		if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
+				       SKB_GSO_UDP_TUNNEL_CSUM)) {
+			hdr_len = skb_inner_transport_offset(skb);
+
+			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
+				hdr_len += sizeof(struct udphdr);
+			else
+				hdr_len += inner_tcp_hdrlen(skb);
+		} else {
+			hdr_len = skb_transport_offset(skb);
+
+			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
+				hdr_len += sizeof(struct udphdr);
+			else
+				hdr_len += tcp_hdrlen(skb);
+		}
 	} else {
 		/* This is a hint as to how much should be linear. */
 		hdr_len = skb_headlen(skb);
@@ -252,7 +263,7 @@ static inline int __virtio_net_hdr_from_skb(const struct sk_buff *skb,
 		else
 			return -EINVAL;
 
-		virtio_net_set_hdrlen(skb, hdr, little_endian,
+		virtio_net_set_hdrlen(skb, hdr, sinfo, little_endian,
 				      feature_hdrlen);
 
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
@@ -443,10 +454,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
         vhdr->hash_hdr.hash_report = 0;
         vhdr->hash_hdr.padding = 0;
 
-	/* Let the basic parsing deal with plain GSO features. */
-	skb_shinfo(skb)->gso_type &= ~tnl_gso_type;
-	ret = virtio_net_hdr_from_skb(skb, hdr, true, false, vlan_hlen);
-	skb_shinfo(skb)->gso_type |= tnl_gso_type;
+	ret = __virtio_net_hdr_from_skb(skb, hdr, true, false,
+					feature_hdrlen, vlan_hlen);
 	if (ret)
 		return ret;
 
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net v8 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Jason Wang]

On Thu, Mar 5, 2026 at 11:19 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> 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>
> ---
>  drivers/net/tun_vnet.h     |  2 +-
>  drivers/net/virtio_net.c   |  6 +++-
>  include/linux/virtio_net.h | 58 ++++++++++++++++++++++++++++++--------
>  3 files changed, 53 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index a5f93b6c4482..fa5cab9d3e55 100644
> --- a/drivers/net/tun_vnet.h
> +++ b/drivers/net/tun_vnet.h
> @@ -244,7 +244,7 @@ tun_vnet_hdr_tnl_from_skb(unsigned int flags,
>
>         if (virtio_net_hdr_tnl_from_skb(skb, tnl_hdr, has_tnl_offload,
>                                         tun_vnet_is_little_endian(flags),
> -                                       vlan_hlen, true)) {
> +                                       vlan_hlen, true, false)) {
>                 struct virtio_net_hdr_v1 *hdr = &tnl_hdr->hash_hdr.hdr;
>                 struct skb_shared_info *sinfo = skb_shinfo(skb);
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 72d6a9c6a5a2..7106333ef904 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3267,8 +3267,12 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>         struct virtio_net_hdr_v1_hash_tunnel *hdr;
>         int num_sg;
>         unsigned hdr_len = vi->hdr_len;
> +       bool feature_hdrlen;
>         bool can_push;
>
> +       feature_hdrlen = virtio_has_feature(vi->vdev,
> +                                           VIRTIO_NET_F_GUEST_HDRLEN);
> +
>         pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
>
>         /* Make sure it's safe to cast between formats */
> @@ -3288,7 +3292,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>
>         if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
>                                         virtio_is_little_endian(vi->vdev), 0,
> -                                       false))
> +                                       false, feature_hdrlen))
>                 return -EPROTO;
>
>         if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 75dabb763c65..23f42bb8ece0 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -207,20 +207,40 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>         return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type);
>  }
>
> -static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> -                                         struct virtio_net_hdr *hdr,
> -                                         bool little_endian,
> -                                         bool has_data_valid,
> -                                         int vlan_hlen)
> +static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
> +                                        struct virtio_net_hdr *hdr,
> +                                        bool little_endian,
> +                                        bool feature_hdrlen)
> +{
> +       u16 hdr_len;
> +
> +       if (feature_hdrlen) {
> +               hdr_len = skb_transport_offset(skb);
> +
> +               if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> +                       hdr_len += sizeof(struct udphdr);
> +               else
> +                       hdr_len += tcp_hdrlen(skb);

I may miss soemthing but this still looks wrong as we could have e.g
USO and other UDP GSO packets.

> +       } else {
> +               /* This is a hint as to how much should be linear. */
> +               hdr_len = skb_headlen(skb);
> +       }
> +
> +       hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> +}
> +
> +static inline int __virtio_net_hdr_from_skb(const struct sk_buff *skb,
> +                                           struct virtio_net_hdr *hdr,
> +                                           bool little_endian,
> +                                           bool has_data_valid,
> +                                           bool feature_hdrlen,
> +                                           int vlan_hlen)
>  {
>         memset(hdr, 0, sizeof(*hdr));   /* no info leak */
>
>         if (skb_is_gso(skb)) {
>                 struct skb_shared_info *sinfo = skb_shinfo(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)
> @@ -231,6 +251,10 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
>                 else
>                         return -EINVAL;
> +
> +               virtio_net_set_hdrlen(skb, hdr, little_endian,
> +                                     feature_hdrlen);
> +
>                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
>                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>         } else
> @@ -250,6 +274,16 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>         return 0;
>  }
>
> +static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> +                                         struct virtio_net_hdr *hdr,
> +                                         bool little_endian,
> +                                         bool has_data_valid,
> +                                         int vlan_hlen)
> +{
> +       return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
> +                                        has_data_valid, false, vlan_hlen);
> +}
> +
>  static inline unsigned int virtio_l3min(bool is_ipv6)
>  {
>         return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
> @@ -385,7 +419,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>                             bool tnl_hdr_negotiated,
>                             bool little_endian,
>                             int vlan_hlen,
> -                           bool has_data_valid)
> +                           bool has_data_valid,
> +                           bool feature_hdrlen)
>  {
>         struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
>         unsigned int inner_nh, outer_th;
> @@ -395,8 +430,9 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>         tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
>                                                     SKB_GSO_UDP_TUNNEL_CSUM);
>         if (!tnl_gso_type)
> -               return virtio_net_hdr_from_skb(skb, hdr, little_endian,
> -                                              has_data_valid, vlan_hlen);
> +               return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
> +                                                has_data_valid,
> +                                                feature_hdrlen, vlan_hlen);
>
>         /* Tunnel support not negotiated but skb ask for it. */
>         if (!tnl_hdr_negotiated)
> --
> 2.32.0.3.g01195cf9f
>

Thanks

Re: [PATCH net v8 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

On Tue, 10 Mar 2026 11:05:46 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Thu, Mar 5, 2026 at 11:19 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > 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>
> > ---
> >  drivers/net/tun_vnet.h     |  2 +-
> >  drivers/net/virtio_net.c   |  6 +++-
> >  include/linux/virtio_net.h | 58 ++++++++++++++++++++++++++++++--------
> >  3 files changed, 53 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> > index a5f93b6c4482..fa5cab9d3e55 100644
> > --- a/drivers/net/tun_vnet.h
> > +++ b/drivers/net/tun_vnet.h
> > @@ -244,7 +244,7 @@ tun_vnet_hdr_tnl_from_skb(unsigned int flags,
> >
> >         if (virtio_net_hdr_tnl_from_skb(skb, tnl_hdr, has_tnl_offload,
> >                                         tun_vnet_is_little_endian(flags),
> > -                                       vlan_hlen, true)) {
> > +                                       vlan_hlen, true, false)) {
> >                 struct virtio_net_hdr_v1 *hdr = &tnl_hdr->hash_hdr.hdr;
> >                 struct skb_shared_info *sinfo = skb_shinfo(skb);
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 72d6a9c6a5a2..7106333ef904 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3267,8 +3267,12 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
> >         struct virtio_net_hdr_v1_hash_tunnel *hdr;
> >         int num_sg;
> >         unsigned hdr_len = vi->hdr_len;
> > +       bool feature_hdrlen;
> >         bool can_push;
> >
> > +       feature_hdrlen = virtio_has_feature(vi->vdev,
> > +                                           VIRTIO_NET_F_GUEST_HDRLEN);
> > +
> >         pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> >
> >         /* Make sure it's safe to cast between formats */
> > @@ -3288,7 +3292,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
> >
> >         if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
> >                                         virtio_is_little_endian(vi->vdev), 0,
> > -                                       false))
> > +                                       false, feature_hdrlen))
> >                 return -EPROTO;
> >
> >         if (vi->mergeable_rx_bufs)
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index 75dabb763c65..23f42bb8ece0 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -207,20 +207,40 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> >         return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type);
> >  }
> >
> > -static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > -                                         struct virtio_net_hdr *hdr,
> > -                                         bool little_endian,
> > -                                         bool has_data_valid,
> > -                                         int vlan_hlen)
> > +static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
> > +                                        struct virtio_net_hdr *hdr,
> > +                                        bool little_endian,
> > +                                        bool feature_hdrlen)
> > +{
> > +       u16 hdr_len;
> > +
> > +       if (feature_hdrlen) {
> > +               hdr_len = skb_transport_offset(skb);
> > +
> > +               if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> > +                       hdr_len += sizeof(struct udphdr);
> > +               else
> > +                       hdr_len += tcp_hdrlen(skb);
>
> I may miss soemthing but this still looks wrong as we could have e.g
> USO and other UDP GSO packets.

Could you say more?

Thanks.


>
> > +       } else {
> > +               /* This is a hint as to how much should be linear. */
> > +               hdr_len = skb_headlen(skb);
> > +       }
> > +
> > +       hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> > +}
> > +
> > +static inline int __virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > +                                           struct virtio_net_hdr *hdr,
> > +                                           bool little_endian,
> > +                                           bool has_data_valid,
> > +                                           bool feature_hdrlen,
> > +                                           int vlan_hlen)
> >  {
> >         memset(hdr, 0, sizeof(*hdr));   /* no info leak */
> >
> >         if (skb_is_gso(skb)) {
> >                 struct skb_shared_info *sinfo = skb_shinfo(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)
> > @@ -231,6 +251,10 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> >                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> >                 else
> >                         return -EINVAL;
> > +
> > +               virtio_net_set_hdrlen(skb, hdr, little_endian,
> > +                                     feature_hdrlen);
> > +
> >                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> >                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> >         } else
> > @@ -250,6 +274,16 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> >         return 0;
> >  }
> >
> > +static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > +                                         struct virtio_net_hdr *hdr,
> > +                                         bool little_endian,
> > +                                         bool has_data_valid,
> > +                                         int vlan_hlen)
> > +{
> > +       return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
> > +                                        has_data_valid, false, vlan_hlen);
> > +}
> > +
> >  static inline unsigned int virtio_l3min(bool is_ipv6)
> >  {
> >         return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
> > @@ -385,7 +419,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >                             bool tnl_hdr_negotiated,
> >                             bool little_endian,
> >                             int vlan_hlen,
> > -                           bool has_data_valid)
> > +                           bool has_data_valid,
> > +                           bool feature_hdrlen)
> >  {
> >         struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> >         unsigned int inner_nh, outer_th;
> > @@ -395,8 +430,9 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >         tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> >                                                     SKB_GSO_UDP_TUNNEL_CSUM);
> >         if (!tnl_gso_type)
> > -               return virtio_net_hdr_from_skb(skb, hdr, little_endian,
> > -                                              has_data_valid, vlan_hlen);
> > +               return __virtio_net_hdr_from_skb(skb, hdr, little_endian,
> > +                                                has_data_valid,
> > +                                                feature_hdrlen, vlan_hlen);
> >
> >         /* Tunnel support not negotiated but skb ask for it. */
> >         if (!tnl_hdr_negotiated)
> > --
> > 2.32.0.3.g01195cf9f
> >
>
> Thanks
>

Re: [PATCH net v8 2/2] virtio-net: correct hdr_len handling for tunnel gso

[Paolo Abeni]

On 3/5/26 4:19 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.
> 
> I observed that the "hdr_len" is 116 for this packet:
> 
>     17:36:18.241105 52:55:00:d1:27:0a > 2e:2c:df:46:a9:e1, ethertype IPv4 (0x0800), length 2912: (tos 0x0, ttl 64, id 45197, offset 0, flags [none], proto UDP (17), length 2898)
>         192.168.122.100.50613 > 192.168.122.1.4789: [bad udp cksum 0x8106 -> 0x26a0!] VXLAN, flags [I] (0x08), vni 1
>     fa:c3:ba:82:05:ee > ce:85:0c:31:77:e5, ethertype IPv4 (0x0800), length 2862: (tos 0x0, ttl 64, id 14678, offset 0, flags [DF], proto TCP (6), length 2848)
>         192.168.3.1.49880 > 192.168.3.2.9898: Flags [P.], cksum 0x9266 (incorrect -> 0xaa20), seq 515667:518463, ack 1, win 64, options [nop,nop,TS val 2990048824 ecr 2798801412], length 2796
> 
> 116 = 14(mac) + 20(ip) + 8(udp) + 8(vxlan) + 14(inner mac) + 20(inner ip) + 32(innner tcp)
> 
> 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 | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 23f42bb8ece0..906157779955 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -209,18 +209,29 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  
>  static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
>  					 struct virtio_net_hdr *hdr,
> +					 struct skb_shared_info *sinfo,
>  					 bool little_endian,
>  					 bool feature_hdrlen)
>  {
>  	u16 hdr_len;
>  
>  	if (feature_hdrlen) {
> -		hdr_len = skb_transport_offset(skb);
> -
> -		if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> -			hdr_len += sizeof(struct udphdr);
> -		else
> -			hdr_len += tcp_hdrlen(skb);
> +		if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> +				       SKB_GSO_UDP_TUNNEL_CSUM)) {
> +			hdr_len = skb_inner_transport_offset(skb);
> +
> +			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> +				hdr_len += sizeof(struct udphdr);
> +			else
> +				hdr_len += inner_tcp_hdrlen(skb);
> +		} else {
> +			hdr_len = skb_transport_offset(skb);
> +
> +			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> +				hdr_len += sizeof(struct udphdr);
> +			else
> +				hdr_len += tcp_hdrlen(skb);
> +		}

I already mentioned the following a couple of times, with no replies...
perhaps the 3rd attempt will be more successful.

I think it's a bad adding tunnel specific check here. It sounds like
layering violation. Instead you could have tunnel-specific variant, say
virtio_net_set_tnl_hdrlen(), and use it in virtio_net_hdr_tnl_from_skb().

Or you could try what i suggested in:

https://lore.kernel.org/netdev/CAF6piCLkv6kFqoq7OQfJ=Su9AVHSQ9J7DzaumOSf5xuf9w-kyA@mail.gmail.com/#t

Thanks,

Paolo

Re: [PATCH net v8 2/2] virtio-net: correct hdr_len handling for tunnel gso

[Xuan Zhuo]

On Tue, 10 Mar 2026 11:05:33 +0100, Paolo Abeni <pabeni@redhat.com> wrote:
> On 3/5/26 4:19 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.
> >
> > I observed that the "hdr_len" is 116 for this packet:
> >
> >     17:36:18.241105 52:55:00:d1:27:0a > 2e:2c:df:46:a9:e1, ethertype IPv4 (0x0800), length 2912: (tos 0x0, ttl 64, id 45197, offset 0, flags [none], proto UDP (17), length 2898)
> >         192.168.122.100.50613 > 192.168.122.1.4789: [bad udp cksum 0x8106 -> 0x26a0!] VXLAN, flags [I] (0x08), vni 1
> >     fa:c3:ba:82:05:ee > ce:85:0c:31:77:e5, ethertype IPv4 (0x0800), length 2862: (tos 0x0, ttl 64, id 14678, offset 0, flags [DF], proto TCP (6), length 2848)
> >         192.168.3.1.49880 > 192.168.3.2.9898: Flags [P.], cksum 0x9266 (incorrect -> 0xaa20), seq 515667:518463, ack 1, win 64, options [nop,nop,TS val 2990048824 ecr 2798801412], length 2796
> >
> > 116 = 14(mac) + 20(ip) + 8(udp) + 8(vxlan) + 14(inner mac) + 20(inner ip) + 32(innner tcp)
> >
> > 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 | 31 ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index 23f42bb8ece0..906157779955 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -209,18 +209,29 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> >
> >  static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
> >  					 struct virtio_net_hdr *hdr,
> > +					 struct skb_shared_info *sinfo,
> >  					 bool little_endian,
> >  					 bool feature_hdrlen)
> >  {
> >  	u16 hdr_len;
> >
> >  	if (feature_hdrlen) {
> > -		hdr_len = skb_transport_offset(skb);
> > -
> > -		if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> > -			hdr_len += sizeof(struct udphdr);
> > -		else
> > -			hdr_len += tcp_hdrlen(skb);
> > +		if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> > +				       SKB_GSO_UDP_TUNNEL_CSUM)) {
> > +			hdr_len = skb_inner_transport_offset(skb);
> > +
> > +			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> > +				hdr_len += sizeof(struct udphdr);
> > +			else
> > +				hdr_len += inner_tcp_hdrlen(skb);
> > +		} else {
> > +			hdr_len = skb_transport_offset(skb);
> > +
> > +			if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
> > +				hdr_len += sizeof(struct udphdr);
> > +			else
> > +				hdr_len += tcp_hdrlen(skb);
> > +		}
>
> I already mentioned the following a couple of times, with no replies...
> perhaps the 3rd attempt will be more successful.
>
> I think it's a bad adding tunnel specific check here. It sounds like
> layering violation. Instead you could have tunnel-specific variant, say
> virtio_net_set_tnl_hdrlen(), and use it in virtio_net_hdr_tnl_from_skb().
>
> Or you could try what i suggested in:
>
> https://lore.kernel.org/netdev/CAF6piCLkv6kFqoq7OQfJ=Su9AVHSQ9J7DzaumOSf5xuf9w-kyA@mail.gmail.com/#t

I saw your suggestions, though I didn't reply directly. I proceeded to adjust
the code based on my understanding of your feedback, but it appears my
understanding was incorrect. After re-reading your suggestions, I think
virtio_net_hdr_from_skb does not need to be changed. Instead, within
virtio_net_hdr_tnl_from_skb, we should directly call the device header length
function based on whether the feature is negotiated.

Thanks.

>
> Thanks,
>
> Paolo
>