diff mbox

iproute2: prints bogus hoplimit

Message ID alpine.LSU.2.00.0902241125020.329@fbirervta.pbzchgretzou.qr
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Jan Engelhardt Feb. 24, 2009, 10:27 a.m. UTC
$ ip -6 r
fc00::/7 dev rtl0  proto kernel  metric 256  mtu 1500 advmss 1440 
hoplimit 4294967295

Most likely, "hoplimit -1" would be the right number, though I am not 
sure if simply changing %u by %d is a correct thing to do, since it 
would affect all fields.

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

Comments

stephen hemminger March 18, 2009, 10:12 p.m. UTC | #1
On Tue, 24 Feb 2009 11:27:52 +0100 (CET)
Jan Engelhardt <jengelh@medozas.de> wrote:

> 
> $ ip -6 r
> fc00::/7 dev rtl0  proto kernel  metric 256  mtu 1500 advmss 1440 
> hoplimit 4294967295
> 
> Most likely, "hoplimit -1" would be the right number, though I am not 
> sure if simply changing %u by %d is a correct thing to do, since it 
> would affect all fields.
> 
> ---
>  ip/iproute.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 6a2ea05..91a8cc0 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -507,7 +507,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  
>  			if (i != RTAX_RTT && i != RTAX_RTTVAR &&
>  			    i != RTAX_RTO_MIN)
> -				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
> +				fprintf(fp, " %d", *(unsigned*)RTA_DATA(mxrta[i]));
>  			else {
>  				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
>  

No fix the kernel please.
--
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
Jan Engelhardt March 18, 2009, 10:35 p.m. UTC | #2
On Wednesday 2009-03-18 23:12, Stephen Hemminger wrote:
>On Tue, 24 Feb 2009 11:27:52 +0100 (CET)
>Jan Engelhardt <jengelh@medozas.de> wrote:
>
>> $ ip -6 r
>> fc00::/7 dev rtl0  proto kernel  metric 256  mtu 1500 advmss 1440 
>> hoplimit 4294967295
>> 
>> Most likely, "hoplimit -1" would be the right number, though I am not 
>> sure if simply changing %u by %d is a correct thing to do, since it 
>> would affect all fields.
>> 
>> ---
>>  ip/iproute.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>  	if (i != RTAX_RTT && i != RTAX_RTTVAR &&
>>  	    i != RTAX_RTO_MIN)
>> -		fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
>> +		fprintf(fp, " %d", *(unsigned*)RTA_DATA(mxrta[i]));
>
>No fix the kernel please.
>
And what should the kernel return?
--
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
stephen hemminger March 18, 2009, 11:29 p.m. UTC | #3
On Wed, 18 Mar 2009 23:35:12 +0100 (CET)
Jan Engelhardt <jengelh@medozas.de> wrote:

> 
> On Wednesday 2009-03-18 23:12, Stephen Hemminger wrote:
> >On Tue, 24 Feb 2009 11:27:52 +0100 (CET)
> >Jan Engelhardt <jengelh@medozas.de> wrote:
> >
> >> $ ip -6 r
> >> fc00::/7 dev rtl0  proto kernel  metric 256  mtu 1500 advmss 1440 
> >> hoplimit 4294967295
> >> 
> >> Most likely, "hoplimit -1" would be the right number, though I am not 
> >> sure if simply changing %u by %d is a correct thing to do, since it 
> >> would affect all fields.
> >> 
> >> ---
> >>  ip/iproute.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>  	if (i != RTAX_RTT && i != RTAX_RTTVAR &&
> >>  	    i != RTAX_RTO_MIN)
> >> -		fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
> >> +		fprintf(fp, " %d", *(unsigned*)RTA_DATA(mxrta[i]));
> >
> >No fix the kernel please.
> >
> And what should the kernel return?

The issue is that the hoplimit metric in ipv6 gets converted
to -1 (if it is zero on the incoming route request)..  

int ip6_route_add(struct fib6_config *cfg)
{
...
	  if (dst_metric(&rt->u.dst, RTAX_HOPLIMIT) == 0)
                rt->u.dst.metrics[RTAX_HOPLIMIT-1] = -1;


What should happen is when ip6 returns the route back
on the dump request (rt6_fill_node), it should translate the -1 back to 0
to keep it symmetrical and logical. The metrics are unsigned.

--
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
David Miller March 19, 2009, 12:51 a.m. UTC | #4
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 18 Mar 2009 15:12:16 -0700

> On Tue, 24 Feb 2009 11:27:52 +0100 (CET)
> Jan Engelhardt <jengelh@medozas.de> wrote:
> 
> > 
> > $ ip -6 r
> > fc00::/7 dev rtl0  proto kernel  metric 256  mtu 1500 advmss 1440 
> > hoplimit 4294967295
> > 
> > Most likely, "hoplimit -1" would be the right number, though I am not 
> > sure if simply changing %u by %d is a correct thing to do, since it 
> > would affect all fields.
 ...
> > diff --git a/ip/iproute.c b/ip/iproute.c
> > index 6a2ea05..91a8cc0 100644
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
> > @@ -507,7 +507,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> >  
> >  			if (i != RTAX_RTT && i != RTAX_RTTVAR &&
> >  			    i != RTAX_RTO_MIN)
> > -				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
> > +				fprintf(fp, " %d", *(unsigned*)RTA_DATA(mxrta[i]));
> >  			else {
> >  				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
> >  
> 
> No fix the kernel please.

I don't know what you mean by this Stephen.  Why don't you take
a look at when and why the kernel uses '-1'?

"-1" is the hoplimit the kernel uses in the dst metric to mean
"use the default".

So this iproute patch is absolutely correct, or alternatively, iproute
can print "default" when it sees '-1'.

--
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
David Miller March 19, 2009, 12:54 a.m. UTC | #5
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 18 Mar 2009 16:29:28 -0700

> The issue is that the hoplimit metric in ipv6 gets converted
> to -1 (if it is zero on the incoming route request)..  
> 
> int ip6_route_add(struct fib6_config *cfg)
> {
> ...
> 	  if (dst_metric(&rt->u.dst, RTAX_HOPLIMIT) == 0)
>                 rt->u.dst.metrics[RTAX_HOPLIMIT-1] = -1;
> 
> 
> What should happen is when ip6 returns the route back
> on the dump request (rt6_fill_node), it should translate the -1 back to 0
> to keep it symmetrical and logical. The metrics are unsigned.

This is how "default" is represented.

If you look, ipv4 does the same thing.
--
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
diff mbox

Patch

diff --git a/ip/iproute.c b/ip/iproute.c
index 6a2ea05..91a8cc0 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -507,7 +507,7 @@  int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 			if (i != RTAX_RTT && i != RTAX_RTTVAR &&
 			    i != RTAX_RTO_MIN)
-				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
+				fprintf(fp, " %d", *(unsigned*)RTA_DATA(mxrta[i]));
 			else {
 				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);