virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v6 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN
@ 2025-11-19  5:55 Xuan Zhuo
  2025-11-19  5:55 ` [PATCH net v6 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
  2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
  0 siblings, 2 replies; 7+ messages in thread
From: Xuan Zhuo @ 2025-11-19  5:55 UTC (permalink / raw)
  To: netdev
  Cc: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, Jiri Pirko, Alvaro Karsz,
	virtualization


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   |  8 +++--
 include/linux/virtio_net.h | 71 ++++++++++++++++++++++++++++++--------
 3 files changed, 64 insertions(+), 17 deletions(-)

--
2.32.0.3.g01195cf9f


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net v6 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-19  5:55 [PATCH net v6 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
@ 2025-11-19  5:55 ` Xuan Zhuo
  2025-11-19 15:27   ` Michael S. Tsirkin
  2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
  1 sibling, 1 reply; 7+ messages in thread
From: Xuan Zhuo @ 2025-11-19  5:55 UTC (permalink / raw)
  To: netdev
  Cc: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, Jiri Pirko, Alvaro Karsz,
	virtualization

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   |  8 ++++--
 include/linux/virtio_net.h | 58 ++++++++++++++++++++++++++++++--------
 3 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index 81662328b2c7..b06aa6f2aade 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)) {
+					false, vlan_hlen)) {
 		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 0369dda5ed60..a62acfaf631b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3317,9 +3317,12 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
 	const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
 	struct virtnet_info *vi = sq->vq->vdev->priv;
 	struct virtio_net_hdr_v1_hash_tunnel *hdr;
-	int num_sg;
 	unsigned hdr_len = vi->hdr_len;
+	bool guest_hdrlen;
 	bool can_push;
+	int num_sg;
+
+	guest_hdrlen = virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_HDRLEN);
 
 	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
 
@@ -3339,7 +3342,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
 		hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
 
 	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
-					virtio_is_little_endian(vi->vdev), 0))
+					virtio_is_little_endian(vi->vdev),
+					guest_hdrlen, 0))
 		return -EPROTO;
 
 	if (vi->mergeable_rx_bufs)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b673c31569f3..ee960ec9a35e 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 guest_hdrlen)
+{
+	u16 hdr_len;
+
+	if (guest_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 guest_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,
+				      guest_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);
@@ -384,6 +418,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 			    struct virtio_net_hdr_v1_hash_tunnel *vhdr,
 			    bool tnl_hdr_negotiated,
 			    bool little_endian,
+			    bool guest_hdrlen,
 			    int vlan_hlen)
 {
 	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
@@ -394,8 +429,8 @@ 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, false,
-					       vlan_hlen);
+		return __virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
+						 guest_hdrlen, vlan_hlen);
 
 	/* Tunnel support not negotiated but skb ask for it. */
 	if (!tnl_hdr_negotiated)
@@ -408,7 +443,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 
 	/* 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);
+	ret = __virtio_net_hdr_from_skb(skb, hdr, true, false,
+					guest_hdrlen, vlan_hlen);
 	skb_shinfo(skb)->gso_type |= tnl_gso_type;
 	if (ret)
 		return ret;
-- 
2.32.0.3.g01195cf9f


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-19  5:55 [PATCH net v6 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
  2025-11-19  5:55 ` [PATCH net v6 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
@ 2025-11-19  5:55 ` Xuan Zhuo
  2025-11-19 15:35   ` Michael S. Tsirkin
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Xuan Zhuo @ 2025-11-19  5:55 UTC (permalink / raw)
  To: netdev
  Cc: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, Jiri Pirko, Alvaro Karsz,
	virtualization

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 | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index ee960ec9a35e..ee8231eb759b 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -215,12 +215,22 @@ static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
 	u16 hdr_len;
 
 	if (guest_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);
@@ -441,11 +451,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,
 					guest_hdrlen, vlan_hlen);
-	skb_shinfo(skb)->gso_type |= tnl_gso_type;
 	if (ret)
 		return ret;
 
