Re: [PATCH RFC] ipv6: fix route selection if kernel is not compiled with CONFIG_IPV6_ROUTER_PREF

[Hannes Frederic Sowa <hannes@stressinduktion.org>]

On Thu, Jul 11, 2013 at 10:04:47AM +0200, Nicolas Dichtel wrote:
> Le 10/07/2013 23:21, Hannes Frederic Sowa a écrit :
> >On Wed, Jul 10, 2013 at 04:10:58PM +0200, Nicolas Dichtel wrote:
> >>I wonder why expires is 0. Even if this route is cached, the flag
> >>RTF_EXPIRES should be set. Am I wrong?
> >
> >rt6_set_from deliberately clears the RTF_EXPIRES when creating a cached 
> >copy
> >of the route if the route is an autoconfigured default route.
> >
> >Maybe the criterion for exclusion of which routes can get into an ecmp 
> >route
> >set should be revisited? This could result in strange effects for users
> >working with two interfaces, both receiving a RA with default routes.
> Agreed. Here is a proposal, what do you think?
> 
> [PATCH] ipv6: don't use autoconfigured route for ecmp
> 
> The intention was already there by checking the flag RTF_EXPIRES, but this 
> flag
> is removed from the default route by rt6_set_from() when this route is 
> cached.
> 
> Let's add a check against RTF_ADDRCONF.
> 
> Spotted-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

I do think the patch is ok. I just wanted to show you a solution I did
on my laptop this night and did not have the time to send yesterday. It
is only compile-tested.

I did strengthen the RTF_EXPIRES check a bit. Also I am not sure what
did stop the search for the first route with the same metric to find a
RTF_EXPIRES route, so I also added the guard there, too.

I fear, I'll need to do a bit more research.

Thanks!

[PATCH RFC] ipv6: routes only qualify for ecmp if their original routes do not expire

Cloned routes get their RTF_EXPIRES flag reset if they are autoconfigured
default routes. Because these routes should not end up in the ecmp route
set, we check if the original route has the RTF_EXPIRES flag set.

Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/ip6_fib.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 192dd1a..3bd5fcd 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -632,6 +632,18 @@ insert_above:
 	return ln;
 }
 
+static bool rt6_qualify_for_ecmp(struct rt6_info *rt0)
+{
+	struct rt6_info *rt;
+
+	if (!(rt0->rt6i_flags & RTF_GATEWAY))
+		return false;
+
+	for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES);
+	     rt = (struct rt6_info *)rt->dst.from);
+	return !(rt && (rt->rt6i_flags & RTF_EXPIRES));
+}
+
 /*
  *	Insert routing information in a node.
  */
@@ -691,9 +703,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 			 * To avoid long list, we only had siblings if the
 			 * route have a gateway.
 			 */
-			if (rt->rt6i_flags & RTF_GATEWAY &&
-			    !(rt->rt6i_flags & RTF_EXPIRES) &&
-			    !(iter->rt6i_flags & RTF_EXPIRES))
+			if (rt6_qualify_for_ecmp(rt) &&
+			    rt6_qualify_for_ecmp(iter))
 				rt->rt6i_nsiblings++;
 		}
 
@@ -715,7 +726,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 		/* Find the first route that have the same metric */
 		sibling = fn->leaf;
 		while (sibling) {
-			if (sibling->rt6i_metric == rt->rt6i_metric) {
+			if (sibling->rt6i_metric == rt->rt6i_metric &&
+			    rt6_qualify_for_ecmp(sibling)) {
 				list_add_tail(&rt->rt6i_siblings,
 					      &sibling->rt6i_siblings);
 				break;
-- 
1.8.1.4