All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw
@ 2018-07-23  7:23 Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 01/12] net: sched: push ops lookup bits into tcf_proto_lookup_ops() Jiri Pirko
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

For the TC clsact offload these days, some of HW drivers need
to hold a magic ball. The reason is, with the first inserted rule inside
HW they need to guess what fields will be used for the matching. If
later on this guess proves to be wrong and user adds a filter with a
different field to match, there's a problem. Mlxsw resolves it now with
couple of patterns. Those try to cover as many match fields as possible.
This aproach is far from optimal, both performance-wise and scale-wise.
Also, there is a combination of filters that in certain order won't
succeed.

Most of the time, when user inserts filters in chain, he knows right away
how the filters are going to look like - what type and option will they
have. For example, he knows that he will only insert filters of type
flower matching destination IP address. He can specify a template that
would cover all the filters in the chain.

This patchset is providing the possibility to user to provide such
template to kernel and propagate it all the way down to device
drivers.

See the examples below.

Create dummy device with clsact first:
# ip link add type dummy
# tc qdisc add dev dummy0 clsact

There is no chain present by by default:
# tc chaintemplate show dev dummy0 ingress

Add chain number 11 by explicit command:
# tc chain add dev dummy0 ingress chain 11
# tc chain show dev dummy0 ingress
chain parent ffff: chain 11 

Add filter to chain number 12 which does not exist. That will create
implicit chain 12:
# tc filter add dev dummy0 ingress protocol ip pref 25 handle 1 chain 12 flower dst_ip 192.168.1.1 action drop
# tc chain show dev dummy0 ingress
chain parent ffff: chain 11
chain parent ffff: chain 12

Delete both chains:
# tc chain del dev dummy0 ingress chain 11
# tc chain del dev dummy0 ingress chain 12
# tc chain show dev dummy0 ingress

Add a chain with template of type flower allowing to insert rules matching
on last 2 bytes of destination mac address:
# tc chain add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

The chain with template is now showed in the list:
# tc chain show dev dummy0 ingress
chain parent ffff: flower chain 0
  dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
  eth_type ipv4

Add another chain (number 22) with template:
# tc chain add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
# tc chain show dev dummy0 ingress
chain parent ffff: flower chain 0
  dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
  eth_type ipv4
chain parent ffff: flower chain 22
  eth_type ipv4
  dst_ip 0.0.0.0/16

Add a filter that fits the template:
# tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop

Addition of filters that does not fit the template would fail:
# tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop
Error: cls_flower: Mask does not fit the template.
We have an error talking to the kernel, -1
# tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop
Error: cls_flower: Mask does not fit the template.
We have an error talking to the kernel, -1

Additions of filters to chain 22:
# tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop
# tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop
Error: cls_flower: Mask does not fit the template.
We have an error talking to the kernel, -1
# tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop
Error: cls_flower: Mask does not fit the template.
We have an error talking to the kernel, -1

---
v3->v4:
- patch 2:
  - new patch
- patch 3:
  - new patch, derived from the previous v3 chaintemplate obj patch
- patch 4:
  - only templates part as chains creation/deletion is now a separate patch
  - don't pass template priv as arg of "change" op
- patch 6:
  - rebased on top of flower cvlan patch and ip tos/ttl patch
- patch 7:
  - templave priv is no longer passed as an arg to "change" op
- patch 11:
  - split from the originally single patch
- patch 12:
  - split from the originally single patch
v2->v3:
- patch 7:
  - rebase on top of the reoffload patchset
- patch 8:
  - rebase on top of the reoffload patchset
v1->v2:
- patch 8:
  - remove leftover extack arg in fl_hw_create_tmplt()
---

Jiri Pirko (12):
  net: sched: push ops lookup bits into tcf_proto_lookup_ops()
  net: sched: Avoid implicit chain 0 creation
  net: sched: introduce chain object to uapi
  net: sched: introduce chain templates
  net: sched: cls_flower: move key/mask dumping into a separate function
  net: sched: cls_flower: change fl_init_dissector to accept mask and
    dissector
  net: sched: cls_flower: implement chain templates
  net: sched: cls_flower: propagate chain teplate creation and
    destruction to drivers
  mlxsw: spectrum: Implement chain template hinting
  selftests: forwarding: move shblock tc support check to a separate
    helper
  selftests: forwarding: add tests for TC chains creation adn
    destruction
  selftests: forwarding: add tests for TC chain templates

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |   5 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   9 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c |  12 +-
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c    |  25 +-
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.h    |   3 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  |  44 +-
 include/net/pkt_cls.h                              |   2 +
 include/net/sch_generic.h                          |  18 +-
 include/uapi/linux/rtnetlink.h                     |   7 +
 net/sched/cls_api.c                                | 512 +++++++++++++++++----
 net/sched/cls_flower.c                             | 250 ++++++++--
 security/selinux/nlmsgtab.c                        |   2 +-
 tools/testing/selftests/net/forwarding/lib.sh      |  12 +
 .../testing/selftests/net/forwarding/tc_chains.sh  |  65 ++-
 .../selftests/net/forwarding/tc_shblocks.sh        |   2 +
 15 files changed, 829 insertions(+), 139 deletions(-)

-- 
2.14.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [patch net-next v4 01/12] net: sched: push ops lookup bits into tcf_proto_lookup_ops()
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 02/12] net: sched: Avoid implicit chain 0 creation Jiri Pirko
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Push all bits that take care of ops lookup, including module loading
outside tcf_proto_create() function, into tcf_proto_lookup_ops()

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_api.c | 53 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 623fe2cfe529..6bf91a12e01a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -39,7 +39,7 @@ static DEFINE_RWLOCK(cls_mod_lock);
 
 /* Find classifier type by string name */
 
-static const struct tcf_proto_ops *tcf_proto_lookup_ops(const char *kind)
+static const struct tcf_proto_ops *__tcf_proto_lookup_ops(const char *kind)
 {
 	const struct tcf_proto_ops *t, *res = NULL;
 
@@ -57,6 +57,33 @@ static const struct tcf_proto_ops *tcf_proto_lookup_ops(const char *kind)
 	return res;
 }
 
+static const struct tcf_proto_ops *
+tcf_proto_lookup_ops(const char *kind, struct netlink_ext_ack *extack)
+{
+	const struct tcf_proto_ops *ops;
+
+	ops = __tcf_proto_lookup_ops(kind);
+	if (ops)
+		return ops;
+#ifdef CONFIG_MODULES
+	rtnl_unlock();
+	request_module("cls_%s", kind);
+	rtnl_lock();
+	ops = __tcf_proto_lookup_ops(kind);
+	/* We dropped the RTNL semaphore in order to perform
+	 * the module load. So, even if we succeeded in loading
+	 * the module we have to replay the request. We indicate
+	 * this using -EAGAIN.
+	 */
+	if (ops) {
+		module_put(ops->owner);
+		return ERR_PTR(-EAGAIN);
+	}
+#endif
+	NL_SET_ERR_MSG(extack, "TC classifier not found");
+	return ERR_PTR(-ENOENT);
+}
+
 /* Register(unregister) new classifier type */
 
 int register_tcf_proto_ops(struct tcf_proto_ops *ops)
@@ -133,27 +160,9 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
 	if (!tp)
 		return ERR_PTR(-ENOBUFS);
 
-	err = -ENOENT;
-	tp->ops = tcf_proto_lookup_ops(kind);
-	if (!tp->ops) {
-#ifdef CONFIG_MODULES
-		rtnl_unlock();
-		request_module("cls_%s", kind);
-		rtnl_lock();
-		tp->ops = tcf_proto_lookup_ops(kind);
-		/* We dropped the RTNL semaphore in order to perform
-		 * the module load. So, even if we succeeded in loading
-		 * the module we have to replay the request. We indicate
-		 * this using -EAGAIN.
-		 */
-		if (tp->ops) {
-			module_put(tp->ops->owner);
-			err = -EAGAIN;
-		} else {
-			NL_SET_ERR_MSG(extack, "TC classifier not found");
-			err = -ENOENT;
-		}
-#endif
+	tp->ops = tcf_proto_lookup_ops(kind, extack);
+	if (IS_ERR(tp->ops)) {
+		err = PTR_ERR(tp->ops);
 		goto errout;
 	}
 	tp->classify = tp->ops->classify;
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 02/12] net: sched: Avoid implicit chain 0 creation
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 01/12] net: sched: push ops lookup bits into tcf_proto_lookup_ops() Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 03/12] net: sched: introduce chain object to uapi Jiri Pirko
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Currently, chain 0 is implicitly created during block creation. However
that does not align with chain object exposure, creation and destruction
api introduced later on. So make the chain 0 behave the same way as any
other chain and only create it when it is needed. Since chain 0 is
somehow special as the qdiscs need to hold pointer to the first chain
tp, this requires to move the chain head change callback infra to the
block structure.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- new patch
---
 include/net/sch_generic.h |  5 ++-
 net/sched/cls_api.c       | 86 +++++++++++++++++++++--------------------------
 2 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 7432100027b7..86f4651784e8 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -300,7 +300,6 @@ typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv);
 
 struct tcf_chain {
 	struct tcf_proto __rcu *filter_chain;
-	struct list_head filter_chain_list;
 	struct list_head list;
 	struct tcf_block *block;
 	u32 index; /* chain index */
@@ -318,6 +317,10 @@ struct tcf_block {
 	bool keep_dst;
 	unsigned int offloadcnt; /* Number of oddloaded filters */
 	unsigned int nooffloaddevcnt; /* Number of devs unable to do offload */
+	struct {
+		struct tcf_chain *chain;
+		struct list_head filter_chain_list;
+	} chain0;
 };
 
 static inline void tcf_block_offload_inc(struct tcf_block *block, u32 *flags)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 6bf91a12e01a..d26fed194870 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -204,11 +204,12 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block *block,
 	chain = kzalloc(sizeof(*chain), GFP_KERNEL);
 	if (!chain)
 		return NULL;
-	INIT_LIST_HEAD(&chain->filter_chain_list);
 	list_add_tail(&chain->list, &block->chain_list);
 	chain->block = block;
 	chain->index = chain_index;
 	chain->refcnt = 1;
+	if (!chain->index)
+		block->chain0.chain = chain;
 	return chain;
 }
 
@@ -218,12 +219,16 @@ static void tcf_chain_head_change_item(struct tcf_filter_chain_list_item *item,
 	if (item->chain_head_change)
 		item->chain_head_change(tp_head, item->chain_head_change_priv);
 }
-static void tcf_chain_head_change(struct tcf_chain *chain,
-				  struct tcf_proto *tp_head)
+
+static void tcf_chain0_head_change(struct tcf_chain *chain,
+				   struct tcf_proto *tp_head)
 {
 	struct tcf_filter_chain_list_item *item;
+	struct tcf_block *block = chain->block;
 
-	list_for_each_entry(item, &chain->filter_chain_list, list)
+	if (chain->index)
+		return;
+	list_for_each_entry(item, &block->chain0.filter_chain_list, list)
 		tcf_chain_head_change_item(item, tp_head);
 }
 
@@ -231,7 +236,7 @@ static void tcf_chain_flush(struct tcf_chain *chain)
 {
 	struct tcf_proto *tp = rtnl_dereference(chain->filter_chain);
 
-	tcf_chain_head_change(chain, NULL);
+	tcf_chain0_head_change(chain, NULL);
 	while (tp) {
 		RCU_INIT_POINTER(chain->filter_chain, tp->next);
 		tcf_proto_destroy(tp, NULL);
@@ -245,8 +250,10 @@ static void tcf_chain_destroy(struct tcf_chain *chain)
 	struct tcf_block *block = chain->block;
 
 	list_del(&chain->list);
+	if (!chain->index)
+		block->chain0.chain = NULL;
 	kfree(chain);
-	if (list_empty(&block->chain_list))
+	if (list_empty(&block->chain_list) && block->refcnt == 0)
 		kfree(block);
 }
 
@@ -346,10 +353,11 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
 }
 
 static int
-tcf_chain_head_change_cb_add(struct tcf_chain *chain,
-			     struct tcf_block_ext_info *ei,
-			     struct netlink_ext_ack *extack)
+tcf_chain0_head_change_cb_add(struct tcf_block *block,
+			      struct tcf_block_ext_info *ei,
+			      struct netlink_ext_ack *extack)
 {
+	struct tcf_chain *chain0 = block->chain0.chain;
 	struct tcf_filter_chain_list_item *item;
 
 	item = kmalloc(sizeof(*item), GFP_KERNEL);
@@ -359,23 +367,25 @@ tcf_chain_head_change_cb_add(struct tcf_chain *chain,
 	}
 	item->chain_head_change = ei->chain_head_change;
 	item->chain_head_change_priv = ei->chain_head_change_priv;
-	if (chain->filter_chain)
-		tcf_chain_head_change_item(item, chain->filter_chain);
-	list_add(&item->list, &chain->filter_chain_list);
+	if (chain0 && chain0->filter_chain)
+		tcf_chain_head_change_item(item, chain0->filter_chain);
+	list_add(&item->list, &block->chain0.filter_chain_list);
 	return 0;
 }
 
 static void
