From: David Ahern dsahern@kernel.org
[ Upstream commit 2f186dd5585c3afb415df80e52f71af16c9d3655 ]
Replace the sleep in kill_procs with slowwait.
Signed-off-by: David Ahern dsahern@kernel.org Reviewed-by: Simon Horman horms@kernel.org Link: https://patch.msgid.link/20250910025828.38900-2-dsahern@kernel.org Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What changed - Replaces a fixed delay with a condition-based wait in `kill_procs()` so test cleanup actually completes before proceeding: `tools/testing/selftests/net/fcnal-test.sh:192`. - Old behavior: `sleep 1` after `killall nettest ping ping6`. - New behavior: `slowwait 2 sh -c 'test -z "$(pgrep "^(nettest|ping|ping6)$")"'` to poll until those processes are gone, up to 2 seconds.
- Why it matters - `kill_procs()` is called at test start to ensure a clean slate: `tools/testing/selftests/net/fcnal-test.sh:161-166`. A fixed sleep can be too short on slower or loaded systems, leaving straggler `ping`/`ping6`/`nettest` processes that interfere with subsequent tests, causing flakiness or false failures. The condition-based wait removes that flakiness by verifying process exit.
- How `slowwait` works (and why it’s safe) - `slowwait()` is a common helper in net selftests that polls every 100ms until a command succeeds or a timeout is hit: `tools/testing/selftests/net/lib.sh:105-110`. It uses `loopy_wait "sleep 0.1" ...`, causing no architectural or API changes, and only affects selftest behavior. - This is consistent with broader selftests usage (e.g., `tools/testing/selftests/net/rtnetlink.sh:314`, `tools/testing/selftests/net/forwarding/lib.sh:566`), standardizing on proven patterns already used across the test suite.
- Scope and risk - Selftests-only change; no in-kernel code touched. - Small and contained; no interface changes. - Failure mode is limited: if the processes don’t exit, `slowwait` times out in 2s and `kill_procs()`’s non-zero exit code is not fatal in callers (no `set -e`); the tests proceed, but the added wait significantly lowers flakiness vs. a blind `sleep 1`. - The `pgrep` anchored regex `^(nettest|ping|ping6)$` targets the exact processes, avoiding false positives.
- Stable backport fit - Fixes a real test bug (flaky cleanup) that affects test reliability on stable trees. - Minimal risk, no architectural changes, not a new feature. - Improves determinism of selftests run against stable kernels, aligning with stable policy to accept selftest reliability fixes.
Conclusion: This is a low-risk, selftests-only robustness fix that improves test reliability and should be backported.
tools/testing/selftests/net/fcnal-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh index cf535c23a959a..dfd368371fb3c 100755 --- a/tools/testing/selftests/net/fcnal-test.sh +++ b/tools/testing/selftests/net/fcnal-test.sh @@ -189,7 +189,7 @@ show_hint() kill_procs() { killall nettest ping ping6 >/dev/null 2>&1 - sleep 1 + slowwait 2 sh -c 'test -z "$(pgrep '"'^(nettest|ping|ping6)$'"')"' }
set_ping_group()