PATCH: ipv6: avoid wraparound for expired lifetimes

PATCH: ipv6: avoid wraparound for expired lifetimes

[Jens Rosenboom]

If the valid or preferred lifetime for an address expires, the kernel
shows huge values for these due to a counter wrap, the following patch
should fix this, at least it did in the test I ran.

--- linux-2.6.30.orig/net/ipv6/addrconf.c	2009-06-10 05:05:27.000000000 +0200
+++ linux-2.6.30/net/ipv6/addrconf.c	2009-06-25 09:33:38.000000000 +0200
@@ -3361,9 +3361,17 @@
 		valid = ifa->valid_lft;
 		if (preferred != INFINITY_LIFE_TIME) {
 			long tval = (jiffies - ifa->tstamp)/HZ;
-			preferred -= tval;
-			if (valid != INFINITY_LIFE_TIME)
-				valid -= tval;
+			if (preferred > tval) {
+				preferred -= tval;
+			} else {
+				preferred = 0;
+			}
+			if (valid != INFINITY_LIFE_TIME) {
+				if (valid > tval) {
+					valid -= tval;
+				} else {
+					valid = 0;
+				}
 		}
 	} else {
 		preferred = INFINITY_LIFE_TIME;

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[Ilpo Järvinen]

On Thu, 25 Jun 2009, Jens Rosenboom wrote:

> If the valid or preferred lifetime for an address expires, the kernel
> shows huge values for these due to a counter wrap,

I suspect we have plenty of potentially counter-wrapped printouts all 
around the kernel. So you're fixing just a tip of the iceberg.

> the following patch
> should fix this, at least it did in the test I ran.

You must be kidding...

> --- linux-2.6.30.orig/net/ipv6/addrconf.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.30/net/ipv6/addrconf.c	2009-06-25 09:33:38.000000000 +0200
> @@ -3361,9 +3361,17 @@
>  		valid = ifa->valid_lft;
>  		if (preferred != INFINITY_LIFE_TIME) {
>  			long tval = (jiffies - ifa->tstamp)/HZ;
> -			preferred -= tval;
> -			if (valid != INFINITY_LIFE_TIME)
> -				valid -= tval;
> +			if (preferred > tval) {
> +				preferred -= tval;
> +			} else {
> +				preferred = 0;
> +			}
> +			if (valid != INFINITY_LIFE_TIME) {
> +				if (valid > tval) {
> +					valid -= tval;
> +				} else {
> +					valid = 0;
> +				}
>  		}

...because of the brace that is missing here, I get:

ijjarvin@pointhope:~/linux/mainline$ make net/ipv6/addrconf.o
  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  SYMLINK include/asm -> include/asm-x86
  CALL    scripts/checksyscalls.sh
  CC      net/ipv6/addrconf.o
net/ipv6/addrconf.c: In function inet6_fill_ifaddr:
net/ipv6/addrconf.c:3393: error: invalid storage class for function 
inet6_fill_ifmcaddr
net/ipv6/addrconf.c:3391: warning: ISO C90 forbids mixed declarations and 
code
net/ipv6/addrconf.c:3418: error: invalid storage class for function 
inet6_fill_ifacaddr
net/ipv6/addrconf.c:3450: error: invalid storage class for function 
inet6_dump_addr
net/ipv6/addrconf.c:3531: error: invalid storage class for function 
inet6_dump_ifaddr
net/ipv6/addrconf.c:3538: error: invalid storage class for function 
inet6_dump_ifmcaddr
net/ipv6/addrconf.c:3546: error: invalid storage class for function 
inet6_dump_ifacaddr
net/ipv6/addrconf.c:3554: error: invalid storage class for function 
inet6_rtm_getaddr
net/ipv6/addrconf.c:3604: error: invalid storage class for function 
inet6_ifa_notify
net/ipv6/addrconf.c:3629: error: invalid storage class for function 
ipv6_store_devconf
net/ipv6/addrconf.c:3674: error: invalid storage class for function 
inet6_if_nlmsg_size
net/ipv6/addrconf.c:3691: error: invalid storage class for function 
__snmp6_fill_stats
net/ipv6/addrconf.c:3706: error: invalid storage class for function 
snmp6_fill_stats
net/ipv6/addrconf.c:3719: error: invalid storage class for function 
inet6_fill_ifinfo
net/ipv6/addrconf.c:3787: error: invalid storage class for function 
inet6_dump_ifinfo
net/ipv6/addrconf.c:3840: error: invalid storage class for function 
inet6_prefix_nlmsg_size
net/ipv6/addrconf.c:3849: error: invalid storage class for function 
inet6_fill_prefix
net/ipv6/addrconf.c:3887: error: invalid storage class for function 
inet6_prefix_notify
net/ipv6/addrconf.c:3911: error: invalid storage class for function 
__ipv6_ifa_notify
net/ipv6/addrconf.c:3939: error: invalid storage class for function 
ipv6_ifa_notify
net/ipv6/addrconf.c:3951: error: invalid storage class for function 
addrconf_sysctl_forward
net/ipv6/addrconf.c:3967: error: invalid storage class for function 
addrconf_sysctl_forward_strategy
net/ipv6/addrconf.c:3999: error: invalid storage class for function 
dev_disable_change
net/ipv6/addrconf.c:4010: error: invalid storage class for function 
addrconf_disable_change
net/ipv6/addrconf.c:4030: error: invalid storage class for function 
addrconf_disable_ipv6
net/ipv6/addrconf.c:4055: error: invalid storage class for function 
addrconf_sysctl_disable
net/ipv6/addrconf.c:4081: error: initializer element is not constant
net/ipv6/addrconf.c:4081: error: (near initialization for 
addrconf_sysctl.addrconf_vars[0].proc_handler)
net/ipv6/addrconf.c:4082: error: initializer element is not constant
net/ipv6/addrconf.c:4082: error: (near initialization for 
addrconf_sysctl.addrconf_vars[0].strategy)
net/ipv6/addrconf.c:4304: error: initializer element is not constant
net/ipv6/addrconf.c:4304: error: (near initialization for 
addrconf_sysctl.addrconf_vars[26].proc_handler)
net/ipv6/addrconf.c:4323: error: invalid storage class for function 
__addrconf_sysctl_register
net/ipv6/addrconf.c:4377: error: invalid storage class for function 
__addrconf_sysctl_unregister
net/ipv6/addrconf.c:4391: error: invalid storage class for function 
addrconf_sysctl_register
net/ipv6/addrconf.c:4401: error: invalid storage class for function 
addrconf_sysctl_unregister
net/ipv6/addrconf.c:4410: error: invalid storage class for function 
addrconf_init_net
net/ipv6/addrconf.c:4461: error: invalid storage class for function 
addrconf_exit_net
net/ipv6/addrconf.c:4473: error: initializer element is not constant
net/ipv6/addrconf.c:4473: error: (near initialization for 
addrconf_ops.init)
net/ipv6/addrconf.c:4474: error: initializer element is not constant
net/ipv6/addrconf.c:4474: error: (near initialization for 
addrconf_ops.exit)
net/ipv6/addrconf.c:4486: error: non-static declaration of 
register_inet6addr_notifier follows static declaration
net/ipv6/addrconf.c:4482: error: previous definition of 
register_inet6addr_notifier was here
net/ipv6/addrconf.c:4493: error: non-static declaration of 
unregister_inet6addr_notifier follows static declaration
net/ipv6/addrconf.c:4489: error: previous definition of 
unregister_inet6addr_notifier was here
net/ipv6/addrconf.c:4601: error: expected declaration or statement at end 
of input
make[1]: *** [net/ipv6/addrconf.o] Error 1
make: *** [net/ipv6/addrconf.o] Error 2
ijjarvin@pointhope:~/linux/mainline$ 


-- 
 i.

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[David Miller]

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 25 Jun 2009 11:40:19 +0300 (EEST)

> On Thu, 25 Jun 2009, Jens Rosenboom wrote:
> 
>> the following patch
>> should fix this, at least it did in the test I ran.
> 
> You must be kidding...

Jens, don't even bother posting patches that fail to
build.

And if you can't get your patches straight, and the
problem is that you're sending something other than
what you actually tested, that's _MUCH_ worse.

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[Jens Rosenboom]

On Thu, Jun 25, 2009 at 11:40:19AM +0300, Ilpo Järvinen wrote:
> On Thu, 25 Jun 2009, Jens Rosenboom wrote:
> 
> > If the valid or preferred lifetime for an address expires, the kernel
> > shows huge values for these due to a counter wrap,
> 
> I suspect we have plenty of potentially counter-wrapped printouts all 
> around the kernel. So you're fixing just a tip of the iceberg.

So are you implying that because I don't fix all of them at once, I
shouldn't bother to start at all?

On Thu, 2009-06-25 at 01:42 -0700, David Miller wrote:
...
> Jens, don't even bother posting patches that fail to
> build.

Sorry for that, here is the correct version.

--- linux-2.6.30.orig/net/ipv6/addrconf.c	2009-06-10 05:05:27.000000000 +0200
+++ linux-2.6.30/net/ipv6/addrconf.c	2009-06-25 10:52:27.000000000 +0200
@@ -3361,9 +3361,18 @@
 		valid = ifa->valid_lft;
 		if (preferred != INFINITY_LIFE_TIME) {
 			long tval = (jiffies - ifa->tstamp)/HZ;
-			preferred -= tval;
-			if (valid != INFINITY_LIFE_TIME)
-				valid -= tval;
+			if (preferred > tval) {
+				preferred -= tval;
+			} else {
+				preferred = 0;
+			}
+			if (valid != INFINITY_LIFE_TIME) {
+				if (valid > tval) {
+					valid -= tval;
+				} else {
+					valid = 0;
+				}
+			}
 		}
 	} else {
 		preferred = INFINITY_LIFE_TIME;

And to show you where this appears:

Output with plain 2.6.30

# ip -6 addr show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
    inet6 2001:db8::202:a5ff:fee8:20be/64 scope global dynamic 
       valid_lft 870sec preferred_lft 7sec
    inet6 fe80::202:a5ff:fee8:20be/64 scope link 
       valid_lft forever preferred_lft forever
# sleep 30
# ip -6 addr show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
    inet6 2001:db8::202:a5ff:fee8:20be/64 scope global deprecated dynamic 
       valid_lft 840sec preferred_lft 4294967266sec
    inet6 fe80::202:a5ff:fee8:20be/64 scope link 
       valid_lft forever preferred_lft forever

Output with patched 2.6.30

# ip -6 addr show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
    inet6 2001:db8::202:a5ff:fee8:12e1/64 scope global dynamic 
       valid_lft 897sec preferred_lft 27sec
    inet6 fe80::202:a5ff:fee8:12e1/64 scope link 
       valid_lft forever preferred_lft forever
# sleep 30
# ip -6 addr show dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
    inet6 2001:db8::202:a5ff:fee8:12e1/64 scope global deprecated dynamic 
       valid_lft 862sec preferred_lft 0sec
    inet6 fe80::202:a5ff:fee8:12e1/64 scope link 
       valid_lft forever preferred_lft forever

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[Wei Yongjun]

Jens Rosenboom wrote:
> On Thu, Jun 25, 2009 at 11:40:19AM +0300, Ilpo Järvinen wrote:
>   
>> On Thu, 25 Jun 2009, Jens Rosenboom wrote:
>>
>>     
>>> If the valid or preferred lifetime for an address expires, the kernel
>>> shows huge values for these due to a counter wrap,
>>>       
>> I suspect we have plenty of potentially counter-wrapped printouts all 
>> around the kernel. So you're fixing just a tip of the iceberg.
>>     
>
> So are you implying that because I don't fix all of them at once, I
> shouldn't bother to start at all?
>
> On Thu, 2009-06-25 at 01:42 -0700, David Miller wrote:
> ...
>   
>> Jens, don't even bother posting patches that fail to
>> build.
>>     
>
> Sorry for that, here is the correct version.
>
> --- linux-2.6.30.orig/net/ipv6/addrconf.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.30/net/ipv6/addrconf.c	2009-06-25 10:52:27.000000000 +0200
> @@ -3361,9 +3361,18 @@
>  		valid = ifa->valid_lft;
>  		if (preferred != INFINITY_LIFE_TIME) {
>  			long tval = (jiffies - ifa->tstamp)/HZ;
> -			preferred -= tval;
> -			if (valid != INFINITY_LIFE_TIME)
> -				valid -= tval;
> +			if (preferred > tval) {
> +				preferred -= tval;
> +			} else {
> +				preferred = 0;
> +			}
> +			if (valid != INFINITY_LIFE_TIME) {
> +				if (valid > tval) {
> +					valid -= tval;
> +				} else {
> +					valid = 0;
> +				}
> +			}
>  		}
>  	} else {
>  		preferred = INFINITY_LIFE_TIME;
>
>   

checkpatch tell me the following errors:

# ./scripts/checkpatch.pl /root/PATCH\ \ ipv6\ \ avoid\ wraparound\ for\ expired\ lifetimes.eml

WARNING: braces {} are not necessary for any arm of this statement
#90: FILE: net/ipv6/addrconf.c:3364:
+                       if (preferred > tval) {
[...]
+                       } else {
[...]

ERROR: spaces required around that '-=' (ctx:WxV)
#91: FILE: net/ipv6/addrconf.c:3365:
+                               preferred -=3D tval;
                                          ^

ERROR: spaces required around that '=' (ctx:WxV)
#93: FILE: net/ipv6/addrconf.c:3367:
+                               preferred =3D 0;
                                          ^

ERROR: spaces required around that '!=' (ctx:WxV)
#95: FILE: net/ipv6/addrconf.c:3369:
+                       if (valid !=3D INFINITY_LIFE_TIME) {
                                  ^

ERROR: spaces required around that '-=' (ctx:WxV)
#97: FILE: net/ipv6/addrconf.c:3371:
+                                       valid -=3D tval;
                                              ^

ERROR: spaces required around that '=' (ctx:WxV)
#99: FILE: net/ipv6/addrconf.c:3373:
+                                       valid =3D 0;
                                              ^

ERROR: Missing Signed-off-by: line(s)

total: 6 errors, 1 warnings, 21 lines checked

/root/PATCH  ipv6  avoid wraparound for expired lifetimes.eml has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.


> And to show you where this appears:
>
> Output with plain 2.6.30
>
> # ip -6 addr show dev eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
>     inet6 2001:db8::202:a5ff:fee8:20be/64 scope global dynamic 
>        valid_lft 870sec preferred_lft 7sec
>     inet6 fe80::202:a5ff:fee8:20be/64 scope link 
>        valid_lft forever preferred_lft forever
> # sleep 30
> # ip -6 addr show dev eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
>     inet6 2001:db8::202:a5ff:fee8:20be/64 scope global deprecated dynamic 
>        valid_lft 840sec preferred_lft 4294967266sec
>     inet6 fe80::202:a5ff:fee8:20be/64 scope link 
>        valid_lft forever preferred_lft forever
>
> Output with patched 2.6.30
>
> # ip -6 addr show dev eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
>     inet6 2001:db8::202:a5ff:fee8:12e1/64 scope global dynamic 
>        valid_lft 897sec preferred_lft 27sec
>     inet6 fe80::202:a5ff:fee8:12e1/64 scope link 
>        valid_lft forever preferred_lft forever
> # sleep 30
> # ip -6 addr show dev eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
>     inet6 2001:db8::202:a5ff:fee8:12e1/64 scope global deprecated dynamic 
>        valid_lft 862sec preferred_lft 0sec
>     inet6 fe80::202:a5ff:fee8:12e1/64 scope link 
>        valid_lft forever preferred_lft forever
>
> --
> 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
>
>
>
>   

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[David Miller]

From: Jens Rosenboom <me@jayr.de>
Date: Thu, 25 Jun 2009 11:06:03 +0200

> Sorry for that, here is the correct version.

Please don't resend just the patch, make a formal fresh
submission, commit message and all.

That way it's get properly tracked as a unit on:

	http://patchwork.ozlabs.org/project/netdev/list/

If you just send the patch I have to go back through the list
archives, find your original changelog message, and then piece them
together with this new patch.  Which make unnecessarily more work for
me.

Re: PATCH: ipv6: avoid wraparound for expired lifetimes

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 943 bytes --]

On Thu, 25 Jun 2009, Jens Rosenboom wrote:

> On Thu, Jun 25, 2009 at 11:40:19AM +0300, Ilpo Järvinen wrote:
> > On Thu, 25 Jun 2009, Jens Rosenboom wrote:
> > 
> > > If the valid or preferred lifetime for an address expires, the kernel
> > > shows huge values for these due to a counter wrap,
> > 
> > I suspect we have plenty of potentially counter-wrapped printouts all 
> > around the kernel. So you're fixing just a tip of the iceberg.
> 
> So are you implying that because I don't fix all of them at once, I
> shouldn't bother to start at all?

I meant that fixing this place alone won't magically fix the rest which 
suffer from the very same symptoms even though you might have never seen 
the other cases in practice (that's why I used the iceberg parable).

I was thinking that some kind of helper would be useful for annotating 
what is happening and changing the other places too (not necessarily in 
the very same patch).

-- 
 i.