-tcf_chain_head_change_cb_del(struct tcf_chain *chain,
-			     struct tcf_block_ext_info *ei)
+tcf_chain0_head_change_cb_del(struct tcf_block *block,
+			      struct tcf_block_ext_info *ei)
 {
+	struct tcf_chain *chain0 = block->chain0.chain;
 	struct tcf_filter_chain_list_item *item;
 
-	list_for_each_entry(item, &chain->filter_chain_list, list) {
+	list_for_each_entry(item, &block->chain0.filter_chain_list, list) {
 		if ((!ei->chain_head_change && !ei->chain_head_change_priv) ||
 		    (item->chain_head_change == ei->chain_head_change &&
 		     item->chain_head_change_priv == ei->chain_head_change_priv)) {
-			tcf_chain_head_change_item(item, NULL);
+			if (chain0)
+				tcf_chain_head_change_item(item, NULL);
 			list_del(&item->list);
 			kfree(item);
 			return;
@@ -411,8 +421,6 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
 					  struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block;
-	struct tcf_chain *chain;
-	int err;
 
 	block = kzalloc(sizeof(*block), GFP_KERNEL);
 	if (!block) {
@@ -422,14 +430,8 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
 	INIT_LIST_HEAD(&block->chain_list);
 	INIT_LIST_HEAD(&block->cb_list);
 	INIT_LIST_HEAD(&block->owner_list);
+	INIT_LIST_HEAD(&block->chain0.filter_chain_list);
 
-	/* Create chain 0 by default, it has to be always present. */
-	chain = tcf_chain_create(block, 0);
-	if (!chain) {
-		NL_SET_ERR_MSG(extack, "Failed to create new tcf chain");
-		err = -ENOMEM;
-		goto err_chain_create;
-	}
 	block->refcnt = 1;
 	block->net = net;
 	block->index = block_index;
@@ -438,10 +440,6 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
 	if (!tcf_block_shared(block))
 		block->q = q;
 	return block;
-
-err_chain_create:
-	kfree(block);
-	return ERR_PTR(err);
 }
 
 static struct tcf_block *tcf_block_lookup(struct net *net, u32 block_index)
@@ -523,11 +521,6 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
 	return block;
 }
 
-static struct tcf_chain *tcf_block_chain_zero(struct tcf_block *block)
-{
-	return list_first_entry(&block->chain_list, struct tcf_chain, list);
-}
-
 struct tcf_block_owner_item {
 	struct list_head list;
 	struct Qdisc *q;
@@ -621,10 +614,9 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
 
 	tcf_block_owner_netif_keep_dst(block, q, ei->binder_type);
 
-	err = tcf_chain_head_change_cb_add(tcf_block_chain_zero(block),
-					   ei, extack);
+	err = tcf_chain0_head_change_cb_add(block, ei, extack);
 	if (err)
-		goto err_chain_head_change_cb_add;
+		goto err_chain0_head_change_cb_add;
 
 	err = tcf_block_offload_bind(block, q, ei, extack);
 	if (err)
@@ -634,15 +626,14 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
 	return 0;
 
 err_block_offload_bind:
-	tcf_chain_head_change_cb_del(tcf_block_chain_zero(block), ei);
-err_chain_head_change_cb_add:
+	tcf_chain0_head_change_cb_del(block, ei);
+err_chain0_head_change_cb_add:
 	tcf_block_owner_del(block, q, ei->binder_type);
 err_block_owner_add:
 	if (created) {
 		if (tcf_block_shared(block))
 			tcf_block_remove(block, net);
 err_block_insert:
-		kfree(tcf_block_chain_zero(block));
 		kfree(block);
 	} else {
 		block->refcnt--;
@@ -682,10 +673,10 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
 
 	if (!block)
 		return;
-	tcf_chain_head_change_cb_del(tcf_block_chain_zero(block), ei);
+	tcf_chain0_head_change_cb_del(block, ei);
 	tcf_block_owner_del(block, q, ei->binder_type);
 
-	if (--block->refcnt == 0) {
+	if (block->refcnt == 1) {
 		if (tcf_block_shared(block))
 			tcf_block_remove(block, block->net);
 
@@ -701,13 +692,14 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
 
 	tcf_block_offload_unbind(block, q, ei);
 
-	if (block->refcnt == 0) {
+	if (block->refcnt == 1) {
 		/* At this point, all the chains should have refcnt >= 1. */
 		list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
 			tcf_chain_put(chain);
 
-		/* Finally, put chain 0 and allow block to be freed. */
-		tcf_chain_put(tcf_block_chain_zero(block));
+		block->refcnt--;
+		if (list_empty(&block->chain_list))
+			kfree(block);
 	}
 }
 EXPORT_SYMBOL(tcf_block_put_ext);
@@ -947,7 +939,7 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain,
 				struct tcf_proto *tp)
 {
 	if (*chain_info->pprev == chain->filter_chain)
-		tcf_chain_head_change(chain, tp);
+		tcf_chain0_head_change(chain, tp);
 	RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain_info));
 	rcu_assign_pointer(*chain_info->pprev, tp);
 	tcf_chain_hold(chain);
@@ -960,7 +952,7 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain,
 	struct tcf_proto *next = rtnl_dereference(chain_info->next);
 
 	if (tp == chain->filter_chain)
-		tcf_chain_head_change(chain, next);
+		tcf_chain0_head_change(chain, next);
 	RCU_INIT_POINTER(*chain_info->pprev, next);
 	tcf_chain_put(chain);
 }
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 01/12] net: sched: push ops lookup bits into tcf_proto_lookup_ops() Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 02/12] net: sched: Avoid implicit chain 0 creation Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-24 22:30   ` Cong Wang
  2018-07-23  7:23 ` [patch net-next v4 04/12] net: sched: introduce chain templates Jiri Pirko
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Allow user to create, destroy, get and dump chain objects. Do that by
extending rtnl commands by the chain-specific ones. User will now be
able to explicitly create or destroy chains (so far this was done only
automatically according the filter/act needs and refcounting). Also, the
user will receive notification about any chain creation or destuction.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- new patch, derived from the previous v3 chaintemplate obj patch
---
 include/net/sch_generic.h      |   1 +
 include/uapi/linux/rtnetlink.h |   7 +
 net/sched/cls_api.c            | 308 +++++++++++++++++++++++++++++++++++++++--
 security/selinux/nlmsgtab.c    |   2 +-
 4 files changed, 309 insertions(+), 9 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 86f4651784e8..81ec8276db9c 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -304,6 +304,7 @@ struct tcf_chain {
 	struct tcf_block *block;
 	u32 index; /* chain index */
 	unsigned int refcnt;
+	bool explicitly_created;
 };
 
 struct tcf_block {
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7d8502313c99..46399367627f 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -150,6 +150,13 @@ enum {
 	RTM_NEWCACHEREPORT = 96,
 #define RTM_NEWCACHEREPORT RTM_NEWCACHEREPORT
 
+	RTM_NEWCHAIN = 100,
+#define RTM_NEWCHAIN RTM_NEWCHAIN
+	RTM_DELCHAIN,
+#define RTM_DELCHAIN RTM_DELCHAIN
+	RTM_GETCHAIN,
+#define RTM_GETCHAIN RTM_GETCHAIN
+
 	__RTM_MAX,
 #define RTM_MAX		(((__RTM_MAX + 3) & ~3) - 1)
 };
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d26fed194870..5225fc557e69 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -262,29 +262,57 @@ static void tcf_chain_hold(struct tcf_chain *chain)
 	++chain->refcnt;
 }
 
-struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
-				bool create)
+static struct tcf_chain *tcf_chain_lookup(struct tcf_block *block,
+					  u32 chain_index)
 {
 	struct tcf_chain *chain;
 
 	list_for_each_entry(chain, &block->chain_list, list) {
-		if (chain->index == chain_index) {
-			tcf_chain_hold(chain);
+		if (chain->index == chain_index)
 			return chain;
-		}
+	}
+	return NULL;
+}
+
+static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
+			   u32 seq, u16 flags, int event, bool unicast);
+
+struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
+				bool create)
+{
+	struct tcf_chain *chain = tcf_chain_lookup(block, chain_index);
+
+	if (chain) {
+		tcf_chain_hold(chain);
+		return chain;
 	}
 
-	return create ? tcf_chain_create(block, chain_index) : NULL;
+	if (!create)
+		return NULL;
+	chain = tcf_chain_create(block, chain_index);
+	if (!chain)
+		return NULL;
+	tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
+			RTM_NEWCHAIN, false);
+	return chain;
 }
 EXPORT_SYMBOL(tcf_chain_get);
 
 void tcf_chain_put(struct tcf_chain *chain)
 {
-	if (--chain->refcnt == 0)
+	if (--chain->refcnt == 0) {
+		tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false);
 		tcf_chain_destroy(chain);
+	}
 }
 EXPORT_SYMBOL(tcf_chain_put);
 
