Re: [PATCH net v4 3/4] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
From: Jason Wang <jasowang@redhat.com>
Date: 2025-10-30 02:53:19
Also in:
virtualization
On Wed, Oct 29, 2025 at 11:09 AM Xuan Zhuo [off-list ref] 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.
quoted hunk ↗ jump to hunk
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