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 "
> 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"
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.
