diff mbox series

[net-next] selftests: drv-net: add checksum tests

Message ID 20240501185432.3593168-1-willemdebruijn.kernel@gmail.com
State New
Headers show
Series [net-next] selftests: drv-net: add checksum tests | expand

Commit Message

Willem de Bruijn May 1, 2024, 6:51 p.m. UTC
From: Willem de Bruijn <willemb@google.com>

Run tools/testing/selftest/net/csum.c as part of drv-net.
This binary covers multiple scenarios, based on arguments given,
for both IPv4 and IPv6:

- Accept UDP correct checksum
- Detect UDP invalid checksum
- Accept TCP correct checksum
- Detect TCP invalid checksum

- Transmit UDP: basic checksum offload
- Transmit UDP: zero checksum conversion

The test direction is reversed between receive and transmit tests, so
that the NIC under test is always the local machine.

In total this adds up to 12 testcases, with more to follow. For
conciseness, I replaced individual functions with a function factory.
It saves a lot of boilerplate, but is a little harder to follow, so
partially here as a point for discussion.

Also detect hardware offload feature availability using Ethtool
netlink and skip tests when either feature is off. This need may be
common for offload feature tests and eventually deserving of a thin
wrapper in lib.py.

Missing are the PF_PACKET based send tests ('-P'). These use
virtio_net_hdr to program hardware checksum offload. Which requires
looking up the local MAC address and (harder) the MAC of the next hop.
I'll have to give it some though how to do that robustly and where
that code would belong.

Tested on Google cloud:

        make -C tools/testing/selftests/ \
                TARGETS="drivers/net drivers/net/hw net" \
                install INSTALL_PATH=/tmp/ksft
        cd /tmp/ksft

	sudo NETIF=ens4 REMOTE_TYPE=ssh \
		REMOTE_ARGS="root@10.40.0.2" \
		LOCAL_V4="10.40.0.1"
		REMOTE_V4="10.40.0.2" \
		LOCAL_V6="<REDACTED>" \
		REMOTE_V6="<REDACTED>" \
		./run_kselftest.sh -t drivers/net/hw:csum.py

	TAP version 13
	1..1
	# timeout set to 0
	# selftests: drivers/net/hw: csum.py
	# KTAP version 1
	# 1..12
	# ok 1 csum.ipv4_rx_tcp
	# ok 2 csum.ipv4_rx_tcp_invalid
	# ok 3 csum.ipv4_rx_udp
	# ok 4 csum.ipv4_rx_udp_invalid
	# ok 5 csum.ipv4_tx_udp_csum_offload
	# ok 6 csum.ipv4_tx_udp_zero_checksum
	# ok 7 csum.ipv6_rx_tcp
	# ok 8 csum.ipv6_rx_tcp_invalid
	# ok 9 csum.ipv6_rx_udp
	# ok 10 csum.ipv6_rx_udp_invalid
	# ok 11 csum.ipv6_tx_udp_csum_offload
	# ok 12 csum.ipv6_tx_udp_zero_checksum
	# # Totals: pass:12 fail:0 xfail:0 xpass:0 skip:0 error:0
	ok 1 selftests: drivers/net/hw: csum.py

Warning that for now transmit errors are not detected, as for those
the receiver runs remotely and failures with bkg are ignored.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../testing/selftests/drivers/net/hw/csum.py  | 114 ++++++++++++++++++
 2 files changed, 115 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/hw/csum.py

Comments

Willem de Bruijn May 2, 2024, 1:38 a.m. UTC | #1
Jakub Kicinski wrote:
> Great! I run it on a couple of older machines. 
> 
> mlx5:
> 
> TAP version 13
> 1..1
> # timeout set to 0
> # selftests: drivers/net/hw: csum.py
> # KTAP version 1
> # 1..12
> # ok 1 csum.ipv4_rx_tcp # SKIP Test requires IPv4 connectivity
> # ok 2 csum.ipv4_rx_tcp_invalid # SKIP Test requires IPv4 connectivity
> # ok 3 csum.ipv4_rx_udp # SKIP Test requires IPv4 connectivity
> # ok 4 csum.ipv4_rx_udp_invalid # SKIP Test requires IPv4 connectivity
> # ok 5 csum.ipv4_tx_udp_csum_offload # SKIP Test requires IPv4 connectivity
> # ok 6 csum.ipv4_tx_udp_zero_checksum # SKIP Test requires IPv4 connectivity
> # ok 7 csum.ipv6_rx_tcp
> # ok 8 csum.ipv6_rx_tcp_invalid
> # ok 9 csum.ipv6_rx_udp
> # ok 10 csum.ipv6_rx_udp_invalid
> # ok 11 csum.ipv6_tx_udp_csum_offload
> # ok 12 csum.ipv6_tx_udp_zero_checksum
> # # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:6 error:0
> ok 1 selftests: drivers/net/hw: csum.py
> 
> bnxt:
> 
> TAP version 13
> 1..1
> # timeout set to 0
> # selftests: drivers/net/hw: csum.py
> # KTAP version 1
> # 1..12
> # ok 1 csum.ipv4_rx_tcp # SKIP Test requires IPv4 connectivity
> # ok 2 csum.ipv4_rx_tcp_invalid # SKIP Test requires IPv4 connectivity
> # ok 3 csum.ipv4_rx_udp # SKIP Test requires IPv4 connectivity
> # ok 4 csum.ipv4_rx_udp_invalid # SKIP Test requires IPv4 connectivity
> # ok 5 csum.ipv4_tx_udp_csum_offload # SKIP Test requires IPv4 connectivity
> # ok 6 csum.ipv4_tx_udp_zero_checksum # SKIP Test requires IPv4 connectivity
> # ok 7 csum.ipv6_rx_tcp
> # ok 8 csum.ipv6_rx_tcp_invalid
> # ok 9 csum.ipv6_rx_udp
> # ok 10 csum.ipv6_rx_udp_invalid
> # ok 11 csum.ipv6_tx_udp_csum_offload # SKIP Test requires tx checksum offload on eth0
> # ok 12 csum.ipv6_tx_udp_zero_checksum # SKIP Test requires tx checksum offload on eth0
> # # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:8 error:0
> ok 1 selftests: drivers/net/hw: csum.py

