diff mbox series

[net,2/2] selftests/net: test nexthop without gw

Message ID 20220706160526.31711-2-nicolas.dichtel@6wind.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,1/2] ip: fix dflt addr selection for connected nexthop | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: linux-kselftest@vger.kernel.org shuah@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch fail ERROR: trailing whitespace WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Nicolas Dichtel July 6, 2022, 4:05 p.m. UTC 
This test implement the scenario described in the previous patch.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 tools/testing/selftests/net/Makefile          |   2 +-
 .../selftests/net/fib_nexthop_nongw.sh        | 125 ++++++++++++++++++
 2 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/net/fib_nexthop_nongw.sh

Comments

Paolo Abeni July 12, 2022, 8:19 a.m. UTC | #1 
On Wed, 2022-07-06 at 18:05 +0200, Nicolas Dichtel wrote:
> This test implement the scenario described in the previous patch.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  tools/testing/selftests/net/Makefile          |   2 +-
>  .../selftests/net/fib_nexthop_nongw.sh        | 125 ++++++++++++++++++
>  2 files changed, 126 insertions(+), 1 deletion(-)
>  create mode 100755 tools/testing/selftests/net/fib_nexthop_nongw.sh
> 
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index ddad703ace34..db05b3764b77 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -11,7 +11,7 @@ TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
>  TEST_PROGS += udpgro_bench.sh udpgro.sh test_vxlan_under_vrf.sh reuseport_addr_any.sh
>  TEST_PROGS += test_vxlan_fdb_changelink.sh so_txtime.sh ipv6_flowlabel.sh
>  TEST_PROGS += tcp_fastopen_backup_key.sh fcnal-test.sh l2tp.sh traceroute.sh
> -TEST_PROGS += fin_ack_lat.sh fib_nexthop_multiprefix.sh fib_nexthops.sh
> +TEST_PROGS += fin_ack_lat.sh fib_nexthop_multiprefix.sh fib_nexthops.sh fib_nexthop_nongw.sh
>  TEST_PROGS += altnames.sh icmp.sh icmp_redirect.sh ip6_gre_headroom.sh
>  TEST_PROGS += route_localnet.sh
>  TEST_PROGS += reuseaddr_ports_exhausted.sh
> diff --git a/tools/testing/selftests/net/fib_nexthop_nongw.sh b/tools/testing/selftests/net/fib_nexthop_nongw.sh
> new file mode 100755
> index 000000000000..6e82562eaf4a
> --- /dev/null
> +++ b/tools/testing/selftests/net/fib_nexthop_nongw.sh
> @@ -0,0 +1,125 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# ns: h1               | ns: h2           

Trailing whitespace above

> +#   192.168.0.1/24     |
> +#            eth0      |
> +#                      |       192.168.1.1/32
> +#            veth0 <---|---> veth1        

same here.

