Message ID | 20201230095204.21467-1-po-hsu.lin@canonical.com (mailing list archive) |
---|---|
State | Awaiting Upstream |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | selftests: xfrm: fix test return value override issue in xfrm_policy.sh | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Guessed tree name to be net-next |
netdev/subject_prefix | warning | Target tree name not specified in the subject |
netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/header_inline | success | Link |
netdev/stable | success | Stable not CCed |
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>
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!
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
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(-)