Nice, thanks for testing!

> On Wed,  1 May 2024 14:51:34 -0400 Willem de Bruijn wrote:
> > Run tools/testing/selftest/net/csum.c as part of drv-net.
> > This binary covers multiple scenarios, based on arguments given,
> > for both IPv4 and IPv6:
> 
> The use of csum.c is the only real concern I have. Could you move it to
> net/lib? I made net/lib into an automatically included target in commit
> b86761ff6374 ("selftests: net: add scaffolding for Netlink tests in Python").
> 
> It has a makefile like any selftest directory, so you should be able to
> do a simple move and minor path adjustments.
> 
> Without this if someone builds and deploys just the drivers/net{,/hw}
> targets the csum binary won't be there :( We could auto-include all of
> net but using the lib target felt a little cleaner.

Can do.

A few more may be in scope eventually: toeplitz, udpgso_bench, gro,
so_txtime. Move them on a case-by-case basis?
 
> > - Accept UDP correct checksum
> > - Detect UDP invalid checksum
> > - Accept TCP correct checksum
> > - Detect TCP invalid checksum
> > 
> > - Transmit UDP: basic checksum offload
> > - Transmit UDP: zero checksum conversion
> > 
> > The test direction is reversed between receive and transmit tests, so
> > that the NIC under test is always the local machine.
> > 
> > In total this adds up to 12 testcases, with more to follow. For
> > conciseness, I replaced individual functions with a function factory.
> > It saves a lot of boilerplate, but is a little harder to follow, so
> > partially here as a point for discussion.
> 
> LGTM, FWIW, but let's hear if anyone feels it's too magical.
> 
> > Warning that for now transmit errors are not detected, as for those
> > the receiver runs remotely and failures with bkg are ignored.
> 
> Should I send a fix for that?

Please do. I did not grasp your suggestion well enough to take a
stab. I may have already spotted the zero conversion test returning
success, while explicit logging of the stderr output shows otherwise.
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 1dd732855d76..4933d045ab66 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0+ OR MIT
 
 TEST_PROGS = \
+	csum.py \
 	devlink_port_split.py \
 	ethtool.sh \
 	ethtool_extended_state.sh \
diff --git a/tools/testing/selftests/drivers/net/hw/csum.py b/tools/testing/selftests/drivers/net/hw/csum.py
new file mode 100755
index 000000000000..e40c510f303d
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/csum.py
@@ -0,0 +1,114 @@ 
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Run the tools/testing/selftests/net/csum testsuite."""
+
+from os import path
+
+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):
+    """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}"
+
+    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}"
+
+    with bkg(rx_cmd, exit_wait=True):
+        wait_port_listen(34000, proto='udp')
+        cmd(tx_cmd, host=cfg.remote)
+
+
+def test_transmit(cfg, ipv4=False, extra_args=None):
+    """Test local nic checksum transmit. Remote host verifies packets."""
+    if not cfg.have_tx_csum:
+        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}"
+
+    # Cannot randomize input when calculating zero checksum
+    if extra_args != "-U -Z":
+        extra_args += " -r 1"
+
+    rx_cmd = f"{cfg.bin_remote} -i {cfg.ifname} -L 1 -n 100 {ip_args} -R {extra_args}"
+    tx_cmd = f"{cfg.bin_local} -i {cfg.ifname} -L 1 -n 100 {ip_args} -T {extra_args}"
+
+    with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
+        wait_port_listen(34000, proto='udp', host=cfg.remote)
+        cmd(tx_cmd)
+
+
+def test_builder(name, cfg, ipv4=False, 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()
+
+        if tx:
+            test_transmit(cfg, ipv4, extra_args)
+        else:
+            test_receive(cfg, ipv4, extra_args)
+
+    if ipv4:
+        f.__name__ = "ipv4_" + name
+    else:
+        f.__name__ = "ipv6_" + name
+    return f
+
+
+def check_nic_features(cfg) -> None:
+    """Test whether Tx and Rx checksum offload are enabled.
+
+       If the device under test has either off, then skip the relevant tests."""
+    cfg.have_tx_csum = False
+    cfg.have_rx_csum = False
+
+    ethnl = EthtoolFamily()
+    features = ethnl.features_get({"header": {"dev-index": cfg.ifindex}})
+    for f in features["active"]["bits"]["bit"]:
+        if f["name"] == "tx-checksum-ip-generic":
+            cfg.have_tx_csum = True
+        elif f["name"] == "rx-checksum":
+            cfg.have_rx_csum = True
+
+
+def main() -> None:
+    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        check_nic_features(cfg)
+
+        cfg.bin_local = path.abspath(path.dirname(__file__) + "/../../../net/csum")
+        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"))
+
+            cases.append(test_builder("rx_udp", cfg, ipv4, False, ""))
+            cases.append(test_builder("rx_udp_invalid", cfg, ipv4, 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"))
+
+        ksft_run(cases=cases, args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()