+static void tcf_chain_put_explicitly_created(struct tcf_chain *chain)
+{
+	if (chain->explicitly_created)
+		tcf_chain_put(chain);
+}
+
 static bool tcf_block_offload_in_use(struct tcf_block *block)
 {
 	return block->offloadcnt;
@@ -694,8 +722,10 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
 
 	if (block->refcnt == 1) {
 		/* At this point, all the chains should have refcnt >= 1. */
-		list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
+		list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
+			tcf_chain_put_explicitly_created(chain);
 			tcf_chain_put(chain);
+		}
 
 		block->refcnt--;
 		if (list_empty(&block->chain_list))
@@ -1609,6 +1639,264 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int tc_chain_fill_node(struct tcf_chain *chain, struct net *net,
+			      struct sk_buff *skb, struct tcf_block *block,
+			      u32 portid, u32 seq, u16 flags, int event)
+{
+	unsigned char *b = skb_tail_pointer(skb);
+	struct nlmsghdr *nlh;
+	struct tcmsg *tcm;
+
+	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*tcm), flags);
+	if (!nlh)
+		goto out_nlmsg_trim;
+	tcm = nlmsg_data(nlh);
+	tcm->tcm_family = AF_UNSPEC;
+	tcm->tcm__pad1 = 0;
+	tcm->tcm__pad2 = 0;
+	tcm->tcm_handle = 0;
+	if (block->q) {
+		tcm->tcm_ifindex = qdisc_dev(block->q)->ifindex;
+		tcm->tcm_parent = block->q->handle;
+	} else {
+		tcm->tcm_ifindex = TCM_IFINDEX_MAGIC_BLOCK;
+		tcm->tcm_block_index = block->index;
+	}
+
+	if (nla_put_u32(skb, TCA_CHAIN, chain->index))
+		goto nla_put_failure;
+
+	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
+	return skb->len;
+
+out_nlmsg_trim:
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -EMSGSIZE;
+}
+
+static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
+			   u32 seq, u16 flags, int event, bool unicast)
+{
+	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+	struct tcf_block *block = chain->block;
+	struct net *net = block->net;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
+	if (tc_chain_fill_node(chain, net, skb, block, portid,
+			       seq, flags, event) <= 0) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
+	if (unicast)
+		return netlink_unicast(net->rtnl, skb, portid, MSG_DONTWAIT);
+
+	return rtnetlink_send(skb, net, portid, RTNLGRP_TC, flags & NLM_F_ECHO);
+}
+
+/* Add/delete/get a chain */
+
+static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
+			struct netlink_ext_ack *extack)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nlattr *tca[TCA_MAX + 1];
+	struct tcmsg *t;
+	u32 parent;
+	u32 chain_index;
+	struct Qdisc *q = NULL;
+	struct tcf_chain *chain = NULL;
+	struct tcf_block *block;
+	unsigned long cl;
+	int err;
+
+	if (n->nlmsg_type != RTM_GETCHAIN &&
+	    !netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
+replay:
+	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+	if (err < 0)
+		return err;
+
+	t = nlmsg_data(n);
+	parent = t->tcm_parent;
+	cl = 0;
+
+	block = tcf_block_find(net, &q, &parent, &cl,
+			       t->tcm_ifindex, t->tcm_block_index, extack);
+	if (IS_ERR(block))
+		return PTR_ERR(block);
+
+	chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0;
+	if (chain_index > TC_ACT_EXT_VAL_MASK) {
+		NL_SET_ERR_MSG(extack, "Specified chain index exceeds upper limit");
+		return -EINVAL;
+	}
+	chain = tcf_chain_lookup(block, chain_index);
+	if (n->nlmsg_type == RTM_NEWCHAIN) {
+		if (chain) {
+			NL_SET_ERR_MSG(extack, "Filter chain already exists");
+			return -EEXIST;
+		}
+		if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+			NL_SET_ERR_MSG(extack, "Need both RTM_NEWCHAIN and NLM_F_CREATE to create a new chain");
+			return -ENOENT;
+		}
+		chain = tcf_chain_create(block, chain_index);
+		if (!chain) {
+			NL_SET_ERR_MSG(extack, "Failed to create filter chain");
+			return -ENOMEM;
+		}
+	} else {
+		if (!chain) {
+			NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
+			return -EINVAL;
+		}
+		tcf_chain_hold(chain);
+	}
+
+	switch (n->nlmsg_type) {
+	case RTM_NEWCHAIN:
+		/* In case the chain was successfully added, take a reference
+		 * to the chain. This ensures that an empty chain
+		 * does not disappear at the end of this function.
+		 */
+		tcf_chain_hold(chain);
+		chain->explicitly_created = true;
+		tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
+				RTM_NEWCHAIN, false);
+		break;
+	case RTM_DELCHAIN:
+		/* Flush the chain first as the user requested chain removal. */
+		tcf_chain_flush(chain);
+		/* In case the chain was successfully deleted, put a reference
+		 * to the chain previously taken during addition.
+		 */
+		tcf_chain_put_explicitly_created(chain);
+		break;
+	case RTM_GETCHAIN:
+		break;
+		err = tc_chain_notify(chain, skb, n->nlmsg_seq,
+				      n->nlmsg_seq, n->nlmsg_type, true);
+		if (err < 0)
+			NL_SET_ERR_MSG(extack, "Failed to send chain notify message");
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		NL_SET_ERR_MSG(extack, "Unsupported message type");
+		goto errout;
+	}
+
+errout:
+	tcf_chain_put(chain);
+	if (err == -EAGAIN)
+		/* Replay the request. */
+		goto replay;
+	return err;
+}
+
+/* called with RTNL */
+static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nlattr *tca[TCA_MAX + 1];
+	struct Qdisc *q = NULL;
+	struct tcf_block *block;
+	struct tcf_chain *chain;
+	struct tcmsg *tcm = nlmsg_data(cb->nlh);
+	long index_start;
+	long index;
+	u32 parent;
+	int err;
+
+	if (nlmsg_len(cb->nlh) < sizeof(*tcm))
+		return skb->len;
+
+	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
+	if (err)
+		return err;
+
+	if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
+		block = tcf_block_lookup(net, tcm->tcm_block_index);
+		if (!block)
+			goto out;
+		/* If we work with block index, q is NULL and parent value
+		 * will never be used in the following code. The check
+		 * in tcf_fill_node prevents it. However, compiler does not
+		 * see that far, so set parent to zero to silence the warning
+		 * about parent being uninitialized.
+		 */
+		parent = 0;
+	} else {
+		const struct Qdisc_class_ops *cops;
+		struct net_device *dev;
+		unsigned long cl = 0;
+
+		dev = __dev_get_by_index(net, tcm->tcm_ifindex);
+		if (!dev)
+			return skb->len;
+
+		parent = tcm->tcm_parent;
+		if (!parent) {
+			q = dev->qdisc;
+			parent = q->handle;
+		} else {
+			q = qdisc_lookup(dev, TC_H_MAJ(tcm->tcm_parent));
+		}
+		if (!q)
+			goto out;
+		cops = q->ops->cl_ops;
+		if (!cops)
+			goto out;
+		if (!cops->tcf_block)
+			goto out;
+		if (TC_H_MIN(tcm->tcm_parent)) {
+			cl = cops->find(q, tcm->tcm_parent);
+			if (cl == 0)
+				goto out;
+		}
+		block = cops->tcf_block(q, cl, NULL);
+		if (!block)
+			goto out;
+		if (tcf_block_shared(block))
+			q = NULL;
+	}
+
+	index_start = cb->args[0];
+	index = 0;
+
+	list_for_each_entry(chain, &block->chain_list, list) {
+		if ((tca[TCA_CHAIN] &&
+		     nla_get_u32(tca[TCA_CHAIN]) != chain->index))
+			continue;
+		if (index < index_start) {
+			index++;
+			continue;
+		}
+		err = tc_chain_fill_node(chain, net, skb, block,
+					 NETLINK_CB(cb->skb).portid,
+					 cb->nlh->nlmsg_seq, NLM_F_MULTI,
+					 RTM_NEWCHAIN);
+		if (err <= 0)
+			break;
+		index++;
+	}
+
+	cb->args[0] = index;
+
+out:
+	/* If we did no progress, the error (EMSGSIZE) is real */
+	if (skb->len == 0 && err)
+		return err;
+	return skb->len;
+}
+
 void tcf_exts_destroy(struct tcf_exts *exts)
 {
 #ifdef CONFIG_NET_CLS_ACT
@@ -1825,6 +2113,10 @@ static int __init tc_filter_init(void)
 	rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_del_tfilter, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_get_tfilter,
 		      tc_dump_tfilter, 0);
+	rtnl_register(PF_UNSPEC, RTM_NEWCHAIN, tc_ctl_chain, NULL, 0);
+	rtnl_register(PF_UNSPEC, RTM_DELCHAIN, tc_ctl_chain, NULL, 0);
+	rtnl_register(PF_UNSPEC, RTM_GETCHAIN, tc_ctl_chain,
+		      tc_dump_chain, 0);
 
 	return 0;
 
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 7b7433a1a34c..74b951f55608 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -159,7 +159,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
 	switch (sclass) {
 	case SECCLASS_NETLINK_ROUTE_SOCKET:
 		/* RTM_MAX always point to RTM_SETxxxx, ie RTM_NEWxxx + 3 */
-		BUILD_BUG_ON(RTM_MAX != (RTM_NEWCACHEREPORT + 3));
+		BUILD_BUG_ON(RTM_MAX != (RTM_NEWCHAIN + 3));
 		err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
 				 sizeof(nlmsg_route_perms));
 		break;
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 04/12] net: sched: introduce chain templates
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (2 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 03/12] net: sched: introduce chain object to uapi Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 05/12] net: sched: cls_flower: move key/mask dumping into a separate function Jiri Pirko
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Allow user to set a template for newly created chains. Template lock
down the chain for particular classifier type/options combinations.
The classifier needs to support templates, otherwise kernel would
reply with error.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- only templates part as chains creation/deletion is now a separate patch
- don't pass template priv as arg of "change" op
---
 include/net/sch_generic.h | 12 +++++++++
 net/sched/cls_api.c       | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 81ec8276db9c..085c509c8674 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -238,6 +238,8 @@ struct tcf_result {
 	};
 };
 
+struct tcf_chain;
+
 struct tcf_proto_ops {
 	struct list_head	head;
 	char			kind[IFNAMSIZ];
@@ -263,10 +265,18 @@ struct tcf_proto_ops {
 					     tc_setup_cb_t *cb, void *cb_priv,
 					     struct netlink_ext_ack *extack);
 	void			(*bind_class)(void *, u32, unsigned long);
+	void *			(*tmplt_create)(struct net *net,
+						struct tcf_chain *chain,
+						struct nlattr **tca,
+						struct netlink_ext_ack *extack);
+	void			(*tmplt_destroy)(void *tmplt_priv);
 
 	/* rtnetlink specific */
 	int			(*dump)(struct net*, struct tcf_proto*, void *,
 					struct sk_buff *skb, struct tcmsg*);
+	int			(*tmplt_dump)(struct sk_buff *skb,
+					      struct net *net,
+					      void *tmplt_priv);
 
 	struct module		*owner;
 };
@@ -305,6 +315,8 @@ struct tcf_chain {
 	u32 index; /* chain index */
 	unsigned int refcnt;
 	bool explicitly_created;
+	const struct tcf_proto_ops *tmplt_ops;
+	void *tmplt_priv;
 };
 
 struct tcf_block {
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 5225fc557e69..fd85b57ecb10 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -298,10 +298,13 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
 }
 EXPORT_SYMBOL(tcf_chain_get);
 
+static void tc_chain_tmplt_del(struct tcf_chain *chain);
+
 void tcf_chain_put(struct tcf_chain *chain)
 {
 	if (--chain->refcnt == 0) {
 		tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false);
+		tc_chain_tmplt_del(chain);
 		tcf_chain_destroy(chain);
 	}
 }
@@ -1258,6 +1261,12 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 		goto errout;
 	}
 
+	if (chain->tmplt_ops && chain->tmplt_ops != tp->ops) {
+		NL_SET_ERR_MSG(extack, "Chain template is set to a different filter kind");
+		err = -EINVAL;
+		goto errout;
+	}
+
 	err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
 			      n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE,
 			      extack);
@@ -1644,8 +1653,13 @@ static int tc_chain_fill_node(struct tcf_chain *chain, struct net *net,
 			      u32 portid, u32 seq, u16 flags, int event)
 {
 	unsigned char *b = skb_tail_pointer(skb);
+	const struct tcf_proto_ops *ops;
 	struct nlmsghdr *nlh;
 	struct tcmsg *tcm;
+	void *priv;
+
+	ops = chain->tmplt_ops;
+	priv = chain->tmplt_priv;
 
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*tcm), flags);
 	if (!nlh)
@@ -1666,6 +1680,13 @@ static int tc_chain_fill_node(struct tcf_chain *chain, struct net *net,
 	if (nla_put_u32(skb, TCA_CHAIN, chain->index))
 		goto nla_put_failure;
 
+	if (ops) {
+		if (nla_put_string(skb, TCA_KIND, ops->kind))
+			goto nla_put_failure;
+		if (ops->tmplt_dump(skb, net, priv) < 0)
+			goto nla_put_failure;
+	}
+
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 	return skb->len;
 
@@ -1699,6 +1720,47 @@ static int tc_chain_notify(struct tcf_chain *chain, struct sk_buff *oskb,
 	return rtnetlink_send(skb, net, portid, RTNLGRP_TC, flags & NLM_F_ECHO);
 }
 
+static int tc_chain_tmplt_add(struct tcf_chain *chain, struct net *net,
+			      struct nlattr **tca,
+			      struct netlink_ext_ack *extack)
+{
+	const struct tcf_proto_ops *ops;
+	void *tmplt_priv;
+
+	/* If kind is not set, user did not specify template. */
+	if (!tca[TCA_KIND])
+		return 0;
+
+	ops = tcf_proto_lookup_ops(nla_data(tca[TCA_KIND]), extack);
+	if (IS_ERR(ops))
+		return PTR_ERR(ops);
+	if (!ops->tmplt_create || !ops->tmplt_destroy || !ops->tmplt_dump) {
+		NL_SET_ERR_MSG(extack, "Chain templates are not supported with specified classifier");
+		return -EOPNOTSUPP;
+	}
+
+	tmplt_priv = ops->tmplt_create(net, chain, tca, extack);
+	if (IS_ERR(tmplt_priv)) {
+		module_put(ops->owner);
+		return PTR_ERR(tmplt_priv);
+	}
+	chain->tmplt_ops = ops;
+	chain->tmplt_priv = tmplt_priv;
+	return 0;
+}
+
+static void tc_chain_tmplt_del(struct tcf_chain *chain)
+{
+	const struct tcf_proto_ops *ops = chain->tmplt_ops;
+
+	/* If template ops are set, no work to do for us. */
+	if (!ops)
+		return;
+
+	ops->tmplt_destroy(chain->tmplt_priv);
+	module_put(ops->owner);
+}
+
 /* Add/delete/get a chain */
 
 static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
@@ -1763,6 +1825,9 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
 
 	switch (n->nlmsg_type) {
 	case RTM_NEWCHAIN:
+		err = tc_chain_tmplt_add(chain, net, tca, extack);
+		if (err)
+			goto errout;
 		/* In case the chain was successfully added, take a reference
 		 * to the chain. This ensures that an empty chain
 		 * does not disappear at the end of this function.
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 05/12] net: sched: cls_flower: move key/mask dumping into a separate function
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (3 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 04/12] net: sched: introduce chain templates Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 06/12] net: sched: cls_flower: change fl_init_dissector to accept mask and dissector Jiri Pirko
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Push key/mask dumping from fl_dump() into a separate function
fl_dump_key(), that will be reused for template dumping.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 62 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 38d74803e2df..ab10a7c88359 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1296,29 +1296,9 @@ static int fl_dump_key_flags(struct sk_buff *skb, u32 flags_key, u32 flags_mask)
 	return nla_put(skb, TCA_FLOWER_KEY_FLAGS_MASK, 4, &_mask);
 }
 
-static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
-		   struct sk_buff *skb, struct tcmsg *t)
+static int fl_dump_key(struct sk_buff *skb, struct net *net,
+		       struct fl_flow_key *key, struct fl_flow_key *mask)
 {
-	struct cls_fl_filter *f = fh;
-	struct nlattr *nest;
-	struct fl_flow_key *key, *mask;
-
-	if (!f)
-		return skb->len;
-
-	t->tcm_handle = f->handle;
-
-	nest = nla_nest_start(skb, TCA_OPTIONS);
-	if (!nest)
-		goto nla_put_failure;
-
-	if (f->res.classid &&
-	    nla_put_u32(skb, TCA_FLOWER_CLASSID, f->res.classid))
-		goto nla_put_failure;
-
-	key = &f->key;
-	mask = &f->mask->key;
-
 	if (mask->indev_ifindex) {
 		struct net_device *dev;
 
@@ -1327,9 +1307,6 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 			goto nla_put_failure;
 	}
 
-	if (!tc_skip_hw(f->flags))
-		fl_hw_update_stats(tp, f);
-
 	if (fl_dump_key_val(skb, key->eth.dst, TCA_FLOWER_KEY_ETH_DST,
 			    mask->eth.dst, TCA_FLOWER_KEY_ETH_DST_MASK,
 			    sizeof(key->eth.dst)) ||
@@ -1505,6 +1482,41 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
 		goto nla_put_failure;
 
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
+static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
+		   struct sk_buff *skb, struct tcmsg *t)
+{
+	struct cls_fl_filter *f = fh;
+	struct nlattr *nest;
+	struct fl_flow_key *key, *mask;
+
+	if (!f)
+		return skb->len;
+
+	t->tcm_handle = f->handle;
+
+	nest = nla_nest_start(skb, TCA_OPTIONS);
+	if (!nest)
+		goto nla_put_failure;
+
+	if (f->res.classid &&
+	    nla_put_u32(skb, TCA_FLOWER_CLASSID, f->res.classid))
+		goto nla_put_failure;
+
+	key = &f->key;
+	mask = &f->mask->key;
+
+	if (fl_dump_key(skb, net, key, mask))
+		goto nla_put_failure;
+
+	if (!tc_skip_hw(f->flags))
+		fl_hw_update_stats(tp, f);
+
 	if (f->flags && nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags))
 		goto nla_put_failure;
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 06/12] net: sched: cls_flower: change fl_init_dissector to accept mask and dissector
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (4 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 05/12] net: sched: cls_flower: move key/mask dumping into a separate function Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 07/12] net: sched: cls_flower: implement chain templates Jiri Pirko
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

This function is going to be used for templates as well, so we need to
pass the pointer separately.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- rebased on top of flower cvlan patch and ip tos/ttl patch
---
 net/sched/cls_flower.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index ab10a7c88359..bb7aa1e9d281 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -826,51 +826,52 @@ static int fl_init_mask_hashtable(struct fl_flow_mask *mask)
 			FL_KEY_SET(keys, cnt, id, member);			\
 	} while(0);
 
-static void fl_init_dissector(struct fl_flow_mask *mask)
+static void fl_init_dissector(struct flow_dissector *dissector,
+			      struct fl_flow_key *mask)
 {
 	struct flow_dissector_key keys[FLOW_DISSECTOR_KEY_MAX];
 	size_t cnt = 0;
 
 	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_CONTROL, control);
 	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_BASIC, basic);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ETH_ADDRS, eth);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_PORTS, tp);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IP, ip);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_TCP, tcp);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ICMP, icmp);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ARP, arp);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_MPLS, mpls);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_VLAN, vlan);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_CVLAN, cvlan);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_KEYID, enc_key_id);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS, enc_ipv4);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS, enc_ipv6);
-	if (FL_KEY_IS_MASKED(&mask->key, enc_ipv4) ||
-	    FL_KEY_IS_MASKED(&mask->key, enc_ipv6))
+	if (FL_KEY_IS_MASKED(mask, enc_ipv4) ||
+	    FL_KEY_IS_MASKED(mask, enc_ipv6))
 		FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_ENC_CONTROL,
 			   enc_control);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_PORTS, enc_tp);
-	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_IP, enc_ip);
 
-	skb_flow_dissector_init(&mask->dissector, keys, cnt);
+	skb_flow_dissector_init(dissector, keys, cnt);
 }
 
 static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
@@ -889,7 +890,7 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
 	if (err)
 		goto errout_free;
 
-	fl_init_dissector(newmask);
+	fl_init_dissector(&newmask->dissector, &newmask->key);
 
 	INIT_LIST_HEAD_RCU(&newmask->filters);
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 07/12] net: sched: cls_flower: implement chain templates
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (5 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 06/12] net: sched: cls_flower: change fl_init_dissector to accept mask and dissector Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 08/12] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers Jiri Pirko
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Use the previously introduced template extension and implement
callback to create, destroy and dump chain template. The existing
parsing and dumping functions are re-used. Also, check if newly added
filters fit the template if it is set.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- templave priv is no longer passed as an arg to "change" op
v2->v3:
- rebase on top of the reoffload patchset
---
 net/sched/cls_flower.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index bb7aa1e9d281..f0c80758a594 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -72,6 +72,13 @@ struct fl_flow_mask {
 	struct list_head list;
 };
 
+struct fl_flow_tmplt {
+	struct fl_flow_key dummy_key;
+	struct fl_flow_key mask;
+	struct flow_dissector dissector;
+	struct tcf_chain *chain;
+};
+
 struct cls_fl_head {
 	struct rhashtable ht;
 	struct list_head masks;
@@ -147,6 +154,23 @@ static void fl_set_masked_key(struct fl_flow_key *mkey, struct fl_flow_key *key,
 		*lmkey++ = *lkey++ & *lmask++;
 }
 
+static bool fl_mask_fits_tmplt(struct fl_flow_tmplt *tmplt,
+			       struct fl_flow_mask *mask)
+{
+	const long *lmask = fl_key_get_start(&mask->key, mask);
+	const long *ltmplt;
+	int i;
+
+	if (!tmplt)
+		return true;
+	ltmplt = fl_key_get_start(&tmplt->mask, mask);
+	for (i = 0; i < fl_mask_range(mask); i += sizeof(long)) {
+		if (~*ltmplt++ & *lmask++)
+			return false;
+	}
+	return true;
+}
+
 static void fl_clear_masked_range(struct fl_flow_key *key,
 				  struct fl_flow_mask *mask)
 {
@@ -939,6 +963,7 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 			struct cls_fl_filter *f, struct fl_flow_mask *mask,
 			unsigned long base, struct nlattr **tb,
 			struct nlattr *est, bool ovr,
+			struct fl_flow_tmplt *tmplt,
 			struct netlink_ext_ack *extack)
 {
 	int err;
@@ -959,6 +984,11 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 	fl_mask_update_range(mask);
 	fl_set_masked_key(&f->mkey, &f->key, mask);
 
+	if (!fl_mask_fits_tmplt(tmplt, mask)) {
+		NL_SET_ERR_MSG_MOD(extack, "Mask does not fit the template");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -1024,7 +1054,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 	}
 
 	err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr,
-			   extack);
+			   tp->chain->tmplt_priv, extack);
 	if (err)
 		goto errout_idr;
 
@@ -1164,6 +1194,52 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 	return 0;
 }
 
+static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
+			     struct nlattr **tca,
+			     struct netlink_ext_ack *extack)
+{
+	struct fl_flow_tmplt *tmplt;
+	struct nlattr **tb;
+	int err;
+
+	if (!tca[TCA_OPTIONS])
+		return ERR_PTR(-EINVAL);
+
+	tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
+	if (!tb)
+		return ERR_PTR(-ENOBUFS);
+	err = nla_parse_nested(tb, TCA_FLOWER_MAX, tca[TCA_OPTIONS],
+			       fl_policy, NULL);
+	if (err)
+		goto errout_tb;
+
+	tmplt = kzalloc(sizeof(*tmplt), GFP_KERNEL);
+	if (!tmplt)
+		goto errout_tb;
+	tmplt->chain = chain;
+	err = fl_set_key(net, tb, &tmplt->dummy_key, &tmplt->mask, extack);
+	if (err)
+		goto errout_tmplt;
+	kfree(tb);
+
+	fl_init_dissector(&tmplt->dissector, &tmplt->mask);
+
+	return tmplt;
+
+errout_tmplt:
+	kfree(tmplt);
+errout_tb:
+	kfree(tb);
+	return ERR_PTR(err);
+}
+
+static void fl_tmplt_destroy(void *tmplt_priv)
+{
+	struct fl_flow_tmplt *tmplt = tmplt_priv;
+
+	kfree(tmplt);
+}
+
 static int fl_dump_key_val(struct sk_buff *skb,
 			   void *val, int val_type,
 			   void *mask, int mask_type, int len)
@@ -1536,6 +1612,31 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	return -1;
 }
 
+static int fl_tmplt_dump(struct sk_buff *skb, struct net *net, void *tmplt_priv)
+{
+	struct fl_flow_tmplt *tmplt = tmplt_priv;
+	struct fl_flow_key *key, *mask;
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, TCA_OPTIONS);
+	if (!nest)
+		goto nla_put_failure;
+
+	key = &tmplt->dummy_key;
+	mask = &tmplt->mask;
+
+	if (fl_dump_key(skb, net, key, mask))
+		goto nla_put_failure;
+
+	nla_nest_end(skb, nest);
+
+	return skb->len;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
 static void fl_bind_class(void *fh, u32 classid, unsigned long cl)
 {
 	struct cls_fl_filter *f = fh;
@@ -1556,6 +1657,9 @@ static struct tcf_proto_ops cls_fl_ops __read_mostly = {
 	.reoffload	= fl_reoffload,
 	.dump		= fl_dump,
 	.bind_class	= fl_bind_class,
+	.tmplt_create	= fl_tmplt_create,
+	.tmplt_destroy	= fl_tmplt_destroy,
+	.tmplt_dump	= fl_tmplt_dump,
 	.owner		= THIS_MODULE,
 };
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 08/12] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (6 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 07/12] net: sched: cls_flower: implement chain templates Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:23 ` [patch net-next v4 09/12] mlxsw: spectrum: Implement chain template hinting Jiri Pirko
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Introduce a couple of flower offload commands in order to propagate
template creation/destruction events down to device drivers.
Drivers may use this information to prepare HW in an optimal way
for future filter insertions.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v2->v3:
- rebase on top of the reoffload patchset
v1->v2:
- remove leftover extack arg in fl_hw_create_tmplt()
---
 include/net/pkt_cls.h  |  2 ++
 net/sched/cls_flower.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e4252a176eec..7673ee03f093 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -726,6 +726,8 @@ enum tc_fl_command {
 	TC_CLSFLOWER_REPLACE,
 	TC_CLSFLOWER_DESTROY,
 	TC_CLSFLOWER_STATS,
+	TC_CLSFLOWER_TMPLT_CREATE,
+	TC_CLSFLOWER_TMPLT_DESTROY,
 };
 
 struct tc_cls_flower_offload {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f0c80758a594..6ccf60364297 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1194,6 +1194,42 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 	return 0;
 }
 
+static void fl_hw_create_tmplt(struct tcf_chain *chain,
+			       struct fl_flow_tmplt *tmplt)
+{
+	struct tc_cls_flower_offload cls_flower = {};
+	struct tcf_block *block = chain->block;
+	struct tcf_exts dummy_exts = { 0, };
+
+	cls_flower.common.chain_index = chain->index;
+	cls_flower.command = TC_CLSFLOWER_TMPLT_CREATE;
+	cls_flower.cookie = (unsigned long) tmplt;
+	cls_flower.dissector = &tmplt->dissector;
+	cls_flower.mask = &tmplt->mask;
+	cls_flower.key = &tmplt->dummy_key;
+	cls_flower.exts = &dummy_exts;
+
+	/* We don't care if driver (any of them) fails to handle this
+	 * call. It serves just as a hint for it.
+	 */
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSFLOWER,
+			 &cls_flower, false);
+}
+
+static void fl_hw_destroy_tmplt(struct tcf_chain *chain,
+				struct fl_flow_tmplt *tmplt)
+{
+	struct tc_cls_flower_offload cls_flower = {};
+	struct tcf_block *block = chain->block;
+
+	cls_flower.common.chain_index = chain->index;
+	cls_flower.command = TC_CLSFLOWER_TMPLT_DESTROY;
+	cls_flower.cookie = (unsigned long) tmplt;
+
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSFLOWER,
+			 &cls_flower, false);
+}
+
 static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
 			     struct nlattr **tca,
 			     struct netlink_ext_ack *extack)
@@ -1224,6 +1260,8 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
 
 	fl_init_dissector(&tmplt->dissector, &tmplt->mask);
 
+	fl_hw_create_tmplt(chain, tmplt);
+
 	return tmplt;
 
 errout_tmplt:
@@ -1237,6 +1275,7 @@ static void fl_tmplt_destroy(void *tmplt_priv)
 {
 	struct fl_flow_tmplt *tmplt = tmplt_priv;
 
+	fl_hw_destroy_tmplt(tmplt->chain, tmplt);
 	kfree(tmplt);
 }
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 09/12] mlxsw: spectrum: Implement chain template hinting
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (7 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 08/12] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers Jiri Pirko
@ 2018-07-23  7:23 ` Jiri Pirko
  2018-07-23  7:24 ` [patch net-next v4 10/12] selftests: forwarding: move shblock tc support check to a separate helper Jiri Pirko
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:23 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Since cld_flower provides information about the filter template for
specific chain, use this information in order to prepare a region.
Use the template to find out what elements are going to be used
and pass that down to mlxsw_sp_acl_tcam_group_add(). Later on, when the
first filter is inserted, the mlxsw_sp_acl_tcam_group_use_patterns()
function would use this element usage information instead of looking
up a pattern.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  5 +++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  9 ++++-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 12 ++++--
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c    | 25 ++++++++++--
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.h    |  3 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  | 44 ++++++++++++++++++++--
 6 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 317c92d6496e..039228525fb1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1455,6 +1455,11 @@ mlxsw_sp_setup_tc_cls_flower(struct mlxsw_sp_acl_block *acl_block,
 		return 0;
 	case TC_CLSFLOWER_STATS:
 		return mlxsw_sp_flower_stats(mlxsw_sp, acl_block, f);
+	case TC_CLSFLOWER_TMPLT_CREATE:
+		return mlxsw_sp_flower_tmplt_create(mlxsw_sp, acl_block, f);
+	case TC_CLSFLOWER_TMPLT_DESTROY:
+		mlxsw_sp_flower_tmplt_destroy(mlxsw_sp, acl_block, f);
+		return 0;
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 016058961542..3db386c2573f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -543,7 +543,8 @@ mlxsw_sp_acl_ruleset_lookup(struct mlxsw_sp *mlxsw_sp,
 struct mlxsw_sp_acl_ruleset *
 mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
 			 struct mlxsw_sp_acl_block *block, u32 chain_index,
-			 enum mlxsw_sp_acl_profile profile);
+			 enum mlxsw_sp_acl_profile profile,
+			 struct mlxsw_afk_element_usage *tmplt_elusage);
 void mlxsw_sp_acl_ruleset_put(struct mlxsw_sp *mlxsw_sp,
 			      struct mlxsw_sp_acl_ruleset *ruleset);
 u16 mlxsw_sp_acl_ruleset_group_id(struct mlxsw_sp_acl_ruleset *ruleset);
@@ -667,6 +668,12 @@ void mlxsw_sp_flower_destroy(struct mlxsw_sp *mlxsw_sp,
 int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
 			  struct mlxsw_sp_acl_block *block,
 			  struct tc_cls_flower_offload *f);
+int mlxsw_sp_flower_tmplt_create(struct mlxsw_sp *mlxsw_sp,
+				 struct mlxsw_sp_acl_block *block,
+				 struct tc_cls_flower_offload *f);
+void mlxsw_sp_flower_tmplt_destroy(struct mlxsw_sp *mlxsw_sp,
+				   struct mlxsw_sp_acl_block *block,
+				   struct tc_cls_flower_offload *f);
 
 /* spectrum_qdisc.c */
 int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index 217621d79e26..4a4739139d11 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -317,7 +317,8 @@ int mlxsw_sp_acl_block_unbind(struct mlxsw_sp *mlxsw_sp,
 static struct mlxsw_sp_acl_ruleset *
 mlxsw_sp_acl_ruleset_create(struct mlxsw_sp *mlxsw_sp,
 			    struct mlxsw_sp_acl_block *block, u32 chain_index,
-			    const struct mlxsw_sp_acl_profile_ops *ops)
+			    const struct mlxsw_sp_acl_profile_ops *ops,
+			    struct mlxsw_afk_element_usage *tmplt_elusage)
 {
 	struct mlxsw_sp_acl *acl = mlxsw_sp->acl;
 	struct mlxsw_sp_acl_ruleset *ruleset;
@@ -337,7 +338,8 @@ mlxsw_sp_acl_ruleset_create(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		goto err_rhashtable_init;
 
-	err = ops->ruleset_add(mlxsw_sp, &acl->tcam, ruleset->priv);
+	err = ops->ruleset_add(mlxsw_sp, &acl->tcam, ruleset->priv,
+			       tmplt_elusage);
 	if (err)
 		goto err_ops_ruleset_add;
 
@@ -419,7 +421,8 @@ mlxsw_sp_acl_ruleset_lookup(struct mlxsw_sp *mlxsw_sp,
 struct mlxsw_sp_acl_ruleset *
 mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
 			 struct mlxsw_sp_acl_block *block, u32 chain_index,
-			 enum mlxsw_sp_acl_profile profile)
+			 enum mlxsw_sp_acl_profile profile,
+			 struct mlxsw_afk_element_usage *tmplt_elusage)
 {
 	const struct mlxsw_sp_acl_profile_ops *ops;
 	struct mlxsw_sp_acl *acl = mlxsw_sp->acl;
@@ -434,7 +437,8 @@ mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
 		mlxsw_sp_acl_ruleset_ref_inc(ruleset);
 		return ruleset;
 	}
-	return mlxsw_sp_acl_ruleset_create(mlxsw_sp, block, chain_index, ops);
+	return mlxsw_sp_acl_ruleset_create(mlxsw_sp, block, chain_index, ops,
+					   tmplt_elusage);
 }
 
 void mlxsw_sp_acl_ruleset_put(struct mlxsw_sp *mlxsw_sp,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
index e06d7d9e5b7f..310fd87895b8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
@@ -189,6 +189,8 @@ struct mlxsw_sp_acl_tcam_group {
 	struct mlxsw_sp_acl_tcam_group_ops *ops;
 	const struct mlxsw_sp_acl_tcam_pattern *patterns;
 	unsigned int patterns_count;
+	bool tmplt_elusage_set;
+	struct mlxsw_afk_element_usage tmplt_elusage;
 };
 
 struct mlxsw_sp_acl_tcam_chunk {
@@ -234,13 +236,19 @@ mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp *mlxsw_sp,
 			    struct mlxsw_sp_acl_tcam *tcam,
 			    struct mlxsw_sp_acl_tcam_group *group,
 			    const struct mlxsw_sp_acl_tcam_pattern *patterns,
-			    unsigned int patterns_count)
+			    unsigned int patterns_count,
+			    struct mlxsw_afk_element_usage *tmplt_elusage)
 {
 	int err;
 
 	group->tcam = tcam;
 	group->patterns = patterns;
 	group->patterns_count = patterns_count;
+	if (tmplt_elusage) {
+		group->tmplt_elusage_set = true;
+		memcpy(&group->tmplt_elusage, tmplt_elusage,
+		       sizeof(group->tmplt_elusage));
+	}
 	INIT_LIST_HEAD(&group->region_list);
 	err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id);
 	if (err)
@@ -449,6 +457,15 @@ mlxsw_sp_acl_tcam_group_use_patterns(struct mlxsw_sp_acl_tcam_group *group,
 	const struct mlxsw_sp_acl_tcam_pattern *pattern;
 	int i;
 
+	/* In case the template is set, we don't have to look up the pattern
+	 * and just use the template.
+	 */
+	if (group->tmplt_elusage_set) {
+		memcpy(out, &group->tmplt_elusage, sizeof(*out));
+		WARN_ON(!mlxsw_afk_element_usage_subset(elusage, out));
+		return;
+	}
+
 	for (i = 0; i < group->patterns_count; i++) {
 		pattern = &group->patterns[i];
 		mlxsw_afk_element_usage_fill(out, pattern->elements,
@@ -865,13 +882,15 @@ struct mlxsw_sp_acl_tcam_flower_rule {
 static int
 mlxsw_sp_acl_tcam_flower_ruleset_add(struct mlxsw_sp *mlxsw_sp,
 				     struct mlxsw_sp_acl_tcam *tcam,
-				     void *ruleset_priv)
+				     void *ruleset_priv,
+				     struct mlxsw_afk_element_usage *tmplt_elusage)
 {
 	struct mlxsw_sp_acl_tcam_flower_ruleset *ruleset = ruleset_priv;
 
 	return mlxsw_sp_acl_tcam_group_add(mlxsw_sp, tcam, &ruleset->group,
 					   mlxsw_sp_acl_tcam_patterns,
-					   MLXSW_SP_ACL_TCAM_PATTERNS_COUNT);
+					   MLXSW_SP_ACL_TCAM_PATTERNS_COUNT,
+					   tmplt_elusage);
 }
 
 static void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
index 68551da2221d..6403ec4f5267 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
@@ -64,7 +64,8 @@ int mlxsw_sp_acl_tcam_priority_get(struct mlxsw_sp *mlxsw_sp,
 struct mlxsw_sp_acl_profile_ops {
 	size_t ruleset_priv_size;
 	int (*ruleset_add)(struct mlxsw_sp *mlxsw_sp,
-			   struct mlxsw_sp_acl_tcam *tcam, void *ruleset_priv);
+			   struct mlxsw_sp_acl_tcam *tcam, void *ruleset_priv,
+			   struct mlxsw_afk_element_usage *tmplt_elusage);
 	void (*ruleset_del)(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv);
 	int (*ruleset_bind)(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv,
 			    struct mlxsw_sp_port *mlxsw_sp_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 201761a3539e..b3cb618775af 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -414,7 +414,7 @@ int mlxsw_sp_flower_replace(struct mlxsw_sp *mlxsw_sp,
 
 	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
 					   f->common.chain_index,
-					   MLXSW_SP_ACL_PROFILE_FLOWER);
+					   MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
 	if (IS_ERR(ruleset))
 		return PTR_ERR(ruleset);
 
@@ -458,7 +458,7 @@ void mlxsw_sp_flower_destroy(struct mlxsw_sp *mlxsw_sp,
 
 	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
 					   f->common.chain_index,
-					   MLXSW_SP_ACL_PROFILE_FLOWER);
+					   MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
 	if (IS_ERR(ruleset))
 		return;
 
@@ -484,7 +484,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
 
 	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
 					   f->common.chain_index,
-					   MLXSW_SP_ACL_PROFILE_FLOWER);
+					   MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
 	if (WARN_ON(IS_ERR(ruleset)))
 		return -EINVAL;
 
@@ -506,3 +506,41 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
 	mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
 	return err;
 }
+
+int mlxsw_sp_flower_tmplt_create(struct mlxsw_sp *mlxsw_sp,
+				 struct mlxsw_sp_acl_block *block,
+				 struct tc_cls_flower_offload *f)
+{
+	struct mlxsw_sp_acl_ruleset *ruleset;
+	struct mlxsw_sp_acl_rule_info rulei;
+	int err;
+
+	memset(&rulei, 0, sizeof(rulei));
+	err = mlxsw_sp_flower_parse(mlxsw_sp, block, &rulei, f);
+	if (err)
+		return err;
+	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
+					   f->common.chain_index,
+					   MLXSW_SP_ACL_PROFILE_FLOWER,
+					   &rulei.values.elusage);
+	if (IS_ERR(ruleset))
+		return PTR_ERR(ruleset);
+	/* keep the reference to the ruleset */
+	return 0;
+}
+
+void mlxsw_sp_flower_tmplt_destroy(struct mlxsw_sp *mlxsw_sp,
+				   struct mlxsw_sp_acl_block *block,
+				   struct tc_cls_flower_offload *f)
+{
+	struct mlxsw_sp_acl_ruleset *ruleset;
+
+	ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
+					   f->common.chain_index,
+					   MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
+	if (IS_ERR(ruleset))
+		return;
+	/* put the reference to the ruleset kept in create */
+	mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
+	mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
+}
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 10/12] selftests: forwarding: move shblock tc support check to a separate helper
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (8 preceding siblings ...)
  2018-07-23  7:23 ` [patch net-next v4 09/12] mlxsw: spectrum: Implement chain template hinting Jiri Pirko
@ 2018-07-23  7:24 ` Jiri Pirko
  2018-07-23  7:24 ` [patch net-next v4 11/12] selftests: forwarding: add tests for TC chains creation adn destruction Jiri Pirko
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

The shared block support is only needed for tc_shblock.sh. No need to
require that for other test.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 tools/testing/selftests/net/forwarding/lib.sh         | 3 +++
 tools/testing/selftests/net/forwarding/tc_shblocks.sh | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 2bb9cf303c53..ec94395a54a7 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -33,7 +33,10 @@ check_tc_version()
 		echo "SKIP: iproute2 too old; tc is missing JSON support"
 		exit 1
 	fi
+}
 
+check_tc_shblock_support()
+{
 	tc filter help 2>&1 | grep block &> /dev/null
 	if [[ $? -ne 0 ]]; then
 		echo "SKIP: iproute2 too old; tc is missing shared block support"
diff --git a/tools/testing/selftests/net/forwarding/tc_shblocks.sh b/tools/testing/selftests/net/forwarding/tc_shblocks.sh
index b5b917203815..9826a446e2c0 100755
--- a/tools/testing/selftests/net/forwarding/tc_shblocks.sh
+++ b/tools/testing/selftests/net/forwarding/tc_shblocks.sh
@@ -105,6 +105,8 @@ cleanup()
 	ip link set $swp2 address $swp2origmac
 }
 
+check_tc_shblock_support
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 11/12] selftests: forwarding: add tests for TC chains creation adn destruction
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (9 preceding siblings ...)
  2018-07-23  7:24 ` [patch net-next v4 10/12] selftests: forwarding: move shblock tc support check to a separate helper Jiri Pirko
@ 2018-07-23  7:24 ` Jiri Pirko
  2018-07-23  7:24 ` [patch net-next v4 12/12] selftests: forwarding: add tests for TC chain templates Jiri Pirko
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Add basic sanity tests for TC chains.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- split from the originally single patch
---
 tools/testing/selftests/net/forwarding/lib.sh      |  9 +++++++++
 .../testing/selftests/net/forwarding/tc_chains.sh  | 23 +++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index ec94395a54a7..158d59ffee40 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -44,6 +44,15 @@ check_tc_shblock_support()
 	fi
 }
 
+check_tc_chain_support()
+{
+	tc help 2>&1|grep chain &> /dev/null
+	if [[ $? -ne 0 ]]; then
+		echo "SKIP: iproute2 too old; tc is missing chain support"
+		exit 1
+	fi
+}
+
 if [[ "$(id -u)" -ne 0 ]]; then
 	echo "SKIP: need root privileges"
 	exit 0
diff --git a/tools/testing/selftests/net/forwarding/tc_chains.sh b/tools/testing/selftests/net/forwarding/tc_chains.sh
index d2c783e94df3..2730887a3b3a 100755
--- a/tools/testing/selftests/net/forwarding/tc_chains.sh
+++ b/tools/testing/selftests/net/forwarding/tc_chains.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-ALL_TESTS="unreachable_chain_test gact_goto_chain_test"
+ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain"
 NUM_NETIFS=2
 source tc_common.sh
 source lib.sh
@@ -80,6 +80,25 @@ gact_goto_chain_test()
 	log_test "gact goto chain ($tcflags)"
 }
 
+create_destroy_chain()
+{
+	RET=0
+
+	tc chain add dev $h2 ingress
+	check_err $? "Failed to create default chain"
+
+	tc chain add dev $h2 ingress chain 1
+	check_err $? "Failed to create chain 1"
+
+	tc chain del dev $h2 ingress
+	check_err $? "Failed to destroy default chain"
+
+	tc chain del dev $h2 ingress chain 1
+	check_err $? "Failed to destroy chain 1"
+
+	log_test "create destroy chain"
+}
+
 setup_prepare()
 {
 	h1=${NETIFS[p1]}
@@ -103,6 +122,8 @@ cleanup()
 	vrf_cleanup
 }
 
+check_tc_chain_support
+
 trap cleanup EXIT
 
 setup_prepare
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next v4 12/12] selftests: forwarding: add tests for TC chain templates
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (10 preceding siblings ...)
  2018-07-23  7:24 ` [patch net-next v4 11/12] selftests: forwarding: add tests for TC chains creation adn destruction Jiri Pirko
@ 2018-07-23  7:24 ` Jiri Pirko
  2018-07-23  7:24 ` [patch iproute2/net-next v4] tc: introduce support for " Jiri Pirko
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Add basic sanity tests for TC chain templates.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- split from the originally single patch
---
 .../testing/selftests/net/forwarding/tc_chains.sh  | 44 +++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_chains.sh b/tools/testing/selftests/net/forwarding/tc_chains.sh
index 2730887a3b3a..031e322e28b3 100755
--- a/tools/testing/selftests/net/forwarding/tc_chains.sh
+++ b/tools/testing/selftests/net/forwarding/tc_chains.sh
@@ -1,7 +1,8 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain"
+ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain \
+	   template_filter_fits"
 NUM_NETIFS=2
 source tc_common.sh
 source lib.sh
@@ -99,6 +100,47 @@ create_destroy_chain()
 	log_test "create destroy chain"
 }
 
+template_filter_fits()
+{
+	RET=0
+
+	tc chain add dev $h2 ingress protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+	tc chain add dev $h2 ingress chain 1 protocol ip \
+		flower src_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+
+	tc filter add dev $h2 ingress protocol ip pref 1 handle 1101 \
+		flower dst_mac $h2mac action drop
+	check_err $? "Failed to insert filter which fits template"
+
+	tc filter add dev $h2 ingress protocol ip pref 1 handle 1102 \
+		flower src_mac $h2mac action drop &> /dev/null
+	check_fail $? "Incorrectly succeded to insert filter which does not template"
+
+	tc filter add dev $h2 ingress chain 1 protocol ip pref 1 handle 1101 \
+		flower src_mac $h2mac action drop
+	check_err $? "Failed to insert filter which fits template"
+
+	tc filter add dev $h2 ingress chain 1 protocol ip pref 1 handle 1102 \
+		flower dst_mac $h2mac action drop &> /dev/null
+	check_fail $? "Incorrectly succeded to insert filter which does not template"
+
+	tc filter del dev $h2 ingress chain 1 protocol ip pref 1 handle 1102 \
+		flower &> /dev/null
+	tc filter del dev $h2 ingress chain 1 protocol ip pref 1 handle 1101 \
+		flower &> /dev/null
+
+	tc filter del dev $h2 ingress protocol ip pref 1 handle 1102 \
+		flower &> /dev/null
+	tc filter del dev $h2 ingress protocol ip pref 1 handle 1101 \
+		flower &> /dev/null
+
+	tc chain del dev $h2 ingress chain 1
+	tc chain del dev $h2 ingress
+
+	log_test "template filter fits"
+}
+
 setup_prepare()
 {
 	h1=${NETIFS[p1]}
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch iproute2/net-next v4] tc: introduce support for chain templates
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (11 preceding siblings ...)
  2018-07-23  7:24 ` [patch net-next v4 12/12] selftests: forwarding: add tests for TC chain templates Jiri Pirko
