iavf: remove active_cvlans and active_svlans bitmaps
authorAhmed Zaki <ahmed.zaki@intel.com>
Thu, 6 Apr 2023 21:35:28 +0000 (15:35 -0600)
committerTony Nguyen <anthony.l.nguyen@intel.com>
Fri, 7 Apr 2023 15:49:50 +0000 (08:49 -0700)
The VLAN filters info is currently being held in a list and 2 bitmaps
(active_cvlans and active_svlans). We are experiencing some racing where
data is not in sync in the list and bitmaps. For example, the VLAN is
initially added to the list but only when the PF replies, it is added to
the bitmap. If a user adds many V2 VLANS before the PF responds:

    while [ $((i++)) ]
        ip l add l eth0 name eth0.$i type vlan id $i

we might end up with more VLAN list entries than the designated limit.
Also, The "ip link show" will show more links added than the PF limit.

On the other and, the bitmaps are only used to check the number of VLAN
filters and to re-enable the filters when the interface goes from DOWN to
UP.

This patch gets rid of the bitmaps and uses the list only. To do that,
the states of the VLAN filter are modified:
1 - IAVF_VLAN_REMOVE: the entry needs to be totally removed after informing
  the PF. This is the "ip link del eth0.$i" path.
2 - IAVF_VLAN_DISABLE: (new) the netdev went down. The filter needs to be
  removed from the PF and then marked INACTIVE.
3 - IAVF_VLAN_INACTIVE: (new) no PF filter exists, but the user did not
  delete the VLAN.

Fixes: 48ccc43ecf10 ("iavf: Add support VIRTCHNL_VF_OFFLOAD_VLAN_V2 during netdev config")
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
drivers/net/ethernet/intel/iavf/iavf.h
drivers/net/ethernet/intel/iavf/iavf_main.c
drivers/net/ethernet/intel/iavf/iavf_virtchnl.c

index 692d6a6421183ec34b4530097714ed149b67adb6..746ff76f2fb1e648882b9a393415823be12d0c9c 100644 (file)
@@ -59,8 +59,6 @@ enum iavf_vsi_state_t {
 struct iavf_vsi {
        struct iavf_adapter *back;
        struct net_device *netdev;
-       unsigned long active_cvlans[BITS_TO_LONGS(VLAN_N_VID)];
-       unsigned long active_svlans[BITS_TO_LONGS(VLAN_N_VID)];
        u16 seid;
        u16 id;
        DECLARE_BITMAP(state, __IAVF_VSI_STATE_SIZE__);
@@ -163,7 +161,9 @@ enum iavf_vlan_state_t {
        IAVF_VLAN_ADD,          /* filter needs to be added */
        IAVF_VLAN_IS_NEW,       /* filter is new, wait for PF answer */
        IAVF_VLAN_ACTIVE,       /* filter is accepted by PF */
-       IAVF_VLAN_REMOVE,       /* filter needs to be removed */
+       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 */
 };
 
 struct iavf_vlan_filter {
@@ -261,6 +261,7 @@ struct iavf_adapter {
        wait_queue_head_t vc_waitqueue;
        struct iavf_q_vector *q_vectors;
        struct list_head vlan_filter_list;
+       int num_vlan_filters;
        struct list_head mac_filter_list;
        struct mutex crit_lock;
        struct mutex client_lock;
index fe9798e4b4acec6a9275b49f6f5a5a8f8185c14d..2de4baff4c20501baab55b7ffddb84b6f8e20aee 100644 (file)
@@ -792,6 +792,7 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter,
 
                list_add_tail(&f->list, &adapter->vlan_filter_list);
                f->state = IAVF_VLAN_ADD;
+               adapter->num_vlan_filters++;
                adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
        }
 
@@ -828,14 +829,18 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
  **/
 static void iavf_restore_filters(struct iavf_adapter *adapter)
 {
-       u16 vid;
+       struct iavf_vlan_filter *f;
 
        /* re-add all VLAN filters */
-       for_each_set_bit(vid, adapter->vsi.active_cvlans, VLAN_N_VID)
-               iavf_add_vlan(adapter, IAVF_VLAN(vid, ETH_P_8021Q));
+       spin_lock_bh(&adapter->mac_vlan_list_lock);
 
-       for_each_set_bit(vid, adapter->vsi.active_svlans, VLAN_N_VID)
-               iavf_add_vlan(adapter, IAVF_VLAN(vid, ETH_P_8021AD));
+       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;
 }
 
 /**
@@ -844,8 +849,7 @@ static void iavf_restore_filters(struct iavf_adapter *adapter)
  */
 u16 iavf_get_num_vlans_added(struct iavf_adapter *adapter)
 {
-       return bitmap_weight(adapter->vsi.active_cvlans, VLAN_N_VID) +
-               bitmap_weight(adapter->vsi.active_svlans, VLAN_N_VID);
+       return adapter->num_vlan_filters;
 }
 
 /**
@@ -928,11 +932,6 @@ static int iavf_vlan_rx_kill_vid(struct net_device *netdev,
                return 0;
 
        iavf_del_vlan(adapter, IAVF_VLAN(vid, be16_to_cpu(proto)));
-       if (proto == cpu_to_be16(ETH_P_8021Q))
-               clear_bit(vid, adapter->vsi.active_cvlans);
-       else
-               clear_bit(vid, adapter->vsi.active_svlans);
-
        return 0;
 }
 
@@ -1293,16 +1292,11 @@ static void iavf_clear_mac_vlan_filters(struct iavf_adapter *adapter)
                }
        }
 
-       /* remove all VLAN filters */
+       /* disable all VLAN filters */
        list_for_each_entry_safe(vlf, vlftmp, &adapter->vlan_filter_list,
-                                list) {
-               if (vlf->state == IAVF_VLAN_ADD) {
-                       list_del(&vlf->list);
-                       kfree(vlf);
-               } else {
-                       vlf->state = IAVF_VLAN_REMOVE;
-               }
-       }
+                                list)
+               vlf->state = IAVF_VLAN_DISABLE;
+
        spin_unlock_bh(&adapter->mac_vlan_list_lock);
 }
 
@@ -2914,6 +2908,7 @@ static void iavf_disable_vf(struct iavf_adapter *adapter)
                list_del(&fv->list);
                kfree(fv);
        }
