netdev - Re: TC: selftests: current timeout (45s) is too low

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: <3acc88b6-a42d-c054-9dae-8aae22348a3e@tessares.net>
Date: Wed, 12 Jul 2023 16:43:09 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Pedro Tammela <pctammela@...atatu.com>,
 Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>,
 Jiri Pirko <jiri@...nulli.us>
Cc: netdev <netdev@...r.kernel.org>, Anders Roxell
 <anders.roxell@...aro.org>, Davide Caratti <dcaratti@...hat.com>
Subject: Re: TC: selftests: current timeout (45s) is too low

Hi Pedro,

On 12/07/2023 15:43, Pedro Tammela wrote:
> I have been involved in tdc for a while now, here are my comments.

Thank you for your reply!

> On 12/07/2023 06:47, Matthieu Baerts wrote:
>> Hi Jamal, Cong, Jiri,
>>
>> When looking for something else [1] in LKFT reports [2], I noticed that
>> the TC selftest ended with a timeout error:
>>
>>    not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>>
>> The timeout has been introduced 3 years ago:
>>
>>    852c8cbf34d3 ("selftests/kselftest/runner.sh: Add 45 second timeout
>> per test")
>>
>> Recently, a new option has been introduced to override the value when
>> executing the code:
>>
>>    f6a01213e3f8 ("selftests: allow runners to override the timeout")
>>
>> But I guess it is still better to set a higher default value for TC
>> tests. This is easy to fix by simply adding "timeout=<seconds>" in a
>> "settings" file in 'tc-testing' directory, e.g.
>>
>>    echo timeout=1200 > tools/testing/selftests/tc-testing/settings
>>
>> I'm sending this email instead of a patch because I don't know which
>> value makes sense. I guess you know how long the tests can take in a
>> (very) slow environment and you might want to avoid this timeout error.
> 
> I believe a timeout between 5-10 to minutes should cover the entire suite

Thank you for your feedback.
If we want to be on the safe side, I guess it is better to put 10
minutes or even 15, no?

>> I also noticed most of the tests were skipped [2], probably because
>> something is missing in the test environment? Do not hesitate to contact
>> the lkft team [3], that's certainly easy to fix and it would increase
>> the TC test coverage when they are validating all the different kernel
>> versions :)
> 
> From the logs it seems like the kernel image is missing the 'ct' action.
> Possibly also missing other actions/tc components, so it seems like a
> kernel config issue.

According to [1], the kconfig is generated by merging these files:

  defconfig, systemd.config [2], tools/testing/selftests/kexec/config,
tools/testing/selftests/net/config,
tools/testing/selftests/net/mptcp/config,
tools/testing/selftests/net/hsr/config,
tools/testing/selftests/net/forwarding/config,
tools/testing/selftests/tc-testing/config

You can see the final .config file in [3].

I can see "CONFIG_NET_ACT_CTINFO(=m)" but not "CONFIG_NET_ACT_CT" while
they are both in tc-testing/config file. Maybe a conflict with another
selftest config?

I don't see any mention of "NET_ACT_CT" in the build logs [4].

Cheers,
Matt

[1]
https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/builds/2SPJN70f1LBiWmZIxl0WNcOmjwN
[2]
https://raw.githubusercontent.com/Linaro/meta-lkft/kirkstone/meta/recipes-kernel/linux/files/systemd.config
[3]
https://storage.tuxsuite.com/public/linaro/lkft/builds/2SPJN70f1LBiWmZIxl0WNcOmjwN/config
[4]
https://storage.tuxsuite.com/public/linaro/lkft/builds/2SPJN70f1LBiWmZIxl0WNcOmjwN/build-debug.log
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net