netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* tdc errors
@ 2022-01-20 17:22 Victor Nogueira
  2022-01-20 19:33 ` Jamal Hadi Salim
  0 siblings, 1 reply; 8+ messages in thread
From: Victor Nogueira @ 2022-01-20 17:22 UTC (permalink / raw)
  To: baowen.zheng
  Cc: simon.horman, netdev, Davide Caratti, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern

Hi,

When running these 2 tdc tests:

7d64 Add police action with skip_hw option
3329 Validate flags of the matchall filter with skip_sw and
police action with skip_hw
I get this error:

Bad action type skip_hw
Usage: ... gact <ACTION> [RAND] [INDEX]
Where: ACTION := reclassify | drop | continue | pass | pipe |
goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
RAND := random <RANDTYPE> <ACTION> <VAL>
RANDTYPE := netrand | determ
VAL : = value not exceeding 10000
JUMP_COUNT := Absolute jump from start of action list
INDEX := index value used

I'm building the kernel on net-next.

I'm compiling the latest iproute2 version.

It seems like the problem is that support is lacking for skip_hw
in police action in iproute2.

This is the command that fails in test 7d64:
tc actions add action police rate 1kbit burst 10k index 100 skip_hw

To run this particular test do:
./tdc.py -e 7d64

cheers,
Victor

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

* Re: tdc errors
  2022-01-20 17:22 tdc errors Victor Nogueira
@ 2022-01-20 19:33 ` Jamal Hadi Salim
  2022-01-21  7:13   ` Baowen Zheng
  2022-01-21  9:36   ` Davide Caratti
  0 siblings, 2 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2022-01-20 19:33 UTC (permalink / raw)
  To: Victor Nogueira, baowen.zheng
  Cc: simon.horman, netdev, Davide Caratti, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern

On 2022-01-20 12:22, Victor Nogueira wrote:
> Hi,
> 
> When running these 2 tdc tests:
> 
> 7d64 Add police action with skip_hw option
> 3329 Validate flags of the matchall filter with skip_sw and
> police action with skip_hw
> I get this error:
> 
> Bad action type skip_hw
> Usage: ... gact <ACTION> [RAND] [INDEX]
> Where: ACTION := reclassify | drop | continue | pass | pipe |
> goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
> RAND := random <RANDTYPE> <ACTION> <VAL>
> RANDTYPE := netrand | determ
> VAL : = value not exceeding 10000
> JUMP_COUNT := Absolute jump from start of action list
> INDEX := index value used
> 
> I'm building the kernel on net-next.
> 
> I'm compiling the latest iproute2 version.
> 
> It seems like the problem is that support is lacking for skip_hw
> in police action in iproute2.
> 


So... How is the robot not reporting this as a regression?
Davide? Basically kernel has the feature but code is missing
in both iproute2 and iproute2-next..

cheers,
jamal

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

* RE: tdc errors
  2022-01-20 19:33 ` Jamal Hadi Salim
@ 2022-01-21  7:13   ` Baowen Zheng
  2022-01-21  9:36   ` Davide Caratti
  1 sibling, 0 replies; 8+ messages in thread
From: Baowen Zheng @ 2022-01-21  7:13 UTC (permalink / raw)
  To: Jamal Hadi Salim, Victor Nogueira
  Cc: Simon Horman, netdev, Davide Caratti, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern

Thanks for bring this to us, we will upstream the single action offload support to the iproute2.
On Friday, January 21, 2022 3:34 AM, Jamal wrote:
>On 2022-01-20 12:22, Victor Nogueira wrote:
>> Hi,
>>
>> When running these 2 tdc tests:
>>
>> 7d64 Add police action with skip_hw option
>> 3329 Validate flags of the matchall filter with skip_sw and police
>> action with skip_hw I get this error:
>>
>> Bad action type skip_hw
>> Usage: ... gact <ACTION> [RAND] [INDEX]
>> Where: ACTION := reclassify | drop | continue | pass | pipe | goto
>> chain <CHAIN_INDEX> | jump <JUMP_COUNT> RAND := random <RANDTYPE>
>> <ACTION> <VAL> RANDTYPE := netrand | determ VAL : = value not
>> exceeding 10000 JUMP_COUNT := Absolute jump from start of action list
>> INDEX := index value used
>>
>> I'm building the kernel on net-next.
>>
>> I'm compiling the latest iproute2 version.
>>
>> It seems like the problem is that support is lacking for skip_hw in
>> police action in iproute2.
>>
>
>
>So... How is the robot not reporting this as a regression?
>Davide? Basically kernel has the feature but code is missing in both iproute2
>and iproute2-next..
>
>cheers,
>jamal

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

* Re: tdc errors
  2022-01-20 19:33 ` Jamal Hadi Salim
  2022-01-21  7:13   ` Baowen Zheng
@ 2022-01-21  9:36   ` Davide Caratti
  2022-01-21 14:11     ` Jamal Hadi Salim
  1 sibling, 1 reply; 8+ messages in thread
From: Davide Caratti @ 2022-01-21  9:36 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Victor Nogueira, Baowen Zheng, Simon Horman,
	Linux Kernel Network Developers, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern

On Thu, Jan 20, 2022 at 8:34 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2022-01-20 12:22, Victor Nogueira wrote:
> > Hi,
> >
> > When running these 2 tdc tests:
> >
> > 7d64 Add police action with skip_hw option
> > 3329 Validate flags of the matchall filter with skip_sw and
> > police action with skip_hw
> > I get this error:
> >
> > Bad action type skip_hw
> > Usage: ... gact <ACTION> [RAND] [INDEX]
> > Where: ACTION := reclassify | drop | continue | pass | pipe |
> > goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
> > RAND := random <RANDTYPE> <ACTION> <VAL>
> > RANDTYPE := netrand | determ
> > VAL : = value not exceeding 10000
> > JUMP_COUNT := Absolute jump from start of action list
> > INDEX := index value used
> >
> > I'm building the kernel on net-next.
> >
> > I'm compiling the latest iproute2 version.
> >
> > It seems like the problem is that support is lacking for skip_hw
> > in police action in iproute2.
> >
>
>
> So... How is the robot not reporting this as a regression?
> Davide? Basically kernel has the feature but code is missing
> in both iproute2 and iproute2-next..

my guess (but it's only a guess) is that also the tc-testing code is
less recent than the code of the kernel under test, so it does not not
contain new items (like 7d64).

But even if we had the latest net-next test code and the latest
net-next kernel under test, we would anyway see unstable test results,
because of the gap with iproute2 code.  My suggestion is to push new
tdc items (that require iproute2 bits, or some change to the kernel
configuration in the build environment) using 'skip: yes' in the JSON
(see [1]), and enable them only when we are sure that all the code
propagated at least to stable trees.

wdyt?

thanks!
-- 
davide

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=255c1c7279abf991


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

* Re: tdc errors
  2022-01-21  9:36   ` Davide Caratti
@ 2022-01-21 14:11     ` Jamal Hadi Salim
  2022-01-21 16:27       ` Shuah Khan
  0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2022-01-21 14:11 UTC (permalink / raw)
  To: Davide Caratti
  Cc: Victor Nogueira, Baowen Zheng, Simon Horman,
	Linux Kernel Network Developers, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern, shuah

On 2022-01-21 04:36, Davide Caratti wrote:
> On Thu, Jan 20, 2022 at 8:34 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:

[..]>>
>> So... How is the robot not reporting this as a regression?
>> Davide? Basically kernel has the feature but code is missing
>> in both iproute2 and iproute2-next..
> 
> my guess (but it's only a guess) is that also the tc-testing code is
> less recent than the code of the kernel under test, so it does not not
> contain new items (like 7d64).

Which kernel(s) + iproute2 version does the bot test?
In this case, the tdc test is in the kernel already..
So in my opinion shouldve just ran and failed and a report
sent indicating failure. Where do the reports go?

+Cc Shuah.

> But even if we had the latest net-next test code and the latest
> net-next kernel under test, we would anyway see unstable test results,
> because of the gap with iproute2 code.  My suggestion is to push new
> tdc items (that require iproute2 bits, or some change to the kernel
> configuration in the build environment) using 'skip: yes' in the JSON
> (see [1]), and enable them only when we are sure that all the code
> propagated at least to stable trees.
> 
> wdyt?
> 

That's better than current status quo but: still has  human dependency
IMO. If we can remove human dependency the bot can do a better job.
Example:
One thing that is often a cause of failures in tdc is kernel config.
A lot of tests fail because the kernel doesnt have the config compiled
in.
Today, we work around that by providing a kernel config file in tdc.
Unfortunately we dont use that config file for anything
meaningful other than to tell the human what kernel options
to ensure are compiled in before running the tests (manually).
Infact the user has to inspect the config file first.

One idea that will help in automation is as follows:
Could we add a "environment dependency" check that will ensure
for a given test the right versions of things and configs exist?
Example check if CONFIG_NET_SCH_ETS is available in the running
kernel before executing "ets tests" or we have iproute2 version
 >= blah before running the policer test with skip_sw feature etc
I think some of this can be done via the pre-test-suite but we may
need granularity at per-test level.

cheers,
jamal

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

* Re: tdc errors
  2022-01-21 14:11     ` Jamal Hadi Salim
@ 2022-01-21 16:27       ` Shuah Khan
  2022-01-31 12:46         ` Jamal Hadi Salim
  0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2022-01-21 16:27 UTC (permalink / raw)
  To: Jamal Hadi Salim, Davide Caratti
  Cc: Victor Nogueira, Baowen Zheng, Simon Horman,
	Linux Kernel Network Developers, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern, shuah, Shuah Khan

On 1/21/22 7:11 AM, Jamal Hadi Salim wrote:
> On 2022-01-21 04:36, Davide Caratti wrote:
>> On Thu, Jan 20, 2022 at 8:34 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> 
> [..]>>
>>> So... How is the robot not reporting this as a regression?
>>> Davide? Basically kernel has the feature but code is missing
>>> in both iproute2 and iproute2-next..
>>
>> my guess (but it's only a guess) is that also the tc-testing code is
>> less recent than the code of the kernel under test, so it does not not
>> contain new items (like 7d64).
> 
> Which kernel(s) + iproute2 version does the bot test?
> In this case, the tdc test is in the kernel already..
> So in my opinion shouldve just ran and failed and a report
> sent indicating failure. Where do the reports go?
> 
> +Cc Shuah.
> 
>> But even if we had the latest net-next test code and the latest
>> net-next kernel under test, we would anyway see unstable test results,
>> because of the gap with iproute2 code.  My suggestion is to push new
>> tdc items (that require iproute2 bits, or some change to the kernel
>> configuration in the build environment) using 'skip: yes' in the JSON
>> (see [1]), and enable them only when we are sure that all the code
>> propagated at least to stable trees.
>>
>> wdyt?
>>
> 
> That's better than current status quo but: still has  human dependency
> IMO. If we can remove human dependency the bot can do a better job.
> Example:
> One thing that is often a cause of failures in tdc is kernel config.
> A lot of tests fail because the kernel doesnt have the config compiled
> in.
> Today, we work around that by providing a kernel config file in tdc.
> Unfortunately we dont use that config file for anything
> meaningful other than to tell the human what kernel options
> to ensure are compiled in before running the tests (manually).
> Infact the user has to inspect the config file first.
> 
> One idea that will help in automation is as follows:
> Could we add a "environment dependency" check that will ensure
> for a given test the right versions of things and configs exist?
> Example check if CONFIG_NET_SCH_ETS is available in the running
> kernel before executing "ets tests" or we have iproute2 version
>  >= blah before running the policer test with skip_sw feature etc
> I think some of this can be done via the pre-test-suite but we may
> need granularity at per-test level.
> 

Several tests check for config support for their dependencies in their
test code - I don't see any of those in tc-testing. Individual tests
are supposed to check for not just the config dependencies, but also
any feature dependency e.g syscall/ioctl.

Couple of way to fix this problem for tc-testing - enhance the test to
check for dependencies and skip with a clear message on why test is
skipped.

A second option is enhancing the tools/testing/selftests/kselftest_deps.sh
script that checks for build depedencies. This tool can be enhanced easily
to check for run-time dependencies and use this in your automation.

Usage: ./kselftest_deps.sh -[p] <compiler> [test_name]

	kselftest_deps.sh [-p] gcc
	kselftest_deps.sh [-p] gcc vm
	kselftest_deps.sh [-p] aarch64-linux-gnu-gcc
	kselftest_deps.sh [-p] aarch64-linux-gnu-gcc vm

- Should be run in selftests directory in the kernel repo.
- Checks if Kselftests can be built/cross-built on a system.
- Parses all test/sub-test Makefile to find library dependencies.
- Runs compile test on a trivial C file with LDLIBS specified
   in the test Makefiles to identify missing library dependencies.
- Prints suggested target list for a system filtering out tests
   failed the build dependency check from the TARGETS in Selftests
   main Makefile when optional -p is specified.
- Prints pass/fail dependency check for each tests/sub-test.
- Prints pass/fail targets and libraries.
- Default: runs dependency checks on all tests.
- Optional test name can be specified to check dependencies for it.

thanks,
-- Shuah



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

* Re: tdc errors
  2022-01-21 16:27       ` Shuah Khan
@ 2022-01-31 12:46         ` Jamal Hadi Salim
  2022-01-31 15:40           ` Shuah Khan
  0 siblings, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2022-01-31 12:46 UTC (permalink / raw)
  To: Shuah Khan, Davide Caratti
  Cc: Victor Nogueira, Baowen Zheng, Simon Horman,
	Linux Kernel Network Developers, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern, shuah

On 2022-01-21 11:27, Shuah Khan wrote:
> On 1/21/22 7:11 AM, Jamal Hadi Salim wrote:
>> On 2022-01-21 04:36, Davide Caratti wrote:
>>> On Thu, Jan 20, 2022 at 8:34 PM Jamal Hadi Salim <jhs@mojatatu.com> 
>>> wrote:

[..]


> 
> Several tests check for config support for their dependencies in their
> test code - I don't see any of those in tc-testing. Individual tests
> are supposed to check for not just the config dependencies, but also
> any feature dependency e.g syscall/ioctl.
> 
> Couple of way to fix this problem for tc-testing - enhance the test to
> check for dependencies and skip with a clear message on why test is
> skipped.
> 
> A second option is enhancing the tools/testing/selftests/kselftest_deps.sh
> script that checks for build depedencies. This tool can be enhanced easily
> to check for run-time dependencies and use this in your automation.
> 
> Usage: ./kselftest_deps.sh -[p] <compiler> [test_name]
> 
>      kselftest_deps.sh [-p] gcc
>      kselftest_deps.sh [-p] gcc vm
>      kselftest_deps.sh [-p] aarch64-linux-gnu-gcc
>      kselftest_deps.sh [-p] aarch64-linux-gnu-gcc vm
> 
> - Should be run in selftests directory in the kernel repo.
> - Checks if Kselftests can be built/cross-built on a system.
> - Parses all test/sub-test Makefile to find library dependencies.
> - Runs compile test on a trivial C file with LDLIBS specified
>    in the test Makefiles to identify missing library dependencies.
> - Prints suggested target list for a system filtering out tests
>    failed the build dependency check from the TARGETS in Selftests
>    main Makefile when optional -p is specified.
> - Prints pass/fail dependency check for each tests/sub-test.
> - Prints pass/fail targets and libraries.
> - Default: runs dependency checks on all tests.
> - Optional test name can be specified to check dependencies for it.
> 

Thanks Shuah. We'll look at this approach.
Question: How do we get reports when the bot finds a regression?


cheers,
jamal

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

* Re: tdc errors
  2022-01-31 12:46         ` Jamal Hadi Salim
@ 2022-01-31 15:40           ` Shuah Khan
  0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2022-01-31 15:40 UTC (permalink / raw)
  To: Jamal Hadi Salim, Davide Caratti
  Cc: Victor Nogueira, Baowen Zheng, Simon Horman,
	Linux Kernel Network Developers, Marcelo Ricardo Leitner,
	Vlad Buslov, David Ahern, shuah, Shuah Khan

On 1/31/22 5:46 AM, Jamal Hadi Salim wrote:
> On 2022-01-21 11:27, Shuah Khan wrote:
>> On 1/21/22 7:11 AM, Jamal Hadi Salim wrote:
>>> On 2022-01-21 04:36, Davide Caratti wrote:
>>>> On Thu, Jan 20, 2022 at 8:34 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> 
> [..]
> 
> 
>>
>> Several tests check for config support for their dependencies in their
>> test code - I don't see any of those in tc-testing. Individual tests
>> are supposed to check for not just the config dependencies, but also
>> any feature dependency e.g syscall/ioctl.
>>
>> Couple of way to fix this problem for tc-testing - enhance the test to
>> check for dependencies and skip with a clear message on why test is
>> skipped.
>>
>> A second option is enhancing the tools/testing/selftests/kselftest_deps.sh
>> script that checks for build depedencies. This tool can be enhanced easily
>> to check for run-time dependencies and use this in your automation.
>>
>> Usage: ./kselftest_deps.sh -[p] <compiler> [test_name]
>>
>>      kselftest_deps.sh [-p] gcc
>>      kselftest_deps.sh [-p] gcc vm
>>      kselftest_deps.sh [-p] aarch64-linux-gnu-gcc
>>      kselftest_deps.sh [-p] aarch64-linux-gnu-gcc vm
>>
>> - Should be run in selftests directory in the kernel repo.
>> - Checks if Kselftests can be built/cross-built on a system.
>> - Parses all test/sub-test Makefile to find library dependencies.
>> - Runs compile test on a trivial C file with LDLIBS specified
>>    in the test Makefiles to identify missing library dependencies.
>> - Prints suggested target list for a system filtering out tests
>>    failed the build dependency check from the TARGETS in Selftests
>>    main Makefile when optional -p is specified.
>> - Prints pass/fail dependency check for each tests/sub-test.
>> - Prints pass/fail targets and libraries.
>> - Default: runs dependency checks on all tests.
>> - Optional test name can be specified to check dependencies for it.
>>
> 
> Thanks Shuah. We'll look at this approach.
> Question: How do we get reports when the bot finds a regression?
> 
> 

You might be able to subscribe to test ring notification or subscribe
to Linux stable mailing list.

thanks,
-- Shuah

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

end of thread, other threads:[~2022-01-31 15:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 17:22 tdc errors Victor Nogueira
2022-01-20 19:33 ` Jamal Hadi Salim
2022-01-21  7:13   ` Baowen Zheng
2022-01-21  9:36   ` Davide Caratti
2022-01-21 14:11     ` Jamal Hadi Salim
2022-01-21 16:27       ` Shuah Khan
2022-01-31 12:46         ` Jamal Hadi Salim
2022-01-31 15:40           ` Shuah Khan

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).