[PATCH net-next v4 0/4] selftests: net: add performance metric reporting

[PATCH net-next v4 0/4] selftests: net: add performance metric reporting

[Stanislav Fomichev]

Let Python selftests report numeric measurements, so that CI can track
the performance of drivers over time, and add two performance tests.

ksft_metric() reports a measurement of the current test case, which is
printed right away as JSON:

  # ktap-metric-json: {"goal":"higher","name":"throughput","value":389.127}

The goal tells a performance metric, which is better higher or lower,
from an error metric, which must be zero. Everything else, such as
units, history and regression thresholds, is up to the consumer.

The tests are in a new drivers/net/perf target, apart from the
functional tests: they need real hardware, take a while, and make no
sense on debug kernels. iperf.py measures single-stream TCP throughput
with iperf3, devmem.py the throughput of udmabuf-backed devmem TCP with
kperf [1].

System monitoring stays in NIPA, which runs each case of the perf tests
in a process of its own and attaches CPU and NIC metrics to its results.
The NIPA changes are at [2]; git clone & ./nipa local view starts an
http server on :44109 with two sample runs.

[1] https://github.com/facebookexperimental/kperf
[2] https://github.com/fomichev/nipa/tree/nipa-local-hw-metrics

v4:
- print metrics right away (Jakub)
- mark the exit code of the perf lib/py on import failures as KSFT_SKIP
  (Björn)
- let kperf-server --kill wait for the sessions of the server, drop the
  pgrep loop (Jakub):
  https://github.com/facebookexperimental/kperf/pull/47

v3:
- drop the timestamp of metrics (Jakub)
- drop the note on nested output from the docs (Jakub)
- report plain numbers, drop kind/type (Jakub)
- replace ksft_metric_policy() with a goal per metric: higher, lower
  or zero (Jakub)
- move the tests to drivers/net/perf, with a lib/py of their own (Jakub)
- fix the format of the Makefile lists (Jakub)
- report metrics from the tests rather than the runners (Jakub)
- drop the kperf section of the README (Jakub)
- stop reporting TCP retransmits
- drop the metric labels, report the throughput of the receiver only

v2:
- move system monitoring to NIPA (Jakub)
- move performance tests to drivers/net/hw/performance (Jakub)
- rename metric shape to type and document supported types (Paolo);
  kind= instead of type= (type shadows a builtin)
- remove direction from the metric example (Paolo)
- document policy processing order (Paolo)
- support producer-selected summary operations, including arbitrary pXX
- label throughput metrics in Gbps
- do not mark the iperf benchmark disruptive
- use 2 GiB udmabuf buffers for kperf devmem
- drop the ncdevmem devmem probe
- use a random kperf control port
- report kperf server cleanup failures

Stanislav Fomichev (4):
  selftests: net: py: add metric output
  selftests: drv-net: add an iperf performance test
  selftests: drv-net: add a kperf runner
  selftests: drv-net: measure devmem performance with kperf

 tools/testing/selftests/Makefile              |   2 +-
 .../testing/selftests/drivers/net/README.rst  |  16 ++
 .../selftests/drivers/net/lib/py/__init__.py  |   4 +-
 .../selftests/drivers/net/lib/py/kperf.py     | 157 ++++++++++++++++++
 .../selftests/drivers/net/lib/py/load.py      |  20 ++-
 .../selftests/drivers/net/perf/Makefile       |  12 ++
 .../testing/selftests/drivers/net/perf/config |   4 +
 .../selftests/drivers/net/perf/devmem.py      |  96 +++++++++++
 .../selftests/drivers/net/perf/iperf.py       |  23 +++
 .../drivers/net/perf/lib/py/__init__.py       |  59 +++++++
 .../selftests/drivers/net/perf/settings       |   1 +
 .../testing/selftests/net/lib/py/__init__.py  |   9 +-
 tools/testing/selftests/net/lib/py/ksft.py    |  20 +++
 13 files changed, 414 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/lib/py/kperf.py
 create mode 100644 tools/testing/selftests/drivers/net/perf/Makefile
 create mode 100644 tools/testing/selftests/drivers/net/perf/config
 create mode 100755 tools/testing/selftests/drivers/net/perf/devmem.py
 create mode 100755 tools/testing/selftests/drivers/net/perf/iperf.py
 create mode 100644 tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
 create mode 100644 tools/testing/selftests/drivers/net/perf/settings

-- 
2.53.0-Meta

[PATCH net-next v4 1/4] selftests: net: py: add metric output

[Stanislav Fomichev]

Add ksft_metric() to report a numeric measurement of the current test case
as JSON. Each metric states its goal: higher or lower for a performance
metric, depending on which way is better, or zero for an error metric, so a
consumer can tell wins from regressions.

Values are plain numbers without a timestamp. A helper for other kinds of
values, or a timestamp for values sampled over time, can be added when a
test needs one.

Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
v3: https://lore.kernel.org/netdev/20261102190827.989117-1-sdf@fomichev.me/
v2: https://lore.kernel.org/netdev/20261005201115.2690830-1-sdf@fomichev.me/
v1: https://lore.kernel.org/netdev/20260916190409.1222272-1-sdf@fomichev.me/
---
 .../testing/selftests/drivers/net/README.rst  | 16 +++++++++++++++
 .../testing/selftests/net/lib/py/__init__.py  |  9 +++++----
 tools/testing/selftests/net/lib/py/ksft.py    | 20 +++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
index 3fe49bce4f3a..f651163c16c8 100644
--- a/tools/testing/selftests/drivers/net/README.rst
+++ b/tools/testing/selftests/drivers/net/README.rst
@@ -259,6 +259,22 @@ ksft_pr()
 
 Use ``ksft_pr()`` instead of ``print()`` to avoid breaking TAP format.
 
