[v6] net/mlx5: support query of AGE action

Message ID c992bb50d5cceb53548793effba6e8dda7942d35.1603115460.git.dekelp@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series [v6] net/mlx5: support query of AGE action |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Dekel Peled Oct. 19, 2020, 1:52 p.m. UTC
Recent patch [1] adds to ethdev the API for query of age action.
This patch implements in MLX5 PMD the query of age action using
this API.

[1] https://mails.dpdk.org/archives/dev/2020-October/184864.html
---
v2: Update age resolution to seconds,
v3: Replace rte_atomic ops with c11 atomics.
v4: Remove redundant blank line in RN.
v5: Fix pointer type compilation issue.
v6: Access time count atomically.
---

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/rel_notes/release_20_11.rst |  8 ++++
 drivers/net/mlx5/mlx5.h                | 21 ++++-----
 drivers/net/mlx5/mlx5_flow.c           | 23 +++++++---
 drivers/net/mlx5/mlx5_flow_dv.c        | 84 +++++++++++++++++++++++++---------
 4 files changed, 97 insertions(+), 39 deletions(-)
  

Comments

Raslan Darawsheh Oct. 20, 2020, 7:18 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Monday, October 19, 2020 4:53 PM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Shahaf Shuler
> <shahafs@nvidia.com>; Matan Azrad <matan@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v6] net/mlx5: support query of AGE action
> 
> Recent patch [1] adds to ethdev the API for query of age action.
> This patch implements in MLX5 PMD the query of age action using
> this API.
> 
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail
> s.dpdk.org%2Farchives%2Fdev%2F2020-
> October%2F184864.html&amp;data=04%7C01%7Crasland%40nvidia.com%7C
> 3eb87054b4ea46d4bfe008d87436eddd%7C43083d15727340c1b7db39efd9ccc1
> 7a%7C0%7C0%7C637387126559473041%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C1000&amp;sdata=CuwWLpphxm3uyQEHkAstAyqUOXfoodfnAvOQmfZpxA
> 0%3D&amp;reserved=0
> ---
> v2: Update age resolution to seconds,
> v3: Replace rte_atomic ops with c11 atomics.
> v4: Remove redundant blank line in RN.
> v5: Fix pointer type compilation issue.
> v6: Access time count atomically.
> ---
> 
> Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  doc/guides/rel_notes/release_20_11.rst |  8 ++++
>  drivers/net/mlx5/mlx5.h                | 21 ++++-----
>  drivers/net/mlx5/mlx5_flow.c           | 23 +++++++---
>  drivers/net/mlx5/mlx5_flow_dv.c        | 84 +++++++++++++++++++++++++-
> --------
>  4 files changed, 97 insertions(+), 39 deletions(-)
> 

Patch applied to next-net-mlx,

Kindest regards
Raslan Darawsheh
  

Patch

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 0ea5541..f833526 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -310,6 +310,14 @@  New Features
   * Extern objects and functions can be plugged into the pipeline.
   * Transaction-oriented table updates.
 
