[PATCH net-next] selftests: net: kmemleak for lwt dst cache tests

[PATCH net-next] selftests: net: kmemleak for lwt dst cache tests

[Justin Iurman]

Force the use of kmemleak to check everything's OK and report results
for each test case. Also, useless sleeps were removed, and the bash
script was renamed to something that makes more sense. Due to kmemleak,
some tests may be false negatives. To mitigate that (i.e., to have more
stable results), the solution of a kmemleak scan at the end (vs. for
each test) was preferred.

Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/config            |   2 +
 ..._ref_loop.sh => kmemleak_lwt_dst_cache.sh} | 146 +++++++++++++-----
 3 files changed, 107 insertions(+), 43 deletions(-)
 rename tools/testing/selftests/net/{lwt_dst_cache_ref_loop.sh => kmemleak_lwt_dst_cache.sh} (66%)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 6d718b478ed8..eba9dbb5387d 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -105,7 +105,7 @@ TEST_PROGS += bpf_offload.py
 TEST_PROGS += ipv6_route_update_soft_lockup.sh
 TEST_PROGS += busy_poll_test.sh
 TEST_GEN_PROGS += proc_net_pktgen
-TEST_PROGS += lwt_dst_cache_ref_loop.sh
+TEST_PROGS += kmemleak_lwt_dst_cache.sh
 
 # YNL files, must be before "include ..lib.mk"
 YNL_GEN_FILES := busy_poller netlink-dumps
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 3cfef5153823..8c162da21ac8 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -116,3 +116,5 @@ CONFIG_NETKIT=y
 CONFIG_NET_PKTGEN=m
 CONFIG_IPV6_ILA=m
 CONFIG_IPV6_RPL_LWTUNNEL=y
+CONFIG_DEBUG_FS=y
+CONFIG_DEBUG_KMEMLEAK=y
diff --git a/tools/testing/selftests/net/lwt_dst_cache_ref_loop.sh b/tools/testing/selftests/net/kmemleak_lwt_dst_cache.sh
similarity index 66%
rename from tools/testing/selftests/net/lwt_dst_cache_ref_loop.sh
rename to tools/testing/selftests/net/kmemleak_lwt_dst_cache.sh
index 881eb399798f..30333f2c83c8 100755
--- a/tools/testing/selftests/net/lwt_dst_cache_ref_loop.sh
+++ b/tools/testing/selftests/net/kmemleak_lwt_dst_cache.sh
@@ -5,14 +5,12 @@
 #
 # WARNING
 # -------
-# This is just a dummy script that triggers encap cases with possible dst cache
-# reference loops in affected lwt users (see list below). Some cases are
-# pathological configurations for simplicity, others are valid. Overall, we
-# don't want this issue to happen, no matter what. In order to catch any
-# reference loops, kmemleak MUST be used. The results alone are always blindly
-# successful, don't rely on them. Note that the following tests may crash the
-# kernel if the fix to prevent lwtunnel_{input|output|xmit}() reentry loops is
-# not present.
+# This script triggers lwt encap use cases, and checks for any dst cache
+# reference loops in affected lwt users (see list below) thanks to kmemleak.
+# Some configurations are pathological and some others are valid. Overall, we
+# don't want this issue to happen, no matter what, so that's why this selftest
+# exists. Note that this script will probably crash the kernel if commit
+# 986ffb3a57c5 ("net: lwtunnel: fix recursion loops") is not included.
 #
 # Affected lwt users so far (please update accordingly if needed):
 #  - ila_lwt (output only)
@@ -21,6 +19,7 @@
 #  - seg6_iptunnel (both input and output)
 
 source lib.sh
