[patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

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 template assigned by default:
# tc filter template show dev dummy0 ingress

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

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

Add another template, this time for chain number 22:
# tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
# tc filter template show dev dummy0 ingress
filter flower chain 0
  dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
  eth_type ipv4
filter 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: 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: 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: 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: Mask does not fit the template.
We have an error talking to the kernel, -1

Removal of a template from non-empty chain would fail:
# tc filter template del dev dummy0 ingress
Error: The chain is not empty, unable to delete template.
We have an error talking to the kernel, -1

Once the chain is flushed, the template could be removed:
# tc filter del dev dummy0 ingress
# tc filter template del dev dummy0 ingress

---
v1->v2:
-patch 6:
  - remove leftover extack arg in fl_hw_create_tmplt()

Jiri Pirko (9):
  net: sched: push ops lookup bits into tcf_proto_lookup_ops()
  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 chain templates

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |   5 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  12 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c |  12 +-
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c    |  25 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  |  44 ++-
 include/net/pkt_cls.h                              |   2 +
 include/net/sch_generic.h                          |  14 +-
 include/uapi/linux/rtnetlink.h                     |   7 +
 net/sched/cls_api.c                                | 424 +++++++++++++++++++--
 net/sched/cls_basic.c                              |   2 +-
 net/sched/cls_bpf.c                                |   3 +-
 net/sched/cls_cgroup.c                             |   2 +-
 net/sched/cls_flow.c                               |   3 +-
 net/sched/cls_flower.c                             | 250 +++++++++---
 net/sched/cls_fw.c                                 |   3 +-
 net/sched/cls_matchall.c                           |   3 +-
 net/sched/cls_route.c                              |   2 +-
 net/sched/cls_rsvp.h                               |   3 +-
 net/sched/cls_tcindex.c                            |   2 +-
 net/sched/cls_u32.c                                |   2 +-
 security/selinux/nlmsgtab.c                        |   2 +-
 tools/testing/selftests/net/forwarding/lib.sh      |  12 +
 .../selftests/net/forwarding/tc_chaintemplates.sh  | 160 ++++++++
 .../selftests/net/forwarding/tc_shblocks.sh        |   2 +
 24 files changed, 900 insertions(+), 96 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_chaintemplates.sh

-- 
2.14.4

[patch net-next v2 1/9] net: sched: push ops lookup bits into tcf_proto_lookup_ops()

[Jiri Pirko]

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 cdc3c87c53e6..db45931bbada 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

[patch net-next v2 2/9] net: sched: introduce chain templates

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduce a group of new tc-rtnl commands to allow user to set per-chain
template. Templates lock down individual chains for particular
classifier type/options combinations. The classifier needs to support
templates, otherwise kernel would reply with error.

For example, to lock chain 22 to allow only filters of type
flower with destination mac address, user needs to do:
  chain 22 flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF

In case the chain already contains some filters it is not possible to
add or remove template. That is permitted only for empty chains.

Alongside with add/del commands, introduce also get/dump and
notifications.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/sch_generic.h      |  14 +-
 include/uapi/linux/rtnetlink.h |   7 +
 net/sched/cls_api.c            | 371 ++++++++++++++++++++++++++++++++++++++++-
 net/sched/cls_basic.c          |   2 +-
 net/sched/cls_bpf.c            |   3 +-
 net/sched/cls_cgroup.c         |   2 +-
 net/sched/cls_flow.c           |   3 +-
 net/sched/cls_flower.c         |   3 +-
 net/sched/cls_fw.c             |   3 +-
 net/sched/cls_matchall.c       |   3 +-
 net/sched/cls_route.c          |   2 +-
 net/sched/cls_rsvp.h           |   3 +-
 net/sched/cls_tcindex.c        |   2 +-
 net/sched/cls_u32.c            |   2 +-
 security/selinux/nlmsgtab.c    |   2 +-
 15 files changed, 405 insertions(+), 17 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 6488daa32f82..f2a27d41fed5 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -235,6 +235,8 @@ struct tcf_result {
 	};
 };
 
+struct tcf_chain;
+
 struct tcf_proto_ops {
 	struct list_head	head;
 	char			kind[IFNAMSIZ];
@@ -250,17 +252,25 @@ struct tcf_proto_ops {
 	int			(*change)(struct net *net, struct sk_buff *,
 					struct tcf_proto*, unsigned long,
 					u32 handle, struct nlattr **,
-					void **, bool,
+					void **, bool, void *tmplt_priv,
 					struct netlink_ext_ack *);
 	int			(*delete)(struct tcf_proto *tp, void *arg,
 					  bool *last,
 					  struct netlink_ext_ack *);
 	void			(*walk)(struct tcf_proto*, struct tcf_walker *arg);
 	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;
 };
@@ -299,6 +309,8 @@ struct tcf_chain {
 	struct tcf_block *block;
 	u32 index; /* chain index */
 	unsigned int refcnt;
+	const struct tcf_proto_ops *tmplt_ops;
+	void *tmplt_priv;
 };
 
 struct tcf_block {
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7d8502313c99..45fd8cc1fdb2 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_NEWCHAINTMPLT = 100,
+#define RTM_NEWCHAINTMPLT RTM_NEWCHAINTMPLT
+	RTM_DELCHAINTMPLT,
+#define RTM_DELCHAINTMPLT RTM_DELCHAINTMPLT
+	RTM_GETCHAINTMPLT,
+#define RTM_GETCHAINTMPLT RTM_GETCHAINTMPLT
+
 	__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 db45931bbada..0c88520f80f2 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -227,7 +227,7 @@ static void tcf_chain_head_change(struct tcf_chain *chain,
 		tcf_chain_head_change_item(item, tp_head);
 }
 
-static void tcf_chain_flush(struct tcf_chain *chain)
+static void tcf_chain_flush(struct tcf_chain *chain, bool destroy_template)
 {
 	struct tcf_proto *tp = rtnl_dereference(chain->filter_chain);
 
@@ -238,6 +238,11 @@ static void tcf_chain_flush(struct tcf_chain *chain)
 		tp = rtnl_dereference(chain->filter_chain);
 		tcf_chain_put(chain);
 	}
+	if (destroy_template && chain->tmplt_ops) {
+		chain->tmplt_ops->tmplt_destroy(chain->tmplt_priv);
+		module_put(chain->tmplt_ops->owner);
+		tcf_chain_put(chain);
+	}
 }
 
 static void tcf_chain_destroy(struct tcf_chain *chain)
@@ -691,7 +696,7 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
 			tcf_chain_hold(chain);
 
 		list_for_each_entry(chain, &block->chain_list, list)
-			tcf_chain_flush(chain);
+			tcf_chain_flush(chain, true);
 	}
 
 	tcf_block_offload_unbind(block, q, ei);
