* [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
@ 2019-06-05 21:08 Lucas Bates
2019-06-07 12:18 ` Davide Caratti
2019-06-14 9:37 ` Nicolas Dichtel
0 siblings, 2 replies; 8+ messages in thread
From: Lucas Bates @ 2019-06-05 21:08 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, davem, jhs, kernel, xiyou.wangcong, jiri,
mleitner, vladbu, dcaratti, Lucas Bates
Apologies for the delay in getting this out. I've been busy
with other things and this change was a little trickier than
I expected.
This patch restores the original behaviour for tdc prior to the
introduction of the plugin system, where the network namespace
functionality was split from the main script.
It introduces the concept of required plugins for testcases,
and will automatically load any plugin that isn't already
enabled when said plugin is required by even one testcase.
Additionally, the -n option for the nsPlugin is deprecated
so the default action is to make use of the namespaces.
Instead, we introduce -N to not use them, but still create
the veth pair.
Comments welcome!
---
tools/testing/selftests/tc-testing/README | 22 ++-
.../selftests/tc-testing/plugin-lib/nsPlugin.py | 26 +++-
.../selftests/tc-testing/tc-tests/filters/fw.json | 162 +++++++++++++++++++++
.../tc-testing/tc-tests/filters/tests.json | 12 ++
tools/testing/selftests/tc-testing/tdc.py | 78 +++++++++-
tools/testing/selftests/tc-testing/tdc_helper.py | 5 +-
6 files changed, 287 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/README b/tools/testing/selftests/tc-testing/README
index f9281e8..22e5da9 100644
--- a/tools/testing/selftests/tc-testing/README
+++ b/tools/testing/selftests/tc-testing/README
@@ -12,10 +12,10 @@ REQUIREMENTS
* Minimum Python version of 3.4. Earlier 3.X versions may work but are not
guaranteed.
-* The kernel must have network namespace support
+* The kernel must have network namespace support if using nsPlugin
* The kernel must have veth support available, as a veth pair is created
- prior to running the tests.
+ prior to running the tests when using nsPlugin.
* The kernel must have the appropriate infrastructure enabled to run all tdc
unit tests. See the config file in this directory for minimum required
@@ -53,8 +53,12 @@ commands being tested must be run as root. The code that enforces
execution by root uid has been moved into a plugin (see PLUGIN
ARCHITECTURE, below).
-If nsPlugin is linked, all tests are executed inside a network
-namespace to prevent conflicts within the host.
+Tests that use a network device should have nsPlugin.py listed as a
+requirement for that test. nsPlugin executes all commands within a
+network namespace and creates a veth pair which may be used in those test
+cases. To disable execution within the namespace, pass the -N option
+to tdc when starting a test run; the veth pair will still be created
+by the plugin.
Running tdc without any arguments will run all tests. Refer to the section
on command line arguments for more information, or run:
@@ -154,8 +158,8 @@ action:
netns:
options for nsPlugin (run commands in net namespace)
- -n, --namespace
- Run commands in namespace as specified in tdc_config.py
+ -N, --no-namespace
+ Do not run commands in a network namespace.
valgrind:
options for valgrindPlugin (run command under test under Valgrind)
@@ -171,7 +175,8 @@ was in the tdc.py script has been moved into the plugins.
The plugins are in the directory plugin-lib. The are executed from
directory plugins. Put symbolic links from plugins to plugin-lib,
-and name them according to the order you want them to run.
+and name them according to the order you want them to run. This is not
+necessary if a test case being run requires a specific plugin to work.
Example:
@@ -223,7 +228,8 @@ directory:
- rootPlugin.py:
implements the enforcement of running as root
- nsPlugin.py:
- sets up a network namespace and runs all commands in that namespace
+ sets up a network namespace and runs all commands in that namespace,
+ while also setting up dummy devices to be used in testing.
- valgrindPlugin.py
runs each command in the execute stage under valgrind,
and checks for leaks.
diff --git a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
index a194b1a..7042618 100644
--- a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
+++ b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
@@ -18,6 +18,8 @@ class SubPlugin(TdcPlugin):
if self.args.namespace:
self._ns_create()
+ else:
+ self._ports_create()
def post_suite(self, index):
'''run commands after test_runner goes into a test loop'''
@@ -27,6 +29,8 @@ class SubPlugin(TdcPlugin):
if self.args.namespace:
self._ns_destroy()
+ else:
+ self._ports_destroy()
def add_args(self, parser):
super().add_args(parser)
@@ -34,8 +38,8 @@ class SubPlugin(TdcPlugin):
'netns',
'options for nsPlugin(run commands in net namespace)')
self.argparser_group.add_argument(
- '-n', '--namespace', action='store_true',
- help='Run commands in namespace')
+ '-N', '--no-namespace', action='store_false', default=True,
+ dest='namespace', help='Run commands in namespace')
return self.argparser
def adjust_command(self, stage, command):
@@ -73,20 +77,30 @@ class SubPlugin(TdcPlugin):
print('adjust_command: return command [{}]'.format(command))
return command
+ def _ports_create(self):
+ cmd = 'ip link add $DEV0 type veth peer name $DEV1'
+ self._exec_cmd('pre', cmd)
+ cmd = 'ip link set $DEV0 up'
+ self._exec_cmd('pre', cmd)
+ if not self.args.namespace:
+ cmd = 'ip link set $DEV1 up'
+ self._exec_cmd('pre', cmd)
+
+ def _ports_destroy(self):
+ cmd = 'ip link del $DEV0'
+ self._exec_cmd('post', cmd)
+
def _ns_create(self):
'''
Create the network namespace in which the tests will be run and set up
the required network devices for it.
'''
+ self._ports_create()
if self.args.namespace:
cmd = 'ip netns add {}'.format(self.args.NAMES['NS'])
self._exec_cmd('pre', cmd)
- cmd = 'ip link add $DEV0 type veth peer name $DEV1'
- self._exec_cmd('pre', cmd)
cmd = 'ip link set $DEV1 netns {}'.format(self.args.NAMES['NS'])
self._exec_cmd('pre', cmd)
- cmd = 'ip link set $DEV0 up'
- self._exec_cmd('pre', cmd)
cmd = 'ip -n {} link set $DEV1 up'.format(self.args.NAMES['NS'])
self._exec_cmd('pre', cmd)
if self.args.device:
diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/fw.json b/tools/testing/selftests/tc-testing/tc-tests/filters/fw.json
index 3b97cfd..e364c77 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/fw.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/fw.json
@@ -6,6 +6,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -25,6 +28,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -44,6 +50,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -63,6 +72,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -82,6 +94,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -101,6 +116,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -120,6 +138,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -139,6 +160,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -158,6 +182,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -177,6 +204,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -196,6 +226,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -215,6 +248,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -234,6 +270,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -253,6 +292,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -272,6 +314,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -291,6 +336,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -310,6 +358,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -329,6 +380,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -348,6 +402,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -367,6 +424,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -386,6 +446,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -405,6 +468,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -424,6 +490,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -443,6 +512,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -462,6 +534,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -481,6 +556,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -500,6 +578,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -519,6 +600,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -538,6 +622,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -557,6 +644,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -576,6 +666,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -595,6 +688,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -614,6 +710,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -633,6 +732,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -652,6 +754,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -671,6 +776,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -690,6 +798,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -709,6 +820,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -728,6 +842,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: protocol 802_3 prio 3 handle 7 fw action ok"
@@ -748,6 +865,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: prio 6 handle 2 fw action continue index 5"
@@ -768,6 +888,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -787,6 +910,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -806,6 +932,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 5 prio 7 fw action pass",
@@ -828,6 +957,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 5 prio 7 fw action pass",
@@ -850,6 +982,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 5 prio 7 fw action pass",
@@ -871,6 +1006,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 1 prio 4 fw action ok",
@@ -892,6 +1030,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 4 prio 2 chain 13 fw action pipe",
@@ -913,6 +1054,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 2 prio 4 fw action drop"
@@ -933,6 +1077,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 3 prio 4 fw action continue"
@@ -953,6 +1100,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 4 prio 2 protocol arp fw action pipe"
@@ -973,6 +1123,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 4 prio 2 fw action pipe flowid 45"
@@ -993,6 +1146,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 1 prio 2 fw action ok"
@@ -1013,6 +1169,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 1 prio 2 fw action ok"
@@ -1033,6 +1192,9 @@
"filter",
"fw"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress",
"$TC filter add dev $DEV1 parent ffff: handle 1 prio 2 fw action ok index 3"
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 e2f92ce..8135778 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
@@ -6,6 +6,9 @@
"filter",
"u32"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
@@ -25,6 +28,9 @@
"filter",
"matchall"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV1 clsact",
"$TC filter add dev $DEV1 protocol all pref 1 ingress handle 0x1234 matchall action ok"
@@ -45,6 +51,9 @@
"filter",
"flower"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV2 ingress",
"./tdc_batch.py $DEV2 $BATCH_FILE --share_action -n 1000000"
@@ -66,6 +75,9 @@
"filter",
"flower"
],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
"setup": [
"$TC qdisc add dev $DEV2 ingress",
"$TC filter add dev $DEV2 protocol ip prio 1 parent ffff: flower dst_mac e4:11:22:11:4a:51 src_mac e4:11:22:11:4a:50 ip_proto tcp src_ip 1.1.1.1 dst_ip 2.2.2.2 action drop"
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index 5cee156..678182a 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -25,6 +25,9 @@ from tdc_helper import *
import TdcPlugin
from TdcResults import *
+class PluginDependencyException(Exception):
+ def __init__(self, missing_pg):
+ self.missing_pg = missing_pg
class PluginMgrTestFail(Exception):
def __init__(self, stage, output, message):
@@ -37,7 +40,7 @@ class PluginMgr:
super().__init__()
self.plugins = {}
self.plugin_instances = []
- self.args = []
+ self.failed_plugins = {}
self.argparser = argparser
# TODO, put plugins in order
@@ -53,6 +56,64 @@ class PluginMgr:
self.plugins[mn] = foo
self.plugin_instances.append(foo.SubPlugin())
+ def load_plugin(self, pgdir, pgname):
+ pgname = pgname[0:-3]
+ foo = importlib.import_module('{}.{}'.format(pgdir, pgname))
+ self.plugins[pgname] = foo
+ self.plugin_instances.append(foo.SubPlugin())
+ self.plugin_instances[-1].check_args(self.args, None)
+
+ def get_required_plugins(self, testlist):
+ '''
+ Get all required plugins from the list of test cases and return
+ all unique items.
+ '''
+ reqs = []
+ for t in testlist:
+ try:
+ if 'requires' in t['plugins']:
+ if isinstance(t['plugins']['requires'], list):
+ reqs.extend(t['plugins']['requires'])
+ else:
+ reqs.append(t['plugins']['requires'])
+ except KeyError:
+ continue
+ reqs = get_unique_item(reqs)
+ return reqs
+
+ def load_required_plugins(self, reqs, parser, args, remaining):
+ '''
+ Get all required plugins from the list of test cases and load any plugin
+ that is not already enabled.
+ '''
+ pgd = ['plugin-lib', 'plugin-lib-custom']
+ pnf = []
+
+ for r in reqs:
+ if r not in self.plugins:
+ fname = '{}.py'.format(r)
+ source_path = []
+ for d in pgd:
+ pgpath = '{}/{}'.format(d, fname)
+ if os.path.isfile(pgpath):
+ source_path.append(pgpath)
+ if len(source_path) == 0:
+ print('ERROR: unable to find required plugin {}'.format(r))
+ pnf.append(fname)
+ continue
+ elif len(source_path) > 1:
+ print('WARNING: multiple copies of plugin {} found, using version found')
+ print('at {}'.format(source_path[0]))
+ pgdir = source_path[0]
+ pgdir = pgdir.split('/')[0]
+ self.load_plugin(pgdir, fname)
+ if len(pnf) > 0:
+ raise PluginDependencyException(pnf)
+
+ parser = self.call_add_args(parser)
+ (args, remaining) = parser.parse_known_args(args=remaining, namespace=args)
+ return args
+
def call_pre_suite(self, testcount, testidlist):
for pgn_inst in self.plugin_instances:
pgn_inst.pre_suite(testcount, testidlist)
@@ -98,6 +159,9 @@ class PluginMgr:
command = pgn_inst.adjust_command(stage, command)
return command
+ def set_args(self, args):
+ self.args = args
+
@staticmethod
def _make_argparser(args):
self.argparser = argparse.ArgumentParser(
@@ -550,6 +614,7 @@ def filter_tests_by_category(args, testlist):
return answer
+
def get_test_cases(args):
"""
If a test case file is specified, retrieve tests from that file.
@@ -611,7 +676,7 @@ def get_test_cases(args):
return allcatlist, allidlist, testcases_by_cats, alltestcases
-def set_operation_mode(pm, args):
+def set_operation_mode(pm, parser, args, remaining):
"""
Load the test case data and process remaining arguments to determine
what the script should do for this run, and call the appropriate
@@ -649,6 +714,12 @@ def set_operation_mode(pm, args):
exit(0)
if len(alltests):
+ req_plugins = pm.get_required_plugins(alltests)
+ try:
+ args = pm.load_required_plugins(req_plugins, parser, args, remaining)
+ except PluginDependencyException as pde:
+ print('The following plugins were not found:')
+ print('{}'.format(pde.missing_pg))
catresults = test_runner(pm, args, alltests)
if args.format == 'none':
print('Test results output suppression requested\n')
@@ -686,11 +757,12 @@ def main():
parser = pm.call_add_args(parser)
(args, remaining) = parser.parse_known_args()
args.NAMES = NAMES
+ pm.set_args(args)
check_default_settings(args, remaining, pm)
if args.verbose > 2:
print('args is {}'.format(args))
- set_operation_mode(pm, args)
+ set_operation_mode(pm, parser, args, remaining)
exit(0)
diff --git a/tools/testing/selftests/tc-testing/tdc_helper.py b/tools/testing/selftests/tc-testing/tdc_helper.py
index 9f35c96..0440d25 100644
--- a/tools/testing/selftests/tc-testing/tdc_helper.py
+++ b/tools/testing/selftests/tc-testing/tdc_helper.py
@@ -17,7 +17,10 @@ def get_categorized_testlist(alltests, ucat):
def get_unique_item(lst):
""" For a list, return a list of the unique items in the list. """
- return list(set(lst))
+ if len(lst) > 1:
+ return list(set(lst))
+ else:
+ return lst
def get_test_categories(alltests):
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-05 21:08 [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc Lucas Bates
@ 2019-06-07 12:18 ` Davide Caratti
2019-06-07 16:09 ` Lucas Bates
2019-06-14 9:37 ` Nicolas Dichtel
1 sibling, 1 reply; 8+ messages in thread
From: Davide Caratti @ 2019-06-07 12:18 UTC (permalink / raw)
To: Lucas Bates, netdev
Cc: nicolas.dichtel, davem, jhs, kernel, xiyou.wangcong, jiri,
mleitner, vladbu, Hangbin Liu
On Wed, 2019-06-05 at 17:08 -0400, Lucas Bates wrote:
> Apologies for the delay in getting this out. I've been busy
> with other things and this change was a little trickier than
> I expected.
>
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
>
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
>
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
>
> Comments welcome!
> ---
hello Lucas,
thanks for the patch, I tested it and verified it successfully on some
items belonging to the 'filter' category.
From what I see, it is a fix for the reported problem (e.g. tests failing
because of 'nsPlugin' uninstalled). And, I want to followup fixing the
bpf.json in tc-actions, so that
# ./tdc.py -l -c bpf | grep eBPF
e939: (actions, bpf) Add eBPF action with valid object-file
282d: (actions, bpf) Add eBPF action with invalid object-file
require the buildebpfPlugin (unless anybody disagrees, I will also revert
the meaning of '-B' also, like you did for '-n')
few comments after a preliminary test:
1) the patch still does not cover the two categories that use $DEV2 (i.e.
flower and concurrency still fail in my environment)
2) I've been reported, and reproduced with latest fedora, a problem in
nsPlugin.py. All tests in the 'filter' category still fail, unless I do
# sed -i "s#ip#/sbin/ip#g" nsPlugin.py
otherwise, the 'prepare' stage fails:
# ./tdc.py -e 5339
-- ns/SubPlugin.__init__
Test 5339: Del entire fw filter
-----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 ingress"
-----> prepare stage *** Error message: "/bin/sh: ip: command not found
"
returncode 127; expected [0]
-----> prepare stage *** Aborting test run.
(maybe we should use a variable for that, instead of hardcoded command
name, like we do for $TC ?)
--
davide
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-07 12:18 ` Davide Caratti
@ 2019-06-07 16:09 ` Lucas Bates
0 siblings, 0 replies; 8+ messages in thread
From: Lucas Bates @ 2019-06-07 16:09 UTC (permalink / raw)
To: Davide Caratti
Cc: Linux Kernel Network Developers, Nicolas Dichtel, David Miller,
Jamal Hadi Salim, kernel, Cong Wang, Jiri Pirko,
Marcelo Ricardo Leitner, Vlad Buslov, Hangbin Liu
On Fri, Jun 7, 2019 at 8:18 AM Davide Caratti <dcaratti@redhat.com> wrote:
> From what I see, it is a fix for the reported problem (e.g. tests failing
> because of 'nsPlugin' uninstalled). And, I want to followup fixing the
> bpf.json in tc-actions, so that
>
> # ./tdc.py -l -c bpf | grep eBPF
> e939: (actions, bpf) Add eBPF action with valid object-file
> 282d: (actions, bpf) Add eBPF action with invalid object-file
>
> require the buildebpfPlugin (unless anybody disagrees, I will also revert
> the meaning of '-B' also, like you did for '-n')
Yes, those should definitely be tagged. I probably should have
included them in the patch.
As for the -B option, that would probably be a good idea. Required
plugins, I think, should always be on and require a user to explicitly
not use them.
So in that case, nsPlugin and buildepfPlugin, being required, should
have options to explicitly disable their features. valgrindPlugin,
since it's not explicitly required to run any given test, doesn't need
that. Maybe we should separate the plugins into different directories
based on this?
>
> few comments after a preliminary test:
> 1) the patch still does not cover the two categories that use $DEV2 (i.e.
> flower and concurrency still fail in my environment)
I will have to try that out and see what is going on. I didn't try
the $DEV2 tests against this.
> 2) I've been reported, and reproduced with latest fedora, a problem in
> nsPlugin.py. All tests in the 'filter' category still fail, unless I do
>
> # sed -i "s#ip#/sbin/ip#g" nsPlugin.py
>
> otherwise, the 'prepare' stage fails:
>
> # ./tdc.py -e 5339
> -- ns/SubPlugin.__init__
> Test 5339: Del entire fw filter
>
> -----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 ingress"
>
> -----> prepare stage *** Error message: "/bin/sh: ip: command not found
> "
> returncode 127; expected [0]
>
> -----> prepare stage *** Aborting test run.
>
> (maybe we should use a variable for that, instead of hardcoded command
> name, like we do for $TC ?)
Ooh... Yes, yes we should. Sounds like the new version of Fedora
isn't including sbin on the path anymore? That's going to require
another minor change that I think I'll submit in a separate patch.
Thanks, Davide!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-05 21:08 [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc Lucas Bates
2019-06-07 12:18 ` Davide Caratti
@ 2019-06-14 9:37 ` Nicolas Dichtel
2019-06-17 2:04 ` Lucas Bates
1 sibling, 1 reply; 8+ messages in thread
From: Nicolas Dichtel @ 2019-06-14 9:37 UTC (permalink / raw)
To: Lucas Bates, netdev
Cc: davem, jhs, kernel, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti
Le 05/06/2019 à 23:08, Lucas Bates a écrit :
> Apologies for the delay in getting this out. I've been busy
> with other things and this change was a little trickier than
> I expected.
>
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
>
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
>
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
>
> Comments welcome!
Thanks for the follow up. I successfully tested your patch, it fixes the netns case.
Note that there is still a bunch of test that fails or are skipped after your
patch, for example:
ok 431 e41d - Add 1M flower filters with 10 parallel tc instances # skipped -
Not executed because DEV2 is not defined
The message is not really explicit, you have to dig into the code to understand
that '-d <dev>' is needed.
Is it not possible to use a dummy interface by default?
From my point of view, if all tests are not successful by default, it scares
users and prevent them to use those tests suite to validate their patches.
Regards,
Nicolas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-14 9:37 ` Nicolas Dichtel
@ 2019-06-17 2:04 ` Lucas Bates
2019-06-18 8:52 ` Nicolas Dichtel
0 siblings, 1 reply; 8+ messages in thread
From: Lucas Bates @ 2019-06-17 2:04 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
kernel, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
Vlad Buslov, Davide Caratti
On Fri, Jun 14, 2019 at 5:37 AM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 05/06/2019 à 23:08, Lucas Bates a écrit :
> > Apologies for the delay in getting this out. I've been busy
> > with other things and this change was a little trickier than
> > I expected.
> >
> > This patch restores the original behaviour for tdc prior to the
> > introduction of the plugin system, where the network namespace
> > functionality was split from the main script.
> >
> > It introduces the concept of required plugins for testcases,
> > and will automatically load any plugin that isn't already
> > enabled when said plugin is required by even one testcase.
> >
> > Additionally, the -n option for the nsPlugin is deprecated
> > so the default action is to make use of the namespaces.
> > Instead, we introduce -N to not use them, but still create
> > the veth pair.
> >
> > Comments welcome!
> Thanks for the follow up. I successfully tested your patch, it fixes the netns case.
Good, thank you for checking it out. I have to add a few more changes
to the patch for the BPF-related tests, but once that's done I'll
submit the finished version.
> Note that there is still a bunch of test that fails or are skipped after your
> patch, for example:
> ok 431 e41d - Add 1M flower filters with 10 parallel tc instances # skipped -
> Not executed because DEV2 is not defined
> The message is not really explicit, you have to dig into the code to understand
> that '-d <dev>' is needed.
> Is it not possible to use a dummy interface by default?
The tests that make use of DEV2 are intended to be run with a physical
NIC. This feature was originally submitted by Chris Mi from Mellanox
back in 2017 (commit 31c2611b) to reproduce a kernel panic, with d052
being the first test case submitted.
Originally they were silently skipped, but once I added TdcResults.py
this changed so they would be tracked and reported as skipped.
> From my point of view, if all tests are not successful by default, it scares
> users and prevent them to use those tests suite to validate their patches.
For me, explicitly telling the user that a test was skipped, and /why/
it was skipped, is far better than excluding the test from the
results: I don't want to waste someone's time with troubleshooting the
script if they're expecting to see results for those tests when
running tdc and nothing appears, or worse yet, stop using it because
they think it doesn't work properly.
I do believe the skip message should be improved so it better
indicates why those tests are being skipped. And the '-d' feature
should be documented. How do these changes sound?
Thanks,
Lucas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-17 2:04 ` Lucas Bates
@ 2019-06-18 8:52 ` Nicolas Dichtel
2019-06-21 0:45 ` Lucas Bates
0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Dichtel @ 2019-06-18 8:52 UTC (permalink / raw)
To: Lucas Bates
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
kernel, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
Vlad Buslov, Davide Caratti
Le 17/06/2019 à 04:04, Lucas Bates a écrit :
> On Fri, Jun 14, 2019 at 5:37 AM Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
[snip]
> The tests that make use of DEV2 are intended to be run with a physical
> NIC. This feature was originally submitted by Chris Mi from Mellanox
> back in 2017 (commit 31c2611b) to reproduce a kernel panic, with d052
> being the first test case submitted.
Ok.
>
> Originally they were silently skipped, but once I added TdcResults.py
> this changed so they would be tracked and reported as skipped.
>
>> From my point of view, if all tests are not successful by default, it scares
>> users and prevent them to use those tests suite to validate their patches.
>
> For me, explicitly telling the user that a test was skipped, and /why/
> it was skipped, is far better than excluding the test from the
> results: I don't want to waste someone's time with troubleshooting the
> script if they're expecting to see results for those tests when
> running tdc and nothing appears, or worse yet, stop using it because
> they think it doesn't work properly.
>
> I do believe the skip message should be improved so it better
> indicates why those tests are being skipped. And the '-d' feature
> should be documented. How do these changes sound?
If the error message is clear enough, I agree with you. The skip message should
not feel like an error message ;-)
Thank you,
Nicolas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-18 8:52 ` Nicolas Dichtel
@ 2019-06-21 0:45 ` Lucas Bates
2019-06-21 7:48 ` Nicolas Dichtel
0 siblings, 1 reply; 8+ messages in thread
From: Lucas Bates @ 2019-06-21 0:45 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
kernel, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
Vlad Buslov, Davide Caratti
On Tue, Jun 18, 2019 at 4:52 AM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> >> From my point of view, if all tests are not successful by default, it scares
> >> users and prevent them to use those tests suite to validate their patches.
> >
> > For me, explicitly telling the user that a test was skipped, and /why/
> > it was skipped, is far better than excluding the test from the
> > results: I don't want to waste someone's time with troubleshooting the
> > script if they're expecting to see results for those tests when
> > running tdc and nothing appears, or worse yet, stop using it because
> > they think it doesn't work properly.
> >
> > I do believe the skip message should be improved so it better
> > indicates why those tests are being skipped. And the '-d' feature
> > should be documented. How do these changes sound?
> If the error message is clear enough, I agree with you. The skip message should
> not feel like an error message ;-)
Very true. I think I just put that one in quickly and meant to come
back to it later, but either way it's a bit too vague.
I'll get that corrected, but I believe I'll add it to a separate patch
after the requires functionality goes in. I want to update some of
the documentation as well.
Thanks,
Lucas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
2019-06-21 0:45 ` Lucas Bates
@ 2019-06-21 7:48 ` Nicolas Dichtel
0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Dichtel @ 2019-06-21 7:48 UTC (permalink / raw)
To: Lucas Bates
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
kernel, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
Vlad Buslov, Davide Caratti
Le 21/06/2019 à 02:45, Lucas Bates a écrit :
[snip]
> Very true. I think I just put that one in quickly and meant to come
> back to it later, but either way it's a bit too vague.
I understand. As a developer, we tend to focus on the technical part, but we
need to remember to look at the big picture at the end ;-)
>
> I'll get that corrected, but I believe I'll add it to a separate patch
> after the requires functionality goes in. I want to update some of
Sure, not problem.
> the documentation as well.
Fine.
Thank you,
Nicolas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-21 7:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05 21:08 [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc Lucas Bates
2019-06-07 12:18 ` Davide Caratti
2019-06-07 16:09 ` Lucas Bates
2019-06-14 9:37 ` Nicolas Dichtel
2019-06-17 2:04 ` Lucas Bates
2019-06-18 8:52 ` Nicolas Dichtel
2019-06-21 0:45 ` Lucas Bates
2019-06-21 7:48 ` Nicolas Dichtel
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).