[PATCH nf-next] selftests: netfilter: fix return value in zone subtest

[PATCH nf-next] selftests: netfilter: fix return value in zone subtest

[Florian Westphal]

zone subtest prints FAIL, but did not set 'ret=1', so kselftest
framework reports success.

While at it, also fail when "nft -f" returns an error.

Noticed thanks to a (broken) local patch that should have caused this
selftest (resp. the kselftest framework) to report a failure.

Fixes: 33b8aad21ac1 ("selftests: netfilter: add a vrf+conntrack testcase")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Can also go via nf, I don't mind either way.

 .../selftests/net/netfilter/conntrack_vrf.sh  | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/netfilter/conntrack_vrf.sh b/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
index 207b79932d91..2dbfabaadf4b 100755
--- a/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
+++ b/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
@@ -84,7 +84,7 @@ busywait $BUSYWAIT_TIMEOUT listener_ready "$ns1"
 # as decided by the first iteration of the ruleset.
 test_ct_zone_in()
 {
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 table testct {
 	chain rawpre {
 		type filter hook prerouting priority raw;
@@ -105,6 +105,12 @@ table testct {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load testct nft ruleset"
+		ret=1
+		return
+	fi
+
 	ip netns exec "$ns1" ping -W 1 -c 1 -I veth0 "$IP0" > /dev/null
 
 	# should be in zone 1, not zone 2
@@ -113,6 +119,7 @@ EOF
 		echo "PASS: entry found in conntrack zone 1"
 	else
 		echo "FAIL: entry not found in conntrack zone 1"
+		ret=1
 		count=$(ip netns exec "$ns0" conntrack -L -s $IP1 -d $IP0 -p icmp --zone 2 2> /dev/null | wc -l)
 		if [ "$count" -eq 1 ]; then
 			echo "FAIL: entry found in zone 2 instead"
@@ -137,7 +144,7 @@ test_masquerade_vrf()
 
 	ip netns exec "$ns0" conntrack -F 2>/dev/null
 
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 flush ruleset
 table ip nat {
 	chain rawout {
@@ -158,6 +165,12 @@ table ip nat {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load ip nat nft ruleset"
+		ret=1
+		return
+	fi
+
 	if ! ip netns exec "$ns0" ip vrf exec tvrf socat -u -4 STDIN TCP:"$IP1":55555 < /dev/null > /dev/null;then
 		echo "FAIL: connect failure with masquerade + sport rewrite on vrf device"
 		ret=1
@@ -184,7 +197,7 @@ EOF
 test_masquerade_veth()
 {
 	ip netns exec "$ns0" conntrack -F 2>/dev/null
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 flush ruleset
 table ip nat {
 	chain postrouting {
@@ -193,6 +206,12 @@ table ip nat {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load ip nat postrouting ruleset"
+		ret=1
+		return
+	fi
+
 	if ! ip netns exec "$ns0" ip vrf exec tvrf socat -u -4 STDIN TCP:"$IP1":55555 < /dev/null > /dev/null;then
 		echo "FAIL: connect failure with masquerade + sport rewrite on veth device"
 		ret=1
-- 
2.55.0

Re: [PATCH nf-next] selftests: netfilter: fix return value in zone subtest

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> zone subtest prints FAIL, but did not set 'ret=1', so kselftest
> framework reports success.
> 
> While at it, also fail when "nft -f" returns an error.
> 
> Noticed thanks to a (broken) local patch that should have caused this
> selftest (resp. the kselftest framework) to report a failure.

https://sashiko.dev/#/patchset/20261006230459.2139-1-fw%40strlen.de

| Does this early return in test_masquerade_vrf() skip the qdisc cleanup?

Yes, but a great many other places don't clean up either.

Tests are supposed to pass, I don't think there is any point in
making scripts more complicated by cleaning up in FAIL cases too.

I don't plan to address this comment. LMK if you disagree.

Re: [PATCH nf-next] selftests: netfilter: fix return value in zone subtest

[Fernando Fernandez Mancera]

On 10/7/26 1:04 AM, Florian Westphal wrote:
> zone subtest prints FAIL, but did not set 'ret=1', so kselftest
> framework reports success.
> 
> While at it, also fail when "nft -f" returns an error.
> 
> Noticed thanks to a (broken) local patch that should have caused this
> selftest (resp. the kselftest framework) to report a failure.
> 
> Fixes: 33b8aad21ac1 ("selftests: netfilter: add a vrf+conntrack testcase")
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Regarding the cleanup commit, we can leave that for the moment we have 
free cycles or for a newbie to take it. I think it would be nice to have 
but not relevant enough for a re-spin.

Thanks Florian!