[v2] ice: wait for reset completion in ice_resume()

[PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Aaron Ma 1 month, 3 weeks ago

ice_resume() schedules an asynchronous PF reset and returns
immediately. The reset runs later in ice_service_task(). If
userspace tries to bring up the net device before the reset
finishes, ice_open() fails with -EBUSY:

  ice_resume()
    ice_schedule_reset()          # sets ICE_PFR_REQ, returns
  ...
  ice_open()
    ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
  ...
  ice_service_task()
    ice_do_reset()
      ice_rebuild()               # clears ICE_PFR_REQ, too late

Reproduced on E800 series NICs during suspend/resume with irdma
enabled, where the aux device probe widens the race window.

Wait for the reset to complete before returning from ice_resume().

Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
v2: reword comment to clarify best-effort semantics (Kohei Enju)

 drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5f92377d4dfc2..a81eb21ea87c1 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
 	/* Restart the service task */
 	mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
 
+	/* Best-effort wait for the scheduled reset to finish so that the
+	 * device is operational before returning. Without this, userspace
+	 * (e.g. NetworkManager) may try to open the net device while the
+	 * asynchronous reset is still in progress, hitting -EBUSY.
+	 */
+	ret = ice_wait_for_reset(pf, 10 * HZ);
+	if (ret)
+		dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
+
 	return 0;
 }
 
-- 
2.43.0

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Paul Menzel 1 month, 2 weeks ago

Dear Aaron,


Thank you for your patch.

Am 24.04.26 um 05:03 schrieb Aaron Ma via Intel-wired-lan:
> ice_resume() schedules an asynchronous PF reset and returns
> immediately. The reset runs later in ice_service_task(). If
> userspace tries to bring up the net device before the reset
> finishes, ice_open() fails with -EBUSY:
> 
>    ice_resume()
>      ice_schedule_reset()          # sets ICE_PFR_REQ, returns
>    ...
>    ice_open()
>      ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
>    ...
>    ice_service_task()
>      ice_do_reset()
>        ice_rebuild()               # clears ICE_PFR_REQ, too late
> 
> Reproduced on E800 series NICs during suspend/resume with irdma
> enabled, where the aux device probe widens the race window.

Please document, how you reproduced it, and also paste possible messages 
by Linux or NetworkManager, so that people can easily search for the commit.

> Wait for the reset to complete before returning from ice_resume().

Please mention the delay length in the commit message.

> Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> v2: reword comment to clarify best-effort semantics (Kohei Enju)
> 
>   drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 5f92377d4dfc2..a81eb21ea87c1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
>   	/* Restart the service task */
>   	mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
>   
> +	/* Best-effort wait for the scheduled reset to finish so that the
> +	 * device is operational before returning. Without this, userspace
> +	 * (e.g. NetworkManager) may try to open the net device while the
> +	 * asynchronous reset is still in progress, hitting -EBUSY.
> +	 */
> +	ret = ice_wait_for_reset(pf, 10 * HZ);

Why not pass a delay in micro/milliseconds?

> +	if (ret)
> +		dev_err(dev, "Wait for reset failed during resume: %d\n", ret);

Mention the delay?

> +
>   	return 0;
>   }
>   


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Aaron Ma 1 month, 2 weeks ago

On Mon, Apr 27, 2026 at 6:13 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Aaron,
>
>
> Thank you for your patch.
>
> Am 24.04.26 um 05:03 schrieb Aaron Ma via Intel-wired-lan:
> > ice_resume() schedules an asynchronous PF reset and returns
> > immediately. The reset runs later in ice_service_task(). If
> > userspace tries to bring up the net device before the reset
> > finishes, ice_open() fails with -EBUSY:
> >
> >    ice_resume()
> >      ice_schedule_reset()          # sets ICE_PFR_REQ, returns
> >    ...
> >    ice_open()
> >      ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
> >    ...
> >    ice_service_task()
> >      ice_do_reset()
> >        ice_rebuild()               # clears ICE_PFR_REQ, too late
> >
> > Reproduced on E800 series NICs during suspend/resume with irdma
> > enabled, where the aux device probe widens the race window.
>
> Please document, how you reproduced it, and also paste possible messages
> by Linux or NetworkManager, so that people can easily search for the commit.
>

The error message is "can't open net device while reset is in progress"
I can add it in v3 if you like.

 > > Wait for the reset to complete before returning from ice_resume().
>
> Please mention the delay length in the commit message.

The timeout is 10 * HZ (10 seconds), matching the existing usage in
ice_devlink_info_get() for the same ice_wait_for_reset() call. In
practice the wait completes in ~300ms.

>
> > Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> > v2: reword comment to clarify best-effort semantics (Kohei Enju)
> >
> >   drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> > index 5f92377d4dfc2..a81eb21ea87c1 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
> >       /* Restart the service task */
> >       mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
> >
> > +     /* Best-effort wait for the scheduled reset to finish so that the
> > +      * device is operational before returning. Without this, userspace
> > +      * (e.g. NetworkManager) may try to open the net device while the
> > +      * asynchronous reset is still in progress, hitting -EBUSY.
> > +      */
> > +     ret = ice_wait_for_reset(pf, 10 * HZ);
>
> Why not pass a delay in micro/milliseconds?