@ 2018-07-23  7:24 ` Jiri Pirko
  2018-07-25 17:03   ` David Ahern
  2018-07-23 16:28 ` [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw David Miller
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Jiri Pirko @ 2018-07-23  7:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@mellanox.com>

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- reworked to chain object
v1->v2:
- moved the template handling
  from "tc filter template" to "tc chaintemplate"
---
 include/uapi/linux/rtnetlink.h |   7 +++
 man/man8/tc.8                  |  26 ++++++++
 tc/tc.c                        |   5 +-
 tc/tc_common.h                 |   1 +
 tc/tc_filter.c                 | 131 +++++++++++++++++++++++++++++------------
 tc/tc_monitor.c                |   5 +-
 6 files changed, 135 insertions(+), 40 deletions(-)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index c3a7d8ecc7b9..8c1d600bfa33 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -150,6 +150,13 @@ enum {
 	RTM_NEWCACHEREPORT = 96,
 #define RTM_NEWCACHEREPORT RTM_NEWCACHEREPORT
 
+	RTM_NEWCHAIN = 100,
+#define RTM_NEWCHAIN RTM_NEWCHAIN
+	RTM_DELCHAIN,
+#define RTM_DELCHAIN RTM_DELCHAIN
+	RTM_GETCHAIN,
+#define RTM_GETCHAIN RTM_GETCHAIN
+
 	__RTM_MAX,
 #define RTM_MAX		(((__RTM_MAX + 3) & ~3) - 1)
 };
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 716dfec5f3a8..fd33f9b2f180 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -58,6 +58,22 @@ tc \- show / manipulate traffic control settings
 .B flowid
 \fIflow-id\fR
 
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain [ add | delete | get ] dev
+\fIDEV\fR
+.B [ parent
+\fIqdisc-id\fR
+.B | root ]\fR filtertype
+[ filtertype specific parameters ]
+
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain [ add | delete | get ] block
+\fIBLOCK_INDEX\fR filtertype
+[ filtertype specific parameters ]
+
+
 .B tc
 .RI "[ " OPTIONS " ]"
 .RI "[ " FORMAT " ]"
@@ -80,6 +96,16 @@ tc \- show / manipulate traffic control settings
 .RI "[ " OPTIONS " ]"
 .B filter show block
 \fIBLOCK_INDEX\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain show dev
+\fIDEV\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain show block
+\fIBLOCK_INDEX\fR
 
 .P
 .B tc
diff --git a/tc/tc.c b/tc/tc.c
index 0d223281ba25..2af67963c1e1 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -197,7 +197,8 @@ static void usage(void)
 	fprintf(stderr,
 		"Usage: tc [ OPTIONS ] OBJECT { COMMAND | help }\n"
 		"       tc [-force] -batch filename\n"
-		"where  OBJECT := { qdisc | class | filter | action | monitor | exec }\n"
+		"where  OBJECT := { qdisc | class | filter | chain |\n"
+		"                   action | monitor | exec }\n"
 		"       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[aw] |\n"
 		"                    -o[neline] | -j[son] | -p[retty] | -c[olor]\n"
 		"                    -b[atch] [filename] | -n[etns] name |\n"
@@ -212,6 +213,8 @@ static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
 		return do_class(argc-1, argv+1);
 	if (matches(*argv, "filter") == 0)
 		return do_filter(argc-1, argv+1, buf, buflen);
+	if (matches(*argv, "chain") == 0)
+		return do_chain(argc-1, argv+1, buf, buflen);
 	if (matches(*argv, "actions") == 0)
 		return do_action(argc-1, argv+1, buf, buflen);
 	if (matches(*argv, "monitor") == 0)
diff --git a/tc/tc_common.h b/tc/tc_common.h
index 49c24616c2c3..272d1727027d 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -8,6 +8,7 @@ extern struct rtnl_handle rth;
 extern int do_qdisc(int argc, char **argv);
 extern int do_class(int argc, char **argv);
 extern int do_filter(int argc, char **argv, void *buf, size_t buflen);
+extern int do_chain(int argc, char **argv, void *buf, size_t buflen);
 extern int do_action(int argc, char **argv, void *buf, size_t buflen);
 extern int do_tcmonitor(int argc, char **argv);
 extern int do_exec(int argc, char **argv);
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c5bb0bffe19b..15044b4bc6ed 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -45,6 +45,13 @@ static void usage(void)
 		"OPTIONS := ... try tc filter add <desired FILTER_KIND> help\n");
 }
 
+static void chain_usage(void)
+{
+	fprintf(stderr,
+		"Usage: tc chain [ add | del | get | show ] [ dev STRING ]\n"
+		"       tc chain [ add | del | get | show ] [ block BLOCK_INDEX ] ]\n");
+}
+
 struct tc_filter_req {
 	struct nlmsghdr		n;
 	struct tcmsg		t;
@@ -85,7 +92,8 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
 	req->n.nlmsg_type = cmd;
 	req->t.tcm_family = AF_UNSPEC;
 
-	if (cmd == RTM_NEWTFILTER && flags & NLM_F_CREATE)
+	if ((cmd == RTM_NEWTFILTER || cmd == RTM_NEWCHAIN) &&
+	    flags & NLM_F_CREATE)
 		protocol = htons(ETH_P_ALL);
 
 	while (argc > 0) {
@@ -261,7 +269,10 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (n->nlmsg_type != RTM_NEWTFILTER &&
 	    n->nlmsg_type != RTM_GETTFILTER &&
-	    n->nlmsg_type != RTM_DELTFILTER) {
+	    n->nlmsg_type != RTM_DELTFILTER &&
+	    n->nlmsg_type != RTM_NEWCHAIN &&
+	    n->nlmsg_type != RTM_GETCHAIN &&
+	    n->nlmsg_type != RTM_DELCHAIN) {
 		fprintf(stderr, "Not a filter(cmd %d)\n", n->nlmsg_type);
 		return 0;
 	}
@@ -273,27 +284,36 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	parse_rtattr(tb, TCA_MAX, TCA_RTA(t), len);
 
-	if (tb[TCA_KIND] == NULL) {
+	if (tb[TCA_KIND] == NULL && (n->nlmsg_type == RTM_NEWTFILTER ||
+				     n->nlmsg_type == RTM_GETTFILTER ||
+				     n->nlmsg_type == RTM_DELTFILTER)) {
 		fprintf(stderr, "print_filter: NULL kind\n");
 		return -1;
 	}
 
 	open_json_object(NULL);
 
-	if (n->nlmsg_type == RTM_DELTFILTER)
+	if (n->nlmsg_type == RTM_DELTFILTER || n->nlmsg_type == RTM_DELCHAIN)
 		print_bool(PRINT_ANY, "deleted", "deleted ", true);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER &&
+	if ((n->nlmsg_type == RTM_NEWTFILTER ||
+	     n->nlmsg_type == RTM_NEWCHAIN) &&
 			(n->nlmsg_flags & NLM_F_CREATE) &&
 			!(n->nlmsg_flags & NLM_F_EXCL))
 		print_bool(PRINT_ANY, "replaced", "replaced ", true);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER &&
+	if ((n->nlmsg_type == RTM_NEWTFILTER ||
+	     n->nlmsg_type == RTM_NEWCHAIN) &&
 			(n->nlmsg_flags & NLM_F_CREATE) &&
 			(n->nlmsg_flags & NLM_F_EXCL))
 		print_bool(PRINT_ANY, "added", "added ", true);
 
-	print_string(PRINT_FP, NULL, "filter ", NULL);
+	if (n->nlmsg_type == RTM_NEWTFILTER ||
+	    n->nlmsg_type == RTM_GETTFILTER ||
+	    n->nlmsg_type == RTM_DELTFILTER)
+		print_string(PRINT_FP, NULL, "filter ", NULL);
+	else
+		print_string(PRINT_FP, NULL, "chain ", NULL);
 	if (t->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
 		if (!filter_block_index ||
 		    filter_block_index != t->tcm_block_index)
@@ -317,7 +337,9 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		}
 	}
 
-	if (t->tcm_info) {
+	if (t->tcm_info && (n->nlmsg_type == RTM_NEWTFILTER ||
+			    n->nlmsg_type == RTM_DELTFILTER ||
+			    n->nlmsg_type == RTM_GETTFILTER)) {
 		f_proto = TC_H_MIN(t->tcm_info);
 		__u32 prio = TC_H_MAJ(t->tcm_info)>>16;
 
@@ -334,7 +356,8 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				print_uint(PRINT_ANY, "pref", "pref %u ", prio);
 		}
 	}
-	print_string(PRINT_ANY, "kind", "%s ", rta_getattr_str(tb[TCA_KIND]));
+	if (tb[TCA_KIND])
+		print_string(PRINT_ANY, "kind", "%s ", rta_getattr_str(tb[TCA_KIND]));
 
 	if (tb[TCA_CHAIN]) {
 		__u32 chain_index = rta_getattr_u32(tb[TCA_CHAIN]);
@@ -345,15 +368,17 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				   chain_index);
 	}
 
-	q = get_filter_kind(RTA_DATA(tb[TCA_KIND]));
-	if (tb[TCA_OPTIONS]) {
-		open_json_object("options");
-		if (q)
-			q->print_fopt(q, fp, tb[TCA_OPTIONS], t->tcm_handle);
-		else
-			print_string(PRINT_FP, NULL,
-				     "[cannot parse parameters]", NULL);
-		close_json_object();
+	if (tb[TCA_KIND]) {
+		q = get_filter_kind(RTA_DATA(tb[TCA_KIND]));
+		if (tb[TCA_OPTIONS]) {
+			open_json_object("options");
+			if (q)
+				q->print_fopt(q, fp, tb[TCA_OPTIONS], t->tcm_handle);
+			else
+				print_string(PRINT_FP, NULL,
+					     "[cannot parse parameters]", NULL);
+			close_json_object();
+		}
 	}
 	print_string(PRINT_FP, NULL, "\n", NULL);
 
@@ -496,17 +521,19 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		argc--; argv++;
 	}
 
-	if (!protocol_set) {
-		fprintf(stderr, "Must specify filter protocol\n");
-		return -1;
-	}
+	if (cmd == RTM_GETTFILTER) {
+		if (!protocol_set) {
+			fprintf(stderr, "Must specify filter protocol\n");
+			return -1;
+		}
 
-	if (!prio) {
-		fprintf(stderr, "Must specify filter priority\n");
-		return -1;
-	}
+		if (!prio) {
+			fprintf(stderr, "Must specify filter priority\n");
+			return -1;
+		}
 
-	req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+		req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+	}
 
 	if (chain_index_set)
 		addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
@@ -516,11 +543,13 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
-	if (k[0])
-		addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
-	else {
-		fprintf(stderr, "Must specify filter type\n");
-		return -1;
+	if (cmd == RTM_GETTFILTER) {
+		if (k[0])
+			addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
+		else {
+			fprintf(stderr, "Must specify filter type\n");
+			return -1;
+		}
 	}
 
 	if (d[0])  {
@@ -539,10 +568,11 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
-	if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
+	if (cmd == RTM_GETTFILTER &&
+	    q->parse_fopt(q, fhandle, argc, argv, &req.n))
 		return 1;
 
-	if (!fhandle) {
+	if (!fhandle && cmd == RTM_GETTFILTER) {
 		fprintf(stderr, "Must specify filter \"handle\"\n");
 		return -1;
 	}
@@ -569,7 +599,7 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 	return 0;
 }
 
-static int tc_filter_list(int argc, char **argv)
+static int tc_filter_list(int cmd, int argc, char **argv)
 {
 	struct {
 		struct nlmsghdr n;
@@ -577,7 +607,7 @@ static int tc_filter_list(int argc, char **argv)
 		char buf[MAX_MSG];
 	} req = {
 		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
-		.n.nlmsg_type = RTM_GETTFILTER,
+		.n.nlmsg_type = cmd,
 		.t.tcm_parent = TC_H_UNSPEC,
 		.t.tcm_family = AF_UNSPEC,
 	};
@@ -725,7 +755,7 @@ static int tc_filter_list(int argc, char **argv)
 int do_filter(int argc, char **argv, void *buf, size_t buflen)
 {
 	if (argc < 1)
-		return tc_filter_list(0, NULL);
+		return tc_filter_list(RTM_GETTFILTER, 0, NULL);
 	if (matches(*argv, "add") == 0)
 		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
 					argc-1, argv+1, buf, buflen);
@@ -742,7 +772,7 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
 		return tc_filter_get(RTM_GETTFILTER, 0,  argc-1, argv+1);
 	if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
 	    || matches(*argv, "lst") == 0)
-		return tc_filter_list(argc-1, argv+1);
+		return tc_filter_list(RTM_GETTFILTER, argc-1, argv+1);
 	if (matches(*argv, "help") == 0) {
 		usage();
 		return 0;
@@ -751,3 +781,28 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
 		*argv);
 	return -1;
 }
+
+int do_chain(int argc, char **argv, void *buf, size_t buflen)
+{
+	if (argc < 1)
+		return tc_filter_list(RTM_GETCHAIN, 0, NULL);
+	if (matches(*argv, "add") == 0) {
+		return tc_filter_modify(RTM_NEWCHAIN, NLM_F_EXCL | NLM_F_CREATE,
+					argc - 1, argv + 1, buf, buflen);
+	} else if (matches(*argv, "delete") == 0) {
+		return tc_filter_modify(RTM_DELCHAIN, 0,
+					argc - 1, argv + 1, buf, buflen);
+	} else if (matches(*argv, "get") == 0) {
+		return tc_filter_get(RTM_GETCHAIN, 0,
+				     argc - 1, argv + 1);
+	} else if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0 ||
+		   matches(*argv, "lst") == 0) {
+		return tc_filter_list(RTM_GETCHAIN, argc - 1, argv + 1);
+	} else if (matches(*argv, "help") == 0) {
+		chain_usage();
+		return 0;
+	}
+	fprintf(stderr, "Command \"%s\" is unknown, try \"tc chain help\".\n",
+		*argv);
+	return -1;
+}
diff --git a/tc/tc_monitor.c b/tc/tc_monitor.c
index 077b138d1ec5..1f1ee08fb9cf 100644
--- a/tc/tc_monitor.c
+++ b/tc/tc_monitor.c
@@ -43,7 +43,10 @@ static int accept_tcmsg(const struct sockaddr_nl *who,
 	if (timestamp)
 		print_timestamp(fp);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER || n->nlmsg_type == RTM_DELTFILTER) {
+	if (n->nlmsg_type == RTM_NEWTFILTER ||
+	    n->nlmsg_type == RTM_DELTFILTER ||
+	    n->nlmsg_type == RTM_NEWCHAIN ||
+	    n->nlmsg_type == RTM_DELCHAIN) {
 		print_filter(who, n, arg);
 		return 0;
 	}
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (12 preceding siblings ...)
  2018-07-23  7:24 ` [patch iproute2/net-next v4] tc: introduce support for " Jiri Pirko
@ 2018-07-23 16:28 ` David Miller
  2018-07-23 23:36 ` Jakub Kicinski
  2018-07-24  3:45 ` David Miller
  15 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2018-07-23 16:28 UTC (permalink / raw)
  To: jiri
  Cc: netdev, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 23 Jul 2018 09:23:03 +0200

> For the TC clsact offload these days, some of HW drivers need
> to hold a magic ball. The reason is, with the first inserted rule inside
> HW they need to guess what fields will be used for the matching. If
> later on this guess proves to be wrong and user adds a filter with a
> different field to match, there's a problem. Mlxsw resolves it now with
> couple of patterns. Those try to cover as many match fields as possible.
> This aproach is far from optimal, both performance-wise and scale-wise.
> Also, there is a combination of filters that in certain order won't
> succeed.
> 
> Most of the time, when user inserts filters in chain, he knows right away
> how the filters are going to look like - what type and option will they
> have. For example, he knows that he will only insert filters of type
> flower matching destination IP address. He can specify a template that
> would cover all the filters in the chain.
> 
> This patchset is providing the possibility to user to provide such
> template to kernel and propagate it all the way down to device
> drivers.

I just want to say I like how small the mlxsw driver patch is :-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (13 preceding siblings ...)
  2018-07-23 16:28 ` [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw David Miller
@ 2018-07-23 23:36 ` Jakub Kicinski
  2018-07-24  3:45 ` David Miller
  15 siblings, 0 replies; 25+ messages in thread
From: Jakub Kicinski @ 2018-07-23 23:36 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, jhs, xiyou.wangcong, simon.horman, john.hurley,
	dsahern, mlxsw, sridhar.samudrala

On Mon, 23 Jul 2018 09:23:03 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> For the TC clsact offload these days, some of HW drivers need
> to hold a magic ball. The reason is, with the first inserted rule inside
> HW they need to guess what fields will be used for the matching. If
> later on this guess proves to be wrong and user adds a filter with a
> different field to match, there's a problem. Mlxsw resolves it now with
> couple of patterns. Those try to cover as many match fields as possible.
> This aproach is far from optimal, both performance-wise and scale-wise.
> Also, there is a combination of filters that in certain order won't
> succeed.
> 
> Most of the time, when user inserts filters in chain, he knows right away
> how the filters are going to look like - what type and option will they
> have. For example, he knows that he will only insert filters of type
> flower matching destination IP address. He can specify a template that
> would cover all the filters in the chain.
> 
> This patchset is providing the possibility to user to provide such
> template to kernel and propagate it all the way down to device
> drivers.

LGTM, thanks for the changes!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw
  2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
                   ` (14 preceding siblings ...)
  2018-07-23 23:36 ` Jakub Kicinski
@ 2018-07-24  3:45 ` David Miller
  15 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2018-07-24  3:45 UTC (permalink / raw)
  To: jiri
  Cc: netdev, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, dsahern, mlxsw, sridhar.samudrala

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 23 Jul 2018 09:23:03 +0200

> For the TC clsact offload these days, some of HW drivers need
> to hold a magic ball. The reason is, with the first inserted rule inside
> HW they need to guess what fields will be used for the matching. If
> later on this guess proves to be wrong and user adds a filter with a
> different field to match, there's a problem. Mlxsw resolves it now with
> couple of patterns. Those try to cover as many match fields as possible.
> This aproach is far from optimal, both performance-wise and scale-wise.
> Also, there is a combination of filters that in certain order won't
> succeed.
> 
> Most of the time, when user inserts filters in chain, he knows right away
> how the filters are going to look like - what type and option will they
> have. For example, he knows that he will only insert filters of type
> flower matching destination IP address. He can specify a template that
> would cover all the filters in the chain.
> 
> This patchset is providing the possibility to user to provide such
> template to kernel and propagate it all the way down to device
> drivers.

Series applied, thanks Jiri!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-23  7:23 ` [patch net-next v4 03/12] net: sched: introduce chain object to uapi Jiri Pirko
@ 2018-07-24 22:30   ` Cong Wang
  2018-07-24 23:20     ` Cong Wang
  0 siblings, 1 reply; 25+ messages in thread
From: Cong Wang @ 2018-07-24 22:30 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

On Mon, Jul 23, 2018 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote:
> +       switch (n->nlmsg_type) {
> +       case RTM_NEWCHAIN:
> +               /* In case the chain was successfully added, take a reference
> +                * to the chain. This ensures that an empty chain
> +                * does not disappear at the end of this function.
> +                */
> +               tcf_chain_hold(chain);
> +               chain->explicitly_created = true;
> +               tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
> +                               RTM_NEWCHAIN, false);
> +               break;
> +       case RTM_DELCHAIN:
> +               /* Flush the chain first as the user requested chain removal. */
> +               tcf_chain_flush(chain);
> +               /* In case the chain was successfully deleted, put a reference
> +                * to the chain previously taken during addition.
> +                */
> +               tcf_chain_put_explicitly_created(chain);
> +               break;

I don't see you send notification to user-space when deleting a chain,
am I missing anything?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-24 22:30   ` Cong Wang
@ 2018-07-24 23:20     ` Cong Wang
  2018-07-25  6:46       ` Jiri Pirko
  0 siblings, 1 reply; 25+ messages in thread
From: Cong Wang @ 2018-07-24 23:20 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

On Tue, Jul 24, 2018 at 3:30 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Jul 23, 2018 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote:
> > +       switch (n->nlmsg_type) {
> > +       case RTM_NEWCHAIN:
> > +               /* In case the chain was successfully added, take a reference
> > +                * to the chain. This ensures that an empty chain
> > +                * does not disappear at the end of this function.
> > +                */
> > +               tcf_chain_hold(chain);
> > +               chain->explicitly_created = true;
> > +               tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
> > +                               RTM_NEWCHAIN, false);
> > +               break;
> > +       case RTM_DELCHAIN:
> > +               /* Flush the chain first as the user requested chain removal. */
> > +               tcf_chain_flush(chain);
> > +               /* In case the chain was successfully deleted, put a reference
> > +                * to the chain previously taken during addition.
> > +                */
> > +               tcf_chain_put_explicitly_created(chain);
> > +               break;
>
> I don't see you send notification to user-space when deleting a chain,
> am I missing anything?

Oh, it is hidden in tcf_chain_put():

void tcf_chain_put(struct tcf_chain *chain)
{
        if (--chain->refcnt == 0) {
                tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false);
                tc_chain_tmplt_del(chain);
                tcf_chain_destroy(chain);
        }
}

So, you only send out notification when the last refcnt is gone.

If the chain that is being deleted by a user is still used by an action,
you return 0 or -EPERM?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-24 23:20     ` Cong Wang
@ 2018-07-25  6:46       ` Jiri Pirko
  2018-07-25 16:40         ` Cong Wang
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Pirko @ 2018-07-25  6:46 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Jul 24, 2018 at 3:30 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> On Mon, Jul 23, 2018 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> > +       switch (n->nlmsg_type) {
>> > +       case RTM_NEWCHAIN:
>> > +               /* In case the chain was successfully added, take a reference
>> > +                * to the chain. This ensures that an empty chain
>> > +                * does not disappear at the end of this function.
>> > +                */
>> > +               tcf_chain_hold(chain);
>> > +               chain->explicitly_created = true;
>> > +               tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
>> > +                               RTM_NEWCHAIN, false);
>> > +               break;
>> > +       case RTM_DELCHAIN:
>> > +               /* Flush the chain first as the user requested chain removal. */
>> > +               tcf_chain_flush(chain);
>> > +               /* In case the chain was successfully deleted, put a reference
>> > +                * to the chain previously taken during addition.
>> > +                */
>> > +               tcf_chain_put_explicitly_created(chain);
>> > +               break;
>>
>> I don't see you send notification to user-space when deleting a chain,
>> am I missing anything?
>
>Oh, it is hidden in tcf_chain_put():
>
>void tcf_chain_put(struct tcf_chain *chain)
>{
>        if (--chain->refcnt == 0) {
>                tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false);
>                tc_chain_tmplt_del(chain);
>                tcf_chain_destroy(chain);
>        }
>}
>
>So, you only send out notification when the last refcnt is gone.
>
>If the chain that is being deleted by a user is still used by an action,
>you return 0 or -EPERM?

