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: <7d92788b-13c5-4f53-8b58-9b6ece26310d@mojatatu.com>
Date: Tue, 23 Jan 2024 13:47:35 -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 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.
>>
>> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
>> ---
>>   .../selftests/tc-testing/scripts/taprio_wait_for_admin.sh    | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh
>> index f5335e8ad6b4..68f2c6eaa802 100755
>> --- a/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh
>> +++ b/tools/testing/selftests/tc-testing/scripts/taprio_wait_for_admin.sh
>> @@ -3,6 +3,11 @@
>>   TC="$1"; shift
>>   ETH="$1"; shift
>>
>> +if ! command -v jq &> /dev/null; then
>> +    echo "Please install jq"
>> +    exit 1
>> +fi
>> +
> 
> 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

ok 10 a7bf - Graft cbs as child of software taprio # skipped - skipped - 
previous setup failed 9 39b4

ok 11 6a83 - Graft cbs as child 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...

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?