+ksft_metric()
+~~~~~~~~~~~~~
+
+Use ``ksft_metric()`` to report a numeric measurement of the current test
+case. Metrics are printed right away as compact JSON. For example::
+
+  ksft_metric("throughput", 100.5, goal="higher")
+
+prints::
+
+  # ktap-metric-json: {"goal":"higher","name":"throughput","value":100.5}
+
+``goal`` tells a performance metric, which is better ``higher`` or ``lower``,
+from an error metric, which must be ``zero``. Metrics do not affect the
+result of the test.
+
 ksft_disruptive
 ~~~~~~~~~~~~~~~
 
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index e269a260d58e..b1696f432c7c 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -8,8 +8,8 @@ from .consts import KSRC
 from .ksft import KsftFailEx, KsftSkipEx, KsftXfailEx, ksft_pr, ksft_eq, \
     ksft_ne, ksft_true, ksft_not_none, ksft_in, ksft_not_in, ksft_is, \
     ksft_ge, ksft_gt, ksft_lt, ksft_raises, ksft_busy_wait, \
-    ktap_result, ksft_disruptive, ksft_setup, ksft_run, ksft_exit, \
-    ksft_variants, KsftNamedVariant
+    ktap_result, ksft_disruptive, ksft_metric, ksft_setup, ksft_run, \
+    ksft_exit, ksft_variants, KsftNamedVariant
 from .netns import NetNS, NetNSEnter, UserNetNS
 from .nsim import NetdevSim, NetdevSimDev
 from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