@@ -1191,9 +1196,15 @@ 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);
+			      chain->tmplt_priv, extack);
 	if (err == 0) {
 		if (tp_created)
 			tcf_chain_tp_insert(chain, &chain_info, tp);
@@ -1274,7 +1285,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	if (prio == 0) {
 		tfilter_notify_chain(net, skb, block, q, parent, n,
 				     chain, RTM_DELTFILTER);
-		tcf_chain_flush(chain);
+		tcf_chain_flush(chain, false);
 		err = 0;
 		goto errout;
 	}
@@ -1570,6 +1581,354 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int tc_tmplt_fill_node(struct tcf_chain *chain, struct net *net,
+			      struct sk_buff *skb, struct tcf_block *block,
+			      struct Qdisc *q, u32 parent,
+			      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)
+		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 (q) {
+		tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
+		tcm->tcm_parent = parent;
+	} else {
+		tcm->tcm_ifindex = TCM_IFINDEX_MAGIC_BLOCK;
+		tcm->tcm_block_index = block->index;
+	}
+
+	if (nla_put_string(skb, TCA_KIND, ops->kind))
+		goto nla_put_failure;
+	if (nla_put_u32(skb, TCA_CHAIN, chain->index))
+		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;
+
+out_nlmsg_trim:
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -EMSGSIZE;
+}
+
+static int tc_tmplt_notify(struct tcf_chain *chain, struct net *net,
+			   struct sk_buff *oskb, struct nlmsghdr *n,
+			   struct tcf_block *block, struct Qdisc *q,
+			   u32 parent, int event, bool unicast)
+{
+	u32 portid = oskb ? NETLINK_CB(oskb).portid : 0;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
+	if (tc_tmplt_fill_node(chain, net, skb, block, q, parent, portid,
+			       n->nlmsg_seq, n->nlmsg_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,
+			      n->nlmsg_flags & NLM_F_ECHO);
+}
+
+static int tc_ctl_tmplt_add(struct tcf_chain *chain, struct net *net,
+			    struct sk_buff *skb, struct nlmsghdr *n,
+			    struct tcf_block *block, struct Qdisc *q,
+			    u32 parent, struct nlattr **tca,
+			    struct netlink_ext_ack *extack)
+{
+	const struct tcf_proto_ops *ops;
+	void *tmplt_priv;
+
+	if (chain->tmplt_ops) {
+		NL_SET_ERR_MSG(extack, "A template is already set for the chain");
+		return -EBUSY;
+	}
+	if (chain->filter_chain) {
+		NL_SET_ERR_MSG(extack, "The chain is not empty, unable to add template");
+		return -EBUSY;
+	}
+	if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+		NL_SET_ERR_MSG(extack, "Need NLM_F_CREATE to create a new chain template");
+		return -ENOENT;
+	}
+	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 this 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;
+	tc_tmplt_notify(chain, net, skb, n, block, q, parent,
+			RTM_NEWCHAINTMPLT, false);
+	return 0;
+}
+
+static int tc_ctl_tmplt_del(struct tcf_chain *chain, struct net *net,
+			    struct sk_buff *skb, struct nlmsghdr *n,
+			    struct tcf_block *block, struct Qdisc *q,
+			    u32 parent, struct netlink_ext_ack *extack)
+{
+	const struct tcf_proto_ops *ops = chain->tmplt_ops;
+
+	if (!ops) {
+		NL_SET_ERR_MSG(extack, "Unable to delete template as this chain does not have template");
+		return -ENOENT;
+	}
+	if (chain->filter_chain) {
+		NL_SET_ERR_MSG(extack, "The chain is not empty, unable to delete template");
+		return -EBUSY;
+	}
+	if (!ops->tmplt_create) {
+		NL_SET_ERR_MSG(extack, "Chain templates are not supported with this classifier");
+		return -EOPNOTSUPP;
+	}
+	tc_tmplt_notify(chain, net, skb, n, block, q, parent,
+			RTM_DELCHAINTMPLT, false);
+	ops->tmplt_destroy(chain->tmplt_priv);
+	module_put(ops->owner);
+	chain->tmplt_ops = NULL;
+	chain->tmplt_priv = NULL;
+	return 0;
+}
+
+static int tc_ctl_tmplt_get(struct tcf_chain *chain, struct net *net,
+			    struct sk_buff *skb, struct nlmsghdr *n,
+			    struct tcf_block *block, struct Qdisc *q,
+			    u32 parent, struct netlink_ext_ack *extack)
+{
+	const struct tcf_proto_ops *ops = chain->tmplt_ops;
+	int err;
+
+	if (!ops) {
+		NL_SET_ERR_MSG(extack, "Unable to get template as this chain does not have template");
+		return -ENOENT;
+	}
+	err = tc_tmplt_notify(chain, net, skb, n, block, q, parent,
+			      RTM_NEWCHAINTMPLT, true);
+	if (err < 0)
+		NL_SET_ERR_MSG(extack, "Failed to send chain template notify message");
+	return err;
+}
+
+/* Add/delete/get a chain template */
+
+static int tc_ctl_tmplt(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_GETCHAINTMPLT &&
+	    !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;
+
+	/* Find filter chain. */
+
+	block = tcf_block_find(net, &q, &parent, &cl,
+			       t->tcm_ifindex, t->tcm_block_index, extack);
+	if (IS_ERR(block)) {
+		err = PTR_ERR(block);
+		goto errout;
+	}
+
+	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");
+		err = -EINVAL;
+		goto errout;
+	}
+	chain = tcf_chain_get(block, chain_index,
+			      n->nlmsg_type == RTM_NEWCHAINTMPLT);
+	if (!chain) {
+		NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
+		err = n->nlmsg_type == RTM_NEWTFILTER ? -ENOMEM : -EINVAL;
+		goto errout;
+	}
+
+	switch (n->nlmsg_type) {
+	case RTM_NEWCHAINTMPLT:
+		err = tc_ctl_tmplt_add(chain, net, skb, n, block,
+				       q, parent, tca, extack);
+		/* In case the chain template was successfully added,
+		 * take a reference to the chain. This ensures that
+		 * an empty chain with template does not disappear
+		 * at the end of this function.
+		 */
+		if (!err)
+			tcf_chain_hold(chain);
+		break;
+	case RTM_DELCHAINTMPLT:
+		err = tc_ctl_tmplt_del(chain, net, skb, n, block,
+				       q, parent, extack);
+		/* In case the chain template was successfully deleted,
+		 * put a reference to the chain previously taken
+		 * during template addition.
+		 */
+		if (!err)
+			tcf_chain_put(chain);
+		break;
+	case RTM_GETCHAINTMPLT:
+		err = tc_ctl_tmplt_get(chain, net, skb, n, block,
+				       q, parent, extack);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		NL_SET_ERR_MSG(extack, "Unsupported message type");
+		break;
+	}
+errout:
+	if (chain)
+		tcf_chain_put(chain);
+	if (err == -EAGAIN)
+		/* Replay the request. */
+		goto replay;
+	return err;
+}
+
+/* called with RTNL */
+static int tc_dump_tmplt(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) ||
+		    !chain->tmplt_ops)
+			continue;
+		if (index < index_start) {
+			index++;
+			continue;
+		}
+		err = tc_tmplt_fill_node(chain, net, skb, block, q, parent,
+					 NETLINK_CB(cb->skb).portid,
+					 cb->nlh->nlmsg_seq, NLM_F_MULTI,
+					 RTM_NEWCHAINTMPLT);
+		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
@@ -1795,6 +2154,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_NEWCHAINTMPLT, tc_ctl_tmplt, NULL, 0);
+	rtnl_register(PF_UNSPEC, RTM_DELCHAINTMPLT, tc_ctl_tmplt, NULL, 0);
+	rtnl_register(PF_UNSPEC, RTM_GETCHAINTMPLT, tc_ctl_tmplt,
+		      tc_dump_tmplt, 0);
 
 	return 0;
 
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 95367f37098d..a690acac7e6e 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -168,7 +168,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 static int basic_change(struct net *net, struct sk_buff *in_skb,
 			struct tcf_proto *tp, unsigned long base, u32 handle,
 			struct nlattr **tca, void **arg, bool ovr,
-			struct netlink_ext_ack *extack)
+			void *tmplt_priv, struct netlink_ext_ack *extack)
 {
 	int err;
 	struct basic_head *head = rtnl_dereference(tp->root);
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 1aa7f6511065..363c43dfb894 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -455,7 +455,8 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
 static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 			  struct tcf_proto *tp, unsigned long base,
 			  u32 handle, struct nlattr **tca,
-			  void **arg, bool ovr, struct netlink_ext_ack *extack)
+			  void **arg, bool ovr, void *tmplt_priv,
+			  struct netlink_ext_ack *extack)
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
 	struct cls_bpf_prog *oldprog = *arg;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 3bc01bdde165..ca5d0315432c 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -78,7 +78,7 @@ static void cls_cgroup_destroy_work(struct work_struct *work)
 static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 			     struct tcf_proto *tp, unsigned long base,
 			     u32 handle, struct nlattr **tca,
-			     void **arg, bool ovr,
+			     void **arg, bool ovr, void *tmplt_priv,
 			     struct netlink_ext_ack *extack)
 {
 	struct nlattr *tb[TCA_CGROUP_MAX + 1];
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 2bb043cd436b..2a21e26fcee0 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -391,7 +391,8 @@ static void flow_destroy_filter_work(struct work_struct *work)
 static int flow_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle, struct nlattr **tca,
-		       void **arg, bool ovr, struct netlink_ext_ack *extack)
+		       void **arg, bool ovr, void *tmplt_priv,
+		       struct netlink_ext_ack *extack)
 {
 	struct flow_head *head = rtnl_dereference(tp->root);
 	struct flow_filter *fold, *fnew;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9e8b26a80fb3..09d6c6e67f9d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -927,7 +927,8 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 static int fl_change(struct net *net, struct sk_buff *in_skb,
 		     struct tcf_proto *tp, unsigned long base,
 		     u32 handle, struct nlattr **tca,
-		     void **arg, bool ovr, struct netlink_ext_ack *extack)
+		     void **arg, bool ovr, void *tmplt_priv,
+		     struct netlink_ext_ack *extack)
 {
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
 	struct cls_fl_filter *fold = *arg;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 29eeeaf3ea44..a1d40d48aa24 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -250,7 +250,8 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 static int fw_change(struct net *net, struct sk_buff *in_skb,
 		     struct tcf_proto *tp, unsigned long base,
 		     u32 handle, struct nlattr **tca, void **arg,
-		     bool ovr, struct netlink_ext_ack *extack)
+		     bool ovr, void *tmplt_priv,
+		     struct netlink_ext_ack *extack)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
 	struct fw_filter *f = *arg;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 47b207ef7762..481e77cbf501 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -152,7 +152,8 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
 static int mall_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle, struct nlattr **tca,
-		       void **arg, bool ovr, struct netlink_ext_ack *extack)
+		       void **arg, bool ovr, void *tmplt_priv,
+		       struct netlink_ext_ack *extack)
 {
 	struct cls_mall_head *head = rtnl_dereference(tp->root);
 	struct nlattr *tb[TCA_MATCHALL_MAX + 1];
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0404aa5fa7cb..321eb746fe01 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -468,7 +468,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 static int route4_change(struct net *net, struct sk_buff *in_skb,
 			 struct tcf_proto *tp, unsigned long base, u32 handle,
 			 struct nlattr **tca, void **arg, bool ovr,
-			 struct netlink_ext_ack *extack)
+			 void *tmplt_priv, struct netlink_ext_ack *extack)
 {
 	struct route4_head *head = rtnl_dereference(tp->root);
 	struct route4_filter __rcu **fp;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index e9ccf7daea7d..371618720ef2 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -477,7 +477,8 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle,
 		       struct nlattr **tca,
-		       void **arg, bool ovr, struct netlink_ext_ack *extack)
+		       void **arg, bool ovr, void *tmplt_priv,
+		       struct netlink_ext_ack *extack)
 {
 	struct rsvp_head *data = rtnl_dereference(tp->root);
 	struct rsvp_filter *f, *nfp;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 32f4bbd82f35..d9fb5d56c60d 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -500,7 +500,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 static int
 tcindex_change(struct net *net, struct sk_buff *in_skb,
 	       struct tcf_proto *tp, unsigned long base, u32 handle,
-	       struct nlattr **tca, void **arg, bool ovr,
+	       struct nlattr **tca, void **arg, bool ovr, void *tmplt_priv,
 	       struct netlink_ext_ack *extack)
 {
 	struct nlattr *opt = tca[TCA_OPTIONS];
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index fb861f90fde6..b500ce62ef3c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -903,7 +903,7 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
 static int u32_change(struct net *net, struct sk_buff *in_skb,
 		      struct tcf_proto *tp, unsigned long base, u32 handle,
 		      struct nlattr **tca, void **arg, bool ovr,
-		      struct netlink_ext_ack *extack)
+		      void *tmplt_priv, struct netlink_ext_ack *extack)
 {
 	struct tc_u_common *tp_c = tp->data;
 	struct tc_u_hnode *ht;
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 7b7433a1a34c..825777efc83e 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_NEWCHAINTMPLT + 3));
 		err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
 				 sizeof(nlmsg_route_perms));
 		break;
-- 
2.14.4

[patch net-next v2 3/9] net: sched: cls_flower: move key/mask dumping into a separate function

[Jiri Pirko]

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 09d6c6e67f9d..76c5516357d5 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1217,29 +1217,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;
 
@@ -1248,9 +1228,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)) ||
@@ -1404,6 +1381,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

[patch net-next v2 4/9] net: sched: cls_flower: change fl_init_dissector to accept mask and dissector

[Jiri Pirko]

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>
---
 net/sched/cls_flower.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 76c5516357d5..9ce4375b3252 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -793,47 +793,48 @@ 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_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);
 
-	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,
@@ -852,7 +853,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

[patch net-next v2 5/9] net: sched: cls_flower: implement chain templates

[Jiri Pirko]

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>
---
 net/sched/cls_flower.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9ce4375b3252..d64d43843a3a 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -70,6 +70,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;
@@ -144,6 +151,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)
 {
@@ -902,6 +926,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;
@@ -922,6 +947,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;
 }
 
@@ -932,6 +962,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		     struct netlink_ext_ack *extack)
 {
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
+	struct fl_flow_tmplt *tmplt = tmplt_priv;
 	struct cls_fl_filter *fold = *arg;
 	struct cls_fl_filter *fnew;
 	struct nlattr **tb;
@@ -988,7 +1019,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);
+			   tmplt, extack);
 	if (err)
 		goto errout_idr;
 
@@ -1089,6 +1120,52 @@ static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
+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)
@@ -1435,6 +1512,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;
@@ -1454,6 +1556,9 @@ static struct tcf_proto_ops cls_fl_ops __read_mostly = {
 	.walk		= fl_walk,
 	.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

[patch net-next v2 6/9] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers

[Jiri Pirko]

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>
---
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 a3c1a2c47cd4..e83968cf9a70 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -715,6 +715,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 d64d43843a3a..614dd558d5f1 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1120,6 +1120,42 @@ static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
+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)
@@ -1150,6 +1186,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:
@@ -1163,6 +1201,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

[patch net-next v2 7/9] mlxsw: spectrum: Implement chain template hinting

