On Mon, 13 Oct 2025 10:11:31 +0200, Paolo Abeni <[email protected]> wrote:
> On 10/13/25 4:06 AM, Xuan Zhuo wrote:
> > The commit a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP
> > GSO tunneling.") introduces support for the UDP GSO tunnel feature in
> > virtio-net.
> >
> > The virtio spec says:
> >
> >     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit 
> > or
> >     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
> >     all the headers up to and including the inner transport.
> >
> > The commit did not update the hdr_len to include the inner transport.
> >
> > Fixes: a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP GSO 
> > tunneling.")
> > Signed-off-by: Xuan Zhuo <[email protected]>
>
> Side note: qemu support for UDP GSO tunneling is available in qemu since
> commit a5289563ad.
>
> > ---
> >  include/linux/virtio_net.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index e059e9c57937..765fd5f471a4 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -403,6 +403,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >     struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> >     unsigned int inner_nh, outer_th;
> >     int tnl_gso_type;
> > +   u16 hdr_len;
> >     int ret;
> >
> >     tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> > @@ -434,6 +435,23 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >     outer_th = skb->transport_header - skb_headroom(skb);
> >     vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
> >     vhdr->outer_th_offset = cpu_to_le16(outer_th);
> > +
> > +   switch (skb->inner_ipproto) {
> > +   case IPPROTO_TCP:
> > +           hdr_len = inner_tcp_hdrlen(skb);
> > +           break;
> > +
> > +   case IPPROTO_UDP:
> > +           hdr_len = sizeof(struct udphdr);
> > +           break;
> > +
> > +   default:
> > +           return -EINVAL;
> > +   }
> > +
> > +   hdr_len += skb_inner_transport_offset(skb);
> > +   hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
>
> I'm not sure this is the correct fix.
>
> virtio_net_hdr_tnl_from_skb() just called virtio_net_hdr_from_skb() on
> the inner header. The virtio spec also specifies:
>
> """
>  If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>  \field{hdr_len} indicates the header length that needs to be replicated
>  for each packet. It's the number of bytes from the beginning of the packet
>  to the beginning of the transport payload.
> """
>
> If `hdr_len` is currently wrong for UDP GSO packets, it's also wrong for
>  plain GSO packets (without UDP tunnel) and the its value should be
> possibly fixed in virtio_net_hdr_from_skb().

YES. This is what the commit #2 does.

Thanks.


>
> Thanks,
>
> Paolo
>

Reply via email to