Re: [PATCH iwl-next v4 2/6] ice: do not add LLDP-specific filter if not necessary

[Simon Horman]

On Fri, Feb 14, 2025 at 09:50:36AM +0100, Larysa Zaremba wrote:
> Commit 34295a3696fb ("ice: implement new LLDP filter command")
> introduced the ability to use LLDP-specific filter that directs all
> LLDP traffic to a single VSI. However, current goal is for all trusted VFs
> to be able to see LLDP neighbors, which is impossible to do with the
> special filter.
> 
> Make using the generic filter the default choice and fall back to special
> one only if a generic filter cannot be added. That way setups with "NVMs
> where an already existent LLDP filter is blocking the creation of a filter
> to allow LLDP packets" will still be able to configure software Rx LLDP on
> PF only, while all other setups would be able to forward them to VFs too.
> 
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@xxxxxxxxxxxxxxx>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@xxxxxxxxx>

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>

...

> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index aaa592ffd2d8..f2e51bacecf8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -6010,15 +6010,21 @@ bool ice_fw_supports_lldp_fltr_ctrl(struct ice_hw *hw)
>  /**
>   * ice_lldp_fltr_add_remove - add or remove a LLDP Rx switch filter
>   * @hw: pointer to HW struct
> - * @vsi_num: absolute HW index for VSI
> + * @vsi: VSI to add the filter to
>   * @add: boolean for if adding or removing a filter
> + *
> + * Return: 0 on success, -EOPNOTSUPP if the operation cannot be performed
> + *	   with this HW or VSI, otherwise an error corresponding to
> + *	   the AQ transaction result.
>   */

Thanks for adding the Return section to the kernel doc.

> -int
> -ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add)
> +int ice_lldp_fltr_add_remove(struct ice_hw *hw, struct ice_vsi *vsi, bool add)
>  {
>  	struct ice_aqc_lldp_filter_ctrl *cmd;
>  	struct ice_aq_desc desc;
>  
> +	if (vsi->type != ICE_VSI_PF || !ice_fw_supports_lldp_fltr_ctrl(hw))
> +		return -EOPNOTSUPP;
> +
>  	cmd = &desc.params.lldp_filter_ctrl;
>  
>  	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_lldp_filter_ctrl);

...