netdev - Re: [PATCH net-next] selftests: net: add helper for checking if nettest is available

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: <ZsIb2joq2MjBwk1u@shredder>
Date: Sun, 18 Aug 2024 19:05:46 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
	pabeni@...hat.com, shuah@...nel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: net: add helper for checking if
 nettest is available

On Sat, Aug 17, 2024 at 11:38:48AM -0700, Jakub Kicinski wrote:
> A few tests check if nettest exists in the $PATH before adding
> $PWD to $PATH and re-checking. They don't discard stderr on
> the first check (and nettest is built as part of selftests,
> so it's pretty normal for it to not be available in system $PATH).
> This leads to output noise:
> 
>   which: no nettest in (/home/virtme/tools/fs/bin:/home/virtme/tools/fs/sbin:/home/virtme/tools/fs/usr/bin:/home/virtme/tools/fs/usr/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin)
> 
> Add a common helper for the check which does silence stderr.
> 
> There is another small functional change hiding here, because pmtu.sh
> used to return from the test case rather than completely exit.
> Building nettest is not hard, there should be no need to maintain
> the ability to selectively skip cases in its absence.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> CC: shuah@...nel.org
> CC: linux-kselftest@...r.kernel.org
> ---
>  tools/testing/selftests/net/fcnal-test.sh         |  9 +--------
>  tools/testing/selftests/net/lib.sh                | 15 +++++++++++++++
>  tools/testing/selftests/net/pmtu.sh               |  8 +-------
>  tools/testing/selftests/net/settings              |  1 +
>  tools/testing/selftests/net/unicast_extensions.sh |  9 +--------
>  5 files changed, 19 insertions(+), 23 deletions(-)

Nice improvement. fib_rule_tests.sh can also benefit from this helper
and so does vrf_route_leaking.sh which assumes 'nettest' is available
in the working directory.

Do you want to fold the diff below into v2 (I tested it)?

diff --git a/tools/testing/selftests/net/fib_rule_tests.sh b/tools/testing/selftests/net/fib_rule_tests.sh
index ec9dbfa6507b..1d58b3b87465 100755
--- a/tools/testing/selftests/net/fib_rule_tests.sh
+++ b/tools/testing/selftests/net/fib_rule_tests.sh
@@ -51,31 +51,6 @@ log_test()
 	fi
 }
 
-check_nettest()
-{
-	if which nettest > /dev/null 2>&1; then
-		return 0
-	fi
-
-	# Add the selftest directory to PATH if not already done
-	if [ "${SELFTEST_PATH}" = "" ]; then
-		SELFTEST_PATH="$(dirname $0)"
-		PATH="${PATH}:${SELFTEST_PATH}"
-
-		# Now retry with the new path
-		if which nettest > /dev/null 2>&1; then
-			return 0
-		fi
-
-		if [ "${ret}" -eq 0 ]; then
-			ret="${ksft_skip}"
-		fi
-		echo "nettest not found (try 'make -C ${SELFTEST_PATH} nettest')"
-	fi
-
-	return 1
-}
-
 setup()
 {
 	set -e
@@ -334,11 +309,6 @@ fib_rule6_connect_test()
 	echo
 	echo "IPv6 FIB rule connect tests"
 
-	if ! check_nettest; then
-		echo "SKIP: Could not run test without nettest tool"
-		return
-	fi
-
 	setup_peer
 	$IP -6 rule add dsfield 0x04 table $RTABLE_PEER
 
@@ -578,11 +548,6 @@ fib_rule4_connect_test()
 	echo
 	echo "IPv4 FIB rule connect tests"
 
-	if ! check_nettest; then
-		echo "SKIP: Could not run test without nettest tool"
-		return
-	fi
-
 	setup_peer
 	$IP -4 rule add dsfield 0x04 table $RTABLE_PEER
 
@@ -674,6 +639,8 @@ if [ ! -x "$(command -v ip)" ]; then
 	exit $ksft_skip
 fi
 
+check_gen_prog "nettest"
+
 # start clean
 cleanup &> /dev/null
 setup
diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh
index 152171fb1fc8..e9c2f71da207 100755
--- a/tools/testing/selftests/net/vrf_route_leaking.sh
+++ b/tools/testing/selftests/net/vrf_route_leaking.sh
@@ -59,7 +59,6 @@
 # while it is forwarded between different vrfs.
 
 source lib.sh
-PATH=$PWD:$PWD/tools/testing/selftests/net:$PATH
 VERBOSE=0
 PAUSE_ON_FAIL=no
 DEFAULT_TTYPE=sym
@@ -636,6 +635,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)
 
+check_gen_prog "nettest"
+
 TESTS_IPV4="ipv4_ping_ttl ipv4_traceroute ipv4_ping_frag ipv4_ping_local ipv4_tcp_local
 ipv4_udp_local ipv4_ping_ttl_asym ipv4_traceroute_asym"
 TESTS_IPV6="ipv6_ping_ttl ipv6_traceroute ipv6_ping_local ipv6_tcp_local ipv6_udp_local