+KMEMLEAK_PATH="/sys/kernel/debug/kmemleak"
 
 check_compatibility()
 {
@@ -106,8 +105,8 @@ setup()
 	ip -netns $beta link set veth0 up &>/dev/null
 	ip -netns $beta link set veth1 up &>/dev/null
 	ip -netns $beta link set lo up &>/dev/null
-	ip -netns $beta route del 2001:db8:2::/64
-	ip -netns $beta route add 2001:db8:2::/64 dev veth1
+	ip -netns $beta route del 2001:db8:2::/64 &>/dev/null
+	ip -netns $beta route add 2001:db8:2::/64 dev veth1 &>/dev/null
 	ip netns exec $beta \
 		sysctl -wq net.ipv6.conf.all.forwarding=1 &>/dev/null
 
@@ -117,115 +116,169 @@ setup()
 	ip -netns $gamma route add 2001:db8:1::/64 \
 		via 2001:db8:2::1 dev veth0 &>/dev/null
 
-	sleep 1
-
 	ip netns exec $alpha ping6 -c 5 -W 1 2001:db8:2::2 &>/dev/null
 	if [ $? != 0 ]; then
 		echo "SKIP: Setup failed."
 		exit $ksft_skip
 	fi
-
-	sleep 1
 }
 
 cleanup()
 {
 	cleanup_ns $alpha $beta $gamma
 	[ $ila_lsmod != 0 ] && modprobe -r ila &>/dev/null
+	kmemleak_clear
+}
+
+name2descr()
+{
+	if [ "$1" == "ila" ] || [ "$1" == "ioam6" ]; then
+		echo "output"
+	elif [ "$1" == "rpl" ] || [ "$1" == "seg6" ]; then
+		echo "input + output"
+	else
+		echo ""
+	fi
+}
+
+log_test_passed()
+{
+	printf "TEST: %-57s  [ OK ]\n" "$1"
+	npassed=$((npassed+1))
+}
+
+log_test_skipped()
+{
+	printf "TEST: %-57s  [SKIP]\n" "$1"
+	nskipped=$((nskipped+1))
+}
+
+log_test_failed()
+{
+	printf "TEST: %-57s  [FAIL]\n" "$1"
+	nfailed=$((nfailed+1))
+}
+
+check_result()
+{
+	if grep -q "$1" <<< "$2"; then
+		log_test_failed "$1 ($(name2descr $1))"
+	else
+		log_test_passed "$1 ($(name2descr $1))"
+	fi
+}
+
+kmemleak_clear()
+{
+	echo clear > "$KMEMLEAK_PATH"
+}
+
+kmemleak_scan()
+{
+	for i in {1..5}; do
+		echo scan > "$KMEMLEAK_PATH"
+	done
+}
+
+kmemleak_result()
+{
+	local output=$(cat "$KMEMLEAK_PATH")
+
+	[ $skip_ila != 0 ] && log_test_skipped "ila ($(name2descr ila))" \
+			   || check_result "ila" "$output"
+
+	[ $skip_ioam6 != 0 ] && log_test_skipped "ioam6 ($(name2descr ioam6))" \
+			   || check_result "ioam6" "$output"
+
+	[ $skip_rpl != 0 ] && log_test_skipped "rpl ($(name2descr rpl))" \
+			   || check_result "rpl" "$output"
+
+	[ $skip_seg6 != 0 ] && log_test_skipped "seg6 ($(name2descr seg6))" \
+			   || check_result "seg6" "$output"
 }
 
 run_ila()
 {
 	if [ $skip_ila != 0 ]; then
-		echo "SKIP: ila (output)"
 		return
 	fi
 
-	ip -netns $beta route del 2001:db8:2::/64
+	ip -netns $beta route del 2001:db8:2::/64 &>/dev/null
 	ip -netns $beta route add 2001:db8:2:0:0:0:0:2/128 \
 		encap ila 2001:db8:2:0 csum-mode no-action ident-type luid \
 			hook-type output \
 		dev veth1 &>/dev/null
-	sleep 1
 
-	echo "TEST: ila (output)"
 	ip netns exec $beta ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
 
-	ip -netns $beta route del 2001:db8:2:0:0:0:0:2/128
-	ip -netns $beta route add 2001:db8:2::/64 dev veth1
-	sleep 1
+	ip -netns $beta route del 2001:db8:2:0:0:0:0:2/128 &>/dev/null
+	ip -netns $beta route add 2001:db8:2::/64 dev veth1 &>/dev/null
 }
 
 run_ioam6()
 {
 	if [ $skip_ioam6 != 0 ]; then
-		echo "SKIP: ioam6 (output)"
 		return
 	fi
 
 	ip -netns $beta route change 2001:db8:2::/64 \
 		encap ioam6 trace prealloc type 0x800000 ns 1 size 4 \
 		dev veth1 &>/dev/null
-	sleep 1
 
-	echo "TEST: ioam6 (output)"
 	ip netns exec $beta ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
+
+	ip -netns $beta route change 2001:db8:2::/64 dev veth1 &>/dev/null
 }
 
 run_rpl()
 {
 	if [ $skip_rpl != 0 ]; then
-		echo "SKIP: rpl (input)"
-		echo "SKIP: rpl (output)"
 		return
 	fi
 
 	ip -netns $beta route change 2001:db8:2::/64 \
 		encap rpl segs 2001:db8:2::2 \
 		dev veth1 &>/dev/null
-	sleep 1
 
-	echo "TEST: rpl (input)"
 	ip netns exec $alpha ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
-
-	echo "TEST: rpl (output)"
 	ip netns exec $beta ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
+
+	ip -netns $beta route change 2001:db8:2::/64 dev veth1 &>/dev/null
 }
 
 run_seg6()
 {
 	if [ $skip_seg6 != 0 ]; then
-		echo "SKIP: seg6 (input)"
-		echo "SKIP: seg6 (output)"
 		return
 	fi
 
 	ip -netns $beta route change 2001:db8:2::/64 \
 		encap seg6 mode inline segs 2001:db8:2::2 \
 		dev veth1 &>/dev/null
-	sleep 1
 
-	echo "TEST: seg6 (input)"
 	ip netns exec $alpha ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
-
-	echo "TEST: seg6 (output)"
 	ip netns exec $beta ping6 -c 2 -W 1 2001:db8:2::2 &>/dev/null
-	sleep 1
+
+	ip -netns $beta route change 2001:db8:2::/64 dev veth1 &>/dev/null
 }
 
 run()
 {
+	kmemleak_clear
+
 	run_ila
 	run_ioam6
 	run_rpl
 	run_seg6
+
+	kmemleak_scan
+	kmemleak_result
 }
 
