[v2,1/3] net/mlx5: separate aging counter pool range

Message ID 1595404727-164521-1-git-send-email-suanmingm@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series [v2,1/3] net/mlx5: separate aging counter pool range |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Suanming Mou July 22, 2020, 7:58 a.m. UTC
  Currently, when allocate the counter or counter based age from group 0,
counter and age may share the same counter dcs ID range. Both age and
pure counter need to sync up with each other's container to check if
the ID range exists and update the min_dcs.

It comes two disadvantages:
1. If the ID range is shared, this counter range will be queried twice
both from age and pure counter container in 1s.
2. The same range counter check between the two container makes the
counter allocate sync min_dcs time to time with extra min_dcs updating.

This patch avoid the same ID range to be shared when allocate the new
pool. If the same ID range exists in other container, just add the
counter to the other container until get new range which saves the
min_dcs sync up time to time.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 69 +++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 41 deletions(-)
  

Comments

Raslan Darawsheh July 22, 2020, 11:35 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Wednesday, July 22, 2020 10:59 AM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> <matan@mellanox.com>
> Cc: Raslan Darawsheh <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH v2 1/3] net/mlx5: separate aging counter pool range
> 
> Currently, when allocate the counter or counter based age from group 0,
> counter and age may share the same counter dcs ID range. Both age and
> pure counter need to sync up with each other's container to check if
> the ID range exists and update the min_dcs.
> 
> It comes two disadvantages:
> 1. If the ID range is shared, this counter range will be queried twice
> both from age and pure counter container in 1s.
> 2. The same range counter check between the two container makes the
> counter allocate sync min_dcs time to time with extra min_dcs updating.
> 
> This patch avoid the same ID range to be shared when allocate the new
> pool. If the same ID range exists in other container, just add the
> counter to the other container until get new range which saves the
> min_dcs sync up time to time.
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 69 +++++++++++++++++------------------
> ------
>  1 file changed, 28 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index f0cc7ad..2fc4457 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -4408,39 +4408,6 @@ struct field_modify_info modify_tcp[] = {
>  }
> 
>  /**
> - * Update the minimum dcs-id for aged or no-aged counter pool.
> - *
> - * @param[in] dev
> - *   Pointer to the Ethernet device structure.
> - * @param[in] pool
> - *   Current counter pool.
> - * @param[in] batch
> - *   Whether the pool is for counter that was allocated by batch command.
> - * @param[in] age
> - *   Whether the counter is for aging.
> - */
> -static void
> -flow_dv_counter_update_min_dcs(struct rte_eth_dev *dev,
> -			struct mlx5_flow_counter_pool *pool,
> -			uint32_t batch, uint32_t age)
> -{
> -	struct mlx5_priv *priv = dev->data->dev_private;
> -	struct mlx5_flow_counter_pool *other;
> -	struct mlx5_pools_container *cont;
> -
> -	cont = MLX5_CNT_CONTAINER(priv->sh, batch, (age ^ 0x1));
> -	other = flow_dv_find_pool_by_id(cont, pool->min_dcs->id);
> -	if (!other)
> -		return;
> -	if (pool->min_dcs->id < other->min_dcs->id) {
> -		rte_atomic64_set(&other->a64_dcs,
> -			rte_atomic64_read(&pool->a64_dcs));
> -	} else {
> -		rte_atomic64_set(&pool->a64_dcs,
> -			rte_atomic64_read(&other->a64_dcs));
> -	}
> -}
> -/**
>   * Prepare a new counter and/or a new counter pool.
>   *
>   * @param[in] dev
> @@ -4467,31 +4434,50 @@ struct field_modify_info modify_tcp[] = {
>  	struct mlx5_counters tmp_tq;
>  	struct mlx5_devx_obj *dcs = NULL;
>  	struct mlx5_flow_counter *cnt;
> +	uint32_t add2other;
>  	uint32_t i;
> 
>  	cont = MLX5_CNT_CONTAINER(priv->sh, batch, age);
>  	if (!batch) {
> +retry:
> +		add2other = 0;
>  		/* bulk_bitmap must be 0 for single counter allocation. */
>  		dcs = mlx5_devx_cmd_flow_counter_alloc(priv->sh->ctx, 0);
>  		if (!dcs)
>  			return NULL;
>  		pool = flow_dv_find_pool_by_id(cont, dcs->id);
> +		/* Check if counter belongs to exist pool ID range. */
>  		if (!pool) {
> -			pool = flow_dv_pool_create(dev, dcs, batch, age);
> -			if (!pool) {
> -				mlx5_devx_cmd_destroy(dcs);
> -				return NULL;
> +			pool = flow_dv_find_pool_by_id
> +			       (MLX5_CNT_CONTAINER
> +			       (priv->sh, batch, (age ^ 0x1)), dcs->id);
> +			/*
> +			 * Pool eixsts, counter will be added to the other
> +			 * container, need to reallocate it later.
> +			 */
> +			if (pool) {
> +				add2other = 1;
> +			} else {
> +				pool = flow_dv_pool_create(dev, dcs, batch,
> +							   age);
> +				if (!pool) {
> +					mlx5_devx_cmd_destroy(dcs);
> +					return NULL;
> +				}
>  			}
> -		} else if (dcs->id < pool->min_dcs->id) {
> +		}
> +		if (dcs->id < pool->min_dcs->id)
>  			rte_atomic64_set(&pool->a64_dcs,
>  					 (int64_t)(uintptr_t)dcs);
> -		}
> -		flow_dv_counter_update_min_dcs(dev,
> -						pool, batch, age);
>  		i = dcs->id % MLX5_COUNTERS_PER_POOL;
>  		cnt = MLX5_POOL_GET_CNT(pool, i);
>  		cnt->pool = pool;
>  		MLX5_GET_POOL_CNT_EXT(pool, i)->dcs = dcs;
> +		if (add2other) {
> +			TAILQ_INSERT_TAIL(&pool->counters[pool-
> >query_gen],
> +					  cnt, next);
> +			goto retry;
> +		}
>  		*cnt_free = cnt;
>  		return pool;
>  	}
> @@ -9985,3 +9971,4 @@ struct field_modify_info modify_tcp[] = {
>  };
> 
>  #endif /* HAVE_IBV_FLOW_DV_SUPPORT */
> +
> --
> 1.8.3.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f0cc7ad..2fc4457 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4408,39 +4408,6 @@  struct field_modify_info modify_tcp[] = {
 }
 
 /**
- * Update the minimum dcs-id for aged or no-aged counter pool.
- *
- * @param[in] dev
- *   Pointer to the Ethernet device structure.
- * @param[in] pool
- *   Current counter pool.
- * @param[in] batch
- *   Whether the pool is for counter that was allocated by batch command.
- * @param[in] age
- *   Whether the counter is for aging.
- */
-static void
-flow_dv_counter_update_min_dcs(struct rte_eth_dev *dev,
-			struct mlx5_flow_counter_pool *pool,
-			uint32_t batch, uint32_t age)
-{
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_flow_counter_pool *other;
-	struct mlx5_pools_container *cont;
-
-	cont = MLX5_CNT_CONTAINER(priv->sh, batch, (age ^ 0x1));
-	other = flow_dv_find_pool_by_id(cont, pool->min_dcs->id);
-	if (!other)
-		return;
-	if (pool->min_dcs->id < other->min_dcs->id) {
-		rte_atomic64_set(&other->a64_dcs,
-			rte_atomic64_read(&pool->a64_dcs));
-	} else {
-		rte_atomic64_set(&pool->a64_dcs,
-			rte_atomic64_read(&other->a64_dcs));
-	}
-}
-/**
  * Prepare a new counter and/or a new counter pool.
  *
  * @param[in] dev
@@ -4467,31 +4434,50 @@  struct field_modify_info modify_tcp[] = {
 	struct mlx5_counters tmp_tq;
 	struct mlx5_devx_obj *dcs = NULL;
 	struct mlx5_flow_counter *cnt;
+	uint32_t add2other;
 	uint32_t i;
 
 	cont = MLX5_CNT_CONTAINER(priv->sh, batch, age);
 	if (!batch) {
+retry:
+		add2other = 0;
 		/* bulk_bitmap must be 0 for single counter allocation. */
 		dcs = mlx5_devx_cmd_flow_counter_alloc(priv->sh->ctx, 0);
 		if (!dcs)
 			return NULL;
 		pool = flow_dv_find_pool_by_id(cont, dcs->id);
+		/* Check if counter belongs to exist pool ID range. */
 		if (!pool) {
-			pool = flow_dv_pool_create(dev, dcs, batch, age);
-			if (!pool) {
-				mlx5_devx_cmd_destroy(dcs);
-				return NULL;
+			pool = flow_dv_find_pool_by_id
+			       (MLX5_CNT_CONTAINER
+			       (priv->sh, batch, (age ^ 0x1)), dcs->id);
+			/*
+			 * Pool eixsts, counter will be added to the other
+			 * container, need to reallocate it later.
+			 */
+			if (pool) {
+				add2other = 1;
+			} else {
+				pool = flow_dv_pool_create(dev, dcs, batch,
+							   age);
+				if (!pool) {
+					mlx5_devx_cmd_destroy(dcs);
+					return NULL;
+				}
 			}
-		} else if (dcs->id < pool->min_dcs->id) {
+		}
+		if (dcs->id < pool->min_dcs->id)
 			rte_atomic64_set(&pool->a64_dcs,
 					 (int64_t)(uintptr_t)dcs);
-		}
-		flow_dv_counter_update_min_dcs(dev,
-						pool, batch, age);
 		i = dcs->id % MLX5_COUNTERS_PER_POOL;
 		cnt = MLX5_POOL_GET_CNT(pool, i);
 		cnt->pool = pool;
 		MLX5_GET_POOL_CNT_EXT(pool, i)->dcs = dcs;
+		if (add2other) {
+			TAILQ_INSERT_TAIL(&pool->counters[pool->query_gen],
+					  cnt, next);
+			goto retry;
+		}
 		*cnt_free = cnt;
 		return pool;
 	}
@@ -9985,3 +9971,4 @@  struct field_modify_info modify_tcp[] = {
 };
 
 #endif /* HAVE_IBV_FLOW_DV_SUPPORT */
+