netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig
@ 2023-07-13 21:16 Matthieu Baerts
  2023-07-13 21:16 ` [PATCH net 1/3] selftests: tc: set timeout to 15 minutes Matthieu Baerts
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-13 21:16 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Shuah Khan, Kees Cook,
	David S. Miller, Paul Blakey, Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Matthieu Baerts, stable

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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
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 <matthieu.baerts@tessares.net>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net 1/3] selftests: tc: set timeout to 15 minutes
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
@ 2023-07-13 21:16 ` Matthieu Baerts
  2023-07-14  2:25   ` shaozhengchao
  2023-07-13 21:16 ` [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep Matthieu Baerts
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-13 21:16 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Shuah Khan, Kees Cook,
	David S. Miller, Paul Blakey, Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Matthieu Baerts, stable

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: stable@vger.kernel.org
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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
  2023-07-13 21:16 ` [PATCH net 1/3] selftests: tc: set timeout to 15 minutes Matthieu Baerts
@ 2023-07-13 21:16 ` Matthieu Baerts
  2023-07-14  2:56   ` shaozhengchao
  2023-07-13 21:16 ` [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig Matthieu Baerts
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-13 21:16 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Shuah Khan, Kees Cook,
	David S. Miller, Paul Blakey, Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Matthieu Baerts, stable

When looking for something else in LKFT reports [1], I noticed most of
the tests were skipped because the "teardown stage" did not complete
successfully.

Pedro found out this is due to the fact CONFIG_NF_FLOW_TABLE is required
but not listed in the 'config' file. Adding it to the list fixes the
issues on LKFT side. CONFIG_NET_ACT_CT is now set to 'm' in the final
kconfig.

Fixes: c34b961a2492 ("net/sched: act_ct: Create nf flow table per zone")
Cc: stable@vger.kernel.org
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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/tc-testing/config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
index 6e73b09c20c8..d1ad29040c02 100644
--- a/tools/testing/selftests/tc-testing/config
+++ b/tools/testing/selftests/tc-testing/config
@@ -5,6 +5,7 @@ CONFIG_NF_CONNTRACK=m
 CONFIG_NF_CONNTRACK_MARK=y
 CONFIG_NF_CONNTRACK_ZONES=y
 CONFIG_NF_CONNTRACK_LABELS=y
+CONFIG_NF_FLOW_TABLE=m
 CONFIG_NF_NAT=m
 CONFIG_NETFILTER_XT_TARGET_LOG=m
 

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
  2023-07-13 21:16 ` [PATCH net 1/3] selftests: tc: set timeout to 15 minutes Matthieu Baerts
  2023-07-13 21:16 ` [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep Matthieu Baerts
@ 2023-07-13 21:16 ` Matthieu Baerts
  2023-07-14  3:25   ` shaozhengchao
  2023-07-13 22:41 ` [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Jamal Hadi Salim
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-13 21:16 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Shuah Khan, Kees Cook,
	David S. Miller, Paul Blakey, Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Matthieu Baerts, stable

When looking at the TC selftest reports, I noticed one test was failing
because /proc/net/nf_conntrack was not available.

  not ok 373 3992 - Add ct action triggering DNAT tuple conflict
  	Could not match regex pattern. Verify command output:
  cat: /proc/net/nf_conntrack: No such file or directory

It is only available if NF_CONNTRACK_PROCFS kconfig is set. So the issue
can be fixed simply by adding it to the list of required kconfig.

Fixes: e46905641316 ("tc-testing: add test for ct DNAT tuple collision")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [1]
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/tc-testing/config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
index d1ad29040c02..71706197ba0f 100644
--- a/tools/testing/selftests/tc-testing/config
+++ b/tools/testing/selftests/tc-testing/config
@@ -5,6 +5,7 @@ CONFIG_NF_CONNTRACK=m
 CONFIG_NF_CONNTRACK_MARK=y
 CONFIG_NF_CONNTRACK_ZONES=y
 CONFIG_NF_CONNTRACK_LABELS=y
+CONFIG_NF_CONNTRACK_PROCFS=y
 CONFIG_NF_FLOW_TABLE=m
 CONFIG_NF_NAT=m
 CONFIG_NETFILTER_XT_TARGET_LOG=m

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
                   ` (2 preceding siblings ...)
  2023-07-13 21:16 ` [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig Matthieu Baerts
@ 2023-07-13 22:41 ` Jamal Hadi Salim
  2023-07-18  8:46 ` Matthieu Baerts
  2023-07-19  0:00 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 13+ messages in thread
From: Jamal Hadi Salim @ 2023-07-13 22:41 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Cong Wang, Jiri Pirko, Shuah Khan, Kees Cook, David S. Miller,
	Paul Blakey, Marcelo Ricardo Leitner, mptcp, Pedro Tammela,
	Shuah Khan, linux-kernel, netdev, linux-kselftest, stable

On Thu, Jul 13, 2023 at 5:17 PM Matthieu Baerts
<matthieu.baerts@tessares.net> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>

For the patchset:
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

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 <matthieu.baerts@tessares.net>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes
  2023-07-13 21:16 ` [PATCH net 1/3] selftests: tc: set timeout to 15 minutes Matthieu Baerts
@ 2023-07-14  2:25   ` shaozhengchao
  2023-07-14 17:49     ` Pedro Tammela
  2023-07-17  8:32     ` Matthieu Baerts
  0 siblings, 2 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-14  2:25 UTC (permalink / raw)
  To: Matthieu Baerts, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest, stable



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: stable@vger.kernel.org
> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
> Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>   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.

Zhengchao Shao

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep
  2023-07-13 21:16 ` [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep Matthieu Baerts
@ 2023-07-14  2:56   ` shaozhengchao
  0 siblings, 0 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-14  2:56 UTC (permalink / raw)
  To: Matthieu Baerts, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest, stable



On 2023/7/14 5:16, Matthieu Baerts wrote:
> When looking for something else in LKFT reports [1], I noticed most of
> the tests were skipped because the "teardown stage" did not complete
> successfully.
> 
> Pedro found out this is due to the fact CONFIG_NF_FLOW_TABLE is required
> but not listed in the 'config' file. Adding it to the list fixes the
> issues on LKFT side. CONFIG_NET_ACT_CT is now set to 'm' in the final
> kconfig.
> 
> Fixes: c34b961a2492 ("net/sched: act_ct: Create nf flow table per zone")
> Cc: stable@vger.kernel.org
> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
> Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>   tools/testing/selftests/tc-testing/config | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
> index 6e73b09c20c8..d1ad29040c02 100644
> --- a/tools/testing/selftests/tc-testing/config
> +++ b/tools/testing/selftests/tc-testing/config
> @@ -5,6 +5,7 @@ CONFIG_NF_CONNTRACK=m
>   CONFIG_NF_CONNTRACK_MARK=y
>   CONFIG_NF_CONNTRACK_ZONES=y
>   CONFIG_NF_CONNTRACK_LABELS=y
> +CONFIG_NF_FLOW_TABLE=m
>   CONFIG_NF_NAT=m
>   CONFIG_NETFILTER_XT_TARGET_LOG=m
>   
> 

Tested-by: Zhengchao Shao <shaozhengchao@huawei.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig
  2023-07-13 21:16 ` [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig Matthieu Baerts
@ 2023-07-14  3:25   ` shaozhengchao
  0 siblings, 0 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-14  3:25 UTC (permalink / raw)
  To: Matthieu Baerts, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest, stable



On 2023/7/14 5:16, Matthieu Baerts wrote:
> When looking at the TC selftest reports, I noticed one test was failing
> because /proc/net/nf_conntrack was not available.
> 
>    not ok 373 3992 - Add ct action triggering DNAT tuple conflict
>    	Could not match regex pattern. Verify command output:
>    cat: /proc/net/nf_conntrack: No such file or directory
> 
> It is only available if NF_CONNTRACK_PROCFS kconfig is set. So the issue
> can be fixed simply by adding it to the list of required kconfig.
> 
> Fixes: e46905641316 ("tc-testing: add test for ct DNAT tuple collision")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/netdev/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [1]
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>   tools/testing/selftests/tc-testing/config | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
> index d1ad29040c02..71706197ba0f 100644
> --- a/tools/testing/selftests/tc-testing/config
> +++ b/tools/testing/selftests/tc-testing/config
> @@ -5,6 +5,7 @@ CONFIG_NF_CONNTRACK=m
>   CONFIG_NF_CONNTRACK_MARK=y
>   CONFIG_NF_CONNTRACK_ZONES=y
>   CONFIG_NF_CONNTRACK_LABELS=y
> +CONFIG_NF_CONNTRACK_PROCFS=y
>   CONFIG_NF_FLOW_TABLE=m
>   CONFIG_NF_NAT=m
>   CONFIG_NETFILTER_XT_TARGET_LOG=m
> 

Tested-by: Zhengchao Shao <shaozhengchao@huawei.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes
  2023-07-14  2:25   ` shaozhengchao
@ 2023-07-14 17:49     ` Pedro Tammela
  2023-07-17  8:32     ` Matthieu Baerts
  1 sibling, 0 replies; 13+ messages in thread
From: Pedro Tammela @ 2023-07-14 17:49 UTC (permalink / raw)
  To: shaozhengchao, Matthieu Baerts, Jamal Hadi Salim, Cong Wang,
	Jiri Pirko, Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Shuah Khan, linux-kernel, netdev, linux-kselftest, stable

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: stable@vger.kernel.org
>> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
>> Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>   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.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes
  2023-07-14  2:25   ` shaozhengchao
  2023-07-14 17:49     ` Pedro Tammela
@ 2023-07-17  8:32     ` Matthieu Baerts
  2023-07-18  1:43       ` shaozhengchao
  1 sibling, 1 reply; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-17  8:32 UTC (permalink / raw)
  To: shaozhengchao, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest, stable

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: stable@vger.kernel.org
>> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
>> Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>   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
www.tessares.net

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 1/3] selftests: tc: set timeout to 15 minutes
  2023-07-17  8:32     ` Matthieu Baerts
@ 2023-07-18  1:43       ` shaozhengchao
  0 siblings, 0 replies; 13+ messages in thread
From: shaozhengchao @ 2023-07-18  1:43 UTC (permalink / raw)
  To: Matthieu Baerts, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Shuah Khan, Kees Cook, David S. Miller, Paul Blakey,
	Marcelo Ricardo Leitner, mptcp
  Cc: Pedro Tammela, Shuah Khan, linux-kernel, netdev, linux-kselftest, stable



On 2023/7/17 16:32, Matthieu Baerts wrote:
> 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: stable@vger.kernel.org
>>> 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/0e061d4a-9a23-9f58-3b35-d8919de332d7@tessares.net/T/ [2]
>>> Suggested-by: Pedro Tammela <pctammela@mojatatu.com>
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>    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!
> 
Hi Matthieu:
> 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?
> 
15 minutes is enough for qdisc and actions. Thanks.

> 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


Reviewed-by: Zhengchao Shao <shaozhengchao@huawei.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
                   ` (3 preceding siblings ...)
  2023-07-13 22:41 ` [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Jamal Hadi Salim
@ 2023-07-18  8:46 ` Matthieu Baerts
  2023-07-19  0:00 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 13+ messages in thread
From: Matthieu Baerts @ 2023-07-18  8:46 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni
  Cc: Jamal Hadi Salim, Marcelo Ricardo Leitner, Paul Blakey,
	Kees Cook, Shuah Khan, Jiri Pirko, Cong Wang, Pedro Tammela,
	Shuah Khan, linux-kernel, netdev, linux-kselftest, stable

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
www.tessares.net

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig
  2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
                   ` (4 preceding siblings ...)
  2023-07-18  8:46 ` Matthieu Baerts
@ 2023-07-19  0:00 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-19  0:00 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: jhs, xiyou.wangcong, jiri, shuah, keescook, davem, paulb,
	marcelo.leitner, mptcp, pctammela, skhan, linux-kernel, netdev,
	linux-kselftest, stable

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-07-19  0:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-13 21:16 [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Matthieu Baerts
2023-07-13 21:16 ` [PATCH net 1/3] selftests: tc: set timeout to 15 minutes Matthieu Baerts
2023-07-14  2:25   ` shaozhengchao
2023-07-14 17:49     ` Pedro Tammela
2023-07-17  8:32     ` Matthieu Baerts
2023-07-18  1:43       ` shaozhengchao
2023-07-13 21:16 ` [PATCH net 2/3] selftests: tc: add 'ct' action kconfig dep Matthieu Baerts
2023-07-14  2:56   ` shaozhengchao
2023-07-13 21:16 ` [PATCH net 3/3] selftests: tc: add ConnTrack procfs kconfig Matthieu Baerts
2023-07-14  3:25   ` shaozhengchao
2023-07-13 22:41 ` [PATCH net 0/3] selftests: tc: increase timeout and add missing kconfig Jamal Hadi Salim
2023-07-18  8:46 ` Matthieu Baerts
2023-07-19  0:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).