netdev - Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

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: <DG7TJFKZNG1J.19BZF87IWD3UG@google.com>
Date: Fri, 06 Feb 2026 10:48:59 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: Hangbin Liu <liuhangbin@...il.com>, <linux-kselftest@...r.kernel.org>
Cc: Shuah Khan <shuah@...nel.org>, Brendan Jackman <jackmanb@...gle.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

On Fri Feb 6, 2026 at 8:17 AM UTC, Hangbin Liu wrote:
> This has been on my todo list for a long time. We should use ktap
> helpers in runner.sh. I saw Brendan did some work in d9e6269e3303
> ("selftests/run_kselftest.sh: exit with error if tests fail") to make
> run_kselftest.sh exit with the correct return value. But we still use a
> custom solution in runner.sh. Let's convert all the print messages to use
> formal ktap helpers. Here’s what I changed:
>
>   1. Move TAP header from runner.sh to run_kselftest.sh, since run_kselftest.sh
>      is the only caller of run_many().
>   2. In run_kselftest.sh, call run_many() in main process to count the
>      pass/fail numbers.
>   3. In run_kselftest.sh, do not generate kselftest_failures_file; just
>      use ktap_print_totals to report the result.
>   4. In runner.sh run_one(), get the return value and use ktap helpers for
>      all pass/fail reporting. This allows counting pass/fail numbers in the
>      main process.
>   5. In runner.sh run_in_netns(), also return the correct rc, so we can
>      count results during wait.
>
> After the change, the printed result looks like:
>
>   not ok 4 4 selftests: clone3: clone3_cap_checkpoint_restore # exit=1
>   # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
>
>   ]# echo $?
>   1

Sorry I'm being a bit lazy by not investigating this myself but the
current intended behaviour is for runner.sh to output correct KTAP,
right? Could you describe what behavioural changes this is expected to
bring - does it fix/change the KTAP output? (Or if it's just a cleanup
with no intended change then please note that in the commit message).

I have not reviewed the code properly yet (I just read enough to check
that it still does the thing I care about i.e. return an error code when
something fails). I can do a proper review next week if you add a bit
more context re the question above though.

> -	wait
> +	# Handle the return values when running in netns.
> +	for pid in "${pids[@]}"; do
> +		wait "$pid"
> +		rc=$?
> +		[ "$rc" -eq "$KSFT_PASS" ] && KTAP_CNT_PASS=$((KTAP_CNT_PASS+1))
> +		[ "$rc" -eq "$KSFT_FAIL" ] && KTAP_CNT_FAIL=$((KTAP_CNT_FAIL+1))
> +		[ "$rc" -eq "$KSFT_SKIP" ] && KTAP_CNT_SKIP=$((KTAP_CNT_SKIP+1))
> +		[ "$rc" -eq "$KSFT_XFAIL" ] && KTAP_CNT_XFAIL=$((KTAP_CNT_XFAIL+1))

These variables don't seeem to have anything to do with KTAP so I think
they should have a different name prefix. I guess KSFT_*? Or maybe
RUNNER_* or something.

Also I guess we should initialise them to 0 so they are still set if no
tests actually get run?

Also I think it's probably time to start documenting the
interface of runner.sh - sorry I probably should have done this when I
created kselftest_failures_file. Can you add a comment at the top to
show that these variables are part of the "API"?