[PATCH net-next V4 0/7] net/sched: Reflect HW offload status in classifiers

[PATCH net-next V4 0/7] net/sched: Reflect HW offload status in classifiers

[Or Gerlitz]

Currently there is no way of querying whether a filter is
offloaded to HW or not when using "both" policy (where none
of skip_sw or skip_hw flags are set by user-space).

Added two new flags, "in hw" and "not in hw" such that user space 
can determine if a filter is actually offloaded to hw. The "in hw" 
UAPI semantics was chosen so it's similar to the "skip hw" flag logic.

If none of these two flags are set, this signals running
over older kernel.

As an example, add one vlan push + fwd rule, one matchall rule and one u32 rule 
without any flags, and another vlan + fwd skip_sw rule, such that the different TC 
classifier attempt to offload all of them -- all over mlx5 SRIOV VF rep:

# tc filter add dev eth2_0 protocol ip parent ffff: 
	flower skip_sw indev eth2_0 src_mac e4:11:22:33:44:50 dst_mac e4:1d:2d:a5:f3:9d 
	action vlan push id 52 action mirred egress redirect dev eth2

# tc filter add dev eth2_0 protocol ip parent ffff: 
	flower indev eth2_0 src_mac e4:11:22:33:44:50 dst_mac e4:11:22:33:44:51 
	action vlan push id 53 action mirred egress redirect dev eth2

# tc filter add dev eth2_0 parent ffff: matchall action mirred egress mirror dev veth1

# tc filter add dev eth2_0 parent ffff: protocol ip prio 99 handle 800:0:1 
	u32 ht 800: flowid 800:1 match ip src 192.168.1.0/24 action drop

Since that VF rep doesn't offload matchall/u32 and can currently offload
only one vlan push rule we expect three of the rules not to be offloaded:

# tc filter show dev eth2_0 parent ffff:

filter protocol ip pref 99 u32 
filter protocol ip pref 99 u32 fh 800: ht divisor 1 
filter protocol ip pref 99 u32 fh 800::1 order 1 key ht 800 bkt 0 flowid 800:1 not in_hw 
  match c0a80100/ffffff00 at 12
	action order 1: gact action drop
	 random type none pass val 0
	 index 8 ref 1 bind 1
 
filter protocol all pref 49150 matchall 
filter protocol all pref 49150 matchall handle 0x1 
  not in_hw
	action order 1: mirred (Egress Mirror to device veth1) pipe
 	index 27 ref 1 bind 1
 
filter protocol ip pref 49151 flower 
filter protocol ip pref 49151 flower handle 0x1 
  indev eth2_0
  dst_mac e4:11:22:33:44:51
  src_mac e4:11:22:33:44:50
  eth_type ipv4
  not in_hw
	action order 1:  vlan push id 53 protocol 802.1Q priority 0 pipe
	 index 20 ref 1 bind 1
 
	action order 2: mirred (Egress Redirect to device eth2) stolen
 	index 26 ref 1 bind 1
 
filter protocol ip pref 49152 flower 
filter protocol ip pref 49152 flower handle 0x1 
  indev eth2_0
  dst_mac e4:1d:2d:a5:f3:9d
  src_mac e4:11:22:33:44:50
  eth_type ipv4
  skip_sw
  in_hw
	action order 1:  vlan push id 52 protocol 802.1Q priority 0 pipe
	 index 19 ref 1 bind 1
 
	action order 2: mirred (Egress Redirect to device eth2) stolen
 	index 25 ref 1 bind 1

v3 --> v4 changes:
 - removed extra parenthesis (Dave)

v2 --> v3 changes:
 - fixed the matchall dump flags patch to do proper checks (Jakub)
 - added the same proper checks to flower where they were missing 
 - that flower patch was added as #1 and hence all the other patches are offed-by-one
 
v1 --> v2 changes:
 - applied feedback from Jakub and Dave -- where none of the skip flags were set, 
   the suggested approach didn't allow user space to distringuish between old kernel
   to a case when offloading to HW worked fine.

Or Gerlitz (7):
  net/sched: cls_flower: Properly handle classifier flags dumping
  net/sched: cls_matchall: Dump the classifier flags
  net/sched: Reflect HW offload status
  net/sched: cls_flower: Reflect HW offload status
  net/sched: cls_matchall: Reflect HW offloading status
  net/sched: cls_u32: Reflect HW offload status
  net/sched: cls_bpf: Reflect HW offload status

 include/net/pkt_cls.h        |  5 +++++
 include/uapi/linux/pkt_cls.h |  6 ++++--
 net/sched/cls_bpf.c          | 13 +++++++++++--
 net/sched/cls_flower.c       |  8 +++++++-
 net/sched/cls_matchall.c     | 15 +++++++++++++--
 net/sched/cls_u32.c          | 10 ++++++++++
 6 files changed, 50 insertions(+), 7 deletions(-)

-- 
2.5.5

[PATCH net-next V4 1/7] net/sched: cls_flower: Properly handle classifier flags dumping

[Or Gerlitz]

Dump the classifier flags only if non zero and make sure to check
the return status of the handler that puts them into the netlink msg.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 net/sched/cls_flower.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 0826c8e..850d982 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1229,7 +1229,8 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 	if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
 		goto nla_put_failure;
 
-	nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags);
+	if (f->flags && nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags))
+		goto nla_put_failure;
 
 	if (tcf_exts_dump(skb, &f->exts))
 		goto nla_put_failure;
-- 
2.5.5

[PATCH net-next V4 2/7] net/sched: cls_matchall: Dump the classifier flags

[Or Gerlitz]

The classifier flags are not dumped to user-space, do that.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Yotam Gigi <yotamg@mellanox.com>
---
 net/sched/cls_matchall.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index f2141cb..35ef1c1 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -244,6 +244,9 @@ static int mall_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 	    nla_put_u32(skb, TCA_MATCHALL_CLASSID, head->res.classid))
 		goto nla_put_failure;
 
+	if (head->flags && nla_put_u32(skb, TCA_MATCHALL_FLAGS, head->flags))
+		goto nla_put_failure;
+
 	if (tcf_exts_dump(skb, &head->exts))
 		goto nla_put_failure;
 
-- 
2.5.5

[PATCH net-next V4 3/7] net/sched: Reflect HW offload status

[Or Gerlitz]

Currently there is no way of querying whether a filter is
offloaded to HW or not when using "both" policy (where none
of skip_sw or skip_hw flags are set by user-space).

Add two new flags, "in hw" and "not in hw" such that user
space can determine if a filter is actually offloaded to
hw or not. The "in hw" UAPI semantics was chosen so it's
similar to the "skip hw" flag logic.

If none of these two flags are set, this signals running
over older kernel.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h        | 5 +++++
 include/uapi/linux/pkt_cls.h | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 71b266c..15cfe15 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -475,6 +475,11 @@ static inline bool tc_flags_valid(u32 flags)
 	return true;
 }
 
+static inline bool tc_in_hw(u32 flags)
+{
+	return (flags & TCA_CLS_FLAGS_IN_HW) ? true : false;
+}
+
 enum tc_fl_command {
 	TC_CLSFLOWER_REPLACE,
 	TC_CLSFLOWER_DESTROY,
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 345551e..7a69f2a 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -103,8 +103,10 @@ enum {
 #define TCA_POLICE_MAX (__TCA_POLICE_MAX - 1)
 
 /* tca flags definitions */
-#define TCA_CLS_FLAGS_SKIP_HW	(1 << 0)
-#define TCA_CLS_FLAGS_SKIP_SW	(1 << 1)
+#define TCA_CLS_FLAGS_SKIP_HW	(1 << 0) /* don't offload filter to HW */
+#define TCA_CLS_FLAGS_SKIP_SW	(1 << 1) /* don't use filter in SW */
+#define TCA_CLS_FLAGS_IN_HW	(1 << 2) /* filter is offloaded to HW */
+#define TCA_CLS_FLAGS_NOT_IN_HW (1 << 3) /* filter isn't offloaded to HW */
 
 /* U32 filters */
 
-- 
2.5.5

[PATCH net-next V4 4/7] net/sched: cls_flower: Reflect HW offload status

[Or Gerlitz]

Flower support for the "in hw" offloading flags.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 850d982..9d0c99d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -273,6 +273,8 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 
 	err = dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle, tp->protocol,
 					    tc);
+	if (!err)
+		f->flags |= TCA_CLS_FLAGS_IN_HW;
 
 	if (tc_skip_sw(f->flags))
 		return err;
@@ -912,6 +914,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 			goto errout;
 	}
 
+	if (!tc_in_hw(fnew->flags))
+		fnew->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+
 	if (fold) {
 		if (!tc_skip_sw(fold->flags))
 			rhashtable_remove_fast(&head->ht, &fold->ht_node,
-- 
2.5.5

[PATCH net-next V4 5/7] net/sched: cls_matchall: Reflect HW offloading status

[Or Gerlitz]

Matchall support for the "in hw" offloading flags.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_matchall.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 35ef1c1..224eb2c 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -56,6 +56,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	struct net_device *dev = tp->q->dev_queue->dev;
 	struct tc_to_netdev offload;
 	struct tc_cls_matchall_offload mall_offload = {0};
+	int err;
 
 	offload.type = TC_SETUP_MATCHALL;
 	offload.cls_mall = &mall_offload;
@@ -63,8 +64,12 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	offload.cls_mall->exts = &head->exts;
 	offload.cls_mall->cookie = cookie;
 
-	return dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle, tp->protocol,
-					     &offload);
+	err = dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle, tp->protocol,
+					    &offload);
+	if (!err)
+		head->flags |= TCA_CLS_FLAGS_IN_HW;
+
+	return err;
 }
 
 static void mall_destroy_hw_filter(struct tcf_proto *tp,
@@ -194,6 +199,9 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
 		}
 	}
 
+	if (!tc_in_hw(new->flags))
+		new->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+
 	*arg = (unsigned long) head;
 	rcu_assign_pointer(tp->root, new);
 	if (head)
-- 
2.5.5

[PATCH net-next V4 6/7] net/sched: cls_u32: Reflect HW offload status

[Or Gerlitz]

U32 support for the "in hw" offloading flags.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
---
 net/sched/cls_u32.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index a6ec3e4b..4dbe0c6 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -523,6 +523,10 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 
 	err = dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle,
 					    tp->protocol, &offload);
+
+	if (!err)
+		n->flags |= TCA_CLS_FLAGS_IN_HW;
+
 	if (tc_skip_sw(flags))
 		return err;
 
@@ -895,6 +899,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 			return err;
 		}
 
+		if (!tc_in_hw(new->flags))
+			new->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+
 		u32_replace_knode(tp, tp_c, new);
 		tcf_unbind_filter(tp, &n->res);
 		call_rcu(&n->rcu, u32_delete_key_rcu);
@@ -1014,6 +1021,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		if (err)
 			goto errhw;
 
+		if (!tc_in_hw(n->flags))
+			n->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+
 		ins = &ht->ht[TC_U32_HASH(handle)];
 		for (pins = rtnl_dereference(*ins); pins;
 		     ins = &pins->next, pins = rtnl_dereference(*ins))
-- 
2.5.5

[PATCH net-next V4 7/7] net/sched: cls_bpf: Reflect HW offload status

[Or Gerlitz]

BPF classifier support for the "in hw" offloading flags.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/sched/cls_bpf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index d9c9701..80f6884 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -148,6 +148,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	struct net_device *dev = tp->q->dev_queue->dev;
 	struct tc_cls_bpf_offload bpf_offload = {};
 	struct tc_to_netdev offload;
+	int err;
 
 	offload.type = TC_SETUP_CLSBPF;
 	offload.cls_bpf = &bpf_offload;
@@ -159,8 +160,13 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	bpf_offload.exts_integrated = prog->exts_integrated;
 	bpf_offload.gen_flags = prog->gen_flags;
 
-	return dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle,
-					     tp->protocol, &offload);
+	err = dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle,
+					    tp->protocol, &offload);
+
+	if (!err && (cmd == TC_CLSBPF_ADD || cmd == TC_CLSBPF_REPLACE))
+		prog->gen_flags |= TCA_CLS_FLAGS_IN_HW;
+
+	return err;
 }
 
 static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
@@ -511,6 +517,9 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 		return ret;
 	}
 
+	if (!tc_in_hw(prog->gen_flags))
+		prog->gen_flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+
 	if (oldprog) {
 		list_replace_rcu(&oldprog->link, &prog->link);
 		tcf_unbind_filter(tp, &oldprog->res);
-- 
2.5.5

Re: [PATCH net-next V4 0/7] net/sched: Reflect HW offload status in classifiers

[David Miller]

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Thu, 16 Feb 2017 10:31:09 +0200

> Currently there is no way of querying whether a filter is
> offloaded to HW or not when using "both" policy (where none
> of skip_sw or skip_hw flags are set by user-space).
> 
> Added two new flags, "in hw" and "not in hw" such that user space 
> can determine if a filter is actually offloaded to hw. The "in hw" 
> UAPI semantics was chosen so it's similar to the "skip hw" flag logic.
> 
> If none of these two flags are set, this signals running
> over older kernel.
 ...

Series applied, thanks Or.