Re: [patch net-next 00/10] net: allow user specify TC filter HW stats type

[Pablo Neira Ayuso <pablo@netfilter.org>]

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

Hi Pirko,

On Tue, Feb 25, 2020 at 05:22:03PM +0100, Jiri Pirko wrote:
[...]
> Eh, that is not that simple. The existing users are used to the fact
> that the actions are providing counters by themselves. Having and
> explicit counter action like this would break that expectation.
> Also, I think it should be up to the driver implementation. Some HW
> might only support stats per rule, not the actions. Driver should fit
> into the existing abstraction, I think it is fine.

Something like the sketch patch that I'm attaching?

The idea behind it is to provide a counter action through the
flow_action API. Then, tc_setup_flow_action() checks if this action
comes with counters in that case the counter action is added.

My patch assumes tcf_vlan_counter() provides tells us what counter
type the user wants, I just introduced this to provide an example.

Thank you.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2103 bytes --]

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index c6f7bd22db60..1a5006091edc 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -138,9 +138,16 @@ enum flow_action_id {
 	FLOW_ACTION_MPLS_PUSH,
 	FLOW_ACTION_MPLS_POP,
 	FLOW_ACTION_MPLS_MANGLE,
+	FLOW_ACTION_COUNTER,
 	NUM_FLOW_ACTIONS,
 };
 
+enum flow_action_counter_type {
+	FLOW_COUNTER_DISABLED		= 0,
+	FLOW_COUNTER_DELAYED,
+	FLOW_COUNTER_IMMEDIATE,
+};
+
 /* This is mirroring enum pedit_header_type definition for easy mapping between
  * tc pedit action. Legacy TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK is mapped to
  * FLOW_ACT_MANGLE_UNSPEC, which is supported by no driver.
@@ -213,6 +220,9 @@ struct flow_action_entry {
 			u8		bos;
 			u8		ttl;
 		} mpls_mangle;
+		struct {				/* FLOW_ACTION_COUNTER */
+			enum flow_action_counter_type	type;
+		} counter;
 	};
 };
 
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 13c33eaf1ca1..984f2129c760 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3435,6 +3435,7 @@ static void tcf_sample_get_group(struct flow_action_entry *entry,
 int tc_setup_flow_action(struct flow_action *flow_action,
 			 const struct tcf_exts *exts)
 {
+	enum flow_action_counter_type counter = FLOW_COUNTER_DISABLED;
 	struct tc_action *act;
 	int i, j, k, err = 0;
 
@@ -3489,6 +3490,7 @@ int tc_setup_flow_action(struct flow_action *flow_action,
 				err = -EOPNOTSUPP;
 				goto err_out_locked;
 			}
+			counter = tcf_vlan_counter(act);
 		} else if (is_tcf_tunnel_set(act)) {
 			entry->id = FLOW_ACTION_TUNNEL_ENCAP;
 			err = tcf_tunnel_encap_get_tunnel(entry, act);
@@ -3567,10 +3569,19 @@ int tc_setup_flow_action(struct flow_action *flow_action,
 			err = -EOPNOTSUPP;
 			goto err_out_locked;
 		}
-		spin_unlock_bh(&act->tcfa_lock);
 
 		if (!is_tcf_pedit(act))
 			j++;
+
+		if (counter) {
+			struct flow_action_entry *entry;
+
+			entry = &flow_action->entries[j++];
+			entry->id = FLOW_ACTION_COUNTER;
+			entry->counter.type = counter;
+			counter = FLOW_COUNTER_DISABLED;
+		}
+		spin_unlock_bh(&act->tcfa_lock);
 	}
 
 err_out: