[PATCH net-next 0/4] selftests: tc-testing: updates to tdc

[PATCH net-next 0/4] selftests: tc-testing: updates to tdc

[Pedro Tammela]

- Patch 1 removes an obscure feature from tdc
- Patch 2 reworks the namespace and devices setup giving a nice speed
boost
- Patch 3 preloads all tc modules when running kselftests
- Patch 4 turns on parallel testing in kselftests

Pedro Tammela (4):
  selftests: tc-testing: drop '-N' argument from nsPlugin
  selftests: tc-testing: rework namespaces and devices setup
  selftests: tc-testing: preload all modules in kselftests
  selftests: tc-testing: use parallel tdc in kselftests

 .../tc-testing/plugin-lib/nsPlugin.py         | 112 +++++++++---------
 tools/testing/selftests/tc-testing/tdc.sh     |  69 ++++++++++-
 2 files changed, 124 insertions(+), 57 deletions(-)

-- 
2.40.1

[PATCH net-next 1/4] selftests: tc-testing: drop '-N' argument from nsPlugin

[Pedro Tammela]

This argument would bypass the net namespace creation and run the test in
the root namespace, even if nsPlugin was specified.
Drop it as it's the same as commenting out the nsPlugin from a test and adds
additional complexity to the plugin code.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 .../tc-testing/plugin-lib/nsPlugin.py         | 49 +++++--------------
 1 file changed, 13 insertions(+), 36 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
index b62429b0fcdb..2297b4568ca9 100644
--- a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
+++ b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
@@ -28,10 +28,7 @@ def prepare_suite(obj, test):
     shadow['DEV2'] = original['DEV2']
     obj.args.NAMES = shadow
 
-    if obj.args.namespace:
-        obj._ns_create()
-    else:
-        obj._ports_create()
+    obj._ns_create()
 
     # Make sure the netns is visible in the fs
     while True:
@@ -75,10 +72,7 @@ class SubPlugin(TdcPlugin):
         if self.args.verbose:
             print('{}.post_case'.format(self.sub_class))
 
-        if self.args.namespace:
-            self._ns_destroy()
-        else:
-            self._ports_destroy()
+        self._ns_destroy()
 
     def post_suite(self, index):
         if self.args.verbose:
