Re: [PATCH net-next v4 2/4] selftests: drv-net: add an iperf performance test

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In commit "selftests: drv-net: add an iperf performance test",
  the author and sign-off addresses differ.
- [Low] The patch adds a new HW-only test directory,
  tools/testing/selftests/drivers/net/perf.

--

selftests: drv-net: add an iperf performance test

This commit adds a new drivers/net/perf selftest target with its own
lib/py, similar to drivers/net/hw. It adds Iperf3Runner.measure(), which
returns an Iperf3Result, and a new iperf.py test that reports TCP
throughput as a ktap metric.

> Reviewed-by: Björn Töpel <bjorn@kernel.org>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

[Severity: Low]
This isn't a bug, but the author and sign-off addresses don't match:

  Author: Stanislav Fomichev <sdf.kernel@gmail.com>
  Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

There is no From: line or second Signed-off-by linking the two, and
checkpatch flags this as a From:/Signed-off-by: email address mismatch.
The other patches in the series have the same mismatch.

Could the author address be changed to match, or a Signed-off-by from
the author address be added?

> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 07c3719f8b04b..c55d7792b27c4 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -159,7 +159,7 @@ TARGETS_HOTPLUG = cpu-hotplug
>  TARGETS_HOTPLUG += memory-hotplug
>  
>  # Networking tests want the net/lib target, include it automatically
> -ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),)
> +ifneq ($(filter net drivers/net drivers/net/hw drivers/net/perf,$(TARGETS)),)
>  ifeq ($(filter net/lib,$(TARGETS)),)
>  	INSTALL_DEP_TARGETS := net/lib
>  endif

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/perf/iperf.py b/tools/testing/selftests/drivers/net/perf/iperf.py
> new file mode 100755
> index 0000000000000..5b6f0bce2a03d
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/perf/iperf.py

[ ... ]

> +def main():
> +    """Run the iperf performance test."""
> +    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:

[Severity: Low]
Should tools/testing/selftests/drivers/net/README.rst be updated to cover
the new drivers/net/perf directory?

With nsim_test=False this is a HW-only test. The "Both modes required"
section of the README still says:

  All tests in drivers/net must support running both against a software device
  and a real device. SW-only tests should instead be placed in net/ or
  drivers/net/netdevsim, HW-only tests in drivers/net/hw.

The build/install example also only lists the existing targets:

  # make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw"

No patch in the series mentions drivers/net/perf in the README. Users who
follow the example won't build or install the new perf tests, and the
placement rule for HW-only tests no longer matches the tree.

> +        ksft_run([tcp], args=(cfg,))
> +    ksft_exit()

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111000246.962871-1-sdf%40fomichev.me