[Jiri Pirko]

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     | 12 +++++-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 12 ++++--
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c    | 25 ++++++++++--
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  | 44 ++++++++++++++++++++--
 5 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 968b88af2ef5..da19fa343d0b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1441,6 +1441,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 4a519d8edec8..b0a8e611e730 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -459,7 +459,8 @@ enum mlxsw_sp_acl_profile {
 struct mlxsw_sp_acl_profile_ops {
 	size_t ruleset_priv_size;
 	int (*ruleset_add)(struct mlxsw_sp *mlxsw_sp,
-			   void *priv, void *ruleset_priv);
+			   void *priv, 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,
@@ -514,7 +515,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);
@@ -594,6 +596,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 79b1fa27a9a4..ea42605c451d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -319,7 +319,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;
@@ -339,7 +340,8 @@ mlxsw_sp_acl_ruleset_create(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		goto err_rhashtable_init;
 
-	err = ops->ruleset_add(mlxsw_sp, acl->priv, ruleset->priv);
+	err = ops->ruleset_add(mlxsw_sp, acl->priv, ruleset->priv,
+			       tmplt_elusage);
 	if (err)
 		goto err_ops_ruleset_add;
 
@@ -421,7 +423,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;
@@ -436,7 +439,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 ad1b548e3cac..d6e4e00dfd3d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
@@ -157,6 +157,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_region {
@@ -216,13 +218,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)
@@ -431,6 +439,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,
@@ -1019,14 +1036,16 @@ struct mlxsw_sp_acl_tcam_flower_rule {
 
 static int
 mlxsw_sp_acl_tcam_flower_ruleset_add(struct mlxsw_sp *mlxsw_sp,
-				     void *priv, void *ruleset_priv)
+				     void *priv, void *ruleset_priv,
+				     struct mlxsw_afk_element_usage *tmplt_elusage)
 {
 	struct mlxsw_sp_acl_tcam_flower_ruleset *ruleset = ruleset_priv;
 	struct mlxsw_sp_acl_tcam *tcam = 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_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 89dbf569dff5..f34b410e5048 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -401,7 +401,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);
 
@@ -445,7 +445,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;
 
@@ -471,7 +471,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;
 
@@ -493,3 +493,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

[patch net-next v2 8/9] selftests: forwarding: move shblock tc support check to a separate helper

[Jiri Pirko]

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 7b18a53aa556..a736d1d7ecdb 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -28,7 +28,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

[patch net-next v2 9/9] selftests: forwarding: add tests for TC chain templates

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Add basic sanity tests for TC chain templates.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 tools/testing/selftests/net/forwarding/lib.sh      |   9 ++
 .../selftests/net/forwarding/tc_chaintemplates.sh  | 160 +++++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_chaintemplates.sh

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index a736d1d7ecdb..128a5b5a8ea9 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -39,6 +39,15 @@ check_tc_shblock_support()
 	fi
 }
 
+check_tc_chaintemplate_support()
+{
+	tc filter help 2>&1|grep template &> /dev/null
+	if [[ $? -ne 0 ]]; then
+		echo "SKIP: iproute2 too old; tc is missing chain template 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_chaintemplates.sh b/tools/testing/selftests/net/forwarding/tc_chaintemplates.sh
new file mode 100755
index 000000000000..21f2c18e973a
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/tc_chaintemplates.sh
@@ -0,0 +1,160 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="template_create_destroy template_filter_fits \
+	   template_create_nonempty template_destroy_nonempty"
+NUM_NETIFS=2
+source tc_common.sh
+source lib.sh
+
+h1_create()
+{
+	simple_if_init $h1 192.0.2.1/24
+}
+
+h1_destroy()
+{
+	simple_if_fini $h1 192.0.2.1/24
+}
+
+h2_create()
+{
+	simple_if_init $h2 192.0.2.2/24
+	tc qdisc add dev $h2 clsact
+}
+
+h2_destroy()
+{
+	tc qdisc del dev $h2 clsact
+	simple_if_fini $h2 192.0.2.2/24
+}
+
+template_create_destroy()
+{
+	RET=0
+
+	tc filter template add dev $h2 ingress protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
+	check_err $? "Failed to create template for default chain"
+
+	tc filter template add dev $h2 ingress chain 1 protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
+	check_err $? "Failed to create template for chain 1"
+
+	tc filter template del dev $h2 ingress
+	check_err $? "Failed to destroy template for default chain"
+
+	tc filter template del dev $h2 ingress chain 1
+	check_err $? "Failed to destroy template for chain 1"
+
+	log_test "template create destroy"
+}
+
+template_filter_fits()
+{
+	RET=0
+
+	tc filter template add dev $h2 ingress protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+	tc filter template 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 1protocol 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 filter template del dev $h2 ingress chain 1
+	tc filter template del dev $h2 ingress
+
+	log_test "template filter fits"
+}
+
+template_create_nonempty()
+{
+	RET=0
+
+	tc filter add dev $h2 ingress protocol ip pref 1 handle 1101 \
+		flower dst_mac $h2mac action drop
+	tc filter template add dev $h2 ingress protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+	check_fail $? "Incorrectly succeded to create template for non-empty chain"
+
+	tc filter template del dev $h2 ingress &> /dev/null
+	tc filter del dev $h2 ingress protocol ip pref 1 handle 1101 flower
+
+	log_test "template create non-empty"
+}
+
+template_destroy_nonempty()
+{
+	RET=0
+
+	tc filter template add dev $h2 ingress protocol ip \
+		flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
+	tc filter add dev $h2 ingress protocol ip pref 1 handle 1101 \
+		flower dst_mac $h2mac action drop
+
+	tc filter template del dev $h2 ingress &> /dev/null
+	check_fail $? "Incorrectly succeded to destroy template for non-empty chain"
+	tc filter del dev $h2 ingress protocol ip pref 1 handle 1101 flower
+	tc filter template del dev $h2 ingress &> /dev/null
+	check_err $? "Failed to destroy template for empty chain"
+
+	log_test "template destroy non-empty"
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	h2=${NETIFS[p2]}
+	h1mac=$(mac_get $h1)
+	h2mac=$(mac_get $h2)
+
+	vrf_prepare
+
+	h1_create
+	h2_create
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	h2_destroy
+	h1_destroy
+
+	vrf_cleanup
+}
+
+check_tc_chaintemplate_support
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
-- 
2.14.4

Re: [patch net-next v2 7/9] mlxsw: spectrum: Implement chain template hinting

[Ido Schimmel]

On Tue, Jun 26, 2018 at 09:59:58AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Since cld_flower provides information about the filter template for

s/cld_flower/cls_flower/

> 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>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Cong Wang]

On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko <jiri@resnulli.us> wrote:
> Create dummy device with clsact first:
> # ip link add type dummy
> # tc qdisc add dev dummy0 clsact
>
> There is no template assigned by default:
> # tc filter template show dev dummy0 ingress
>
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

Now you are extending 'tc filter' command with a new
subcommand 'template', which looks weird.

Why not make it a new property of filter like you did for chain?
Like:

tc filter add dev dummy0 ingress proto ip template flower

which is much better IMHO.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> Create dummy device with clsact first:
>> # ip link add type dummy
>> # tc qdisc add dev dummy0 clsact
>>
>> There is no template assigned by default:
>> # tc filter template show dev dummy0 ingress
>>
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
>Now you are extending 'tc filter' command with a new
>subcommand 'template', which looks weird.
>
>Why not make it a new property of filter like you did for chain?
>Like:
>
>tc filter add dev dummy0 ingress proto ip template flower

But unlike chain, this is not a filter property. For chain, when you add
filter, you add it to a specific chain. That makes sense.
But for template, you need to add the template first. Then, later on,
you add filters which either match or does not match the template.
Does not make sense to have "template" the filter property as you
suggest.

>
>which is much better IMHO.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Samudrala, Sridhar]

On 6/26/2018 11:05 PM, Jiri Pirko wrote:
> Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangcong@gmail.com wrote:
>> On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>> Create dummy device with clsact first:
>>> # ip link add type dummy
>>> # tc qdisc add dev dummy0 clsact
>>>
>>> There is no template assigned by default:
>>> # tc filter template show dev dummy0 ingress
>>>
>>> Add a template of type flower allowing to insert rules matching on last
>>> 2 bytes of destination mac address:
>>> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>> Now you are extending 'tc filter' command with a new
>> subcommand 'template', which looks weird.
>>
>> Why not make it a new property of filter like you did for chain?
>> Like:
>>
>> tc filter add dev dummy0 ingress proto ip template flower
> But unlike chain, this is not a filter property. For chain, when you add
> filter, you add it to a specific chain. That makes sense.
> But for template, you need to add the template first. Then, later on,
> you add filters which either match or does not match the template.

So can we say that template defines the types of rules(match fields/masks) that
can be added to a specific chain and there is 1-1 relationship between a template
and a chain?

Without attaching a template to a chain, i guess it is possible to add different
types of rules to a chain?


> Does not make sense to have "template" the filter property as you
> suggest.

template seems to a chain property.

>> which is much better IMHO.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Wed, Jun 27, 2018 at 08:34:46AM CEST, sridhar.samudrala@intel.com wrote:
>
>On 6/26/2018 11:05 PM, Jiri Pirko wrote:
>> Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangcong@gmail.com wrote:
>> > On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> > > Create dummy device with clsact first:
>> > > # ip link add type dummy
>> > > # tc qdisc add dev dummy0 clsact
>> > > 
>> > > There is no template assigned by default:
>> > > # tc filter template show dev dummy0 ingress
>> > > 
>> > > Add a template of type flower allowing to insert rules matching on last
>> > > 2 bytes of destination mac address:
>> > > # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>> > Now you are extending 'tc filter' command with a new
>> > subcommand 'template', which looks weird.
>> > 
>> > Why not make it a new property of filter like you did for chain?
>> > Like:
>> > 
>> > tc filter add dev dummy0 ingress proto ip template flower
>> But unlike chain, this is not a filter property. For chain, when you add
>> filter, you add it to a specific chain. That makes sense.
>> But for template, you need to add the template first. Then, later on,
>> you add filters which either match or does not match the template.
>
>So can we say that template defines the types of rules(match fields/masks) that
>can be added to a specific chain and there is 1-1 relationship between a template
>and a chain?

yes

>
>Without attaching a template to a chain, i guess it is possible to add different
>types of rules to a chain?

yes

>
>
>> Does not make sense to have "template" the filter property as you
>> suggest.
>
>template seems to a chain property.

yes

>
>> > which is much better IMHO.
>

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 26 Jun 2018 09:59:51 +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.

This series doesn't apply cleanly to net-next, and also there seems to still
be some discussion about how the iproute2 command line should look.

Thanks.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 06:48:26AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 26 Jun 2018 09:59:51 +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.
>
>This series doesn't apply cleanly to net-next, and also there seems to still
>be some discussion about how the iproute2 command line should look.

Will re-spin. Thanks.

>
>Thanks.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jamal Hadi Salim]

On 26/06/18 03:59 AM, 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.
> 

Is this just restricted to hardware offload? Example it will make sense
for u32 in s/ware as well (i.e flexible TCAM like TCAM based
classification). i.e it is possible that rules the user enters
end up being worst case a linked list lookup, yes? And allocating
space for a tuple that is not in use is a waste of space.

If yes, then I would reword the above as something like:

