RE: [Intel-wired-lan] [PATCH iwl-net] iavf: fix VLAN filter lost on add/delete race

[Romanowski, Rafal]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of Petr
> Oros
> Sent: Wednesday, February 25, 2026 3:57 PM
> To: Schmidt, Michal <mschmidt@xxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>;
> Ahmed Zaki <ahmed.zaki@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; intel-wired-
> lan@xxxxxxxxxxxxxxxx; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net] iavf: fix VLAN filter lost on
> add/delete race
> 
> 
> On 2/25/26 12:17, Michal Schmidt wrote:
> > On Wed, Feb 25, 2026 at 11:02 AM Petr Oros <poros@xxxxxxxxxx> wrote:
> >> When iavf_add_vlan() finds an existing filter in IAVF_VLAN_REMOVE
> >> state, it transitions the filter to IAVF_VLAN_ACTIVE assuming the
> >> pending delete can simply be cancelled. However, there is no
> >> guarantee that iavf_del_vlans() has not already processed the delete
> >> AQ request and removed the filter from the PF. In that case the
> >> filter remains in the driver's list as IAVF_VLAN_ACTIVE but is no
> >> longer programmed on the NIC. Since iavf_add_vlans() only picks up
> >> filters in IAVF_VLAN_ADD state, the filter is never re-added, and
> >> spoof checking drops all traffic for that VLAN.
> >>
> >>    CPU0                       CPU1                     Workqueue
> >>    ----                       ----                     ---------
> >>    iavf_del_vlan(vlan 100)
> >>      f->state = REMOVE
> >>      schedule AQ_DEL_VLAN
> >>                               iavf_add_vlan(vlan 100)
> >>                                 f->state = ACTIVE
> >>                                                        iavf_del_vlans()
> >>                                                          f is ACTIVE, skip
> >>                                                        iavf_add_vlans()
> >>                                                          f is ACTIVE,
> >> skip
> >>
> >>    Filter is ACTIVE in driver but absent from NIC.
> > I don't get it. If, as the diagram shows, iavf_del_vlans() skipped it,
> > then how does the filter become absent from NIC?
> 
> VLAN teardown            Workqueue           VLAN setup
> -------------            ---------           ----------
> iavf_del_vlan(100)
>    f->state = REMOVE
>    schedule AQ_DEL_VLAN
>                                               iavf_open()
>                                                 restore_filters()
>                                                   f is REMOVE, skip
>                                                 iavf_add_vlan(100)
>                                                  f->state = ACTIVE
> 
>                           iavf_del_vlans()
>                            f is ACTIVE, skip
>                           iavf_add_vlans()
>                            f is ACTIVE, skip
> 
> Is it clear like this?
> 
> > Michal
> >
> >> Transition to IAVF_VLAN_ADD instead and schedule
> >> IAVF_FLAG_AQ_ADD_VLAN_FILTER so iavf_add_vlans() re-programs the
> >> filter.  A duplicate add is idempotent on the PF.
> >>
> >> Fixes: 0c0da0e95105 ("iavf: refactor VLAN filter states")
> >>
> >> Signed-off-by: Petr Oros <poros@xxxxxxxxxx>
> >> ---
> >>   drivers/net/ethernet/intel/iavf/iavf_main.c | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> index 4b0fc8f354bc90..6046b93c7f3472 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.
> >> +               /* Re-add the filter since we cannot tell whether the
> >> +                * pending delete has already been processed by the PF.
> >> +                * A duplicate add is harmless.
> >>                   */
> >> -               f->state = IAVF_VLAN_ACTIVE;
> >> +               f->state = IAVF_VLAN_ADD;
> >> +               iavf_schedule_aq_request(adapter,
> >> +
> >> + IAVF_FLAG_AQ_ADD_VLAN_FILTER);
> >>          }
> >>
> >>   clearout:
> >> --
> >> 2.52.0

Tested-by: Rafal Romanowski <rafal.romanowski@xxxxxxxxx>