netdev - RE: [PATCH iwl-net] ice: implement AQ download pkg retry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <SJ2PR18MB56353F95F209E75987D4DA79A2EE2@SJ2PR18MB5635.namprd18.prod.outlook.com>
Date: Fri, 17 May 2024 07:26:08 +0000
From: Naveen Mamindlapalli <naveenm@...vell.com>
To: Wojciech Drewek <wojciech.drewek@...el.com>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
Subject: RE: [PATCH iwl-net] ice: implement AQ download pkg retry


> -----Original Message-----
> From: Wojciech Drewek <wojciech.drewek@...el.com>
> Sent: Thursday, May 16, 2024 7:34 PM
> To: netdev@...r.kernel.org
> Cc: intel-wired-lan@...ts.osuosl.org
> Subject: [PATCH iwl-net] ice: implement AQ download pkg retry
> 
> ice_aqc_opc_download_pkg (0x0C40) AQ sporadically returns error due to FW
> issue. Fix this by retrying five times before moving to Safe Mode.
> 
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ddp.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c
> b/drivers/net/ethernet/intel/ice/ice_ddp.c
> index ce5034ed2b24..19e2111fcf08 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
> @@ -1339,6 +1339,7 @@ ice_dwnld_cfg_bufs_no_lock(struct ice_hw *hw, struct
> ice_buf *bufs, u32 start,
> 
>  	for (i = 0; i < count; i++) {
>  		bool last = false;
> +		int try_cnt = 0;
>  		int status;
> 
>  		bh = (struct ice_buf_hdr *)(bufs + start + i); @@ -1346,8
> +1347,22 @@ ice_dwnld_cfg_bufs_no_lock(struct ice_hw *hw, struct ice_buf
> *bufs, u32 start,
>  		if (indicate_last)
>  			last = ice_is_last_download_buffer(bh, i, count);
> 
> -		status = ice_aq_download_pkg(hw, bh, ICE_PKG_BUF_SIZE,
> last,
> -					     &offset, &info, NULL);
> +		while (try_cnt < 5) {
> +			status = ice_aq_download_pkg(hw, bh,
> ICE_PKG_BUF_SIZE,
> +						     last, &offset, &info,
> +						     NULL);
> +			if (hw->adminq.sq_last_status != ICE_AQ_RC_ENOSEC
> &&
> +			    hw->adminq.sq_last_status != ICE_AQ_RC_EBADSIG)
> +				break;
> +
> +			try_cnt++;
> +			msleep(20);
> +		}
> +
> +		if (try_cnt)
> +			dev_dbg(ice_hw_to_dev(hw),
> +				"ice_aq_download_pkg failed, number of retries:
> %d\n",
> +				try_cnt);

Do you really need this dbg statement when try_cnt < 5? Is it not misleading in success case (with retries)?

Thanks,
Naveen

> 
>  		/* Save AQ status from download package */
>  		if (status) {
> --
> 2.40.1
>