+npassed=0
+nskipped=0
+nfailed=0
+
 if [ "$(id -u)" -ne 0 ]; then
 	echo "SKIP: Need root privileges."
 	exit $ksft_skip
@@ -236,6 +289,11 @@ if [ ! -x "$(command -v ip)" ]; then
 	exit $ksft_skip
 fi
 
+if [ ! -e $KMEMLEAK_PATH ]; then
+	echo "SKIP: Kmemleak not available."
+	exit $ksft_skip
+fi
+
 check_compatibility
 
 trap cleanup EXIT
@@ -243,4 +301,8 @@ trap cleanup EXIT
 setup
 run
 
+if [ $nfailed != 0 ]; then
+	exit $ksft_fail
+fi
+
 exit $ksft_pass
-- 
2.34.1

Re: [PATCH net-next] selftests: net: kmemleak for lwt dst cache tests

[Jakub Kicinski]

On Thu, 17 Apr 2025 15:08:30 +0200 Justin Iurman wrote:
> Force the use of kmemleak to check everything's OK and report results
> for each test case. Also, useless sleeps were removed, and the bash
> script was renamed to something that makes more sense. Due to kmemleak,
> some tests may be false negatives. To mitigate that (i.e., to have more
> stable results), the solution of a kmemleak scan at the end (vs. for
> each test) was preferred.

I'm open to other opinions but to me this is not a scalable direction.
Checking for splats and memleaks does not belong in any particular
test script, it belongs in whatever executes the test. We could add 
some standard way to get ksft to scan for splats and leaks, maybe in
tools/testing/selftests/kselftest/runner.sh ?

> +++ b/tools/testing/selftests/net/config
> +CONFIG_DEBUG_KMEMLEAK=y

Enabling kmemleak for all netdev runners is too heavy, if others
feel like we should have the test case do the scanning - let's 
detect if kmemleak is available. The expectation is that the user
added kernel/configs/debug.config to their build if they want
debug checks.
-- 
pw-bot: cr