Re: [PATCH iwl-next 3/8] ixgbe: prevent adding duplicate FDIR perfect filter rules

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 5/8/26 05:12, Aleksandr Loktionov wrote:
> From: Piotr Skajewski <piotrx.skajewski@intel.com>
> 
> When the same flow specification is added twice (same 5-tuple with
> different sw_idx values), ixgbe_add_ethtool_fdir_entry() silently
> programs the duplicate into hardware using a second FDIR table slot.
> This wastes a scarce FDIR entry and can cause confusing behaviour
> when deleting rules.
> 
> Add a helper ixgbe_match_ethtool_fdir_entry() that walks the in-kernel
> filter list before programming hardware.  If an entry with an
> identical filter (excluding the sw_idx) already exists, the new add
> request is rejected with -EEXIST.
> 
> Signed-off-by: Piotr Skajewski <piotrx.skajewski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 27 +++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index ba049b3..a2009df 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -2938,6 +2938,23 @@ static int ixgbe_flowspec_to_flow_type(struct ethtool_rx_flow_spec *fsp,
>   	return 1;
>   }
>   
> +static bool ixgbe_match_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
> +					   struct ixgbe_fdir_filter *input)
> +{
> +	struct ixgbe_fdir_filter *rule = NULL;
> +
> +	hlist_for_each_entry(rule, &adapter->fdir_filter_list, fdir_node) {
> +		if (rule->sw_idx == input->sw_idx)
> +			continue;

this if makes it possible to add rule with the same filter AND the same
sw_idx

> +		if (!memcmp(&rule->filter, &input->filter,
> +			    sizeof(rule->filter))) {
> +			e_warn(drv, "FDIR filter already exists\n");

return code of -EEXIST returned to the user is enough,
no need for a warning here

if you really want a dmesg entry, then make it info and of not alarming
wording

> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
>   static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
>   					struct ethtool_rxnfc *cmd)
>   {
> @@ -2947,7 +2964,7 @@ static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
>   	struct ixgbe_fdir_filter *input;
>   	union ixgbe_atr_input mask;
>   	u8 queue;
> -	int err;
> +	int err = -EINVAL;

RCT

>   
>   	if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE))
>   		return -EOPNOTSUPP;
> @@ -3050,6 +3067,12 @@ static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
>   	/* apply mask and compute/store hash */
>   	ixgbe_atr_compute_perfect_hash_82599(&input->filter, &mask);
>   
> +	/* check for a duplicate filter */
> +	if (ixgbe_match_ethtool_fdir_entry(adapter, input)) {
> +		err = -EEXIST;
> +		goto err_out_w_lock;
> +	}
> +
>   	/* program filters to filter memory */
>   	err = ixgbe_fdir_write_perfect_filter_82599(hw,
>   				&input->filter, input->sw_idx, queue);
> @@ -3065,7 +3088,7 @@ static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
>   	spin_unlock(&adapter->fdir_perfect_lock);
>   err_out:
>   	kfree(input);
> -	return -EINVAL;
> +	return err;
>   }
>   
>   static int ixgbe_del_ethtool_fdir_entry(struct ixgbe_adapter *adapter,