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

Message-ID: <1883bb47-869d-46ac-a231-0a109645df22@mojatatu.com>
Date: Wed, 24 Jan 2024 14:26:38 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: Davide Caratti <dcaratti@...hat.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

On 24/01/2024 07:31, Davide Caratti wrote:
> 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.

Oh indeed! Thanks for the suggestion this is much nicer.
I will send a v2!

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