Le 03/09/2015 01:44, Roopa Prabhu a écrit :
From: Roopa Prabhu <ro...@cumulusnetworks.com>

Problem:
The ecmp route replace support for ipv6 in the kernel, deletes the
existing ecmp route too early, ie when it installs the first nexthop.
If there is an error in installing the subsequent nexthops, its too late
to recover the already deleted existing route

This patch fixes the problem with the following:
a) Changes the existing multipath route add code to a two stage process:
   build rt6_infos + insert them
        ip6_route_add rt6_info creation code is moved into
        ip6_route_info_create.
b) This ensures that all errors are caught during building rt6_infos
   and we fail early
c) Separates multipath add and del code. Because add needs the special
   two stage mode in a) and delete essentially does not care.
d) In any event if the code fails during inserting a route again, a
   warning is printed (This should be unlikely)

Before the patch:
$ip -6 route show
3000:1000:1000:1000::2 via fe80::202:ff:fe00:b dev swp49s0 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:d dev swp49s1 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:f dev swp49s2 metric 1024

/* Try replacing the route with a duplicate nexthop */
$ip -6 route change 3000:1000:1000:1000::2/128 nexthop via
fe80::202:ff:fe00:b dev swp49s0 nexthop via fe80::202:ff:fe00:d dev
swp49s1 nexthop via fe80::202:ff:fe00:d dev swp49s1
RTNETLINK answers: File exists

$ip -6 route show
/* previously added ecmp route 3000:1000:1000:1000::2 dissappears from
  * kernel */

After the patch:
$ip -6 route show
3000:1000:1000:1000::2 via fe80::202:ff:fe00:b dev swp49s0 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:d dev swp49s1 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:f dev swp49s2 metric 1024

/* Try replacing the route with a duplicate nexthop */
$ip -6 route change 3000:1000:1000:1000::2/128 nexthop via
fe80::202:ff:fe00:b dev swp49s0 nexthop via fe80::202:ff:fe00:d dev
swp49s1 nexthop via fe80::202:ff:fe00:d dev swp49s1
RTNETLINK answers: File exists

$ip -6 route show
3000:1000:1000:1000::2 via fe80::202:ff:fe00:b dev swp49s0 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:d dev swp49s1 metric 1024
3000:1000:1000:1000::2 via fe80::202:ff:fe00:f dev swp49s2 metric 1024

Fixes: 4a287eba2de3 ("IPv6 routing, NLM_F_* flag support: REPLACE and EXCL flags 
support, warn about missing CREATE flag")
ECMP was added one year after this patch. The right tag is:
Fixes: 51ebd3181572 ("ipv6: add support of equal cost multipath (ECMP)")

Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com>
---
v2 - fix a rt6_info leak in cleanup on error

This bug is present in 4.1 kernel and 4.2 too.
Since 4.2 is out or almost out, I am submitting the patch against net-next.
I can respin against net if needed. I have tried to keep the changes local
to route.c closer to the netlink message handling. Most of the changes move
code into separate functions.

  net/ipv6/route.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++-------
  1 file changed, 183 insertions(+), 26 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
[snip]
+static void ip6_print_replace_route_err(struct list_head *rt6_nh_list)
+{
+       struct rt6_nh *nh;
+       char *errstr = "IPV6: unexpected error replacing route";
Generally, it's better to not break log. It eases grep.
Something shorter may be enough: "ECMPv6", the log level already indicates
that it's an error (which is always unexpected ;-)).

+
+       list_for_each_entry(nh, rt6_nh_list, next) {
+               printk(KERN_WARNING "%s: %pI6 nexthop %pI6 ifi %d\n",
pr_warn() or pr_err()?

+                      errstr, &nh->r_cfg.fc_dst, &nh->r_cfg.fc_gateway,
+                      nh->r_cfg.fc_ifindex);
+       }
+}

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to