0 and the chain stays there until the action is removed. Hmm, do you thing
that -EPERM should be returned in that case? The thing is, we have to
flush the chain in order to see the action references are there. We would
have to have 2 ref counters, one for filter, one for actions.
What do you think?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-25  6:46       ` Jiri Pirko
@ 2018-07-25 16:40         ` Cong Wang
  2018-07-26  7:38           ` Jiri Pirko
  0 siblings, 1 reply; 25+ messages in thread
From: Cong Wang @ 2018-07-25 16:40 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

On Tue, Jul 24, 2018 at 11:49 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
> >So, you only send out notification when the last refcnt is gone.
> >
> >If the chain that is being deleted by a user is still used by an action,
> >you return 0 or -EPERM?
>
> 0 and the chain stays there until the action is removed. Hmm, do you thing
> that -EPERM should be returned in that case? The thing is, we have to
> flush the chain in order to see the action references are there. We would
> have to have 2 ref counters, one for filter, one for actions.
> What do you think?

_If_ RTM_DELCHAIN does decrease the chain refcnt, then it is
broken:

# tc chain add X... (refcnt == 1)
# tc action add ... goto chain X (refcnt==2)
# tc chain del X ... (refcnt== 1)
# tc chain del X ... (refcnt==0)

RTM_DELCHAIN should just test if refcnt is 1, if it is, delete it,
otherwise return -EPERM. This is how we handle tc standalone
actions, see tcf_idr_delete_index().

Yes, you might need two refcnt's here.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch iproute2/net-next v4] tc: introduce support for chain templates
  2018-07-23  7:24 ` [patch iproute2/net-next v4] tc: introduce support for " Jiri Pirko
@ 2018-07-25 17:03   ` David Ahern
  0 siblings, 0 replies; 25+ messages in thread
From: David Ahern @ 2018-07-25 17:03 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
	john.hurley, mlxsw, sridhar.samudrala

On 7/23/18 1:24 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v3->v4:
> - reworked to chain object
> v1->v2:
> - moved the template handling
>   from "tc filter template" to "tc chaintemplate"
> ---
>  include/uapi/linux/rtnetlink.h |   7 +++
>  man/man8/tc.8                  |  26 ++++++++
>  tc/tc.c                        |   5 +-
>  tc/tc_common.h                 |   1 +
>  tc/tc_filter.c                 | 131 +++++++++++++++++++++++++++++------------
>  tc/tc_monitor.c                |   5 +-
>  6 files changed, 135 insertions(+), 40 deletions(-)
> 

applied to iproute2-next. Thanks

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-25 16:40         ` Cong Wang
@ 2018-07-26  7:38           ` Jiri Pirko
  2018-07-26 10:06             ` Jiri Pirko
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Pirko @ 2018-07-26  7:38 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

Wed, Jul 25, 2018 at 06:40:44PM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Jul 24, 2018 at 11:49 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
>> >So, you only send out notification when the last refcnt is gone.
>> >
>> >If the chain that is being deleted by a user is still used by an action,
>> >you return 0 or -EPERM?
>>
>> 0 and the chain stays there until the action is removed. Hmm, do you thing
>> that -EPERM should be returned in that case? The thing is, we have to
>> flush the chain in order to see the action references are there. We would
>> have to have 2 ref counters, one for filter, one for actions.
>> What do you think?
>
>_If_ RTM_DELCHAIN does decrease the chain refcnt, then it is
>broken:
>
># tc chain add X... (refcnt == 1)
># tc action add ... goto chain X (refcnt==2)
># tc chain del X ... (refcnt== 1)
># tc chain del X ... (refcnt==0)
>
>RTM_DELCHAIN should just test if refcnt is 1, if it is, delete it,
>otherwise return -EPERM. This is how we handle tc standalone
>actions, see tcf_idr_delete_index().
>
>Yes, you might need two refcnt's here.

Okay. Sounds good. I'm on it. 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-26  7:38           ` Jiri Pirko
@ 2018-07-26 10:06             ` Jiri Pirko
  2018-07-26 12:27               ` Jiri Pirko
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Pirko @ 2018-07-26 10:06 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

Thu, Jul 26, 2018 at 09:38:39AM CEST, jiri@resnulli.us wrote:
>Wed, Jul 25, 2018 at 06:40:44PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Tue, Jul 24, 2018 at 11:49 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>>
>>> Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
>>> >So, you only send out notification when the last refcnt is gone.
>>> >
>>> >If the chain that is being deleted by a user is still used by an action,
>>> >you return 0 or -EPERM?
>>>
>>> 0 and the chain stays there until the action is removed. Hmm, do you thing
>>> that -EPERM should be returned in that case? The thing is, we have to
>>> flush the chain in order to see the action references are there. We would
>>> have to have 2 ref counters, one for filter, one for actions.
>>> What do you think?
>>
>>_If_ RTM_DELCHAIN does decrease the chain refcnt, then it is
>>broken:
>>
>># tc chain add X... (refcnt == 1)
>># tc action add ... goto chain X (refcnt==2)
>># tc chain del X ... (refcnt== 1)
>># tc chain del X ... (refcnt==0)
>>
>>RTM_DELCHAIN should just test if refcnt is 1, if it is, delete it,
>>otherwise return -EPERM. This is how we handle tc standalone
>>actions, see tcf_idr_delete_index().
>>
>>Yes, you might need two refcnt's here.
>
>Okay. Sounds good. I'm on it. 

Actually, I found an issue. The action to "goto chain" might be attached
to a filter in the same chain. That is completely legitimate usage.
When I do:
# tc chain del X
I expect the chain to be flushed and removed. If there is an action
there with "goto" to the same chain, the command should be successful.
However, I don't see any easy way to find out if the chain is referenced
only by actions used by filters in the same chain :/

Thoughts?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi
  2018-07-26 10:06             ` Jiri Pirko
@ 2018-07-26 12:27               ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2018-07-26 12:27 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jakub Kicinski, Simon Horman, john.hurley, David Ahern, mlxsw,
	sridhar.samudrala

Thu, Jul 26, 2018 at 12:06:14PM CEST, jiri@resnulli.us wrote:
>Thu, Jul 26, 2018 at 09:38:39AM CEST, jiri@resnulli.us wrote:
>>Wed, Jul 25, 2018 at 06:40:44PM CEST, xiyou.wangcong@gmail.com wrote:
>>>On Tue, Jul 24, 2018 at 11:49 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>
>>>> Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote:
>>>> >So, you only send out notification when the last refcnt is gone.
>>>> >
>>>> >If the chain that is being deleted by a user is still used by an action,
>>>> >you return 0 or -EPERM?
>>>>
>>>> 0 and the chain stays there until the action is removed. Hmm, do you thing
>>>> that -EPERM should be returned in that case? The thing is, we have to
>>>> flush the chain in order to see the action references are there. We would
>>>> have to have 2 ref counters, one for filter, one for actions.
>>>> What do you think?
>>>
>>>_If_ RTM_DELCHAIN does decrease the chain refcnt, then it is
>>>broken:
>>>
>>># tc chain add X... (refcnt == 1)
>>># tc action add ... goto chain X (refcnt==2)
>>># tc chain del X ... (refcnt== 1)
>>># tc chain del X ... (refcnt==0)
>>>
>>>RTM_DELCHAIN should just test if refcnt is 1, if it is, delete it,
>>>otherwise return -EPERM. This is how we handle tc standalone
>>>actions, see tcf_idr_delete_index().
>>>
>>>Yes, you might need two refcnt's here.
>>
>>Okay. Sounds good. I'm on it. 
>
>Actually, I found an issue. The action to "goto chain" might be attached
>to a filter in the same chain. That is completely legitimate usage.
>When I do:
># tc chain del X
>I expect the chain to be flushed and removed. If there is an action
>there with "goto" to the same chain, the command should be successful.
>However, I don't see any easy way to find out if the chain is referenced
>only by actions used by filters in the same chain :/
>
>Thoughts?

I'm now working on a patch that would treat empty chains implicitly
created or deleted by user that only are referenced by action as a
zombie ones. They won't be visible on dump. User won't know about them,
they would only serve as a place holder for "goto chain" actions.
I think it is reasonable. What do you think.

Will send the RFC in few hours.


>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-07-26 13:46 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-23  7:23 [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 01/12] net: sched: push ops lookup bits into tcf_proto_lookup_ops() Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 02/12] net: sched: Avoid implicit chain 0 creation Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 03/12] net: sched: introduce chain object to uapi Jiri Pirko
2018-07-24 22:30   ` Cong Wang
2018-07-24 23:20     ` Cong Wang
2018-07-25  6:46       ` Jiri Pirko
2018-07-25 16:40         ` Cong Wang
2018-07-26  7:38           ` Jiri Pirko
2018-07-26 10:06             ` Jiri Pirko
2018-07-26 12:27               ` Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 04/12] net: sched: introduce chain templates Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 05/12] net: sched: cls_flower: move key/mask dumping into a separate function Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 06/12] net: sched: cls_flower: change fl_init_dissector to accept mask and dissector Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 07/12] net: sched: cls_flower: implement chain templates Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 08/12] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers Jiri Pirko
2018-07-23  7:23 ` [patch net-next v4 09/12] mlxsw: spectrum: Implement chain template hinting Jiri Pirko
2018-07-23  7:24 ` [patch net-next v4 10/12] selftests: forwarding: move shblock tc support check to a separate helper Jiri Pirko
2018-07-23  7:24 ` [patch net-next v4 11/12] selftests: forwarding: add tests for TC chains creation adn destruction Jiri Pirko
2018-07-23  7:24 ` [patch net-next v4 12/12] selftests: forwarding: add tests for TC chain templates Jiri Pirko
2018-07-23  7:24 ` [patch iproute2/net-next v4] tc: introduce support for " Jiri Pirko
2018-07-25 17:03   ` David Ahern
2018-07-23 16:28 ` [patch net-next v4 00/12] sched: introduce chain templates support with offloading to mlxsw David Miller
2018-07-23 23:36 ` Jakub Kicinski
2018-07-24  3:45 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.