netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
@ 2007-12-20  3:31 Satoru SATOH
  2007-12-20  8:35 ` YOSHIFUJI Hideaki / 吉藤英明
  2007-12-20  9:12 ` Jarek Poplawski
  0 siblings, 2 replies; 13+ messages in thread
From: Satoru SATOH @ 2007-12-20  3:31 UTC (permalink / raw)
  To: netdev

"ip route show" does not print correct value when larger rto_min is
set (e.g. 3sec).

This problem is because of overflow in print_route() and
the patch below is a workaround fix for that.

[root test]# ./iproute2.git.org/ip/ip route show dev eth1
192.168.140.0/24  proto kernel  scope link  src 192.168.140.130
169.254.0.0/16  scope link
[root test]# ./iproute2.git.org/ip/ip route change 192.168.140.0/24
dev eth1 rto_min 3s
[root test]# ./iproute2.git.org/ip/ip route show dev eth1
192.168.140.0/24  scope link  rto_min lock 2ms     <-- wrong
169.254.0.0/16  scope link
[root test]# ./iproute2.git/ip/ip route show dev eth1  # patched version
192.168.140.0/24  scope link  rto_min lock 3000ms  <-- correct
169.254.0.0/16  scope link
[root test]#


Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>

 ip/iproute.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index f4200ae..fa722c6 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
                                fprintf(fp, " %u",
*(unsigned*)RTA_DATA(mxrta[i]));
                        else {
                                unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
+                               unsigned hz1 = hz / 1000;

-                               val *= 1000;
                                if (i == RTAX_RTT)
                                        val /= 8;
                                else if (i == RTAX_RTTVAR)
                                        val /= 4;
-                               if (val >= hz)
-                                       fprintf(fp, " %ums", val/hz);
+                               if (val >= hz1)
+                                       fprintf(fp, " %ums", val/hz1);
                                else
-                                       fprintf(fp, " %.2fms", (float)val/hz);
+                                       fprintf(fp, " %.2fms", (float)val/hz1);
                        }
                }
        }

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20  3:31 [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly Satoru SATOH
@ 2007-12-20  8:35 ` YOSHIFUJI Hideaki / 吉藤英明
  2007-12-20  9:12 ` Jarek Poplawski
  1 sibling, 0 replies; 13+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-20  8:35 UTC (permalink / raw)
  To: satoru.satoh; +Cc: netdev, yoshfuji

In article <d72b7ace0712191931w37f02645u8d5eb5eef9c1fba4@mail.gmail.com> (at Thu, 20 Dec 2007 12:31:27 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:

> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..fa722c6 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
>                                 fprintf(fp, " %u",
> *(unsigned*)RTA_DATA(mxrta[i]));
>                         else {
>                                 unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +                               unsigned hz1 = hz / 1000;
> 
> -                               val *= 1000;
>                                 if (i == RTAX_RTT)

I think this is incorrect; hz might not be 1000; e.g. 250 etc.

--yoshfuji

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20  3:31 [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly Satoru SATOH
  2007-12-20  8:35 ` YOSHIFUJI Hideaki / 吉藤英明
@ 2007-12-20  9:12 ` Jarek Poplawski
  2007-12-20 16:21   ` Satoru SATOH
  1 sibling, 1 reply; 13+ messages in thread
From: Jarek Poplawski @ 2007-12-20  9:12 UTC (permalink / raw)
  To: Satoru SATOH; +Cc: netdev

On 20-12-2007 04:31, Satoru SATOH wrote:
> "ip route show" does not print correct value when larger rto_min is
> set (e.g. 3sec).
> 
> This problem is because of overflow in print_route() and
> the patch below is a workaround fix for that.
> 
...
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
>                                 fprintf(fp, " %u",
> *(unsigned*)RTA_DATA(mxrta[i]));
>                         else {
>                                 unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +                               unsigned hz1 = hz / 1000;
...
> +                               if (val >= hz1)
> +                                       fprintf(fp, " %ums", val/hz1);
...

Probably I miss something or my iproute sources are too old, but:
does this work with hz < 1000?

Regards,
Jarek P.

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20  9:12 ` Jarek Poplawski
@ 2007-12-20 16:21   ` Satoru SATOH
  2007-12-20 20:24     ` Jarek Poplawski
  0 siblings, 1 reply; 13+ messages in thread
From: Satoru SATOH @ 2007-12-20 16:21 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev

i see. HZ can be < 1000.. i should be wrong.

however, i got the following,

[root iproute2.org]# ./ip/ip route change 192.168.140.0/24 dev eth1 rto_min 4s
[root iproute2.org]# gdb -q ./ip/ip
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) br iproute.c:512
Breakpoint 1 at 0x804fc8d: file iproute.c, line 512.
(gdb) r route show dev eth1
Starting program: /root/iproute2.org/ip/ip route show dev eth1

Breakpoint 1, print_route (who=0xbfb9854c, n=0xbfb94528, arg=0x6404c0)
at iproute.c:512
512                                     unsigned val =
*(unsigned*)RTA_DATA(mxrta[i]);
(gdb) l 512,522
512                                     unsigned val =
*(unsigned*)RTA_DATA(mxrta[i]);
513
514                                     val *= 1000;
515                                     if (i == RTAX_RTT)
516                                             val /= 8;
517                                     else if (i == RTAX_RTTVAR)
518                                             val /= 4;
519                                     if (val >= hz)
520                                             fprintf(fp, " %ums", val/hz);
521                                     else
522                                             fprintf(fp, " %.2fms",
(float)val/hz);
(gdb) p hz
$1 = 1000000000
(gdb) n
514                                     val *= 1000;
(gdb) p val
$2 = 4000000000
(gdb) p val/ (hz / 1000)
$3 = 4000
(gdb) n
515                                     if (i == RTAX_RTT)
(gdb) p val
$4 = 1385447424
(gdb) c
Continuing.
192.168.140.0/24  scope link  rto_min lock 1ms

Program exited normally.
(gdb)


Thanks,
Satoru SATOH

2007/12/20, Jarek Poplawski <jarkao2@gmail.com>:
> On 20-12-2007 04:31, Satoru SATOH wrote:
> > "ip route show" does not print correct value when larger rto_min is
> > set (e.g. 3sec).
> >
> > This problem is because of overflow in print_route() and
> > the patch below is a workaround fix for that.
> >
> ...
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
> > @@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
> > struct nlmsghdr *n, void *arg)
> >                                 fprintf(fp, " %u",
> > *(unsigned*)RTA_DATA(mxrta[i]));
> >                         else {
> >                                 unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> > +                               unsigned hz1 = hz / 1000;
> ...
> > +                               if (val >= hz1)
> > +                                       fprintf(fp, " %ums", val/hz1);
> ...
>
> Probably I miss something or my iproute sources are too old, but:
> does this work with hz < 1000?
>
> Regards,
> Jarek P.

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20 16:21   ` Satoru SATOH
@ 2007-12-20 20:24     ` Jarek Poplawski
  2007-12-20 20:31       ` Jarek Poplawski
  0 siblings, 1 reply; 13+ messages in thread
From: Jarek Poplawski @ 2007-12-20 20:24 UTC (permalink / raw)
  To: Satoru SATOH; +Cc: netdev

Satoru SATOH wrote, On 12/20/2007 05:21 PM:

> i see. HZ can be < 1000.. i should be wrong.
> 
> however, i got the following,
> 
> [root iproute2.org]# ./ip/ip route change 192.168.140.0/24 dev eth1 rto_min 4s
> [root iproute2.org]# gdb -q ./ip/ip 

...

> (gdb) p hz
> $1 = 1000000000

That's why I had some doubts! I didn't study this enough, but my
(older) version definitely showed hz == 100. Maybe I'm wrong, but
looking into lib/util.c it seems this could be set differently
depending on system's configuration (or even kernel version).

So, probably this patch could sometimes work even for HZ < 1000,
but since it's your patch, I hope you do some additional checking
if it's always like this...

Cheers,
Jarek P.

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20 20:24     ` Jarek Poplawski
@ 2007-12-20 20:31       ` Jarek Poplawski
  2007-12-21  2:24         ` Satoru SATOH
  0 siblings, 1 reply; 13+ messages in thread
From: Jarek Poplawski @ 2007-12-20 20:31 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Satoru SATOH, netdev

Jarek Poplawski wrote, On 12/20/2007 09:24 PM:
...

> but since it's your patch, I hope you do some additional checking
> if it's always like this...


...or maybe only changing this all a little bit will make it look safer!

Jarek P.

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-20 20:31       ` Jarek Poplawski
@ 2007-12-21  2:24         ` Satoru SATOH
  2007-12-21  8:34           ` Jarek Poplawski
  2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 2 replies; 13+ messages in thread
From: Satoru SATOH @ 2007-12-21  2:24 UTC (permalink / raw)
  To: netdev

2007/12/21, Jarek Poplawski <jarkao2@gmail.com>:
> Jarek Poplawski wrote, On 12/20/2007 09:24 PM:
> ...
>
> > but since it's your patch, I hope you do some additional checking
> > if it's always like this...
>
>
> ...or maybe only changing this all a little bit will make it look safer!
>
> Jarek P.


OK, how about this?

Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>

 ip/iproute.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index f4200ae..c771b34 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -510,16 +510,20 @@ int print_route(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
 				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
 			else {
 				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
+				unsigned hz1 = hz;
+				if (hz1 > 1000)
+					hz1 /= 1000;
+				else
+					val *= 1000;

-				val *= 1000;
 				if (i == RTAX_RTT)
 					val /= 8;
 				else if (i == RTAX_RTTVAR)
 					val /= 4;
-				if (val >= hz)
-					fprintf(fp, " %ums", val/hz);
+				if (val >= hz1)
+					fprintf(fp, " %ums", val/hz1);
 				else
-					fprintf(fp, " %.2fms", (float)val/hz);
+					fprintf(fp, " %.2fms", (float)val/hz1);
 			}
 		}
 	}


Thanks,
Satoru SATOH

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21  2:24         ` Satoru SATOH
@ 2007-12-21  8:34           ` Jarek Poplawski
  2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 0 replies; 13+ messages in thread
From: Jarek Poplawski @ 2007-12-21  8:34 UTC (permalink / raw)
  To: Satoru SATOH; +Cc: netdev

On 21-12-2007 03:24, Satoru SATOH wrote:
> 2007/12/21, Jarek Poplawski <jarkao2@gmail.com>:
>> Jarek Poplawski wrote, On 12/20/2007 09:24 PM:
>> ...
>>
>>> but since it's your patch, I hope you do some additional checking
>>> if it's always like this...
>>
>> ...or maybe only changing this all a little bit will make it look safer!
>>
>> Jarek P.
> 
> 
> OK, how about this?
> 
> Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>
> 
>  ip/iproute.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..c771b34 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,20 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
>  				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>  			else {
>  				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +				unsigned hz1 = hz;
> +				if (hz1 > 1000)

Looks OK (safe) to me: it's compatible both with old and new way.
I'd only suggest to maybe change this to '(hz1 > 1024)', because
it's the biggest HZ currently in the kernel, so this compatibility
should be 100%. I think, you could leave 1 empty line before this
'if', as well. (Btw., aren't these overflows connected with
CONFIG_HIGH_RES_TIMERS?)

On the other hand this 'hz' still looks 'strange' here - I don't
understand, why, a bit earlier it's:

	if (!hz)
		hz = get_hz();

while 'else' would use: hz == get_user_hz();
So, probably I miss something, but even after your patch, there
could be different outputs here...

Thanks,
Jarek P.

PS: did you CC Stephen Hemminger on this?

> +					hz1 /= 1000;
> +				else
> +					val *= 1000;
> 
> -				val *= 1000;
>  				if (i == RTAX_RTT)
>  					val /= 8;
>  				else if (i == RTAX_RTTVAR)
>  					val /= 4;
> -				if (val >= hz)
> -					fprintf(fp, " %ums", val/hz);
> +				if (val >= hz1)
> +					fprintf(fp, " %ums", val/hz1);
>  				else
> -					fprintf(fp, " %.2fms", (float)val/hz);
> +					fprintf(fp, " %.2fms", (float)val/hz1);
>  			}
>  		}
>  	}
> 
> 
> Thanks,
> Satoru SATOH

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21  2:24         ` Satoru SATOH
  2007-12-21  8:34           ` Jarek Poplawski
@ 2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
  2007-12-21 11:18             ` Bill Fink
  2007-12-21 13:49             ` Satoru SATOH
  1 sibling, 2 replies; 13+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-21  8:53 UTC (permalink / raw)
  To: satoru.satoh, shemminger; +Cc: netdev, yoshfuji

In article <d72b7ace0712201824k3754962cse377256cb5452b8a@mail.gmail.com> (at Fri, 21 Dec 2007 11:24:54 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:

> 2007/12/21, Jarek Poplawski <jarkao2@gmail.com>:
> > Jarek Poplawski wrote, On 12/20/2007 09:24 PM:
> > ...
> >
> > > but since it's your patch, I hope you do some additional checking
> > > if it's always like this...
> >
> >
> > ...or maybe only changing this all a little bit will make it look safer!
> >
> > Jarek P.
> 
> 
> OK, how about this?
> 
> Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>
> 
>  ip/iproute.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..c771b34 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,20 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
>  				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>  			else {
>  				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +				unsigned hz1 = hz;
> +				if (hz1 > 1000)

Why don't you simply use unsigned long long (or maybe uint64_t) here?

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

--- 
diff --git a/ip/iproute.c b/ip/iproute.c
index f4200ae..db9a3b6 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -509,16 +509,21 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			    i != RTAX_RTO_MIN)
 				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
 			else {
-				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
+				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
+				unsigned div = 1;
 
-				val *= 1000;
 				if (i == RTAX_RTT)
-					val /= 8;
+					div = 8;
 				else if (i == RTAX_RTTVAR)
-					val /= 4;
-				if (val >= hz)
-					fprintf(fp, " %ums", val/hz);
+					div = 4;
 				else
+					div = 1;
+
+				val = val * 1000ULL / div;
+
+				if (val >= hz) {
+					fprintf(fp, " %llums", val/hz);
+				} else
 					fprintf(fp, " %.2fms", (float)val/hz);
 			}
 		}

--yoshfuji

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
@ 2007-12-21 11:18             ` Bill Fink
  2007-12-21 13:49             ` Satoru SATOH
  1 sibling, 0 replies; 13+ messages in thread
From: Bill Fink @ 2007-12-21 11:18 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / ____________; +Cc: satoru.satoh, shemminger, netdev

On Fri, 21 Dec 2007, YOSHIFUJI Hideaki wrote:

> In article <d72b7ace0712201824k3754962cse377256cb5452b8a@mail.gmail.com> (at Fri, 21 Dec 2007 11:24:54 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:
> 
> > 2007/12/21, Jarek Poplawski <jarkao2@gmail.com>:
> > > Jarek Poplawski wrote, On 12/20/2007 09:24 PM:
> > > ...
> > >
> > > > but since it's your patch, I hope you do some additional checking
> > > > if it's always like this...
> > >
> > >
> > > ...or maybe only changing this all a little bit will make it look safer!
> > >
> > > Jarek P.
> > 
> > 
> > OK, how about this?
> > 
> > Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>
> > 
> >  ip/iproute.c |   12 ++++++++----
> >  1 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/ip/iproute.c b/ip/iproute.c
> > index f4200ae..c771b34 100644
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
> > @@ -510,16 +510,20 @@ int print_route(const struct sockaddr_nl *who,
> > struct nlmsghdr *n, void *arg)
> >  				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
> >  			else {
> >  				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> > +				unsigned hz1 = hz;
> > +				if (hz1 > 1000)
> 
> Why don't you simply use unsigned long long (or maybe uint64_t) here?

I was wondering that too.  And maybe change the "(float)" cast
to "(double)" in the fprintf.

						-Bill



> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> 
> --- 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..db9a3b6 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -509,16 +509,21 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  			    i != RTAX_RTO_MIN)
>  				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>  			else {
> -				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
> +				unsigned div = 1;
>  
> -				val *= 1000;
>  				if (i == RTAX_RTT)
> -					val /= 8;
> +					div = 8;
>  				else if (i == RTAX_RTTVAR)
> -					val /= 4;
> -				if (val >= hz)
> -					fprintf(fp, " %ums", val/hz);
> +					div = 4;
>  				else
> +					div = 1;
> +
> +				val = val * 1000ULL / div;
> +
> +				if (val >= hz) {
> +					fprintf(fp, " %llums", val/hz);
> +				} else
>  					fprintf(fp, " %.2fms", (float)val/hz);
>  			}
>  		}

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
  2007-12-21 11:18             ` Bill Fink
@ 2007-12-21 13:49             ` Satoru SATOH
  2007-12-21 13:58               ` YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 1 reply; 13+ messages in thread
From: Satoru SATOH @ 2007-12-21 13:49 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: shemminger, netdev

I agree.

I mistakenly thought hz in that context must be larger than 1000..
As it's uncertain, your's looks much simpler and better.

(btw, the lines "else .... div = 1" is not needed, is it?)

Thanks,
Satoru SATOH

2007/12/21, YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org>:
(snip)
> Why don't you simply use unsigned long long (or maybe uint64_t) here?
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>
> ---
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..db9a3b6 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -509,16 +509,21 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>                             i != RTAX_RTO_MIN)
>                                 fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>                         else {
> -                               unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +                               unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
> +                               unsigned div = 1;
>
> -                               val *= 1000;
>                                 if (i == RTAX_RTT)
> -                                       val /= 8;
> +                                       div = 8;
>                                 else if (i == RTAX_RTTVAR)
> -                                       val /= 4;
> -                               if (val >= hz)
> -                                       fprintf(fp, " %ums", val/hz);
> +                                       div = 4;
>                                 else
> +                                       div = 1;
> +
> +                               val = val * 1000ULL / div;
> +
> +                               if (val >= hz) {
> +                                       fprintf(fp, " %llums", val/hz);
> +                               } else
>                                         fprintf(fp, " %.2fms", (float)val/hz);
>                         }
>                 }
>
> --yoshfuji

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21 13:49             ` Satoru SATOH
@ 2007-12-21 13:58               ` YOSHIFUJI Hideaki / 吉藤英明
  2007-12-21 17:14                 ` Stephen Hemminger
  0 siblings, 1 reply; 13+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-21 13:58 UTC (permalink / raw)
  To: satoru.satoh, shemminger; +Cc: netdev, yoshfuji

In article <d72b7ace0712210549u51043bf8k585fd70e1ded7d8a@mail.gmail.com> (at Fri, 21 Dec 2007 22:49:59 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:

> I agree.
> 
> I mistakenly thought hz in that context must be larger than 1000..
> As it's uncertain, your's looks much simpler and better.
> 
> (btw, the lines "else .... div = 1" is not needed, is it?)

Simplest fix is as follows:

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
--
diff --git a/ip/iproute.c b/ip/iproute.c
index f4200ae..7a885b0 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -509,7 +509,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			    i != RTAX_RTO_MIN)
 				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
 			else {
-				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
+				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
 
 				val *= 1000;
 				if (i == RTAX_RTT)
@@ -517,7 +517,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				else if (i == RTAX_RTTVAR)
 					val /= 4;
 				if (val >= hz)
-					fprintf(fp, " %ums", val/hz);
+					fprintf(fp, " %llums", val/hz);
 				else
 					fprintf(fp, " %.2fms", (float)val/hz);
 			}

-- 
YOSHIFUJI Hideaki @ USAGI Project  <yoshfuji@linux-ipv6.org>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

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

* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
  2007-12-21 13:58               ` YOSHIFUJI Hideaki / 吉藤英明
@ 2007-12-21 17:14                 ` Stephen Hemminger
  0 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2007-12-21 17:14 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明
  Cc: satoru.satoh, netdev, yoshfuji

On Fri, 21 Dec 2007 22:58:04 +0900 (JST)
YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> wrote:

> In article <d72b7ace0712210549u51043bf8k585fd70e1ded7d8a@mail.gmail.com> (at Fri, 21 Dec 2007 22:49:59 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:
> 
> > I agree.
> > 
> > I mistakenly thought hz in that context must be larger than 1000..
> > As it's uncertain, your's looks much simpler and better.
> > 
> > (btw, the lines "else .... div = 1" is not needed, is it?)
> 
> Simplest fix is as follows:
> 
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> --
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..7a885b0 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -509,7 +509,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  			    i != RTAX_RTO_MIN)
>  				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>  			else {
> -				unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> +				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
>  
>  				val *= 1000;
>  				if (i == RTAX_RTT)
> @@ -517,7 +517,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  				else if (i == RTAX_RTTVAR)
>  					val /= 4;
>  				if (val >= hz)
> -					fprintf(fp, " %ums", val/hz);
> +					fprintf(fp, " %llums", val/hz);
>  				else
>  					fprintf(fp, " %.2fms", (float)val/hz);
>  			}
> 

applied thanks.

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

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

end of thread, other threads:[~2007-12-21 17:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-20  3:31 [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly Satoru SATOH
2007-12-20  8:35 ` YOSHIFUJI Hideaki / 吉藤英明
2007-12-20  9:12 ` Jarek Poplawski
2007-12-20 16:21   ` Satoru SATOH
2007-12-20 20:24     ` Jarek Poplawski
2007-12-20 20:31       ` Jarek Poplawski
2007-12-21  2:24         ` Satoru SATOH
2007-12-21  8:34           ` Jarek Poplawski
2007-12-21  8:53           ` YOSHIFUJI Hideaki / 吉藤英明
2007-12-21 11:18             ` Bill Fink
2007-12-21 13:49             ` Satoru SATOH
2007-12-21 13:58               ` YOSHIFUJI Hideaki / 吉藤英明
2007-12-21 17:14                 ` Stephen Hemminger

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