Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH net-next v2] selftests: net: add ctl_file_write() helper
@ 2026-09-09 18:00 Jakub Kicinski
  2026-09-09 18:39 ` Nimrod Oren
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jakub Kicinski @ 2026-09-09 18:00 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
	bobbyeshleman, shuah, cjubran, cratiu, noren, willemb, petrm,
	linux-kselftest

Setting a sysctl or a sysfs attribute for the duration of a test and
putting the old value back has been open coded multiple times.

We generally avoid creating library helpers but this one is very
common, and the defer is a little tricky as using the same function
for defer as the initial write leads to an infinite loop (not that
I would ever make such mistake!)

Some of the conversions are not identical, but arguably ctl_file_write()
semantics are more correct.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - rebase
 - drop the irq.py change which wasn't identical
v1: https://lore.kernel.org/20260904174134.3789400-1-kuba@kernel.org

CC: bobbyeshleman@gmail.com
CC: shuah@kernel.org
CC: cjubran@nvidia.com
CC: cratiu@nvidia.com
CC: noren@nvidia.com
CC: willemb@google.com
CC: petrm@nvidia.com
CC: linux-kselftest@vger.kernel.org
---
 .../testing/selftests/drivers/net/gro_lib.py  | 17 +++---------
 .../drivers/net/hw/devlink_rate_tc_bw.py      |  5 ++--
 .../drivers/net/hw/lib/py/__init__.py         |  4 +--
 .../selftests/drivers/net/hw/toeplitz.py      | 10 +++----
 .../selftests/drivers/net/lib/py/__init__.py  |  4 +--
 .../selftests/drivers/net/ring_reconfig.py    | 26 ++++---------------
 .../testing/selftests/net/lib/py/__init__.py  |  4 +--
 tools/testing/selftests/net/lib/py/utils.py   | 22 ++++++++++++++++
 8 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/gro_lib.py b/tools/testing/selftests/drivers/net/gro_lib.py
index 45ded8477a50..b7ac0660adc0 100644
--- a/tools/testing/selftests/drivers/net/gro_lib.py
+++ b/tools/testing/selftests/drivers/net/gro_lib.py
@@ -48,7 +48,7 @@ import re
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import NetDrvEpEnv, KsftFailEx, KsftXfailEx
 from lib.py import NetdevFamily, EthtoolFamily
-from lib.py import bkg, cmd, defer, ethtool, ip
+from lib.py import bkg, cmd, ctl_file_write, defer, ethtool, ip
 from lib.py import ksft_variants, KsftNamedVariant
 
 
@@ -85,17 +85,6 @@ GRO_DPORT = 8000
     return getattr(cfg, attr)
 
 
-def _write_defer_restore(cfg, path, val, defer_undo=False):
-    with open(path, "r", encoding="utf-8") as fp:
-        orig_val = fp.read().strip()
-        if str(val) == orig_val:
-            return
-    with open(path, "w", encoding="utf-8") as fp:
-        fp.write(val)
-    if defer_undo:
-        defer(_write_defer_restore, cfg, path, orig_val)
-
-
 def _set_mtu_restore(dev, mtu, host):
     if dev['mtu'] < mtu:
         ip(f"link set dev {dev['ifname']} mtu {mtu}", host=host)
@@ -250,8 +239,8 @@ def _run_gro_bin(cfg, test_name, protocol=None, num_flows=None,
         flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
         irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
 
-        _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
-        _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
+        ctl_file_write(flush_path, "200000")
+        ctl_file_write(irq_path, "10")
 
         _set_ethtool_feat(cfg.ifname, cfg.feat,
                           {"generic-receive-offload": True,
diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
index 4e4faa9275bb..38d712755257 100755
--- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
+++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
@@ -63,7 +63,7 @@ from lib.py import ksft_pr, ksft_run, ksft_exit
 from lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx
 from lib.py import NetDrvEpEnv, DevlinkFamily
 from lib.py import NlError
-from lib.py import cmd, defer, ethtool, ip
+from lib.py import cmd, ctl_file_write, defer, ethtool, ip
 from lib.py import Iperf3Runner
 
 
@@ -113,8 +113,7 @@ from lib.py import Iperf3Runner
     except Exception as exc:
         raise KsftSkipEx(f"Failed to enable switchdev mode on {cfg.pci}") from exc
     try:
-        cmd(f"echo 1 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
-        defer(cmd, f"echo 0 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
+        ctl_file_write(f"/sys/class/net/{cfg.ifname}/device/sriov_numvfs", 1)
     except Exception as exc:
         raise KsftSkipEx(f"Failed to enable SR-IOV on {cfg.ifname}") from exc
 
diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
index 8a58cb17cc06..81e1d1865cd5 100644
--- a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
@@ -24,7 +24,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
     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, tool
+        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_pr, ksft_run, \
@@ -40,7 +40,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
                "CmdExitFailure",
                "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
                "fd_read_timeout", "ip", "rand_port", "rand_ports",
-               "wait_port_listen", "wait_file", "tool",
+               "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_pr", "ksft_run",
diff --git a/tools/testing/selftests/drivers/net/hw/toeplitz.py b/tools/testing/selftests/drivers/net/hw/toeplitz.py
index 571732198b93..962d1aca2bc9 100755
--- a/tools/testing/selftests/drivers/net/hw/toeplitz.py
+++ b/tools/testing/selftests/drivers/net/hw/toeplitz.py
@@ -13,7 +13,7 @@ import os
 import socket
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import NetDrvEpEnv, EthtoolFamily, NetdevFamily
-from lib.py import cmd, bkg, rand_port, defer
+from lib.py import cmd, bkg, ctl_file_write, rand_port, defer
 from lib.py import ksft_in
 from lib.py import ksft_variants, KsftNamedVariant, KsftSkipEx, KsftFailEx
 
@@ -113,7 +113,7 @@ QUEUE_CAP = 8
 
 
 def _configure_rps(cfg, rps_cpus):
-    """Configure RPS for all Rx queues."""
+    """Configure RPS for all Rx queues, restored at the end of the test."""
 
     mask = 0
     for cpu in rps_cpus:
@@ -123,9 +123,8 @@ QUEUE_CAP = 8
 
     # Set RPS bitmap for all rx queues
     for rps_file in glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*/rps_cpus"):
-        with open(rps_file, "w", encoding="utf-8") as fp:
-            # sysfs expects hex without '0x' prefix, toeplitz.c needs the prefix
-            fp.write(mask[2:])
+        # sysfs expects hex without '0x' prefix, toeplitz.c needs the prefix
+        ctl_file_write(rps_file, mask[2:])
 
     return mask
 
@@ -208,7 +207,6 @@ QUEUE_CAP = 8
         # Get CPUs not used by Rx queues and configure them for RPS
         rps_cpus = _get_unused_rps_cpus(cfg, count=2)
         rps_mask = _configure_rps(cfg, rps_cpus)
-        defer(_configure_rps, cfg, [])
         rx_cmd += ["-r", rps_mask]
         ksft_pr(f"RPS using CPUs: {rps_cpus}, mask: {rps_mask}")
 
diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
index ee903bcf3207..591b1e6c7eea 100644
--- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
@@ -24,7 +24,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
     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, tc, wait_port_listen, \
-        wait_file
+        wait_file, ctl_file_write
     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_pr, ksft_run, \
@@ -38,7 +38,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
                "CmdExitFailure",
                "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
                "fd_read_timeout", "ip", "rand_port", "rand_ports", "tc",
-               "wait_port_listen", "wait_file",
+               "wait_port_listen", "wait_file", "ctl_file_write",
                "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
                "KsftSkipEx", "KsftFailEx", "KsftXfailEx",
                "ksft_disruptive", "ksft_exit", "ksft_pr", "ksft_run",
diff --git a/tools/testing/selftests/drivers/net/ring_reconfig.py b/tools/testing/selftests/drivers/net/ring_reconfig.py
index 2bc329b77134..6682d4badfbf 100755
--- a/tools/testing/selftests/drivers/net/ring_reconfig.py
+++ b/tools/testing/selftests/drivers/net/ring_reconfig.py
@@ -13,7 +13,7 @@ from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import ksft_eq
 from lib.py import KsftSkipEx, KsftXfailEx
 from lib.py import NetDrvEpEnv, EthtoolFamily, GenerateTraffic
-from lib.py import cmd, defer, rand_port, tc, NlError
+from lib.py import cmd, ctl_file_write, defer, rand_port, tc, NlError
 
 # Added in Python 3.13; fallback to 61 for x86/ARM/MIPS
 SO_TXTIME = getattr(socket, "SO_TXTIME", 61)
@@ -166,22 +166,6 @@ MAX_TX_RING = 1024
         GenerateTraffic(cfg).wait_pkts_and_stop(10000)
 
 
-def _write_file(path, val):
-    """Write val to a file."""
-    with open(path, "w", encoding="utf-8") as fp:
-        fp.write(str(val))
-
-
-def _write_sysfs(path, val):
-    """Write val to a sysfs file, restoring the original value on exit."""
-    with open(path, "r", encoding="utf-8") as fp:
-        orig_val = fp.read().strip()
-    if str(val) == orig_val:
-        return
-    _write_file(path, val)
-    defer(_write_file, path, orig_val)
-
-
 def _get_qdisc_backlog(cfg, mq_handle, queue):
     """Return the qdisc backlog (bytes) for the given TX queue's leaf."""
     target_parent = f"{mq_handle}{queue + 1:x}"
@@ -342,8 +326,8 @@ MAX_TX_RING = 1024
     # Slow completions so the ring stays full after FQ releases packets
     napi_defer = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
     gro_timeout = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
-    _write_sysfs(napi_defer, 100)
-    _write_sysfs(gro_timeout, 1000000000)
+    ctl_file_write(napi_defer, 100)
+    ctl_file_write(gro_timeout, 1000000000)
 
     port = rand_port()
     # A single flow must overflow the ring, so send twice the ring depth and
@@ -384,8 +368,8 @@ MAX_TX_RING = 1024
     cfg.eth.rings_set(ehdr | {'tx': tx_cur})
 
     # Let completions proceed normally
-    _write_sysfs(napi_defer, 0)
-    _write_sysfs(gro_timeout, 0)
+    ctl_file_write(napi_defer, 0)
+    ctl_file_write(gro_timeout, 0)
 
     # Poll for backlog to drain
     for _ in range(100):
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index 34935886b6ad..71df5880b356 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -14,7 +14,7 @@ from .netns import NetNS, NetNSEnter, UserNetNS
 from .nsim import NetdevSim, NetdevSimDev
 from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
     bpftool, ip, ethtool, bpftrace, rand_port, rand_ports, wait_port_listen, \
-    wait_file, tool, tc
+    ctl_file_write, wait_file, tool, tc
 from .bpf import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
 from .ynl import NlError, NlctrlFamily, YnlFamily, \
     EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily, RtnlRouteFamily
@@ -29,7 +29,7 @@ __all__ = ["KSRC",
            "NetNS", "NetNSEnter", "UserNetNS",
            "CmdExitFailure", "fd_read_timeout", "cmd", "bkg", "defer",
            "bpftool", "ip", "ethtool", "bpftrace", "rand_port", "rand_ports",
-           "wait_port_listen", "wait_file", "tool", "tc",
+           "wait_port_listen", "ctl_file_write", "wait_file", "tool", "tc",
            "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
            "NetdevSim", "NetdevSimDev",
            "NetshaperFamily", "DevlinkFamily", "PSPFamily", "NlError",
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 184bb04343f6..1727bc418073 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -357,6 +357,28 @@ GLOBAL_DEFER_ARMED = False
         time.sleep(sleep)
 
 
+def _ctl_file_write(path, val):
+    with open(path, "w", encoding="utf-8") as fp:
+        fp.write(str(val))
+
+
+def ctl_file_write(path, val):
+    """
+    Write @val to a control file - a sysctl, a sysfs attribute, configfs...
+    and defer() restoring the old value, so needs a defer queue.
+
+    Writing a value which is already set is skipped, there is nothing
+    to restore in that case.
+    """
+    with open(path, "r", encoding="utf-8") as fp:
+        old = fp.read().strip()
+    if old == str(val):
+        return
+
+    _ctl_file_write(path, val)
+    defer(_ctl_file_write, path, old)
+
+
 def wait_file(fname, test_fn, sleep=0.005, deadline=5, encoding='utf-8'):
     """
     Wait for file contents on the local system to satisfy a condition.
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] selftests: net: add ctl_file_write() helper
  2026-09-09 18:00 [PATCH net-next v2] selftests: net: add ctl_file_write() helper Jakub Kicinski
@ 2026-09-09 18:39 ` Nimrod Oren
  2026-09-09 18:42 ` Bobby Eshleman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Nimrod Oren @ 2026-09-09 18:39 UTC (permalink / raw)
  To: Jakub Kicinski, davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, bobbyeshleman,
	shuah, cjubran, cratiu, willemb, petrm, linux-kselftest

On 09/09/2026 21:00, Jakub Kicinski wrote:
> Setting a sysctl or a sysfs attribute for the duration of a test and
> putting the old value back has been open coded multiple times.
> 
> We generally avoid creating library helpers but this one is very
> common, and the defer is a little tricky as using the same function
> for defer as the initial write leads to an infinite loop (not that
> I would ever make such mistake!)
> 
> Some of the conversions are not identical, but arguably ctl_file_write()
> semantics are more correct.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - rebase
>  - drop the irq.py change which wasn't identical
> v1: https://lore.kernel.org/20260904174134.3789400-1-kuba@kernel.org
> 
> CC: bobbyeshleman@gmail.com
> CC: shuah@kernel.org
> CC: cjubran@nvidia.com
> CC: cratiu@nvidia.com
> CC: noren@nvidia.com
> CC: willemb@google.com
> CC: petrm@nvidia.com
> CC: linux-kselftest@vger.kernel.org
> ---
>  .../testing/selftests/drivers/net/gro_lib.py  | 17 +++---------
>  .../drivers/net/hw/devlink_rate_tc_bw.py      |  5 ++--
>  .../drivers/net/hw/lib/py/__init__.py         |  4 +--
>  .../selftests/drivers/net/hw/toeplitz.py      | 10 +++----
>  .../selftests/drivers/net/lib/py/__init__.py  |  4 +--
>  .../selftests/drivers/net/ring_reconfig.py    | 26 ++++---------------
>  .../testing/selftests/net/lib/py/__init__.py  |  4 +--
>  tools/testing/selftests/net/lib/py/utils.py   | 22 ++++++++++++++++
>  8 files changed, 42 insertions(+), 50 deletions(-)
Reviewed-by: Nimrod Oren <noren@nvidia.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] selftests: net: add ctl_file_write() helper
  2026-09-09 18:00 [PATCH net-next v2] selftests: net: add ctl_file_write() helper Jakub Kicinski
  2026-09-09 18:39 ` Nimrod Oren