@@ -24,8 +24,9 @@ __all__ = ["KSRC",
            "KsftFailEx", "KsftSkipEx", "KsftXfailEx", "ksft_pr", "ksft_eq",
            "ksft_ne", "ksft_true", "ksft_not_none", "ksft_in", "ksft_not_in",
            "ksft_is", "ksft_ge", "ksft_gt", "ksft_lt", "ksft_raises",
-           "ksft_busy_wait", "ktap_result", "ksft_disruptive", "ksft_setup",
-           "ksft_run", "ksft_exit", "ksft_variants", "KsftNamedVariant",
+           "ksft_busy_wait", "ktap_result", "ksft_disruptive", "ksft_metric",
+           "ksft_setup", "ksft_run", "ksft_exit", "ksft_variants",
+           "KsftNamedVariant",
            "NetNS", "NetNSEnter", "UserNetNS",
            "CmdExitFailure", "fd_read_timeout", "cmd", "bkg", "defer",
            "bpftool", "ip", "ethtool", "bpftrace", "rand_port", "rand_ports",
diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
index 81287c2daff0..572ee17b4f4f 100644
--- a/tools/testing/selftests/net/lib/py/ksft.py
+++ b/tools/testing/selftests/net/lib/py/ksft.py
@@ -4,6 +4,7 @@ import fnmatch
 import functools
 import getopt
 import inspect
+import json
 import os
 import signal
 import sys
@@ -93,6 +94,25 @@ KSFT_DISRUPTIVE = True
     print(pfx, prefixed, **kwargs)
 
 
+def ksft_metric(name, value, *, goal):
+    """
+    Report a numeric measurement of the current test case.
+
+    Metrics are printed right away. They do not affect the result.
+
+    Args:
+      name - name of the metric, e.g. "throughput"
+      value - the measured number
+      goal - "higher" or "lower" for a performance metric, depending on
+             which way is better, "zero" for an error metric, which must
+             be zero
+    """
+    metric = {"name": name, "value": value, "goal": goal}
+    payload = json.dumps(metric, allow_nan=False, separators=(",", ":"),
+                         sort_keys=True)
+    ksft_pr(f"ktap-metric-json: {payload}")
+
+
 def _fail(*args):
     global KSFT_RESULT
     KSFT_RESULT = False
-- 
2.53.0-Meta

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

[Stanislav Fomichev]

Add the drivers/net/perf target, with a lib/py of its own like the one of
drivers/net/hw. Add Iperf3Runner.measure(), which returns an Iperf3Result
that other results can join later, and report its throughput from the test.

Single-stream iperf3 throughput is flaky and can change considerably
between back-to-back runs on the same hosts.

Tested on two mlx5 hosts. Captured metric output:

  # # ktap-metric-json: {"goal":"higher","name":"throughput","value":49.92071602072054}

Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/Makefile              |  2 +-
 .../selftests/drivers/net/lib/py/load.py      | 20 ++++++-
 .../selftests/drivers/net/perf/Makefile       | 11 ++++
 .../testing/selftests/drivers/net/perf/config |  1 +
 .../selftests/drivers/net/perf/iperf.py       | 23 ++++++++
 .../drivers/net/perf/lib/py/__init__.py       | 57 +++++++++++++++++++
 .../selftests/drivers/net/perf/settings       |  1 +
 7 files changed, 111 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/perf/Makefile
 create mode 100644 tools/testing/selftests/drivers/net/perf/config
 create mode 100755 tools/testing/selftests/drivers/net/perf/iperf.py
 create mode 100644 tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
 create mode 100644 tools/testing/selftests/drivers/net/perf/settings

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 07c3719f8b04..c55d7792b27c 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/lib/py/load.py b/tools/testing/selftests/drivers/net/lib/py/load.py
index e24660e5c27f..f337b576c4b4 100644
--- a/tools/testing/selftests/drivers/net/lib/py/load.py
+++ b/tools/testing/selftests/drivers/net/lib/py/load.py
@@ -3,10 +3,18 @@
 import re
 import time
 import json
+from dataclasses import dataclass
 
 from lib.py import ksft_pr, cmd, ip, rand_port, wait_port_listen
 
 
+@dataclass
+class Iperf3Result:
+    """Average throughput (Gbps) of an iperf3 run."""
+
+    throughput: float
+
+
 class Iperf3Runner:
     """
     Sets up and runs iperf3 traffic.
@@ -53,9 +61,9 @@ from lib.py import ksft_pr, cmd, ip, rand_port, wait_port_listen
             kwargs["timeout"] = duration + 5
         return cmd(cmdline, **kwargs)
 
-    def measure_bandwidth(self, reverse=False):
+    def measure(self, reverse=False):
         """
-        Runs an iperf3 measurement and returns the average bandwidth (Gbps).
+        Runs an iperf3 measurement and returns an Iperf3Result.
         Discards the first and last few reporting intervals and uses only the
         middle part of the run where throughput is typically stable.
         """
@@ -78,7 +86,13 @@ from lib.py import ksft_pr, cmd, ip, rand_port, wait_port_listen
 
         avg = sum(stable) / len(stable)
 
-        return avg
+        return Iperf3Result(avg)
+
+    def measure_bandwidth(self, reverse=False):
+        """
+        Runs an iperf3 measurement and returns the average bandwidth (Gbps).
+        """
+        return self.measure(reverse=reverse).throughput
 
 
 class GenerateTraffic:
diff --git a/tools/testing/selftests/drivers/net/perf/Makefile b/tools/testing/selftests/drivers/net/perf/Makefile
new file mode 100644
index 000000000000..31b0b60de4f6
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TEST_PROGS := \
+	iperf.py \
+# end of TEST_PROGS
+
+TEST_INCLUDES := \
+	$(wildcard lib/py/*.py ../lib/py/*.py) \
+# end of TEST_INCLUDES
+
+include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/perf/config b/tools/testing/selftests/drivers/net/perf/config
new file mode 100644
index 000000000000..6450f34933e6
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/config
@@ -0,0 +1 @@
+CONFIG_IPV6=y
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 000000000000..5b6f0bce2a03
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/iperf.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Measure TCP throughput with iperf3."""
+
+from lib.py import Iperf3Runner, NetDrvEpEnv, ksft_exit, ksft_metric, ksft_run
+
+
+def tcp(cfg):
+    """Measure TCP throughput."""
+    result = Iperf3Runner(cfg).measure()
+    ksft_metric("throughput", result.throughput, goal="higher")
+
+
+def main():
+    """Run the iperf performance test."""
+    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        ksft_run([tcp], args=(cfg,))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
diff --git a/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py b/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
new file mode 100644
index 000000000000..f16606cedd00
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Driver test environment (performance tests).
+NetDrvEnv and NetDrvEpEnv are the main environment classes.
+NetDrvContEnv extends NetDrvEpEnv with netkit container support.
+Former is for local host only tests, latter creates / connects
+to a remote endpoint. See NIPA wiki for more information about
+running and writing driver tests.
+"""
+
+import sys
+from pathlib import Path
+
+KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
+
+try:
+    sys.path.append(KSFT_DIR.as_posix())
+
+    # Import one by one to avoid pylint false positives
+    from net.lib.py import NetNS, NetNSEnter, NetdevSimDev, UserNetNS
+    from net.lib.py import EthtoolFamily, NetdevFamily, NetshaperFamily, \
+        NlError, RtnlFamily, DevlinkFamily, PSPFamily, Netlink
+    from net.lib.py import CmdExitFailure
+    from net.lib.py import bkg, cmd, bpftool, bpftrace, defer, ethtool, \
+        fd_read_timeout, ip, rand_port, rand_ports, wait_port_listen, \
+        wait_file, ctl_file_write, tool
+    from net.lib.py import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
+    from net.lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx
+    from net.lib.py import ksft_disruptive, ksft_exit, ksft_metric, ksft_pr, \
+        ksft_run, ksft_setup, ksft_variants, KsftNamedVariant
+    from net.lib.py import ksft_eq, ksft_ge, ksft_in, ksft_is, ksft_lt, \
+        ksft_ne, ksft_not_in, ksft_raises, ksft_true, ksft_gt, ksft_not_none
+    from drivers.net.lib.py import set_ethtool_feat
+    from drivers.net.lib.py import GenerateTraffic, Remote, Iperf3Runner
+    from drivers.net.lib.py import NetDrvEnv, NetDrvEpEnv, NetDrvContEnv
+
+    __all__ = ["NetNS", "NetNSEnter", "NetdevSimDev", "UserNetNS",
+               "EthtoolFamily", "NetdevFamily", "NetshaperFamily",
+               "NlError", "RtnlFamily", "DevlinkFamily", "PSPFamily", "Netlink",
+               "CmdExitFailure",
+               "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
+               "fd_read_timeout", "ip", "rand_port", "rand_ports",
+               "wait_port_listen", "wait_file", "ctl_file_write", "tool",
+               "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
+               "KsftSkipEx", "KsftFailEx", "KsftXfailEx",
+               "ksft_disruptive", "ksft_exit", "ksft_metric", "ksft_pr",
+               "ksft_run", "ksft_setup", "ksft_variants", "KsftNamedVariant",
+               "ksft_eq", "ksft_ge", "ksft_in", "ksft_is", "ksft_lt",
+               "ksft_ne", "ksft_not_in", "ksft_raises", "ksft_true", "ksft_gt",
+               "ksft_not_none",
+               "NetDrvEnv", "NetDrvEpEnv", "NetDrvContEnv", "GenerateTraffic",
+               "Remote", "Iperf3Runner", "set_ethtool_feat"]
+except ModuleNotFoundError as e:
+    print("Failed importing `net` library from kernel sources")
+    print(str(e))
+    sys.exit(4)  # KSFT_SKIP
diff --git a/tools/testing/selftests/drivers/net/perf/settings b/tools/testing/selftests/drivers/net/perf/settings
new file mode 100644
index 000000000000..e7b9417537fb
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/settings
@@ -0,0 +1 @@
+timeout=0
-- 
2.53.0-Meta

[PATCH net-next v4 3/4] selftests: drv-net: add a kperf runner

[Stanislav Fomichev]

Add a helper to manage kperf servers, run the client, and return the
aggregate throughput of both ends. Use a random control port and a unique
PID file per server, as the hosts may share /tmp, and report server cleanup
failures. kperf-server --kill waits for the sessions of a server to exit,
as they may still be releasing devmem and RX steering, which the next test
needs. A server which can log does so to a file next to its PID file, and
the end of both logs is printed when the client fails, as the client can
only tell that a server went away.

Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 .../selftests/drivers/net/lib/py/__init__.py  |   4 +-
 .../selftests/drivers/net/lib/py/kperf.py     | 157 ++++++++++++++++++
 .../drivers/net/perf/lib/py/__init__.py       |   6 +-
 3 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/lib/py/kperf.py

diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
index 40ccdcf0c378..7a32f6516a12 100644
--- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
@@ -49,11 +49,13 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
 
     from .env import NetDrvEnv, NetDrvEpEnv, NetDrvContEnv
     from .feat import set_ethtool_feat
+    from .kperf import KperfResult, KperfRunner
     from .load import GenerateTraffic, Iperf3Runner
     from .remote import Remote
 
     __all__ += ["NetDrvEnv", "NetDrvEpEnv", "NetDrvContEnv", "GenerateTraffic",
-                "Remote", "Iperf3Runner", "set_ethtool_feat"]
+                "Remote", "Iperf3Runner", "KperfResult", "KperfRunner",
+                "set_ethtool_feat"]
 except ModuleNotFoundError as e:
     print("Failed importing `net` library from kernel sources")
     print(str(e))
diff --git a/tools/testing/selftests/drivers/net/lib/py/kperf.py b/tools/testing/selftests/drivers/net/lib/py/kperf.py
new file mode 100644
index 000000000000..8ee8c5ba2ff7
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/lib/py/kperf.py
@@ -0,0 +1,157 @@
+# SPDX-License-Identifier: GPL-2.0
+
+"""Run and parse kperf network performance measurements."""
+
+import re
+import shlex
+import subprocess
+import uuid
+from dataclasses import dataclass
+
+from lib.py import cmd, ksft_pr, rand_port, wait_port_listen
+
+
+_SECTION_RE = re.compile(r"==\s+(Source|Target)\b")
+_RATE_RE = re.compile(r"\b(Tx|Rx)\s*([0-9]+(?:\.[0-9]+)?)\s+Gbps\b")
+
+
+@dataclass
+class KperfResult:
+    """Aggregate throughput (Gbps) of both endpoints."""
+
+    source_throughput: float
+    target_throughput: float
+
+
+class KperfRunner:
+    """Manage kperf servers and run measurements between test endpoints."""
+
+    def __init__(self, env):
+        self.env = env
+        self._pid_prefix = f"/tmp/ksft-kperf-{uuid.uuid4().hex[:8]}"
+        self._port = rand_port()
+        self._started = []
+        env.require_cmd("kperf-client", local=True)
+        env.require_cmd("kperf-server", local=True, remote=True)
+
+    def _pid_file(self, remote):
+        # The hosts may share /tmp (netns), and kperf-server kills the
+        # process named in an existing PID file.
+        return f"{self._pid_prefix}-{'remote' if remote else 'local'}.pid"
+
+    def _log_file(self, remote):
+        return f"{self._pid_prefix}-{'remote' if remote else 'local'}.log"
+
+    def _server(self, remote, kill=False):
+        host = self.env.remote if remote else None
+        address = self.env.remote_addr if remote else self.env.addr
+        args = ["kperf-server", "--pid-file", self._pid_file(remote)]
+        if kill:
+            args.append("--kill")
+        else:
+            args += ["--addr", address, "--port", str(self._port)]
+            # The output of a daemon is lost, unless it can log
+            usage = cmd("kperf-server --help", host=host, fail=False)
+            if "--log-file" in usage.stdout + usage.stderr:
+                args += ["--log-file", self._log_file(remote)]
+        return cmd(shlex.join(args) if remote else args, host=host,
+                   fail=not kill)
+
+    def _print_logs(self):
+        """Print the end of the logs of the servers, to tell why the client
+        failed.
+        """
+        for remote in self._started:
+            host = self.env.remote if remote else None
+            args = ["tail", "-n", "30", self._log_file(remote)]
+            log = cmd(shlex.join(args) if remote else args, host=host,
+                      fail=False)
+            if log.stdout.strip():
+                name = "remote" if remote else "local"
+                ksft_pr(f"kperf-server log ({name}):\n{log.stdout.strip()}")
+
+    def __enter__(self):
+        try:
+            for remote in (False, True):
+                self._server(remote)
+                self._started.append(remote)
+            wait_port_listen(self._port)
+            wait_port_listen(self._port, host=self.env.remote)
+        except Exception:
+            self.close()
+            raise
+        return self
+
+    def close(self):
+        """Stop the started kperf servers, which waits for their sessions."""
+        failures = []
+        while self._started:
+            remote = self._started.pop()
+            command = self._server(remote, kill=True)
+            if command.ret:
+                failures.append(command)
+            host = self.env.remote if remote else None
+            args = ["rm", "-f", self._log_file(remote)]
+            cmd(shlex.join(args) if remote else args, host=host, fail=False)
+        if failures:
+            details = "\n".join(str(command) for command in failures)
+            raise RuntimeError(f"failed to stop kperf server(s)\n{details}")
+
+    def __exit__(self, _exc_type, _exc_value, _exc_tb):
+        self.close()
+
+    def run(self, client_args=()):
+        """Run kperf and return its KperfResult."""
+        argv = ["kperf-client", "--src", self.env.remote_addr,
+                "--dst", self.env.addr, "--src-svc", str(self._port),
+                "--dst-svc", str(self._port)]
+        argv += [str(arg) for arg in client_args]
+        command = cmd(argv, background=True)
+        try:
+            command.process(terminate=False, fail=False, timeout=300)
+        except subprocess.TimeoutExpired as error:
+            command.proc.kill()
+            command.process(terminate=False, fail=False)
+            self._print_logs()
+            raise RuntimeError(f"kperf client timed out\n{command!r}") \
+                from error
+        if command.ret:
+            self._print_logs()
+            raise RuntimeError(
+                f"kperf client exited with status {command.ret}\n{command!r}")
+
+        try:
+            result = self.parse(command.stdout + "\n" + command.stderr)
+        except ValueError:
+            ksft_pr(command)
+            self._print_logs()
+            raise
+
+        return result
+
+    @staticmethod
+    def parse(output):
+        """Parse human-readable kperf client output."""
+        throughput = {"Source": [], "Target": []}
+        section = None
+
+        for line in output.splitlines():
+            match = _SECTION_RE.search(line)
+            if match:
+                section = match.group(1)
+                continue
+            if section is None:
+                continue
+
+            match = _RATE_RE.search(line)
+            if match:
+                direction, value = match.groups()
+                if (section, direction) in (("Source", "Tx"),
+                                             ("Target", "Rx")):
+                    throughput[section].append(float(value))
+
+        if not all(throughput.values()):
+            raise ValueError("incomplete kperf result")
+
+        return KperfResult(sum(throughput["Source"]),
+                           sum(throughput["Target"]))
diff --git a/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py b/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
index f16606cedd00..fcc5234cdd57 100644
--- a/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/perf/lib/py/__init__.py
@@ -32,7 +32,8 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
     from net.lib.py import ksft_eq, ksft_ge, ksft_in, ksft_is, ksft_lt, \
         ksft_ne, ksft_not_in, ksft_raises, ksft_true, ksft_gt, ksft_not_none
     from drivers.net.lib.py import set_ethtool_feat
-    from drivers.net.lib.py import GenerateTraffic, Remote, Iperf3Runner
+    from drivers.net.lib.py import GenerateTraffic, Remote, Iperf3Runner, \
+        KperfResult, KperfRunner
     from drivers.net.lib.py import NetDrvEnv, NetDrvEpEnv, NetDrvContEnv
 
     __all__ = ["NetNS", "NetNSEnter", "NetdevSimDev", "UserNetNS",
@@ -50,7 +51,8 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
                "ksft_ne", "ksft_not_in", "ksft_raises", "ksft_true", "ksft_gt",
                "ksft_not_none",
                "NetDrvEnv", "NetDrvEpEnv", "NetDrvContEnv", "GenerateTraffic",
-               "Remote", "Iperf3Runner", "set_ethtool_feat"]
+               "Remote", "Iperf3Runner", "KperfResult", "KperfRunner",
+               "set_ethtool_feat"]
 except ModuleNotFoundError as e:
     print("Failed importing `net` library from kernel sources")
     print(str(e))
-- 
2.53.0-Meta

[PATCH net-next v4 4/4] selftests: drv-net: measure devmem performance with kperf

[Stanislav Fomichev]

Add a separate udmabuf-backed devmem performance test using KperfRunner.
Keep correctness cases in drivers/net/hw. Skip the test when kperf or
/dev/udmabuf is missing, and fail it when the kperf run fails.

Example metric output:

  # # ktap-metric-json: {"goal":"higher","name":"throughput","value":389.127}

Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 .../selftests/drivers/net/perf/Makefile       |  1 +
 .../testing/selftests/drivers/net/perf/config |  3 +
 .../selftests/drivers/net/perf/devmem.py      | 96 +++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/perf/devmem.py

diff --git a/tools/testing/selftests/drivers/net/perf/Makefile b/tools/testing/selftests/drivers/net/perf/Makefile
index 31b0b60de4f6..1a90537d2726 100644
--- a/tools/testing/selftests/drivers/net/perf/Makefile
+++ b/tools/testing/selftests/drivers/net/perf/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 TEST_PROGS := \
+	devmem.py \
 	iperf.py \
 # end of TEST_PROGS
 
diff --git a/tools/testing/selftests/drivers/net/perf/config b/tools/testing/selftests/drivers/net/perf/config
index 6450f34933e6..111e9b58610d 100644
--- a/tools/testing/selftests/drivers/net/perf/config
+++ b/tools/testing/selftests/drivers/net/perf/config
@@ -1 +1,4 @@
 CONFIG_IPV6=y
+CONFIG_NET_DEVMEM=y
+CONFIG_SYNC_FILE=y
+CONFIG_UDMABUF=y
diff --git a/tools/testing/selftests/drivers/net/perf/devmem.py b/tools/testing/selftests/drivers/net/perf/devmem.py
new file mode 100755
index 000000000000..4c1fb9b51fff
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/perf/devmem.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Measure udmabuf-backed devmem TCP throughput with kperf."""
+
+from lib.py import cmd, defer, ethtool, EthtoolFamily, KperfRunner
+from lib.py import ksft_disruptive, ksft_exit, ksft_metric, ksft_run
+from lib.py import KsftSkipEx, NetDrvEpEnv
+
+
+UDMABUF_LIMIT = "/sys/module/udmabuf/parameters/size_limit_mb"
+UDMABUF_SIZE_MB = 2048
+
+
+def _set_udmabuf_limit(host) -> None:
+    current = cmd(f"cat {UDMABUF_LIMIT}", host=host).stdout.strip()
+    if int(current) >= UDMABUF_SIZE_MB:
+        return
+
+    cmd(f"echo {UDMABUF_SIZE_MB} > {UDMABUF_LIMIT}", host=host, shell=True)
+    defer(cmd, f"echo {current} > {UDMABUF_LIMIT}", host=host, shell=True)
+
+
+def _enable_hw_gro(cfg) -> None:
+    features = ethtool(f"-k {cfg.ifname}", json=True)[0]
+    hw_gro = features["rx-gro-hw"]
+    if not hw_gro["active"]:
+        if hw_gro["fixed"]:
+            raise KsftSkipEx("Test requires rx-gro-hw support")
+        ethtool(f"-K {cfg.ifname} rx-gro-hw on")
+        defer(ethtool, f"-K {cfg.ifname} rx-gro-hw off")
+
+
+@ksft_disruptive
+def kperf(cfg) -> None:
+    """Measure udmabuf-backed devmem performance with kperf."""
+    runner = KperfRunner(cfg)
+
+    for name, host in (("local", None), ("remote", cfg.remote)):
+        if cmd("test -c /dev/udmabuf", host=host, fail=False).ret:
+            raise KsftSkipEx(f"Test requires /dev/udmabuf on the {name} host")
+        _set_udmabuf_limit(host)
+
+    # kperf reprograms the RSS indirection table for its queues.
+    defer(ethtool, f"-X {cfg.ifname} default")
+
+    _enable_hw_gro(cfg)
+
+    ethnl = EthtoolFamily()
+    rings = ethnl.rings_get({"header": {"dev-index": cfg.ifindex}})
+    restore = {
+        "header": {"dev-index": cfg.ifindex},
+        "tcp-data-split": rings.get("tcp-data-split", "unknown"),
+    }
+    config = {
+        "header": {"dev-index": cfg.ifindex},
+        "tcp-data-split": "enabled",
+    }
+    # Memory providers require a zero threshold where the driver has one.
+    if "hds-thresh" in rings:
+        restore["hds-thresh"] = rings["hds-thresh"]
+        config["hds-thresh"] = 0
+    defer(ethnl.rings_set, restore)
+    ethnl.rings_set(config)
+
+    devmem_args = [
+        "--time", "15",
+        "--num-connections", "4",
+        "--read-size", str(64 << 20),
+        "--write-size", str(64 << 20),
+        "--pin-off", "4",
+        "--num-rx-queues", "4",
+        "--dmabuf-rx-size-mb", str(UDMABUF_SIZE_MB),
+        "--dmabuf-tx-size-mb", str(UDMABUF_SIZE_MB),
+        "--validate", "no",
+        "--devmem-rx",
+        "--devmem-tx",
+        "--devmem-rx-memory", "host",
+        "--devmem-tx-memory", "host",
+    ]
+
+    with runner:
+        result = runner.run(client_args=devmem_args)
+
+    ksft_metric("throughput", result.target_throughput, goal="higher")
+
+
+def main() -> None:
+    """Run the devmem performance test."""
+    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        ksft_run([kperf], args=(cfg,))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.53.0-Meta

Re: [PATCH net-next v4 1/4] selftests: net: py: add metric output

[netdev-bot+sashiko]

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 tools/testing/selftests/net/lib/py/ksft.py, the ksft_metric()
  docstring ("They do not affect the result.") and…
- [Low] ksft_metric() in tools/testing/selftests/net/lib/py/ksft.py does
  not check that `goal` is one of "higher", "lower" or "zero", or that…
- [Low] tools/testing/selftests/drivers/net/README.rst now documents
  ksft_metric() under "Guidance for test authors", next to ksft_pr(),…

--

selftests: net: py: add metric output

This commit adds ksft_metric() to the Python net selftest library. The
helper prints a numeric measurement of the current test case as a compact
JSON line prefixed with "ktap-metric-json:", tagged with a goal of higher,
lower or zero. It is exported from net/lib/py and documented in
drivers/net/README.rst.

> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index 3fe49bce4f3a5..f651163c16c8c 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -259,6 +259,22 @@ ksft_pr()
>  
>  Use ``ksft_pr()`` instead of ``print()`` to avoid breaking TAP format.
>  
> +ksft_metric()
> +~~~~~~~~~~~~~
> +
> +Use ``ksft_metric()`` to report a numeric measurement of the current test
> +case. Metrics are printed right away as compact JSON. For example::

[Severity: Low]
Can tests in drivers/net/ and drivers/net/hw/ import this helper the way
the README suggests?

This section sits next to ksft_pr(), defer(), ksft_disruptive and
ksft_variants. All of those are re-exported by
drivers/net/lib/py/__init__.py and drivers/net/hw/lib/py/__init__.py.
Both files use an explicit import list and an explicit __all__:

    from net.lib.py import ksft_disruptive, ksft_exit, ksft_pr, ksft_run, \
        ksft_setup, ksft_variants, KsftNamedVariant

This patch only adds ksft_metric to net/lib/py/__init__.py. So a test in
drivers/net/ or drivers/net/hw/ that does "from lib.py import ksft_metric"
would fail with ImportError when the module loads.

Later patches in the series add drivers/net/perf/lib/py/__init__.py,
which does export ksft_metric. The drivers/net and drivers/net/hw libs
still don't export it at the end of the series.

Should those two re-export lists be updated as well?

[ ... ]

> diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
> index 81287c2daff09..572ee17b4f4f2 100644
> --- a/tools/testing/selftests/net/lib/py/ksft.py
> +++ b/tools/testing/selftests/net/lib/py/ksft.py

[ ... ]

> @@ -93,6 +94,25 @@ def ksft_pr(*objs, **kwargs):
>      print(pfx, prefixed, **kwargs)
>  
>  
> +def ksft_metric(name, value, *, goal):
> +    """
> +    Report a numeric measurement of the current test case.
> +
> +    Metrics are printed right away. They do not affect the result.
> +
> +    Args:
> +      name - name of the metric, e.g. "throughput"
> +      value - the measured number
> +      goal - "higher" or "lower" for a performance metric, depending on
> +             which way is better, "zero" for an error metric, which must
> +             be zero
> +    """
> +    metric = {"name": name, "value": value, "goal": goal}

[Severity: Low]
This isn't a bug, but should ksft_metric() check that goal is one of
"higher", "lower" or "zero", and that value is a number?

Right now, a misspelled goal, a string or None value, or a bool (which
is encoded as true) still produces a well-formed ktap-metric-json line.
A consumer may not be able to classify that line.

The callers added later in the series, drivers/net/perf/iperf.py and
drivers/net/perf/devmem.py, both pass goal="higher" with a float
throughput. So this would only catch mistakes in future callers.

> +    payload = json.dumps(metric, allow_nan=False, separators=(",", ":"),
> +                         sort_keys=True)

[Severity: Low]
Is "They do not affect the result" true for every input?

With allow_nan=False, json.dumps() raises ValueError for NaN or
+/-Infinity, and TypeError for values it cannot encode. ksft_metric()
does not catch these, so they reach the generic handler in ksft_run():

        except BaseException as e:
            ...
            KSFT_RESULT = False
            cnt_key = 'fail'

The test case is then reported as failed. The README makes the same
claim: "Metrics do not affect the result of the test."

The callers in the series always pass finite throughput values. Also,
allow_nan=False looks like a deliberate choice to keep the output strict
JSON.

Could the docstring and the README say that non-finite or non-numeric
values raise an exception and fail the test case?

> +    ksft_pr(f"ktap-metric-json: {payload}")

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

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

[netdev-bot+sashiko]

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

Re: [PATCH net-next v4 3/4] selftests: drv-net: add a kperf runner

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] KperfRunner.close() in
  tools/testing/selftests/drivers/net/lib/py/kperf.py is meant to keep…
