diff mbox series

[net-next,4/5] selftests: drv-net: construct environment for running tests which require an endpoint

Message ID 20240412233705.1066444-5-kuba@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series selftests: drv-net: support testing with a remote system | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
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: 932 this patch: 932
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 938 this patch: 938
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: 944 this patch: 944
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
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
netdev/contest success net-next-2024-04-13--15-00 (tests: 960)

Commit Message

Jakub Kicinski April 12, 2024, 11:37 p.m. UTC
Nothing surprising here, hopefully. Wrap the variables from
the environment into a class or spawn a netdevsim based env
and pass it to the tests.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../testing/selftests/drivers/net/README.rst  | 31 +++++++
 .../selftests/drivers/net/lib/py/env.py       | 93 ++++++++++++++++++-
 .../testing/selftests/net/lib/py/__init__.py  |  1 +
 tools/testing/selftests/net/lib/py/netns.py   | 31 +++++++
 4 files changed, 155 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/lib/py/netns.py

Comments

Willem de Bruijn April 14, 2024, 4:45 p.m. UTC | #1
Jakub Kicinski wrote:
> Nothing surprising here, hopefully. Wrap the variables from
> the environment into a class or spawn a netdevsim based env
> and pass it to the tests.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  .../testing/selftests/drivers/net/README.rst  | 31 +++++++
>  .../selftests/drivers/net/lib/py/env.py       | 93 ++++++++++++++++++-
>  .../testing/selftests/net/lib/py/__init__.py  |  1 +
>  tools/testing/selftests/net/lib/py/netns.py   | 31 +++++++
>  4 files changed, 155 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/net/lib/py/netns.py
> 
> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index 5ef7c417d431..ffc15fe5d555 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -23,8 +23,39 @@ Variables can be set in the environment or by creating a net.config
>    # Variable set in a file
>    NETIF=eth0
>  
> +Please note that the config parser is very simple, if there are
> +any non-alphanumeric characters in the value it needs to be in
> +double quotes.
> +
>  NETIF
>  ~~~~~
>  
>  Name of the netdevice against which the test should be executed.
>  When empty or not set software devices will be used.
> +
> +LOCAL_V4, LOCAL_V6, EP_V4, EP_V6
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Local and remote (endpoint) IP addresses.

Overall, this is really cool stuff (obviously)!

REMOTE instead of EP?

Apparently I missed the earlier discussion. Would it also be possible
to have both sides be remote. Where the test runner might run on the
build host, but the kernel under test is run on two test machines.

To a certain extent, same for having two equivalent child network
namespaces isolated from the runner's environment.

> +
> +EP_TYPE
> +~~~~~~~
> +
> +Communication method used to run commands on the endpoint.
> +Test framework supports using ``netns`` and ``ssh`` channels.
> +``netns`` assumes the "remote" interface is part of the same
> +host, just moved to the specified netns.
> +``ssh`` communicates with remote endpoint over ``ssh`` and ``scp``.
> +
> +Communication methods are defined by classes in ``lib/py/ep_{name}.py``.
> +It should be possible to add a new method without modifying any of
> +the framework, by simply adding an appropriately named file to ``lib/py``.
> +
> +EP_ARGS
> +~~~~~~~
> +
> +Arguments used to construct the communication channel.
> +Communication channel dependent::
> +
> +  for netns - name of the "remote" namespace
> +  for ssh - name/address of the remote host
> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index a081e168f3db..f63be0a72a53 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py
> @@ -4,7 +4,8 @@ import os
>  import shlex
>  from pathlib import Path
>  from lib.py import ip
> -from lib.py import NetdevSimDev
> +from lib.py import NetNS, NetdevSimDev
> +from .endpoint import Endpoint
>  
>  
>  def _load_env_file(src_path):
> @@ -59,3 +60,93 @@ from lib.py import NetdevSimDev
>              self._ns = None
>  
>  
> +class NetDrvEpEnv:
> +    """
> +    Class for an environment with a local device and "remote endpoint"
> +    which can be used to send traffic in.
> +
> +    For local testing it creates two network namespaces and a pair
> +    of netdevsim devices.
> +    """
> +    def __init__(self, src_path):
> +
> +        self.env = _load_env_file(src_path)
> +
> +        # Things we try to destroy
> +        self.endpoint = None
> +        # These are for local testing state
> +        self._netns = None
> +        self._ns = None
> +        self._ns_peer = None
> +
> +        if "NETIF" in self.env:
> +            self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]
> +
> +            self.v4 = self.env.get("LOCAL_V4")
> +            self.v6 = self.env.get("LOCAL_V6")
> +            self.ep_v4 = self.env.get("EP_V4")
> +            self.ep_v6 = self.env.get("EP_V6")
> +            ep_type = self.env["EP_TYPE"]
> +            ep_args = self.env["EP_ARGS"]
> +        else:
> +            self.create_local()
> +
> +            self.dev = self._ns.nsims[0].dev
> +
> +            self.v4 = "192.0.2.1"
> +            self.v6 ="0100::1"
> +            self.ep_v4 = "192.0.2.2"
> +            self.ep_v6 = "0100::2"

