diff mbox series

[net-next,v3,3/4] selftests: drv-net: store addresses in dict indexed by ipver

Message ID 20250217194200.3011136-4-kuba@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series selftests: drv-net: add a simple TSO test | 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: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 26 (+1) this patch: 26 (+1)
netdev/cc_maintainers warning 5 maintainers not CCed: shuah@kernel.org linux-kselftest@vger.kernel.org sdf@fomichev.me jdamato@fastly.com almasrymina@google.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
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: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 216 lines checked
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 fail net-next-2025-02-18--03-00 (tests: 891)

Commit Message

Jakub Kicinski Feb. 17, 2025, 7:41 p.m. UTC
Looks like more and more tests want to iterate over IP version,
run the same test over ipv4 and ipv6. The current naming of
members in the env class makes it a bit awkward, we have
separate members for ipv4 and ipv6 parameters.

Store the parameters inside dicts, so that tests can easily
index them with ip version.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v3:
 - new patch
---
 .../testing/selftests/drivers/net/hw/csum.py  | 48 +++++++------------
 .../selftests/drivers/net/hw/devmem.py        |  6 +--
 .../selftests/drivers/net/lib/py/env.py       | 47 +++++++++---------
 tools/testing/selftests/drivers/net/ping.py   | 12 ++---
 4 files changed, 50 insertions(+), 63 deletions(-)

Comments

Willem de Bruijn Feb. 18, 2025, 1:15 a.m. UTC | #1
Jakub Kicinski wrote:
> Looks like more and more tests want to iterate over IP version,
> run the same test over ipv4 and ipv6. The current naming of
> members in the env class makes it a bit awkward, we have
> separate members for ipv4 and ipv6 parameters.
> 
> Store the parameters inside dicts, so that tests can easily
> index them with ip version.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

> +++ b/tools/testing/selftests/drivers/net/ping.py
> @@ -8,17 +8,17 @@ from lib.py import bkg, cmd, wait_port_listen, rand_port
>  
>  
>  def test_v4(cfg) -> None:
> -    cfg.require_v4()
> +    cfg.require_ipver("4")
>  
> -    cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
> -    cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
> +    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> +    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["4"]}", host=cfg.remote)

Here and below, intended to use single quote around constant?

>  
>  def test_v6(cfg) -> None:
> -    cfg.require_v6()
> +    cfg.require_ipver("6")
>  
> -    cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
> -    cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
> +    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["6"]}")
> +    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["6"]}", host=cfg.remote)
>  
>  
>  def test_tcp(cfg) -> None:
> -- 
> 2.48.1
>
Stanislav Fomichev Feb. 18, 2025, 4:21 a.m. UTC | #2
On 02/17, Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > Looks like more and more tests want to iterate over IP version,
> > run the same test over ipv4 and ipv6. The current naming of
> > members in the env class makes it a bit awkward, we have
> > separate members for ipv4 and ipv6 parameters.
> > 
> > Store the parameters inside dicts, so that tests can easily
> > index them with ip version.
> > 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> > +++ b/tools/testing/selftests/drivers/net/ping.py
> > @@ -8,17 +8,17 @@ from lib.py import bkg, cmd, wait_port_listen, rand_port
> >  
> >  
> >  def test_v4(cfg) -> None:
> > -    cfg.require_v4()
> > +    cfg.require_ipver("4")
> >  
> > -    cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
> > -    cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
> > +    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> > +    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["4"]}", host=cfg.remote)
> 
> Here and below, intended to use single quote around constant?

Let's kick it off the testing queue as well..

# overriding timeout to 90
# selftests: drivers/net: ping.py
#   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./ping.py", line 13
#     cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
#                                               ^
# SyntaxError: f-string: unmatched '['

---
pw-bot: cr
Jakub Kicinski Feb. 18, 2025, 2:48 p.m. UTC | #3
On Mon, 17 Feb 2025 20:21:56 -0800 Stanislav Fomichev wrote:
> > >  def test_v4(cfg) -> None:
> > > -    cfg.require_v4()
> > > +    cfg.require_ipver("4")
> > >  
> > > -    cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
> > > -    cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
> > > +    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> > > +    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["4"]}", host=cfg.remote)  
> > 
> > Here and below, intended to use single quote around constant?  
> 
> Let's kick it off the testing queue as well..
> 
> # overriding timeout to 90
> # selftests: drivers/net: ping.py
> #   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./ping.py", line 13
> #     cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> #                                               ^
> # SyntaxError: f-string: unmatched '['

