[iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Ahern]

Currently 'ip route get' does not show the table the lookup result comes
from and prior to kernel commit c36ba6603a11 the response from the kernel
was hardcoded to the main table. From the discussion this appears to be
a leftover from the route cache where the cached entry lost the table id
and so the result was hardcoded to main table.

c36ba6603a11 added the RTM_F_LOOKUP_TABLE flag to maintain that behavior
but to allow new tools to ask for the actual table id for the lookup.
This patch adds that flag to ip route get request and if the result is
not the main table shows the table id.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 ip/iproute.c              | 6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 8f49e6289003..bae43d5d8fb6 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -421,9 +421,9 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	if (tb[RTA_OIF] && filter.oifmask != -1)
 		fprintf(fp, "dev %s ", ll_index_to_name(*(int*)RTA_DATA(tb[RTA_OIF])));
 
+	if (table && (table != RT_TABLE_MAIN || show_details > 0) && !filter.tb)
+		fprintf(fp, " table %s ", rtnl_rttable_n2a(table, b1, sizeof(b1)));
 	if (!(r->rtm_flags&RTM_F_CLONED)) {
-		if ((table != RT_TABLE_MAIN || show_details > 0) && !filter.tb)
-			fprintf(fp, " table %s ", rtnl_rttable_n2a(table, b1, sizeof(b1)));
 		if ((r->rtm_protocol != RTPROT_BOOT || show_details > 0) && filter.protocolmask != -1)
 			fprintf(fp, " proto %s ", rtnl_rtprot_n2a(r->rtm_protocol, b1, sizeof(b1)));
 		if ((r->rtm_scope != RT_SCOPE_UNIVERSE || show_details > 0) && filter.scopemask != -1)
@@ -1638,6 +1638,8 @@ static int iproute_get(int argc, char **argv)
 	if (req.r.rtm_family == AF_UNSPEC)
 		req.r.rtm_family = AF_INET;
 
+	req.r.rtm_flags |= RTM_F_LOOKUP_TABLE;
+
 	if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
 		exit(2);
 
-- 
1.9.1

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[Stephen Hemminger]

On Mon, 21 Sep 2015 11:19:48 -0700
David Ahern <dsa@cumulusnetworks.com> wrote:

> Currently 'ip route get' does not show the table the lookup result comes
> from and prior to kernel commit c36ba6603a11 the response from the kernel
> was hardcoded to the main table. From the discussion this appears to be
> a leftover from the route cache where the cached entry lost the table id
> and so the result was hardcoded to main table.
> 
> c36ba6603a11 added the RTM_F_LOOKUP_TABLE flag to maintain that behavior
> but to allow new tools to ask for the actual table id for the lookup.
> This patch adds that flag to ip route get request and if the result is
> not the main table shows the table id.
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
>  ip/iproute.c              | 6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 8f49e6289003..bae43d5d8fb6 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -421,9 +421,9 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  	if (tb[RTA_OIF] && filter.oifmask != -1)
>  		fprintf(fp, "dev %s ", ll_index_to_name(*(int*)RTA_DATA(tb[RTA_OIF])));
>  
> +	if (table && (table != RT_TABLE_MAIN || show_details > 0) && !filter.tb)
> +		fprintf(fp, " table %s ", rtnl_rttable_n2a(table, b1, sizeof(b1)));
>  	if (!(r->rtm_flags&RTM_F_CLONED)) {
> -		if ((table != RT_TABLE_MAIN || show_details > 0) && !filter.tb)
> -			fprintf(fp, " table %s ", rtnl_rttable_n2a(table, b1, sizeof(b1)));
>  		if ((r->rtm_protocol != RTPROT_BOOT || show_details > 0) && filter.protocolmask != -1)
>  			fprintf(fp, " proto %s ", rtnl_rtprot_n2a(r->rtm_protocol, b1, sizeof(b1)));
>  		if ((r->rtm_scope != RT_SCOPE_UNIVERSE || show_details > 0) && filter.scopemask != -1)
> @@ -1638,6 +1638,8 @@ static int iproute_get(int argc, char **argv)
>  	if (req.r.rtm_family == AF_UNSPEC)
>  		req.r.rtm_family = AF_INET;
>  
> +	req.r.rtm_flags |= RTM_F_LOOKUP_TABLE;
> +
>  	if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
>  		exit(2);
>  

How will this work (or not) on older kernels?

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Ahern]

On 9/21/15 3:19 PM, Stephen Hemminger wrote:
>> @@ -1638,6 +1638,8 @@ static int iproute_get(int argc, char **argv)
>>   	if (req.r.rtm_family == AF_UNSPEC)
>>   		req.r.rtm_family = AF_INET;
>>
>> +	req.r.rtm_flags |= RTM_F_LOOKUP_TABLE;
>> +
>>   	if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
>>   		exit(2);
>>
>
> How will this work (or not) on older kernels?
>

It works just fine. First test used the wrong VM and was puzzled to not 
see the table id in the output. Then I realized the older kernel did not 
recognize the RTM_F_LOOKUP_TABLE; silently ignores the flag. With a 
kernel that does recognize it I get the table id in the output when it 
is not main.

David

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 21 Sep 2015 15:28:53 -0600

> On 9/21/15 3:19 PM, Stephen Hemminger wrote:
>>> @@ -1638,6 +1638,8 @@ static int iproute_get(int argc, char **argv)
>>>   	if (req.r.rtm_family == AF_UNSPEC)
>>>   		req.r.rtm_family = AF_INET;
>>>
>>> +	req.r.rtm_flags |= RTM_F_LOOKUP_TABLE;
>>> +
>>>   	if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0)
>>>   		exit(2);
>>>
>>
>> How will this work (or not) on older kernels?
>>
> 
> It works just fine. First test used the wrong VM and was puzzled to
> not see the table id in the output. Then I realized the older kernel
> did not recognize the RTM_F_LOOKUP_TABLE; silently ignores the
> flag. With a kernel that does recognize it I get the table id in the
> output when it is not main.

I think if it always gave MAIN in older kernels, iproute should continue
to do so.

You can't just remove the table ID output just because you disagree with
the semantics given by old kernels.

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Ahern]

On 9/21/15 3:58 PM, David Miller wrote:
> I think if it always gave MAIN in older kernels, iproute should continue
> to do so.
>
> You can't just remove the table ID output just because you disagree with
> the semantics given by old kernels.
>

Current semantics are maintained. Kernel was hardcoded to return main; 
iproute2 was hardcoded to not show main.

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 21 Sep 2015 16:03:00 -0600

> On 9/21/15 3:58 PM, David Miller wrote:
>> I think if it always gave MAIN in older kernels, iproute should
>> continue
>> to do so.
>>
>> You can't just remove the table ID output just because you disagree
>> with
>> the semantics given by old kernels.
>>
> 
> Current semantics are maintained. Kernel was hardcoded to return main;
> iproute2 was hardcoded to not show main.

Since iproute2 always showed MAIN, it should conitnue to do so when
run on older kernels.

And again this is regardless of whether you disagree with those
semantics or not.

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Ahern]

On 9/21/15 4:03 PM, David Miller wrote:
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Mon, 21 Sep 2015 16:03:00 -0600
>
>> On 9/21/15 3:58 PM, David Miller wrote:
>>> I think if it always gave MAIN in older kernels, iproute should
>>> continue
>>> to do so.
>>>
>>> You can't just remove the table ID output just because you disagree
>>> with
>>> the semantics given by old kernels.
>>>
>>
>> Current semantics are maintained. Kernel was hardcoded to return main;
>> iproute2 was hardcoded to not show main.
>
> Since iproute2 always showed MAIN, it should conitnue to do so when
> run on older kernels.
>
> And again this is regardless of whether you disagree with those
> semantics or not.
>

Dave:

ip does *not* show the table id or string today:

root@vm-wheezy2:~# ip route get 10.2.1.254
10.2.1.254 dev eth1 src 10.2.1.2
cache


With the new flag a AND kernel that supports it ip will only show the 
table id IF it is not main:

root@vm-wheezy2:~# ./ip route get 10.0.0.20
10.0.0.20 dev eth0  src 10.0.0.2
     cache

root@vm-wheezy2:~# ./ip route get 10.2.1.254
10.2.1.254 dev eth1  table 10  src 10.2.1.2
     cache

That's my point. I have not changed existing users.

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 21 Sep 2015 16:23:13 -0600

> With the new flag a AND kernel that supports it ip will only show the
> table id IF it is not main:
> 
> root@vm-wheezy2:~# ./ip route get 10.0.0.20
> 10.0.0.20 dev eth0  src 10.0.0.2
>     cache
> 
> root@vm-wheezy2:~# ./ip route get 10.2.1.254
> 10.2.1.254 dev eth1  table 10  src 10.2.1.2
>     cache
> 
> That's my point. I have not changed existing users.

Ok, thanks for the clarification.  I misread the conditional
statements around the area your patch touches, sorry.

Re: [iproute2 net-next] ip route: Add RTM_F_LOOKUP_TABLE flag and show table id

[Stephen Hemminger]

On Mon, 21 Sep 2015 11:19:48 -0700
David Ahern <dsa@cumulusnetworks.com> wrote:

> Currently 'ip route get' does not show the table the lookup result comes
> from and prior to kernel commit c36ba6603a11 the response from the kernel
> was hardcoded to the main table. From the discussion this appears to be
> a leftover from the route cache where the cached entry lost the table id
> and so the result was hardcoded to main table.
> 
> c36ba6603a11 added the RTM_F_LOOKUP_TABLE flag to maintain that behavior
> but to allow new tools to ask for the actual table id for the lookup.
> This patch adds that flag to ip route get request and if the result is
> not the main table shows the table id.
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

Applied, the compatibility concerns seem to be abated.