diff mbox series

[iwl-next,v3,12/13] iavf: refactor add/del FDIR filters

Message ID 20240710204015.124233-13-ahmed.zaki@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series ice: iavf: add support for TC U32 filters on VFs | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 816 this patch: 816
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 4 maintainers not CCed: edumazet@google.com przemyslaw.kitszel@intel.com pabeni@redhat.com kuba@kernel.org
netdev/build_clang success Errors and warnings before: 821 this patch: 821
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 825 this patch: 825
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 205 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 52 this patch: 51
netdev/source_inline success Was 0 now: 0

Commit Message

Ahmed Zaki July 10, 2024, 8:40 p.m. UTC 
In preparation for a second type of FDIR filters that can be added by
tc-u32, move the add/del of the FDIR logic to be entirely contained in
iavf_fdir.c.

The iavf_find_fdir_fltr_by_loc() is renamed to iavf_find_fdir_fltr()
to be more agnostic to the filter ID parameter (for now @loc, which is
relevant only to current FDIR filters added via ethtool).

Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h        |  5 ++
 .../net/ethernet/intel/iavf/iavf_ethtool.c    | 56 ++-------------
 drivers/net/ethernet/intel/iavf/iavf_fdir.c   | 72 +++++++++++++++++--
 drivers/net/ethernet/intel/iavf/iavf_fdir.h   |  7 +-
 4 files changed, 83 insertions(+), 57 deletions(-)

Comments

Simon Horman July 22, 2024, 3:04 p.m. UTC | #1 
On Wed, Jul 10, 2024 at 02:40:14PM -0600, Ahmed Zaki wrote:
> In preparation for a second type of FDIR filters that can be added by
> tc-u32, move the add/del of the FDIR logic to be entirely contained in
> iavf_fdir.c.
> 
> The iavf_find_fdir_fltr_by_loc() is renamed to iavf_find_fdir_fltr()
> to be more agnostic to the filter ID parameter (for now @loc, which is
> relevant only to current FDIR filters added via ethtool).
> 
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf.h        |  5 ++
>  .../net/ethernet/intel/iavf/iavf_ethtool.c    | 56 ++-------------
>  drivers/net/ethernet/intel/iavf/iavf_fdir.c   | 72 +++++++++++++++++--
>  drivers/net/ethernet/intel/iavf/iavf_fdir.h   |  7 +-
>  4 files changed, 83 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index 23a6557fc3db..85bd6a85cf2d 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -444,6 +444,11 @@ struct iavf_adapter {
>  	spinlock_t adv_rss_lock;	/* protect the RSS management list */
>  };
>  
> +/* Must be called with fdir_fltr_lock lock held */
> +static inline bool iavf_fdir_max_reached(struct iavf_adapter *adapter)
> +{
> +	return (adapter->fdir_active_fltr >= IAVF_MAX_FDIR_FILTERS);

nit: these parentheses seem unnecessary.

> +}
>  
>  /* Ethtool Private Flags */
>  

...

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_fdir.c b/drivers/net/ethernet/intel/iavf/iavf_fdir.c

...

> +/**
> + * iavf_fdir_del_fltr - delete a flow director filter from the list
> + * @adapter: pointer to the VF adapter structure
> + * @loc: location to delete.
> + *
> + * Return: 0 on success or negative errno on failure.
> + */
> +int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc)
> +{
> +	struct iavf_fdir_fltr *fltr = NULL;
> +	int err = 0;
> +
> +	spin_lock_bh(&adapter->fdir_fltr_lock);
> +	fltr = iavf_find_fdir_fltr(adapter, loc);
> +
> +	if (fltr) {
> +		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
> +			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
> +		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
> +			list_del(&fltr->list);
> +			kfree(fltr);
> +			adapter->fdir_active_fltr--;
> +			fltr = NULL;
> +		} else {
> +			err = -EBUSY;
> +		}
> +	} else if (adapter->fdir_active_fltr) {
> +		err = -EINVAL;
> +	}
> +
> +	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
> +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);

It seems that prior to this change the condition and call to
iavf_schedule_aq_request were not protected by fdir_fltr_lock, and now they
are. If so, is this change intentional.

> +
> +	spin_unlock_bh(&adapter->fdir_fltr_lock);
> +	return err;
>  }

