DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action
@ 2021-04-26 12:42 Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 1/5] net/mlx5: support flow count action handle Michael Baum
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko

Add support of indirect action API for count action.

Michael Baum (5):
  net/mlx5: support flow count action handle
  app/testpmd: remove indirect RSS action query
  app/testpmd: support indirect counter action query
  net/mlx5: fix flow age event triggering
  net/mlx5: use aging by counter when counter is existed

 app/test-pmd/config.c                  | 152 +++++++-------
 doc/guides/nics/mlx5.rst               |  13 +-
 doc/guides/rel_notes/release_21_05.rst |   1 +
 drivers/net/mlx5/mlx5.c                |   8 +-
 drivers/net/mlx5/mlx5.h                |   9 +-
 drivers/net/mlx5/mlx5_defs.h           |   2 +-
 drivers/net/mlx5/mlx5_flow.c           |   6 +
 drivers/net/mlx5/mlx5_flow.h           |   3 +-
 drivers/net/mlx5/mlx5_flow_dv.c        | 369 ++++++++++++++++++++++-----------
 drivers/net/mlx5/mlx5_flow_verbs.c     |   2 +-
 10 files changed, 360 insertions(+), 205 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 1/5] net/mlx5: support flow count action handle
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
@ 2021-04-26 12:42 ` Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query Michael Baum
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko

Existing API supports counter action to count traffic of a single flow.
The user can share the count action among different flows using the
shared flag and the same counter ID in the count action configuration.

Recent patch [1] introduced the indirect action API.
Using this API, an action can be created as indirect, unattached to any
flow rule.
Multiple flows can then be created using the same indirect action.
The new API also supports query operation of an indirect action.

The new API is more efficient because the driver gets it's own handler
for the count action instead of managing a mapping between the user ID
to the driver handle.

Support create, query and destroy indirect action operations for flow
count action.

Application will use the indirect action query operation to query this
count action.

In the meantime the old sharing mechanism (with the sharing flag)
continues to be supported, and the user can choose the way he wants to
share the counter.
The new indirect action API is only supported in DevX, so sharing
counter action in Verbs can only be done through the old mechanism.

[1] https://mails.dpdk.org/archives/dev/2020-July/174110.html

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/nics/mlx5.rst               |  13 +-
 doc/guides/rel_notes/release_21_05.rst |   1 +
 drivers/net/mlx5/mlx5.h                |   7 +-
 drivers/net/mlx5/mlx5_defs.h           |   2 +-
 drivers/net/mlx5/mlx5_flow.c           |   6 +
 drivers/net/mlx5/mlx5_flow.h           |   3 +-
 drivers/net/mlx5/mlx5_flow_dv.c        | 222 ++++++++++++++++++++++-----------
 drivers/net/mlx5/mlx5_flow_verbs.c     |   2 +-
 8 files changed, 175 insertions(+), 81 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 2bb4f18..b2c357e 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1692,10 +1692,15 @@ Supported hardware offloads
    |                       | |               | | rdma-core 33  |
    |                       | |               | | ConnectX-5    |
    +-----------------------+-----------------+-----------------+
-   | Age                   | |  DPDK 20.11   | | DPDK 20.11    |
-   |                       | |  OFED 5.2     | | OFED 5.2      |
-   |                       | |  rdma-core 32 | | rdma-core 32  |
-   |                       | |  ConnectX-6 Dx| | ConnectX-6 Dx |
+   | Age                   | | DPDK 20.11    | | DPDK 20.11    |
+   |                       | | OFED 5.2      | | OFED 5.2      |
+   |                       | | rdma-core 32  | | rdma-core 32  |
+   |                       | | ConnectX-6 Dx | | ConnectX-6 Dx |
+   +-----------------------+-----------------+-----------------+
+   | Count                 | | DPDK 21.05    | | DPDK 21.05    |
+   |                       | | OFED 4.6      | | OFED 4.6      |
+   |                       | | rdma-core 24  | | rdma-core 23  |
+   |                       | | ConnectX-5    | | ConnectX-5    |
    +-----------------------+-----------------+-----------------+
 
 Notes for metadata
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index b3224dc..740a729 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -161,6 +161,7 @@ New Features
   Updated the Mellanox mlx5 driver with new features and improvements, including:
 
   * Added support for VXLAN and NVGRE encap as sample actions.
+  * Added support for flow COUNT action handle.
   * Support push VLAN on ingress traffic and pop VLAN on egress traffic in E-Switch mode.
   * Added support for ASO (Advanced Steering Operation) meter.
 
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 378b68e..626abb4 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -291,7 +291,7 @@ struct mlx5_drop {
 #define MLX5_MAX_PENDING_QUERIES 4
 #define MLX5_CNT_CONTAINER_RESIZE 64
 #define MLX5_CNT_SHARED_OFFSET 0x80000000
-#define IS_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
+#define IS_LEGACY_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
 #define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
 			   MLX5_CNT_BATCH_OFFSET)
 #define MLX5_CNT_SIZE (sizeof(struct mlx5_flow_counter))
@@ -353,7 +353,10 @@ struct flow_counter_stats {
 
 /* Shared counters information for counters. */
 struct mlx5_flow_counter_shared {
-	uint32_t id; /**< User counter ID. */
+	union {
+		uint32_t refcnt; /* Only for shared action management. */
+		uint32_t id; /* User counter ID for legacy sharing. */
+	};
 };
 
 /* Shared counter configuration. */
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 6e9c4b9..906aa43 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -193,7 +193,7 @@
 #define MLX5_HAIRPIN_JUMBO_LOG_SIZE (14 + 2)
 
 /* Maximum number of indirect actions supported by rte_flow */
-#define MLX5_MAX_INDIRECT_ACTIONS 2
+#define MLX5_MAX_INDIRECT_ACTIONS 3
 
 /*
  * Linux definition of static_assert is found in /usr/include/assert.h.
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 34a4bb1..4f8591e 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3540,6 +3540,12 @@ struct mlx5_translated_action_handle {
 			translated[handle->index].conf =
 				&shared_rss->origin;
 			break;
+		case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+			translated[handle->index].type =
+						(enum rte_flow_action_type)
+						MLX5_RTE_FLOW_ACTION_TYPE_COUNT;
+			translated[handle->index].conf = (void *)(uintptr_t)idx;
+			break;
 		case MLX5_INDIRECT_ACTION_TYPE_AGE:
 			if (priv->sh->flow_hit_aso_en) {
 				translated[handle->index].type =
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 56908ae..432fd25 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -36,8 +36,8 @@ enum mlx5_rte_flow_action_type {
 	MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS,
 	MLX5_RTE_FLOW_ACTION_TYPE_TUNNEL_SET,
 	MLX5_RTE_FLOW_ACTION_TYPE_AGE,
-	MLX5_RTE_FLOW_ACTION_TYPE_JUMP,
 	MLX5_RTE_FLOW_ACTION_TYPE_COUNT,
+	MLX5_RTE_FLOW_ACTION_TYPE_JUMP,
 };
 
 #define MLX5_INDIRECT_ACTION_TYPE_OFFSET 30
@@ -45,6 +45,7 @@ enum mlx5_rte_flow_action_type {
 enum {
 	MLX5_INDIRECT_ACTION_TYPE_RSS,
 	MLX5_INDIRECT_ACTION_TYPE_AGE,
+	MLX5_INDIRECT_ACTION_TYPE_COUNT,
 };
 
 /* Matches on selected register. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index b07c135..7174e9b 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3147,12 +3147,32 @@ struct field_modify_info modify_tcp[] = {
 }
 
 /**
+ * Check if action counter is shared by either old or new mechanism.
+ *
+ * @param[in] action
+ *   Pointer to the action structure.
+ *
+ * @return
+ *   True when counter is shared, false otherwise.
+ */
+static inline bool
+is_shared_action_count(const struct rte_flow_action *action)
+{
+	const struct rte_flow_action_count *count =
+			(const struct rte_flow_action_count *)action->conf;
+
+	if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
+		return true;
+	return !!(count && count->shared);
+}
+
+/**
  * Validate count action.
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
- * @param[in] action
- *   Pointer to the action structure.
+ * @param[in] shared
+ *   Indicator if action is shared.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[out] error
@@ -3162,13 +3182,11 @@ struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_count(struct rte_eth_dev *dev,
-			      const struct rte_flow_action *action,
+flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
 			      uint64_t action_flags,
 			      struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_action_count *count;
 
 	if (!priv->config.devx)
 		goto notsup_err;
@@ -3176,8 +3194,7 @@ struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "duplicate count actions set");
-	count = (const struct rte_flow_action_count *)action->conf;
-	if (count && count->shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
+	if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
 	    !priv->sh->flow_hit_aso_en)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -5254,7 +5271,7 @@ struct mlx5_hlist_entry *
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = flow_dv_validate_action_count
-				(dev, act,
+				(dev, is_shared_action_count(act),
 				 *action_flags | sub_action_flags,
 				 error);
 			if (ret < 0)
@@ -5424,7 +5441,7 @@ struct mlx5_hlist_entry *
  * @param[in] idx
  *   mlx5 flow counter index in the container.
  * @param[out] ppool
- *   mlx5 flow counter pool in the container,
+ *   mlx5 flow counter pool in the container.
  *
  * @return
  *   Pointer to the counter, NULL otherwise.
@@ -5554,7 +5571,7 @@ struct mlx5_hlist_entry *
  *
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
- * @param[in] cnt
+ * @param[in] counter
  *   Index to the flow counter.
  * @param[out] pkts
  *   The statistics value of packets.
@@ -5795,6 +5812,13 @@ struct mlx5_hlist_entry *
 	if (!fallback && !priv->sh->cmng.query_thread_on)
 		/* Start the asynchronous batch query by the host thread. */
 		mlx5_set_query_alarm(priv->sh);
+	/*
+	 * When the count action isn't shared (by ID), shared_info field is
+	 * used for indirect action API's refcnt.
+	 * When the counter action is not shared neither by ID nor by indirect
+	 * action API, shared info must be 1.
+	 */
+	cnt_free->shared_info.refcnt = 1;
 	return cnt_idx;
 err:
 	if (cnt_free) {
@@ -5941,9 +5965,26 @@ struct mlx5_hlist_entry *
 		return;
 	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	MLX5_ASSERT(pool);
-	if (IS_SHARED_CNT(counter) &&
+	/*
+	 * If the counter action is shared by ID, the l3t_clear_entry function
+	 * reduces its references counter. If after the reduction the action is
+	 * still referenced, the function returns here and does not release it.
+	 */
+	if (IS_LEGACY_SHARED_CNT(counter) &&
 	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
 		return;
+	/*
+	 * If the counter action is shared by indirect action API, the atomic
+	 * function reduces its references counter. If after the reduction the
+	 * action is still referenced, the function returns here and does not
+	 * release it.
+	 * When the counter action is not shared neither by ID nor by indirect
+	 * action API, shared info is 1 before the reduction, so this condition
+	 * is failed and function doesn't return here.
+	 */
+	if (!IS_LEGACY_SHARED_CNT(counter) &&
+	    __atomic_sub_fetch(&cnt->shared_info.refcnt, 1, __ATOMIC_RELAXED))
+		return;
 	if (pool->is_aged)
 		flow_dv_counter_remove_from_age(dev, counter, cnt);
 	cnt->pool = pool;
@@ -5955,7 +5996,6 @@ struct mlx5_hlist_entry *
 	 * container counter list. The list changes while query starts. In
 	 * this case, lock will not be needed as query callback and release
 	 * function both operate with the different list.
-	 *
 	 */
 	if (!priv->sh->cmng.counter_fallback) {
 		rte_spinlock_lock(&pool->csl);
@@ -6274,7 +6314,6 @@ struct mlx5_hlist_entry *
 	const struct rte_flow_action_raw_encap *encap;
 	const struct rte_flow_action_rss *rss = NULL;
 	const struct rte_flow_action_rss *sample_rss = NULL;
-	const struct rte_flow_action_count *count = NULL;
 	const struct rte_flow_action_count *sample_count = NULL;
 	const struct rte_flow_item_tcp nic_tcp_mask = {
 		.hdr = {
@@ -6653,6 +6692,7 @@ struct mlx5_hlist_entry *
 	}
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		int type = actions->type;
+		bool shared_count = false;
 
 		if (!mlx5_flow_os_action_supported(type))
 			return rte_flow_error_set(error, ENOTSUP,
@@ -6802,13 +6842,14 @@ struct mlx5_hlist_entry *
 			action_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
 			++actions_n;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
 		case RTE_FLOW_ACTION_TYPE_COUNT:
-			ret = flow_dv_validate_action_count(dev, actions,
+			shared_count = is_shared_action_count(actions);
+			ret = flow_dv_validate_action_count(dev, shared_count,
 							    action_flags,
 							    error);
 			if (ret < 0)
 				return ret;
-			count = actions->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			++actions_n;
 			break;
@@ -7116,7 +7157,7 @@ struct mlx5_hlist_entry *
 			 * mutual exclusion with share counter actions.
 			 */
 			if (!priv->sh->flow_hit_aso_en) {
-				if (count && count->shared)
+				if (shared_count)
 					return rte_flow_error_set
 						(error, EINVAL,
 						RTE_FLOW_ERROR_TYPE_ACTION,
@@ -9932,6 +9973,8 @@ struct mlx5_hlist_entry *
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
+ * @param[in] dev_flow
+ *   Pointer to the mlx5_flow.
  * @param[out] count
  *   Pointer to the counter action configuration.
  * @param[in] age
@@ -9955,7 +9998,7 @@ struct mlx5_hlist_entry *
 		counter = flow_dv_counter_alloc(dev, !!age);
 	if (!counter || age == NULL)
 		return counter;
-	age_param  = flow_dv_counter_idx_get_age(dev, counter);
+	age_param = flow_dv_counter_idx_get_age(dev, counter);
 	age_param->context = age->context ? age->context :
 		(void *)(uintptr_t)(dev_flow->flow_idx);
 	age_param->timeout = age->timeout;
@@ -11266,12 +11309,12 @@ struct mlx5_cache_entry *
 		const uint8_t *rss_key;
 		struct mlx5_flow_tbl_resource *tbl;
 		struct mlx5_aso_age_action *age_act;
+		struct mlx5_flow_counter *cnt_act;
 		uint32_t port_id = 0;
 		struct mlx5_flow_dv_port_id_action_resource port_id_resource;
 		int action_type = actions->type;
 		const struct rte_flow_action *found_action = NULL;
 		uint32_t jump_group = 0;
-		struct mlx5_flow_counter *cnt;
 
 		if (!mlx5_flow_os_action_supported(action_type))
 			return rte_flow_error_set(error, ENOTSUP,
@@ -11412,6 +11455,15 @@ struct mlx5_cache_entry *
 			dev_flow->dv.actions[actions_n++] = age_act->dr_action;
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
+			flow->counter = (uint32_t)(uintptr_t)(action->conf);
+			cnt_act = flow_dv_counter_get_by_idx(dev, flow->counter,
+							     NULL);
+			__atomic_fetch_add(&cnt_act->shared_info.refcnt, 1,
+					   __ATOMIC_RELAXED);
+			/* Save information first, will apply later. */
+			action_flags |= MLX5_FLOW_ACTION_COUNT;
+			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
 			if (priv->sh->flow_hit_aso_en && attr->group) {
 				/*
@@ -11452,12 +11504,6 @@ struct mlx5_cache_entry *
 				age = action->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
-		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
-			cnt = flow_dv_counter_get_by_idx(dev,
-				(uint32_t)(uintptr_t)action->conf, NULL);
-			MLX5_ASSERT(cnt != NULL);
-			dev_flow->dv.actions[actions_n++] = cnt->action;
-			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
 			dev_flow->dv.actions[actions_n++] =
 						priv->sh->pop_vlan_action;
@@ -13329,6 +13375,11 @@ struct mlx5_cache_entry *
 							 (void *)(uintptr_t)idx;
 		}
 		break;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		ret = flow_dv_translate_create_counter(dev, NULL, NULL, NULL);
+		idx = (MLX5_INDIRECT_ACTION_TYPE_COUNT <<
+		       MLX5_INDIRECT_ACTION_TYPE_OFFSET) | ret;
+		break;
 	default:
 		rte_flow_error_set(err, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "action type not supported");
@@ -13362,11 +13413,25 @@ struct mlx5_cache_entry *
 	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
 	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
+	struct mlx5_flow_counter *cnt;
+	uint32_t no_flow_refcnt = 1;
 	int ret;
 
 	switch (type) {
 	case MLX5_INDIRECT_ACTION_TYPE_RSS:
 		return __flow_dv_action_rss_release(dev, idx, error);
+	case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+		cnt = flow_dv_counter_get_by_idx(dev, idx, NULL);
+		if (!__atomic_compare_exchange_n(&cnt->shared_info.refcnt,
+						 &no_flow_refcnt, 1, false,
+						 __ATOMIC_ACQUIRE,
+						 __ATOMIC_RELAXED))
+			return rte_flow_error_set(error, EBUSY,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "Indirect count action has references");
+		flow_dv_counter_free(dev, idx);
+		return 0;
 	case MLX5_INDIRECT_ACTION_TYPE_AGE:
 		ret = flow_dv_aso_age_release(dev, idx);
 		if (ret)
@@ -13494,37 +13559,6 @@ struct mlx5_cache_entry *
 	}
 }
 
-static int
-flow_dv_action_query(struct rte_eth_dev *dev,
-		     const struct rte_flow_action_handle *handle, void *data,
-		     struct rte_flow_error *error)
-{
-	struct mlx5_age_param *age_param;
-	struct rte_flow_query_age *resp;
-	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
-	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
-	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
-
-	switch (type) {
-	case MLX5_INDIRECT_ACTION_TYPE_AGE:
-		age_param = &flow_aso_age_get_by_idx(dev, idx)->age_params;
-		resp = data;
-		resp->aged = __atomic_load_n(&age_param->state,
-					      __ATOMIC_RELAXED) == AGE_TMOUT ?
-									  1 : 0;
-		resp->sec_since_last_hit_valid = !resp->aged;
-		if (resp->sec_since_last_hit_valid)
-			resp->sec_since_last_hit = __atomic_load_n
-			     (&age_param->sec_since_last_hit, __ATOMIC_RELAXED);
-		return 0;
-	default:
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ACTION,
-					  NULL,
-					  "action type query not supported");
-	}
-}
-
 /**
  * Destroy the meter sub policy table rules.
  * Lock free, (mutex should be acquired by caller).
@@ -14076,14 +14110,14 @@ struct mlx5_cache_entry *
 }
 
 /**
- * Query a dv flow  rule for its statistics via devx.
+ * Query a DV flow rule for its statistics via DevX.
  *
  * @param[in] dev
  *   Pointer to Ethernet device.
- * @param[in] flow
- *   Pointer to the sub flow.
+ * @param[in] cnt_idx
+ *   Index to the flow counter.
  * @param[out] data
- *   data retrieved by the query.
+ *   Data retrieved by the query.
  * @param[out] error
  *   Perform verbose error reporting if not NULL.
  *
@@ -14091,8 +14125,8 @@ struct mlx5_cache_entry *
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
-		    void *data, struct rte_flow_error *error)
+flow_dv_query_count(struct rte_eth_dev *dev, uint32_t cnt_idx, void *data,
+		    struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct rte_flow_query_count *qc = data;
@@ -14102,19 +14136,16 @@ struct mlx5_cache_entry *
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL,
 					  "counters are not supported");
-	if (flow->counter) {
+	if (cnt_idx) {
 		uint64_t pkts, bytes;
 		struct mlx5_flow_counter *cnt;
-
-		cnt = flow_dv_counter_get_by_idx(dev, flow->counter,
-						 NULL);
-		int err = _flow_dv_query_count(dev, flow->counter, &pkts,
-					       &bytes);
+		int err = _flow_dv_query_count(dev, cnt_idx, &pkts, &bytes);
 
 		if (err)
 			return rte_flow_error_set(error, -err,
 					RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					NULL, "cannot read counters");
+		cnt = flow_dv_counter_get_by_idx(dev, cnt_idx, NULL);
 		qc->hits_set = 1;
 		qc->bytes_set = 1;
 		qc->hits = pkts - cnt->hits;
@@ -14131,6 +14162,38 @@ struct mlx5_cache_entry *
 				  "counters are not available");
 }
 
+static int
+flow_dv_action_query(struct rte_eth_dev *dev,
+		     const struct rte_flow_action_handle *handle, void *data,
+		     struct rte_flow_error *error)
+{
+	struct mlx5_age_param *age_param;
+	struct rte_flow_query_age *resp;
+	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
+	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
+	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
+
+	switch (type) {
+	case MLX5_INDIRECT_ACTION_TYPE_AGE:
+		age_param = &flow_aso_age_get_by_idx(dev, idx)->age_params;
+		resp = data;
+		resp->aged = __atomic_load_n(&age_param->state,
+					      __ATOMIC_RELAXED) == AGE_TMOUT ?
+									  1 : 0;
+		resp->sec_since_last_hit_valid = !resp->aged;
+		if (resp->sec_since_last_hit_valid)
+			resp->sec_since_last_hit = __atomic_load_n
+			     (&age_param->sec_since_last_hit, __ATOMIC_RELAXED);
+		return 0;
+	case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+		return flow_dv_query_count(dev, idx, data, error);
+	default:
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "action type query not supported");
+	}
+}
+
 /**
  * Query a flow rule AGE action for aging information.
  *
@@ -14200,7 +14263,8 @@ struct mlx5_cache_entry *
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
-			ret = flow_dv_query_count(dev, flow, data, error);
+			ret = flow_dv_query_count(dev, flow->counter, data,
+						  error);
 			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
 			ret = flow_dv_query_age(dev, flow, data, error);
@@ -15242,7 +15306,7 @@ struct mlx5_cache_entry *
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] conf
- *   Shared action configuration.
+ *   Indirect action configuration.
  * @param[in] action
  *   The indirect action object to validate.
  * @param[out] error
@@ -15269,22 +15333,36 @@ struct mlx5_cache_entry *
 		 * sufficient, it is set to devx_obj_ops.
 		 * Otherwise, it is set to ibv_obj_ops.
 		 * ibv_obj_ops doesn't support ind_table_modify operation.
-		 * In this case the shared RSS action can't be used.
+		 * In this case the indirect RSS action can't be used.
 		 */
 		if (priv->obj_ops.ind_table_modify == NULL)
 			return rte_flow_error_set
 					(err, ENOTSUP,
 					 RTE_FLOW_ERROR_TYPE_ACTION,
 					 NULL,
-					 "shared RSS action not supported");
+					 "Indirect RSS action not supported");
 		return mlx5_validate_action_rss(dev, action, err);
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		if (!priv->sh->aso_age_mng)
 			return rte_flow_error_set(err, ENOTSUP,
 						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 						NULL,
-					     "shared age action not supported");
+						"Indirect age action not supported");
 		return flow_dv_validate_action_age(0, action, dev, err);
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		/*
+		 * There are two mechanisms to share the action count.
+		 * The old mechanism uses the shared field to share, while the
+		 * new mechanism uses the indirect action API.
+		 * This validation comes to make sure that the two mechanisms
+		 * are not combined.
+		 */
+		if (is_shared_action_count(action))
+			return rte_flow_error_set(err, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "Mix shared and indirect counter is not supported");
+		return flow_dv_validate_action_count(dev, true, 0, err);
 	default:
 		return rte_flow_error_set(err, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 0fdafbb..fe96733 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -357,7 +357,7 @@ struct ibv_spec_header {
 	struct mlx5_flow_counter *cnt;
 
 	cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
-	if (IS_SHARED_CNT(counter) &&
+	if (IS_LEGACY_SHARED_CNT(counter) &&
 	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
 		return;
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 1/5] net/mlx5: support flow count action handle Michael Baum
@ 2021-04-26 12:42 ` Michael Baum
  2021-04-29 13:46   ` Ori Kam
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter " Michael Baum
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko

The port_action_handle_query function supports query operation for
indirect RSS action.

No driver currently supports this operation, and this support is
unnecessary.

Remove it.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/config.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e189062..a9805cc 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1626,7 +1626,6 @@ struct rte_flow_action_handle *
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
-	case RTE_FLOW_ACTION_TYPE_RSS:
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		data = &default_data;
 		break;
@@ -1638,12 +1637,6 @@ struct rte_flow_action_handle *
 	if (rte_flow_action_handle_query(port_id, pia->handle, data, &error))
 		ret = port_flow_complain(&error);
 	switch (pia->type) {
-	case RTE_FLOW_ACTION_TYPE_RSS:
-		if (!ret)
-			printf("Shared RSS action:\n\trefs:%u\n",
-			       *((uint32_t *)data));
-		data = NULL;
-		break;
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		if (!ret) {
 			struct rte_flow_query_age *resp = data;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter action query
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 1/5] net/mlx5: support flow count action handle Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query Michael Baum
@ 2021-04-26 12:42 ` Michael Baum
  2021-04-29 13:45   ` Ori Kam
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 4/5] net/mlx5: fix flow age event triggering Michael Baum
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko

Counter action query was implemented as part of flow query, but was not
implemented as part of indirect action query.

This patch adds the required implementation.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/config.c | 145 +++++++++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 68 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a9805cc..bedbfcb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1618,90 +1618,99 @@ struct rte_flow_action_handle *
 {
 	struct rte_flow_error error;
 	struct port_indirect_action *pia;
-	uint64_t default_data;
-	void *data = NULL;
-	int ret = 0;
+	union {
+		struct rte_flow_query_count count;
+		struct rte_flow_query_age age;
+		struct rte_flow_action_conntrack ct;
+	} query;
 
 	pia = action_get_by_id(port_id, id);
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
 	case RTE_FLOW_ACTION_TYPE_AGE:
-		data = &default_data;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
 		break;
 	default:
-		printf("Indirect action %u (type: %d) on port %u doesn't"
-		       " support query\n", id, pia->type, port_id);
-		return -1;
+		printf("Indirect action %u (type: %d) on port %u doesn't support query\n",
+		       id, pia->type, port_id);
+		return -ENOTSUP;
 	}
-	if (rte_flow_action_handle_query(port_id, pia->handle, data, &error))
-		ret = port_flow_complain(&error);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x55, sizeof(error));
+	memset(&query, 0, sizeof(query));
+	if (rte_flow_action_handle_query(port_id, pia->handle, &query, &error))
+		return port_flow_complain(&error);
 	switch (pia->type) {
 	case RTE_FLOW_ACTION_TYPE_AGE:
-		if (!ret) {
-			struct rte_flow_query_age *resp = data;
-
-			printf("AGE:\n"
-			       " aged: %u\n"
-			       " sec_since_last_hit_valid: %u\n"
-			       " sec_since_last_hit: %" PRIu32 "\n",
-			       resp->aged,
-			       resp->sec_since_last_hit_valid,
-			       resp->sec_since_last_hit);
-		}
-		data = NULL;
+		printf("Indirect AGE action:\n"
+		       " aged: %u\n"
+		       " sec_since_last_hit_valid: %u\n"
+		       " sec_since_last_hit: %" PRIu32 "\n",
+		       query.age.aged,
+		       query.age.sec_since_last_hit_valid,
+		       query.age.sec_since_last_hit);
+		break;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		printf("Indirect COUNT action:\n"
+		       " hits_set: %u\n"
+		       " bytes_set: %u\n"
+		       " hits: %" PRIu64 "\n"
+		       " bytes: %" PRIu64 "\n",
+		       query.count.hits_set,
+		       query.count.bytes_set,
+		       query.count.hits,
+		       query.count.bytes);
 		break;
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
-		if (!ret) {
-			struct rte_flow_action_conntrack *ct = data;
-
-			printf("Conntrack Context:\n"
-			       "  Peer: %u, Flow dir: %s, Enable: %u\n"
-			       "  Live: %u, SACK: %u, CACK: %u\n"
-			       "  Packet dir: %s, Liberal: %u, State: %u\n"
-			       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
-			       "  Last Seq: %u, Last ACK: %u\n"
-			       "  Last Win: %u, Last End: %u\n",
-			       ct->peer_port,
-			       ct->is_original_dir ? "Original" : "Reply",
-			       ct->enable, ct->live_connection,
-			       ct->selective_ack, ct->challenge_ack_passed,
-			       ct->last_direction ? "Original" : "Reply",
-			       ct->liberal_mode, ct->state,
-			       ct->max_ack_window, ct->retransmission_limit,
-			       ct->last_index, ct->last_seq, ct->last_ack,
-			       ct->last_window, ct->last_end);
-			printf("  Original Dir:\n"
-			       "    scale: %u, fin: %u, ack seen: %u\n"
-			       " unacked data: %u\n    Sent end: %u,"
-			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
-			       ct->original_dir.scale,
-			       ct->original_dir.close_initiated,
-			       ct->original_dir.last_ack_seen,
-			       ct->original_dir.data_unacked,
-			       ct->original_dir.sent_end,
-			       ct->original_dir.reply_end,
-			       ct->original_dir.max_win,
-			       ct->original_dir.max_ack);
-			printf("  Reply Dir:\n"
-			       "    scale: %u, fin: %u, ack seen: %u\n"
-			       " unacked data: %u\n    Sent end: %u,"
-			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
-			       ct->reply_dir.scale,
-			       ct->reply_dir.close_initiated,
-			       ct->reply_dir.last_ack_seen,
-			       ct->reply_dir.data_unacked,
-			       ct->reply_dir.sent_end, ct->reply_dir.reply_end,
-			       ct->reply_dir.max_win, ct->reply_dir.max_ack);
-		}
-		data = NULL;
+		printf("Conntrack Context:\n"
+		       "  Peer: %u, Flow dir: %s, Enable: %u\n"
+		       "  Live: %u, SACK: %u, CACK: %u\n"
+		       "  Packet dir: %s, Liberal: %u, State: %u\n"
+		       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
+		       "  Last Seq: %u, Last ACK: %u\n"
+		       "  Last Win: %u, Last End: %u\n",
+		       query.ct.peer_port,
+		       query.ct.is_original_dir ? "Original" : "Reply",
+		       query.ct.enable, query.ct.live_connection,
+		       query.ct.selective_ack, query.ct.challenge_ack_passed,
+		       query.ct.last_direction ? "Original" : "Reply",
+		       query.ct.liberal_mode, query.ct.state,
+		       query.ct.max_ack_window, query.ct.retransmission_limit,
+		       query.ct.last_index, query.ct.last_seq,
+		       query.ct.last_ack, query.ct.last_window,
+		       query.ct.last_end);
+		printf("  Original Dir:\n"
+		       "    scale: %u, fin: %u, ack seen: %u\n"
+		       " unacked data: %u\n    Sent end: %u,"
+		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
+		       query.ct.original_dir.scale,
+		       query.ct.original_dir.close_initiated,
+		       query.ct.original_dir.last_ack_seen,
+		       query.ct.original_dir.data_unacked,
+		       query.ct.original_dir.sent_end,
+		       query.ct.original_dir.reply_end,
+		       query.ct.original_dir.max_win,
+		       query.ct.original_dir.max_ack);
+		printf("  Reply Dir:\n"
+		       "    scale: %u, fin: %u, ack seen: %u\n"
+		       " unacked data: %u\n    Sent end: %u,"
+		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
+		       query.ct.reply_dir.scale,
+		       query.ct.reply_dir.close_initiated,
+		       query.ct.reply_dir.last_ack_seen,
+		       query.ct.reply_dir.data_unacked,
+		       query.ct.reply_dir.sent_end,
+		       query.ct.reply_dir.reply_end,
+		       query.ct.reply_dir.max_win,
+		       query.ct.reply_dir.max_ack);
 		break;
 	default:
-		printf("Indirect action %u (type: %d) on port %u doesn't"
-		       " support query\n", id, pia->type, port_id);
-		ret = -1;
+		printf("Indirect action %u (type: %d) on port %u doesn't support query\n",
+		       id, pia->type, port_id);
+		break;
 	}
-	return ret;
+	return 0;
 }
 
 static struct port_flow_tunnel *
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 4/5] net/mlx5: fix flow age event triggering
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
                   ` (2 preceding siblings ...)
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter " Michael Baum
@ 2021-04-26 12:42 ` Michael Baum
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 5/5] net/mlx5: use aging by counter when counter is existed Michael Baum
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
  5 siblings, 0 replies; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev
  Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko, stable,
	David Bouyeure

A FLOW_AGE event should be invoked when a new aged-out flow is detected
by the PMD after the last user get-aged query calling.
The PMD manages 2 flags for this information and check them in order to
decide if an event should be invoked:
MLX5_AGE_EVENT_NEW - a new aged-out flow was detected. after the last
check.
MLX5_AGE_TRIGGER - get-aged query was called after the last aged-out
flow.
The 2 flags were unset after the event invoking.

When the user calls get-aged query from the event callback, the TRIGGER
flag was set inside the user callback and unset directly after the
callback what may stop the event invoking forever.

Unset the TRIGGER flag before the event invoking in order to allow set
it by the user callback.

Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
Cc: stable@dpdk.org

Reported-by: David Bouyeure <david.bouyeure@fraudbuster.mobi>
Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c | 8 +++++---
 drivers/net/mlx5/mlx5.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 19ffa16..10d44c7 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -662,11 +662,13 @@ static LIST_HEAD(, mlx5_dev_ctx_shared) mlx5_dev_ctx_list =
 		age_info = &sh->port[i].age_info;
 		if (!MLX5_AGE_GET(age_info, MLX5_AGE_EVENT_NEW))
 			continue;
-		if (MLX5_AGE_GET(age_info, MLX5_AGE_TRIGGER))
+		MLX5_AGE_UNSET(age_info, MLX5_AGE_EVENT_NEW);
+		if (MLX5_AGE_GET(age_info, MLX5_AGE_TRIGGER)) {
+			MLX5_AGE_UNSET(age_info, MLX5_AGE_TRIGGER);
 			rte_eth_dev_callback_process
 				(&rte_eth_devices[sh->port[i].devx_ih_port_id],
 				RTE_ETH_EVENT_FLOW_AGED, NULL);
-		age_info->flags = 0;
+		}
 	}
 }
 
@@ -675,7 +677,7 @@ static LIST_HEAD(, mlx5_dev_ctx_shared) mlx5_dev_ctx_list =
  *
  * @param[in] sh
  *   Pointer to mlx5_dev_ctx_shared object.
- * @param[in] sh
+ * @param[in] config
  *   Pointer to user dev config.
  */
 static void
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 626abb4..dfa029c 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -563,6 +563,8 @@ struct mlx5_geneve_tlv_option_resource {
 #define MLX5_AGE_TRIGGER		2
 #define MLX5_AGE_SET(age_info, BIT) \
 	((age_info)->flags |= (1 << (BIT)))
+#define MLX5_AGE_UNSET(age_info, BIT) \
+	((age_info)->flags &= ~(1 << (BIT)))
 #define MLX5_AGE_GET(age_info, BIT) \
 	((age_info)->flags & (1 << (BIT)))
 #define GET_PORT_AGE_INFO(priv) \
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 5/5] net/mlx5: use aging by counter when counter is existed
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
                   ` (3 preceding siblings ...)
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 4/5] net/mlx5: fix flow age event triggering Michael Baum
@ 2021-04-26 12:42 ` Michael Baum
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
  5 siblings, 0 replies; 18+ messages in thread
From: Michael Baum @ 2021-04-26 12:42 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko

The driver support 2 mechanisms in order to support AGE action:
1. Aging by counter - HW counter will be configured to the flow traffic,
the driver polls the counter values efficiently to detect flow timeout.
2. Aging by ASO flow hit bit - HW ASO flow-hit bit is allocated for the
flow, the driver polls the bit efficiently to detect flow timeout.

ASO bit is only single bit resource while counter is 16 bytes, hence, it
is better to use ASO instead of counter for aging.

When a non-shared COUNT action is also configured to the flow, the
driver can use the same counter also for AGE action and no need to
create more ASO action for it.

The current code always uses ASO when it is supported in the device,
change it to reuse the non-shared counter if it exists in the flow.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 149 +++++++++++++++++++++++++++-------------
 1 file changed, 102 insertions(+), 47 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7174e9b..9cd1c45 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7143,6 +7143,12 @@ struct mlx5_hlist_entry *
 						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 									   NULL,
 			  "Shared ASO age action is not supported for group 0");
+			if (action_flags & MLX5_FLOW_ACTION_AGE)
+				return rte_flow_error_set
+						  (error, EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   NULL,
+						   "duplicate age actions set");
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			++actions_n;
 			break;
@@ -11163,6 +11169,47 @@ struct mlx5_cache_entry *
 }
 
 /**
+ * Prepares DV flow counter with aging configuration.
+ * Gets it by index when exists, creates a new one when doesn't.
+ *
+ * @param[in] dev
+ *   Pointer to rte_eth_dev structure.
+ * @param[in] dev_flow
+ *   Pointer to the mlx5_flow.
+ * @param[in, out] flow
+ *   Pointer to the sub flow.
+ * @param[in] count
+ *   Pointer to the counter action configuration.
+ * @param[in] age
+ *   Pointer to the aging action configuration.
+ * @param[out] error
+ *   Pointer to the error structure.
+ *
+ * @return
+ *   Pointer to the counter, NULL otherwise.
+ */
+static struct mlx5_flow_counter *
+flow_dv_prepare_counter(struct rte_eth_dev *dev,
+			struct mlx5_flow *dev_flow,
+			struct rte_flow *flow,
+			const struct rte_flow_action_count *count,
+			const struct rte_flow_action_age *age,
+			struct rte_flow_error *error)
+{
+	if (!flow->counter) {
+		flow->counter = flow_dv_translate_create_counter(dev, dev_flow,
+								 count, age);
+		if (!flow->counter) {
+			rte_flow_error_set(error, rte_errno,
+					   RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					   "cannot create counter object.");
+			return NULL;
+		}
+	}
+	return flow_dv_counter_get_by_idx(dev, flow->counter, NULL);
+}
+
+/**
  * Fill the flow with DV spec, lock free
  * (mutex should be acquired by caller).
  *
@@ -11215,7 +11262,7 @@ struct mlx5_cache_entry *
 	} mhdr_dummy;
 	struct mlx5_flow_dv_modify_hdr_resource *mhdr_res = &mhdr_dummy.res;
 	const struct rte_flow_action_count *count = NULL;
-	const struct rte_flow_action_age *age = NULL;
+	const struct rte_flow_action_age *non_shared_age = NULL;
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	uint32_t tag_be;
 	union mlx5_flow_tbl_key tbl_key;
@@ -11230,6 +11277,7 @@ struct mlx5_cache_entry *
 	const struct rte_flow_action_sample *sample = NULL;
 	struct mlx5_flow_sub_actions_list *sample_act;
 	uint32_t sample_act_pos = UINT32_MAX;
+	uint32_t age_act_pos = UINT32_MAX;
 	uint32_t num_of_dest = 0;
 	int tmp_actions_n = 0;
 	uint32_t table;
@@ -11452,7 +11500,12 @@ struct mlx5_cache_entry *
 			age_act = flow_aso_age_get_by_idx(dev, flow->age);
 			__atomic_fetch_add(&age_act->refcnt, 1,
 					   __ATOMIC_RELAXED);
-			dev_flow->dv.actions[actions_n++] = age_act->dr_action;
+			age_act_pos = actions_n++;
+			action_flags |= MLX5_FLOW_ACTION_AGE;
+			break;
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			non_shared_age = action->conf;
+			age_act_pos = actions_n++;
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			break;
 		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
@@ -11464,31 +11517,6 @@ struct mlx5_cache_entry *
 			/* Save information first, will apply later. */
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
-		case RTE_FLOW_ACTION_TYPE_AGE:
-			if (priv->sh->flow_hit_aso_en && attr->group) {
-				/*
-				 * Create one shared age action, to be used
-				 * by all sub-flows.
-				 */
-				if (!flow->age) {
-					flow->age =
-						flow_dv_translate_create_aso_age
-							(dev, action->conf,
-							 error);
-					if (!flow->age)
-						return rte_flow_error_set
-						(error, rte_errno,
-						 RTE_FLOW_ERROR_TYPE_ACTION,
-						 NULL,
-						 "can't create ASO age action");
-				}
-				dev_flow->dv.actions[actions_n++] =
-					  (flow_aso_age_get_by_idx
-						(dev, flow->age))->dr_action;
-				action_flags |= MLX5_FLOW_ACTION_AGE;
-				break;
-			}
-			/* Fall-through */
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			if (!dev_conf->devx) {
 				return rte_flow_error_set
@@ -11498,10 +11526,7 @@ struct mlx5_cache_entry *
 					       "count action not supported");
 			}
 			/* Save information first, will apply later. */
-			if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT)
-				count = action->conf;
-			else
-				age = action->conf;
+			count = action->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
@@ -11801,27 +11826,57 @@ struct mlx5_cache_entry *
 				dev_flow->dv.actions[modify_action_position] =
 					handle->dvh.modify_hdr->action;
 			}
+			/*
+			 * Handle AGE and COUNT action by single HW counter
+			 * when they are not shared.
+			 */
+			if (action_flags & MLX5_FLOW_ACTION_AGE) {
+				if ((non_shared_age &&
+				     count && !count->shared) ||
+				    !(priv->sh->flow_hit_aso_en &&
+				      attr->group)) {
+					/* Creates age by counters. */
+					cnt_act = flow_dv_prepare_counter
+								(dev, dev_flow,
+								 flow, count,
+								 non_shared_age,
+								 error);
+					if (!cnt_act)
+						return -rte_errno;
+					dev_flow->dv.actions[age_act_pos] =
+								cnt_act->action;
+					break;
+				}
+				if (!flow->age && non_shared_age) {
+					flow->age =
+						flow_dv_translate_create_aso_age
+								(dev,
+								 non_shared_age,
+								 error);
+					if (!flow->age)
+						return rte_flow_error_set
+						    (error, rte_errno,
+						     RTE_FLOW_ERROR_TYPE_ACTION,
+						     NULL,
+						     "can't create ASO age action");
+				}
+				age_act = flow_aso_age_get_by_idx(dev,
+								  flow->age);
+				dev_flow->dv.actions[age_act_pos] =
+							     age_act->dr_action;
+			}
 			if (action_flags & MLX5_FLOW_ACTION_COUNT) {
 				/*
 				 * Create one count action, to be used
 				 * by all sub-flows.
 				 */
-				if (!flow->counter) {
-					flow->counter =
-						flow_dv_translate_create_counter
-							(dev, dev_flow, count,
-							 age);
-					if (!flow->counter)
-						return rte_flow_error_set
-						(error, rte_errno,
-						 RTE_FLOW_ERROR_TYPE_ACTION,
-						 NULL, "cannot create counter"
-						 " object.");
-				}
-				dev_flow->dv.actions[actions_n] =
-					  (flow_dv_counter_get_by_idx(dev,
-					  flow->counter, NULL))->action;
-				actions_n++;
+				cnt_act = flow_dv_prepare_counter(dev, dev_flow,
+								  flow, count,
+								  NULL, error);
+				if (!cnt_act)
+					return -rte_errno;
+				dev_flow->dv.actions[actions_n++] =
+								cnt_act->action;
 			}
 		default:
 			break;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action.
  2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
                   ` (4 preceding siblings ...)
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 5/5] net/mlx5: use aging by counter when counter is existed Michael Baum
@ 2021-04-29  9:55 ` Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle Viacheslav Ovsiienko
                     ` (5 more replies)
  5 siblings, 6 replies; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit

v1: http://patches.dpdk.org/project/dpdk/patch/20210426124250.42771-2-michaelba@nvidia.com/
V2: patch rebase 

Michael Baum (5):
  net/mlx5: support flow count action handle
  app/testpmd: remove indirect RSS action query
  app/testpmd: support indirect counter action query
  net/mlx5: fix flow age event triggering
  net/mlx5: use aging by counter when counter is existed

 app/test-pmd/config.c                  | 152 +++++-----
 doc/guides/nics/mlx5.rst               |  13 +-
 doc/guides/rel_notes/release_21_05.rst |   1 +
 drivers/net/mlx5/mlx5.c                |   8 +-
 drivers/net/mlx5/mlx5.h                |   9 +-
 drivers/net/mlx5/mlx5_defs.h           |   2 +-
 drivers/net/mlx5/mlx5_flow.c           |   6 +
 drivers/net/mlx5/mlx5_flow.h           |   3 +-
 drivers/net/mlx5/mlx5_flow_dv.c        | 369 +++++++++++++++++--------
 drivers/net/mlx5/mlx5_flow_verbs.c     |   2 +-
 10 files changed, 360 insertions(+), 205 deletions(-)

-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
@ 2021-04-29  9:55   ` Viacheslav Ovsiienko
  2021-04-30  8:34     ` Ferruh Yigit
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: remove indirect RSS action query Viacheslav Ovsiienko
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit, Michael Baum