For very flexible classifiers such as TCAM based ones,
one could add arbitrary tuple rules which tend to be inefficient both
from a space and lookup performance. One approach, taken by Mlxsw,
is to assume a multi filter tuple arrangement which is inefficient
from a space perspective when the user-specified rules dont make
use of pre-provisioned tuple space.
Typically users already know what tuples are of interest to them:
for example for ipv4 route lookup purposes they may just want to
lookup destination IP with a specified mask etc.
This feature allows user to provide good hints to the classifier to
optimize.


> 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 template assigned by default:
> # tc filter template show dev dummy0 ingress
> 
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
> 
> The template is now showed in the list:
> # tc filter template show dev dummy0 ingress
> filter flower chain 0
>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>    eth_type ipv4
> 
> Add another template, this time for chain number 22:
> # tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
> # tc filter template show dev dummy0 ingress
> filter flower chain 0
>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>    eth_type ipv4
> filter 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: 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: 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: 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: Mask does not fit the template.
> We have an error talking to the kernel, -1
> 
> Removal of a template from non-empty chain would fail:
> # tc filter template del dev dummy0 ingress
> Error: The chain is not empty, unable to delete template.
> We have an error talking to the kernel, -1
> 
> Once the chain is flushed, the template could be removed:
> # tc filter del dev dummy0 ingress
> # tc filter template del dev dummy0 ingress
> 

BTW: unlike the other comments on this - I think the syntax above
is fine ;-> Chain are already either explicitly or are implicitly
(case of chain 0) specified.

Assuming that one cant add a new template to a chain if it already
has at least one filter (even if no template has been added).

I like it - it may help making u32 more friendly to humans in some
cases.

cheers,
jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>
>On 26/06/18 03:59 AM, 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.
>> 
>
>Is this just restricted to hardware offload? Example it will make sense
>for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>classification). i.e it is possible that rules the user enters
>end up being worst case a linked list lookup, yes? And allocating
>space for a tuple that is not in use is a waste of space.

I'm afraid I don't understand clearly what you say. This is not
restricted to hw offload. The templates apply to all filters, no matter
if they are offloaded or not.


>
>If yes, then I would reword the above as something like:
>
>For very flexible classifiers such as TCAM based ones,
>one could add arbitrary tuple rules which tend to be inefficient both
>from a space and lookup performance. One approach, taken by Mlxsw,
>is to assume a multi filter tuple arrangement which is inefficient
>from a space perspective when the user-specified rules dont make
>use of pre-provisioned tuple space.
>Typically users already know what tuples are of interest to them:
>for example for ipv4 route lookup purposes they may just want to
>lookup destination IP with a specified mask etc.
>This feature allows user to provide good hints to the classifier to
>optimize.
>
>
>> 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 template assigned by default:
>> # tc filter template show dev dummy0 ingress
>> 
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>> 
>> The template is now showed in the list:
>> # tc filter template show dev dummy0 ingress
>> filter flower chain 0
>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>    eth_type ipv4
>> 
>> Add another template, this time for chain number 22:
>> # tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> # tc filter template show dev dummy0 ingress
>> filter flower chain 0
>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>    eth_type ipv4
>> filter 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: 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: 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: 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: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> 
>> Removal of a template from non-empty chain would fail:
>> # tc filter template del dev dummy0 ingress
>> Error: The chain is not empty, unable to delete template.
>> We have an error talking to the kernel, -1
>> 
>> Once the chain is flushed, the template could be removed:
>> # tc filter del dev dummy0 ingress
>> # tc filter template del dev dummy0 ingress
>> 
>
>BTW: unlike the other comments on this - I think the syntax above
>is fine ;-> Chain are already either explicitly or are implicitly
>(case of chain 0) specified.
>
>Assuming that one cant add a new template to a chain if it already
>has at least one filter (even if no template has been added).
>
>I like it - it may help making u32 more friendly to humans in some
>cases.
>
>cheers,
>jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jamal Hadi Salim]

On 28/06/18 09:22 AM, Jiri Pirko wrote:
> Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>>
>> On 26/06/18 03:59 AM, 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.
>>>
>>
>> Is this just restricted to hardware offload? Example it will make sense
>> for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>> classification). i.e it is possible that rules the user enters
>> end up being worst case a linked list lookup, yes? And allocating
>> space for a tuple that is not in use is a waste of space.
> 
> I'm afraid I don't understand clearly what you say.

Well - I was trying to understand what you said ;->

I think what you are getting at is two issues:
a) space in the tcams - if the user is just going to enter
rules which use one tuple (dst ip for example) the hardware
would be better off told that this is the case so it doesnt
allocate space in anticipation that someone is going to
specify src ip later on.
b) lookup speed in tcams - without the template hint a
selection of rules may end up looking like a linked list
which is not optimal for lookup

> This is not
> restricted to hw offload. The templates apply to all filters, no matter
> if they are offloaded or not.
> 

Do you save anything with flower(in s/w) if you only added a template
with say dst ip/mask? I can see it will make sense for u32 which is more
flexible and protocol independent.

cheers,
jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 03:54:17PM CEST, jhs@mojatatu.com wrote:
>On 28/06/18 09:22 AM, Jiri Pirko wrote:
>> Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>> > 
>> > On 26/06/18 03:59 AM, 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.
>> > > 
>> > 
>> > Is this just restricted to hardware offload? Example it will make sense
>> > for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>> > classification). i.e it is possible that rules the user enters
>> > end up being worst case a linked list lookup, yes? And allocating
>> > space for a tuple that is not in use is a waste of space.
>> 
>> I'm afraid I don't understand clearly what you say.
>
>Well - I was trying to understand what you said ;->
>
>I think what you are getting at is two issues:
>a) space in the tcams - if the user is just going to enter
>rules which use one tuple (dst ip for example) the hardware
>would be better off told that this is the case so it doesnt
>allocate space in anticipation that someone is going to
>specify src ip later on.

Yes.

>b) lookup speed in tcams - without the template hint a
>selection of rules may end up looking like a linked list
>which is not optimal for lookup

Well. Not really, but wider keys have bigger overheads in general. So
the motivation is to have the keys as small as possible for both
performance and capacity reasons.

>
>> This is not
>> restricted to hw offload. The templates apply to all filters, no matter
>> if they are offloaded or not.
>> 
>
>Do you save anything with flower(in s/w) if you only added a template
>with say dst ip/mask? I can see it will make sense for u32 which is more
>flexible and protocol independent.

No benefit for flower s/w path at this point. Perhaps the hashtables
could be organized in more optimal way with the hint. I didn't look at
it.

>
>cheers,
>jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Cong Wang]

On Wed, Jun 27, 2018 at 9:48 PM David Miller <davem@davemloft.net> wrote:
>
> This series doesn't apply cleanly to net-next, and also there seems to still
> be some discussion about how the iproute2 command line should look.
>

I am sure you know this, so just to be clear:

A redesign of "how iproute2 command line should look" usually means a
redesign in the kernel code too. Apparently, 'tc chaintemplate' is a new
subsystem under TC, while a 'tc filter template' is merely a new TC filter
attribute.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Cong Wang]

On Sat, Jun 30, 2018 at 3:13 AM Jiri Pirko <jiri@resnulli.us> wrote:
> Okay. So that would allow either create a chain or "chain with
> template". Once that is done, there would be no means to manipulate the
> template. One can only remove the chain.
>
> What about refounting? I think it would make sense that this implicit
> chain addition would take one reference. That means if later on the last
> filter is removed, the chain would stay there until user removes it by
> hand.

Yeah, it is very similar to tc actions. So you can take a look
at how tc actions are refcnt'ed.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Sat, Jun 30, 2018 at 12:18:02AM CEST, xiyou.wangcong@gmail.com wrote:
>On Fri, Jun 29, 2018 at 10:06 AM Samudrala, Sridhar
><sridhar.samudrala@intel.com> wrote:
>>
>> So instead of introducing 'chaintemplate' object in the kernel, can't we add 'chain'
>> object in the kernel that takes the 'template' as an attribute?
>
>This is exactly what I mean above. Making the chain a standalone object
>in kernel would benefit:
>
>1. Align with 'tc chain' in iproute2, add/del an object is natural
>
>2. Template is an attribute of this object when creating it:
># tc chain add template ....
># tc chain add ... # non-template chain

Okay. So that would allow either create a chain or "chain with
template". Once that is done, there would be no means to manipulate the
template. One can only remove the chain.