-- 
2.32.0.3.g01195cf9f


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net v6 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-19  5:55 ` [PATCH net v6 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
@ 2025-11-19 15:27   ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 15:27 UTC (permalink / raw)
  To: Xuan Zhuo
  Cc: netdev, Willem de Bruijn, Jason Wang, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Eugenio Pérez, Jiri Pirko, Alvaro Karsz, virtualization

On Wed, Nov 19, 2025 at 01:55:21PM +0800, Xuan Zhuo 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   |  8 ++++--
>  include/linux/virtio_net.h | 58 ++++++++++++++++++++++++++++++--------
>  3 files changed, 54 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index 81662328b2c7..b06aa6f2aade 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)) {
> +					false, vlan_hlen)) {
>  		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 0369dda5ed60..a62acfaf631b 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3317,9 +3317,12 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>  	const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
>  	struct virtnet_info *vi = sq->vq->vdev->priv;
>  	struct virtio_net_hdr_v1_hash_tunnel *hdr;
> -	int num_sg;
>  	unsigned hdr_len = vi->hdr_len;
> +	bool guest_hdrlen;
>  	bool can_push;
> +	int num_sg;
> +
> +	guest_hdrlen = virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_HDRLEN);
>  
>  	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
>  
> @@ -3339,7 +3342,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>  		hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
>  
>  	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
> -					virtio_is_little_endian(vi->vdev), 0))
> +					virtio_is_little_endian(vi->vdev),
> +					guest_hdrlen, 0))
>  		return -EPROTO;
>  
>  	if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index b673c31569f3..ee960ec9a35e 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 guest_hdrlen)
> +{
> +	u16 hdr_len;
> +
> +	if (guest_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);
> +}


So this adds code that is broken for tunnels, then the follow up patch
fixes it up. I don't feel it's worth it, just squash the two patches
together please.


> +
> +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 guest_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,
> +				      guest_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);
> @@ -384,6 +418,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  			    struct virtio_net_hdr_v1_hash_tunnel *vhdr,
>  			    bool tnl_hdr_negotiated,
>  			    bool little_endian,
> +			    bool guest_hdrlen,
>  			    int vlan_hlen)
>  {
>  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> @@ -394,8 +429,8 @@ 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, false,
> -					       vlan_hlen);
> +		return __virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
> +						 guest_hdrlen, vlan_hlen);
>  
>  	/* Tunnel support not negotiated but skb ask for it. */
>  	if (!tnl_hdr_negotiated)
> @@ -408,7 +443,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  
>  	/* 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);
> +	ret = __virtio_net_hdr_from_skb(skb, hdr, true, false,
> +					guest_hdrlen, vlan_hlen);
>  	skb_shinfo(skb)->gso_type |= tnl_gso_type;
>  	if (ret)
>  		return ret;
> -- 
> 2.32.0.3.g01195cf9f


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
@ 2025-11-19 15:35   ` Michael S. Tsirkin
  2025-11-20  0:43   ` kernel test robot
  2025-11-20  0:54   ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 15:35 UTC (permalink / raw)
  To: Xuan Zhuo
  Cc: netdev, Willem de Bruijn, Jason Wang, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Eugenio Pérez, Jiri Pirko, Alvaro Karsz, virtualization

On Wed, Nov 19, 2025 at 01:55:22PM +0800, 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 | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index ee960ec9a35e..ee8231eb759b 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -215,12 +215,22 @@ static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
>  	u16 hdr_len;
>  
>  	if (guest_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);

BTW  I noticed that include/linux/virtio_net.h really should include
linux/tcp.h for tcp_hdrlen and uapi/linux/udp.h for struct udphdr


Not a new issue so you do not have to resolve it in this patchset,
though.



> @@ -441,11 +451,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,
>  					guest_hdrlen, vlan_hlen);
> -	skb_shinfo(skb)->gso_type |= tnl_gso_type;
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.32.0.3.g01195cf9f


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
  2025-11-19 15:35   ` Michael S. Tsirkin
