diff mbox

[net-next,3/3] net: Add table id from route lookup to route response

Message ID 1441212034-37464-3-git-send-email-dsa@cumulusnetworks.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

David Ahern Sept. 2, 2015, 4:40 p.m. UTC
rt_fill_info which is called for 'route get' requests hardcodes the
table id as RT_TABLE_MAIN which is not correct when multiple tables
are used. Use the newly added table id in the rtable to send back
the correct table.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 net/ipv4/route.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Graf Sept. 2, 2015, 6:43 p.m. UTC | #1
On 09/02/15 at 09:40am, David Ahern wrote:
> rt_fill_info which is called for 'route get' requests hardcodes the
> table id as RT_TABLE_MAIN which is not correct when multiple tables
> are used. Use the newly added table id in the rtable to send back
> the correct table.
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

What RTM_GETROUTE returns is not the actual route but a description
of the routing decision which is why table id, scope, protocol, and
prefix length are hardcoded. This is indicated by the RTM_F_CLONED
flag. What you propose would break userspace ABI.
--
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 Sept. 2, 2015, 6:49 p.m. UTC | #2
From: Thomas Graf <tgraf@suug.ch>
Date: Wed, 2 Sep 2015 20:43:46 +0200

> On 09/02/15 at 09:40am, David Ahern wrote:
>> rt_fill_info which is called for 'route get' requests hardcodes the
>> table id as RT_TABLE_MAIN which is not correct when multiple tables
>> are used. Use the newly added table id in the rtable to send back
>> the correct table.
>> 
>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> 
> What RTM_GETROUTE returns is not the actual route but a description
> of the routing decision which is why table id, scope, protocol, and
> prefix length are hardcoded. This is indicated by the RTM_F_CLONED
> flag. What you propose would break userspace ABI.

Agreed, I don't think we can do this.
--
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 Ahern Sept. 2, 2015, 6:51 p.m. UTC | #3
On 9/2/15 12:49 PM, David Miller wrote:
> From: Thomas Graf <tgraf@suug.ch>
> Date: Wed, 2 Sep 2015 20:43:46 +0200
>
>> On 09/02/15 at 09:40am, David Ahern wrote:
>>> rt_fill_info which is called for 'route get' requests hardcodes the
>>> table id as RT_TABLE_MAIN which is not correct when multiple tables
>>> are used. Use the newly added table id in the rtable to send back
>>> the correct table.
>>>
>>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
>>
>> What RTM_GETROUTE returns is not the actual route but a description
>> of the routing decision which is why table id, scope, protocol, and
>> prefix length are hardcoded. This is indicated by the RTM_F_CLONED
>> flag. What you propose would break userspace ABI.
>
> Agreed, I don't think we can do this.
>

Doesn't the table used to come up with the decision matter for IPv4? 
ie., hardcoding to MAIN is misleading when there is absolutely no way 
the decision comes from that table. IPv6 already returns the table id.

Or is your response that it breaks ABI and hence not going to fix.
--
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
Thomas Graf Sept. 2, 2015, 7:08 p.m. UTC | #4
On 09/02/15 at 12:51pm, David Ahern wrote:
> On 9/2/15 12:49 PM, David Miller wrote:
> >From: Thomas Graf <tgraf@suug.ch>
> >Date: Wed, 2 Sep 2015 20:43:46 +0200
> >
> >>On 09/02/15 at 09:40am, David Ahern wrote:
> >>>rt_fill_info which is called for 'route get' requests hardcodes the
> >>>table id as RT_TABLE_MAIN which is not correct when multiple tables
> >>>are used. Use the newly added table id in the rtable to send back
> >>>the correct table.
> >>>
> >>>Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> >>
> >>What RTM_GETROUTE returns is not the actual route but a description
> >>of the routing decision which is why table id, scope, protocol, and
> >>prefix length are hardcoded. This is indicated by the RTM_F_CLONED
> >>flag. What you propose would break userspace ABI.
> >
> >Agreed, I don't think we can do this.
> >
> 
> Doesn't the table used to come up with the decision matter for IPv4? ie.,
> hardcoding to MAIN is misleading when there is absolutely no way the
> decision comes from that table. IPv6 already returns the table id.
> 
> Or is your response that it breaks ABI and hence not going to fix.

This behaviour comes back from when we still had the IPv4 routing cache
which was flat. I'm not against exposing the table id but you have
to use a new attribute for it.
--
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
Andy Gospodarek Sept. 2, 2015, 7:43 p.m. UTC | #5
On Wed, Sep 02, 2015 at 09:08:36PM +0200, Thomas Graf wrote:
> On 09/02/15 at 12:51pm, David Ahern wrote:
> > On 9/2/15 12:49 PM, David Miller wrote:
> > >From: Thomas Graf <tgraf@suug.ch>
> > >Date: Wed, 2 Sep 2015 20:43:46 +0200
> > >
> > >>On 09/02/15 at 09:40am, David Ahern wrote:
> > >>>rt_fill_info which is called for 'route get' requests hardcodes the
> > >>>table id as RT_TABLE_MAIN which is not correct when multiple tables
> > >>>are used. Use the newly added table id in the rtable to send back
> > >>>the correct table.
> > >>>
> > >>>Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> > >>
> > >>What RTM_GETROUTE returns is not the actual route but a description
> > >>of the routing decision which is why table id, scope, protocol, and
> > >>prefix length are hardcoded. This is indicated by the RTM_F_CLONED
> > >>flag. What you propose would break userspace ABI.
> > >
> > >Agreed, I don't think we can do this.
> > >
> > 
> > Doesn't the table used to come up with the decision matter for IPv4? ie.,
> > hardcoding to MAIN is misleading when there is absolutely no way the
> > decision comes from that table. IPv6 already returns the table id.
> > 
> > Or is your response that it breaks ABI and hence not going to fix.
> 
> This behaviour comes back from when we still had the IPv4 routing cache
> which was flat.

So before the routing cache was removed, was the response always
RTA_TABLE_MAIN since there was no way to indicate which table may have
route if it came from the cache?
--
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
Thomas Graf Sept. 2, 2015, 8:18 p.m. UTC | #6
On 09/02/15 at 03:43pm, Andy Gospodarek wrote:
> On Wed, Sep 02, 2015 at 09:08:36PM +0200, Thomas Graf wrote:
> > This behaviour comes back from when we still had the IPv4 routing cache
> > which was flat.
> 
> So before the routing cache was removed, was the response always
> RTA_TABLE_MAIN since there was no way to indicate which table may have
> route if it came from the cache?

Yes, from that perspective, get and list are very different in
behaviour. Again, I'm not against including this information
but we can't break compatibility.
--
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 Sept. 2, 2015, 10:33 p.m. UTC | #7
From: Andy Gospodarek <gospo@cumulusnetworks.com>
Date: Wed, 2 Sep 2015 15:43:27 -0400

> On Wed, Sep 02, 2015 at 09:08:36PM +0200, Thomas Graf wrote:
>> On 09/02/15 at 12:51pm, David Ahern wrote:
>> > On 9/2/15 12:49 PM, David Miller wrote:
>> > >From: Thomas Graf <tgraf@suug.ch>
>> > >Date: Wed, 2 Sep 2015 20:43:46 +0200
>> > >
>> > >>On 09/02/15 at 09:40am, David Ahern wrote:
>> > >>>rt_fill_info which is called for 'route get' requests hardcodes the
>> > >>>table id as RT_TABLE_MAIN which is not correct when multiple tables
>> > >>>are used. Use the newly added table id in the rtable to send back
>> > >>>the correct table.
>> > >>>
>> > >>>Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
>> > >>
>> > >>What RTM_GETROUTE returns is not the actual route but a description
>> > >>of the routing decision which is why table id, scope, protocol, and
>> > >>prefix length are hardcoded. This is indicated by the RTM_F_CLONED
>> > >>flag. What you propose would break userspace ABI.
>> > >
>> > >Agreed, I don't think we can do this.
>> > >
>> > 
>> > Doesn't the table used to come up with the decision matter for IPv4? ie.,
>> > hardcoding to MAIN is misleading when there is absolutely no way the
>> > decision comes from that table. IPv6 already returns the table id.
>> > 
>> > Or is your response that it breaks ABI and hence not going to fix.
>> 
>> This behaviour comes back from when we still had the IPv4 routing cache
>> which was flat.
> 
> So before the routing cache was removed, was the response always
> RTA_TABLE_MAIN since there was no way to indicate which table may have
> route if it came from the cache?

Right.  In fact, it was possible for routes from multiple tables to
end up evaluating to the same routing cache entry.  So there could be
a many to one relationship back then.
--
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/net/ipv4/route.c b/net/ipv4/route.c
index 92acc95b7578..2738bf4132db 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2325,8 +2325,8 @@  static int rt_fill_info(struct net *net,  __be32 dst, __be32 src,
 	r->rtm_dst_len	= 32;
 	r->rtm_src_len	= 0;
 	r->rtm_tos	= fl4->flowi4_tos;
-	r->rtm_table	= RT_TABLE_MAIN;
-	if (nla_put_u32(skb, RTA_TABLE, RT_TABLE_MAIN))
+	r->rtm_table	= rt->rt_table_id;
+	if (nla_put_u32(skb, RTA_TABLE, rt->rt_table_id))
 		goto nla_put_failure;
 	r->rtm_type	= rt->rt_type;
 	r->rtm_scope	= RT_SCOPE_UNIVERSE;