| Message ID | 20240624130859.953608-5-nicolas.dichtel@6wind.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vrf: fix source address selection with route leak | expand |
On Mon, Jun 24, 2024 at 03:07:56PM +0200, Nicolas Dichtel wrote: > The goal is to check that the source address selected by the kernel is > routable when a leaking route is used. > > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > .../selftests/net/vrf_route_leaking.sh | 38 ++++++++++++++++++- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh > index 2da32f4c479b..6c59e0bbbde3 100755 > --- a/tools/testing/selftests/net/vrf_route_leaking.sh > +++ b/tools/testing/selftests/net/vrf_route_leaking.sh > @@ -533,6 +533,38 @@ ipv6_ping_frag_asym() > ipv6_ping_frag asym > } > > +ipv4_ping_local() > +{ > + local ttype="$1" > + > + [ "x$ttype" = "x" ] && ttype="$DEFAULT_TTYPE" Hi Nicolas, I see this pattern already elsewhere in this file, but shellecheck flags that: 1. No arguments are passed to ipv4_ping_local 2. The condition can be more simply expressed as [ "$ttype" = "" ] (my 2c worth would be [ -z "$ttype" ]) Nit picking aside, I'm genuinely curious about 1, is it actually the case? > + > + log_section "IPv4 ($ttype route): VRF ICMP local error route lookup ping" > + > + setup_"$ttype" > + > + check_connectivity || return > + > + run_cmd ip netns exec $r1 ip vrf exec blue ping -c1 -w1 ${H2_N2_IP} > + log_test $? 0 "VRF ICMP local IPv4" > +} ... > @@ -594,12 +626,14 @@ do > ipv4_traceroute|traceroute) ipv4_traceroute;;& > ipv4_traceroute_asym|traceroute) ipv4_traceroute_asym;;& > ipv4_ping_frag|ping) ipv4_ping_frag;;& > + ipv4_ping_local|ping) ipv4_ping_local;;& > > ipv6_ping_ttl|ping) ipv6_ping_ttl;;& > ipv6_ping_ttl_asym|ping) ipv6_ping_ttl_asym;;& > ipv6_traceroute|traceroute) ipv6_traceroute;;& > ipv6_traceroute_asym|traceroute) ipv6_traceroute_asym;;& > ipv6_ping_frag|ping) ipv6_ping_frag;;& > + ipv6_ping_local|ping) ipv6_ping_local;;& > > # setup namespaces and config, but do not run any tests > setup_sym|setup) setup_sym; exit 0;; > -- > 2.43.1 > >
Le 27/06/2024 à 12:57, Simon Horman a écrit : > On Mon, Jun 24, 2024 at 03:07:56PM +0200, Nicolas Dichtel wrote: >> The goal is to check that the source address selected by the kernel is >> routable when a leaking route is used. >> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> >> --- >> .../selftests/net/vrf_route_leaking.sh | 38 ++++++++++++++++++- >> 1 file changed, 36 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh >> index 2da32f4c479b..6c59e0bbbde3 100755 >> --- a/tools/testing/selftests/net/vrf_route_leaking.sh >> +++ b/tools/testing/selftests/net/vrf_route_leaking.sh >> @@ -533,6 +533,38 @@ ipv6_ping_frag_asym() >> ipv6_ping_frag asym >> } >> >> +ipv4_ping_local() >> +{ >> + local ttype="$1" >> + >> + [ "x$ttype" = "x" ] && ttype="$DEFAULT_TTYPE" > > Hi Nicolas, > > I see this pattern already elsewhere in this file, but shellecheck flags that: > > 1. No arguments are passed to ipv4_ping_local Yes, I don't add an asymmetric version of this test, I don't think that's relevant. I wanted to keep it to be consistent with other tests. > 2. The condition can be more simply expressed as [ "$ttype" = "" ] > (my 2c worth would be [ -z "$ttype" ]) Yeah, I asked myself also, but like for the previous point, I wanted to be consistent with other tests. I will remove this test. Thank you, Nicolas
diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh index 2da32f4c479b..6c59e0bbbde3 100755 --- a/tools/testing/selftests/net/vrf_route_leaking.sh +++ b/tools/testing/selftests/net/vrf_route_leaking.sh @@ -533,6 +533,38 @@ ipv6_ping_frag_asym() ipv6_ping_frag asym } +ipv4_ping_local() +{ + local ttype="$1" + + [ "x$ttype" = "x" ] && ttype="$DEFAULT_TTYPE" + + log_section "IPv4 ($ttype route): VRF ICMP local error route lookup ping" + + setup_"$ttype" + + check_connectivity || return + + run_cmd ip netns exec $r1 ip vrf exec blue ping -c1 -w1 ${H2_N2_IP} + log_test $? 0 "VRF ICMP local IPv4" +} + +ipv6_ping_local() +{ + local ttype="$1" + + [ "x$ttype" = "x" ] && ttype="$DEFAULT_TTYPE" + + log_section "IPv6 ($ttype route): VRF ICMP local error route lookup ping" + + setup_"$ttype" + + check_connectivity6 || return + + run_cmd ip netns exec $r1 ip vrf exec blue ${ping6} -c1 -w1 ${H2_N2_IP6} + log_test $? 0 "VRF ICMP local IPv6" +} + ################################################################################ # usage @@ -555,8 +587,8 @@ EOF # Some systems don't have a ping6 binary anymore command -v ping6 > /dev/null 2>&1 && ping6=$(command -v ping6) || ping6=$(command -v ping) -TESTS_IPV4="ipv4_ping_ttl ipv4_traceroute ipv4_ping_frag ipv4_ping_ttl_asym ipv4_traceroute_asym" -TESTS_IPV6="ipv6_ping_ttl ipv6_traceroute ipv6_ping_ttl_asym ipv6_traceroute_asym" +TESTS_IPV4="ipv4_ping_ttl ipv4_traceroute ipv4_ping_frag ipv4_ping_local ipv4_ping_ttl_asym ipv4_traceroute_asym" +TESTS_IPV6="ipv6_ping_ttl ipv6_traceroute ipv6_ping_local ipv6_ping_ttl_asym ipv6_traceroute_asym" ret=0 nsuccess=0 @@ -594,12 +626,14 @@ do ipv4_traceroute|traceroute) ipv4_traceroute;;& ipv4_traceroute_asym|traceroute) ipv4_traceroute_asym;;& ipv4_ping_frag|ping) ipv4_ping_frag;;& + ipv4_ping_local|ping) ipv4_ping_local;;& ipv6_ping_ttl|ping) ipv6_ping_ttl;;& ipv6_ping_ttl_asym|ping) ipv6_ping_ttl_asym;;& ipv6_traceroute|traceroute) ipv6_traceroute;;& ipv6_traceroute_asym|traceroute) ipv6_traceroute_asym;;& ipv6_ping_frag|ping) ipv6_ping_frag;;& + ipv6_ping_local|ping) ipv6_ping_local;;& # setup namespaces and config, but do not run any tests setup_sym|setup) setup_sym; exit 0;;
The goal is to check that the source address selected by the kernel is routable when a leaking route is used. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- .../selftests/net/vrf_route_leaking.sh | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-)