Re: [PATCH net-next 3/3] net: ipv6: Add option to dump multipath routes via RTA_MULTIPATH attribute

2017-01-16 Thread David Ahern
upon further review ...

On 1/15/17 1:07 PM, David Ahern wrote:
> To maintain backwards compatibility, a user has to request the change
> in behavior. Unfortunately, adding a flag to the header similar to a
> previous patch does not work here as the netlink header for route dumps
> can be either rtgenmsg or ifinfomsg. sysctl is the other commonly used
> option for backwards compatibility but it is overly abused and is not
> really appropriate for this situation since each request should be able
> to specify the preference for the RTA_MULTIPATH attribute (e.g, one
> command may understand RTA_MULTIPATH for IPv6 while another may not).

The ancillary header is supposed to be struct rtmsg, so the change in behavior 
can be done by requiring the rtmsg struct and then passing the 
RTM_F_ALL_NEXTHOPS flag in rtm_flags. This makes the user api for delete 
requests and dumps consistent.

...

> @@ -422,9 +436,22 @@ static int inet6_dump_fib(struct sk_buff *skb, struct 
> netlink_callback *cb)
>   cb->args[2] = (long)w;
>   }
>  
> + hdrlen = nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg) ?
> +  sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
> +
> + if (nlmsg_parse(cb->nlh, hdrlen, nlattr, RTA_MAX, NULL) >= 0) {
> + /* existence of RTA_MULTIPATH attribute in dump
> +  * request means user wants multipath routes
> +  * returned using RTA_MULTIPATH attribute
> +  */
> + if (nlattr[RTA_MULTIPATH])
> + flags |= RTM_F_ALL_NEXTHOPS;
> + }
> +

And the above becomes:

+   /* sadly, the size of the ancillary header can vary: the correct
+* header is struct rtmsg as passed by libnl. iproute2 sends
+* ifinfomsg [+ filter]. Technically, passing only rtgenmsg is
+* sufficient since the header has not been parsed before. Luckily,
+* the struct sizes sufficiently vary to detect the permutations.
+* For the dump request to contain flags require rtmsg header.
+*/
+   if (nlmsg_len(cb->nlh) == sizeof(struct rtmsg)) {
+   struct rtmsg *rtm = nlmsg_data(cb->nlh);
+
+   flags = rtm->rtm_flags;
+   }


[PATCH net-next 3/3] net: ipv6: Add option to dump multipath routes via RTA_MULTIPATH attribute

2017-01-15 Thread David Ahern
IPv6 returns multipath routes as a series of individual routes making
their display different than IPv4 and putting the burden on the user to
see that a route is part of a multipath route. This patch addresses this
difference, allowing users to request multipath routes to be returned
using the RTA_MULTIPATH attribute.

To maintain backwards compatibility, a user has to request the change
in behavior. Unfortunately, adding a flag to the header similar to a
previous patch does not work here as the netlink header for route dumps
can be either rtgenmsg or ifinfomsg. sysctl is the other commonly used
option for backwards compatibility but it is overly abused and is not
really appropriate for this situation since each request should be able
to specify the preference for the RTA_MULTIPATH attribute (e.g, one
command may understand RTA_MULTIPATH for IPv6 while another may not).

This patch relies on the existence of the RTA_MULTIPATH attribute
appeneded to the dump request -- similar to how attributes can be appended
to link dumps. Kernel side if the RTA_MULTIPATH attribute exists in the
dump request then the RTM_F_ALL_NEXTHOPS is set in a new rtm_flags
variable added to rt6_rtnl_dump_arg. The rtm_flags is then used by
rt6_dump_route to decided if the RTA_MULTIPATH attribute is used.

If it is enabled then rt6_fill_node walks the sibling list for a route
and encodes each as a next hop within RTA_MULTIPATH, and then
fib6_dump_node advances its cursor to the last sibling route in the
current route.

The end result is that IPv6 multipath routes can be displayed in a format
similar to IPv4:

$ ip -6 ro ls vrf red
2001:db8::/120 metric 1024
nexthop via 2001:db8:1::62  dev eth1 weight 1
nexthop via 2001:db8:1::61  dev eth1 weight 1
nexthop via 2001:db8:1::60  dev eth1 weight 1
nexthop via 2001:db8:1::59  dev eth1 weight 1
2001:db8:1::/120 dev eth1 proto kernel metric 256  pref medium
...

Suggested-by: Dinesh Dutt 
Signed-off-by: David Ahern 
---
 include/net/ip6_route.h |   1 +
 net/ipv6/ip6_fib.c  |  29 -
 net/ipv6/route.c| 107 +++-
 3 files changed, 116 insertions(+), 21 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 9dc2c182a263..7620974826a5 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -154,6 +154,7 @@ struct rt6_rtnl_dump_arg {
struct sk_buff *skb;
struct netlink_callback *cb;
struct net *net;
+   unsigned int rtm_flags;
 };
 
 int rt6_dump_route(struct rt6_info *rt, void *p_arg);
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index ef5485204522..429713819c9f 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -308,16 +308,27 @@ static void __net_init fib6_tables_init(struct net *net)
 
 static int fib6_dump_node(struct fib6_walker *w)
 {
+   struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *)w->args;
int res;
struct rt6_info *rt;
 
for (rt = w->leaf; rt; rt = rt->dst.rt6_next) {
-   res = rt6_dump_route(rt, w->args);
+   res = rt6_dump_route(rt, arg);
if (res < 0) {
/* Frame is full, suspend walking */
w->leaf = rt;
return 1;
}
+
+   /* if multipath routes are dumped in one route with
+* the RTA_MULTIPATH attribute, then jump rt to point
+* to the last sibling of this route (no need to dump
+* the sibling routes again)
+*/
+   if ((arg->rtm_flags & RTM_F_ALL_NEXTHOPS) && rt->rt6i_nsiblings)
+   rt = list_last_entry(&rt->rt6i_siblings,
+struct rt6_info,
+rt6i_siblings);
}
w->leaf = NULL;
return 0;
@@ -392,13 +403,16 @@ static int fib6_dump_table(struct fib6_table *table, 
struct sk_buff *skb,
 static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
struct net *net = sock_net(skb->sk);
+   struct nlattr *nlattr[RTA_MAX + 1];
unsigned int h, s_h;
unsigned int e = 0, s_e;
struct rt6_rtnl_dump_arg arg;
struct fib6_walker *w;
struct fib6_table *tb;
struct hlist_head *head;
+   unsigned int flags = 0;
int res = 0;
+   int hdrlen;
 
s_h = cb->args[0];
s_e = cb->args[1];
@@ -422,9 +436,22 @@ static int inet6_dump_fib(struct sk_buff *skb, struct 
netlink_callback *cb)
cb->args[2] = (long)w;
}
 
+   hdrlen = nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg) ?
+sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
+
+   if (nlmsg_parse(cb->nlh, hdrlen, nlattr, RTA_MAX, NULL) >= 0) {
+   /* existence of RTA_MULTIPATH attribute in dump
+