netdev - Re: [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout in ice_release

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <568da81f-d892-4921-ac87-2a2de78c422a@sangfor.com.cn>
Date: Sat, 6 Dec 2025 21:08:00 +0800
From: Ding Hui <dinghui@...gfor.com.cn>
To: Simon Horman <horms@...nel.org>
Cc: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
 "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
 "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "Keller, Jacob E" <jacob.e.keller@...el.com>,
 "pabeni@...hat.com" <pabeni@...hat.com>, "kuba@...nel.org"
 <kuba@...nel.org>,
 "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout in
 ice_release_res()

On 2025/12/6 17:46, Simon Horman wrote:
> On Sat, Dec 06, 2025 at 10:42:36AM +0800, Ding Hui wrote:
>> On 2025/12/6 5:09, Loktionov, Aleksandr wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf
>>>> Of Ding Hui
>>>> Sent: Friday, December 5, 2025 9:16 AM
>>>> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Kitszel,
>>>> Przemyslaw <przemyslaw.kitszel@...el.com>; andrew+netdev@...n.ch;
>>>> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
>>>> pabeni@...hat.com; Keller, Jacob E <jacob.e.keller@...el.com>; intel-
>>>> wired-lan@...ts.osuosl.org
>>>> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Ding, Hui
>>>> <dinghui@...gfor.com.cn>
>>>> Subject: [Intel-wired-lan] [PATCH net-next] ice: Fix incorrect timeout
>>>> in ice_release_res()
>>>>
>>>> The commit 5f6df173f92e ("ice: implement and use rd32_poll_timeout for
>>>> ice_sq_done timeout") converted ICE_CTL_Q_SQ_CMD_TIMEOUT from jiffies
>>>> to microseconds.
>>>>
>>>> But the ice_release_res() function was missed, and its logic still
>>>> treats ICE_CTL_Q_SQ_CMD_TIMEOUT as a jiffies value.
>>>>
>>>> So correct the issue by usecs_to_jiffies().
>>>>
>>>
>>> Please add a brief "how verified" paragraph (platform + steps).
>>> This is a unit-conversion fix in a timeout path; a short test description helps reviewers and stable backports validate the change.
>>>
>> Sorry for not being able to provide the verification information, as
>> I haven't actually encountered this issue.
>>
>> The ice_release_res() is almost always invoked during downloading DDP
>> when modprobe ice.
>>
>> IMO, it seems like that only when the NIC hardware or firmware enters
>> a bad state causing single command to fail or timeout (1 second), and
>> then here do the retry logic (10 senconds).
>>
>> So it's hard to validate on healthy NIC, maybe inject faults in low level
>> function, such as ice_sq_send_cmd().
> 
> In that case I would suggest adding something like this:
> 
> Found by inspection (or static analysis, or a specific tool if publicly
> available, ...).
> Compile tested only.
> 

Sure, I'll send v2 later.

>>
>>> And you can add my:
>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>>>
>>>
>>>> Fixes: 5f6df173f92e ("ice: implement and use rd32_poll_timeout for
>>>> ice_sq_done timeout")
>>>> Signed-off-by: Ding Hui <dinghui@...gfor.com.cn>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>>>> b/drivers/net/ethernet/intel/ice/ice_common.c
>>>> index 6fb0c1e8ae7c..5005c299deb1 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>>>> @@ -1885,7 +1885,7 @@ void ice_release_res(struct ice_hw *hw, enum
>>>> ice_aq_res_ids res)
>>>>    	/* there are some rare cases when trying to release the
>>>> resource
>>>>    	 * results in an admin queue timeout, so handle them correctly
>>>>    	 */
>>>> -	timeout = jiffies + 10 * ICE_CTL_Q_SQ_CMD_TIMEOUT;
>>>> +	timeout = jiffies + 10 *
>>>> usecs_to_jiffies(ICE_CTL_Q_SQ_CMD_TIMEOUT);
>>>>    	do {
>>>>    		status = ice_aq_release_res(hw, res, 0, NULL);
>>>>    		if (status != -EIO)
>>>> --
>>>> 2.17.1
>>>
>>>
>>>
>>
>> -- 
>> Thanks,
>> - Ding Hui
>>
>>
> 
> 

-- 
Thanks,
-dinghui