- [Low] In KperfRunner.__enter__
  (tools/testing/selftests/drivers/net/lib/py/kperf.py), a server is…
- [Low] selftests: drv-net: KperfRunner.__enter__() misses BaseException
  cleanup

--

selftests: drv-net: add a kperf runner

This adds a KperfRunner helper to the drv-net python library. It starts
kperf servers on the local and remote hosts, runs kperf-client, and
returns the aggregate throughput of both ends. Each server gets its own
PID and log file, and close() stops the servers with kperf-server --kill
and reports any cleanup failures.

> diff --git a/tools/testing/selftests/drivers/net/lib/py/kperf.py b/tools/testing/selftests/drivers/net/lib/py/kperf.py
> new file mode 100644
> index 0000000000000..8ee8c5ba2ff7b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/lib/py/kperf.py

[ ... ]

> +    def _server(self, remote, kill=False):
> +        host = self.env.remote if remote else None
> +        address = self.env.remote_addr if remote else self.env.addr
> +        args = ["kperf-server", "--pid-file", self._pid_file(remote)]
> +        if kill:
> +            args.append("--kill")
> +        else:
> +            args += ["--addr", address, "--port", str(self._port)]
> +            # The output of a daemon is lost, unless it can log
> +            usage = cmd("kperf-server --help", host=host, fail=False)
> +            if "--log-file" in usage.stdout + usage.stderr:
> +                args += ["--log-file", self._log_file(remote)]
> +        return cmd(shlex.join(args) if remote else args, host=host,
> +                   fail=not kill)

