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

[patch net-next 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

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                             | 251 +++++++++---
 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, 901 insertions(+), 96 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_chaintemplates.sh

-- 
2.14.4

[patch net-next 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 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 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 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 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 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>
---
 include/net/pkt_cls.h  |  2 ++
 net/sched/cls_flower.c | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 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..276ba25a09c3 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1120,6 +1120,43 @@ 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 netlink_ext_ack *extack)
+{
+	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 +1187,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, extack);
+
 	return tmplt;
 
 errout_tmplt:
@@ -1163,6 +1202,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 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 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 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

[patch iproute2/net-next] tc: introduce support for chain templates

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/uapi/linux/rtnetlink.h |  7 ++++
 man/man8/tc.8                  | 26 ++++++++++++
 tc/tc_filter.c                 | 95 ++++++++++++++++++++++++++++++------------
 tc/tc_monitor.c                |  5 ++-
 4 files changed, 105 insertions(+), 28 deletions(-)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index c3a7d8ecc7b9..dddb05e5cca8 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/man/man8/tc.8 b/man/man8/tc.8
index 840880fbdba6..291eeb2ce8ca 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -58,6 +58,22 @@ tc \- show / manipulate traffic control settings
 .B flowid
 \fIflow-id\fR
 
+.B tc
+.RI "[ " OPTIONS " ]"
+.B filter template [ add | delete | get ] dev
+\fIDEV\fR
+.B [ parent
+\fIqdisc-id\fR
+.B | root ]\fR filtertype
+[ filtertype specific parameters ]
+
+.B tc
+.RI "[ " OPTIONS " ]"
+.B filter template [ add | delete | get ] block
+\fIBLOCK_INDEX\fR filtertype
+[ filtertype specific parameters ]
+
+
 .B tc
 .RI "[ " OPTIONS " ]"
 .RI "[ " FORMAT " ]"
@@ -80,6 +96,16 @@ tc \- show / manipulate traffic control settings
 .RI "[ " OPTIONS " ]"
 .B filter show block
 \fIBLOCK_INDEX\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B filter template show dev
+\fIDEV\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B filter template show block
+\fIBLOCK_INDEX\fR
 
 .P
 .B tc
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c5bb0bffe19b..94d7692fe2fa 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -39,6 +39,8 @@ static void usage(void)
 		"\n"
 		"       tc filter show [ dev STRING ] [ root | ingress | egress | parent CLASSID ]\n"
 		"       tc filter show [ block BLOCK_INDEX ]\n"
+		"       tc filter template [ add | del | get | show ] [ dev STRING ]\n"
+		"       tc filter template [ add | del | get | show ] [ block BLOCK_INDEX ] ]\n"
 		"Where:\n"
 		"FILTER_TYPE := { rsvp | u32 | bpf | fw | route | etc. }\n"
 		"FILTERID := ... format depends on classifier, see there\n"
@@ -85,7 +87,8 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
 	req->n.nlmsg_type = cmd;
 	req->t.tcm_family = AF_UNSPEC;
 
-	if (cmd == RTM_NEWTFILTER && flags & NLM_F_CREATE)
+	if ((cmd == RTM_NEWTFILTER || cmd == RTM_NEWCHAINTMPLT) &&
+	    flags & NLM_F_CREATE)
 		protocol = htons(ETH_P_ALL);
 
 	while (argc > 0) {
@@ -261,7 +264,10 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (n->nlmsg_type != RTM_NEWTFILTER &&
 	    n->nlmsg_type != RTM_GETTFILTER &&
-	    n->nlmsg_type != RTM_DELTFILTER) {
+	    n->nlmsg_type != RTM_DELTFILTER &&
+	    n->nlmsg_type != RTM_NEWCHAINTMPLT &&
+	    n->nlmsg_type != RTM_GETCHAINTMPLT &&
+	    n->nlmsg_type != RTM_DELCHAINTMPLT) {
 		fprintf(stderr, "Not a filter(cmd %d)\n", n->nlmsg_type);
 		return 0;
 	}
@@ -280,20 +286,26 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	open_json_object(NULL);
 
-	if (n->nlmsg_type == RTM_DELTFILTER)
+	if (n->nlmsg_type == RTM_DELTFILTER || n->nlmsg_type == RTM_DELCHAINTMPLT)
 		print_bool(PRINT_ANY, "deleted", "deleted ", true);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER &&
+	if ((n->nlmsg_type == RTM_NEWTFILTER ||
+	     n->nlmsg_type == RTM_NEWCHAINTMPLT) &&
 			(n->nlmsg_flags & NLM_F_CREATE) &&
 			!(n->nlmsg_flags & NLM_F_EXCL))
 		print_bool(PRINT_ANY, "replaced", "replaced ", true);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER &&
+	if ((n->nlmsg_type == RTM_NEWTFILTER ||
+	     n->nlmsg_type == RTM_NEWCHAINTMPLT) &&
 			(n->nlmsg_flags & NLM_F_CREATE) &&
 			(n->nlmsg_flags & NLM_F_EXCL))
 		print_bool(PRINT_ANY, "added", "added ", true);
 
-	print_string(PRINT_FP, NULL, "filter ", NULL);
+	if (n->nlmsg_type == RTM_NEWTFILTER ||
+	    n->nlmsg_type == RTM_DELTFILTER)
+		print_string(PRINT_FP, NULL, "filter ", NULL);
+	else
+		print_string(PRINT_FP, NULL, "filter template ", NULL);
 	if (t->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
 		if (!filter_block_index ||
 		    filter_block_index != t->tcm_block_index)
@@ -317,7 +329,9 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		}
 	}
 
-	if (t->tcm_info) {
+	if (t->tcm_info && (n->nlmsg_type == RTM_NEWTFILTER ||
+			    n->nlmsg_type == RTM_DELTFILTER ||
+			    n->nlmsg_type == RTM_GETTFILTER)) {
 		f_proto = TC_H_MIN(t->tcm_info);
 		__u32 prio = TC_H_MAJ(t->tcm_info)>>16;
 
@@ -496,17 +510,19 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		argc--; argv++;
 	}
 
-	if (!protocol_set) {
-		fprintf(stderr, "Must specify filter protocol\n");
-		return -1;
-	}
+	if (cmd == RTM_GETTFILTER) {
+		if (!protocol_set) {
+			fprintf(stderr, "Must specify filter protocol\n");
+			return -1;
+		}
 
-	if (!prio) {
-		fprintf(stderr, "Must specify filter priority\n");
-		return -1;
-	}
+		if (!prio) {
+			fprintf(stderr, "Must specify filter priority\n");
+			return -1;
+		}
 
-	req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+		req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+	}
 
 	if (chain_index_set)
 		addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
@@ -516,11 +532,13 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
-	if (k[0])
-		addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
-	else {
-		fprintf(stderr, "Must specify filter type\n");
-		return -1;
+	if (cmd == RTM_GETTFILTER) {
+		if (k[0])
+			addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
+		else {
+			fprintf(stderr, "Must specify filter type\n");
+			return -1;
+		}
 	}
 
 	if (d[0])  {
@@ -539,10 +557,11 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
-	if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
+	if (cmd == RTM_GETTFILTER &&
+	    q->parse_fopt(q, fhandle, argc, argv, &req.n))
 		return 1;
 
-	if (!fhandle) {
+	if (!fhandle && cmd == RTM_GETTFILTER) {
 		fprintf(stderr, "Must specify filter \"handle\"\n");
 		return -1;
 	}
@@ -569,7 +588,7 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 	return 0;
 }
 
-static int tc_filter_list(int argc, char **argv)
+static int tc_filter_list(int cmd, int argc, char **argv)
 {
 	struct {
 		struct nlmsghdr n;
@@ -577,7 +596,7 @@ static int tc_filter_list(int argc, char **argv)
 		char buf[MAX_MSG];
 	} req = {
 		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
-		.n.nlmsg_type = RTM_GETTFILTER,
+		.n.nlmsg_type = cmd,
 		.t.tcm_parent = TC_H_UNSPEC,
 		.t.tcm_family = AF_UNSPEC,
 	};
@@ -722,10 +741,30 @@ static int tc_filter_list(int argc, char **argv)
 	return 0;
 }
 
+static int do_tmplt(int argc, char **argv, void *buf, size_t buflen)
+{
+	if (matches(*argv, "add") == 0) {
+		return tc_filter_modify(RTM_NEWCHAINTMPLT, NLM_F_EXCL | NLM_F_CREATE,
+					argc - 1, argv + 1, buf, buflen);
+	} else if (matches(*argv, "delete") == 0) {
+		return tc_filter_modify(RTM_DELCHAINTMPLT, 0,
+					argc - 1, argv + 1, buf, buflen);
+	} else if (matches(*argv, "get") == 0) {
+		return tc_filter_get(RTM_GETCHAINTMPLT, 0,
+				     argc - 1, argv + 1);
+	} else if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0 ||
+		   matches(*argv, "lst") == 0) {
+		return tc_filter_list(RTM_GETCHAINTMPLT, argc - 1, argv + 1);
+	}
+	fprintf(stderr, "Command \"%s\" is unknown, try \"tc filter help\".\n",
+		*argv);
+	return -1;
+}
+
 int do_filter(int argc, char **argv, void *buf, size_t buflen)
 {
 	if (argc < 1)
-		return tc_filter_list(0, NULL);
+		return tc_filter_list(RTM_GETTFILTER, 0, NULL);
 	if (matches(*argv, "add") == 0)
 		return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
 					argc-1, argv+1, buf, buflen);
@@ -742,11 +781,13 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
 		return tc_filter_get(RTM_GETTFILTER, 0,  argc-1, argv+1);
 	if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
 	    || matches(*argv, "lst") == 0)
-		return tc_filter_list(argc-1, argv+1);
+		return tc_filter_list(RTM_GETTFILTER, argc-1, argv+1);
 	if (matches(*argv, "help") == 0) {
 		usage();
 		return 0;
 	}
+	if (matches(*argv, "template") == 0)
+		return do_tmplt(argc - 1, argv + 1, buf, buflen);
 	fprintf(stderr, "Command \"%s\" is unknown, try \"tc filter help\".\n",
 		*argv);
 	return -1;
diff --git a/tc/tc_monitor.c b/tc/tc_monitor.c
index 077b138d1ec5..33633837432c 100644
--- a/tc/tc_monitor.c
+++ b/tc/tc_monitor.c
@@ -43,7 +43,10 @@ static int accept_tcmsg(const struct sockaddr_nl *who,
 	if (timestamp)
 		print_timestamp(fp);
 
-	if (n->nlmsg_type == RTM_NEWTFILTER || n->nlmsg_type == RTM_DELTFILTER) {
+	if (n->nlmsg_type == RTM_NEWTFILTER ||
+	    n->nlmsg_type == RTM_DELTFILTER ||
+	    n->nlmsg_type == RTM_NEWCHAINTMPLT ||
+	    n->nlmsg_type == RTM_DELCHAINTMPLT) {
 		print_filter(who, n, arg);
 		return 0;
 	}
-- 
2.14.4

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

[Jakub Kicinski]

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

Perhaps it's lack of sleep, but this paragraph threw me a little off
the track.  IIUC the goal of this set is to provide a way to inform the
HW about expected matches before any rule is programmed into the HW.
Not before any rule is added to a particular chain.  One can just use
the first rule in the chain to make a guess about the chain, but thanks
to this set user can configure *all* chains before any rules are added.

And that's needed because once any rule is added the tcam config can no
longer be easily modified?

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

[Jakub Kicinski]

On Mon, 25 Jun 2018 23:01:45 +0200, Jiri Pirko wrote:
> 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>

> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index d64d43843a3a..276ba25a09c3 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1120,6 +1120,43 @@ 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 netlink_ext_ack *extack)
> +{
> +	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;

Did you skip extack on purpose?

> +	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);
> +}

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

[Jiri Pirko]

Tue, Jun 26, 2018 at 07:00:50AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, 25 Jun 2018 23:01:45 +0200, Jiri Pirko wrote:
>> 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>
>
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index d64d43843a3a..276ba25a09c3 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -1120,6 +1120,43 @@ 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 netlink_ext_ack *extack)
>> +{
>> +	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;
>
>Did you skip extack on purpose?

Oh, the extack is leftover. I will remove it in v2.

>
>> +	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);
>> +}

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

