From c964368f8263574af25d0631bb86145d8a5e189a Mon Sep 17 00:00:00 2001 From: David Ahern Date: Fri, 24 May 2019 16:37:07 -0700 Subject: [PATCH] selftest: Fixes for icmp_redirect test I was really surprised that the IPv6 mtu exception followed by redirect test was passing as nothing about the code suggests it should. The problem is actually with the logic in the test script. Fix the test cases as follows: 1. add debug function to dump the initial and redirect gateway addresses for ipv6. This is shown only in verbose mode. It helps verify the output of 'route get'. 2. fix the check_exception logic for the reset case to make sure that for IPv4 neither mtu nor redirect appears in the 'route get' output. For IPv6, make sure mtu is not present and the gateway is the initial R1 lladdr. 3. fix the reset logic by using a function to delete the routes added by initial_route_*. This format works better for the nexthop version of the tests. While improving the test cases, go ahead and ensure that forwarding is disabled since IPv6 redirect requires it. Also, runs with kernel debugging enabled sometimes show a failure with one of the ipv4 tests, so spread the pings over longer time interval. The end result is that 2 tests now show failures: TEST: IPv6: mtu exception plus redirect [FAIL] and the VRF version. This is a bug in the IPv6 logic that will need to be fixed separately. Redirect followed by MTU works because __ip6_rt_update_pmtu hits the 'if (!rt6_cache_allowed_for_pmtu(rt6))' path and updates the mtu on the exception rt6_info. MTU followed by redirect does not have this logic. rt6_do_redirect creates a new exception and then rt6_insert_exception removes the old one which has the MTU exception. Fixes: 6985dbf5fdb9 ("selftests: Add redirect tests") Signed-off-by: David Ahern Signed-off-by: David S. Miller --- tools/testing/selftests/net/icmp_redirect.sh | 46 ++++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/net/icmp_redirect.sh b/tools/testing/selftests/net/icmp_redirect.sh index e8d8496f5b16c..76a7c4472dc33 100755 --- a/tools/testing/selftests/net/icmp_redirect.sh +++ b/tools/testing/selftests/net/icmp_redirect.sh @@ -80,6 +80,13 @@ log_test() fi } +log_debug() +{ + if [ "$VERBOSE" = "1" ]; then + echo "$*" + fi +} + run_cmd() { local cmd="$*" @@ -167,6 +174,7 @@ setup() case "${ns}" in h[12]) ip netns exec $ns sysctl -q -w net.ipv4.conf.all.accept_redirects=1 + ip netns exec $ns sysctl -q -w net.ipv6.conf.all.forwarding=0 ip netns exec $ns sysctl -q -w net.ipv6.conf.all.accept_redirects=1 ip netns exec $ns sysctl -q -w net.ipv6.conf.all.keep_addr_on_down=1 ;; @@ -250,12 +258,14 @@ setup() echo "Error: Failed to get link-local address of r1's eth0" exit 1 fi + log_debug "initial gateway is R1's lladdr = ${R1_LLADDR}" R2_LLADDR=$(get_linklocal r2 eth0) if [ $? -ne 0 ]; then echo "Error: Failed to get link-local address of r2's eth0" exit 1 fi + log_debug "initial gateway is R2's lladdr = ${R2_LLADDR}" } change_h2_mtu() @@ -289,15 +299,26 @@ check_exception() ip -netns h1 ro get ${H1_VRF_ARG} ${H2_N2_IP} | \ grep -q "cache expires [0-9]*sec${mtu}" else + # want to verify that neither mtu nor redirected appears in + # the route get output. The -v will wipe out the cache line + # if either are set so the last grep -q will not find a match ip -netns h1 ro get ${H1_VRF_ARG} ${H2_N2_IP} | \ - grep -q "cache" + grep -E -v 'mtu|redirected' | grep -q "cache" fi log_test $? 0 "IPv4: ${desc}" if [ "$with_redirect" = "yes" ]; then - ip -netns h1 -6 ro get ${H1_VRF_ARG} ${H2_N2_IP6} | grep -q "${H2_N2_IP6} from :: via ${R2_LLADDR} dev br0.*${mtu}" + ip -netns h1 -6 ro get ${H1_VRF_ARG} ${H2_N2_IP6} | \ + grep -q "${H2_N2_IP6} from :: via ${R2_LLADDR} dev br0.*${mtu}" + elif [ -n "${mtu}" ]; then + ip -netns h1 -6 ro get ${H1_VRF_ARG} ${H2_N2_IP6} | \ + grep -q "${mtu}" else - ip -netns h1 -6 ro get ${H1_VRF_ARG} ${H2_N2_IP6} | grep -q "${mtu}" + # IPv6 is a bit harder. First strip out the match if it + # contains an mtu exception and then look for the first + # gateway - R1's lladdr + ip -netns h1 -6 ro get ${H1_VRF_ARG} ${H2_N2_IP6} | \ + grep -v "mtu" | grep -q "${R1_LLADDR}" fi log_test $? 0 "IPv6: ${desc}" } @@ -306,8 +327,8 @@ run_ping() { local sz=$1 - run_cmd ip netns exec h1 ping -q -M want -i 0.2 -c 10 -w 2 -s ${sz} ${H1_PING_ARG} ${H2_N2_IP} - run_cmd ip netns exec h1 ${ping6} -q -M want -i 0.2 -c 10 -w 2 -s ${sz} ${H1_PING_ARG} ${H2_N2_IP6} + run_cmd ip netns exec h1 ping -q -M want -i 0.5 -c 10 -w 2 -s ${sz} ${H1_PING_ARG} ${H2_N2_IP} + run_cmd ip netns exec h1 ${ping6} -q -M want -i 0.5 -c 10 -w 2 -s ${sz} ${H1_PING_ARG} ${H2_N2_IP6} } replace_route_legacy() @@ -317,6 +338,17 @@ replace_route_legacy() run_cmd ip -netns r1 -6 ro replace ${H2_N2_6} via ${R2_LLADDR} dev eth0 } +reset_route_legacy() +{ + run_cmd ip -netns r1 ro del ${H2_N2} + run_cmd ip -netns r1 -6 ro del ${H2_N2_6} + + run_cmd ip -netns h1 ro del ${H1_VRF_ARG} ${H2_N2} + run_cmd ip -netns h1 -6 ro del ${H1_VRF_ARG} ${H2_N2_6} + + initial_route_legacy +} + initial_route_legacy() { # r1 to h2 via r2 and eth1 @@ -373,9 +405,7 @@ do_test() # remove exceptions and restore routing change_h2_mtu 1500 - ip -netns h1 li set br0 down - ip -netns h1 li set br0 up - eval initial_route_${ttype} + eval reset_route_${ttype} check_connectivity if [ $? -ne 0 ]; then -- 2.39.5