All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] Implement filter terse dump mode support
@ 2020-05-14 11:40 Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 11:40 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner, Vlad Buslov

Implement support for terse dump mode which provides only essential
classifier/action info (handle, stats, cookie, etc.). Use new
TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
kernel.

Implement classifier-action terse dump mode

Output rate of current upstream kernel TC filter dump implementation if
relatively low (~100k rules/sec depending on configuration). This
constraint impacts performance of software switch implementation that
rely on TC for their datapath implementation and periodically call TC
filter dump to update rules stats. Moreover, TC filter dump output a lot
of static data that don't change during the filter lifecycle (filter
key, specific action details, etc.) which constitutes significant
portion of payload on resulting netlink packets and increases amount of
syscalls necessary to dump all filters on particular Qdisc. In order to
significantly improve filter dump rate this patch sets implement new
mode of TC filter dump operation named "terse dump" mode. In this mode
only parameters necessary to identify the filter (handle, action cookie,
etc.) and data that can change during filter lifecycle (filter flags,
action stats, etc.) are preserved in dump output while everything else
is omitted.

Userspace API is implemented using new TCA_DUMP_FLAGS tlv with only
available flag value TCA_DUMP_FLAGS_TERSE. Internally, new API requires
individual classifier support (new tcf_proto_ops->terse_dump()
callback). Support for action terse dump is implemented in act API and
don't require changing individual action implementations.

The following table provides performance comparison between regular
filter dump and new terse dump mode for two classifier-action profiles:
one minimal config with L2 flower classifier and single gact action and
another heavier config with L2+5tuple flower classifier with
tunnel_key+mirred actions.

 Classifier-action type      |        dump |  terse dump | X improvement 
                             | (rules/sec) | (rules/sec) |               
-----------------------------+-------------+-------------+---------------
 L2 with gact                |       141.8 |       293.2 |          2.07 
 L2+5tuple tunnel_key+mirred |        76.4 |       198.8 |          2.60 

Benchmark details: to measure the rate tc filter dump and terse dump
commands are invoked on ingress Qdisc that have one million filters
configured using following commands.

> time sudo tc -s filter show dev ens1f0 ingress >/dev/null

> time sudo tc -s filter show terse dev ens1f0 ingress >/dev/null

Value in results table is calculated by dividing 1000000 total rules by
"real" time reported by time command.

Setup details: 2x Intel(R) Xeon(R) CPU E5-2620 v3 @ 2.40GHz, 32GB memory

Vlad Buslov (4):
  net: sched: introduce terse dump flag
  net: sched: implement terse dump support in act
  net: sched: cls_flower: implement terse dump support
  selftests: implement flower classifier terse dump tests

 include/net/act_api.h                         |  2 +-
 include/net/pkt_cls.h                         |  1 +
 include/net/sch_generic.h                     |  4 ++
 include/uapi/linux/rtnetlink.h                |  6 ++
 net/sched/act_api.c                           | 30 ++++++--
 net/sched/cls_api.c                           | 70 ++++++++++++++++---
 net/sched/cls_flower.c                        | 43 ++++++++++++
 .../tc-testing/tc-tests/filters/tests.json    | 38 ++++++++++
 8 files changed, 177 insertions(+), 17 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH net-next 1/4] net: sched: introduce terse dump flag
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
@ 2020-05-14 11:40 ` Vlad Buslov
  2020-05-14 15:00     ` kbuild test robot
  2020-05-14 20:11   ` David Miller
  2020-05-14 11:40 ` [PATCH net-next 2/4] net: sched: implement terse dump support in act Vlad Buslov
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 11:40 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner,
	Vlad Buslov, Jiri Pirko

Add new TCA_DUMP_FLAGS attribute and use it in cls API to request terse
filter output from classifiers with TCA_DUMP_FLAGS_TERSE flag. This option
is intended to be used to improve performance of TC filter dump when
userland only needs to obtain stats and not the whole classifier/action
data. Extend struct tcf_proto_ops with new terse_dump() callback that must
be defined by supporting classifier implementations.

Support of the options in specific classifiers and actions is
implemented in following patches in the series.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/sch_generic.h      |  4 ++++
 include/uapi/linux/rtnetlink.h |  6 +++++
 net/sched/cls_api.c            | 41 +++++++++++++++++++++++++++-------
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index ab87a8b86a32..c510b03b9751 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -330,6 +330,10 @@ struct tcf_proto_ops {
 	int			(*dump)(struct net*, struct tcf_proto*, void *,
 					struct sk_buff *skb, struct tcmsg*,
 					bool);
+	int			(*terse_dump)(struct net *net,
+					      struct tcf_proto *tp, void *fh,
+					      struct sk_buff *skb,
+					      struct tcmsg *t, bool rtnl_held);
 	int			(*tmplt_dump)(struct sk_buff *skb,
 					      struct net *net,
 					      void *tmplt_priv);
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 4a8c5b745157..073e71ef6bdd 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -609,11 +609,17 @@ enum {
 	TCA_HW_OFFLOAD,
 	TCA_INGRESS_BLOCK,
 	TCA_EGRESS_BLOCK,
+	TCA_DUMP_FLAGS,
 	__TCA_MAX
 };
 
 #define TCA_MAX (__TCA_MAX - 1)
 
+#define TCA_DUMP_FLAGS_TERSE (1 << 0) /* Means that in dump user gets only basic
+				       * data necessary to identify the objects
+				       * (handle, cookie, etc.) and stats.
+				       */
+
 #define TCA_RTA(r)  ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcmsg))))
 #define TCA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcmsg))
 
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 299b963c796e..d754f2718914 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1851,7 +1851,7 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
 			 struct tcf_proto *tp, struct tcf_block *block,
 			 struct Qdisc *q, u32 parent, void *fh,
 			 u32 portid, u32 seq, u16 flags, int event,
-			 bool rtnl_held)
+			 bool terse_dump, bool rtnl_held)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;
@@ -1878,6 +1878,14 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
 		goto nla_put_failure;
 	if (!fh) {
 		tcm->tcm_handle = 0;
+	} else if (terse_dump) {
+		if (tp->ops->terse_dump) {
+			if (tp->ops->terse_dump(net, tp, fh, skb, tcm,
+						rtnl_held) < 0)
+				goto nla_put_failure;
+		} else {
+			goto cls_op_not_supp;
+		}
 	} else {
 		if (tp->ops->dump &&
 		    tp->ops->dump(net, tp, fh, skb, tcm, rtnl_held) < 0)
@@ -1888,6 +1896,7 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
 
 out_nlmsg_trim:
 nla_put_failure:
+cls_op_not_supp:
 	nlmsg_trim(skb, b);
 	return -1;
 }
