| Message ID | 20240501185432.3593168-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] selftests: drv-net: add checksum tests | expand |
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 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. > - 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?
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.
On Wed, 01 May 2024 21:38:24 -0400 Willem de Bruijn wrote: > A few more may be in scope eventually: toeplitz, udpgso_bench, gro, > so_txtime. Move them on a case-by-case basis? If you plan to use them all in hw tests - no preference, you can move them all in one fell swoop if you prefer. > > 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. On it..
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()