[PATCH net 0/3] There are some bugfix for the HNS3 ethernet driver

[PATCH net 0/3] There are some bugfix for the HNS3 ethernet driver

[Jijie Shao]

There are some bugfix for the HNS3 ethernet driver

Jian Shen (1):
  net: hns3: mark unexcuted loopback test result as UNEXECUTED

Jie Wang (1):
  net: hns3: fix index limit to support all queue stats

Yonglong Liu (1):
  net: hns3: fix kernel crash when devlink reload during pf
    initialization

 .../hns3/hns3_common/hclge_comm_tqp_stats.c      |  2 +-
 .../net/ethernet/hisilicon/hns3/hns3_ethtool.c   | 16 +++++++++++++++-
 .../hisilicon/hns3/hns3pf/hclge_devlink.c        |  5 +++--
 3 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.30.0

[PATCH net 1/3] net: hns3: fix index limit to support all queue stats

[Jijie Shao]

From: Jie Wang <wangjie125@huawei.com>

Currently, hns hardware supports more than 512 queues and the index limit
in hclge_comm_tqps_update_stats is useless. So this patch remove it.

Fixes: 287db5c40d15 ("net: hns3: create new set of common tqp stats APIs for PF and VF reuse")
Signed-off-by: Jie Wang <wangjie125@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
index f3c9395d8351..618f66d9586b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
@@ -85,7 +85,7 @@ int hclge_comm_tqps_update_stats(struct hnae3_handle *handle,
 		hclge_comm_cmd_setup_basic_desc(&desc, HCLGE_OPC_QUERY_TX_STATS,
 						true);
 
-		desc.data[0] = cpu_to_le32(tqp->index & 0x1ff);
+		desc.data[0] = cpu_to_le32(tqp->index);
 		ret = hclge_comm_cmd_send(hw, &desc, 1);
 		if (ret) {
 			dev_err(&hw->cmq.csq.pdev->dev,
-- 
2.30.0

Re: [PATCH net 1/3] net: hns3: fix index limit to support all queue stats

[Michal Kubiak]

On Fri, Mar 15, 2024 at 06:07:46PM +0800, Jijie Shao wrote:
> From: Jie Wang <wangjie125@huawei.com>
> 
> Currently, hns hardware supports more than 512 queues and the index limit
> in hclge_comm_tqps_update_stats is useless. So this patch remove it.

typo: remove -> removes

> 
> Fixes: 287db5c40d15 ("net: hns3: create new set of common tqp stats APIs for PF and VF reuse")
> Signed-off-by: Jie Wang <wangjie125@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  .../ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
> index f3c9395d8351..618f66d9586b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c
> @@ -85,7 +85,7 @@ int hclge_comm_tqps_update_stats(struct hnae3_handle *handle,
>  		hclge_comm_cmd_setup_basic_desc(&desc, HCLGE_OPC_QUERY_TX_STATS,
>  						true);
>  
> -		desc.data[0] = cpu_to_le32(tqp->index & 0x1ff);
> +		desc.data[0] = cpu_to_le32(tqp->index);
>  		ret = hclge_comm_cmd_send(hw, &desc, 1);
>  		if (ret) {
>  			dev_err(&hw->cmq.csq.pdev->dev,
> -- 
> 2.30.0
> 
> 


Thanks,
Michal

[PATCH net 2/3] net: hns3: fix kernel crash when devlink reload during pf initialization

[Jijie Shao]

From: Yonglong Liu <liuyonglong@huawei.com>

The devlink reload process will access the hardware resources,
but the register operation is before the hardware initialize
done, so, if process the devlink reload during initializetion,
may lead the kernel crash. This patch check whether the NIC is
initialized to fix the problem.

Fixes: b741269b2759 ("net: hns3: add support for registering devlink for PF")
Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 9a939c0b217f..80db4f7b05f6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
@@ -40,8 +40,9 @@ static int hclge_devlink_reload_down(struct devlink *devlink, bool netns_change,
 	struct pci_dev *pdev = hdev->pdev;
 	int ret;
 
-	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state)) {
-		dev_err(&pdev->dev, "reset is handling\n");
+	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state) ||
+	    !test_bit(HCLGE_STATE_NIC_REGISTERED, &hdev->state)) {
+		dev_err(&pdev->dev, "reset is handling or driver removed\n");
 		return -EBUSY;
 	}
 
-- 
2.30.0

Re: [PATCH net 2/3] net: hns3: fix kernel crash when devlink reload during pf initialization

[Michal Kubiak]

On Fri, Mar 15, 2024 at 06:07:47PM +0800, Jijie Shao wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
> 
> The devlink reload process will access the hardware resources,
> but the register operation is before the hardware initialize
> done, so, if process the devlink reload during initializetion,

typo: initializetion -> initialization
Moreover, I would suggest to fix the syntax of that description because
it seems to be not obvious.

e.g. the register operation is done before the hardware is initialized
(but I may not get your intention)

> may lead the kernel crash. This patch check whether the NIC is

-> may lead to kernel crash

> initialized to fix the problem.

Maybe just: "Fix this by adding the check if the NIC initialization check" ?

> 
> Fixes: b741269b2759 ("net: hns3: add support for registering devlink for PF")
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> index 9a939c0b217f..80db4f7b05f6 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> @@ -40,8 +40,9 @@ static int hclge_devlink_reload_down(struct devlink *devlink, bool netns_change,
>  	struct pci_dev *pdev = hdev->pdev;
>  	int ret;
>  
> -	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state)) {
> -		dev_err(&pdev->dev, "reset is handling\n");
> +	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state) ||
> +	    !test_bit(HCLGE_STATE_NIC_REGISTERED, &hdev->state)) {
> +		dev_err(&pdev->dev, "reset is handling or driver removed\n");
>  		return -EBUSY;
>  	}
>  
> -- 
> 2.30.0
> 
> 


Thanks,
Michal

[PATCH net 3/3] net: hns3: mark unexcuted loopback test result as UNEXECUTED

[Jijie Shao]

From: Jian Shen <shenjian15@huawei.com>

Currently, loopback test maybe skipped when resetting, but the test
result will still show as 'PASS', because the driver doesn't set
ETH_TEST_FL_FAILED flag. Fixes it by set the flag and initialize the
value to be UNEXECUTED.

Fixes: 4c8dab1c709c ("net: hns3: reconstruct function hns3_self_test")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3_ethtool.c   | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 999a0ee162a6..6804ac5725d4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -78,6 +78,9 @@ static const struct hns3_stats hns3_rxq_stats[] = {
 #define HNS3_NIC_LB_TEST_NO_MEM_ERR	1
 #define HNS3_NIC_LB_TEST_TX_CNT_ERR	2
 #define HNS3_NIC_LB_TEST_RX_CNT_ERR	3
+#define HNS3_NIC_LB_TEST_UNEXECUTED	4
+
+static int hns3_get_sset_count(struct net_device *netdev, int stringset);
 
 static int hns3_lp_setup(struct net_device *ndev, enum hnae3_loop loop, bool en)
 {
@@ -418,18 +421,29 @@ static void hns3_do_external_lb(struct net_device *ndev,
 static void hns3_self_test(struct net_device *ndev,
 			   struct ethtool_test *eth_test, u64 *data)
 {
+	int cnt = hns3_get_sset_count(ndev, ETH_SS_TEST);
 	struct hns3_nic_priv *priv = netdev_priv(ndev);
 	struct hnae3_handle *h = priv->ae_handle;
 	int st_param[HNAE3_LOOP_NONE][2];
 	bool if_running = netif_running(ndev);
+	int i;
+
+	/* initialize the loopback test result, avoiding mark unexcuted loopback
+	 * test as PASS.
+	 */
+	for (i = 0; i < cnt; i++)
+		data[i] = HNS3_NIC_LB_TEST_UNEXECUTED;
 
 	if (hns3_nic_resetting(ndev)) {
 		netdev_err(ndev, "dev resetting!");
+		eth_test->flags |= ETH_TEST_FL_FAILED;
 		return;
 	}
 
-	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE))
+	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE)) {
+		eth_test->flags |= ETH_TEST_FL_FAILED;
 		return;
+	}
 
 	if (netif_msg_ifdown(h))
 		netdev_info(ndev, "self test start\n");
-- 
2.30.0

Re: [PATCH net 3/3] net: hns3: mark unexcuted loopback test result as UNEXECUTED

[Michal Kubiak]

On Fri, Mar 15, 2024 at 06:07:48PM +0800, Jijie Shao wrote:
> From: Jian Shen <shenjian15@huawei.com>
> 
> Currently, loopback test maybe skipped when resetting, but the test
typo: maybe skipped -> may be skipped

> result will still show as 'PASS', because the driver doesn't set
> ETH_TEST_FL_FAILED flag. Fixes it by set the flag and initialize the
> value to be UNEXECUTED.

Fix it by setting the flag and initializating the value to UNEXECUTED.


> 
> Fixes: 4c8dab1c709c ("net: hns3: reconstruct function hns3_self_test")
> Signed-off-by: Jian Shen <shenjian15@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  .../net/ethernet/hisilicon/hns3/hns3_ethtool.c   | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index 999a0ee162a6..6804ac5725d4 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -78,6 +78,9 @@ static const struct hns3_stats hns3_rxq_stats[] = {
>  #define HNS3_NIC_LB_TEST_NO_MEM_ERR	1
>  #define HNS3_NIC_LB_TEST_TX_CNT_ERR	2
>  #define HNS3_NIC_LB_TEST_RX_CNT_ERR	3
> +#define HNS3_NIC_LB_TEST_UNEXECUTED	4
> +
> +static int hns3_get_sset_count(struct net_device *netdev, int stringset);
>  
>  static int hns3_lp_setup(struct net_device *ndev, enum hnae3_loop loop, bool en)
>  {
> @@ -418,18 +421,29 @@ static void hns3_do_external_lb(struct net_device *ndev,
>  static void hns3_self_test(struct net_device *ndev,
>  			   struct ethtool_test *eth_test, u64 *data)
>  {
> +	int cnt = hns3_get_sset_count(ndev, ETH_SS_TEST);
>  	struct hns3_nic_priv *priv = netdev_priv(ndev);
>  	struct hnae3_handle *h = priv->ae_handle;
>  	int st_param[HNAE3_LOOP_NONE][2];
>  	bool if_running = netif_running(ndev);
> +	int i;
> +
> +	/* initialize the loopback test result, avoiding mark unexcuted loopbacki

syntax: avoid marking an unexecuted...

> +	 * test as PASS.
> +	 */
> +	for (i = 0; i < cnt; i++)
> +		data[i] = HNS3_NIC_LB_TEST_UNEXECUTED;
>  
>  	if (hns3_nic_resetting(ndev)) {
>  		netdev_err(ndev, "dev resetting!");
> +		eth_test->flags |= ETH_TEST_FL_FAILED;
>  		return;
>  	}
>  
> -	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE))
> +	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE)) {
> +		eth_test->flags |= ETH_TEST_FL_FAILED;
>  		return;
> +	}

Can setting that flag be optimized? For example:


  	if (hns3_nic_resetting(ndev)) {
  		netdev_err(ndev, "dev resetting!");
		goto failure;
  	}

 	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE))
		goto failure;

[...]

	if (netif_msg_ifdown(h))
		netdev_info(ndev, "self test end\n");
failure:
 	eth_test->flags |= ETH_TEST_FL_FAILED;
}

>  
>  	if (netif_msg_ifdown(h))
>  		netdev_info(ndev, "self test start\n");
> -- 
> 2.30.0
> 
> 

Thanks,
Michal

Re: [PATCH net 3/3] net: hns3: mark unexcuted loopback test result as UNEXECUTED

[Jijie Shao]

on 2024/3/16 2:07, Michal Kubiak wrote:
> On Fri, Mar 15, 2024 at 06:07:48PM +0800, Jijie Shao wrote:
>> From: Jian Shen <shenjian15@huawei.com>
>>
>> Currently, loopback test maybe skipped when resetting, but the test
> typo: maybe skipped -> may be skipped
>
>> result will still show as 'PASS', because the driver doesn't set
>> ETH_TEST_FL_FAILED flag. Fixes it by set the flag and initialize the
>> value to be UNEXECUTED.
> Fix it by setting the flag and initializating the value to UNEXECUTED.
>
>
>> Fixes: 4c8dab1c709c ("net: hns3: reconstruct function hns3_self_test")
>> Signed-off-by: Jian Shen <shenjian15@huawei.com>

Thanks，

I have sent V2 to fix them.

Jijie