Jakub Kicinski wrote:
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 "
cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
cmd="$stdbuf ./$kselftest_interp$BASENAME_TEST $kselftest_cmd_args"
That could work.
Is reporting one KTAP and exitcode per directory vs per packetdrill invocation good/bad/neither?
Three other issues if this is calling packetdrill directly is - passing the non-trivial IP specific flags - running twice, for IPv4 and IPv6 - chdir into the directory of the pkt file
That can be addressed by instead calling a small wrapper shell script.
That would do the test_func_builder part of packetdrill_ksft.py. But without the need to handle netns, popen/cmd, etc, and thus the ksft dependencies.