[dpdk-dev] [PATCH] net/mlx5: fix adjust priority for drop queue

[dpdk-dev] [PATCH] net/mlx5: fix adjust priority for drop queue

[Nelio Laranjeiro]

Drop queue should also adjust their priority according the most specific
layer in the pattern they are matching to avoid dropping all the traffic.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_flow.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 26cf593af..549ae6916 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1185,6 +1185,14 @@ priv_flow_convert(struct priv *priv,
 		parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
 			attr->priority +
 			hash_rxq_init[parser->layer].flow_priority;
+	} else if (parser->drop) {
+		/*
+		 * Drop queue priority needs to be adjusted to
+		 * their most specific layer priority.
+		 */
+		parser->drop_q.ibv_attr->priority =
+			attr->priority +
+			hash_rxq_init[parser->layer].flow_priority;
 	}
 exit_free:
 	/* Only verification is expected, all resources should be released. */
-- 
2.11.0

Re: [dpdk-dev] [PATCH] net/mlx5: fix adjust priority for drop queue

[Yongseok Koh]

On Tue, Oct 24, 2017 at 01:33:19PM +0200, Nelio Laranjeiro wrote:
> Drop queue should also adjust their priority according the most specific
> layer in the pattern they are matching to avoid dropping all the traffic.
> 
> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 26cf593af..549ae6916 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1185,6 +1185,14 @@ priv_flow_convert(struct priv *priv,
>  		parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
>  			attr->priority +
>  			hash_rxq_init[parser->layer].flow_priority;
> +	} else if (parser->drop) {

Isn't this condition redundant?

And the following could be more consistent (and readable) with the similar code
above. But I'd leave it as your choice.

	if (parser->drop) {
	...
	} else if (parser->queues_n == 1) {
	...
	} else {
	...
	}

> +		/*
> +		 * Drop queue priority needs to be adjusted to
> +		 * their most specific layer priority.
> +		 */
> +		parser->drop_q.ibv_attr->priority =
> +			attr->priority +
> +			hash_rxq_init[parser->layer].flow_priority;
>  	}
>  exit_free:
>  	/* Only verification is expected, all resources should be released. */
> -- 


Thanks,
Yongseok

Re: [dpdk-dev] [PATCH] net/mlx5: fix adjust priority for drop queue

[Nélio Laranjeiro]

On Tue, Oct 24, 2017 at 10:55:03AM -0700, Yongseok Koh wrote:
> On Tue, Oct 24, 2017 at 01:33:19PM +0200, Nelio Laranjeiro wrote:
> > Drop queue should also adjust their priority according the most specific
> > layer in the pattern they are matching to avoid dropping all the traffic.
> > 
> > Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> > index 26cf593af..549ae6916 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -1185,6 +1185,14 @@ priv_flow_convert(struct priv *priv,
> >  		parser->queue[HASH_RXQ_ETH].ibv_attr->priority =
> >  			attr->priority +
> >  			hash_rxq_init[parser->layer].flow_priority;
> > +	} else if (parser->drop) {
> 
> Isn't this condition redundant?

I reach a point that all this "drop" is redundant and could use the
Ethernet entry to store it, but in RC phases making a such huge change
is not easy.

> And the following could be more consistent (and readable) with the similar code
> above. But I'd leave it as your choice.
> 
> 	if (parser->drop) {
> 	...
> 	} else if (parser->queues_n == 1) {
> 	...
> 	} else {
> 	...
> 	}

Agreed.

Thanks,

-- 
Nélio Laranjeiro
6WIND

[dpdk-dev] [PATCH v2] net/mlx5: fix adjust priority for drop queue

[Nelio Laranjeiro]

Drop queue should also adjust their priority according the most specific
layer in the pattern they are matching to avoid dropping all the traffic.

Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

---

Changes in v2:

 * Reorder the if, else if, else statement to improve readability
---
 drivers/net/mlx5/mlx5_flow.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 26cf593af..f392f1f65 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1174,9 +1174,17 @@ priv_flow_convert(struct priv *priv,
 	 * Last step. Complete missing specification to reach the RSS
 	 * configuration.
 	 */
-	if (parser->queues_n > 1) {
+	if (parser->drop) {
+		/*
+		 * Drop queue priority needs to be adjusted to
+		 * their most specific layer priority.
+		 */
+		parser->drop_q.ibv_attr->priority =
+			attr->priority +
+			hash_rxq_init[parser->layer].flow_priority;
+	} else if (parser->queues_n > 1) {
 		priv_flow_convert_finalise(priv, parser);
-	} else if (!parser->drop) {
+	} else {
 		/*
 		 * Action queue have their priority overridden with
 		 * Ethernet priority, this priority needs to be adjusted to
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix adjust priority for drop queue

[Yongseok Koh]

> On Oct 25, 2017, at 1:25 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> Drop queue should also adjust their priority according the most specific
> layer in the pattern they are matching to avoid dropping all the traffic.
> 
> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> ---
> 
> Changes in v2:
> 
> * Reorder the if, else if, else statement to improve readability
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

Re: [dpdk-dev] [PATCH v2] net/mlx5: fix adjust priority for drop queue

[Ferruh Yigit]

On 10/25/2017 9:41 AM, Yongseok Koh wrote:
> 
>> On Oct 25, 2017, at 1:25 AM, Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
>>
>> Drop queue should also adjust their priority according the most specific
>> layer in the pattern they are matching to avoid dropping all the traffic.
>>
>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
>>
>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

> Acked-by: Yongseok Koh <yskoh@mellanox.com>

Applied to dpdk-next-net/master, thanks.