* Re: Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
[not found] <20040129221538.J24747@fi.muni.cz>
@ 2004-01-29 23:37 ` YOSHIFUJI Hideaki / 吉藤英明
2004-01-29 23:39 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-01-29 23:37 UTC (permalink / raw)
To: kas; +Cc: linux-kernel, netdev, yoshfuji
In article <20040129221538.J24747@fi.muni.cz> (at Thu, 29 Jan 2004 22:15:38 +0100), Jan Kasprzak <kas@informatics.muni.cz> says:
> while compiling the kernel (2.6.1) I have spotted the following warning:
>
> net/ipv6/ndisc.c: In function `ndisc_router_discovery':
> net/ipv6/ndisc.c:1113: warning: comparison is always true due to limited range of data type
:
> The corresponding lines are these:
>
> __u32 rtime = ntohl(ra_msg->retrans_timer);
>
> Here ---> if (rtime && rtime/1000 < (MAX_SCHEDULE_TIMEOUT/HZ)) {
> rtime = (rtime*HZ)/1000;
> if (rtime < HZ/10)
> rtime = HZ/10;
> in6_dev->nd_parms->retrans_time = rtime;
> }
:
> The MAX_SCHEDULE_TIMEOUT is #defined to LONG_MAX in include/linux/sched.h,
> which is 2^63-1 or so on AMD64. I propose the following fix:
:
+#define MAX_SCHEDULE_TIMEOUT_32 (MAX_SCHEDULE_TIMEOUT/HZ > (1U<<31) ? \
+ (1U<<31) : MAX_SCHEDULE_TIMEOUT/HZ)
Well,... ok for now.
For long term solution, I think it is better to store timing variables
in "unsinged long" type instead of int.
I think there's several places to be fixed.
We'll need proc_doulongvec_jiffies(), proc_doulongvec_userhz_jiffies().
--yoshfuji
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
2004-01-29 23:37 ` Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c YOSHIFUJI Hideaki / 吉藤英明
@ 2004-01-29 23:39 ` David S. Miller
2004-01-30 0:40 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2004-01-29 23:39 UTC (permalink / raw)
To: yoshfuji; +Cc: kas, linux-kernel, netdev
On Fri, 30 Jan 2004 08:37:43 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:
> For long term solution, I think it is better to store timing variables
> in "unsinged long" type instead of int.
I think this is the only fix to even consider, even in the short term.
The macro suggested is just too much of a hack. :)
We can just ignore the silly warning until we are able to find time
to do the correct fix.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
2004-01-29 23:39 ` David S. Miller
@ 2004-01-30 0:40 ` Andi Kleen
2004-01-30 0:56 ` David S. Miller
0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2004-01-30 0:40 UTC (permalink / raw)
To: David S. Miller; +Cc: yoshfuji, kas, linux-kernel, netdev
On Thu, 29 Jan 2004 15:39:53 -0800
"David S. Miller" <davem@redhat.com> wrote:
> On Fri, 30 Jan 2004 08:37:43 +0900 (JST)
> YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:
>
> > For long term solution, I think it is better to store timing variables
> > in "unsinged long" type instead of int.
>
> I think this is the only fix to even consider, even in the short term.
> The macro suggested is just too much of a hack. :)
>
> We can just ignore the silly warning until we are able to find time
> to do the correct fix.
Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
FWIW only IPv6, keyboard driver and ACPI are spewing warnings on x86-64 in a defconfig
compile.
-Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
2004-01-30 0:40 ` Andi Kleen
@ 2004-01-30 0:56 ` David S. Miller
2004-01-30 1:07 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2004-01-30 0:56 UTC (permalink / raw)
To: Andi Kleen; +Cc: yoshfuji, kas, linux-kernel, netdev
On Fri, 30 Jan 2004 01:40:31 +0100
Andi Kleen <ak@suse.de> wrote:
> Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
Nope, for some reason gcc-3.2.3 on my systems is missing it.
Otherwise I would have killed this earlier :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
2004-01-30 0:56 ` David S. Miller
@ 2004-01-30 1:07 ` Andi Kleen
0 siblings, 0 replies; 6+ messages in thread
From: Andi Kleen @ 2004-01-30 1:07 UTC (permalink / raw)
To: David S. Miller; +Cc: yoshfuji, kas, linux-kernel, netdev
On Thu, 29 Jan 2004 16:56:45 -0800
"David S. Miller" <davem@redhat.com> wrote:
> On Fri, 30 Jan 2004 01:40:31 +0100
> Andi Kleen <ak@suse.de> wrote:
>
> > Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
>
> Nope, for some reason gcc-3.2.3 on my systems is missing it.
gcc 3.3 is printing it.
-Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c
@ 2004-01-29 21:15 Jan Kasprzak
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kasprzak @ 2004-01-29 21:15 UTC (permalink / raw)
To: linux-kernel
Hello, all!
while compiling the kernel (2.6.1) I have spotted the following warning:
net/ipv6/ndisc.c: In function `ndisc_router_discovery':
net/ipv6/ndisc.c:1113: warning: comparison is always true due to limited range of data type
net/ipv6/ndisc.c:1121: warning: comparison is always true due to limited range of data type
The corresponding lines are these:
__u32 rtime = ntohl(ra_msg->retrans_timer);
Here ---> if (rtime && rtime/1000 < (MAX_SCHEDULE_TIMEOUT/HZ)) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
rtime = HZ/10;
in6_dev->nd_parms->retrans_time = rtime;
}
rtime = ntohl(ra_msg->reachable_time);
and here --> if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
rtime = (rtime*HZ)/1000;
The MAX_SCHEDULE_TIMEOUT is #defined to LONG_MAX in include/linux/sched.h,
which is 2^63-1 or so on AMD64. I propose the following fix:
--- net/ipv6/ndisc.c.orig 2004-01-29 22:03:50.553745520 +0100
+++ net/ipv6/ndisc.c 2004-01-29 22:06:39.973989728 +0100
@@ -995,6 +995,9 @@
}
}
+#define MAX_SCHEDULE_TIMEOUT_32 (MAX_SCHEDULE_TIMEOUT/HZ > (1U<<31) ? \
+ (1U<<31) : MAX_SCHEDULE_TIMEOUT/HZ)
+
static void ndisc_router_discovery(struct sk_buff *skb)
{
struct ra_msg *ra_msg = (struct ra_msg *) skb->h.raw;
@@ -1110,7 +1113,7 @@
if (in6_dev->nd_parms) {
__u32 rtime = ntohl(ra_msg->retrans_timer);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
+ if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT_32) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
rtime = HZ/10;
@@ -1118,7 +1121,7 @@
}
rtime = ntohl(ra_msg->reachable_time);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
+ if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT_32/3) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
Do you think this is a correct fix? If so, please apply. Hope
this helps,
-Yenya
--
| Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/ Czech Linux Homepage: http://www.linux.cz/ |
Any compiler or language that likes to hide things like memory allocations
behind your back just isn't a good choice for a kernel. --Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-01-30 1:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040129221538.J24747@fi.muni.cz>
2004-01-29 23:37 ` Patch: IPv6/AMD64: bug in net/ipv6/ndisc.c YOSHIFUJI Hideaki / 吉藤英明
2004-01-29 23:39 ` David S. Miller
2004-01-30 0:40 ` Andi Kleen
2004-01-30 0:56 ` David S. Miller
2004-01-30 1:07 ` Andi Kleen
2004-01-29 21:15 Jan Kasprzak
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).