* [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
@ 2013-06-16 3:14 Gao feng
2013-06-20 6:05 ` David Miller
0 siblings, 1 reply; 19+ messages in thread
From: Gao feng @ 2013-06-16 3:14 UTC (permalink / raw)
To: davem; +Cc: kumaran.4353, netdev, Gao feng
If we disable all of the net interfaces, and enable
un-lo interface before lo interface, we already allocated
the addrconf dst in ipv6_add_addr. So we shouldn't allocate
it again when we enable lo interface.
Otherwise the message below will be triggered.
unregister_netdevice: waiting for sit1 to become free. Usage count = 1
This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
"net IPv6 : Fix broken IPv6 routing table after loopback down-up"
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/ipv6/addrconf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1bbf744..77b6261 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2655,6 +2655,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;
+
sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, 0);
/* Failure cases are ignored */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2013-06-16 3:14 [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo Gao feng
@ 2013-06-20 6:05 ` David Miller
2013-12-31 3:57 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2013-06-20 6:05 UTC (permalink / raw)
To: gaofeng; +Cc: kumaran.4353, netdev
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Sun, 16 Jun 2013 11:14:30 +0800
> If we disable all of the net interfaces, and enable
> un-lo interface before lo interface, we already allocated
> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
> it again when we enable lo interface.
>
> Otherwise the message below will be triggered.
> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
>
> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
This is the second such regression added by that commit :-/
Applied and queue up for -stable, thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2013-06-20 6:05 ` David Miller
@ 2013-12-31 3:57 ` Hannes Frederic Sowa
2014-01-02 5:48 ` chenweilong
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-31 3:57 UTC (permalink / raw)
To: David Miller; +Cc: gaofeng, kumaran.4353, netdev, chenweilong
On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Sun, 16 Jun 2013 11:14:30 +0800
>
> > If we disable all of the net interfaces, and enable
> > un-lo interface before lo interface, we already allocated
> > the addrconf dst in ipv6_add_addr. So we shouldn't allocate
> > it again when we enable lo interface.
> >
> > Otherwise the message below will be triggered.
> > unregister_netdevice: waiting for sit1 to become free. Usage count = 1
> >
> > This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> > "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> >
> > Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>
> This is the second such regression added by that commit :-/
>
> Applied and queue up for -stable, thanks.
Hmm, and this change also has a regression and breaks the original fix. :/
https://bugzilla.kernel.org/show_bug.cgi?id=67951
I tried to track it down but it seems pretty complicated. Maybe we have to
special-case the take-down of the loopback device.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2013-12-31 3:57 ` Hannes Frederic Sowa
@ 2014-01-02 5:48 ` chenweilong
2014-01-02 6:03 ` Hannes Frederic Sowa
2014-01-02 6:54 ` Hannes Frederic Sowa
0 siblings, 2 replies; 19+ messages in thread
From: chenweilong @ 2014-01-02 5:48 UTC (permalink / raw)
To: David Miller, gaofeng, kumaran.4353, netdev
On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
> On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
>> From: Gao feng <gaofeng@cn.fujitsu.com>
>> Date: Sun, 16 Jun 2013 11:14:30 +0800
>>
>>> If we disable all of the net interfaces, and enable
>>> un-lo interface before lo interface, we already allocated
>>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
>>> it again when we enable lo interface.
>>>
>>> Otherwise the message below will be triggered.
>>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
>>>
>>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>>>
>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>
>> This is the second such regression added by that commit :-/
>>
>> Applied and queue up for -stable, thanks.
>
> Hmm, and this change also has a regression and breaks the original fix. :/
>
> https://bugzilla.kernel.org/show_bug.cgi?id=67951
>
> I tried to track it down but it seems pretty complicated. Maybe we have to
> special-case the take-down of the loopback device.
>
>
>
Hi,
When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
but IPv4 connection were still OK.
Is it designed like that or a bug?
Thanks
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 5:48 ` chenweilong
@ 2014-01-02 6:03 ` Hannes Frederic Sowa
2014-01-02 8:13 ` chenweilong
2014-01-02 6:54 ` Hannes Frederic Sowa
1 sibling, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-02 6:03 UTC (permalink / raw)
To: chenweilong; +Cc: David Miller, gaofeng, kumaran.4353, netdev
On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
> > On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
> >> From: Gao feng <gaofeng@cn.fujitsu.com>
> >> Date: Sun, 16 Jun 2013 11:14:30 +0800
> >>
> >>> If we disable all of the net interfaces, and enable
> >>> un-lo interface before lo interface, we already allocated
> >>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
> >>> it again when we enable lo interface.
> >>>
> >>> Otherwise the message below will be triggered.
> >>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
> >>>
> >>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> >>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> >>>
> >>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >>
> >> This is the second such regression added by that commit :-/
> >>
> >> Applied and queue up for -stable, thanks.
> >
> > Hmm, and this change also has a regression and breaks the original fix. :/
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=67951
> >
> > I tried to track it down but it seems pretty complicated. Maybe we have to
> > special-case the take-down of the loopback device.
> >
> >
> >
>
> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
> but IPv4 connection were still OK.
>
> Is it designed like that or a bug?
It is designed this way.
Greetings,
Hannes
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 5:48 ` chenweilong
2014-01-02 6:03 ` Hannes Frederic Sowa
@ 2014-01-02 6:54 ` Hannes Frederic Sowa
2014-01-02 7:58 ` chenweilong
1 sibling, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-02 6:54 UTC (permalink / raw)
To: chenweilong; +Cc: David Miller, gaofeng, kumaran.4353, netdev
On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
> > On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
> >> From: Gao feng <gaofeng@cn.fujitsu.com>
> >> Date: Sun, 16 Jun 2013 11:14:30 +0800
> >>
> >>> If we disable all of the net interfaces, and enable
> >>> un-lo interface before lo interface, we already allocated
> >>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
> >>> it again when we enable lo interface.
> >>>
> >>> Otherwise the message below will be triggered.
> >>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
> >>>
> >>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> >>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> >>>
> >>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >>
> >> This is the second such regression added by that commit :-/
> >>
> >> Applied and queue up for -stable, thanks.
> >
> > Hmm, and this change also has a regression and breaks the original fix. :/
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=67951
> >
> > I tried to track it down but it seems pretty complicated. Maybe we have to
> > special-case the take-down of the loopback device.
> >
> >
> >
>
> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
> but IPv4 connection were still OK.
>
> Is it designed like that or a bug?
This seems to solve the loopback up/down problem, but there are still
some issues with up/down of interfaces and routing table interactions.
We enable routes over interfaces when interface is actually down and
kick manually specified on-link routes when we actually should try to
keep them and just disable them.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c16345..61d752a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2629,8 +2629,10 @@ static void init_loopback(struct net_device *dev)
if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE))
continue;
- if (sp_ifa->rt)
+ if (sp_ifa->rt) {
+ ip6_ins_rt(sp_ifa->rt);
continue;
+ }
sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, false);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 6:54 ` Hannes Frederic Sowa
@ 2014-01-02 7:58 ` chenweilong
2014-01-02 8:23 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: chenweilong @ 2014-01-02 7:58 UTC (permalink / raw)
To: David Miller, gaofeng, kumaran.4353, netdev, Hannes Frederic Sowa
On 2014/1/2 14:54, Hannes Frederic Sowa wrote:
> On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
>> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
>>> On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
>>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>>> Date: Sun, 16 Jun 2013 11:14:30 +0800
>>>>
>>>>> If we disable all of the net interfaces, and enable
>>>>> un-lo interface before lo interface, we already allocated
>>>>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
>>>>> it again when we enable lo interface.
>>>>>
>>>>> Otherwise the message below will be triggered.
>>>>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
>>>>>
>>>>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>>>>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>>>>>
>>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>>
>>>> This is the second such regression added by that commit :-/
>>>>
>>>> Applied and queue up for -stable, thanks.
>>>
>>> Hmm, and this change also has a regression and breaks the original fix. :/
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
>>>
>>> I tried to track it down but it seems pretty complicated. Maybe we have to
>>> special-case the take-down of the loopback device.
>>>
>>>
>>>
>>
>> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
>> but IPv4 connection were still OK.
>>
>> Is it designed like that or a bug?
>
> This seems to solve the loopback up/down problem, but there are still
> some issues with up/down of interfaces and routing table interactions.
>
> We enable routes over interfaces when interface is actually down and
> kick manually specified on-link routes when we actually should try to
> keep them and just disable them.
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 6c16345..61d752a 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2629,8 +2629,10 @@ static void init_loopback(struct net_device *dev)
> if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE))
> continue;
>
> - if (sp_ifa->rt)
> + if (sp_ifa->rt) {
> + ip6_ins_rt(sp_ifa->rt);
> continue;
> + }
>
> sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, false);
>
>
> .
>
I test the patch,it has the problem Gao feng reported.
How about this:
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d5fa5b8..5e2db6e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2609,10 +2609,13 @@ 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, false);
/* Failure cases are ignored */
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 6:03 ` Hannes Frederic Sowa
@ 2014-01-02 8:13 ` chenweilong
2014-01-02 8:32 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: chenweilong @ 2014-01-02 8:13 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
On 2014/1/2 14:03, Hannes Frederic Sowa wrote:
> On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
>> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
>>> On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
>>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>>> Date: Sun, 16 Jun 2013 11:14:30 +0800
>>>>
>>>>> If we disable all of the net interfaces, and enable
>>>>> un-lo interface before lo interface, we already allocated
>>>>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
>>>>> it again when we enable lo interface.
>>>>>
>>>>> Otherwise the message below will be triggered.
>>>>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
>>>>>
>>>>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>>>>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>>>>>
>>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>>
>>>> This is the second such regression added by that commit :-/
>>>>
>>>> Applied and queue up for -stable, thanks.
>>>
>>> Hmm, and this change also has a regression and breaks the original fix. :/
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
>>>
>>> I tried to track it down but it seems pretty complicated. Maybe we have to
>>> special-case the take-down of the loopback device.
>>>
>>>
>>>
>>
>> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
>> but IPv4 connection were still OK.
>>
>> Is it designed like that or a bug?
>
> It is designed this way.
>
> Greetings,
>
> Hannes
>
>
> .
>
Can you give more explanation about this please?
Thanks!
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 7:58 ` chenweilong
@ 2014-01-02 8:23 ` Hannes Frederic Sowa
2014-01-02 9:33 ` chenweilong
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-02 8:23 UTC (permalink / raw)
To: chenweilong; +Cc: David Miller, gaofeng, kumaran.4353, netdev
On Thu, Jan 02, 2014 at 03:58:22PM +0800, chenweilong wrote:
> On 2014/1/2 14:54, Hannes Frederic Sowa wrote:
> > On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
> >> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
> >>> On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
> >>>> From: Gao feng <gaofeng@cn.fujitsu.com>
> >>>> Date: Sun, 16 Jun 2013 11:14:30 +0800
> >>>>
> >>>>> If we disable all of the net interfaces, and enable
> >>>>> un-lo interface before lo interface, we already allocated
> >>>>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
> >>>>> it again when we enable lo interface.
> >>>>>
> >>>>> Otherwise the message below will be triggered.
> >>>>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
> >>>>>
> >>>>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> >>>>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> >>>>>
> >>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> >>>>
> >>>> This is the second such regression added by that commit :-/
> >>>>
> >>>> Applied and queue up for -stable, thanks.
> >>>
> >>> Hmm, and this change also has a regression and breaks the original fix. :/
> >>>
> >>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
> >>>
> >>> I tried to track it down but it seems pretty complicated. Maybe we have to
> >>> special-case the take-down of the loopback device.
> >>>
> >>>
> >>>
> >>
> >> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
> >> but IPv4 connection were still OK.
> >>
> >> Is it designed like that or a bug?
> >
> > This seems to solve the loopback up/down problem, but there are still
> > some issues with up/down of interfaces and routing table interactions.
> >
> > We enable routes over interfaces when interface is actually down and
> > kick manually specified on-link routes when we actually should try to
> > keep them and just disable them.
> >
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 6c16345..61d752a 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -2629,8 +2629,10 @@ static void init_loopback(struct net_device *dev)
> > if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE))
> > continue;
> >
> > - if (sp_ifa->rt)
> > + if (sp_ifa->rt) {
> > + ip6_ins_rt(sp_ifa->rt);
> > continue;
> > + }
> >
> > sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, false);
> >
> >
> > .
> >
> I test the patch,it has the problem Gao feng reported.
Thanks for testing. I wonder why.
> How about this:
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index d5fa5b8..5e2db6e 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2609,10 +2609,13 @@ 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);
> + }
>
It could work, but it looks like a band-aid for another problem to me. I am
not sure if it is in init_loopback, yet.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 8:13 ` chenweilong
@ 2014-01-02 8:32 ` Hannes Frederic Sowa
2014-01-02 9:08 ` chenweilong
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-02 8:32 UTC (permalink / raw)
To: chenweilong; +Cc: netdev
On Thu, Jan 02, 2014 at 04:13:40PM +0800, chenweilong wrote:
> Can you give more explanation about this please?
If you look at the routing table for local addresses (ip -6 route list table
local), you see that every address which is RTF_LOCAL (and thus passes up the
packets via ip6_input) has as its device the loopback device.
In case you shutdown loopback you clear all those routes, thus incoming
packets won't find the corresponding rt6_info which directs the packets to
passed up the stack.
I guess it was done to satisfy the scoped architecture of IPv6.
Does this help you?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 8:32 ` Hannes Frederic Sowa
@ 2014-01-02 9:08 ` chenweilong
0 siblings, 0 replies; 19+ messages in thread
From: chenweilong @ 2014-01-02 9:08 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
On 2014/1/2 16:32, Hannes Frederic Sowa wrote:
> On Thu, Jan 02, 2014 at 04:13:40PM +0800, chenweilong wrote:
>> Can you give more explanation about this please?
>
> If you look at the routing table for local addresses (ip -6 route list table
> local), you see that every address which is RTF_LOCAL (and thus passes up the
> packets via ip6_input) has as its device the loopback device.
>
> In case you shutdown loopback you clear all those routes, thus incoming
> packets won't find the corresponding rt6_info which directs the packets to
> passed up the stack.
>
> I guess it was done to satisfy the scoped architecture of IPv6.
>
> Does this help you?
>
>
>
Yes,thanks very much!
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 8:23 ` Hannes Frederic Sowa
@ 2014-01-02 9:33 ` chenweilong
2014-01-03 6:53 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: chenweilong @ 2014-01-02 9:33 UTC (permalink / raw)
To: David Miller, gaofeng, kumaran.4353, netdev
On 2014/1/2 16:23, Hannes Frederic Sowa wrote:
> On Thu, Jan 02, 2014 at 03:58:22PM +0800, chenweilong wrote:
>> On 2014/1/2 14:54, Hannes Frederic Sowa wrote:
>>> On Thu, Jan 02, 2014 at 01:48:46PM +0800, chenweilong wrote:
>>>> On 2013/12/31 11:57, Hannes Frederic Sowa wrote:
>>>>> On Wed, Jun 19, 2013 at 11:05:32PM -0700, David Miller wrote:
>>>>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>>>>> Date: Sun, 16 Jun 2013 11:14:30 +0800
>>>>>>
>>>>>>> If we disable all of the net interfaces, and enable
>>>>>>> un-lo interface before lo interface, we already allocated
>>>>>>> the addrconf dst in ipv6_add_addr. So we shouldn't allocate
>>>>>>> it again when we enable lo interface.
>>>>>>>
>>>>>>> Otherwise the message below will be triggered.
>>>>>>> unregister_netdevice: waiting for sit1 to become free. Usage count = 1
>>>>>>>
>>>>>>> This problem is introduced by commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>>>>>>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>>>>>>>
>>>>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>>>>
>>>>>> This is the second such regression added by that commit :-/
>>>>>>
>>>>>> Applied and queue up for -stable, thanks.
>>>>>
>>>>> Hmm, and this change also has a regression and breaks the original fix. :/
>>>>>
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
>>>>>
>>>>> I tried to track it down but it seems pretty complicated. Maybe we have to
>>>>> special-case the take-down of the loopback device.
>>>>>
>>>>>
>>>>>
>>>>
>>>> When I did the tests,If 'ifconfig lo down',all IPv6 connection broken,
>>>> but IPv4 connection were still OK.
>>>>
>>>> Is it designed like that or a bug?
>>>
>>> This seems to solve the loopback up/down problem, but there are still
>>> some issues with up/down of interfaces and routing table interactions.
>>>
>>> We enable routes over interfaces when interface is actually down and
>>> kick manually specified on-link routes when we actually should try to
>>> keep them and just disable them.
>>>
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index 6c16345..61d752a 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -2629,8 +2629,10 @@ static void init_loopback(struct net_device *dev)
>>> if (sp_ifa->flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE))
>>> continue;
>>>
>>> - if (sp_ifa->rt)
>>> + if (sp_ifa->rt) {
>>> + ip6_ins_rt(sp_ifa->rt);
>>> continue;
>>> + }
>>>
>>> sp_rt = addrconf_dst_alloc(idev, &sp_ifa->addr, false);
>>>
>>>
>>> .
>>>
>> I test the patch,it has the problem Gao feng reported.
>
> Thanks for testing. I wonder why.
>
>> How about this:
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index d5fa5b8..5e2db6e 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2609,10 +2609,13 @@ 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);
>> + }
>>
>
> It could work, but it looks like a band-aid for another problem to me. I am
> not sure if it is in init_loopback, yet.
>
>
> .
>
Fix some sapce errors in my post.
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);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-02 9:33 ` chenweilong
@ 2014-01-03 6:53 ` Hannes Frederic Sowa
2014-01-08 7:50 ` Gao feng
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-03 6:53 UTC (permalink / raw)
To: chenweilong; +Cc: David Miller, gaofeng, kumaran.4353, netdev
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'll try to just disable routes without removing them at all when we set an
interface to down at the weekend.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-03 6:53 ` Hannes Frederic Sowa
@ 2014-01-08 7:50 ` Gao feng
2014-01-08 8:05 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: Gao feng @ 2014-01-08 7:50 UTC (permalink / raw)
To: chenweilong, David Miller, kumaran.4353, netdev
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);
>
> 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.
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-08 7:50 ` Gao feng
@ 2014-01-08 8:05 ` Hannes Frederic Sowa
2014-01-08 8:42 ` Gao feng
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-08 8:05 UTC (permalink / raw)
To: Gao feng; +Cc: chenweilong, David Miller, kumaran.4353, netdev
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).
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.
Thanks,
Hannes
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-08 8:05 ` Hannes Frederic Sowa
@ 2014-01-08 8:42 ` Gao feng
2014-01-08 8:55 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: Gao feng @ 2014-01-08 8:42 UTC (permalink / raw)
To: chenweilong, David Miller, kumaran.4353, netdev
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...
Thanks!
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-08 8:42 ` Gao feng
@ 2014-01-08 8:55 ` Hannes Frederic Sowa
2014-01-17 2:02 ` chenweilong
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-08 8:55 UTC (permalink / raw)
To: Gao feng; +Cc: chenweilong, David Miller, kumaran.4353, netdev
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-08 8:55 ` Hannes Frederic Sowa
@ 2014-01-17 2:02 ` chenweilong
2014-01-17 4:09 ` Hannes Frederic Sowa
0 siblings, 1 reply; 19+ messages in thread
From: chenweilong @ 2014-01-17 2:02 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Gao feng, David Miller, kumaran.4353, netdev
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?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo
2014-01-17 2:02 ` chenweilong
@ 2014-01-17 4:09 ` Hannes Frederic Sowa
0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2014-01-17 4:09 UTC (permalink / raw)
To: chenweilong; +Cc: Gao feng, David Miller, kumaran.4353, netdev
On Fri, Jan 17, 2014 at 10:02:17AM +0800, chenweilong wrote:
> It's quite a long time, How's your patch going on?
I apologize, it has gotten a bit off my radar lately, but I have the branch
still around and will resurrect it asap. I think it is a problem which really
should be solved.
Thanks,
Hannes
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2014-01-17 4:09 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-16 3:14 [PATCH] ipv6: don't call addrconf_dst_alloc again when enable lo Gao feng
2013-06-20 6:05 ` David Miller
2013-12-31 3:57 ` Hannes Frederic Sowa
2014-01-02 5:48 ` chenweilong
2014-01-02 6:03 ` Hannes Frederic Sowa
2014-01-02 8:13 ` chenweilong
2014-01-02 8:32 ` Hannes Frederic Sowa
2014-01-02 9:08 ` chenweilong
2014-01-02 6:54 ` Hannes Frederic Sowa
2014-01-02 7:58 ` chenweilong
2014-01-02 8:23 ` Hannes Frederic Sowa
2014-01-02 9:33 ` chenweilong
2014-01-03 6:53 ` Hannes Frederic Sowa
2014-01-08 7:50 ` Gao feng
2014-01-08 8:05 ` Hannes Frederic Sowa
2014-01-08 8:42 ` Gao feng
2014-01-08 8:55 ` Hannes Frederic Sowa
2014-01-17 2:02 ` chenweilong
2014-01-17 4:09 ` Hannes Frederic Sowa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).