diff mbox series

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

Message ID 20231114160442.1023815-3-pctammela@mojatatu.com (mailing list archive)
State Accepted
Commit fa63d353ddfb8a2d7688220a45b84e1507d211cf
Delegated to: Netdev Maintainers
Headers show
Series selftests: tc-testing: updates to tdc | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 1 maintainers not CCed: linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 116 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pedro Tammela Nov. 14, 2023, 4:04 p.m. UTC
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(-)

Comments

Simon Horman Nov. 16, 2023, 2:11 p.m. UTC | #1
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>
diff mbox series

Patch

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