icmp.sh backgrounds ping and then starts tcpdump, with no synchronization between the two. If tcpdump is not capturing yet by the time the ICMP unreachable replies come back, the test fails even though the kernel behaved correctly: FAIL - got ICMP response from , should be 192.0.0.8 The empty address is misleading: it means the capture matched nothing before its 10s timeout, not that the reply had a wrong source address. NS1's Icmp InDestUnreachs counter still increments across such a failure, so the replies were generated and did reach NS1. Ping sends several probes over its 3s deadline, so losing every one of them takes a multi-second stall in tcpdump startup, which does happen on loaded CI hosts [1]. Fix it by starting tcpdump first and waiting for its "listening" banner via slowwait() before sending traffic. Run ping synchronously, then kill and reap tcpdump if it did not already exit on -c 1. Fixes: 7e9838b7915e ("selftests/net: Add icmp.sh for testing ICMP dummy address responses") Link: https://openqa.opensuse.org/tests/5907626/logfile?filename=icmp_sh.tap.txt#line-2 [1] Signed-off-by: Ricardo B. Marlière (SUSE) --- Changes in v2: - Drop timeout(1) wrapper from tcpdump invocation. - Run ping synchronously in the foreground. - Kill and reap tcpdump PID after ping returns if it did not exit on -c 1. - Simplify commit message description of the fix. - Link to v1: https://patch.msgid.link/20260901-selftests-net-icmp_race-v1-1-0b2a27e91a0c@marliere.net To: "David S. Miller" To: Eric Dumazet To: Jakub Kicinski To: Paolo Abeni To: Simon Horman To: Shuah Khan To: Toke Høiland-Jørgensen To: David Ahern Cc: netdev@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- tools/testing/selftests/net/icmp.sh | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/net/icmp.sh b/tools/testing/selftests/net/icmp.sh index 824cb0e35eff..1670caef2b90 100755 --- a/tools/testing/selftests/net/icmp.sh +++ b/tools/testing/selftests/net/icmp.sh @@ -28,10 +28,11 @@ RT2=172.16.0.0/24 H2_IP6=2001:db8:1::2 TMPFILE=$(mktemp) +TCPDUMP_ERR=$(mktemp) cleanup() { - rm -f "$TMPFILE" + rm -f "$TMPFILE" "$TCPDUMP_ERR" cleanup_ns $NS1 $NS2 } @@ -53,11 +54,22 @@ ip -netns $NS2 route add $RT2 via inet6 $H1_IP6 # Make sure ns2 will respond with ICMP unreachable ip netns exec $NS2 sysctl -qw net.ipv4.icmp_ratelimit=0 net.ipv4.ip_forward=1 -# Run the test - a ping runs in the background, and we capture ICMP responses -# with tcpdump; -c 1 means it should exit on the first ping, but add a timeout -# in case something goes wrong -ip netns exec $NS1 ping -w 3 -i 0.5 $PINGADDR >/dev/null & -ip netns exec $NS1 timeout 10 tcpdump -tpni veth0 -c 1 'icmp and icmp[icmptype] != icmp-echo' > $TMPFILE 2>/dev/null +# Run the test - start tcpdump and wait for it to be capturing before +# sending any traffic. -c 1 means tcpdump exits on its own once it captures +# a reply; kill it afterwards in case it is still waiting. +ip netns exec $NS1 tcpdump -tpni veth0 -c 1 \ + 'icmp and icmp[icmptype] != icmp-echo' > $TMPFILE 2>$TCPDUMP_ERR & +TCPDUMP_PID=$! +if ! slowwait 3 grep -qs "listening" "$TCPDUMP_ERR"; then + echo "FAIL - tcpdump did not start listening" + cat "$TCPDUMP_ERR" + exit 1 +fi + +ip netns exec $NS1 ping -w 3 -i 0.5 $PINGADDR >/dev/null + +kill $TCPDUMP_PID 2>/dev/null +wait $TCPDUMP_PID 2>/dev/null # Parse response and check for dummy address # tcpdump output looks like: --- base-commit: 08df884136f1c1197bab2a27814404fd329d9aac change-id: 20260901-selftests-net-icmp_race-6268d886249e Best regards, -- Ricardo B. Marlière (SUSE)