@@ -1908,7 +1917,7 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
 
 	if (tcf_fill_node(net, skb, tp, block, q, parent, fh, portid,
 			  n->nlmsg_seq, n->nlmsg_flags, event,
-			  rtnl_held) <= 0) {
+			  false, rtnl_held) <= 0) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
@@ -1940,7 +1949,7 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
 
 	if (tcf_fill_node(net, skb, tp, block, q, parent, fh, portid,
 			  n->nlmsg_seq, n->nlmsg_flags, RTM_DELTFILTER,
-			  rtnl_held) <= 0) {
+			  false, rtnl_held) <= 0) {
 		NL_SET_ERR_MSG(extack, "Failed to build del event notification");
 		kfree_skb(skb);
 		return -EINVAL;
@@ -2501,6 +2510,7 @@ struct tcf_dump_args {
 	struct tcf_block *block;
 	struct Qdisc *q;
 	u32 parent;
+	bool terse_dump;
 };
 
 static int tcf_node_dump(struct tcf_proto *tp, void *n, struct tcf_walker *arg)
@@ -2511,12 +2521,12 @@ static int tcf_node_dump(struct tcf_proto *tp, void *n, struct tcf_walker *arg)
 	return tcf_fill_node(net, a->skb, tp, a->block, a->q, a->parent,
 			     n, NETLINK_CB(a->cb->skb).portid,
 			     a->cb->nlh->nlmsg_seq, NLM_F_MULTI,
-			     RTM_NEWTFILTER, true);
+			     RTM_NEWTFILTER, a->terse_dump, true);
 }
 
 static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
 			   struct sk_buff *skb, struct netlink_callback *cb,
-			   long index_start, long *p_index)
+			   long index_start, long *p_index, bool terse)
 {
 	struct net *net = sock_net(skb->sk);
 	struct tcf_block *block = chain->block;
@@ -2545,7 +2555,7 @@ static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
 			if (tcf_fill_node(net, skb, tp, block, q, parent, NULL,
 					  NETLINK_CB(cb->skb).portid,
 					  cb->nlh->nlmsg_seq, NLM_F_MULTI,
-					  RTM_NEWTFILTER, true) <= 0)
+					  RTM_NEWTFILTER, false, true) <= 0)
 				goto errout;
 			cb->args[1] = 1;
 		}
@@ -2561,6 +2571,7 @@ static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
 		arg.w.skip = cb->args[1] - 1;
 		arg.w.count = 0;
 		arg.w.cookie = cb->args[2];
+		arg.terse_dump = terse;
 		tp->ops->walk(tp, &arg.w, true);
 		cb->args[2] = arg.w.cookie;
 		cb->args[1] = arg.w.count + 1;
@@ -2574,6 +2585,12 @@ static bool tcf_chain_dump(struct tcf_chain *chain, struct Qdisc *q, u32 parent,
 	return false;
 }
 