[Jiri Pirko]

Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> For the TC clsact offload these days, some of HW drivers need
>> to hold a magic ball. The reason is, with the first inserted rule inside
>> HW they need to guess what fields will be used for the matching. If
>> later on this guess proves to be wrong and user adds a filter with a
>> different field to match, there's a problem. Mlxsw resolves it now with
>> couple of patterns. Those try to cover as many match fields as possible.
>> This aproach is far from optimal, both performance-wise and scale-wise.
>> Also, there is a combination of filters that in certain order won't
>> succeed.
>> 
>> Most of the time, when user inserts filters in chain, he knows right away
>> how the filters are going to look like - what type and option will they
>> have. For example, he knows that he will only insert filters of type
>> flower matching destination IP address. He can specify a template that
>> would cover all the filters in the chain.
>
>Perhaps it's lack of sleep, but this paragraph threw me a little off
>the track.  IIUC the goal of this set is to provide a way to inform the
>HW about expected matches before any rule is programmed into the HW.
>Not before any rule is added to a particular chain.  One can just use
>the first rule in the chain to make a guess about the chain, but thanks
>to this set user can configure *all* chains before any rules are added.

The template is per-chain. User can use template for chain x and
not-use it for chain y. Up to him.

>
>And that's needed because once any rule is added the tcam config can no
>longer be easily modified?

