diff mbox

[dpdk-dev] net/sfc: fix crash on Tx queue release

Message ID 1490800132-1833-1-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Andrew Rybchenko March 29, 2017, 3:08 p.m. UTC 
  Datapath TxQ handle is passed to release function, not control path.

Fixes: 5808dbaf751f ("net/sfc: factor out libefx-based Tx datapath")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
Please, squash the patch into 5808dbaf751f.

 drivers/net/sfc/sfc_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Ferruh Yigit March 29, 2017, 4:25 p.m. UTC | #1 
On 3/29/2017 4:08 PM, Andrew Rybchenko wrote:
> Datapath TxQ handle is passed to release function, not control path.
> 
> Fixes: 5808dbaf751f ("net/sfc: factor out libefx-based Tx datapath")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> Please, squash the patch into 5808dbaf751f.

Hi Andrew,

To clarify, for the commits in next-net tree that are not merged yet, we
can benefit from sub-tree re-writable option and squash some fixes,
-without over using it.

But for this patch, as far as I can see, original code added with
commit: b1b7ad933b39 ("net/sfc: set up and release Tx queues")
which is part of v17.02 release.

So, please send a patch with correct fixes line and it won't be squashed.

Thanks,
ferruh
Andrew Rybchenko March 29, 2017, 4:37 p.m. UTC | #2 
On 03/29/2017 07:25 PM, Ferruh Yigit wrote:
> On 3/29/2017 4:08 PM, Andrew Rybchenko wrote:
>> Datapath TxQ handle is passed to release function, not control path.
>>
>> Fixes: 5808dbaf751f ("net/sfc: factor out libefx-based Tx datapath")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> Please, squash the patch into 5808dbaf751f.
> Hi Andrew,

Hi Ferruh,

> To clarify, for the commits in next-net tree that are not merged yet, we
> can benefit from sub-tree re-writable option and squash some fixes,
> -without over using it.

Yes, it is perfectly clear.

> But for this patch, as far as I can see, original code added with
> commit: b1b7ad933b39 ("net/sfc: set up and release Tx queues")
> which is part of v17.02 release.

If we take a look a lines, yes, these lines belong to
b1b7ad933b39 ("net/sfc: set up and release Tx queues")
however the breakage is introduced by
19f92f3ec37c ("net/sfc: factor out libefx-based Tx datapath")
since it registers pointer to the so-called datapath Tx queue in the 
device data
instead of previous used control path Tx queue pointer.

> So, please send a patch with correct fixes line and it won't be squashed.

Correct fixes line for the patch with current dpdk-next-net is:

Fixes: 19f92f3ec37c ("net/sfc: factor out libefx-based Tx datapath")

which is in dpdk-next-net tree only. Should I resend it?

Thanks,
Andrew.
Ferruh Yigit March 29, 2017, 4:59 p.m. UTC | #3 
On 3/29/2017 5:37 PM, Andrew Rybchenko wrote:
> On 03/29/2017 07:25 PM, Ferruh Yigit wrote:
>> On 3/29/2017 4:08 PM, Andrew Rybchenko wrote:
>>> Datapath TxQ handle is passed to release function, not control path.
>>>
>>> Fixes: 5808dbaf751f ("net/sfc: factor out libefx-based Tx datapath")
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>> Please, squash the patch into 5808dbaf751f.
>> Hi Andrew,
> 
> Hi Ferruh,
> 
>> To clarify, for the commits in next-net tree that are not merged yet, we
>> can benefit from sub-tree re-writable option and squash some fixes,
>> -without over using it.
> 
> Yes, it is perfectly clear.
> 
>> But for this patch, as far as I can see, original code added with
>> commit: b1b7ad933b39 ("net/sfc: set up and release Tx queues")
>> which is part of v17.02 release.
> 
> If we take a look a lines, yes, these lines belong to
> b1b7ad933b39 ("net/sfc: set up and release Tx queues")
> however the breakage is introduced by
> 19f92f3ec37c ("net/sfc: factor out libefx-based Tx datapath")
> since it registers pointer to the so-called datapath Tx queue in the
> device data
> instead of previous used control path Tx queue pointer.

Oh, I see now, I was checking updated lines.

> 
>> So, please send a patch with correct fixes line and it won't be squashed.
> 
> Correct fixes line for the patch with current dpdk-next-net is:
> 
> Fixes: 19f92f3ec37c ("net/sfc: factor out libefx-based Tx datapath")
> 
> which is in dpdk-next-net tree only. Should I resend it?

I saw you already sent another patch, that is good, thanks.

> 
> Thanks,
> Andrew.
diff mbox

Patch

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index b745714..bba8b01 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -485,13 +485,15 @@  sfc_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 static void
 sfc_tx_queue_release(void *queue)
 {
-	struct sfc_txq *txq = queue;
+	struct sfc_dp_txq *dp_txq = queue;
+	struct sfc_txq *txq;
 	unsigned int sw_index;
 	struct sfc_adapter *sa;
 
-	if (txq == NULL)
+	if (dp_txq == NULL)
 		return;
 
+	txq = sfc_txq_by_dp_txq(dp_txq);
 	sw_index = sfc_txq_sw_index(txq);
 
 	SFC_ASSERT(txq->evq != NULL);