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

login
register
mail settings
Submitter Hannes Frederic Sowa
Date July 12, 2013, 7:20 p.m.
Message ID <20130712192023.GP12611@order.stressinduktion.org>
Download mbox | patch
Permalink /patch/258782/
State RFC
Delegated to: David Miller
Headers show

Comments

Hannes Frederic Sowa - July 12, 2013, 7:20 p.m.
On Fri, Jul 12, 2013 at 09:01:17PM +0200, Nicolas Dichtel wrote:
> Le 12/07/2013 18:19, Hannes Frederic Sowa a écrit :
> >On Fri, Jul 12, 2013 at 02:04:45PM +0200, Nicolas Dichtel wrote:
> >>It's possible to add a glue to check this counter when we play with these
> >>flags, but it's ugly.
> >>
> >>Maybe the check against RTF_EXPIRES is fundamentally wrong. Checking
> >>RTF_ADDRCONF|RTF_DYNAMIC should be enough, what do you think?
> >
> >Yes, this seems to be the best option now. I will audit the source if
> >RTF_ADDRCONF and RTF_DYNAMIC are immutable after dst construction and
> >if other errors could arise for that and would go with this solution then.
> >
> >What do you think about making ecmp routes explicit by adding RTF_ECMP
> >flag?
> Why not, but you will have to be careful on insertion and deletion. Next 
> hop can be added one by one and deleted one by one.

Ok, we can have a look to do so in -next.

> >
> >>In another hand, we can discuss about the initial assumption, that was
> >>"only static routes are part of ECMP routes". I'm thinking of what are the
> >>consequence if we accept to accept all routes, without checking any flags.
> >
> >I don't have a good feeling about that. But I may be wrong.
> Same for me, but for now, I don't have any argument ;-) The above solution 
> is probably the better way for now.

To go without RTF_EXPIRES seems the way to go. I still am unsure if we need to
propagate the RTF_DYNAMIC flag in case we update the expiration date on a
route.


I hope to have identified all possible site-effects later today.

Thanks,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa - July 12, 2013, 9:48 p.m.
On Fri, Jul 12, 2013 at 09:20:23PM +0200, Hannes Frederic Sowa wrote:
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -682,6 +682,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
>                                         rt->rt6i_nsiblings = 0;
>                                 if (!(iter->rt6i_flags & RTF_EXPIRES))
>                                         return -EEXIST;
> +                               iter->rt6i_flags |= rt->rt6i_flags & RTF_DYNAMIC;
>                                 if (!(rt->rt6i_flags & RTF_EXPIRES))
>                                         rt6_clean_expires(iter);
>                                 else
> 

This was not necessary and just complicated things because we would have to
special-case the toggling of the RTF_DYNAMIC flag to also get removed from the
ecmp route set.

Thanks,

  Hannes

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

Patch

--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -682,6 +682,7 @@  static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
                                        rt->rt6i_nsiblings = 0;
                                if (!(iter->rt6i_flags & RTF_EXPIRES))
                                        return -EEXIST;
+                               iter->rt6i_flags |= rt->rt6i_flags & RTF_DYNAMIC;
                                if (!(rt->rt6i_flags & RTF_EXPIRES))
                                        rt6_clean_expires(iter);
                                else