Re: [PATCH net v2 2/2] ipv6: fix ECMP route replacement
From: Nicolas Dichtel <hidden>
Date: 2015-05-14 18:59:05
Also in:
lkml
Le 13/05/2015 21:59, Michal Kubecek a écrit :
quoted hunk ↗ jump to hunk
When replacing an IPv6 multipath route with "ip route replace", i.e. NLM_F_CREATE | NLM_F_REPLACE, fib6_add_rt2node() replaces only first matching route without fixing its siblings, resulting in corrupted siblings linked list; removing one of the siblings can then end in an infinite loop. Replacing the whole set of nexthops does IMHO make more sense than replacing a random one. We also need to remove the NLM_F_REPLACE flag after replacing old nexthops by first new so that each subsequent nexthop does not replace previous one. Fixes: 51ebd3181572 ("ipv6: add support of equal cost multipath (ECMP)") Signed-off-by: Michal Kubecek <redacted> --- net/ipv6/ip6_fib.c | 17 ++++++++++++++--- net/ipv6/route.c | 8 +++++--- 2 files changed, 19 insertions(+), 6 deletions(-)diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 96dbffff5a24..abf4e4e5bdab 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c@@ -815,6 +815,8 @@ add: } } else { + struct rt6_info *next; + if (!found) { if (add) goto add;@@ -828,15 +830,24 @@ add: *ins = rt; rt->rt6i_node = fn; - rt->dst.rt6_next = iter->dst.rt6_next; + + /* skip potential siblings */ + next = iter->dst.rt6_next; + while (next && next->rt6i_metric == rt->rt6i_metric) + next = next->dst.rt6_next;
I wonder if we should not loop over the siblings list here (rt->rt6i_siblings). Only routes that match 'rt6_qualify_for_ecmp()' are siblings.
+ rt->dst.rt6_next = next;
+
atomic_inc(&rt->rt6i_ref);
inet6_rt_notify(RTM_NEWROUTE, rt, info);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
}
- fib6_purge_rt(iter, fn, info->nl_net);
- rt6_release(iter);
+ while (iter != next) {
+ fib6_purge_rt(iter, fn, info->nl_net);
+ rt6_release(iter);
+ iter = iter->dst.rt6_next;
+ }Same here.