[PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling

Petr Oros posted 4 patches 3 weeks ago
[PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling
Posted by Petr Oros 3 weeks ago
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
RE: [Intel-wired-lan] [PATCH iwl-next 4/4] iavf: harden VLAN filter state machine race handling
Posted by Loktionov, Aleksandr 3 weeks ago

> -----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>