[PATCH net-next 0/2][pull request] Intel Wired LAN Driver Updates 2023-07-10 (i40e)

[PATCH net-next 0/2][pull request] Intel Wired LAN Driver Updates 2023-07-10 (i40e)

[Tony Nguyen]

This series contains updates to i40e driver only.

Ivan Vecera adds waiting for VF to complete initialization on VF related
configuration callbacks.

The following are changes since commit 6843306689aff3aea608e4d2630b2a5a0137f827:
  Merge tag 'net-6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 40GbE

Ivan Vecera (2):
  i40e: Add helper for VF inited state check with timeout
  i40e: Wait for pending VF reset in VF set callbacks

 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 63 +++++++++++--------
 1 file changed, 36 insertions(+), 27 deletions(-)

-- 
2.38.1

[PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Move the check for VF inited state (with optional up-to 300ms
timeout to separate helper i40e_check_vf_init_timeout() that
will be used in the following commit.

Tested-by: Ma Yuying <yuma@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index be59ba3774e1..b84b6b675fa7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4304,6 +4304,36 @@ static int i40e_validate_vf(struct i40e_pf *pf, int vf_id)
 	return ret;
 }
 
+/**
+ * i40e_check_vf_init_timeout
+ * @vf: the virtual function
+ *
+ * Check that the VF's initialization was successfully done and if not
+ * wait up to 300ms for its finish.
+ *
+ * Returns true when VF is initialized, false on timeout
+ **/
+static bool i40e_check_vf_init_timeout(struct i40e_vf *vf)
+{
+	int i;
+
+	/* When the VF is resetting wait until it is done.
+	 * It can take up to 200 milliseconds, but wait for
+	 * up to 300 milliseconds to be safe.
+	 */
+	for (i = 0; i < 15; i++) {
+		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
+			return true;
+
+		msleep(20);
+	}
+
+	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
+		vf->vf_id);
+
+	return false;
+}
+
 /**
  * i40e_ndo_set_vf_mac
  * @netdev: network interface device structure
@@ -4322,7 +4352,6 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 	int ret = 0;
 	struct hlist_node *h;
 	int bkt;
-	u8 i;
 
 	if (test_and_set_bit(__I40E_VIRTCHNL_OP_PENDING, pf->state)) {
 		dev_warn(&pf->pdev->dev, "Unable to configure VFs, other operation is pending.\n");
@@ -4335,21 +4364,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		goto error_param;
 
 	vf = &pf->vf[vf_id];
-
-	/* When the VF is resetting wait until it is done.
-	 * It can take up to 200 milliseconds,
-	 * but wait for up to 300 milliseconds to be safe.
-	 * Acquire the VSI pointer only after the VF has been
-	 * properly initialized.
-	 */
-	for (i = 0; i < 15; i++) {
-		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
-			break;
-		msleep(20);
-	}
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error_param;
 	}
-- 
2.38.1

Re: [PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Leon Romanovsky]

On Mon, Jul 10, 2023 at 09:40:29AM -0700, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Move the check for VF inited state (with optional up-to 300ms
> timeout to separate helper i40e_check_vf_init_timeout() that
> will be used in the following commit.
> 
> Tested-by: Ma Yuying <yuma@redhat.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 47 ++++++++++++-------
>  1 file changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index be59ba3774e1..b84b6b675fa7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4304,6 +4304,36 @@ static int i40e_validate_vf(struct i40e_pf *pf, int vf_id)
>  	return ret;
>  }
>  
> +/**
> + * i40e_check_vf_init_timeout
> + * @vf: the virtual function
> + *
> + * Check that the VF's initialization was successfully done and if not
> + * wait up to 300ms for its finish.
> + *
> + * Returns true when VF is initialized, false on timeout
> + **/
> +static bool i40e_check_vf_init_timeout(struct i40e_vf *vf)
> +{
> +	int i;
> +
> +	/* When the VF is resetting wait until it is done.
> +	 * It can take up to 200 milliseconds, but wait for
> +	 * up to 300 milliseconds to be safe.
> +	 */
> +	for (i = 0; i < 15; i++) {
> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
> +			return true;
> +
> +		msleep(20);
> +	}
> +
> +	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
> +		vf->vf_id);

