netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net] selftests: netfilter: tone-down conntrack clash test
@ 2025-07-21 22:36 Florian Westphal
  2025-07-23  1:30 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Florian Westphal @ 2025-07-21 22:36 UTC (permalink / raw)
  To: netdev
  Cc: netfilter-devel, Jakub Kicinski, Paolo Abeni, David S. Miller,
	Eric Dumazet, Florian Westphal

The test is supposed to observe that the 'clash_resolve' stat counter
incremented (i.e., the code path was covered).
This check was incorrect, 'conntrack -S' needs to be called in the
revevant namespace, not the initial netns.

The clash resolution logic in conntrack is only exercised when multiple
packets with the same udp quadruple race. Depending on kernel config,
number of CPUs, scheduling policy etc.  this might not trigger even
after several retries.  Thus the script eventually returns SKIP if the
retry count is exceeded.

The udpclash tool with also exit with a failure if it did not observe
the expected number of replies.

In the script, make a note of this but do not fail anymore, just check if
the clash resolution logic triggered after all.

Remove the 'single-core' test: while unlikely, with preemptible kernel it
should be possible to also trigger clash resolution logic.

With this change the test will either SKIP or pass.

Hard error could be restored later once its clear whats going on, so
also dump 'conntrack -S' when some packets went missing to see if
conntrack dropped them on insert.

Fixes: 78a588363587 ("selftests: netfilter: add conntrack clash resolution test case")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Change since v1:
  - leave udpclash tool as-is, relax script error handling instead
  - fix commit message to mention the conntrack -S bug fix
  - get rid of the single-cpu shortcut

 .../net/netfilter/conntrack_clash.sh          | 45 +++++++++----------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/net/netfilter/conntrack_clash.sh b/tools/testing/selftests/net/netfilter/conntrack_clash.sh
index 3712c1b9b38b..606a43a60f73 100755
--- a/tools/testing/selftests/net/netfilter/conntrack_clash.sh
+++ b/tools/testing/selftests/net/netfilter/conntrack_clash.sh
@@ -93,32 +93,28 @@ ping_test()
 run_one_clash_test()
 {
 	local ns="$1"
-	local daddr="$2"
-	local dport="$3"
+	local ctns="$2"
+	local daddr="$3"
+	local dport="$4"
 	local entries
 	local cre
 
 	if ! ip netns exec "$ns" ./udpclash $daddr $dport;then
-		echo "FAIL: did not receive expected number of replies for $daddr:$dport"
-		ret=1
-		return 1
+		echo "INFO: did not receive expected number of replies for $daddr:$dport"
+		ip netns exec "$ctns" conntrack -S
+		# don't fail: check if clash resolution triggered after all.
 	fi
 
-	entries=$(conntrack -S | wc -l)
-	cre=$(conntrack -S | grep -v "clash_resolve=0" | wc -l)
+	entries=$(ip netns exec "$ctns" conntrack -S | wc -l)
+	cre=$(ip netns exec "$ctns" conntrack -S | grep "clash_resolve=0" | wc -l)
 
-	if [ "$cre" -ne "$entries" ] ;then
+	if [ "$cre" -ne "$entries" ];then
 		clash_resolution_active=1
 		return 0
 	fi
 
-	# 1 cpu -> parallel insertion impossible
-	if [ "$entries" -eq 1 ]; then
-		return 0
-	fi
-
-	# not a failure: clash resolution logic did not trigger, but all replies
-	# were received.  With right timing, xmit completed sequentially and
+	# not a failure: clash resolution logic did not trigger.
+	# With right timing, xmit completed sequentially and
 	# no parallel insertion occurs.
 	return $ksft_skip
 }
@@ -126,20 +122,23 @@ run_one_clash_test()
 run_clash_test()
 {
 	local ns="$1"
-	local daddr="$2"
-	local dport="$3"
+	local ctns="$2"
+	local daddr="$3"
+	local dport="$4"
+	local softerr=0
 
 	for i in $(seq 1 10);do
-		run_one_clash_test "$ns" "$daddr" "$dport"
+		run_one_clash_test "$ns" "$ctns" "$daddr" "$dport"
 		local rv=$?
 		if [ $rv -eq 0 ];then
 			echo "PASS: clash resolution test for $daddr:$dport on attempt $i"
 			return 0
-		elif [ $rv -eq 1 ];then
-			echo "FAIL: clash resolution test for $daddr:$dport on attempt $i"
-			return 1
+		elif [ $rv -eq $ksft_skip ]; then
+			softerr=1
 		fi
 	done
+
+	[ $softerr -eq 1 ] && echo "SKIP: clash resolution for $daddr:$dport did not trigger"
 }
 
 ip link add veth0 netns "$nsclient1" type veth peer name veth0 netns "$nsrouter"
@@ -161,11 +160,11 @@ spawn_servers "$nsclient2"
 
 # exercise clash resolution with nat:
 # nsrouter is supposed to dnat to 10.0.2.1:900{0,1,2,3}.
-run_clash_test "$nsclient1" 10.0.1.99 "$dport"
+run_clash_test "$nsclient1" "$nsrouter" 10.0.1.99 "$dport"
 
 # exercise clash resolution without nat.
 load_simple_ruleset "$nsclient2"
-run_clash_test "$nsclient2" 127.0.0.1 9001
+run_clash_test "$nsclient2" "$nsclient2" 127.0.0.1 9001
 
 if [ $clash_resolution_active -eq 0 ];then
 	[ "$ret" -eq 0 ] && ret=$ksft_skip
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2 net] selftests: netfilter: tone-down conntrack clash test
  2025-07-21 22:36 [PATCH v2 net] selftests: netfilter: tone-down conntrack clash test Florian Westphal
@ 2025-07-23  1:30 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-23  1:30 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev, netfilter-devel, kuba, pabeni, davem, edumazet

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 22 Jul 2025 00:36:49 +0200 you wrote:
> The test is supposed to observe that the 'clash_resolve' stat counter
> incremented (i.e., the code path was covered).
> This check was incorrect, 'conntrack -S' needs to be called in the
> revevant namespace, not the initial netns.
> 
> The clash resolution logic in conntrack is only exercised when multiple
> packets with the same udp quadruple race. Depending on kernel config,
> number of CPUs, scheduling policy etc.  this might not trigger even
> after several retries.  Thus the script eventually returns SKIP if the
> retry count is exceeded.
> 
> [...]

Here is the summary with links:
  - [v2,net] selftests: netfilter: tone-down conntrack clash test
    https://git.kernel.org/netdev/net/c/dca56cc8b5c3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-07-23  1:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21 22:36 [PATCH v2 net] selftests: netfilter: tone-down conntrack clash test Florian Westphal
2025-07-23  1:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).