What about refounting? I think it would make sense that this implicit
chain addition would take one reference. That means if later on the last
filter is removed, the chain would stay there until user removes it by
hand.

Okay. Sounds good to me. Will do.

Thanks!


>
>3. Easier for sharing by qdiscs:
># tc chain add X block Y ...
># tc filter add ... chain X block Y ...
># tc qdisc add dev eth0 block Y ...
>
>The current 'ingress_block 22 ingress' syntax is ugly.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Cong Wang]

On Fri, Jun 29, 2018 at 10:06 AM Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
> So instead of introducing 'chaintemplate' object in the kernel, can't we add 'chain'
> object in the kernel that takes the 'template' as an attribute?

This is exactly what I mean above. Making the chain a standalone object
in kernel would benefit:

1. Align with 'tc chain' in iproute2, add/del an object is natural

2. Template is an attribute of this object when creating it:
# tc chain add template ....
# tc chain add ... # non-template chain

3. Easier for sharing by qdiscs:
# tc chain add X block Y ...
# tc filter add ... chain X block Y ...
# tc qdisc add dev eth0 block Y ...

The current 'ingress_block 22 ingress' syntax is ugly.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Samudrala, Sridhar]

On 6/29/2018 6:05 AM, Jiri Pirko wrote:
> Fri, Jun 29, 2018 at 02:54:36PM CEST, dsahern@gmail.com wrote:
>> On 6/29/18 6:48 AM, Jiri Pirko wrote:
>>> Fri, Jun 29, 2018 at 02:12:21PM CEST, jhs@mojatatu.com wrote:
>>>> On 29/06/18 04:39 AM, Jiri Pirko wrote:
>>>>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>>>>>> On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>>>> Add a template of type flower allowing to insert rules matching on last
>>>>>>> 2 bytes of destination mac address:
>>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>>>>>
>>>>>>> The template is now showed in the list:
>>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>>> chaintemplate flower chain 0
>>>>>>>     dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>>>     eth_type ipv4
>>>>>>>
>>>>>>> Add another template, this time for chain number 22:
>>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>>> chaintemplate flower chain 0
>>>>>>>     dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>>>     eth_type ipv4
>>>>>>> chaintemplate flower chain 22
>>>>>>>     eth_type ipv4
>>>>>>>     dst_ip 0.0.0.0/16
>>>>>> So, if I want to check the template of a chain, I have to use
>>>>>> 'tc chaintemplate... chain X'.
>>>>>>
>>>>>> If I want to check the filters in a chain, I have to use
>>>>>> 'tc filter show .... chain X'.
>>>>>>
>>>>>> If you introduce 'tc chain', it would just need one command:
>>>>>> `tc chain show ... X` which could list its template first and
>>>>>> followed by filters in this chain, something like:
>>>>>>
>>>>>> # tc chain show dev eth0 chain X
>>>>>> template: # could be none
>>>>>> ....
>>>>>> filter1
>>>>>> ...
>>>>>> filter2
>>>>>> ...
>>>>>>
>>>>>> Isn't it more elegant?
>>>>> Well, that is just another iproute2 command. It would use the same
>>>>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
>>>>> Let's do it in a follow-up iproute2 patch.
>>>>>
>>>> Half a dozen or 6 - take your pick, really.
>>>> I would call the template an attribute as opposed to a stand alone
>>>> object i.e A chain of filters may have a template. If you have to
>>>> introduce a new object then Sridhar's suggested syntax seems appealing.
>>> I think what I have makes sense. Maps nicely to what it really is inside
>>> kernel. What Cong proposes looks like nice extension of userspace app to
>>> do more things in one go. Will address that in followup iproute2 patch.
>> The resolution of the syntax affect the uapi changes proposed. You are
>> wanting to add new RTM commands which suggests new objects. If a
>> template is an attribute of an existing object then the netlink API
>> should indicate that.
> There is no existing "chain" object. So no, no uapi changes.

So instead of introducing 'chaintemplate' object in the kernel, can't we add 'chain'
object in the kernel that takes the 'template' as an attribute?

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Miller]

From: David Ahern <dsahern@gmail.com>
Date: Fri, 29 Jun 2018 06:54:36 -0600

> The resolution of the syntax affect the uapi changes proposed. You are
> wanting to add new RTM commands which suggests new objects. If a
> template is an attribute of an existing object then the netlink API
> should indicate that.

+1

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Fri, Jun 29, 2018 at 02:54:36PM CEST, dsahern@gmail.com wrote:
>On 6/29/18 6:48 AM, Jiri Pirko wrote:
>> Fri, Jun 29, 2018 at 02:12:21PM CEST, jhs@mojatatu.com wrote:
>>> On 29/06/18 04:39 AM, Jiri Pirko wrote:
>>>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>>>>> On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>>> Add a template of type flower allowing to insert rules matching on last
>>>>>> 2 bytes of destination mac address:
>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>>>>
>>>>>> The template is now showed in the list:
>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>> chaintemplate flower chain 0
>>>>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>>    eth_type ipv4
>>>>>>
>>>>>> Add another template, this time for chain number 22:
>>>>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>>> chaintemplate flower chain 0
>>>>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>>    eth_type ipv4
>>>>>> chaintemplate flower chain 22
>>>>>>    eth_type ipv4
>>>>>>    dst_ip 0.0.0.0/16
>>>>>
>>>>> So, if I want to check the template of a chain, I have to use
>>>>> 'tc chaintemplate... chain X'.
>>>>>
>>>>> If I want to check the filters in a chain, I have to use
>>>>> 'tc filter show .... chain X'.
>>>>>
>>>>> If you introduce 'tc chain', it would just need one command:
>>>>> `tc chain show ... X` which could list its template first and
>>>>> followed by filters in this chain, something like:
>>>>>
>>>>> # tc chain show dev eth0 chain X
>>>>> template: # could be none
>>>>> ....
>>>>> filter1
>>>>> ...
>>>>> filter2
>>>>> ...
>>>>>
>>>>> Isn't it more elegant?
>>>>
>>>> Well, that is just another iproute2 command. It would use the same
>>>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
>>>> Let's do it in a follow-up iproute2 patch.
>>>>
>>>
>>> Half a dozen or 6 - take your pick, really.
>>> I would call the template an attribute as opposed to a stand alone
>>> object i.e A chain of filters may have a template. If you have to
>>> introduce a new object then Sridhar's suggested syntax seems appealing.
>> 
>> I think what I have makes sense. Maps nicely to what it really is inside
>> kernel. What Cong proposes looks like nice extension of userspace app to
>> do more things in one go. Will address that in followup iproute2 patch.
>
>The resolution of the syntax affect the uapi changes proposed. You are
>wanting to add new RTM commands which suggests new objects. If a
>template is an attribute of an existing object then the netlink API
>should indicate that.

There is no existing "chain" object. So no, no uapi changes.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Ahern]

On 6/29/18 6:48 AM, Jiri Pirko wrote:
> Fri, Jun 29, 2018 at 02:12:21PM CEST, jhs@mojatatu.com wrote:
>> On 29/06/18 04:39 AM, Jiri Pirko wrote:
>>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>>>> On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Add a template of type flower allowing to insert rules matching on last
>>>>> 2 bytes of destination mac address:
>>>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>>>
>>>>> The template is now showed in the list:
>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>> chaintemplate flower chain 0
>>>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>    eth_type ipv4
>>>>>
>>>>> Add another template, this time for chain number 22:
>>>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>>>> # tc chaintemplate show dev dummy0 ingress
>>>>> chaintemplate flower chain 0
>>>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>>    eth_type ipv4
>>>>> chaintemplate flower chain 22
>>>>>    eth_type ipv4
>>>>>    dst_ip 0.0.0.0/16
>>>>
>>>> So, if I want to check the template of a chain, I have to use
>>>> 'tc chaintemplate... chain X'.
>>>>
>>>> If I want to check the filters in a chain, I have to use
>>>> 'tc filter show .... chain X'.
>>>>
>>>> If you introduce 'tc chain', it would just need one command:
>>>> `tc chain show ... X` which could list its template first and
>>>> followed by filters in this chain, something like:
>>>>
>>>> # tc chain show dev eth0 chain X
>>>> template: # could be none
>>>> ....
>>>> filter1
>>>> ...
>>>> filter2
>>>> ...
>>>>
>>>> Isn't it more elegant?
>>>
>>> Well, that is just another iproute2 command. It would use the same
>>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
>>> Let's do it in a follow-up iproute2 patch.
>>>
>>
>> Half a dozen or 6 - take your pick, really.
>> I would call the template an attribute as opposed to a stand alone
>> object i.e A chain of filters may have a template. If you have to
>> introduce a new object then Sridhar's suggested syntax seems appealing.
> 
> I think what I have makes sense. Maps nicely to what it really is inside
> kernel. What Cong proposes looks like nice extension of userspace app to
> do more things in one go. Will address that in followup iproute2 patch.

