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

[Jacob Keller <jacob.e.keller@intel.com>]

On 1/16/2023 2:56 AM, Michal Swiatkowski wrote:
> 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?
> 

I can do that, but the main challenge is the number of patches. We're
going to have to send the ice driver implementation as well as the
inet_vdcm driver implementation in the VFIO subfolder. This could easily
hit 15 patches, so I've been trying to find as many places where we can
split patches to other series as possible.

It is definitely odd now, and I could refactor this part to not be a
switch/case and make it switch/case later with the changes that
introduce scalable? But I don't really want the changes that switch to
using ice_vsi_requires_vf to be part of that patch...
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan