diff mbox

[net-next] net/sched: cls_flower: Introduce support in SKIP SW flag

Message ID 20160605141118.10986-1-amir@vadai.me
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Amir Vadai June 5, 2016, 2:11 p.m. UTC
From: Amir Vadai <amirva@mellanox.com>

In order to make a filter processed only by hardware, skip_sw flag
should be supplied. This is an addition to the already existing skip_hw
flag (filter will be processed by software only). If no flag is
specified, filter will be processed by both software and hardware.

If only hardware offloaded filters exist, fl_classify() will return
without doing anything.

A following userspace patch will be sent once kernel patch is accepted.

Example:

tc filter add dev enp0s9 protocol ip prio 20 parent ffff: \
	flower \
		ip_proto 6 \
		indev enp0s9 \
		skip_sw \
	action skbedit mark 0x1234

Signed-off-by: Amir Vadai <amirva@mellanox.com>
---
 net/sched/cls_flower.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

Comments

Jiri Pirko June 7, 2016, 12:14 p.m. UTC | #1
Sun, Jun 05, 2016 at 04:11:18PM CEST, amir@vadai.me wrote:
>From: Amir Vadai <amirva@mellanox.com>
>
>In order to make a filter processed only by hardware, skip_sw flag
>should be supplied. This is an addition to the already existing skip_hw
>flag (filter will be processed by software only). If no flag is
>specified, filter will be processed by both software and hardware.
>
>If only hardware offloaded filters exist, fl_classify() will return
>without doing anything.
>
>A following userspace patch will be sent once kernel patch is accepted.
>
>Example:
>
>tc filter add dev enp0s9 protocol ip prio 20 parent ffff: \
>	flower \
>		ip_proto 6 \
>		indev enp0s9 \
>		skip_sw \
>	action skbedit mark 0x1234
>
>Signed-off-by: Amir Vadai <amirva@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
John Fastabend June 7, 2016, 3:37 p.m. UTC | #2
On 16-06-05 07:11 AM, Amir Vadai wrote:
> From: Amir Vadai <amirva@mellanox.com>
> 
> In order to make a filter processed only by hardware, skip_sw flag
> should be supplied. This is an addition to the already existing skip_hw
> flag (filter will be processed by software only). If no flag is
> specified, filter will be processed by both software and hardware.
> 
> If only hardware offloaded filters exist, fl_classify() will return
> without doing anything.
> 
> A following userspace patch will be sent once kernel patch is accepted.
> 
> Example:
> 
> tc filter add dev enp0s9 protocol ip prio 20 parent ffff: \
> 	flower \
> 		ip_proto 6 \
> 		indev enp0s9 \
> 		skip_sw \
> 	action skbedit mark 0x1234
> 
> Signed-off-by: Amir Vadai <amirva@mellanox.com>
> ---



Looks good to me. Although we need to do the same error propagation in
flower that Jakub just added to cls_u32.

Acked-by: John Fastabend <john.r.fastabend@intel.com>
David Miller June 7, 2016, 10:50 p.m. UTC | #3
From: Amir Vadai <amir@vadai.me>
Date: Sun,  5 Jun 2016 17:11:18 +0300

> From: Amir Vadai <amirva@mellanox.com>
> 
> In order to make a filter processed only by hardware, skip_sw flag
> should be supplied. This is an addition to the already existing skip_hw
> flag (filter will be processed by software only). If no flag is
> specified, filter will be processed by both software and hardware.
> 
> If only hardware offloaded filters exist, fl_classify() will return
> without doing anything.
> 
> A following userspace patch will be sent once kernel patch is accepted.
> 
> Example:
> 
> tc filter add dev enp0s9 protocol ip prio 20 parent ffff: \
> 	flower \
> 		ip_proto 6 \
> 		indev enp0s9 \
> 		skip_sw \
> 	action skbedit mark 0x1234
> 
> Signed-off-by: Amir Vadai <amirva@mellanox.com>

Applied.
Amir Vadai June 8, 2016, 6:04 a.m. UTC | #4
On Tue, Jun 07, 2016 at 08:37:40AM -0700, John Fastabend wrote:
> On 16-06-05 07:11 AM, Amir Vadai wrote:
> > From: Amir Vadai <amirva@mellanox.com>
> > 
> > In order to make a filter processed only by hardware, skip_sw flag
> > should be supplied. This is an addition to the already existing skip_hw
> > flag (filter will be processed by software only). If no flag is
> > specified, filter will be processed by both software and hardware.
> > 
> > If only hardware offloaded filters exist, fl_classify() will return
> > without doing anything.
> > 
> > A following userspace patch will be sent once kernel patch is accepted.
> > 
> > Example:
> > 
> > tc filter add dev enp0s9 protocol ip prio 20 parent ffff: \
> > 	flower \
> > 		ip_proto 6 \
> > 		indev enp0s9 \
> > 		skip_sw \
> > 	action skbedit mark 0x1234
> > 
> > Signed-off-by: Amir Vadai <amirva@mellanox.com>
> > ---
> 
> 
> 
> Looks good to me. Although we need to do the same error propagation in
> flower that Jakub just added to cls_u32.
Thanks John,
I will send a patch to return error when add to hw is failing and skip_sw
is set.

> 
> Acked-by: John Fastabend <john.r.fastabend@intel.com>
>
diff mbox

Patch

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 730aaca..d737492 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -66,6 +66,7 @@  struct cls_fl_filter {
 	struct fl_flow_key key;
 	struct list_head list;
 	u32 handle;
+	u32 flags;
 	struct rcu_head	rcu;
 };
 
@@ -123,6 +124,9 @@  static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 	struct fl_flow_key skb_key;
 	struct fl_flow_key skb_mkey;
 
+	if (!atomic_read(&head->ht.nelems))
+		return -1;
+
 	fl_clear_masked_range(&skb_key, &head->mask);
 	skb_key.indev_ifindex = skb->skb_iif;
 	/* skb_flow_dissect() does not set n_proto in case an unknown protocol,
@@ -136,7 +140,7 @@  static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 	f = rhashtable_lookup_fast(&head->ht,
 				   fl_key_get_start(&skb_mkey, &head->mask),
 				   head->ht_params);
-	if (f) {
+	if (f && !(f->flags & TCA_CLS_FLAGS_SKIP_SW)) {
 		*res = f->res;
 		return tcf_exts_exec(skb, &f->exts, res);
 	}
@@ -524,7 +528,6 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 	struct cls_fl_filter *fnew;
 	struct nlattr *tb[TCA_FLOWER_MAX + 1];
 	struct fl_flow_mask mask = {};
-	u32 flags = 0;
 	int err;
 
 	if (!tca[TCA_OPTIONS])
@@ -552,8 +555,14 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 	}
 	fnew->handle = handle;
 
-	if (tb[TCA_FLOWER_FLAGS])
-		flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
+	if (tb[TCA_FLOWER_FLAGS]) {
+		fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
+
+		if (!tc_flags_valid(fnew->flags)) {
+			err = -EINVAL;
+			goto errout;
+		}
+	}
 
 	err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr);
 	if (err)
@@ -563,10 +572,12 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 	if (err)
 		goto errout;
 
-	err = rhashtable_insert_fast(&head->ht, &fnew->ht_node,
-				     head->ht_params);
-	if (err)
-		goto errout;
+	if (!(fnew->flags & TCA_CLS_FLAGS_SKIP_SW)) {
+		err = rhashtable_insert_fast(&head->ht, &fnew->ht_node,
+					     head->ht_params);
+		if (err)
+			goto errout;
+	}
 
 	fl_hw_replace_filter(tp,
 			     &head->dissector,
@@ -574,7 +585,7 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 			     &fnew->key,
 			     &fnew->exts,
 			     (unsigned long)fnew,
-			     flags);
+			     fnew->flags);
 
 	if (fold) {
 		rhashtable_remove_fast(&head->ht, &fold->ht_node,
@@ -734,6 +745,8 @@  static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 				  sizeof(key->tp.dst))))
 		goto nla_put_failure;
 
+	nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags);
+
 	if (tcf_exts_dump(skb, &f->exts))
 		goto nla_put_failure;