netdev - Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with ksft

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240828090120.71be0b20@kernel.org>
Date: Wed, 28 Aug 2024 09:01:20 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 davem@...emloft.net, edumazet@...gle.com, ncardwell@...gle.com,
 shuah@...nel.org, linux-kselftest@...r.kernel.org, fw@...len.de, Willem de
 Bruijn <willemb@...gle.com>, "Matthieu Baerts (NGI0)" <matttbe@...nel.org>,
 martineau@...nel.org
Subject: Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with
 ksft

On Wed, 28 Aug 2024 10:03:55 -0400 Willem de Bruijn wrote:
> > > A single script is much
> > > simpler, optionally with nested KTAP (not supported yet by ksft). But,
> > > I'm afraid that running time without intermediate output will be very
> > > long when we integrate all packetdrill scripts.  
> > 
> > If I read correctly, this runs the scripts in the given directory 
> > sequentially (as opposed to the default pktdrill run_all.py behavior 
> > that uses many concurrent threads).
> > 
> > I guess/fear that running all the pktdrill tests in a single batch would 
> > take quite a long time, which in turn could be not so good for CI 
> > integration. Currently there are a couple of CI test-cases with runtime   
> >  > 1h, but that is bad ;)  
> 
> Very good point, thanks! This is the third packetdrill runner that I'm
> writing. I should know this by now.. Let me see whether I can use
> run_all.py rather than reinvent the wheel here.

Do we have to worry about this now? If you're planning to add a runner
specifically for packetdrill... IDK if we should. We already have a few
runners, not to mention that run_kselftest.sh itself can run all the
test cases in parallel in separate network namespaces!

What I was wondering is whether we can use shebang to direct the .pkt
files to be "executed" by the python script. Alternatively we can add
support to specifying "interpreter" for a given test in ksft infra
(kinda like we can pass cmd line arguments to a test). Completely
untested but it should give better idea what I mean than a description:

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 74954f6a8f94..429c279e9c6e 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -56,6 +56,7 @@ run_one()
 	export kselftest_timeout="$kselftest_default_timeout"
 
 	# Safe default if tr not available
+	kselftest_interp_ref="KSELFTEST_INTERP"
 	kselftest_cmd_args_ref="KSELFTEST_ARGS"
 
 	# Optional arguments for this command, possibly defined as an
@@ -78,6 +79,14 @@ run_one()
 					$TR_CMD [:lower:] [:upper:])
 		kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
 	fi
+	# Optional interpreter to run the test case
+	if [ -n "$TR_CMD" ]; then
+		SUFFIX_SANITIZED=$(echo "${BASENAME_TEST#*.}" | \
+				       $TR_CMD -d "[:blank:][:cntrl:]" | \
+				       $TR_CMD -c "[:alnum:]_" "_" | \
+				       $TR_CMD [:lower:] [:upper:])
+		kselftest_interp_ref="KSELFTEST_${SUFFIX_SANITIZED}_INTERP"
+	fi
 
 	# Load per-test-directory kselftest "settings" file.
 	settings="$BASE_DIR/$DIR/settings"
@@ -110,8 +119,12 @@ run_one()
 		if [ -x /usr/bin/stdbuf ]; then
 			stdbuf="/usr/bin/stdbuf --output=L "
 		fi
+		eval kselftest_interp="\$${kselftest_interp_ref:-}"
+		# Add a space at the end if interpreter is set to work in $cmd
+		[ -n "$kselftest_interp" ] && \
+		    kselftest_interp="$kselftest_interp "
 		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
-		cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
+		cmd="$stdbuf ./$kselftest_interp$BASENAME_TEST $kselftest_cmd_args"
 		if [ ! -x "$TEST" ]; then
 			echo "# Warning: file $TEST is not executable"