netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
@ 2013-12-09  5:54 Asano, Yasushi
  2013-12-09 23:47 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 21+ messages in thread
From: Asano, Yasushi @ 2013-12-09  5:54 UTC (permalink / raw)
  To: netdev

from: Yasushi Asano  <yasushi.asano@jp.fujitsu.com>

There is a problem when setting the lifetime of an IPv6 address.
When I set preferred_lft to a value not zero or infinity, while valid_lft is infinity(0xffffffff)
preferred lifetime is set to forever and does not update.
Therefore preferred lifetime never becomes deprecated.

I think valid lifetime and preferred lifetime should be set independently,
even if valid lifetime is infinity, preferred lifetime must expire correctly (meaning it must eventually become deprecated)

I made a patch for 3.12 stable to solve the problem.

< console log before patching >
using the ip addr command to verify the problem
------------------------------------------------------------------------------------------------------------
# # ip addr add 2002:100:10:1::100/64 dev eth1 valid_lft 0xffffffff preferred_lft 20
# # ip addr show eth1

3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
link/ether 00:04:9f:02:00:5e brd ff:ff:ff:ff:ff:ff
inet 192.168.0.101/24 brd 192.168.0.255 scope global eth1
inet6 2002:100:10:1::100/64 scope global       <----- The address doesn't become deprecated after 20seconds.
valid_lft forever preferred_lft forever        <----- preferred_lft becomes forever instead of 20seconds.
inet6 fe80::204:9fff:fe02:5e/64 scope link            Therefore lifetime(preferred_lft) is not updating.
valid_lft forever preferred_lft forever
------------------------------------------------------------------------------------------------------------

< console log after patching >
using the ip addr command to verify it runs correctly
------------------------------------------------------------------------------------------------------------
# ifconfig eth1 192.168.0.101 netmask 255.255.255.0
# ip addr add 2002:100:10:1::100/64 dev eth1 valid_lft 0xffffffff preferred_lft 20
# ip addr show eth1
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
    link/ether 00:04:9f:02:00:5e brd ff:ff:ff:ff:ff:ff
    inet 192.168.0.101/24 brd 192.168.0.255 scope global eth1
    inet6 2002:100:10:1::100/64 scope global dynamic   <------------- "global dynamic"
       valid_lft forever preferred_lft 16sec           <--------------it begins counting down from 20seconds
    inet6 fe80::204:9fff:fe02:5e/64 scope link
       valid_lft forever preferred_lft forever
# ip addr show eth1
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
    link/ether 00:04:9f:02:00:5e brd ff:ff:ff:ff:ff:ff
    inet 192.168.0.101/24 brd 192.168.0.255 scope global eth1
    inet6 2002:100:10:1::100/64 scope global dynamic   <------------ "global dynamic"
       valid_lft forever preferred_lft 11sec           <------------ it continues counting down. 
    inet6 fe80::204:9fff:fe02:5e/64 scope link
       valid_lft forever preferred_lft forever
# ip addr show eth1
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
    link/ether 00:04:9f:02:00:5e brd ff:ff:ff:ff:ff:ff
    inet 192.168.0.101/24 brd 192.168.0.255 scope global eth1
    inet6 2002:100:10:1::100/64 scope global deprecated dynamic <- "deprecated dynamic"
       valid_lft forever preferred_lft 0sec            <----------- it expired because it became zero seconds and it changed to "deprecated".
    inet6 fe80::204:9fff:fe02:5e/64 scope link
       valid_lft forever preferred_lft forever
#

------------------------------------------------------------------------------------------------------------
net/ipv6/addrconf.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)


diff -buprNE a/net/ipv6/addrconf.c b-1/net/ipv6/addrconf.c
--- a/net/ipv6/addrconf.c	2012-06-13 09:40:27.000000000 +0900
+++ b-1/net/ipv6/addrconf.c	2013-11-08 12:00:59.107840672 +0900
@@ -755,6 +755,7 @@ static void ipv6_del_addr(struct inet6_i
 					if (!onlink)
 						onlink = -1;
 
+					if (ifp->valid_lft != INFINITY_LIFE_TIME) {
 					spin_lock(&ifa->lock);
 
 					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
@@ -771,6 +772,7 @@ static void ipv6_del_addr(struct inet6_i
 			}
 		}
 	}
+	}
 	write_unlock_bh(&idev->lock);
 
 	addrconf_del_timer(ifp);