...
Ahmed Zaki July 24, 2024, 4:14 p.m. UTC | #2 
On 2024-07-22 9:04 a.m., Simon Horman wrote:
> On Wed, Jul 10, 2024 at 02:40:14PM -0600, Ahmed Zaki wrote:
>> In preparation for a second type of FDIR filters that can be added by
>> tc-u32, move the add/del of the FDIR logic to be entirely contained in
>> iavf_fdir.c.
>>
>> The iavf_find_fdir_fltr_by_loc() is renamed to iavf_find_fdir_fltr()
>> to be more agnostic to the filter ID parameter (for now @loc, which is
>> relevant only to current FDIR filters added via ethtool).
>>
>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
>> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
>> ---
>>   drivers/net/ethernet/intel/iavf/iavf.h        |  5 ++
>>   .../net/ethernet/intel/iavf/iavf_ethtool.c    | 56 ++-------------
>>   drivers/net/ethernet/intel/iavf/iavf_fdir.c   | 72 +++++++++++++++++--
>>   drivers/net/ethernet/intel/iavf/iavf_fdir.h   |  7 +-
>>   4 files changed, 83 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
>> index 23a6557fc3db..85bd6a85cf2d 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf.h
>> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
>> @@ -444,6 +444,11 @@ struct iavf_adapter {
>>   	spinlock_t adv_rss_lock;	/* protect the RSS management list */
>>   };
>>   
>> +/* Must be called with fdir_fltr_lock lock held */
>> +static inline bool iavf_fdir_max_reached(struct iavf_adapter *adapter)
>> +{
>> +	return (adapter->fdir_active_fltr >= IAVF_MAX_FDIR_FILTERS);
> 
> nit: these parentheses seem unnecessary.
> 
>> +}
>>   
>>   /* Ethtool Private Flags */
>>   
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_fdir.c b/drivers/net/ethernet/intel/iavf/iavf_fdir.c
> 
> ...
> 
>> +/**
>> + * iavf_fdir_del_fltr - delete a flow director filter from the list
>> + * @adapter: pointer to the VF adapter structure
>> + * @loc: location to delete.
>> + *
>> + * Return: 0 on success or negative errno on failure.
>> + */
>> +int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc)
>> +{
>> +	struct iavf_fdir_fltr *fltr = NULL;
>> +	int err = 0;
>> +
>> +	spin_lock_bh(&adapter->fdir_fltr_lock);
>> +	fltr = iavf_find_fdir_fltr(adapter, loc);
>> +
>> +	if (fltr) {
>> +		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
>> +			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
>> +		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
>> +			list_del(&fltr->list);
>> +			kfree(fltr);
>> +			adapter->fdir_active_fltr--;
>> +			fltr = NULL;
>> +		} else {
>> +			err = -EBUSY;
>> +		}
>> +	} else if (adapter->fdir_active_fltr) {
>> +		err = -EINVAL;
>> +	}
>> +
>> +	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
>> +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);
> 
> It seems that prior to this change the condition and call to
> iavf_schedule_aq_request were not protected by fdir_fltr_lock, and now they
> are. If so, is this change intentional.
> 

yes it is, fltr is member of the list that should be protected by the 
spinlock.

All other nits and changes will be part of v4.

Thanks for the review.
Ahmed
Simon Horman July 24, 2024, 4:30 p.m. UTC | #3 
On Wed, Jul 24, 2024 at 10:14:19AM -0600, Ahmed Zaki wrote:

...

> > > +/**
> > > + * iavf_fdir_del_fltr - delete a flow director filter from the list
> > > + * @adapter: pointer to the VF adapter structure
> > > + * @loc: location to delete.
> > > + *
> > > + * Return: 0 on success or negative errno on failure.
> > > + */
> > > +int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc)
> > > +{
> > > +	struct iavf_fdir_fltr *fltr = NULL;
> > > +	int err = 0;
> > > +
> > > +	spin_lock_bh(&adapter->fdir_fltr_lock);
> > > +	fltr = iavf_find_fdir_fltr(adapter, loc);
> > > +
> > > +	if (fltr) {
> > > +		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
> > > +			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
> > > +		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
> > > +			list_del(&fltr->list);
> > > +			kfree(fltr);
> > > +			adapter->fdir_active_fltr--;
> > > +			fltr = NULL;
> > > +		} else {
> > > +			err = -EBUSY;
> > > +		}
> > > +	} else if (adapter->fdir_active_fltr) {
> > > +		err = -EINVAL;
> > > +	}
> > > +
> > > +	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
> > > +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);
> > 
> > It seems that prior to this change the condition and call to
> > iavf_schedule_aq_request were not protected by fdir_fltr_lock, and now they
> > are. If so, is this change intentional.
> > 
> 
> yes it is, fltr is member of the list that should be protected by the
> spinlock.

Thanks,

I would suggest moving this into a separate patch: changing locking is a
bit different to refactoring.

Or, if not, at least mentioning it in the patch description.
Ahmed Zaki July 24, 2024, 7:28 p.m. UTC | #4 
On 2024-07-24 10:30 a.m., Simon Horman wrote:
> On Wed, Jul 24, 2024 at 10:14:19AM -0600, Ahmed Zaki wrote:
> 
> ...
> 
>>>> +/**
>>>> + * iavf_fdir_del_fltr - delete a flow director filter from the list
>>>> + * @adapter: pointer to the VF adapter structure
>>>> + * @loc: location to delete.
>>>> + *
>>>> + * Return: 0 on success or negative errno on failure.
>>>> + */
>>>> +int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc)
>>>> +{
>>>> +	struct iavf_fdir_fltr *fltr = NULL;
>>>> +	int err = 0;
>>>> +
>>>> +	spin_lock_bh(&adapter->fdir_fltr_lock);
>>>> +	fltr = iavf_find_fdir_fltr(adapter, loc);
>>>> +
>>>> +	if (fltr) {
>>>> +		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
>>>> +			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
>>>> +		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
>>>> +			list_del(&fltr->list);
>>>> +			kfree(fltr);
>>>> +			adapter->fdir_active_fltr--;
>>>> +			fltr = NULL;
>>>> +		} else {
>>>> +			err = -EBUSY;
>>>> +		}
>>>> +	} else if (adapter->fdir_active_fltr) {
>>>> +		err = -EINVAL;
>>>> +	}
>>>> +
>>>> +	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
>>>> +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);
>>>
>>> It seems that prior to this change the condition and call to
>>> iavf_schedule_aq_request were not protected by fdir_fltr_lock, and now they
>>> are. If so, is this change intentional.
>>>
>>
>> yes it is, fltr is member of the list that should be protected by the
>> spinlock.
> 
> Thanks,
> 
> I would suggest moving this into a separate patch: changing locking is a
> bit different to refactoring.
> 
> Or, if not, at least mentioning it in the patch description.

I will mention it in the commit message. A separate patch is an overkill 
IMHO since the function location has changed and the patch would not be 
applied to previous versions.

Thanks,
Ahmed
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 23a6557fc3db..85bd6a85cf2d 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -444,6 +444,11 @@  struct iavf_adapter {
 	spinlock_t adv_rss_lock;	/* protect the RSS management list */
 };
 
