| Message ID | 20090625090603.GP21357@jayr.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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 > > > > -- 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
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. -- 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
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).
--- 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;