I had to explain how to run the driver tests twice already. Improve the README so we can just point to it. Also fix a potential problem with the SSH remote sessions.
Jakub Kicinski (4): selftests: drv-net: force pseudo-terminal allocation in ssh selftests: drv-net: extend the README with more info and example selftests: drv-net: reimplement the config parser selftests: drv-net: validate the environment
.../testing/selftests/drivers/net/README.rst | 95 ++++++++++++++++--- .../selftests/drivers/net/lib/py/env.py | 46 ++++++--- .../drivers/net/lib/py/remote_ssh.py | 2 +- 3 files changed, 118 insertions(+), 25 deletions(-)
This is not yet needed, because we don't terminate remote background commands. But once we do, if we run ssh without -t the ssh session may close and the program may carry on happily running.
I have hit this problem experimenting with mausezahn, let's fix it already to avoid someone else wasting time debugging it.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- tools/testing/selftests/drivers/net/lib/py/remote_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py index 924addde19a3..294a4ed8284e 100644 --- a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py +++ b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py @@ -20,7 +20,7 @@ from lib.py import cmd self._tmpdir = None
def cmd(self, comm): - return subprocess.Popen(["ssh", "-q", self.name, comm], + return subprocess.Popen(["ssh", "-t", "-q", self.name, comm], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
def _mktmp(self):
On Wed, 24 Apr 2024 15:14:41 -0700 Jakub Kicinski wrote:
This is not yet needed, because we don't terminate remote background commands. But once we do, if we run ssh without -t the ssh session may close and the program may carry on happily running.
I have hit this problem experimenting with mausezahn, let's fix it already to avoid someone else wasting time debugging it.
Signed-off-by: Jakub Kicinski kuba@kernel.org
tools/testing/selftests/drivers/net/lib/py/remote_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py index 924addde19a3..294a4ed8284e 100644 --- a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py +++ b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py @@ -20,7 +20,7 @@ from lib.py import cmd self._tmpdir = None def cmd(self, comm):
return subprocess.Popen(["ssh", "-q", self.name, comm],
return subprocess.Popen(["ssh", "-t", "-q", self.name, comm], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
This seems to mess up the local terminal. I guess we'll cross that bridge when we get there... I'll drop this patch when applying.
Add more info to the README. It's also now copied to GitHub for increased visibility:
https://github.com/linux-netdev/nipa/wiki/Running-driver-tests
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../testing/selftests/drivers/net/README.rst | 97 ++++++++++++++++--- 1 file changed, 85 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst index 0cbab33dad1f..3b6a29e6564b 100644 --- a/tools/testing/selftests/drivers/net/README.rst +++ b/tools/testing/selftests/drivers/net/README.rst @@ -1,18 +1,42 @@ -Running tests -============= +.. SPDX-License-Identifier: GPL-2.0
-Tests are executed within kselftest framework like any other tests. -By default tests execute against software drivers such as netdevsim. -All tests must support running against a real device (SW-only tests -should instead be placed in net/ or drivers/net/netdevsim, HW-only -tests in drivers/net/hw). +Running driver tests +====================
-Set appropriate variables to point the tests at a real device. +Networking driver tests are executed within kselftest framework like any +other tests. They support testing both real device drivers and emulated / +software drivers (latter mostly to test the core parts of the stack). + +SW mode +~~~~~~~ + +By default, when no extra parameters are set or exported, tests execute +against software drivers such as netdevsim. No extra preparation is required +the software devices are created and destroyed as part of the test. +In this mode the tests are indistinguishable from other selftests and +(for example) can be run under ``virtme-ng`` like the core networking selftests. + +HW mode +~~~~~~~ + +Executing tests against a real device requires external preparation. +The netdevice against which tests will be run must exist, be running +(in UP state) and be configured with an IP address. + +Refer to list of :ref:`Variables` later in this file to set up running +the tests against a real device. + +Both modes required +~~~~~~~~~~~~~~~~~~~ + +All tests in drivers/net must support running both against a software device +and a real device. SW-only tests should instead be placed in net/ or +drivers/net/netdevsim, HW-only tests in drivers/net/hw.
Variables =========
-Variables can be set in the environment or by creating a net.config +The variables can be set in the environment or by creating a net.config file in the same directory as this README file. Example::
$ NETIF=eth0 ./some_test.sh @@ -23,9 +47,9 @@ 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. +Local test (which don't require endpoint for sending / receiving traffic) +need only the ``NETIF`` variable. Remaining variables define the endpoint +and communication method.
NETIF ~~~~~ @@ -61,3 +85,52 @@ Arguments used to construct the communication channel.
for netns - name of the "remote" namespace for ssh - name/address of the remote host + +Example +======= + +Build the selftests:: + + # make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" + +"Install" the tests and copy them over to the target machine:: + + # make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" \ + install INSTALL_PATH=/tmp/ksft-net-drv + + # rsync -ra --delete /tmp/ksft-net-drv root@192.168.1.1:/root/ + +On the target machine, running the tests will use netdevsim by default:: + + [/root] # ./ksft-net-drv/run_kselftest.sh -t drivers/net:ping.py + TAP version 13 + 1..1 + # timeout set to 45 + # selftests: drivers/net: ping.py + # KTAP version 1 + # 1..3 + # ok 1 ping.test_v4 + # ok 2 ping.test_v6 + # ok 3 ping.test_tcp + # # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 + ok 1 selftests: drivers/net: ping.py + +Create a config with remote info:: + + [/root] # cat > ./ksft-net-drv/drivers/net/net.config <<EOF + NETIF=eth0 + LOCAL_V4=192.168.1.1 + REMOTE_V4=192.168.1.2 + REMOTE_TYPE=ssh + REMOTE_ARGS=root@192.168.1.2 + EOF + +Run the test:: + + [/root] # ./ksft-net-drv/drivers/net/ping.py + KTAP version 1 + 1..3 + ok 1 ping.test_v4 + ok 2 ping.test_v6 # SKIP Test requires IPv6 connectivity + ok 3 ping.test_tcp + # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:1 error:0
The shell lexer is not helping much, do very basic parsing manually.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/lib/py/env.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index a3db1bb1afeb..a6a5a5f9c6db 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0
import os -import shlex from pathlib import Path from lib.py import KsftSkipEx from lib.py import cmd, ip @@ -16,17 +15,20 @@ from .remote import Remote if not (src_dir / "net.config").exists(): return env
- lexer = shlex.shlex(open((src_dir / "net.config").as_posix(), 'r').read()) - k = None - for token in lexer: - if k is None: - k = token - env[k] = "" - elif token == "=": - pass - else: - env[k] = token - k = None + with open((src_dir / "net.config").as_posix(), 'r') as fp: + for line in fp.readlines(): + full_file = line + # Strip comments + pos = line.find("#") + if pos >= 0: + line = line[:pos] + line = line.strip() + if not line: + continue + pos = line.find("=") + if pos <= 0: + raise Exception("Can't parse configuration line:", full_file) + env[line[:pos]] = line[pos+1:] return env
Jakub Kicinski wrote:
The shell lexer is not helping much, do very basic parsing manually.
Signed-off-by: Jakub Kicinski kuba@kernel.org
.../selftests/drivers/net/lib/py/env.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index a3db1bb1afeb..a6a5a5f9c6db 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 import os -import shlex from pathlib import Path from lib.py import KsftSkipEx from lib.py import cmd, ip @@ -16,17 +15,20 @@ from .remote import Remote if not (src_dir / "net.config").exists(): return env
- lexer = shlex.shlex(open((src_dir / "net.config").as_posix(), 'r').read())
- k = None
- for token in lexer:
if k is None:k = tokenenv[k] = ""elif token == "=":passelse:env[k] = tokenk = None
- with open((src_dir / "net.config").as_posix(), 'r') as fp:
for line in fp.readlines():full_file = line# Strip commentspos = line.find("#")if pos >= 0:line = line[:pos]line = line.strip()if not line:continuepos = line.find("=")if pos <= 0:raise Exception("Can't parse configuration line:", full_file)env[line[:pos]] = line[pos+1:]
Or line.split. But bikeshedding python is a deep rabbit hole :)
Throw a slightly more helpful exception when env variables are partially populated. Prior to this change we'd get a dictionary key exception somewhere later on.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/lib/py/env.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index a6a5a5f9c6db..fda4967503de 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -88,6 +88,7 @@ from .remote import Remote self._ns_peer = None
if "NETIF" in self.env: + self._check_env() self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]
self.v4 = self.env.get("LOCAL_V4") @@ -143,6 +144,25 @@ from .remote import Remote ip(f"-6 addr add dev {self._ns_peer.nsims[0].ifname} {self.nsim_v6_pfx}2/64 nodad", ns=self._netns) ip(f" link set dev {self._ns_peer.nsims[0].ifname} up", ns=self._netns)
+ def _check_env(self): + vars_needed = [ + ["LOCAL_V4", "LOCAL_V6"], + ["REMOTE_V4", "REMOTE_V6"], + ["REMOTE_TYPE"], + ["REMOTE_ARGS"] + ] + missing = [] + + for choice in vars_needed: + for entry in choice: + if entry in self.env: + break + else: + missing.append(choice) + if missing: + raise Exception("Invalid environment, missing configuration:", missing, + "Please see tools/testing/selftests/drivers/net/README.rst") + def __enter__(self): return self
Jakub Kicinski wrote:
Throw a slightly more helpful exception when env variables are partially populated. Prior to this change we'd get a dictionary key exception somewhere later on.
Signed-off-by: Jakub Kicinski kuba@kernel.org
.../selftests/drivers/net/lib/py/env.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index a6a5a5f9c6db..fda4967503de 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -88,6 +88,7 @@ from .remote import Remote self._ns_peer = None if "NETIF" in self.env:
self._check_env() self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]self.v4 = self.env.get("LOCAL_V4") @@ -143,6 +144,25 @@ from .remote import Remote ip(f"-6 addr add dev {self._ns_peer.nsims[0].ifname} {self.nsim_v6_pfx}2/64 nodad", ns=self._netns) ip(f" link set dev {self._ns_peer.nsims[0].ifname} up", ns=self._netns)
- def _check_env(self):
vars_needed = [["LOCAL_V4", "LOCAL_V6"],["REMOTE_V4", "REMOTE_V6"],["REMOTE_TYPE"],["REMOTE_ARGS"]]missing = []for choice in vars_needed:for entry in choice:if entry in self.env:breakelse:missing.append(choice)if missing:raise Exception("Invalid environment, missing configuration:", missing,"Please see tools/testing/selftests/drivers/net/README.rst")
I suppose this could still reach the exception if a file contains LOCAL_V4 and REMOTE_V6 or vice versa.
But this is best effort anyway.
Jakub Kicinski wrote:
I had to explain how to run the driver tests twice already. Improve the README so we can just point to it. Also fix a potential problem with the SSH remote sessions.
Jakub Kicinski (4): selftests: drv-net: force pseudo-terminal allocation in ssh selftests: drv-net: extend the README with more info and example selftests: drv-net: reimplement the config parser selftests: drv-net: validate the environment
.../testing/selftests/drivers/net/README.rst | 95 ++++++++++++++++--- .../selftests/drivers/net/lib/py/env.py | 46 ++++++--- .../drivers/net/lib/py/remote_ssh.py | 2 +- 3 files changed, 118 insertions(+), 25 deletions(-)
Left two minor points comments inline, but nothing important. The clear documentation is really helpful, thanks.
Reviewed-by: Willem de Bruijn willemb@google.com
linux-kselftest-mirror@lists.linaro.org
-
Jakub Kicinski -
Willem de Bruijn