diff mbox series

[v2,5/5] net/hns3: fix queue enabling status not store after FLR

Message ID 1604634716-43484-6-git-send-email-oulijun@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series bugfix and cleanups for hns3 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Lijun Ou Nov. 6, 2020, 3:51 a.m. UTC 
  From: Chengchang Tang <tangchengchang@huawei.com>

The FLR will resets the queue enabling status. In the
current code, the queue enabling status is not restored
after the reset. Therefore, if upper layer users have
called queue start/stop function before the reset, the
behavior after the reset is not as expected.

This patch fix it by add a queue enabling status restore
function to the reset handler.

Fixes: fa29fe45a7b4 ("net/hns3: support queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    |  5 +++++
 drivers/net/hns3/hns3_ethdev_vf.c |  5 +++++
 drivers/net/hns3/hns3_rxtx.c      | 20 ++++++++++++++++++++
 drivers/net/hns3/hns3_rxtx.h      |  1 +
 4 files changed, 31 insertions(+)

Comments

Ferruh Yigit Nov. 6, 2020, 4:57 p.m. UTC | #1 
On 11/6/2020 3:51 AM, Lijun Ou wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> The FLR will resets the queue enabling status. In the
> current code, the queue enabling status is not restored
> after the reset. Therefore, if upper layer users have
> called queue start/stop function before the reset, the
> behavior after the reset is not as expected.
> 
> This patch fix it by add a queue enabling status restore
> function to the reset handler.
> 

Can you please re-work above description, it looks like it can be improved.


> Fixes: fa29fe45a7b4 ("net/hns3: support queue start and stop")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
>   drivers/net/hns3/hns3_ethdev.c    |  5 +++++
>   drivers/net/hns3/hns3_ethdev_vf.c |  5 +++++
>   drivers/net/hns3/hns3_rxtx.c      | 20 ++++++++++++++++++++
>   drivers/net/hns3/hns3_rxtx.h      |  1 +
>   4 files changed, 31 insertions(+)
> 
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index b27cf67..e177549 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -5567,6 +5567,11 @@ hns3_start_service(struct hns3_adapter *hns)
>   		/* Enable interrupt of all rx queues before enabling queues */
>   		hns3_dev_all_rx_queue_intr_enable(hw, true);
>   		/*
> +		 * Enable state of each rxq and txq will be recovered after
> +		 * reset, so we need restore them before enable all tqps;
> +		 */

Same with the comments (this + below one) please. Also instead of "enable state" 
or "enabling status", does it make sense to just say 'state'?
Lijun Ou Nov. 9, 2020, 2:27 p.m. UTC | #2 
在 2020/11/7 0:57, Ferruh Yigit 写道:
> On 11/6/2020 3:51 AM, Lijun Ou wrote:
>> From: Chengchang Tang <tangchengchang@huawei.com>
>>
>> The FLR will resets the queue enabling status. In the
>> current code, the queue enabling status is not restored
>> after the reset. Therefore, if upper layer users have
>> called queue start/stop function before the reset, the
>> behavior after the reset is not as expected.
>>
>> This patch fix it by add a queue enabling status restore
>> function to the reset handler.
>>
> 
> Can you please re-work above description, it looks like it can be improved.
> 
Thanks. I will try to re-work in next version.
> 
>> Fixes: fa29fe45a7b4 ("net/hns3: support queue start and stop")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>>   drivers/net/hns3/hns3_ethdev.c    |  5 +++++
>>   drivers/net/hns3/hns3_ethdev_vf.c |  5 +++++
>>   drivers/net/hns3/hns3_rxtx.c      | 20 ++++++++++++++++++++
>>   drivers/net/hns3/hns3_rxtx.h      |  1 +
>>   4 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/net/hns3/hns3_ethdev.c 
>> b/drivers/net/hns3/hns3_ethdev.c
>> index b27cf67..e177549 100644
>> --- a/drivers/net/hns3/hns3_ethdev.c
>> +++ b/drivers/net/hns3/hns3_ethdev.c
>> @@ -5567,6 +5567,11 @@ hns3_start_service(struct hns3_adapter *hns)
>>           /* Enable interrupt of all rx queues before enabling queues */
>>           hns3_dev_all_rx_queue_intr_enable(hw, true);
>>           /*
>> +         * Enable state of each rxq and txq will be recovered after
>> +         * reset, so we need restore them before enable all tqps;
>> +         */
> 
> Same with the comments (this + below one) please. Also instead of 
> "enable state" or "enabling status", does it make sense to just say 
> 'state'?
good advice.
> 
> .
>
diff mbox series

Patch

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index b27cf67..e177549 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5567,6 +5567,11 @@  hns3_start_service(struct hns3_adapter *hns)
 		/* Enable interrupt of all rx queues before enabling queues */
 		hns3_dev_all_rx_queue_intr_enable(hw, true);
 		/*
+		 * Enable state of each rxq and txq will be recovered after
+		 * reset, so we need restore them before enable all tqps;
+		 */
+		hns3_restore_tqp_enable_state(hw);
+		/*
 		 * When finished the initialization, enable queues to receive
 		 * and transmit packets.
 		 */
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 2f6d91b..511cd26 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2424,6 +2424,11 @@  hns3vf_start_service(struct hns3_adapter *hns)
 		/* Enable interrupt of all rx queues before enabling queues */
 		hns3_dev_all_rx_queue_intr_enable(hw, true);
 		/*
+		 * Enable state of each rxq and txq will be recovered after
+		 * reset, so we need restore them before enable all tqps;
+		 */
+		hns3_restore_tqp_enable_state(hw);
+		/*
 		 * When finished the initialization, enable queues to receive
 		 * and transmit packets.
 		 */
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 285c06d..c76e635 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -503,6 +503,26 @@  hns3_start_all_rxqs(struct rte_eth_dev *dev)
 }
 
 void
+hns3_restore_tqp_enable_state(struct hns3_hw *hw)
+{
+	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
+	uint16_t i;
+
+	for (i = 0; i < hw->data->nb_rx_queues; i++) {
+		rxq = hw->data->rx_queues[i];
+		if (rxq != NULL)
+			hns3_enable_rxq(rxq, rxq->enabled);
+	}
+
+	for (i = 0; i < hw->data->nb_tx_queues; i++) {
+		txq = hw->data->tx_queues[i];
+		if (txq != NULL)
+			hns3_enable_txq(txq, txq->enabled);
+	}
+}
+
+void
 hns3_stop_all_txqs(struct rte_eth_dev *dev)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index 8b32abe..6538848 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -677,5 +677,6 @@  uint32_t hns3_get_tqp_reg_offset(uint16_t idx);
 int hns3_start_all_txqs(struct rte_eth_dev *dev);
 int hns3_start_all_rxqs(struct rte_eth_dev *dev);
 void hns3_stop_all_txqs(struct rte_eth_dev *dev);
+void hns3_restore_tqp_enable_state(struct hns3_hw *hw);
 
 #endif /* _HNS3_RXTX_H_ */