[dpdk-dev,v2] net/mlx5: fix flow creation on port start
Checks
Commit Message
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>
---
On v2:
- Fixing the issue from the hrxq_get and hrxq_new functions.
---
drivers/net/mlx5/mlx5_flow.c | 4 ++--
drivers/net/mlx5/mlx5_rxq.c | 8 ++++++--
2 files changed, 8 insertions(+), 4 deletions(-)
Comments
On Mon, Nov 06, 2017 at 04:00:25PM +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>
> ---
> On v2:
> - Fixing the issue from the hrxq_get and hrxq_new functions.
> ---
> drivers/net/mlx5/mlx5_flow.c | 4 ++--
> drivers/net/mlx5/mlx5_rxq.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 5f49bf5ff..bcb06f84a 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1781,7 +1781,7 @@ priv_flow_create_action_queue_rss(struct priv *priv,
> parser->rss_conf.rss_key_len,
> hash_fields,
> parser->queues,
> - hash_fields ? parser->queues_n : 1);
> + parser->queues_n);
> if (flow->frxq[i].hrxq)
> continue;
> flow->frxq[i].hrxq =
> @@ -1790,7 +1790,7 @@ priv_flow_create_action_queue_rss(struct priv *priv,
> parser->rss_conf.rss_key_len,
> hash_fields,
> parser->queues,
> - hash_fields ? parser->queues_n : 1);
> + parser->queues_n);
> if (!flow->frxq[i].hrxq) {
> rte_flow_error_set(error, ENOMEM,
> RTE_FLOW_ERROR_TYPE_HANDLE,
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index a1f382b1f..18c13b3c7 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -1265,7 +1265,8 @@ mlx5_priv_ind_table_ibv_verify(struct priv *priv)
> * @param hash_fields
> * Verbs protocol hash field to make the RSS on.
> * @param queues
> - * Queues entering in hash queue.
> + * Queues entering in hash queue. In case of empty hash_fields only the
> + * first queue index will be taken for the indirection table.
> * @param queues_n
> * Number of queues.
> *
> @@ -1280,6 +1281,7 @@ mlx5_priv_hrxq_new(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
> struct mlx5_ind_table_ibv *ind_tbl;
> struct ibv_qp *qp;
>
> + queues_n = hash_fields ? queues_n : 1;
> ind_tbl = mlx5_priv_ind_table_ibv_get(priv, queues, queues_n);
> if (!ind_tbl)
> ind_tbl = mlx5_priv_ind_table_ibv_new(priv, queues, queues_n);
> @@ -1332,7 +1334,8 @@ mlx5_priv_hrxq_new(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
> * @param rss_conf
> * RSS configuration for the Rx hash queue.
> * @param queues
> - * Queues entering in hash queue.
> + * Queues entering in hash queue. In case of empty hash_fields only the
> + * first queue index will be taken for the indirection table.
> * @param queues_n
> * Number of queues.
> *
> @@ -1345,6 +1348,7 @@ mlx5_priv_hrxq_get(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
> {
> struct mlx5_hrxq *hrxq;
>
> + queues_n = hash_fields ? queues_n : 1;
> LIST_FOREACH(hrxq, &priv->hrxqs, next) {
> struct mlx5_ind_table_ibv *ind_tbl;
>
> --
> 2.12.0
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
On 11/6/2017 6:32 AM, Nélio Laranjeiro wrote:
> On Mon, Nov 06, 2017 at 04:00:25PM +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>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Applied to dpdk-next-net/master, thanks.
@@ -1781,7 +1781,7 @@ priv_flow_create_action_queue_rss(struct priv *priv,
parser->rss_conf.rss_key_len,
hash_fields,
parser->queues,
- hash_fields ? parser->queues_n : 1);
+ parser->queues_n);
if (flow->frxq[i].hrxq)
continue;
flow->frxq[i].hrxq =
@@ -1790,7 +1790,7 @@ priv_flow_create_action_queue_rss(struct priv *priv,
parser->rss_conf.rss_key_len,
hash_fields,
parser->queues,
- hash_fields ? parser->queues_n : 1);
+ parser->queues_n);
if (!flow->frxq[i].hrxq) {
rte_flow_error_set(error, ENOMEM,
RTE_FLOW_ERROR_TYPE_HANDLE,
@@ -1265,7 +1265,8 @@ mlx5_priv_ind_table_ibv_verify(struct priv *priv)
* @param hash_fields
* Verbs protocol hash field to make the RSS on.
* @param queues
- * Queues entering in hash queue.
+ * Queues entering in hash queue. In case of empty hash_fields only the
+ * first queue index will be taken for the indirection table.
* @param queues_n
* Number of queues.
*
@@ -1280,6 +1281,7 @@ mlx5_priv_hrxq_new(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
struct mlx5_ind_table_ibv *ind_tbl;
struct ibv_qp *qp;
+ queues_n = hash_fields ? queues_n : 1;
ind_tbl = mlx5_priv_ind_table_ibv_get(priv, queues, queues_n);
if (!ind_tbl)
ind_tbl = mlx5_priv_ind_table_ibv_new(priv, queues, queues_n);
@@ -1332,7 +1334,8 @@ mlx5_priv_hrxq_new(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
* @param rss_conf
* RSS configuration for the Rx hash queue.
* @param queues
- * Queues entering in hash queue.
+ * Queues entering in hash queue. In case of empty hash_fields only the
+ * first queue index will be taken for the indirection table.
* @param queues_n
* Number of queues.
*
@@ -1345,6 +1348,7 @@ mlx5_priv_hrxq_get(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len,
{
struct mlx5_hrxq *hrxq;
+ queues_n = hash_fields ? queues_n : 1;
LIST_FOREACH(hrxq, &priv->hrxqs, next) {
struct mlx5_ind_table_ibv *ind_tbl;