Huh, it worked for me locally, must be a python version thing..

Python 3.13.2

>>> a={"a": ' '}
>>> f"test{a["a"]}test"
'test test'
Willem de Bruijn Feb. 18, 2025, 3:03 p.m. UTC | #4
Jakub Kicinski wrote:
> On Mon, 17 Feb 2025 20:21:56 -0800 Stanislav Fomichev wrote:
> > > >  def test_v4(cfg) -> None:
> > > > -    cfg.require_v4()
> > > > +    cfg.require_ipver("4")
> > > >  
> > > > -    cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
> > > > -    cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
> > > > +    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> > > > +    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["4"]}", host=cfg.remote)  
> > > 
> > > Here and below, intended to use single quote around constant?  
> > 
> > Let's kick it off the testing queue as well..
> > 
> > # overriding timeout to 90
> > # selftests: drivers/net: ping.py
> > #   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./ping.py", line 13
> > #     cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
> > #                                               ^
> > # SyntaxError: f-string: unmatched '['
> 
> Huh, it worked for me locally, must be a python version thing..
> 
> Python 3.13.2
> 
> >>> a={"a": ' '}
> >>> f"test{a["a"]}test"
> 'test test'

Failed for me on 3.11.

Apparently this was indeed addressed in 3.12:

https://realpython.com/python-f-strings/#using-quotation-marks
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/hw/csum.py b/tools/testing/selftests/drivers/net/hw/csum.py
index cd477f3440ca..701aca1361e0 100755
--- a/tools/testing/selftests/drivers/net/hw/csum.py
+++ b/tools/testing/selftests/drivers/net/hw/csum.py
@@ -9,15 +9,12 @@  from lib.py import ksft_run, ksft_exit, KsftSkipEx
 from lib.py import EthtoolFamily, NetDrvEpEnv
 from lib.py import bkg, cmd, wait_port_listen
 
-def test_receive(cfg, ipv4=False, extra_args=None):
+def test_receive(cfg, ipver="6", extra_args=None):
     """Test local nic checksum receive. Remote host sends crafted packets."""
     if not cfg.have_rx_csum:
         raise KsftSkipEx(f"Test requires rx checksum offload on {cfg.ifname}")
 
-    if ipv4:
-        ip_args = f"-4 -S {cfg.remote_v4} -D {cfg.v4}"
-    else:
-        ip_args = f"-6 -S {cfg.remote_v6} -D {cfg.v6}"
+    ip_args = f"-{ipver} -S {cfg.remote_addr_v[ipver]} -D {cfg.addr_v[ipver]}"
 
     rx_cmd = f"{cfg.bin_local} -i {cfg.ifname} -n 100 {ip_args} -r 1 -R {extra_args}"
     tx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -n 100 {ip_args} -r 1 -T {extra_args}"
@@ -27,17 +24,14 @@  from lib.py import bkg, cmd, wait_port_listen
         cmd(tx_cmd, host=cfg.remote)
 
 
-def test_transmit(cfg, ipv4=False, extra_args=None):
+def test_transmit(cfg, ipver="6", extra_args=None):
     """Test local nic checksum transmit. Remote host verifies packets."""
     if (not cfg.have_tx_csum_generic and
-        not (cfg.have_tx_csum_ipv4 and ipv4) and
-        not (cfg.have_tx_csum_ipv6 and not ipv4)):
+        not (cfg.have_tx_csum_ipv4 and ipver == "4") and
+        not (cfg.have_tx_csum_ipv6 and ipver == "6")):
         raise KsftSkipEx(f"Test requires tx checksum offload on {cfg.ifname}")
 
-    if ipv4:
-        ip_args = f"-4 -S {cfg.v4} -D {cfg.remote_v4}"
-    else:
-        ip_args = f"-6 -S {cfg.v6} -D {cfg.remote_v6}"
+    ip_args = f"-{ipver} -S {cfg.addr_v[ipver]} -D {cfg.remote_addr_v[ipver]}"
 
     # Cannot randomize input when calculating zero checksum
     if extra_args != "-U -Z":
@@ -51,26 +45,20 @@  from lib.py import bkg, cmd, wait_port_listen
         cmd(tx_cmd)
 
 
-def test_builder(name, cfg, ipv4=False, tx=False, extra_args=""):
+def test_builder(name, cfg, ipver="6", tx=False, extra_args=""):
     """Construct specific tests from the common template.
 
        Most tests follow the same basic pattern, differing only in
        Direction of the test and optional flags passed to csum."""
     def f(cfg):
-        if ipv4:
-            cfg.require_v4()
-        else:
-            cfg.require_v6()
+        cfg.require_ipver(ipver)
 
         if tx:
-            test_transmit(cfg, ipv4, extra_args)
+            test_transmit(cfg, ipver, extra_args)
         else:
-            test_receive(cfg, ipv4, extra_args)
+            test_receive(cfg, ipver, extra_args)
 
-    if ipv4:
-        f.__name__ = "ipv4_" + name
-    else:
-        f.__name__ = "ipv6_" + name
+    f.__name__ = f"ipv{ipver}_" + name
     return f
 
 
@@ -104,15 +92,15 @@  from lib.py import bkg, cmd, wait_port_listen
         cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)
 
         cases = []
-        for ipv4 in [True, False]:
-            cases.append(test_builder("rx_tcp", cfg, ipv4, False, "-t"))
-            cases.append(test_builder("rx_tcp_invalid", cfg, ipv4, False, "-t -E"))
+        for ipver in ["4", "6"]:
+            cases.append(test_builder("rx_tcp", cfg, ipver, False, "-t"))
+            cases.append(test_builder("rx_tcp_invalid", cfg, ipver, False, "-t -E"))
 
-            cases.append(test_builder("rx_udp", cfg, ipv4, False, ""))
-            cases.append(test_builder("rx_udp_invalid", cfg, ipv4, False, "-E"))
+            cases.append(test_builder("rx_udp", cfg, ipver, False, ""))
+            cases.append(test_builder("rx_udp_invalid", cfg, ipver, False, "-E"))
 
-            cases.append(test_builder("tx_udp_csum_offload", cfg, ipv4, True, "-U"))
-            cases.append(test_builder("tx_udp_zero_checksum", cfg, ipv4, True, "-U -Z"))
+            cases.append(test_builder("tx_udp_csum_offload", cfg, ipver, True, "-U"))
+            cases.append(test_builder("tx_udp_zero_checksum", cfg, ipver, True, "-U -Z"))
 
         ksft_run(cases=cases, args=(cfg, ))
     ksft_exit()
diff --git a/tools/testing/selftests/drivers/net/hw/devmem.py b/tools/testing/selftests/drivers/net/hw/devmem.py
index 1223f0f5c10c..3947e9157115 100755
--- a/tools/testing/selftests/drivers/net/hw/devmem.py
+++ b/tools/testing/selftests/drivers/net/hw/devmem.py
@@ -21,15 +21,15 @@  from lib.py import ksft_disruptive
 
 @ksft_disruptive
 def check_rx(cfg) -> None:
-    cfg.require_v6()
+    cfg.require_ipver("6")
     require_devmem(cfg)
 
     port = rand_port()
-    listen_cmd = f"./ncdevmem -l -f {cfg.ifname} -s {cfg.v6} -p {port}"
+    listen_cmd = f"./ncdevmem -l -f {cfg.ifname} -s {cfg.addr_v['6']} -p {port}"
 
     with bkg(listen_cmd) as socat:
         wait_port_listen(port)
-        cmd(f"echo -e \"hello\\nworld\"| socat -u - TCP6:[{cfg.v6}]:{port}", host=cfg.remote, shell=True)
+        cmd(f"echo -e \"hello\\nworld\"| socat -u - TCP6:[{cfg.addr_v['6']}]:{port}", host=cfg.remote, shell=True)
 
     ksft_eq(socat.stdout.strip(), "hello\nworld")
 
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index 128acff4f753..96b33b5ef9dd 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -113,6 +113,9 @@  from .remote import Remote
         self._ns = None
         self._ns_peer = None
 
