Re: [Intel-wired-lan] [PATCH net-next v1 1/2] ice: Introduce enabling promiscuous mode on multiple VF's

["Wilczynski, Michal" <michal.wilczynski@intel.com>]

Hi,

Thanks for your review,

Applied fixes for your comments in v2.


On 6/29/2022 12:06 AM, Tony Nguyen wrote:
>
>
> On 6/24/2022 3:16 AM, Michal Wilczynski wrote:
>
> ...
>
>> @@ -3695,79 +3696,63 @@ bool ice_is_vsi_dflt_vsi(struct ice_sw *sw, 
>> struct ice_vsi *vsi)
>>    *  Otherwise try to set the VSI passed in as the switch's default 
>> VSI and
>>    *  return the result.
>>    */
>> -int ice_set_dflt_vsi(struct ice_sw *sw, struct ice_vsi *vsi)
>> +int ice_set_dflt_vsi(struct ice_vsi *vsi)
>
> drivers/net/ethernet/intel/ice/ice_lib.c:3733: warning: Excess 
> function parameter 'sw' description in 'ice_set_dflt_vsi'
>
>
>>   {
>>       struct device *dev;
>>       int status;
>
> ...
>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c 
>> b/drivers/net/ethernet/intel/ice/ice_switch.c
>> index 8d8f3eec79ee..f6cd73e9d451 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
>> @@ -1298,6 +1298,9 @@ static 
>> DECLARE_BITMAP(recipe_to_profile[ICE_MAX_NUM_RECIPES],
>>   static DECLARE_BITMAP(profile_to_recipe[ICE_MAX_NUM_PROFILES],
>>                 ICE_MAX_NUM_RECIPES);
>>   +static bool
>> +ice_vsi_uses_fltr(struct ice_fltr_mgmt_list_entry *fm_entry, u16 
>> vsi_handle);
>> +
>
> It looks like the function can be moved up to avoid the forward 
> declaration.
>
>>   /**
>>    * ice_init_def_sw_recp - initialize the recipe book keeping tables
>>    * @hw: pointer to the HW struct
>
> ...
>
>> @@ -3882,54 +3878,66 @@ int ice_cfg_dflt_vsi(struct ice_hw *hw, u16 
>> vsi_handle, bool set, u8 direction)
>>       f_info.flag = direction;
>>       f_info.fltr_act = ICE_FWD_TO_VSI;
>>       f_info.fwd_id.hw_vsi_id = hw_vsi_id;
>> +    f_info.vsi_handle = vsi_handle;
>>         if (f_info.flag & ICE_FLTR_RX) {
>>           f_info.src = hw->port_info->lport;
>>           f_info.src_id = ICE_SRC_ID_LPORT;
>> -        if (!set)
>> -            f_info.fltr_rule_id =
>> -                hw->port_info->dflt_rx_vsi_rule_id;
>> +
>
> Odd place for newline
>
>>       } else if (f_info.flag & ICE_FLTR_TX) {
>>           f_info.src_id = ICE_SRC_ID_VSI;
>>           f_info.src = hw_vsi_id;
>> -        if (!set)
>> -            f_info.fltr_rule_id =
>> -                hw->port_info->dflt_tx_vsi_rule_id;
>
> ...
>
>> +/**
>> + * ice_check_if_dflt_vsi - check if VSI is default VSI
>> + * @pi: pointer to the port_info structure
>> + * @vsi_handle: vsi handle to check for in filter list
>> + * @rule_exists: indicates if there are any VSI's in the rule list
>> + *
>> + * checks if the VSI is in a default VSI list, and also indicates
>> + * if the default VSI list is empty
>> + */
>> +bool ice_check_if_dflt_vsi(struct ice_port_info *pi, u16 vsi_handle,
>> +               bool *rule_exists)
>
> ice driver uses GNU style for multiline function declarations (same 
> comment for patch 2 as well)
> e.g.
>
> bool
> ice_check_if_dflt_vsi(struct ice_port_info *pi...
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan