Commit 84f5ca6c authored by Alan Brady's avatar Alan Brady Committed by Jeff Kirsher

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: default avatarAlan Brady <alan.brady@intel.com>
Tested-by: default avatarAndrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent 4a2ce27b
...@@ -1774,19 +1774,22 @@ i40e_update_filter_state(int count, ...@@ -1774,19 +1774,22 @@ i40e_update_filter_state(int count,
**/ **/
int i40e_sync_vsi_filters(struct i40e_vsi *vsi) 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 hlist_head tmp_add_list, tmp_del_list;
struct i40e_mac_filter *f, *add_head = NULL;
struct i40e_hw *hw = &vsi->back->hw; 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; bool promisc_changed = false;
char vsi_name[16] = "PF"; char vsi_name[16] = "PF";
int filter_list_len = 0; int filter_list_len = 0;
u32 changed_flags = 0;
i40e_status aq_ret = 0; i40e_status aq_ret = 0;
u32 changed_flags = 0;
struct hlist_node *h; struct hlist_node *h;
int retval = 0;
struct i40e_pf *pf; struct i40e_pf *pf;
int num_add = 0; int num_add = 0;
int num_del = 0; int num_del = 0;
int retval = 0;
int aq_err = 0; int aq_err = 0;
u16 cmd_flags; u16 cmd_flags;
int list_size; int list_size;
...@@ -1825,11 +1828,75 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) ...@@ -1825,11 +1828,75 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
hash_del(&f->hlist); hash_del(&f->hlist);
hlist_add_head(&f->hlist, &tmp_del_list); hlist_add_head(&f->hlist, &tmp_del_list);
vsi->active_filters--; vsi->active_filters--;
/* Avoid counting removed filters */
continue;
} }
if (f->state == I40E_FILTER_NEW) { if (f->state == I40E_FILTER_NEW) {
hash_del(&f->hlist); hash_del(&f->hlist);
hlist_add_head(&f->hlist, &tmp_add_list); 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); spin_unlock_bh(&vsi->mac_filter_hash_lock);
} }
...@@ -2150,6 +2217,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) ...@@ -2150,6 +2217,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
err_no_memory: err_no_memory:
/* Restore elements on the temporary add and delete lists */ /* Restore elements on the temporary add and delete lists */
spin_lock_bh(&vsi->mac_filter_hash_lock); 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_del_list);
i40e_undo_filter_entries(vsi, &tmp_add_list); i40e_undo_filter_entries(vsi, &tmp_add_list);
spin_unlock_bh(&vsi->mac_filter_hash_lock); spin_unlock_bh(&vsi->mac_filter_hash_lock);
...@@ -2403,9 +2471,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) ...@@ -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) int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
{ {
struct net_device *netdev = vsi->netdev; struct net_device *netdev = vsi->netdev;
struct i40e_mac_filter *f, *add_f; struct i40e_mac_filter *f;
struct hlist_node *h; struct hlist_node *h;
int filter_count = 0;
int bkt; int bkt;
/* Locked once because all functions invoked below iterates list */ /* Locked once because all functions invoked below iterates list */
...@@ -2419,49 +2486,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) ...@@ -2419,49 +2486,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
__i40e_del_filter(vsi, f); __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); spin_unlock_bh(&vsi->mac_filter_hash_lock);
/* schedule our worker thread which will take care of /* schedule our worker thread which will take care of
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment