On Sun, Feb 25, 2018 at 7:08 PM, David Ahern <d...@cumulusnetworks.com> wrote:
> On 2/24/18 10:44 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu <ro...@cumulusnetworks.com>
>>
>> uapi for ip_proto, sport and dport range match
>> in fib rules.
>>
>> Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com>
>> ---
>> include/net/fib_rules.h | 31 +++++++++++++-
>> include/uapi/linux/fib_rules.h | 8 ++++
>> net/core/fib_rules.c | 94
>> +++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 130 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
>> index b3d2162..6d99202 100644
>> --- a/include/net/fib_rules.h
>> +++ b/include/net/fib_rules.h
>> @@ -11,6 +11,11 @@
>> #include <net/rtnetlink.h>
>> #include <net/fib_notifier.h>
>>
>> +struct fib_port_range {
>> + __u16 start;
>> + __u16 end;
>
> u16 for kernel headers; __u16 is for uapi.
>
ack,
>> +};
>> +
>> struct fib_kuid_range {
>> kuid_t start;
>> kuid_t end;
>> @@ -27,7 +32,7 @@ struct fib_rule {
>> u8 action;
>> u8 l3mdev;
>> u8 proto;
>> - /* 1 byte hole, try to use */
>> + u8 ip_proto;
>> u32 target;
>> __be64 tun_id;
>> struct fib_rule __rcu *ctarget;
>> @@ -40,6 +45,8 @@ struct fib_rule {
>> char iifname[IFNAMSIZ];
>> char oifname[IFNAMSIZ];
>> struct fib_kuid_range uid_range;
>> + struct fib_port_range sport_range;
>> + struct fib_port_range dport_range;
>> struct rcu_head rcu;
>> };
>>
>> @@ -144,6 +151,28 @@ static inline u32 frh_get_table(struct fib_rule_hdr
>> *frh, struct nlattr **nla)
>> return frh->table;
>> }
>>
>> +static inline bool fib_rule_port_inrange(struct fib_port_range *a,
>> + __be16 port)
>> +{
>> + if (!a->start)
>> + return true;
>> + return ntohs(port) >= a->start &&
>> + ntohs(port) <= a->end;
>> +}
>> +
>> +static inline bool fib_rule_port_range_valid(const struct fib_port_range *a)
>> +{
>> + return a->start > 0 && a->end < 0xffff &&
>> + a->start <= a->end;
>> +}
>> +
>> +static inline bool fib_rule_port_range_compare(struct fib_port_range *a,
>> + struct fib_port_range *b)
>> +{
>> + return a->start == b->start &&
>> + a->end == b->end;
>> +}
>> +
>> struct fib_rules_ops *fib_rules_register(const struct fib_rules_ops *,
>> struct net *);
>> void fib_rules_unregister(struct fib_rules_ops *);
>> diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
>> index 77d90ae..232df14 100644
>> --- a/include/uapi/linux/fib_rules.h
>> +++ b/include/uapi/linux/fib_rules.h
>> @@ -35,6 +35,11 @@ struct fib_rule_uid_range {
>> __u32 end;
>> };
>>
>> +struct fib_rule_port_range {
>> + __u16 start;
>> + __u16 end;
>> +};
>> +
>> enum {
>> FRA_UNSPEC,
>> FRA_DST, /* destination address */
>> @@ -59,6 +64,9 @@ enum {
>> FRA_L3MDEV, /* iif or oif is l3mdev goto its table */
>> FRA_UID_RANGE, /* UID range */
>> FRA_PROTOCOL, /* Originator of the rule */
>> + FRA_IP_PROTO, /* ip proto */
>> + FRA_SPORT_RANGE, /* sport */
>> + FRA_DPORT_RANGE, /* dport */
>> __FRA_MAX
>> };
>>
>> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
>> index a6aea80..5008235 100644
>> --- a/net/core/fib_rules.c
>> +++ b/net/core/fib_rules.c
>> @@ -33,6 +33,10 @@ bool fib_rule_matchall(const struct fib_rule *rule)
>> if (!uid_eq(rule->uid_range.start, fib_kuid_range_unset.start) ||
>> !uid_eq(rule->uid_range.end, fib_kuid_range_unset.end))
>> return false;
>> + if (fib_rule_port_range_valid(&rule->sport_range))
>> + return false;
>> + if (fib_rule_port_range_valid(&rule->dport_range))
>> + return false;
>
> Seems like that should be a check that start and end are both not 0.
> Given the uses of fib_rule_port_range_valid, perhaps another helper is
> needed to make this more readable -- e.g., fib_rule_port_range_set --
> which would be used here and fill_rule.
yeah, was trying to not add two helpers. But, i sure can.
>
>
>> return true;
>> }
>> EXPORT_SYMBOL_GPL(fib_rule_matchall);
>> @@ -221,6 +225,12 @@ static int nla_put_uid_range(struct sk_buff *skb,
>> struct fib_kuid_range *range)
>> return nla_put(skb, FRA_UID_RANGE, sizeof(out), &out);
>> }
>>
>> +static int nla_put_port_range(struct sk_buff *skb, int attrtype,
>> + struct fib_port_range *range)
>> +{
>> + return nla_put(skb, attrtype, sizeof(*range), range);
>> +}
>> +
>> static int fib_rule_match(struct fib_rule *rule, struct fib_rules_ops *ops,
>> struct flowi *fl, int flags,
>> struct fib_lookup_arg *arg)
>> @@ -425,6 +435,17 @@ static int rule_exists(struct fib_rules_ops *ops,
>> struct fib_rule_hdr *frh,
>> !uid_eq(r->uid_range.end, rule->uid_range.end))
>> continue;
>>
>> + if (r->ip_proto != rule->ip_proto)
>> + continue;
>> +
>> + if (!fib_rule_port_range_compare(&r->sport_range,
>> + &rule->sport_range))
>> + continue;
>> +
>> + if (!fib_rule_port_range_compare(&r->dport_range,
>> + &rule->dport_range))
>> + continue;
>> +
>> if (!ops->compare(r, frh, tb))
>> continue;
>> return 1;
>> @@ -432,6 +453,20 @@ static int rule_exists(struct fib_rules_ops *ops,
>> struct fib_rule_hdr *frh,
>> return 0;
>> }
>>
>> +static int nla_get_port_range(struct nlattr *pattr,
>> + struct fib_port_range *port_range)
>> +{
>> + const struct fib_port_range *pr = nla_data(pattr);
>
> Isn't the data struct from userspace fib_rule_port_range? that begs the
> question of whether a separate struct definition is needed for kernel
> side. Why not just use the one for both?
yep, will fix. I was following the uid_range code. But, looking back
at it, i think uid range has other reasons to have a separate struct.
will move to one.
>
>
>> +
>> + if (!fib_rule_port_range_valid(pr))
>> + return -EINVAL;
>> +
>> + port_range->start = pr->start;
>> + port_range->end = pr->end;
>> +
>> + return 0;
>> +}
>> +
>> int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr *nlh,
>> struct netlink_ext_ack *extack)
>> {
>> @@ -569,6 +604,23 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr
>> *nlh,
>> rule->uid_range = fib_kuid_range_unset;
>> }
>>
>> + if (tb[FRA_IP_PROTO])
>> + rule->ip_proto = nla_get_u8(tb[FRA_IP_PROTO]);
>> +
>> + if (tb[FRA_SPORT_RANGE]) {
>> + err = nla_get_port_range(tb[FRA_SPORT_RANGE],
>> + &rule->sport_range);
>> + if (err)
>> + goto errout_free;
>> + }
>> +
>> + if (tb[FRA_DPORT_RANGE]) {
>> + err = nla_get_port_range(tb[FRA_DPORT_RANGE],
>> + &rule->dport_range);
>> + if (err)
>> + goto errout_free;
>> + }
>> +
>> if ((nlh->nlmsg_flags & NLM_F_EXCL) &&
>> rule_exists(ops, frh, tb, rule)) {
>> err = -EEXIST;
>> @@ -638,6 +690,8 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr
>> *nlh,
>> struct fib_rule *rule, *r;
>> struct nlattr *tb[FRA_MAX+1];
>> struct fib_kuid_range range;
>> + struct fib_port_range *sprange = NULL;
>> + struct fib_port_range *dprange = NULL;
>> int err = -EINVAL;
>>
>> if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*frh)))
>> @@ -667,6 +721,22 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr
>> *nlh,
>> range = fib_kuid_range_unset;
>> }
>>
>> + if (tb[FRA_SPORT_RANGE]) {
>> + sprange = nla_data(tb[FRA_SPORT_RANGE]);
>> + if (!fib_rule_port_range_valid(sprange)) {
>> + err = -EINVAL;
>> + goto errout;
>> + }
>> + }
>> +
>> + if (tb[FRA_DPORT_RANGE]) {
>> + dprange = nla_data(tb[FRA_DPORT_RANGE]);
>> + if (!fib_rule_port_range_valid(dprange)) {
>> + err = -EINVAL;
>> + goto errout;
>> + }
>> + }
>> +
>> list_for_each_entry(rule, &ops->rules_list, list) {
>> if (tb[FRA_PROTOCOL] &&
>> (rule->proto != nla_get_u8(tb[FRA_PROTOCOL])))
>> @@ -712,6 +782,18 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr
>> *nlh,
>> !uid_eq(rule->uid_range.end, range.end)))
>> continue;
>>
>> + if (tb[FRA_IP_PROTO] &&
>> + (rule->ip_proto != nla_get_u8(tb[FRA_IP_PROTO])))
>> + continue;
>> +
>> + if (sprange &&
>> + !fib_rule_port_range_compare(&rule->sport_range, sprange))
>> + continue;
>> +
>> + if (dprange &&
>> + !fib_rule_port_range_compare(&rule->dport_range, dprange))
>> + continue;
>> +
>> if (!ops->compare(rule, frh, tb))
>> continue;
>>
>> @@ -790,7 +872,10 @@ static inline size_t fib_rule_nlmsg_size(struct
>> fib_rules_ops *ops,
>> + nla_total_size(4) /* FRA_FWMASK */
>> + nla_total_size_64bit(8) /* FRA_TUN_ID */
>> + nla_total_size(sizeof(struct fib_kuid_range))
>> - + nla_total_size(1); /* FRA_PROTOCOL */
>> + + nla_total_size(1) /* FRA_PROTOCOL */
>> + + nla_total_size(1) /* FRA_IP_PROTO */
>> + + nla_total_size(sizeof(struct fib_port_range)) /*
>> FRA_SPORT_RANGE */
>> + + nla_total_size(sizeof(struct fib_port_range)); /*
>> FRA_DPORT_RANGE */
>>
>> if (ops->nlmsg_payload)
>> payload += ops->nlmsg_payload(rule);
>> @@ -855,7 +940,12 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct
>> fib_rule *rule,
>> (rule->l3mdev &&
>> nla_put_u8(skb, FRA_L3MDEV, rule->l3mdev)) ||
>> (uid_range_set(&rule->uid_range) &&
>> - nla_put_uid_range(skb, &rule->uid_range)))
>> + nla_put_uid_range(skb, &rule->uid_range)) ||
>> + (fib_rule_port_range_valid(&rule->sport_range) &&
>> + nla_put_port_range(skb, FRA_SPORT_RANGE, &rule->sport_range)) ||
>> + (fib_rule_port_range_valid(&rule->dport_range) &&
>> + nla_put_port_range(skb, FRA_DPORT_RANGE, &rule->dport_range)) ||
>> + (rule->ip_proto && nla_put_u8(skb, FRA_IP_PROTO, rule->ip_proto)))
>> goto nla_put_failure;
>>
>> if (rule->suppress_ifgroup != -1) {
>>
>
