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] [day] [month] [year] [list]

Message-ID: <aYX8dot4fTh3FJFI@fedora>
Date: Fri, 6 Feb 2026 14:36:38 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Brendan Jackman <jackmanb@...gle.com>
Cc: linux-kselftest@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
	netdev@...r.kernel.org
Subject: Re: [PATCH selftests] selftests: Use ktap helpers for runner.sh

Hi Brendan,
On Fri, Feb 06, 2026 at 10:48:59AM +0000, Brendan Jackman wrote:
> 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

For runner.sh, I just want to make it using the formal helper in
ktap_helpers.sh other than manually echo.

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

It changes the run_kselftest.sh's output, with a total result. e.g.

# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

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

Yes, I also care about the return code. This patch doesn't break it.

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

These variables are defined in ktap_helpers.sh and used by the ktap helpers.

Thanks
Hangbin