Re: [net-next 05/15] net/mlx5: fs, move hardware fte deletion function reset

[Jacob Keller <jacob.e.keller@intel.com>]

On 9/11/2024 1:17 PM, Saeed Mahameed wrote:
> From: Mark Bloch <mbloch@nvidia.com>
> 
> Downstream patches will need this as we might not want to reset
> it when a pending rule is connected to the FTE.
> 

At first I didn't quite understand the motivation here but...

> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index e32725487702..899d91577a54 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -683,6 +683,8 @@ static void del_hw_fte(struct fs_node *node)
>  				       fte->index, fg->id);
>  		node->active = false;
>  	}
> +	/* Avoid double call to del_hw_fte */
> +	fte->node.del_hw_func = NULL;
>  }
>  
>  static void del_sw_fte(struct fs_node *node)
> @@ -2265,8 +2267,6 @@ void mlx5_del_flow_rules(struct mlx5_flow_handle *handle)
>  		tree_remove_node(&handle->rule[i]->node, true);
>  	if (list_empty(&fte->node.children)) {
>  		fte->node.del_hw_func(&fte->node);
> -		/* Avoid double call to del_hw_fte */
> -		fte->node.del_hw_func = NULL;

I see. You were previously clearing del_hw_func after calling the
function. Now, it gets cleared by the del_hw_func implementation?

That does feel slightly brittle to me when thinking about this as an
API. On the other hand, it also seems reasonable to ensure that the
function gets cleared whenever you call it since it should only be
called once?

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  		up_write_ref_node(&fte->node, false);
>  		tree_put_node(&fte->node, false);
>  	} else if (fte->dests_size) {