From mboxrd@z Thu Jan 1 00:00:00 1970 From: chenweilong Subject: Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo Date: Fri, 17 Jan 2014 10:02:17 +0800 Message-ID: <52D88F29.5010404@huawei.com> References: <20131231035718.GB27636@order.stressinduktion.org> <52C4FDBE.5030607@huawei.com> <20140102065429.GB22494@order.stressinduktion.org> <52C51C1E.9040603@huawei.com> <20140102082325.GC22494@order.stressinduktion.org> <52C5325B.8020900@huawei.com> <20140103065349.GK22494@order.stressinduktion.org> <52CD0331.8040204@cn.fujitsu.com> <20140108080528.GD9007@order.stressinduktion.org> <52CD0F86.30605@cn.fujitsu.com> <20140108085554.GF9007@order.stressinduktion.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Gao feng , David Miller , , To: Hannes Frederic Sowa Return-path: Received: from szxga02-in.huawei.com ([119.145.14.65]:6896 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751770AbaAQCC1 (ORCPT ); Thu, 16 Jan 2014 21:02:27 -0500 In-Reply-To: <20140108085554.GF9007@order.stressinduktion.org> Sender: netdev-owner@vger.kernel.org List-ID: On 2014/1/8 16:55, Hannes Frederic Sowa wrote: > On Wed, Jan 08, 2014 at 04:42:46PM +0800, Gao feng wrote: >> On 01/08/2014 04:05 PM, Hannes Frederic Sowa wrote: >>> On Wed, Jan 08, 2014 at 03:50:09PM +0800, Gao feng wrote: >>>> On 01/03/2014 02:53 PM, Hannes Frederic Sowa wrote: >>>>> On Thu, Jan 02, 2014 at 05:33:15PM +0800, chenweilong wrote: >>>>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >>>>>> index 62d1799..d2f8c0a 100644 >>>>>> --- a/net/ipv6/addrconf.c >>>>>> +++ b/net/ipv6/addrconf.c >>>>>> @@ -2422,8 +2422,9 @@ static void init_loopback(struct net_device *dev) >>>>>> if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE)) >>>>>> continue; >>>>>> >>>>>> - if (sp_ifa->rt) >>>>>> - continue; >>>>>> + if (sp_ifa->rt && sp_ifa->rt->dst.dev == dev) { >>>>>> + ip6_del_rt(sp_ifa->rt); >>>>>> + } >>>>>> >>>>>> sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, 0); >>>>>> >>>>> >>>>> Maybe this change would not be that bad after all, as those ifa attached dsts >>>>> are already dead and queued up for gc and should not get inserted back. >>>> >>>> I like this idea, maybe the below patch is better. we only need to delete this >>>> route when it has been added to garbage list. >>>> >>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >>>> index 1a341f7..4dca886 100644 >>>> --- a/net/ipv6/addrconf.c >>>> +++ b/net/ipv6/addrconf.c >>>> @@ -2610,8 +2610,16 @@ static void init_loopback(struct net_device *dev) >>>> if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE)) >>>> continue; >>>> >>>> - if (sp_ifa->rt) >>>> - continue; >>>> + if (sp_ifa->rt) { >>>> + /* This dst has been added to garbage list when >>>> + * lo device down, delete this obsolete dst and >>>> + * reallocate new router for ifa. */ >>>> + if (sp_ifa->rt->dst.obsolete > 0) { >>>> + ip6_del_rt(sp_ifa->rt); >>>> + sp_ifa->rt = NULL; >>>> + } else >>>> + continue; >>>> + } >>>> >>>> sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, false); >>> >>> It looks like it can work but I don't know if we should just fix this the >>> clean way (see below). >>> >>>>> I'll try to just disable routes without removing them at all when we set an >>>>> interface to down at the weekend. >>>>> >>>> >>>> How do you decide which route should be disabled? use rt6_flags? I don't know >>>> if your way will cause miscarriage. >>> >>> What I did so far is that I added a new function next to rt6_ifdown that >>> only gets called if interface gets shutdown but not unregistered (from >>> addrconf_ifdown). >>> >> >> rt6_ifdown has alreay put this device related routes to the garbage list. >> >>> fib6_clean_all then iterates over the whole routing table with a new predicate >>> function which checks in the same way like fib6_ifdown, if it is a matching route >>> (the interfaces match up) and if so, toggles a new "DEAD" flag in rt6i_flags. >>> >>> When bringing up the interface I distinguish between up and register and just >>> clear this death flag from the routes on bringing it up. >>> >>> fib lookup code then does not honour those routes. >>> >>> I had no time to test this thoroughly at the weekend and still have some code >>> paths were I am unsure. Do you see any problems with this so far? We could >>> then delete the special cases on loopback interface init. >> >> So you add a special case in the general network interface down logic. I think this >> is more complex... > > The problem is, that we only have a reference to ifp->rt, the loopback > RTF_LOCAL route. Currently we silently remove all other routes this interface > has. Sure, they come back soon with RAs and such, but this is not the way this > should work. > > Greetings, > > Hannes > > > . > Hi, It's quite a long time, How's your patch going on?