Yes.

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

[Jakub Kicinski]

On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> For the TC clsact offload these days, some of HW drivers need
>>> to hold a magic ball. The reason is, with the first inserted rule inside
>>> HW they need to guess what fields will be used for the matching. If
>>> later on this guess proves to be wrong and user adds a filter with a
>>> different field to match, there's a problem. Mlxsw resolves it now with
>>> couple of patterns. Those try to cover as many match fields as possible.
>>> This aproach is far from optimal, both performance-wise and scale-wise.
>>> Also, there is a combination of filters that in certain order won't
>>> succeed.
>>>
>>> Most of the time, when user inserts filters in chain, he knows right away
>>> how the filters are going to look like - what type and option will they
>>> have. For example, he knows that he will only insert filters of type
>>> flower matching destination IP address. He can specify a template that
>>> would cover all the filters in the chain.
>>
>>Perhaps it's lack of sleep, but this paragraph threw me a little off
>>the track.  IIUC the goal of this set is to provide a way to inform the
>>HW about expected matches before any rule is programmed into the HW.
>>Not before any rule is added to a particular chain.  One can just use
>>the first rule in the chain to make a guess about the chain, but thanks
>>to this set user can configure *all* chains before any rules are added.
>
> The template is per-chain. User can use template for chain x and
> not-use it for chain y. Up to him.

Makes sense.

I can't help but wonder if it'd be better to associate the
constraints/rules with chains instead of creating a new "template"
object.  It seems more natural to create a chain with specific
constraints in place than add and delete template of which there can
be at most one to a chain...  Perhaps that's more about the user space
tc command line.  Anyway, not a strong objection, just a thought.

>>And that's needed because once any rule is added the tcam config can no
>>longer be easily modified?
>
> Yes.

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

