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] The new helper build_udptnl6_frame() in tools/testing/selftests/net/tun.c (lines 873-908) gets the header… -- selftests: net: tun: test more outer header mismatches of UDP tunnel GSO This commit adds tap tests for outer header checks on UDP tunnel GSO packets. It adds a new 4in6 frame builder, build_udptnl6_frame(), with a positive control and a family mismatch test. It also adds VLAN and L4 protocol mismatch tests that reuse the existing 4in4 builder. Closes: https://lore.kernel.org/netdev/[email protected]/ > diff --git a/tools/testing/selftests/net/tun.c > b/tools/testing/selftests/net/tun.c > index cfeca53863ce2..a135d3aa10d18 100644 > --- a/tools/testing/selftests/net/tun.c > +++ b/tools/testing/selftests/net/tun.c > @@ -869,6 +869,103 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type) > ASSERT_EQ(ret, len); > } > > +/* Build a 4in6 Geneve GSO frame whose outer ethertype is @l3proto. */ > +static int build_udptnl6_frame(uint8_t *buf, uint16_t l3proto, > + int payload_len) > +{ > + int hlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN; > + uint8_t *outer_udph, *cur = buf; > + int pktlen; > + > + memset(buf, 0, TUN_VNET_TNL_SIZE + hlen + payload_len); > + > + cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400, > + AF_INET6, AF_INET); > + cur += build_eth(cur, l3proto, param_hwaddr_outer_dst, > + param_hwaddr_outer_src); > + > + pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - ETH_HLEN - > + sizeof(struct ipv6hdr) + payload_len; [Severity: Medium] Are hlen and pktlen both 14 bytes too small here? UDP_TUNNEL_GENEVE_4IN6_HDRLEN counts only the inner Ethernet header: #define UDP_TUNNEL_GENEVE_4IN6_HDRLEN \ (ETH_HLEN + sizeof(struct iphdr) + sizeof(struct ipv6hdr) + \ GENEVE_HLEN + 2 * sizeof(struct udphdr)) The 4in4 sibling build_udptnl_frame() adds the outer Ethernet header separately. It subtracts only the outer L3 header: int hlen = ETH_HLEN + vlan_len + UDP_TUNNEL_GENEVE_4IN4_HDRLEN; ... pktlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN - sizeof(struct iphdr) + payload_len; build_gso_packet_into_tun() follows the same convention for 4in6 on tap. In build_udptnl6_frame(), the virtio header's hdr_len is set to 98. The real header stack is 112 bytes. The memset() also clears 14 bytes less than the frame that gets written. pktlen works out to 44 + payload_len. The data after the outer IPv6 header is outer UDP + Geneve + inner Ethernet + inner IPv4 + inner UDP + payload, which is 58 + payload_len. So ip6h->payload_len and the outer udph->len are both 14 bytes short. build_udp_packet_csum() sums only ntohs(udph->len) bytes, so the outer UDP checksum is also wrong. outer_proto_ok_ipv6 passes today only because the tun write path does not cross-check those lengths. Would it start failing if outer header validation became stricter? And could outer_proto_bad_family then get EINVAL from the length mismatch instead of the family check it is meant to cover? The later patch in the series, "selftests: net: tuntap: write UDP tunnel offsets in little endian", leaves build_udptnl6_frame() unchanged. Something like this would match build_udptnl_frame(): int hlen = ETH_HLEN + UDP_TUNNEL_GENEVE_4IN6_HDRLEN; ... pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - sizeof(struct ipv6hdr) + payload_len; > + cur += build_ipv6_header(cur, IPPROTO_UDP, 0, pktlen, > + ¶m_ipaddr6_outer_dst, > + ¶m_ipaddr6_outer_src); > + > + outer_udph = cur; > + cur += build_udp_header(cur, UDP_SRC_PORT, VN_PORT, > + pktlen - sizeof(struct udphdr)); [ ... ] > + build_udp_packet_csum(outer_udph, AF_INET6, false); > + > + return cur - buf; > +} > + > +TEST_F(tun_vnet_udptnl_proto, outer_proto_ok_ipv6) > +{ > + int len, ret; > + > + len = build_udptnl6_frame(self->pkt, ETH_P_IPV6, 2800); > + ret = write(self->fd, self->pkt, len); > + ASSERT_EQ(ret, len); > +} > + > +/* An IPv4 ethertype on a declared IPv6 tunnel. */ > +TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_family) > +{ > + int len, ret; > + > + len = build_udptnl6_frame(self->pkt, ETH_P_IP, 2800); > + ret = write(self->fd, self->pkt, len); > + ASSERT_EQ(ret, -1); > + ASSERT_EQ(errno, EINVAL); > +} [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105115715.1466924-1-edumazet%40kernel.org
