Re: iproute2: prints bogus hoplimit

Previous thread: PROBLEM: IPv6 autoconf/accept_ra default values by Kolbjørn Barmen on Tuesday, February 24, 2009 - 2:50 am. (8 messages)

Next thread: iproute2: replace linktype number by name for tunnels by Jan Engelhardt on Tuesday, February 24, 2009 - 3:31 am. (1 message)
From: Jan Engelhardt
Date: Tuesday, February 24, 2009 - 3:27 am

$ 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]);
 
-- 
--

From: Stephen Hemminger
Date: Wednesday, March 18, 2009 - 3:12 pm

On Tue, 24 Feb 2009 11:27:52 +0100 (CET)

No fix the kernel please.
--

From: Jan Engelhardt
Date: Wednesday, March 18, 2009 - 3:35 pm

And what should the kernel return?
--

From: Stephen Hemminger
Date: Wednesday, March 18, 2009 - 4:29 pm

On Wed, 18 Mar 2009 23:35:12 +0100 (CET)

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.

--

From: David Miller
Date: Wednesday, March 18, 2009 - 5:54 pm

From: Stephen Hemminger <shemminger@vyatta.com>

This is how "default" is represented.

If you look, ipv4 does the same thing.
--

From: David Miller
Date: Wednesday, March 18, 2009 - 5:51 pm

From: Stephen Hemminger <shemminger@vyatta.com>

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

--

From: Stephen Hemminger
Date: Wednesday, March 18, 2009 - 6:01 pm

On Wed, 18 Mar 2009 17:51:09 -0700 (PDT)

if I create route with:
   ip route add ... hoplimit 0
then to do
   ip route show 
one would expect the hoplimit in the show to match the add ??
Whether the kernel fudges it in rt6_fill or utility has special case
code really doesn't matter.

The correct patch in utility would be something like:

diff --git a/ip/iproute.c b/ip/iproute.c
index 6a2ea05..bf0f31b 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -493,6 +493,8 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			mxlock = *(unsigned*)RTA_DATA(mxrta[RTAX_LOCK]);
 
 		for (i=2; i<= RTAX_MAX; i++) {
+			unsigned val;
+
 			if (mxrta[i] == NULL)
 				continue;
 			if (!hz)
@@ -505,21 +507,31 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			if (mxlock & (1<<i))
 				fprintf(fp, " lock");
 
-			if (i != RTAX_RTT && i != RTAX_RTTVAR &&
-			    i != RTAX_RTO_MIN)
-				fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
-			else {
-				unsigned long long val = *(unsigned*)RTA_DATA(mxrta[i]);
+			val = *(unsigned*)RTA_DATA(mxrta[i]);
+			switch (i) {
+			case RTAX_HOPLIMIT:
+				if ((long)val == -1)
+					val = 0;
+				/* fall through */
+			default:
+				fprintf(fp, " %u", val);
+				break;
 
+			case RTAX_RTT:
+			case RTAX_RTTVAR:
+			case RTAX_RTO_MIN:
 				val *= 1000;
 				if (i == RTAX_RTT)
 					val /= 8;
 				else if (i == RTAX_RTTVAR)
 					val /= 4;
+
 				if (val >= hz)
-					fprintf(fp, " %llums", val/hz);
+					fprintf(fp, " %llums",
+						(unsigned long long) val / hz);
 				else
-					fprintf(fp, " %.2fms", (float)val/hz);
+					fprintf(fp, " %.2fms", 
+						(double)val / hz);
 			}
 		}
 	}

--

From: David Miller
Date: Wednesday, March 18, 2009 - 6:06 pm

From: Stephen Hemminger <shemminger@vyatta.com>

Even if the kernel should change, the tool should behave correctly
for all existing Linux kernels out there so you'd need the iproute2

Sure.
--

Previous thread: PROBLEM: IPv6 autoconf/accept_ra default values by Kolbjørn Barmen on Tuesday, February 24, 2009 - 2:50 am. (8 messages)

Next thread: iproute2: replace linktype number by name for tunnels by Jan Engelhardt on Tuesday, February 24, 2009 - 3:31 am. (1 message)