diff mbox series

[net,v1] i40e: Fix of warning message during remove iavf driver and fix of call stack during rmmod i40e driver

Message ID 20210416092251.272749-1-karen.sornek@intel.com
State Changes Requested
Headers show
Series [net,v1] i40e: Fix of warning message during remove iavf driver and fix of call stack during rmmod i40e driver | expand

Commit Message

Karen Sornek April 16, 2021, 9:22 a.m. UTC 
Restored part of reset functionality used when reset is called
from the VF to reset itself. Without this fix warning message
is displayed when vf is removing via sysfs.

Removes the reason of the vf crashing during reset by ensuring
that the reset message request to pf is performed.
Refactoring code to use one function instead of two.
Without this patch, during stress tests, there was a possibility of
non-execution of reset request correctly which is the cause of vf
reinitialization errors and consequently in rmmod driver suspended
the system.

Fixes: eeeddbb80640("i40e: drop i40e_pf *pf from i40e_vc_disable_vf()")
Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
Signed-off-by: Karen Sornek <karen.sornek@intel.com>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 53 ++++++++-----------
 1 file changed, 22 insertions(+), 31 deletions(-)

Comments

Tony Nguyen April 20, 2021, 4:52 p.m. UTC | #1 
On Fri, 2021-04-16 at 11:22 +0200, Karen Sornek wrote:

The title is a bit long, can reduce it a bit?

> Restored part of reset functionality used when reset is called
> from the VF to reset itself. Without this fix warning message
> is displayed when vf is removing via sysfs.
> 
> Removes the reason of the vf crashing during reset by ensuring
> that the reset message request to pf is performed.
> Refactoring code to use one function instead of two.
> Without this patch, during stress tests, there was a possibility of
> non-execution of reset request correctly which is the cause of vf
> reinitialization errors and consequently in rmmod driver suspended
> the system.
> 
> Fixes: eeeddbb80640("i40e: drop i40e_pf *pf from
> i40e_vc_disable_vf()")

You're missing a space.
Fixes tag: Fixes: eeeddbb80640("i40e: drop i40e_pf *pf from
i40e_vc_disable_vf()")
	Has these problem(s):
		- missing space between the SHA1 and the subject

> Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
> Signed-off-by: Karen Sornek <karen.sornek@intel.com>
> ---
>  .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 53 ++++++++---------
> --
>  1 file changed, 22 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index f6121a0c4..ee2c5aba4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -183,17 +183,20 @@ void i40e_vc_notify_vf_reset(struct i40e_vf
> *vf)
>  /***********************misc routines*****************************/
>  
>  /**
> - * i40e_vc_disable_vf
> + * i40e_vc_reset_vf
>   * @vf: pointer to the VF info
>   *
> - * Disable the VF through a SW reset.
> + * @notify_vf: notify vf about reset or not
> + *
> + * Reset VF handler.
>   **/
> -static inline void i40e_vc_disable_vf(struct i40e_vf *vf)
> +static inline void i40e_vc_reset_vf(struct i40e_vf *vf, bool
> notify_vf)

Please remove the inline. In c files, let the compiler decide what to
do.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index f6121a0c4..ee2c5aba4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -183,17 +183,20 @@  void i40e_vc_notify_vf_reset(struct i40e_vf *vf)
 /***********************misc routines*****************************/
 
 /**
- * i40e_vc_disable_vf
+ * i40e_vc_reset_vf
  * @vf: pointer to the VF info
  *
- * Disable the VF through a SW reset.
+ * @notify_vf: notify vf about reset or not
+ *
+ * Reset VF handler.
  **/
-static inline void i40e_vc_disable_vf(struct i40e_vf *vf)
+static inline void i40e_vc_reset_vf(struct i40e_vf *vf, bool notify_vf)
 {
 	struct i40e_pf *pf = vf->pf;
 	int i;
 
-	i40e_vc_notify_vf_reset(vf);
+	if (notify_vf)
+		i40e_vc_notify_vf_reset(vf);
 
 	/* We want to ensure that an actual reset occurs initiated after this
 	 * function was called. However, we do not want to wait forever, so
@@ -211,9 +214,14 @@  static inline void i40e_vc_disable_vf(struct i40e_vf *vf)
 		usleep_range(10000, 20000);
 	}
 
-	dev_warn(&vf->pf->pdev->dev,
-		 "Failed to initiate reset for VF %d after 200 milliseconds\n",
-		 vf->vf_id);
+	if (notify_vf)
+		dev_warn(&vf->pf->pdev->dev,
+			 "Failed to initiate reset for VF %d after 200 milliseconds\n",
+			 vf->vf_id);
+	else
+		dev_dbg(&vf->pf->pdev->dev,
+			"Failed to initiate reset for VF %d after 200 milliseconds\n",
+			vf->vf_id);
 }
 
 /**
@@ -2126,20 +2134,6 @@  err:
 	return ret;
 }
 
-/**
- * i40e_vc_reset_vf_msg
- * @vf: pointer to the VF info
- *
- * called from the VF to reset itself,
- * unlike other virtchnl messages, PF driver
- * doesn't send the response back to the VF
- **/
-static void i40e_vc_reset_vf_msg(struct i40e_vf *vf)
-{
-	if (test_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states))
-		i40e_reset_vf(vf, false);
-}
-
 /**
  * i40e_vc_config_promiscuous_mode_msg
  * @vf: pointer to the VF info
@@ -2628,8 +2622,7 @@  static int i40e_vc_request_queues_msg(struct i40e_vf *vf, u8 *msg)
 	} else {
 		/* successful request */
 		vf->num_req_queues = req_pairs;
-		i40e_vc_notify_vf_reset(vf);
-		i40e_reset_vf(vf, false);
+		i40e_vc_reset_vf(vf, true);
 		return 0;
 	}
 
@@ -3832,8 +3825,7 @@  static int i40e_vc_add_qch_msg(struct i40e_vf *vf, u8 *msg)
 	vf->num_req_queues = 0;
 
 	/* reset the VF in order to allocate resources */
-	i40e_vc_notify_vf_reset(vf);
-	i40e_reset_vf(vf, false);
+	i40e_vc_reset_vf(vf, true);
 
 	return I40E_SUCCESS;
 
@@ -3873,8 +3865,7 @@  static int i40e_vc_del_qch_msg(struct i40e_vf *vf, u8 *msg)
 	}
 
 	/* reset the VF in order to allocate resources */
-	i40e_vc_notify_vf_reset(vf);
-	i40e_reset_vf(vf, false);
+	i40e_vc_reset_vf(vf, true);
 
 	return I40E_SUCCESS;
 
@@ -3936,7 +3927,7 @@  int i40e_vc_process_vf_msg(struct i40e_pf *pf, s16 vf_id, u32 v_opcode,
 		i40e_vc_notify_vf_link_state(vf);
 		break;
 	case VIRTCHNL_OP_RESET_VF:
-		i40e_vc_reset_vf_msg(vf);
+		i40e_vc_reset_vf(vf, false);
 		ret = 0;
 		break;
 	case VIRTCHNL_OP_CONFIG_PROMISCUOUS_MODE:
@@ -4190,7 +4181,7 @@  int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 	/* Force the VF interface down so it has to bring up with new MAC
 	 * address
 	 */
-	i40e_vc_disable_vf(vf);
+	i40e_vc_reset_vf(vf, true);
 	dev_info(&pf->pdev->dev, "Bring down and up the VF interface to make this change effective.\n");
 
 error_param:
@@ -4254,7 +4245,7 @@  int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id,
 		/* duplicate request, so just return success */
 		goto error_pvid;
 
-	i40e_vc_disable_vf(vf);
+	i40e_vc_reset_vf(vf, true);
 	/* During reset the VF got a new VSI, so refresh a pointer. */
 	vsi = pf->vsi[vf->lan_vsi_idx];
 	/* Locked once because multiple functions below iterate list */
@@ -4632,7 +4623,7 @@  int i40e_ndo_set_vf_trust(struct net_device *netdev, int vf_id, bool setting)
 		goto out;
 
 	vf->trusted = setting;
-	i40e_vc_disable_vf(vf);
+	i40e_vc_reset_vf(vf, true);
 	dev_info(&pf->pdev->dev, "VF %u is now %strusted\n",
 		 vf_id, setting ? "" : "un");