From: Michael Baum <michaelba@nvidia.com>

Existing API supports counter action to count traffic of a single flow.
The user can share the count action among different flows using the
shared flag and the same counter ID in the count action configuration.

Recent patch [1] introduced the indirect action API.
Using this API, an action can be created as indirect, unattached to any
flow rule.
Multiple flows can then be created using the same indirect action.
The new API also supports query operation of an indirect action.

The new API is more efficient because the driver gets it's own handler
for the count action instead of managing a mapping between the user ID
to the driver handle.

Support create, query and destroy indirect action operations for flow
count action.

Application will use the indirect action query operation to query this
count action.

In the meantime the old sharing mechanism (with the sharing flag)
continues to be supported, and the user can choose the way he wants to
share the counter.
The new indirect action API is only supported in DevX, so sharing
counter action in Verbs can only be done through the old mechanism.

[1] https://mails.dpdk.org/archives/dev/2020-July/174110.html

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/nics/mlx5.rst               |  13 +-
 doc/guides/rel_notes/release_21_05.rst |   1 +
 drivers/net/mlx5/mlx5.h                |   7 +-
 drivers/net/mlx5/mlx5_defs.h           |   2 +-
 drivers/net/mlx5/mlx5_flow.c           |   6 +
 drivers/net/mlx5/mlx5_flow.h           |   3 +-
 drivers/net/mlx5/mlx5_flow_dv.c        | 222 +++++++++++++++++--------
 drivers/net/mlx5/mlx5_flow_verbs.c     |   2 +-
 8 files changed, 175 insertions(+), 81 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 2bb4f18a08..b2c357e016 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1692,10 +1692,15 @@ Supported hardware offloads
    |                       | |               | | rdma-core 33  |
    |                       | |               | | ConnectX-5    |
    +-----------------------+-----------------+-----------------+
