Message-ID: <d369fe0f-d632-270d-7036-6021d9ae787a@tessares.net>
Date: Wed, 12 Jul 2023 16:45:53 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Davide Caratti <dcaratti@...hat.com>,
 Pedro Tammela <pctammela@...atatu.com>
Cc: Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang
 <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>,
 netdev <netdev@...r.kernel.org>, Anders Roxell <anders.roxell@...aro.org>
Subject: Re: TC: selftests: current timeout (45s) is too low

Hi Davide,

On 12/07/2023 16:02, Davide Caratti wrote:
> hello!
> 
> On Wed, Jul 12, 2023 at 3:43 PM Pedro Tammela <pctammela@...atatu.com> wrote:
>>
>> Hi Matthieu,
>>
>> I have been involved in tdc for a while now, here are my comments.
>>
>> 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
> 
> finding a good default is not easy, because some kernel (e.g. those
> built with debug options) are very slow .
Thank you for your feedback!

I agree it is not be easy. From what I see, lkft doesn't run the
selftests with a debug kconfig. I guess we can assume these tests are
either ran in a slow environment or with a debug kconfig but not both,
otherwise the timeout would be too high -- at least that's what we did
with MPTCP :)

Is 15 minute a good value to start with?

> Maybe we can leverage also on the other value in tdc_config.py [1] -
> or at least ensure that the setting in 'setting' is consistent.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=d37e56df23f9e
I don't know the TC test env but it looks like it is a timeout that is
used in case of issues. I guess this timeout should in theory never be
fired except in case of big problem, no?
If we use it and if there are 543 tests -- according to the logs -- the
global timeout would be just under 1h50 :)
This timeout in the worst case scenario when nothing is working, I guess
it makes sense to have this kselftest timeout below that.

> 
> [...]
> 
>>> 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.
> 
> when I run tdc I use to do:
> 
> #  yes | make kselftest-merge
> 
> so that the kconfigs are not forgot :)

It looks like it is more than what lkft is using (see my reply to
Pedro). Maybe a conflict with just the ones lkft is using then?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Powered by blists - more mailing lists