+/* Must be called with fdir_fltr_lock lock held */
+static inline bool iavf_fdir_max_reached(struct iavf_adapter *adapter)
+{
+	return (adapter->fdir_active_fltr >= IAVF_MAX_FDIR_FILTERS);
+}
 
 /* Ethtool Private Flags */
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
index 52273f7eab2c..7ab445eeee18 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
@@ -927,7 +927,7 @@  iavf_get_ethtool_fdir_entry(struct iavf_adapter *adapter,
 
 	spin_lock_bh(&adapter->fdir_fltr_lock);
 
-	rule = iavf_find_fdir_fltr_by_loc(adapter, fsp->location);
+	rule = iavf_find_fdir_fltr(adapter, fsp->location);
 	if (!rule) {
 		ret = -EINVAL;
 		goto release_lock;
@@ -1263,15 +1263,7 @@  static int iavf_add_fdir_ethtool(struct iavf_adapter *adapter, struct ethtool_rx
 		return -EINVAL;
 
 	spin_lock_bh(&adapter->fdir_fltr_lock);
-	if (adapter->fdir_active_fltr >= IAVF_MAX_FDIR_FILTERS) {
-		spin_unlock_bh(&adapter->fdir_fltr_lock);
-		dev_err(&adapter->pdev->dev,
-			"Unable to add Flow Director filter because VF reached the limit of max allowed filters (%u)\n",
-			IAVF_MAX_FDIR_FILTERS);
-		return -ENOSPC;
-	}
-
-	if (iavf_find_fdir_fltr_by_loc(adapter, fsp->location)) {
+	if (iavf_find_fdir_fltr(adapter, fsp->location)) {
 		dev_err(&adapter->pdev->dev, "Failed to add Flow Director filter, it already exists\n");
 		spin_unlock_bh(&adapter->fdir_fltr_lock);
 		return -EEXIST;
@@ -1291,23 +1283,10 @@  static int iavf_add_fdir_ethtool(struct iavf_adapter *adapter, struct ethtool_rx
 	}
 
 	err = iavf_add_fdir_fltr_info(adapter, fsp, fltr);
-	if (err)
-		goto ret;
-
-	spin_lock_bh(&adapter->fdir_fltr_lock);
-	iavf_fdir_list_add_fltr(adapter, fltr);
-	adapter->fdir_active_fltr++;
-
-	if (adapter->link_up)
-		fltr->state = IAVF_FDIR_FLTR_ADD_REQUEST;
-	else
-		fltr->state = IAVF_FDIR_FLTR_INACTIVE;
-	spin_unlock_bh(&adapter->fdir_fltr_lock);
+	if (!err)
+		err = iavf_fdir_add_fltr(adapter, fltr);
 
-	if (adapter->link_up)
-		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_ADD_FDIR_FILTER);
-ret:
-	if (err && fltr)
+	if (err)
 		kfree(fltr);
 
 	mutex_unlock(&adapter->crit_lock);
@@ -1324,34 +1303,11 @@  static int iavf_add_fdir_ethtool(struct iavf_adapter *adapter, struct ethtool_rx
 static int iavf_del_fdir_ethtool(struct iavf_adapter *adapter, struct ethtool_rxnfc *cmd)
 {
 	struct ethtool_rx_flow_spec *fsp = (struct ethtool_rx_flow_spec *)&cmd->fs;
-	struct iavf_fdir_fltr *fltr = NULL;
-	int err = 0;
 
 	if (!(adapter->flags & IAVF_FLAG_FDIR_ENABLED))
 		return -EOPNOTSUPP;
 
-	spin_lock_bh(&adapter->fdir_fltr_lock);
-	fltr = iavf_find_fdir_fltr_by_loc(adapter, fsp->location);
-	if (fltr) {
-		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
-			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
-		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
-			list_del(&fltr->list);
-			kfree(fltr);
-			adapter->fdir_active_fltr--;
-			fltr = NULL;
-		} else {
-			err = -EBUSY;
-		}
-	} else if (adapter->fdir_active_fltr) {
-		err = -EINVAL;
-	}
-	spin_unlock_bh(&adapter->fdir_fltr_lock);
-
-	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
-		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);
-
-	return err;
+	return iavf_fdir_del_fltr(adapter, fsp->location);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/iavf/iavf_fdir.c b/drivers/net/ethernet/intel/iavf/iavf_fdir.c
index 2d47b0b4640e..1e1daf71dfa0 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_fdir.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_fdir.c
@@ -815,13 +815,14 @@  bool iavf_fdir_is_dup_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr *
 }
 
 /**
- * iavf_find_fdir_fltr_by_loc - find filter with location
+ * iavf_find_fdir_fltr - find FDIR filter
  * @adapter: pointer to the VF adapter structure
  * @loc: location to find.
  *
- * Returns pointer to Flow Director filter if found or null
+ * Returns: pointer to Flow Director filter if found or NULL. Lock must be held.
  */
-struct iavf_fdir_fltr *iavf_find_fdir_fltr_by_loc(struct iavf_adapter *adapter, u32 loc)
+struct iavf_fdir_fltr *iavf_find_fdir_fltr(struct iavf_adapter *adapter,
+					   u32 loc)
 {
 	struct iavf_fdir_fltr *rule;
 
@@ -833,14 +834,26 @@  struct iavf_fdir_fltr *iavf_find_fdir_fltr_by_loc(struct iavf_adapter *adapter,
 }
 
 /**
- * iavf_fdir_list_add_fltr - add a new node to the flow director filter list
+ * iavf_fdir_add_fltr - add a new node to the flow director filter list
  * @adapter: pointer to the VF adapter structure
  * @fltr: filter node to add to structure
+ *
+ * Return: 0 on success or negative errno on failure.
  */
-void iavf_fdir_list_add_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr *fltr)
+int iavf_fdir_add_fltr(struct iavf_adapter *adapter,
+		       struct iavf_fdir_fltr *fltr)
 {
 	struct iavf_fdir_fltr *rule, *parent = NULL;
 
+	spin_lock_bh(&adapter->fdir_fltr_lock);
+	if (iavf_fdir_max_reached(adapter)) {
+		spin_unlock_bh(&adapter->fdir_fltr_lock);
+		dev_err(&adapter->pdev->dev,
+			"Unable to add Flow Director filter (limit (%u) reached)\n",
+			IAVF_MAX_FDIR_FILTERS);
+		return -ENOSPC;
+	}
+
 	list_for_each_entry(rule, &adapter->fdir_list_head, list) {
 		if (rule->loc >= fltr->loc)
 			break;
@@ -851,4 +864,53 @@  void iavf_fdir_list_add_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr
 		list_add(&fltr->list, &parent->list);
 	else
 		list_add(&fltr->list, &adapter->fdir_list_head);
+	adapter->fdir_active_fltr++;
+
+	if (adapter->link_up)
+		fltr->state = IAVF_FDIR_FLTR_ADD_REQUEST;
+	else
+		fltr->state = IAVF_FDIR_FLTR_INACTIVE;
+	spin_unlock_bh(&adapter->fdir_fltr_lock);
+
+	if (adapter->link_up)
+		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_ADD_FDIR_FILTER);
+
+	return 0;
+}
+
+/**
+ * iavf_fdir_del_fltr - delete a flow director filter from the list
+ * @adapter: pointer to the VF adapter structure
+ * @loc: location to delete.
+ *
+ * Return: 0 on success or negative errno on failure.
+ */
+int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc)
+{
+	struct iavf_fdir_fltr *fltr = NULL;
+	int err = 0;
+
+	spin_lock_bh(&adapter->fdir_fltr_lock);
+	fltr = iavf_find_fdir_fltr(adapter, loc);
+
+	if (fltr) {
+		if (fltr->state == IAVF_FDIR_FLTR_ACTIVE) {
+			fltr->state = IAVF_FDIR_FLTR_DEL_REQUEST;
+		} else if (fltr->state == IAVF_FDIR_FLTR_INACTIVE) {
+			list_del(&fltr->list);
+			kfree(fltr);
+			adapter->fdir_active_fltr--;
+			fltr = NULL;
+		} else {
+			err = -EBUSY;
+		}
+	} else if (adapter->fdir_active_fltr) {
+		err = -EINVAL;
+	}
+
+	if (fltr && fltr->state == IAVF_FDIR_FLTR_DEL_REQUEST)
+		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_FDIR_FILTER);
+
+	spin_unlock_bh(&adapter->fdir_fltr_lock);
+	return err;
 }
diff --git a/drivers/net/ethernet/intel/iavf/iavf_fdir.h b/drivers/net/ethernet/intel/iavf/iavf_fdir.h
index d31bd923ba8c..5c85eb25fa2a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_fdir.h
+++ b/drivers/net/ethernet/intel/iavf/iavf_fdir.h
@@ -128,6 +128,9 @@  int iavf_validate_fdir_fltr_masks(struct iavf_adapter *adapter,
 int iavf_fill_fdir_add_msg(struct iavf_adapter *adapter, struct iavf_fdir_fltr *fltr);
 void iavf_print_fdir_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr *fltr);
 bool iavf_fdir_is_dup_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr *fltr);
-void iavf_fdir_list_add_fltr(struct iavf_adapter *adapter, struct iavf_fdir_fltr *fltr);
-struct iavf_fdir_fltr *iavf_find_fdir_fltr_by_loc(struct iavf_adapter *adapter, u32 loc);
+int iavf_fdir_add_fltr(struct iavf_adapter *adapter,
+		       struct iavf_fdir_fltr *fltr);
+int iavf_fdir_del_fltr(struct iavf_adapter *adapter, u32 loc);
+struct iavf_fdir_fltr *iavf_find_fdir_fltr(struct iavf_adapter *adapter,
+					   u32 loc);
 #endif /* _IAVF_FDIR_H_ */