On Wed, Oct 29, 2025 at 4:09 AM Xuan Zhuo <[email protected]> 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
diffs
Description: Binary data
