Re: [PATCH net-next 3/3] selftests: traceroute: Add ICMP extensions tests

[Ido Schimmel <idosch@nvidia.com>]

On Wed, Oct 22, 2025 at 06:12:13PM -0400, Willem de Bruijn wrote:
> Ido Schimmel wrote:
> > Test that ICMP extensions are reported correctly when enabled and not
> > reported when disabled. Test both IPv4 and IPv6 and using different
> > packet sizes, to make sure trimming / padding works correctly.
> > 
> > Disable ICMP rate limiting (defaults to 1 per-second per-target) so that
> > the kernel will always generate ICMP errors when needed.
> 
> This reminds me that when I added SOL_IP/IP_RECVERR_4884, the selftest
> was not integrated into kselftests. Commit eba75c587e81 points to
> 
> https://github.com/wdebruij/kerneltools/blob/master/tests/recv_icmp_v2.c

Yes, I saw that :)

> 
> It might be useful to verify that the kernel recv path that parses
> RFC 4884 compliant ICMP messages correctly handles these RFC 4884
> messages.
> 
> But traceroute parsing the data is sufficient validation that packet
> generation is compliant with the RFCs.

We plan to:

1. Add RFC 5837 support to tracepath using the socket options that you
added (instead of assuming that the ICMP extensions are at a fixed
offset like traceroute does).

2. Add a kernel selftest for these socket options. If you want to do
that yourself now that the kernel can generate ICMP extensions (assuming
the patches are accepted), that's fine too.

I already verified that traceroute, wireshark and tcpdump correctly
parse the ICMP messages generated by this series, so I don't expect to
encounter any problems when we integrate this with tracepath.

> 
> > Reviewed-by: Petr Machata <petrm@nvidia.com>
> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > ---
> >  tools/testing/selftests/net/traceroute.sh | 280 ++++++++++++++++++++++
> >  1 file changed, 280 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
> > index dbb34c7e09ce..a57c61bd0b25 100755
> > --- a/tools/testing/selftests/net/traceroute.sh
> > +++ b/tools/testing/selftests/net/traceroute.sh
> > @@ -59,6 +59,8 @@ create_ns()
> >  	ip netns exec ${ns} ip -6 ro add unreachable default metric 8192
> >  
> >  	ip netns exec ${ns} sysctl -qw net.ipv4.ip_forward=1
> > +	ip netns exec ${ns} sysctl -qw net.ipv4.icmp_ratelimit=0
> > +	ip netns exec ${ns} sysctl -qw net.ipv6.icmp.ratelimit=0
> >  	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.keep_addr_on_down=1
> >  	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.forwarding=1
> >  	ip netns exec ${ns} sysctl -qw net.ipv6.conf.default.forwarding=1
> > @@ -297,6 +299,142 @@ run_traceroute6_vrf()
> >  	cleanup_traceroute6_vrf
> >  }
> >  
> > +################################################################################
> > +# traceroute6 with ICMP extensions test
> > +#
> > +# Verify that in this scenario
> > +#
> > +# ----                          ----                          ----
> > +# |H1|--------------------------|R1|--------------------------|H2|
> > +# ----            N1            ----            N2            ----
> > +#
> > +# ICMP extensions are correctly reported. The loopback interfaces on all the
> > +# nodes are assigned global addresses and the interfaces connecting the nodes
> > +# are assigned IPv6 link-local addresses.
> > +
> > +cleanup_traceroute6_ext()
> > +{
> > +	cleanup_all_ns
> > +}
> > +
> > +setup_traceroute6_ext()
> > +{
> > +	# Start clean
> > +	cleanup_traceroute6_ext
> > +
> > +	setup_ns h1 r1 h2
> > +	create_ns "$h1"
> > +	create_ns "$r1"
> > +	create_ns "$h2"
> > +
> > +	# Setup N1
> > +	connect_ns "$h1" eth1 - fe80::1/64 "$r1" eth1 - fe80::2/64
> > +	# Setup N2
> > +	connect_ns "$r1" eth2 - fe80::3/64 "$h2" eth2 - fe80::4/64
> > +
> > +	# Setup H1
> > +	ip -n "$h1" address add 2001:db8:1::1/128 dev lo
> 
> nodad or not needed in this lo special case?

I believe IFF_LOOPBACK is equivalent to IFA_F_NODAD. See the check at
the beginning of addrconf_dad_begin().

> 
> > +	ip -n "$h1" route add ::/0 nexthop via fe80::2 dev eth1
> > +
> > +	# Setup R1
> > +	ip -n "$r1" address add 2001:db8:1::2/128 dev lo
> > +	ip -n "$r1" route add 2001:db8:1::1/128 nexthop via fe80::1 dev eth1
> > +	ip -n "$r1" route add 2001:db8:1::3/128 nexthop via fe80::4 dev eth2
> > +
> > +	# Setup H2
> > +	ip -n "$h2" address add 2001:db8:1::3/128 dev lo
> > +	ip -n "$h2" route add ::/0 nexthop via fe80::3 dev eth2
> > +
> > +	# Prime the network
> > +	ip netns exec "$h1" ping6 -c5 2001:db8:1::3 >/dev/null 2>&1
> > +}

[...]

> > +################################################################################
> > +# traceroute with ICMP extensions test
> > +#
> > +# Verify that in this scenario
> > +#
> > +# ----                          ----                          ----
> > +# |H1|--------------------------|R1|--------------------------|H2|
> > +# ----            N1            ----            N2            ----
> > +#
> > +# ICMP extensions are correctly reported. The loopback interfaces on all the
> > +# nodes are assigned global addresses and the interfaces connecting the nodes
> > +# are assigned IPv6 link-local addresses.
> > +
> > +cleanup_traceroute_ext()
> > +{
> > +	cleanup_all_ns
> > +}
> > +
> > +setup_traceroute_ext()
> > +{
> > +	# Start clean
> > +	cleanup_traceroute_ext
> > +
> > +	setup_ns h1 r1 h2
> > +	create_ns "$h1"
> > +	create_ns "$r1"
> > +	create_ns "$h2"
> > +
> > +	# Setup N1
> > +	connect_ns "$h1" eth1 - fe80::1/64 "$r1" eth1 - fe80::2/64
> > +	# Setup N2
> > +	connect_ns "$r1" eth2 - fe80::3/64 "$h2" eth2 - fe80::4/64
> 
> Stray IPv6 addresses in this IPv4 test?

No, that's intentional :) The use case I'm interested in supporting is
an unnumbered network where router interfaces are only assigned IPv6
link-local addresses and IPv4 routes are configured with IPv6 nexthops.
In these networks only the loopback / VRF interface is configured with
an IPv4 address.

In fact, there are networks where nodes do not have an IPv4 address at
all. In these networks ICMP messages will be generated with a source IP
of 192.0.0.8 (see INADDR_DUMMY in __icmp_send()). That's one motivation
for the Node Identification Object which we might support in the future:
https://datatracker.ietf.org/doc/html/draft-ietf-intarea-extended-icmp-nodeid-04

> As a matter of fact, is it feasible to merge the IPv4 and IPv6 tests
> with some basic variables like $TRACEROUTE, $SYSCTL_PATH and $ADDR?
> 
> (I appreciate that you spent more time looking at that, fine to leave
> if it is not practical to do so.)

Will look into it.

Thanks!