* [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode
2025-10-29 3:09 [PATCH net v4 0/4] fixes two virtio-net related bugs Xuan Zhuo
@ 2025-10-29 3:09 ` Xuan Zhuo
2025-11-09 17:04 ` Alyssa Ross
2025-11-09 21:34 ` Michael S. Tsirkin
2025-10-29 3:09 ` [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device Xuan Zhuo
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Xuan Zhuo @ 2025-10-29 3:09 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
checksummed packets handling") is to record the flags in advance, as
their value may be overwritten in the XDP case. However, the flags
recorded under big mode are incorrect, because in big mode, the passed
buf does not point to the rx buffer, but rather to the page of the
submitted buffer. This commit fixes this issue.
For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
buf address when unmapping for small packets") fixed it.
Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e8a179aaa49..59f116b609f6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2625,22 +2625,28 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
return;
}
- /* 1. Save the flags early, as the XDP program might overwrite them.
+ /* About the flags below:
+ * 1. Save the flags early, as the XDP program might overwrite them.
* These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
* stay valid after XDP processing.
* 2. XDP doesn't work with partially checksummed packets (refer to
* virtnet_xdp_set()), so packets marked as
* VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
*/
- flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
- if (vi->mergeable_rx_bufs)
+ if (vi->mergeable_rx_bufs) {
+ flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
stats);
- else if (vi->big_packets)
+ } else if (vi->big_packets) {
+ void *p = page_address((struct page *)buf);
+
+ flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
skb = receive_big(dev, vi, rq, buf, len, stats);
- else
+ } else {
+ flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
+ }
if (unlikely(!skb))
return;
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode
2025-10-29 3:09 ` [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode Xuan Zhuo
@ 2025-11-09 17:04 ` Alyssa Ross
2025-11-09 21:34 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Alyssa Ross @ 2025-11-09 17:04 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
On Wed, Oct 29, 2025 at 11:09:10AM +0800, Xuan Zhuo wrote:
> The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
> checksummed packets handling") is to record the flags in advance, as
> their value may be overwritten in the XDP case. However, the flags
> recorded under big mode are incorrect, because in big mode, the passed
> buf does not point to the rx buffer, but rather to the page of the
> submitted buffer. This commit fixes this issue.
>
> For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
> buf address when unmapping for small packets") fixed it.
>
> Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/virtio_net.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
Fixes my networking with CSUM offload enabled in Cloud Hypervisor, thanks!
Tested-by: Alyssa Ross <hi@alyssa.is>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode
2025-10-29 3:09 ` [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode Xuan Zhuo
2025-11-09 17:04 ` Alyssa Ross
@ 2025-11-09 21:34 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-09 21:34 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jason Wang, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09:10AM +0800, Xuan Zhuo wrote:
> The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
> checksummed packets handling") is to record the flags in advance, as
> their value may be overwritten in the XDP case. However, the flags
> recorded under big mode are incorrect, because in big mode, the passed
> buf does not point to the rx buffer, but rather to the page of the
> submitted buffer. This commit fixes this issue.
>
> For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
> buf address when unmapping for small packets") fixed it.
>
> Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8e8a179aaa49..59f116b609f6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2625,22 +2625,28 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
> }
>
> - /* 1. Save the flags early, as the XDP program might overwrite them.
> + /* About the flags below:
> + * 1. Save the flags early, as the XDP program might overwrite them.
> * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
> * stay valid after XDP processing.
> * 2. XDP doesn't work with partially checksummed packets (refer to
> * virtnet_xdp_set()), so packets marked as
> * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
> */
> - flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
>
> - if (vi->mergeable_rx_bufs)
> + if (vi->mergeable_rx_bufs) {
> + flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
> stats);
> - else if (vi->big_packets)
> + } else if (vi->big_packets) {
> + void *p = page_address((struct page *)buf);
> +
> + flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
> skb = receive_big(dev, vi, rq, buf, len, stats);
> - else
> + } else {
> + flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
> skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
> + }
>
> if (unlikely(!skb))
> return;
> --
> 2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device.
2025-10-29 3:09 [PATCH net v4 0/4] fixes two virtio-net related bugs Xuan Zhuo
2025-10-29 3:09 ` [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode Xuan Zhuo
@ 2025-10-29 3:09 ` Xuan Zhuo
2025-10-30 2:42 ` Jason Wang
2025-11-09 21:37 ` Michael S. Tsirkin
2025-10-29 3:09 ` [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Xuan Zhuo @ 2025-10-29 3:09 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
Although `virtio_net_hdr_from_skb` is used in several places outside of
`virtio-net.c`, the `hdr_len` field is only utilized by the device
according to the specification. Therefore, we do not need to set
`hdr_len` unless the header is actually passed to the device.
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
include/linux/virtio_net.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 4d1780848d0e..710ae0d2d336 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -218,9 +218,14 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
if (skb_is_gso(skb)) {
struct skb_shared_info *sinfo = skb_shinfo(skb);
- /* This is a hint as to how much should be linear. */
- hdr->hdr_len = __cpu_to_virtio16(little_endian,
- skb_headlen(skb));
+ /* In certain code paths (such as the af_packet.c receive path),
+ * this function may be called without a transport header.
+ * In this case, we do not need to set the hdr_len.
+ */
+ if (skb_transport_header_was_set(skb))
+ 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)
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device.
2025-10-29 3:09 ` [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device Xuan Zhuo
@ 2025-10-30 2:42 ` Jason Wang
2025-11-09 21:37 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Jason Wang @ 2025-10-30 2:42 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Michael S. Tsirkin, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> Although `virtio_net_hdr_from_skb` is used in several places outside of
> `virtio-net.c`, the `hdr_len` field is only utilized by the device
> according to the specification. Therefore, we do not need to set
> `hdr_len` unless the header is actually passed to the device.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
I wonder if this will cause any issue consider hdr_len is just a hint.
E.g device needs to survive from hdr_len = 0.
> ---
> include/linux/virtio_net.h | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 4d1780848d0e..710ae0d2d336 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -218,9 +218,14 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> if (skb_is_gso(skb)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
>
> - /* This is a hint as to how much should be linear. */
> - hdr->hdr_len = __cpu_to_virtio16(little_endian,
> - skb_headlen(skb));
> + /* In certain code paths (such as the af_packet.c receive path),
> + * this function may be called without a transport header.
> + * In this case, we do not need to set the hdr_len.
> + */
> + if (skb_transport_header_was_set(skb))
> + 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)
> --
> 2.32.0.3.g01195cf9f
>
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device.
2025-10-29 3:09 ` [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device Xuan Zhuo
2025-10-30 2:42 ` Jason Wang
@ 2025-11-09 21:37 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-09 21:37 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jason Wang, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09:11AM +0800, Xuan Zhuo wrote:
> Although `virtio_net_hdr_from_skb` is used in several places outside of
> `virtio-net.c`, the `hdr_len` field is only utilized by the device
> according to the specification. Therefore, we do not need to set
> `hdr_len` unless the header is actually passed to the device.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
maybe ... but what is this patch trying to achieve?
it does not seem harmful to set it ...
> ---
> include/linux/virtio_net.h | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 4d1780848d0e..710ae0d2d336 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -218,9 +218,14 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> if (skb_is_gso(skb)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
>
> - /* This is a hint as to how much should be linear. */
> - hdr->hdr_len = __cpu_to_virtio16(little_endian,
> - skb_headlen(skb));
> + /* In certain code paths (such as the af_packet.c receive path),
> + * this function may be called without a transport header.
> + * In this case, we do not need to set the hdr_len.
> + */
> + if (skb_transport_header_was_set(skb))
> + 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)
> --
> 2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-10-29 3:09 [PATCH net v4 0/4] fixes two virtio-net related bugs Xuan Zhuo
2025-10-29 3:09 ` [PATCH net v4 1/4] virtio-net: fix incorrect flags recording in big mode Xuan Zhuo
2025-10-29 3:09 ` [PATCH net v4 2/4] virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device Xuan Zhuo
@ 2025-10-29 3:09 ` Xuan Zhuo
2025-10-30 2:53 ` Jason Wang
2025-11-09 21:42 ` Michael S. Tsirkin
2025-10-29 3:09 ` [PATCH net v4 4/4] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
2025-10-30 8:02 ` [PATCH net v4 0/4] fixes two virtio-net related bugs Michael S. Tsirkin
4 siblings, 2 replies; 19+ messages in thread
From: Xuan Zhuo @ 2025-10-29 3:09 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heng Qi, Willem de Bruijn, 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>
---
include/linux/virtio_net.h | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 710ae0d2d336..6ef0b737d548 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -217,25 +217,35 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
if (skb_is_gso(skb)) {
struct skb_shared_info *sinfo = skb_shinfo(skb);
+ u16 hdr_len = 0;
/* In certain code paths (such as the af_packet.c receive path),
* this function may be called without a transport header.
* In this case, we do not need to set the hdr_len.
*/
if (skb_transport_header_was_set(skb))
- hdr->hdr_len = __cpu_to_virtio16(little_endian,
- skb_headlen(skb));
+ hdr_len = skb_transport_offset(skb);
hdr->gso_size = __cpu_to_virtio16(little_endian,
sinfo->gso_size);
- if (sinfo->gso_type & SKB_GSO_TCPV4)
+ if (sinfo->gso_type & SKB_GSO_TCPV4) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
- else if (sinfo->gso_type & SKB_GSO_TCPV6)
+ if (hdr_len)
+ hdr_len += tcp_hdrlen(skb);
+ } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else if (sinfo->gso_type & SKB_GSO_UDP_L4)
+ if (hdr_len)
+ hdr_len += tcp_hdrlen(skb);
+ } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
- else
+ if (hdr_len)
+ hdr_len += sizeof(struct udphdr);
+ } else {
return -EINVAL;
+ }
+
+ hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
+
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-10-29 3:09 ` [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
@ 2025-10-30 2:53 ` Jason Wang
2025-11-09 21:41 ` Michael S. Tsirkin
2025-11-09 21:42 ` Michael S. Tsirkin
1 sibling, 1 reply; 19+ messages in thread
From: Jason Wang @ 2025-10-30 2:53 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Michael S. Tsirkin, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09 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.
I still think it would be more safe to check the feature and switch to
the new behaviour if it is set. This seems to be more safe.
But I'm fine if it's agreed that this could be the way to go.
>
> Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> include/linux/virtio_net.h | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 710ae0d2d336..6ef0b737d548 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -217,25 +217,35 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>
> if (skb_is_gso(skb)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
> + u16 hdr_len = 0;
>
> /* In certain code paths (such as the af_packet.c receive path),
> * this function may be called without a transport header.
> * In this case, we do not need to set the hdr_len.
> */
> if (skb_transport_header_was_set(skb))
> - hdr->hdr_len = __cpu_to_virtio16(little_endian,
> - skb_headlen(skb));
> + hdr_len = skb_transport_offset(skb);
>
> hdr->gso_size = __cpu_to_virtio16(little_endian,
> sinfo->gso_size);
> - if (sinfo->gso_type & SKB_GSO_TCPV4)
> + if (sinfo->gso_type & SKB_GSO_TCPV4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> - else if (sinfo->gso_type & SKB_GSO_TCPV6)
> + if (hdr_len)
> + hdr_len += tcp_hdrlen(skb);
> + } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> - else if (sinfo->gso_type & SKB_GSO_UDP_L4)
> + if (hdr_len)
> + hdr_len += tcp_hdrlen(skb);
> + } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> - else
> + if (hdr_len)
> + hdr_len += sizeof(struct udphdr);
> + } else {
> return -EINVAL;
> + }
> +
> + hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> +
> if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> } else
> --
> 2.32.0.3.g01195cf9f
>
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-10-30 2:53 ` Jason Wang
@ 2025-11-09 21:41 ` Michael S. Tsirkin
2025-11-10 7:16 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-09 21:41 UTC (permalink / raw)
To: Jason Wang
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> On Wed, Oct 29, 2025 at 11:09 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.
>
> I still think it would be more safe to check the feature
which feature VIRTIO_NET_F_GUEST_HDRLEN ?
> and switch to
> the new behaviour if it is set. This seems to be more safe.
>
> But I'm fine if it's agreed that this could be the way to go.
>
> >
> > Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> > include/linux/virtio_net.h | 22 ++++++++++++++++------
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index 710ae0d2d336..6ef0b737d548 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -217,25 +217,35 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> >
> > if (skb_is_gso(skb)) {
> > struct skb_shared_info *sinfo = skb_shinfo(skb);
> > + u16 hdr_len = 0;
> >
> > /* In certain code paths (such as the af_packet.c receive path),
> > * this function may be called without a transport header.
> > * In this case, we do not need to set the hdr_len.
> > */
> > if (skb_transport_header_was_set(skb))
> > - hdr->hdr_len = __cpu_to_virtio16(little_endian,
> > - skb_headlen(skb));
> > + hdr_len = skb_transport_offset(skb);
> >
> > hdr->gso_size = __cpu_to_virtio16(little_endian,
> > sinfo->gso_size);
> > - if (sinfo->gso_type & SKB_GSO_TCPV4)
> > + if (sinfo->gso_type & SKB_GSO_TCPV4) {
> > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> > - else if (sinfo->gso_type & SKB_GSO_TCPV6)
> > + if (hdr_len)
> > + hdr_len += tcp_hdrlen(skb);
> > + } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
> > hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> > - else if (sinfo->gso_type & SKB_GSO_UDP_L4)
> > + if (hdr_len)
> > + hdr_len += tcp_hdrlen(skb);
> > + } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> > hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> > - else
> > + if (hdr_len)
> > + hdr_len += sizeof(struct udphdr);
> > + } else {
> > return -EINVAL;
> > + }
> > +
> > + hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> > +
> > if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> > hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> > } else
> > --
> > 2.32.0.3.g01195cf9f
> >
>
> Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-11-09 21:41 ` Michael S. Tsirkin
@ 2025-11-10 7:16 ` Jason Wang
2025-11-10 7:26 ` Michael S. Tsirkin
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2025-11-10 7:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Mon, Nov 10, 2025 at 5:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> > On Wed, Oct 29, 2025 at 11:09 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.
> >
> > I still think it would be more safe to check the feature
>
> which feature VIRTIO_NET_F_GUEST_HDRLEN ?
>
Yes.
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-11-10 7:16 ` Jason Wang
@ 2025-11-10 7:26 ` Michael S. Tsirkin
2025-11-10 7:39 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-10 7:26 UTC (permalink / raw)
To: Jason Wang
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Mon, Nov 10, 2025 at 03:16:08PM +0800, Jason Wang wrote:
> On Mon, Nov 10, 2025 at 5:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> > > On Wed, Oct 29, 2025 at 11:09 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.
> > >
> > > I still think it would be more safe to check the feature
> >
> > which feature VIRTIO_NET_F_GUEST_HDRLEN ?
> >
>
> Yes.
>
> Thanks
Seems more conservative for sure, though an extra mode to maintain isn't
great. Hmm?
--
MST
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-11-10 7:26 ` Michael S. Tsirkin
@ 2025-11-10 7:39 ` Jason Wang
2025-11-10 16:10 ` Michael S. Tsirkin
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2025-11-10 7:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Mon, Nov 10, 2025 at 3:27 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Nov 10, 2025 at 03:16:08PM +0800, Jason Wang wrote:
> > On Mon, Nov 10, 2025 at 5:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> > > > On Wed, Oct 29, 2025 at 11:09 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.
> > > >
> > > > I still think it would be more safe to check the feature
> > >
> > > which feature VIRTIO_NET_F_GUEST_HDRLEN ?
> > >
> >
> > Yes.
> >
> > Thanks
>
> Seems more conservative for sure, though an extra mode to maintain isn't
> great. Hmm?
Considering it's not a lot of code, it might be worth it to reduce the risk.
But I'm fine if you think we can go with this patch.
Thanks
>
> --
> MST
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-11-10 7:39 ` Jason Wang
@ 2025-11-10 16:10 ` Michael S. Tsirkin
2025-11-11 0:45 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-10 16:10 UTC (permalink / raw)
To: Jason Wang
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Mon, Nov 10, 2025 at 03:39:50PM +0800, Jason Wang wrote:
> On Mon, Nov 10, 2025 at 3:27 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Nov 10, 2025 at 03:16:08PM +0800, Jason Wang wrote:
> > > On Mon, Nov 10, 2025 at 5:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> > > > > On Wed, Oct 29, 2025 at 11:09 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.
> > > > >
> > > > > I still think it would be more safe to check the feature
> > > >
> > > > which feature VIRTIO_NET_F_GUEST_HDRLEN ?
> > > >
> > >
> > > Yes.
> > >
> > > Thanks
> >
> > Seems more conservative for sure, though an extra mode to maintain isn't
> > great. Hmm?
>
> Considering it's not a lot of code, it might be worth it to reduce the risk.
>
> But I'm fine if you think we can go with this patch.
>
> Thanks
hard to say what does "not a lot of code" mean here.
but generally if VIRTIO_NET_F_GUEST_HDRLEN is not set
just doing a quick skb_headlen and not poking at
the transport things sounds like a win.
I'd like to at least see the patch along the lines you propose,
and we will judge if it's too much mess to support.
> >
> > --
> > MST
> >
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-11-10 16:10 ` Michael S. Tsirkin
@ 2025-11-11 0:45 ` Jason Wang
0 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2025-11-11 0:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Xuan Zhuo, netdev, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Tue, Nov 11, 2025 at 12:10 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Nov 10, 2025 at 03:39:50PM +0800, Jason Wang wrote:
> > On Mon, Nov 10, 2025 at 3:27 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Mon, Nov 10, 2025 at 03:16:08PM +0800, Jason Wang wrote:
> > > > On Mon, Nov 10, 2025 at 5:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Thu, Oct 30, 2025 at 10:53:01AM +0800, Jason Wang wrote:
> > > > > > On Wed, Oct 29, 2025 at 11:09 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.
> > > > > >
> > > > > > I still think it would be more safe to check the feature
> > > > >
> > > > > which feature VIRTIO_NET_F_GUEST_HDRLEN ?
> > > > >
> > > >
> > > > Yes.
> > > >
> > > > Thanks
> > >
> > > Seems more conservative for sure, though an extra mode to maintain isn't
> > > great. Hmm?
> >
> > Considering it's not a lot of code, it might be worth it to reduce the risk.
> >
> > But I'm fine if you think we can go with this patch.
> >
> > Thanks
>
> hard to say what does "not a lot of code" mean here.
> but generally if VIRTIO_NET_F_GUEST_HDRLEN is not set
> just doing a quick skb_headlen and not poking at
> the transport things sounds like a win.
Yes, this is exactly what I meant.
>
> I'd like to at least see the patch along the lines you propose,
> and we will judge if it's too much mess to support.
+1.
Thanks
>
>
> > >
> > > --
> > > MST
> > >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
2025-10-29 3:09 ` [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
2025-10-30 2:53 ` Jason Wang
@ 2025-11-09 21:42 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-11-09 21:42 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jason Wang, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09:12AM +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>
ca you supply some examples when this is wrong please?
> ---
> include/linux/virtio_net.h | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 710ae0d2d336..6ef0b737d548 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -217,25 +217,35 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>
> if (skb_is_gso(skb)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
> + u16 hdr_len = 0;
>
> /* In certain code paths (such as the af_packet.c receive path),
> * this function may be called without a transport header.
> * In this case, we do not need to set the hdr_len.
> */
you actually do initialize it, just to 0.
the comment is confusing.
> if (skb_transport_header_was_set(skb))
> - hdr->hdr_len = __cpu_to_virtio16(little_endian,
> - skb_headlen(skb));
> + hdr_len = skb_transport_offset(skb);
better:
else
hdr_len = 0;
>
> hdr->gso_size = __cpu_to_virtio16(little_endian,
> sinfo->gso_size);
> - if (sinfo->gso_type & SKB_GSO_TCPV4)
> + if (sinfo->gso_type & SKB_GSO_TCPV4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> - else if (sinfo->gso_type & SKB_GSO_TCPV6)
> + if (hdr_len)
> + hdr_len += tcp_hdrlen(skb);
> + } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> - else if (sinfo->gso_type & SKB_GSO_UDP_L4)
> + if (hdr_len)
> + hdr_len += tcp_hdrlen(skb);
> + } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> - else
> + if (hdr_len)
> + hdr_len += sizeof(struct udphdr);
> + } else {
> return -EINVAL;
> + }
> +
> + hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> +
> if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> } else
> --
> 2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v4 4/4] virtio-net: correct hdr_len handling for tunnel gso
2025-10-29 3:09 [PATCH net v4 0/4] fixes two virtio-net related bugs Xuan Zhuo
` (2 preceding siblings ...)
2025-10-29 3:09 ` [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN Xuan Zhuo
@ 2025-10-29 3:09 ` Xuan Zhuo
2025-10-29 9:51 ` Paolo Abeni
2025-10-30 8:02 ` [PATCH net v4 0/4] fixes two virtio-net related bugs Michael S. Tsirkin
4 siblings, 1 reply; 19+ messages in thread
From: Xuan Zhuo @ 2025-10-29 3:09 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heng Qi, Willem de Bruijn, 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 | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 6ef0b737d548..46b04816d333 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -207,6 +207,14 @@ 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_tcp_hdrlen(const struct sk_buff *skb, bool tnl)
+{
+ if (tnl)
+ return inner_tcp_hdrlen(skb);
+
+ return tcp_hdrlen(skb);
+}
+
static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
struct virtio_net_hdr *hdr,
bool little_endian,
@@ -217,25 +225,33 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
if (skb_is_gso(skb)) {
struct skb_shared_info *sinfo = skb_shinfo(skb);
+ bool tnl = false;
u16 hdr_len = 0;
- /* In certain code paths (such as the af_packet.c receive path),
- * this function may be called without a transport header.
- * In this case, we do not need to set the hdr_len.
- */
- if (skb_transport_header_was_set(skb))
- hdr_len = skb_transport_offset(skb);
+ if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
+ SKB_GSO_UDP_TUNNEL_CSUM)) {
+ tnl = true;
+ hdr_len = skb_inner_transport_offset(skb);
+
+ } else {
+ /* In certain code paths (such as the af_packet.c receive path),
+ * this function may be called without a transport header.
+ * In this case, we do not need to set the hdr_len.
+ */
+ if (skb_transport_header_was_set(skb))
+ hdr_len = skb_transport_offset(skb);
+ }
hdr->gso_size = __cpu_to_virtio16(little_endian,
sinfo->gso_size);
if (sinfo->gso_type & SKB_GSO_TCPV4) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
if (hdr_len)
- hdr_len += tcp_hdrlen(skb);
+ hdr_len += virtio_net_tcp_hdrlen(skb, tnl);
} else if (sinfo->gso_type & SKB_GSO_TCPV6) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
if (hdr_len)
- hdr_len += tcp_hdrlen(skb);
+ hdr_len += virtio_net_tcp_hdrlen(skb, tnl);
} else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
if (hdr_len)
@@ -420,10 +436,7 @@ 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;
if (ret)
return ret;
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net v4 4/4] virtio-net: correct hdr_len handling for tunnel gso
2025-10-29 3:09 ` [PATCH net v4 4/4] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
@ 2025-10-29 9:51 ` Paolo Abeni
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Abeni @ 2025-10-29 9:51 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
[-- Attachment #1.1: Type: text/plain, Size: 3766 bytes --]
On Wed, Oct 29, 2025 at 4:09 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com>
wrote:
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 6ef0b737d548..46b04816d333 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -207,6 +207,14 @@ 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_tcp_hdrlen(const struct sk_buff *skb, bool
tnl)
> +{
> + if (tnl)
> + return inner_tcp_hdrlen(skb);
> +
> + return tcp_hdrlen(skb);
> +}
> +
> static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> struct virtio_net_hdr *hdr,
> bool little_endian,
> @@ -217,25 +225,33 @@ static inline int virtio_net_hdr_from_skb(const
struct sk_buff *skb,
>
> if (skb_is_gso(skb)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
> + bool tnl = false;
> u16 hdr_len = 0;
>
> - /* In certain code paths (such as the af_packet.c receive
path),
> - * this function may be called without a transport header.
> - * In this case, we do not need to set the hdr_len.
> - */
> - if (skb_transport_header_was_set(skb))
> - hdr_len = skb_transport_offset(skb);
> + if (sinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> + SKB_GSO_UDP_TUNNEL_CSUM)) {
> + tnl = true;
> + hdr_len = skb_inner_transport_offset(skb);
> +
> + } else {
> + /* In certain code paths (such as the af_packet.c
receive path),
> + * this function may be called without a
transport header.
> + * In this case, we do not need to set the
hdr_len.
> + */
> + if (skb_transport_header_was_set(skb))
> + hdr_len = skb_transport_offset(skb);
> + }
>
> hdr->gso_size = __cpu_to_virtio16(little_endian,
> sinfo->gso_size);
> if (sinfo->gso_type & SKB_GSO_TCPV4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> if (hdr_len)
> - hdr_len += tcp_hdrlen(skb);
> + hdr_len += virtio_net_tcp_hdrlen(skb,
tnl);
> } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> if (hdr_len)
> - hdr_len += tcp_hdrlen(skb);
> + hdr_len += virtio_net_tcp_hdrlen(skb,
tnl);
> } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> if (hdr_len)
I think it's a bit of a pity that the non-UDP tunnel path had to do all the
additional conditionals.
The (completely untested) alternative attached here would reduce them a bit.
Still I'm a bit concerned by all the 'if (hdr_len)' unconditionally
sprinkled around by the previous patch. The virtio spec says that hdr_len
is valid if and only if the VIRTIO_NET_F_GUEST_HDRLEN feature has been
negotiated.
What about moving hdr->hdr_len initialization in a separate helper and
calling it only when the relevant feature has been negotiated?
Thanks,
Paolo
[-- Attachment #1.2: Type: text/html, Size: 4839 bytes --]
[-- Attachment #2: diffs --]
[-- Type: application/octet-stream, Size: 853 bytes --]
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 6ef0b737d548..672492b9458f 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -420,10 +420,13 @@ 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;
+ /* The basic parsing will look only for the transport header,
+ * let it refer to the relevant one
+ */
+ outer_th = skb->transport_header - skb_headroom(skb);
+ skb->transport_header = skb->inner_transport_header;
ret = virtio_net_hdr_from_skb(skb, hdr, true, false, vlan_hlen);
- skb_shinfo(skb)->gso_type |= tnl_gso_type;
+ skb->transport_header = outer_th + skb_headroom(skb);
if (ret)
return ret;
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net v4 0/4] fixes two virtio-net related bugs.
2025-10-29 3:09 [PATCH net v4 0/4] fixes two virtio-net related bugs Xuan Zhuo
` (3 preceding siblings ...)
2025-10-29 3:09 ` [PATCH net v4 4/4] virtio-net: correct hdr_len handling for tunnel gso Xuan Zhuo
@ 2025-10-30 8:02 ` Michael S. Tsirkin
4 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2025-10-30 8:02 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jason Wang, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heng Qi, Willem de Bruijn, Jiri Pirko, Alvaro Karsz,
virtualization
On Wed, Oct 29, 2025 at 11:09:09AM +0800, Xuan Zhuo wrote:
> As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
> Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
> small amount of code duplication.
>
> Commit #3 is new to fix the hdr len in tunnel gso feature.
I think these are completely independent right?
When there is no dependency it is best to send
patches separately, this way they do not block each other.
>
> Thanks.
>
> v4:
> 1. add commit "virtio-net: Ensure hdr_len is not set unless the header is forwarded to the device." @Jason
>
> v3:
> 1. recode the #3 for tunnel gso, and test it
>
>
>
> Xuan Zhuo (4):
> virtio-net: fix incorrect flags recording in big mode
> virtio-net: Ensure hdr_len is not set unless the header is forwarded
> to the device.
> virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
> virtio-net: correct hdr_len handling for tunnel gso
>
> drivers/net/virtio_net.c | 16 +++++++++----
> include/linux/virtio_net.h | 48 ++++++++++++++++++++++++++++++--------
> 2 files changed, 49 insertions(+), 15 deletions(-)
>
> --
> 2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 19+ messages in thread