From: Petr Oros <poros@redhat.com>
To: netdev@vger.kernel.org
Cc: jacob.e.keller@intel.com, Petr Oros <poros@redhat.com>,
Tony Nguyen <anthony.l.nguyen@intel.com>,
Przemek Kitszel <przemyslaw.kitszel@intel.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
Subject: [PATCH RFC iwl-next 3/4] iavf: wait for PF confirmation before removing VLAN filters
Date: Mon, 2 Mar 2026 12:40:24 +0100 [thread overview]
Message-ID: <20260302114025.1017985-4-poros@redhat.com> (raw)
In-Reply-To: <20260302114025.1017985-1-poros@redhat.com>
The VLAN filter DELETE path was asymmetric with the ADD path: ADD
waits for PF confirmation (ADD -> ADDING -> ACTIVE), but DELETE
immediately frees the filter struct after sending the DEL message
without waiting for the PF response.
This is problematic because:
- If the PF rejects the DEL, the filter remains in HW but the driver
has already freed the tracking structure, losing sync.
- Race conditions between DEL pending and other operations
(add, reset) cannot be properly resolved if the filter struct
is already gone.
Add IAVF_VLAN_REMOVING state to make the DELETE path symmetric:
REMOVE -> REMOVING (send DEL) -> PF confirms -> kfree
-> PF rejects -> ACTIVE
In iavf_del_vlans(), transition filters from REMOVE to REMOVING
instead of immediately freeing them. The new DEL completion handler
in iavf_virtchnl_completion() frees filters on success or reverts
them to ACTIVE on error.
Update iavf_add_vlan() to handle the REMOVING state: if a DEL is
pending and the user re-adds the same VLAN, queue it for ADD so
it gets re-programmed after the PF processes the DEL.
The !VLAN_FILTERING_ALLOWED early-exit path still frees filters
directly since no PF message is sent in that case.
Signed-off-by: Petr Oros <poros@redhat.com>
---
drivers/net/ethernet/intel/iavf/iavf.h | 1 +
drivers/net/ethernet/intel/iavf/iavf_main.c | 9 +++--
.../net/ethernet/intel/iavf/iavf_virtchnl.c | 37 +++++++++++++------
3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 1ad00690622c8e..f9ad814d18b1da 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -161,6 +161,7 @@ enum iavf_vlan_state_t {
IAVF_VLAN_ADDING, /* ADD sent to PF, waiting for response */
IAVF_VLAN_ACTIVE, /* PF confirmed, filter is in HW */
IAVF_VLAN_REMOVE, /* filter queued for DEL from PF */
+ IAVF_VLAN_REMOVING, /* DEL sent to PF, waiting for response */
};
struct iavf_vlan_filter {
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index f1ab68f5f4050b..212a23ead20c57 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -782,10 +782,13 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter,
adapter->num_vlan_filters++;
iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_ADD_VLAN_FILTER);
} else if (f->state == IAVF_VLAN_REMOVE) {
- /* IAVF_VLAN_REMOVE means that VLAN wasn't yet removed.
- * We can safely only change the state here.
- */
+ /* DEL not yet sent to PF, cancel it */
f->state = IAVF_VLAN_ACTIVE;
+ } else if (f->state == IAVF_VLAN_REMOVING) {
+ /* DEL already sent to PF, re-add after completion */
+ f->state = IAVF_VLAN_ADD;
+ iavf_schedule_aq_request(adapter,
+ IAVF_FLAG_AQ_ADD_VLAN_FILTER);
}
clearout:
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index d62c0d6394149e..9f2b64bb4ed9e7 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -948,12 +948,10 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vvfl->vsi_id = adapter->vsi_res->vsi_id;
vvfl->num_elements = count;
- list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
+ list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->state == IAVF_VLAN_REMOVE) {
vvfl->vlan_id[i] = f->vlan.vid;
- list_del(&f->list);
- kfree(f);
- adapter->num_vlan_filters--;
+ f->state = IAVF_VLAN_REMOVING;
i++;
if (i == count)
break;
@@ -990,7 +988,7 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vvfl_v2->vport_id = adapter->vsi_res->vsi_id;
vvfl_v2->num_elements = count;
- list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
+ list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->state == IAVF_VLAN_REMOVE) {
struct virtchnl_vlan_supported_caps *filtering_support =
&adapter->vlan_v2_caps.filtering.filtering_support;
@@ -1005,9 +1003,7 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vlan->tci = f->vlan.vid;
vlan->tpid = f->vlan.tpid;
- list_del(&f->list);
- kfree(f);
- adapter->num_vlan_filters--;
+ f->state = IAVF_VLAN_REMOVING;
i++;
if (i == count)
break;
@@ -2370,10 +2366,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
ether_addr_copy(adapter->hw.mac.addr, netdev->dev_addr);
wake_up(&adapter->vc_waitqueue);
break;
- case VIRTCHNL_OP_DEL_VLAN:
- dev_err(&adapter->pdev->dev, "Failed to delete VLAN filter, error %s\n",
- iavf_stat_str(&adapter->hw, v_retval));
- break;
case VIRTCHNL_OP_DEL_ETH_ADDR:
dev_err(&adapter->pdev->dev, "Failed to delete MAC filter, error %s\n",
iavf_stat_str(&adapter->hw, v_retval));
@@ -2896,6 +2888,27 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
spin_unlock_bh(&adapter->mac_vlan_list_lock);
}
break;
+ case VIRTCHNL_OP_DEL_VLAN:
+ case VIRTCHNL_OP_DEL_VLAN_V2: {
+ struct iavf_vlan_filter *f, *ftmp;
+
+ spin_lock_bh(&adapter->mac_vlan_list_lock);
+ list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list,
+ list) {
+ if (f->state == IAVF_VLAN_REMOVING) {
+ if (v_retval) {
+ /* PF rejected DEL, filter is still in HW */
+ f->state = IAVF_VLAN_ACTIVE;
+ } else {
+ list_del(&f->list);
+ kfree(f);
+ adapter->num_vlan_filters--;
+ }
+ }
+ }
+ spin_unlock_bh(&adapter->mac_vlan_list_lock);
+ }
+ break;
case VIRTCHNL_OP_ENABLE_VLAN_STRIPPING:
/* PF enabled vlan strip on this VF.
* Update netdev->features if needed to be in sync with ethtool.
--
2.52.0
next prev parent reply other threads:[~2026-03-02 11:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-02 11:40 [PATCH RFC iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
2026-03-02 11:40 ` [PATCH RFC iwl-next 1/4] iavf: rename IAVF_VLAN_IS_NEW to IAVF_VLAN_ADDING Petr Oros
2026-03-16 11:34 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-03-02 11:40 ` [PATCH RFC iwl-next 2/4] iavf: stop removing VLAN filters from PF on interface down Petr Oros
2026-03-16 11:35 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-03-02 11:40 ` Petr Oros [this message]
2026-03-02 11:40 ` [PATCH RFC iwl-next 4/4] iavf: harden VLAN filter state machine race handling Petr Oros
2026-03-06 13:12 ` [PATCH RFC iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260302114025.1017985-4-poros@redhat.com \
--to=poros@redhat.com \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox