Re: [Intel-wired-lan] [PATCH net-next 04/13] ice: add helper function for checking VSI VF requirement

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Fri, Jan 13, 2023 at 02:37:26PM -0800, Jacob Keller wrote:
> A few places in ice_lib.c WARN if the VSI type is VF and the VF pointer is
> NULL. This helps protect against accidentally creating a ICE_VSI_VF without
> providing a VF pointer.
> 
> A future change is going to introduce another type of VSI which has the
> same requirement, ICE_VSI_ADI. Instead of expanding each WARN_ON check to
> include both types, introduce a helper function to do this check. The
> ice_vsi_requires_vf function returns true if the VSI *must* have a VF, and
> returns false otherwise.
> 
> Of specific note is that some VSI types may optionally have a VF but do not
> require them, such as the ICE_VSI_CTRL type. These return false.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 26 +++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index f89279ede9a1..79555e22a9be 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -34,6 +34,26 @@ const char *ice_vsi_type_str(enum ice_vsi_type vsi_type)
>  	}
>  }
>  
> +/**
> + * ice_vsi_requires_vf - Does this VSI type always require a VF?
> + * @vsi_type: the VSI type
> + *
> + * Returns true if the VSI type *must* have a VF pointer. Returns false
> + * otherwise. In particular, VSI types which may *optionally* have a VF
> + * pointer return false.
> + *
> + * Used to WARN in cases where we always expect a VF pointer to be non-NULL.
> + */
> +static int ice_vsi_requires_vf(enum ice_vsi_type vsi_type)
> +{
> +	switch (vsi_type) {
> +	case ICE_VSI_VF:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
Looks a little strange right now. Maybe send this whole patch with SIOV
series?

> +
>  /**
>   * ice_vsi_ctrl_all_rx_rings - Start or stop a VSI's Rx rings
>   * @vsi: the VSI being configured
> @@ -175,7 +195,7 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi)
>  	struct ice_pf *pf = vsi->back;
>  	struct ice_vf *vf = vsi->vf;
>  
> -	if (WARN_ON(vsi_type == ICE_VSI_VF && !vf))
> +	if (WARN_ON(ice_vsi_requires_vf(vsi_type) && !vf))
>  		return;
>  
>  	switch (vsi_type) {
> @@ -2854,7 +2874,7 @@ int ice_vsi_cfg(struct ice_vsi *vsi, enum ice_vsi_type vsi_type,
>  	struct ice_pf *pf = vsi->back;
>  	int ret;
>  
> -	if (WARN_ON(vsi_type == ICE_VSI_VF && !vf))
> +	if (WARN_ON(ice_vsi_requires_vf(vsi_type) && !vf))
>  		return -EINVAL;
>  
>  	vsi->type = vsi_type;
> @@ -3487,7 +3507,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi)
>  		return -EINVAL;
>  
>  	pf = vsi->back;
> -	if (WARN_ON(vsi->type == ICE_VSI_VF && !vsi->vf))
> +	if (WARN_ON(ice_vsi_requires_vf(vsi->type) && !vsi->vf))
>  		return -EINVAL;
>  
>  	coalesce = kcalloc(vsi->num_q_vectors,
> -- 
> 2.38.1.420.g319605f8f00e
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan