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

[Paolo Abeni <pabeni@redhat.com>]

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