-   | Age                   | |  DPDK 20.11   | | DPDK 20.11    |
-   |                       | |  OFED 5.2     | | OFED 5.2      |
-   |                       | |  rdma-core 32 | | rdma-core 32  |
-   |                       | |  ConnectX-6 Dx| | ConnectX-6 Dx |
+   | Age                   | | DPDK 20.11    | | DPDK 20.11    |
+   |                       | | OFED 5.2      | | OFED 5.2      |
+   |                       | | rdma-core 32  | | rdma-core 32  |
+   |                       | | ConnectX-6 Dx | | ConnectX-6 Dx |
+   +-----------------------+-----------------+-----------------+
+   | Count                 | | DPDK 21.05    | | DPDK 21.05    |
+   |                       | | OFED 4.6      | | OFED 4.6      |
+   |                       | | rdma-core 24  | | rdma-core 23  |
+   |                       | | ConnectX-5    | | ConnectX-5    |
    +-----------------------+-----------------+-----------------+
 
 Notes for metadata
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 133a0dbbf2..4628ae3ad9 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -161,6 +161,7 @@ New Features
   Updated the Mellanox mlx5 driver with new features and improvements, including:
 
   * Added support for VXLAN and NVGRE encap as sample actions.
+  * Added support for flow COUNT action handle.
   * Support push VLAN on ingress traffic and pop VLAN on egress traffic in E-Switch mode.
   * Added support for pre-defined meter policy API.
   * Added support for ASO (Advanced Steering Operation) meter.
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index d4aec717f0..efb80a4a49 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -291,7 +291,7 @@ struct mlx5_drop {
 #define MLX5_MAX_PENDING_QUERIES 4
 #define MLX5_CNT_CONTAINER_RESIZE 64
 #define MLX5_CNT_SHARED_OFFSET 0x80000000
-#define IS_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
+#define IS_LEGACY_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
 #define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
 			   MLX5_CNT_BATCH_OFFSET)
 #define MLX5_CNT_SIZE (sizeof(struct mlx5_flow_counter))