[Jiri Pirko]

Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
>>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> For the TC clsact offload these days, some of HW drivers need
>>>> to hold a magic ball. The reason is, with the first inserted rule inside
>>>> HW they need to guess what fields will be used for the matching. If
>>>> later on this guess proves to be wrong and user adds a filter with a
>>>> different field to match, there's a problem. Mlxsw resolves it now with
>>>> couple of patterns. Those try to cover as many match fields as possible.
>>>> This aproach is far from optimal, both performance-wise and scale-wise.
>>>> Also, there is a combination of filters that in certain order won't
>>>> succeed.
>>>>
>>>> Most of the time, when user inserts filters in chain, he knows right away
>>>> how the filters are going to look like - what type and option will they
>>>> have. For example, he knows that he will only insert filters of type
>>>> flower matching destination IP address. He can specify a template that
>>>> would cover all the filters in the chain.
>>>
>>>Perhaps it's lack of sleep, but this paragraph threw me a little off
>>>the track.  IIUC the goal of this set is to provide a way to inform the
>>>HW about expected matches before any rule is programmed into the HW.
>>>Not before any rule is added to a particular chain.  One can just use
>>>the first rule in the chain to make a guess about the chain, but thanks
>>>to this set user can configure *all* chains before any rules are added.
>>
>> The template is per-chain. User can use template for chain x and
>> not-use it for chain y. Up to him.
>
>Makes sense.
>
>I can't help but wonder if it'd be better to associate the
>constraints/rules with chains instead of creating a new "template"
>object.  It seems more natural to create a chain with specific
>constraints in place than add and delete template of which there can
>be at most one to a chain...  Perhaps that's more about the user space
>tc command line.  Anyway, not a strong objection, just a thought.

Hmm. I don't think it is good idea. User should see the template in a
"show" command per chain. We would have to have 2 show commands, one to
list the template objects and one to list templates per chains. It makes
things more complicated for no good reason. I think that this simple
chain-lock is easier and serves the purpose.

>
>>>And that's needed because once any rule is added the tcam config can no
>>>longer be easily modified?
>>
>> Yes.

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

[Jakub Kicinski]

On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
> >On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:  
> >> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:  
> >>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:  
> >>>> From: Jiri Pirko <jiri@mellanox.com>
> >>>>
> >>>> For the TC clsact offload these days, some of HW drivers need
> >>>> to hold a magic ball. The reason is, with the first inserted rule inside
> >>>> HW they need to guess what fields will be used for the matching. If
> >>>> later on this guess proves to be wrong and user adds a filter with a
> >>>> different field to match, there's a problem. Mlxsw resolves it now with
> >>>> couple of patterns. Those try to cover as many match fields as possible.
> >>>> This aproach is far from optimal, both performance-wise and scale-wise.
> >>>> Also, there is a combination of filters that in certain order won't
> >>>> succeed.
> >>>>
> >>>> Most of the time, when user inserts filters in chain, he knows right away
> >>>> how the filters are going to look like - what type and option will they
> >>>> have. For example, he knows that he will only insert filters of type
> >>>> flower matching destination IP address. He can specify a template that
> >>>> would cover all the filters in the chain.  
> >>>
> >>>Perhaps it's lack of sleep, but this paragraph threw me a little off
> >>>the track.  IIUC the goal of this set is to provide a way to inform the
> >>>HW about expected matches before any rule is programmed into the HW.
> >>>Not before any rule is added to a particular chain.  One can just use
> >>>the first rule in the chain to make a guess about the chain, but thanks
> >>>to this set user can configure *all* chains before any rules are added.  
> >>
> >> The template is per-chain. User can use template for chain x and
> >> not-use it for chain y. Up to him.  
> >
> >Makes sense.
> >
> >I can't help but wonder if it'd be better to associate the
> >constraints/rules with chains instead of creating a new "template"
> >object.  It seems more natural to create a chain with specific
> >constraints in place than add and delete template of which there can
> >be at most one to a chain...  Perhaps that's more about the user space
> >tc command line.  Anyway, not a strong objection, just a thought.  
> 
> Hmm. I don't think it is good idea. User should see the template in a
> "show" command per chain. We would have to have 2 show commands, one to
> list the template objects and one to list templates per chains. It makes
> things more complicated for no good reason. I think that this simple
> chain-lock is easier and serves the purpose.

Hm, I think the dump is fine, what I was thinking about was:

# tc chain add dev dummy0 ingress chain_index 22 \
     ^^^^^
	template proto ip \
	^^^^^^^^
	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

instead of:

# tc filter template add dev dummy0 ingress \
     ^^^^^^^^^^^^^^^
	proto ip chain_index 22 \
	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

And then delete becomes:

# tc chain del dev dummy0 ingress chain_index 22
Error: The chain is not empty.

The fact that template is very much like a filter is sort of an
implementation detail, from user perspective it may be more intuitive
to model template as an attribute of the chain, not a filter object
added to a chain.

But I could well be the only person who feels that way :)

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