+static const u32 tca_dump_flags_allowed = TCA_DUMP_FLAGS_TERSE;
+
+static const struct nla_policy tcf_tfilter_dump_policy[TCA_MAX + 1] = {
+	[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
+};
+
 /* called with RTNL */
 static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 {
@@ -2583,6 +2600,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	struct Qdisc *q = NULL;
 	struct tcf_block *block;
 	struct tcmsg *tcm = nlmsg_data(cb->nlh);
+	bool terse_dump = false;
 	long index_start;
 	long index;
 	u32 parent;
@@ -2592,10 +2610,17 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		return skb->len;
 
 	err = nlmsg_parse_deprecated(cb->nlh, sizeof(*tcm), tca, TCA_MAX,
-				     NULL, cb->extack);
+				     tcf_tfilter_dump_policy, cb->extack);
 	if (err)
 		return err;
 
+	if (tca[TCA_DUMP_FLAGS]) {
+		struct nla_bitfield32 flags =
+			nla_get_bitfield32(tca[TCA_DUMP_FLAGS]);
+
+		terse_dump = flags.value & TCA_DUMP_FLAGS_TERSE;
+	}
+
 	if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
 		block = tcf_block_refcnt_get(net, tcm->tcm_block_index);
 		if (!block)
@@ -2653,7 +2678,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		    nla_get_u32(tca[TCA_CHAIN]) != chain->index)
 			continue;
 		if (!tcf_chain_dump(chain, q, parent, skb, cb,
-				    index_start, &index)) {
+				    index_start, &index, terse_dump)) {
 			tcf_chain_put(chain);
 			err = -EMSGSIZE;
 			break;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH net-next 2/4] net: sched: implement terse dump support in act
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
@ 2020-05-14 11:40 ` Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 3/4] net: sched: cls_flower: implement terse dump support Vlad Buslov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 11:40 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner,
	Vlad Buslov, Jiri Pirko

Extend tcf_action_dump() with boolean argument 'terse' that is used to
request terse-mode action dump. In terse mode only essential data needed to
identify particular action (action kind, cookie, etc.) and its stats is put
to resulting skb and everything else is omitted. Implement
tcf_exts_terse_dump() helper in cls API that is intended to be used to
request terse dump of all exts (actions) attached to the filter.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/act_api.h |  2 +-
 include/net/pkt_cls.h |  1 +
 net/sched/act_api.c   | 30 +++++++++++++++++++++++-------
 net/sched/cls_api.c   | 29 ++++++++++++++++++++++++++++-
 4 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index c24d7643548e..1b4bfc4437be 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -193,7 +193,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 				    bool rtnl_held,
 				    struct netlink_ext_ack *extack);
 int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[], int bind,
-		    int ref);
+		    int ref, bool terse);
 int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
 
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 04aa0649f3b0..ed65619cbc47 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -325,6 +325,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
 void tcf_exts_destroy(struct tcf_exts *exts);
 void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src);
 int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
+int tcf_exts_terse_dump(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
 
 /**
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fbbec2e562f5..8ac7eb0a8309 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -766,12 +766,10 @@ tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 	return a->ops->dump(skb, a, bind, ref);
 }
 
-int
-tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+static int
+tcf_action_dump_terse(struct sk_buff *skb, struct tc_action *a)
 {
-	int err = -EINVAL;
 	unsigned char *b = skb_tail_pointer(skb);
-	struct nlattr *nest;
 	struct tc_cookie *cookie;
 
 	if (nla_put_string(skb, TCA_KIND, a->ops->kind))
@@ -789,6 +787,23 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 	}
 	rcu_read_unlock();
 
+	return 0;
+
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -1;
+}
+
+int
+tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
+{
+	int err = -EINVAL;
+	unsigned char *b = skb_tail_pointer(skb);
+	struct nlattr *nest;
+
+	if (tcf_action_dump_terse(skb, a))
+		goto nla_put_failure;
+
 	if (a->hw_stats != TCA_ACT_HW_STATS_ANY &&
 	    nla_put_bitfield32(skb, TCA_ACT_HW_STATS,
 			       a->hw_stats, TCA_ACT_HW_STATS_ANY))
@@ -820,7 +835,7 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 EXPORT_SYMBOL(tcf_action_dump_1);
 
 int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[],
-		    int bind, int ref)
+		    int bind, int ref, bool terse)
 {
 	struct tc_action *a;
 	int err = -EINVAL, i;
@@ -831,7 +846,8 @@ int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[],
 		nest = nla_nest_start_noflag(skb, i + 1);
 		if (nest == NULL)
 			goto nla_put_failure;
-		err = tcf_action_dump_1(skb, a, bind, ref);
+		err = terse ? tcf_action_dump_terse(skb, a) :
+			tcf_action_dump_1(skb, a, bind, ref);
 		if (err < 0)
 			goto errout;
 		nla_nest_end(skb, nest);
@@ -1133,7 +1149,7 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 	if (!nest)
 		goto out_nlmsg_trim;
 
-	if (tcf_action_dump(skb, actions, bind, ref) < 0)
+	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
 		goto out_nlmsg_trim;
 
 	nla_nest_end(skb, nest);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d754f2718914..4d83d503f680 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3181,7 +3181,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 			if (nest == NULL)
 				goto nla_put_failure;
 
-			if (tcf_action_dump(skb, exts->actions, 0, 0) < 0)
+			if (tcf_action_dump(skb, exts->actions, 0, 0, false)
+			    < 0)
 				goto nla_put_failure;
 			nla_nest_end(skb, nest);
 		} else if (exts->police) {
@@ -3205,6 +3206,32 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 }
 EXPORT_SYMBOL(tcf_exts_dump);
 
+int tcf_exts_terse_dump(struct sk_buff *skb, struct tcf_exts *exts)
+{
+#ifdef CONFIG_NET_CLS_ACT
+	struct nlattr *nest;
+
+	if (!exts->action || !tcf_exts_has_actions(exts))
+		return 0;
+
+	nest = nla_nest_start_noflag(skb, exts->action);
+	if (!nest)
+		goto nla_put_failure;
+
+	if (tcf_action_dump(skb, exts->actions, 0, 0, true) < 0)
+		goto nla_put_failure;
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -1;
+#else
+	return 0;
+#endif
+}
+EXPORT_SYMBOL(tcf_exts_terse_dump);
+
 
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts)
 {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH net-next 3/4] net: sched: cls_flower: implement terse dump support
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 2/4] net: sched: implement terse dump support in act Vlad Buslov
@ 2020-05-14 11:40 ` Vlad Buslov
  2020-05-14 11:40 ` [PATCH net-next 4/4] selftests: implement flower classifier terse dump tests Vlad Buslov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 11:40 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner,
	Vlad Buslov, Jiri Pirko

Implement tcf_proto_ops->terse_dump() callback for flower classifier. Only
dump handle, flags and action data in terse mode.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 43 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 74a0febcafb8..0c574700da75 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2768,6 +2768,48 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	return -1;
 }
 
+static int fl_terse_dump(struct net *net, struct tcf_proto *tp, void *fh,
+			 struct sk_buff *skb, struct tcmsg *t, bool rtnl_held)
+{
+	struct cls_fl_filter *f = fh;
+	struct nlattr *nest;
+	bool skip_hw;
+
+	if (!f)
+		return skb->len;
+
+	t->tcm_handle = f->handle;
+
+	nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
+	if (!nest)
+		goto nla_put_failure;
+
+	spin_lock(&tp->lock);
+
+	skip_hw = tc_skip_hw(f->flags);
+
+	if (f->flags && nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags))
+		goto nla_put_failure_locked;
+
+	spin_unlock(&tp->lock);
+
+	if (!skip_hw)
+		fl_hw_update_stats(tp, f, rtnl_held);
+
+	if (tcf_exts_terse_dump(skb, &f->exts))
+		goto nla_put_failure;
+
+	nla_nest_end(skb, nest);
+
+	return skb->len;
+
+nla_put_failure_locked:
+	spin_unlock(&tp->lock);
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -1;
+}
+
 static int fl_tmplt_dump(struct sk_buff *skb, struct net *net, void *tmplt_priv)
 {
 	struct fl_flow_tmplt *tmplt = tmplt_priv;
@@ -2832,6 +2874,7 @@ static struct tcf_proto_ops cls_fl_ops __read_mostly = {
 	.hw_add		= fl_hw_add,
 	.hw_del		= fl_hw_del,
 	.dump		= fl_dump,
+	.terse_dump	= fl_terse_dump,
 	.bind_class	= fl_bind_class,
 	.tmplt_create	= fl_tmplt_create,
 	.tmplt_destroy	= fl_tmplt_destroy,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH net-next 4/4] selftests: implement flower classifier terse dump tests
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
                   ` (2 preceding siblings ...)
  2020-05-14 11:40 ` [PATCH net-next 3/4] net: sched: cls_flower: implement terse dump support Vlad Buslov
@ 2020-05-14 11:40 ` Vlad Buslov
  2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
  2020-05-14 17:26 ` [PATCH net-next 0/4] " Jakub Kicinski
  5 siblings, 0 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 11:40 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner, Vlad Buslov

Implement two basic tests to verify terse dump functionality of flower
classifier:

- Test that verifies that terse dump works.

- Test that verifies that terse dump doesn't print filter key.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 .../tc-testing/tc-tests/filters/tests.json    | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
index 8877f7b2b809..2d180865c857 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
@@ -87,5 +87,43 @@
         "teardown": [
             "$TC qdisc del dev $DEV2 ingress"
         ]
+    },
+    {
+        "id": "7c65",
+        "name": "Add flower filter and then terse dump it",
+        "category": [
+            "filter",
+            "flower"
+        ],
+        "setup": [
+            "$TC qdisc add dev $DEV2 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV2 protocol ip pref 1 ingress flower dst_mac e4:11:22:11:4a:51 action drop",
+        "expExitCode": "0",
+        "verifyCmd": "$TC filter show terse dev $DEV2 ingress",
+        "matchPattern": "filter protocol ip pref 1 flower.*handle",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV2 ingress"
+        ]
+    },
+    {
+        "id": "d45e",
+        "name": "Add flower filter and verify that terse dump doesn't output filter key",
+        "category": [
+            "filter",
+            "flower"
+        ],
+        "setup": [
+            "$TC qdisc add dev $DEV2 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV2 protocol ip pref 1 ingress flower dst_mac e4:11:22:11:4a:51 action drop",
+        "expExitCode": "0",
+        "verifyCmd": "$TC filter show terse dev $DEV2 ingress",
+        "matchPattern": "  dst_mac e4:11:22:11:4a:51",
+        "matchCount": "0",
+        "teardown": [
+            "$TC qdisc del dev $DEV2 ingress"
+        ]
     }
 ]
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH iproute2-next 0/2] Implement filter terse dump mode support
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
                   ` (3 preceding siblings ...)
  2020-05-14 11:40 ` [PATCH net-next 4/4] selftests: implement flower classifier terse dump tests Vlad Buslov
@ 2020-05-14 13:23 ` Vlad Buslov
  2020-05-14 13:23   ` [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing Vlad Buslov
                     ` (2 more replies)
  2020-05-14 17:26 ` [PATCH net-next 0/4] " Jakub Kicinski
  5 siblings, 3 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 13:23 UTC (permalink / raw)
  To: dsahern, stephen
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, marcelo.leitner,
	dcaratti, Vlad Buslov

Implement support for terse dump mode which provides only essential
classifier/action info (handle, stats, cookie, etc.). Use new
TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
kernel.

Vlad Buslov (2):
  tc: skip actions that don't have options attribute when printing
  tc: implement support for terse dump

 include/uapi/linux/rtnetlink.h |  6 ++++++
 tc/m_bpf.c                     |  2 +-
 tc/m_connmark.c                |  2 +-
 tc/m_csum.c                    |  2 +-
 tc/m_ct.c                      |  2 +-
 tc/m_ctinfo.c                  |  2 +-
 tc/m_gact.c                    |  2 +-
 tc/m_ife.c                     |  2 +-
 tc/m_ipt.c                     |  2 +-
 tc/m_mirred.c                  |  2 +-
 tc/m_mpls.c                    |  2 +-
 tc/m_nat.c                     |  2 +-
 tc/m_pedit.c                   |  2 +-
 tc/m_sample.c                  |  2 +-
 tc/m_simple.c                  |  2 +-
 tc/m_skbedit.c                 |  2 +-
 tc/m_skbmod.c                  |  2 +-
 tc/m_tunnel_key.c              |  2 +-
 tc/m_vlan.c                    |  2 +-
 tc/m_xt.c                      |  2 +-
 tc/m_xt_old.c                  |  2 +-
 tc/tc_filter.c                 | 12 ++++++++++++
 22 files changed, 38 insertions(+), 20 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing
  2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
@ 2020-05-14 13:23   ` Vlad Buslov
  2020-05-14 13:42     ` Jiri Pirko
  2020-05-14 13:23   ` [PATCH iproute2-next 2/2] tc: implement support for terse dump Vlad Buslov
  2020-05-21 18:01   ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support David Ahern
  2 siblings, 1 reply; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 13:23 UTC (permalink / raw)
  To: dsahern, stephen
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, marcelo.leitner,
	dcaratti, Vlad Buslov

Modify implementations that return error from action_until->print_aopt()
callback to silently skip actions that don't have their corresponding
TCA_ACT_OPTIONS attribute set (some actions already behave like this). This
is necessary to support terse dump mode in following patch in the series.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 tc/m_bpf.c        | 2 +-
 tc/m_connmark.c   | 2 +-
 tc/m_csum.c       | 2 +-
 tc/m_ct.c         | 2 +-
 tc/m_ctinfo.c     | 2 +-
 tc/m_gact.c       | 2 +-
 tc/m_ife.c        | 2 +-
 tc/m_ipt.c        | 2 +-
 tc/m_mirred.c     | 2 +-
 tc/m_mpls.c       | 2 +-
 tc/m_nat.c        | 2 +-
 tc/m_pedit.c      | 2 +-
 tc/m_sample.c     | 2 +-
 tc/m_simple.c     | 2 +-
 tc/m_skbedit.c    | 2 +-
 tc/m_skbmod.c     | 2 +-
 tc/m_tunnel_key.c | 2 +-
 tc/m_vlan.c       | 2 +-
 tc/m_xt.c         | 2 +-
 tc/m_xt_old.c     | 2 +-
 20 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index e8d704b557f9..6b231cb3d7b0 100644
--- a/tc/m_bpf.c
+++ b/tc/m_bpf.c
@@ -162,7 +162,7 @@ static int bpf_print_opt(struct action_util *au, FILE *f, struct rtattr *arg)
 	int d_ok = 0;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_ACT_BPF_MAX, arg);
 
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 4b2dc4e25ef8..38546c75b30c 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -111,7 +111,7 @@ static int print_connmark(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_connmark *ci;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_CONNMARK_MAX, arg);
 	if (tb[TCA_CONNMARK_PARMS] == NULL) {
diff --git a/tc/m_csum.c b/tc/m_csum.c
index afbee9c8de0f..347e5e90e967 100644
--- a/tc/m_csum.c
+++ b/tc/m_csum.c
@@ -167,7 +167,7 @@ print_csum(struct action_util *au, FILE *f, struct rtattr *arg)
 	int uflag_count = 0;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_CSUM_MAX, arg);
 