> +# Validate source address selection for route without gateway
> +
> +PAUSE_ON_FAIL=no
> +VERBOSE=0
> +ret=0
> +
> +################################################################################
> +# helpers
> +
> +log_test()
> +{
> +	local rc=$1
> +	local expected=$2
> +	local msg="$3"
> +
> +	if [ ${rc} -eq ${expected} ]; then
> +		printf "TEST: %-60s  [ OK ]\n" "${msg}"
> +		nsuccess=$((nsuccess+1))
> +	else
> +		ret=1
> +		nfail=$((nfail+1))
> +		printf "TEST: %-60s  [FAIL]\n" "${msg}"
> +		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> +			echo
> +			echo "hit enter to continue, 'q' to quit"
> +			read a
> +			[ "$a" = "q" ] && exit 1
> +		fi
> +	fi
> +
> +	[ "$VERBOSE" = "1" ] && echo
> +}
> +
> +run_cmd()
> +{
> +	local cmd="$*"
> +	local out
> +	local rc
> +
> +	if [ "$VERBOSE" = "1" ]; then
> +		echo "COMMAND: $cmd"
> +	fi
> +
> +	out=$(eval $cmd 2>&1)
> +	rc=$?
> +	if [ "$VERBOSE" = "1" -a -n "$out" ]; then
> +		echo "$out"
> +	fi
> +
> +	[ "$VERBOSE" = "1" ] && echo
> +
> +	return $rc
> +}
> +
> +################################################################################
> +# config
> +setup()
> +{
> +	ip netns add h1
> +	ip -n h1 link set lo up
> +	ip netns add h2
> +	ip -n h2 link set lo up
> +	sleep 1

Why is this needed here? same question for the 'sleep 2' after the
setup.

> +
> +	# Add a fake eth0 to support an ip address
> +	ip -n h1 link add name eth0 type dummy
> +	ip -n h1 link set eth0 up
> +	ip -n h1 address add 192.168.0.1/24 dev eth0
> +
> +	# Configure veths (same @mac, arp off)
> +	ip -n h1 link add name veth0 type veth peer name veth1 netns h2
> +	ip -n h1 link set veth0 address 00:09:c0:26:05:82
> +	ip -n h1 link set veth0 arp off

As in the example in the previous commit, I suggest to drop the
apparently not relevant  'arp off'/ static macs 

> +	ip -n h1 link set veth0 up
> +
> +	ip -n h2 link set veth1 address 00:09:c0:26:05:82
> +	ip -n h2 link set veth1 arp off
> +	ip -n h2 link set veth1 up
> +
> +	# Configure @IP in the peer netns
> +	ip -n h2 address add 192.168.1.1/32 dev veth1
> +	ip -n h2 route add default dev veth1
> +
> +	# Add a nexthop without @gw and use it in a route
> +	ip -n h1 nexthop add id 1 dev veth0
> +	ip -n h1 route add 192.168.1.1 nhid 1
> +}
> +
> +cleanup()
> +{
> +	ip netns del h1 2>/dev/null
> +	ip netns del h2 2>/dev/null
> +}

This become more roboust if you add a

trap cleanup EXIT

additionally, with the above you could remove the explicit cleanups
below

