| Message ID | 20190405122024.2073-1-nicolas.dichtel@6wind.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] selftests: add a tc matchall test case | expand |
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Fri, 5 Apr 2019 14:20:24 +0200 > This is a follow up of the commit 0db6f8befc32 ("net/sched: fix ->get > helper of the matchall cls"). > > To test it: > $ cd tools/testing/selftests/tc-testing > $ ln -s ../plugin-lib/nsPlugin.py plugins/20-nsPlugin.py > $ ./tdc.py -n -e 2638 > > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Applied, thanks Nicolas. > I'm not sure to understand the goal of this series: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=45b3a3762721 > > But after it, tc selftests are not intuitive anymore. When naively running them > (./tdy.py), all fw filter tests fail. It's hard to understand that the problem > is that they require the 'nsPlugin' plugin to be enabled (by adding a symlink > *and* providing the '-n' option to tdc.py). > > Brenda, why putting the netns code into a plugin that is disabled by default, > knowing that this code is mandatory to run the test? Brenda, please respond.
On Sun, Apr 7, 2019 at 10:32 PM David Miller <davem@davemloft.net> wrote: > > I'm not sure to understand the goal of this series: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=45b3a3762721 > > > > But after it, tc selftests are not intuitive anymore. When naively running them > > (./tdy.py), all fw filter tests fail. It's hard to understand that the problem > > is that they require the 'nsPlugin' plugin to be enabled (by adding a symlink > > *and* providing the '-n' option to tdc.py). > > > > Brenda, why putting the netns code into a plugin that is disabled by default, > > knowing that this code is mandatory to run the test? > > Brenda, please respond. This would be a documentation issue - the usage of nsPlugin is not covered in the README. I'll submit a patch by end of the day tomorrow to address it. The namespace code isn't mandatory, strictly speaking. It's not required at all when running the action tests, only with the filter tests as those need a specific device to be attached to. The actual problem is that the nsPlugin creates a veth pair using the names found in the config file (DEV0/DEV1) - but if you aren't using the plugin, then a port won't be created. The intention is that you'd create a port manually before starting the tests.
From: Lucas Bates <lucasb@mojatatu.com> Date: Mon, 8 Apr 2019 18:24:08 -0400 > On Sun, Apr 7, 2019 at 10:32 PM David Miller <davem@davemloft.net> wrote: >> > I'm not sure to understand the goal of this series: >> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=45b3a3762721 >> > >> > But after it, tc selftests are not intuitive anymore. When naively running them >> > (./tdy.py), all fw filter tests fail. It's hard to understand that the problem >> > is that they require the 'nsPlugin' plugin to be enabled (by adding a symlink >> > *and* providing the '-n' option to tdc.py). >> > >> > Brenda, why putting the netns code into a plugin that is disabled by default, >> > knowing that this code is mandatory to run the test? >> >> Brenda, please respond. > > This would be a documentation issue - the usage of nsPlugin is not > covered in the README. I'll submit a patch by end of the day tomorrow > to address it. > > The namespace code isn't mandatory, strictly speaking. It's not > required at all when running the action tests, only with the filter > tests as those need a specific device to be attached to. > > The actual problem is that the nsPlugin creates a veth pair using the > names found in the config file (DEV0/DEV1) - but if you aren't using > the plugin, then a port won't be created. The intention is that you'd > create a port manually before starting the tests. That's poor behavior. If I just want to run all the tests, I should be able to just execute the script with no special arguments. This setup sucks.
Le 09/04/2019 à 00:45, David Miller a écrit : > From: Lucas Bates <lucasb@mojatatu.com> [snip] >> This would be a documentation issue - the usage of nsPlugin is not >> covered in the README. I'll submit a patch by end of the day tomorrow >> to address it. >> >> The namespace code isn't mandatory, strictly speaking. It's not >> required at all when running the action tests, only with the filter >> tests as those need a specific device to be attached to. >> >> The actual problem is that the nsPlugin creates a veth pair using the >> names found in the config file (DEV0/DEV1) - but if you aren't using >> the plugin, then a port won't be created. The intention is that you'd >> create a port manually before starting the tests. > > That's poor behavior. > > If I just want to run all the tests, I should be able to just execute > the script with no special arguments. +1 If we want that people use those kind of tests, it must work without any operations. Regards, Nicolas
diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json index 99a5ffca1088..2d096b2abf2c 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json +++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json @@ -18,6 +18,26 @@ "$TC qdisc del dev $DEV1 ingress" ] }, + { + "id": "2638", + "name": "Add matchall and try to get it", + "category": [ + "filter", + "matchall" + ], + "setup": [ + "$TC qdisc add dev $DEV1 clsact", + "$TC filter add dev $DEV1 protocol all pref 1 ingress handle 0x1234 matchall action ok" + ], + "cmdUnderTest": "$TC filter get dev $DEV1 protocol all pref 1 ingress handle 0x1234 matchall", + "expExitCode": "0", + "verifyCmd": "$TC filter show dev $DEV1 ingress", + "matchPattern": "filter protocol all pref 1 matchall chain 0 handle 0x1234", + "matchCount": "1", + "teardown": [ + "$TC qdisc del dev $DEV1 clsact" + ] + }, { "id": "d052", "name": "Add 1M filters with the same action",
This is a follow up of the commit 0db6f8befc32 ("net/sched: fix ->get helper of the matchall cls"). To test it: $ cd tools/testing/selftests/tc-testing $ ln -s ../plugin-lib/nsPlugin.py plugins/20-nsPlugin.py $ ./tdc.py -n -e 2638 Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- I'm not sure to understand the goal of this series: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=45b3a3762721 But after it, tc selftests are not intuitive anymore. When naively running them (./tdy.py), all fw filter tests fail. It's hard to understand that the problem is that they require the 'nsPlugin' plugin to be enabled (by adding a symlink *and* providing the '-n' option to tdc.py). Brenda, why putting the netns code into a plugin that is disabled by default, knowing that this code is mandatory to run the test? Regards, Nicolas .../tc-testing/tc-tests/filters/tests.json | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+)