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.
> +
> + spin_unlock_bh(&adapter->fdir_fltr_lock);
> + return err;
> }
...
