| Message ID | 20180715163519.12917-1-dsahern@kernel.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net] net/ipv6: Do not allow device only routes via the multipath API | expand |
On 07/15/2018 09:35 AM, dsahern@kernel.org wrote: > From: David Ahern <dsahern@gmail.com> > > Eric reported that reverting the patch that fixed and simplified IPv6 > multipath routes means reverting back to invalid userspace notifications. > eg., > $ ip -6 route add 2001:db8:1::/64 nexthop dev eth0 nexthop dev eth1 > > only generates a single notification: > 2001:db8:1::/64 dev eth0 metric 1024 pref medium > > While working on a fix for this problem I found another case that is just > broken completely - a multipath route with a gateway followed by device > followed by gateway: > $ ip -6 ro add 2001:db8:103::/64 > nexthop via 2001:db8:1::64 > nexthop dev dummy2 > nexthop via 2001:db8:3::64 > > In this case the device only route is dropped completely - no notification > to userpsace but no addition to the FIB either: > > $ ip -6 ro ls > 2001:db8:1::/64 dev dummy1 proto kernel metric 256 pref medium > 2001:db8:2::/64 dev dummy2 proto kernel metric 256 pref medium > 2001:db8:3::/64 dev dummy3 proto kernel metric 256 pref medium > 2001:db8:103::/64 metric 1024 > nexthop via 2001:db8:1::64 dev dummy1 weight 1 > nexthop via 2001:db8:3::64 dev dummy3 weight 1 pref medium > fe80::/64 dev dummy1 proto kernel metric 256 pref medium > fe80::/64 dev dummy2 proto kernel metric 256 pref medium > fe80::/64 dev dummy3 proto kernel metric 256 pref medium > > Really, IPv6 multipath is just FUBAR'ed beyond repair when it comes to > device only routes, so do not allow it all. > > This change will break any scripts relying on the mpath api for insert, > but I don't see any other way to handle the permutations. Besides, since > the routes are added to the FIB as standalone (non-multipath) routes the > kernel is not doing what the user requested, so it might as well tell the > user that. Yes, I guess we have no real choice for the moment. Thanks David Reviewed-by: Eric Dumazet <edumazet@google.com>
On 7/16/18 10:09 AM, Eric Dumazet wrote:
> Yes, I guess we have no real choice for the moment.
It is unfortunate that we are forever stuck with this mess from a short
sighted implementation years ago. From a uapi perspective, dev-only
nexthops and proper add-to/append/replace semantics should have been a
part of the code from the beginning.
From: dsahern@kernel.org Date: Sun, 15 Jul 2018 09:35:19 -0700 > From: David Ahern <dsahern@gmail.com> > > Eric reported that reverting the patch that fixed and simplified IPv6 > multipath routes means reverting back to invalid userspace notifications. > eg., > $ ip -6 route add 2001:db8:1::/64 nexthop dev eth0 nexthop dev eth1 > > only generates a single notification: > 2001:db8:1::/64 dev eth0 metric 1024 pref medium > > While working on a fix for this problem I found another case that is just > broken completely - a multipath route with a gateway followed by device > followed by gateway: > $ ip -6 ro add 2001:db8:103::/64 > nexthop via 2001:db8:1::64 > nexthop dev dummy2 > nexthop via 2001:db8:3::64 > > In this case the device only route is dropped completely - no notification > to userpsace but no addition to the FIB either: > > $ ip -6 ro ls > 2001:db8:1::/64 dev dummy1 proto kernel metric 256 pref medium > 2001:db8:2::/64 dev dummy2 proto kernel metric 256 pref medium > 2001:db8:3::/64 dev dummy3 proto kernel metric 256 pref medium > 2001:db8:103::/64 metric 1024 > nexthop via 2001:db8:1::64 dev dummy1 weight 1 > nexthop via 2001:db8:3::64 dev dummy3 weight 1 pref medium > fe80::/64 dev dummy1 proto kernel metric 256 pref medium > fe80::/64 dev dummy2 proto kernel metric 256 pref medium > fe80::/64 dev dummy3 proto kernel metric 256 pref medium > > Really, IPv6 multipath is just FUBAR'ed beyond repair when it comes to > device only routes, so do not allow it all. > > This change will break any scripts relying on the mpath api for insert, > but I don't see any other way to handle the permutations. Besides, since > the routes are added to the FIB as standalone (non-multipath) routes the > kernel is not doing what the user requested, so it might as well tell the > user that. > > Reported-by: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: David Ahern <dsahern@gmail.com> Applied, thanks David. Is this a -stable candidate?
On 7/16/18 3:09 PM, David Miller wrote: > > Is this a -stable candidate? > I think so. The API is not doing what the user requested, even though the route add does not fail.
From: David Ahern <dsahern@gmail.com> Date: Tue, 17 Jul 2018 06:13:43 -0600 > On 7/16/18 3:09 PM, David Miller wrote: >> >> Is this a -stable candidate? >> > > I think so. The API is not doing what the user requested, even though > the route add does not fail. Ok, queued up, thanks.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 63f99411f0de..1f1f0f318d74 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4388,6 +4388,13 @@ static int ip6_route_multipath_add(struct fib6_config *cfg, rt = NULL; goto cleanup; } + if (!rt6_qualify_for_ecmp(rt)) { + err = -EINVAL; + NL_SET_ERR_MSG(extack, + "Device only routes can not be added for IPv6 using the multipath API."); + fib6_info_release(rt); + goto cleanup; + } rt->fib6_nh.nh_weight = rtnh->rtnh_hops + 1;