+* **Updated Mellanox mlx5 driver.**
+
+  Updated Mellanox mlx5 driver with new features and improvements, including:
+
+  * Updated the supported timeout for Age action to the maximal value supported
+    by rte_flow API.
+  * Added support of Age action query.
+
 * **Add new AVX512 specific classify algorithms for ACL library.**
 
   * Added new ``RTE_ACL_CLASSIFY_AVX512X16`` vector implementation,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 1408cf9..afa2f31 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -276,7 +276,6 @@  struct mlx5_drop {
 #define CNT_SIZE (sizeof(struct mlx5_flow_counter))
 #define CNTEXT_SIZE (sizeof(struct mlx5_flow_counter_ext))
 #define AGE_SIZE (sizeof(struct mlx5_age_param))
-#define MLX5_AGING_TIME_DELAY	7
 #define CNT_POOL_TYPE_EXT	(1 << 0)
 #define CNT_POOL_TYPE_AGE	(1 << 1)
 #define IS_EXT_POOL(pool) (((pool)->type) & CNT_POOL_TYPE_EXT)
@@ -315,9 +314,7 @@  struct mlx5_drop {
  */
 #define POOL_IDX_INVALID UINT16_MAX
 
-struct mlx5_flow_counter_pool;
-
-/*age status*/
+/* Age status. */
 enum {
 	AGE_FREE, /* Initialized state. */
 	AGE_CANDIDATE, /* Counter assigned to flows. */
@@ -337,10 +334,11 @@  enum {
 
 /* Counter age parameter. */
 struct mlx5_age_param {
-	rte_atomic16_t state; /**< Age state. */
+	uint16_t state; /**< Age state (atomically accessed). */
 	uint16_t port_id; /**< Port id of the counter. */
-	uint32_t timeout:15; /**< Age timeout in unit of 0.1sec. */
-	uint32_t expire:16; /**< Expire time(0.1sec) in the future. */
+	uint32_t timeout:24; /**< Aging timeout in seconds. */
+	uint32_t sec_since_last_hit;
+	/**< Time in seconds since last hit (atomically accessed). */
 	void *context; /**< Flow counter age context. */
 };
 
@@ -349,7 +347,6 @@  struct flow_counter_stats {
 	uint64_t bytes;
 };
 
-struct mlx5_flow_counter_pool;
 /* Generic counters information. */
 struct mlx5_flow_counter {
 	TAILQ_ENTRY(mlx5_flow_counter) next;
@@ -391,6 +388,8 @@  struct mlx5_flow_counter_pool {
 		rte_atomic64_t a64_dcs;
 	};
 	/* The devx object of the minimum counter ID. */
+	uint64_t time_of_last_age_check;
+	/* System time (from rte_rdtsc()) read in the last aging check. */
 	uint32_t index:28; /* Pool index in container. */
 	uint32_t type:2; /* Memory type behind the counter array. */
 	uint32_t skip_cnt:1; /* Pool contains skipped counter. */
@@ -400,8 +399,6 @@  struct mlx5_flow_counter_pool {
 	struct mlx5_counter_stats_raw *raw_hw; /* The raw on HW working. */
 };
 
-struct mlx5_counter_stats_raw;
-
 /* Memory management structure for group of counter statistics raws. */
 struct mlx5_counter_stats_mem_mng {
 	LIST_ENTRY(mlx5_counter_stats_mem_mng) next;
@@ -463,10 +460,12 @@  struct mlx5_flow_default_miss_resource {
 	((age_info)->flags & (1 << (BIT)))
 #define GET_PORT_AGE_INFO(priv) \
 	(&((priv)->sh->port[(priv)->dev_port - 1].age_info))
+/* Current time in seconds. */
+#define MLX5_CURR_TIME_SEC	(rte_rdtsc() / rte_get_tsc_hz())
 
 /* Aging information for per port. */
 struct mlx5_age_info {
-	uint8_t flags; /*Indicate if is new event or need be trigered*/
+	uint8_t flags; /* Indicate if is new event or need to be triggered. */
 	struct mlx5_counters aged_counters; /* Aged flow counter list. */
 	rte_spinlock_t aged_sl; /* Aged flow counter list lock. */
 };
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 62cd0ee..c56dac8 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -6694,7 +6694,7 @@  struct mlx5_meter_domains_infos *
 	offset = batch ? 0 : dcs->id % MLX5_COUNTERS_PER_POOL;
 	/*
 	 * Identify the counters released between query trigger and query
-	 * handle more effiecntly. The counter released in this gap period
+	 * handle more efficiently. The counter released in this gap period
 	 * should wait for a new round of query as the new arrived packets
 	 * will not be taken into account.
 	 */
@@ -6748,19 +6748,26 @@  struct mlx5_meter_domains_infos *
 	struct mlx5_age_param *age_param;
 	struct mlx5_counter_stats_raw *cur = pool->raw_hw;
 	struct mlx5_counter_stats_raw *prev = pool->raw;
-	uint16_t curr = rte_rdtsc() / (rte_get_tsc_hz() / 10);
+	const uint64_t curr_time = MLX5_CURR_TIME_SEC;
+	const uint32_t time_delta = curr_time - pool->time_of_last_age_check;
+	uint16_t expected = AGE_CANDIDATE;
 	uint32_t i;
 
+	pool->time_of_last_age_check = curr_time;
 	for (i = 0; i < MLX5_COUNTERS_PER_POOL; ++i) {
 		cnt = MLX5_POOL_GET_CNT(pool, i);
 		age_param = MLX5_CNT_TO_AGE(cnt);
-		if (rte_atomic16_read(&age_param->state) != AGE_CANDIDATE)
+		if (__atomic_load_n(&age_param->state,
+				    __ATOMIC_RELAXED) != AGE_CANDIDATE)
 			continue;
 		if (cur->data[i].hits != prev->data[i].hits) {
-			age_param->expire = curr + age_param->timeout;
+			__atomic_store_n(&age_param->sec_since_last_hit, 0,
+					 __ATOMIC_RELAXED);
 			continue;
 		}
-		if ((uint16_t)(curr - age_param->expire) >= (UINT16_MAX / 2))
+		if (__atomic_add_fetch(&age_param->sec_since_last_hit,
+				       time_delta,
+				       __ATOMIC_RELAXED) <= age_param->timeout)
 			continue;
 		/**
 		 * Hold the lock first, or if between the
@@ -6771,8 +6778,10 @@  struct mlx5_meter_domains_infos *
 		priv = rte_eth_devices[age_param->port_id].data->dev_private;
 		age_info = GET_PORT_AGE_INFO(priv);
 		rte_spinlock_lock(&age_info->aged_sl);
-		if (rte_atomic16_cmpset((volatile uint16_t *)&age_param->state,
-					AGE_CANDIDATE, AGE_TMOUT)) {
+		if (__atomic_compare_exchange_n(&age_param->state, &expected,
+						AGE_TMOUT, false,
+						__ATOMIC_RELAXED,
+						__ATOMIC_RELAXED)) {
 			TAILQ_INSERT_TAIL(&age_info->aged_counters, cnt, next);
 			MLX5_AGE_SET(age_info, MLX5_AGE_EVENT_NEW);
 		}
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index b7ad847..d3a3f23 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4179,14 +4179,14 @@  struct field_modify_info modify_tcp[] = {
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "configuration cannot be null");
-	if (age->timeout >= UINT16_MAX / 2 / 10)
-		return rte_flow_error_set(error, ENOTSUP,
+	if (!(age->timeout))
+		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
-					  "Max age time: 3275 seconds");
+					  "invalid timeout value 0");
 	if (action_flags & MLX5_FLOW_ACTION_AGE)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
-					  "Duplicate age ctions set");
+					  "duplicate age actions set");
 	return 0;
 }
 
@@ -4926,6 +4926,7 @@  struct field_modify_info modify_tcp[] = {
 	TAILQ_INIT(&pool->counters[1]);
 	TAILQ_INSERT_HEAD(&cont->pool_list, pool, next);
 	pool->index = n_valid;
+	pool->time_of_last_age_check = MLX5_CURR_TIME_SEC;
 	cont->pools[n_valid] = pool;
 	if (!batch) {
 		int base = RTE_ALIGN_FLOOR(dcs->id, MLX5_COUNTERS_PER_POOL);
@@ -5048,7 +5049,7 @@  struct field_modify_info modify_tcp[] = {
 			       (MLX5_CNT_CONTAINER
 			       (priv->sh, batch, (age ^ 0x1)), dcs->id);
 			/*
-			 * Pool eixsts, counter will be added to the other
+			 * Pool exists, counter will be added to the other
 			 * container, need to reallocate it later.
 			 */
 			if (pool) {
@@ -5329,11 +5330,13 @@  struct field_modify_info modify_tcp[] = {
 	struct mlx5_age_info *age_info;
 	struct mlx5_age_param *age_param;
 	struct mlx5_priv *priv = dev->data->dev_private;
+	uint16_t expected = AGE_CANDIDATE;
 
 	age_info = GET_PORT_AGE_INFO(priv);
 	age_param = flow_dv_counter_idx_get_age(dev, counter);
-	if (!rte_atomic16_cmpset((volatile uint16_t *)&age_param->state,
-				 AGE_CANDIDATE, AGE_FREE)) {
+	if (!__atomic_compare_exchange_n(&age_param->state, &expected,
+					 AGE_FREE, false, __ATOMIC_RELAXED,
+					 __ATOMIC_RELAXED)) {
 		/**
 		 * We need the lock even it is age timeout,
 		 * since counter may still in process.
@@ -5341,7 +5344,7 @@  struct field_modify_info modify_tcp[] = {
 		rte_spinlock_lock(&age_info->aged_sl);
 		TAILQ_REMOVE(&age_info->aged_counters, cnt, next);
 		rte_spinlock_unlock(&age_info->aged_sl);
-		rte_atomic16_set(&age_param->state, AGE_FREE);
+		__atomic_store_n(&age_param->state, AGE_FREE, __ATOMIC_RELAXED);
 	}
 }
 
@@ -8531,22 +8534,12 @@  struct field_modify_info modify_tcp[] = {
 	if (!counter || age == NULL)
 		return counter;
 	age_param  = flow_dv_counter_idx_get_age(dev, counter);
-	/*
-	 * The counter age accuracy may have a bit delay. Have 3/4
-	 * second bias on the timeount in order to let it age in time.
-	 */
 	age_param->context = age->context ? age->context :
 		(void *)(uintptr_t)(dev_flow->flow_idx);
-	/*
-	 * The counter age accuracy may have a bit delay. Have 3/4
-	 * second bias on the timeount in order to let it age in time.
-	 */
-	age_param->timeout = age->timeout * 10 - MLX5_AGING_TIME_DELAY;
-	/* Set expire time in unit of 0.1 sec. */
+	age_param->timeout = age->timeout;
 	age_param->port_id = dev->data->port_id;
-	age_param->expire = age_param->timeout +
-			rte_rdtsc() / (rte_get_tsc_hz() / 10);
-	rte_atomic16_set(&age_param->state, AGE_CANDIDATE);
+	__atomic_store_n(&age_param->sec_since_last_hit, 0, __ATOMIC_RELAXED);
+	__atomic_store_n(&age_param->state, AGE_CANDIDATE, __ATOMIC_RELAXED);
 	return counter;
 }
 /**
@@ -10955,6 +10948,52 @@  struct field_modify_info modify_tcp[] = {
 }
 
 /**
+ * Query a flow rule AGE action for aging information.
+ *
+ * @param[in] dev
+ *   Pointer to Ethernet device.
+ * @param[in] flow
+ *   Pointer to the sub flow.
+ * @param[out] data
+ *   data retrieved by the query.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_query_age(struct rte_eth_dev *dev, struct rte_flow *flow,
+		  void *data, struct rte_flow_error *error)
+{
+	struct rte_flow_query_age *resp = data;
+
+	if (flow->counter) {
+		struct mlx5_age_param *age_param =
+				flow_dv_counter_idx_get_age(dev, flow->counter);
+
+		if (!age_param || !age_param->timeout)
+			return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					 NULL, "cannot read age 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;
+	}
+	return rte_flow_error_set(error, EINVAL,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL,
+				  "age data not available");
+}
+
+/**
  * Query a flow.
  *
  * @see rte_flow_query()
@@ -10976,6 +11015,9 @@  struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = flow_dv_query_count(dev, flow, data, error);
 			break;
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			ret = flow_dv_query_age(dev, flow, data, error);
+			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,