Hi,
> -----Original Message-----
> From: Bing Zhao <bi...@nvidia.com>
> Sent: Monday, November 18, 2024 07:53
> To: Dariusz Sosnowski <dsosnow...@nvidia.com>; Slava Ovsiienko
> <viachesl...@nvidia.com>; dev@dpdk.org; Raslan Darawsheh
> <rasl...@nvidia.com>
> Cc: Ori Kam <or...@nvidia.com>; Suanming Mou <suanmi...@nvidia.com>;
> Matan Azrad <ma...@nvidia.com>; Maayan Kashani
> <mkash...@nvidia.com>
> Subject: [PATCH] net/mlx5: fix the leak of action data list
>
> In the actions construction of NT2HWS, the `masks` parameter is always set to
> NULL and all the actions will be translated in the "construct" stage as
> non-fixed
> ones.
Please replace NT2HWS in commit message with "non-template flow API".
>
> In the stage of translating actions template, the actions data would be
> allocated from the pool and managed in a list. The list would be released when
> destroying the template with the actions. In the NT2HWS implementation, the
> temporary template was freed directly and the actions will be destroyed with
> the flow rule deletion. No other rule would use this list anymore.
>
> The actions data in the list should be freed when the actions construction is
> done.
>
> Fixes: ff4064d5b1fe ("net/mlx5: support bulk actions in non-template flow")
> Cc: mkash...@nvidia.com
>
> Signed-off-by: Bing Zhao <bi...@nvidia.com>
> ---
> drivers/net/mlx5/mlx5_flow_hw.c | 63 ++++++++++++++++++++-------------
> 1 file changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_hw.c
> b/drivers/net/mlx5/mlx5_flow_hw.c index 50dbaa27ab..7233ac46c4 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> @@ -966,18 +966,15 @@ __flow_hw_actions_release(struct rte_eth_dev
> *dev, struct mlx5_hw_actions *acts) }
>
> /**
> - * Destroy DR actions created by action template.
> - *
> - * For DR actions created during table creation's action translate.
> - * Need to destroy the DR action when destroying the table.
> + * Release the action data back into the pool without destory any action.
> *
> * @param[in] dev
> * Pointer to the rte_eth_dev structure.
> * @param[in] acts
> * Pointer to the template HW steering DR actions.
> */
> -static void
> -__flow_hw_action_template_destroy(struct rte_eth_dev *dev, struct
> mlx5_hw_actions *acts)
> +static inline void
> +__flow_hw_act_data_flush(struct rte_eth_dev *dev, struct
> +mlx5_hw_actions *acts)
> {
> struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_action_construct_data *data; @@ -987,7 +984,23 @@
> __flow_hw_action_template_destroy(struct rte_eth_dev *dev, struct
> mlx5_hw_action
> LIST_REMOVE(data, next);
> mlx5_ipool_free(priv->acts_ipool, data->idx);
> }
> +}
>
> +/*
> + * Destroy DR actions created by action template.
> + *
> + * For DR actions created during table creation's action translate.
> + * Need to destroy the DR action when destroying the table.
> + *
> + * @param[in] dev
> + * Pointer to the rte_eth_dev structure.
> + * @param[in] acts
> + * Pointer to the template HW steering DR actions.
> + */
> +static void
> +__flow_hw_action_template_destroy(struct rte_eth_dev *dev, struct
> +mlx5_hw_actions *acts) {
> + __flow_hw_act_data_flush(dev, acts);
> __flow_hw_actions_release(dev, acts);
> }
>
> @@ -13492,14 +13505,14 @@ flow_nta_build_template_mask(const struct
> rte_flow_action actions[],
>
> static int
> flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
> - const struct rte_flow_attr *attr,
> - const struct rte_flow_action actions[],
> - struct rte_flow_hw *flow,
> - struct mlx5_flow_hw_action_params *ap,
> - struct mlx5_hw_actions *hw_acts,
> - uint64_t item_flags, uint64_t action_flags,
> - bool external,
> - struct rte_flow_error *error)
> + const struct rte_flow_attr *attr,
> + const struct rte_flow_action actions[],
> + struct rte_flow_hw *flow,
> + struct mlx5_flow_hw_action_params *ap,
> + struct mlx5_hw_actions *hw_acts,
> + uint64_t item_flags, uint64_t action_flags,
> + bool external,
> + struct rte_flow_error *error)
> {
> int ret = 0;
> uint32_t src_group = 0;
> @@ -13542,7 +13555,7 @@ flow_hw_translate_flow_actions(struct
> rte_eth_dev *dev,
> table = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*table), 0,
> SOCKET_ID_ANY);
> if (!table)
> return rte_flow_error_set(error, ENOMEM,
> RTE_FLOW_ERROR_TYPE_ACTION,
> - actions, "Failed to allocate dummy
> table");
> + actions, "Failed to allocate
> dummy table");
> at = __flow_hw_actions_template_create(dev, &template_attr,
> actions, masks, true, error);
> if (!at) {
> ret = -rte_errno;
> @@ -13556,27 +13569,29 @@ flow_hw_translate_flow_actions(struct
> rte_eth_dev *dev,
> memcpy(&table->cfg.attr.flow_attr, attr, sizeof(*attr));
> table->ats[0].action_template = at;
> ret = __flow_hw_translate_actions_template(dev, &table->cfg,
> hw_acts, at,
> - &table->mpctx, true, error);
> + &table->mpctx, true,
> error);
> if (ret)
> goto end;
> /* handle bulk actions register. */
> ret = flow_hw_encap_decap_resource_register(dev, table, hw_acts,
> flow, error);
> if (ret)
> - goto clean_up;
> + goto end;
> ret = flow_hw_modify_hdr_resource_register(dev, table, hw_acts,
> flow, error);
> if (ret)
> - goto clean_up;
> + goto end;
> table->ats[0].acts = *hw_acts;
> ret = flow_hw_actions_construct(dev, flow, ap,
> - &table->ats[0], item_flags, table,
> - actions, hw_acts->rule_acts, 0, error);
> + &table->ats[0], item_flags, table,
> + actions, hw_acts->rule_acts, 0,
> error);
> if (ret)
> - goto clean_up;
> + goto end;
> goto end;
> -clean_up:
> - /* Make sure that there is no garbage in the actions. */
> - __flow_hw_action_template_destroy(dev, hw_acts);
> end:
> + if (ret)
> + /* Make sure that there is no garbage in the actions. */
> + __flow_hw_action_template_destroy(dev, hw_acts);
> + else
> + __flow_hw_act_data_flush(dev, hw_acts);
> if (table)
> mlx5_free(table);
> if (at)
> --
> 2.34.1
Best regards,
Dariusz Sosnowski
