[PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE
From: Roopa Prabhu
This is a followup to fib rules sport, dport match support.
Having them supported in getroute makes it easier to test
fib rule lookups. Used by fib rule self tests. Before this patch
getroute used same skb to pass through the route lookup and
for the netlink getroute reply msg. This patch allocates separate
skb's to keep flow dissector happy.
Signed-off-by: Roopa Prabhu
---
include/uapi/linux/rtnetlink.h | 2 +
net/ipv4/route.c | 151 ++---
2 files changed, 115 insertions(+), 38 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9b15005..630ecf4 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -327,6 +327,8 @@ enum rtattr_type_t {
RTA_PAD,
RTA_UID,
RTA_TTL_PROPAGATE,
+ RTA_SPORT,
+ RTA_DPORT,
__RTA_MAX
};
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 1412a7b..e91ed62 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net,
struct flowi4 *flp4,
EXPORT_SYMBOL_GPL(ip_route_output_flow);
/* called with rcu_read_lock held */
-static int rt_fill_info(struct net *net, __be32 dst, __be32 src, u32 table_id,
- struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
- u32 seq)
+static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
+ struct rtable *rt, u32 table_id, struct flowi4 *fl4,
+ struct sk_buff *skb, u32 portid, u32 seq)
{
- struct rtable *rt = skb_rtable(skb);
struct rtmsg *r;
struct nlmsghdr *nlh;
unsigned long expires = 0;
@@ -2651,6 +2650,14 @@ 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_be16(skb, RTA_SPORT, fl4->fl4_sport))
+ goto nla_put_failure;
+
+ if (fl4->fl4_dport &&
+ nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
+ goto nla_put_failure;
+
error = rt->dst.error;
if (rt_is_input_route(rt)) {
@@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net, __be32 dst,
__be32 src, u32 table_id,
}
} else
#endif
- if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
+ if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
goto nla_put_failure;
}
@@ -2683,35 +2690,86 @@ 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_be16(attr);
+
+ if (p <= 0 || p >= 0x)
+ return -EINVAL;
+
+ *port = p;
+ return 0;
+}
+
+static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr
*nlh,
+ __be32 dst, __be32 src, struct flowi4 *fl4,
+ struct rtable *rt, struct fib_result *res)
+{
+ struct net *net = sock_net(in_skb->sk);
+ struct rtmsg *rtm = nlmsg_data(nlh);
+ u32 table_id = RT_TABLE_MAIN;
+ struct sk_buff *skb;
+ int err = 0;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb) {
+ err = -ENOMEM;
+ return err;
+ }
+
+ if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
+ table_id = res->table ? res->table->tb_id : 0;
+
+ if (rtm->rtm_flags & RTM_F_FIB_MATCH)
+ err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
+ nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
+ rt->rt_type, res->prefix, res->prefixlen,
+ fl4->flowi4_tos, res->fi, 0);
+ else
+ err = rt_fill_info(net, dst, src, rt, table_id,
+ fl4, skb, NETLINK_CB(in_skb).portid,
+ nlh->nlmsg_seq);
+ if (err < 0)
+ goto errout;
+
+ return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
+
+errout:
+ kfree_skb(skb);
+ return err;
+}
+
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];
+ __be16 sport = 0, dport = 0;
struct fib_result res = {};
struct rtable *rt = NULL;
+ struct sk_buff *skb;
+ struct rtmsg *rtm;
struct flowi4 fl4;
+ struct iphdr *iph;
+ struct udphdr *udph;
__be32 dst = 0;
__be32 src = 0;
+ kuid_t uid;
Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE
On 5/6/18 6:59 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu
>
> This is a followup to fib rules sport, dport match support.
> Having them supported in getroute makes it easier to test
> fib rule lookups. Used by fib rule self tests. Before this patch
> getroute used same skb to pass through the route lookup and
> for the netlink getroute reply msg. This patch allocates separate
> skb's to keep flow dissector happy.
>
> Signed-off-by: Roopa Prabhu
> ---
> include/uapi/linux/rtnetlink.h | 2 +
> net/ipv4/route.c | 151
> ++---
> 2 files changed, 115 insertions(+), 38 deletions(-)
>
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 9b15005..630ecf4 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -327,6 +327,8 @@ enum rtattr_type_t {
> RTA_PAD,
> RTA_UID,
> RTA_TTL_PROPAGATE,
> + RTA_SPORT,
> + RTA_DPORT,
If you are going to add sport and dport because of the potential for FIB
rules, you need to add ip-proto as well. I realize existing code assumed
UDP, but the FIB rules cover any IP proto. Yes, I know this makes the
change much larger to generate tcp, udp as well as iphdr options; the
joys of new features. ;-)
I also suggest a comment that these new RTA attributes are used for
GETROUTE only.
And you need to add the new entries to rtm_ipv4_policy.
> __RTA_MAX
> };
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 1412a7b..e91ed62 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net,
> struct flowi4 *flp4,
> EXPORT_SYMBOL_GPL(ip_route_output_flow);
>
> /* called with rcu_read_lock held */
> -static int rt_fill_info(struct net *net, __be32 dst, __be32 src, u32
> table_id,
> - struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
> - u32 seq)
> +static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
> + struct rtable *rt, u32 table_id, struct flowi4 *fl4,
> + struct sk_buff *skb, u32 portid, u32 seq)
> {
> - struct rtable *rt = skb_rtable(skb);
> struct rtmsg *r;
> struct nlmsghdr *nlh;
> unsigned long expires = 0;
> @@ -2651,6 +2650,14 @@ 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_be16(skb, RTA_SPORT, fl4->fl4_sport))
> + goto nla_put_failure;
> +
> + if (fl4->fl4_dport &&
> + nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
> + goto nla_put_failure;
Why return the attributes to the user? I can't see any value in that.
UID option is not returned either so there is precedence.
> +
> error = rt->dst.error;
>
> if (rt_is_input_route(rt)) {
> @@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net, __be32 dst,
> __be32 src, u32 table_id,
> }
> } else
> #endif
> - if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
> + if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
> goto nla_put_failure;
> }
>
> @@ -2683,35 +2690,86 @@ 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_be16(attr);
__be16 p;
> +
> + if (p <= 0 || p >= 0x)
> + return -EINVAL;
This check is not needed by definition of be16.
> +
> + *port = p;
> + return 0;
> +}
> +
> +static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr
> *nlh,
> +__be32 dst, __be32 src, struct flowi4 *fl4,
> +struct rtable *rt, struct fib_result *res)
> +{
> + struct net *net = sock_net(in_skb->sk);
> + struct rtmsg *rtm = nlmsg_data(nlh);
> + u32 table_id = RT_TABLE_MAIN;
> + struct sk_buff *skb;
> + int err = 0;
> +
> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> + if (!skb) {
> + err = -ENOMEM;
> + return err;
> + }
just 'return -ENOMEM' and without the {}.
> +
> + if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
> + table_id = res->table ? res->table->tb_id : 0;
> +
> + if (rtm->rtm_flags & RTM_F_FIB_MATCH)
> + err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
> + nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
> + rt->rt_type, res->prefix, res->prefixlen,
> + fl4->flowi4_tos, res->fi, 0);
> + else
> + err = rt_fill_info(net, dst, src, rt, table_id,
> +
Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE
Hi Roopa,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Roopa-Prabhu/fib-rule-selftest/20180507-094538
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
net/ipv4/route.c:1271:31: sparse: expression using sizeof(void)
net/ipv4/route.c:1271:31: sparse: expression using sizeof(void)
net/ipv4/route.c:1274:16: sparse: expression using sizeof(void)
net/ipv4/route.c:1274:16: sparse: expression using sizeof(void)
net/ipv4/route.c:1295:15: sparse: expression using sizeof(void)
net/ipv4/route.c:688:38: sparse: expression using sizeof(void)
net/ipv4/route.c:712:38: sparse: expression using sizeof(void)
net/ipv4/route.c:782:46: sparse: incorrect type in argument 2 (different
base types) @@expected unsigned int [unsigned] [usertype] key @@got ed
int [unsigned] [usertype] key @@
net/ipv4/route.c:782:46:expected unsigned int [unsigned] [usertype] key
net/ipv4/route.c:782:46:got restricted __be32 [usertype] new_gw
>> net/ipv4/route.c:2695:29: sparse: incorrect type in initializer (different
>> base types) @@expected int [signed] p @@got restint [signed] p @@
net/ipv4/route.c:2695:29:expected int [signed] p
net/ipv4/route.c:2695:29:got restricted __be16
>> net/ipv4/route.c:2700:15: sparse: incorrect type in assignment (different
>> base types) @@expected restricted __be16 [usertype] @@got
>> 6 [usertype] @@
net/ipv4/route.c:2700:15:expected restricted __be16 [usertype]
net/ipv4/route.c:2700:15:got int [signed] p
>> net/ipv4/route.c:2816:27: sparse: incorrect type in assignment (different
>> base types) @@expected restricted __be16 [usertype] len @@got 6
>> [usertype] len @@
net/ipv4/route.c:2816:27:expected restricted __be16 [usertype] len
net/ipv4/route.c:2816:27:got unsigned long
vim +2695 net/ipv4/route.c
2692
2693 static int nla_get_port(struct nlattr *attr, __be16 *port)
2694 {
> 2695 int p = nla_get_be16(attr);
2696
2697 if (p <= 0 || p >= 0x)
2698 return -EINVAL;
2699
> 2700 *port = p;
2701 return 0;
2702 }
2703
2704 static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct
nlmsghdr *nlh,
2705 __be32 dst, __be32 src, struct
flowi4 *fl4,
2706 struct rtable *rt, struct fib_result
*res)
2707 {
2708 struct net *net = sock_net(in_skb->sk);
2709 struct rtmsg *rtm = nlmsg_data(nlh);
2710 u32 table_id = RT_TABLE_MAIN;
2711 struct sk_buff *skb;
2712 int err = 0;
2713
2714 skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
2715 if (!skb) {
2716 err = -ENOMEM;
2717 return err;
2718 }
2719
2720 if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
2721 table_id = res->table ? res->table->tb_id : 0;
2722
2723 if (rtm->rtm_flags & RTM_F_FIB_MATCH)
2724 err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
2725 nlh->nlmsg_seq, RTM_NEWROUTE,
table_id,
2726 rt->rt_type, res->prefix,
res->prefixlen,
2727 fl4->flowi4_tos, res->fi, 0);
2728 else
2729 err = rt_fill_info(net, dst, src, rt, table_id,
2730 fl4, skb, NETLINK_CB(in_skb).portid,
2731 nlh->nlmsg_seq);
2732 if (err < 0)
2733 goto errout;
2734
2735 return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
2736
2737 errout:
2738 kfree_skb(skb);
2739 return err;
2740 }
2741
2742 static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr
*nlh,
2743 struct netlink_ext_ack *extack)
2744 {
2745 struct net *net = sock_net(in_skb->sk);
2746 struct nlattr *tb[RTA_MAX+1];
2747 __be16 sport = 0, dport = 0;
2748 struct fib_result res = {};
2749 struct rtable *rt = NULL;
2750 struct sk_buff *skb;
2751 struct rtmsg *rtm;
2752 struct flowi4 fl4;
2753 struct iphdr *iph;
2754 struct udphdr *udph;
2755 __be32 dst = 0;
2756 __be32 src = 0;
2757 kuid_t uid;
2758 u32 iif;
2759 int err;
2760 int mark;
2761
2762 err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
rtm_ipv4_policy,
2763extack);
2764 if (err < 0)
2765
Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE
On Sun, May 6, 2018 at 6:46 PM, David Ahern wrote:
> On 5/6/18 6:59 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> This is a followup to fib rules sport, dport match support.
>> Having them supported in getroute makes it easier to test
>> fib rule lookups. Used by fib rule self tests. Before this patch
>> getroute used same skb to pass through the route lookup and
>> for the netlink getroute reply msg. This patch allocates separate
>> skb's to keep flow dissector happy.
>>
>> Signed-off-by: Roopa Prabhu
>> ---
>> include/uapi/linux/rtnetlink.h | 2 +
>> net/ipv4/route.c | 151
>> ++---
>> 2 files changed, 115 insertions(+), 38 deletions(-)
>>
>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> index 9b15005..630ecf4 100644
>> --- a/include/uapi/linux/rtnetlink.h
>> +++ b/include/uapi/linux/rtnetlink.h
>> @@ -327,6 +327,8 @@ enum rtattr_type_t {
>> RTA_PAD,
>> RTA_UID,
>> RTA_TTL_PROPAGATE,
>> + RTA_SPORT,
>> + RTA_DPORT,
>
> If you are going to add sport and dport because of the potential for FIB
> rules, you need to add ip-proto as well. I realize existing code assumed
> UDP, but the FIB rules cover any IP proto. Yes, I know this makes the
> change much larger to generate tcp, udp as well as iphdr options; the
> joys of new features. ;-)
:) sure..like i mentioned in the cover letter..., i was thinking of
submitting follow up patches for more ip_proto.
since i will be spinning v3, let me see if i can include that too.
>
> I also suggest a comment that these new RTA attributes are used for
> GETROUTE only.
sure
>
> And you need to add the new entries to rtm_ipv4_policy.
>
>
>> __RTA_MAX
>> };
ack,
>>
>> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
>> index 1412a7b..e91ed62 100644
>> --- a/net/ipv4/route.c
>> +++ b/net/ipv4/route.c
>> @@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net,
>> struct flowi4 *flp4,
>> EXPORT_SYMBOL_GPL(ip_route_output_flow);
>>
>> /* called with rcu_read_lock held */
>> -static int rt_fill_info(struct net *net, __be32 dst, __be32 src, u32
>> table_id,
>> - struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
>> - u32 seq)
>> +static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
>> + struct rtable *rt, u32 table_id, struct flowi4 *fl4,
>> + struct sk_buff *skb, u32 portid, u32 seq)
>> {
>> - struct rtable *rt = skb_rtable(skb);
>> struct rtmsg *r;
>> struct nlmsghdr *nlh;
>> unsigned long expires = 0;
>> @@ -2651,6 +2650,14 @@ 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_be16(skb, RTA_SPORT, fl4->fl4_sport))
>> + goto nla_put_failure;
>> +
>> + if (fl4->fl4_dport &&
>> + nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
>> + goto nla_put_failure;
>
> Why return the attributes to the user? I can't see any value in that.
> UID option is not returned either so there is precedence.
hmm..i do see UID returned just 2 lines above. :)
In the least i think it will confirm that the kernel did see your attributes :).
>
>
>> +
>> error = rt->dst.error;
>>
>> if (rt_is_input_route(rt)) {
>> @@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net, __be32 dst,
>> __be32 src, u32 table_id,
>> }
>> } else
>> #endif
>> - if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
>> + if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
>> goto nla_put_failure;
>> }
>>
>> @@ -2683,35 +2690,86 @@ 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_be16(attr);
>
> __be16 p;
>
>> +
>> + if (p <= 0 || p >= 0x)
>> + return -EINVAL;
>
> This check is not needed by definition of be16.
ack, will fix the kbuild sparse warning also for both v4 and v6.
>
>> +
>> + *port = p;
>> + return 0;
>> +}
>> +
>> +static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr
>> *nlh,
>> +__be32 dst, __be32 src, struct flowi4 *fl4,
>> +struct rtable *rt, struct fib_result *res)
>> +{
>> + struct net *net = sock_net(in_skb->sk);
>> + struct rtmsg *rtm = nlmsg_data(nlh);
>> + u32 table_id = RT_TABLE_MAIN;
>> + struct sk_buff *skb;
>> + int err = 0;
>> +
>> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
>> + if (!skb) {
>> + err = -ENOMEM;
>> + return err;
>> +