@@ -93,24 +87,11 @@ class SubPlugin(TdcPlugin):
 
             subprocess.run(cmd, shell=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
 
-    def add_args(self, parser):
-        super().add_args(parser)
-        self.argparser_group = self.argparser.add_argument_group(
-            'netns',
-            'options for nsPlugin(run commands in net namespace)')
-        self.argparser_group.add_argument(
-            '-N', '--no-namespace', action='store_false', default=True,
-            dest='namespace', help='Don\'t run commands in namespace')
-        return self.argparser
-
     def adjust_command(self, stage, command):
         super().adjust_command(stage, command)
         cmdform = 'list'
         cmdlist = list()
 
-        if not self.args.namespace:
-            return command
-
         if self.args.verbose:
             print('{}.adjust_command'.format(self.sub_class))
 
@@ -144,8 +125,6 @@ class SubPlugin(TdcPlugin):
         cmds.append(self._replace_keywords('link add $DEV0 type veth peer name $DEV1'))
         cmds.append(self._replace_keywords('link set $DEV0 up'))
         cmds.append(self._replace_keywords('link add $DUMMY type dummy'))
-        if not self.args.namespace:
-            cmds.append(self._replace_keywords('link set $DEV1 up'))
 
         return cmds
 
@@ -161,18 +140,17 @@ class SubPlugin(TdcPlugin):
     def _ns_create_cmds(self):
         cmds = []
 
-        if self.args.namespace:
-            ns = self.args.NAMES['NS']
+        ns = self.args.NAMES['NS']
 
-            cmds.append(self._replace_keywords('netns add {}'.format(ns)))
-            cmds.append(self._replace_keywords('link set $DEV1 netns {}'.format(ns)))
-            cmds.append(self._replace_keywords('link set $DUMMY netns {}'.format(ns)))
-            cmds.append(self._replace_keywords('netns exec {} $IP link set $DEV1 up'.format(ns)))
-            cmds.append(self._replace_keywords('netns exec {} $IP link set $DUMMY up'.format(ns)))
+        cmds.append(self._replace_keywords('netns add {}'.format(ns)))
+        cmds.append(self._replace_keywords('link set $DEV1 netns {}'.format(ns)))
+        cmds.append(self._replace_keywords('link set $DUMMY netns {}'.format(ns)))
+        cmds.append(self._replace_keywords('netns exec {} $IP link set $DEV1 up'.format(ns)))
+        cmds.append(self._replace_keywords('netns exec {} $IP link set $DUMMY up'.format(ns)))
 
-            if self.args.device:
-                cmds.append(self._replace_keywords('link set $DEV2 netns {}'.format(ns)))
-                cmds.append(self._replace_keywords('netns exec {} $IP link set $DEV2 up'.format(ns)))
+        if self.args.device:
+            cmds.append(self._replace_keywords('link set $DEV2 netns {}'.format(ns)))
+            cmds.append(self._replace_keywords('netns exec {} $IP link set $DEV2 up'.format(ns)))
 
         return cmds
 
@@ -192,9 +170,8 @@ class SubPlugin(TdcPlugin):
         Destroy the network namespace for testing (and any associated network
         devices as well)
         '''
-        if self.args.namespace:
-            self._exec_cmd('post', self._ns_destroy_cmd())
-            self._ports_destroy()
+        self._exec_cmd('post', self._ns_destroy_cmd())
+        self._ports_destroy()
 
     @cached_property
     def _proc(self):
-- 
2.40.1

Re: [PATCH net-next 1/4] selftests: tc-testing: drop '-N' argument from nsPlugin

[Simon Horman]

On Tue, Nov 14, 2023 at 01:04:39PM -0300, Pedro Tammela wrote:
> This argument would bypass the net namespace creation and run the test in
> the root namespace, even if nsPlugin was specified.
> Drop it as it's the same as commenting out the nsPlugin from a test and adds
> additional complexity to the plugin code.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net-next 2/4] selftests: tc-testing: rework namespaces and devices setup

[Pedro Tammela]

As mentioned in the TC Workshop 0x17, our recent changes to tdc broke
downstream CI systems like tuxsuite. The issue is the classic problem
with rcu/workqueue objects where you can miss them if not enough wall time
has passed. The latter is subjective to the system and kernel config,
in my machine could be nanoseconds while in another could be microseconds
or more.

In order to make the suite deterministic, poll for the existence
of the objects in a reasonable manner. Talking netlink directly is the
the best solution in order to avoid paying the cost of multiple
'fork()' calls, so introduce a netlink based setup routine using
pyroute2. We leave the iproute2 one as a fallback when pyroute2 is not
available.

Also rework the iproute2 side to mimic the netlink routine where it
creates DEV0 as the peer of DEV1 and moves DEV1 into the net namespace.
This way when the namespace is deleted DEV0 is also deleted
automatically, leaving no margin for resource leaks.

Another bonus of this change is that our setup time sped up by a factor
of 2 when using netlink.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 .../tc-testing/plugin-lib/nsPlugin.py         | 69 +++++++++++++------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
index 2297b4568ca9..62974bd3a4a5 100644
--- a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
+++ b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
@@ -9,6 +9,14 @@ from TdcPlugin import TdcPlugin
 
 from tdc_config import *
 
+try:
+    from pyroute2 import netns
+    from pyroute2 import IPRoute
+    netlink = True
+except ImportError:
+    netlink = False
+    print("!!! Consider installing pyroute2 !!!")
+
 def prepare_suite(obj, test):
     original = obj.args.NAMES
 
@@ -28,7 +36,10 @@ def prepare_suite(obj, test):
     shadow['DEV2'] = original['DEV2']
     obj.args.NAMES = shadow
 
-    obj._ns_create()
+    if netlink == True:
+        obj._nl_ns_create()
+    else:
+        obj._ns_create()
 
     # Make sure the netns is visible in the fs
     while True:
@@ -67,7 +78,6 @@ class SubPlugin(TdcPlugin):
         if test_skip:
             return
 
-
     def post_case(self):
         if self.args.verbose:
             print('{}.post_case'.format(self.sub_class))
@@ -119,23 +129,41 @@ class SubPlugin(TdcPlugin):
             print('adjust_command:  return command [{}]'.format(command))
         return command
 
-    def _ports_create_cmds(self):
-        cmds = []
+    def _nl_ns_create(self):
+        ns = self.args.NAMES["NS"];
+        dev0 = self.args.NAMES["DEV0"];
+        dev1 = self.args.NAMES["DEV1"];
+        dummy = self.args.NAMES["DUMMY"];
 
-        cmds.append(self._replace_keywords('link add $DEV0 type veth peer name $DEV1'))
-        cmds.append(self._replace_keywords('link set $DEV0 up'))
-        cmds.append(self._replace_keywords('link add $DUMMY type dummy'))
-
-        return cmds
-
-    def _ports_create(self):
-        self._exec_cmd_batched('pre', self._ports_create_cmds())
-
-    def _ports_destroy_cmd(self):
-        return self._replace_keywords('link del $DEV0')
-
-    def _ports_destroy(self):
-        self._exec_cmd('post', self._ports_destroy_cmd())
+        if self.args.verbose:
+            print('{}._nl_ns_create'.format(self.sub_class))
+
+        netns.create(ns)
+        netns.pushns(newns=ns)
+        with IPRoute() as ip:
+            ip.link('add', ifname=dev1, kind='veth', peer={'ifname': dev0, 'net_ns_fd':'/proc/1/ns/net'})
+            ip.link('add', ifname=dummy, kind='dummy')
+            while True:
+                try:
+                    dev1_idx = ip.link_lookup(ifname=dev1)[0]
+                    dummy_idx = ip.link_lookup(ifname=dummy)[0]
+                    ip.link('set', index=dev1_idx, state='up')
+                    ip.link('set', index=dummy_idx, state='up')
+                    break
+                except:
+                    time.sleep(0.1)
+                    continue
+        netns.popns()
+
+        with IPRoute() as ip:
+            while True:
+                try:
+                    dev0_idx = ip.link_lookup(ifname=dev0)[0]
+                    ip.link('set', index=dev0_idx, state='up')
+                    break
+                except:
+                    time.sleep(0.1)
+                    continue
 
     def _ns_create_cmds(self):
         cmds = []
@@ -143,10 +171,13 @@ class SubPlugin(TdcPlugin):
         ns = self.args.NAMES['NS']
 
         cmds.append(self._replace_keywords('netns add {}'.format(ns)))
+        cmds.append(self._replace_keywords('link add $DEV1 type veth peer name $DEV0'))
         cmds.append(self._replace_keywords('link set $DEV1 netns {}'.format(ns)))
+        cmds.append(self._replace_keywords('link add $DUMMY type dummy'.format(ns)))
         cmds.append(self._replace_keywords('link set $DUMMY netns {}'.format(ns)))
         cmds.append(self._replace_keywords('netns exec {} $IP link set $DEV1 up'.format(ns)))
         cmds.append(self._replace_keywords('netns exec {} $IP link set $DUMMY up'.format(ns)))
+        cmds.append(self._replace_keywords('link set $DEV0 up'.format(ns)))
 
         if self.args.device:
             cmds.append(self._replace_keywords('link set $DEV2 netns {}'.format(ns)))
@@ -159,7 +190,6 @@ class SubPlugin(TdcPlugin):
         Create the network namespace in which the tests will be run and set up
         the required network devices for it.
         '''
-        self._ports_create()
         self._exec_cmd_batched('pre', self._ns_create_cmds())
 
     def _ns_destroy_cmd(self):
@@ -171,7 +201,6 @@ class SubPlugin(TdcPlugin):
         devices as well)
         '''
         self._exec_cmd('post', self._ns_destroy_cmd())
-        self._ports_destroy()
 
     @cached_property
     def _proc(self):
-- 
2.40.1

Re: [PATCH net-next 2/4] selftests: tc-testing: rework namespaces and devices setup

[Simon Horman]

On Tue, Nov 14, 2023 at 01:04:40PM -0300, Pedro Tammela wrote:
> As mentioned in the TC Workshop 0x17, our recent changes to tdc broke
> downstream CI systems like tuxsuite. The issue is the classic problem
> with rcu/workqueue objects where you can miss them if not enough wall time
> has passed. The latter is subjective to the system and kernel config,
> in my machine could be nanoseconds while in another could be microseconds
> or more.
> 
> In order to make the suite deterministic, poll for the existence
> of the objects in a reasonable manner. Talking netlink directly is the
> the best solution in order to avoid paying the cost of multiple
> 'fork()' calls, so introduce a netlink based setup routine using
> pyroute2. We leave the iproute2 one as a fallback when pyroute2 is not
> available.
> 
> Also rework the iproute2 side to mimic the netlink routine where it
> creates DEV0 as the peer of DEV1 and moves DEV1 into the net namespace.
> This way when the namespace is deleted DEV0 is also deleted
> automatically, leaving no margin for resource leaks.
> 
> Another bonus of this change is that our setup time sped up by a factor
> of 2 when using netlink.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net-next 3/4] selftests: tc-testing: preload all modules in kselftests

[Pedro Tammela]

While running tdc tests in parallel it can race over the module loading
done by tc and fail the run with random errors.
So avoid this by preloading all modules before running tdc in kselftests.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 tools/testing/selftests/tc-testing/tdc.sh | 65 ++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh
index eb357bd7923c..ae08b7a47c42 100755
--- a/tools/testing/selftests/tc-testing/tdc.sh
+++ b/tools/testing/selftests/tc-testing/tdc.sh
@@ -1,7 +1,68 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
 
-modprobe netdevsim
-modprobe sch_teql
+# If a module is required and was not compiled
+# the test that requires it will fail anyways
+try_modprobe() {
+   modprobe -q -R "$1"
+   if [ $? -ne 0 ]; then
+      echo "Module $1 not found... skipping."
+   else
+      modprobe "$1"
+   fi
+}
+
+try_modprobe netdevsim
+try_modprobe act_bpf
+try_modprobe act_connmark
+try_modprobe act_csum
+try_modprobe act_ct
+try_modprobe act_ctinfo
+try_modprobe act_gact
+try_modprobe act_gate
+try_modprobe act_ipt
+try_modprobe act_mirred
+try_modprobe act_mpls
+try_modprobe act_nat
+try_modprobe act_pedit
+try_modprobe act_police
+try_modprobe act_sample
+try_modprobe act_simple
+try_modprobe act_skbedit
+try_modprobe act_skbmod
+try_modprobe act_tunnel_key
+try_modprobe act_vlan
+try_modprobe cls_basic
+try_modprobe cls_bpf
+try_modprobe cls_cgroup
+try_modprobe cls_flow
+try_modprobe cls_flower
+try_modprobe cls_fw
+try_modprobe cls_matchall
+try_modprobe cls_route
+try_modprobe cls_u32
+try_modprobe em_canid
+try_modprobe em_cmp
+try_modprobe em_ipset
+try_modprobe em_ipt
+try_modprobe em_meta
+try_modprobe em_nbyte
+try_modprobe em_text
+try_modprobe em_u32
+try_modprobe sch_cake
+try_modprobe sch_cbs
+try_modprobe sch_choke
+try_modprobe sch_codel
+try_modprobe sch_drr
+try_modprobe sch_etf
+try_modprobe sch_ets
+try_modprobe sch_fq
+try_modprobe sch_fq_codel
+try_modprobe sch_fq_pie
+try_modprobe sch_gred
+try_modprobe sch_hfsc
+try_modprobe sch_hhf
+try_modprobe sch_htb
+try_modprobe sch_teql
 ./tdc.py -c actions --nobuildebpf
 ./tdc.py -c qdisc
-- 
2.40.1

Re: [PATCH net-next 3/4] selftests: tc-testing: preload all modules in kselftests

[Simon Horman]

On Tue, Nov 14, 2023 at 01:04:41PM -0300, Pedro Tammela wrote:
> While running tdc tests in parallel it can race over the module loading
> done by tc and fail the run with random errors.
> So avoid this by preloading all modules before running tdc in kselftests.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net-next 4/4] selftests: tc-testing: use parallel tdc in kselftests

[Pedro Tammela]

Leverage parallel tests in kselftests using all the available cpus.
We tested this in tuxsuite and locally extensively and it seems it's ready for prime time.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 tools/testing/selftests/tc-testing/tdc.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh
index ae08b7a47c42..4dbe50bde5a0 100755
--- a/tools/testing/selftests/tc-testing/tdc.sh
+++ b/tools/testing/selftests/tc-testing/tdc.sh
@@ -64,5 +64,5 @@ try_modprobe sch_hfsc
 try_modprobe sch_hhf
 try_modprobe sch_htb
 try_modprobe sch_teql
-./tdc.py -c actions --nobuildebpf
-./tdc.py -c qdisc
+./tdc.py -J`nproc` -c actions --nobuildebpf
+./tdc.py -J`nproc` -c qdisc
-- 
2.40.1

Re: [PATCH net-next 4/4] selftests: tc-testing: use parallel tdc in kselftests

[Simon Horman]

On Tue, Nov 14, 2023 at 01:04:42PM -0300, Pedro Tammela wrote:
> Leverage parallel tests in kselftests using all the available cpus.
> We tested this in tuxsuite and locally extensively and it seems it's ready for prime time.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 0/4] selftests: tc-testing: updates to tdc

[Jamal Hadi Salim]

On Tue, Nov 14, 2023 at 11:05 AM Pedro Tammela <pctammela@mojatatu.com> wrote:
>
> - Patch 1 removes an obscure feature from tdc
> - Patch 2 reworks the namespace and devices setup giving a nice speed
> boost
> - Patch 3 preloads all tc modules when running kselftests
> - Patch 4 turns on parallel testing in kselftests
>
> Pedro Tammela (4):
>   selftests: tc-testing: drop '-N' argument from nsPlugin
>   selftests: tc-testing: rework namespaces and devices setup
>   selftests: tc-testing: preload all modules in kselftests
>   selftests: tc-testing: use parallel tdc in kselftests
>

To the series:
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

>  .../tc-testing/plugin-lib/nsPlugin.py         | 112 +++++++++---------
>  tools/testing/selftests/tc-testing/tdc.sh     |  69 ++++++++++-
>  2 files changed, 124 insertions(+), 57 deletions(-)
>
> --
> 2.40.1
>

Re: [PATCH net-next 0/4] selftests: tc-testing: updates to tdc

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Tue, 14 Nov 2023 13:04:38 -0300 you wrote:
> - Patch 1 removes an obscure feature from tdc
> - Patch 2 reworks the namespace and devices setup giving a nice speed
> boost
> - Patch 3 preloads all tc modules when running kselftests
> - Patch 4 turns on parallel testing in kselftests
> 
> Pedro Tammela (4):
>   selftests: tc-testing: drop '-N' argument from nsPlugin
>   selftests: tc-testing: rework namespaces and devices setup
>   selftests: tc-testing: preload all modules in kselftests
>   selftests: tc-testing: use parallel tdc in kselftests
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] selftests: tc-testing: drop '-N' argument from nsPlugin
    https://git.kernel.org/netdev/net-next/c/9ffa01cab069
  - [net-next,2/4] selftests: tc-testing: rework namespaces and devices setup
    https://git.kernel.org/netdev/net-next/c/fa63d353ddfb
  - [net-next,3/4] selftests: tc-testing: preload all modules in kselftests
    https://git.kernel.org/netdev/net-next/c/bb9623c337f5
  - [net-next,4/4] selftests: tc-testing: use parallel tdc in kselftests
    https://git.kernel.org/netdev/net-next/c/04fd47bf70f9

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