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) <[email protected]>
---
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/[email protected]
To: "David S. Miller" <[email protected]>
To: Eric Dumazet <[email protected]>
To: Jakub Kicinski <[email protected]>
To: Paolo Abeni <[email protected]>
To: Simon Horman <[email protected]>
To: Shuah Khan <[email protected]>
To: Toke Høiland-Jørgensen <[email protected]>
To: David Ahern <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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) <[email protected]>