This error is not accurate in the edge case, when VF state changed to
be INIT during msleep() while i was 14.

Thanks

> +
> +	return false;
> +}
> +
>  /**
>   * i40e_ndo_set_vf_mac
>   * @netdev: network interface device structure
> @@ -4322,7 +4352,6 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  	int ret = 0;
>  	struct hlist_node *h;
>  	int bkt;
> -	u8 i;
>  
>  	if (test_and_set_bit(__I40E_VIRTCHNL_OP_PENDING, pf->state)) {
>  		dev_warn(&pf->pdev->dev, "Unable to configure VFs, other operation is pending.\n");
> @@ -4335,21 +4364,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  		goto error_param;
>  
>  	vf = &pf->vf[vf_id];
> -
> -	/* When the VF is resetting wait until it is done.
> -	 * It can take up to 200 milliseconds,
> -	 * but wait for up to 300 milliseconds to be safe.
> -	 * Acquire the VSI pointer only after the VF has been
> -	 * properly initialized.
> -	 */
> -	for (i = 0; i < 15; i++) {
> -		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
> -			break;
> -		msleep(20);
> -	}
> -	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
> -		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
> -			vf_id);
> +	if (!i40e_check_vf_init_timeout(vf)) {
>  		ret = -EAGAIN;
>  		goto error_param;
>  	}
> -- 
> 2.38.1
> 
> 

Re: [PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Jakub Kicinski]

On Tue, 11 Jul 2023 15:09:04 +0300 Leon Romanovsky wrote:
> > +	for (i = 0; i < 15; i++) {
> > +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
> > +			return true;
> > +
> > +		msleep(20);
> > +	}
> > +
> > +	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
> > +		vf->vf_id);  
> 
> This error is not accurate in the edge case, when VF state changed to
> be INIT during msleep() while i was 14.

Right, it's a change in behavior from existing code,
old code re-checked if INIT is set after the last sleep.
-- 
pw-bot: cr

Re: [PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Ivan Vecera]

On 12. 07. 23 2:37, Jakub Kicinski wrote:
> On Tue, 11 Jul 2023 15:09:04 +0300 Leon Romanovsky wrote:
>>> +	for (i = 0; i < 15; i++) {
>>> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>>> +			return true;
>>> +
>>> +		msleep(20);
>>> +	}
>>> +
>>> +	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
>>> +		vf->vf_id);
>>
>> This error is not accurate in the edge case, when VF state changed to
>> be INIT during msleep() while i was 14.
> 
> Right, it's a change in behavior from existing code,
> old code re-checked if INIT is set after the last sleep.

I will simplify this and add an extra check.

Thanks for the review..

I.

Re: [PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Ivan Vecera]

On 11. 07. 23 14:09, Leon Romanovsky wrote:
> On Mon, Jul 10, 2023 at 09:40:29AM -0700, Tony Nguyen wrote:
>> From: Ivan Vecera <ivecera@redhat.com>
>>
>> Move the check for VF inited state (with optional up-to 300ms
>> timeout to separate helper i40e_check_vf_init_timeout() that
>> will be used in the following commit.
>>
>> Tested-by: Ma Yuying <yuma@redhat.com>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> Reviewed-by: Simon Horman <simon.horman@corigine.com>
>> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
>> ---
>>   .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 47 ++++++++++++-------
>>   1 file changed, 31 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> index be59ba3774e1..b84b6b675fa7 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> @@ -4304,6 +4304,36 @@ static int i40e_validate_vf(struct i40e_pf *pf, int vf_id)
>>   	return ret;
>>   }
>>   
>> +/**
>> + * i40e_check_vf_init_timeout
>> + * @vf: the virtual function
>> + *
>> + * Check that the VF's initialization was successfully done and if not
>> + * wait up to 300ms for its finish.
>> + *
>> + * Returns true when VF is initialized, false on timeout
>> + **/
>> +static bool i40e_check_vf_init_timeout(struct i40e_vf *vf)
>> +{
>> +	int i;
>> +
>> +	/* When the VF is resetting wait until it is done.
>> +	 * It can take up to 200 milliseconds, but wait for
>> +	 * up to 300 milliseconds to be safe.
>> +	 */
>> +	for (i = 0; i < 15; i++) {
>> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>> +			return true;
>> +
>> +		msleep(20);
>> +	}
>> +
>> +	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
>> +		vf->vf_id);
> 
> This error is not accurate in the edge case, when VF state changed to
> be INIT during msleep() while i was 14.
> 
> Thanks

