[PATCH] selftests: xfrm: fix test return value override issue in xfrm_policy.sh

[PATCH] selftests: xfrm: fix test return value override issue in xfrm_policy.sh

[Po-Hsu Lin]

When running this xfrm_policy.sh test script, even with some cases
marked as FAIL, the overall test result will still be PASS:

$ sudo ./xfrm_policy.sh
PASS: policy before exception matches
FAIL: expected ping to .254 to fail (exceptions)
PASS: direct policy matches (exceptions)
PASS: policy matches (exceptions)
FAIL: expected ping to .254 to fail (exceptions and block policies)
PASS: direct policy matches (exceptions and block policies)
PASS: policy matches (exceptions and block policies)
FAIL: expected ping to .254 to fail (exceptions and block policies after hresh changes)
PASS: direct policy matches (exceptions and block policies after hresh changes)
PASS: policy matches (exceptions and block policies after hresh changes)
FAIL: expected ping to .254 to fail (exceptions and block policies after hthresh change in ns3)
PASS: direct policy matches (exceptions and block policies after hthresh change in ns3)
PASS: policy matches (exceptions and block policies after hthresh change in ns3)
FAIL: expected ping to .254 to fail (exceptions and block policies after htresh change to normal)
PASS: direct policy matches (exceptions and block policies after htresh change to normal)
PASS: policy matches (exceptions and block policies after htresh change to normal)
PASS: policies with repeated htresh change
$ echo $?
0

This is because the $lret in check_xfrm() is not a local variable.
Therefore when a test failed in check_exceptions(), the non-zero $lret
will later get reset to 0 when the next test calls check_xfrm().

With this fix, the final return value will be 1. Make it easier for
testers to spot this failure.

Fixes: 39aa6928d462d0 ("xfrm: policy: fix netlink/pf_key policy lookups")
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/xfrm_policy.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/xfrm_policy.sh b/tools/testing/selftests/net/xfrm_policy.sh
index 7a1bf94..5922941 100755
--- a/tools/testing/selftests/net/xfrm_policy.sh
+++ b/tools/testing/selftests/net/xfrm_policy.sh
@@ -202,7 +202,7 @@ check_xfrm() {
 	# 1: iptables -m policy rule count != 0
 	rval=$1
 	ip=$2
-	lret=0
+	local lret=0
 
 	ip netns exec ns1 ping -q -c 1 10.0.2.$ip > /dev/null
 
-- 
2.7.4

Re: [PATCH] selftests: xfrm: fix test return value override issue in xfrm_policy.sh

[Florian Westphal]

Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
> When running this xfrm_policy.sh test script, even with some cases
> marked as FAIL, the overall test result will still be PASS:
> 
> $ sudo ./xfrm_policy.sh
> PASS: policy before exception matches
> FAIL: expected ping to .254 to fail (exceptions)
> PASS: direct policy matches (exceptions)
> PASS: policy matches (exceptions)
> FAIL: expected ping to .254 to fail (exceptions and block policies)
> PASS: direct policy matches (exceptions and block policies)
> PASS: policy matches (exceptions and block policies)
> FAIL: expected ping to .254 to fail (exceptions and block policies after hresh changes)
> PASS: direct policy matches (exceptions and block policies after hresh changes)
> PASS: policy matches (exceptions and block policies after hresh changes)
> FAIL: expected ping to .254 to fail (exceptions and block policies after hthresh change in ns3)
> PASS: direct policy matches (exceptions and block policies after hthresh change in ns3)
> PASS: policy matches (exceptions and block policies after hthresh change in ns3)
> FAIL: expected ping to .254 to fail (exceptions and block policies after htresh change to normal)
> PASS: direct policy matches (exceptions and block policies after htresh change to normal)
> PASS: policy matches (exceptions and block policies after htresh change to normal)
> PASS: policies with repeated htresh change
> $ echo $?
> 0
> 
> This is because the $lret in check_xfrm() is not a local variable.

Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH] selftests: xfrm: fix test return value override issue in xfrm_policy.sh

[Steffen Klassert]

On Wed, Dec 30, 2020 at 05:52:04PM +0800, Po-Hsu Lin wrote:
> When running this xfrm_policy.sh test script, even with some cases
> marked as FAIL, the overall test result will still be PASS:
> 
> $ sudo ./xfrm_policy.sh
> PASS: policy before exception matches
> FAIL: expected ping to .254 to fail (exceptions)
> PASS: direct policy matches (exceptions)
> PASS: policy matches (exceptions)
> FAIL: expected ping to .254 to fail (exceptions and block policies)
> PASS: direct policy matches (exceptions and block policies)
> PASS: policy matches (exceptions and block policies)
> FAIL: expected ping to .254 to fail (exceptions and block policies after hresh changes)
> PASS: direct policy matches (exceptions and block policies after hresh changes)
> PASS: policy matches (exceptions and block policies after hresh changes)
> FAIL: expected ping to .254 to fail (exceptions and block policies after hthresh change in ns3)
> PASS: direct policy matches (exceptions and block policies after hthresh change in ns3)
> PASS: policy matches (exceptions and block policies after hthresh change in ns3)
> FAIL: expected ping to .254 to fail (exceptions and block policies after htresh change to normal)
> PASS: direct policy matches (exceptions and block policies after htresh change to normal)
> PASS: policy matches (exceptions and block policies after htresh change to normal)
> PASS: policies with repeated htresh change
> $ echo $?
> 0
> 
> This is because the $lret in check_xfrm() is not a local variable.
> Therefore when a test failed in check_exceptions(), the non-zero $lret
> will later get reset to 0 when the next test calls check_xfrm().
> 
> With this fix, the final return value will be 1. Make it easier for
> testers to spot this failure.
> 
> Fixes: 39aa6928d462d0 ("xfrm: policy: fix netlink/pf_key policy lookups")
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

Applied to the ipsec tree, thanks!