From: David Ahern <hidden> Date: 2015-09-02 16:40:41
All callers to rt_dst_alloc have nearly the same initialization following
a successful allocation. Consolidate it into rt_dst_alloc.
Signed-off-by: David Ahern <redacted>
---
net/ipv4/route.c | 85 ++++++++++++++++++++++----------------------------------
1 file changed, 33 insertions(+), 52 deletions(-)
From: David Ahern <hidden> Date: 2015-09-02 16:40:42
Add the FIB table id to rtable to make the information available for
IPv4 as it is for IPv6.
Signed-off-by: David Ahern <redacted>
---
drivers/net/vrf.c | 2 ++
include/net/route.h | 2 ++
net/ipv4/route.c | 8 ++++++++
net/ipv4/xfrm4_policy.c | 1 +
4 files changed, 13 insertions(+)
From: David Ahern <hidden> Date: 2015-09-02 16:40:44
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 <redacted>
---
net/ipv4/route.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Thomas Graf <tgraf@suug.ch> Date: 2015-09-02 18:43:49
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 <redacted>
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.
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 <redacted>
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.
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 <redacted>
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.
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 <redacted>
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.
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 <redacted>
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?
From: David Ahern <hidden> Date: 2015-09-02 20:16:32
IPv4 ABI has the table hardcoded as RT_TABLE_MAIN regardless of the table
hit for the route lookup. Add the table using a new attribute,
RTA_TABLE_LOOKUP, to maintain the ABI yet return the right table id.
Signed-off-by: David Ahern <redacted>
---
Thomas: Something like this?
The current ABI is returning wrong data in some cases; that seems worse
to me than breaking the ABI.
include/uapi/linux/rtnetlink.h | 1 +
net/ipv4/route.c | 5 +++++
net/ipv6/route.c | 4 ++++
3 files changed, 10 insertions(+)
@@ -2922,6 +2922,10 @@ static int rt6_fill_node(struct net *net,rtm->rtm_table=table;if(nla_put_u32(skb,RTA_TABLE,table))gotonla_put_failure;++if(table&&nla_put_u32(skb,RTA_TABLE_LOOKUP,table))+gotonla_put_failure;+if(rt->rt6i_flags&RTF_REJECT){switch(rt->dst.error){case-EINVAL:
From: Thomas Graf <tgraf@suug.ch> Date: 2015-09-02 20:18:15
On 09/02/15 at 03:43pm, Andy Gospodarek wrote:
On Wed, Sep 02, 2015 at 09:08:36PM +0200, Thomas Graf wrote:
quoted
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.
From: Thomas Graf <tgraf@suug.ch> Date: 2015-09-02 20:23:18
On 09/02/15 at 01:16pm, David Ahern wrote:
IPv4 ABI has the table hardcoded as RT_TABLE_MAIN regardless of the table
hit for the route lookup. Add the table using a new attribute,
RTA_TABLE_LOOKUP, to maintain the ABI yet return the right table id.
Signed-off-by: David Ahern <redacted>
---
Thomas: Something like this?
The current ABI is returning wrong data in some cases; that seems worse
to me than breaking the ABI.
Another option is to introduce a new flag bundled with RTM_GETROUTE
which fixes RTM_GETROUTE altogether and makes it return the actual
route instead of a simulated cache entry.
From: David Ahern <hidden> Date: 2015-09-02 20:34:16
On 9/2/15 2:23 PM, Thomas Graf wrote:
On 09/02/15 at 01:16pm, David Ahern wrote:
quoted
IPv4 ABI has the table hardcoded as RT_TABLE_MAIN regardless of the table
hit for the route lookup. Add the table using a new attribute,
RTA_TABLE_LOOKUP, to maintain the ABI yet return the right table id.
Signed-off-by: David Ahern <redacted>
---
Thomas: Something like this?
The current ABI is returning wrong data in some cases; that seems worse
to me than breaking the ABI.
Another option is to introduce a new flag bundled with RTM_GETROUTE
which fixes RTM_GETROUTE altogether and makes it return the actual
route instead of a simulated cache entry.
I like that better; it least then information is not duplicated. Thanks for the suggestion.
From: Alexander Duyck <hidden> Date: 2015-09-02 20:41:11
On 09/02/2015 01:16 PM, David Ahern wrote:
quoted hunk
IPv4 ABI has the table hardcoded as RT_TABLE_MAIN regardless of the table
hit for the route lookup. Add the table using a new attribute,
RTA_TABLE_LOOKUP, to maintain the ABI yet return the right table id.
Signed-off-by: David Ahern <redacted>
---
Thomas: Something like this?
The current ABI is returning wrong data in some cases; that seems worse
to me than breaking the ABI.
include/uapi/linux/rtnetlink.h | 1 +
net/ipv4/route.c | 5 +++++
net/ipv6/route.c | 4 ++++
3 files changed, 10 insertions(+)
@@ -2328,6 +2328,11 @@ static int rt_fill_info(struct net *net, __be32 dst, __be32 src,r->rtm_table=RT_TABLE_MAIN;if(nla_put_u32(skb,RTA_TABLE,RT_TABLE_MAIN))gotonla_put_failure;++if(rt->rt_table_id&&rt->rt_table_id!=RT_TABLE_MAIN&&+nla_put_u32(skb,RTA_TABLE_LOOKUP,rt->rt_table_id))+gotonla_put_failure;+r->rtm_type=rt->rt_type;r->rtm_scope=RT_SCOPE_UNIVERSE;r->rtm_protocol=RTPROT_UNSPEC;
Why not implement this this same for IPv4 and IPv6? It looks like it is only included if it is non-zer and not MAIN in the above case, and then below as long as a table ID is non-zero you are setting the value. Why not just include the value in all cases where it is defined just like for IPv6?
@@ -2922,6 +2922,10 @@ static int rt6_fill_node(struct net *net,rtm->rtm_table=table;if(nla_put_u32(skb,RTA_TABLE,table))gotonla_put_failure;++if(table&&nla_put_u32(skb,RTA_TABLE_LOOKUP,table))+gotonla_put_failure;+if(rt->rt6i_flags&RTF_REJECT){switch(rt->dst.error){case-EINVAL:
From: David Ahern <hidden> Date: 2015-09-02 20:47:11
On 9/2/15 2:41 PM, Alexander Duyck wrote:
Why not implement this this same for IPv4 and IPv6? It looks like it is
only included if it is non-zer and not MAIN in the above case, and then
below as long as a table ID is non-zero you are setting the value. Why
not just include the value in all cases where it is defined just like
for IPv6?
I like Thomas' suggestion to add an rtm_flag better. We only need to fix IPv4 which hardcodes the tableid. Adding a flag, e.g.,
+#define RTM_F_LOOKUP_TABLE 0x1000 /* set rtm_table to FIB lookup result */
signifies the caller wants the real table. When set rt_fill_info sets rtm_table to the actual table id. This allows updated tools to work properly for both ipv4 and ipv6 and without breaking existing userspace.
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 <redacted>
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.