diff --git a/tc/m_ct.c b/tc/m_ct.c
index 70d186e859f4..20cc9c8a3102 100644
--- a/tc/m_ct.c
+++ b/tc/m_ct.c
@@ -444,7 +444,7 @@ static int print_ct(struct action_util *au, FILE *f, struct rtattr *arg)
 	int ct_action = 0;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_CT_MAX, arg);
 	if (tb[TCA_CT_PARMS] == NULL) {
diff --git a/tc/m_ctinfo.c b/tc/m_ctinfo.c
index e5c1b43642a7..5475fe4d43da 100644
--- a/tc/m_ctinfo.c
+++ b/tc/m_ctinfo.c
@@ -189,7 +189,7 @@ static int print_ctinfo(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_ctinfo *ci;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_CTINFO_MAX, arg);
 	if (!tb[TCA_CTINFO_ACT]) {
diff --git a/tc/m_gact.c b/tc/m_gact.c
index 33f326f823d1..2722e9b805f7 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -172,7 +172,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct rtattr *tb[TCA_GACT_MAX + 1];
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_GACT_MAX, arg);
 
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 6a85e087ede1..2491ddb861c2 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -228,7 +228,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 	SPRINT_BUF(b2);
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_IFE_MAX, arg);
 
diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index cc95eab7fefb..046b310e64ab 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -433,7 +433,7 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg)
 	__u32 hook;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	lib_dir = getenv("IPTABLES_LIB_DIR");
 	if (!lib_dir)
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index d2bdf4074a73..7c6351865788 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -282,7 +282,7 @@ print_mirred(struct action_util *au, FILE *f, struct rtattr *arg)
 	const char *dev;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_MIRRED_MAX, arg);
 
diff --git a/tc/m_mpls.c b/tc/m_mpls.c
index 3d5d9b25fbf6..1a3bfdda104b 100644
--- a/tc/m_mpls.c
+++ b/tc/m_mpls.c
@@ -198,7 +198,7 @@ static int print_mpls(struct action_util *au, FILE *f, struct rtattr *arg)
 	__u32 val;
 
 	if (!arg)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_MPLS_MAX, arg);
 
diff --git a/tc/m_nat.c b/tc/m_nat.c
index 56e8f47cdefd..63de71014efd 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -147,7 +147,7 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
 	int len;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_NAT_MAX, arg);
 
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 51dcf10930e8..ec71fcf9922e 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -746,7 +746,7 @@ static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
 	int err;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_PEDIT_MAX, arg);
 
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 4a30513a6247..e2467a93444a 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -144,7 +144,7 @@ static int print_sample(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_sample *p;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_SAMPLE_MAX, arg);
 
diff --git a/tc/m_simple.c b/tc/m_simple.c
index 70897d6b7c13..bc86be27cbcc 100644
--- a/tc/m_simple.c
+++ b/tc/m_simple.c
@@ -166,7 +166,7 @@ static int print_simple(struct action_util *au, FILE *f, struct rtattr *arg)
 	char *simpdata;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_DEF_MAX, arg);
 
diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index 9afe2f0c049d..37625c5ab067 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -199,7 +199,7 @@ static int print_skbedit(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_skbedit *p;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_SKBEDIT_MAX, arg);
 
diff --git a/tc/m_skbmod.c b/tc/m_skbmod.c
index d38a5c1921e7..e13d3f16bfcb 100644
--- a/tc/m_skbmod.c
+++ b/tc/m_skbmod.c
@@ -169,7 +169,7 @@ static int print_skbmod(struct action_util *au, FILE *f, struct rtattr *arg)
 	SPRINT_BUF(b2);
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_SKBMOD_MAX, arg);
 
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index bfec90724d72..f700f6d86c82 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -671,7 +671,7 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_tunnel_key *parm;
 
 	if (!arg)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_TUNNEL_KEY_MAX, arg);
 
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 1096ba0fbf12..afc9b475ae0a 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -183,7 +183,7 @@ static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_vlan *parm;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	parse_rtattr_nested(tb, TCA_VLAN_MAX, arg);
 
diff --git a/tc/m_xt.c b/tc/m_xt.c
index 487ba25ad391..deaf96a26f75 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -320,7 +320,7 @@ print_ipt(struct action_util *au, FILE *f, struct rtattr *arg)
 	__u32 hook;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	/* copy tcipt_globals because .opts will be modified by iptables */
 	struct xtables_globals tmp_tcipt_globals = tcipt_globals;
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 6a4509a9982f..db014898590d 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -358,7 +358,7 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg)
 	__u32 hook;
 
 	if (arg == NULL)