@ 2025-11-20  0:43   ` kernel test robot
  2025-11-20  0:54   ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-11-20  0:43 UTC (permalink / raw)
  To: Xuan Zhuo, netdev
  Cc: llvm, oe-kbuild-all, Willem de Bruijn, Jason Wang, Andrew Lunn,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, Jiri Pirko, Alvaro Karsz,
	virtualization

Hi Xuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio-net-correct-hdr_len-handling-for-VIRTIO_NET_F_GUEST_HDRLEN/20251119-135650
base:   net/main
patch link:    https://lore.kernel.org/r/20251119055522.617-3-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20251120/202511200846.lqEgKwk3-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511200846.lqEgKwk3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511200846.lqEgKwk3-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/tun.c:59:
>> include/linux/virtio_net.h:218:7: error: use of undeclared identifier 'sinfo'
     218 |                 if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
         |                     ^
   1 error generated.


vim +/sinfo +218 include/linux/virtio_net.h

   209	
   210	static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
   211						 struct virtio_net_hdr *hdr,
   212						 bool little_endian,
   213						 bool guest_hdrlen)
   214	{
   215		u16 hdr_len;
   216	
   217		if (guest_hdrlen) {
 > 218			if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
   219					       SKB_GSO_UDP_TUNNEL_CSUM)) {
   220				hdr_len = skb_inner_transport_offset(skb);
   221	
   222				if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
   223					hdr_len += sizeof(struct udphdr);
   224				else
   225					hdr_len += inner_tcp_hdrlen(skb);
   226			} else {
   227				hdr_len = skb_transport_offset(skb);
   228	
   229				if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
   230					hdr_len += sizeof(struct udphdr);
   231				else
   232					hdr_len += tcp_hdrlen(skb);
   233			}
   234		} else {
   235			/* This is a hint as to how much should be linear. */
   236			hdr_len = skb_headlen(skb);
   237		}
   238	
   239		hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
   240	}
   241	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
  2025-11-19 15:35   ` Michael S. Tsirkin
  2025-11-20  0:43   ` kernel test robot
@ 2025-11-20  0:54   ` kernel test robot
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-11-20  0:54 UTC (permalink / raw)
  To: Xuan Zhuo, netdev
  Cc: llvm, oe-kbuild-all, Willem de Bruijn, Jason Wang, Andrew Lunn,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin,
	Xuan Zhuo, Eugenio Pérez, Jiri Pirko, Alvaro Karsz,
	virtualization

Hi Xuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Xuan-Zhuo/virtio-net-correct-hdr_len-handling-for-VIRTIO_NET_F_GUEST_HDRLEN/20251119-135650
base:   net/main
patch link:    https://lore.kernel.org/r/20251119055522.617-3-xuanzhuo%40linux.alibaba.com
patch subject: [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso
config: arm-jornada720_defconfig (https://download.01.org/0day-ci/archive/20251120/202511200817.29vdvG8S-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 0bba1e76581bad04e7d7f09f5115ae5e2989e0d9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511200817.29vdvG8S-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511200817.29vdvG8S-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/packet/af_packet.c:86:
>> include/linux/virtio_net.h:218:7: error: use of undeclared identifier 'sinfo'
     218 |                 if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
         |                     ^~~~~
   1 error generated.


vim +/sinfo +218 include/linux/virtio_net.h

   209	
   210	static inline void virtio_net_set_hdrlen(const struct sk_buff *skb,
   211						 struct virtio_net_hdr *hdr,
   212						 bool little_endian,
   213						 bool guest_hdrlen)
   214	{
   215		u16 hdr_len;
   216	
   217		if (guest_hdrlen) {
 > 218			if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
   219					       SKB_GSO_UDP_TUNNEL_CSUM)) {
   220				hdr_len = skb_inner_transport_offset(skb);
   221	
   222				if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
   223					hdr_len += sizeof(struct udphdr);
   224				else
   225					hdr_len += inner_tcp_hdrlen(skb);
   226			} else {
   227				hdr_len = skb_transport_offset(skb);
   228	
   229				if (hdr->gso_type == VIRTIO_NET_HDR_GSO_UDP_L4)
   230					hdr_len += sizeof(struct udphdr);
   231				else
   232					hdr_len += tcp_hdrlen(skb);
   233			}
   234		} else {
   235			/* This is a hint as to how much should be linear. */
   236			hdr_len = skb_headlen(skb);
   237		}
   238	
   239		hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
   240	}
   241	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-20  0:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-19  5:55 [PATCH net v6 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
2025-11-19  5:55 ` [PATCH net v6 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
2025-11-19 15:27   ` Michael S. Tsirkin
2025-11-19  5:55 ` [PATCH net v6 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
2025-11-19 15:35   ` Michael S. Tsirkin
2025-11-20  0:43   ` kernel test robot
2025-11-20  0:54   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).