linux-um.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v5 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN
@ 2025-11-11 11:12 Xuan Zhuo
  2025-11-11 11:12 ` [PATCH net v5 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
  2025-11-11 11:12 ` [PATCH net v5 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-11 11:12 UTC (permalink / raw)
  To: netdev
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Willem de Bruijn,
	Jason Wang, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, Simon Horman, Jiri Pirko, Alvaro Karsz,
	linux-um, 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 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

 arch/um/drivers/vector_transports.c |  1 +
 drivers/net/tun_vnet.h              |  4 +--
 drivers/net/virtio_net.c            |  9 +++++--
 include/linux/virtio_net.h          | 40 +++++++++++++++++++++++------
 net/packet/af_packet.c              |  5 ++--
 5 files changed, 45 insertions(+), 14 deletions(-)

--
2.32.0.3.g01195cf9f



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

* [PATCH net v5 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-11 11:12 [PATCH net v5 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
@ 2025-11-11 11:12 ` Xuan Zhuo
  2025-11-11 11:33   ` Michael S. Tsirkin
  2025-11-13 14:39   ` Paolo Abeni
  2025-11-11 11:12 ` [PATCH net v5 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
  1 sibling, 2 replies; 7+ messages in thread
From: Xuan Zhuo @ 2025-11-11 11:12 UTC (permalink / raw)
  To: netdev
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Willem de Bruijn,
	Jason Wang, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, Simon Horman, Jiri Pirko, Alvaro Karsz,
	linux-um, 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>
---
 arch/um/drivers/vector_transports.c |  1 +
 drivers/net/tun_vnet.h              |  4 ++--
 drivers/net/virtio_net.c            |  9 +++++++--
 include/linux/virtio_net.h          | 26 +++++++++++++++++++++-----
 net/packet/af_packet.c              |  5 +++--
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index 0794d23f07cb..03c5baa1d0c1 100644
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -121,6 +121,7 @@ static int raw_form_header(uint8_t *header,
 		vheader,
 		virtio_legacy_is_little_endian(),
 		false,
+		false,
 		0
 	);
 
diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index 81662328b2c7..0d376bc70dd7 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -214,7 +214,7 @@ static inline int tun_vnet_hdr_from_skb(unsigned int flags,
 
 	if (virtio_net_hdr_from_skb(skb, hdr,
 				    tun_vnet_is_little_endian(flags), true,
-				    vlan_hlen)) {
+				    false, vlan_hlen)) {
 		struct skb_shared_info *sinfo = skb_shinfo(skb);
 
 		if (net_ratelimit()) {
@@ -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..b335c88a8cd6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3317,9 +3317,13 @@ 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 hdrlen_negotiated;
 	bool can_push;
+	int num_sg;
+
+	hdrlen_negotiated = virtio_has_feature(vi->vdev,
+					       VIRTIO_NET_F_GUEST_HDRLEN);
 
 	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
 
@@ -3339,7 +3343,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),
+					hdrlen_negotiated, 0))
 		return -EPROTO;
 
 	if (vi->mergeable_rx_bufs)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index b673c31569f3..3cd8b2ebc197 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -211,16 +211,15 @@ 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 hdrlen_negotiated,
 					  int vlan_hlen)
 {
 	memset(hdr, 0, sizeof(*hdr));   /* no info leak */
 
 	if (skb_is_gso(skb)) {
 		struct skb_shared_info *sinfo = skb_shinfo(skb);
+		u16 hdr_len;
 
-		/* 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 +230,21 @@ 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;
+
+		if (hdrlen_negotiated) {
+			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);
+
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
 			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
 	} else
@@ -384,6 +398,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 hdrlen_negotiated,
 			    int vlan_hlen)
 {
 	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
@@ -395,7 +410,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 						    SKB_GSO_UDP_TUNNEL_CSUM);
 	if (!tnl_gso_type)
 		return virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
-					       vlan_hlen);
+					       hdrlen_negotiated, vlan_hlen);
 
 	/* Tunnel support not negotiated but skb ask for it. */
 	if (!tnl_hdr_negotiated)
@@ -408,7 +423,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, hdrlen_negotiated,
+				      vlan_hlen);
 	skb_shinfo(skb)->gso_type |= tnl_gso_type;
 	if (ret)
 		return ret;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 173e6edda08f..6982f4ab1c73 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2093,7 +2093,8 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb,
 		return -EINVAL;
 	*len -= vnet_hdr_sz;
 
-	if (virtio_net_hdr_from_skb(skb, (struct virtio_net_hdr *)&vnet_hdr, vio_le(), true, 0))
+	if (virtio_net_hdr_from_skb(skb, (struct virtio_net_hdr *)&vnet_hdr,
+				    vio_le(), true, false, 0))
 		return -EINVAL;
 
 	return memcpy_to_msg(msg, (void *)&vnet_hdr, vnet_hdr_sz);
@@ -2361,7 +2362,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (vnet_hdr_sz &&
 	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
 				    sizeof(struct virtio_net_hdr),
-				    vio_le(), true, 0)) {
+				    vio_le(), true, false, 0)) {
 		if (po->tp_version == TPACKET_V3)
 			prb_clear_blk_fill_status(&po->rx_ring);
 		goto drop_n_account;
-- 
2.32.0.3.g01195cf9f



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

* [PATCH net v5 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-11 11:12 [PATCH net v5 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
  2025-11-11 11:12 ` [PATCH net v5 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
@ 2025-11-11 11:12 ` Xuan Zhuo
  2025-11-13 14:50   ` Paolo Abeni
  1 sibling, 1 reply; 7+ messages in thread
From: Xuan Zhuo @ 2025-11-11 11:12 UTC (permalink / raw)
  To: netdev
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Willem de Bruijn,
	Jason Wang, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S. Tsirkin, Xuan Zhuo,
	Eugenio Pérez, Simon Horman, Jiri Pirko, Alvaro Karsz,
	linux-um, 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 | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 3cd8b2ebc197..432b17979d17 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -232,12 +232,23 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
 			return -EINVAL;
 
 		if (hdrlen_negotiated) {
-			hdr_len = skb_transport_offset(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);
+			}
 
-			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);
@@ -421,11 +432,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, hdrlen_negotiated,
 				      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 v5 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-11 11:12 ` [PATCH net v5 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
@ 2025-11-11 11:33   ` Michael S. Tsirkin
  2025-11-13 14:39   ` Paolo Abeni
  1 sibling, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2025-11-11 11:33 UTC (permalink / raw)
  To: Xuan Zhuo
  Cc: netdev, Richard Weinberger, Anton Ivanov, Johannes Berg,
	Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Eugenio Pérez,
	Simon Horman, Jiri Pirko, Alvaro Karsz, linux-um, virtualization

On Tue, Nov 11, 2025 at 07:12:11PM +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>
> ---
>  arch/um/drivers/vector_transports.c |  1 +
>  drivers/net/tun_vnet.h              |  4 ++--
>  drivers/net/virtio_net.c            |  9 +++++++--
>  include/linux/virtio_net.h          | 26 +++++++++++++++++++++-----
>  net/packet/af_packet.c              |  5 +++--
>  5 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index 0794d23f07cb..03c5baa1d0c1 100644
> --- a/arch/um/drivers/vector_transports.c
> +++ b/arch/um/drivers/vector_transports.c
> @@ -121,6 +121,7 @@ static int raw_form_header(uint8_t *header,
>  		vheader,
>  		virtio_legacy_is_little_endian(),
>  		false,
> +		false,
>  		0
>  	);
>  
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index 81662328b2c7..0d376bc70dd7 100644
> --- a/drivers/net/tun_vnet.h
> +++ b/drivers/net/tun_vnet.h
> @@ -214,7 +214,7 @@ static inline int tun_vnet_hdr_from_skb(unsigned int flags,
>  
>  	if (virtio_net_hdr_from_skb(skb, hdr,
>  				    tun_vnet_is_little_endian(flags), true,
> -				    vlan_hlen)) {
> +				    false, vlan_hlen)) {
>  		struct skb_shared_info *sinfo = skb_shinfo(skb);
>  
>  		if (net_ratelimit()) {
> @@ -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..b335c88a8cd6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3317,9 +3317,13 @@ 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 hdrlen_negotiated;
>  	bool can_push;
> +	int num_sg;
> +
> +	hdrlen_negotiated = virtio_has_feature(vi->vdev,
> +					       VIRTIO_NET_F_GUEST_HDRLEN);
>  
>  	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
>  
> @@ -3339,7 +3343,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),
> +					hdrlen_negotiated, 0))
>  		return -EPROTO;
>  
>  	if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index b673c31569f3..3cd8b2ebc197 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -211,16 +211,15 @@ 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 hdrlen_negotiated,
>  					  int vlan_hlen)

Took me a while to figure out why does tun pass false here.

The reason is that this flag is really only dealing with guest
hdrlen.  so how about guest_hdrlen to mirror spec
or if you like xmit_hdrlen?





>  {
>  	memset(hdr, 0, sizeof(*hdr));   /* no info leak */
>  
>  	if (skb_is_gso(skb)) {
>  		struct skb_shared_info *sinfo = skb_shinfo(skb);
> +		u16 hdr_len;
>  
> -		/* 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 +230,21 @@ 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;
> +
> +		if (hdrlen_negotiated) {
> +			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);
> +
>  		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
>  			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>  	} else
> @@ -384,6 +398,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 hdrlen_negotiated,
>  			    int vlan_hlen)
>  {
>  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> @@ -395,7 +410,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  						    SKB_GSO_UDP_TUNNEL_CSUM);
>  	if (!tnl_gso_type)
>  		return virtio_net_hdr_from_skb(skb, hdr, little_endian, false,
> -					       vlan_hlen);
> +					       hdrlen_negotiated, vlan_hlen);
>  
>  	/* Tunnel support not negotiated but skb ask for it. */
>  	if (!tnl_hdr_negotiated)
> @@ -408,7 +423,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, hdrlen_negotiated,
> +				      vlan_hlen);
>  	skb_shinfo(skb)->gso_type |= tnl_gso_type;
>  	if (ret)
>  		return ret;
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 173e6edda08f..6982f4ab1c73 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2093,7 +2093,8 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb,
>  		return -EINVAL;
>  	*len -= vnet_hdr_sz;
>  
> -	if (virtio_net_hdr_from_skb(skb, (struct virtio_net_hdr *)&vnet_hdr, vio_le(), true, 0))
> +	if (virtio_net_hdr_from_skb(skb, (struct virtio_net_hdr *)&vnet_hdr,
> +				    vio_le(), true, false, 0))
>  		return -EINVAL;
>  
>  	return memcpy_to_msg(msg, (void *)&vnet_hdr, vnet_hdr_sz);
> @@ -2361,7 +2362,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if (vnet_hdr_sz &&
>  	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
>  				    sizeof(struct virtio_net_hdr),
> -				    vio_le(), true, 0)) {
> +				    vio_le(), true, false, 0)) {
>  		if (po->tp_version == TPACKET_V3)
>  			prb_clear_blk_fill_status(&po->rx_ring);
>  		goto drop_n_account;
> -- 
> 2.32.0.3.g01195cf9f



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

* Re: [PATCH net v5 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-11 11:12 ` [PATCH net v5 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
  2025-11-11 11:33   ` Michael S. Tsirkin
@ 2025-11-13 14:39   ` Paolo Abeni
  2025-11-13 15:59     ` Michael S. Tsirkin
  1 sibling, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2025-11-13 14:39 UTC (permalink / raw)
  To: Xuan Zhuo, netdev
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Willem de Bruijn,
	Jason Wang, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Michael S. Tsirkin, Eugenio Pérez,
	Simon Horman, Jiri Pirko, Alvaro Karsz, linux-um, virtualization

On 11/11/25 12:12 PM, 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>

IMHO this looks like more a new feature - namely,
VIRTIO_NET_F_GUEST_HDRLEN support - than a fix.

[...]
> @@ -2361,7 +2362,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if (vnet_hdr_sz &&
>  	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
>  				    sizeof(struct virtio_net_hdr),
> -				    vio_le(), true, 0)) {
> +				    vio_le(), true, false, 0)) {
>  		if (po->tp_version == TPACKET_V3)
>  			prb_clear_blk_fill_status(&po->rx_ring);
>  		goto drop_n_account;
To reduce the diffstat, what about creating a __virtio_net_hdr_from_skb()
variant (please find a better name) allowing the extra `hdrlen_negotiated`
argument, define virtio_net_hdr_from_skb() as a wrapper of such helper
withthe extra arg == false, and use the helper in the few places that
really could use hdrlen?



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

* Re: [PATCH net v5 2/2] virtio-net: correct hdr_len handling for tunnel gso
  2025-11-11 11:12 ` [PATCH net v5 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
@ 2025-11-13 14:50   ` Paolo Abeni
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2025-11-13 14:50 UTC (permalink / raw)
  To: Xuan Zhuo, netdev
  Cc: Richard Weinberger, Anton Ivanov, Johannes Berg, Willem de Bruijn,
	Jason Wang, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Michael S. Tsirkin, Eugenio Pérez,
	Simon Horman, Jiri Pirko, Alvaro Karsz, linux-um, virtualization

On 11/11/25 12:12 PM, 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 | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 3cd8b2ebc197..432b17979d17 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -232,12 +232,23 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>  			return -EINVAL;
>  
>  		if (hdrlen_negotiated) {
> -			hdr_len = skb_transport_offset(skb);
> +			if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> +					       SKB_GSO_UDP_TUNNEL_CSUM)) {

I'm personally not a huge fan of adding UDP tunnel specific check to the
generic code, did you tried something along the lines suggested here:

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

?

Thanks,

Paolo



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

* Re: [PATCH net v5 1/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
  2025-11-13 14:39   ` Paolo Abeni
@ 2025-11-13 15:59     ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2025-11-13 15:59 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Xuan Zhuo, netdev, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Willem de Bruijn, Jason Wang, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Eugenio Pérez,
	Simon Horman, Jiri Pirko, Alvaro Karsz, linux-um, virtualization

On Thu, Nov 13, 2025 at 03:39:35PM +0100, Paolo Abeni wrote:
> On 11/11/25 12:12 PM, 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>
> 
> IMHO this looks like more a new feature - namely,
> VIRTIO_NET_F_GUEST_HDRLEN support - than a fix.


I mean if guest negotiates VIRTIO_NET_F_GUEST_HDRLEN but the header
length is wrong then yes it is broken and this is a fix.


> [...]
> > @@ -2361,7 +2362,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> >  	if (vnet_hdr_sz &&
> >  	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
> >  				    sizeof(struct virtio_net_hdr),
> > -				    vio_le(), true, 0)) {
> > +				    vio_le(), true, false, 0)) {
> >  		if (po->tp_version == TPACKET_V3)
> >  			prb_clear_blk_fill_status(&po->rx_ring);
> >  		goto drop_n_account;
> To reduce the diffstat, what about creating a __virtio_net_hdr_from_skb()
> variant (please find a better name) allowing the extra `hdrlen_negotiated`
> argument, define virtio_net_hdr_from_skb() as a wrapper of such helper
> withthe extra arg == false, and use the helper in the few places that
> really could use hdrlen?



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

end of thread, other threads:[~2025-11-13 15:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 11:12 [PATCH net v5 0/2] virtio-net: fix for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
2025-11-11 11:12 ` [PATCH net v5 1/2] virtio-net: correct hdr_len handling " Xuan Zhuo
2025-11-11 11:33   ` Michael S. Tsirkin
2025-11-13 14:39   ` Paolo Abeni
2025-11-13 15:59     ` Michael S. Tsirkin
2025-11-11 11:12 ` [PATCH net v5 2/2] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
2025-11-13 14:50   ` Paolo Abeni

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).