> +
> +################################################################################
> +# main
> +
> +while getopts :pv o
> +do
> +	case $o in
> +		p) PAUSE_ON_FAIL=yes;;
> +		v) VERBOSE=1;;
> +	esac
> +done
> +
> +cleanup
> +setup
> +sleep 2
> +
> +run_cmd ip -netns h1 route get 192.168.1.1
> +log_test $? 0 "nexthop: get route with nexthop without gw"
> +run_cmd ip netns exec h1 ping -c1 192.168.1.1
> +log_test $? 0 "nexthop: ping through nexthop without gw"
> +
> +cleanup
> +
> +exit $ret
Nicolas Dichtel July 12, 2022, 9:19 a.m. UTC | #2 
Le 12/07/2022 à 10:19, Paolo Abeni a écrit :
[snip]
>> +################################################################################
>> +# config
>> +setup()
>> +{
>> +	ip netns add h1
>> +	ip -n h1 link set lo up
>> +	ip netns add h2
>> +	ip -n h2 link set lo up
>> +	sleep 1
> 
> Why is this needed here? same question for the 'sleep 2' after the
> setup.

The 'sleep 2' after the setup was 'copy & paste'. I will remove it.

The bug was initially spotted in 'init_net' and when I first tried to reproduce
it with netns, I didn't succeed without the 'sleep 1'. I didn't analyzed more
deeply. In fact, when I replay the test now, it fails as expected.
Let's remove it also.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index ddad703ace34..db05b3764b77 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -11,7 +11,7 @@  TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
 TEST_PROGS += udpgro_bench.sh udpgro.sh test_vxlan_under_vrf.sh reuseport_addr_any.sh
 TEST_PROGS += test_vxlan_fdb_changelink.sh so_txtime.sh ipv6_flowlabel.sh
 TEST_PROGS += tcp_fastopen_backup_key.sh fcnal-test.sh l2tp.sh traceroute.sh
-TEST_PROGS += fin_ack_lat.sh fib_nexthop_multiprefix.sh fib_nexthops.sh
+TEST_PROGS += fin_ack_lat.sh fib_nexthop_multiprefix.sh fib_nexthops.sh fib_nexthop_nongw.sh
 TEST_PROGS += altnames.sh icmp.sh icmp_redirect.sh ip6_gre_headroom.sh
 TEST_PROGS += route_localnet.sh
 TEST_PROGS += reuseaddr_ports_exhausted.sh
diff --git a/tools/testing/selftests/net/fib_nexthop_nongw.sh b/tools/testing/selftests/net/fib_nexthop_nongw.sh
new file mode 100755
index 000000000000..6e82562eaf4a
--- /dev/null
+++ b/tools/testing/selftests/net/fib_nexthop_nongw.sh
@@ -0,0 +1,125 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# ns: h1               | ns: h2           
+#   192.168.0.1/24     |
+#            eth0      |
+#                      |       192.168.1.1/32
+#            veth0 <---|---> veth1        
+# Validate source address selection for route without gateway
+
+PAUSE_ON_FAIL=no
+VERBOSE=0
+ret=0
+
+################################################################################
+# helpers
+
+log_test()
+{
+	local rc=$1
+	local expected=$2
+	local msg="$3"
+
+	if [ ${rc} -eq ${expected} ]; then
+		printf "TEST: %-60s  [ OK ]\n" "${msg}"
+		nsuccess=$((nsuccess+1))
+	else
+		ret=1
+		nfail=$((nfail+1))
+		printf "TEST: %-60s  [FAIL]\n" "${msg}"
+		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+			echo
+			echo "hit enter to continue, 'q' to quit"
+			read a
+			[ "$a" = "q" ] && exit 1
+		fi
+	fi
+
+	[ "$VERBOSE" = "1" ] && echo
+}
+
+run_cmd()
+{
+	local cmd="$*"
+	local out
+	local rc
+
+	if [ "$VERBOSE" = "1" ]; then
+		echo "COMMAND: $cmd"
+	fi
+
+	out=$(eval $cmd 2>&1)
+	rc=$?
+	if [ "$VERBOSE" = "1" -a -n "$out" ]; then
+		echo "$out"
+	fi
+
+	[ "$VERBOSE" = "1" ] && echo
+
+	return $rc
+}
+
+################################################################################
+# config
+setup()
+{
+	ip netns add h1
+	ip -n h1 link set lo up
+	ip netns add h2
+	ip -n h2 link set lo up
+	sleep 1
+
+	# Add a fake eth0 to support an ip address
+	ip -n h1 link add name eth0 type dummy
+	ip -n h1 link set eth0 up
+	ip -n h1 address add 192.168.0.1/24 dev eth0
+
+	# Configure veths (same @mac, arp off)
+	ip -n h1 link add name veth0 type veth peer name veth1 netns h2
+	ip -n h1 link set veth0 address 00:09:c0:26:05:82
+	ip -n h1 link set veth0 arp off
+	ip -n h1 link set veth0 up
+
+	ip -n h2 link set veth1 address 00:09:c0:26:05:82
+	ip -n h2 link set veth1 arp off
+	ip -n h2 link set veth1 up
+
+	# Configure @IP in the peer netns
+	ip -n h2 address add 192.168.1.1/32 dev veth1
+	ip -n h2 route add default dev veth1
+
+	# Add a nexthop without @gw and use it in a route
+	ip -n h1 nexthop add id 1 dev veth0
+	ip -n h1 route add 192.168.1.1 nhid 1
+}
+
+cleanup()
+{
+	ip netns del h1 2>/dev/null
+	ip netns del h2 2>/dev/null
+}
+
+################################################################################
+# main
+
+while getopts :pv o
+do
+	case $o in
+		p) PAUSE_ON_FAIL=yes;;
+		v) VERBOSE=1;;
+	esac
+done
+
+cleanup
+setup
+sleep 2
+
+run_cmd ip -netns h1 route get 192.168.1.1
+log_test $? 0 "nexthop: get route with nexthop without gw"
+run_cmd ip netns exec h1 ping -c1 192.168.1.1
+log_test $? 0 "nexthop: ping through nexthop without gw"
+
+cleanup
+
+exit $ret