[Jiri Pirko]

Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
>> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
>> >On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:  
>> >> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:  
>> >>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:  
>> >>>> From: Jiri Pirko <jiri@mellanox.com>
>> >>>>
>> >>>> For the TC clsact offload these days, some of HW drivers need
>> >>>> to hold a magic ball. The reason is, with the first inserted rule inside
>> >>>> HW they need to guess what fields will be used for the matching. If
>> >>>> later on this guess proves to be wrong and user adds a filter with a
>> >>>> different field to match, there's a problem. Mlxsw resolves it now with
>> >>>> couple of patterns. Those try to cover as many match fields as possible.
>> >>>> This aproach is far from optimal, both performance-wise and scale-wise.
>> >>>> Also, there is a combination of filters that in certain order won't
>> >>>> succeed.
>> >>>>
>> >>>> Most of the time, when user inserts filters in chain, he knows right away
>> >>>> how the filters are going to look like - what type and option will they
>> >>>> have. For example, he knows that he will only insert filters of type
>> >>>> flower matching destination IP address. He can specify a template that
>> >>>> would cover all the filters in the chain.  
>> >>>
>> >>>Perhaps it's lack of sleep, but this paragraph threw me a little off
>> >>>the track.  IIUC the goal of this set is to provide a way to inform the
>> >>>HW about expected matches before any rule is programmed into the HW.
>> >>>Not before any rule is added to a particular chain.  One can just use
>> >>>the first rule in the chain to make a guess about the chain, but thanks
>> >>>to this set user can configure *all* chains before any rules are added.  
>> >>
>> >> The template is per-chain. User can use template for chain x and
>> >> not-use it for chain y. Up to him.  
>> >
>> >Makes sense.
>> >
>> >I can't help but wonder if it'd be better to associate the
>> >constraints/rules with chains instead of creating a new "template"
>> >object.  It seems more natural to create a chain with specific
>> >constraints in place than add and delete template of which there can
>> >be at most one to a chain...  Perhaps that's more about the user space
>> >tc command line.  Anyway, not a strong objection, just a thought.  
>> 
>> Hmm. I don't think it is good idea. User should see the template in a
>> "show" command per chain. We would have to have 2 show commands, one to
>> list the template objects and one to list templates per chains. It makes
>> things more complicated for no good reason. I think that this simple
>> chain-lock is easier and serves the purpose.
>
>Hm, I think the dump is fine, what I was thinking about was:
>
># tc chain add dev dummy0 ingress chain_index 22 \
>     ^^^^^
>	template proto ip \
>	^^^^^^^^
>	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

Okay, I got it. I see 2 issues.
1) user might expect to add a chain without the template. But that does
   not make sense really. Chains are created/deleted implicitly
   according to refcount.
2) there is not chain object like this available to user. Adding it just
   for template looks odd. Also, the "filter" and "template" are very
   much alike. They both are added to a chain, they both implicitly
   create chain if it does not exist, etc.

if you don't like "tc filter template add dev dummy0 ingress", how
about:
"tc template add dev dummy0 ingress ..."
"tc template add dev dummy0 ingress chain 22 ..."
that makes more sense I think.


>
>instead of:
>
># tc filter template add dev dummy0 ingress \
>     ^^^^^^^^^^^^^^^
>	proto ip chain_index 22 \
>	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
>And then delete becomes:
>
># tc chain del dev dummy0 ingress chain_index 22
>Error: The chain is not empty.
>
>The fact that template is very much like a filter is sort of an
>implementation detail, from user perspective it may be more intuitive
>to model template as an attribute of the chain, not a filter object
>added to a chain.
>
>But I could well be the only person who feels that way :)

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

[Samudrala, Sridhar]

On 6/27/2018 12:50 AM, Jiri Pirko wrote:
> Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
>> On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
>>> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
>>>> On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
>>>>>> On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
>>>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>>>
>>>>>>> For the TC clsact offload these days, some of HW drivers need
>>>>>>> to hold a magic ball. The reason is, with the first inserted rule inside
>>>>>>> HW they need to guess what fields will be used for the matching. If
>>>>>>> later on this guess proves to be wrong and user adds a filter with a
>>>>>>> different field to match, there's a problem. Mlxsw resolves it now with
>>>>>>> couple of patterns. Those try to cover as many match fields as possible.
>>>>>>> This aproach is far from optimal, both performance-wise and scale-wise.
>>>>>>> Also, there is a combination of filters that in certain order won't
>>>>>>> succeed.
>>>>>>>
>>>>>>> Most of the time, when user inserts filters in chain, he knows right away
>>>>>>> how the filters are going to look like - what type and option will they
>>>>>>> have. For example, he knows that he will only insert filters of type
>>>>>>> flower matching destination IP address. He can specify a template that
>>>>>>> would cover all the filters in the chain.
>>>>>> Perhaps it's lack of sleep, but this paragraph threw me a little off
>>>>>> the track.  IIUC the goal of this set is to provide a way to inform the
>>>>>> HW about expected matches before any rule is programmed into the HW.
>>>>>> Not before any rule is added to a particular chain.  One can just use
>>>>>> the first rule in the chain to make a guess about the chain, but thanks
>>>>>> to this set user can configure *all* chains before any rules are added.
>>>>> The template is per-chain. User can use template for chain x and
>>>>> not-use it for chain y. Up to him.
>>>> Makes sense.
>>>>
>>>> I can't help but wonder if it'd be better to associate the
>>>> constraints/rules with chains instead of creating a new "template"
>>>> object.  It seems more natural to create a chain with specific
>>>> constraints in place than add and delete template of which there can
>>>> be at most one to a chain...  Perhaps that's more about the user space
>>>> tc command line.  Anyway, not a strong objection, just a thought.
>>> Hmm. I don't think it is good idea. User should see the template in a
>>> "show" command per chain. We would have to have 2 show commands, one to
>>> list the template objects and one to list templates per chains. It makes
>>> things more complicated for no good reason. I think that this simple
>>> chain-lock is easier and serves the purpose.
>> Hm, I think the dump is fine, what I was thinking about was:
>>
>> # tc chain add dev dummy0 ingress chain_index 22 \
>>      ^^^^^
>> 	template proto ip \
>> 	^^^^^^^^
>> 	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
> Okay, I got it. I see 2 issues.
> 1) user might expect to add a chain without the template. But that does
>     not make sense really. Chains are created/deleted implicitly
>     according to refcount.
> 2) there is not chain object like this available to user. Adding it just
>     for template looks odd. Also, the "filter" and "template" are very
>     much alike. They both are added to a chain, they both implicitly
>     create chain if it does not exist, etc.
>
> if you don't like "tc filter template add dev dummy0 ingress", how
> about:
> "tc template add dev dummy0 ingress ..."
> "tc template add dev dummy0 ingress chain 22 ..."
> that makes more sense I think.

Isn't it possible to avoid introducing another keyword 'template',

Can't we just do
       tc chain add dev dummy0 ingress flower chain_index 0
to create a chain that takes any types of flower rules with index 0
and
      tc chain add dev dummy0 ingress flower chain_index 22
             dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
      tc chain add dev dummy0 ingress flower chain_index 23
             dst_ip 192.168.0.0/16
to create 2 chains 22 and 23 that allow rules with specific fields.

To create a chain that takes decap rules like
   filter protocol ip pref 2 flower chain 25 handle 0x2
     dst_mac fe:24:9a:23:4c:5c
     src_mac ce:1d:58:eb:a0:35
     eth_type ipv4
     enc_dst_ip 192.168.100.20
     enc_src_ip 192.168.100.22
     enc_key_id 100
     enc_dst_port 4789

can we create a chain using this format

tc chain add dev dummy0 ingress flower chain_index 25
     dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
     src_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
     enc_dst_ip 192.168.100.0/24
     enc_src_ip 192.168.100.0/24
     eth_type ipv4/arp
     enc_key_id 0/ffff
     enc_dst_port 0/ffff

When adding a filter to a pre-defined chain, does the filter need
to specify all the fields that are included in the chain's template?

>
>
>> instead of:
>>
>> # tc filter template add dev dummy0 ingress \
>>      ^^^^^^^^^^^^^^^
>> 	proto ip chain_index 22 \
>> 	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>
>> And then delete becomes:
>>
>> # tc chain del dev dummy0 ingress chain_index 22
>> Error: The chain is not empty.
>>
>> The fact that template is very much like a filter is sort of an
>> implementation detail, from user perspective it may be more intuitive
>> to model template as an attribute of the chain, not a filter object
>> added to a chain.
>>
>> But I could well be the only person who feels that way :)

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

[Cong Wang]

On Wed, Jun 27, 2018 at 9:46 AM Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
> On 6/27/2018 12:50 AM, Jiri Pirko wrote:
> > if you don't like "tc filter template add dev dummy0 ingress", how
> > about:
> > "tc template add dev dummy0 ingress ..."
> > "tc template add dev dummy0 ingress chain 22 ..."
> > that makes more sense I think.

Better than 'tc filter template', but this doesn't reflect 'template'
is a template of tc filter, it could be an action etc., since it is in the
same position with 'tc action/filter/qdisc'.


>
> Isn't it possible to avoid introducing another keyword 'template',
>
> Can't we just do
>        tc chain add dev dummy0 ingress flower chain_index 0
> to create a chain that takes any types of flower rules with index 0
> and
>       tc chain add dev dummy0 ingress flower chain_index 22
>              dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>       tc chain add dev dummy0 ingress flower chain_index 23
>              dst_ip 192.168.0.0/16
> to create 2 chains 22 and 23 that allow rules with specific fields.

Sounds good too. Since filter chain can be shared by qdiscs,
a 'tc chain' sub-command makes sense, and would probably make
it easier to be shared.

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

[Jakub Kicinski]