[ ... ]

> +    def __enter__(self):
> +        try:
> +            for remote in (False, True):
> +                self._server(remote)
> +                self._started.append(remote)

[Severity: Low]
A server is added to self._started only after self._server(remote)
returns successfully. What happens if kperf-server has already
daemonized and written its PID file, and the start command then raises?

For example, the remote ssh session could drop and exit with 255, or it
could stall past the default 20 second cmd() timeout after the daemon
has detached. CmdExitFailure or TimeoutExpired would be raised before
the append.

The except block below calls self.close(), which only walks
self._started, so --kill is never issued for that PID file. Could this
leave the daemon running, along with its PID and log files, and holding
devmem or RX steering into later tests?

Since --kill already runs with fail=False, one option is to record the
server before starting it. Another is to always issue --kill for both
PID files during cleanup.

> +            wait_port_listen(self._port)
> +            wait_port_listen(self._port, host=self.env.remote)
> +        except Exception:
> +            self.close()
> +            raise
> +        return self
> +
> +    def close(self):
> +        """Stop the started kperf servers, which waits for their sessions."""
> +        failures = []
> +        while self._started:
> +            remote = self._started.pop()
> +            command = self._server(remote, kill=True)
> +            if command.ret:
> +                failures.append(command)

[Severity: Low]
Does this rollback still run if the test is interrupted while the
servers are starting?