@@ -2137,7 +2139,6 @@ static int inet6_addr_add(struct net *ne
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
@@ -3177,9 +3178,11 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
+
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
 				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
 					next = ifp->tstamp + ifp->valid_lft * HZ;
-
+				}
 				spin_unlock(&ifp->lock);
 
 				if (deprecate) {
@@ -3309,7 +3312,6 @@ static int inet6_addr_modify(struct inet
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-09  5:54 [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity Asano, Yasushi
@ 2013-12-09 23:47 ` Hannes Frederic Sowa
  2013-12-12  0:58   ` Asano, Yasushi
  0 siblings, 1 reply; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-09 23:47 UTC (permalink / raw)
  To: Asano, Yasushi; +Cc: netdev

On Mon, Dec 09, 2013 at 05:54:37AM +0000, Asano, Yasushi wrote:
> from: Yasushi Asano  <yasushi.asano@jp.fujitsu.com>
> 
> There is a problem when setting the lifetime of an IPv6 address.
> When I set preferred_lft to a value not zero or infinity, while valid_lft is infinity(0xffffffff)
> preferred lifetime is set to forever and does not update.
> Therefore preferred lifetime never becomes deprecated.
> 
> I think valid lifetime and preferred lifetime should be set independently,
> even if valid lifetime is infinity, preferred lifetime must expire correctly (meaning it must eventually become deprecated)
> 
> I made a patch for 3.12 stable to solve the problem.

This indeed could be improved. Thanks for the patch. But you should base
it on net-next so it can be applied and please clean up the warnings
and errors if you run the patch through ./scripts/checkpatch --strict.

Thank you,

  Hannes

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-09 23:47 ` Hannes Frederic Sowa
@ 2013-12-12  0:58   ` Asano, Yasushi
  2013-12-12  1:03     ` Hannes Frederic Sowa
  0 siblings, 1 reply; 21+ messages in thread
From: Asano, Yasushi @ 2013-12-12  0:58 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev

> -----Original Message-----
> From: netdev-owner@vger.kernel.org
> [mailto:netdev-owner@vger.kernel.org] On Behalf Of Hannes Frederic Sowa
> Sent: Tuesday, December 10, 2013 8:47 AM
> To: Asano, Yasushi
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime
> state-changing behavior while valid_lft is infinity
> Importance: High
> 
> On Mon, Dec 09, 2013 at 05:54:37AM +0000, Asano, Yasushi wrote:
> > from: Yasushi Asano  <yasushi.asano@jp.fujitsu.com>
> >
> > There is a problem when setting the lifetime of an IPv6 address.
> > When I set preferred_lft to a value not zero or infinity, while valid_lft
> is infinity(0xffffffff)
> > preferred lifetime is set to forever and does not update.
> > Therefore preferred lifetime never becomes deprecated.
> >
> > I think valid lifetime and preferred lifetime should be set independently,
> > even if valid lifetime is infinity, preferred lifetime must expire
> correctly (meaning it must eventually become deprecated)
> >
> > I made a patch for 3.12 stable to solve the problem.
> 
> This indeed could be improved. Thanks for the patch. But you should base
> it on net-next so it can be applied and please clean up the warnings
> and errors if you run the patch through ./scripts/checkpatch --strict.
> 
> Thank you,
> 
>   Hannes
> 
I made a patch based on net-next and emailed ML using git send-email last night.

Subject: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
Date: Wed, 11 Dec 2013 19:57:32 +0900
Message-ID: <1386759452-22159-1-git-send-email-yasushi.asano@jp.fujitsu.com>
X-Mailer: git-send-email 1.7.12.4

Thank you for your time and assistance regarding this matter.
Yasushi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12  0:58   ` Asano, Yasushi
@ 2013-12-12  1:03     ` Hannes Frederic Sowa
  2013-12-12 10:01       ` yazzep
                         ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-12  1:03 UTC (permalink / raw)
  To: Asano, Yasushi; +Cc: netdev

On Thu, Dec 12, 2013 at 12:58:10AM +0000, Asano, Yasushi wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org
> > [mailto:netdev-owner@vger.kernel.org] On Behalf Of Hannes Frederic Sowa
> > Sent: Tuesday, December 10, 2013 8:47 AM
> > To: Asano, Yasushi
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime
> > state-changing behavior while valid_lft is infinity
> > Importance: High
> > 
> > On Mon, Dec 09, 2013 at 05:54:37AM +0000, Asano, Yasushi wrote:
> > > from: Yasushi Asano  <yasushi.asano@jp.fujitsu.com>
> > >
> > > There is a problem when setting the lifetime of an IPv6 address.
> > > When I set preferred_lft to a value not zero or infinity, while valid_lft
> > is infinity(0xffffffff)
> > > preferred lifetime is set to forever and does not update.
> > > Therefore preferred lifetime never becomes deprecated.
> > >
> > > I think valid lifetime and preferred lifetime should be set independently,
> > > even if valid lifetime is infinity, preferred lifetime must expire
> > correctly (meaning it must eventually become deprecated)
> > >
> > > I made a patch for 3.12 stable to solve the problem.
> > 
> > This indeed could be improved. Thanks for the patch. But you should base
> > it on net-next so it can be applied and please clean up the warnings
> > and errors if you run the patch through ./scripts/checkpatch --strict.
> > 
> > Thank you,
> > 
> >   Hannes
> > 
> I made a patch based on net-next and emailed ML using git send-email last night.
> 
> Subject: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
> Date: Wed, 11 Dec 2013 19:57:32 +0900
> Message-ID: <1386759452-22159-1-git-send-email-yasushi.asano@jp.fujitsu.com>
> X-Mailer: git-send-email 1.7.12.4
> 
> Thank you for your time and assistance regarding this matter.
> Yasushi

Huch? I have not seen anything nor has patchworks:
http://patchwork.ozlabs.org/project/netdev/list/

Greetings,

  Hannes

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12  1:03     ` Hannes Frederic Sowa
@ 2013-12-12 10:01       ` yazzep
  2013-12-12 17:19         ` David Miller
  2013-12-12 10:15       ` yazzep
  2013-12-12 10:46       ` Asano, Yasushi
  2 siblings, 1 reply; 21+ messages in thread
From: yazzep @ 2013-12-12 10:01 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

 Fixed a problem with setting the lifetime of an IPv6
 address. When setting preferred_lft to a value not zero or
 infinity, while valid_lft is infinity(0xffffffff) preferred
 lifetime is set to forever and does not update. Therefore
 preferred lifetime never becomes deprecated. valid lifetime
 and preferred lifetime should be set independently, even if
 valid lifetime is infinity, preferred lifetime must expire
 correctly (meaning it must eventually become deprecated)


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3c3425e..17b4097 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -948,18 +948,22 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 					if (!onlink)
 						onlink = -1;
 
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
+					if (ifp->valid_lft !=
+						INFINITY_LIFE_TIME) {
+						spin_lock(&ifa->lock);
+
+						lifetime = addrconf_timeout_fixup(
+								ifa->valid_lft,	HZ);
+						/*
+						 * Note: Because this address is
+						 * not permanent, lifetime <
+						 * LONG_MAX / HZ here.
+						 */
+						if (time_before(expires,
+								ifa->tstamp + lifetime * HZ))
+							expires = ifa->tstamp +	lifetime * HZ;
+						spin_unlock(&ifa->lock);
+					}
 				}
 			}
 		}
@@ -2415,7 +2419,6 @@ static int inet6_addr_add(struct net *net, int ifindex,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
@@ -3497,8 +3500,12 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
-					next = ifp->tstamp + ifp->valid_lft * HZ;
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
+					if (time_before(ifp->tstamp +
+							ifp->valid_lft * HZ, next))
+						next = ifp->tstamp +
+							ifp->valid_lft * HZ;
+				}
 
 				spin_unlock(&ifp->lock);
 
@@ -3635,7 +3642,6 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12  1:03     ` Hannes Frederic Sowa
  2013-12-12 10:01       ` yazzep
@ 2013-12-12 10:15       ` yazzep
  2013-12-12 14:06         ` Sergei Shtylyov
  2013-12-13 17:33         ` yazzep
  2013-12-12 10:46       ` Asano, Yasushi
  2 siblings, 2 replies; 21+ messages in thread
From: yazzep @ 2013-12-12 10:15 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

 Fixed a problem with setting the lifetime of an IPv6
 address. When setting preferred_lft to a value not zero or
 infinity, while valid_lft is infinity(0xffffffff) preferred
 lifetime is set to forever and does not update. Therefore
 preferred lifetime never becomes deprecated. valid lifetime
 and preferred lifetime should be set independently, even if
 valid lifetime is infinity, preferred lifetime must expire
 correctly (meaning it must eventually become deprecated)


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3c3425e..17b4097 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -948,18 +948,22 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 					if (!onlink)
 						onlink = -1;
 
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
+					if (ifp->valid_lft !=
+						INFINITY_LIFE_TIME) {
+						spin_lock(&ifa->lock);
+
+						lifetime = addrconf_timeout_fixup(
+								ifa->valid_lft,	HZ);
+						/*
+						 * Note: Because this address is
+						 * not permanent, lifetime <
+						 * LONG_MAX / HZ here.
+						 */
+						if (time_before(expires,
+								ifa->tstamp + lifetime * HZ))
+							expires = ifa->tstamp +	lifetime * HZ;
+						spin_unlock(&ifa->lock);
+					}
 				}
 			}
 		}
@@ -2415,7 +2419,6 @@ static int inet6_addr_add(struct net *net, int ifindex,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
@@ -3497,8 +3500,12 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
-					next = ifp->tstamp + ifp->valid_lft * HZ;
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
+					if (time_before(ifp->tstamp +
+							ifp->valid_lft * HZ, next))
+						next = ifp->tstamp +
+							ifp->valid_lft * HZ;
+				}
 
 				spin_unlock(&ifp->lock);
 
@@ -3635,7 +3642,6 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12  1:03     ` Hannes Frederic Sowa
  2013-12-12 10:01       ` yazzep
  2013-12-12 10:15       ` yazzep
@ 2013-12-12 10:46       ` Asano, Yasushi
  2 siblings, 0 replies; 21+ messages in thread
From: Asano, Yasushi @ 2013-12-12 10:46 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev

> -----Original Message-----
> From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
> Sent: Thursday, December 12, 2013 10:04 AM
> To: Asano, Yasushi/浅野 恭史
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime
> state-changing behavior while valid_lft is infinity
> Importance: High
> 
> On Thu, Dec 12, 2013 at 12:58:10AM +0000, Asano, Yasushi wrote:
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org
> > > [mailto:netdev-owner@vger.kernel.org] On Behalf Of Hannes Frederic Sowa
> > > Sent: Tuesday, December 10, 2013 8:47 AM
> > > To: Asano, Yasushi
> > > Cc: netdev@vger.kernel.org
> > > Subject: Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime
> > > state-changing behavior while valid_lft is infinity
> > > Importance: High
> > >
> > > On Mon, Dec 09, 2013 at 05:54:37AM +0000, Asano, Yasushi wrote:
> > > > from: Yasushi Asano  <yasushi.asano@jp.fujitsu.com>
> > > >
> > > > There is a problem when setting the lifetime of an IPv6 address.
> > > > When I set preferred_lft to a value not zero or infinity, while valid_lft
> > > is infinity(0xffffffff)
> > > > preferred lifetime is set to forever and does not update.
> > > > Therefore preferred lifetime never becomes deprecated.
> > > >
> > > > I think valid lifetime and preferred lifetime should be set
> independently,
> > > > even if valid lifetime is infinity, preferred lifetime must expire
> > > correctly (meaning it must eventually become deprecated)
> > > >
> > > > I made a patch for 3.12 stable to solve the problem.
> > >
> > > This indeed could be improved. Thanks for the patch. But you should
> base
> > > it on net-next so it can be applied and please clean up the warnings
> > > and errors if you run the patch through ./scripts/checkpatch --strict.
> > >
> > > Thank you,
> > >
> > >   Hannes
> > >
> > I made a patch based on net-next and emailed ML using git send-email last
> night.
> >
> > Subject: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing
> behavior while valid_lft is infinity
> > Date: Wed, 11 Dec 2013 19:57:32 +0900
> > Message-ID:
> <1386759452-22159-1-git-send-email-yasushi.asano@jp.fujitsu.com>
> > X-Mailer: git-send-email 1.7.12.4
> >
> > Thank you for your time and assistance regarding this matter.
> > Yasushi
> 
> Huch? I have not seen anything nor has patchworks:
> http://patchwork.ozlabs.org/project/netdev/list/
> 
> Greetings,
> 
>   Hannes

I'm sorry, I couldn't send an email correctly.
I tried it again. but I sent the e-mail twice by mistake. Sorry again.

I sent a patch based on  net-next

Subject: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
Date: Thu, 12 Dec 2013 19:15:47 +0900
Message-ID: <1386843347-2725-1-git-send-email-yazzep@gmail.com>

Thank you for your time and assistance regarding this matter.
Yasushi


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12 10:15       ` yazzep
@ 2013-12-12 14:06         ` Sergei Shtylyov
  2013-12-13 17:33         ` yazzep
  1 sibling, 0 replies; 21+ messages in thread
From: Sergei Shtylyov @ 2013-12-12 14:06 UTC (permalink / raw)
  To: yazzep, netdev; +Cc: Yasushi Asano

Hello.

On 12-12-2013 14:15, yazzep@gmail.com wrote:

> From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

>   Fixed a problem with setting the lifetime of an IPv6
>   address. When setting preferred_lft to a value not zero or
>   infinity, while valid_lft is infinity(0xffffffff) preferred
>   lifetime is set to forever and does not update. Therefore
>   preferred lifetime never becomes deprecated. valid lifetime
>   and preferred lifetime should be set independently, even if
>   valid lifetime is infinity, preferred lifetime must expire
>   correctly (meaning it must eventually become deprecated)


> Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> ---
>   net/ipv6/addrconf.c |   34 ++++++++++++++++++++--------------
>   1 files changed, 20 insertions(+), 14 deletions(-)

> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3c3425e..17b4097 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -948,18 +948,22 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
[...]
> +					if (ifp->valid_lft !=
> +						INFINITY_LIFE_TIME) {

    Please start the continuation line right under 'ifp', according to the 
networking coding style -- this ways it's easier on the eyes and don't mix 
with the following block.

> +						spin_lock(&ifa->lock);
> +
> +						lifetime = addrconf_timeout_fixup(
> +								ifa->valid_lft,	HZ);
> +						/*
> +						 * Note: Because this address is

    s/Because/because/, so that the grammar is correct.

> +						 * not permanent, lifetime <
> +						 * LONG_MAX / HZ here.
> +						 */

    Networking code assumes slightly other style of multi-line comments:

/* bla
  * bla
  */

WBR, Sergei

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12 10:01       ` yazzep
@ 2013-12-12 17:19         ` David Miller
  0 siblings, 0 replies; 21+ messages in thread
From: David Miller @ 2013-12-12 17:19 UTC (permalink / raw)
  To: yazzep; +Cc: netdev, yasushi.asano

From: yazzep@gmail.com
Date: Thu, 12 Dec 2013 19:01:40 +0900

> From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> 
>  Fixed a problem with setting the lifetime of an IPv6
>  address. When setting preferred_lft to a value not zero or
>  infinity, while valid_lft is infinity(0xffffffff) preferred
>  lifetime is set to forever and does not update. Therefore
>  preferred lifetime never becomes deprecated. valid lifetime
>  and preferred lifetime should be set independently, even if
>  valid lifetime is infinity, preferred lifetime must expire
>  correctly (meaning it must eventually become deprecated)

Please do not indent your entire commit message with a leading space.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-12 10:15       ` yazzep
  2013-12-12 14:06         ` Sergei Shtylyov
@ 2013-12-13 17:33         ` yazzep
  2013-12-14  9:19           ` Hannes Frederic Sowa
  1 sibling, 1 reply; 21+ messages in thread
From: yazzep @ 2013-12-13 17:33 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

Fixed a problem with setting the lifetime of an IPv6
address. When setting preferred_lft to a value not zero or
infinity, while valid_lft is infinity(0xffffffff) preferred
lifetime is set to forever and does not update. Therefore
preferred lifetime never becomes deprecated. valid lifetime
and preferred lifetime should be set independently, even if
valid lifetime is infinity, preferred lifetime must expire
correctly (meaning it must eventually become deprecated)


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3c3425e..00c135b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -948,18 +948,21 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 					if (!onlink)
 						onlink = -1;
 
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
+					if (ifp->valid_lft !=
+					    INFINITY_LIFE_TIME) {
+						spin_lock(&ifa->lock);
+
+						lifetime = addrconf_timeout_fixup(
+								ifa->valid_lft,	HZ);
+						/* Note: because this address is
+						 * not permanent, lifetime <
+						 * LONG_MAX / HZ here.
+						 */
+						if (time_before(expires,
+								ifa->tstamp + lifetime * HZ))
+							expires = ifa->tstamp +	lifetime * HZ;
+						spin_unlock(&ifa->lock);
+					}
 				}
 			}
 		}
@@ -2415,7 +2418,6 @@ static int inet6_addr_add(struct net *net, int ifindex,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
@@ -3497,8 +3499,12 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
-					next = ifp->tstamp + ifp->valid_lft * HZ;
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
+					if (time_before(ifp->tstamp +
+							ifp->valid_lft * HZ, next))
+						next = ifp->tstamp +
+							ifp->valid_lft * HZ;
+				}
 
 				spin_unlock(&ifp->lock);
 
@@ -3635,7 +3641,6 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	} else {
 		expires = 0;
 		flags = 0;
-		ifa_flags |= IFA_F_PERMANENT;
 	}
 
 	timeout = addrconf_timeout_fixup(prefered_lft, HZ);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-13 17:33         ` yazzep
@ 2013-12-14  9:19           ` Hannes Frederic Sowa
  2013-12-14  9:22             ` Hannes Frederic Sowa
                               ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-14  9:19 UTC (permalink / raw)
  To: yazzep; +Cc: netdev, Yasushi Asano

On Sat, Dec 14, 2013 at 02:33:48AM +0900, yazzep@gmail.com wrote:
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3c3425e..00c135b 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -948,18 +948,21 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>  					if (!onlink)
>  						onlink = -1;
>  
> -					spin_lock(&ifa->lock);
> -
> -					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
> -					/*
> -					 * Note: Because this address is
> -					 * not permanent, lifetime <
> -					 * LONG_MAX / HZ here.
> -					 */
> -					if (time_before(expires,
> -							ifa->tstamp + lifetime * HZ))
> -						expires = ifa->tstamp + lifetime * HZ;
> -					spin_unlock(&ifa->lock);
> +					if (ifp->valid_lft !=
> +					    INFINITY_LIFE_TIME) {
> +						spin_lock(&ifa->lock);
> +
> +						lifetime = addrconf_timeout_fixup(
> +								ifa->valid_lft,	HZ);
> +						/* Note: because this address is
> +						 * not permanent, lifetime <
> +						 * LONG_MAX / HZ here.
> +						 */
> +						if (time_before(expires,
> +								ifa->tstamp + lifetime * HZ))
> +							expires = ifa->tstamp +	lifetime * HZ;
> +						spin_unlock(&ifa->lock);
> +					}

Sorry, this does not make sense to me. Below you remove the IFA_F_PERMANENT
expression and here you change code depending only on that flag. Is this meant
as a shortcut? Please explain, maybe I am missing something.

>  				}
>  			}
>  		}
> @@ -2415,7 +2418,6 @@ static int inet6_addr_add(struct net *net, int ifindex,
>  	} else {
>  		expires = 0;
>  		flags = 0;
> -		ifa_flags |= IFA_F_PERMANENT;

I find this very suspicious. IFA_F_PERMANENT may only be removed if valid_lft
!= inifinity and preferred_lft != infinity if at all (see below).

We have to be very careful here with interactions of prefix routes.

>  	timeout = addrconf_timeout_fixup(prefered_lft, HZ);
> @@ -3497,8 +3499,12 @@ restart:
>  					ifp->flags |= IFA_F_DEPRECATED;
>  				}
>  
> -				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
> -					next = ifp->tstamp + ifp->valid_lft * HZ;
> +				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
> +					if (time_before(ifp->tstamp +
> +							ifp->valid_lft * HZ, next))
> +						next = ifp->tstamp +
> +							ifp->valid_lft * HZ;
> +				}

You could reduce one level of indentation if you collapse those two conditions.

>  
>  				spin_unlock(&ifp->lock);
>  
> @@ -3635,7 +3641,6 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  	} else {
>  		expires = 0;
>  		flags = 0;
> -		ifa_flags |= IFA_F_PERMANENT;
>  	}

Ditto, I do think we cannot remove this flag unconditionally.

Actually, I don't think it is the correct way to remove the IFA_F_PERMANENT
here at all. We break the interaction with prefix routes and won't remove them
at the time of address deletion any more. This is definitely a no-go, sorry.

The prefix was added by hand and should get removed immediately if a user
removes the address (well actually it is an error we set up a prefix route in
the first place, but it is how things are now).

IMHO, the changes need to be made in addrconf_verify so that the route does
get marked as deprecated. Please check the places where we mark an interface
address as deprecated.

Thanks,

  Hannes

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-14  9:19           ` Hannes Frederic Sowa
@ 2013-12-14  9:22             ` Hannes Frederic Sowa
  2013-12-29  5:57             ` Asano, Yasushi
                               ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-14  9:22 UTC (permalink / raw)
  To: yazzep, netdev, Yasushi Asano

