* [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
@ 2018-02-25 5:44 ` Roopa Prabhu
2018-02-25 15:04 ` Nikolay Aleksandrov
2018-02-26 3:08 ` David Ahern
2018-02-25 5:44 ` [PATCH net-next 2/5] ipv4: fib_rules: support match on sport, dport and ip proto Roopa Prabhu
` (4 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 5:44 UTC (permalink / raw)
To: davem, netdev; +Cc: dsa, nikolay, idosch
From: Roopa Prabhu <roopa@cumulusnetworks.com>
uapi for ip_proto, sport and dport range match
in fib rules.
Signed-off-by: Roopa Prabhu <roopa@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;
+};
+
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;
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);
+
+ 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) {
--
2.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-25 5:44 ` [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport Roopa Prabhu
@ 2018-02-25 15:04 ` Nikolay Aleksandrov
2018-02-25 15:27 ` Nikolay Aleksandrov
2018-02-25 17:58 ` Roopa Prabhu
2018-02-26 3:08 ` David Ahern
1 sibling, 2 replies; 21+ messages in thread
From: Nikolay Aleksandrov @ 2018-02-25 15:04 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: dsa, idosch
On 25/02/18 07:44, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> uapi for ip_proto, sport and dport range match
> in fib rules.
>
> Signed-off-by: Roopa Prabhu <roopa@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(-)
>
You should probably update validate_rulemsg() as well, these aren't added in the per-proto
policies and nothing validates if the attribute data is actually there. Maybe I'm missing
something obvious, but it looks like many other FRA_ attributes don't have such checks.
> 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;
> +};
> +
> 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;
Can start be == 0 ?
IIUC this check is unnecessary because when you're adding the new rule,
you do a check for start > 0 so it shouldn't be possible to be 0.
> + 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;
nit: alignment (also can be on a single line)
> +}
> +
> +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;
nit: alignment (also can be on a single line)
> +}
> +
> 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;
> 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);
> +
> + 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])))
nit: no need for the extra braces
> + 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) {
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-25 15:04 ` Nikolay Aleksandrov
@ 2018-02-25 15:27 ` Nikolay Aleksandrov
2018-02-25 17:58 ` Roopa Prabhu
1 sibling, 0 replies; 21+ messages in thread
From: Nikolay Aleksandrov @ 2018-02-25 15:27 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: dsa, idosch
On 25/02/18 17:04, Nikolay Aleksandrov wrote:
> On 25/02/18 07:44, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> uapi for ip_proto, sport and dport range match
>> in fib rules.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
[snip]
>> 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;
>
> Can start be == 0 ?
> IIUC this check is unnecessary because when you're adding the new rule,
> you do a check for start > 0 so it shouldn't be possible to be 0.
Nevermind this comment, I spoke too soon and saw the match later. :-)
>
>> + 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;
>
> nit: alignment (also can be on a single line)
>
>> +}
>> +
>> +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;
>
> nit: alignment (also can be on a single line)
>
>> +}
>> +
>> 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;
>> 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);
>> +
>> + 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])))
>
> nit: no need for the extra braces
>
>> + 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) {
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-25 15:04 ` Nikolay Aleksandrov
2018-02-25 15:27 ` Nikolay Aleksandrov
@ 2018-02-25 17:58 ` Roopa Prabhu
1 sibling, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 17:58 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: David Miller, netdev, David Ahern, Ido Schimmel
On Sun, Feb 25, 2018 at 7:04 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 25/02/18 07:44, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> uapi for ip_proto, sport and dport range match
>> in fib rules.
>>
>> Signed-off-by: Roopa Prabhu <roopa@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(-)
>>
>
> You should probably update validate_rulemsg() as well, these aren't added in the per-proto
> policies and nothing validates if the attribute data is actually there. Maybe I'm missing
> something obvious, but it looks like many other FRA_ attributes don't have such checks.
yeah, I added the sport checks there initially and later removed it
since I did not see any of the
other FRA_* attributes there. and then ended up adding it in the
respective rule add and del functions where other attributes
were validated for consistency. I can submit a follow on patch to
move all FRA_* attribute validations to validate_rulemsg().
I can sure start with the new attribute validation in this series in
validate_rulemsg in v2
>
>> 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;
>> +};
>> +
>> 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;
>
> Can start be == 0 ?
> IIUC this check is unnecessary because when you're adding the new rule,
> you do a check for start > 0 so it shouldn't be possible to be 0.
>
>> + 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;
>
> nit: alignment (also can be on a single line)
>
>> +}
>> +
>> +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;
>
> nit: alignment (also can be on a single line)
>
>> +}
>> +
>> 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;
>> 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);
>> +
>> + 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])))
>
> nit: no need for the extra braces
>
>> + 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) {
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-25 5:44 ` [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport Roopa Prabhu
2018-02-25 15:04 ` Nikolay Aleksandrov
@ 2018-02-26 3:08 ` David Ahern
2018-02-26 3:38 ` Roopa Prabhu
1 sibling, 1 reply; 21+ messages in thread
From: David Ahern @ 2018-02-26 3:08 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: nikolay, idosch
On 2/24/18 10:44 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> uapi for ip_proto, sport and dport range match
> in fib rules.
>
> Signed-off-by: Roopa Prabhu <roopa@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.
> +};
> +
> 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.
> 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?
> +
> + 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) {
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport
2018-02-26 3:08 ` David Ahern
@ 2018-02-26 3:38 ` Roopa Prabhu
0 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-26 3:38 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, netdev, Nikolay Aleksandrov, Ido Schimmel
On Sun, Feb 25, 2018 at 7:08 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 2/24/18 10:44 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> uapi for ip_proto, sport and dport range match
>> in fib rules.
>>
>> Signed-off-by: Roopa Prabhu <roopa@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) {
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net-next 2/5] ipv4: fib_rules: support match on sport, dport and ip proto
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
2018-02-25 5:44 ` [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport Roopa Prabhu
@ 2018-02-25 5:44 ` Roopa Prabhu
2018-02-25 5:44 ` [PATCH net-next 3/5] ipv6: fib6_rules: support for " Roopa Prabhu
` (3 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 5:44 UTC (permalink / raw)
To: davem, netdev; +Cc: dsa, nikolay, idosch
From: Roopa Prabhu <roopa@cumulusnetworks.com>
support to match on src port, dst port and ip protocol.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
net/ipv4/fib_rules.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 35d646a..9d55c90 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -182,6 +182,15 @@ static int fib4_rule_match(struct fib_rule *rule, struct flowi *fl, int flags)
if (r->tos && (r->tos != fl4->flowi4_tos))
return 0;
+ if (rule->ip_proto && (rule->ip_proto != fl4->flowi4_proto))
+ return 0;
+
+ if (!fib_rule_port_inrange(&rule->sport_range, fl4->fl4_sport))
+ return 0;
+
+ if (!fib_rule_port_inrange(&rule->dport_range, fl4->fl4_dport))
+ return 0;
+
return 1;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH net-next 3/5] ipv6: fib6_rules: support for match on sport, dport and ip proto
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
2018-02-25 5:44 ` [PATCH net-next 1/5] net: fib_rules: support for match on ip_proto, sport and dport Roopa Prabhu
2018-02-25 5:44 ` [PATCH net-next 2/5] ipv4: fib_rules: support match on sport, dport and ip proto Roopa Prabhu
@ 2018-02-25 5:44 ` Roopa Prabhu
2018-02-25 5:44 ` [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it Roopa Prabhu
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 5:44 UTC (permalink / raw)
To: davem, netdev; +Cc: dsa, nikolay, idosch
From: Roopa Prabhu <roopa@cumulusnetworks.com>
support to match on src port, dst port and ip protocol.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
net/ipv6/fib6_rules.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 95a2c9e..678d664 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -223,6 +223,15 @@ static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags)
if (r->tclass && r->tclass != ip6_tclass(fl6->flowlabel))
return 0;
+ if (rule->ip_proto && (rule->ip_proto != fl6->flowi4_proto))
+ return 0;
+
+ if (!fib_rule_port_inrange(&rule->sport_range, fl6->fl4_sport))
+ return 0;
+
+ if (!fib_rule_port_inrange(&rule->dport_range, fl6->fl4_dport))
+ return 0;
+
return 1;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
` (2 preceding siblings ...)
2018-02-25 5:44 ` [PATCH net-next 3/5] ipv6: fib6_rules: support for " Roopa Prabhu
@ 2018-02-25 5:44 ` Roopa Prabhu
2018-02-25 15:08 ` Nikolay Aleksandrov
` (2 more replies)
2018-02-25 5:44 ` [PATCH net-next 5/5] ipv6: " Roopa Prabhu
2018-02-26 3:20 ` [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match David Ahern
5 siblings, 3 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 5:44 UTC (permalink / raw)
To: davem, netdev; +Cc: dsa, nikolay, idosch
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Dissect flow in fwd path if fib rules require it. Controlled by
a flag to avoid penatly for the common case. Flag is set when fib
rules with sport, dport and proto match that require flow dissect
are installed. Also passes the dissected hash keys to the multipath
hash function when applicable to avoid dissecting the flow again.
icmp packets will continue to use inner header for hash
calculations (Thanks to Nikolay Aleksandrov for some review here).
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/net/ip_fib.h | 2 +-
include/net/netns/ipv4.h | 1 +
net/ipv4/fib_rules.c | 6 ++++++
net/ipv4/fib_semantics.c | 2 +-
net/ipv4/route.c | 52 +++++++++++++++++++++++++++++++++++-------------
5 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f805243..5ada772 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
- const struct sk_buff *skb);
+ const struct sk_buff *skb, struct flow_keys *flkeys);
#endif
void fib_select_multipath(struct fib_result *res, int hash);
void fib_select_path(struct net *net, struct fib_result *res,
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 44668c2..87b8fdc 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -52,6 +52,7 @@ struct netns_ipv4 {
#ifdef CONFIG_IP_MULTIPLE_TABLES
struct fib_rules_ops *rules_ops;
bool fib_has_custom_rules;
+ bool fib_rules_require_fldissect;
struct fib_table __rcu *fib_main;
struct fib_table __rcu *fib_default;
#endif
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 9d55c90..83aa786 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
}
#endif
+ if (rule->ip_proto ||
+ fib_rule_port_range_valid(&rule->sport_range) ||
+ fib_rule_port_range_valid(&rule->dport_range))
+ net->ipv4.fib_rules_require_fldissect = true;
+
rule4->src_len = frh->src_len;
rule4->srcmask = inet_make_mask(rule4->src_len);
rule4->dst_len = frh->dst_len;
@@ -398,6 +403,7 @@ int __net_init fib4_rules_init(struct net *net)
goto fail;
net->ipv4.rules_ops = ops;
net->ipv4.fib_has_custom_rules = false;
+ net->ipv4.fib_rules_require_fldissect = false;
return 0;
fail:
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index cd46d76..b0c398a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1770,7 +1770,7 @@ void fib_select_path(struct net *net, struct fib_result *res,
#ifdef CONFIG_IP_ROUTE_MULTIPATH
if (res->fi->fib_nhs > 1) {
- int h = fib_multipath_hash(res->fi, fl4, skb);
+ int h = fib_multipath_hash(res->fi, fl4, skb, NULL);
fib_select_multipath(res, h);
}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 26eefa2..72dd6c6 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1783,7 +1783,7 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
/* if skb is set it will be used and fl4 can be NULL */
int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
- const struct sk_buff *skb)
+ const struct sk_buff *skb, struct flow_keys *flkeys)
{
struct net *net = fi->fib_net;
struct flow_keys hash_keys;
@@ -1810,14 +1810,23 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
if (skb->l4_hash)
return skb_get_hash_raw(skb) >> 1;
memset(&hash_keys, 0, sizeof(hash_keys));
- skb_flow_dissect_flow_keys(skb, &keys, flag);
- hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
- hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
- hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
- hash_keys.ports.src = keys.ports.src;
- hash_keys.ports.dst = keys.ports.dst;
- hash_keys.basic.ip_proto = keys.basic.ip_proto;
+ if (flkeys) {
+ hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
+ hash_keys.addrs.v4addrs.src = flkeys->addrs.v4addrs.src;
+ hash_keys.addrs.v4addrs.dst = flkeys->addrs.v4addrs.dst;
+ hash_keys.ports.src = flkeys->ports.src;
+ hash_keys.ports.dst = flkeys->ports.dst;
+ hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
+ } else {
+ skb_flow_dissect_flow_keys(skb, &keys, flag);
+ hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
+ hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
+ hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
+ hash_keys.ports.src = keys.ports.src;
+ hash_keys.ports.dst = keys.ports.dst;
+ hash_keys.basic.ip_proto = keys.basic.ip_proto;
+ }
} else {
memset(&hash_keys, 0, sizeof(hash_keys));
hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -1838,11 +1847,12 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
static int ip_mkroute_input(struct sk_buff *skb,
struct fib_result *res,
struct in_device *in_dev,
- __be32 daddr, __be32 saddr, u32 tos)
+ __be32 daddr, __be32 saddr, u32 tos,
+ struct flow_keys *hkeys)
{
#ifdef CONFIG_IP_ROUTE_MULTIPATH
if (res->fi && res->fi->fib_nhs > 1) {
- int h = fib_multipath_hash(res->fi, NULL, skb);
+ int h = fib_multipath_hash(res->fi, NULL, skb, hkeys);
fib_select_multipath(res, h);
}
@@ -1868,13 +1878,14 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
struct fib_result *res)
{
struct in_device *in_dev = __in_dev_get_rcu(dev);
+ struct flow_keys *flkeys = NULL, _flkeys;
+ struct net *net = dev_net(dev);
struct ip_tunnel_info *tun_info;
- struct flowi4 fl4;
+ int err = -EINVAL;
unsigned int flags = 0;
u32 itag = 0;
struct rtable *rth;
- int err = -EINVAL;
- struct net *net = dev_net(dev);
+ struct flowi4 fl4;
bool do_cache;
/* IP on this device is disabled. */
@@ -1933,6 +1944,19 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
fl4.daddr = daddr;
fl4.saddr = saddr;
fl4.flowi4_uid = sock_net_uid(net, NULL);
+
+#ifdef CONFIG_IP_MULTIPLE_TABLES
+ if (net->ipv4.fib_rules_require_fldissect) {
+ unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
+
+ memset(&_flkeys, 0, sizeof(_flkeys));
+ skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
+ fl4.fl4_sport = _flkeys.ports.src;
+ fl4.fl4_dport = _flkeys.ports.dst;
+ fl4.flowi4_proto = _flkeys.basic.ip_proto;
+ flkeys = &_flkeys;
+ }
+#endif
err = fib_lookup(net, &fl4, res, 0);
if (err != 0) {
if (!IN_DEV_FORWARD(in_dev))
@@ -1958,7 +1982,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res->type != RTN_UNICAST)
goto martian_destination;
- err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos);
+ err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
out: return err;
brd_input:
--
2.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 ` [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it Roopa Prabhu
@ 2018-02-25 15:08 ` Nikolay Aleksandrov
2018-02-26 3:17 ` David Ahern
2018-02-26 9:10 ` Paolo Abeni
2 siblings, 0 replies; 21+ messages in thread
From: Nikolay Aleksandrov @ 2018-02-25 15:08 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: dsa, idosch
On 25/02/18 07:44, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Dissect flow in fwd path if fib rules require it. Controlled by
> a flag to avoid penatly for the common case. Flag is set when fib
> rules with sport, dport and proto match that require flow dissect
> are installed. Also passes the dissected hash keys to the multipath
> hash function when applicable to avoid dissecting the flow again.
> icmp packets will continue to use inner header for hash
> calculations (Thanks to Nikolay Aleksandrov for some review here).
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/net/ip_fib.h | 2 +-
> include/net/netns/ipv4.h | 1 +
> net/ipv4/fib_rules.c | 6 ++++++
> net/ipv4/fib_semantics.c | 2 +-
> net/ipv4/route.c | 52 +++++++++++++++++++++++++++++++++++-------------
> 5 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index f805243..5ada772 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
>
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> - const struct sk_buff *skb);
> + const struct sk_buff *skb, struct flow_keys *flkeys);
> #endif
> void fib_select_multipath(struct fib_result *res, int hash);
> void fib_select_path(struct net *net, struct fib_result *res,
> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> index 44668c2..87b8fdc 100644
> --- a/include/net/netns/ipv4.h
> +++ b/include/net/netns/ipv4.h
> @@ -52,6 +52,7 @@ struct netns_ipv4 {
> #ifdef CONFIG_IP_MULTIPLE_TABLES
> struct fib_rules_ops *rules_ops;
> bool fib_has_custom_rules;
> + bool fib_rules_require_fldissect;
> struct fib_table __rcu *fib_main;
> struct fib_table __rcu *fib_default;
> #endif
> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
> index 9d55c90..83aa786 100644
> --- a/net/ipv4/fib_rules.c
> +++ b/net/ipv4/fib_rules.c
> @@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
> }
> #endif
>
> + if (rule->ip_proto ||
> + fib_rule_port_range_valid(&rule->sport_range) ||
> + fib_rule_port_range_valid(&rule->dport_range))
> + net->ipv4.fib_rules_require_fldissect = true;
> +
> rule4->src_len = frh->src_len;
> rule4->srcmask = inet_make_mask(rule4->src_len);
> rule4->dst_len = frh->dst_len;
> @@ -398,6 +403,7 @@ int __net_init fib4_rules_init(struct net *net)
> goto fail;
> net->ipv4.rules_ops = ops;
> net->ipv4.fib_has_custom_rules = false;
> + net->ipv4.fib_rules_require_fldissect = false;
> return 0;
>
> fail:
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index cd46d76..b0c398a 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1770,7 +1770,7 @@ void fib_select_path(struct net *net, struct fib_result *res,
>
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> if (res->fi->fib_nhs > 1) {
> - int h = fib_multipath_hash(res->fi, fl4, skb);
> + int h = fib_multipath_hash(res->fi, fl4, skb, NULL);
>
> fib_select_multipath(res, h);
> }
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 26eefa2..72dd6c6 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1783,7 +1783,7 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
>
> /* if skb is set it will be used and fl4 can be NULL */
> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> - const struct sk_buff *skb)
> + const struct sk_buff *skb, struct flow_keys *flkeys)
> {
> struct net *net = fi->fib_net;
> struct flow_keys hash_keys;
> @@ -1810,14 +1810,23 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> if (skb->l4_hash)
> return skb_get_hash_raw(skb) >> 1;
> memset(&hash_keys, 0, sizeof(hash_keys));
> - skb_flow_dissect_flow_keys(skb, &keys, flag);
>
> - hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> - hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
> - hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
> - hash_keys.ports.src = keys.ports.src;
> - hash_keys.ports.dst = keys.ports.dst;
> - hash_keys.basic.ip_proto = keys.basic.ip_proto;
> + if (flkeys) {
> + hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> + hash_keys.addrs.v4addrs.src = flkeys->addrs.v4addrs.src;
> + hash_keys.addrs.v4addrs.dst = flkeys->addrs.v4addrs.dst;
> + hash_keys.ports.src = flkeys->ports.src;
> + hash_keys.ports.dst = flkeys->ports.dst;
> + hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
> + } else {
> + skb_flow_dissect_flow_keys(skb, &keys, flag);
> + hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> + hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
> + hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
> + hash_keys.ports.src = keys.ports.src;
> + hash_keys.ports.dst = keys.ports.dst;
> + hash_keys.basic.ip_proto = keys.basic.ip_proto;
> + }
> } else {
> memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> @@ -1838,11 +1847,12 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> static int ip_mkroute_input(struct sk_buff *skb,
> struct fib_result *res,
> struct in_device *in_dev,
> - __be32 daddr, __be32 saddr, u32 tos)
> + __be32 daddr, __be32 saddr, u32 tos,
> + struct flow_keys *hkeys)
> {
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> if (res->fi && res->fi->fib_nhs > 1) {
> - int h = fib_multipath_hash(res->fi, NULL, skb);
> + int h = fib_multipath_hash(res->fi, NULL, skb, hkeys);
>
> fib_select_multipath(res, h);
> }
> @@ -1868,13 +1878,14 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> struct fib_result *res)
> {
> struct in_device *in_dev = __in_dev_get_rcu(dev);
> + struct flow_keys *flkeys = NULL, _flkeys;
> + struct net *net = dev_net(dev);
> struct ip_tunnel_info *tun_info;
> - struct flowi4 fl4;
> + int err = -EINVAL;
> unsigned int flags = 0;
> u32 itag = 0;
> struct rtable *rth;
> - int err = -EINVAL;
> - struct net *net = dev_net(dev);
> + struct flowi4 fl4;
> bool do_cache;
>
> /* IP on this device is disabled. */
> @@ -1933,6 +1944,19 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> fl4.daddr = daddr;
> fl4.saddr = saddr;
> fl4.flowi4_uid = sock_net_uid(net, NULL);
> +
> +#ifdef CONFIG_IP_MULTIPLE_TABLES
> + if (net->ipv4.fib_rules_require_fldissect) {
> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +
> + memset(&_flkeys, 0, sizeof(_flkeys));
skb_flow_dissect_flow_keys() does the memset for you.
> + skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
> + fl4.fl4_sport = _flkeys.ports.src;
> + fl4.fl4_dport = _flkeys.ports.dst;
> + fl4.flowi4_proto = _flkeys.basic.ip_proto;
> + flkeys = &_flkeys;
> + }
> +#endif
> err = fib_lookup(net, &fl4, res, 0);
> if (err != 0) {
> if (!IN_DEV_FORWARD(in_dev))
> @@ -1958,7 +1982,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> if (res->type != RTN_UNICAST)
> goto martian_destination;
>
> - err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos);
> + err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
> out: return err;
>
> brd_input:
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 ` [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it Roopa Prabhu
2018-02-25 15:08 ` Nikolay Aleksandrov
@ 2018-02-26 3:17 ` David Ahern
2018-02-26 9:10 ` Paolo Abeni
2 siblings, 0 replies; 21+ messages in thread
From: David Ahern @ 2018-02-26 3:17 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: nikolay, idosch
On 2/24/18 10:44 PM, Roopa Prabhu wrote:
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 26eefa2..72dd6c6 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1783,7 +1783,7 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
>
> /* if skb is set it will be used and fl4 can be NULL */
update that comment for flow_keys.
> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> - const struct sk_buff *skb)
> + const struct sk_buff *skb, struct flow_keys *flkeys)
> {
> struct net *net = fi->fib_net;
> struct flow_keys hash_keys;
> @@ -1810,14 +1810,23 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> if (skb->l4_hash)
> return skb_get_hash_raw(skb) >> 1;
> memset(&hash_keys, 0, sizeof(hash_keys));
> - skb_flow_dissect_flow_keys(skb, &keys, flag);
>
> - hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> - hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
> - hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
> - hash_keys.ports.src = keys.ports.src;
> - hash_keys.ports.dst = keys.ports.dst;
> - hash_keys.basic.ip_proto = keys.basic.ip_proto;
> + if (flkeys) {
> + hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> + hash_keys.addrs.v4addrs.src = flkeys->addrs.v4addrs.src;
> + hash_keys.addrs.v4addrs.dst = flkeys->addrs.v4addrs.dst;
> + hash_keys.ports.src = flkeys->ports.src;
> + hash_keys.ports.dst = flkeys->ports.dst;
> + hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
> + } else {
> + skb_flow_dissect_flow_keys(skb, &keys, flag);
> + hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> + hash_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
> + hash_keys.addrs.v4addrs.dst = keys.addrs.v4addrs.dst;
> + hash_keys.ports.src = keys.ports.src;
> + hash_keys.ports.dst = keys.ports.dst;
> + hash_keys.basic.ip_proto = keys.basic.ip_proto;
> + }
> } else {
> memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> @@ -1838,11 +1847,12 @@ int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> static int ip_mkroute_input(struct sk_buff *skb,
> struct fib_result *res,
> struct in_device *in_dev,
> - __be32 daddr, __be32 saddr, u32 tos)
> + __be32 daddr, __be32 saddr, u32 tos,
> + struct flow_keys *hkeys)
> {
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> if (res->fi && res->fi->fib_nhs > 1) {
> - int h = fib_multipath_hash(res->fi, NULL, skb);
> + int h = fib_multipath_hash(res->fi, NULL, skb, hkeys);
>
> fib_select_multipath(res, h);
> }
> @@ -1868,13 +1878,14 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> struct fib_result *res)
> {
> struct in_device *in_dev = __in_dev_get_rcu(dev);
> + struct flow_keys *flkeys = NULL, _flkeys;
> + struct net *net = dev_net(dev);
> struct ip_tunnel_info *tun_info;
> - struct flowi4 fl4;
> + int err = -EINVAL;
> unsigned int flags = 0;
> u32 itag = 0;
> struct rtable *rth;
> - int err = -EINVAL;
> - struct net *net = dev_net(dev);
> + struct flowi4 fl4;
> bool do_cache;
>
> /* IP on this device is disabled. */
> @@ -1933,6 +1944,19 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> fl4.daddr = daddr;
> fl4.saddr = saddr;
> fl4.flowi4_uid = sock_net_uid(net, NULL);
> +
> +#ifdef CONFIG_IP_MULTIPLE_TABLES
> + if (net->ipv4.fib_rules_require_fldissect) {
> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +
> + memset(&_flkeys, 0, sizeof(_flkeys));
> + skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
> + fl4.fl4_sport = _flkeys.ports.src;
> + fl4.fl4_dport = _flkeys.ports.dst;
> + fl4.flowi4_proto = _flkeys.basic.ip_proto;
> + flkeys = &_flkeys;
> + }
> +#endif
I think a helper that compiles out if the config is disabled would be
better; this function is already long enough.
> err = fib_lookup(net, &fl4, res, 0);
> if (err != 0) {
> if (!IN_DEV_FORWARD(in_dev))
> @@ -1958,7 +1982,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> if (res->type != RTN_UNICAST)
> goto martian_destination;
>
> - err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos);
> + err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
> out: return err;
>
> brd_input:
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 ` [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it Roopa Prabhu
2018-02-25 15:08 ` Nikolay Aleksandrov
2018-02-26 3:17 ` David Ahern
@ 2018-02-26 9:10 ` Paolo Abeni
2018-02-26 15:36 ` Roopa Prabhu
2 siblings, 1 reply; 21+ messages in thread
From: Paolo Abeni @ 2018-02-26 9:10 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: dsa, nikolay, idosch
On Sat, 2018-02-24 at 21:44 -0800, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Dissect flow in fwd path if fib rules require it. Controlled by
> a flag to avoid penatly for the common case. Flag is set when fib
> rules with sport, dport and proto match that require flow dissect
> are installed. Also passes the dissected hash keys to the multipath
> hash function when applicable to avoid dissecting the flow again.
> icmp packets will continue to use inner header for hash
> calculations (Thanks to Nikolay Aleksandrov for some review here).
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/net/ip_fib.h | 2 +-
> include/net/netns/ipv4.h | 1 +
> net/ipv4/fib_rules.c | 6 ++++++
> net/ipv4/fib_semantics.c | 2 +-
> net/ipv4/route.c | 52 +++++++++++++++++++++++++++++++++++-------------
> 5 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index f805243..5ada772 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
>
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
> - const struct sk_buff *skb);
> + const struct sk_buff *skb, struct flow_keys *flkeys);
> #endif
> void fib_select_multipath(struct fib_result *res, int hash);
> void fib_select_path(struct net *net, struct fib_result *res,
> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> index 44668c2..87b8fdc 100644
> --- a/include/net/netns/ipv4.h
> +++ b/include/net/netns/ipv4.h
> @@ -52,6 +52,7 @@ struct netns_ipv4 {
> #ifdef CONFIG_IP_MULTIPLE_TABLES
> struct fib_rules_ops *rules_ops;
> bool fib_has_custom_rules;
> + bool fib_rules_require_fldissect;
> struct fib_table __rcu *fib_main;
> struct fib_table __rcu *fib_default;
> #endif
> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
> index 9d55c90..83aa786 100644
> --- a/net/ipv4/fib_rules.c
> +++ b/net/ipv4/fib_rules.c
> @@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
> }
> #endif
>
> + if (rule->ip_proto ||
> + fib_rule_port_range_valid(&rule->sport_range) ||
> + fib_rule_port_range_valid(&rule->dport_range))
> + net->ipv4.fib_rules_require_fldissect = true;
> +
> rule4->src_len = frh->src_len;
> rule4->srcmask = inet_make_mask(rule4->src_len);
> rule4->dst_len = frh->dst_len;
What about using 'fib_rules_require_fldissect' to conditionally avoid
all the tests introduced by patch 2/5 ? Perhaps even using a static key
for that?
It would be great if the kernel would be able to clear this flag when
no more needed. I know it's not the current behaviour for other similar
flags, but I hope we can improve ;)
Both points above also apply to the ipv6 code path.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it
2018-02-26 9:10 ` Paolo Abeni
@ 2018-02-26 15:36 ` Roopa Prabhu
0 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-26 15:36 UTC (permalink / raw)
To: Paolo Abeni
Cc: David Miller, netdev, David Ahern, Nikolay Aleksandrov,
Ido Schimmel
On Mon, Feb 26, 2018 at 1:10 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Sat, 2018-02-24 at 21:44 -0800, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> Dissect flow in fwd path if fib rules require it. Controlled by
>> a flag to avoid penatly for the common case. Flag is set when fib
>> rules with sport, dport and proto match that require flow dissect
>> are installed. Also passes the dissected hash keys to the multipath
>> hash function when applicable to avoid dissecting the flow again.
>> icmp packets will continue to use inner header for hash
>> calculations (Thanks to Nikolay Aleksandrov for some review here).
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/net/ip_fib.h | 2 +-
>> include/net/netns/ipv4.h | 1 +
>> net/ipv4/fib_rules.c | 6 ++++++
>> net/ipv4/fib_semantics.c | 2 +-
>> net/ipv4/route.c | 52 +++++++++++++++++++++++++++++++++++-------------
>> 5 files changed, 47 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>> index f805243..5ada772 100644
>> --- a/include/net/ip_fib.h
>> +++ b/include/net/ip_fib.h
>> @@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
>>
>> #ifdef CONFIG_IP_ROUTE_MULTIPATH
>> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
>> - const struct sk_buff *skb);
>> + const struct sk_buff *skb, struct flow_keys *flkeys);
>> #endif
>> void fib_select_multipath(struct fib_result *res, int hash);
>> void fib_select_path(struct net *net, struct fib_result *res,
>> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
>> index 44668c2..87b8fdc 100644
>> --- a/include/net/netns/ipv4.h
>> +++ b/include/net/netns/ipv4.h
>> @@ -52,6 +52,7 @@ struct netns_ipv4 {
>> #ifdef CONFIG_IP_MULTIPLE_TABLES
>> struct fib_rules_ops *rules_ops;
>> bool fib_has_custom_rules;
>> + bool fib_rules_require_fldissect;
>> struct fib_table __rcu *fib_main;
>> struct fib_table __rcu *fib_default;
>> #endif
>> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
>> index 9d55c90..83aa786 100644
>> --- a/net/ipv4/fib_rules.c
>> +++ b/net/ipv4/fib_rules.c
>> @@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
>> }
>> #endif
>>
>> + if (rule->ip_proto ||
>> + fib_rule_port_range_valid(&rule->sport_range) ||
>> + fib_rule_port_range_valid(&rule->dport_range))
>> + net->ipv4.fib_rules_require_fldissect = true;
>> +
>> rule4->src_len = frh->src_len;
>> rule4->srcmask = inet_make_mask(rule4->src_len);
>> rule4->dst_len = frh->dst_len;
>
> What about using 'fib_rules_require_fldissect' to conditionally avoid
> all the tests introduced by patch 2/5 ? Perhaps even using a static key
> for that?
yep, ack
>
> It would be great if the kernel would be able to clear this flag when
> no more needed. I know it's not the current behaviour for other similar
> flags, but I hope we can improve ;)
>
yep agreed. I did lean on the existing fib rule flags behavior.
Nikolay suggested I can move this to a counter (which i had been
debating as well).
that gives the ability to clear on deletes.
> Both points above also apply to the ipv6 code path.
>
ack.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net-next 5/5] ipv6: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
` (3 preceding siblings ...)
2018-02-25 5:44 ` [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it Roopa Prabhu
@ 2018-02-25 5:44 ` Roopa Prabhu
2018-02-25 15:10 ` Nikolay Aleksandrov
2018-02-26 3:19 ` David Ahern
2018-02-26 3:20 ` [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match David Ahern
5 siblings, 2 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 5:44 UTC (permalink / raw)
To: davem, netdev; +Cc: dsa, nikolay, idosch
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Dissect flow in fwd path if fib rules require it. Controlled by
a flag to avoid penatly for the common case. Flag is set when fib
rules with sport, dport and proto match that require flow dissect
are installed. Also passes the dissected hash keys to the multipath
hash function when applicable to avoid dissecting the flow again.
icmp packets will continue to use inner header for hash
calculations.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/net/ip6_route.h | 3 ++-
include/net/netns/ipv6.h | 1 +
net/ipv6/fib6_rules.c | 5 +++++
net/ipv6/icmp.c | 2 +-
net/ipv6/route.c | 45 ++++++++++++++++++++++++++++++++++++---------
5 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 27d23a6..218f89c 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -127,7 +127,8 @@ static inline int ip6_route_get_saddr(struct net *net, struct rt6_info *rt,
struct rt6_info *rt6_lookup(struct net *net, const struct in6_addr *daddr,
const struct in6_addr *saddr, int oif, int flags);
-u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb);
+u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
+ struct flow_keys *hkeys);
struct dst_entry *icmp6_dst_alloc(struct net_device *dev, struct flowi6 *fl6);
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 987cc45..7aca00e 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -72,6 +72,7 @@ struct netns_ipv6 {
unsigned long ip6_rt_last_gc;
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
bool fib6_has_custom_rules;
+ bool fib6_rules_require_fldissect;
struct rt6_info *ip6_prohibit_entry;
struct rt6_info *ip6_blk_hole_entry;
struct fib6_table *fib6_local_tbl;
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 678d664..e3a7861 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -267,6 +267,11 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
rule6->dst.plen = frh->dst_len;
rule6->tclass = frh->tos;
+ if (rule->ip_proto ||
+ fib_rule_port_range_valid(&rule->sport_range) ||
+ fib_rule_port_range_valid(&rule->dport_range))
+ net->ipv6.fib6_rules_require_fldissect = true;
+
net->ipv6.fib6_has_custom_rules = true;
err = 0;
errout:
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 4fa4f1b..b0778d3 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -522,7 +522,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
fl6.fl6_icmp_type = type;
fl6.fl6_icmp_code = code;
fl6.flowi6_uid = sock_net_uid(net, NULL);
- fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
+ fl6.mp_hash = rt6_multipath_hash(&fl6, skb, NULL);
security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
sk = icmpv6_xmit_lock(net);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index aa709b6..778212b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -460,7 +460,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
* case it will always be non-zero. Otherwise now is the time to do it.
*/
if (!fl6->mp_hash)
- fl6->mp_hash = rt6_multipath_hash(fl6, NULL);
+ fl6->mp_hash = rt6_multipath_hash(fl6, NULL, NULL);
if (fl6->mp_hash <= atomic_read(&match->rt6i_nh_upper_bound))
return match;
@@ -1786,10 +1786,12 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
static void ip6_multipath_l3_keys(const struct sk_buff *skb,
- struct flow_keys *keys)
+ struct flow_keys *keys,
+ struct flow_keys *flkeys)
{
const struct ipv6hdr *outer_iph = ipv6_hdr(skb);
const struct ipv6hdr *key_iph = outer_iph;
+ struct flow_keys *_flkeys = flkeys;
const struct ipv6hdr *inner_iph;
const struct icmp6hdr *icmph;
struct ipv6hdr _inner_iph;
@@ -1811,22 +1813,31 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
goto out;
key_iph = inner_iph;
+ _flkeys = NULL;
out:
memset(keys, 0, sizeof(*keys));
keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
- keys->addrs.v6addrs.src = key_iph->saddr;
- keys->addrs.v6addrs.dst = key_iph->daddr;
- keys->tags.flow_label = ip6_flowinfo(key_iph);
- keys->basic.ip_proto = key_iph->nexthdr;
+ if (_flkeys) {
+ keys->addrs.v6addrs.src = _flkeys->addrs.v6addrs.src;
+ keys->addrs.v6addrs.dst = _flkeys->addrs.v6addrs.dst;
+ keys->tags.flow_label = _flkeys->tags.flow_label;
+ keys->basic.ip_proto = _flkeys->basic.ip_proto;
+ } else {
+ keys->addrs.v6addrs.src = key_iph->saddr;
+ keys->addrs.v6addrs.dst = key_iph->daddr;
+ keys->tags.flow_label = ip6_flowinfo(key_iph);
+ keys->basic.ip_proto = key_iph->nexthdr;
+ }
}
/* if skb is set it will be used and fl6 can be NULL */
-u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb)
+u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
+ struct flow_keys *flkeys)
{
struct flow_keys hash_keys;
if (skb) {
- ip6_multipath_l3_keys(skb, &hash_keys);
+ ip6_multipath_l3_keys(skb, &hash_keys, flkeys);
return flow_hash_from_keys(&hash_keys) >> 1;
}
@@ -1847,12 +1858,27 @@ void ip6_route_input(struct sk_buff *skb)
.flowi6_mark = skb->mark,
.flowi6_proto = iph->nexthdr,
};
+ struct flow_keys *flkeys = NULL, _flkeys;
tun_info = skb_tunnel_info(skb);
if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
+
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
+ if (net->ipv6.fib6_rules_require_fldissect) {
+ unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
+
+ memset(&_flkeys, 0, sizeof(_flkeys));
+ skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
+ fl6.fl6_sport = _flkeys.ports.src;
+ fl6.fl6_dport = _flkeys.ports.dst;
+ fl6.flowi6_proto = _flkeys.basic.ip_proto;
+ flkeys = &_flkeys;
+ }
+#endif
+
if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
- fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
+ fl6.mp_hash = rt6_multipath_hash(&fl6, skb, flkeys);
skb_dst_drop(skb);
skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
}
@@ -4896,6 +4922,7 @@ static int __net_init ip6_route_net_init(struct net *net)
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
net->ipv6.fib6_has_custom_rules = false;
+ net->ipv6.fib6_rules_require_fldissect = false;
net->ipv6.ip6_prohibit_entry = kmemdup(&ip6_prohibit_entry_template,
sizeof(*net->ipv6.ip6_prohibit_entry),
GFP_KERNEL);
--
2.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net-next 5/5] ipv6: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 ` [PATCH net-next 5/5] ipv6: " Roopa Prabhu
@ 2018-02-25 15:10 ` Nikolay Aleksandrov
2018-02-25 17:51 ` Roopa Prabhu
2018-02-26 3:19 ` David Ahern
1 sibling, 1 reply; 21+ messages in thread
From: Nikolay Aleksandrov @ 2018-02-25 15:10 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: dsa, idosch
On 25/02/18 07:44, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Dissect flow in fwd path if fib rules require it. Controlled by
> a flag to avoid penatly for the common case. Flag is set when fib
> rules with sport, dport and proto match that require flow dissect
> are installed. Also passes the dissected hash keys to the multipath
> hash function when applicable to avoid dissecting the flow again.
> icmp packets will continue to use inner header for hash
> calculations.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/net/ip6_route.h | 3 ++-
> include/net/netns/ipv6.h | 1 +
> net/ipv6/fib6_rules.c | 5 +++++
> net/ipv6/icmp.c | 2 +-
> net/ipv6/route.c | 45 ++++++++++++++++++++++++++++++++++++---------
> 5 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 27d23a6..218f89c 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -127,7 +127,8 @@ static inline int ip6_route_get_saddr(struct net *net, struct rt6_info *rt,
>
> struct rt6_info *rt6_lookup(struct net *net, const struct in6_addr *daddr,
> const struct in6_addr *saddr, int oif, int flags);
> -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb);
> +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
> + struct flow_keys *hkeys);
>
> struct dst_entry *icmp6_dst_alloc(struct net_device *dev, struct flowi6 *fl6);
>
> diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
> index 987cc45..7aca00e 100644
> --- a/include/net/netns/ipv6.h
> +++ b/include/net/netns/ipv6.h
> @@ -72,6 +72,7 @@ struct netns_ipv6 {
> unsigned long ip6_rt_last_gc;
> #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> bool fib6_has_custom_rules;
> + bool fib6_rules_require_fldissect;
> struct rt6_info *ip6_prohibit_entry;
> struct rt6_info *ip6_blk_hole_entry;
> struct fib6_table *fib6_local_tbl;
> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> index 678d664..e3a7861 100644
> --- a/net/ipv6/fib6_rules.c
> +++ b/net/ipv6/fib6_rules.c
> @@ -267,6 +267,11 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
> rule6->dst.plen = frh->dst_len;
> rule6->tclass = frh->tos;
>
> + if (rule->ip_proto ||
> + fib_rule_port_range_valid(&rule->sport_range) ||
> + fib_rule_port_range_valid(&rule->dport_range))
> + net->ipv6.fib6_rules_require_fldissect = true;
> +
> net->ipv6.fib6_has_custom_rules = true;
> err = 0;
> errout:
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index 4fa4f1b..b0778d3 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -522,7 +522,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
> fl6.fl6_icmp_type = type;
> fl6.fl6_icmp_code = code;
> fl6.flowi6_uid = sock_net_uid(net, NULL);
> - fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
> + fl6.mp_hash = rt6_multipath_hash(&fl6, skb, NULL);
> security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
>
> sk = icmpv6_xmit_lock(net);
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index aa709b6..778212b 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -460,7 +460,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
> * case it will always be non-zero. Otherwise now is the time to do it.
> */
> if (!fl6->mp_hash)
> - fl6->mp_hash = rt6_multipath_hash(fl6, NULL);
> + fl6->mp_hash = rt6_multipath_hash(fl6, NULL, NULL);
>
> if (fl6->mp_hash <= atomic_read(&match->rt6i_nh_upper_bound))
> return match;
> @@ -1786,10 +1786,12 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
> EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
>
> static void ip6_multipath_l3_keys(const struct sk_buff *skb,
> - struct flow_keys *keys)
> + struct flow_keys *keys,
> + struct flow_keys *flkeys)
> {
> const struct ipv6hdr *outer_iph = ipv6_hdr(skb);
> const struct ipv6hdr *key_iph = outer_iph;
> + struct flow_keys *_flkeys = flkeys;
> const struct ipv6hdr *inner_iph;
> const struct icmp6hdr *icmph;
> struct ipv6hdr _inner_iph;
> @@ -1811,22 +1813,31 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
> goto out;
>
> key_iph = inner_iph;
> + _flkeys = NULL;
> out:
> memset(keys, 0, sizeof(*keys));
> keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> - keys->addrs.v6addrs.src = key_iph->saddr;
> - keys->addrs.v6addrs.dst = key_iph->daddr;
> - keys->tags.flow_label = ip6_flowinfo(key_iph);
> - keys->basic.ip_proto = key_iph->nexthdr;
> + if (_flkeys) {
> + keys->addrs.v6addrs.src = _flkeys->addrs.v6addrs.src;
> + keys->addrs.v6addrs.dst = _flkeys->addrs.v6addrs.dst;
> + keys->tags.flow_label = _flkeys->tags.flow_label;
> + keys->basic.ip_proto = _flkeys->basic.ip_proto;
> + } else {
> + keys->addrs.v6addrs.src = key_iph->saddr;
> + keys->addrs.v6addrs.dst = key_iph->daddr;
> + keys->tags.flow_label = ip6_flowinfo(key_iph);
> + keys->basic.ip_proto = key_iph->nexthdr;
> + }
> }
>
> /* if skb is set it will be used and fl6 can be NULL */
> -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb)
> +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
> + struct flow_keys *flkeys)
> {
> struct flow_keys hash_keys;
>
> if (skb) {
> - ip6_multipath_l3_keys(skb, &hash_keys);
> + ip6_multipath_l3_keys(skb, &hash_keys, flkeys);
> return flow_hash_from_keys(&hash_keys) >> 1;
> }
>
> @@ -1847,12 +1858,27 @@ void ip6_route_input(struct sk_buff *skb)
> .flowi6_mark = skb->mark,
> .flowi6_proto = iph->nexthdr,
> };
> + struct flow_keys *flkeys = NULL, _flkeys;
>
> tun_info = skb_tunnel_info(skb);
> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
> +
> +#ifdef CONFIG_IPV6_MULTIPLE_TABLES
> + if (net->ipv6.fib6_rules_require_fldissect) {
> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +
> + memset(&_flkeys, 0, sizeof(_flkeys));
Same here, skb_flow_dissect_flow_keys zeroes the flow_keys.
> + skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
> + fl6.fl6_sport = _flkeys.ports.src;
> + fl6.fl6_dport = _flkeys.ports.dst;
> + fl6.flowi6_proto = _flkeys.basic.ip_proto;
> + flkeys = &_flkeys;
> + }
> +#endif
> +
> if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
> - fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
> + fl6.mp_hash = rt6_multipath_hash(&fl6, skb, flkeys);
> skb_dst_drop(skb);
> skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
> }
> @@ -4896,6 +4922,7 @@ static int __net_init ip6_route_net_init(struct net *net)
>
> #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> net->ipv6.fib6_has_custom_rules = false;
> + net->ipv6.fib6_rules_require_fldissect = false;
> net->ipv6.ip6_prohibit_entry = kmemdup(&ip6_prohibit_entry_template,
> sizeof(*net->ipv6.ip6_prohibit_entry),
> GFP_KERNEL);
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 5/5] ipv6: route: dissect flow in input path if fib rules need it
2018-02-25 15:10 ` Nikolay Aleksandrov
@ 2018-02-25 17:51 ` Roopa Prabhu
0 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-25 17:51 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: David Miller, netdev, David Ahern, Ido Schimmel
On Sun, Feb 25, 2018 at 7:10 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 25/02/18 07:44, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> Dissect flow in fwd path if fib rules require it. Controlled by
>> a flag to avoid penatly for the common case. Flag is set when fib
>> rules with sport, dport and proto match that require flow dissect
>> are installed. Also passes the dissected hash keys to the multipath
>> hash function when applicable to avoid dissecting the flow again.
>> icmp packets will continue to use inner header for hash
>> calculations.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/net/ip6_route.h | 3 ++-
>> include/net/netns/ipv6.h | 1 +
>> net/ipv6/fib6_rules.c | 5 +++++
>> net/ipv6/icmp.c | 2 +-
>> net/ipv6/route.c | 45 ++++++++++++++++++++++++++++++++++++---------
>> 5 files changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
>> index 27d23a6..218f89c 100644
>> --- a/include/net/ip6_route.h
>> +++ b/include/net/ip6_route.h
>> @@ -127,7 +127,8 @@ static inline int ip6_route_get_saddr(struct net *net, struct rt6_info *rt,
>>
>> struct rt6_info *rt6_lookup(struct net *net, const struct in6_addr *daddr,
>> const struct in6_addr *saddr, int oif, int flags);
>> -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb);
>> +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
>> + struct flow_keys *hkeys);
>>
>> struct dst_entry *icmp6_dst_alloc(struct net_device *dev, struct flowi6 *fl6);
>>
>> diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
>> index 987cc45..7aca00e 100644
>> --- a/include/net/netns/ipv6.h
>> +++ b/include/net/netns/ipv6.h
>> @@ -72,6 +72,7 @@ struct netns_ipv6 {
>> unsigned long ip6_rt_last_gc;
>> #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>> bool fib6_has_custom_rules;
>> + bool fib6_rules_require_fldissect;
>> struct rt6_info *ip6_prohibit_entry;
>> struct rt6_info *ip6_blk_hole_entry;
>> struct fib6_table *fib6_local_tbl;
>> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
>> index 678d664..e3a7861 100644
>> --- a/net/ipv6/fib6_rules.c
>> +++ b/net/ipv6/fib6_rules.c
>> @@ -267,6 +267,11 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
>> rule6->dst.plen = frh->dst_len;
>> rule6->tclass = frh->tos;
>>
>> + if (rule->ip_proto ||
>> + fib_rule_port_range_valid(&rule->sport_range) ||
>> + fib_rule_port_range_valid(&rule->dport_range))
>> + net->ipv6.fib6_rules_require_fldissect = true;
>> +
>> net->ipv6.fib6_has_custom_rules = true;
>> err = 0;
>> errout:
>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>> index 4fa4f1b..b0778d3 100644
>> --- a/net/ipv6/icmp.c
>> +++ b/net/ipv6/icmp.c
>> @@ -522,7 +522,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
>> fl6.fl6_icmp_type = type;
>> fl6.fl6_icmp_code = code;
>> fl6.flowi6_uid = sock_net_uid(net, NULL);
>> - fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
>> + fl6.mp_hash = rt6_multipath_hash(&fl6, skb, NULL);
>> security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
>>
>> sk = icmpv6_xmit_lock(net);
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index aa709b6..778212b 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -460,7 +460,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
>> * case it will always be non-zero. Otherwise now is the time to do it.
>> */
>> if (!fl6->mp_hash)
>> - fl6->mp_hash = rt6_multipath_hash(fl6, NULL);
>> + fl6->mp_hash = rt6_multipath_hash(fl6, NULL, NULL);
>>
>> if (fl6->mp_hash <= atomic_read(&match->rt6i_nh_upper_bound))
>> return match;
>> @@ -1786,10 +1786,12 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
>> EXPORT_SYMBOL_GPL(ip6_route_input_lookup);
>>
>> static void ip6_multipath_l3_keys(const struct sk_buff *skb,
>> - struct flow_keys *keys)
>> + struct flow_keys *keys,
>> + struct flow_keys *flkeys)
>> {
>> const struct ipv6hdr *outer_iph = ipv6_hdr(skb);
>> const struct ipv6hdr *key_iph = outer_iph;
>> + struct flow_keys *_flkeys = flkeys;
>> const struct ipv6hdr *inner_iph;
>> const struct icmp6hdr *icmph;
>> struct ipv6hdr _inner_iph;
>> @@ -1811,22 +1813,31 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
>> goto out;
>>
>> key_iph = inner_iph;
>> + _flkeys = NULL;
>> out:
>> memset(keys, 0, sizeof(*keys));
>> keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>> - keys->addrs.v6addrs.src = key_iph->saddr;
>> - keys->addrs.v6addrs.dst = key_iph->daddr;
>> - keys->tags.flow_label = ip6_flowinfo(key_iph);
>> - keys->basic.ip_proto = key_iph->nexthdr;
>> + if (_flkeys) {
>> + keys->addrs.v6addrs.src = _flkeys->addrs.v6addrs.src;
>> + keys->addrs.v6addrs.dst = _flkeys->addrs.v6addrs.dst;
>> + keys->tags.flow_label = _flkeys->tags.flow_label;
>> + keys->basic.ip_proto = _flkeys->basic.ip_proto;
>> + } else {
>> + keys->addrs.v6addrs.src = key_iph->saddr;
>> + keys->addrs.v6addrs.dst = key_iph->daddr;
>> + keys->tags.flow_label = ip6_flowinfo(key_iph);
>> + keys->basic.ip_proto = key_iph->nexthdr;
>> + }
>> }
>>
>> /* if skb is set it will be used and fl6 can be NULL */
>> -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb)
>> +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb,
>> + struct flow_keys *flkeys)
>> {
>> struct flow_keys hash_keys;
>>
>> if (skb) {
>> - ip6_multipath_l3_keys(skb, &hash_keys);
>> + ip6_multipath_l3_keys(skb, &hash_keys, flkeys);
>> return flow_hash_from_keys(&hash_keys) >> 1;
>> }
>>
>> @@ -1847,12 +1858,27 @@ void ip6_route_input(struct sk_buff *skb)
>> .flowi6_mark = skb->mark,
>> .flowi6_proto = iph->nexthdr,
>> };
>> + struct flow_keys *flkeys = NULL, _flkeys;
>>
>> tun_info = skb_tunnel_info(skb);
>> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
>> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
>> +
>> +#ifdef CONFIG_IPV6_MULTIPLE_TABLES
>> + if (net->ipv6.fib6_rules_require_fldissect) {
>> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
>> +
>> + memset(&_flkeys, 0, sizeof(_flkeys));
>
> Same here, skb_flow_dissect_flow_keys zeroes the flow_keys.
>
ack, will fix in v2
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 5/5] ipv6: route: dissect flow in input path if fib rules need it
2018-02-25 5:44 ` [PATCH net-next 5/5] ipv6: " Roopa Prabhu
2018-02-25 15:10 ` Nikolay Aleksandrov
@ 2018-02-26 3:19 ` David Ahern
2018-02-26 3:40 ` Roopa Prabhu
1 sibling, 1 reply; 21+ messages in thread
From: David Ahern @ 2018-02-26 3:19 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: nikolay, idosch
On 2/24/18 10:44 PM, Roopa Prabhu wrote:
> @@ -1847,12 +1858,27 @@ void ip6_route_input(struct sk_buff *skb)
> .flowi6_mark = skb->mark,
> .flowi6_proto = iph->nexthdr,
> };
> + struct flow_keys *flkeys = NULL, _flkeys;
>
> tun_info = skb_tunnel_info(skb);
> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
> +
> +#ifdef CONFIG_IPV6_MULTIPLE_TABLES
> + if (net->ipv6.fib6_rules_require_fldissect) {
> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +
> + memset(&_flkeys, 0, sizeof(_flkeys));
> + skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
> + fl6.fl6_sport = _flkeys.ports.src;
> + fl6.fl6_dport = _flkeys.ports.dst;
> + fl6.flowi6_proto = _flkeys.basic.ip_proto;
> + flkeys = &_flkeys;
> + }
> +#endif
same here - helper versus inline.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 5/5] ipv6: route: dissect flow in input path if fib rules need it
2018-02-26 3:19 ` David Ahern
@ 2018-02-26 3:40 ` Roopa Prabhu
0 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-26 3:40 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, netdev, Nikolay Aleksandrov, Ido Schimmel
On Sun, Feb 25, 2018 at 7:19 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 2/24/18 10:44 PM, Roopa Prabhu wrote:
>
>> @@ -1847,12 +1858,27 @@ void ip6_route_input(struct sk_buff *skb)
>> .flowi6_mark = skb->mark,
>> .flowi6_proto = iph->nexthdr,
>> };
>> + struct flow_keys *flkeys = NULL, _flkeys;
>>
>> tun_info = skb_tunnel_info(skb);
>> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
>> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
>> +
>> +#ifdef CONFIG_IPV6_MULTIPLE_TABLES
>> + if (net->ipv6.fib6_rules_require_fldissect) {
>> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
>> +
>> + memset(&_flkeys, 0, sizeof(_flkeys));
>> + skb_flow_dissect_flow_keys(skb, &_flkeys, flag);
>> + fl6.fl6_sport = _flkeys.ports.src;
>> + fl6.fl6_dport = _flkeys.ports.dst;
>> + fl6.flowi6_proto = _flkeys.basic.ip_proto;
>> + flkeys = &_flkeys;
>> + }
>> +#endif
>
> same here - helper versus inline.
>
ack
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match
2018-02-25 5:44 [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match Roopa Prabhu
` (4 preceding siblings ...)
2018-02-25 5:44 ` [PATCH net-next 5/5] ipv6: " Roopa Prabhu
@ 2018-02-26 3:20 ` David Ahern
2018-02-26 3:39 ` Roopa Prabhu
5 siblings, 1 reply; 21+ messages in thread
From: David Ahern @ 2018-02-26 3:20 UTC (permalink / raw)
To: Roopa Prabhu, davem, netdev; +Cc: nikolay, idosch
On 2/24/18 10:44 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This series extends fib rule match support to include sport, dport
> and ip proto match (to complete the 5-tuple match support).
> Common use-cases of Policy based routing in the data center require
> 5-tuple match. The last 2 patches in the series add a call to flow dissect
> in the fwd path if required by the installed fib rules (controlled by a flag).
>
> v1:
> - Fix errors reported by kbuild and feedback on RFC series
> - extend port match uapi to accomodate port ranges
Would be good to have a test script under tools/testing/selftests/net
that covers expectations for good and bad cases. Can create one based on
tools/testing/selftests/net/fib-onlink-tests.sh
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match
2018-02-26 3:20 ` [PATCH net-next 0/5] fib_rules: support sport, dport and ip proto match David Ahern
@ 2018-02-26 3:39 ` Roopa Prabhu
0 siblings, 0 replies; 21+ messages in thread
From: Roopa Prabhu @ 2018-02-26 3:39 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, netdev, Nikolay Aleksandrov, Ido Schimmel
On Sun, Feb 25, 2018 at 7:20 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 2/24/18 10:44 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This series extends fib rule match support to include sport, dport
>> and ip proto match (to complete the 5-tuple match support).
>> Common use-cases of Policy based routing in the data center require
>> 5-tuple match. The last 2 patches in the series add a call to flow dissect
>> in the fwd path if required by the installed fib rules (controlled by a flag).
>>
>> v1:
>> - Fix errors reported by kbuild and feedback on RFC series
>> - extend port match uapi to accomodate port ranges
>
> Would be good to have a test script under tools/testing/selftests/net
> that covers expectations for good and bad cases. Can create one based on
> tools/testing/selftests/net/fib-onlink-tests.sh
yep, that would help. Let me see if I can change my current bash test
script to selftests
^ permalink raw reply [flat|nested] 21+ messages in thread