Would you like to add an extra check after the cycle or just increase 
limit from 15 to 16 (there will be an extra msleep)...

Ivan
> 
>> +
>> +	return false;
>> +}
>> +
>>   /**
>>    * i40e_ndo_set_vf_mac
>>    * @netdev: network interface device structure
>> @@ -4322,7 +4352,6 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>>   	int ret = 0;
>>   	struct hlist_node *h;
>>   	int bkt;
>> -	u8 i;
>>   
>>   	if (test_and_set_bit(__I40E_VIRTCHNL_OP_PENDING, pf->state)) {
>>   		dev_warn(&pf->pdev->dev, "Unable to configure VFs, other operation is pending.\n");
>> @@ -4335,21 +4364,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>>   		goto error_param;
>>   
>>   	vf = &pf->vf[vf_id];
>> -
>> -	/* When the VF is resetting wait until it is done.
>> -	 * It can take up to 200 milliseconds,
>> -	 * but wait for up to 300 milliseconds to be safe.
>> -	 * Acquire the VSI pointer only after the VF has been
>> -	 * properly initialized.
>> -	 */
>> -	for (i = 0; i < 15; i++) {
>> -		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>> -			break;
>> -		msleep(20);
>> -	}
>> -	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
>> -		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
>> -			vf_id);
>> +	if (!i40e_check_vf_init_timeout(vf)) {
>>   		ret = -EAGAIN;
>>   		goto error_param;
>>   	}
>> -- 
>> 2.38.1
>>
>>
> 

Re: [PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Jakub Kicinski]

On Wed, 12 Jul 2023 15:18:29 +0200 Ivan Vecera wrote:
> > This error is not accurate in the edge case, when VF state changed to
> > be INIT during msleep() while i was 14.
> > 
> > Thanks  
> 
> Would you like to add an extra check after the cycle or just increase 
> limit from 15 to 16 (there will be an extra msleep)...

15 or 16 is of lesser importance, the bigger irritant is that the last
sleep is no pointless. I'd move the condition check into the middle of
the loop:

for (i = 0;; i++) {
	if (test())
		return GOOD;

	if (i >= 15)
		return BAD;

	take_a_nap();
}

[PATCH net-next 2/2] i40e: Wait for pending VF reset in VF set callbacks

[Tony Nguyen]

From: Ivan Vecera <ivecera@redhat.com>

Commit 028daf80117376 ("i40e: Fix attach VF to VM issue") fixed
a race between i40e_ndo_set_vf_mac() and i40e_reset_vf() during
an attachment of VF device to VM. This issue is not related to
setting MAC address only but also VLAN assignment to particular
VF because the newer libvirt sets configured MAC address as well
as an optional VLAN. The same behavior is also for i40e's
.ndo_set_vf_rate and .ndo_set_vf_spoofchk where the callbacks
just check if the VF was initialized but not wait for the finish
of pending reset.

Reproducer:
[root@host ~]# virsh attach-interface guest hostdev --managed 0000:02:02.0 --mac 52:54:00:b4:aa:bb
error: Failed to attach interface
error: Cannot set interface MAC/vlanid to 52:54:00:b4:aa:bb/0 for ifname enp2s0f0 vf 0: Resource temporarily unavailable

Fix this issue by using i40e_check_vf_init_timeout() helper to check
whether a reset of particular VF was finished in i40e's
.ndo_set_vf_vlan, .ndo_set_vf_rate and .ndo_set_vf_spoofchk callbacks.