On Sat, Dec 14, 2013 at 10:19:17AM +0100, Hannes Frederic Sowa wrote:
> IMHO, the changes need to be made in addrconf_verify so that the route does
 
								   ^^^^
I actually meant address, sorry. The route must stay valid of course.

> get marked as deprecated. Please check the places where we mark an interface
> address as deprecated.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-14  9:19           ` Hannes Frederic Sowa
  2013-12-14  9:22             ` Hannes Frederic Sowa
@ 2013-12-29  5:57             ` Asano, Yasushi
  2013-12-29  7:34             ` [PATCH] " yasushi.asano
                               ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Asano, Yasushi @ 2013-12-29  5:57 UTC (permalink / raw)
  To: Hannes Frederic Sowa, yazzep; +Cc: netdev

> Ditto, I do think we cannot remove this flag unconditionally.
> 
> Actually, I don't think it is the correct way to remove the IFA_F_PERMANENT
> here at all. We break the interaction with prefix routes and won't remove
> them
> at the time of address deletion any more. This is definitely a no-go, sorry.
> 
> The prefix was added by hand and should get removed immediately if a user
> removes the address (well actually it is an error we set up a prefix route
> in
> the first place, but it is how things are now).
> 
> IMHO, the changes need to be made in addrconf_verify so that the route does
> get marked as deprecated. Please check the places where we mark an interface
> address as deprecated.
> 
> Thanks,
> 
>   Hannes