@ 2026-09-09 18:42 ` Bobby Eshleman
  2026-09-10 18:00 ` netdev-bot+sashiko
  2026-09-11  1:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Bobby Eshleman @ 2026-09-09 18:42 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
	cjubran, cratiu, noren, willemb, petrm, linux-kselftest

On Wed, Sep 09, 2026 at 11:00:09AM -0700, Jakub Kicinski wrote:
> Setting a sysctl or a sysfs attribute for the duration of a test and
> putting the old value back has been open coded multiple times.
> 
> We generally avoid creating library helpers but this one is very
> common, and the defer is a little tricky as using the same function
> for defer as the initial write leads to an infinite loop (not that
> I would ever make such mistake!)
> 
> Some of the conversions are not identical, but arguably ctl_file_write()
> semantics are more correct.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - rebase
>  - drop the irq.py change which wasn't identical
> v1: https://lore.kernel.org/20260904174134.3789400-1-kuba@kernel.org
> 
> CC: bobbyeshleman@gmail.com
> CC: shuah@kernel.org
> CC: cjubran@nvidia.com
> CC: cratiu@nvidia.com
> CC: noren@nvidia.com
> CC: willemb@google.com
> CC: petrm@nvidia.com
> CC: linux-kselftest@vger.kernel.org
> ---
>  .../testing/selftests/drivers/net/gro_lib.py  | 17 +++---------
>  .../drivers/net/hw/devlink_rate_tc_bw.py      |  5 ++--
>  .../drivers/net/hw/lib/py/__init__.py         |  4 +--
>  .../selftests/drivers/net/hw/toeplitz.py      | 10 +++----
>  .../selftests/drivers/net/lib/py/__init__.py  |  4 +--
>  .../selftests/drivers/net/ring_reconfig.py    | 26 ++++---------------
>  .../testing/selftests/net/lib/py/__init__.py  |  4 +--
>  tools/testing/selftests/net/lib/py/utils.py   | 22 ++++++++++++++++
>  8 files changed, 42 insertions(+), 50 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/gro_lib.py b/tools/testing/selftests/drivers/net/gro_lib.py
> index 45ded8477a50..b7ac0660adc0 100644
> --- a/tools/testing/selftests/drivers/net/gro_lib.py
> +++ b/tools/testing/selftests/drivers/net/gro_lib.py
> @@ -48,7 +48,7 @@ import re
>  from lib.py import ksft_run, ksft_exit, ksft_pr
>  from lib.py import NetDrvEpEnv, KsftFailEx, KsftXfailEx
>  from lib.py import NetdevFamily, EthtoolFamily
> -from lib.py import bkg, cmd, defer, ethtool, ip
> +from lib.py import bkg, cmd, ctl_file_write, defer, ethtool, ip
>  from lib.py import ksft_variants, KsftNamedVariant
>  
>  
> @@ -85,17 +85,6 @@ GRO_DPORT = 8000
>      return getattr(cfg, attr)
>  
>  
> -def _write_defer_restore(cfg, path, val, defer_undo=False):
> -    with open(path, "r", encoding="utf-8") as fp:
> -        orig_val = fp.read().strip()
> -        if str(val) == orig_val:
> -            return
> -    with open(path, "w", encoding="utf-8") as fp:
> -        fp.write(val)
> -    if defer_undo:
> -        defer(_write_defer_restore, cfg, path, orig_val)
> -
> -
>  def _set_mtu_restore(dev, mtu, host):
>      if dev['mtu'] < mtu:
>          ip(f"link set dev {dev['ifname']} mtu {mtu}", host=host)
> @@ -250,8 +239,8 @@ def _run_gro_bin(cfg, test_name, protocol=None, num_flows=None,
>          flush_path = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
>          irq_path = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
>  
> -        _write_defer_restore(cfg, flush_path, "200000", defer_undo=True)
> -        _write_defer_restore(cfg, irq_path, "10", defer_undo=True)
> +        ctl_file_write(flush_path, "200000")
> +        ctl_file_write(irq_path, "10")
>  
>          _set_ethtool_feat(cfg.ifname, cfg.feat,
>                            {"generic-receive-offload": True,
> diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
> index 4e4faa9275bb..38d712755257 100755
> --- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
> +++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
> @@ -63,7 +63,7 @@ from lib.py import ksft_pr, ksft_run, ksft_exit
>  from lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx
>  from lib.py import NetDrvEpEnv, DevlinkFamily
>  from lib.py import NlError
> -from lib.py import cmd, defer, ethtool, ip
> +from lib.py import cmd, ctl_file_write, defer, ethtool, ip
>  from lib.py import Iperf3Runner
>  
>  
> @@ -113,8 +113,7 @@ from lib.py import Iperf3Runner
>      except Exception as exc:
>          raise KsftSkipEx(f"Failed to enable switchdev mode on {cfg.pci}") from exc
>      try:
> -        cmd(f"echo 1 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
> -        defer(cmd, f"echo 0 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
> +        ctl_file_write(f"/sys/class/net/{cfg.ifname}/device/sriov_numvfs", 1)
>      except Exception as exc:
>          raise KsftSkipEx(f"Failed to enable SR-IOV on {cfg.ifname}") from exc
>  
> diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> index 8a58cb17cc06..81e1d1865cd5 100644
> --- a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> +++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> @@ -24,7 +24,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
>      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, tool
> +        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_pr, ksft_run, \
> @@ -40,7 +40,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve()
>                 "CmdExitFailure",
>                 "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
>                 "fd_read_timeout", "ip", "rand_port", "rand_ports",
> -               "wait_port_listen", "wait_file", "tool",
> +               "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_pr", "ksft_run",
> diff --git a/tools/testing/selftests/drivers/net/hw/toeplitz.py b/tools/testing/selftests/drivers/net/hw/toeplitz.py
> index 571732198b93..962d1aca2bc9 100755
> --- a/tools/testing/selftests/drivers/net/hw/toeplitz.py
> +++ b/tools/testing/selftests/drivers/net/hw/toeplitz.py
> @@ -13,7 +13,7 @@ import os
>  import socket
>  from lib.py import ksft_run, ksft_exit, ksft_pr
>  from lib.py import NetDrvEpEnv, EthtoolFamily, NetdevFamily
> -from lib.py import cmd, bkg, rand_port, defer
> +from lib.py import cmd, bkg, ctl_file_write, rand_port, defer
>  from lib.py import ksft_in
>  from lib.py import ksft_variants, KsftNamedVariant, KsftSkipEx, KsftFailEx
>  
> @@ -113,7 +113,7 @@ QUEUE_CAP = 8
>  
>  
>  def _configure_rps(cfg, rps_cpus):
> -    """Configure RPS for all Rx queues."""
> +    """Configure RPS for all Rx queues, restored at the end of the test."""
>  
>      mask = 0
>      for cpu in rps_cpus:
> @@ -123,9 +123,8 @@ QUEUE_CAP = 8
>  
>      # Set RPS bitmap for all rx queues
>      for rps_file in glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*/rps_cpus"):
> -        with open(rps_file, "w", encoding="utf-8") as fp:
> -            # sysfs expects hex without '0x' prefix, toeplitz.c needs the prefix
> -            fp.write(mask[2:])
> +        # sysfs expects hex without '0x' prefix, toeplitz.c needs the prefix
> +        ctl_file_write(rps_file, mask[2:])
>  
>      return mask
>  
> @@ -208,7 +207,6 @@ QUEUE_CAP = 8
>          # Get CPUs not used by Rx queues and configure them for RPS
>          rps_cpus = _get_unused_rps_cpus(cfg, count=2)
>          rps_mask = _configure_rps(cfg, rps_cpus)
> -        defer(_configure_rps, cfg, [])
>          rx_cmd += ["-r", rps_mask]
>          ksft_pr(f"RPS using CPUs: {rps_cpus}, mask: {rps_mask}")
>  
> diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
> index ee903bcf3207..591b1e6c7eea 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
> @@ -24,7 +24,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
>      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, tc, wait_port_listen, \
> -        wait_file
> +        wait_file, ctl_file_write
>      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_pr, ksft_run, \
> @@ -38,7 +38,7 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
>                 "CmdExitFailure",
>                 "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
>                 "fd_read_timeout", "ip", "rand_port", "rand_ports", "tc",
> -               "wait_port_listen", "wait_file",
> +               "wait_port_listen", "wait_file", "ctl_file_write",
>                 "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
>                 "KsftSkipEx", "KsftFailEx", "KsftXfailEx",
>                 "ksft_disruptive", "ksft_exit", "ksft_pr", "ksft_run",
> diff --git a/tools/testing/selftests/drivers/net/ring_reconfig.py b/tools/testing/selftests/drivers/net/ring_reconfig.py
> index 2bc329b77134..6682d4badfbf 100755
> --- a/tools/testing/selftests/drivers/net/ring_reconfig.py
> +++ b/tools/testing/selftests/drivers/net/ring_reconfig.py
> @@ -13,7 +13,7 @@ from lib.py import ksft_run, ksft_exit, ksft_pr
>  from lib.py import ksft_eq
>  from lib.py import KsftSkipEx, KsftXfailEx
>  from lib.py import NetDrvEpEnv, EthtoolFamily, GenerateTraffic
> -from lib.py import cmd, defer, rand_port, tc, NlError
> +from lib.py import cmd, ctl_file_write, defer, rand_port, tc, NlError
>  
>  # Added in Python 3.13; fallback to 61 for x86/ARM/MIPS
>  SO_TXTIME = getattr(socket, "SO_TXTIME", 61)
> @@ -166,22 +166,6 @@ MAX_TX_RING = 1024
>          GenerateTraffic(cfg).wait_pkts_and_stop(10000)
>  
>  
> -def _write_file(path, val):
> -    """Write val to a file."""
> -    with open(path, "w", encoding="utf-8") as fp:
> -        fp.write(str(val))
> -
> -
> -def _write_sysfs(path, val):
> -    """Write val to a sysfs file, restoring the original value on exit."""
> -    with open(path, "r", encoding="utf-8") as fp:
> -        orig_val = fp.read().strip()
> -    if str(val) == orig_val:
> -        return
> -    _write_file(path, val)
> -    defer(_write_file, path, orig_val)
> -
> -
>  def _get_qdisc_backlog(cfg, mq_handle, queue):
>      """Return the qdisc backlog (bytes) for the given TX queue's leaf."""
>      target_parent = f"{mq_handle}{queue + 1:x}"
> @@ -342,8 +326,8 @@ MAX_TX_RING = 1024
>      # Slow completions so the ring stays full after FQ releases packets
>      napi_defer = f"/sys/class/net/{cfg.ifname}/napi_defer_hard_irqs"
>      gro_timeout = f"/sys/class/net/{cfg.ifname}/gro_flush_timeout"
> -    _write_sysfs(napi_defer, 100)
> -    _write_sysfs(gro_timeout, 1000000000)
> +    ctl_file_write(napi_defer, 100)
> +    ctl_file_write(gro_timeout, 1000000000)
>  
>      port = rand_port()
>      # A single flow must overflow the ring, so send twice the ring depth and
> @@ -384,8 +368,8 @@ MAX_TX_RING = 1024
>      cfg.eth.rings_set(ehdr | {'tx': tx_cur})
>  
>      # Let completions proceed normally
> -    _write_sysfs(napi_defer, 0)
> -    _write_sysfs(gro_timeout, 0)
> +    ctl_file_write(napi_defer, 0)
> +    ctl_file_write(gro_timeout, 0)
>  
>      # Poll for backlog to drain
>      for _ in range(100):
> diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
> index 34935886b6ad..71df5880b356 100644
> --- a/tools/testing/selftests/net/lib/py/__init__.py
> +++ b/tools/testing/selftests/net/lib/py/__init__.py
> @@ -14,7 +14,7 @@ from .netns import NetNS, NetNSEnter, UserNetNS
>  from .nsim import NetdevSim, NetdevSimDev
>  from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
>      bpftool, ip, ethtool, bpftrace, rand_port, rand_ports, wait_port_listen, \
> -    wait_file, tool, tc
> +    ctl_file_write, wait_file, tool, tc
>  from .bpf import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
>  from .ynl import NlError, NlctrlFamily, YnlFamily, \
>      EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily, RtnlRouteFamily
> @@ -29,7 +29,7 @@ __all__ = ["KSRC",
>             "NetNS", "NetNSEnter", "UserNetNS",
>             "CmdExitFailure", "fd_read_timeout", "cmd", "bkg", "defer",
>             "bpftool", "ip", "ethtool", "bpftrace", "rand_port", "rand_ports",
> -           "wait_port_listen", "wait_file", "tool", "tc",
> +           "wait_port_listen", "ctl_file_write", "wait_file", "tool", "tc",
>             "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
>             "NetdevSim", "NetdevSimDev",
>             "NetshaperFamily", "DevlinkFamily", "PSPFamily", "NlError",
> diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> index 184bb04343f6..1727bc418073 100644
> --- a/tools/testing/selftests/net/lib/py/utils.py
> +++ b/tools/testing/selftests/net/lib/py/utils.py
> @@ -357,6 +357,28 @@ GLOBAL_DEFER_ARMED = False
>          time.sleep(sleep)
>  
>  
> +def _ctl_file_write(path, val):
> +    with open(path, "w", encoding="utf-8") as fp:
> +        fp.write(str(val))
> +
> +
> +def ctl_file_write(path, val):
> +    """
> +    Write @val to a control file - a sysctl, a sysfs attribute, configfs...
> +    and defer() restoring the old value, so needs a defer queue.
> +
> +    Writing a value which is already set is skipped, there is nothing
> +    to restore in that case.
> +    """
> +    with open(path, "r", encoding="utf-8") as fp:
> +        old = fp.read().strip()
> +    if old == str(val):
> +        return
> +
> +    _ctl_file_write(path, val)
> +    defer(_ctl_file_write, path, old)
> +
> +
>  def wait_file(fname, test_fn, sleep=0.005, deadline=5, encoding='utf-8'):
>      """
>      Wait for file contents on the local system to satisfy a condition.
> -- 
> 2.55.0
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] selftests: net: add ctl_file_write() helper
  2026-09-09 18:00 [PATCH net-next v2] selftests: net: add ctl_file_write() helper Jakub Kicinski
  2026-09-09 18:39 ` Nimrod Oren
  2026-09-09 18:42 ` Bobby Eshleman
@ 2026-09-10 18:00 ` netdev-bot+sashiko
  2026-09-11  1:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 18:00 UTC (permalink / raw)
  To: kuba
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	bobbyeshleman, shuah, cjubran, cratiu, noren, willemb, petrm,
	linux-kselftest

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

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

- [Medium] In `setup_vf()`
  (tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py:110-118)…

--

selftests: net: add ctl_file_write() helper

This adds a ctl_file_write() helper to the net selftest python library which
writes a value to a sysctl/sysfs/configfs file and defers restoring the old
value, and converts several open coded versions of the same pattern in
gro_lib.py, devlink_rate_tc_bw.py, toeplitz.py and ring_reconfig.py to use it.

> diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
> index 4e4faa9275bb..38d712755257 100755
> --- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
> +++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py

[ ... ]

> @@ -113,8 +113,7 @@ def setup_vf(cfg, set_tc_mapping=True):
>      except Exception as exc:
>          raise KsftSkipEx(f"Failed to enable switchdev mode on {cfg.pci}") from exc
>      try:
> -        cmd(f"echo 1 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
> -        defer(cmd, f"echo 0 > /sys/class/net/{cfg.ifname}/device/sriov_numvfs", shell=True)
> +        ctl_file_write(f"/sys/class/net/{cfg.ifname}/device/sriov_numvfs", 1)
>      except Exception as exc:
>          raise KsftSkipEx(f"Failed to enable SR-IOV on {cfg.ifname}") from exc

[Severity: Medium]
Is sriov_numvfs a good fit for the "skip the write if the value is already
set" semantics?

If the interface already has one VF enabled when setup_vf() runs,
ctl_file_write() takes the early return:

tools/testing/selftests/net/lib/py/utils.py:ctl_file_write() {
    with open(path, "r", encoding="utf-8") as fp:
        old = fp.read().strip()
    if old == str(val):
        return
    ...
}

so no defer is queued and the VF is never destroyed at the end of the test.
The previous code wrote 1 unconditionally, and the kernel accepts that write
even when the count already matches:

drivers/pci/iov.c:sriov_numvfs_store() {
	...
	if (num_vfs == pdev->sriov->num_VFs)
		goto exit;
	...
}

so the "echo 0" teardown was always armed.

Does the VF teardown matter for anything besides the VF itself here?
setup_vlans_on_vf() registers no defers of its own:

tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py:setup_vlans_on_vf() {
    for config in vlan_configs:
        vlan_dev = f"{vf_ifc}.{config['vlan_id']}"
        ip(f"link add link {vf_ifc} name {vlan_dev} type vlan id {config['vlan_id']}")
        ip(f"addr add {config['ip']}/29 dev {vlan_dev}")
        ...
}

and setup_vf() itself adds "tc qdisc add dev {vf_ifc} root handle 5 mqprio"
with no defer, while setup_devlink_rate() programs rate-tx-max/rate-tc-bws on
the VF port with no defer. Those all used to disappear together with the VF.

With the VF surviving, does the second and every later test case in the run
fail when setup_test_environment() re-runs setup_vlans_on_vf() and
"ip link add link <vf> name <vf>.101 type vlan id 101" hits "File exists"?
The VF would also be left carrying the 125 Mbps tx_max rate limit and the
mqprio qdisc for unrelated later tests.

This state is reachable on a host where an admin pre-enabled one VF, and also
after any earlier run of this same test that was killed or timed out before
its defer queue drained.

One option would be to keep the explicit write plus defer here, since this
attribute instantiates a device rather than holding a tunable value.

[ ... ]

> diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> index 184bb04343f6..1727bc418073 100644
> --- a/tools/testing/selftests/net/lib/py/utils.py
> +++ b/tools/testing/selftests/net/lib/py/utils.py
> @@ -357,6 +357,28 @@ def wait_port_listen(port, proto="tcp", ns=None, host=None, sleep=0.005, deadlin
>          time.sleep(sleep)
>  
>  
> +def _ctl_file_write(path, val):
> +    with open(path, "w", encoding="utf-8") as fp:
> +        fp.write(str(val))
> +
> +
> +def ctl_file_write(path, val):
> +    """
> +    Write @val to a control file - a sysctl, a sysfs attribute, configfs...
> +    and defer() restoring the old value, so needs a defer queue.
> +
> +    Writing a value which is already set is skipped, there is nothing
> +    to restore in that case.
> +    """
> +    with open(path, "r", encoding="utf-8") as fp:
> +        old = fp.read().strip()
> +    if old == str(val):
> +        return
> +
> +    _ctl_file_write(path, val)
> +    defer(_ctl_file_write, path, old)

Would it be worth documenting in this docstring that the skip makes the
helper unsuitable for attributes whose write has a side effect beyond the
value, such as sriov_numvfs creating and destroying VF devices?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909180009.1894019-1-kuba%40kernel.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] selftests: net: add ctl_file_write() helper
  2026-09-09 18:00 [PATCH net-next v2] selftests: net: add ctl_file_write() helper Jakub Kicinski
                   ` (2 preceding siblings ...)
  2026-09-10 18:00 ` netdev-bot+sashiko
@ 2026-09-11  1:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-11  1:50 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
	bobbyeshleman, shuah, cjubran, cratiu, noren, willemb, petrm,
	linux-kselftest

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  9 Sep 2026 11:00:09 -0700 you wrote:
> Setting a sysctl or a sysfs attribute for the duration of a test and
> putting the old value back has been open coded multiple times.
> 
> We generally avoid creating library helpers but this one is very
> common, and the defer is a little tricky as using the same function
> for defer as the initial write leads to an infinite loop (not that
> I would ever make such mistake!)
> 
> [...]

Here is the summary with links:
  - [net-next,v2] selftests: net: add ctl_file_write() helper
    https://git.kernel.org/netdev/net-next/c/348ea4642f56

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-11  1:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 18:00 [PATCH net-next v2] selftests: net: add ctl_file_write() helper Jakub Kicinski
2026-09-09 18:39 ` Nimrod Oren
2026-09-09 18:42 ` Bobby Eshleman
2026-09-10 18:00 ` netdev-bot+sashiko
2026-09-11  1:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox