[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <66355d15d44d_3de36e29464@willemb.c.googlers.com.notmuch>
Date: Fri, 03 May 2024 17:54:29 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
netdev@...r.kernel.org
Cc: davem@...emloft.net,
kuba@...nel.org,
edumazet@...gle.com,
pabeni@...hat.com,
linux-kselftest@...r.kernel.org,
Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next v2] selftests: drv-net: add checksum tests
Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@...gle.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.
>
> 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:
>
> make -C tools/testing/selftests/ \
> TARGETS="drivers/net drivers/net/hw" \
> install INSTALL_PATH=/tmp/ksft
> cd /tmp/ksft
>
> sudo NETIF=ens4 REMOTE_TYPE=ssh \
> REMOTE_ARGS="root@...40.0.2" \
> LOCAL_V4="10.40.0.1"
Missing backslash
> REMOTE_V4="10.40.0.2" \
> ./run_kselftest.sh -t drivers/net/hw:csum.py
>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
>
> ---
>
> Changes
> - v1->v2
> - remove dependency on tools/testing/selftests/net: move csum
> - remove test output from git commit message:
> has noisy (expected) failures on test platform after bkg changes
> ---
> .../testing/selftests/drivers/net/hw/Makefile | 1 +
> .../testing/selftests/drivers/net/hw/csum.py | 114 ++++++++++++++++++
> tools/testing/selftests/net/.gitignore | 1 -
> tools/testing/selftests/net/Makefile | 1 -
> tools/testing/selftests/net/lib/.gitignore | 2 +
> tools/testing/selftests/net/lib/Makefile | 7 ++
> tools/testing/selftests/net/{ => lib}/csum.c | 0
> 7 files changed, 124 insertions(+), 2 deletions(-)
> create mode 100755 tools/testing/selftests/drivers/net/hw/csum.py
> create mode 100644 tools/testing/selftests/net/lib/.gitignore
> rename tools/testing/selftests/net/{ => lib}/csum.c (100%)
>
> 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..7e3a955fc426
> --- /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
Also need to test for "tx-checksum-ipv4" and "tx-checksum-ipv6".
Will respin.
> + elif f["name"] == "rx-checksum":
> + cfg.have_rx_csum = True
Powered by blists - more mailing lists