[RFC PATCH v4 net-next 0/4] flow_offload: Re-add per-action statistics

[RFC PATCH v4 net-next 0/4] flow_offload: Re-add per-action statistics

[Edward Cree]

When the flow_offload infrastructure was added, per-action statistics,
 which were previously possible for drivers to support in TC offload,
 were not plumbed through, perhaps because the drivers in the tree did
 not implement them.
In TC (and in the previous offload API) statistics are per-action,
 though generally only on 'delivery' actions like mirred, ok and shot.
 Actions also have an index, which may be supplied by the user, which
 allows the sharing of entities such as counters between multiple rules.
The existing driver implementations did not support this, however,
 instead allocating a single counter per rule.  The flow_offload API did
 not support this either, as (a) the information that two actions were
 actually the same action never reached the driver, and (b) the
 TC_CLSFLOWER_STATS callback is only able to return a single set of
 stats, which are added to all counters for actions on the rule.
Patch #1 of this series fixes (a) by adding a cookie member to struct
 flow_action_entry, while the remaining patches fix (b) by adding a per-
 action offload callback (TC_SETUP_ACTION/TC_ACTION_STATS) for
 RTM_GETACTION, and also (patch #4) calling it for each action when
 dumping a rule.
For drivers supporting per-action stats (of which none yet exist
 upstream, hence RFC), this also means that RTM_GETACTION will return
 up-to-date stats, rather than stale values from the last time the rule
 was dumped.

Changed in v4:
* Incorporated RTM_GETACTION offload series
* Replaced TC_CLSFLOWER_STATS stats-array with multiple TC_SETUP_ACTION
  callbacks
* dropped header inclusion fix (submitted separately)
* re-added RFC tags

Changed in v3:
* replaced action_index with cookie, so drivers don't have to look at
  flow_action_id to distinguish shared actions
* removed RFC tags

Changed in v2:
* use array instead of new callback for getting stats
* remove CVLAN patch (posted separately for net)
* add header inclusion fix

Edward Cree (4):
  flow_offload: add a cookie to flow_action_entry
  net/sched: add callback to get stats on an action from clsflower
    offload
  net/sched: add action block binding to other classifiers
  net/sched: call action stats offload in flower or mall dump

 include/linux/netdevice.h  |  1 +
 include/net/act_api.h      |  7 +++++-
 include/net/flow_offload.h |  2 ++
 include/net/pkt_cls.h      | 18 ++++++++++++++
 net/sched/act_api.c        | 51 ++++++++++++++++++++++++++++++++++++++
 net/sched/cls_api.c        |  2 ++
 net/sched/cls_bpf.c        | 10 +++++++-
 net/sched/cls_flower.c     | 13 ++++++++++
 net/sched/cls_matchall.c   | 12 +++++++++
 net/sched/cls_u32.c        |  7 ++++++
 10 files changed, 121 insertions(+), 2 deletions(-)

[RFC PATCH v4 net-next 1/4] flow_offload: add a cookie to flow_action_entry

[Edward Cree]

Populated with the address of the struct tc_action from which it was made.
Required for support of shared counters (and possibly other shared per-
 action entities in future).

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 include/net/flow_offload.h | 1 +
 net/sched/cls_api.c        | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 36fdb85c974d..d526696958f6 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -140,6 +140,7 @@ enum flow_action_mangle_base {
 
 struct flow_action_entry {
 	enum flow_action_id		id;
+	unsigned long			cookie;
 	union {
 		u32			chain_index;	/* FLOW_ACTION_GOTO */
 		struct net_device	*dev;		/* FLOW_ACTION_REDIRECT */
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d4699156974a..5411cec17af5 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3195,6 +3195,7 @@ int tc_setup_flow_action(struct flow_action *flow_action,
 		struct flow_action_entry *entry;
 
 		entry = &flow_action->entries[j];
+		entry->cookie = (unsigned long)act;
 		if (is_tcf_gact_ok(act)) {
 			entry->id = FLOW_ACTION_ACCEPT;
 		} else if (is_tcf_gact_shot(act)) {

[RFC PATCH v4 net-next 2/4] net/sched: add callback to get stats on an action from clsflower offload

[Edward Cree]

When handling RTM_GETACTION (e.g. tc actions get/list), make a callback to
 blocks with hardware offload of the action to update stats from hardware.
In order to support this, track each action/block binding by allocating a
 struct tc_action_block_binding and adding it to a list on the action.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 include/linux/netdevice.h |  1 +
 include/net/act_api.h     |  2 +-
 include/net/pkt_cls.h     | 18 ++++++++++++++
 net/sched/act_api.c       | 50 +++++++++++++++++++++++++++++++++++++++
 net/sched/cls_flower.c    |  7 ++++++
 5 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..dee84954a1c7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -849,6 +849,7 @@ enum tc_setup_type {
 	TC_SETUP_QDISC_ETF,
 	TC_SETUP_ROOT_QDISC,
 	TC_SETUP_QDISC_GRED,
+	TC_SETUP_ACTION,
 };
 
 /* These structures hold the attributes of bpf state that are being passed
diff --git a/include/net/act_api.h b/include/net/act_api.h
index c61a1bf4e3de..38d1769f279b 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -40,6 +40,7 @@ struct tc_action {
 	struct gnet_stats_queue __percpu *cpu_qstats;
 	struct tc_cookie	__rcu *act_cookie;
 	struct tcf_chain	__rcu *goto_chain;
+	struct list_head		hw_blocks;
 };
 #define tcf_index	common.tcfa_index
 #define tcf_refcnt	common.tcfa_refcnt
@@ -199,5 +200,4 @@ static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
 #endif
 }
 
-
 #endif
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 514e3c80ecc1..880048879904 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -342,6 +342,14 @@ static inline void tcf_exts_put_net(struct tcf_exts *exts)
 	for (; 0; (void)(i), (void)(a), (void)(exts))
 #endif
 
+struct tc_action_block_binding {
+	struct list_head list;
+	struct tcf_block *block;
+};
+
+void tc_bind_action_blocks(struct tcf_exts *exts, struct tcf_block *block);
+void tc_unbind_action_blocks(struct tcf_exts *exts, struct tcf_block *block);
+
 static inline void
 tcf_exts_stats_update(const struct tcf_exts *exts,
 		      u64 bytes, u64 packets, u64 lastuse)
@@ -953,4 +961,14 @@ struct tc_root_qopt_offload {
 	bool ingress;
 };
 
+enum tc_action_command {
+	TC_ACTION_STATS,
+};
+
+struct tc_action_offload {
+	enum tc_action_command command;
+	unsigned long cookie;
+	struct flow_stats stats;
+};
+
 #endif
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index c42ecf4b3c10..239fc28456d9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -427,6 +427,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
 			goto err3;
 	}
 	spin_lock_init(&p->tcfa_lock);
+	INIT_LIST_HEAD(&p->hw_blocks);
 	p->tcfa_index = index;
 	p->tcfa_tm.install = jiffies;
 	p->tcfa_tm.lastuse = jiffies;
@@ -753,6 +754,20 @@ tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 	return a->ops->dump(skb, a, bind, ref);
 }
 
+static void tcf_action_update_stats(struct tc_action *a)
+{
+	struct tc_action_block_binding *bind;
+	struct tc_action_offload offl = {};
+
+	offl.command = TC_ACTION_STATS;
+	offl.cookie = (unsigned long)a;
+	ASSERT_RTNL();
+	list_for_each_entry(bind, &a->hw_blocks, list)
+		tc_setup_cb_call(bind->block, TC_SETUP_ACTION, &offl, false);
+	tcf_action_stats_update(a, offl.stats.bytes, offl.stats.pkts,
+				offl.stats.lastused, true);
+}
+
 int
 tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 {
@@ -763,6 +778,7 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 
 	if (nla_put_string(skb, TCA_KIND, a->ops->kind))
 		goto nla_put_failure;
+	tcf_action_update_stats(a);
 	if (tcf_action_copy_stats(skb, a, 0))
 		goto nla_put_failure;
 
@@ -1538,6 +1554,40 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+/** Add a binding for %block to hw_blocks list of each action in %exts */
+void tc_bind_action_blocks(struct tcf_exts *exts, struct tcf_block *block)
+{
+	struct tc_action_block_binding *bind;
+	struct tc_action *act;
+	int i;
+
+	tcf_exts_for_each_action(i, act, exts) {
+		bind = kzalloc(sizeof(*bind), GFP_KERNEL);
+		if (WARN_ON_ONCE(!bind))
+			continue; /* just skip it, stats won't update timely */
+		bind->block = block;
+		list_add_tail(&bind->list, &act->hw_blocks);
+	}
+}
+EXPORT_SYMBOL(tc_bind_action_blocks);
+
+/** Remove one instance of %block from binding list of each action in %exts */
+void tc_unbind_action_blocks(struct tcf_exts *exts, struct tcf_block *block)
+{
+	struct tc_action_block_binding *bind;
+	struct tc_action *act;
+	int i;
+
+	tcf_exts_for_each_action(i, act, exts)
+		list_for_each_entry(bind, &act->hw_blocks, list)
+			if (bind->block == block) {
+				list_del(&bind->list);
+				kfree(bind);
+				break;
+			}
+}
+EXPORT_SYMBOL(tc_unbind_action_blocks);
+
 static int __init tc_action_init(void)
 {
 	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f6685fc53119..252d102702bb 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -394,6 +394,8 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f,
 	cls_flower.cookie = (unsigned long) f;
 
 	tc_setup_cb_call(block, TC_SETUP_CLSFLOWER, &cls_flower, false);
+	if (f->in_hw_count)
+		tc_unbind_action_blocks(&f->exts, block);
 	spin_lock(&tp->lock);
 	list_del_init(&f->hw_list);
 	tcf_block_offload_dec(block, &f->flags);
@@ -448,6 +450,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 		goto errout;
 	} else if (err > 0) {
 		f->in_hw_count = err;
+		tc_bind_action_blocks(&f->exts, block);
 		err = 0;
 		spin_lock(&tp->lock);
 		tcf_block_offload_inc(block, &f->flags);
@@ -1789,10 +1792,14 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 			goto next_flow;
 		}
 
+		if (add && !f->in_hw_count)
+			tc_bind_action_blocks(&f->exts, block);
 		spin_lock(&tp->lock);
 		tc_cls_offload_cnt_update(block, &f->in_hw_count, &f->flags,
 					  add);
 		spin_unlock(&tp->lock);
+		if (!add && !f->in_hw_count)
+			tc_unbind_action_blocks(&f->exts, block);
 next_flow:
 		__fl_put(f);
 	}

[RFC PATCH v4 net-next 3/4] net/sched: add action block binding to other classifiers

[Edward Cree]

cls_matchall, cls_u32, and cls_bpf all have offloads as well, so they also
 need to bind actions to blocks for RTM_GETACTION stats collection.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 net/sched/cls_bpf.c      | 10 +++++++++-
 net/sched/cls_matchall.c |  7 +++++++
 net/sched/cls_u32.c      |  7 +++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 27365ed3fe0b..c99e53cbf83d 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -165,8 +165,11 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	cls_bpf.name = obj->bpf_name;
 	cls_bpf.exts_integrated = obj->exts_integrated;
 
-	if (oldprog)
+	if (oldprog) {
+		if (oldprog->in_hw_count)
+			tc_unbind_action_blocks(&oldprog->exts, block);
 		tcf_block_offload_dec(block, &oldprog->gen_flags);
+	}
 
 	err = tc_setup_cb_call(block, TC_SETUP_CLSBPF, &cls_bpf, skip_sw);
 	if (prog) {
@@ -175,6 +178,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 			return err;
 		} else if (err > 0) {
 			prog->in_hw_count = err;
+			tc_bind_action_blocks(&prog->exts, block);
 			tcf_block_offload_inc(block, &prog->gen_flags);
 		}
 	}
@@ -683,8 +687,12 @@ static int cls_bpf_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 			continue;
 		}
 
+		if (add && !prog->in_hw_count)
+			tc_bind_action_blocks(&prog->exts, block);
 		tc_cls_offload_cnt_update(block, &prog->in_hw_count,
 					  &prog->gen_flags, add);
+		if (!add && !prog->in_hw_count)
+			tc_unbind_action_blocks(&prog->exts, block);
 	}
 
 	return 0;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index db42d97a2006..5cb4832d1b3b 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -79,6 +79,8 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp,
 	cls_mall.cookie = cookie;
 
 	tc_setup_cb_call(block, TC_SETUP_CLSMATCHALL, &cls_mall, false);
+	if (head->in_hw_count)
+		tc_unbind_action_blocks(&head->exts, block);
 	tcf_block_offload_dec(block, &head->flags);
 }
 
@@ -120,6 +122,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 		return err;
 	} else if (err > 0) {
 		head->in_hw_count = err;
+		tc_bind_action_blocks(&head->exts, block);
 		tcf_block_offload_inc(block, &head->flags);
 	}
 