On Wed, 27 Jun 2018 09:50:17 +0200, Jiri Pirko wrote:
> Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
> >On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:  
> >> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:  
> >> >On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:    
> >> >> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:    
> >> >>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:    
> >> >>>> From: Jiri Pirko <jiri@mellanox.com>
> >> >>>>
> >> >>>> For the TC clsact offload these days, some of HW drivers need
> >> >>>> to hold a magic ball. The reason is, with the first inserted rule inside
> >> >>>> HW they need to guess what fields will be used for the matching. If
> >> >>>> later on this guess proves to be wrong and user adds a filter with a
> >> >>>> different field to match, there's a problem. Mlxsw resolves it now with
> >> >>>> couple of patterns. Those try to cover as many match fields as possible.
> >> >>>> This aproach is far from optimal, both performance-wise and scale-wise.
> >> >>>> Also, there is a combination of filters that in certain order won't
> >> >>>> succeed.
> >> >>>>
> >> >>>> Most of the time, when user inserts filters in chain, he knows right away
> >> >>>> how the filters are going to look like - what type and option will they
> >> >>>> have. For example, he knows that he will only insert filters of type
> >> >>>> flower matching destination IP address. He can specify a template that
> >> >>>> would cover all the filters in the chain.    
> >> >>>
> >> >>>Perhaps it's lack of sleep, but this paragraph threw me a little off
> >> >>>the track.  IIUC the goal of this set is to provide a way to inform the
> >> >>>HW about expected matches before any rule is programmed into the HW.
> >> >>>Not before any rule is added to a particular chain.  One can just use
> >> >>>the first rule in the chain to make a guess about the chain, but thanks
> >> >>>to this set user can configure *all* chains before any rules are added.    
> >> >>
> >> >> The template is per-chain. User can use template for chain x and
> >> >> not-use it for chain y. Up to him.    
> >> >
> >> >Makes sense.
> >> >
> >> >I can't help but wonder if it'd be better to associate the
> >> >constraints/rules with chains instead of creating a new "template"
> >> >object.  It seems more natural to create a chain with specific
> >> >constraints in place than add and delete template of which there can
> >> >be at most one to a chain...  Perhaps that's more about the user space
> >> >tc command line.  Anyway, not a strong objection, just a thought.    
> >> 
> >> Hmm. I don't think it is good idea. User should see the template in a
> >> "show" command per chain. We would have to have 2 show commands, one to
> >> list the template objects and one to list templates per chains. It makes
> >> things more complicated for no good reason. I think that this simple
> >> chain-lock is easier and serves the purpose.  
> >
> >Hm, I think the dump is fine, what I was thinking about was:
> >
> ># tc chain add dev dummy0 ingress chain_index 22 \
> >     ^^^^^
> >	template proto ip \
> >	^^^^^^^^
> >	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF  
> 
> Okay, I got it. I see 2 issues.
> 1) user might expect to add a chain without the template. But that does
>    not make sense really. Chains are created/deleted implicitly
>    according to refcount.
> 2) there is not chain object like this available to user. Adding it just
>    for template looks odd. Also, the "filter" and "template" are very
>    much alike. They both are added to a chain, they both implicitly
>    create chain if it does not exist, etc.

Yeah, that part makes is tricky :/

> if you don't like "tc filter template add dev dummy0 ingress", how
> about:
> "tc template add dev dummy0 ingress ..."
> "tc template add dev dummy0 ingress chain 22 ..."
> that makes more sense I think.

Mmm..  how about:

 tc chaintemplate add dev dummy0 ingress...

or

 tc restrictedchain add dev dummy0 ingress chain_index XX template ...

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

[Jiri Pirko]

Wed, Jun 27, 2018 at 08:36:24PM CEST, jakub.kicinski@netronome.com wrote:
>On Wed, 27 Jun 2018 09:50:17 +0200, Jiri Pirko wrote:
>> Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
>> >On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:  
>> >> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:  
>> >> >On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:    
>> >> >> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:    
>> >> >>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:    
>> >> >>>> From: Jiri Pirko <jiri@mellanox.com>
>> >> >>>>
>> >> >>>> For the TC clsact offload these days, some of HW drivers need
>> >> >>>> to hold a magic ball. The reason is, with the first inserted rule inside
>> >> >>>> HW they need to guess what fields will be used for the matching. If
>> >> >>>> later on this guess proves to be wrong and user adds a filter with a
>> >> >>>> different field to match, there's a problem. Mlxsw resolves it now with
>> >> >>>> couple of patterns. Those try to cover as many match fields as possible.
>> >> >>>> This aproach is far from optimal, both performance-wise and scale-wise.
>> >> >>>> Also, there is a combination of filters that in certain order won't
>> >> >>>> succeed.
>> >> >>>>
>> >> >>>> Most of the time, when user inserts filters in chain, he knows right away
>> >> >>>> how the filters are going to look like - what type and option will they
>> >> >>>> have. For example, he knows that he will only insert filters of type
>> >> >>>> flower matching destination IP address. He can specify a template that
>> >> >>>> would cover all the filters in the chain.    
>> >> >>>
>> >> >>>Perhaps it's lack of sleep, but this paragraph threw me a little off
>> >> >>>the track.  IIUC the goal of this set is to provide a way to inform the
>> >> >>>HW about expected matches before any rule is programmed into the HW.
>> >> >>>Not before any rule is added to a particular chain.  One can just use
>> >> >>>the first rule in the chain to make a guess about the chain, but thanks
>> >> >>>to this set user can configure *all* chains before any rules are added.    
>> >> >>
>> >> >> The template is per-chain. User can use template for chain x and
>> >> >> not-use it for chain y. Up to him.    
>> >> >
>> >> >Makes sense.
>> >> >
>> >> >I can't help but wonder if it'd be better to associate the
>> >> >constraints/rules with chains instead of creating a new "template"
>> >> >object.  It seems more natural to create a chain with specific
>> >> >constraints in place than add and delete template of which there can
>> >> >be at most one to a chain...  Perhaps that's more about the user space
>> >> >tc command line.  Anyway, not a strong objection, just a thought.    
>> >> 
>> >> Hmm. I don't think it is good idea. User should see the template in a
>> >> "show" command per chain. We would have to have 2 show commands, one to
>> >> list the template objects and one to list templates per chains. It makes
>> >> things more complicated for no good reason. I think that this simple
>> >> chain-lock is easier and serves the purpose.  
>> >
>> >Hm, I think the dump is fine, what I was thinking about was:
>> >
>> ># tc chain add dev dummy0 ingress chain_index 22 \
>> >     ^^^^^
>> >	template proto ip \
>> >	^^^^^^^^
>> >	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF  
>> 
>> Okay, I got it. I see 2 issues.
>> 1) user might expect to add a chain without the template. But that does
>>    not make sense really. Chains are created/deleted implicitly
>>    according to refcount.
>> 2) there is not chain object like this available to user. Adding it just
>>    for template looks odd. Also, the "filter" and "template" are very
>>    much alike. They both are added to a chain, they both implicitly
>>    create chain if it does not exist, etc.
>
>Yeah, that part makes is tricky :/
>
>> if you don't like "tc filter template add dev dummy0 ingress", how
>> about:
>> "tc template add dev dummy0 ingress ..."
>> "tc template add dev dummy0 ingress chain 22 ..."
>> that makes more sense I think.
>
>Mmm..  how about:
>
> tc chaintemplate add dev dummy0 ingress...

