Re: [dpdk-dev] [PATCH] net/mlx5: fix flow creation on port start

["Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>]

Hi Shahaf,

On Sun, Nov 05, 2017 at 05:44:49PM +0200, Shahaf Shuler wrote:
> While the PMD avoids from creating hash RXQ with no hash fields and
> array of queues after the port was allready started, it lacks such
> protection when re-creating the flows after the port restarts.
> 
> This may lead to inconsist behaviour for flows depending if they were
> created before or after the port start.
> 
> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> Cc: nelio.laranjeiro@6wind.com
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 5f49bf5ff..b0a196a4b 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2290,22 +2290,27 @@ priv_flow_start(struct priv *priv, struct mlx5_flows *list)
>  			continue;
>  		}
>  		for (i = 0; i != hash_rxq_init_n; ++i) {
> +			uint64_t hash_fields = hash_rxq_init[i].hash_fields;
>  			if (!flow->frxq[i].ibv_attr)
>  				continue;
>  			flow->frxq[i].hrxq =
>  				mlx5_priv_hrxq_get(priv, flow->rss_conf.rss_key,
>  						   flow->rss_conf.rss_key_len,
> -						   hash_rxq_init[i].hash_fields,
> +						   hash_fields,
>  						   (*flow->queues),
> -						   flow->queues_n);
> +						   hash_fields ?
> +						   flow->queues_n :
> +						   1);
>  			if (flow->frxq[i].hrxq)
>  				goto flow_create;
>  			flow->frxq[i].hrxq =
>  				mlx5_priv_hrxq_new(priv, flow->rss_conf.rss_key,
>  						   flow->rss_conf.rss_key_len,
> -						   hash_rxq_init[i].hash_fields,
> +						   hash_fields,
>  						   (*flow->queues),
> -						   flow->queues_n);
> +						   hash_fields ?
> +						   flow->queues_n :
> +						   1);
>  			if (!flow->frxq[i].hrxq) {
>  				DEBUG("Flow %p cannot be applied",
>  				      (void *)flow);
> -- 
> 2.12.0
 
I would suggest to modify directly the mlx5_priv_hrxq_get() and
mlx5_priv_hrxq_new() to handle this and update the comment of both
functions, as the two places where those are called are tricking the
queue numbers the same way, it can be directly handled by the function
itself.

Thanks,

-- 
Nélio Laranjeiro
6WIND