-		return -1;
+		return 0;
 
 	set_lib_dir();
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH iproute2-next 2/2] tc: implement support for terse dump
  2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
  2020-05-14 13:23   ` [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing Vlad Buslov
@ 2020-05-14 13:23   ` Vlad Buslov
  2020-05-14 13:42     ` Jiri Pirko
  2020-05-21 18:01   ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support David Ahern
  2 siblings, 1 reply; 17+ messages in thread
From: Vlad Buslov @ 2020-05-14 13:23 UTC (permalink / raw)
  To: dsahern, stephen
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, marcelo.leitner,
	dcaratti, Vlad Buslov

Implement support for classifier/action terse dump using new TCA_DUMP_FLAGS
tlv with only available flag value TCA_DUMP_FLAGS_TERSE. Set the flag when
user requested it with following example CLI:

> tc -s filter show terse dev ens1f0 ingress

In terse mode dump only outputs essential data needed to identify the
filter and action (handle, cookie, etc.) and stats, if requested by the
user. The intention is to significantly improve rule dump rate by omitting
all static data that do not change after rule is created.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 include/uapi/linux/rtnetlink.h |  6 ++++++
 tc/tc_filter.c                 | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9d802cd7f695..bcb1ba4d0146 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -609,11 +609,17 @@ enum {
 	TCA_HW_OFFLOAD,
 	TCA_INGRESS_BLOCK,
 	TCA_EGRESS_BLOCK,
+	TCA_DUMP_FLAGS,
 	__TCA_MAX
 };
 
 #define TCA_MAX (__TCA_MAX - 1)
 
+#define TCA_DUMP_FLAGS_TERSE (1 << 0) /* Means that in dump user gets only basic
+				       * data necessary to identify the objects
+				       * (handle, cookie, etc.) and stats.
+				       */
+
 #define TCA_RTA(r)  ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcmsg))))
 #define TCA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcmsg))
 
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c591a19f3123..6a82f9bb42fb 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -595,6 +595,7 @@ static int tc_filter_list(int cmd, int argc, char **argv)
 		.t.tcm_parent = TC_H_UNSPEC,
 		.t.tcm_family = AF_UNSPEC,
 	};
+	bool terse_dump = false;
 	char d[IFNAMSIZ] = {};
 	__u32 prio = 0;
 	__u32 protocol = 0;
@@ -687,6 +688,8 @@ static int tc_filter_list(int cmd, int argc, char **argv)
 				invarg("invalid chain index value", *argv);
 			filter_chain_index_set = 1;
 			filter_chain_index = chain_index;
+		} else if (matches(*argv, "terse") == 0) {
+			terse_dump = true;
 		} else if (matches(*argv, "help") == 0) {
 			usage();
 		} else {
@@ -721,6 +724,15 @@ static int tc_filter_list(int cmd, int argc, char **argv)
 	if (filter_chain_index_set)
 		addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
 
+	if (terse_dump) {
+		struct nla_bitfield32 flags = {
+			.value = TCA_DUMP_FLAGS_TERSE,
+			.selector = TCA_DUMP_FLAGS_TERSE
+		};
+
+		addattr_l(&req.n, MAX_MSG, TCA_DUMP_FLAGS, &flags, sizeof(flags));
+	}
+
 	if (rtnl_dump_request_n(&rth, &req.n) < 0) {
 		perror("Cannot send dump request");
 		return 1;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing
  2020-05-14 13:23   ` [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing Vlad Buslov
@ 2020-05-14 13:42     ` Jiri Pirko
  0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2020-05-14 13:42 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: dsahern, stephen, netdev, davem, jhs, xiyou.wangcong,
	marcelo.leitner, dcaratti

Thu, May 14, 2020 at 03:23:05PM CEST, vladbu@mellanox.com wrote:
>Modify implementations that return error from action_until->print_aopt()
>callback to silently skip actions that don't have their corresponding
>TCA_ACT_OPTIONS attribute set (some actions already behave like this). This
>is necessary to support terse dump mode in following patch in the series.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Adding forgotten tag:
Reviewed-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 2/2] tc: implement support for terse dump
  2020-05-14 13:23   ` [PATCH iproute2-next 2/2] tc: implement support for terse dump Vlad Buslov
@ 2020-05-14 13:42     ` Jiri Pirko
  0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2020-05-14 13:42 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: dsahern, stephen, netdev, davem, jhs, xiyou.wangcong,
	marcelo.leitner, dcaratti

Thu, May 14, 2020 at 03:23:06PM CEST, vladbu@mellanox.com wrote:
>Implement support for classifier/action terse dump using new TCA_DUMP_FLAGS
>tlv with only available flag value TCA_DUMP_FLAGS_TERSE. Set the flag when
>user requested it with following example CLI:
>
>> tc -s filter show terse dev ens1f0 ingress
>
>In terse mode dump only outputs essential data needed to identify the
>filter and action (handle, cookie, etc.) and stats, if requested by the
>user. The intention is to significantly improve rule dump rate by omitting
>all static data that do not change after rule is created.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Adding forgotten tag:
Reviewed-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/4] net: sched: introduce terse dump flag
  2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
@ 2020-05-14 15:00     ` kbuild test robot
  2020-05-14 20:11   ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2020-05-14 15:00 UTC (permalink / raw)
  To: Vlad Buslov, netdev
  Cc: kbuild-all, davem, jhs, xiyou.wangcong, jiri, dcaratti,
	marcelo.leitner, Vlad Buslov, Jiri Pirko

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

Hi Vlad,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.7-rc5 next-20200512]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Vlad-Buslov/Implement-filter-terse-dump-mode-support/20200514-194322
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 6545be82807cc01712411321730656ad8ad30474
config: um-allmodconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/net/rtnetlink.h:6:0,
from include/net/sch_generic.h:20,
from include/linux/filter.h:25,
from include/net/sock.h:59,
from net/sched/cls_api.c:27:
>> net/sched/cls_api.c:2591:43: error: initializer element is not constant
[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
^
include/net/netlink.h:370:48: note: in definition of macro 'NLA_POLICY_BITFIELD32'
{ .type = NLA_BITFIELD32, .bitfield32_valid = valid }
^~~~~
net/sched/cls_api.c:2591:43: note: (near initialization for 'tcf_tfilter_dump_policy[15].<anonymous>.bitfield32_valid')
[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
^
include/net/netlink.h:370:48: note: in definition of macro 'NLA_POLICY_BITFIELD32'
{ .type = NLA_BITFIELD32, .bitfield32_valid = valid }
^~~~~

vim +2591 net/sched/cls_api.c

  2589	
  2590	static const struct nla_policy tcf_tfilter_dump_policy[TCA_MAX + 1] = {
> 2591		[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
  2592	};
  2593	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22594 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/4] net: sched: introduce terse dump flag
@ 2020-05-14 15:00     ` kbuild test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2020-05-14 15:00 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]

Hi Vlad,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.7-rc5 next-20200512]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Vlad-Buslov/Implement-filter-terse-dump-mode-support/20200514-194322
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 6545be82807cc01712411321730656ad8ad30474
config: um-allmodconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/net/rtnetlink.h:6:0,
from include/net/sch_generic.h:20,
from include/linux/filter.h:25,
from include/net/sock.h:59,
from net/sched/cls_api.c:27:
>> net/sched/cls_api.c:2591:43: error: initializer element is not constant
[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
^
include/net/netlink.h:370:48: note: in definition of macro 'NLA_POLICY_BITFIELD32'
{ .type = NLA_BITFIELD32, .bitfield32_valid = valid }
^~~~~
net/sched/cls_api.c:2591:43: note: (near initialization for 'tcf_tfilter_dump_policy[15].<anonymous>.bitfield32_valid')
[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
^
include/net/netlink.h:370:48: note: in definition of macro 'NLA_POLICY_BITFIELD32'
{ .type = NLA_BITFIELD32, .bitfield32_valid = valid }
^~~~~

vim +2591 net/sched/cls_api.c

  2589	
  2590	static const struct nla_policy tcf_tfilter_dump_policy[TCA_MAX + 1] = {
> 2591		[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
  2592	};
  2593	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 22594 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] Implement filter terse dump mode support
  2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
                   ` (4 preceding siblings ...)
  2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
@ 2020-05-14 17:26 ` Jakub Kicinski
  2020-05-15 11:25   ` Vlad Buslov
  5 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-05-14 17:26 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner

On Thu, 14 May 2020 14:40:22 +0300 Vlad Buslov wrote:
> Implement support for terse dump mode which provides only essential
> classifier/action info (handle, stats, cookie, etc.). Use new
> TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
> kernel.

Looks reasonable:

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Apart from the change in NLA_BITFIELD policy build bot already pointed
out there is also an unnecessary new line after a function in patch 2.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/4] net: sched: introduce terse dump flag
  2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
  2020-05-14 15:00     ` kbuild test robot
@ 2020-05-14 20:11   ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2020-05-14 20:11 UTC (permalink / raw)
  To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri, dcaratti, marcelo.leitner, jiri

From: Vlad Buslov <vladbu@mellanox.com>
Date: Thu, 14 May 2020 14:40:23 +0300

> +static const u32 tca_dump_flags_allowed = TCA_DUMP_FLAGS_TERSE;
> +
> +static const struct nla_policy tcf_tfilter_dump_policy[TCA_MAX + 1] = {
> +	[TCA_DUMP_FLAGS] = NLA_POLICY_BITFIELD32(tca_dump_flags_allowed),
> +};

The compiler apparently dones't consider your NLA_POLICY_BITFIELD32()
argument constant as be kbuild robot.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] Implement filter terse dump mode support
  2020-05-14 17:26 ` [PATCH net-next 0/4] " Jakub Kicinski
@ 2020-05-15 11:25   ` Vlad Buslov
  0 siblings, 0 replies; 17+ messages in thread
From: Vlad Buslov @ 2020-05-15 11:25 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Vlad Buslov, netdev, davem, jhs, xiyou.wangcong, jiri, dcaratti,
	marcelo.leitner

On Thu 14 May 2020 at 20:26, Jakub Kicinski <kuba@kernel.org> wrote:
> On Thu, 14 May 2020 14:40:22 +0300 Vlad Buslov wrote:
>> Implement support for terse dump mode which provides only essential
>> classifier/action info (handle, stats, cookie, etc.). Use new
>> TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
>> kernel.
>
> Looks reasonable:
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
> Apart from the change in NLA_BITFIELD policy build bot already pointed
> out there is also an unnecessary new line after a function in patch 2.

Hi Jakub,

Thanks for reviewing my code!
I'll send V2 with those two issues fixed.

Regards,
Vlad

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 0/2] Implement filter terse dump mode support
  2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
  2020-05-14 13:23   ` [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing Vlad Buslov
  2020-05-14 13:23   ` [PATCH iproute2-next 2/2] tc: implement support for terse dump Vlad Buslov
@ 2020-05-21 18:01   ` David Ahern
  2020-06-01 18:58     ` David Ahern
  2 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2020-05-21 18:01 UTC (permalink / raw)
  To: Vlad Buslov, stephen
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, marcelo.leitner, dcaratti

On 5/14/20 7:23 AM, Vlad Buslov wrote:
> Implement support for terse dump mode which provides only essential
> classifier/action info (handle, stats, cookie, etc.). Use new
> TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
> kernel.
> 

FYI: I am waiting for the discussion on this feature to settle before
applying.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 0/2] Implement filter terse dump mode support
  2020-05-21 18:01   ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support David Ahern
@ 2020-06-01 18:58     ` David Ahern
  0 siblings, 0 replies; 17+ messages in thread
From: David Ahern @ 2020-06-01 18:58 UTC (permalink / raw)
  To: Vlad Buslov, stephen
  Cc: netdev, davem, jhs, xiyou.wangcong, jiri, marcelo.leitner, dcaratti

On 5/21/20 12:01 PM, David Ahern wrote:
> On 5/14/20 7:23 AM, Vlad Buslov wrote:
>> Implement support for terse dump mode which provides only essential
>> classifier/action info (handle, stats, cookie, etc.). Use new
>> TCA_DUMP_FLAGS_TERSE flag to prevent copying of unnecessary data from
>> kernel.
>>
> 
> FYI: I am waiting for the discussion on this feature to settle before
> applying.
> 

Where did the kernel side end up with this discussion?

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-06-01 18:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14 11:40 [PATCH net-next 0/4] Implement filter terse dump mode support Vlad Buslov
2020-05-14 11:40 ` [PATCH net-next 1/4] net: sched: introduce terse dump flag Vlad Buslov
2020-05-14 15:00   ` kbuild test robot
2020-05-14 15:00     ` kbuild test robot
2020-05-14 20:11   ` David Miller
2020-05-14 11:40 ` [PATCH net-next 2/4] net: sched: implement terse dump support in act Vlad Buslov
2020-05-14 11:40 ` [PATCH net-next 3/4] net: sched: cls_flower: implement terse dump support Vlad Buslov
2020-05-14 11:40 ` [PATCH net-next 4/4] selftests: implement flower classifier terse dump tests Vlad Buslov
2020-05-14 13:23 ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support Vlad Buslov
2020-05-14 13:23   ` [PATCH iproute2-next 1/2] tc: skip actions that don't have options attribute when printing Vlad Buslov
2020-05-14 13:42     ` Jiri Pirko
2020-05-14 13:23   ` [PATCH iproute2-next 2/2] tc: implement support for terse dump Vlad Buslov
2020-05-14 13:42     ` Jiri Pirko
2020-05-21 18:01   ` [PATCH iproute2-next 0/2] Implement filter terse dump mode support David Ahern
2020-06-01 18:58     ` David Ahern
2020-05-14 17:26 ` [PATCH net-next 0/4] " Jakub Kicinski
2020-05-15 11:25   ` Vlad Buslov

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