In order to select a nexthop for multipath routes, fib_select_multipath()
is used with legacy nexthops and nexthop_select_path_hthr() is used with
nexthop objects. Those two functions perform a validity test on the
neighbor related to each nexthop but their logic is structured differently.
This causes a divergence in behavior and nexthop_select_path_hthr() may
return a nexthop that failed the neighbor validity test even if there was
one that passed.
Refactor nexthop_select_path_hthr() to make it more similar to
fib_select_multipath() and fix the problem mentioned above.
v2:
Removed unnecessary "first" variable in "nexthop: Do not return invalid
nexthop object during multipath selection".
v1:
https://lore.kernel.org/netdev/[email protected]/
---
Benjamin Poirier (4):
nexthop: Factor out hash threshold fdb nexthop selection
nexthop: Factor out neighbor validity check
nexthop: Do not return invalid nexthop object during multipath selection
selftests: net: Add test cases for nexthop groups with invalid neighbors
net/ipv4/nexthop.c | 61 +++++++++----
tools/testing/selftests/net/fib_nexthops.sh | 129 ++++++++++++++++++++++++++++
2 files changed, 171 insertions(+), 19 deletions(-)
---
base-commit: 36395b2efe905650cd179d67411ffee3b770268b
change-id: 20230719-nh_select-0303d55a1fb0
Best regards,
--
Benjamin Poirier <[email protected]>
Add test cases for hash threshold (multipath) nexthop groups with invalid
neighbors. Check that a nexthop with invalid neighbor is not selected when
there is another nexthop with a valid neighbor. Check that there is no
crash when there is no nexthop with a valid neighbor.
The first test fails before the previous commit in this series.
Signed-off-by: Benjamin Poirier <[email protected]>
Reviewed-by: David Ahern <[email protected]>
---
tools/testing/selftests/net/fib_nexthops.sh | 129 ++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index 0f5e88c8f4ff..54ec2b7b7b8c 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -29,6 +29,7 @@ IPV4_TESTS="
ipv4_large_res_grp
ipv4_compat_mode
ipv4_fdb_grp_fcnal
+ ipv4_mpath_select
ipv4_torture
ipv4_res_torture
"
@@ -42,6 +43,7 @@ IPV6_TESTS="
ipv6_large_res_grp
ipv6_compat_mode
ipv6_fdb_grp_fcnal
+ ipv6_mpath_select
ipv6_torture
ipv6_res_torture
"
@@ -370,6 +372,27 @@ check_large_res_grp()
log_test $? 0 "Dump large (x$buckets) nexthop buckets"
}
+get_route_dev()
+{
+ local pfx="$1"
+ local out
+
+ if out=$($IP -j route get "$pfx" | jq -re ".[0].dev"); then
+ echo "$out"
+ fi
+}
+
+check_route_dev()
+{
+ local pfx="$1"
+ local expected="$2"
+ local out
+
+ out=$(get_route_dev "$pfx")
+
+ check_output "$out" "$expected"
+}
+
start_ip_monitor()
{
local mtype=$1
@@ -575,6 +598,112 @@ ipv4_fdb_grp_fcnal()
$IP link del dev vx10
}
+ipv4_mpath_select()
+{
+ local rc dev match h addr
+
+ echo
+ echo "IPv4 multipath selection"
+ echo "------------------------"
+ if [ ! -x "$(command -v jq)" ]; then
+ echo "SKIP: Could not run test; need jq tool"
+ return $ksft_skip
+ fi
+
+ # Use status of existing neighbor entry when determining nexthop for
+ # multipath routes.
+ local -A gws
+ gws=([veth1]=172.16.1.2 [veth3]=172.16.2.2)
+ local -A other_dev
+ other_dev=([veth1]=veth3 [veth3]=veth1)
+
+ run_cmd "$IP nexthop add id 1 via ${gws["veth1"]} dev veth1"
+ run_cmd "$IP nexthop add id 2 via ${gws["veth3"]} dev veth3"
+ run_cmd "$IP nexthop add id 1001 group 1/2"
+ run_cmd "$IP ro add 172.16.101.0/24 nhid 1001"
+ rc=0
+ for dev in veth1 veth3; do
+ match=0
+ for h in {1..254}; do
+ addr="172.16.101.$h"
+ if [ "$(get_route_dev "$addr")" = "$dev" ]; then
+ match=1
+ break
+ fi
+ done
+ if (( match == 0 )); then
+ echo "SKIP: Did not find a route using device $dev"
+ return $ksft_skip
+ fi
+ run_cmd "$IP neigh add ${gws[$dev]} dev $dev nud failed"
+ if ! check_route_dev "$addr" "${other_dev[$dev]}"; then
+ rc=1
+ break
+ fi
+ run_cmd "$IP neigh del ${gws[$dev]} dev $dev"
+ done
+ log_test $rc 0 "Use valid neighbor during multipath selection"
+
+ run_cmd "$IP neigh add 172.16.1.2 dev veth1 nud incomplete"
+ run_cmd "$IP neigh add 172.16.2.2 dev veth3 nud incomplete"
+ run_cmd "$IP route get 172.16.101.1"
+ # if we did not crash, success
+ log_test $rc 0 "Multipath selection with no valid neighbor"
+}
+
+ipv6_mpath_select()
+{
+ local rc dev match h addr
+
+ echo
+ echo "IPv6 multipath selection"
+ echo "------------------------"
+ if [ ! -x "$(command -v jq)" ]; then
+ echo "SKIP: Could not run test; need jq tool"
+ return $ksft_skip
+ fi
+
+ # Use status of existing neighbor entry when determining nexthop for
+ # multipath routes.
+ local -A gws
+ gws=([veth1]=2001:db8:91::2 [veth3]=2001:db8:92::2)
+ local -A other_dev
+ other_dev=([veth1]=veth3 [veth3]=veth1)
+
+ run_cmd "$IP nexthop add id 1 via ${gws["veth1"]} dev veth1"
+ run_cmd "$IP nexthop add id 2 via ${gws["veth3"]} dev veth3"
+ run_cmd "$IP nexthop add id 1001 group 1/2"
+ run_cmd "$IP ro add 2001:db8:101::/64 nhid 1001"
+ rc=0
+ for dev in veth1 veth3; do
+ match=0
+ for h in {1..65535}; do
+ addr=$(printf "2001:db8:101::%x" $h)
+ if [ "$(get_route_dev "$addr")" = "$dev" ]; then
+ match=1
+ break
+ fi
+ done
+ if (( match == 0 )); then
+ echo "SKIP: Did not find a route using device $dev"
+ return $ksft_skip
+ fi
+ run_cmd "$IP neigh add ${gws[$dev]} dev $dev nud failed"
+ if ! check_route_dev "$addr" "${other_dev[$dev]}"; then
+ rc=1
+ break
+ fi
+ run_cmd "$IP neigh del ${gws[$dev]} dev $dev"
+ done
+ log_test $rc 0 "Use valid neighbor during multipath selection"
+
+ run_cmd "$IP neigh add 2001:db8:91::2 dev veth1 nud incomplete"
+ run_cmd "$IP neigh add 2001:db8:92::2 dev veth3 nud incomplete"
+ run_cmd "$IP route get 2001:db8:101::1"
+ # if we did not crash, success
+ log_test $rc 0 "Multipath selection with no valid neighbor"
+}
+
################################################################################
# basic operations (add, delete, replace) on nexthops and nexthop groups
#
--
2.40.1
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
On Wed, 19 Jul 2023 13:57:06 +0000 you wrote:
> In order to select a nexthop for multipath routes, fib_select_multipath()
> is used with legacy nexthops and nexthop_select_path_hthr() is used with
> nexthop objects. Those two functions perform a validity test on the
> neighbor related to each nexthop but their logic is structured differently.
> This causes a divergence in behavior and nexthop_select_path_hthr() may
> return a nexthop that failed the neighbor validity test even if there was
> one that passed.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/4] nexthop: Factor out hash threshold fdb nexthop selection
https://git.kernel.org/netdev/net-next/c/eedd47a6ec9f
- [net-next,v2,2/4] nexthop: Factor out neighbor validity check
https://git.kernel.org/netdev/net-next/c/4bb5239b4334
- [net-next,v2,3/4] nexthop: Do not return invalid nexthop object during multipath selection
https://git.kernel.org/netdev/net-next/c/75f5f04c7bd2
- [net-next,v2,4/4] selftests: net: Add test cases for nexthop groups with invalid neighbors
https://git.kernel.org/netdev/net-next/c/c7e95bbda822
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html