@@ -320,7 +323,11 @@ static int mall_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 		return 0;
 	}
 
+	if (add && !head->in_hw_count)
+		tc_bind_action_blocks(&head->exts, block);
 	tc_cls_offload_cnt_update(block, &head->in_hw_count, &head->flags, add);
+	if (!add && !head->in_hw_count)
+		tc_unbind_action_blocks(&head->exts, block);
 
 	return 0;
 }
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4b8710a266cc..84f067d9b4a4 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -534,6 +534,8 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	cls_u32.knode.handle = n->handle;
 
 	tc_setup_cb_call(block, TC_SETUP_CLSU32, &cls_u32, false);
+	if (n->in_hw_count)
+		tc_unbind_action_blocks(&n->exts, block);
 	tcf_block_offload_dec(block, &n->flags);
 }
 
@@ -569,6 +571,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 		return err;
 	} else if (err > 0) {
 		n->in_hw_count = err;
+		tc_bind_action_blocks(&n->exts, block);
 		tcf_block_offload_inc(block, &n->flags);
 	}
 
@@ -1223,7 +1226,11 @@ static int u32_reoffload_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 		return 0;
 	}
 
+	if (add && !n->in_hw_count)
+		tc_bind_action_blocks(&n->exts, block);
 	tc_cls_offload_cnt_update(block, &n->in_hw_count, &n->flags, add);
