From 6e9a12f85a7567bb9a41d5230468886bd6a27b20 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 11 Aug 2025 16:13:33 -0700 Subject: selftests: net: terminate bkg() commands on exception There is a number of: with bkg("socat ..LISTEN..", exit_wait=True) uses in the tests. If whatever is supposed to send the traffic fails we will get stuck in the bkg(). Try to kill the process in case of exception, to avoid the long wait. A specific example where this happens is the devmem Tx tests. Reviewed-by: Joe Damato Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20250811231334.561137-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tools/testing/selftests/net/lib/py/utils.py') diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index f395c90fb0f1..4ac9249c85ab 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -117,6 +117,7 @@ class bkg(cmd): shell=shell, fail=fail, ns=ns, host=host, ksft_wait=ksft_wait) self.terminate = not exit_wait and not ksft_wait + self._exit_wait = exit_wait self.check_fail = fail if shell and self.terminate: @@ -127,7 +128,9 @@ class bkg(cmd): return self def __exit__(self, ex_type, ex_value, ex_tb): - return self.process(terminate=self.terminate, fail=self.check_fail) + # Force termination on exception + terminate = self.terminate or (self._exit_wait and ex_type) + return self.process(terminate=terminate, fail=self.check_fail) global_defer_queue = [] -- cgit v1.2.3 From f09fc24dd9a5ec989dfdde7090624924ede6ddc7 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 12 Aug 2025 07:20:54 -0700 Subject: selftests: drv-net: wait for carrier On fast machines the tests run in quick succession so even when tests clean up after themselves the carrier may need some time to come back. Specifically in NIPA when ping.py runs right after netpoll_basic.py the first ping command fails. Since the context manager callbacks are now common NetDrvEpEnv gets an ip link up call as well. Reviewed-by: Joe Damato Link: https://patch.msgid.link/20250812142054.750282-1-kuba@kernel.org Signed-off-by: Jakub Kicinski --- .../selftests/drivers/net/lib/py/__init__.py | 2 +- tools/testing/selftests/drivers/net/lib/py/env.py | 41 +++++++++++----------- tools/testing/selftests/net/lib/py/utils.py | 18 ++++++++++ 3 files changed, 39 insertions(+), 22 deletions(-) (limited to 'tools/testing/selftests/net/lib/py/utils.py') diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py index 8711c67ad658..a07b56a75c8a 100644 --- a/tools/testing/selftests/drivers/net/lib/py/__init__.py +++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py @@ -15,7 +15,7 @@ try: NlError, RtnlFamily, DevlinkFamily from net.lib.py import CmdExitFailure from net.lib.py import bkg, cmd, bpftool, bpftrace, defer, ethtool, \ - fd_read_timeout, ip, rand_port, tool, wait_port_listen + fd_read_timeout, ip, rand_port, tool, wait_port_listen, wait_file from net.lib.py import fd_read_timeout from net.lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx from net.lib.py import ksft_disruptive, ksft_exit, ksft_pr, ksft_run, \ diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index 1b8bd648048f..c1f3b608c6d8 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -4,7 +4,7 @@ import os import time from pathlib import Path from lib.py import KsftSkipEx, KsftXfailEx -from lib.py import ksft_setup +from lib.py import ksft_setup, wait_file from lib.py import cmd, ethtool, ip, CmdExitFailure from lib.py import NetNS, NetdevSimDev from .remote import Remote @@ -25,6 +25,9 @@ class NetDrvEnvBase: self.env = self._load_env_file() + # Following attrs must be set be inheriting classes + self.dev = None + def _load_env_file(self): env = os.environ.copy() @@ -48,6 +51,22 @@ class NetDrvEnvBase: env[pair[0]] = pair[1] return ksft_setup(env) + def __del__(self): + pass + + def __enter__(self): + ip(f"link set dev {self.dev['ifname']} up") + wait_file(f"/sys/class/net/{self.dev['ifname']}/carrier", + lambda x: x.strip() == "1") + + return self + + def __exit__(self, ex_type, ex_value, ex_tb): + """ + __exit__ gets called at the end of a "with" block. + """ + self.__del__() + class NetDrvEnv(NetDrvEnvBase): """ @@ -72,17 +91,6 @@ class NetDrvEnv(NetDrvEnvBase): self.ifname = self.dev['ifname'] self.ifindex = self.dev['ifindex'] - def __enter__(self): - ip(f"link set dev {self.dev['ifname']} up") - - return self - - def __exit__(self, ex_type, ex_value, ex_tb): - """ - __exit__ gets called at the end of a "with" block. - """ - self.__del__() - def __del__(self): if self._ns: self._ns.remove() @@ -219,15 +227,6 @@ class NetDrvEpEnv(NetDrvEnvBase): raise Exception("Can't resolve remote interface name, multiple interfaces match") return v6[0]["ifname"] if v6 else v4[0]["ifname"] - def __enter__(self): - return self - - def __exit__(self, ex_type, ex_value, ex_tb): - """ - __exit__ gets called at the end of a "with" block. - """ - self.__del__() - def __del__(self): if self._ns: self._ns.remove() diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index 4ac9249c85ab..b188cac49738 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -252,3 +252,21 @@ def wait_port_listen(port, proto="tcp", ns=None, host=None, sleep=0.005, deadlin if time.monotonic() > end: raise Exception("Waiting for port listen timed out") time.sleep(sleep) + + +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. + test_fn() should take one argument (file contents) and return whether + condition is met. + """ + end = time.monotonic() + deadline + + with open(fname, "r", encoding=encoding) as fp: + while True: + if test_fn(fp.read()): + break + fp.seek(0) + if time.monotonic() > end: + raise TimeoutError("Wait for file contents failed", fname) + time.sleep(sleep) -- cgit v1.2.3 From 23313771c7b99b3b8dba169bc71dae619d41ab56 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 1 Sep 2025 03:00:07 -0700 Subject: net: selftests: clean up tools/testing/selftests/net/lib/py/utils.py This patch improves the utils.py module by removing unused imports (errno, random), simplifying the fd_read_timeout() function by eliminating unnecessary else clause, and cleaning up code style in the defer class constructor. Additionally, it renames the parameter in rand_port() from 'type' to 'stype' to avoid shadowing the built-in Python name 'type', improving code clarity and preventing potential issues. These changes enhance code readability and maintainability without affecting functionality. Signed-off-by: Breno Leitao Reviewed-by: Simon Horman Link: https://patch.msgid.link/20250901-fix-v1-1-df0abb67481e@debian.org Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'tools/testing/selftests/net/lib/py/utils.py') diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index b188cac49738..1cdc8e6d6b60 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -1,9 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 -import errno import json as _json import os -import random import re import select import socket @@ -21,8 +19,7 @@ def fd_read_timeout(fd, timeout): rlist, _, _ = select.select([fd], [], [], timeout) if rlist: return os.read(fd, 1024) - else: - raise TimeoutError("Timeout waiting for fd read") + raise TimeoutError("Timeout waiting for fd read") class cmd: @@ -138,8 +135,6 @@ global_defer_queue = [] class defer: def __init__(self, func, *args, **kwargs): - global global_defer_queue - if not callable(func): raise Exception("defer created with un-callable object, did you call the function instead of passing its name?") @@ -227,11 +222,11 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None): return cmd_obj -def rand_port(type=socket.SOCK_STREAM): +def rand_port(stype=socket.SOCK_STREAM): """ Get a random unprivileged port. """ - with socket.socket(socket.AF_INET6, type) as s: + with socket.socket(socket.AF_INET6, stype) as s: s.bind(("", 0)) return s.getsockname()[1] -- cgit v1.2.3 From bc1a767f695d597de529f4f0c6fff1d55f1b5395 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Sat, 30 Aug 2025 11:43:17 -0700 Subject: selftests: net: py: don't default to shell=True Overhead of using shell=True is quite significant. Micro-benchmark of running ethtool --help shows that non-shell run is 2x faster. Runtime of the XDP tests also shows improvement: this patch: 2m34s 2m21s 2m18s 2m18s before: 2m54s 2m36s 2m34s Reviewed-by: Breno Leitao Link: https://patch.msgid.link/20250830184317.696121-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'tools/testing/selftests/net/lib/py/utils.py') diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index 1cdc8e6d6b60..cb40ecef9456 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -26,9 +26,12 @@ class cmd: """ Execute a command on local or remote host. + @shell defaults to false, and class will try to split @comm into a list + if it's a string with spaces. + Use bkg() instead to run a command in the background. """ - def __init__(self, comm, shell=True, fail=True, ns=None, background=False, + def __init__(self, comm, shell=None, fail=True, ns=None, background=False, host=None, timeout=5, ksft_wait=None): if ns: comm = f'ip netns exec {ns} ' + comm @@ -42,6 +45,10 @@ class cmd: if host: self.proc = host.cmd(comm) else: + # If user doesn't explicitly request shell try to avoid it. + if shell is None and isinstance(comm, str) and ' ' in comm: + comm = comm.split() + # ksft_wait lets us wait for the background process to fully start, # we pass an FD to the child process, and wait for it to write back. # Similarly term_fd tells child it's time to exit. @@ -108,7 +115,7 @@ class bkg(cmd): with bkg("my_binary", ksft_wait=5): """ - def __init__(self, comm, shell=True, fail=None, ns=None, host=None, + def __init__(self, comm, shell=None, fail=None, ns=None, host=None, exit_wait=False, ksft_wait=None): super().__init__(comm, background=True, shell=shell, fail=fail, ns=ns, host=host, -- cgit v1.2.3