@@ -353,7 +353,10 @@ struct flow_counter_stats {
 
 /* Shared counters information for counters. */
 struct mlx5_flow_counter_shared {
-	uint32_t id; /**< User counter ID. */
+	union {
+		uint32_t refcnt; /* Only for shared action management. */
+		uint32_t id; /* User counter ID for legacy sharing. */
+	};
 };
 
 /* Shared counter configuration. */
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 6e9c4b9cdd..906aa43c5a 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -193,7 +193,7 @@
 #define MLX5_HAIRPIN_JUMBO_LOG_SIZE (14 + 2)
 
 /* Maximum number of indirect actions supported by rte_flow */
-#define MLX5_MAX_INDIRECT_ACTIONS 2
+#define MLX5_MAX_INDIRECT_ACTIONS 3
 
 /*
  * Linux definition of static_assert is found in /usr/include/assert.h.
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 15ed5ec7a2..1923abe411 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3540,6 +3540,12 @@ flow_action_handles_translate(struct rte_eth_dev *dev,
 			translated[handle->index].conf =
 				&shared_rss->origin;
 			break;
+		case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+			translated[handle->index].type =
+						(enum rte_flow_action_type)
+						MLX5_RTE_FLOW_ACTION_TYPE_COUNT;
+			translated[handle->index].conf = (void *)(uintptr_t)idx;
+			break;
 		case MLX5_INDIRECT_ACTION_TYPE_AGE:
 			if (priv->sh->flow_hit_aso_en) {
 				translated[handle->index].type =
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 56908ae08b..432fd25dd6 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -36,8 +36,8 @@ enum mlx5_rte_flow_action_type {
 	MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS,
 	MLX5_RTE_FLOW_ACTION_TYPE_TUNNEL_SET,
 	MLX5_RTE_FLOW_ACTION_TYPE_AGE,
-	MLX5_RTE_FLOW_ACTION_TYPE_JUMP,
 	MLX5_RTE_FLOW_ACTION_TYPE_COUNT,
+	MLX5_RTE_FLOW_ACTION_TYPE_JUMP,
 };
 
 #define MLX5_INDIRECT_ACTION_TYPE_OFFSET 30
@@ -45,6 +45,7 @@ enum mlx5_rte_flow_action_type {
 enum {
 	MLX5_INDIRECT_ACTION_TYPE_RSS,
 	MLX5_INDIRECT_ACTION_TYPE_AGE,
+	MLX5_INDIRECT_ACTION_TYPE_COUNT,
 };
 
 /* Matches on selected register. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d810466242..b74bac1083 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3146,13 +3146,33 @@ flow_dv_validate_action_set_tag(struct rte_eth_dev *dev,
 	return 0;
 }
 
+/**
+ * Check if action counter is shared by either old or new mechanism.
+ *
+ * @param[in] action
+ *   Pointer to the action structure.
+ *
+ * @return
+ *   True when counter is shared, false otherwise.
+ */
+static inline bool
+is_shared_action_count(const struct rte_flow_action *action)
+{
+	const struct rte_flow_action_count *count =
+			(const struct rte_flow_action_count *)action->conf;
+
+	if ((int)action->type == MLX5_RTE_FLOW_ACTION_TYPE_COUNT)
+		return true;
+	return !!(count && count->shared);
+}
+
 /**
  * Validate count action.
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
- * @param[in] action
- *   Pointer to the action structure.
+ * @param[in] shared
+ *   Indicator if action is shared.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[out] error
@@ -3162,13 +3182,11 @@ flow_dv_validate_action_set_tag(struct rte_eth_dev *dev,
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_count(struct rte_eth_dev *dev,
-			      const struct rte_flow_action *action,
+flow_dv_validate_action_count(struct rte_eth_dev *dev, bool shared,
 			      uint64_t action_flags,
 			      struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_action_count *count;
 
 	if (!priv->config.devx)
 		goto notsup_err;
@@ -3176,8 +3194,7 @@ flow_dv_validate_action_count(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "duplicate count actions set");
-	count = (const struct rte_flow_action_count *)action->conf;
-	if (count && count->shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
+	if (shared && (action_flags & MLX5_FLOW_ACTION_AGE) &&
 	    !priv->sh->flow_hit_aso_en)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -5254,7 +5271,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = flow_dv_validate_action_count
-				(dev, act,
+				(dev, is_shared_action_count(act),
 				 *action_flags | sub_action_flags,
 				 error);
 			if (ret < 0)
@@ -5424,7 +5441,7 @@ flow_dv_modify_hdr_resource_register
  * @param[in] idx
  *   mlx5 flow counter index in the container.
  * @param[out] ppool
- *   mlx5 flow counter pool in the container,
+ *   mlx5 flow counter pool in the container.
  *
  * @return
  *   Pointer to the counter, NULL otherwise.
@@ -5554,7 +5571,7 @@ flow_dv_container_resize(struct rte_eth_dev *dev)
  *
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
- * @param[in] cnt
+ * @param[in] counter
  *   Index to the flow counter.
  * @param[out] pkts
  *   The statistics value of packets.
@@ -5795,6 +5812,13 @@ flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t age)
 	if (!fallback && !priv->sh->cmng.query_thread_on)
 		/* Start the asynchronous batch query by the host thread. */
 		mlx5_set_query_alarm(priv->sh);
+	/*
+	 * When the count action isn't shared (by ID), shared_info field is
+	 * used for indirect action API's refcnt.
+	 * When the counter action is not shared neither by ID nor by indirect
+	 * action API, shared info must be 1.
+	 */
+	cnt_free->shared_info.refcnt = 1;
 	return cnt_idx;
 err:
 	if (cnt_free) {
@@ -5941,9 +5965,26 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
 		return;
 	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	MLX5_ASSERT(pool);
-	if (IS_SHARED_CNT(counter) &&
+	/*
+	 * If the counter action is shared by ID, the l3t_clear_entry function
+	 * reduces its references counter. If after the reduction the action is
+	 * still referenced, the function returns here and does not release it.
+	 */
+	if (IS_LEGACY_SHARED_CNT(counter) &&
 	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
 		return;
+	/*
+	 * If the counter action is shared by indirect action API, the atomic
+	 * function reduces its references counter. If after the reduction the
+	 * action is still referenced, the function returns here and does not
+	 * release it.
+	 * When the counter action is not shared neither by ID nor by indirect
+	 * action API, shared info is 1 before the reduction, so this condition
+	 * is failed and function doesn't return here.
+	 */
+	if (!IS_LEGACY_SHARED_CNT(counter) &&
+	    __atomic_sub_fetch(&cnt->shared_info.refcnt, 1, __ATOMIC_RELAXED))
+		return;
 	if (pool->is_aged)
 		flow_dv_counter_remove_from_age(dev, counter, cnt);
 	cnt->pool = pool;
@@ -5955,7 +5996,6 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
 	 * container counter list. The list changes while query starts. In
 	 * this case, lock will not be needed as query callback and release
 	 * function both operate with the different list.
-	 *
 	 */
 	if (!priv->sh->cmng.counter_fallback) {
 		rte_spinlock_lock(&pool->csl);
@@ -6274,7 +6314,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	const struct rte_flow_action_raw_encap *encap;
 	const struct rte_flow_action_rss *rss = NULL;
 	const struct rte_flow_action_rss *sample_rss = NULL;
-	const struct rte_flow_action_count *count = NULL;
 	const struct rte_flow_action_count *sample_count = NULL;
 	const struct rte_flow_item_tcp nic_tcp_mask = {
 		.hdr = {
@@ -6653,6 +6692,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	}
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		int type = actions->type;
+		bool shared_count = false;
 
 		if (!mlx5_flow_os_action_supported(type))
 			return rte_flow_error_set(error, ENOTSUP,
@@ -6802,13 +6842,14 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			action_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
 			++actions_n;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
 		case RTE_FLOW_ACTION_TYPE_COUNT:
-			ret = flow_dv_validate_action_count(dev, actions,
+			shared_count = is_shared_action_count(actions);
+			ret = flow_dv_validate_action_count(dev, shared_count,
 							    action_flags,
 							    error);
 			if (ret < 0)
 				return ret;
-			count = actions->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			++actions_n;
 			break;
@@ -7116,7 +7157,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 			 * mutual exclusion with share counter actions.
 			 */
 			if (!priv->sh->flow_hit_aso_en) {
-				if (count && count->shared)
+				if (shared_count)
 					return rte_flow_error_set
 						(error, EINVAL,
 						RTE_FLOW_ERROR_TYPE_ACTION,
@@ -9932,6 +9973,8 @@ flow_dv_translate_action_port_id(struct rte_eth_dev *dev,
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
+ * @param[in] dev_flow
+ *   Pointer to the mlx5_flow.
  * @param[out] count
  *   Pointer to the counter action configuration.
  * @param[in] age
@@ -9955,7 +9998,7 @@ flow_dv_translate_create_counter(struct rte_eth_dev *dev,
 		counter = flow_dv_counter_alloc(dev, !!age);
 	if (!counter || age == NULL)
 		return counter;
-	age_param  = flow_dv_counter_idx_get_age(dev, counter);
+	age_param = flow_dv_counter_idx_get_age(dev, counter);
 	age_param->context = age->context ? age->context :
 		(void *)(uintptr_t)(dev_flow->flow_idx);
 	age_param->timeout = age->timeout;
@@ -11266,12 +11309,12 @@ flow_dv_translate(struct rte_eth_dev *dev,
 		const uint8_t *rss_key;
 		struct mlx5_flow_tbl_resource *tbl;
 		struct mlx5_aso_age_action *age_act;
+		struct mlx5_flow_counter *cnt_act;
 		uint32_t port_id = 0;
 		struct mlx5_flow_dv_port_id_action_resource port_id_resource;
 		int action_type = actions->type;
 		const struct rte_flow_action *found_action = NULL;
 		uint32_t jump_group = 0;
-		struct mlx5_flow_counter *cnt;
 
 		if (!mlx5_flow_os_action_supported(action_type))
 			return rte_flow_error_set(error, ENOTSUP,
@@ -11412,6 +11455,15 @@ flow_dv_translate(struct rte_eth_dev *dev,
 			dev_flow->dv.actions[actions_n++] = age_act->dr_action;
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
+			flow->counter = (uint32_t)(uintptr_t)(action->conf);
+			cnt_act = flow_dv_counter_get_by_idx(dev, flow->counter,
+							     NULL);
+			__atomic_fetch_add(&cnt_act->shared_info.refcnt, 1,
+					   __ATOMIC_RELAXED);
+			/* Save information first, will apply later. */
+			action_flags |= MLX5_FLOW_ACTION_COUNT;
+			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
 			if (priv->sh->flow_hit_aso_en && attr->group) {
 				/*
@@ -11452,12 +11504,6 @@ flow_dv_translate(struct rte_eth_dev *dev,
 				age = action->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
-		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
-			cnt = flow_dv_counter_get_by_idx(dev,
-				(uint32_t)(uintptr_t)action->conf, NULL);
-			MLX5_ASSERT(cnt != NULL);
-			dev_flow->dv.actions[actions_n++] = cnt->action;
-			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
 			dev_flow->dv.actions[actions_n++] =
 						priv->sh->pop_vlan_action;
@@ -13329,6 +13375,11 @@ flow_dv_action_create(struct rte_eth_dev *dev,
 							 (void *)(uintptr_t)idx;
 		}
 		break;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		ret = flow_dv_translate_create_counter(dev, NULL, NULL, NULL);
+		idx = (MLX5_INDIRECT_ACTION_TYPE_COUNT <<
+		       MLX5_INDIRECT_ACTION_TYPE_OFFSET) | ret;
+		break;
 	default:
 		rte_flow_error_set(err, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "action type not supported");
@@ -13362,11 +13413,25 @@ flow_dv_action_destroy(struct rte_eth_dev *dev,
 	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
 	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
+	struct mlx5_flow_counter *cnt;
+	uint32_t no_flow_refcnt = 1;
 	int ret;
 
 	switch (type) {
 	case MLX5_INDIRECT_ACTION_TYPE_RSS:
 		return __flow_dv_action_rss_release(dev, idx, error);
+	case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+		cnt = flow_dv_counter_get_by_idx(dev, idx, NULL);
+		if (!__atomic_compare_exchange_n(&cnt->shared_info.refcnt,
+						 &no_flow_refcnt, 1, false,
+						 __ATOMIC_ACQUIRE,
+						 __ATOMIC_RELAXED))
+			return rte_flow_error_set(error, EBUSY,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "Indirect count action has references");
+		flow_dv_counter_free(dev, idx);
+		return 0;
 	case MLX5_INDIRECT_ACTION_TYPE_AGE:
 		ret = flow_dv_aso_age_release(dev, idx);
 		if (ret)
@@ -13494,37 +13559,6 @@ flow_dv_action_update(struct rte_eth_dev *dev,
 	}
 }
 
-static int
-flow_dv_action_query(struct rte_eth_dev *dev,
-		     const struct rte_flow_action_handle *handle, void *data,
-		     struct rte_flow_error *error)
-{
-	struct mlx5_age_param *age_param;
-	struct rte_flow_query_age *resp;
-	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
-	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
-	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
-
-	switch (type) {
-	case MLX5_INDIRECT_ACTION_TYPE_AGE:
-		age_param = &flow_aso_age_get_by_idx(dev, idx)->age_params;
-		resp = data;
-		resp->aged = __atomic_load_n(&age_param->state,
-					      __ATOMIC_RELAXED) == AGE_TMOUT ?
-									  1 : 0;
-		resp->sec_since_last_hit_valid = !resp->aged;
-		if (resp->sec_since_last_hit_valid)
-			resp->sec_since_last_hit = __atomic_load_n
-			     (&age_param->sec_since_last_hit, __ATOMIC_RELAXED);
-		return 0;
-	default:
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ACTION,
-					  NULL,
-					  "action type query not supported");
-	}
-}
-
 /**
  * Destroy the meter sub policy table rules.
  * Lock free, (mutex should be acquired by caller).
@@ -14076,14 +14110,14 @@ flow_dv_create_mtr_policy_acts(struct rte_eth_dev *dev,
 }
 
 /**
- * Query a dv flow  rule for its statistics via devx.
+ * Query a DV flow rule for its statistics via DevX.
  *
  * @param[in] dev
  *   Pointer to Ethernet device.
- * @param[in] flow
- *   Pointer to the sub flow.
+ * @param[in] cnt_idx
+ *   Index to the flow counter.
  * @param[out] data
- *   data retrieved by the query.
+ *   Data retrieved by the query.
  * @param[out] error
  *   Perform verbose error reporting if not NULL.
  *
@@ -14091,8 +14125,8 @@ flow_dv_create_mtr_policy_acts(struct rte_eth_dev *dev,
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
-		    void *data, struct rte_flow_error *error)
+flow_dv_query_count(struct rte_eth_dev *dev, uint32_t cnt_idx, void *data,
+		    struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct rte_flow_query_count *qc = data;
@@ -14102,19 +14136,16 @@ flow_dv_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL,
 					  "counters are not supported");
-	if (flow->counter) {
+	if (cnt_idx) {
 		uint64_t pkts, bytes;
 		struct mlx5_flow_counter *cnt;
-
-		cnt = flow_dv_counter_get_by_idx(dev, flow->counter,
-						 NULL);
-		int err = _flow_dv_query_count(dev, flow->counter, &pkts,
-					       &bytes);
+		int err = _flow_dv_query_count(dev, cnt_idx, &pkts, &bytes);
 
 		if (err)
 			return rte_flow_error_set(error, -err,
 					RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					NULL, "cannot read counters");
+		cnt = flow_dv_counter_get_by_idx(dev, cnt_idx, NULL);
 		qc->hits_set = 1;
 		qc->bytes_set = 1;
 		qc->hits = pkts - cnt->hits;
@@ -14131,6 +14162,38 @@ flow_dv_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
 				  "counters are not available");
 }
 
+static int
+flow_dv_action_query(struct rte_eth_dev *dev,
+		     const struct rte_flow_action_handle *handle, void *data,
+		     struct rte_flow_error *error)
+{
+	struct mlx5_age_param *age_param;
+	struct rte_flow_query_age *resp;
+	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
+	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
+	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
+
+	switch (type) {
+	case MLX5_INDIRECT_ACTION_TYPE_AGE:
+		age_param = &flow_aso_age_get_by_idx(dev, idx)->age_params;
+		resp = data;
+		resp->aged = __atomic_load_n(&age_param->state,
+					      __ATOMIC_RELAXED) == AGE_TMOUT ?
+									  1 : 0;
+		resp->sec_since_last_hit_valid = !resp->aged;
+		if (resp->sec_since_last_hit_valid)
+			resp->sec_since_last_hit = __atomic_load_n
+			     (&age_param->sec_since_last_hit, __ATOMIC_RELAXED);
+		return 0;
+	case MLX5_INDIRECT_ACTION_TYPE_COUNT:
+		return flow_dv_query_count(dev, idx, data, error);
+	default:
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "action type query not supported");
+	}
+}
+
 /**
  * Query a flow rule AGE action for aging information.
  *
@@ -14200,7 +14263,8 @@ flow_dv_query(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
-			ret = flow_dv_query_count(dev, flow, data, error);
+			ret = flow_dv_query_count(dev, flow->counter, data,
+						  error);
 			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
 			ret = flow_dv_query_age(dev, flow, data, error);
@@ -15241,7 +15305,7 @@ flow_dv_counter_allocate(struct rte_eth_dev *dev)
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] conf
- *   Shared action configuration.
+ *   Indirect action configuration.
  * @param[in] action
  *   The indirect action object to validate.
  * @param[out] error
@@ -15268,22 +15332,36 @@ flow_dv_action_validate(struct rte_eth_dev *dev,
 		 * sufficient, it is set to devx_obj_ops.
 		 * Otherwise, it is set to ibv_obj_ops.
 		 * ibv_obj_ops doesn't support ind_table_modify operation.
-		 * In this case the shared RSS action can't be used.
+		 * In this case the indirect RSS action can't be used.
 		 */
 		if (priv->obj_ops.ind_table_modify == NULL)
 			return rte_flow_error_set
 					(err, ENOTSUP,
 					 RTE_FLOW_ERROR_TYPE_ACTION,
 					 NULL,
-					 "shared RSS action not supported");
+					 "Indirect RSS action not supported");
 		return mlx5_validate_action_rss(dev, action, err);
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		if (!priv->sh->aso_age_mng)
 			return rte_flow_error_set(err, ENOTSUP,
 						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 						NULL,
-					     "shared age action not supported");
+						"Indirect age action not supported");
 		return flow_dv_validate_action_age(0, action, dev, err);
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		/*
+		 * There are two mechanisms to share the action count.
+		 * The old mechanism uses the shared field to share, while the
+		 * new mechanism uses the indirect action API.
+		 * This validation comes to make sure that the two mechanisms
+		 * are not combined.
+		 */
+		if (is_shared_action_count(action))
+			return rte_flow_error_set(err, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "Mix shared and indirect counter is not supported");
+		return flow_dv_validate_action_count(dev, true, 0, err);
 	default:
 		return rte_flow_error_set(err, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 0fdafbba4e..fe9673310a 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -357,7 +357,7 @@ flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter)
 	struct mlx5_flow_counter *cnt;
 
 	cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
-	if (IS_SHARED_CNT(counter) &&
+	if (IS_LEGACY_SHARED_CNT(counter) &&
 	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
 		return;
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 2/5] app/testpmd: remove indirect RSS action query
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle Viacheslav Ovsiienko
@ 2021-04-29  9:55   ` Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: support indirect counter " Viacheslav Ovsiienko
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit, Michael Baum

From: Michael Baum <michaelba@nvidia.com>

The port_action_handle_query function supports query operation for
indirect RSS action.

No driver currently supports this operation, and this support is
unnecessary.

Remove it.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/config.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e189062efd..a9805cc198 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1626,7 +1626,6 @@ port_action_handle_query(portid_t port_id, uint32_t id)
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
-	case RTE_FLOW_ACTION_TYPE_RSS:
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		data = &default_data;
 		break;
@@ -1638,12 +1637,6 @@ port_action_handle_query(portid_t port_id, uint32_t id)
 	if (rte_flow_action_handle_query(port_id, pia->handle, data, &error))
 		ret = port_flow_complain(&error);
 	switch (pia->type) {
-	case RTE_FLOW_ACTION_TYPE_RSS:
-		if (!ret)
-			printf("Shared RSS action:\n\trefs:%u\n",
-			       *((uint32_t *)data));
-		data = NULL;
-		break;
 	case RTE_FLOW_ACTION_TYPE_AGE:
 		if (!ret) {
 			struct rte_flow_query_age *resp = data;
-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 3/5] app/testpmd: support indirect counter action query
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: remove indirect RSS action query Viacheslav Ovsiienko
@ 2021-04-29  9:55   ` Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix flow age event triggering Viacheslav Ovsiienko
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit, Michael Baum

From: Michael Baum <michaelba@nvidia.com>

Counter action query was implemented as part of flow query, but was not
implemented as part of indirect action query.

This patch adds the required implementation.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/config.c | 145 ++++++++++++++++++++++--------------------
 1 file changed, 77 insertions(+), 68 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a9805cc198..bedbfcbb7b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1618,90 +1618,99 @@ port_action_handle_query(portid_t port_id, uint32_t id)
 {
 	struct rte_flow_error error;
 	struct port_indirect_action *pia;
-	uint64_t default_data;
-	void *data = NULL;
-	int ret = 0;
+	union {
+		struct rte_flow_query_count count;
+		struct rte_flow_query_age age;
+		struct rte_flow_action_conntrack ct;
+	} query;
 
 	pia = action_get_by_id(port_id, id);
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
 	case RTE_FLOW_ACTION_TYPE_AGE:
-		data = &default_data;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
 		break;
 	default:
-		printf("Indirect action %u (type: %d) on port %u doesn't"
-		       " support query\n", id, pia->type, port_id);
-		return -1;
+		printf("Indirect action %u (type: %d) on port %u doesn't support query\n",
+		       id, pia->type, port_id);
+		return -ENOTSUP;
 	}
-	if (rte_flow_action_handle_query(port_id, pia->handle, data, &error))
-		ret = port_flow_complain(&error);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x55, sizeof(error));
+	memset(&query, 0, sizeof(query));
+	if (rte_flow_action_handle_query(port_id, pia->handle, &query, &error))
+		return port_flow_complain(&error);
 	switch (pia->type) {
 	case RTE_FLOW_ACTION_TYPE_AGE:
-		if (!ret) {
-			struct rte_flow_query_age *resp = data;
-
-			printf("AGE:\n"
-			       " aged: %u\n"
-			       " sec_since_last_hit_valid: %u\n"
-			       " sec_since_last_hit: %" PRIu32 "\n",
-			       resp->aged,
-			       resp->sec_since_last_hit_valid,
-			       resp->sec_since_last_hit);
-		}
-		data = NULL;
+		printf("Indirect AGE action:\n"
+		       " aged: %u\n"
+		       " sec_since_last_hit_valid: %u\n"
+		       " sec_since_last_hit: %" PRIu32 "\n",
+		       query.age.aged,
+		       query.age.sec_since_last_hit_valid,
+		       query.age.sec_since_last_hit);
+		break;
+	case RTE_FLOW_ACTION_TYPE_COUNT:
+		printf("Indirect COUNT action:\n"
+		       " hits_set: %u\n"
+		       " bytes_set: %u\n"
+		       " hits: %" PRIu64 "\n"
+		       " bytes: %" PRIu64 "\n",
+		       query.count.hits_set,
+		       query.count.bytes_set,
+		       query.count.hits,
+		       query.count.bytes);
 		break;
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
-		if (!ret) {
-			struct rte_flow_action_conntrack *ct = data;
-
-			printf("Conntrack Context:\n"
-			       "  Peer: %u, Flow dir: %s, Enable: %u\n"
-			       "  Live: %u, SACK: %u, CACK: %u\n"
-			       "  Packet dir: %s, Liberal: %u, State: %u\n"
-			       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
-			       "  Last Seq: %u, Last ACK: %u\n"
-			       "  Last Win: %u, Last End: %u\n",
-			       ct->peer_port,
-			       ct->is_original_dir ? "Original" : "Reply",
-			       ct->enable, ct->live_connection,
-			       ct->selective_ack, ct->challenge_ack_passed,
-			       ct->last_direction ? "Original" : "Reply",
-			       ct->liberal_mode, ct->state,
-			       ct->max_ack_window, ct->retransmission_limit,
-			       ct->last_index, ct->last_seq, ct->last_ack,
-			       ct->last_window, ct->last_end);
-			printf("  Original Dir:\n"
-			       "    scale: %u, fin: %u, ack seen: %u\n"
-			       " unacked data: %u\n    Sent end: %u,"
-			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
-			       ct->original_dir.scale,
-			       ct->original_dir.close_initiated,
-			       ct->original_dir.last_ack_seen,
-			       ct->original_dir.data_unacked,
-			       ct->original_dir.sent_end,
-			       ct->original_dir.reply_end,
-			       ct->original_dir.max_win,
-			       ct->original_dir.max_ack);
-			printf("  Reply Dir:\n"
-			       "    scale: %u, fin: %u, ack seen: %u\n"
-			       " unacked data: %u\n    Sent end: %u,"
-			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
-			       ct->reply_dir.scale,
-			       ct->reply_dir.close_initiated,
-			       ct->reply_dir.last_ack_seen,
-			       ct->reply_dir.data_unacked,
-			       ct->reply_dir.sent_end, ct->reply_dir.reply_end,
-			       ct->reply_dir.max_win, ct->reply_dir.max_ack);
-		}
-		data = NULL;
+		printf("Conntrack Context:\n"
+		       "  Peer: %u, Flow dir: %s, Enable: %u\n"
+		       "  Live: %u, SACK: %u, CACK: %u\n"
+		       "  Packet dir: %s, Liberal: %u, State: %u\n"
+		       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
+		       "  Last Seq: %u, Last ACK: %u\n"
+		       "  Last Win: %u, Last End: %u\n",
+		       query.ct.peer_port,
+		       query.ct.is_original_dir ? "Original" : "Reply",
+		       query.ct.enable, query.ct.live_connection,
+		       query.ct.selective_ack, query.ct.challenge_ack_passed,
+		       query.ct.last_direction ? "Original" : "Reply",
+		       query.ct.liberal_mode, query.ct.state,
+		       query.ct.max_ack_window, query.ct.retransmission_limit,
+		       query.ct.last_index, query.ct.last_seq,
+		       query.ct.last_ack, query.ct.last_window,
+		       query.ct.last_end);
+		printf("  Original Dir:\n"
+		       "    scale: %u, fin: %u, ack seen: %u\n"
+		       " unacked data: %u\n    Sent end: %u,"
+		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
+		       query.ct.original_dir.scale,
+		       query.ct.original_dir.close_initiated,
+		       query.ct.original_dir.last_ack_seen,
+		       query.ct.original_dir.data_unacked,
+		       query.ct.original_dir.sent_end,
+		       query.ct.original_dir.reply_end,
+		       query.ct.original_dir.max_win,
+		       query.ct.original_dir.max_ack);
+		printf("  Reply Dir:\n"
+		       "    scale: %u, fin: %u, ack seen: %u\n"
+		       " unacked data: %u\n    Sent end: %u,"
+		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
+		       query.ct.reply_dir.scale,
+		       query.ct.reply_dir.close_initiated,
+		       query.ct.reply_dir.last_ack_seen,
+		       query.ct.reply_dir.data_unacked,
+		       query.ct.reply_dir.sent_end,
+		       query.ct.reply_dir.reply_end,
+		       query.ct.reply_dir.max_win,
+		       query.ct.reply_dir.max_ack);
 		break;
 	default:
-		printf("Indirect action %u (type: %d) on port %u doesn't"
-		       " support query\n", id, pia->type, port_id);
-		ret = -1;
+		printf("Indirect action %u (type: %d) on port %u doesn't support query\n",
+		       id, pia->type, port_id);
+		break;
 	}
-	return ret;
+	return 0;
 }
 
 static struct port_flow_tunnel *
-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix flow age event triggering
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
                     ` (2 preceding siblings ...)
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: support indirect counter " Viacheslav Ovsiienko
@ 2021-04-29  9:55   ` Viacheslav Ovsiienko
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 5/5] net/mlx5: use aging by counter when counter is existed Viacheslav Ovsiienko
  2021-04-30 10:43   ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Ferruh Yigit
  5 siblings, 0 replies; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit, Michael Baum, stable

From: Michael Baum <michaelba@nvidia.com>

A FLOW_AGE event should be invoked when a new aged-out flow is detected
by the PMD after the last user get-aged query calling.
The PMD manages 2 flags for this information and check them in order to
decide if an event should be invoked:
MLX5_AGE_EVENT_NEW - a new aged-out flow was detected. after the last
check.
MLX5_AGE_TRIGGER - get-aged query was called after the last aged-out
flow.
The 2 flags were unset after the event invoking.

When the user calls get-aged query from the event callback, the TRIGGER
flag was set inside the user callback and unset directly after the
callback what may stop the event invoking forever.

Unset the TRIGGER flag before the event invoking in order to allow set
it by the user callback.

Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
Cc: stable@dpdk.org

Reported-by: David Bouyeure <david.bouyeure@fraudbuster.mobi>
Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c | 8 +++++---
 drivers/net/mlx5/mlx5.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 19ffa16769..10d44c7bbe 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -662,11 +662,13 @@ mlx5_age_event_prepare(struct mlx5_dev_ctx_shared *sh)
 		age_info = &sh->port[i].age_info;
 		if (!MLX5_AGE_GET(age_info, MLX5_AGE_EVENT_NEW))
 			continue;
-		if (MLX5_AGE_GET(age_info, MLX5_AGE_TRIGGER))
+		MLX5_AGE_UNSET(age_info, MLX5_AGE_EVENT_NEW);
+		if (MLX5_AGE_GET(age_info, MLX5_AGE_TRIGGER)) {
+			MLX5_AGE_UNSET(age_info, MLX5_AGE_TRIGGER);
 			rte_eth_dev_callback_process
 				(&rte_eth_devices[sh->port[i].devx_ih_port_id],
 				RTE_ETH_EVENT_FLOW_AGED, NULL);
-		age_info->flags = 0;
+		}
 	}
 }
 
@@ -675,7 +677,7 @@ mlx5_age_event_prepare(struct mlx5_dev_ctx_shared *sh)
  *
  * @param[in] sh
  *   Pointer to mlx5_dev_ctx_shared object.
- * @param[in] sh
+ * @param[in] config
  *   Pointer to user dev config.
  */
 static void
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index efb80a4a49..79ae8fbef1 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -563,6 +563,8 @@ struct mlx5_geneve_tlv_option_resource {
 #define MLX5_AGE_TRIGGER		2
 #define MLX5_AGE_SET(age_info, BIT) \
 	((age_info)->flags |= (1 << (BIT)))
+#define MLX5_AGE_UNSET(age_info, BIT) \
+	((age_info)->flags &= ~(1 << (BIT)))
 #define MLX5_AGE_GET(age_info, BIT) \
 	((age_info)->flags & (1 << (BIT)))
 #define GET_PORT_AGE_INFO(priv) \
-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2 5/5] net/mlx5: use aging by counter when counter is existed
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
                     ` (3 preceding siblings ...)
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix flow age event triggering Viacheslav Ovsiienko
@ 2021-04-29  9:55   ` Viacheslav Ovsiienko
  2021-04-30 10:43   ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Ferruh Yigit
  5 siblings, 0 replies; 18+ messages in thread
From: Viacheslav Ovsiienko @ 2021-04-29  9:55 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, ferruh.yigit, Michael Baum

From: Michael Baum <michaelba@nvidia.com>

The driver support 2 mechanisms in order to support AGE action:
1. Aging by counter - HW counter will be configured to the flow traffic,
the driver polls the counter values efficiently to detect flow timeout.
2. Aging by ASO flow hit bit - HW ASO flow-hit bit is allocated for the
flow, the driver polls the bit efficiently to detect flow timeout.

ASO bit is only single bit resource while counter is 16 bytes, hence, it
is better to use ASO instead of counter for aging.

When a non-shared COUNT action is also configured to the flow, the
driver can use the same counter also for AGE action and no need to
create more ASO action for it.

The current code always uses ASO when it is supported in the device,
change it to reuse the non-shared counter if it exists in the flow.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 149 ++++++++++++++++++++++----------
 1 file changed, 102 insertions(+), 47 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index b74bac1083..2fb6621017 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7143,6 +7143,12 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 									   NULL,
 			  "Shared ASO age action is not supported for group 0");
+			if (action_flags & MLX5_FLOW_ACTION_AGE)
+				return rte_flow_error_set
+						  (error, EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   NULL,
+						   "duplicate age actions set");
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			++actions_n;
 			break;
@@ -11162,6 +11168,47 @@ flow_dv_translate_create_aso_age(struct rte_eth_dev *dev,
 	return age_idx;
 }
 
+/**
+ * Prepares DV flow counter with aging configuration.
+ * Gets it by index when exists, creates a new one when doesn't.
+ *
+ * @param[in] dev
+ *   Pointer to rte_eth_dev structure.
+ * @param[in] dev_flow
+ *   Pointer to the mlx5_flow.
+ * @param[in, out] flow
+ *   Pointer to the sub flow.
+ * @param[in] count
+ *   Pointer to the counter action configuration.
+ * @param[in] age
+ *   Pointer to the aging action configuration.
+ * @param[out] error
+ *   Pointer to the error structure.
+ *
+ * @return
+ *   Pointer to the counter, NULL otherwise.
+ */
+static struct mlx5_flow_counter *
+flow_dv_prepare_counter(struct rte_eth_dev *dev,
+			struct mlx5_flow *dev_flow,
+			struct rte_flow *flow,
+			const struct rte_flow_action_count *count,
+			const struct rte_flow_action_age *age,
+			struct rte_flow_error *error)
+{
+	if (!flow->counter) {
+		flow->counter = flow_dv_translate_create_counter(dev, dev_flow,
+								 count, age);
+		if (!flow->counter) {
+			rte_flow_error_set(error, rte_errno,
+					   RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					   "cannot create counter object.");
+			return NULL;
+		}
+	}
+	return flow_dv_counter_get_by_idx(dev, flow->counter, NULL);
+}
+
 /**
  * Fill the flow with DV spec, lock free
  * (mutex should be acquired by caller).
@@ -11215,7 +11262,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
 	} mhdr_dummy;
 	struct mlx5_flow_dv_modify_hdr_resource *mhdr_res = &mhdr_dummy.res;
 	const struct rte_flow_action_count *count = NULL;
-	const struct rte_flow_action_age *age = NULL;
+	const struct rte_flow_action_age *non_shared_age = NULL;
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	uint32_t tag_be;
 	union mlx5_flow_tbl_key tbl_key;
@@ -11230,6 +11277,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
 	const struct rte_flow_action_sample *sample = NULL;
 	struct mlx5_flow_sub_actions_list *sample_act;
 	uint32_t sample_act_pos = UINT32_MAX;
+	uint32_t age_act_pos = UINT32_MAX;
 	uint32_t num_of_dest = 0;
 	int tmp_actions_n = 0;
 	uint32_t table;
@@ -11452,7 +11500,12 @@ flow_dv_translate(struct rte_eth_dev *dev,
 			age_act = flow_aso_age_get_by_idx(dev, flow->age);
 			__atomic_fetch_add(&age_act->refcnt, 1,
 					   __ATOMIC_RELAXED);
-			dev_flow->dv.actions[actions_n++] = age_act->dr_action;
+			age_act_pos = actions_n++;
+			action_flags |= MLX5_FLOW_ACTION_AGE;
+			break;
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			non_shared_age = action->conf;
+			age_act_pos = actions_n++;
 			action_flags |= MLX5_FLOW_ACTION_AGE;
 			break;
 		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
@@ -11464,31 +11517,6 @@ flow_dv_translate(struct rte_eth_dev *dev,
 			/* Save information first, will apply later. */
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
-		case RTE_FLOW_ACTION_TYPE_AGE:
-			if (priv->sh->flow_hit_aso_en && attr->group) {
-				/*
-				 * Create one shared age action, to be used
-				 * by all sub-flows.
-				 */
-				if (!flow->age) {
-					flow->age =
-						flow_dv_translate_create_aso_age
-							(dev, action->conf,
-							 error);
-					if (!flow->age)
-						return rte_flow_error_set
-						(error, rte_errno,
-						 RTE_FLOW_ERROR_TYPE_ACTION,
-						 NULL,
-						 "can't create ASO age action");
-				}
-				dev_flow->dv.actions[actions_n++] =
-					  (flow_aso_age_get_by_idx
-						(dev, flow->age))->dr_action;
-				action_flags |= MLX5_FLOW_ACTION_AGE;
-				break;
-			}
-			/* Fall-through */
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			if (!dev_conf->devx) {
 				return rte_flow_error_set
@@ -11498,10 +11526,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
 					       "count action not supported");
 			}
 			/* Save information first, will apply later. */
-			if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT)
-				count = action->conf;
-			else
-				age = action->conf;
+			count = action->conf;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
@@ -11801,27 +11826,57 @@ flow_dv_translate(struct rte_eth_dev *dev,
 				dev_flow->dv.actions[modify_action_position] =
 					handle->dvh.modify_hdr->action;
 			}
+			/*
+			 * Handle AGE and COUNT action by single HW counter
+			 * when they are not shared.
+			 */
+			if (action_flags & MLX5_FLOW_ACTION_AGE) {
+				if ((non_shared_age &&
+				     count && !count->shared) ||
+				    !(priv->sh->flow_hit_aso_en &&
+				      attr->group)) {
+					/* Creates age by counters. */
+					cnt_act = flow_dv_prepare_counter
+								(dev, dev_flow,
+								 flow, count,
+								 non_shared_age,
+								 error);
+					if (!cnt_act)
+						return -rte_errno;
+					dev_flow->dv.actions[age_act_pos] =
+								cnt_act->action;
+					break;
+				}
+				if (!flow->age && non_shared_age) {
+					flow->age =
+						flow_dv_translate_create_aso_age
+								(dev,
+								 non_shared_age,
+								 error);
+					if (!flow->age)
+						return rte_flow_error_set
+						    (error, rte_errno,
+						     RTE_FLOW_ERROR_TYPE_ACTION,
+						     NULL,
+						     "can't create ASO age action");
+				}
+				age_act = flow_aso_age_get_by_idx(dev,
+								  flow->age);
+				dev_flow->dv.actions[age_act_pos] =
+							     age_act->dr_action;
+			}
 			if (action_flags & MLX5_FLOW_ACTION_COUNT) {
 				/*
 				 * Create one count action, to be used
 				 * by all sub-flows.
 				 */
-				if (!flow->counter) {
-					flow->counter =
-						flow_dv_translate_create_counter
-							(dev, dev_flow, count,
-							 age);
-					if (!flow->counter)
-						return rte_flow_error_set
-						(error, rte_errno,
-						 RTE_FLOW_ERROR_TYPE_ACTION,
-						 NULL, "cannot create counter"
-						 " object.");
-				}
-				dev_flow->dv.actions[actions_n] =
-					  (flow_dv_counter_get_by_idx(dev,
-					  flow->counter, NULL))->action;
-				actions_n++;
+				cnt_act = flow_dv_prepare_counter(dev, dev_flow,
+								  flow, count,
+								  NULL, error);
+				if (!cnt_act)
+					return -rte_errno;
+				dev_flow->dv.actions[actions_n++] =
+								cnt_act->action;
 			}
 		default:
 			break;
-- 
2.18.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter action query
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter " Michael Baum
@ 2021-04-29 13:45   ` Ori Kam
  0 siblings, 0 replies; 18+ messages in thread
From: Ori Kam @ 2021-04-29 13:45 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Slava Ovsiienko

Hi Michael,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Michael Baum
> Sent: Monday, April 26, 2021 3:43 PM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Subject: [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter
> action query
> 
> Counter action query was implemented as part of flow query, but was not
> implemented as part of indirect action query.
> 
> This patch adds the required implementation.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  app/test-pmd/config.c | 145 +++++++++++++++++++++++++++---------------
> --------
>  1 file changed, 77 insertions(+), 68 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> a9805cc..bedbfcb 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1618,90 +1618,99 @@ struct rte_flow_action_handle *  {
>  	struct rte_flow_error error;
>  	struct port_indirect_action *pia;
> -	uint64_t default_data;
> -	void *data = NULL;
> -	int ret = 0;
> +	union {
> +		struct rte_flow_query_count count;
> +		struct rte_flow_query_age age;
> +		struct rte_flow_action_conntrack ct;
> +	} query;
> 
>  	pia = action_get_by_id(port_id, id);
>  	if (!pia)
>  		return -EINVAL;
>  	switch (pia->type) {
>  	case RTE_FLOW_ACTION_TYPE_AGE:
> -		data = &default_data;
> +	case RTE_FLOW_ACTION_TYPE_COUNT:
>  		break;
>  	default:
> -		printf("Indirect action %u (type: %d) on port %u doesn't"
> -		       " support query\n", id, pia->type, port_id);
> -		return -1;
> +		printf("Indirect action %u (type: %d) on port %u doesn't
> support query\n",
> +		       id, pia->type, port_id);
> +		return -ENOTSUP;
>  	}
> -	if (rte_flow_action_handle_query(port_id, pia->handle, data,
> &error))
> -		ret = port_flow_complain(&error);
> +	/* Poisoning to make sure PMDs update it in case of error. */
> +	memset(&error, 0x55, sizeof(error));
> +	memset(&query, 0, sizeof(query));
> +	if (rte_flow_action_handle_query(port_id, pia->handle, &query,
> &error))
> +		return port_flow_complain(&error);
>  	switch (pia->type) {
>  	case RTE_FLOW_ACTION_TYPE_AGE:
> -		if (!ret) {
> -			struct rte_flow_query_age *resp = data;
> -
> -			printf("AGE:\n"
> -			       " aged: %u\n"
> -			       " sec_since_last_hit_valid: %u\n"
> -			       " sec_since_last_hit: %" PRIu32 "\n",
> -			       resp->aged,
> -			       resp->sec_since_last_hit_valid,
> -			       resp->sec_since_last_hit);
> -		}
> -		data = NULL;
> +		printf("Indirect AGE action:\n"
> +		       " aged: %u\n"
> +		       " sec_since_last_hit_valid: %u\n"
> +		       " sec_since_last_hit: %" PRIu32 "\n",
> +		       query.age.aged,
> +		       query.age.sec_since_last_hit_valid,
> +		       query.age.sec_since_last_hit);
> +		break;
> +	case RTE_FLOW_ACTION_TYPE_COUNT:
> +		printf("Indirect COUNT action:\n"
> +		       " hits_set: %u\n"
> +		       " bytes_set: %u\n"
> +		       " hits: %" PRIu64 "\n"
> +		       " bytes: %" PRIu64 "\n",
> +		       query.count.hits_set,
> +		       query.count.bytes_set,
> +		       query.count.hits,
> +		       query.count.bytes);
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> -		if (!ret) {
> -			struct rte_flow_action_conntrack *ct = data;
> -
> -			printf("Conntrack Context:\n"
> -			       "  Peer: %u, Flow dir: %s, Enable: %u\n"
> -			       "  Live: %u, SACK: %u, CACK: %u\n"
> -			       "  Packet dir: %s, Liberal: %u, State: %u\n"
> -			       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
> -			       "  Last Seq: %u, Last ACK: %u\n"
> -			       "  Last Win: %u, Last End: %u\n",
> -			       ct->peer_port,
> -			       ct->is_original_dir ? "Original" : "Reply",
> -			       ct->enable, ct->live_connection,
> -			       ct->selective_ack, ct->challenge_ack_passed,
> -			       ct->last_direction ? "Original" : "Reply",
> -			       ct->liberal_mode, ct->state,
> -			       ct->max_ack_window, ct->retransmission_limit,
> -			       ct->last_index, ct->last_seq, ct->last_ack,
> -			       ct->last_window, ct->last_end);
> -			printf("  Original Dir:\n"
> -			       "    scale: %u, fin: %u, ack seen: %u\n"
> -			       " unacked data: %u\n    Sent end: %u,"
> -			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
> -			       ct->original_dir.scale,
> -			       ct->original_dir.close_initiated,
> -			       ct->original_dir.last_ack_seen,
> -			       ct->original_dir.data_unacked,
> -			       ct->original_dir.sent_end,
> -			       ct->original_dir.reply_end,
> -			       ct->original_dir.max_win,
> -			       ct->original_dir.max_ack);
> -			printf("  Reply Dir:\n"
> -			       "    scale: %u, fin: %u, ack seen: %u\n"
> -			       " unacked data: %u\n    Sent end: %u,"
> -			       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
> -			       ct->reply_dir.scale,
> -			       ct->reply_dir.close_initiated,
> -			       ct->reply_dir.last_ack_seen,
> -			       ct->reply_dir.data_unacked,
> -			       ct->reply_dir.sent_end, ct->reply_dir.reply_end,
> -			       ct->reply_dir.max_win, ct->reply_dir.max_ack);
> -		}
> -		data = NULL;
> +		printf("Conntrack Context:\n"
> +		       "  Peer: %u, Flow dir: %s, Enable: %u\n"
> +		       "  Live: %u, SACK: %u, CACK: %u\n"
> +		       "  Packet dir: %s, Liberal: %u, State: %u\n"
> +		       "  Factor: %u, Retrans: %u, TCP flags: %u\n"
> +		       "  Last Seq: %u, Last ACK: %u\n"
> +		       "  Last Win: %u, Last End: %u\n",
> +		       query.ct.peer_port,
> +		       query.ct.is_original_dir ? "Original" : "Reply",
> +		       query.ct.enable, query.ct.live_connection,
> +		       query.ct.selective_ack, query.ct.challenge_ack_passed,
> +		       query.ct.last_direction ? "Original" : "Reply",
> +		       query.ct.liberal_mode, query.ct.state,
> +		       query.ct.max_ack_window,
> query.ct.retransmission_limit,
> +		       query.ct.last_index, query.ct.last_seq,
> +		       query.ct.last_ack, query.ct.last_window,
> +		       query.ct.last_end);
> +		printf("  Original Dir:\n"
> +		       "    scale: %u, fin: %u, ack seen: %u\n"
> +		       " unacked data: %u\n    Sent end: %u,"
> +		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
> +		       query.ct.original_dir.scale,
> +		       query.ct.original_dir.close_initiated,
> +		       query.ct.original_dir.last_ack_seen,
> +		       query.ct.original_dir.data_unacked,
> +		       query.ct.original_dir.sent_end,
> +		       query.ct.original_dir.reply_end,
> +		       query.ct.original_dir.max_win,
> +		       query.ct.original_dir.max_ack);
> +		printf("  Reply Dir:\n"
> +		       "    scale: %u, fin: %u, ack seen: %u\n"
> +		       " unacked data: %u\n    Sent end: %u,"
> +		       "    Reply end: %u, Max win: %u, Max ACK: %u\n",
> +		       query.ct.reply_dir.scale,
> +		       query.ct.reply_dir.close_initiated,
> +		       query.ct.reply_dir.last_ack_seen,
> +		       query.ct.reply_dir.data_unacked,
> +		       query.ct.reply_dir.sent_end,
> +		       query.ct.reply_dir.reply_end,
> +		       query.ct.reply_dir.max_win,
> +		       query.ct.reply_dir.max_ack);
>  		break;
>  	default:
> -		printf("Indirect action %u (type: %d) on port %u doesn't"
> -		       " support query\n", id, pia->type, port_id);
> -		ret = -1;
> +		printf("Indirect action %u (type: %d) on port %u doesn't
> support query\n",
> +		       id, pia->type, port_id);
> +		break;
>  	}
> -	return ret;
> +	return 0;
>  }
> 
>  static struct port_flow_tunnel *
> --
> 1.8.3.1

[Ori Kam] 
Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query
  2021-04-26 12:42 ` [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query Michael Baum
@ 2021-04-29 13:46   ` Ori Kam
  0 siblings, 0 replies; 18+ messages in thread
From: Ori Kam @ 2021-04-29 13:46 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Slava Ovsiienko

Hi Michael,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Michael Baum
> Sent: Monday, April 26, 2021 3:43 PM
> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action
> query
> 
> The port_action_handle_query function supports query operation for
> indirect RSS action.
> 
> No driver currently supports this operation, and this support is unnecessary.
> 
> Remove it.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  app/test-pmd/config.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> e189062..a9805cc 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1626,7 +1626,6 @@ struct rte_flow_action_handle *
>  	if (!pia)
>  		return -EINVAL;
>  	switch (pia->type) {
> -	case RTE_FLOW_ACTION_TYPE_RSS:
>  	case RTE_FLOW_ACTION_TYPE_AGE:
>  		data = &default_data;
>  		break;
> @@ -1638,12 +1637,6 @@ struct rte_flow_action_handle *
>  	if (rte_flow_action_handle_query(port_id, pia->handle, data,
> &error))
>  		ret = port_flow_complain(&error);
>  	switch (pia->type) {
> -	case RTE_FLOW_ACTION_TYPE_RSS:
> -		if (!ret)
> -			printf("Shared RSS action:\n\trefs:%u\n",
> -			       *((uint32_t *)data));
> -		data = NULL;
> -		break;
>  	case RTE_FLOW_ACTION_TYPE_AGE:
>  		if (!ret) {
>  			struct rte_flow_query_age *resp = data;
> --
> 1.8.3.1
Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle Viacheslav Ovsiienko
@ 2021-04-30  8:34     ` Ferruh Yigit
  2021-04-30  9:01       ` Slava Ovsiienko
  0 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2021-04-30  8:34 UTC (permalink / raw)
  To: Viacheslav Ovsiienko, dev, Andrew Rybchenko
  Cc: rasland, matan, Michael Baum, Thomas Monjalon

On 4/29/2021 10:55 AM, Viacheslav Ovsiienko wrote:
> From: Michael Baum <michaelba@nvidia.com>
> 
> Existing API supports counter action to count traffic of a single flow.
> The user can share the count action among different flows using the
> shared flag and the same counter ID in the count action configuration.
> 
> Recent patch [1] introduced the indirect action API.
> Using this API, an action can be created as indirect, unattached to any
> flow rule.
> Multiple flows can then be created using the same indirect action.
> The new API also supports query operation of an indirect action.
> 
> The new API is more efficient because the driver gets it's own handler
> for the count action instead of managing a mapping between the user ID
> to the driver handle.
> 
> Support create, query and destroy indirect action operations for flow
> count action.
> 
> Application will use the indirect action query operation to query this
> count action.
> 
> In the meantime the old sharing mechanism (with the sharing flag)
> continues to be supported, and the user can choose the way he wants to
> share the counter.
> The new indirect action API is only supported in DevX, so sharing
> counter action in Verbs can only be done through the old mechanism.
> 

There is already a deprecation note to remove the 'shared' flag from counter
action [2] in favor of 'shared actions'.

Shared action become "indirect actions" [3] which this patch implements

Is it a good time to remove the old sharing mechanism from driver since touching
on it?


[2]
https://git.dpdk.org/dpdk/tree/doc/guides/rel_notes/deprecation.rst?h=v21.05-rc1#n95

[3]
https://git.dpdk.org/dpdk/commit/?h=v21.05-rc1&id=4b61b8774be9


> [1] https://mails.dpdk.org/archives/dev/2020-July/174110.html
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>

<...>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle
  2021-04-30  8:34     ` Ferruh Yigit
@ 2021-04-30  9:01       ` Slava Ovsiienko
  2021-04-30  9:22         ` Ferruh Yigit
  0 siblings, 1 reply; 18+ messages in thread
From: Slava Ovsiienko @ 2021-04-30  9:01 UTC (permalink / raw)
  To: Ferruh Yigit, dev, Andrew Rybchenko
  Cc: Raslan Darawsheh, Matan Azrad, Michael Baum, NBU-Contact-Thomas Monjalon

Hi, Ferruh

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, April 30, 2021 11:35
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Michael Baum <michaelba@nvidia.com>; NBU-
> Contact-Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [PATCH v2 1/5] net/mlx5: support flow count action handle
> 
> On 4/29/2021 10:55 AM, Viacheslav Ovsiienko wrote:
> > From: Michael Baum <michaelba@nvidia.com>
> >
> > Existing API supports counter action to count traffic of a single flow.
> > The user can share the count action among different flows using the
> > shared flag and the same counter ID in the count action configuration.
> >
> > Recent patch [1] introduced the indirect action API.
> > Using this API, an action can be created as indirect, unattached to
> > any flow rule.
> > Multiple flows can then be created using the same indirect action.
> > The new API also supports query operation of an indirect action.
> >
> > The new API is more efficient because the driver gets it's own handler
> > for the count action instead of managing a mapping between the user ID
> > to the driver handle.
> >
> > Support create, query and destroy indirect action operations for flow
> > count action.
> >
> > Application will use the indirect action query operation to query this
> > count action.
> >
> > In the meantime the old sharing mechanism (with the sharing flag)
> > continues to be supported, and the user can choose the way he wants to
> > share the counter.
> > The new indirect action API is only supported in DevX, so sharing
> > counter action in Verbs can only be done through the old mechanism.
> >
> 
> There is already a deprecation note to remove the 'shared' flag from counter
> action [2] in favor of 'shared actions'.
> 
> Shared action become "indirect actions" [3] which this patch implements
> 
> Is it a good time to remove the old sharing mechanism from driver since
> touching on it?

As I can see:
- it is planned to 21.11 as deprecation note says
- now there Is no support for new "indirect actions" with Verbs (not sure
 we will provide), so shared counters API might be still useful
- our CI did not migrate to the new counter approach yet (even DPDK patch is not merged yet 😊)
- hence, l would prefer keep shared counter support (by id and flag) in 21.05 
 (and very likely in 21.08), just to maintain maximal backward compatibility as long
as possible

With best regards,
Slava

> 
> 
> [2]
> https://git.dpdk.org/dpdk/tree/doc/guides/rel_notes/deprecation.rst?h=v2
> 1.05-rc1#n95
> 
> [3]
> https://git.dpdk.org/dpdk/commit/?h=v21.05-rc1&id=4b61b8774be9
> 
> 
> > [1] https://mails.dpdk.org/archives/dev/2020-July/174110.html
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > Acked-by: Matan Azrad <matan@nvidia.com>
> 
> <...>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle
  2021-04-30  9:01       ` Slava Ovsiienko
@ 2021-04-30  9:22         ` Ferruh Yigit
  0 siblings, 0 replies; 18+ messages in thread
From: Ferruh Yigit @ 2021-04-30  9:22 UTC (permalink / raw)
  To: Slava Ovsiienko, dev, Andrew Rybchenko
  Cc: Raslan Darawsheh, Matan Azrad, Michael Baum, NBU-Contact-Thomas Monjalon

On 4/30/2021 10:01 AM, Slava Ovsiienko wrote:
> Hi, Ferruh
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Friday, April 30, 2021 11:35
>> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Andrew
>> Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Cc: Raslan Darawsheh <rasland@nvidia.com>; Matan Azrad
>> <matan@nvidia.com>; Michael Baum <michaelba@nvidia.com>; NBU-
>> Contact-Thomas Monjalon <thomas@monjalon.net>
>> Subject: Re: [PATCH v2 1/5] net/mlx5: support flow count action handle
>>
>> On 4/29/2021 10:55 AM, Viacheslav Ovsiienko wrote:
>>> From: Michael Baum <michaelba@nvidia.com>
>>>
>>> Existing API supports counter action to count traffic of a single flow.
>>> The user can share the count action among different flows using the
>>> shared flag and the same counter ID in the count action configuration.
>>>
>>> Recent patch [1] introduced the indirect action API.
>>> Using this API, an action can be created as indirect, unattached to
>>> any flow rule.
>>> Multiple flows can then be created using the same indirect action.
>>> The new API also supports query operation of an indirect action.
>>>
>>> The new API is more efficient because the driver gets it's own handler
>>> for the count action instead of managing a mapping between the user ID
>>> to the driver handle.
>>>
>>> Support create, query and destroy indirect action operations for flow
>>> count action.
>>>
>>> Application will use the indirect action query operation to query this
>>> count action.
>>>
>>> In the meantime the old sharing mechanism (with the sharing flag)
>>> continues to be supported, and the user can choose the way he wants to
>>> share the counter.
>>> The new indirect action API is only supported in DevX, so sharing
>>> counter action in Verbs can only be done through the old mechanism.
>>>
>>
>> There is already a deprecation note to remove the 'shared' flag from counter
>> action [2] in favor of 'shared actions'.
>>
>> Shared action become "indirect actions" [3] which this patch implements
>>
>> Is it a good time to remove the old sharing mechanism from driver since
>> touching on it?
> 
> As I can see:
> - it is planned to 21.11 as deprecation note says

Yes, deprecation is for 21.11, but I though it might be easier for you to remove
it now while it is under rework, but I can see you prefer to keep it.

> - now there Is no support for new "indirect actions" with Verbs (not sure
>  we will provide), so shared counters API might be still useful
> - our CI did not migrate to the new counter approach yet (even DPDK patch is not merged yet 😊)
> - hence, l would prefer keep shared counter support (by id and flag) in 21.05 
>  (and very likely in 21.08), just to maintain maximal backward compatibility as long
> as possible
> 

OK

> With best regards,
> Slava
> 
>>
>>
>> [2]
>> https://git.dpdk.org/dpdk/tree/doc/guides/rel_notes/deprecation.rst?h=v2
>> 1.05-rc1#n95
>>
>> [3]
>> https://git.dpdk.org/dpdk/commit/?h=v21.05-rc1&id=4b61b8774be9
>>
>>
>>> [1] https://mails.dpdk.org/archives/dev/2020-July/174110.html
>>>
>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
>>> Acked-by: Matan Azrad <matan@nvidia.com>
>>
>> <...>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action.
  2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
                     ` (4 preceding siblings ...)
  2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 5/5] net/mlx5: use aging by counter when counter is existed Viacheslav Ovsiienko
@ 2021-04-30 10:43   ` Ferruh Yigit
  5 siblings, 0 replies; 18+ messages in thread
From: Ferruh Yigit @ 2021-04-30 10:43 UTC (permalink / raw)
  To: Viacheslav Ovsiienko, dev; +Cc: rasland, matan

On 4/29/2021 10:55 AM, Viacheslav Ovsiienko wrote:
> v1: http://patches.dpdk.org/project/dpdk/patch/20210426124250.42771-2-michaelba@nvidia.com/
> V2: patch rebase 
> 
> Michael Baum (5):
>   net/mlx5: support flow count action handle
>   app/testpmd: remove indirect RSS action query
>   app/testpmd: support indirect counter action query
>   net/mlx5: fix flow age event triggering
>   net/mlx5: use aging by counter when counter is existed
> 

Carried Ori's ack to testpmd patches from first version.

Series applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-04-30 10:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 12:42 [dpdk-dev] [PATCH 0/5] net/mlx5: add indirect count action Michael Baum
2021-04-26 12:42 ` [dpdk-dev] [PATCH 1/5] net/mlx5: support flow count action handle Michael Baum
2021-04-26 12:42 ` [dpdk-dev] [PATCH 2/5] app/testpmd: remove indirect RSS action query Michael Baum
2021-04-29 13:46   ` Ori Kam
2021-04-26 12:42 ` [dpdk-dev] [PATCH 3/5] app/testpmd: support indirect counter " Michael Baum
2021-04-29 13:45   ` Ori Kam
2021-04-26 12:42 ` [dpdk-dev] [PATCH 4/5] net/mlx5: fix flow age event triggering Michael Baum
2021-04-26 12:42 ` [dpdk-dev] [PATCH 5/5] net/mlx5: use aging by counter when counter is existed Michael Baum
2021-04-29  9:55 ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Viacheslav Ovsiienko
2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 1/5] net/mlx5: support flow count action handle Viacheslav Ovsiienko
2021-04-30  8:34     ` Ferruh Yigit
2021-04-30  9:01       ` Slava Ovsiienko
2021-04-30  9:22         ` Ferruh Yigit
2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: remove indirect RSS action query Viacheslav Ovsiienko
2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: support indirect counter " Viacheslav Ovsiienko
2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix flow age event triggering Viacheslav Ovsiienko
2021-04-29  9:55   ` [dpdk-dev] [PATCH v2 5/5] net/mlx5: use aging by counter when counter is existed Viacheslav Ovsiienko
2021-04-30 10:43   ` [dpdk-dev] [PATCH v2 0/5] Add support of indirect action API for count action Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).