Re: [PATCH net-next 04/12] net/mlx5: fs, add counter object to flow destination

[Tariq Toukan <ttoukan.linux@gmail.com>]

On 12/12/2024 19:20, Simon Horman wrote:
> On Wed, Dec 11, 2024 at 03:42:15PM +0200, Tariq Toukan wrote:
>> From: Moshe Shemesh <moshe@nvidia.com>
>>
>> Currently mlx5_flow_destination includes counter_id which is assigned in
>> case we use flow counter on the flow steering rule. However, counter_id
>> is not enough data in case of using HW Steering. Thus, have mlx5_fc
>> object as part of mlx5_flow_destination instead of counter_id and assign
>> it where needed.
>>
>> In case counter_id is received from user space, create a local counter
>> object to represent it.
>>
>> Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
>> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
>> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> 
> Unfortunately, I think that this misses two counter_id instances
> in mlx5_vnet.c and the following is needed:
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 5f581e71e201..36099047560d 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1952,7 +1952,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
>   		goto out_free;
>   
>   #if defined(CONFIG_MLX5_VDPA_STEERING_DEBUG)
> -	dests[1].counter_id = mlx5_fc_id(node->ucast_counter.counter);
> +	dests[1].counter = node->ucast_counter.counter;
>   #endif
>   	node->ucast_rule = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dests, NUM_DESTS);
>   	if (IS_ERR(node->ucast_rule)) {
> @@ -1961,7 +1961,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
>   	}
>   
>   #if defined(CONFIG_MLX5_VDPA_STEERING_DEBUG)
> -	dests[1].counter_id = mlx5_fc_id(node->mcast_counter.counter);
> +	dests[1].counter = node->mcast_counter.counter;
>   #endif
>   
>   	memset(dmac_c, 0, ETH_ALEN);
> 
> You can observe this with an allmodconfig build.
> 

Thanks, will fix.

> 
> Also, please consider including a "Returns:" section in
> the Kernel doc of mlx5_fc_local_create().
> 

I'll add.