All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i40e: acquire VSI pointer only after VF is initialized
@ 2020-11-30 13:12 ` Stefan Assmann
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Assmann @ 2020-11-30 13:12 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: netdev, davem, anthony.l.nguyen, lihong.yang, sassmann

This change simplifies the VF initialization check and also minimizes
the delay between acquiring the VSI pointer and using it. As known by
the commit being fixed, there is a risk of the VSI pointer getting
changed. Therefore minimize the delay between getting and using the
pointer.

Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC addresses")
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 729c4f0d5ac5..bf6034c3a6ea 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		goto error_param;
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	/* 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.
-	 * If the VF is indeed in reset, the vsi pointer has
-	 * to show on the newly loaded vsi under pf->vsi[id].
+	 * 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)) {
-			if (i > 0)
-				vsi = pf->vsi[vf->lan_vsi_idx];
+		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
 			break;
-		}
 		msleep(20);
 	}
 	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
@@ -4068,6 +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		ret = -EAGAIN;
 		goto error_param;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	if (is_multicast_ether_addr(mac)) {
 		dev_err(&pf->pdev->dev,
-- 
2.28.0


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

* [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is initialized
@ 2020-11-30 13:12 ` Stefan Assmann
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Assmann @ 2020-11-30 13:12 UTC (permalink / raw)
  To: intel-wired-lan

This change simplifies the VF initialization check and also minimizes
the delay between acquiring the VSI pointer and using it. As known by
the commit being fixed, there is a risk of the VSI pointer getting
changed. Therefore minimize the delay between getting and using the
pointer.

Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC addresses")
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 729c4f0d5ac5..bf6034c3a6ea 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		goto error_param;
 
 	vf = &pf->vf[vf_id];
