i40e: fix MAC filters when removing VLANs

Currently there exists a bug where adding at least one VLAN and then
removing all VLANs leaves the mac filters for the VSI with an incorrect
value for 'vid' which indicates the mac filter's VLAN status.

The current implementation for handling the removal of VLANs is wrong
for a couple reasons. The first is that when i40e_vsi_kill_vlan
iterates through the MAC filters, it fails to account for the MAC filter
status; i.e. it's not accommodating for filters that are about to be
deleted. The second problem is that MAC filters can be deleted in other
places (specifically i40e_set_rx_mode). Thus if it occurs that all the
VLAN MAC filters get deleted we need to switch out of VLAN mode, but the
code path through i40e_vsi_kill_vlan has already been executed and we're
now stuck in VLAN mode.

This patch fixes the issue by removing the check from i40e_vsi_kill_vlan
and puts the check instead in i40e_sync_vsi_filters where we're
guaranteed to see all filter deletions and can properly detect when we
need to switch out of VLAN mode.

Change-ID: Ib38fe6034b356eee9a0e20b8a9eeed5ff2debcd9
Signed-off-by: Alan Brady <alan.brady@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 35f06ba6bc4e3ee2f11447233b213e2c2c7a73fe..6cf9089531859344390b6019179db1e9ef377499 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1774,19 +1774,22 @@ i40e_update_filter_state(int count,
  **/
 int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 {
-       struct i40e_mac_filter *f, *add_head = NULL;
        struct hlist_head tmp_add_list, tmp_del_list;
+       struct i40e_mac_filter *f, *add_head = NULL;
        struct i40e_hw *hw = &vsi->back->hw;
+       unsigned int vlan_any_filters = 0;
+       unsigned int non_vlan_filters = 0;
+       unsigned int vlan_filters = 0;
        bool promisc_changed = false;
        char vsi_name[16] = "PF";
        int filter_list_len = 0;
-       u32 changed_flags = 0;
        i40e_status aq_ret = 0;
+       u32 changed_flags = 0;
        struct hlist_node *h;
-       int retval = 0;
        struct i40e_pf *pf;
        int num_add = 0;
        int num_del = 0;
+       int retval = 0;
        int aq_err = 0;
        u16 cmd_flags;
        int list_size;
@@ -1825,11 +1828,75 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
                                hash_del(&f->hlist);
                                hlist_add_head(&f->hlist, &tmp_del_list);
                                vsi->active_filters--;
+
+                               /* Avoid counting removed filters */
+                               continue;
                        }
                        if (f->state == I40E_FILTER_NEW) {
                                hash_del(&f->hlist);
                                hlist_add_head(&f->hlist, &tmp_add_list);
                        }
+
+                       /* Count the number of each type of filter we have
+                        * remaining, ignoring any filters we're about to
+                        * delete.
+                        */
+                       if (f->vlan > 0)
+                               vlan_filters++;
+                       else if (!f->vlan)
+                               non_vlan_filters++;
+                       else
+                               vlan_any_filters++;
+               }
+
+               /* We should never have VLAN=-1 filters at the same time as we
+                * have either VLAN=0 or VLAN>0 filters, so warn about this
+                * case here to help catch any issues.
+                */
+               WARN_ON(vlan_any_filters && (vlan_filters + non_vlan_filters));
+
+               /* If we only have VLAN=0 filters remaining, and don't have
+                * any other VLAN filters, we need to convert these VLAN=0
+                * filters into VLAN=-1 (I40E_VLAN_ANY) so that we operate
+                * correctly in non-VLAN mode and receive all traffic tagged
+                * or untagged.
+                */
+               if (non_vlan_filters && !vlan_filters) {
+                       hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f,
+                                          hlist) {
+                               /* Only replace VLAN=0 filters */
+                               if (f->vlan)
+                                       continue;
+
+                               /* Allocate a replacement element */
+                               add_head = kzalloc(sizeof(*add_head),
+                                                  GFP_KERNEL);
+                               if (!add_head)
+                                       goto err_no_memory_locked;
+
+                               /* Copy the filter, with new state and VLAN */
+                               *add_head = *f;
+                               add_head->state = I40E_FILTER_NEW;
+                               add_head->vlan = I40E_VLAN_ANY;
+
+                               /* Move the replacement to the add list */
+                               INIT_HLIST_NODE(&add_head->hlist);
+                               hlist_add_head(&add_head->hlist,
+                                              &tmp_add_list);
+
+                               /* Move the original to the delete list */
+                               f->state = I40E_FILTER_REMOVE;
+                               hash_del(&f->hlist);
+                               hlist_add_head(&f->hlist, &tmp_del_list);
+                               vsi->active_filters--;
+                       }
+
+                       /* Also update any filters on the tmp_add list */
+                       hlist_for_each_entry(f, &tmp_add_list, hlist) {
+                               if (!f->vlan)
+                                       f->vlan = I40E_VLAN_ANY;
+                       }
+                       add_head = NULL;
                }
                spin_unlock_bh(&vsi->mac_filter_hash_lock);
        }
@@ -2150,6 +2217,7 @@ out:
 err_no_memory:
        /* Restore elements on the temporary add and delete lists */
        spin_lock_bh(&vsi->mac_filter_hash_lock);
+err_no_memory_locked:
        i40e_undo_filter_entries(vsi, &tmp_del_list);
        i40e_undo_filter_entries(vsi, &tmp_add_list);
        spin_unlock_bh(&vsi->mac_filter_hash_lock);
@@ -2403,9 +2471,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
 int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
 {
        struct net_device *netdev = vsi->netdev;
-       struct i40e_mac_filter *f, *add_f;
+       struct i40e_mac_filter *f;
        struct hlist_node *h;
-       int filter_count = 0;
        int bkt;
 
        /* Locked once because all functions invoked below iterates list */
@@ -2419,49 +2486,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
                        __i40e_del_filter(vsi, f);
        }
 
-       /* go through all the filters for this VSI and if there is only
-        * vid == 0 it means there are no other filters, so vid 0 must
-        * be replaced with -1. This signifies that we should from now
-        * on accept any traffic (with any tag present, or untagged)
-        */
-       hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) {
-               if (vsi->netdev) {
-                       if (f->vlan &&
-                           ether_addr_equal(netdev->dev_addr, f->macaddr))
-                               filter_count++;
-               }
-
-               if (f->vlan)
-                       filter_count++;
-       }
-
-       if (!filter_count && vsi->netdev) {
-               i40e_del_filter(vsi, netdev->dev_addr, 0);
-               f = i40e_add_filter(vsi, netdev->dev_addr, I40E_VLAN_ANY);
-               if (!f) {
-                       dev_info(&vsi->back->pdev->dev,
-                                "Could not add filter %d for %pM\n",
-                                I40E_VLAN_ANY, netdev->dev_addr);
-                       spin_unlock_bh(&vsi->mac_filter_hash_lock);
-                       return -ENOMEM;
-               }
-       }
-
-       if (!filter_count) {
-               hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
-                       if (!f->vlan)
-                               __i40e_del_filter(vsi, f);
-                       add_f = i40e_add_filter(vsi, f->macaddr, I40E_VLAN_ANY);
-                       if (!add_f) {
-                               dev_info(&vsi->back->pdev->dev,
-                                        "Could not add filter %d for %pM\n",
-                                        I40E_VLAN_ANY, f->macaddr);
-                               spin_unlock_bh(&vsi->mac_filter_hash_lock);
-                               return -ENOMEM;
-                       }
-               }
-       }
-
        spin_unlock_bh(&vsi->mac_filter_hash_lock);
 
        /* schedule our worker thread which will take care of