[PATCH net v2 0/2] selftests: net: fix cmd.process() timeout handling

[PATCH net v2 0/2] selftests: net: fix cmd.process() timeout handling

[Gal Pressman]

Pass the timeout argument correctly in cmd.process().
As Jakub noted, fixing the timeout broke the bpftrace() command in
netpoll_basic.py, so fix it first.

Changelog:
v1->v2: https://lore.kernel.org/all/20260215125149.2106612-1-gal@nvidia.com/
* Add patch #1.

Gal Pressman (2):
  selftests: net: pass bpftrace timeout to cmd()
  selftests: net: fix timeout passed as positional argument to
    communicate()

 tools/testing/selftests/net/lib/py/utils.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.52.0

[PATCH net v2 1/2] selftests: net: pass bpftrace timeout to cmd()

[Gal Pressman]

The bpftrace() helper configures an interval based exit timer but does
not propagate the timeout to the cmd object, which defaults to 5
seconds. Since the default BPFTRACE_TIMEOUT is 10 seconds, cmd.process()
always raises a TimeoutExpired exception before bpftrace has a chance to
exit gracefully.

Pass timeout+5 to cmd() to allow bpftrace to complete gracefully.

Note: this issue is masked by a bug in the way cmd() passes timeout,
this is fixed in the next commit.

Fixes: 3c561c547c39 ("selftests: drv-net: add helper/wrapper for bpftrace")
Reviewed-by: Nimrod Oren <noren@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
---
 tools/testing/selftests/net/lib/py/utils.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 85884f3e827b..ce4fa4aabebe 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -240,8 +240,9 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
         cmd_arr += ['-f', 'json', '-q']
     if timeout:
         expr += ' interval:s:' + str(timeout) + ' { exit(); }'
+        timeout += 5
     cmd_arr += ['-e', expr]
-    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
+    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False, timeout=timeout)
     if json:
         # bpftrace prints objects as lines
         ret = {}
-- 
2.52.0

Re: [PATCH net v2 1/2] selftests: net: pass bpftrace timeout to cmd()

[Jakub Kicinski]

On Sun, 8 Mar 2026 21:21:25 +0200 Gal Pressman wrote:
> @@ -240,8 +240,9 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
>          cmd_arr += ['-f', 'json', '-q']
>      if timeout:
>          expr += ' interval:s:' + str(timeout) + ' { exit(); }'
> +        timeout += 5

it may be easy to miss that input arg is modified so..

>      cmd_arr += ['-e', expr]
> -    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
> +    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False, timeout=timeout)

timeout=(timeout + 5)

here instead could potentially be cleaner, but up to you.

Re: [PATCH net v2 1/2] selftests: net: pass bpftrace timeout to cmd()

[Gal Pressman]

On 10/03/2026 4:41, Jakub Kicinski wrote:
> On Sun, 8 Mar 2026 21:21:25 +0200 Gal Pressman wrote:
>> @@ -240,8 +240,9 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
>>          cmd_arr += ['-f', 'json', '-q']
>>      if timeout:
>>          expr += ' interval:s:' + str(timeout) + ' { exit(); }'
>> +        timeout += 5
> 
> it may be easy to miss that input arg is modified so..
> 
>>      cmd_arr += ['-e', expr]
>> -    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
>> +    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False, timeout=timeout)
> 
> timeout=(timeout + 5)
> 
> here instead could potentially be cleaner, but up to you.

timeout is None by default, so I figured my version looks better than:
timeout=(timeout + 5) if timeout else None

[PATCH net v2 2/2] selftests: net: fix timeout passed as positional argument to communicate()

[Gal Pressman]

The cited commit refactored the hardcoded timeout=5 into a parameter,
but dropped the keyword from the communicate() call.
Since Popen.communicate()'s first positional argument is 'input' (not
'timeout'), the timeout value is silently treated as stdin input and the
call never enforces a timeout.

Pass timeout as a keyword argument to restore the intended behavior.

Fixes: 1cf270424218 ("net: selftest: add test for netdev netlink queue-get API")
Reviewed-by: Nimrod Oren <noren@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
---
 tools/testing/selftests/net/lib/py/utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index ce4fa4aabebe..4c98ddafdeab 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -88,7 +88,7 @@ class cmd:
             os.write(self.ksft_term_fd, b"1")
         if terminate:
             self.proc.terminate()
-        stdout, stderr = self.proc.communicate(timeout)
+        stdout, stderr = self.proc.communicate(timeout=timeout)
         self.stdout = stdout.decode("utf-8")
         self.stderr = stderr.decode("utf-8")
         self.proc.stdout.close()
-- 
2.52.0

Re: [PATCH net v2 0/2] selftests: net: fix cmd.process() timeout handling

[Jakub Kicinski]

On Sun, 8 Mar 2026 21:21:24 +0200 Gal Pressman wrote:
> Pass the timeout argument correctly in cmd.process().
> As Jakub noted, fixing the timeout broke the bpftrace() command in
> netpoll_basic.py, so fix it first.

patchwork says this didn't get selftest-ed which usually means there's
going to be a conflict with net-next. Can you respin this against
net-next? 

Re: [PATCH net v2 0/2] selftests: net: fix cmd.process() timeout handling

[Gal Pressman]

On 10/03/2026 4:40, Jakub Kicinski wrote:
> On Sun, 8 Mar 2026 21:21:24 +0200 Gal Pressman wrote:
>> Pass the timeout argument correctly in cmd.process().
>> As Jakub noted, fixing the timeout broke the bpftrace() command in
>> netpoll_basic.py, so fix it first.
> 
> patchwork says this didn't get selftest-ed which usually means there's
> going to be a conflict with net-next. Can you respin this against
> net-next? 

I'm not seeing any conflicts, but sure, will resubmit to net-next.