* [PATCH iwl-next 1/4] iavf: rename IAVF_VLAN_IS_NEW to IAVF_VLAN_ADDING
2026-03-16 10:42 [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
@ 2026-03-16 10:42 ` Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 2/4] iavf: stop removing VLAN filters from PF on interface down Petr Oros
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Petr Oros @ 2026-03-16 10:42 UTC (permalink / raw)
To: netdev
Cc: jacob.e.keller, Petr Oros, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, intel-wired-lan, linux-kernel
Rename the IAVF_VLAN_IS_NEW state to IAVF_VLAN_ADDING to better
describe what the state represents: an ADD request has been sent to
the PF and is waiting for a response.
This is a pure rename with no behavioral change, preparing for a
cleanup of the VLAN filter state machine.
Signed-off-by: Petr Oros <poros@redhat.com>
---
drivers/net/ethernet/intel/iavf/iavf.h | 2 +-
drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index a87e0c6d4017ad..8e6db72828ae14 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -158,7 +158,7 @@ struct iavf_vlan {
enum iavf_vlan_state_t {
IAVF_VLAN_INVALID,
IAVF_VLAN_ADD, /* filter needs to be added */
- IAVF_VLAN_IS_NEW, /* filter is new, wait for PF answer */
+ IAVF_VLAN_ADDING, /* ADD sent to PF, waiting for response */
IAVF_VLAN_ACTIVE, /* filter is accepted by PF */
IAVF_VLAN_DISABLE, /* filter needs to be deleted by PF, then marked INACTIVE */
IAVF_VLAN_INACTIVE, /* filter is inactive, we are in IFF_DOWN */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 88156082a41da6..5114934fe81fa6 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -746,7 +746,7 @@ static void iavf_vlan_add_reject(struct iavf_adapter *adapter)
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_IS_NEW) {
+ if (f->state == IAVF_VLAN_ADDING) {
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
@@ -812,7 +812,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
if (f->state == IAVF_VLAN_ADD) {
vvfl->vlan_id[i] = f->vlan.vid;
i++;
- f->state = IAVF_VLAN_IS_NEW;
+ f->state = IAVF_VLAN_ADDING;
if (i == count)
break;
}
@@ -874,7 +874,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
vlan->tpid = f->vlan.tpid;
i++;
- f->state = IAVF_VLAN_IS_NEW;
+ f->state = IAVF_VLAN_ADDING;
}
}
@@ -2911,7 +2911,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
spin_lock_bh(&adapter->mac_vlan_list_lock);
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
- if (f->state == IAVF_VLAN_IS_NEW)
+ if (f->state == IAVF_VLAN_ADDING)
f->state = IAVF_VLAN_ACTIVE;
}
spin_unlock_bh(&adapter->mac_vlan_list_lock);
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH iwl-next 2/4] iavf: stop removing VLAN filters from PF on interface down
2026-03-16 10:42 [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 1/4] iavf: rename IAVF_VLAN_IS_NEW to IAVF_VLAN_ADDING Petr Oros
@ 2026-03-16 10:42 ` Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 3/4] iavf: wait for PF confirmation before removing VLAN filters Petr Oros
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Petr Oros @ 2026-03-16 10:42 UTC (permalink / raw)
To: netdev
Cc: jacob.e.keller, Petr Oros, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, intel-wired-lan, linux-kernel
When a VF goes down, the driver currently sends DEL_VLAN to the PF for
every VLAN filter (ACTIVE -> DISABLE -> send DEL -> INACTIVE), then
re-adds them all on UP (INACTIVE -> ADD -> send ADD -> ADDING ->
ACTIVE). This round-trip is unnecessary because:
1. The PF disables the VF's queues via VIRTCHNL_OP_DISABLE_QUEUES,
which already prevents all RX/TX traffic regardless of VLAN filter
state.
2. The VLAN filters remaining in PF HW while the VF is down is
harmless - packets matching those filters have nowhere to go with
queues disabled.
3. The DEL+ADD cycle during down/up creates race windows where the
VLAN filter list is incomplete. With spoofcheck enabled, the PF
enables TX VLAN filtering on the first non-zero VLAN add, blocking
traffic for any VLANs not yet re-added.
Remove the entire DISABLE/INACTIVE state machinery:
- Remove IAVF_VLAN_DISABLE and IAVF_VLAN_INACTIVE enum values
- Remove iavf_restore_filters() and its call from iavf_open()
- Remove VLAN filter handling from iavf_clear_mac_vlan_filters(),
rename it to iavf_clear_mac_filters()
- Remove DEL_VLAN_FILTER scheduling from iavf_down()
- Remove all DISABLE/INACTIVE handling from iavf_del_vlans()
VLAN filters now stay ACTIVE across down/up cycles. Only explicit
user removal (ndo_vlan_rx_kill_vid) or PF/VF reset triggers VLAN
filter deletion/re-addition.
Signed-off-by: Petr Oros <poros@redhat.com>
---
drivers/net/ethernet/intel/iavf/iavf.h | 6 +--
drivers/net/ethernet/intel/iavf/iavf_main.c | 39 ++-----------------
.../net/ethernet/intel/iavf/iavf_virtchnl.c | 33 +++-------------
3 files changed, 12 insertions(+), 66 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 8e6db72828ae14..1ad00690622c8e 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -159,10 +159,8 @@ enum iavf_vlan_state_t {
IAVF_VLAN_INVALID,
IAVF_VLAN_ADD, /* filter needs to be added */
IAVF_VLAN_ADDING, /* ADD sent to PF, waiting for response */
- IAVF_VLAN_ACTIVE, /* filter is accepted by PF */
- IAVF_VLAN_DISABLE, /* filter needs to be deleted by PF, then marked INACTIVE */
- IAVF_VLAN_INACTIVE, /* filter is inactive, we are in IFF_DOWN */
- IAVF_VLAN_REMOVE, /* filter needs to be removed from list */
+ IAVF_VLAN_ACTIVE, /* PF confirmed, filter is in HW */
+ IAVF_VLAN_REMOVE, /* filter queued for DEL from PF */
};
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 86c1964f42e101..b38ce496a95c75 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -823,27 +823,6 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
spin_unlock_bh(&adapter->mac_vlan_list_lock);
}
-/**
- * iavf_restore_filters
- * @adapter: board private structure
- *
- * Restore existing non MAC filters when VF netdev comes back up
- **/
-static void iavf_restore_filters(struct iavf_adapter *adapter)
-{
- struct iavf_vlan_filter *f;
-
- /* re-add all VLAN filters */
- spin_lock_bh(&adapter->mac_vlan_list_lock);
-
- list_for_each_entry(f, &adapter->vlan_filter_list, list) {
- if (f->state == IAVF_VLAN_INACTIVE)
- f->state = IAVF_VLAN_ADD;
- }
-
- spin_unlock_bh(&adapter->mac_vlan_list_lock);
- adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
-}
/**
* iavf_get_num_vlans_added - get number of VLANs added
@@ -1262,13 +1241,12 @@ static void iavf_up_complete(struct iavf_adapter *adapter)
}
/**
- * iavf_clear_mac_vlan_filters - Remove mac and vlan filters not sent to PF
- * yet and mark other to be removed.
+ * iavf_clear_mac_filters - Remove MAC filters not sent to PF yet and mark
+ * others to be removed.
* @adapter: board private structure
**/
-static void iavf_clear_mac_vlan_filters(struct iavf_adapter *adapter)
+static void iavf_clear_mac_filters(struct iavf_adapter *adapter)
{
- struct iavf_vlan_filter *vlf, *vlftmp;
struct iavf_mac_filter *f, *ftmp;
spin_lock_bh(&adapter->mac_vlan_list_lock);
@@ -1287,11 +1265,6 @@ static void iavf_clear_mac_vlan_filters(struct iavf_adapter *adapter)
}
}
- /* disable all VLAN filters */
- list_for_each_entry_safe(vlf, vlftmp, &adapter->vlan_filter_list,
- list)
- vlf->state = IAVF_VLAN_DISABLE;
-
spin_unlock_bh(&adapter->mac_vlan_list_lock);
}
@@ -1387,7 +1360,7 @@ void iavf_down(struct iavf_adapter *adapter)
iavf_napi_disable_all(adapter);
iavf_irq_disable(adapter);
- iavf_clear_mac_vlan_filters(adapter);
+ iavf_clear_mac_filters(adapter);
iavf_clear_cloud_filters(adapter);
iavf_clear_fdir_filters(adapter);
iavf_clear_adv_rss_conf(adapter);
@@ -1404,8 +1377,6 @@ void iavf_down(struct iavf_adapter *adapter)
*/
if (!list_empty(&adapter->mac_filter_list))
adapter->aq_required |= IAVF_FLAG_AQ_DEL_MAC_FILTER;
- if (!list_empty(&adapter->vlan_filter_list))
- adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;
if (!list_empty(&adapter->cloud_filter_list))
adapter->aq_required |= IAVF_FLAG_AQ_DEL_CLOUD_FILTER;
if (!list_empty(&adapter->fdir_list_head))
@@ -4502,8 +4473,6 @@ static int iavf_open(struct net_device *netdev)
iavf_add_filter(adapter, adapter->hw.mac.addr);
spin_unlock_bh(&adapter->mac_vlan_list_lock);
- /* Restore filters that were removed with IFF_DOWN */
- iavf_restore_filters(adapter);
iavf_restore_fdir_filters(adapter);
iavf_configure(adapter);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 5114934fe81fa6..d62c0d6394149e 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -911,22 +911,12 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
spin_lock_bh(&adapter->mac_vlan_list_lock);
list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) {
- /* since VLAN capabilities are not allowed, we dont want to send
- * a VLAN delete request because it will most likely fail and
- * create unnecessary errors/noise, so just free the VLAN
- * filters marked for removal to enable bailing out before
- * sending a virtchnl message
- */
if (f->state == IAVF_VLAN_REMOVE &&
!VLAN_FILTERING_ALLOWED(adapter)) {
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
- } else if (f->state == IAVF_VLAN_DISABLE &&
- !VLAN_FILTERING_ALLOWED(adapter)) {
- f->state = IAVF_VLAN_INACTIVE;
- } else if (f->state == IAVF_VLAN_REMOVE ||
- f->state == IAVF_VLAN_DISABLE) {
+ } else if (f->state == IAVF_VLAN_REMOVE) {
count++;
}
}
@@ -959,13 +949,7 @@ 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) {
- if (f->state == IAVF_VLAN_DISABLE) {
- vvfl->vlan_id[i] = f->vlan.vid;
- f->state = IAVF_VLAN_INACTIVE;
- i++;
- if (i == count)
- break;
- } else if (f->state == IAVF_VLAN_REMOVE) {
+ if (f->state == IAVF_VLAN_REMOVE) {
vvfl->vlan_id[i] = f->vlan.vid;
list_del(&f->list);
kfree(f);
@@ -1007,8 +991,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) {
- if (f->state == IAVF_VLAN_DISABLE ||
- f->state == IAVF_VLAN_REMOVE) {
+ if (f->state == IAVF_VLAN_REMOVE) {
struct virtchnl_vlan_supported_caps *filtering_support =
&adapter->vlan_v2_caps.filtering.filtering_support;
struct virtchnl_vlan *vlan;
@@ -1022,13 +1005,9 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
vlan->tci = f->vlan.vid;
vlan->tpid = f->vlan.tpid;
- if (f->state == IAVF_VLAN_DISABLE) {
- f->state = IAVF_VLAN_INACTIVE;
- } else {
- list_del(&f->list);
- kfree(f);
- adapter->num_vlan_filters--;
- }
+ list_del(&f->list);
+ kfree(f);
+ adapter->num_vlan_filters--;
i++;
if (i == count)
break;
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH iwl-next 3/4] iavf: wait for PF confirmation before removing VLAN filters
2026-03-16 10:42 [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 1/4] iavf: rename IAVF_VLAN_IS_NEW to IAVF_VLAN_ADDING Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 2/4] iavf: stop removing VLAN filters from PF on interface down Petr Oros
@ 2026-03-16 10:42 ` Petr Oros
2026-03-16 10:42 ` [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling Petr Oros
2026-03-19 18:07 ` [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Simon Horman
4 siblings, 0 replies; 7+ messages in thread
From: Petr Oros @ 2026-03-16 10:42 UTC (permalink / raw)
To: netdev
Cc: jacob.e.keller, Petr Oros, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, intel-wired-lan, linux-kernel
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 b38ce496a95c75..89e5aae20d5573 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..d0b7b810679399 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, keep filter */
+ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling
2026-03-16 10:42 [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
` (2 preceding siblings ...)
2026-03-16 10:42 ` [PATCH iwl-next 3/4] iavf: wait for PF confirmation before removing VLAN filters Petr Oros
@ 2026-03-16 10:42 ` Petr Oros
2026-03-16 11:37 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-03-19 18:07 ` [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Simon Horman
4 siblings, 1 reply; 7+ messages in thread
From: Petr Oros @ 2026-03-16 10:42 UTC (permalink / raw)
To: netdev
Cc: jacob.e.keller, Petr Oros, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, intel-wired-lan, linux-kernel
Address remaining race windows in the VLAN filter state machine that
were identified during cross-state analysis of ADD and DEL paths.
1. Add VIRTCHNL_OP_ADD_VLAN to the success completion handler.
The V1 ADD_VLAN opcode had no success handler -- filters sent via V1
stayed in ADDING state permanently. Add a fallthrough case so V1
filters also transition ADDING -> ACTIVE on PF confirmation.
Critically, add an `if (v_retval) break` guard: the error switch
in iavf_virtchnl_completion() does NOT return after handling errors,
it falls through to the success switch. Without this guard, a
PF-rejected ADD would incorrectly mark ADDING filters as ACTIVE,
creating a driver/HW mismatch where the driver believes the filter
is installed but the PF never accepted it.
For V2, this is harmless: iavf_vlan_add_reject() in the error
block already kfree'd all ADDING filters, so the success handler
finds nothing to transition.
2. Skip DEL on filters already in REMOVING state.
In iavf_del_vlan(), if a filter is in IAVF_VLAN_REMOVING (DEL
already sent to PF, waiting for response), do not overwrite to
REMOVE and schedule a redundant DEL. The pending DEL's
completion handler will either kfree the filter (PF confirms)
or revert to ACTIVE (PF rejects).
Without this, the sequence DEL(pending) -> user-del -> second DEL
could result in PF returning an error for the second DEL (filter
already gone), causing the completion handler to incorrectly revert
a deleted filter back to ACTIVE.
Signed-off-by: Petr Oros <poros@redhat.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 5 ++++-
drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 4 ++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 89e5aae20d5573..1ffc0ce3f35602 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -816,11 +816,14 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
list_del(&f->list);
kfree(f);
adapter->num_vlan_filters--;
- } else {
+ } else if (f->state != IAVF_VLAN_REMOVING) {
f->state = IAVF_VLAN_REMOVE;
iavf_schedule_aq_request(adapter,
IAVF_FLAG_AQ_DEL_VLAN_FILTER);
}
+ /* If REMOVING, DEL is already sent to PF; completion
+ * handler will free the filter when PF confirms.
+ */
}
spin_unlock_bh(&adapter->mac_vlan_list_lock);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index d0b7b810679399..147adb76f64141 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -2877,9 +2877,13 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
spin_unlock_bh(&adapter->adv_rss_lock);
}
break;
+ case VIRTCHNL_OP_ADD_VLAN:
case VIRTCHNL_OP_ADD_VLAN_V2: {
struct iavf_vlan_filter *f;
+ if (v_retval)
+ break;
+
spin_lock_bh(&adapter->mac_vlan_list_lock);
list_for_each_entry(f, &adapter->vlan_filter_list, list) {
if (f->state == IAVF_VLAN_ADDING)
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* RE: [Intel-wired-lan] [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling
2026-03-16 10:42 ` [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling Petr Oros
@ 2026-03-16 11:37 ` Loktionov, Aleksandr
0 siblings, 0 replies; 7+ messages in thread
From: Loktionov, Aleksandr @ 2026-03-16 11:37 UTC (permalink / raw)
To: Oros, Petr, netdev@vger.kernel.org
Cc: Kitszel, Przemyslaw, Eric Dumazet, linux-kernel@vger.kernel.org,
Andrew Lunn, Nguyen, Anthony L, intel-wired-lan@lists.osuosl.org,
Keller, Jacob E, Jakub Kicinski, Paolo Abeni, David S. Miller
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Petr Oros
> Sent: Monday, March 16, 2026 11:42 AM
> To: netdev@vger.kernel.org
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Eric Dumazet
> <edumazet@google.com>; linux-kernel@vger.kernel.org; Andrew Lunn
> <andrew+netdev@lunn.ch>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; intel-wired-lan@lists.osuosl.org;
> Keller, Jacob E <jacob.e.keller@intel.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller
> <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-next 4/4] iavf: harden VLAN
> filter state machine race handling
>
> Address remaining race windows in the VLAN filter state machine that
> were identified during cross-state analysis of ADD and DEL paths.
>
> 1. Add VIRTCHNL_OP_ADD_VLAN to the success completion handler.
>
> The V1 ADD_VLAN opcode had no success handler -- filters sent via
> V1
> stayed in ADDING state permanently. Add a fallthrough case so V1
> filters also transition ADDING -> ACTIVE on PF confirmation.
>
> Critically, add an `if (v_retval) break` guard: the error switch
> in iavf_virtchnl_completion() does NOT return after handling
> errors,
> it falls through to the success switch. Without this guard, a
> PF-rejected ADD would incorrectly mark ADDING filters as ACTIVE,
> creating a driver/HW mismatch where the driver believes the filter
> is installed but the PF never accepted it.
>
> For V2, this is harmless: iavf_vlan_add_reject() in the error
> block already kfree'd all ADDING filters, so the success handler
> finds nothing to transition.
>
> 2. Skip DEL on filters already in REMOVING state.
>
> In iavf_del_vlan(), if a filter is in IAVF_VLAN_REMOVING (DEL
> already sent to PF, waiting for response), do not overwrite to
> REMOVE and schedule a redundant DEL. The pending DEL's
> completion handler will either kfree the filter (PF confirms)
> or revert to ACTIVE (PF rejects).
>
> Without this, the sequence DEL(pending) -> user-del -> second DEL
> could result in PF returning an error for the second DEL (filter
> already gone), causing the completion handler to incorrectly revert
> a deleted filter back to ACTIVE.
>
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 5 ++++-
> drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 4 ++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 89e5aae20d5573..1ffc0ce3f35602 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -816,11 +816,14 @@ static void iavf_del_vlan(struct iavf_adapter
> *adapter, struct iavf_vlan vlan)
> list_del(&f->list);
> kfree(f);
> adapter->num_vlan_filters--;
> - } else {
> + } else if (f->state != IAVF_VLAN_REMOVING) {
> f->state = IAVF_VLAN_REMOVE;
> iavf_schedule_aq_request(adapter,
>
> IAVF_FLAG_AQ_DEL_VLAN_FILTER);
> }
> + /* If REMOVING, DEL is already sent to PF; completion
> + * handler will free the filter when PF confirms.
> + */
> }
>
> spin_unlock_bh(&adapter->mac_vlan_list_lock);
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> index d0b7b810679399..147adb76f64141 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> @@ -2877,9 +2877,13 @@ void iavf_virtchnl_completion(struct
> iavf_adapter *adapter,
> spin_unlock_bh(&adapter->adv_rss_lock);
> }
> break;
> + case VIRTCHNL_OP_ADD_VLAN:
> case VIRTCHNL_OP_ADD_VLAN_V2: {
> struct iavf_vlan_filter *f;
>
> + if (v_retval)
> + break;
> +
> spin_lock_bh(&adapter->mac_vlan_list_lock);
> list_for_each_entry(f, &adapter->vlan_filter_list, list)
> {
> if (f->state == IAVF_VLAN_ADDING)
> --
> 2.52.0
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races
2026-03-16 10:42 [PATCH iwl-next 0/4] iavf: fix VLAN filter state machine races Petr Oros
` (3 preceding siblings ...)
2026-03-16 10:42 ` [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling Petr Oros
@ 2026-03-19 18:07 ` Simon Horman
4 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2026-03-19 18:07 UTC (permalink / raw)
To: Petr Oros
Cc: netdev, jacob.e.keller, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
intel-wired-lan, linux-kernel
On Mon, Mar 16, 2026 at 11:42:05AM +0100, Petr Oros wrote:
> The iavf VLAN filter state machine has several design issues that lead
> to race conditions between userspace add/del calls and the watchdog
> task's virtchnl processing. Filters can get lost or leak HW resources,
> especially during interface down/up cycles and namespace moves.
>
> The root problems:
>
> 1) On interface down, all VLAN filters are sent as DEL to PF and
> re-added on interface up. This is unnecessary and creates multiple
> race windows (details below).
>
> 2) The DELETE path immediately frees the filter struct after sending
> the DEL message, without waiting for PF confirmation. If the PF
> rejects the DEL, the filter remains in HW but the driver lost its
> tracking structure. Race conditions between a pending DEL and
> add/reset operations cannot be resolved because the struct is gone.
>
> 3) VIRTCHNL_OP_ADD_VLAN (V1) had no success completion handler, so
> filters stayed in IS_NEW state permanently.
...
> This series addresses all three issues:
>
> Patch 1 renames IS_NEW to ADDING for clarity.
>
> Patch 2 removes the DISABLE/INACTIVE state machinery so VLAN filters
> stay ACTIVE across down/up cycles. This is the core behavioral
> change -- VLAN filters are no longer sent as DEL to PF on interface
> down, and iavf_restore_filters() is removed since there is nothing
> to restore.
>
> Patch 3 adds a REMOVING state to make the DELETE path symmetric with
> ADD -- filters are only freed after PF confirms the deletion. If the
> PF rejects the DEL, the filter reverts to ACTIVE instead of being
> lost.
>
> Patch 4 hardens the remaining race windows: adds V1 ADD success
> handler and prevents redundant DEL on filters already in REMOVING
> state.
For the series:
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread