Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] selftests: net: tun: UDP tunnel GSO tests still fail on big-endian hosts --
commit 19b0a422177cb62b0357c401d17b939feb10dca3 Author: Eric Dumazet <[email protected]> selftests: net: tuntap: write UDP tunnel offsets in little endian This commit converts the outer_th_offset and inner_nh_offset fields of struct virtio_net_hdr_v1_hash_tunnel to little-endian in the tuntap selftests while leaving the base virtio_net_hdr_v1 fields in host byte order. > diff --git a/tools/testing/selftests/net/tuntap_helpers.h > b/tools/testing/selftests/net/tuntap_helpers.h > index ecad83fae1582..dc85cd9bc0aec 100644 > --- a/tools/testing/selftests/net/tuntap_helpers.h > +++ b/tools/testing/selftests/net/tuntap_helpers.h [ ... ] > @@ -375,11 +377,14 @@ static inline int > build_virtio_net_hdr_v1_hash_tunnel(uint8_t *buf, bool is_tap, > inner_iphlen = (inner_family == AF_INET) ? sizeof(struct iphdr) : > sizeof(struct ipv6hdr); > > - vh_tunnel->outer_th_offset = eth_hlen + outer_iphlen; > - vh_tunnel->inner_nh_offset = vh_tunnel->outer_th_offset + ETH_HLEN + > - GENEVE_HLEN + sizeof(struct udphdr); > + outer_th = eth_hlen + outer_iphlen; > + inner_nh = outer_th + ETH_HLEN + GENEVE_HLEN + sizeof(struct udphdr); > > - vh->csum_start = vh_tunnel->inner_nh_offset + inner_iphlen; > + /* The tunnel offsets are always little endian. */ > + vh_tunnel->outer_th_offset = htole16(outer_th); > + vh_tunnel->inner_nh_offset = htole16(inner_nh); > + > + vh->csum_start = inner_nh + inner_iphlen; > vh->csum_offset = __builtin_offsetof(struct udphdr, check); > vh->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > vh->hdr_len = hdr_len; [Severity: Medium] Will the UDP tunnel GSO tests still fail on big-endian hosts because the kernel hardcodes little_endian = true when delegating the base header fields in virtio_net_hdr_tnl_to_skb() and virtio_net_hdr_tnl_from_skb()? In tools/testing/selftests/net/tun.c, tun_open() does not set TUNSETVNETLE, so tun_vnet_is_little_endian() is false and __tun_vnet_hdr_get() reads hdr_len, csum_start, and csum_offset in host order. However, when tun_vnet_hdr_tnl_to_skb() calls virtio_net_hdr_tnl_to_skb() with little_endian = false: include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() { ... inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); inner_nh = le16_to_cpu(vhdr->inner_nh_offset); outer_th = le16_to_cpu(vhdr->outer_th_offset); if (outer_th < outer_l3min || inner_nh < outer_th + sizeof(struct udphdr) || inner_th < inner_nh + inner_l3min) return -EINVAL; /* Let the basic parsing deal with plain GSO features. */ ret = __virtio_net_hdr_to_skb(skb, hdr, true, hdr->gso_type & ~gso_tunnel_type); if (ret) return ret; if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th)) return -EINVAL; ... } Because __virtio_net_hdr_to_skb() is called with little_endian hardcoded to true instead of passing little_endian, it byte-swaps the host-order csum_start, csum_offset, and gso_size on big-endian hosts. That causes __virtio_net_hdr_to_skb() to return -EINVAL (either in pskb_may_pull() or the skb->csum_offset check) before virtio_net_tnl_offsets_ok() is reached in tun_vnet_udptnl_proto, tun_vnet_udptnl_proto_l3, and tun_vnet_udptnl. Similarly, on the receive path in virtio_net_hdr_tnl_from_skb(): include/linux/virtio_net.h:virtio_net_hdr_tnl_from_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); skb_shinfo(skb)->gso_type |= tnl_gso_type; ... } virtio_net_hdr_from_skb() is also called with little_endian hardcoded to true, writing hdr->gso_size in little-endian while recv_gso_packet in tools/testing/selftests/net/tun.c compares vh->gso_size against host-order variant->gso_size. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
