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@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> Signed-off-by: Jijie Shao <shaojijie@xxxxxxxxxx>
> ---
>  .../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