Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] selftests: net: tun: UDP tunnel GSO tests still fail on big- endian hosts -- > diff --git a/tools/testing/selftests/net/tuntap_helpers.h > b/tools/testing/selftests/net/tuntap_helpers.h > --- 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] Does this fully fix the big-endian case? The commit message says csum_start follows the tun vnet header endianness, but that holds for only part of the kernel path. tun_open() never issues TUNSETVNETLE, so on a big-endian host tun_vnet_is_little_endian() is false. virtio_net_hdr_tnl_to_skb() does read csum_start with __virtio16_to_cpu(little_endian, ...) for the initial offset ordering check. It then calls __virtio_net_hdr_to_skb(skb, hdr, true, ...) with little_endian hardcoded to true. Inside __virtio_net_hdr_to_skb(), csum_start, csum_offset and gso_size get byte swapped. build_virtio_net_hdr_v1_hash_tunnel() still writes all three in host order, so: csum_start 84 becomes 21504 csum_offset 6 becomes 1536 gso_size 1400 becomes 30725 The 2800 byte frames used by tun_vnet_udptnl_proto and tun_vnet_udptnl_proto_l3 are much smaller than that. The pskb_may_pull() for the checksum location therefore fails, and write() returns -EINVAL before virtio_net_tnl_offsets_ok() is ever reached. The outer_proto_bad* cases would still see EINVAL, but for the wrong reason, and the positive cases would still fail. The receive side has the same problem. virtio_net_hdr_tnl_from_skb() calls virtio_net_hdr_from_skb(skb, hdr, true, false, vlan_hlen), so gso_size comes back little endian. recv_gso_packet() then compares vh->gso_size directly against the host order variant->gso_size. So the UDP tunnel tests in tun.c would still fail on big-endian hosts after this patch. Which way do you want to fix it? One option is for the tests to switch the device to little endian with TUNSETVNETLE and write the base header fields with htole16() too. The other is to treat the hardcoded true in virtio_net_hdr_tnl_to_skb() and virtio_net_hdr_tnl_from_skb() as the real bug. The same function already decodes csum_start with little_endian a few lines earlier. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105115715.1466924-1-edumazet%40kernel.org
