Re: [PATCH iwl-net v3 2/2] iavf: return 0 when TC flower filter not found after qdisc teardown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iavf: race condition in clsflower teardown logic
--

commit 3c89e574f59d7231645a9c5af1ee68174ff89b80
Author: Kiran Patil <kiran.patil@intel.com>

iavf: return 0 when TC flower filter not found after qdisc teardown

When an egress qdisc is destroyed, the driver proactively deletes all
associated cloud filters, decrementing num_cloud_filters to zero. This commit
returns 0 instead of -EINVAL when the netdev layer subsequently tries to delete
a filter that isn't found, provided num_cloud_filters is zero, to avoid
confusing upper layers.

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 2890e3c8005ee..c502e0a5391f3 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -4163,7 +4163,8 @@ static int iavf_delete_clsflower(struct iavf_adapter *adapter,
>  	if (filter) {
>  		filter->del = true;
>  		adapter->aq_required |= IAVF_FLAG_AQ_DEL_CLOUD_FILTER;
> -	} else {
> +	} else if (adapter->num_cloud_filters) {
> +		/* other filters still tracked, so this one is a genuine miss */
>  		err = -EINVAL;
>  	}

[Severity: Low]
Can this logic incorrectly assume that a non-zero adapter->num_cloud_filters
means a genuine miss, ignoring that new filters might be added before the
asynchronous teardown completes?

If a user executes 'tc qdisc del' followed immediately by 'tc qdisc add' and
'tc filter add', the qdisc deletion initiates a deferred block destruction
via tcf_block_put(), which queues work to tc_filter_wq.

Meanwhile, the new qdisc and filter can be added successfully, incrementing
adapter->num_cloud_filters.

When the deferred teardown work finally executes iavf_delete_clsflower()
for the old filter, the filter will not be found in the list.

Because adapter->num_cloud_filters is now non-zero from the newly added
filter, the driver takes this 'else if' branch and incorrectly returns -EINVAL.

Using a global counter to infer asynchronous teardown context seems brittle and
can negate the patch's intended fix under these race conditions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006143723.2883728-1-aleksandr.loktionov@intel.com?part=2