Use FC00::/7 ULA addresses?

> +            ep_type = "netns"
> +            ep_args = self._netns.name
> +
> +        self.endpoint = Endpoint(ep_type, ep_args)
> +
> +        self.addr = self.v6 if self.v6 else self.v4
> +        self.ep_addr = self.ep_v6 if self.ep_v6 else self.ep_v4
> +
> +        self.ifname = self.dev['ifname']
> +        self.ifindex = self.dev['ifindex']
> +
Paolo Abeni April 15, 2024, 9:28 a.m. UTC | #2
On Fri, 2024-04-12 at 16:37 -0700, Jakub Kicinski wrote:
> Nothing surprising here, hopefully. Wrap the variables from
> the environment into a class or spawn a netdevsim based env
> and pass it to the tests.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  .../testing/selftests/drivers/net/README.rst  | 31 +++++++
>  .../selftests/drivers/net/lib/py/env.py       | 93 ++++++++++++++++++-
>  .../testing/selftests/net/lib/py/__init__.py  |  1 +
>  tools/testing/selftests/net/lib/py/netns.py   | 31 +++++++
>  4 files changed, 155 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/net/lib/py/netns.py
> 
> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index 5ef7c417d431..ffc15fe5d555 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -23,8 +23,39 @@ Variables can be set in the environment or by creating a net.config
>    # Variable set in a file
>    NETIF=eth0
>  
> +Please note that the config parser is very simple, if there are
> +any non-alphanumeric characters in the value it needs to be in
> +double quotes.
> +
>  NETIF
>  ~~~~~
>  
>  Name of the netdevice against which the test should be executed.
>  When empty or not set software devices will be used.
> +
> +LOCAL_V4, LOCAL_V6, EP_V4, EP_V6
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Local and remote (endpoint) IP addresses.
> +
> +EP_TYPE
> +~~~~~~~
> +
> +Communication method used to run commands on the endpoint.
> +Test framework supports using ``netns`` and ``ssh`` channels.
> +``netns`` assumes the "remote" interface is part of the same
> +host, just moved to the specified netns.
> +``ssh`` communicates with remote endpoint over ``ssh`` and ``scp``.
> +
> +Communication methods are defined by classes in ``lib/py/ep_{name}.py``.
> +It should be possible to add a new method without modifying any of
> +the framework, by simply adding an appropriately named file to ``lib/py``.
> +
> +EP_ARGS
> +~~~~~~~
> +
> +Arguments used to construct the communication channel.
> +Communication channel dependent::
> +
> +  for netns - name of the "remote" namespace
> +  for ssh - name/address of the remote host
> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index a081e168f3db..f63be0a72a53 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py
> @@ -4,7 +4,8 @@ import os
>  import shlex
>  from pathlib import Path
>  from lib.py import ip
> -from lib.py import NetdevSimDev
> +from lib.py import NetNS, NetdevSimDev
> +from .endpoint import Endpoint
>  
>  
>  def _load_env_file(src_path):
> @@ -59,3 +60,93 @@ from lib.py import NetdevSimDev
>              self._ns = None
>  
>  
> +class NetDrvEpEnv:
> +    """
> +    Class for an environment with a local device and "remote endpoint"
> +    which can be used to send traffic in.
> +
> +    For local testing it creates two network namespaces and a pair
> +    of netdevsim devices.
> +    """
> +    def __init__(self, src_path):
> +
> +        self.env = _load_env_file(src_path)
> +
> +        # Things we try to destroy
> +        self.endpoint = None
> +        # These are for local testing state
> +        self._netns = None
> +        self._ns = None
> +        self._ns_peer = None
> +
> +        if "NETIF" in self.env:
> +            self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]
> +
> +            self.v4 = self.env.get("LOCAL_V4")
> +            self.v6 = self.env.get("LOCAL_V6")
> +            self.ep_v4 = self.env.get("EP_V4")
> +            self.ep_v6 = self.env.get("EP_V6")
> +            ep_type = self.env["EP_TYPE"]
> +            ep_args = self.env["EP_ARGS"]
> +        else:
> +            self.create_local()
> +
> +            self.dev = self._ns.nsims[0].dev
> +
> +            self.v4 = "192.0.2.1"
> +            self.v6 ="0100::1"

