Clean up tests which expect shell=True without explicitly passing that param to cmd(). There seems to be only one such case, and in fact it's better converted to a direct write.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- tools/testing/selftests/drivers/net/napi_threaded.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/napi_threaded.py b/tools/testing/selftests/drivers/net/napi_threaded.py index ed66efa481b0..f4be72b2145a 100755 --- a/tools/testing/selftests/drivers/net/napi_threaded.py +++ b/tools/testing/selftests/drivers/net/napi_threaded.py @@ -24,7 +24,8 @@ from lib.py import cmd, defer, ethtool
def _set_threaded_state(cfg, threaded) -> None: - cmd(f"echo {threaded} > /sys/class/net/{cfg.ifname}/threaded") + with open(f"/sys/class/net/{cfg.ifname}/threaded", "wb") as fp: + fp.write(str(threaded).encode('utf-8'))
def _setup_deferred_cleanup(cfg) -> None:
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
Signed-off-by: Jakub Kicinski kuba@kernel.org --- tools/testing/selftests/net/lib/py/utils.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index b188cac49738..e7155b6db9a3 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -29,9 +29,12 @@ import time """ 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 @@ -45,6 +48,10 @@ import time 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. @@ -111,7 +118,7 @@ import time
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,
linux-kselftest-mirror@lists.linaro.org
-
Jakub Kicinski