diff mbox series

[net-next,3/5] ipv4: support sport, dport and ip protocol in RTM_GETROUTE

Message ID 1523911298-8965-4-git-send-email-roopa@cumulusnetworks.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series fib rules extack support and selftest | expand

Commit Message

Roopa Prabhu April 16, 2018, 8:41 p.m. UTC 
From: Roopa Prabhu <roopa@cumulusnetworks.com>

This is a followup to fib rules sport, dport and ip proto
match support. Having them supported in getroute
makes it easier to test fib rule lookups. Used by fib rule
self tests.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/uapi/linux/rtnetlink.h |  3 ++
 net/ipv4/route.c               | 75 +++++++++++++++++++++++++++++++++---------
 2 files changed, 63 insertions(+), 15 deletions(-)

Comments

David Miller April 16, 2018, 10:58 p.m. UTC | #1 
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Mon, 16 Apr 2018 13:41:36 -0700

> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 9b15005..7947252 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -327,6 +327,9 @@ enum rtattr_type_t {
>  	RTA_PAD,
>  	RTA_UID,
>  	RTA_TTL_PROPAGATE,
> +	RTA_SPORT,
> +	RTA_DPORT,
> +	RTA_IP_PROTO,
>  	__RTA_MAX
>  };
>  
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index ccb25d8..ae55711 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2663,6 +2663,18 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>  			from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
>  		goto nla_put_failure;
>  
> +	if (fl4->fl4_sport &&
> +	    nla_put_u16(skb, RTA_SPORT, ntohs(fl4->fl4_sport)))
> +		goto nla_put_failure;

The addreeses are given over netlink in network byte order, so let's
be consistent and do the same for the ports et al. as well.

Thanks.
Roopa Prabhu April 17, 2018, 3:57 a.m. UTC | #2 
On Mon, Apr 16, 2018 at 3:58 PM, David Miller <davem@davemloft.net> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> Date: Mon, 16 Apr 2018 13:41:36 -0700
>
>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> index 9b15005..7947252 100644
>> --- a/include/uapi/linux/rtnetlink.h
>> +++ b/include/uapi/linux/rtnetlink.h
>> @@ -327,6 +327,9 @@ enum rtattr_type_t {
>>       RTA_PAD,
>>       RTA_UID,
>>       RTA_TTL_PROPAGATE,
>> +     RTA_SPORT,
>> +     RTA_DPORT,
>> +     RTA_IP_PROTO,
>>       __RTA_MAX
>>  };
>>
>> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
>> index ccb25d8..ae55711 100644
>> --- a/net/ipv4/route.c
>> +++ b/net/ipv4/route.c
>> @@ -2663,6 +2663,18 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>>                       from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
>>               goto nla_put_failure;
>>
>> +     if (fl4->fl4_sport &&
>> +         nla_put_u16(skb, RTA_SPORT, ntohs(fl4->fl4_sport)))
>> +             goto nla_put_failure;
>
> The addreeses are given over netlink in network byte order, so let's
> be consistent and do the same for the ports et al. as well.
>

will do, thanks.
Ido Schimmel April 17, 2018, 8:10 a.m. UTC | #3 
On Mon, Apr 16, 2018 at 01:41:36PM -0700, Roopa Prabhu wrote:
> @@ -2757,6 +2796,12 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>  	fl4.flowi4_oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0;
>  	fl4.flowi4_mark = mark;
>  	fl4.flowi4_uid = uid;
> +	if (sport)
> +		fl4.fl4_sport = sport;
> +	if (dport)
> +		fl4.fl4_dport = dport;
> +	if (ip_proto)
> +		fl4.flowi4_proto = ip_proto;

Hi Roopa,

This info isn't set in the synthesized skb, but only in the flow info
and therefore not used for input routes. I see you added a test case,
but it's only for output routes. I believe an input route test case will
fail.