The kselftest SIGTERM handler, _ksft_intr() in
tools/testing/selftests/net/lib/py/ksft.py, raises KsftTerminate,
which subclasses KeyboardInterrupt. Neither of them derives from
Exception. A SIGTERM timeout from the runner, or a Ctrl-C, can arrive
while the second kperf-server is being started or while __enter__()
is polling in wait_port_listen(). In that case this handler is
skipped.

The with statement also does not call __exit__() when __enter__()
raises. So the servers already recorded in self._started never get a
kperf-server --kill. They would keep running in the background and
listening on the control port. Their PID and log files under
/tmp/ksft-kperf-* would also be left behind.

The handler re-raises anyway, so would it make sense to catch
BaseException here? That way the started servers are also stopped on
SIGTERM and KeyboardInterrupt.

[Severity: Medium]
Can the kill command raise here instead of returning a non-zero ret?

_server(remote, kill=True) calls cmd() with fail=False but keeps the
default timeout from tools/testing/selftests/net/lib/py/utils.py:

    def __init__(self, comm, shell=None, fail=True, expect_fail=False, ns=None,
                 background=False, host=None, timeout=20, ksft_ready=None,

That leads to cmd.process()->_process_terminate(), which does:

    stdout, stderr = self.proc.communicate(timeout=timeout)

fail=False only affects the return code check after communicate()
returns. If the kill takes longer than 20 seconds, subprocess.
TimeoutExpired is raised anyway. The commit message says this can
happen, since --kill waits for sessions that may still be releasing
devmem and RX steering. A stalled ssh link would trigger it too.

When that happens, the exception leaves the while self._started loop.
pop() handles the remote server first, so if the remote kill times
out:

  - the local kperf-server is never killed
  - neither log file is removed by the rm -f
  - the RuntimeError that collects the failures is skipped
  - the timed out kill subprocess keeps running, because communicate()
    does not kill the child on timeout

__exit__ calls close() only once, so would a stale server be left
holding the devmem or RX steering resources that the next test needs?

On the __enter__ error path, the TimeoutExpired from close() also
becomes the exception that is raised, and the original setup error is
only chained as context.

Would it make sense to pass a larger timeout to the kill command? The
loop could also catch exceptions per server, so the remaining servers
and log files still get cleaned up.

> +            host = self.env.remote if remote else None
> +            args = ["rm", "-f", self._log_file(remote)]
> +            cmd(shlex.join(args) if remote else args, host=host, fail=False)
> +        if failures:
> +            details = "\n".join(str(command) for command in failures)
> +            raise RuntimeError(f"failed to stop kperf server(s)\n{details}")
> +
> +    def __exit__(self, _exc_type, _exc_value, _exc_tb):
> +        self.close()

[ ... ]

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

Re: [PATCH net-next v4 4/4] selftests: drv-net: measure devmem performance with kperf

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 4 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] kperf() in tools/testing/selftests/drivers/net/perf/devmem.py
  never checks whether the device supports devmem before it changes the…