Sorry about not getting back to you sooner.
I have been thinking and testing about the correct way of fixing the problem.
I understand it's not the correct way to remove the IFA_F_PERMANENT. 
I agree with you that the changes need to be made in addrconf_verify.
and I think the changes also need to be made in inet6_fill_ifaddr
so that the "preferred" is set INFINITY_LIFE_TIME unconditionally while 
ifa->flags is IFA_F_PERMANENT.
It means, "preferred" is set to INFINITY_LIFE_TIME when I set preferred_lft a value 
not zero or infinity, while valid_lft is infinity(0xffffffff)
I am going to send a new patch soon.
Your kind consideration of this matter would be sincerely appreciated.

Yasushi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-14  9:19           ` Hannes Frederic Sowa
  2013-12-14  9:22             ` Hannes Frederic Sowa
  2013-12-29  5:57             ` Asano, Yasushi
@ 2013-12-29  7:34             ` yasushi.asano
  2013-12-29  7:47             ` yazzep
  2013-12-29 12:04             ` [PATCH 1/1] ipv6 addrconf:fix " Yasushi Asano
  4 siblings, 0 replies; 21+ messages in thread
From: yasushi.asano @ 2013-12-29  7:34 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd2d7d0..796d52a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3479,7 +3479,8 @@ restart:
 					 &inet6_addr_lst[i], addr_lst) {
 			unsigned long age;
 
-			if (ifp->flags & IFA_F_PERMANENT)
+			if ((ifp->flags & IFA_F_PERMANENT) &&
+			    (ifp->prefered_lft == INFINITY_LIFE_TIME))
 				continue;
 
 			spin_lock(&ifp->lock);
@@ -3504,8 +3505,12 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
-					next = ifp->tstamp + ifp->valid_lft * HZ;
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
+				    if (time_before(ifp->tstamp +
+					    ifp->valid_lft * HZ, next))
+						next = ifp->tstamp +
+							 ifp->valid_lft * HZ;
+				}
 
 				spin_unlock(&ifp->lock);
 