ice_wait_for_reset() takes jiffies — that's the existing API.

>
> > +     if (ret)
> > +             dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
>
> Mention the delay?

Good point. I'll include the timeout in the error message in v3.

Thanks,
Aaron


>
> > +
> >       return 0;
> >   }
> >
>
>
> Kind regards,
>
> Paul

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Paul Menzel 1 month, 2 weeks ago

Dear Aaron,


Thank you for your reply.

Am 28.04.26 um 09:53 schrieb Aaron Ma:
> On Mon, Apr 27, 2026 at 6:13 PM Paul Menzel wrote:

>> Am 24.04.26 um 05:03 schrieb Aaron Ma via Intel-wired-lan:
>>> ice_resume() schedules an asynchronous PF reset and returns
>>> immediately. The reset runs later in ice_service_task(). If
>>> userspace tries to bring up the net device before the reset
>>> finishes, ice_open() fails with -EBUSY:
>>>
>>>     ice_resume()
>>>       ice_schedule_reset()          # sets ICE_PFR_REQ, returns
>>>     ...
>>>     ice_open()
>>>       ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
>>>     ...
>>>     ice_service_task()
>>>       ice_do_reset()
>>>         ice_rebuild()               # clears ICE_PFR_REQ, too late
>>>
>>> Reproduced on E800 series NICs during suspend/resume with irdma
>>> enabled, where the aux device probe widens the race window.
>>
>> Please document, how you reproduced it, and also paste possible messages
>> by Linux or NetworkManager, so that people can easily search for the commit.
> 
> The error message is "can't open net device while reset is in progress"
> I can add it in v3 if you like.

Yes, that’d be great.

>   > > Wait for the reset to complete before returning from ice_resume().
>>
>> Please mention the delay length in the commit message.
> 
> The timeout is 10 * HZ (10 seconds), matching the existing usage in
> ice_devlink_info_get() for the same ice_wait_for_reset() call. In
> practice the wait completes in ~300ms.

I often wonder, where the delay values come from. Maybe mention, that 
you copied it.

>>> Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>> ---
>>> v2: reword comment to clarify best-effort semantics (Kohei Enju)
>>>
>>>    drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>> index 5f92377d4dfc2..a81eb21ea87c1 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>> @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
>>>        /* Restart the service task */
>>>        mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
>>>
>>> +     /* Best-effort wait for the scheduled reset to finish so that the
>>> +      * device is operational before returning. Without this, userspace
>>> +      * (e.g. NetworkManager) may try to open the net device while the
>>> +      * asynchronous reset is still in progress, hitting -EBUSY.
>>> +      */
>>> +     ret = ice_wait_for_reset(pf, 10 * HZ);
>>
>> Why not pass a delay in micro/milliseconds?
> 
> ice_wait_for_reset() takes jiffies — that's the existing API.

It’s recommended to use `msecs_to_jiffies()` to make it HZ invariant.

>>> +     if (ret)
>>> +             dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
>>
>> Mention the delay?
> 
> Good point. I'll include the timeout in the error message in v3.

Awesome.

[…]


Thanks,

Paul

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Przemek Kitszel 1 month, 2 weeks ago

>>>> +     ret = ice_wait_for_reset(pf, 10 * HZ);
>>>
>>> Why not pass a delay in micro/milliseconds?
>>
>> ice_wait_for_reset() takes jiffies — that's the existing API.
> 
> It’s recommended to use `msecs_to_jiffies()` to make it HZ invariant.

there is also secs_to_jiffies()

> 
>>>> +     if (ret)
>>>> +             dev_err(dev, "Wait for reset failed during resume: 
>>>> %d\n", ret);
>>>
>>> Mention the delay?
>>
>> Good point. I'll include the timeout in the error message in v3.
> 
> Awesome.
> 
> […]
> 
> 
> Thanks,
> 
> Paul

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Aaron Ma 1 month, 2 weeks ago

On Tue, Apr 28, 2026 at 9:08 PM Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
>
> >>>> +     ret = ice_wait_for_reset(pf, 10 * HZ);
> >>>
> >>> Why not pass a delay in micro/milliseconds?
> >>
> >> ice_wait_for_reset() takes jiffies — that's the existing API.
> >
> > It’s recommended to use `msecs_to_jiffies()` to make it HZ invariant.
>
> there is also secs_to_jiffies()

Thank you very much.

V4 is sent.

Aaron

>
> >
> >>>> +     if (ret)
> >>>> +             dev_err(dev, "Wait for reset failed during resume:
> >>>> %d\n", ret);
> >>>
> >>> Mention the delay?
> >>
> >> Good point. I'll include the timeout in the error message in v3.
> >
> > Awesome.
> >
> > […]
> >
> >
> > Thanks,
> >
> > Paul
>

Re: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Przemek Kitszel 1 month, 2 weeks ago

On 4/24/26 05:03, Aaron Ma via Intel-wired-lan wrote:
> ice_resume() schedules an asynchronous PF reset and returns
> immediately. The reset runs later in ice_service_task(). If
> userspace tries to bring up the net device before the reset
> finishes, ice_open() fails with -EBUSY:
> 
>    ice_resume()
>      ice_schedule_reset()          # sets ICE_PFR_REQ, returns
>    ...
>    ice_open()
>      ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
>    ...
>    ice_service_task()
>      ice_do_reset()
>        ice_rebuild()               # clears ICE_PFR_REQ, too late
> 
> Reproduced on E800 series NICs during suspend/resume with irdma
> enabled, where the aux device probe widens the race window.
> 
> Wait for the reset to complete before returning from ice_resume().
> 
> Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>

thank you,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

> ---
> v2: reword comment to clarify best-effort semantics (Kohei Enju)
> 
>   drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 5f92377d4dfc2..a81eb21ea87c1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
>   	/* Restart the service task */
>   	mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
>   
> +	/* Best-effort wait for the scheduled reset to finish so that the
> +	 * device is operational before returning. Without this, userspace
> +	 * (e.g. NetworkManager) may try to open the net device while the
> +	 * asynchronous reset is still in progress, hitting -EBUSY.
> +	 */
> +	ret = ice_wait_for_reset(pf, 10 * HZ);
> +	if (ret)
> +		dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
> +
>   	return 0;
>   }
>

Re: [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Kohei Enju 1 month, 3 weeks ago

On 04/24 11:03, Aaron Ma wrote:
> ice_resume() schedules an asynchronous PF reset and returns
> immediately. The reset runs later in ice_service_task(). If
> userspace tries to bring up the net device before the reset
> finishes, ice_open() fails with -EBUSY:
> 
>   ice_resume()
>     ice_schedule_reset()          # sets ICE_PFR_REQ, returns
>   ...
>   ice_open()
>     ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
>   ...
>   ice_service_task()
>     ice_do_reset()
>       ice_rebuild()               # clears ICE_PFR_REQ, too late
> 
> Reproduced on E800 series NICs during suspend/resume with irdma
> enabled, where the aux device probe widens the race window.
> 
> Wait for the reset to complete before returning from ice_resume().
> 
> Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> v2: reword comment to clarify best-effort semantics (Kohei Enju)
> 
>  drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 5f92377d4dfc2..a81eb21ea87c1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
>  	/* Restart the service task */
>  	mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
>  
> +	/* Best-effort wait for the scheduled reset to finish so that the
> +	 * device is operational before returning. Without this, userspace
> +	 * (e.g. NetworkManager) may try to open the net device while the
> +	 * asynchronous reset is still in progress, hitting -EBUSY.
> +	 */

Thanks for the update!

Reviewed-by: Kohei Enju <kohei@enjuk.jp>

> +	ret = ice_wait_for_reset(pf, 10 * HZ);
> +	if (ret)
> +		dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
> +
>  	return 0;
>  }
>  
> -- 
> 2.43.0
>

RE: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion in ice_resume()

Posted by Loktionov, Aleksandr 1 month, 3 weeks ago


> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Aaron Ma via Intel-wired-lan
> Sent: Friday, April 24, 2026 5:04 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>; Jesse Brandeburg
> <jesse.brandeburg@intel.com>; intel-wired-lan@lists.osuosl.org; Kohei
> Enju <kohei@enjuk.jp>
> Subject: [Intel-wired-lan] [PATCH v2] ice: wait for reset completion
> in ice_resume()
> 
> ice_resume() schedules an asynchronous PF reset and returns
> immediately. The reset runs later in ice_service_task(). If userspace
> tries to bring up the net device before the reset finishes, ice_open()
> fails with -EBUSY:
> 
>   ice_resume()
>     ice_schedule_reset()          # sets ICE_PFR_REQ, returns
>   ...
>   ice_open()
>     ice_is_reset_in_progress()    # ICE_PFR_REQ still set, -EBUSY
>   ...
>   ice_service_task()
>     ice_do_reset()
>       ice_rebuild()               # clears ICE_PFR_REQ, too late
> 
> Reproduced on E800 series NICs during suspend/resume with irdma
> enabled, where the aux device probe widens the race window.
> 
> Wait for the reset to complete before returning from ice_resume().
> 
> Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> v2: reword comment to clarify best-effort semantics (Kohei Enju)
> 
>  drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 5f92377d4dfc2..a81eb21ea87c1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
>  	/* Restart the service task */
>  	mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf-
> >serv_tmr_period));
> 
> +	/* Best-effort wait for the scheduled reset to finish so that
> the
> +	 * device is operational before returning. Without this,
> userspace
> +	 * (e.g. NetworkManager) may try to open the net device while
> the
> +	 * asynchronous reset is still in progress, hitting -EBUSY.
> +	 */
> +	ret = ice_wait_for_reset(pf, 10 * HZ);
> +	if (ret)
> +		dev_err(dev, "Wait for reset failed during resume:
> %d\n", ret);
> +
>  	return 0;
>  }
> 
> --
> 2.43.0


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>