| Message ID | 20240905091410.26909-1-michal.swiatkowski@linux.intel.com |
|---|---|
| State | Under Review |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [iwl-net,v2] iavf: allow changing VLAN state without calling PF | expand |
On 9/5/24 11:14, Michal Swiatkowski wrote: > First case: >> ip l a l $VF name vlanx type vlan id 100 >> ip l d vlanx >> ip l a l $VF name vlanx type vlan id 100 > > As workqueue can be execute after sometime, there is a window to have > call trace like that: > - iavf_del_vlan > - iavf_add_vlan > - iavf_del_vlans (wq) > > It means that our VLAN 100 will change the state from IAVF_VLAN_ACTIVE > to IAVF_VLAN_REMOVE (iavf_del_vlan). After that in iavf_add_vlan state > won't be changed because VLAN 100 is on the filter list. The final > result is that the VLAN 100 filter isn't added in hardware (no > iavf_add_vlans call). > > To fix that change the state if the filter wasn't removed yet directly > to active. It is save as IAVF_VLAN_REMOVE means that virtchnl message > wasn't sent yet. > > Second case: >> ip l a l $VF name vlanx type vlan id 100 > Any type of VF reset ex. change trust >> ip l s $PF vf $VF_NUM trust on >> ip l d vlanx >> ip l a l $VF name vlanx type vlan id 100 > > In case of reset iavf driver is responsible for readding all filters > that are being used. To do that all VLAN filters state are changed to > IAVF_VLAN_ADD. Here is even longer window for changing VLAN state from > kernel side, as workqueue isn't called immediately. We can have call > trace like that: > > - changing to IAVF_VLAN_ADD (after reset) > - iavf_del_vlan (called from kernel ops) > - iavf_del_vlans (wq) > > Not exsisitng VLAN filters will be removed from hardware. It isn't a > bug, ice driver will handle it fine. However, we can have call trace > like that: > > - changing to IAVF_VLAN_ADD (after reset) > - iavf_del_vlan (called from kernel ops) > - iavf_add_vlan (called from kernel ops) > - iavf_del_vlans (wq) > > With fix for previous case we end up with no VLAN filters in hardware. > We have to remove VLAN filters if the state is IAVF_VLAN_ADD and delete > VLAN was called. It is save as IAVF_VLAN_ADD means that virtchnl message > wasn't sent yet. > > Fixes: 0c0da0e95105 ("iavf: refactor VLAN filter states") > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > --- > v2: https://lore.kernel.org/netdev/20240904120052.24561-1-michal.swiatkowski@linux.intel.com/ > * add missing state in case of delete > --- > drivers/net/ethernet/intel/iavf/iavf_main.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index ff11bafb3b4f..3eae0a456e86 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -773,6 +773,11 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter, f->state = IAVF_VLAN_ADD; 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. + */ + f->state = IAVF_VLAN_ACTIVE; } clearout: @@ -793,8 +798,18 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan) f = iavf_find_vlan(adapter, vlan); if (f) { - f->state = IAVF_VLAN_REMOVE; - iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_VLAN_FILTER); + /* IAVF_ADD_VLAN means that VLAN wasn't even added yet. + * Remove it from the list. + */ + if (f->state == IAVF_VLAN_ADD) { + list_del(&f->list); + kfree(f); + adapter->num_vlan_filters--; + } else { + f->state = IAVF_VLAN_REMOVE; + iavf_schedule_aq_request(adapter, + IAVF_FLAG_AQ_DEL_VLAN_FILTER); + } } spin_unlock_bh(&adapter->mac_vlan_list_lock);