Minor nit, what about using 2001:db8:, for more consistency with
existing 'net' self-tests?

Also +1 on Willem suggestion to possibly have both endpoints remote.

(Very cool stuff, ça va sans dire ;)

Thanks,

Paolo
Jakub Kicinski April 15, 2024, 2:31 p.m. UTC | #3
On Sun, 14 Apr 2024 12:45:43 -0400 Willem de Bruijn wrote:
> Overall, this is really cool stuff (obviously)!
> 
> REMOTE instead of EP?

If I have to (:
Endpoint isn't great.
But remote doesn't seem much better, and it doesn't have a nice
abbreviation :(

> Apparently I missed the earlier discussion. Would it also be possible
> to have both sides be remote. Where the test runner might run on the
> build host, but the kernel under test is run on two test machines.
> 
> To a certain extent, same for having two equivalent child network
> namespaces isolated from the runner's environment.

I was thinking about it (and even wrote one large test which uses
2 namespaces [1]). But I could not convince myself that the added
complication is worth it.

[1] https://github.com/kuba-moo/linux/blob/psp/tools/net/ynl/psp.py

Local namespace testing is one thing, entering the namespace from
python and using the right process abstraction to make sure garbage
collector doesn't collect the namespace before the test exits it
(sigh) is all doable. But we lose the ability interact with the local
system directly when the endpoint is remote. No local FW access with
read/write, we have to "cat" and "echo" like in bash. No YNL access,
unless we ship specs and CLI over.

So I concluded that we're better off leaning on kselftest for
remote/remote. make install, copy the tests over, run them remotely.
I may be biased tho, I don't have much use for remote/remote in my
development env.

> Use FC00::/7 ULA addresses?

Doesn't ULA have some magic address selection rules which IETF 
is just trying to fix now? IIUC 0100:: is the documentation prefix,
so shouldn't be too bad?
Willem de Bruijn April 15, 2024, 3:28 p.m. UTC | #4
Jakub Kicinski wrote:
> On Sun, 14 Apr 2024 12:45:43 -0400 Willem de Bruijn wrote:
> > Overall, this is really cool stuff (obviously)!
> > 
> > REMOTE instead of EP?
> 
> If I have to (:
> Endpoint isn't great.
> But remote doesn't seem much better, and it doesn't have a nice
> abbreviation :(

It pairs well with local.

Since in some tests the (local) machine under test is the sender and
in others it is the receiver, we cannot use SERVER/CLIENT or so.
 
> > Apparently I missed the earlier discussion. Would it also be possible
> > to have both sides be remote. Where the test runner might run on the
> > build host, but the kernel under test is run on two test machines.
> > 
> > To a certain extent, same for having two equivalent child network
> > namespaces isolated from the runner's environment.
> 
> I was thinking about it (and even wrote one large test which uses
> 2 namespaces [1]). But I could not convince myself that the added
> complication is worth it.
> 
> [1] https://github.com/kuba-moo/linux/blob/psp/tools/net/ynl/psp.py
> 
> Local namespace testing is one thing, entering the namespace from
> python and using the right process abstraction to make sure garbage
> collector doesn't collect the namespace before the test exits it
> (sigh) is all doable. But we lose the ability interact with the local
> system directly when the endpoint is remote. No local FW access with
> read/write, we have to "cat" and "echo" like in bash. No YNL access,
> unless we ship specs and CLI over.

In cases like testing jumbo frames (or other MTU, like 4K),
configuration changes will have to be made on both the machine under
test and the remote traffic generator/sink. It seems to me
unavoidable. Most of the two-machine tests I require an equal amount
of setup on both sides. But again, cart before the horse. We can
always revisit this later if needed.
 
> So I concluded that we're better off leaning on kselftest for
> remote/remote. make install, copy the tests over, run them remotely.
> I may be biased tho, I don't have much use for remote/remote in my
> development env.
> 
> > Use FC00::/7 ULA addresses?
> 
> Doesn't ULA have some magic address selection rules which IETF 
> is just trying to fix now? IIUC 0100:: is the documentation prefix,
> so shouldn't be too bad?

RFC 6666 defines this as the "Discard Prefix".
Jakub Kicinski April 15, 2024, 5:36 p.m. UTC | #5
On Mon, 15 Apr 2024 11:28:47 -0400 Willem de Bruijn wrote:
> > If I have to (:
> > Endpoint isn't great.
> > But remote doesn't seem much better, and it doesn't have a nice
> > abbreviation :(  
> 
> It pairs well with local.
> 
> Since in some tests the (local) machine under test is the sender and
> in others it is the receiver, we cannot use SERVER/CLIENT or so.

Alright.

> > > Use FC00::/7 ULA addresses?  
> > 
> > Doesn't ULA have some magic address selection rules which IETF 
> > is just trying to fix now? IIUC 0100:: is the documentation prefix,
> > so shouldn't be too bad?  
> 
> RFC 6666 defines this as the "Discard Prefix".

Alright, let me use Paolo's suggestion of 2001:db8:
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
index 5ef7c417d431..ffc15fe5d555 100644
--- a/tools/testing/selftests/drivers/net/README.rst
+++ b/tools/testing/selftests/drivers/net/README.rst
@@ -23,8 +23,39 @@  Variables can be set in the environment or by creating a net.config
   # Variable set in a file
   NETIF=eth0
 
+Please note that the config parser is very simple, if there are
+any non-alphanumeric characters in the value it needs to be in
+double quotes.
+
 NETIF
 ~~~~~
 
 Name of the netdevice against which the test should be executed.
 When empty or not set software devices will be used.
+
+LOCAL_V4, LOCAL_V6, EP_V4, EP_V6
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Local and remote (endpoint) IP addresses.
+
+EP_TYPE
+~~~~~~~
+
+Communication method used to run commands on the endpoint.
+Test framework supports using ``netns`` and ``ssh`` channels.
+``netns`` assumes the "remote" interface is part of the same
+host, just moved to the specified netns.
+``ssh`` communicates with remote endpoint over ``ssh`` and ``scp``.
+
+Communication methods are defined by classes in ``lib/py/ep_{name}.py``.
+It should be possible to add a new method without modifying any of
+the framework, by simply adding an appropriately named file to ``lib/py``.
+
+EP_ARGS
+~~~~~~~
+
+Arguments used to construct the communication channel.
+Communication channel dependent::
+
+  for netns - name of the "remote" namespace
+  for ssh - name/address of the remote host
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index a081e168f3db..f63be0a72a53 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -4,7 +4,8 @@  import os
 import shlex
 from pathlib import Path
 from lib.py import ip
-from lib.py import NetdevSimDev
+from lib.py import NetNS, NetdevSimDev
+from .endpoint import Endpoint
 
 
 def _load_env_file(src_path):
@@ -59,3 +60,93 @@  from lib.py import NetdevSimDev
             self._ns = None
 
 
+class NetDrvEpEnv:
+    """
+    Class for an environment with a local device and "remote endpoint"
+    which can be used to send traffic in.
+
+    For local testing it creates two network namespaces and a pair
+    of netdevsim devices.
+    """
+    def __init__(self, src_path):
+
+        self.env = _load_env_file(src_path)
+
+        # Things we try to destroy
+        self.endpoint = None
+        # These are for local testing state
+        self._netns = None
+        self._ns = None
+        self._ns_peer = None
+
+        if "NETIF" in self.env:
+            self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]
+
+            self.v4 = self.env.get("LOCAL_V4")
+            self.v6 = self.env.get("LOCAL_V6")
+            self.ep_v4 = self.env.get("EP_V4")
+            self.ep_v6 = self.env.get("EP_V6")
+            ep_type = self.env["EP_TYPE"]
+            ep_args = self.env["EP_ARGS"]
+        else:
+            self.create_local()
+
+            self.dev = self._ns.nsims[0].dev
+
+            self.v4 = "192.0.2.1"
+            self.v6 ="0100::1"
+            self.ep_v4 = "192.0.2.2"
+            self.ep_v6 = "0100::2"
+            ep_type = "netns"
+            ep_args = self._netns.name
+
+        self.endpoint = Endpoint(ep_type, ep_args)
+
+        self.addr = self.v6 if self.v6 else self.v4
+        self.ep_addr = self.ep_v6 if self.ep_v6 else self.ep_v4
+
+        self.ifname = self.dev['ifname']
+        self.ifindex = self.dev['ifindex']
+
+    def create_local(self):
+        self._netns = NetNS()
+        self._ns = NetdevSimDev()
+        self._ns_peer = NetdevSimDev(ns=self._netns)
+
+        with open("/proc/self/ns/net") as nsfd0, \
+             open("/var/run/netns/" + self._netns.name) as nsfd1:
+            ifi0 = self._ns.nsims[0].ifindex
+            ifi1 = self._ns_peer.nsims[0].ifindex
+            NetdevSimDev.ctrl_write('link_device',
+                                    f'{nsfd0.fileno()}:{ifi0} {nsfd1.fileno()}:{ifi1}')
+
+        ip(f"   addr add dev {self._ns.nsims[0].ifname} 192.0.2.1/24")
+        ip(f"-6 addr add dev {self._ns.nsims[0].ifname} 0100::1/64 nodad")
+        ip(f"   link set dev {self._ns.nsims[0].ifname} up")
+
+        ip(f"   addr add dev {self._ns_peer.nsims[0].ifname} 192.0.2.2/24", ns=self._netns)
+        ip(f"-6 addr add dev {self._ns_peer.nsims[0].ifname} 0100::2/64 nodad", ns=self._netns)
+        ip(f"   link set dev {self._ns_peer.nsims[0].ifname} up", ns=self._netns)
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, ex_type, ex_value, ex_tb):
+        """
+        __exit__ gets called at the end of a "with" block.
+        """
+        self.__del__()
+
+    def __del__(self):
+        if self._ns:
+            self._ns.remove()
+            self._ns = None
+        if self._ns_peer:
+            self._ns_peer.remove()
+            self._ns_peer = None
+        if self._netns:
+            del self._netns
+            self._netns = None
+        if self.endpoint:
+            del self.endpoint
+            self.endpoint = None
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index ded7102df18a..b6d498d125fe 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -2,6 +2,7 @@ 
 
 from .consts import KSRC
 from .ksft import *
+from .netns import NetNS
 from .nsim import *
 from .utils import *
 from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily
diff --git a/tools/testing/selftests/net/lib/py/netns.py b/tools/testing/selftests/net/lib/py/netns.py
new file mode 100644
index 000000000000..ecff85f9074f
--- /dev/null
+++ b/tools/testing/selftests/net/lib/py/netns.py
@@ -0,0 +1,31 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+from .utils import ip
+import random
+import string
+
+
+class NetNS:
+    def __init__(self, name=None):
+        if name:
+            self.name = name
+        else:
+            self.name = ''.join(random.choice(string.ascii_lowercase) for _ in range(8))
+        ip('netns add ' + self.name)
+
+    def __del__(self):
+        if self.name:
+            ip('netns del ' + self.name)
+            self.name = None
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, ex_type, ex_value, ex_tb):
+        self.__del__()
+
+    def __str__(self):
+        return self.name
+
+    def __repr__(self):
+        return f"NetNS({self.name})"