netdev - Re: [PATCH net-next 2/4] selftests: tc-testing: check if 'jq' is available in taprio script

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: <CAKa-r6vJPGQjE4YAtofa-=Pog8a_2Tu5mGcxLjhkoGCqu0JENQ@mail.gmail.com>
Date: Wed, 24 Jan 2024 11:31:43 +0100
From: Davide Caratti <dcaratti@...hat.com>
To: Pedro Tammela <pctammela@...atatu.com>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com, 
	jiri@...nulli.us, shuah@...nel.org, kuba@...nel.org, vladimir.oltean@....com, 
	edumazet@...gle.com, pabeni@...hat.com, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next 2/4] selftests: tc-testing: check if 'jq' is
 available in taprio script

hello Pedro, thanks for your answer!

On Tue, Jan 23, 2024 at 5:47 PM Pedro Tammela <pctammela@...atatu.com> wrote:
>
> On 23/01/2024 10:17, Davide Caratti wrote:
> > hi Pedro,
> >
> > On Tue, Jan 23, 2024 at 1:28 PM Pedro Tammela <pctammela@...atatu.com> wrote:
> >>
> >> If 'jq' is not available the taprio tests that use this script will
> >> run forever. Check if it exists before entering the while loop.

[...]

> > nit: what about returning $KSFT_SKIP (that is 4) if jq is not there?
> > so the test does not fail.
> > thanks!
>
> Since these scripts are run in the setup phase, it has a special treatment.
>
> Take for example this run:
> ok 1 ba39 - Add taprio Qdisc to multi-queue device (8 queues)
> ok 2 9462 - Add taprio Qdisc with multiple sched-entry
> ok 3 8d92 - Add taprio Qdisc with txtime-delay
> ok 4 d092 - Delete taprio Qdisc with valid handle
> ok 5 8471 - Show taprio class
> ok 6 0a85 - Add taprio Qdisc to single-queue device
> ok 7 3e1e - Add taprio Qdisc with an invalid cycle-time
> ok 8 39b4 - Reject grafting taprio as child qdisc of software taprio #
> skipped - "-----> prepare stage" did not complete successfully
>
> ok 9 e8a1 - Reject grafting taprio as child qdisc of offloaded taprio #
> skipped - skipped - previous setup failed 9 39b4

[...]

> As of today it returns 0, success in ksft, even though it clearly
> wasn't. Looking at the code any failures in the setup/teardown phase
> will stop the run, skip all the remaining tests but still return success.
>
> About returning skip from the script, aside from marking it as skip and
> continuing the suite, we would need to run a silent teardown, one that
> executes all commands in the specified teardown but
> ignores errors. In this case we are assuming all setup steps follow KSFT
> return codes. Not sure if it it's reasonable or not...

wouldn't this be fixed by adding this line:

"dependsOn" : "command -v jq >/dev/null"

to test scenarios 39b4 and e8a1 ? I'm asking this because jq is used
also in verifyCmd after the script, to parse results.
Background for this question: I see tdc skipping both setup and
teardown stages for each test case in taprio.json where this line:

     "dependsOn": "command -v ciao >/dev/null",

is present. Rather than doing a setup +  silent teardown, just do
nothing and go to the next test.

> As your suggestion is not a blocker, I would rather address the above
> problems in a follow up series since they will require some refactoring.
> WDYT?

no objections, but I'm curious to see if "dependsOn" would fix this case :)
thanks!
-- 
davide