+       adapter->num_vlan_filters = 0;
 
        spin_unlock_bh(&adapter->mac_vlan_list_lock);
 
@@ -3131,9 +3126,6 @@ continue_reset:
        adapter->aq_required |= IAVF_FLAG_AQ_ADD_CLOUD_FILTER;
        iavf_misc_irq_enable(adapter);
 
-       bitmap_clear(adapter->vsi.active_cvlans, 0, VLAN_N_VID);
-       bitmap_clear(adapter->vsi.active_svlans, 0, VLAN_N_VID);
-
        mod_delayed_work(adapter->wq, &adapter->watchdog_task, 2);
 
        /* We were running when the reset started, so we need to restore some
index 5047b4c83718db7fd9d4e1170a035b2f1de84784..9afbbdac35903f1ae018c00fe28213ece3227d3f 100644 (file)
@@ -643,15 +643,9 @@ 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->vlan.tpid == ETH_P_8021Q)
-                               clear_bit(f->vlan.vid,
-                                         adapter->vsi.active_cvlans);
-                       else
-                               clear_bit(f->vlan.vid,
-                                         adapter->vsi.active_svlans);
-
                        list_del(&f->list);
                        kfree(f);
+                       adapter->num_vlan_filters--;
                }
        }
        spin_unlock_bh(&adapter->mac_vlan_list_lock);
@@ -824,7 +818,12 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
                    !VLAN_FILTERING_ALLOWED(adapter)) {
                        list_del(&f->list);
                        kfree(f);
-               } else if (f->state == IAVF_VLAN_REMOVE) {
+                       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) {
                        count++;
                }
        }
@@ -856,11 +855,18 @@ 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_REMOVE) {
+                       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) {
+                               vvfl->vlan_id[i] = f->vlan.vid;
                                list_del(&f->list);
                                kfree(f);
+                               adapter->num_vlan_filters--;
+                               i++;
                                if (i == count)
                                        break;
                        }
@@ -900,7 +906,8 @@ 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_REMOVE) {
+                       if (f->state == IAVF_VLAN_DISABLE ||
+                           f->state == IAVF_VLAN_REMOVE) {
                                struct virtchnl_vlan_supported_caps *filtering_support =
                                        &adapter->vlan_v2_caps.filtering.filtering_support;
                                struct virtchnl_vlan *vlan;
@@ -914,8 +921,13 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
                                vlan->tci = f->vlan.vid;
                                vlan->tpid = f->vlan.tpid;
 
-                               list_del(&f->list);
-                               kfree(f);
+                               if (f->state == IAVF_VLAN_DISABLE) {
+                                       f->state = IAVF_VLAN_INACTIVE;
+                               } else {
+                                       list_del(&f->list);
+                                       kfree(f);
+                                       adapter->num_vlan_filters--;
+                               }
                                i++;
                                if (i == count)
                                        break;
@@ -2443,15 +2455,8 @@ 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_IS_NEW)
                                f->state = IAVF_VLAN_ACTIVE;
-                               if (f->vlan.tpid == ETH_P_8021Q)
-                                       set_bit(f->vlan.vid,
-                                               adapter->vsi.active_cvlans);
-                               else
-                                       set_bit(f->vlan.vid,
-                                               adapter->vsi.active_svlans);
-                       }
                }
                spin_unlock_bh(&adapter->mac_vlan_list_lock);
                }