netdev - Re: [PATCH net 4/4] selftests: vrf_route

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240627105734.GF3104@kernel.org>
Date: Thu, 27 Jun 2024 11:57:34 +0100
From: Simon Horman <horms@...nel.org>
To: Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net 4/4] selftests: vrf_route_leaking: add local ping test

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@...nd.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
> 
>