[PATCH net] selftests: drv-net: pp_alloc_fail: restore ring size after timeout

[PATCH net] selftests: drv-net: pp_alloc_fail: restore ring size after timeout

[Nimrod Oren]

The test restores the original RX ring size only if the command to
change it returns successfully. The cmd() helper raises TimeoutExpired
after 20 seconds but does not terminate the ethtool subprocess or wait
for it to finish. The subprocess may therefore continue running and
leave the new ring size applied despite the timeout, while the test
skips restoration.

This was observed in a NIPA run. The first attempt reported:

    subprocess.TimeoutExpired: Command '['ethtool', '-G', 'ens25f1np1',
    'rx', '2048']' timed out after 20 seconds

https://netdev.bots.linux.dev/logview.html?f=/logs/hwksft/CX7-dbg/results/763842/test-outputs/36-pp-alloc-fail-py/stdout

The subsequent retry attempt then reported:

    subprocess.TimeoutExpired: Command '['ethtool', '-G', 'ens25f1np1',
    'rx', '4096']' timed out after 20 seconds

https://netdev.bots.linux.dev/logview.html?f=/logs/hwksft/CX7-dbg/results/763842/test-outputs/36-pp-alloc-fail-py-retry/stdout

The retry attempted to increase the ring from 2048 to 4096, showing that
the first command had applied the change despite the timeout.

Register the restoration with defer() before running the command so it
is attempted if cmd() times out. If the restoration also times out, its
ethtool subprocess is likewise left running and may eventually restore
the original ring size, avoiding a persistent configuration leak.

Fixes: 9da271f825e4 ("selftests: drv-net-hw: add test for memory allocation failures with page pool")
Reviewed-by: Yael Chemla <ychemla@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Nimrod Oren <noren@nvidia.com>
---
Would it be preferable for cmd() to terminate and wait for
timed-out subprocesses?
---
 .../testing/selftests/drivers/net/hw/pp_alloc_fail.py  | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py
index 2a51b60df8a1..9e7e5cb6a7e8 100755
--- a/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py
+++ b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py
@@ -13,7 +13,7 @@ from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import KsftSkipEx, KsftFailEx
 from lib.py import NetdevFamily, NlError
 from lib.py import NetDrvEpEnv
-from lib.py import cmd, tool, GenerateTraffic
+from lib.py import cmd, defer, ethtool, tool, GenerateTraffic
 
 
 def _write_fail_config(config):
@@ -77,7 +77,6 @@ def test_pp_alloc(cfg, netdevnl):
     if 'rx-alloc-fail' not in stats:
         raise KsftSkipEx("Driver does not report 'rx-alloc-fail' via qstats")
 
-    set_g = False
     traffic = None
     try:
         traffic = GenerateTraffic(cfg)
@@ -112,10 +111,11 @@ def test_pp_alloc(cfg, netdevnl):
             new_g = None
 
         if new_g:
-            set_g = cmd(f"ethtool -G {cfg.ifname} rx {new_g}", fail=False).ret == 0
-            if set_g:
+            restore_g = defer(ethtool, f"-G {cfg.ifname} rx {g['rx']}")
+            if cmd(f"ethtool -G {cfg.ifname} rx {new_g}", fail=False).ret == 0:
                 ksft_pr("ethtool -G change retval: success")
             else:
+                restore_g.cancel()
                 ksft_pr("ethtool -G change retval: did not succeed", new_g)
         else:
             ksft_pr("ethtool -G change retval: did not try")
@@ -127,8 +127,6 @@ def test_pp_alloc(cfg, netdevnl):
         if traffic:
             traffic.stop()
         time.sleep(0.1)
-        if set_g:
-            cmd(f"ethtool -G {cfg.ifname} rx {g['rx']}")
 
 
 def main() -> None:
-- 
2.45.0