The resolution of the syntax affect the uapi changes proposed. You are
wanting to add new RTM commands which suggests new objects. If a
template is an attribute of an existing object then the netlink API
should indicate that.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Fri, Jun 29, 2018 at 02:12:21PM CEST, jhs@mojatatu.com wrote:
>On 29/06/18 04:39 AM, Jiri Pirko wrote:
>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>> > On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> > > Add a template of type flower allowing to insert rules matching on last
>> > > 2 bytes of destination mac address:
>> > > # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>> > > 
>> > > The template is now showed in the list:
>> > > # tc chaintemplate show dev dummy0 ingress
>> > > chaintemplate flower chain 0
>> > >    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> > >    eth_type ipv4
>> > > 
>> > > Add another template, this time for chain number 22:
>> > > # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> > > # tc chaintemplate show dev dummy0 ingress
>> > > chaintemplate flower chain 0
>> > >    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> > >    eth_type ipv4
>> > > chaintemplate flower chain 22
>> > >    eth_type ipv4
>> > >    dst_ip 0.0.0.0/16
>> > 
>> > So, if I want to check the template of a chain, I have to use
>> > 'tc chaintemplate... chain X'.
>> > 
>> > If I want to check the filters in a chain, I have to use
>> > 'tc filter show .... chain X'.
>> > 
>> > If you introduce 'tc chain', it would just need one command:
>> > `tc chain show ... X` which could list its template first and
>> > followed by filters in this chain, something like:
>> > 
>> > # tc chain show dev eth0 chain X
>> > template: # could be none
>> > ....
>> > filter1
>> > ...
>> > filter2
>> > ...
>> > 
>> > Isn't it more elegant?
>> 
>> Well, that is just another iproute2 command. It would use the same
>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
>> Let's do it in a follow-up iproute2 patch.
>> 
>
>Half a dozen or 6 - take your pick, really.
>I would call the template an attribute as opposed to a stand alone
>object i.e A chain of filters may have a template. If you have to
>introduce a new object then Sridhar's suggested syntax seems appealing.

I think what I have makes sense. Maps nicely to what it really is inside
kernel. What Cong proposes looks like nice extension of userspace app to
do more things in one go. Will address that in followup iproute2 patch.

>
>cheers,
>jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jamal Hadi Salim]

On 29/06/18 04:39 AM, Jiri Pirko wrote:
> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>> On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>> Add a template of type flower allowing to insert rules matching on last
>>> 2 bytes of destination mac address:
>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>
>>> The template is now showed in the list:
>>> # tc chaintemplate show dev dummy0 ingress
>>> chaintemplate flower chain 0
>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>    eth_type ipv4
>>>
>>> Add another template, this time for chain number 22:
>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>> # tc chaintemplate show dev dummy0 ingress
>>> chaintemplate flower chain 0
>>>    dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>    eth_type ipv4
>>> chaintemplate flower chain 22
>>>    eth_type ipv4
>>>    dst_ip 0.0.0.0/16
>>
>> So, if I want to check the template of a chain, I have to use
>> 'tc chaintemplate... chain X'.
>>
>> If I want to check the filters in a chain, I have to use
>> 'tc filter show .... chain X'.
>>
>> If you introduce 'tc chain', it would just need one command:
>> `tc chain show ... X` which could list its template first and
>> followed by filters in this chain, something like:
>>
>> # tc chain show dev eth0 chain X
>> template: # could be none
>> ....
>> filter1
>> ...
>> filter2
>> ...
>>
>> Isn't it more elegant?
> 
> Well, that is just another iproute2 command. It would use the same
> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
> Let's do it in a follow-up iproute2 patch.
> 

Half a dozen or 6 - take your pick, really.
I would call the template an attribute as opposed to a stand alone
object i.e A chain of filters may have a template. If you have to
introduce a new object then Sridhar's suggested syntax seems appealing.

cheers,
jamal

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangcong@gmail.com wrote:
>On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>
>> The template is now showed in the list:
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0
>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>   eth_type ipv4
>>
>> Add another template, this time for chain number 22:
>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0
>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>   eth_type ipv4
>> chaintemplate flower chain 22
>>   eth_type ipv4
>>   dst_ip 0.0.0.0/16
>
>So, if I want to check the template of a chain, I have to use
>'tc chaintemplate... chain X'.
>
>If I want to check the filters in a chain, I have to use
>'tc filter show .... chain X'.
>
>If you introduce 'tc chain', it would just need one command:
>`tc chain show ... X` which could list its template first and
>followed by filters in this chain, something like:
>
># tc chain show dev eth0 chain X
>template: # could be none
>....
>filter1
>...
>filter2
>...
>
>Isn't it more elegant?

Well, that is just another iproute2 command. It would use the same
kernel uapi. Filters+templates. Sure, why not. Can be easily introduced.
Let's do it in a follow-up iproute2 patch.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Cong Wang]

On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
> The template is now showed in the list:
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0
>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>   eth_type ipv4
>
> Add another template, this time for chain number 22:
> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0
>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>   eth_type ipv4
> chaintemplate flower chain 22
>   eth_type ipv4
>   dst_ip 0.0.0.0/16

So, if I want to check the template of a chain, I have to use
'tc chaintemplate... chain X'.

If I want to check the filters in a chain, I have to use
'tc filter show .... chain X'.

If you introduce 'tc chain', it would just need one command:
`tc chain show ... X` which could list its template first and
followed by filters in this chain, something like:

# tc chain show dev eth0 chain X
template: # could be none
....
filter1
...
filter2
...

Isn't it more elegant?

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 05:50:08PM CEST, dsahern@gmail.com wrote:
>On 6/28/18 9:37 AM, Jiri Pirko wrote:
>>>>>
>>>>> Why this restriction? It's a template, so why can't it be removed
>>>>> regardless of whether there are filters?
>>>>
>>>> That means you could start to insert filters that does not match the
>>>> original template. I wanted to avoid it. The chain is utilized in hw for
>>>> the original template, the filter insertion would have to be sanitized
>>>> in driver. With this restriction, drivers can depend on filters always
>>>> be fitting.
>>>>
>>>
>>> Then the hardware driver should have that restriction not the core tc code.
>> 
>> But why? The same restriction would be in all drivers. I believe it is
>> better to have in in tc in single place. Drivers can then depend on it.
>> Do you have a usecase where you need to remove template for non-empty
>> chain?
>> 
>
>If the hardware has the limitation then the driver should be rejecting a
>change.

The behaviour I defend is symmetrical with "template add". There is also
possible to add the template only if the chain is empty.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Ahern]

On 6/28/18 9:37 AM, Jiri Pirko wrote:
>>>>
>>>> Why this restriction? It's a template, so why can't it be removed
>>>> regardless of whether there are filters?
>>>
>>> That means you could start to insert filters that does not match the
>>> original template. I wanted to avoid it. The chain is utilized in hw for
>>> the original template, the filter insertion would have to be sanitized
>>> in driver. With this restriction, drivers can depend on filters always
>>> be fitting.
>>>
>>
>> Then the hardware driver should have that restriction not the core tc code.
> 
> But why? The same restriction would be in all drivers. I believe it is
> better to have in in tc in single place. Drivers can then depend on it.
> Do you have a usecase where you need to remove template for non-empty
> chain?
> 

