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.samudr...@intel.com>
Reviewed-by: Marcin Szycik <marcin.szy...@linux.intel.com>
Signed-off-by: Ahmed Zaki <ahmed.z...@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