@@ -3804,7 +3809,8 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	put_ifaddrmsg(nlh, ifa->prefix_len, ifa->flags, rt_scope(ifa->scope),
 		      ifa->idev->dev->ifindex);
 
-	if (!(ifa->flags&IFA_F_PERMANENT)) {
+	if (!((ifa->flags&IFA_F_PERMANENT) &&
+	      (ifa->prefered_lft == INFINITY_LIFE_TIME))) {
 		preferred = ifa->prefered_lft;
 		valid = ifa->valid_lft;
 		if (preferred != INFINITY_LIFE_TIME) {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-14  9:19           ` Hannes Frederic Sowa
                               ` (2 preceding siblings ...)
  2013-12-29  7:34             ` [PATCH] " yasushi.asano
@ 2013-12-29  7:47             ` yazzep
  2013-12-29 15:32               ` Hannes Frederic Sowa
  2013-12-30  2:17               ` Hannes Frederic Sowa
  2013-12-29 12:04             ` [PATCH 1/1] ipv6 addrconf:fix " Yasushi Asano
  4 siblings, 2 replies; 21+ messages in thread
From: yazzep @ 2013-12-29  7:47 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

Fixed a problem with setting the lifetime of an IPv6
address. When setting preferred_lft to a value not zero or
infinity, while valid_lft is infinity(0xffffffff) preferred
lifetime is set to forever and does not update. Therefore
preferred lifetime never becomes deprecated. valid lifetime
and preferred lifetime should be set independently, even if
valid lifetime is infinity, preferred lifetime must expire
correctly (meaning it must eventually become deprecated)


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd2d7d0..796d52a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3479,7 +3479,8 @@ restart:
 					 &inet6_addr_lst[i], addr_lst) {
 			unsigned long age;
 
-			if (ifp->flags & IFA_F_PERMANENT)
+			if ((ifp->flags & IFA_F_PERMANENT) &&
+			    (ifp->prefered_lft == INFINITY_LIFE_TIME))
 				continue;
 
 			spin_lock(&ifp->lock);
@@ -3504,8 +3505,12 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
-					next = ifp->tstamp + ifp->valid_lft * HZ;
+				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
+				    if (time_before(ifp->tstamp +
+					    ifp->valid_lft * HZ, next))
+						next = ifp->tstamp +
+							 ifp->valid_lft * HZ;
+				}
 
 				spin_unlock(&ifp->lock);
 
@@ -3804,7 +3809,8 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	put_ifaddrmsg(nlh, ifa->prefix_len, ifa->flags, rt_scope(ifa->scope),
 		      ifa->idev->dev->ifindex);
 
-	if (!(ifa->flags&IFA_F_PERMANENT)) {
+	if (!((ifa->flags&IFA_F_PERMANENT) &&
+	      (ifa->prefered_lft == INFINITY_LIFE_TIME))) {
 		preferred = ifa->prefered_lft;
 		valid = ifa->valid_lft;
 		if (preferred != INFINITY_LIFE_TIME) {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-14  9:19           ` Hannes Frederic Sowa
                               ` (3 preceding siblings ...)
  2013-12-29  7:47             ` yazzep
@ 2013-12-29 12:04             ` Yasushi Asano
  4 siblings, 0 replies; 21+ messages in thread
From: Yasushi Asano @ 2013-12-29 12:04 UTC (permalink / raw)
  To: netdev Mailing List


> 
> Ditto, I do think we cannot remove this flag unconditionally.
> 
> Actually, I don't think it is the correct way to remove the IFA_F_PERMANENT
> here at all. We break the interaction with prefix routes and won't remove them
> at the time of address deletion any more. This is definitely a no-go, sorry.
> 
> The prefix was added by hand and should get removed immediately if a user
> removes the address (well actually it is an error we set up a prefix route in
> the first place, but it is how things are now).
> 
> IMHO, the changes need to be made in addrconf_verify so that the route does
> get marked as deprecated. Please check the places where we mark an interface
> address as deprecated.
> 
> Thanks,
> 
>  Hannes

Sorry about not getting back to you sooner.
I have been thinking and testing about the correct way of fixing the problem.
I understand it's not the correct way to remove the IFA_F_PERMANENT.
I agree with you that the changes need to be made in addrconf_verify.
and I think changes also need to be made in inet6_fill_ifaddr
so that the "preferred" is set INFINITY_LIFE_TIME unconditionally while ifa->flags is IFA_F_PERMANENT.
It means, "preferred" is set to INFINITY_LIFE_TIME when I set preferred_lft a value not zero or infinity, while valid_lft is infinity(0xffffffff)
I sent a new patch to netdev-ml today.
Your kind consideration of this matter would be sincerely appreciated.

Yasushi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-29  7:47             ` yazzep
@ 2013-12-29 15:32               ` Hannes Frederic Sowa
  2013-12-30  2:17               ` Hannes Frederic Sowa
  1 sibling, 0 replies; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-29 15:32 UTC (permalink / raw)
  To: yazzep; +Cc: netdev, Yasushi Asano

On Sun, Dec 29, 2013 at 04:47:40PM +0900, yazzep@gmail.com wrote:
> From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> 
> Fixed a problem with setting the lifetime of an IPv6
> address. When setting preferred_lft to a value not zero or
> infinity, while valid_lft is infinity(0xffffffff) preferred
> lifetime is set to forever and does not update. Therefore
> preferred lifetime never becomes deprecated. valid lifetime
> and preferred lifetime should be set independently, even if
> valid lifetime is infinity, preferred lifetime must expire
> correctly (meaning it must eventually become deprecated)

This looks much better. I'll give it a try and do a review today.

> Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> ---
>  net/ipv6/addrconf.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cd2d7d0..796d52a 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3479,7 +3479,8 @@ restart:
>  					 &inet6_addr_lst[i], addr_lst) {
>  			unsigned long age;
>  
> -			if (ifp->flags & IFA_F_PERMANENT)
> +			if ((ifp->flags & IFA_F_PERMANENT) &&
> +			    (ifp->prefered_lft == INFINITY_LIFE_TIME))
>  				continue;

I would like to see a comment here why IFA_F_PERMANENT could have a
non-infinity life time. Something like that we always handle manually set
routes as IFA_F_PERMANENT but those routes could also have a prefered_lft of
non-infinity, so we must check if those interface prefix needs to be
deprecated.

>  			spin_lock(&ifp->lock);
> @@ -3504,8 +3505,12 @@ restart:
>  					ifp->flags |= IFA_F_DEPRECATED;
>  				}
>  
> -				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
> -					next = ifp->tstamp + ifp->valid_lft * HZ;
> +				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
> +				    if (time_before(ifp->tstamp +
> +					    ifp->valid_lft * HZ, next))
> +						next = ifp->tstamp +
> +							 ifp->valid_lft * HZ;
> +				}
>  
>  				spin_unlock(&ifp->lock);
>  
> @@ -3804,7 +3809,8 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
>  	put_ifaddrmsg(nlh, ifa->prefix_len, ifa->flags, rt_scope(ifa->scope),
>  		      ifa->idev->dev->ifindex);
>  
> -	if (!(ifa->flags&IFA_F_PERMANENT)) {
> +	if (!((ifa->flags&IFA_F_PERMANENT) &&
> +	      (ifa->prefered_lft == INFINITY_LIFE_TIME))) {
>  		preferred = ifa->prefered_lft;
>  		valid = ifa->valid_lft;
>  		if (preferred != INFINITY_LIFE_TIME) {

Thank you,

  Hannes

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-29  7:47             ` yazzep
  2013-12-29 15:32               ` Hannes Frederic Sowa
@ 2013-12-30  2:17               ` Hannes Frederic Sowa
  2013-12-31  3:04                 ` [PATCH] ipv6 addrconf: fix " Yasushi Asano
  1 sibling, 1 reply; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-30  2:17 UTC (permalink / raw)
  To: yazzep; +Cc: netdev, Yasushi Asano

On Sun, Dec 29, 2013 at 04:47:40PM +0900, yazzep@gmail.com wrote:

Subject: Re: [PATCH] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity

Please a space between addrconf: and fix.

> Fixed a problem with setting the lifetime of an IPv6
> address. When setting preferred_lft to a value not zero or
> infinity, while valid_lft is infinity(0xffffffff) preferred
> lifetime is set to forever and does not update. Therefore
> preferred lifetime never becomes deprecated. valid lifetime
> and preferred lifetime should be set independently, even if
> valid lifetime is infinity, preferred lifetime must expire
> correctly (meaning it must eventually become deprecated)
> 
> 
> Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> ---
>  net/ipv6/addrconf.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cd2d7d0..796d52a 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3479,7 +3479,8 @@ restart:
>  					 &inet6_addr_lst[i], addr_lst) {
>  			unsigned long age;
>  
> -			if (ifp->flags & IFA_F_PERMANENT)
> +			if ((ifp->flags & IFA_F_PERMANENT) &&
> +			    (ifp->prefered_lft == INFINITY_LIFE_TIME))
>  				continue;

As in the first mail, a comment would be nice how permanent addresses where
prefered_lft != INFINITY_LIFE_TIME could come from.

Maybe we could also add this to the header file where IFA_F_PERMANENT is
declared. There was recently confusion about that with another patch.

>  			spin_lock(&ifp->lock);
> @@ -3504,8 +3505,12 @@ restart:
>  					ifp->flags |= IFA_F_DEPRECATED;
>  				}
>  
> -				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
> -					next = ifp->tstamp + ifp->valid_lft * HZ;
> +				if (ifp->valid_lft != INFINITY_LIFE_TIME) {
> +				    if (time_before(ifp->tstamp +
> +					    ifp->valid_lft * HZ, next))
> +						next = ifp->tstamp +
> +							 ifp->valid_lft * HZ;
> +				}

Please combine the two if-tests here (with &&).

>  
>  				spin_unlock(&ifp->lock);
>  
> @@ -3804,7 +3809,8 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
>  	put_ifaddrmsg(nlh, ifa->prefix_len, ifa->flags, rt_scope(ifa->scope),
>  		      ifa->idev->dev->ifindex);
>  
> -	if (!(ifa->flags&IFA_F_PERMANENT)) {
> +	if (!((ifa->flags&IFA_F_PERMANENT) &&
> +	      (ifa->prefered_lft == INFINITY_LIFE_TIME))) {
>  		preferred = ifa->prefered_lft;
>  		valid = ifa->valid_lft;
>  		if (preferred != INFINITY_LIFE_TIME) {

Otherwise the patch looks good and my testing showed no problems.

Thank you!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] ipv6 addrconf: fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-30  2:17               ` Hannes Frederic Sowa
@ 2013-12-31  3:04                 ` Yasushi Asano
  2013-12-31 19:05                   ` Hannes Frederic Sowa
  2014-01-03  0:35                   ` David Miller
  0 siblings, 2 replies; 21+ messages in thread
From: Yasushi Asano @ 2013-12-31  3:04 UTC (permalink / raw)
  To: netdev; +Cc: Yasushi Asano

From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

Fixed a problem with setting the lifetime of an IPv6
address. When setting preferred_lft to a value not zero or
infinity, while valid_lft is infinity(0xffffffff) preferred
lifetime is set to forever and does not update. Therefore
preferred lifetime never becomes deprecated. valid lifetime
and preferred lifetime should be set independently, even if
valid lifetime is infinity, preferred lifetime must expire
correctly (meaning it must eventually become deprecated)


Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
---
 net/ipv6/addrconf.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd2d7d0..a9371fc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3479,7 +3479,12 @@ restart:
 					 &inet6_addr_lst[i], addr_lst) {
 			unsigned long age;
 
-			if (ifp->flags & IFA_F_PERMANENT)
+			/* When setting preferred_lft to a value not zero or
+			 * infinity, while valid_lft is infinity
+			 * IFA_F_PERMANENT has a non-infinity life time.
+			 */
+			if ((ifp->flags & IFA_F_PERMANENT) &&
+			    (ifp->prefered_lft == INFINITY_LIFE_TIME))
 				continue;
 
 			spin_lock(&ifp->lock);
@@ -3504,7 +3509,8 @@ restart:
 					ifp->flags |= IFA_F_DEPRECATED;
 				}
 
-				if (time_before(ifp->tstamp + ifp->valid_lft * HZ, next))
+				if ((ifp->valid_lft != INFINITY_LIFE_TIME) &&
+				    (time_before(ifp->tstamp + ifp->valid_lft * HZ, next)))
 					next = ifp->tstamp + ifp->valid_lft * HZ;
 
 				spin_unlock(&ifp->lock);
@@ -3804,7 +3810,8 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	put_ifaddrmsg(nlh, ifa->prefix_len, ifa->flags, rt_scope(ifa->scope),
 		      ifa->idev->dev->ifindex);
 
-	if (!(ifa->flags&IFA_F_PERMANENT)) {
+	if (!((ifa->flags&IFA_F_PERMANENT) &&
+	      (ifa->prefered_lft == INFINITY_LIFE_TIME))) {
 		preferred = ifa->prefered_lft;
 		valid = ifa->valid_lft;
 		if (preferred != INFINITY_LIFE_TIME) {
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] ipv6 addrconf: fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-31  3:04                 ` [PATCH] ipv6 addrconf: fix " Yasushi Asano
@ 2013-12-31 19:05                   ` Hannes Frederic Sowa
  2014-01-03  0:35                   ` David Miller
  1 sibling, 0 replies; 21+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-31 19:05 UTC (permalink / raw)
  To: Yasushi Asano; +Cc: netdev, Yasushi Asano

On Tue, Dec 31, 2013 at 12:04:19PM +0900, Yasushi Asano wrote:
> From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> 
> Fixed a problem with setting the lifetime of an IPv6
> address. When setting preferred_lft to a value not zero or
> infinity, while valid_lft is infinity(0xffffffff) preferred
> lifetime is set to forever and does not update. Therefore
> preferred lifetime never becomes deprecated. valid lifetime
> and preferred lifetime should be set independently, even if
> valid lifetime is infinity, preferred lifetime must expire
> correctly (meaning it must eventually become deprecated)
> 
> 
> Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ipv6 addrconf: fix preferred lifetime state-changing behavior while valid_lft is infinity
  2013-12-31  3:04                 ` [PATCH] ipv6 addrconf: fix " Yasushi Asano
  2013-12-31 19:05                   ` Hannes Frederic Sowa
@ 2014-01-03  0:35                   ` David Miller
  1 sibling, 0 replies; 21+ messages in thread
From: David Miller @ 2014-01-03  0:35 UTC (permalink / raw)
  To: yazzep; +Cc: netdev, yasushi.asano

From: Yasushi Asano <yazzep@gmail.com>
Date: Tue, 31 Dec 2013 12:04:19 +0900

> From: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
> 
> Fixed a problem with setting the lifetime of an IPv6
> address. When setting preferred_lft to a value not zero or
> infinity, while valid_lft is infinity(0xffffffff) preferred
> lifetime is set to forever and does not update. Therefore
> preferred lifetime never becomes deprecated. valid lifetime
> and preferred lifetime should be set independently, even if
> valid lifetime is infinity, preferred lifetime must expire
> correctly (meaning it must eventually become deprecated)
> 
> 
> Signed-off-by: Yasushi Asano <yasushi.asano@jp.fujitsu.com>

Applied.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-01-03  0:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-09  5:54 [PATCH 1/1] ipv6 addrconf:fix preferred lifetime state-changing behavior while valid_lft is infinity Asano, Yasushi
2013-12-09 23:47 ` Hannes Frederic Sowa
2013-12-12  0:58   ` Asano, Yasushi
2013-12-12  1:03     ` Hannes Frederic Sowa
2013-12-12 10:01       ` yazzep
2013-12-12 17:19         ` David Miller
2013-12-12 10:15       ` yazzep
2013-12-12 14:06         ` Sergei Shtylyov
2013-12-13 17:33         ` yazzep
2013-12-14  9:19           ` Hannes Frederic Sowa
2013-12-14  9:22             ` Hannes Frederic Sowa
2013-12-29  5:57             ` Asano, Yasushi
2013-12-29  7:34             ` [PATCH] " yasushi.asano
2013-12-29  7:47             ` yazzep
2013-12-29 15:32               ` Hannes Frederic Sowa
2013-12-30  2:17               ` Hannes Frederic Sowa
2013-12-31  3:04                 ` [PATCH] ipv6 addrconf: fix " Yasushi Asano
2013-12-31 19:05                   ` Hannes Frederic Sowa
2014-01-03  0:35                   ` David Miller
2013-12-29 12:04             ` [PATCH 1/1] ipv6 addrconf:fix " Yasushi Asano
2013-12-12 10:46       ` Asano, Yasushi

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).