This looks fine to me.

>
>or
>
> tc restrictedchain add dev dummy0 ingress chain_index XX template ...

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

[Jiri Pirko]

Wed, Jun 27, 2018 at 07:04:32PM CEST, xiyou.wangcong@gmail.com wrote:
>On Wed, Jun 27, 2018 at 9:46 AM Samudrala, Sridhar
><sridhar.samudrala@intel.com> wrote:
>>
>> On 6/27/2018 12:50 AM, Jiri Pirko wrote:
>> > if you don't like "tc filter template add dev dummy0 ingress", how
>> > about:
>> > "tc template add dev dummy0 ingress ..."
>> > "tc template add dev dummy0 ingress chain 22 ..."
>> > that makes more sense I think.
>
>Better than 'tc filter template', but this doesn't reflect 'template'
>is a template of tc filter, it could be an action etc., since it is in the

It's a template of filter per chain. I don't understand how it could be
an action...


>same position with 'tc action/filter/qdisc'.
>
>
>>
>> Isn't it possible to avoid introducing another keyword 'template',
>>
>> Can't we just do
>>        tc chain add dev dummy0 ingress flower chain_index 0
>> to create a chain that takes any types of flower rules with index 0
>> and
>>       tc chain add dev dummy0 ingress flower chain_index 22
>>              dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>       tc chain add dev dummy0 ingress flower chain_index 23
>>              dst_ip 192.168.0.0/16
>> to create 2 chains 22 and 23 that allow rules with specific fields.
>
>Sounds good too. Since filter chain can be shared by qdiscs,
>a 'tc chain' sub-command makes sense, and would probably make
>it easier to be shared.

We don't have such specific object. It is implicit. We create it
whenever someone users it. Either filter of chain. I don't like new "tc
chain" object in cmdline. It really isn't.

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

[Cong Wang]

On Wed, Jun 27, 2018 at 11:19 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Wed, Jun 27, 2018 at 07:04:32PM CEST, xiyou.wangcong@gmail.com wrote:
> >On Wed, Jun 27, 2018 at 9:46 AM Samudrala, Sridhar
> ><sridhar.samudrala@intel.com> wrote:
> >>
> >> On 6/27/2018 12:50 AM, Jiri Pirko wrote:
> >> > if you don't like "tc filter template add dev dummy0 ingress", how
> >> > about:
> >> > "tc template add dev dummy0 ingress ..."
> >> > "tc template add dev dummy0 ingress chain 22 ..."
> >> > that makes more sense I think.
> >
> >Better than 'tc filter template', but this doesn't reflect 'template'
> >is a template of tc filter, it could be an action etc., since it is in the
>
> It's a template of filter per chain. I don't understand how it could be
> an action...

It's because you have that in your mind from very beginning.

Think about what a new TC user's reaction is to 'tc template'
after he/she learns 'tc qdisc/filter/action'. It could be a template
of either of these 3 literately...


>
>
> >same position with 'tc action/filter/qdisc'.
> >
> >
> >>
> >> Isn't it possible to avoid introducing another keyword 'template',
> >>
> >> Can't we just do
> >>        tc chain add dev dummy0 ingress flower chain_index 0
> >> to create a chain that takes any types of flower rules with index 0
> >> and
> >>       tc chain add dev dummy0 ingress flower chain_index 22
> >>              dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
> >>       tc chain add dev dummy0 ingress flower chain_index 23
> >>              dst_ip 192.168.0.0/16
> >> to create 2 chains 22 and 23 that allow rules with specific fields.
> >
> >Sounds good too. Since filter chain can be shared by qdiscs,
> >a 'tc chain' sub-command makes sense, and would probably make
> >it easier to be shared.
>
> We don't have such specific object. It is implicit. We create it
> whenever someone users it. Either filter of chain. I don't like new "tc
> chain" object in cmdline. It really isn't.

I discussed this with you at netconf, it is similar to tc actions,
tc actions can be shared not because they are implicitly created,
but because they could be created alone via `tc action add ...`.

If you don't share the chain, it is perfectly fine to create it
implicitly. If you do share, as in current code base, making it
standalone is reasonable.