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

Message ID 20171105154449.93962-1-shahafs@mellanox.com
State Superseded, archived
Headers show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Shahaf Shuler Nov. 5, 2017, 3:44 p.m.
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(-)

Comments

Nélio Laranjeiro Nov. 6, 2017, 7:43 a.m. | #1
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,

Patch

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);