Tested-by: Ma Yuying <yuma@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c   | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index b84b6b675fa7..4741ba14ab27 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4466,13 +4466,11 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id,
 	}
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error_pvid;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	if (le16_to_cpu(vsi->info.pvid) == vlanprio)
 		/* duplicate request, so just return success */
@@ -4616,13 +4614,11 @@ int i40e_ndo_set_vf_bw(struct net_device *netdev, int vf_id, int min_tx_rate,
 	}
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
 	if (ret)
@@ -4789,9 +4785,7 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable)
 	}
 
 	vf = &(pf->vf[vf_id]);
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto out;
 	}
-- 
2.38.1

Re: [PATCH net-next 2/2] i40e: Wait for pending VF reset in VF set callbacks

[Leon Romanovsky]

On Mon, Jul 10, 2023 at 09:40:30AM -0700, Tony Nguyen wrote:
> From: Ivan Vecera <ivecera@redhat.com>
> 
> Commit 028daf80117376 ("i40e: Fix attach VF to VM issue") fixed
> a race between i40e_ndo_set_vf_mac() and i40e_reset_vf() during
> an attachment of VF device to VM. This issue is not related to
> setting MAC address only but also VLAN assignment to particular
> VF because the newer libvirt sets configured MAC address as well
> as an optional VLAN. The same behavior is also for i40e's
> .ndo_set_vf_rate and .ndo_set_vf_spoofchk where the callbacks
> just check if the VF was initialized but not wait for the finish
> of pending reset.
> 
> Reproducer:
> [root@host ~]# virsh attach-interface guest hostdev --managed 0000:02:02.0 --mac 52:54:00:b4:aa:bb
> error: Failed to attach interface
> error: Cannot set interface MAC/vlanid to 52:54:00:b4:aa:bb/0 for ifname enp2s0f0 vf 0: Resource temporarily unavailable
> 
> Fix this issue by using i40e_check_vf_init_timeout() helper to check
> whether a reset of particular VF was finished in i40e's
> .ndo_set_vf_vlan, .ndo_set_vf_rate and .ndo_set_vf_spoofchk callbacks.
> 
> Tested-by: Ma Yuying <yuma@redhat.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c   | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

[PATCH net-next 1/2] i40e: Add helper for VF inited state check with timeout

[Ivan Vecera]

Move the check for VF inited state (with optional up-to 300ms
timeout to separate helper i40e_check_vf_init_timeout() that
will be used in the following commit.

Tested-by: Ma Yuying <yuma@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index be59ba3774e1..b84b6b675fa7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4304,6 +4304,36 @@ static int i40e_validate_vf(struct i40e_pf *pf, int vf_id)
 	return ret;
 }
 
+/**
+ * i40e_check_vf_init_timeout
+ * @vf: the virtual function
+ *
+ * Check that the VF's initialization was successfully done and if not
+ * wait up to 300ms for its finish.
+ *
+ * Returns true when VF is initialized, false on timeout
+ **/
+static bool i40e_check_vf_init_timeout(struct i40e_vf *vf)
+{
+	int i;
+
+	/* When the VF is resetting wait until it is done.
+	 * It can take up to 200 milliseconds, but wait for
+	 * up to 300 milliseconds to be safe.
+	 */
+	for (i = 0; i < 15; i++) {
+		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
+			return true;
+
+		msleep(20);
+	}
+
+	dev_err(&vf->pf->pdev->dev, "VF %d still in reset. Try again.\n",
+		vf->vf_id);
+
+	return false;
+}
+
 /**
  * i40e_ndo_set_vf_mac
  * @netdev: network interface device structure
@@ -4322,7 +4352,6 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 	int ret = 0;
 	struct hlist_node *h;
 	int bkt;
-	u8 i;
 
 	if (test_and_set_bit(__I40E_VIRTCHNL_OP_PENDING, pf->state)) {
 		dev_warn(&pf->pdev->dev, "Unable to configure VFs, other operation is pending.\n");
@@ -4335,21 +4364,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		goto error_param;
 
 	vf = &pf->vf[vf_id];
-
-	/* When the VF is resetting wait until it is done.
-	 * It can take up to 200 milliseconds,
-	 * but wait for up to 300 milliseconds to be safe.
-	 * Acquire the VSI pointer only after the VF has been
-	 * properly initialized.
-	 */
-	for (i = 0; i < 15; i++) {
-		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
-			break;
-		msleep(20);
-	}
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error_param;
 	}