If the hardware has the limitation then the driver should be rejecting a
change.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 05:10:45PM CEST, dsahern@gmail.com wrote:
>On 6/28/18 8:29 AM, Jiri Pirko wrote:
>> Thu, Jun 28, 2018 at 04:18:47PM CEST, dsahern@gmail.com wrote:
>>> On 6/28/18 7:08 AM, Jiri Pirko wrote:
>>>> Create dummy device with clsact first:
>>>> # ip link add type dummy
>>>> # tc qdisc add dev dummy0 clsact
>>>>
>>>> There is no template assigned by default:
>>>> # tc chaintemplate show dev dummy0 ingress
>>>>
>>>> Add a template of type flower allowing to insert rules matching on last
>>>> 2 bytes of destination mac address:
>>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>>
>>>> The template is now showed in the list:
>>>> # tc chaintemplate show dev dummy0 ingress
>>>> chaintemplate flower chain 0 
>>>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>   eth_type ipv4
>>>>
>>>> Add another template, this time for chain number 22:
>>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>>> # tc chaintemplate show dev dummy0 ingress
>>>> chaintemplate flower chain 0 
>>>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>>   eth_type ipv4
>>>> chaintemplate 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: 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: 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: 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: Mask does not fit the template.
>>>> We have an error talking to the kernel, -1
>>>>
>>>> Removal of a template from non-empty chain would fail:
>>>> # tc chaintemplate del dev dummy0 ingress
>>>> Error: The chain is not empty, unable to delete template.
>>>> We have an error talking to the kernel, -1
>>>
>>> Why this restriction? It's a template, so why can't it be removed
>>> regardless of whether there are filters?
>> 
>> That means you could start to insert filters that does not match the
>> original template. I wanted to avoid it. The chain is utilized in hw for
>> the original template, the filter insertion would have to be sanitized
>> in driver. With this restriction, drivers can depend on filters always
>> be fitting.
>> 
>
>Then the hardware driver should have that restriction not the core tc code.

But why? The same restriction would be in all drivers. I believe it is
better to have in in tc in single place. Drivers can then depend on it.
Do you have a usecase where you need to remove template for non-empty
chain?

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Ahern]

On 6/28/18 8:29 AM, Jiri Pirko wrote:
> Thu, Jun 28, 2018 at 04:18:47PM CEST, dsahern@gmail.com wrote:
>> On 6/28/18 7:08 AM, Jiri Pirko wrote:
>>> Create dummy device with clsact first:
>>> # ip link add type dummy
>>> # tc qdisc add dev dummy0 clsact
>>>
>>> There is no template assigned by default:
>>> # tc chaintemplate show dev dummy0 ingress
>>>
>>> Add a template of type flower allowing to insert rules matching on last
>>> 2 bytes of destination mac address:
>>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>>
>>> The template is now showed in the list:
>>> # tc chaintemplate show dev dummy0 ingress
>>> chaintemplate flower chain 0 
>>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>   eth_type ipv4
>>>
>>> Add another template, this time for chain number 22:
>>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>>> # tc chaintemplate show dev dummy0 ingress
>>> chaintemplate flower chain 0 
>>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>>   eth_type ipv4
>>> chaintemplate 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: 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: 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: 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: Mask does not fit the template.
>>> We have an error talking to the kernel, -1
>>>
>>> Removal of a template from non-empty chain would fail:
>>> # tc chaintemplate del dev dummy0 ingress
>>> Error: The chain is not empty, unable to delete template.
>>> We have an error talking to the kernel, -1
>>
>> Why this restriction? It's a template, so why can't it be removed
>> regardless of whether there are filters?
> 
> That means you could start to insert filters that does not match the
> original template. I wanted to avoid it. The chain is utilized in hw for
> the original template, the filter insertion would have to be sanitized
> in driver. With this restriction, drivers can depend on filters always
> be fitting.
> 

Then the hardware driver should have that restriction not the core tc code.

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Thu, Jun 28, 2018 at 04:18:47PM CEST, dsahern@gmail.com wrote:
>On 6/28/18 7:08 AM, Jiri Pirko wrote:
>> Create dummy device with clsact first:
>> # ip link add type dummy
>> # tc qdisc add dev dummy0 clsact
>> 
>> There is no template assigned by default:
>> # tc chaintemplate show dev dummy0 ingress
>> 
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>> 
>> The template is now showed in the list:
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0 
>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>   eth_type ipv4
>> 
>> Add another template, this time for chain number 22:
>> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> # tc chaintemplate show dev dummy0 ingress
>> chaintemplate flower chain 0 
>>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>>   eth_type ipv4
>> chaintemplate 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: 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: 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: 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: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> 
>> Removal of a template from non-empty chain would fail:
>> # tc chaintemplate del dev dummy0 ingress
>> Error: The chain is not empty, unable to delete template.
>> We have an error talking to the kernel, -1
>
>Why this restriction? It's a template, so why can't it be removed
>regardless of whether there are filters?

That means you could start to insert filters that does not match the
original template. I wanted to avoid it. The chain is utilized in hw for
the original template, the filter insertion would have to be sanitized
in driver. With this restriction, drivers can depend on filters always
be fitting.


>
>> 
>> Once the chain is flushed, the template could be removed:
>> # tc filter del dev dummy0 ingress
>> # tc chaintemplate del dev dummy0 ingress
>> 
>

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[David Ahern]

On 6/28/18 7:08 AM, Jiri Pirko wrote:
> Create dummy device with clsact first:
> # ip link add type dummy
> # tc qdisc add dev dummy0 clsact
> 
> There is no template assigned by default:
> # tc chaintemplate show dev dummy0 ingress
> 
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
> 
> The template is now showed in the list:
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0 
>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>   eth_type ipv4
> 
> Add another template, this time for chain number 22:
> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0 
>   dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>   eth_type ipv4
> chaintemplate 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: 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: 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: 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: Mask does not fit the template.
> We have an error talking to the kernel, -1
> 
> Removal of a template from non-empty chain would fail:
> # tc chaintemplate del dev dummy0 ingress
> Error: The chain is not empty, unable to delete template.
> We have an error talking to the kernel, -1

Why this restriction? It's a template, so why can't it be removed
regardless of whether there are filters?

> 
> Once the chain is flushed, the template could be removed:
> # tc filter del dev dummy0 ingress
> # tc chaintemplate del dev dummy0 ingress
> 

Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

Oh, this is v3 already. The changelog should be:

---
v2->v3:
- patch 5:
  - rebase on top of the reoffload pathset
- patch 6:
  - rebase on top of the reoffload pathset
- patch 9:
  - adjust to the userspace cmdline changes
v1->v2:
- patch 6:
  - remove leftover extack arg in fl_hw_create_tmplt()

[patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw

[Jiri Pirko]

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 template assigned by default:
# tc chaintemplate show dev dummy0 ingress

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

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

Add another template, this time for chain number 22:
# tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
# tc chaintemplate show dev dummy0 ingress
chaintemplate flower chain 0 
  dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
  eth_type ipv4
chaintemplate 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: 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: 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: 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: Mask does not fit the template.
We have an error talking to the kernel, -1

Removal of a template from non-empty chain would fail:
# tc chaintemplate del dev dummy0 ingress
Error: The chain is not empty, unable to delete template.
We have an error talking to the kernel, -1

Once the chain is flushed, the template could be removed:
# tc filter del dev dummy0 ingress
# tc chaintemplate del dev dummy0 ingress

---
v1->v2:
- patch 5:
  - rebase on top of the reoffload pathset
- patch 6:
  - remove leftover extack arg in fl_hw_create_tmplt()
  - rebase on top of the reoffload pathset
- patch 9:
  - adjust to the userspace cmdline changes

Jiri Pirko (9):
  net: sched: push ops lookup bits into tcf_proto_lookup_ops()
  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 chain templates

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |   5 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  12 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c |  12 +-
 .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c    |  25 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  |  44 ++-
 include/net/pkt_cls.h                              |   2 +
 include/net/sch_generic.h                          |  14 +-
 include/uapi/linux/rtnetlink.h                     |   7 +
 net/sched/cls_api.c                                | 424 +++++++++++++++++++--
 net/sched/cls_basic.c                              |   2 +-
 net/sched/cls_bpf.c                                |   3 +-
 net/sched/cls_cgroup.c                             |   2 +-
 net/sched/cls_flow.c                               |   3 +-
 net/sched/cls_flower.c                             | 250 +++++++++---
 net/sched/cls_fw.c                                 |   3 +-
 net/sched/cls_matchall.c                           |   3 +-
 net/sched/cls_route.c                              |   2 +-
 net/sched/cls_rsvp.h                               |   3 +-
 net/sched/cls_tcindex.c                            |   2 +-
 net/sched/cls_u32.c                                |   2 +-
 security/selinux/nlmsgtab.c                        |   2 +-
 tools/testing/selftests/net/forwarding/lib.sh      |  12 +
 .../selftests/net/forwarding/tc_chaintemplates.sh  | 160 ++++++++
 .../selftests/net/forwarding/tc_shblocks.sh        |   2 +
 24 files changed, 900 insertions(+), 96 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_chaintemplates.sh

-- 
2.14.4