diff mbox

ipv6: reallocate addrconf router for ipv6 address when lo device up

Message ID 1390460277-23451-1-git-send-email-gaofeng@cn.fujitsu.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Gao feng Jan. 23, 2014, 6:57 a.m. UTC
commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
"net IPv6 : Fix broken IPv6 routing table after loopback down-up"
allocates addrconf router for ipv6 address when lo device up.
but commit a881ae1f625c599b460cc8f8a7fcb1c438f699ad
"ipv6:don't call addrconf_dst_alloc again when enable lo" breaks
this behavior.

Since the addrconf router is moved to the garbage list when
lo device down, we should delete this router and rellocate
a new one for ipv6 address when lo device up.

This patch solves bug 67951 on bugzilla
https://bugzilla.kernel.org/show_bug.cgi?id=67951

CC: Sabrina Dubroca <sd@queasysnail.net>
CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
Reported-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/ipv6/addrconf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Hannes Frederic Sowa Jan. 23, 2014, 9:03 p.m. UTC | #1
On Thu, Jan 23, 2014 at 02:57:57PM +0800, Gao feng wrote:
> commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> allocates addrconf router for ipv6 address when lo device up.
> but commit a881ae1f625c599b460cc8f8a7fcb1c438f699ad
> "ipv6:don't call addrconf_dst_alloc again when enable lo" breaks
> this behavior.
> 
> Since the addrconf router is moved to the garbage list when
> lo device down, we should delete this router and rellocate
> a new one for ipv6 address when lo device up.
> 
> This patch solves bug 67951 on bugzilla
> https://bugzilla.kernel.org/show_bug.cgi?id=67951
> 
> CC: Sabrina Dubroca <sd@queasysnail.net>
> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Reported-by: Weilong Chen <chenweilong@huawei.com>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/ipv6/addrconf.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 4b6b720..6eecd9d 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2611,8 +2611,17 @@ 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 a new router for ifa.
> +				 */
> +				if (sp_ifa->rt->dst.obsolete > 0) {
> +					ip6_del_rt(sp_ifa->rt);

We should not delete dst.obsolete > 0 routes. I think a ip6_put_rt is just
fine here, no?

Greetings,

  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
Sergei Shtylyov Jan. 23, 2014, 9:13 p.m. UTC | #2
Hello.

On 23-01-2014 10:57, Gao feng wrote:

> commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
> allocates addrconf router for ipv6 address when lo device up.
> but commit a881ae1f625c599b460cc8f8a7fcb1c438f699ad
> "ipv6:don't call addrconf_dst_alloc again when enable lo" breaks
> this behavior.

> Since the addrconf router is moved to the garbage list when
> lo device down, we should delete this router and rellocate
> a new one for ipv6 address when lo device up.

> This patch solves bug 67951 on bugzilla
> https://bugzilla.kernel.org/show_bug.cgi?id=67951

> CC: Sabrina Dubroca <sd@queasysnail.net>
> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Reported-by: Weilong Chen <chenweilong@huawei.com>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>   net/ipv6/addrconf.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)

> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 4b6b720..6eecd9d 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2611,8 +2611,17 @@ 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 a new router for ifa.
> +				 */
> +				if (sp_ifa->rt->dst.obsolete > 0) {
> +					ip6_del_rt(sp_ifa->rt);
> +					sp_ifa->rt = NULL;
> +				} else
> +					continue;

    *else* arm should have {} when the other arm of *if* statement has it -- 
see Documentation/CodingStyle.

WBR, Sergei

--
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
Gao feng Jan. 24, 2014, 1:14 a.m. UTC | #3
On 01/24/2014 05:03 AM, Hannes Frederic Sowa wrote:
> On Thu, Jan 23, 2014 at 02:57:57PM +0800, Gao feng wrote:
>> commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>> allocates addrconf router for ipv6 address when lo device up.
>> but commit a881ae1f625c599b460cc8f8a7fcb1c438f699ad
>> "ipv6:don't call addrconf_dst_alloc again when enable lo" breaks
>> this behavior.
>>
>> Since the addrconf router is moved to the garbage list when
>> lo device down, we should delete this router and rellocate
>> a new one for ipv6 address when lo device up.
>>
>> This patch solves bug 67951 on bugzilla
>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
>>
>> CC: Sabrina Dubroca <sd@queasysnail.net>
>> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Reported-by: Weilong Chen <chenweilong@huawei.com>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  net/ipv6/addrconf.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 4b6b720..6eecd9d 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2611,8 +2611,17 @@ 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 a new router for ifa.
>> +				 */
>> +				if (sp_ifa->rt->dst.obsolete > 0) {
>> +					ip6_del_rt(sp_ifa->rt);
> 
> We should not delete dst.obsolete > 0 routes. I think a ip6_put_rt is just
> fine here, no?
> 


Yes, definitely. this route has already been delete from route tree in fib6_clean_all.
no need delete it again.

Thanks, I will post v2 patch.
--
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
Gao feng Jan. 24, 2014, 1:14 a.m. UTC | #4
On 01/24/2014 05:13 AM, Sergei Shtylyov wrote:
> Hello.
> 
> On 23-01-2014 10:57, Gao feng wrote:
> 
>> commit 25fb6ca4ed9cad72f14f61629b68dc03c0d9713f
>> "net IPv6 : Fix broken IPv6 routing table after loopback down-up"
>> allocates addrconf router for ipv6 address when lo device up.
>> but commit a881ae1f625c599b460cc8f8a7fcb1c438f699ad
>> "ipv6:don't call addrconf_dst_alloc again when enable lo" breaks
>> this behavior.
> 
>> Since the addrconf router is moved to the garbage list when
>> lo device down, we should delete this router and rellocate
>> a new one for ipv6 address when lo device up.
> 
>> This patch solves bug 67951 on bugzilla
>> https://bugzilla.kernel.org/show_bug.cgi?id=67951
> 
>> CC: Sabrina Dubroca <sd@queasysnail.net>
>> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Reported-by: Weilong Chen <chenweilong@huawei.com>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>   net/ipv6/addrconf.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 4b6b720..6eecd9d 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2611,8 +2611,17 @@ 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 a new router for ifa.
>> +                 */
>> +                if (sp_ifa->rt->dst.obsolete > 0) {
>> +                    ip6_del_rt(sp_ifa->rt);
>> +                    sp_ifa->rt = NULL;
>> +                } else
>> +                    continue;
> 
>    *else* arm should have {} when the other arm of *if* statement has it -- see Documentation/CodingStyle.
> 

Thanks!

will add in v2.


--
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
diff mbox

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4b6b720..6eecd9d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2611,8 +2611,17 @@  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 a 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);