- [Medium] _enable_hw_gro() in
  tools/testing/selftests/drivers/net/perf/devmem.py (lines 24-31) makes…
- [Medium] In kperf()
  (tools/testing/selftests/drivers/net/perf/devmem.py), `defer(ethtool,…
- [Medium] kperf() saves `rings.get("tcp-data-split", "unknown")` (line
  53) and writes that value back explicitly in the deferred restore…

--

selftests: drv-net: measure devmem performance with kperf

This commit adds a udmabuf-backed devmem TCP throughput test to
drivers/net/perf. The test uses KperfRunner and reports the result as a
ktap metric. Before running kperf with devmem RX/TX, it raises the
udmabuf size limit and enables HW GRO and tcp-data-split.

> diff --git a/tools/testing/selftests/drivers/net/perf/devmem.py b/tools/testing/selftests/drivers/net/perf/devmem.py
> new file mode 100755
> index 0000000000000..4c1fb9b51fff9
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/perf/devmem.py
> @@ -0,0 +1,96 @@

[ ... ]

> +def _enable_hw_gro(cfg) -> None:
> +    features = ethtool(f"-k {cfg.ifname}", json=True)[0]
> +    hw_gro = features["rx-gro-hw"]
> +    if not hw_gro["active"]:
> +        if hw_gro["fixed"]:
> +            raise KsftSkipEx("Test requires rx-gro-hw support")
> +        ethtool(f"-K {cfg.ifname} rx-gro-hw on")
> +        defer(ethtool, f"-K {cfg.ifname} rx-gro-hw off")

[Severity: Medium]

Is HW GRO actually required for devmem here? The commit message says the
test skips "when kperf or /dev/udmabuf is missing". _enable_hw_gro() adds
a third skip condition: rx-gro-hw being fixed off.

Devmem does not depend on NETIF_F_GRO_HW in general. As far as I can tell,
only mlx5 ties tcp-data-split to GRO_HW, in
mlx5e_ethtool_set_tcp_data_split().

fbnic registers fbnic_queue_mgmt_ops and advertises
ETHTOOL_RING_USE_TCP_DATA_SPLIT, but it never sets NETIF_F_GRO_HW. Its
rx-gro-hw is reported as "off [fixed]", so this test would always skip
on fbnic.

On NICs where rx-gro-hw is off but not fixed, turning it on also changes
what the reported throughput metric measures compared with the device's
default configuration.

Could the HW GRO requirement be dropped? If it is needed, could the
commit message mention it and explain why?

> +
> +
> +@ksft_disruptive
> +def kperf(cfg) -> None:

[ ... ]

> +    # kperf reprograms the RSS indirection table for its queues.
> +    defer(ethtool, f"-X {cfg.ifname} default")
> +
> +    _enable_hw_gro(cfg)

[Severity: Medium]

Should this defer be registered later? It is armed before
_enable_hw_gro() can raise KsftSkipEx, and before anything has touched
RSS.

If rx-gro-hw is fixed off, the test skips, but ksft_flush_defer() still
runs "ethtool -X <ifname> default". On a device without set_rxfh support
that command exits non-zero, and ksft_flush_defer() sets
KSFT_RESULT = False. The output then becomes:

  not ok 1 ... # SKIP Test requires rx-gro-hw support

ktap_result() also clears KSFT_RESULT_ALL, so ksft_exit() returns 1, and
a plain skip turns into an overall failure.

On devices that do support set_rxfh (fbnic, for example, which always
takes the skip path above), any custom RSS indirection table gets reset
even though kperf never ran.

On the success path, too, the cleanup resets RSS to the default instead
of restoring the previous table.

> +
> +    ethnl = EthtoolFamily()
> +    rings = ethnl.rings_get({"header": {"dev-index": cfg.ifindex}})
> +    restore = {
> +        "header": {"dev-index": cfg.ifindex},
> +        "tcp-data-split": rings.get("tcp-data-split", "unknown"),
> +    }

[Severity: Medium]

Will this restore return the device to its original HDS configuration?
The other in-tree tests that enable tcp-data-split restore "unknown"
instead of writing back the reported value. These include configure_nic()
in hw/devmem_lib.py, hw/iou-zcrx.py, hw/nk_qlease.py and hds.py.

The snapshot is also taken after _enable_hw_gro() has already changed
the device. On bnxt, bnxt_get_ringparam() reports the live state rather
than hds_config:

drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:bnxt_get_ringparam() {
    ...
	if (bp->flags & BNXT_FLAG_AGG_RINGS) {
		...
		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
	} else {
		...
		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_DISABLED;
	}
    ...
}

With HW GRO enabled, the snapshot therefore reads "enabled". The deferred
rings_set() then sends ENABLED explicitly, and ethnl_set_rings() saves it
as the user configuration:

	dev->cfg_pending->hds_config = kernel_ringparam.tcp_data_split;

bnxt_set_ringparam() clears BNXT_FLAG_HDS only for UNKNOWN. Header split
stays forced on, and hds_config stays ENABLED instead of going back to
auto.

If the snapshot read "disabled" instead, bnxt_set_ringparam() rejects it
with -EINVAL because it differs from hds_config. The whole restore,
including hds-thresh, is dropped, and the failing defer marks the test
as failed.

In both cases header split is left on, and later non-frags XDP attaches
are refused in dev_xdp_attach():

	if (dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED &&
	    prog && !prog->aux->xdp_has_frags) {
		NL_SET_ERR_MSG(extack, "unable to install XDP to device using tcp-data-split");
		return -EBUSY;

That can break later tests on the same machine.

Would it be more robust to restore "unknown", as hw/devmem_lib.py does?

> +    config = {
> +        "header": {"dev-index": cfg.ifindex},
> +        "tcp-data-split": "enabled",
> +    }
> +    # Memory providers require a zero threshold where the driver has one.
> +    if "hds-thresh" in rings:
> +        restore["hds-thresh"] = rings["hds-thresh"]
> +        config["hds-thresh"] = 0
> +    defer(ethnl.rings_set, restore)
> +    ethnl.rings_set(config)

[Severity: Medium]

What happens here on a NIC that has HW GRO but no header-split or devmem
support? drivers/net/perf is a generic suite. The only skip conditions
are a missing kperf, a missing /dev/udmabuf, or rx-gro-hw being fixed
off.

Take virtio_net with guest TSO as an example. rx-gro-hw is active by
default, so _enable_hw_gro() passes. virtio_net does not advertise
ETHTOOL_RING_USE_TCP_DATA_SPLIT, so this rings_set() gets -EOPNOTSUPP
from ethnl_set_rings_validate(), and ksft_run() counts the NlError as a
failure.

The restore defer registered just above also sends tcp-data-split, so it
fails the same way. qede, hns3 and bnx2x appear to behave the same.

idpf and ice support tcp-data-split but have no queue_mgmt_ops. There
this rings_set() succeeds, but devmem binding fails, kperf-client exits
non-zero, and KperfRunner.run() raises RuntimeError. That is also
reported as a failure rather than a skip.

The hw tests handle this case with require_devmem() in hw/devmem_lib.py:

    if not cfg.devmem_supported[RX_PAGE_SIZE_DEFAULT]:
        raise KsftSkipEx("Test requires devmem support")

Should this test do an equivalent probe before it modifies the device?

[ ... ]

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