Received-SPF: pass (google.com: domain of linux-kernel+bounces-104764-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 15 Mar 2024 19:07:32 +0100
From: Michal Kubiak <michal.kubiak@intel.com>
To: Jijie Shao <shaojijie@huawei.com>
CC: <yisen.zhuang@huawei.com>, <salil.mehta@huawei.com>,
	<davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <shenjian15@huawei.com>, <wangjie125@huawei.com>,
	<liuyonglong@huawei.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 3/3] net: hns3: mark unexcuted loopback test result
 as UNEXECUTED
Message-ID: <ZfSOZG5vHbIY3IFU@localhost.localdomain>
References: <20240315100748.2913882-1-shaojijie@huawei.com>
 <20240315100748.2913882-4-shaojijie@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240315100748.2913882-4-shaojijie@huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?SLeNEzz7XB/GG72HFDP55ZUfbDVOTQGrOTDWo5OtQ0dFyIkXe2A462pdQOnm?=
 =?us-ascii?Q?9YasPOFG3as+LNMNw5naFsuAh6BVHs/ouLCOzolYixXabKoedIn9jh736kRM?=
 =?us-ascii?Q?GSFe+i/jjGYw/wy7Hnc0t1OC6se/8au+2mOKn16HEjkMKeEnG+qOSt1JwrGo?=
 =?us-ascii?Q?/8zaaMqPjEC/hhXQg5dBWzkr3og4mmJI0k0jrTW95EzF2F+k7dWs7FVFpN3M?=
 =?us-ascii?Q?JtsFmTwlElqDErPWbD78gA74+rmAo0UqJQMyhnS2AFNS/3PUiCu7nqcTDTB+?=
 =?us-ascii?Q?5yhjuF8Lof77lTvCh/heJSnunHYY2Rqeeh5A7o2/ipyQNS5AZWDx+q3X+tvl?=
 =?us-ascii?Q?ArsRdCOSjgTdLbI5yFHCOsoDyGIFjYkARqCuUh+fbg7Z7eoSWVRosp5k0Lm9?=
 =?us-ascii?Q?35FwGuN7E7Wa47eMbTN7d5cSZ3He1UQhK+1CtwSKAfTEvV158u+xxlBM1JZO?=
 =?us-ascii?Q?eHo3wv8Rk1FjArLLewASSvh0Wtzzk3djbnOesg576QVJ42TSRvs8u616C6Gj?=
 =?us-ascii?Q?yUgPxt8DWGvOZmQPuuppyFB4Sgljks9VoetYDom6zzLVLByaLXmwMf1qsFTw?=
 =?us-ascii?Q?Fb849Av+BS2v4V9OU/mf3g+y/2VHh6cdm63wcp0coziCOfWl7HzyJwZIs17C?=
 =?us-ascii?Q?rlEocVXtLNGWVE5iNJCNQ3gFYCMfoziBajoXjddd3TCYwjQwLZrkWrRe4OzM?=
 =?us-ascii?Q?+aM9i69JiyAK/EJUV+RBmbx/ZFGZs/4iB/XQad/GsYp4SKuJ3bQNyvuPITXV?=
 =?us-ascii?Q?H2uKyeyfHT+SU+4m2loyvq2aw6GrU4bQQ4LMYOGL4btLFZs8SUYlOC6i5GFa?=
 =?us-ascii?Q?+WB1HWrgFtmTMuonipaA8kIH/Z79PLfYM9qJAzxJ+1R7peZJUVIiCzsDZiA8?=
 =?us-ascii?Q?sYjE0rkG5IGl19yubiZJAXu3jxe0W/M0P7We2uI6AmvTlUa+gX3ID7Tr+rbD?=
 =?us-ascii?Q?8qG7fTTVs3gvqYshELla6crvRoQoEM//T3V4Oqpds2D4OzpwC+Q9k4YZ6ox7?=
 =?us-ascii?Q?gOdUivSx5M3yoPQf+knxNRiAIKrw+aEzR4L2uhC2FPezTywoNoYpOkknX2oK?=
 =?us-ascii?Q?Ezz1JfzwZbjST+irug6Px8fyF/Hd/RDF32PAsA7bYm/BSYzekyE/XE2s9eSH?=
 =?us-ascii?Q?fI/O9fj85ekejf85O1mM0oV0TEZcAjQSpyQ1I1aIBaiR9Sc6CzH1ygNozbQQ?=
 =?us-ascii?Q?gzhADfAnvbwUMuGH/1I18zp6SPGU+7SqKDih0us09JUxwNkt+aknsSnVZkmS?=
 =?us-ascii?Q?U1CuDf6aG1+i3bWQkhCkYnVxCHjzIIGAVd65t0QmPw4L8THIVojeVSYyYxBV?=
 =?us-ascii?Q?pU/RXNRAcOoN0iNAc5C8agziSyRAtiKZSwn1SJACo+SnIHdqiF4sji2cNUA8?=
 =?us-ascii?Q?Bo6NB/YAZczgJD2OYCMgkLy2Q+cstNbWMhpbHwPSe32s4wOb2BUsmlymnUOf?=
 =?us-ascii?Q?18vRWIr2N8Dd23vvSqM5VJ7+5PIpZCgSAJN61QknsYKP5RG5psezor0pHi02?=
 =?us-ascii?Q?E/YQjF4x2kyoLoo7lGdGNE1kt9ZZ4PkPFgze2n2wm3NnXl03ut3u0PmkIDaS?=
 =?us-ascii?Q?U658ibvO/SHH4XSPRbnK4guYAsiJHc+p4i7qUz1qOF0FEft+SIAgehjJyzEV?=
 =?us-ascii?Q?Sw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 758c79a1-7fb4-479c-4f10-08dc451acfe6
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5782.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2024 18:07:44.0294
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: V4dp5CXG6H+aqX4k6RjT0CfEUJXwPtBlYL8D7j8rW3TGLEjJC7r1xJcFOE7ARhrkZyLUqmR+FMi0Ep6oTP+0Ig==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB7014
X-OriginatorOrg: intel.com

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