-	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	/* 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.
-	 * If the VF is indeed in reset, the vsi pointer has
-	 * to show on the newly loaded vsi under pf->vsi[id].
+	 * 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)) {
-			if (i > 0)
-				vsi = pf->vsi[vf->lan_vsi_idx];
+		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
 			break;
-		}
 		msleep(20);
 	}
 	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
@@ -4068,6 +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		ret = -EAGAIN;
 		goto error_param;
 	}
+	vsi = pf->vsi[vf->lan_vsi_idx];
 
 	if (is_multicast_ether_addr(mac)) {
 		dev_err(&pf->pdev->dev,
-- 
2.28.0


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

* Re: [PATCH] i40e: acquire VSI pointer only after VF is initialized
  2020-11-30 13:12 ` [Intel-wired-lan] " Stefan Assmann
@ 2020-11-30 19:31   ` Jacob Keller
  -1 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2020-11-30 19:31 UTC (permalink / raw)
  To: Stefan Assmann, intel-wired-lan
  Cc: netdev, davem, anthony.l.nguyen, lihong.yang



On 11/30/2020 5:12 AM, Stefan Assmann wrote:
> This change simplifies the VF initialization check and also minimizes
> the delay between acquiring the VSI pointer and using it. As known by
> the commit being fixed, there is a risk of the VSI pointer getting
> changed. Therefore minimize the delay between getting and using the
> pointer.
> 
> Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC addresses")
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>

Ok.

> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 729c4f0d5ac5..bf6034c3a6ea 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  		goto error_param;
>  
>  	vf = &pf->vf[vf_id];
> -	vsi = pf->vsi[vf->lan_vsi_idx];
>  
>  	/* 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.
> -	 * If the VF is indeed in reset, the vsi pointer has
> -	 * to show on the newly loaded vsi under pf->vsi[id].
> +	 * 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)) {
> -			if (i > 0)
> -				vsi = pf->vsi[vf->lan_vsi_idx];
> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>  			break;
> -		}
>  		msleep(20);
>  	}
>  	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
> @@ -4068,6 +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  		ret = -EAGAIN;
>  		goto error_param;
>  	}
> +	vsi = pf->vsi[vf->lan_vsi_idx];
>  

Yea, this makes more sense to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  	if (is_multicast_ether_addr(mac)) {
>  		dev_err(&pf->pdev->dev,
> 

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

* [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is initialized
@ 2020-11-30 19:31   ` Jacob Keller
  0 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2020-11-30 19:31 UTC (permalink / raw)
  To: intel-wired-lan



On 11/30/2020 5:12 AM, Stefan Assmann wrote:
> This change simplifies the VF initialization check and also minimizes
> the delay between acquiring the VSI pointer and using it. As known by
> the commit being fixed, there is a risk of the VSI pointer getting
> changed. Therefore minimize the delay between getting and using the
> pointer.
> 
> Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC addresses")
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>

Ok.

> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 729c4f0d5ac5..bf6034c3a6ea 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  		goto error_param;
>  
>  	vf = &pf->vf[vf_id];
> -	vsi = pf->vsi[vf->lan_vsi_idx];
>  
>  	/* 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.
> -	 * If the VF is indeed in reset, the vsi pointer has
> -	 * to show on the newly loaded vsi under pf->vsi[id].
> +	 * 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)) {
> -			if (i > 0)
> -				vsi = pf->vsi[vf->lan_vsi_idx];
> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>  			break;
> -		}
>  		msleep(20);
>  	}
>  	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
> @@ -4068,6 +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
>  		ret = -EAGAIN;
>  		goto error_param;
>  	}
> +	vsi = pf->vsi[vf->lan_vsi_idx];
>  

Yea, this makes more sense to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  	if (is_multicast_ether_addr(mac)) {
>  		dev_err(&pf->pdev->dev,
> 

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

* RE: [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is initialized
  2020-11-30 13:12 ` [Intel-wired-lan] " Stefan Assmann
@ 2021-01-12 17:37   ` Jankowski, Konrad0
  -1 siblings, 0 replies; 6+ messages in thread
From: Jankowski, Konrad0 @ 2021-01-12 17:37 UTC (permalink / raw)
  To: Stefan Assmann, intel-wired-lan; +Cc: netdev, davem



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Stefan Assmann
> Sent: poniedziałek, 30 listopada 2020 14:13
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> Subject: [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is
> initialized
> 
> This change simplifies the VF initialization check and also minimizes the delay
> between acquiring the VSI pointer and using it. As known by the commit
> being fixed, there is a risk of the VSI pointer getting changed. Therefore
> minimize the delay between getting and using the pointer.
> 
> Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC
> addresses")
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 729c4f0d5ac5..bf6034c3a6ea 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device
> *netdev, int vf_id, u8 *mac)
>  		goto error_param;
> 
>  	vf = &pf->vf[vf_id];
> -	vsi = pf->vsi[vf->lan_vsi_idx];
> 
>  	/* 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.
> -	 * If the VF is indeed in reset, the vsi pointer has
> -	 * to show on the newly loaded vsi under pf->vsi[id].
> +	 * 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)) {
> -			if (i > 0)
> -				vsi = pf->vsi[vf->lan_vsi_idx];
> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>  			break;
> -		}
>  		msleep(20);
>  	}
>  	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) { @@ -4068,6
> +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id,
> u8 *mac)
>  		ret = -EAGAIN;
>  		goto error_param;
>  	}
> +	vsi = pf->vsi[vf->lan_vsi_idx];
> 
>  	if (is_multicast_ether_addr(mac)) {
>  		dev_err(&pf->pdev->dev,

Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
 


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

* [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is initialized
@ 2021-01-12 17:37   ` Jankowski, Konrad0
  0 siblings, 0 replies; 6+ messages in thread
From: Jankowski, Konrad0 @ 2021-01-12 17:37 UTC (permalink / raw)
  To: intel-wired-lan



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Stefan Assmann
> Sent: poniedzia?ek, 30 listopada 2020 14:13
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH] i40e: acquire VSI pointer only after VF is
> initialized
> 
> This change simplifies the VF initialization check and also minimizes the delay
> between acquiring the VSI pointer and using it. As known by the commit
> being fixed, there is a risk of the VSI pointer getting changed. Therefore
> minimize the delay between getting and using the pointer.
> 
> Fixes: 9889707b06ac ("i40e: Fix crash caused by stress setting of VF MAC
> addresses")
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 729c4f0d5ac5..bf6034c3a6ea 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -4046,20 +4046,16 @@ int i40e_ndo_set_vf_mac(struct net_device
> *netdev, int vf_id, u8 *mac)
>  		goto error_param;
> 
>  	vf = &pf->vf[vf_id];
> -	vsi = pf->vsi[vf->lan_vsi_idx];
> 
>  	/* 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.
> -	 * If the VF is indeed in reset, the vsi pointer has
> -	 * to show on the newly loaded vsi under pf->vsi[id].
> +	 * 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)) {
> -			if (i > 0)
> -				vsi = pf->vsi[vf->lan_vsi_idx];
> +		if (test_bit(I40E_VF_STATE_INIT, &vf->vf_states))
>  			break;
> -		}
>  		msleep(20);
>  	}
>  	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) { @@ -4068,6
> +4064,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id,
> u8 *mac)
>  		ret = -EAGAIN;
>  		goto error_param;
>  	}
> +	vsi = pf->vsi[vf->lan_vsi_idx];
> 
>  	if (is_multicast_ether_addr(mac)) {
>  		dev_err(&pf->pdev->dev,

Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
 


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

end of thread, other threads:[~2021-01-12 17:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30 13:12 [PATCH] i40e: acquire VSI pointer only after VF is initialized Stefan Assmann
2020-11-30 13:12 ` [Intel-wired-lan] " Stefan Assmann
2020-11-30 19:31 ` Jacob Keller
2020-11-30 19:31   ` [Intel-wired-lan] " Jacob Keller
2021-01-12 17:37 ` Jankowski, Konrad0
2021-01-12 17:37   ` Jankowski, Konrad0

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.