+	if (!add && !n->in_hw_count)
+		tc_unbind_action_blocks(&n->exts, block);
 
 	return 0;
 }

[RFC PATCH v4 net-next 4/4] net/sched: call action stats offload in flower or mall dump

[Edward Cree]

Drivers that support per-action stats should implement TC_ACTION_STATS
 rather than TC_CLSFLOWER_STATS/TC_CLSMATCHALL_STATS.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 include/net/act_api.h      | 5 +++++
 include/net/flow_offload.h | 1 +
 net/sched/act_api.c        | 3 ++-
 net/sched/cls_api.c        | 1 +
 net/sched/cls_flower.c     | 6 ++++++
 net/sched/cls_matchall.c   | 5 +++++
 6 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 38d1769f279b..76054524a2de 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -187,6 +187,11 @@ int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
 			     struct netlink_ext_ack *newchain);
 struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
 					 struct tcf_chain *newchain);
+void tcf_action_update_stats(struct tc_action *a);
+#else /* CONFIG_NET_CLS_ACT */
+static inline void tcf_action_update_stats(struct tc_action *a)
+{
+}
 #endif /* CONFIG_NET_CLS_ACT */
 
 static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index d526696958f6..813a91ae9d9a 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -140,6 +140,7 @@ enum flow_action_mangle_base {
 
 struct flow_action_entry {
 	enum flow_action_id		id;
+	bool				want_stats;
 	unsigned long			cookie;
 	union {
 		u32			chain_index;	/* FLOW_ACTION_GOTO */
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 239fc28456d9..82f9c1f1acd6 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -754,7 +754,7 @@ tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 	return a->ops->dump(skb, a, bind, ref);
 }
 
-static void tcf_action_update_stats(struct tc_action *a)
+void tcf_action_update_stats(struct tc_action *a)
 {
 	struct tc_action_block_binding *bind;
 	struct tc_action_offload offl = {};
@@ -767,6 +767,7 @@ static void tcf_action_update_stats(struct tc_action *a)
 	tcf_action_stats_update(a, offl.stats.bytes, offl.stats.pkts,
 				offl.stats.lastused, true);
 }
+EXPORT_SYMBOL(tcf_action_update_stats);
 
 int
 tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 5411cec17af5..bb9b0d7ec1c0 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3195,6 +3195,7 @@ int tc_setup_flow_action(struct flow_action *flow_action,
 		struct flow_action_entry *entry;
 
 		entry = &flow_action->entries[j];
+		entry->want_stats = act->ops && act->ops->stats_update;
 		entry->cookie = (unsigned long)act;
 		if (is_tcf_gact_ok(act)) {
 			entry->id = FLOW_ACTION_ACCEPT;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 252d102702bb..fcd5615ca2bc 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -477,6 +477,8 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f,
 {
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
+	struct tc_action *act;
+	int i;
 
 	if (!rtnl_held)
 		rtnl_lock();
@@ -492,6 +494,10 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f,
 			      cls_flower.stats.pkts,
 			      cls_flower.stats.lastused);
 
+	tcf_exts_for_each_action(i, act, &f->exts)
+		if (act->ops && act->ops->stats_update)
+			tcf_action_update_stats(act);
+
 	if (!rtnl_held)
 		rtnl_unlock();
 }
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 5cb4832d1b3b..c51beed83b1e 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -338,6 +338,8 @@ static void mall_stats_hw_filter(struct tcf_proto *tp,
 {
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
+	struct tc_action *act;
+	int i;
 
 	tc_cls_common_offload_init(&cls_mall.common, tp, head->flags, NULL);
 	cls_mall.command = TC_CLSMATCHALL_STATS;
@@ -347,6 +349,9 @@ static void mall_stats_hw_filter(struct tcf_proto *tp,
 
 	tcf_exts_stats_update(&head->exts, cls_mall.stats.bytes,
 			      cls_mall.stats.pkts, cls_mall.stats.lastused);
+	tcf_exts_for_each_action(i, act, &head->exts)
+		if (act->ops && act->ops->stats_update)
+			tcf_action_update_stats(act);
 }
 
 static int mall_dump(struct net *net, struct tcf_proto *tp, void *fh,