From 84f5ca6cf4ea1d91fa9ee858ff95211127b0166b Mon Sep 17 00:00:00 2001 From: Alan Brady Date: Wed, 5 Oct 2016 09:30:39 -0700 Subject: [PATCH] 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 Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 120 ++++++++++++-------- 1 file changed, 72 insertions(+), 48 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 35f06ba6bc4e..6cf908953185 100644 --- 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 -- 2.30.2