[dpdk-dev,2/4] net/mlx5: fix flow single queue

Message ID 07145c9505263ba30de44a5c5186431935b98884.1520950386.git.nelio.laranjeiro@6wind.com (mailing list archive)
State Accepted, archived
Delegated to: Shahaf Shuler
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Nélio Laranjeiro March 13, 2018, 2:17 p.m. UTC
  The flow is created with any steering being applied in the NIC when the
device is handling a single Rx queue.

Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
Cc: stable@dpdk.org

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

Comments

Ferruh Yigit March 21, 2018, 6:45 p.m. UTC | #1
On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> The flow is created with any steering being applied in the NIC when the
> device is handling a single Rx queue.
> 
> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")

This patch is from current release and still in next-net. If it is possible to
split this patch to fix issue in this particular commit it can be squashed into
original fix.

If not commit id for fixes will be wrong and needs to be fixed when merged into
main repo.

> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

<...>
  
Ferruh Yigit March 28, 2018, 9:04 a.m. UTC | #2
On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>> The flow is created with any steering being applied in the NIC when the
>> device is handling a single Rx queue.
>>
>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
> 
> This patch is from current release and still in next-net. If it is possible to
> split this patch to fix issue in this particular commit it can be squashed into
> original fix.

Cc'ed Shahaf.

> 
> If not commit id for fixes will be wrong and needs to be fixed when merged into
> main repo.
> 
>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS flow")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> <...>
>
  
Shahaf Shuler March 28, 2018, 11:13 a.m. UTC | #3
Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:

> > On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:

> >> The flow is created with any steering being applied in the NIC when

> >> the device is handling a single Rx queue.

> >>

> >> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single

> >> target queue")

> >

> > This patch is from current release and still in next-net. If it is

> > possible to split this patch to fix issue in this particular commit it

> > can be squashed into original fix.

> 


We can say it only fixes the above and squash the commits. 

> Cc'ed Shahaf.

> 

> >

> > If not commit id for fixes will be wrong and needs to be fixed when

> > merged into main repo.

> >

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

> >> flow")

> >> Cc: stable@dpdk.org

> >>

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

> >

> > <...>

> >
  
Nélio Laranjeiro March 28, 2018, 11:34 a.m. UTC | #4
On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> > On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> > > On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> > >> The flow is created with any steering being applied in the NIC when
> > >> the device is handling a single Rx queue.
> > >>
> > >> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
> > >> target queue")
> > >
> > > This patch is from current release and still in next-net. If it is
> > > possible to split this patch to fix issue in this particular commit it
> > > can be squashed into original fix.
> > 
> 
> We can say it only fixes the above and squash the commits. 

Indeed they are fixing the same issue.

> > Cc'ed Shahaf.
> > 
> > >
> > > If not commit id for fixes will be wrong and needs to be fixed when
> > > merged into main repo.
> > >
> > >> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
> > >> flow")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > >
> > > <...>
> > >
>
  
Ferruh Yigit March 28, 2018, 1:16 p.m. UTC | #5
On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
> On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
>> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
>>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
>>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>>>>> The flow is created with any steering being applied in the NIC when
>>>>> the device is handling a single Rx queue.
>>>>>
>>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
>>>>> target queue")
>>>>
>>>> This patch is from current release and still in next-net. If it is
>>>> possible to split this patch to fix issue in this particular commit it
>>>> can be squashed into original fix.
>>>
>>
>> We can say it only fixes the above and squash the commits. 
> 
> Indeed they are fixing the same issue.

Can you please confirm it is safe to squash this commit [1] into [2]?

[1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
[2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")

> 
>>> Cc'ed Shahaf.
>>>
>>>>
>>>> If not commit id for fixes will be wrong and needs to be fixed when
>>>> merged into main repo.
>>>>
>>>>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
>>>>> flow")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>>
>>>> <...>
>>>>
>>
>
  
Nélio Laranjeiro March 28, 2018, 3:09 p.m. UTC | #6
On Wed, Mar 28, 2018 at 02:16:44PM +0100, Ferruh Yigit wrote:
> On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
> > On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
> >> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
> >>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
> >>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
> >>>>> The flow is created with any steering being applied in the NIC when
> >>>>> the device is handling a single Rx queue.
> >>>>>
> >>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
> >>>>> target queue")
> >>>>
> >>>> This patch is from current release and still in next-net. If it is
> >>>> possible to split this patch to fix issue in this particular commit it
> >>>> can be squashed into original fix.
> >>>
> >>
> >> We can say it only fixes the above and squash the commits. 
> > 
> > Indeed they are fixing the same issue.
> 
> Can you please confirm it is safe to squash this commit [1] into [2]?
> 
> [1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
> [2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")

I confirm, [1] is a fix fixing a bug introduced in [2].  Both are needed
to fix the same bug, they should be squash in a single commit.

The confusion came from the fact commit [2] was already integrated in
shshaf's branch, when the issue was raised using another scenario.

Sorry for the confusion.

> >>> Cc'ed Shahaf.
> >>>
> >>>>
> >>>> If not commit id for fixes will be wrong and needs to be fixed when
> >>>> merged into main repo.
> >>>>
> >>>>> Fixes: 8086cf08b2f0 ("net/mlx5: handle RSS hash configuration in RSS
> >>>>> flow")
> >>>>> Cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >>>>
> >>>> <...>
> >>>>
> >>
> > 
> 


Regards,
  
Ferruh Yigit March 28, 2018, 4:38 p.m. UTC | #7
On 3/28/2018 4:09 PM, Nélio Laranjeiro wrote:
> On Wed, Mar 28, 2018 at 02:16:44PM +0100, Ferruh Yigit wrote:
>> On 3/28/2018 12:34 PM, Nélio Laranjeiro wrote:
>>> On Wed, Mar 28, 2018 at 11:13:42AM +0000, Shahaf Shuler wrote:
>>>> Wednesday, March 28, 2018 12:05 PM, Ferruh Yigit:
>>>>> On 3/21/2018 6:45 PM, Ferruh Yigit wrote:
>>>>>> On 3/13/2018 2:17 PM, Nelio Laranjeiro wrote:
>>>>>>> The flow is created with any steering being applied in the NIC when
>>>>>>> the device is handling a single Rx queue.
>>>>>>>
>>>>>>> Fixes: cede123a158f ("net/mlx5: fix flow creation with a single
>>>>>>> target queue")
>>>>>>
>>>>>> This patch is from current release and still in next-net. If it is
>>>>>> possible to split this patch to fix issue in this particular commit it
>>>>>> can be squashed into original fix.
>>>>>
>>>>
>>>> We can say it only fixes the above and squash the commits. 
>>>
>>> Indeed they are fixing the same issue.
>>
>> Can you please confirm it is safe to squash this commit [1] into [2]?
>>
>> [1]: ebfe9a38cb47 ("net/mlx5: fix flow single queue")
>> [2]: cede123a158f ("net/mlx5: fix flow creation with a single target queue")
> 
> I confirm, [1] is a fix fixing a bug introduced in [2].  Both are needed
> to fix the same bug, they should be squash in a single commit.

[1] Squashed into relevant commit [2] in next-net, thanks.

(There were a few conflicts, please check latest next-net.)
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e1ffb38a5..e7040daad 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -937,11 +937,12 @@  mlx5_flow_convert_finalise(struct mlx5_flow_parse *parser)
 	/* Remove any other flow not matching the pattern. */
 	if (parser->queues_n == 1) {
 		for (i = 0; i != hash_rxq_init_n; ++i) {
-			if (i == parser->layer || !parser->queue[i].ibv_attr)
+			if (i == HASH_RXQ_ETH)
 				continue;
 			rte_free(parser->queue[i].ibv_attr);
 			parser->queue[i].ibv_attr = NULL;
 		}
+		return;
 	}
 	if (parser->layer == HASH_RXQ_ETH) {
 		goto fill;
@@ -1807,6 +1808,7 @@  mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 	struct priv *priv = dev->data->dev_private;
 	int ret;
 	unsigned int i;
+	unsigned int flows_n = 0;
 
 	assert(priv->pd);
 	assert(priv->ctx);
@@ -1830,12 +1832,18 @@  mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 					   NULL, "flow rule creation failure");
 			goto error;
 		}
+		++flows_n;
 		DRV_LOG(DEBUG, "port %u %p type %d QP %p ibv_flow %p",
 			dev->data->port_id,
 			(void *)flow, i,
 			(void *)flow->frxq[i].hrxq,
 			(void *)flow->frxq[i].ibv_flow);
 	}
+	if (!flows_n) {
+		rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL, "internal error in flow creation");
+		goto error;
+	}
 	for (i = 0; i != parser->queues_n; ++i) {
 		struct mlx5_rxq_data *q =
 			(*priv->rxqs)[parser->queues[i]];