+        self.addr_v        = { "4": None, "6": None }
+        self.remote_addr_v = { "4": None, "6": None }
+
         if "NETIF" in self.env:
             if nsim_test is True:
                 raise KsftXfailEx("Test only works on netdevsim")
@@ -120,10 +123,10 @@  from .remote import Remote
 
             self.dev = ip("-d 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")
+            self.addr_v["4"] = self.env.get("LOCAL_V4")
+            self.addr_v["6"] = self.env.get("LOCAL_V6")
+            self.remote_addr_v["4"] = self.env.get("REMOTE_V4")
+            self.remote_addr_v["6"] = self.env.get("REMOTE_V6")
             kind = self.env["REMOTE_TYPE"]
             args = self.env["REMOTE_ARGS"]
         else:
@@ -134,22 +137,22 @@  from .remote import Remote
 
             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"
+            self.addr_v["4"] = self.nsim_v4_pfx + "1"
+            self.addr_v["6"] = self.nsim_v6_pfx + "1"
+            self.remote_addr_v["4"] = self.nsim_v4_pfx + "2"
+            self.remote_addr_v["6"] = 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.addr_ipver = "6" if self.addr_v["6"] else "4"
+        self.addr = self.addr_v[self.addr_ipver]
+        self.remote_addr = self.remote_addr_v[self.addr_ipver]
 
-        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.baddr = f"[{self.addr_v['6']}]" if self.addr_v["6"] else self.addr_v["4"]
+        self.remote_baddr = f"[{self.remote_addr_v['6']}]" if self.remote_addr_v["6"] else self.remote_addr_v["4"]
 
         self.ifname = self.dev['ifname']
         self.ifindex = self.dev['ifindex']
@@ -205,10 +208,10 @@  from .remote import Remote
 
     def resolve_remote_ifc(self):
         v4 = v6 = None
-        if self.remote_v4:
-            v4 = ip("addr show to " + self.remote_v4, json=True, host=self.remote)
-        if self.remote_v6:
-            v6 = ip("addr show to " + self.remote_v6, json=True, host=self.remote)
+        if self.remote_addr_v["4"]:
+            v4 = ip("addr show to " + self.remote_addr_v["4"], json=True, host=self.remote)
+        if self.remote_addr_v["6"]:
+            v6 = ip("addr show to " + self.remote_addr_v["6"], json=True, host=self.remote)
         if v4 and v6 and v4[0]["ifname"] != v6[0]["ifname"]:
             raise Exception("Can't resolve remote interface name, v4 and v6 don't match")
         if (v4 and len(v4) > 1) or (v6 and len(v6) > 1):
@@ -238,13 +241,9 @@  from .remote import 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_ipver(self, ipver):
+        if not self.addr_v[ipver] or not self.remote_addr_v[ipver]:
+            raise KsftSkipEx(f"Test requires IPv{ipver} connectivity")
 
     def _require_cmd(self, comm, key, host=None):
         cached = self._required_cmd.get(comm, {})
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py
index eb83e7b48797..6c5c21cb7265 100755
--- a/tools/testing/selftests/drivers/net/ping.py
+++ b/tools/testing/selftests/drivers/net/ping.py
@@ -8,17 +8,17 @@  from lib.py import bkg, cmd, wait_port_listen, rand_port
 
 
 def test_v4(cfg) -> None:
-    cfg.require_v4()
+    cfg.require_ipver("4")
 
-    cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
-    cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
+    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["4"]}")
+    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["4"]}", host=cfg.remote)
 
 
 def test_v6(cfg) -> None:
-    cfg.require_v6()
+    cfg.require_ipver("6")
 
-    cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
-    cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
+    cmd(f"ping -c 1 -W0.5 {cfg.remote_addr_v["6"]}")
+    cmd(f"ping -c 1 -W0.5 {cfg.addr_v["6"]}", host=cfg.remote)
 
 
 def test_tcp(cfg) -> None: