netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] improve i40e parallel VF reset handling
@ 2025-05-16  9:47 Robert Malz
  2025-05-16  9:47 ` [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress Robert Malz
  2025-05-16  9:47 ` [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset Robert Malz
  0 siblings, 2 replies; 7+ messages in thread
From: Robert Malz @ 2025-05-16  9:47 UTC (permalink / raw)
  To: netdev, intel-wired-lan, anthony.l.nguyen, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller

When the i40e driver receives VF reset requests from multiple sources,
some requests may not be handled. For example, a VFLR interrupt might
be ignored if it occurs while a VF is already resetting as part of an
`ndo` request. In such scenarios, the VFLR is lost and, depending on
timing, the VF may be left uninitialized. This can cause the VF driver
to become stuck in an initialization loop until another VF reset is
triggered.

Currently, in i40e_vc_reset_vf, the driver attempts to reset the VF up
to 20 times, logging an error if all attempts fail. This logic assumes
that i40e_reset_vf returns false when another reset is already in
progress. However, i40e_reset_vf currently always returns true, which
causes overlapping resets to be silently ignored.

The first patch updates i40e_reset_vf to return false if a reset is
already in progress. This aligns with the retry logic used in
i40e_vc_reset_vf.

While the first patch addresses resets triggered via ndo operations,
VFLR interrupts can also initiate VF resets. In that case, the driver
directly calls i40e_reset_vf, and if the reset is skipped due to
another one being in progress, the VF reest is not retried. The
second patch addresses this by re-setting the I40E_VFLR_EVENT_PENDING
bit, ensuring the VFLR is handled during the next service task execution.

---
Changes in v2:
- Patch 1: modified doc string for i40e_reset_vf function
- Patch 2: removed unnecessary doc string changes from the patch

Robert Malz (2):
  i40e: return false from i40e_reset_vf if reset is in progress
  i40e: retry VFLR handling if there is ongoing VF reset

 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress
  2025-05-16  9:47 [PATCH v2 0/2] improve i40e parallel VF reset handling Robert Malz
@ 2025-05-16  9:47 ` Robert Malz
  2025-05-16 21:06   ` Nelson, Shannon
  2025-05-19 21:22   ` Tony Nguyen
  2025-05-16  9:47 ` [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset Robert Malz
  1 sibling, 2 replies; 7+ messages in thread
From: Robert Malz @ 2025-05-16  9:47 UTC (permalink / raw)
  To: netdev, intel-wired-lan, anthony.l.nguyen, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller

The function i40e_vc_reset_vf attempts, up to 20 times, to handle a
VF reset request, using the return value of i40e_reset_vf as an indicator
of whether the reset was successfully triggered. Currently, i40e_reset_vf
always returns true, which causes new reset requests to be ignored if a
different VF reset is already in progress.

This patch updates the return value of i40e_reset_vf to reflect when
another VF reset is in progress, allowing the caller to properly use
the retry mechanism.

Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
Signed-off-by: Robert Malz <robert.malz@canonical.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 1120f8e4bb67..2f1aa18bcfb8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1546,8 +1546,8 @@ static void i40e_cleanup_reset_vf(struct i40e_vf *vf)
  * @vf: pointer to the VF structure
  * @flr: VFLR was issued or not
  *
- * Returns true if the VF is in reset, resets successfully, or resets
- * are disabled and false otherwise.
+ * Returns true if reset was performed successfully or if resets are
+ * disabled. False if reset is already in progress.
  **/
 bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
 {
@@ -1566,7 +1566,7 @@ bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
 
 	/* If VF is being reset already we don't need to continue. */
 	if (test_and_set_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
-		return true;
+		return false;
 
 	i40e_trigger_vf_reset(vf, flr);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset
  2025-05-16  9:47 [PATCH v2 0/2] improve i40e parallel VF reset handling Robert Malz
  2025-05-16  9:47 ` [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress Robert Malz
@ 2025-05-16  9:47 ` Robert Malz
  2025-05-16 21:06   ` Nelson, Shannon
  1 sibling, 1 reply; 7+ messages in thread
From: Robert Malz @ 2025-05-16  9:47 UTC (permalink / raw)
  To: netdev, intel-wired-lan, anthony.l.nguyen, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller

When a VFLR interrupt is received during a VF reset initiated from a
different source, the VFLR may be not fully handled. This can
leave the VF in an undefined state.
To address this, set the I40E_VFLR_EVENT_PENDING bit again during VFLR
handling if the reset is not yet complete. This ensures the driver
will properly complete the VF reset in such scenarios.

Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
Signed-off-by: Robert Malz <robert.malz@canonical.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 2f1aa18bcfb8..6b13ac85016f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4328,7 +4328,10 @@ int i40e_vc_process_vflr_event(struct i40e_pf *pf)
 		reg = rd32(hw, I40E_GLGEN_VFLRSTAT(reg_idx));
 		if (reg & BIT(bit_idx))
 			/* i40e_reset_vf will clear the bit in GLGEN_VFLRSTAT */
-			i40e_reset_vf(vf, true);
+			if (!i40e_reset_vf(vf, true)) {
+				/* At least one VF did not finish resetting, retry next time */
+				set_bit(__I40E_VFLR_EVENT_PENDING, pf->state);
+			}
 	}
 
 	return 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress
  2025-05-16  9:47 ` [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress Robert Malz
@ 2025-05-16 21:06   ` Nelson, Shannon
  2025-05-19 21:22   ` Tony Nguyen
  1 sibling, 0 replies; 7+ messages in thread
From: Nelson, Shannon @ 2025-05-16 21:06 UTC (permalink / raw)
  To: Robert Malz, netdev, intel-wired-lan, anthony.l.nguyen,
	przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller

On 5/16/2025 2:47 AM, Robert Malz wrote:
> 
> The function i40e_vc_reset_vf attempts, up to 20 times, to handle a
> VF reset request, using the return value of i40e_reset_vf as an indicator
> of whether the reset was successfully triggered. Currently, i40e_reset_vf
> always returns true, which causes new reset requests to be ignored if a
> different VF reset is already in progress.
> 
> This patch updates the return value of i40e_reset_vf to reflect when
> another VF reset is in progress, allowing the caller to properly use
> the retry mechanism.
> 
> Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
> Signed-off-by: Robert Malz <robert.malz@canonical.com>

Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>

> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 1120f8e4bb67..2f1aa18bcfb8 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -1546,8 +1546,8 @@ static void i40e_cleanup_reset_vf(struct i40e_vf *vf)
>    * @vf: pointer to the VF structure
>    * @flr: VFLR was issued or not
>    *
> - * Returns true if the VF is in reset, resets successfully, or resets
> - * are disabled and false otherwise.
> + * Returns true if reset was performed successfully or if resets are
> + * disabled. False if reset is already in progress.
>    **/
>   bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
>   {
> @@ -1566,7 +1566,7 @@ bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
> 
>          /* If VF is being reset already we don't need to continue. */
>          if (test_and_set_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
> -               return true;
> +               return false;
> 
>          i40e_trigger_vf_reset(vf, flr);
> 
> --
> 2.34.1
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset
  2025-05-16  9:47 ` [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset Robert Malz
@ 2025-05-16 21:06   ` Nelson, Shannon
  0 siblings, 0 replies; 7+ messages in thread
From: Nelson, Shannon @ 2025-05-16 21:06 UTC (permalink / raw)
  To: Robert Malz, netdev, intel-wired-lan, anthony.l.nguyen,
	przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller



On 5/16/2025 2:47 AM, Robert Malz wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> When a VFLR interrupt is received during a VF reset initiated from a
> different source, the VFLR may be not fully handled. This can
> leave the VF in an undefined state.
> To address this, set the I40E_VFLR_EVENT_PENDING bit again during VFLR
> handling if the reset is not yet complete. This ensures the driver
> will properly complete the VF reset in such scenarios.
> 
> Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
> Signed-off-by: Robert Malz <robert.malz@canonical.com>

Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>

> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 2f1aa18bcfb8..6b13ac85016f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4328,7 +4328,10 @@ int i40e_vc_process_vflr_event(struct i40e_pf *pf)
>                  reg = rd32(hw, I40E_GLGEN_VFLRSTAT(reg_idx));
>                  if (reg & BIT(bit_idx))
>                          /* i40e_reset_vf will clear the bit in GLGEN_VFLRSTAT */
> -                       i40e_reset_vf(vf, true);
> +                       if (!i40e_reset_vf(vf, true)) {
> +                               /* At least one VF did not finish resetting, retry next time */
> +                               set_bit(__I40E_VFLR_EVENT_PENDING, pf->state);
> +                       }
>          }
> 
>          return 0;
> --
> 2.34.1
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress
  2025-05-16  9:47 ` [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress Robert Malz
  2025-05-16 21:06   ` Nelson, Shannon
@ 2025-05-19 21:22   ` Tony Nguyen
  2025-05-20  8:33     ` Robert Malz
  1 sibling, 1 reply; 7+ messages in thread
From: Tony Nguyen @ 2025-05-19 21:22 UTC (permalink / raw)
  To: Robert Malz, netdev, intel-wired-lan, przemyslaw.kitszel,
	andrew+netdev, davem, edumazet, kuba, pabeni,
	sylwesterx.dziedziuch, mateusz.palczewski, jacob.e.keller



On 5/16/2025 2:47 AM, Robert Malz wrote:
> The function i40e_vc_reset_vf attempts, up to 20 times, to handle a
> VF reset request, using the return value of i40e_reset_vf as an indicator
> of whether the reset was successfully triggered. Currently, i40e_reset_vf
> always returns true, which causes new reset requests to be ignored if a
> different VF reset is already in progress.
> 
> This patch updates the return value of i40e_reset_vf to reflect when
> another VF reset is in progress, allowing the caller to properly use
> the retry mechanism.
> 
> Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
> Signed-off-by: Robert Malz <robert.malz@canonical.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 1120f8e4bb67..2f1aa18bcfb8 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -1546,8 +1546,8 @@ static void i40e_cleanup_reset_vf(struct i40e_vf *vf)
>    * @vf: pointer to the VF structure
>    * @flr: VFLR was issued or not
>    *
> - * Returns true if the VF is in reset, resets successfully, or resets
> - * are disabled and false otherwise.
> + * Returns true if reset was performed successfully or if resets are
> + * disabled. False if reset is already in progress.

nit but if we are editing this, let's make kdoc happy. Please start with 
Return: or Returns:

Thanks,
Tony

>    **/
>   bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
>   {
> @@ -1566,7 +1566,7 @@ bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
>   
>   	/* If VF is being reset already we don't need to continue. */
>   	if (test_and_set_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
> -		return true;
> +		return false;
>   
>   	i40e_trigger_vf_reset(vf, flr);
>   


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress
  2025-05-19 21:22   ` Tony Nguyen
@ 2025-05-20  8:33     ` Robert Malz
  0 siblings, 0 replies; 7+ messages in thread
From: Robert Malz @ 2025-05-20  8:33 UTC (permalink / raw)
  To: Tony Nguyen
  Cc: netdev, intel-wired-lan, przemyslaw.kitszel, andrew+netdev, davem,
	edumazet, kuba, pabeni, sylwesterx.dziedziuch, mateusz.palczewski,
	jacob.e.keller

On Mon, May 19, 2025 at 11:22 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
>
>
> On 5/16/2025 2:47 AM, Robert Malz wrote:
> > The function i40e_vc_reset_vf attempts, up to 20 times, to handle a
> > VF reset request, using the return value of i40e_reset_vf as an indicator
> > of whether the reset was successfully triggered. Currently, i40e_reset_vf
> > always returns true, which causes new reset requests to be ignored if a
> > different VF reset is already in progress.
> >
> > This patch updates the return value of i40e_reset_vf to reflect when
> > another VF reset is in progress, allowing the caller to properly use
> > the retry mechanism.
> >
> > Fixes: 52424f974bc5 ("i40e: Fix VF hang when reset is triggered on another VF")
> > Signed-off-by: Robert Malz <robert.malz@canonical.com>
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > index 1120f8e4bb67..2f1aa18bcfb8 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > @@ -1546,8 +1546,8 @@ static void i40e_cleanup_reset_vf(struct i40e_vf *vf)
> >    * @vf: pointer to the VF structure
> >    * @flr: VFLR was issued or not
> >    *
> > - * Returns true if the VF is in reset, resets successfully, or resets
> > - * are disabled and false otherwise.
> > + * Returns true if reset was performed successfully or if resets are
> > + * disabled. False if reset is already in progress.
>
> nit but if we are editing this, let's make kdoc happy. Please start with
> Return: or Returns:
>
> Thanks,
> Tony
>

Hey Tony, thanks for the review. Fixed in v3.
Regards,
Robert

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-05-20  8:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-16  9:47 [PATCH v2 0/2] improve i40e parallel VF reset handling Robert Malz
2025-05-16  9:47 ` [PATCH v2 1/2] i40e: return false from i40e_reset_vf if reset is in progress Robert Malz
2025-05-16 21:06   ` Nelson, Shannon
2025-05-19 21:22   ` Tony Nguyen
2025-05-20  8:33     ` Robert Malz
2025-05-16  9:47 ` [PATCH v2 2/2] i40e: retry VFLR handling if there is ongoing VF reset Robert Malz
2025-05-16 21:06   ` Nelson, Shannon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).