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 <[email protected]> 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/[email protected]?part=2