Also, note that the skb as synthesized now is invalid - iph->ihl is 0
for example - so the flow dissector will spit it out. It effectively
means that route get is broken when L4 hashing is used. It also affects
output routes because since commit 3765d35ed8b9 ("net: ipv4: Convert
inet_rtm_getroute to rcu versions of route lookup") the skb is used to
calculate the multipath hash.
Roopa Prabhu April 18, 2018, 2:41 a.m. UTC | #4 
On Tue, Apr 17, 2018 at 1:10 AM, Ido Schimmel <idosch@idosch.org> wrote:
> On Mon, Apr 16, 2018 at 01:41:36PM -0700, Roopa Prabhu wrote:
>> @@ -2757,6 +2796,12 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>>       fl4.flowi4_oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0;
>>       fl4.flowi4_mark = mark;
>>       fl4.flowi4_uid = uid;
>> +     if (sport)
>> +             fl4.fl4_sport = sport;
>> +     if (dport)
>> +             fl4.fl4_dport = dport;
>> +     if (ip_proto)
>> +             fl4.flowi4_proto = ip_proto;
>
> Hi Roopa,
>
> This info isn't set in the synthesized skb, but only in the flow info
> and therefore not used for input routes. I see you added a test case,
> but it's only for output routes. I believe an input route test case will
> fail.

yep. I made a note for myself to work thru the input case and missed before
i sent the series.

>
> Also, note that the skb as synthesized now is invalid - iph->ihl is 0
> for example - so the flow dissector will spit it out. It effectively
> means that route get is broken when L4 hashing is used. It also affects
> output routes because since commit 3765d35ed8b9 ("net: ipv4: Convert
> inet_rtm_getroute to rcu versions of route lookup") the skb is used to
> calculate the multipath hash.

yep, remember that. will look. thanks Ido.
diff mbox series

Patch

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9b15005..7947252 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -327,6 +327,9 @@  enum rtattr_type_t {
 	RTA_PAD,
 	RTA_UID,
 	RTA_TTL_PROPAGATE,
+	RTA_SPORT,
+	RTA_DPORT,
+	RTA_IP_PROTO,
 	__RTA_MAX
 };
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ccb25d8..ae55711 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2663,6 +2663,18 @@  static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
 			from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
 		goto nla_put_failure;
 
+	if (fl4->fl4_sport &&
+	    nla_put_u16(skb, RTA_SPORT, ntohs(fl4->fl4_sport)))
+		goto nla_put_failure;
+
+	if (fl4->fl4_dport &&
+	    nla_put_u16(skb, RTA_DPORT, ntohs(fl4->fl4_dport)))
+		goto nla_put_failure;
+
+	if (fl4->flowi4_proto &&
+	    nla_put_u8(skb, RTA_IP_PROTO, fl4->flowi4_proto))
+		goto nla_put_failure;
+
 	error = rt->dst.error;
 
 	if (rt_is_input_route(rt)) {
@@ -2695,35 +2707,48 @@  static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
 	return -EMSGSIZE;
 }
 
+static int nla_get_port(struct nlattr *attr, __be16 *port)
+{
+	int p = nla_get_u16(attr);
+
+	if (p <= 0 || p >= 0xffff)
+		return -EINVAL;
+
+	*port = htons(p);
+	return 0;
+}
+
 static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			     struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(in_skb->sk);
-	struct rtmsg *rtm;
 	struct nlattr *tb[RTA_MAX+1];
+	u32 table_id = RT_TABLE_MAIN;
+	__be16 sport = 0, dport = 0;
 	struct fib_result res = {};
 	struct rtable *rt = NULL;
+	struct sk_buff *skb;
+	struct rtmsg *rtm;
 	struct flowi4 fl4;
+	u8 ip_proto = 0;
 	__be32 dst = 0;
 	__be32 src = 0;
+	kuid_t uid;
 	u32 iif;
 	int err;
 	int mark;
-	struct sk_buff *skb;
-	u32 table_id = RT_TABLE_MAIN;
-	kuid_t uid;
 
 	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy,
 			  extack);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	rtm = nlmsg_data(nlh);
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb) {
 		err = -ENOBUFS;
-		goto errout;
+		return err;
 	}
 
 	/* Reserve room for dummy headers, this skb can pass
@@ -2740,6 +2765,20 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		uid = make_kuid(current_user_ns(), nla_get_u32(tb[RTA_UID]));
 	else
 		uid = (iif ? INVALID_UID : current_uid());
+	if (tb[RTA_SPORT]) {
+		err = nla_get_port(tb[RTA_SPORT], &sport);
+		if (err)
+			goto errout_free;
+	}
+
+	if (tb[RTA_DPORT]) {
+		err = nla_get_port(tb[RTA_DPORT], &dport);
+		if (err)
+			goto errout_free;
+	}
+
+	if (tb[RTA_IP_PROTO])
+		ip_proto = nla_get_u8(tb[RTA_IP_PROTO]);
 
 	/* Bugfix: need to give ip_route_input enough of an IP header to
 	 * not gag.
@@ -2757,6 +2796,12 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	fl4.flowi4_oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0;
 	fl4.flowi4_mark = mark;
 	fl4.flowi4_uid = uid;
+	if (sport)
+		fl4.fl4_sport = sport;
+	if (dport)
+		fl4.fl4_dport = dport;
+	if (ip_proto)
+		fl4.flowi4_proto = ip_proto;
 
 	rcu_read_lock();
 
@@ -2766,7 +2811,7 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		dev = dev_get_by_index_rcu(net, iif);
 		if (!dev) {
 			err = -ENODEV;
-			goto errout_free;
+			goto errout_rcu;
 		}
 
 		skb->protocol	= htons(ETH_P_IP);
@@ -2789,7 +2834,7 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	}
 
 	if (err)
-		goto errout_free;
+		goto errout_rcu;
 
 	if (rtm->rtm_flags & RTM_F_NOTIFY)
 		rt->rt_flags |= RTCF_NOTIFY;
@@ -2802,7 +2847,7 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			err = fib_props[res.type].error;
 			if (!err)
 				err = -EHOSTUNREACH;
-			goto errout_free;
+			goto errout_rcu;
 		}
 		err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
 				    nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
@@ -2813,18 +2858,18 @@  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 				   NETLINK_CB(in_skb).portid, nlh->nlmsg_seq);
 	}
 	if (err < 0)
-		goto errout_free;
+		goto errout_rcu;
 
 	rcu_read_unlock();
 
-	err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
-errout:
-	return err;
+	return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
 
 errout_free:
-	rcu_read_unlock();
 	kfree_skb(skb);
-	goto errout;
+	return err;
+errout_rcu:
+	rcu_read_unlock();
+	goto errout_free;
 }
 
 void ip_rt_multicast_event(struct in_device *in_dev)