* [PATCH net-next v5 1/7] selftests: drv-net: define endpoint structures
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 2/7] selftests: drv-net: factor out parsing of the env Jakub Kicinski
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Define the remote endpoint "model". To execute most meaningful device
driver tests we need to be able to communicate with a remote system,
and have it send traffic to the device under test.
Various test environments will have different requirements.
0) "Local" netdevsim-based testing can simply use net namespaces.
netdevsim supports connecting two devices now, to form a veth-like
construct.
1) Similarly on hosts with multiple NICs, the NICs may be connected
together with a loopback cable or internal device loopback.
One interface may be placed into separate netns, and tests
would proceed much like in the netdevsim case. Note that
the loopback config or the moving of one interface
into a netns is not expected to be part of selftest code.
2) Some systems may need to communicate with the remote endpoint
via SSH.
3) Last but not least environment may have its own custom communication
method.
Fundamentally we only need two operations:
- run a command remotely
- deploy a binary (if some tool we need is built as part of kselftests)
Wrap these two in a class. Use dynamic loading to load the Remote
class. This will allow very easy definition of other communication
methods without bothering upstream code base.
Stick to the "simple" / "no unnecessary abstractions" model for
referring to the remote endpoints. The host / remote object are
passed as an argument to the usual cmd() or ip() invocation.
For example:
ip("link show", json=True, host=remote)
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v4:
- bump timeout to 5 sec, in case of really slow SSH
v3:
- make Remote() return Popen() object
- always operate on absolute paths
v2:
- rename Endpoint -> Remote
---
.../selftests/drivers/net/lib/py/__init__.py | 1 +
.../selftests/drivers/net/lib/py/remote.py | 15 +++++++
.../drivers/net/lib/py/remote_netns.py | 21 ++++++++++
.../drivers/net/lib/py/remote_ssh.py | 39 +++++++++++++++++++
tools/testing/selftests/net/lib/py/utils.py | 17 ++++----
5 files changed, 85 insertions(+), 8 deletions(-)
create mode 100644 tools/testing/selftests/drivers/net/lib/py/remote.py
create mode 100644 tools/testing/selftests/drivers/net/lib/py/remote_netns.py
create mode 100644 tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
index 4653dffcd962..4789c1a4282d 100644
--- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
@@ -15,3 +15,4 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
sys.exit(4)
from .env import *
+from .remote import Remote
diff --git a/tools/testing/selftests/drivers/net/lib/py/remote.py b/tools/testing/selftests/drivers/net/lib/py/remote.py
new file mode 100644
index 000000000000..b1780b987722
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/py/remote.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+
+import os
+import importlib
+
+_modules = {}
+
+def Remote(kind, args, src_path):
+ global _modules
+
+ if kind not in _modules:
+ _modules[kind] = importlib.import_module("..remote_" + kind, __name__)
+
+ dir_path = os.path.abspath(src_path + "/../")
+ return getattr(_modules[kind], "Remote")(args, dir_path)
diff --git a/tools/testing/selftests/drivers/net/lib/py/remote_netns.py b/tools/testing/selftests/drivers/net/lib/py/remote_netns.py
new file mode 100644
index 000000000000..7d5eeb0271bc
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/py/remote_netns.py
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+
+import os
+import subprocess
+
+from lib.py import cmd
+
+
+class Remote:
+ def __init__(self, name, dir_path):
+ self.name = name
+ self.dir_path = dir_path
+
+ def cmd(self, comm):
+ return subprocess.Popen(["ip", "netns", "exec", self.name, "bash", "-c", comm],
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+ def deploy(self, what):
+ if os.path.isabs(what):
+ return what
+ return os.path.abspath(self.dir_path + "/" + what)
diff --git a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
new file mode 100644
index 000000000000..924addde19a3
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0
+
+import os
+import string
+import subprocess
+import random
+
+from lib.py import cmd
+
+
+class Remote:
+ def __init__(self, name, dir_path):
+ self.name = name
+ self.dir_path = dir_path
+ self._tmpdir = None
+
+ def __del__(self):
+ if self._tmpdir:
+ cmd("rm -rf " + self._tmpdir, host=self)
+ self._tmpdir = None
+
+ def cmd(self, comm):
+ return subprocess.Popen(["ssh", "-q", self.name, comm],
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+ def _mktmp(self):
+ return ''.join(random.choice(string.ascii_lowercase) for _ in range(8))
+
+ def deploy(self, what):
+ if not self._tmpdir:
+ self._tmpdir = "/tmp/" + self._mktmp()
+ cmd("mkdir " + self._tmpdir, host=self)
+ file_name = self._tmpdir + "/" + self._mktmp() + os.path.basename(what)
+
+ if not os.path.isabs(what):
+ what = os.path.abspath(self.dir_path + "/" + what)
+
+ cmd(f"scp {what} {self.name}:{file_name}")
+ return file_name
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 19612348c30d..7347d0c0ff05 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -4,10 +4,8 @@ import json as _json
import subprocess
class cmd:
- def __init__(self, comm, shell=True, fail=True, ns=None, background=False):
+ def __init__(self, comm, shell=True, fail=True, ns=None, background=False, host=None):
if ns:
- if isinstance(ns, NetNS):
- ns = ns.name
comm = f'ip netns exec {ns} ' + comm
self.stdout = None
@@ -15,15 +13,18 @@ import subprocess
self.ret = None
self.comm = comm
- self.proc = subprocess.Popen(comm, shell=shell, stdout=subprocess.PIPE,
- stderr=subprocess.PIPE)
+ if host:
+ self.proc = host.cmd(comm)
+ else:
+ self.proc = subprocess.Popen(comm, shell=shell, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
if not background:
self.process(terminate=False, fail=fail)
def process(self, terminate=True, fail=None):
if terminate:
self.proc.terminate()
- stdout, stderr = self.proc.communicate()
+ stdout, stderr = self.proc.communicate(timeout=5)
self.stdout = stdout.decode("utf-8")
self.stderr = stderr.decode("utf-8")
self.proc.stdout.close()
@@ -37,12 +38,12 @@ import subprocess
(self.proc.args, stdout, stderr))
-def ip(args, json=None, ns=None):
+def ip(args, json=None, ns=None, host=None):
cmd_str = "ip "
if json:
cmd_str += '-j '
cmd_str += args
- cmd_obj = cmd(cmd_str, ns=ns)
+ cmd_obj = cmd(cmd_str, ns=ns, host=host)
if json:
return _json.loads(cmd_obj.stdout)
return cmd_obj
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 2/7] selftests: drv-net: factor out parsing of the env
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 1/7] selftests: drv-net: define endpoint structures Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 3/7] selftests: drv-net: construct environment for running tests which require an endpoint Jakub Kicinski
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
The tests with a remote end will use a different class,
for clarity, but will also need to parse the env.
So factor parsing the env out to a function.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
.../selftests/drivers/net/lib/py/env.py | 43 +++++++++++--------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index e1abe9491daf..a081e168f3db 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -6,12 +6,36 @@ from pathlib import Path
from lib.py import ip
from lib.py import NetdevSimDev
+
+def _load_env_file(src_path):
+ env = os.environ.copy()
+
+ src_dir = Path(src_path).parent.resolve()
+ 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
+ return env
+
+
class NetDrvEnv:
+ """
+ Class for a single NIC / host env, with no remote end
+ """
def __init__(self, src_path):
self._ns = None
- self.env = os.environ.copy()
- self._load_env_file(src_path)
+ self.env = _load_env_file(src_path)
if 'NETIF' in self.env:
self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0]
@@ -34,19 +58,4 @@ from lib.py import NetdevSimDev
self._ns.remove()
self._ns = None
- def _load_env_file(self, src_path):
- src_dir = Path(src_path).parent.resolve()
- if not (src_dir / "net.config").exists():
- return
- lexer = shlex.shlex(open((src_dir / "net.config").as_posix(), 'r').read())
- k = None
- for token in lexer:
- if k is None:
- k = token
- self.env[k] = ""
- elif token == "=":
- pass
- else:
- self.env[k] = token
- k = None
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 3/7] selftests: drv-net: construct environment for running tests which require an endpoint
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 1/7] selftests: drv-net: define endpoint structures Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 2/7] selftests: drv-net: factor out parsing of the env Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 4/7] selftests: drv-net: add a trivial ping test Jakub Kicinski
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
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 | 33 +++++++
.../selftests/drivers/net/lib/py/env.py | 98 ++++++++++++++++++-
.../testing/selftests/net/lib/py/__init__.py | 1 +
tools/testing/selftests/net/lib/py/netns.py | 31 ++++++
4 files changed, 162 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..0cbab33dad1f 100644
--- a/tools/testing/selftests/drivers/net/README.rst
+++ b/tools/testing/selftests/drivers/net/README.rst
@@ -23,8 +23,41 @@ 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, REMOTE_V4, REMOTE_V6
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Local and remote endpoint IP addresses.
+
+REMOTE_TYPE
+~~~~~~~~~~~
+
+Communication method used to run commands on the remote endpoint.
+Test framework has built-in support for ``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``.
+Using persistent SSH connections is strongly encouraged to avoid
+the latency of SSH connection setup on every command.
+
+Communication methods are defined by classes in ``lib/py/remote_{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``.
+
+REMOTE_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..579c5b34e6fd 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 .remote import Remote
def _load_env_file(src_path):
@@ -59,3 +60,98 @@ 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.
+ """
+
+ # Network prefixes used for local tests
+ nsim_v4_pfx = "192.0.2."
+ nsim_v6_pfx = "2001:db8::"
+
+ def __init__(self, src_path):
+
+ self.env = _load_env_file(src_path)
+
+ # Things we try to destroy
+ self.remote = 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.remote_v4 = self.env.get("REMOTE_V4")
+ self.remote_v6 = self.env.get("REMOTE_V6")
+ kind = self.env["REMOTE_TYPE"]
+ args = self.env["REMOTE_ARGS"]
+ else:
+ self.create_local()
+
+ self.dev = self._ns.nsims[0].dev
+
+ self.v4 = self.nsim_v4_pfx + "1"
+ self.v6 = self.nsim_v6_pfx + "1"
+ self.remote_v4 = self.nsim_v4_pfx + "2"
+ self.remote_v6 = self.nsim_v6_pfx + "2"
+ kind = "netns"
+ args = self._netns.name
+
+ self.remote = Remote(kind, args, src_path)
+
+ self.addr = self.v6 if self.v6 else self.v4
+ self.remote_addr = self.remote_v6 if self.remote_v6 else self.remote_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} {self.nsim_v4_pfx}1/24")
+ ip(f"-6 addr add dev {self._ns.nsims[0].ifname} {self.nsim_v6_pfx}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} {self.nsim_v4_pfx}2/24", ns=self._netns)
+ 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 __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.remote:
+ del self.remote
+ self.remote = 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})"
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 4/7] selftests: drv-net: add a trivial ping test
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (2 preceding siblings ...)
2024-04-20 2:52 ` [PATCH net-next v5 3/7] selftests: drv-net: construct environment for running tests which require an endpoint Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 5/7] selftests: net: support matching cases by name prefix Jakub Kicinski
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Add a very simple test for testing with a remote system.
Both IPv4 and IPv6 connectivity is optional, later change
will add checks to skip tests based on available addresses.
Using netdevsim:
$ ./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..2
# ok 1 ping.test_v4
# ok 2 ping.test_v6
# # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: drivers/net: ping.py
Command line SSH:
$ NETIF=virbr0 REMOTE_TYPE=ssh REMOTE_ARGS=root@192.168.122.123 \
LOCAL_V4=192.168.122.1 REMOTE_V4=192.168.122.123 \
./tools/testing/selftests/drivers/net/ping.py
KTAP version 1
1..2
ok 1 ping.test_v4
ok 2 ping.test_v6 # SKIP Test requires IPv6 connectivity
# Totals: pass:1 fail:0 xfail:1 xpass:0 skip:0 error:0
Existing devices placed in netns (and using net.config):
$ cat drivers/net/net.config
NETIF=veth0
REMOTE_TYPE=netns
REMOTE_ARGS=red
LOCAL_V4="192.168.1.1"
REMOTE_V4="192.168.1.2"
$ ./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..2
# ok 1 ping.test_v4
# ok 2 ping.test_v6 # SKIP Test requires IPv6 connectivity
# # Totals: pass:1 fail:0 xfail:1 xpass:0 skip:0 error:0
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v4:
- move adding the "require_XYZ()" checks to last patch
---
tools/testing/selftests/drivers/net/Makefile | 5 +++-
tools/testing/selftests/drivers/net/ping.py | 27 ++++++++++++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/drivers/net/ping.py
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index 379cdb1960a7..754ec643768a 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -2,6 +2,9 @@
TEST_INCLUDES := $(wildcard lib/py/*.py)
-TEST_PROGS := stats.py
+TEST_PROGS := \
+ ping.py \
+ stats.py \
+# end of TEST_PROGS
include ../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py
new file mode 100755
index 000000000000..e75908d7c558
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/ping.py
@@ -0,0 +1,27 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import NetDrvEpEnv
+from lib.py import cmd
+
+
+def test_v4(cfg) -> None:
+ cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
+ cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
+
+
+def test_v6(cfg) -> None:
+ cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
+ cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
+
+
+def main() -> None:
+ with NetDrvEpEnv(__file__) as cfg:
+ ksft_run([test_v4, test_v6],
+ args=(cfg, ))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 5/7] selftests: net: support matching cases by name prefix
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (3 preceding siblings ...)
2024-04-20 2:52 ` [PATCH net-next v5 4/7] selftests: drv-net: add a trivial ping test Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 6/7] selftests: drv-net: add a TCP ping test case (and useful helpers) Jakub Kicinski
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
While writing tests with a lot more cases I got tired of having
to jump back and forth to add the name of the test to the ksft_run()
list. Most unittest frameworks do some name matching, e.g. assume
that functions with names starting with test_ are test cases.
Support similar flow in ksft_run(). Let the author list the desired
prefixes. globals() need to be passed explicitly, IDK how to work
around that.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v4:
- spell the code out a little to make it clearer
---
tools/testing/selftests/drivers/net/ping.py | 3 +--
tools/testing/selftests/net/lib/py/ksft.py | 13 ++++++++++++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py
index e75908d7c558..9f65a0764aab 100755
--- a/tools/testing/selftests/drivers/net/ping.py
+++ b/tools/testing/selftests/drivers/net/ping.py
@@ -18,8 +18,7 @@ from lib.py import cmd
def main() -> None:
with NetDrvEpEnv(__file__) as cfg:
- ksft_run([test_v4, test_v6],
- args=(cfg, ))
+ ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
ksft_exit()
diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
index e7f79f6185b0..f84e9fdd0032 100644
--- a/tools/testing/selftests/net/lib/py/ksft.py
+++ b/tools/testing/selftests/net/lib/py/ksft.py
@@ -99,7 +99,18 @@ KSFT_RESULT_ALL = True
print(res)
-def ksft_run(cases, args=()):
+def ksft_run(cases=None, globs=None, case_pfx=None, args=()):
+ cases = cases or []
+
+ if globs and case_pfx:
+ for key, value in globs.items():
+ if not callable(value):
+ continue
+ for prefix in case_pfx:
+ if key.startswith(prefix):
+ cases.append(value)
+ break
+
totals = {"pass": 0, "fail": 0, "skip": 0, "xfail": 0}
print("KTAP version 1")
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 6/7] selftests: drv-net: add a TCP ping test case (and useful helpers)
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (4 preceding siblings ...)
2024-04-20 2:52 ` [PATCH net-next v5 5/7] selftests: net: support matching cases by name prefix Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-20 2:52 ` [PATCH net-next v5 7/7] selftests: drv-net: add require_XYZ() helpers for validating env Jakub Kicinski
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
More complex tests often have to spawn a background process,
like a server which will respond to requests or tcpdump.
Add support for creating such processes using the with keyword:
with bkg("my-daemon", ..):
# my-daemon is alive in this block
My initial thought was to add this support to cmd() directly
but it runs the command in the constructor, so by the time
we __enter__ it's too late to make sure we used "background=True".
Second useful helper transplanted from net_helper.sh is
wait_port_listen().
The test itself uses socat, which insists on v6 addresses
being wrapped in [], it's not the only command which requires
this format, so add the wrapped address to env. The hope
is to save test code from checking if address is v6.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
.../selftests/drivers/net/lib/py/env.py | 5 +++
tools/testing/selftests/drivers/net/ping.py | 21 ++++++++-
tools/testing/selftests/net/lib/py/utils.py | 43 +++++++++++++++++++
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index 579c5b34e6fd..dd5cb0226a31 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -110,6 +110,11 @@ from .remote import Remote
self.addr = self.v6 if self.v6 else self.v4
self.remote_addr = self.remote_v6 if self.remote_v6 else self.remote_v4
+ self.addr_ipver = "6" if self.v6 else "4"
+ # Bracketed addresses, some commands need IPv6 to be inside []
+ self.baddr = f"[{self.v6}]" if self.v6 else self.v4
+ self.remote_baddr = f"[{self.remote_v6}]" if self.remote_v6 else self.remote_v4
+
self.ifname = self.dev['ifname']
self.ifindex = self.dev['ifindex']
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py
index 9f65a0764aab..4b49de59231c 100755
--- a/tools/testing/selftests/drivers/net/ping.py
+++ b/tools/testing/selftests/drivers/net/ping.py
@@ -2,8 +2,9 @@
# SPDX-License-Identifier: GPL-2.0
from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq
from lib.py import NetDrvEpEnv
-from lib.py import cmd
+from lib.py import bkg, cmd, wait_port_listen, rand_port
def test_v4(cfg) -> None:
@@ -16,6 +17,24 @@ from lib.py import cmd
cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
+def test_tcp(cfg) -> None:
+ port = rand_port()
+ listen_cmd = f"socat -{cfg.addr_ipver} -t 2 -u TCP-LISTEN:{port},reuseport STDOUT"
+
+ with bkg(listen_cmd, exit_wait=True) as nc:
+ wait_port_listen(port)
+
+ cmd(f"echo ping | socat -t 2 -u STDIN TCP:{cfg.baddr}:{port}",
+ shell=True, host=cfg.remote)
+ ksft_eq(nc.stdout.strip(), "ping")
+
+ with bkg(listen_cmd, host=cfg.remote, exit_wait=True) as nc:
+ wait_port_listen(port, host=cfg.remote)
+
+ cmd(f"echo ping | socat -t 2 -u STDIN TCP:{cfg.remote_baddr}:{port}", shell=True)
+ ksft_eq(nc.stdout.strip(), "ping")
+
+
def main() -> None:
with NetDrvEpEnv(__file__) as cfg:
ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 7347d0c0ff05..d3715e6c21f2 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -1,7 +1,11 @@
# SPDX-License-Identifier: GPL-2.0
import json as _json
+import random
+import re
import subprocess
+import time
+
class cmd:
def __init__(self, comm, shell=True, fail=True, ns=None, background=False, host=None):
@@ -38,6 +42,20 @@ import subprocess
(self.proc.args, stdout, stderr))
+class bkg(cmd):
+ def __init__(self, comm, shell=True, fail=True, ns=None, host=None,
+ exit_wait=False):
+ super().__init__(comm, background=True,
+ shell=shell, fail=fail, ns=ns, host=host)
+ self.terminate = not exit_wait
+
+ def __enter__(self):
+ return self
+
+ def __exit__(self, ex_type, ex_value, ex_tb):
+ return self.process(terminate=self.terminate)
+
+
def ip(args, json=None, ns=None, host=None):
cmd_str = "ip "
if json:
@@ -47,3 +65,28 @@ import subprocess
if json:
return _json.loads(cmd_obj.stdout)
return cmd_obj
+
+
+def rand_port():
+ """
+ Get unprivileged port, for now just random, one day we may decide to check if used.
+ """
+ return random.randint(1024, 65535)
+
+
+def wait_port_listen(port, proto="tcp", ns=None, host=None, sleep=0.005, deadline=5):
+ end = time.monotonic() + deadline
+
+ pattern = f":{port:04X} .* "
+ if proto == "tcp": # for tcp protocol additionally check the socket state
+ pattern += "0A"
+ pattern = re.compile(pattern)
+
+ while True:
+ data = cmd(f'cat /proc/net/{proto}*', ns=ns, host=host, shell=True).stdout
+ for row in data.split("\n"):
+ if pattern.search(row):
+ return
+ if time.monotonic() > end:
+ raise Exception("Waiting for port listen timed out")
+ time.sleep(sleep)
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v5 7/7] selftests: drv-net: add require_XYZ() helpers for validating env
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (5 preceding siblings ...)
2024-04-20 2:52 ` [PATCH net-next v5 6/7] selftests: drv-net: add a TCP ping test case (and useful helpers) Jakub Kicinski
@ 2024-04-20 2:52 ` Jakub Kicinski
2024-04-21 14:59 ` Willem de Bruijn
2024-04-21 14:47 ` [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Willem de Bruijn
2024-04-23 17:20 ` patchwork-bot+netdevbpf
8 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-20 2:52 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Wrap typical checks like whether given command used by the test
is available in helpers.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
.../selftests/drivers/net/lib/py/env.py | 29 ++++++++++++++++++-
tools/testing/selftests/drivers/net/ping.py | 6 ++++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index dd5cb0226a31..a3db1bb1afeb 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -3,7 +3,8 @@
import os
import shlex
from pathlib import Path
-from lib.py import ip
+from lib.py import KsftSkipEx
+from lib.py import cmd, ip
from lib.py import NetNS, NetdevSimDev
from .remote import Remote
@@ -118,6 +119,8 @@ from .remote import Remote
self.ifname = self.dev['ifname']
self.ifindex = self.dev['ifindex']
+ self._required_cmd = {}
+
def create_local(self):
self._netns = NetNS()
self._ns = NetdevSimDev()
@@ -160,3 +163,27 @@ from .remote import Remote
if self.remote:
del self.remote
self.remote = None
+
+ def require_v4(self):
+ if not self.v4 or not self.remote_v4:
+ raise KsftSkipEx("Test requires IPv4 connectivity")
+
+ def require_v6(self):
+ if not self.v6 or not self.remote_v6:
+ raise KsftSkipEx("Test requires IPv6 connectivity")
+
+ def _require_cmd(self, comm, key, host=None):
+ cached = self._required_cmd.get(comm, {})
+ if cached.get(key) is None:
+ cached[key] = cmd("command -v -- " + comm, fail=False,
+ shell=True, host=host).ret == 0
+ self._required_cmd[comm] = cached
+ return cached[key]
+
+ def require_cmd(self, comm, local=True, remote=False):
+ if local:
+ if not self._require_cmd(comm, "local"):
+ raise KsftSkipEx("Test requires command: " + comm)
+ if remote:
+ if not self._require_cmd(comm, "remote"):
+ raise KsftSkipEx("Test requires (remote) command: " + comm)
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py
index 4b49de59231c..eb83e7b48797 100755
--- a/tools/testing/selftests/drivers/net/ping.py
+++ b/tools/testing/selftests/drivers/net/ping.py
@@ -8,16 +8,22 @@ from lib.py import bkg, cmd, wait_port_listen, rand_port
def test_v4(cfg) -> None:
+ cfg.require_v4()
+
cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
def test_v6(cfg) -> None:
+ cfg.require_v6()
+
cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
def test_tcp(cfg) -> None:
+ cfg.require_cmd("socat", remote=True)
+
port = rand_port()
listen_cmd = f"socat -{cfg.addr_ipver} -t 2 -u TCP-LISTEN:{port},reuseport STDOUT"
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v5 7/7] selftests: drv-net: add require_XYZ() helpers for validating env
2024-04-20 2:52 ` [PATCH net-next v5 7/7] selftests: drv-net: add require_XYZ() helpers for validating env Jakub Kicinski
@ 2024-04-21 14:59 ` Willem de Bruijn
0 siblings, 0 replies; 16+ messages in thread
From: Willem de Bruijn @ 2024-04-21 14:59 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Jakub Kicinski wrote:
> Wrap typical checks like whether given command used by the test
> is available in helpers.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> def test_v4(cfg) -> None:
> + cfg.require_v4()
> +
Probably every platform has ping. But since it is not a built-int and
this patch adds cfg.require_cmd, maybe add it for ping if respinning.
> cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
> cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
>
>
> def test_v6(cfg) -> None:
> + cfg.require_v6()
> +
> cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
> cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
>
>
> def test_tcp(cfg) -> None:
> + cfg.require_cmd("socat", remote=True)
> +
> port = rand_port()
> listen_cmd = f"socat -{cfg.addr_ipver} -t 2 -u TCP-LISTEN:{port},reuseport STDOUT"
>
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (6 preceding siblings ...)
2024-04-20 2:52 ` [PATCH net-next v5 7/7] selftests: drv-net: add require_XYZ() helpers for validating env Jakub Kicinski
@ 2024-04-21 14:47 ` Willem de Bruijn
2024-04-23 17:57 ` Willem de Bruijn
2024-04-23 17:20 ` patchwork-bot+netdevbpf
8 siblings, 1 reply; 16+ messages in thread
From: Willem de Bruijn @ 2024-04-21 14:47 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Jakub Kicinski wrote:
> Hi!
>
> Implement support for tests which require access to a remote system /
> endpoint which can generate traffic.
> This series concludes the "groundwork" for upstream driver tests.
>
> I wanted to support the three models which came up in discussions:
> - SW testing with netdevsim
> - "local" testing with two ports on the same system in a loopback
> - "remote" testing via SSH
> so there is a tiny bit of an abstraction which wraps up how "remote"
> commands are executed. Otherwise hopefully there's nothing surprising.
>
> I'm only adding a ping test. I had a bigger one written but I was
> worried we'll get into discussing the details of the test itself
> and how I chose to hack up netdevsim, instead of the test infra...
> So that test will be a follow up :)
>
> v5:
> - fix rand port generation, and wrap it in a helper in case
> the random thing proves to be flaky
> - reuseaddr
> - explicitly select the address family
> v4: https://lore.kernel.org/all/20240418233844.2762396-1-kuba@kernel.org
> - improve coding style of patch 5
> - switch from netcat to socat (patch 6)
> - support exit_wait for bkg() in context manager
> - add require_XYZ() helpers (patch 7)
> - increase timeouts a little (1,3 -> 5 sec)
> v3: https://lore.kernel.org/all/20240417231146.2435572-1-kuba@kernel.org
> - first two patches are new
> - make Remote::cmd() return Popen() object (patch 3)
> - always operate on absolute paths (patch 3)
> - last two patches are new
> v2: https://lore.kernel.org/all/20240416004556.1618804-1-kuba@kernel.org
> - rename endpoint -> remote
> - use 2001:db8:: v6 prefix
> - add a note about persistent SSH connections
> - add the kernel config
> v1: https://lore.kernel.org/all/20240412233705.1066444-1-kuba@kernel.org
>
> Jakub Kicinski (7):
> selftests: drv-net: define endpoint structures
> selftests: drv-net: factor out parsing of the env
> selftests: drv-net: construct environment for running tests which
> require an endpoint
> selftests: drv-net: add a trivial ping test
> selftests: net: support matching cases by name prefix
> selftests: drv-net: add a TCP ping test case (and useful helpers)
> selftests: drv-net: add require_XYZ() helpers for validating env
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-21 14:47 ` [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Willem de Bruijn
@ 2024-04-23 17:57 ` Willem de Bruijn
2024-04-24 1:53 ` Jakub Kicinski
0 siblings, 1 reply; 16+ messages in thread
From: Willem de Bruijn @ 2024-04-23 17:57 UTC (permalink / raw)
To: Willem de Bruijn, Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel, Jakub Kicinski
Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > Hi!
> >
> > Implement support for tests which require access to a remote system /
> > endpoint which can generate traffic.
> > This series concludes the "groundwork" for upstream driver tests.
> >
> > I wanted to support the three models which came up in discussions:
> > - SW testing with netdevsim
> > - "local" testing with two ports on the same system in a loopback
> > - "remote" testing via SSH
> > so there is a tiny bit of an abstraction which wraps up how "remote"
> > commands are executed. Otherwise hopefully there's nothing surprising.
> >
> > I'm only adding a ping test. I had a bigger one written but I was
> > worried we'll get into discussing the details of the test itself
> > and how I chose to hack up netdevsim, instead of the test infra...
> > So that test will be a follow up :)
> >
> > v5:
> > - fix rand port generation, and wrap it in a helper in case
> > the random thing proves to be flaky
> > - reuseaddr
> > - explicitly select the address family
> > v4: https://lore.kernel.org/all/20240418233844.2762396-1-kuba@kernel.org
> > - improve coding style of patch 5
> > - switch from netcat to socat (patch 6)
> > - support exit_wait for bkg() in context manager
> > - add require_XYZ() helpers (patch 7)
> > - increase timeouts a little (1,3 -> 5 sec)
> > v3: https://lore.kernel.org/all/20240417231146.2435572-1-kuba@kernel.org
> > - first two patches are new
> > - make Remote::cmd() return Popen() object (patch 3)
> > - always operate on absolute paths (patch 3)
> > - last two patches are new
> > v2: https://lore.kernel.org/all/20240416004556.1618804-1-kuba@kernel.org
> > - rename endpoint -> remote
> > - use 2001:db8:: v6 prefix
> > - add a note about persistent SSH connections
> > - add the kernel config
> > v1: https://lore.kernel.org/all/20240412233705.1066444-1-kuba@kernel.org
> >
> > Jakub Kicinski (7):
> > selftests: drv-net: define endpoint structures
> > selftests: drv-net: factor out parsing of the env
> > selftests: drv-net: construct environment for running tests which
> > require an endpoint
> > selftests: drv-net: add a trivial ping test
> > selftests: net: support matching cases by name prefix
> > selftests: drv-net: add a TCP ping test case (and useful helpers)
> > selftests: drv-net: add require_XYZ() helpers for validating env
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
Too late, but
Tested-by: Willem de Bruijn <willemb@google.com>
I tried this yesterday on a pair of Google cloud instances. In
anticipation of converting some of my tests, like csum to this.
Only possible non-obvious observation is that some kselftests expect
as root, and the ssh remote logic extends that to expecting ssh
root access to the remote host.
Would it make sense to explicitly add sudo for all privileged
operations, to allow for non-root ssh and scp?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-23 17:57 ` Willem de Bruijn
@ 2024-04-24 1:53 ` Jakub Kicinski
2024-04-24 14:13 ` Willem de Bruijn
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-24 1:53 UTC (permalink / raw)
To: Willem de Bruijn
Cc: davem, netdev, edumazet, pabeni, shuah, petrm, linux-kselftest
On Tue, 23 Apr 2024 13:57:45 -0400 Willem de Bruijn wrote:
> Only possible non-obvious observation is that some kselftests expect
> as root, and the ssh remote logic extends that to expecting ssh
> root access to the remote host.
>
> Would it make sense to explicitly add sudo for all privileged
> operations, to allow for non-root ssh and scp?
I haven't thought about this part much, TBH. I'm not aware of any
scheme used in other tests.
IIUC the problem is that we need root locally, and then try to SSH
over to remote. But normally the SSH keys belong to the non-root
user, so SSH'ing as root is annoying?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-24 1:53 ` Jakub Kicinski
@ 2024-04-24 14:13 ` Willem de Bruijn
2024-04-24 18:57 ` Jakub Kicinski
0 siblings, 1 reply; 16+ messages in thread
From: Willem de Bruijn @ 2024-04-24 14:13 UTC (permalink / raw)
To: Jakub Kicinski, Willem de Bruijn
Cc: davem, netdev, edumazet, pabeni, shuah, petrm, linux-kselftest
Jakub Kicinski wrote:
> On Tue, 23 Apr 2024 13:57:45 -0400 Willem de Bruijn wrote:
> > Only possible non-obvious observation is that some kselftests expect
> > as root, and the ssh remote logic extends that to expecting ssh
> > root access to the remote host.
> >
> > Would it make sense to explicitly add sudo for all privileged
> > operations, to allow for non-root ssh and scp?
>
> I haven't thought about this part much, TBH. I'm not aware of any
> scheme used in other tests.
> IIUC the problem is that we need root locally, and then try to SSH
> over to remote. But normally the SSH keys belong to the non-root
> user, so SSH'ing as root is annoying?
Yeah. It requires "PermitRootLogin yes" in your sshd_config and
installing root keys.
It's not a huge issue, but if we do want to fix it, doing so will be
easier early rather than when more tests are added with implicit
dependency on having root.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-24 14:13 ` Willem de Bruijn
@ 2024-04-24 18:57 ` Jakub Kicinski
2024-04-24 20:59 ` Willem de Bruijn
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2024-04-24 18:57 UTC (permalink / raw)
To: Willem de Bruijn
Cc: davem, netdev, edumazet, pabeni, shuah, petrm, linux-kselftest
On Wed, 24 Apr 2024 10:13:41 -0400 Willem de Bruijn wrote:
> > I haven't thought about this part much, TBH. I'm not aware of any
> > scheme used in other tests.
> > IIUC the problem is that we need root locally, and then try to SSH
> > over to remote. But normally the SSH keys belong to the non-root
> > user, so SSH'ing as root is annoying?
>
> Yeah. It requires "PermitRootLogin yes" in your sshd_config and
> installing root keys.
>
> It's not a huge issue, but if we do want to fix it, doing so will be
> easier early rather than when more tests are added with implicit
> dependency on having root.
You know what, we need a diagram. We currently expect:
------------ -------------
| | | |
| Local user | ---------->| Remote user |
| | / | |
------------ / -------------
/
/
------------ / -------------
| >*exec*< | / | |
| Local root |-------------U---------------->| Remote root |
| | ? | |
------------ -------------
We run locally as root. Putting sudo on all local commands would be
annoying.
On remote we don't currently explicitly say whether we need root.
The user is basically implicitly controlled by the REMOTE_ARGS
and ssh config.
REMOTE_ARGS="john@machine"
will make us log in as john. But *from* root, so pub key of root needs
to be deployed.
We can support:
------------ -------------
| | | |
| Local user | ? | Remote user |
| ,--------------------U-------------->| |
------/-----` \ -------------
| ?su back to user? \
| \
------------ \ -------------
| >*exec*< | \ | |
| Local root | `--------->| Remote root |
| | | |
------------ -------------
but it's unclear whether that's all you're asking for, or also:
------------ -------------
| | | |
| Local user | | Remote user |
| ,----------------------------------->->?cond sudo? |
------/-----` -----|-------
| su back to user |
| |
------------ -----v-------
| >*exec*< | | |
| Local root | | Remote root |
| | | |
------------ -------------
which would require us to annotate privileged remote commands.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-24 18:57 ` Jakub Kicinski
@ 2024-04-24 20:59 ` Willem de Bruijn
0 siblings, 0 replies; 16+ messages in thread
From: Willem de Bruijn @ 2024-04-24 20:59 UTC (permalink / raw)
To: Jakub Kicinski, Willem de Bruijn
Cc: davem, netdev, edumazet, pabeni, shuah, petrm, linux-kselftest
Jakub Kicinski wrote:
> On Wed, 24 Apr 2024 10:13:41 -0400 Willem de Bruijn wrote:
> > > I haven't thought about this part much, TBH. I'm not aware of any
> > > scheme used in other tests.
> > > IIUC the problem is that we need root locally, and then try to SSH
> > > over to remote. But normally the SSH keys belong to the non-root
> > > user, so SSH'ing as root is annoying?
> >
> > Yeah. It requires "PermitRootLogin yes" in your sshd_config and
> > installing root keys.
> >
> > It's not a huge issue, but if we do want to fix it, doing so will be
> > easier early rather than when more tests are added with implicit
> > dependency on having root.
>
> You know what, we need a diagram. We currently expect:
>
>
> ------------ -------------
> | | | |
> | Local user | ---------->| Remote user |
> | | / | |
> ------------ / -------------
> /
> /
> ------------ / -------------
> | >*exec*< | / | |
> | Local root |-------------U---------------->| Remote root |
> | | ? | |
> ------------ -------------
>
>
> We run locally as root. Putting sudo on all local commands would be
> annoying.
>
> On remote we don't currently explicitly say whether we need root.
> The user is basically implicitly controlled by the REMOTE_ARGS
> and ssh config.
>
> REMOTE_ARGS="john@machine"
>
> will make us log in as john. But *from* root, so pub key of root needs
> to be deployed.
>
> We can support:
>
> ------------ -------------
> | | | |
> | Local user | ? | Remote user |
> | ,--------------------U-------------->| |
> ------/-----` \ -------------
> | ?su back to user? \
> | \
> ------------ \ -------------
> | >*exec*< | \ | |
> | Local root | `--------->| Remote root |
> | | | |
> ------------ -------------
>
> but it's unclear whether that's all you're asking for, or also:
>
> ------------ -------------
> | | | |
> | Local user | | Remote user |
> | ,----------------------------------->->?cond sudo? |
> ------/-----` -----|-------
> | su back to user |
> | |
> ------------ -----v-------
> | >*exec*< | | |
> | Local root | | Remote root |
> | | | |
> ------------ -------------
>
> which would require us to annotate privileged remote commands.
For many tests the peer traffic generator/sink will not need to be
root.
But I already see some counter-examples, such as the PF_PACKET
packet generation on the transmitter for checksum receive tests.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system
2024-04-20 2:52 [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Jakub Kicinski
` (7 preceding siblings ...)
2024-04-21 14:47 ` [PATCH net-next v5 0/7] selftests: drv-net: support testing with a remote system Willem de Bruijn
@ 2024-04-23 17:20 ` patchwork-bot+netdevbpf
8 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-23 17:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, shuah, petrm, linux-kselftest,
willemdebruijn.kernel
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 19 Apr 2024 19:52:30 -0700 you wrote:
> Hi!
>
> Implement support for tests which require access to a remote system /
> endpoint which can generate traffic.
> This series concludes the "groundwork" for upstream driver tests.
>
> I wanted to support the three models which came up in discussions:
> - SW testing with netdevsim
> - "local" testing with two ports on the same system in a loopback
> - "remote" testing via SSH
> so there is a tiny bit of an abstraction which wraps up how "remote"
> commands are executed. Otherwise hopefully there's nothing surprising.
>
> [...]
Here is the summary with links:
- [net-next,v5,1/7] selftests: drv-net: define endpoint structures
https://git.kernel.org/netdev/net-next/c/1a20a9a0ddef
- [net-next,v5,2/7] selftests: drv-net: factor out parsing of the env
https://git.kernel.org/netdev/net-next/c/543389295085
- [net-next,v5,3/7] selftests: drv-net: construct environment for running tests which require an endpoint
https://git.kernel.org/netdev/net-next/c/1880f272d2f9
- [net-next,v5,4/7] selftests: drv-net: add a trivial ping test
https://git.kernel.org/netdev/net-next/c/a48a87c08664
- [net-next,v5,5/7] selftests: net: support matching cases by name prefix
https://git.kernel.org/netdev/net-next/c/01b431641c33
- [net-next,v5,6/7] selftests: drv-net: add a TCP ping test case (and useful helpers)
https://git.kernel.org/netdev/net-next/c/31611cea8f0f
- [net-next,v5,7/7] selftests: drv-net: add require_XYZ() helpers for validating env
https://git.kernel.org/netdev/net-next/c/f1e68a1a4a40
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 16+ messages in thread