| Message ID | 20240420025237.3309296-1-kuba@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests: drv-net: support testing with a remote system | expand |
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>
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!
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?
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?
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.
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.
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.
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 tools/testing/selftests/drivers/net/Makefile | 5 +- .../testing/selftests/drivers/net/README.rst | 33 ++++ .../selftests/drivers/net/lib/py/__init__.py | 1 + .../selftests/drivers/net/lib/py/env.py | 175 ++++++++++++++++-- .../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/drivers/net/ping.py | 51 +++++ .../testing/selftests/net/lib/py/__init__.py | 1 + tools/testing/selftests/net/lib/py/ksft.py | 13 +- tools/testing/selftests/net/lib/py/netns.py | 31 ++++ tools/testing/selftests/net/lib/py/utils.py | 60 +++++- 12 files changed, 416 insertions(+), 29 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 create mode 100755 tools/testing/selftests/drivers/net/ping.py create mode 100644 tools/testing/selftests/net/lib/py/netns.py