* [dpdk-dev] [PATCH 1/3] net/mlx5: fix unfreed memory on ASO age close
2020-11-18 8:59 [dpdk-dev] [PATCH 0/3] ASO age action fixes Dekel Peled
@ 2020-11-18 8:59 ` Dekel Peled
2020-11-18 8:59 ` [dpdk-dev] [PATCH 2/3] net/mlx5: fix input register for ASO object Dekel Peled
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Dekel Peled @ 2020-11-18 8:59 UTC (permalink / raw)
To: viacheslavo, shahafs, matan; +Cc: dev
Recent patch introduced the use of ASO flow hit action for age action.
The relevant management struct uses dynamically allocated memory.
This memory was not freed on closing.
This patch adds memory freeing as needed.
Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/net/mlx5/mlx5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index ede5fd44ab..627e511b12 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -419,7 +419,7 @@ mlx5_flow_aso_age_mng_close(struct mlx5_dev_ctx_shared *sh)
}
mlx5_free(sh->aso_age_mng->pools);
}
- memset(&sh->aso_age_mng, 0, sizeof(sh->aso_age_mng));
+ mlx5_free(sh->aso_age_mng);
}
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [dpdk-dev] [PATCH 2/3] net/mlx5: fix input register for ASO object
2020-11-18 8:59 [dpdk-dev] [PATCH 0/3] ASO age action fixes Dekel Peled
2020-11-18 8:59 ` [dpdk-dev] [PATCH 1/3] net/mlx5: fix unfreed memory on ASO age close Dekel Peled
@ 2020-11-18 8:59 ` Dekel Peled
2020-11-18 8:59 ` [dpdk-dev] [PATCH 3/3] common/mlx5: move to formal ASO action API Dekel Peled
2020-11-19 16:18 ` [dpdk-dev] [PATCH 0/3] ASO age action fixes Raslan Darawsheh
3 siblings, 0 replies; 5+ messages in thread
From: Dekel Peled @ 2020-11-18 8:59 UTC (permalink / raw)
To: viacheslavo, shahafs, matan; +Cc: dev
Existing code uses the hard-coded value REG_C_5 as input for function
mlx5dv_dr_action_create_flow_hit().
This patch updates function mlx5_flow_get_reg_id() to return the
selected REG_C value for ASO Flow Hit operation.
The returned value is used, after reducing offset REG_C_0, as input
for function mlx5dv_dr_action_create_flow_hit().
Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/net/mlx5/linux/mlx5_os.c | 36 +++++++++++++++++++-----------
drivers/net/mlx5/mlx5_flow.c | 1 +
drivers/net/mlx5/mlx5_flow.h | 1 +
drivers/net/mlx5/mlx5_flow_dv.c | 38 +++++++++++++++++++++++++-------
4 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 4b7fff4eff..468ef3356f 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1135,17 +1135,6 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
err = -err;
goto error;
}
-#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
- if (config->hca_attr.flow_hit_aso) {
- sh->flow_hit_aso_en = 1;
- err = mlx5_flow_aso_age_mng_init(sh);
- if (err) {
- err = -err;
- goto error;
- }
- DRV_LOG(DEBUG, "Flow Hit ASO is supported.");
- }
-#endif /* HAVE_MLX5DV_DR_ACTION_FLOW_HIT */
/* Check relax ordering support. */
if (!haswell_broadwell_cpu) {
sh->cmng.relaxed_ordering_write =
@@ -1192,8 +1181,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
DRV_LOG(WARNING, "No available register for"
" meter.");
} else {
- priv->mtr_color_reg = ffs(reg_c_mask) - 1 +
- REG_C_0;
+ /*
+ * The meter color register is used by the
+ * flow-hit feature as well.
+ * The flow-hit feature must use REG_C_3
+ * Prefer REG_C_3 if it is available.
+ */
+ if (reg_c_mask & (1 << (REG_C_3 - REG_C_0)))
+ priv->mtr_color_reg = REG_C_3;
+ else
+ priv->mtr_color_reg = ffs(reg_c_mask)
+ - 1 + REG_C_0;
priv->mtr_en = 1;
priv->mtr_reg_share =
config->hca_attr.qos.flow_meter_reg_share;
@@ -1202,6 +1200,18 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
}
}
#endif
+#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
+ if (config->hca_attr.flow_hit_aso &&
+ priv->mtr_color_reg == REG_C_3) {
+ sh->flow_hit_aso_en = 1;
+ err = mlx5_flow_aso_age_mng_init(sh);
+ if (err) {
+ err = -err;
+ goto error;
+ }
+ DRV_LOG(DEBUG, "Flow Hit ASO is supported.");
+ }
+#endif /* HAVE_MLX5DV_DR_ACTION_FLOW_HIT */
#if defined(HAVE_MLX5DV_DR) && defined(HAVE_MLX5_DR_CREATE_ACTION_FLOW_SAMPLE)
if (config->hca_attr.log_max_ft_sampler_num > 0 &&
config->dv_flow_en) {
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 33dbbd9eef..bd2e588187 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -777,6 +777,7 @@ mlx5_flow_get_reg_id(struct rte_eth_dev *dev,
return priv->mtr_color_reg != REG_C_2 ? REG_C_2 :
REG_C_3;
case MLX5_MTR_COLOR:
+ case MLX5_ASO_FLOW_HIT: /* Both features use the same REG_C. */
MLX5_ASSERT(priv->mtr_color_reg != REG_NON);
return priv->mtr_color_reg;
case MLX5_COPY_MARK:
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index afddcfc12c..0322db9adc 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -80,6 +80,7 @@ enum mlx5_feature_name {
MLX5_COPY_MARK,
MLX5_MTR_COLOR,
MLX5_MTR_SFX,
+ MLX5_ASO_FLOW_HIT,
};
/* Default queue number. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3d69957da7..ae8967ec58 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -9449,12 +9449,14 @@ flow_dv_age_pool_create(struct rte_eth_dev *dev,
*
* @param[in] dev
* Pointer to the Ethernet device structure.
+ * @param[out] error
+ * Pointer to the error structure.
*
* @return
* Index to ASO age action on success, 0 otherwise and rte_errno is set.
*/
static uint32_t
-flow_dv_aso_age_alloc(struct rte_eth_dev *dev)
+flow_dv_aso_age_alloc(struct rte_eth_dev *dev, struct rte_flow_error *error)
{
struct mlx5_priv *priv = dev->data->dev_private;
const struct mlx5_aso_age_pool *pool;
@@ -9469,7 +9471,9 @@ flow_dv_aso_age_alloc(struct rte_eth_dev *dev)
LIST_REMOVE(age_free, next);
} else if (!flow_dv_age_pool_create(dev, &age_free)) {
rte_spinlock_unlock(&mng->free_sl);
- return 0; /* 0 is an error.*/
+ rte_flow_error_set(error, rte_errno, RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "failed to create ASO age pool");
+ return 0; /* 0 is an error. */
}
rte_spinlock_unlock(&mng->free_sl);
pool = container_of
@@ -9477,15 +9481,30 @@ flow_dv_aso_age_alloc(struct rte_eth_dev *dev)
(age_free - age_free->offset), const struct mlx5_aso_age_pool,
actions);
if (!age_free->dr_action) {
+ int reg_c = mlx5_flow_get_reg_id(dev, MLX5_ASO_FLOW_HIT, 0,
+ error);
+
+ if (reg_c < 0) {
+ rte_flow_error_set(error, rte_errno,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "failed to get reg_c "
+ "for ASO flow hit");
+ return 0; /* 0 is an error. */
+ }
age_free->dr_action = mlx5_glue->dr_action_create_flow_hit
(pool->flow_hit_aso_obj->obj,
- age_free->offset, REG_C_5);
+ age_free->offset,
+ (reg_c - REG_C_0));
if (!age_free->dr_action) {
rte_errno = errno;
rte_spinlock_lock(&mng->free_sl);
LIST_INSERT_HEAD(&mng->free, age_free, next);
rte_spinlock_unlock(&mng->free_sl);
- return 0; /* 0 is an error.*/
+ rte_flow_error_set(error, rte_errno,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "failed to create ASO "
+ "flow hit action");
+ return 0; /* 0 is an error. */
}
}
__atomic_store_n(&age_free->refcnt, 1, __ATOMIC_RELAXED);
@@ -9499,18 +9518,21 @@ flow_dv_aso_age_alloc(struct rte_eth_dev *dev)
* Pointer to rte_eth_dev structure.
* @param[in] age
* Pointer to the aging action configuration.
+ * @param[out] error
+ * Pointer to the error structure.
*
* @return
* Index to flow counter on success, 0 otherwise.
*/
static uint32_t
flow_dv_translate_create_aso_age(struct rte_eth_dev *dev,
- const struct rte_flow_action_age *age)
+ const struct rte_flow_action_age *age,
+ struct rte_flow_error *error)
{
uint32_t age_idx = 0;
struct mlx5_aso_age_action *aso_age;
- age_idx = flow_dv_aso_age_alloc(dev);
+ age_idx = flow_dv_aso_age_alloc(dev, error);
if (!age_idx)
return 0;
aso_age = flow_aso_age_get_by_idx(dev, age_idx);
@@ -9822,7 +9844,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
case RTE_FLOW_ACTION_TYPE_AGE:
if (priv->sh->flow_hit_aso_en && attr->group) {
flow->age = flow_dv_translate_create_aso_age
- (dev, action->conf);
+ (dev, action->conf, error);
if (!flow->age)
return rte_flow_error_set
(error, rte_errno,
@@ -11512,7 +11534,7 @@ flow_dv_action_create(struct rte_eth_dev *dev,
MLX5_SHARED_ACTION_TYPE_OFFSET) | ret;
break;
case RTE_FLOW_ACTION_TYPE_AGE:
- ret = flow_dv_translate_create_aso_age(dev, action->conf);
+ ret = flow_dv_translate_create_aso_age(dev, action->conf, err);
idx = (MLX5_SHARED_ACTION_TYPE_AGE <<
MLX5_SHARED_ACTION_TYPE_OFFSET) | ret;
if (ret) {
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [dpdk-dev] [PATCH 3/3] common/mlx5: move to formal ASO action API
2020-11-18 8:59 [dpdk-dev] [PATCH 0/3] ASO age action fixes Dekel Peled
2020-11-18 8:59 ` [dpdk-dev] [PATCH 1/3] net/mlx5: fix unfreed memory on ASO age close Dekel Peled
2020-11-18 8:59 ` [dpdk-dev] [PATCH 2/3] net/mlx5: fix input register for ASO object Dekel Peled
@ 2020-11-18 8:59 ` Dekel Peled
2020-11-19 16:18 ` [dpdk-dev] [PATCH 0/3] ASO age action fixes Raslan Darawsheh
3 siblings, 0 replies; 5+ messages in thread
From: Dekel Peled @ 2020-11-18 8:59 UTC (permalink / raw)
To: viacheslavo, shahafs, matan; +Cc: dev
Existing code uses the previous API offered by rdma-core in order
to create ASO Flow Hit action.
A general API is now formally released, to create ASO action of any type.
This patch moves the MLX5 PMD code to use the formal API.
Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/common/mlx5/linux/meson.build | 4 +--
drivers/common/mlx5/linux/mlx5_glue.c | 38 ++++++++++++++++-----------
drivers/common/mlx5/linux/mlx5_glue.h | 6 ++---
drivers/net/mlx5/linux/mlx5_os.c | 4 +--
drivers/net/mlx5/mlx5_flow_dv.c | 11 +++++---
5 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
index 87f7bfda51..63b78e4bce 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -181,8 +181,8 @@ has_sym_args = [
[ 'HAVE_MLX5_DR_CREATE_ACTION_DEST_ARRAY', 'infiniband/mlx5dv.h',
'mlx5dv_dr_action_create_dest_array'],
[ 'HAVE_DEVLINK', 'linux/devlink.h', 'DEVLINK_GENL_NAME' ],
- [ 'HAVE_MLX5DV_DR_ACTION_FLOW_HIT', 'infiniband/mlx5dv.h',
- 'mlx5dv_dr_action_create_flow_hit'],
+ [ 'HAVE_MLX5_DR_CREATE_ACTION_ASO', 'infiniband/mlx5dv.h',
+ 'mlx5dv_dr_action_create_aso' ],
]
config = configuration_data()
foreach arg:has_sym_args
diff --git a/drivers/common/mlx5/linux/mlx5_glue.c b/drivers/common/mlx5/linux/mlx5_glue.c
index cc6670c5bd..8146c79287 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.c
+++ b/drivers/common/mlx5/linux/mlx5_glue.c
@@ -811,6 +811,27 @@ mlx5_glue_dv_modify_flow_action_meter(void *action,
#endif
}
+static void *
+mlx5_glue_dv_create_flow_action_aso(struct mlx5dv_dr_domain *domain,
+ void *aso_obj,
+ uint32_t offset,
+ uint32_t flags,
+ uint8_t return_reg_c)
+{
+#if defined(HAVE_MLX5DV_DR) && defined(HAVE_MLX5_DR_CREATE_ACTION_ASO)
+ return mlx5dv_dr_action_create_aso(domain, aso_obj, offset,
+ flags, return_reg_c);
+#else
+ (void)domain;
+ (void)aso_obj;
+ (void)offset;
+ (void)flags;
+ (void)return_reg_c;
+ errno = ENOTSUP;
+ return NULL;
+#endif
+}
+
static void *
mlx5_glue_dr_create_flow_action_default_miss(void)
{
@@ -1281,21 +1302,6 @@ mlx5_glue_dv_free_pp(struct mlx5dv_pp *pp)
#endif
}
-static void *
-mlx5_glue_dr_action_create_flow_hit(struct mlx5dv_devx_obj *devx_obj,
- uint32_t offset, uint8_t reg_c_index)
-{
-#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
- return mlx5dv_dr_action_create_flow_hit(devx_obj, offset, reg_c_index);
-#else
- (void)(devx_obj);
- (void)(offset);
- (void)(reg_c_index);
- errno = ENOTSUP;
- return NULL;
-#endif
-}
-
__rte_cache_aligned
const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
.version = MLX5_GLUE_VERSION,
@@ -1379,6 +1385,7 @@ const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
.dv_create_flow_action_tag = mlx5_glue_dv_create_flow_action_tag,
.dv_create_flow_action_meter = mlx5_glue_dv_create_flow_action_meter,
.dv_modify_flow_action_meter = mlx5_glue_dv_modify_flow_action_meter,
+ .dv_create_flow_action_aso = mlx5_glue_dv_create_flow_action_aso,
.dr_create_flow_action_default_miss =
mlx5_glue_dr_create_flow_action_default_miss,
.dv_destroy_flow = mlx5_glue_dv_destroy_flow,
@@ -1415,5 +1422,4 @@ const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
.dv_free_var = mlx5_glue_dv_free_var,
.dv_alloc_pp = mlx5_glue_dv_alloc_pp,
.dv_free_pp = mlx5_glue_dv_free_pp,
- .dr_action_create_flow_hit = mlx5_glue_dr_action_create_flow_hit,
};
diff --git a/drivers/common/mlx5/linux/mlx5_glue.h b/drivers/common/mlx5/linux/mlx5_glue.h
index c5d7853ea9..8be446a902 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.h
+++ b/drivers/common/mlx5/linux/mlx5_glue.h
@@ -344,9 +344,9 @@ struct mlx5_glue {
(void *domain,
size_t num_dest,
struct mlx5dv_dr_action_dest_attr *dests[]);
- void *(*dr_action_create_flow_hit)(struct mlx5dv_devx_obj *devx_obj,
- uint32_t offset,
- uint8_t reg_c_index);
+ void *(*dv_create_flow_action_aso)
+ (struct mlx5dv_dr_domain *domain, void *aso_obj,
+ uint32_t offset, uint32_t flags, uint8_t return_reg_c);
};
extern const struct mlx5_glue *mlx5_glue;
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 468ef3356f..2dfed4b16f 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1200,7 +1200,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
}
}
#endif
-#ifdef HAVE_MLX5DV_DR_ACTION_FLOW_HIT
+#ifdef HAVE_MLX5_DR_CREATE_ACTION_ASO
if (config->hca_attr.flow_hit_aso &&
priv->mtr_color_reg == REG_C_3) {
sh->flow_hit_aso_en = 1;
@@ -1211,7 +1211,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
}
DRV_LOG(DEBUG, "Flow Hit ASO is supported.");
}
-#endif /* HAVE_MLX5DV_DR_ACTION_FLOW_HIT */
+#endif /* HAVE_MLX5_DR_CREATE_ACTION_ASO */
#if defined(HAVE_MLX5DV_DR) && defined(HAVE_MLX5_DR_CREATE_ACTION_FLOW_SAMPLE)
if (config->hca_attr.log_max_ft_sampler_num > 0 &&
config->dv_flow_en) {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index ae8967ec58..b3e71fc564 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -9491,10 +9491,13 @@ flow_dv_aso_age_alloc(struct rte_eth_dev *dev, struct rte_flow_error *error)
"for ASO flow hit");
return 0; /* 0 is an error. */
}
- age_free->dr_action = mlx5_glue->dr_action_create_flow_hit
- (pool->flow_hit_aso_obj->obj,
- age_free->offset,
- (reg_c - REG_C_0));
+#ifdef HAVE_MLX5_DR_CREATE_ACTION_ASO
+ age_free->dr_action = mlx5_glue->dv_create_flow_action_aso
+ (priv->sh->rx_domain,
+ pool->flow_hit_aso_obj->obj, age_free->offset,
+ MLX5DV_DR_ACTION_FLAGS_ASO_FIRST_HIT_SET,
+ (reg_c - REG_C_0));
+#endif /* HAVE_MLX5_DR_CREATE_ACTION_ASO */
if (!age_free->dr_action) {
rte_errno = errno;
rte_spinlock_lock(&mng->free_sl);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH 0/3] ASO age action fixes
2020-11-18 8:59 [dpdk-dev] [PATCH 0/3] ASO age action fixes Dekel Peled
` (2 preceding siblings ...)
2020-11-18 8:59 ` [dpdk-dev] [PATCH 3/3] common/mlx5: move to formal ASO action API Dekel Peled
@ 2020-11-19 16:18 ` Raslan Darawsheh
3 siblings, 0 replies; 5+ messages in thread
From: Raslan Darawsheh @ 2020-11-19 16:18 UTC (permalink / raw)
To: Dekel Peled, Slava Ovsiienko, Shahaf Shuler, Matan Azrad; +Cc: dev@dpdk.org
Hi,
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Wednesday, November 18, 2020 11:00 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Shahaf Shuler
> <shahafs@nvidia.com>; Matan Azrad <matan@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 0/3] ASO age action fixes
>
> The age action using ASO was recently introduced in MLX5 PMD.
> This series includes fixes for this feature, repairing issues found
> and updating it to the released rdma-core API.
>
> Dekel Peled (3):
> net/mlx5: fix unfreed memory on ASO age close
> net/mlx5: fix input register for ASO object
> common/mlx5: move to formal ASO action API
>
> drivers/common/mlx5/linux/meson.build | 4 +--
> drivers/common/mlx5/linux/mlx5_glue.c | 38 ++++++++++++----------
> drivers/common/mlx5/linux/mlx5_glue.h | 6 ++--
> drivers/net/mlx5/linux/mlx5_os.c | 36 +++++++++++++--------
> drivers/net/mlx5/mlx5.c | 2 +-
> drivers/net/mlx5/mlx5_flow.c | 1 +
> drivers/net/mlx5/mlx5_flow.h | 1 +
> drivers/net/mlx5/mlx5_flow_dv.c | 45 +++++++++++++++++++++------
> 8 files changed, 88 insertions(+), 45 deletions(-)
>
> --
> 2.25.1
Series applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
^ permalink raw reply [flat|nested] 5+ messages in thread