2023-07-13 22:19:53

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig

When looking for something else in LKFT reports [1], I noticed that the
TC selftest ended with a timeout error:

not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds

I also noticed most of the tests were skipped because the "teardown
stage" did not complete successfully. It was due to missing kconfig.

These patches fix these two errors plus an extra one because this
selftest reads info from "/proc/net/nf_conntrack". Thank you Pedro for
having helped me fixing these issues [2].

Link: https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230711/testrun/18267241/suite/kselftest-tc-testing/test/tc-testing_tdc_sh/log [1]
Link: https://lore.kernel.org/netdev/[email protected]/T/ [2]
Signed-off-by: Matthieu Baerts <[email protected]>
---
Matthieu Baerts (3):
selftests: tc: set timeout to 15 minutes
selftests: tc: add 'ct' action kconfig dep
selftests: tc: add ConnTrack procfs kconfig

tools/testing/selftests/tc-testing/config | 2 ++
tools/testing/selftests/tc-testing/settings | 1 +
2 files changed, 3 insertions(+)
---
base-commit: 9d23aac8a85f69239e585c8656c6fdb21be65695
change-id: 20230713-tc-selftests-lkft-363e4590f105

Best regards,
--
Matthieu Baerts <[email protected]>



2023-07-13 22:20:22

by Matthieu Baerts

[permalink] [raw]
Subject: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes

When looking for something else in LKFT reports [1], I noticed that the
TC selftest ended with a timeout error:

not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds

The timeout had been introduced 3 years ago, see the Fixes commit below.

This timeout is only in place when executing the selftests via the
kselftests runner scripts. I guess this is not what most TC devs are
using and nobody noticed the issue before.

The new timeout is set to 15 minutes as suggested by Pedro [2]. It looks
like it is plenty more time than what it takes in "normal" conditions.

Fixes: 852c8cbf34d3 ("selftests/kselftest/runner.sh: Add 45 second timeout per test")
Cc: [email protected]
Link: https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230711/testrun/18267241/suite/kselftest-tc-testing/test/tc-testing_tdc_sh/log [1]
Link: https://lore.kernel.org/netdev/[email protected]/T/ [2]
Suggested-by: Pedro Tammela <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/tc-testing/settings | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/tc-testing/settings b/tools/testing/selftests/tc-testing/settings
new file mode 100644
index 000000000000..e2206265f67c
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/settings
@@ -0,0 +1 @@
+timeout=900

--
2.40.1


2023-07-13 23:02:49

by Jamal Hadi Salim

[permalink] [raw]
Subject: Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig

On Thu, Jul 13, 2023 at 5:17 PM Matthieu Baerts
<[email protected]> wrote:
>
> When looking for something else in LKFT reports [1], I noticed that the
> TC selftest ended with a timeout error:
>
> not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>
> I also noticed most of the tests were skipped because the "teardown
> stage" did not complete successfully. It was due to missing kconfig.
>
> These patches fix these two errors plus an extra one because this
> selftest reads info from "/proc/net/nf_conntrack". Thank you Pedro for
> having helped me fixing these issues [2].
>
> Link: https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230711/testrun/18267241/suite/kselftest-tc-testing/test/tc-testing_tdc_sh/log [1]
> Link: https://lore.kernel.org/netdev/[email protected]/T/ [2]
> Signed-off-by: Matthieu Baerts <[email protected]>

For the patchset:
Acked-by: Jamal Hadi Salim <[email protected]>

Thanks for all the effort Matthieu!

cheers,
jamal
> ---
> Matthieu Baerts (3):
> selftests: tc: set timeout to 15 minutes
> selftests: tc: add 'ct' action kconfig dep
> selftests: tc: add ConnTrack procfs kconfig
>
> tools/testing/selftests/tc-testing/config | 2 ++
> tools/testing/selftests/tc-testing/settings | 1 +
> 2 files changed, 3 insertions(+)
> ---
> base-commit: 9d23aac8a85f69239e585c8656c6fdb21be65695
> change-id: 20230713-tc-selftests-lkft-363e4590f105
>
> Best regards,
> --
> Matthieu Baerts <[email protected]>
>

2023-07-14 18:10:03

by Pedro Tammela

[permalink] [raw]
Subject: Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes

On 13/07/2023 23:25, shaozhengchao wrote:
>
>
> On 2023/7/14 5:16, Matthieu Baerts wrote:
>> When looking for something else in LKFT reports [1], I noticed that the
>> TC selftest ended with a timeout error:
>>
>>    not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>>
>> The timeout had been introduced 3 years ago, see the Fixes commit below.
>>
>> This timeout is only in place when executing the selftests via the
>> kselftests runner scripts. I guess this is not what most TC devs are
>> using and nobody noticed the issue before.
>>
>> The new timeout is set to 15 minutes as suggested by Pedro [2]. It looks
>> like it is plenty more time than what it takes in "normal" conditions.
>>
>> Fixes: 852c8cbf34d3 ("selftests/kselftest/runner.sh: Add 45 second
>> timeout per test")
>> Cc: [email protected]
>> Link:
>> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230711/testrun/18267241/suite/kselftest-tc-testing/test/tc-testing_tdc_sh/log [1]
>> Link:
>> https://lore.kernel.org/netdev/[email protected]/T/ [2]
>> Suggested-by: Pedro Tammela <[email protected]>
>> Signed-off-by: Matthieu Baerts <[email protected]>
>> ---
>>   tools/testing/selftests/tc-testing/settings | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/testing/selftests/tc-testing/settings
>> b/tools/testing/selftests/tc-testing/settings
>> new file mode 100644
>> index 000000000000..e2206265f67c
>> --- /dev/null
>> +++ b/tools/testing/selftests/tc-testing/settings
>> @@ -0,0 +1 @@
>> +timeout=900
>>
> I remember last year when I tested all the tdc cases(qdisc + filter +
> action + infra) in my vm machine, it took me nearly 20 minutes.
> So I think it should be more than 1200 seconds if all cases need to be
> tested.
>
> Maybe we should really optimize the parallel execution process of tdc.

Let's try to spend some cycles improving the tdc code performance first.
TDC boils down essentially to:
- Setup namespace (if needed)
- Setup network interfaces
- Spawn a few processes
- Match a regex
- Bring down namespace

Nothing above screams expensive, so I'm sure there are some low hanging
fruits to improve the overall wall time even in debug kernels.


2023-07-17 09:14:23

by Matthieu Baerts

[permalink] [raw]
Subject: Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes

Hi Zhengchao Shao,

On 14/07/2023 04:25, shaozhengchao wrote:
>
>
> On 2023/7/14 5:16, Matthieu Baerts wrote:
>> When looking for something else in LKFT reports [1], I noticed that the
>> TC selftest ended with a timeout error:
>>
>>    not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>>
>> The timeout had been introduced 3 years ago, see the Fixes commit below.
>>
>> This timeout is only in place when executing the selftests via the
>> kselftests runner scripts. I guess this is not what most TC devs are
>> using and nobody noticed the issue before.
>>
>> The new timeout is set to 15 minutes as suggested by Pedro [2]. It looks
>> like it is plenty more time than what it takes in "normal" conditions.
>>
>> Fixes: 852c8cbf34d3 ("selftests/kselftest/runner.sh: Add 45 second
>> timeout per test")
>> Cc: [email protected]
>> Link:
>> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230711/testrun/18267241/suite/kselftest-tc-testing/test/tc-testing_tdc_sh/log [1]
>> Link:
>> https://lore.kernel.org/netdev/[email protected]/T/ [2]
>> Suggested-by: Pedro Tammela <[email protected]>
>> Signed-off-by: Matthieu Baerts <[email protected]>
>> ---
>>   tools/testing/selftests/tc-testing/settings | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/testing/selftests/tc-testing/settings
>> b/tools/testing/selftests/tc-testing/settings
>> new file mode 100644
>> index 000000000000..e2206265f67c
>> --- /dev/null
>> +++ b/tools/testing/selftests/tc-testing/settings
>> @@ -0,0 +1 @@
>> +timeout=900
>>
> I remember last year when I tested all the tdc cases(qdisc + filter +
> action + infra) in my vm machine, it took me nearly 20 minutes.
> So I think it should be more than 1200 seconds if all cases need to be
> tested.

Thank you for your feedback!

Be careful that here, it is the timeout to run "tdc.sh" only which is
currently limited to:

./tdc.py -c actions --nobuildebpf
./tdc.py -c qdisc

(not "filter", nor "infra" then)

I guess for this, 15 minutes is more than enough, no?

At least on my side, I ran it in a i386 VM without KVM and it took less
than 3 minutes [1].

Cheers,
Matt

[1]
https://tuxapi.tuxsuite.com/v1/groups/community/projects/matthieu.baerts/tests/2SWHb7PJfqkUX1m8rLu3GXbsHE0/logs?format=html
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net

2023-07-18 09:01:25

by Matthieu Baerts

[permalink] [raw]
Subject: Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig

Hi David, Jakub, Paolo,

On 13/07/2023 23:16, Matthieu Baerts wrote:
> When looking for something else in LKFT reports [1], I noticed that the
> TC selftest ended with a timeout error:
>
> not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>
> I also noticed most of the tests were skipped because the "teardown
> stage" did not complete successfully. It was due to missing kconfig.
>
> These patches fix these two errors plus an extra one because this
> selftest reads info from "/proc/net/nf_conntrack". Thank you Pedro for
> having helped me fixing these issues [2].

It looks like this series is marked as "Changes Requested" on Patchwork
[1] but I think that's a mistake. There was one discussion on-going on
the first patch but it looks like the proposed version is OK.

I didn't see any instructions to pw-bot and nothing on the website [2].

Do you prefer if I re-send it?

Cheers,
Matt

[1]
https://patchwork.kernel.org/project/netdevbpf/list/?series=765455&state=*
[2] https://patchwork.hopto.org/pw-bot.html
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net

2023-07-19 00:20:47

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

On Thu, 13 Jul 2023 23:16:43 +0200 you wrote:
> When looking for something else in LKFT reports [1], I noticed that the
> TC selftest ended with a timeout error:
>
> not ok 1 selftests: tc-testing: tdc.sh # TIMEOUT 45 seconds
>
> I also noticed most of the tests were skipped because the "teardown
> stage" did not complete successfully. It was due to missing kconfig.
>
> [...]

Here is the summary with links:
- [net,1/3] selftests: tc: set timeout to 15 minutes
https://git.kernel.org/netdev/net/c/fda05798c22a
- [net,2/3] selftests: tc: add 'ct' action kconfig dep
https://git.kernel.org/netdev/net/c/719b4774a8cb
- [net,3/3] selftests: tc: add ConnTrack procfs kconfig
https://git.kernel.org/netdev/net/c/031c99e71fed

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html