-- 
2.39.3

[PATCH net-next 2/2] i40e: Wait for pending VF reset in VF set callbacks

[Ivan Vecera]

Commit 028daf80117376 ("i40e: Fix attach VF to VM issue") fixed
a race between i40e_ndo_set_vf_mac() and i40e_reset_vf() during
an attachment of VF device to VM. This issue is not related to
setting MAC address only but also VLAN assignment to particular
VF because the newer libvirt sets configured MAC address as well
as an optional VLAN. The same behavior is also for i40e's
.ndo_set_vf_rate and .ndo_set_vf_spoofchk where the callbacks
just check if the VF was initialized but not wait for the finish
of pending reset.

Reproducer:
[root@host ~]# virsh attach-interface guest hostdev --managed 0000:02:02.0 --mac 52:54:00:b4:aa:bb
error: Failed to attach interface
error: Cannot set interface MAC/vlanid to 52:54:00:b4:aa:bb/0 for ifname enp2s0f0 vf 0: Resource temporarily unavailable

Fix this issue by using i40e_check_vf_init_timeout() helper to check
whether a reset of particular VF was finished in i40e's
.ndo_set_vf_vlan, .ndo_set_vf_rate and .ndo_set_vf_spoofchk callbacks.

Tested-by: Ma Yuying <yuma@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../net/ethernet/intel/i40e/i40e_virtchnl_pf.c   | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index b84b6b675fa7..4741ba14ab27 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4466,13 +4466,11 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id,
 	}
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error_pvid;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	if (le16_to_cpu(vsi->info.pvid) == vlanprio)
 		/* duplicate request, so just return success */
@@ -4616,13 +4614,11 @@ int i40e_ndo_set_vf_bw(struct net_device *netdev, int vf_id, int min_tx_rate,
 	}
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto error;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
 	if (ret)
@@ -4789,9 +4785,7 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable)
 	}
 
 	vf = &(pf->vf[vf_id]);
-	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
-		dev_err(&pf->pdev->dev, "VF %d still in reset. Try again.\n",
-			vf_id);
+	if (!i40e_check_vf_init_timeout(vf)) {
 		ret = -EAGAIN;
 		goto out;
 	}
-- 
2.39.3

Re: [PATCH net-next 2/2] i40e: Wait for pending VF reset in VF set callbacks

[Simon Horman]

On Tue, Jun 13, 2023 at 02:16:10PM +0200, Ivan Vecera wrote:
> Commit 028daf80117376 ("i40e: Fix attach VF to VM issue") fixed
> a race between i40e_ndo_set_vf_mac() and i40e_reset_vf() during
> an attachment of VF device to VM. This issue is not related to
> setting MAC address only but also VLAN assignment to particular
> VF because the newer libvirt sets configured MAC address as well
> as an optional VLAN. The same behavior is also for i40e's
> .ndo_set_vf_rate and .ndo_set_vf_spoofchk where the callbacks
> just check if the VF was initialized but not wait for the finish
> of pending reset.
> 
> Reproducer:
> [root@host ~]# virsh attach-interface guest hostdev --managed 0000:02:02.0 --mac 52:54:00:b4:aa:bb
> error: Failed to attach interface
> error: Cannot set interface MAC/vlanid to 52:54:00:b4:aa:bb/0 for ifname enp2s0f0 vf 0: Resource temporarily unavailable
> 
> Fix this issue by using i40e_check_vf_init_timeout() helper to check
> whether a reset of particular VF was finished in i40e's
> .ndo_set_vf_vlan, .ndo_set_vf_rate and .ndo_set_vf_spoofchk callbacks.
> 
> Tested-by: Ma Yuying <yuma@redhat.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>