From ff00f3a9670f97203b33ac00eeca81d75768919e Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 4 Oct 2016 12:20:16 +0100 Subject: [PATCH 01/22] i40e: Add missing \n to end of dev_err message Trival fix, dev_err message is missing a \n, so add it. Signed-off-by: Colin Ian King Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 54b8ee2583f14..64e54a562f411 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -2940,7 +2940,7 @@ int i40e_ndo_set_vf_bw(struct net_device *netdev, int vf_id, int min_tx_rate, } if (max_tx_rate > speed) { - dev_err(&pf->pdev->dev, "Invalid max tx rate %d specified for VF %d.", + dev_err(&pf->pdev->dev, "Invalid max tx rate %d specified for VF %d.\n", max_tx_rate, vf->vf_id); ret = -EINVAL; goto error; From 1bc87e807a6510eb33402258ec1df9bf22b6ff1f Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:31 -0700 Subject: [PATCH 02/22] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Originally the is_vf and is_netdev fields were added in order to distinguish between VF and netdev filters in a single VSI. However, it can be noted that we use separate VSI for SRIOV VFs and for netdev VSI. Thus, since a single VSI should only ever have one type of filter, we can simply remove the checks and remove the typing. In a similar fashion, we can note that the only remaining way to get multiple filters of a single type is through a debug command that was added to debugfs. This command is useless in practice, and results in causing bugs if we keep counter tracking but lose the is_vf and is_netdev protections as desired above. Since the only time we'd actually have a counter value besides 0 and 1 is through use of this debugfs hook, we can remove this unnecessary command, and the entire counter logic it required. We vastly simplify mac filters by removing (a) the distinction between VF and netdev filters (b) counting logic (c) the ability to add and remove filters bypassing the stack via debugfs Change-ID: Idf916dd2a1159b1188ddbab5bef6b85ea6bf27d9 Signed-off-by: Jacob Keller Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 19 +- .../net/ethernet/intel/i40e/i40e_debugfs.c | 88 +------ drivers/net/ethernet/intel/i40e/i40e_fcoe.c | 8 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 227 ++++++------------ .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 19 +- 5 files changed, 89 insertions(+), 272 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 5a6f8518b4e1f..844559188c6b7 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -458,9 +458,6 @@ struct i40e_mac_filter { u8 macaddr[ETH_ALEN]; #define I40E_VLAN_ANY -1 s16 vlan; - u8 counter; /* number of instances of this filter */ - bool is_vf; /* filter belongs to a VF */ - bool is_netdev; /* filter belongs to a netdev */ enum i40e_filter_state state; }; @@ -723,10 +720,8 @@ u32 i40e_get_global_fd_count(struct i40e_pf *pf); bool i40e_set_ntuple(struct i40e_pf *pf, netdev_features_t features); void i40e_set_ethtool_ops(struct net_device *netdev); struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan, - bool is_vf, bool is_netdev); -void i40e_del_filter(struct i40e_vsi *vsi, u8 *macaddr, s16 vlan, - bool is_vf, bool is_netdev); + u8 *macaddr, s16 vlan); +void i40e_del_filter(struct i40e_vsi *vsi, u8 *macaddr, s16 vlan); int i40e_sync_vsi_filters(struct i40e_vsi *vsi); struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type, u16 uplink, u32 param1); @@ -817,13 +812,11 @@ int i40e_vsi_open(struct i40e_vsi *vsi); void i40e_vlan_stripping_disable(struct i40e_vsi *vsi); int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid); int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid); -struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev); -int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev); +struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, + u8 *macaddr); +int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr); bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi); -struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev); +struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr); #ifdef I40E_FCOE int __i40e_setup_tc(struct net_device *netdev, u32 handle, __be16 proto, struct tc_to_netdev *tc); diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c index 0354632fe2f8d..8f6ed8d28412b 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c @@ -168,9 +168,9 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid) pf->hw.mac.port_addr); list_for_each_entry(f, &vsi->mac_filter_list, list) { dev_info(&pf->pdev->dev, - " mac_filter_list: %pM vid=%d, is_netdev=%d is_vf=%d counter=%d, state %s\n", - f->macaddr, f->vlan, f->is_netdev, f->is_vf, - f->counter, i40e_filter_state_string[f->state]); + " mac_filter_hash: %pM vid=%d, state %s\n", + f->macaddr, f->vlan, + i40e_filter_state_string[f->state]); } dev_info(&pf->pdev->dev, " active_filters %d, promisc_threshold %d, overflow promisc %s\n", vsi->active_filters, vsi->promisc_threshold, @@ -867,86 +867,6 @@ static ssize_t i40e_dbg_command_write(struct file *filp, dev_info(&pf->pdev->dev, "deleting relay %d\n", veb_seid); i40e_veb_release(pf->veb[i]); - - } else if (strncmp(cmd_buf, "add macaddr", 11) == 0) { - struct i40e_mac_filter *f; - int vlan = 0; - u8 ma[6]; - int ret; - - cnt = sscanf(&cmd_buf[11], - "%i %hhx:%hhx:%hhx:%hhx:%hhx:%hhx %i", - &vsi_seid, - &ma[0], &ma[1], &ma[2], &ma[3], &ma[4], &ma[5], - &vlan); - if (cnt == 7) { - vlan = 0; - } else if (cnt != 8) { - dev_info(&pf->pdev->dev, - "add macaddr: bad command string, cnt=%d\n", - cnt); - goto command_write_done; - } - - vsi = i40e_dbg_find_vsi(pf, vsi_seid); - if (!vsi) { - dev_info(&pf->pdev->dev, - "add macaddr: VSI %d not found\n", vsi_seid); - goto command_write_done; - } - - spin_lock_bh(&vsi->mac_filter_list_lock); - f = i40e_add_filter(vsi, ma, vlan, false, false); - spin_unlock_bh(&vsi->mac_filter_list_lock); - ret = i40e_sync_vsi_filters(vsi); - if (f && !ret) - dev_info(&pf->pdev->dev, - "add macaddr: %pM vlan=%d added to VSI %d\n", - ma, vlan, vsi_seid); - else - dev_info(&pf->pdev->dev, - "add macaddr: %pM vlan=%d to VSI %d failed, f=%p ret=%d\n", - ma, vlan, vsi_seid, f, ret); - - } else if (strncmp(cmd_buf, "del macaddr", 11) == 0) { - int vlan = 0; - u8 ma[6]; - int ret; - - cnt = sscanf(&cmd_buf[11], - "%i %hhx:%hhx:%hhx:%hhx:%hhx:%hhx %i", - &vsi_seid, - &ma[0], &ma[1], &ma[2], &ma[3], &ma[4], &ma[5], - &vlan); - if (cnt == 7) { - vlan = 0; - } else if (cnt != 8) { - dev_info(&pf->pdev->dev, - "del macaddr: bad command string, cnt=%d\n", - cnt); - goto command_write_done; - } - - vsi = i40e_dbg_find_vsi(pf, vsi_seid); - if (!vsi) { - dev_info(&pf->pdev->dev, - "del macaddr: VSI %d not found\n", vsi_seid); - goto command_write_done; - } - - spin_lock_bh(&vsi->mac_filter_list_lock); - i40e_del_filter(vsi, ma, vlan, false, false); - spin_unlock_bh(&vsi->mac_filter_list_lock); - ret = i40e_sync_vsi_filters(vsi); - if (!ret) - dev_info(&pf->pdev->dev, - "del macaddr: %pM vlan=%d removed from VSI %d\n", - ma, vlan, vsi_seid); - else - dev_info(&pf->pdev->dev, - "del macaddr: %pM vlan=%d from VSI %d failed, ret=%d\n", - ma, vlan, vsi_seid, ret); - } else if (strncmp(cmd_buf, "add pvid", 8) == 0) { i40e_status ret; u16 vid; @@ -1615,8 +1535,6 @@ static ssize_t i40e_dbg_command_write(struct file *filp, dev_info(&pf->pdev->dev, " del vsi [vsi_seid]\n"); dev_info(&pf->pdev->dev, " add relay \n"); dev_info(&pf->pdev->dev, " del relay \n"); - dev_info(&pf->pdev->dev, " add macaddr [vlan]\n"); - dev_info(&pf->pdev->dev, " del macaddr [vlan]\n"); dev_info(&pf->pdev->dev, " add pvid \n"); dev_info(&pf->pdev->dev, " del pvid \n"); dev_info(&pf->pdev->dev, " dump switch\n"); diff --git a/drivers/net/ethernet/intel/i40e/i40e_fcoe.c b/drivers/net/ethernet/intel/i40e/i40e_fcoe.c index 58e6c1570335a..0c2a00d666adf 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_fcoe.c +++ b/drivers/net/ethernet/intel/i40e/i40e_fcoe.c @@ -1523,10 +1523,10 @@ void i40e_fcoe_config_netdev(struct net_device *netdev, struct i40e_vsi *vsi) */ netdev->dev_port = 1; spin_lock_bh(&vsi->mac_filter_list_lock); - i40e_add_filter(vsi, hw->mac.san_addr, 0, false, false); - i40e_add_filter(vsi, (u8[6]) FC_FCOE_FLOGI_MAC, 0, false, false); - i40e_add_filter(vsi, FIP_ALL_FCOE_MACS, 0, false, false); - i40e_add_filter(vsi, FIP_ALL_ENODE_MACS, 0, false, false); + i40e_add_filter(vsi, hw->mac.san_addr, 0); + i40e_add_filter(vsi, (u8[6]) FC_FCOE_FLOGI_MAC, 0); + i40e_add_filter(vsi, FIP_ALL_FCOE_MACS, 0); + i40e_add_filter(vsi, FIP_ALL_ENODE_MACS, 0); spin_unlock_bh(&vsi->mac_filter_list_lock); /* use san mac */ diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index d78a4dc7b00b6..ed49113d858fb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1145,14 +1145,11 @@ void i40e_update_stats(struct i40e_vsi *vsi) * @vsi: the VSI to be searched * @macaddr: the MAC address * @vlan: the vlan - * @is_vf: make sure its a VF filter, else doesn't matter - * @is_netdev: make sure its a netdev filter, else doesn't matter * * Returns ptr to the filter object or NULL **/ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan, - bool is_vf, bool is_netdev) + u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; @@ -1161,9 +1158,7 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, list_for_each_entry(f, &vsi->mac_filter_list, list) { if ((ether_addr_equal(macaddr, f->macaddr)) && - (vlan == f->vlan) && - (!is_vf || f->is_vf) && - (!is_netdev || f->is_netdev)) + (vlan == f->vlan)) return f; } return NULL; @@ -1173,14 +1168,11 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, * i40e_find_mac - Find a mac addr in the macvlan filters list * @vsi: the VSI to be searched * @macaddr: the MAC address we are searching for - * @is_vf: make sure its a VF filter, else doesn't matter - * @is_netdev: make sure its a netdev filter, else doesn't matter * * Returns the first filter with the provided MAC address or NULL if * MAC address was not found **/ -struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev) +struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr) { struct i40e_mac_filter *f; @@ -1188,9 +1180,7 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr, return NULL; list_for_each_entry(f, &vsi->mac_filter_list, list) { - if ((ether_addr_equal(macaddr, f->macaddr)) && - (!is_vf || f->is_vf) && - (!is_netdev || f->is_netdev)) + if ((ether_addr_equal(macaddr, f->macaddr))) return f; } return NULL; @@ -1221,26 +1211,21 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) * i40e_put_mac_in_vlan - Make macvlan filters from macaddrs and vlans * @vsi: the VSI to be searched * @macaddr: the mac address to be filtered - * @is_vf: true if it is a VF - * @is_netdev: true if it is a netdev * * Goes through all the macvlan filters and adds a * macvlan filter for each unique vlan that already exists * * Returns first filter found on success, else NULL **/ -struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev) +struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr) { struct i40e_mac_filter *f; list_for_each_entry(f, &vsi->mac_filter_list, list) { if (vsi->info.pvid) f->vlan = le16_to_cpu(vsi->info.pvid); - if (!i40e_find_filter(vsi, macaddr, f->vlan, - is_vf, is_netdev)) { - if (!i40e_add_filter(vsi, macaddr, f->vlan, - is_vf, is_netdev)) + if (!i40e_find_filter(vsi, macaddr, f->vlan)) { + if (!i40e_add_filter(vsi, macaddr, f->vlan)) return NULL; } } @@ -1253,15 +1238,12 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr, * i40e_del_mac_all_vlan - Remove a MAC filter from all VLANS * @vsi: the VSI to be searched * @macaddr: the mac address to be removed - * @is_vf: true if it is a VF - * @is_netdev: true if it is a netdev * * Removes a given MAC address from a VSI, regardless of VLAN * * Returns 0 for success, or error **/ -int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr, - bool is_vf, bool is_netdev) +int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr) { struct i40e_mac_filter *f = NULL; int changed = 0; @@ -1269,13 +1251,8 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr, WARN(!spin_is_locked(&vsi->mac_filter_list_lock), "Missing mac_filter_list_lock\n"); list_for_each_entry(f, &vsi->mac_filter_list, list) { - if ((ether_addr_equal(macaddr, f->macaddr)) && - (is_vf == f->is_vf) && - (is_netdev == f->is_netdev)) { - f->counter--; - changed = 1; - if (f->counter == 0) - f->state = I40E_FILTER_REMOVE; + if ((ether_addr_equal(macaddr, f->macaddr))) { + f->state = I40E_FILTER_REMOVE; } } if (changed) { @@ -1291,8 +1268,6 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr, * @vsi: the VSI to be searched * @macaddr: the MAC address * @vlan: the vlan - * @is_vf: make sure its a VF filter, else doesn't matter - * @is_netdev: make sure its a netdev filter, else doesn't matter * * Returns ptr to the filter object or NULL when no memory available. * @@ -1300,11 +1275,9 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr, * being held. **/ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan, - bool is_vf, bool is_netdev) + u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; - int changed = false; if (!vsi || !macaddr) return NULL; @@ -1316,11 +1289,11 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, if (is_broadcast_ether_addr(macaddr)) return NULL; - f = i40e_find_filter(vsi, macaddr, vlan, is_vf, is_netdev); + f = i40e_find_filter(vsi, macaddr, vlan); if (!f) { f = kzalloc(sizeof(*f), GFP_ATOMIC); if (!f) - goto add_filter_out; + return NULL; ether_addr_copy(f->macaddr, macaddr); f->vlan = vlan; @@ -1332,32 +1305,24 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, f->state = I40E_FILTER_FAILED; else f->state = I40E_FILTER_NEW; - changed = true; INIT_LIST_HEAD(&f->list); list_add_tail(&f->list, &vsi->mac_filter_list); - } - - /* increment counter and add a new flag if needed */ - if (is_vf) { - if (!f->is_vf) { - f->is_vf = true; - f->counter++; - } - } else if (is_netdev) { - if (!f->is_netdev) { - f->is_netdev = true; - f->counter++; - } - } else { - f->counter++; - } - if (changed) { vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; vsi->back->flags |= I40E_FLAG_FILTER_SYNC; } -add_filter_out: + /* If we're asked to add a filter that has been marked for removal, it + * is safe to simply restore it to active state. __i40e_del_filter + * will have simply deleted any filters which were previously marked + * NEW or FAILED, so if it is currently marked REMOVE it must have + * previously been ACTIVE. Since we haven't yet run the sync filters + * task, just restore this filter to the ACTIVE state so that the + * sync task leaves it in place + */ + if (f->state == I40E_FILTER_REMOVE) + f->state = I40E_FILTER_ACTIVE; + return f; } @@ -1366,8 +1331,6 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, * @vsi: the VSI to be searched * @macaddr: the MAC address * @vlan: the vlan - * @is_vf: make sure it's a VF filter, else doesn't matter - * @is_netdev: make sure it's a netdev filter, else doesn't matter * * NOTE: This function is expected to be called with mac_filter_list_lock * being held. @@ -1375,56 +1338,28 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() * instead of list_for_each_entry(). **/ -void i40e_del_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan, - bool is_vf, bool is_netdev) +void i40e_del_filter(struct i40e_vsi *vsi, u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; if (!vsi || !macaddr) return; - f = i40e_find_filter(vsi, macaddr, vlan, is_vf, is_netdev); - if (!f || f->counter == 0) + f = i40e_find_filter(vsi, macaddr, vlan); + if (!f) return; - if (is_vf) { - if (f->is_vf) { - f->is_vf = false; - f->counter--; - } - } else if (is_netdev) { - if (f->is_netdev) { - f->is_netdev = false; - f->counter--; - } + if ((f->state == I40E_FILTER_FAILED) || + (f->state == I40E_FILTER_NEW)) { + /* this one never got added by the FW. Just remove it, + * no need to sync anything. + */ + list_del(&f->list); + kfree(f); } else { - /* make sure we don't remove a filter in use by VF or netdev */ - int min_f = 0; - - min_f += (f->is_vf ? 1 : 0); - min_f += (f->is_netdev ? 1 : 0); - - if (f->counter > min_f) - f->counter--; - } - - /* counter == 0 tells sync_filters_subtask to - * remove the filter from the firmware's list - */ - if (f->counter == 0) { - if ((f->state == I40E_FILTER_FAILED) || - (f->state == I40E_FILTER_NEW)) { - /* this one never got added by the FW. Just remove it, - * no need to sync anything. - */ - list_del(&f->list); - kfree(f); - } else { - f->state = I40E_FILTER_REMOVE; - vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; - vsi->back->flags |= I40E_FLAG_FILTER_SYNC; - } + f->state = I40E_FILTER_REMOVE; + vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; + vsi->back->flags |= I40E_FLAG_FILTER_SYNC; } } @@ -1467,8 +1402,8 @@ static int i40e_set_mac(struct net_device *netdev, void *p) netdev_info(netdev, "set new mac address %pM\n", addr->sa_data); spin_lock_bh(&vsi->mac_filter_list_lock); - i40e_del_mac_all_vlan(vsi, netdev->dev_addr, false, true); - i40e_put_mac_in_vlan(vsi, addr->sa_data, false, true); + i40e_del_mac_all_vlan(vsi, netdev->dev_addr); + i40e_put_mac_in_vlan(vsi, addr->sa_data); spin_unlock_bh(&vsi->mac_filter_list_lock); ether_addr_copy(netdev->dev_addr, addr->sa_data); if (vsi->type == I40E_VSI_MAIN) { @@ -1653,33 +1588,26 @@ static void i40e_set_rx_mode(struct net_device *netdev) /* add addr if not already in the filter list */ netdev_for_each_uc_addr(uca, netdev) { - if (!i40e_find_mac(vsi, uca->addr, false, true)) { + if (!i40e_find_mac(vsi, uca->addr)) { if (i40e_is_vsi_in_vlan(vsi)) - i40e_put_mac_in_vlan(vsi, uca->addr, - false, true); + i40e_put_mac_in_vlan(vsi, uca->addr); else - i40e_add_filter(vsi, uca->addr, I40E_VLAN_ANY, - false, true); + i40e_add_filter(vsi, uca->addr, I40E_VLAN_ANY); } } netdev_for_each_mc_addr(mca, netdev) { - if (!i40e_find_mac(vsi, mca->addr, false, true)) { + if (!i40e_find_mac(vsi, mca->addr)) { if (i40e_is_vsi_in_vlan(vsi)) - i40e_put_mac_in_vlan(vsi, mca->addr, - false, true); + i40e_put_mac_in_vlan(vsi, mca->addr); else - i40e_add_filter(vsi, mca->addr, I40E_VLAN_ANY, - false, true); + i40e_add_filter(vsi, mca->addr, I40E_VLAN_ANY); } } /* remove filter if not in netdev list */ list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - if (!f->is_netdev) - continue; - netdev_for_each_mc_addr(mca, netdev) if (ether_addr_equal(mca->addr, f->macaddr)) goto bottom_of_search_loop; @@ -1693,7 +1621,7 @@ static void i40e_set_rx_mode(struct net_device *netdev) goto bottom_of_search_loop; /* f->macaddr wasn't found in uc, mc, or ha list so delete it */ - i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY, false, true); + i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY); bottom_of_search_loop: continue; @@ -1838,13 +1766,11 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* Create a list of filters to delete. */ list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { if (f->state == I40E_FILTER_REMOVE) { - WARN_ON(f->counter != 0); /* Move the element into temporary del_list */ list_move_tail(&f->list, &tmp_del_list); vsi->active_filters--; } if (f->state == I40E_FILTER_NEW) { - WARN_ON(f->counter == 0); /* Move the element into temporary add_list */ list_move_tail(&f->list, &tmp_add_list); } @@ -2323,17 +2249,12 @@ static void i40e_vlan_rx_register(struct net_device *netdev, u32 features) int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) { struct i40e_mac_filter *f, *ftmp, *add_f; - bool is_netdev, is_vf; - - is_vf = (vsi->type == I40E_VSI_SRIOV); - is_netdev = !!(vsi->netdev); /* Locked once because all functions invoked below iterates list*/ spin_lock_bh(&vsi->mac_filter_list_lock); - if (is_netdev) { - add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, vid, - is_vf, is_netdev); + if (vsi->netdev) { + add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, vid); if (!add_f) { dev_info(&vsi->back->pdev->dev, "Could not add vlan filter %d for %pM\n", @@ -2344,7 +2265,7 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) } list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - add_f = i40e_add_filter(vsi, f->macaddr, vid, is_vf, is_netdev); + add_f = i40e_add_filter(vsi, f->macaddr, vid); if (!add_f) { dev_info(&vsi->back->pdev->dev, "Could not add vlan filter %d for %pM\n", @@ -2360,13 +2281,11 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) * (and not all tags along with untagged) */ if (vid > 0) { - if (is_netdev && i40e_find_filter(vsi, vsi->netdev->dev_addr, - I40E_VLAN_ANY, - is_vf, is_netdev)) { + if (vsi->netdev && i40e_find_filter(vsi, vsi->netdev->dev_addr, + I40E_VLAN_ANY)) { i40e_del_filter(vsi, vsi->netdev->dev_addr, - I40E_VLAN_ANY, is_vf, is_netdev); - add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, 0, - is_vf, is_netdev); + I40E_VLAN_ANY); + add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, 0); if (!add_f) { dev_info(&vsi->back->pdev->dev, "Could not add filter 0 for %pM\n", @@ -2380,13 +2299,10 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) /* Do not assume that I40E_VLAN_ANY should be reset to VLAN 0 */ if (vid > 0 && !vsi->info.pvid) { list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - if (!i40e_find_filter(vsi, f->macaddr, I40E_VLAN_ANY, - is_vf, is_netdev)) + if (!i40e_find_filter(vsi, f->macaddr, I40E_VLAN_ANY)) continue; - i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY, - is_vf, is_netdev); - add_f = i40e_add_filter(vsi, f->macaddr, - 0, is_vf, is_netdev); + i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY); + add_f = i40e_add_filter(vsi, f->macaddr, 0); if (!add_f) { dev_info(&vsi->back->pdev->dev, "Could not add filter 0 for %pM\n", @@ -2417,20 +2333,16 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) { struct net_device *netdev = vsi->netdev; struct i40e_mac_filter *f, *ftmp, *add_f; - bool is_vf, is_netdev; int filter_count = 0; - is_vf = (vsi->type == I40E_VSI_SRIOV); - is_netdev = !!(netdev); - /* Locked once because all functions invoked below iterates list */ spin_lock_bh(&vsi->mac_filter_list_lock); - if (is_netdev) - i40e_del_filter(vsi, netdev->dev_addr, vid, is_vf, is_netdev); + if (vsi->netdev) + i40e_del_filter(vsi, netdev->dev_addr, vid); list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) - i40e_del_filter(vsi, f->macaddr, vid, is_vf, is_netdev); + i40e_del_filter(vsi, f->macaddr, vid); /* 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 @@ -2438,7 +2350,7 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) * on accept any traffic (with any tag present, or untagged) */ list_for_each_entry(f, &vsi->mac_filter_list, list) { - if (is_netdev) { + if (vsi->netdev) { if (f->vlan && ether_addr_equal(netdev->dev_addr, f->macaddr)) filter_count++; @@ -2448,10 +2360,9 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) filter_count++; } - if (!filter_count && is_netdev) { - i40e_del_filter(vsi, netdev->dev_addr, 0, is_vf, is_netdev); - f = i40e_add_filter(vsi, netdev->dev_addr, I40E_VLAN_ANY, - is_vf, is_netdev); + 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", @@ -2463,9 +2374,8 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) if (!filter_count) { list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - i40e_del_filter(vsi, f->macaddr, 0, is_vf, is_netdev); - add_f = i40e_add_filter(vsi, f->macaddr, I40E_VLAN_ANY, - is_vf, is_netdev); + i40e_del_filter(vsi, f->macaddr, 0); + 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", @@ -9155,7 +9065,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi) SET_NETDEV_DEV(netdev, &pf->pdev->dev); ether_addr_copy(mac_addr, hw->mac.perm_addr); spin_lock_bh(&vsi->mac_filter_list_lock); - i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY, false, true); + i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY); spin_unlock_bh(&vsi->mac_filter_list_lock); } else { /* relate the VSI_VMDQ name to the VSI_MAIN name */ @@ -9164,7 +9074,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi) random_ether_addr(mac_addr); spin_lock_bh(&vsi->mac_filter_list_lock); - i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY, false, false); + i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY); spin_unlock_bh(&vsi->mac_filter_list_lock); } @@ -9525,8 +9435,7 @@ int i40e_vsi_release(struct i40e_vsi *vsi) spin_lock_bh(&vsi->mac_filter_list_lock); list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) - i40e_del_filter(vsi, f->macaddr, f->vlan, - f->is_vf, f->is_netdev); + i40e_del_filter(vsi, f->macaddr, f->vlan); spin_unlock_bh(&vsi->mac_filter_list_lock); i40e_sync_vsi_filters(vsi); diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 64e54a562f411..37e6afcd01b0a 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -689,8 +689,8 @@ static int i40e_alloc_vsi_res(struct i40e_vf *vf, enum i40e_vsi_type type) spin_lock_bh(&vsi->mac_filter_list_lock); if (is_valid_ether_addr(vf->default_lan_addr.addr)) { f = i40e_add_filter(vsi, vf->default_lan_addr.addr, - vf->port_vlan_id ? vf->port_vlan_id : -1, - true, false); + vf->port_vlan_id ? + vf->port_vlan_id : -1); if (!f) dev_info(&pf->pdev->dev, "Could not add MAC filter %pM for VF %d\n", @@ -1933,14 +1933,12 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) for (i = 0; i < al->num_elements; i++) { struct i40e_mac_filter *f; - f = i40e_find_mac(vsi, al->list[i].addr, true, false); + f = i40e_find_mac(vsi, al->list[i].addr); if (!f) { if (i40e_is_vsi_in_vlan(vsi)) - f = i40e_put_mac_in_vlan(vsi, al->list[i].addr, - true, false); + f = i40e_put_mac_in_vlan(vsi, al->list[i].addr); else - f = i40e_add_filter(vsi, al->list[i].addr, -1, - true, false); + f = i40e_add_filter(vsi, al->list[i].addr, -1); } if (!f) { @@ -2006,7 +2004,7 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) spin_lock_bh(&vsi->mac_filter_list_lock); /* delete addresses from the list */ for (i = 0; i < al->num_elements; i++) - if (i40e_del_mac_all_vlan(vsi, al->list[i].addr, true, false)) { + if (i40e_del_mac_all_vlan(vsi, al->list[i].addr)) { ret = I40E_ERR_INVALID_MAC_ADDR; spin_unlock_bh(&vsi->mac_filter_list_lock); goto error_param; @@ -2722,14 +2720,13 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac) /* delete the temporary mac address */ if (!is_zero_ether_addr(vf->default_lan_addr.addr)) i40e_del_filter(vsi, vf->default_lan_addr.addr, - vf->port_vlan_id ? vf->port_vlan_id : -1, - true, false); + vf->port_vlan_id ? vf->port_vlan_id : -1); /* Delete all the filters for this VSI - we're going to kill it * anyway. */ list_for_each_entry(f, &vsi->mac_filter_list, list) - i40e_del_filter(vsi, f->macaddr, f->vlan, true, false); + i40e_del_filter(vsi, f->macaddr, f->vlan); spin_unlock_bh(&vsi->mac_filter_list_lock); From 6622f5cdbaf3786314d76969d2aab132b36ba2e8 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:32 -0700 Subject: [PATCH 03/22] i40e: make use of __dev_uc_sync and __dev_mc_sync The kernel provides __dev_uc_sync and __dev_mc_sync in order for drivers which need individual notification of add and delete for each filter. These functions allow us to vastly simplify our .set_rx_mode handler. We need to implement two functions for sync and unsync which add and remove filters respectively. This change avoids a very complex and inefficient algorithm which resulted in an abnormal latency for the .set_rx_mode NDO operation. The resulting code after this change is more readable, more efficient, and less code. Due to the callback signature used by these functions we also must update several other functions to take a const u8 * pointer. Change-Id: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 10 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 113 +++++++++++--------- 2 files changed, 69 insertions(+), 54 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 844559188c6b7..a9c02289049f1 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -720,8 +720,8 @@ u32 i40e_get_global_fd_count(struct i40e_pf *pf); bool i40e_set_ntuple(struct i40e_pf *pf, netdev_features_t features); void i40e_set_ethtool_ops(struct net_device *netdev); struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan); -void i40e_del_filter(struct i40e_vsi *vsi, u8 *macaddr, s16 vlan); + const u8 *macaddr, s16 vlan); +void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan); int i40e_sync_vsi_filters(struct i40e_vsi *vsi); struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type, u16 uplink, u32 param1); @@ -813,10 +813,10 @@ void i40e_vlan_stripping_disable(struct i40e_vsi *vsi); int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid); int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid); struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, - u8 *macaddr); -int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr); + const u8 *macaddr); +int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr); bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi); -struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr); +struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr); #ifdef I40E_FCOE int __i40e_setup_tc(struct net_device *netdev, u32 handle, __be16 proto, struct tc_to_netdev *tc); diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index ed49113d858fb..c63dc01537657 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1149,7 +1149,7 @@ void i40e_update_stats(struct i40e_vsi *vsi) * Returns ptr to the filter object or NULL **/ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan) + const u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; @@ -1172,7 +1172,7 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, * Returns the first filter with the provided MAC address or NULL if * MAC address was not found **/ -struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr) +struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr) { struct i40e_mac_filter *f; @@ -1217,7 +1217,8 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) * * Returns first filter found on success, else NULL **/ -struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr) +struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, + const u8 *macaddr) { struct i40e_mac_filter *f; @@ -1243,7 +1244,7 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, u8 *macaddr) * * Returns 0 for success, or error **/ -int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr) +int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr) { struct i40e_mac_filter *f = NULL; int changed = 0; @@ -1275,7 +1276,7 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr) * being held. **/ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, - u8 *macaddr, s16 vlan) + const u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; @@ -1338,7 +1339,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() * instead of list_for_each_entry(). **/ -void i40e_del_filter(struct i40e_vsi *vsi, u8 *macaddr, s16 vlan) +void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; @@ -1567,6 +1568,52 @@ static void i40e_vsi_setup_queue_map(struct i40e_vsi *vsi, ctxt->info.valid_sections |= cpu_to_le16(sections); } +/** + * i40e_addr_sync - Callback for dev_(mc|uc)_sync to add address + * @netdev: the netdevice + * @addr: address to add + * + * Called by __dev_(mc|uc)_sync when an address needs to be added. We call + * __dev_(uc|mc)_sync from .set_rx_mode and guarantee to hold the hash lock. + */ +static int i40e_addr_sync(struct net_device *netdev, const u8 *addr) +{ + struct i40e_netdev_priv *np = netdev_priv(netdev); + struct i40e_vsi *vsi = np->vsi; + struct i40e_mac_filter *f; + + if (i40e_is_vsi_in_vlan(vsi)) + f = i40e_put_mac_in_vlan(vsi, addr); + else + f = i40e_add_filter(vsi, addr, I40E_VLAN_ANY); + + if (f) + return 0; + else + return -ENOMEM; +} + +/** + * i40e_addr_unsync - Callback for dev_(mc|uc)_sync to remove address + * @netdev: the netdevice + * @addr: address to add + * + * Called by __dev_(mc|uc)_sync when an address needs to be removed. We call + * __dev_(uc|mc)_sync from .set_rx_mode and guarantee to hold the hash lock. + */ +static int i40e_addr_unsync(struct net_device *netdev, const u8 *addr) +{ + struct i40e_netdev_priv *np = netdev_priv(netdev); + struct i40e_vsi *vsi = np->vsi; + + if (i40e_is_vsi_in_vlan(vsi)) + i40e_del_mac_all_vlan(vsi, addr); + else + i40e_del_filter(vsi, addr, I40E_VLAN_ANY); + + return 0; +} + /** * i40e_set_rx_mode - NDO callback to set the netdev filters * @netdev: network interface device structure @@ -1578,54 +1625,13 @@ static void i40e_set_rx_mode(struct net_device *netdev) #endif { struct i40e_netdev_priv *np = netdev_priv(netdev); - struct i40e_mac_filter *f, *ftmp; struct i40e_vsi *vsi = np->vsi; - struct netdev_hw_addr *uca; - struct netdev_hw_addr *mca; - struct netdev_hw_addr *ha; spin_lock_bh(&vsi->mac_filter_list_lock); - /* add addr if not already in the filter list */ - netdev_for_each_uc_addr(uca, netdev) { - if (!i40e_find_mac(vsi, uca->addr)) { - if (i40e_is_vsi_in_vlan(vsi)) - i40e_put_mac_in_vlan(vsi, uca->addr); - else - i40e_add_filter(vsi, uca->addr, I40E_VLAN_ANY); - } - } - - netdev_for_each_mc_addr(mca, netdev) { - if (!i40e_find_mac(vsi, mca->addr)) { - if (i40e_is_vsi_in_vlan(vsi)) - i40e_put_mac_in_vlan(vsi, mca->addr); - else - i40e_add_filter(vsi, mca->addr, I40E_VLAN_ANY); - } - } + __dev_uc_sync(netdev, i40e_addr_sync, i40e_addr_unsync); + __dev_mc_sync(netdev, i40e_addr_sync, i40e_addr_unsync); - /* remove filter if not in netdev list */ - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - - netdev_for_each_mc_addr(mca, netdev) - if (ether_addr_equal(mca->addr, f->macaddr)) - goto bottom_of_search_loop; - - netdev_for_each_uc_addr(uca, netdev) - if (ether_addr_equal(uca->addr, f->macaddr)) - goto bottom_of_search_loop; - - for_each_dev_addr(netdev, ha) - if (ether_addr_equal(ha->addr, f->macaddr)) - goto bottom_of_search_loop; - - /* f->macaddr wasn't found in uc, mc, or ha list so delete it */ - i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY); - -bottom_of_search_loop: - continue; - } spin_unlock_bh(&vsi->mac_filter_list_lock); /* check for other flag changes */ @@ -9434,8 +9440,17 @@ int i40e_vsi_release(struct i40e_vsi *vsi) } spin_lock_bh(&vsi->mac_filter_list_lock); + + /* clear the sync flag on all filters */ + if (vsi->netdev) { + __dev_uc_unsync(vsi->netdev, NULL); + __dev_mc_unsync(vsi->netdev, NULL); + } + + /* make sure any remaining filters are marked for deletion */ list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) i40e_del_filter(vsi, f->macaddr, f->vlan); + spin_unlock_bh(&vsi->mac_filter_list_lock); i40e_sync_vsi_filters(vsi); From 35ec2ff37c5a4897e75d408828adac737b17d2bf Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:33 -0700 Subject: [PATCH 04/22] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan A future patch will be modifying these functions and making a call to a static function which currently is defined after these functions. Move them in a separate patch to ease review and ensure the moved code is correct. Change-ID: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 113 ++++++++++---------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index c63dc01537657..f7fe020e5e2a3 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1207,63 +1207,6 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) return false; } -/** - * i40e_put_mac_in_vlan - Make macvlan filters from macaddrs and vlans - * @vsi: the VSI to be searched - * @macaddr: the mac address to be filtered - * - * Goes through all the macvlan filters and adds a - * macvlan filter for each unique vlan that already exists - * - * Returns first filter found on success, else NULL - **/ -struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, - const u8 *macaddr) -{ - struct i40e_mac_filter *f; - - list_for_each_entry(f, &vsi->mac_filter_list, list) { - if (vsi->info.pvid) - f->vlan = le16_to_cpu(vsi->info.pvid); - if (!i40e_find_filter(vsi, macaddr, f->vlan)) { - if (!i40e_add_filter(vsi, macaddr, f->vlan)) - return NULL; - } - } - - return list_first_entry_or_null(&vsi->mac_filter_list, - struct i40e_mac_filter, list); -} - -/** - * i40e_del_mac_all_vlan - Remove a MAC filter from all VLANS - * @vsi: the VSI to be searched - * @macaddr: the mac address to be removed - * - * Removes a given MAC address from a VSI, regardless of VLAN - * - * Returns 0 for success, or error - **/ -int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr) -{ - struct i40e_mac_filter *f = NULL; - int changed = 0; - - WARN(!spin_is_locked(&vsi->mac_filter_list_lock), - "Missing mac_filter_list_lock\n"); - list_for_each_entry(f, &vsi->mac_filter_list, list) { - if ((ether_addr_equal(macaddr, f->macaddr))) { - f->state = I40E_FILTER_REMOVE; - } - } - if (changed) { - vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; - vsi->back->flags |= I40E_FLAG_FILTER_SYNC; - return 0; - } - return -ENOENT; -} - /** * i40e_add_filter - Add a mac/vlan filter to the VSI * @vsi: the VSI to be searched @@ -1364,6 +1307,62 @@ void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) } } +/** + * i40e_put_mac_in_vlan - Make macvlan filters from macaddrs and vlans + * @vsi: the VSI to be searched + * @macaddr: the mac address to be filtered + * + * Goes through all the macvlan filters and adds a + * macvlan filter for each unique vlan that already exists + * + * Returns first filter found on success, else NULL + **/ +struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, + const u8 *macaddr) +{ + struct i40e_mac_filter *f; + + list_for_each_entry(f, &vsi->mac_filter_list, list) { + if (vsi->info.pvid) + f->vlan = le16_to_cpu(vsi->info.pvid); + if (!i40e_find_filter(vsi, macaddr, f->vlan)) { + if (!i40e_add_filter(vsi, macaddr, f->vlan)) + return NULL; + } + } + + return list_first_entry_or_null(&vsi->mac_filter_list, + struct i40e_mac_filter, list); +} + +/** + * i40e_del_mac_all_vlan - Remove a MAC filter from all VLANS + * @vsi: the VSI to be searched + * @macaddr: the mac address to be removed + * + * Removes a given MAC address from a VSI, regardless of VLAN + * + * Returns 0 for success, or error + **/ +int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr) +{ + struct i40e_mac_filter *f = NULL; + int changed = 0; + + WARN(!spin_is_locked(&vsi->mac_filter_list_lock), + "Missing mac_filter_list_lock\n"); + list_for_each_entry(f, &vsi->mac_filter_list, list) { + if (ether_addr_equal(macaddr, f->macaddr)) + f->state = I40E_FILTER_REMOVE; + } + if (changed) { + vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; + vsi->back->flags |= I40E_FLAG_FILTER_SYNC; + return 0; + } + return -ENOENT; +} + /** * i40e_set_mac - NDO callback to set mac address * @netdev: network interface device structure From 5feb3d7b0c0a66c8537ca8a79d8f80f54783e73f Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:34 -0700 Subject: [PATCH 05/22] i40e: refactor i40e_put_mac_in_vlan to avoid changing f->vlan When a PVID has been assigned to a VSI, the function i40e_put_mac_in_vlan arbitrarily modifies all filters to have the same VLAN. This is obviously incorrect because it could be modifying active filters without putting them into the NEW state. The correct method is to remove then re-add filters which is already done in the code where we assign the PVID. Fix this issue and a few other minor nits at the same time. First, when we have a PVID don't even bother looping and simply add the filter with the PVID immediately. In the case of the loop, we now can remove several checks. We also don't need to use i40e_find_filter first before calling i40e_add_filter, since i40e_add_filter implicitly does a lookup already. Finally, update the return semantics of this function so that on failure to add a filter it returns NULL, but on success, it returns the last filter added. Otherwise, we're just returning the last filter in the list. An alternative fix might be to return 0 or an error code, but this is pretty invasive to every call site. Change-ID: I2325dfd843aec76d89fb0d7cb0e7c4f290a34840 Signed-off-by: Jacob Keller Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index f7fe020e5e2a3..c09609339bd2e 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1312,27 +1312,28 @@ void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) * @vsi: the VSI to be searched * @macaddr: the mac address to be filtered * - * Goes through all the macvlan filters and adds a - * macvlan filter for each unique vlan that already exists + * Goes through all the macvlan filters and adds a macvlan filter for each + * unique vlan that already exists. If a PVID has been assigned, instead only + * add the macaddr to that VLAN. * - * Returns first filter found on success, else NULL + * Returns last filter added on success, else NULL **/ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, const u8 *macaddr) { - struct i40e_mac_filter *f; + struct i40e_mac_filter *f, *add = NULL; + + if (vsi->info.pvid) + return i40e_add_filter(vsi, macaddr, + le16_to_cpu(vsi->info.pvid)); list_for_each_entry(f, &vsi->mac_filter_list, list) { - if (vsi->info.pvid) - f->vlan = le16_to_cpu(vsi->info.pvid); - if (!i40e_find_filter(vsi, macaddr, f->vlan)) { - if (!i40e_add_filter(vsi, macaddr, f->vlan)) - return NULL; - } + add = i40e_add_filter(vsi, macaddr, f->vlan); + if (!add) + return NULL; } - return list_first_entry_or_null(&vsi->mac_filter_list, - struct i40e_mac_filter, list); + return add; } /** From 57b341d66684e380cae332eadcc210e177a9f289 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:35 -0700 Subject: [PATCH 06/22] i40e: When searching all MAC/VLAN filters, ignore removed filters When adding new MAC address filters, the driver determines if it should behave in VLAN mode (where all MAC addresses get assigned to every existing VLAN) or in non-VLAN mode where MAC addresses get assigned the VLAN_ANY identifier. Under some circumstances it is possible that a VLAN has been marked for removal (such that all filters of that VLAN are set to I40E_FILTER_REMOVE), and a subsequent call to i40e_put_mac_in_vlan may occur prior to the driver subtask that syncs filters to the hardware. In this case, we may add filters to the new removed VLAN, even though it should have been removed. This is most obvious when first adding a new VLAN. We will delete all filters which are in I40E_VLAN_ANY (-1) and then re-add them as in VLAN 0 (untagged). Then before we sync filters, we will add new MAC address filter, which will be added to every VLAN that exists. Unfortunately, this will include I40E_VLAN_ANY, so we will end up incorrectly adding filters to the -1 VLAN. This can be fixed by simply skipping all filters which are marked for removal. A similar check is not necessary in i40e_del_mac_all_vlan, since we are deleting, and any filter which we find already marked for removal would simply be deleted again, which doesn't cause any issues. Change-Id: I7962154013ce02fe950584690aeeb3ed853d0086 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index c09609339bd2e..86ad9537d44f4 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1328,6 +1328,8 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, le16_to_cpu(vsi->info.pvid)); list_for_each_entry(f, &vsi->mac_filter_list, list) { + if (f->state == I40E_FILTER_REMOVE) + continue; add = i40e_add_filter(vsi, macaddr, f->vlan); if (!add) return NULL; @@ -2271,6 +2273,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) } list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + if (f->state == I40E_FILTER_REMOVE) + continue; add_f = i40e_add_filter(vsi, f->macaddr, vid); if (!add_f) { dev_info(&vsi->back->pdev->dev, @@ -2305,6 +2309,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) /* Do not assume that I40E_VLAN_ANY should be reset to VLAN 0 */ if (vid > 0 && !vsi->info.pvid) { list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + if (f->state == I40E_FILTER_REMOVE) + continue; if (!i40e_find_filter(vsi, f->macaddr, I40E_VLAN_ANY)) continue; i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY); From 290d255719af42ae4baaf49165a69b9ad47f2049 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:36 -0700 Subject: [PATCH 07/22] i40e: implement __i40e_del_filter and use where applicable When inside a loop where we call i40e_del_filter we use an O(n^2) pattern where i40e_del_filter calls i40e_find_filter for us. We can avoid this O(n^2) logic by factoring a function, __i40e_del_filter() out from the i40e_del_filter code. This allows us to re-use the delete logic where appropriate without having to search for the filter twice. This new function benefits several functions including i40e_vsi_add_vlan, i40e_vsi_kill_vlan, i40e_del_mac_vlan_all, and i40e_vsi_release. Change-ID: I75fabe0f53bf73f56b80d342e5fdcfcc28f4d3eb Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 92 +++++++++++++-------- 1 file changed, 59 insertions(+), 33 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 86ad9537d44f4..25db8d80b6841 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1271,10 +1271,13 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, } /** - * i40e_del_filter - Remove a mac/vlan filter from the VSI - * @vsi: the VSI to be searched - * @macaddr: the MAC address - * @vlan: the vlan + * __i40e_del_filter - Remove a specific filter from the VSI + * @vsi: VSI to remove from + * @f: the filter to remove from the list + * + * This function should be called instead of i40e_del_filter only if you know + * the exact filter you will remove already, such as via i40e_find_filter or + * i40e_find_mac. * * NOTE: This function is expected to be called with mac_filter_list_lock * being held. @@ -1282,14 +1285,8 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() * instead of list_for_each_entry(). **/ -void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) +static void __i40e_del_filter(struct i40e_vsi *vsi, struct i40e_mac_filter *f) { - struct i40e_mac_filter *f; - - if (!vsi || !macaddr) - return; - - f = i40e_find_filter(vsi, macaddr, vlan); if (!f) return; @@ -1307,6 +1304,29 @@ void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) } } +/** + * i40e_del_filter - Remove a MAC/VLAN filter from the VSI + * @vsi: the VSI to be searched + * @macaddr: the MAC address + * @vlan: the VLAN + * + * NOTE: This function is expected to be called with mac_filter_list_lock + * being held. + * ANOTHER NOTE: This function MUST be called from within the context of + * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() + * instead of list_for_each_entry(). + **/ +void i40e_del_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) +{ + struct i40e_mac_filter *f; + + if (!vsi || !macaddr) + return; + + f = i40e_find_filter(vsi, macaddr, vlan); + __i40e_del_filter(vsi, f); +} + /** * i40e_put_mac_in_vlan - Make macvlan filters from macaddrs and vlans * @vsi: the VSI to be searched @@ -1349,21 +1369,22 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, **/ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr) { - struct i40e_mac_filter *f = NULL; - int changed = 0; + struct i40e_mac_filter *f, *ftmp; + bool found = false; WARN(!spin_is_locked(&vsi->mac_filter_list_lock), "Missing mac_filter_list_lock\n"); - list_for_each_entry(f, &vsi->mac_filter_list, list) { - if (ether_addr_equal(macaddr, f->macaddr)) - f->state = I40E_FILTER_REMOVE; + list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + if (ether_addr_equal(macaddr, f->macaddr)) { + __i40e_del_filter(vsi, f); + found = true; + } } - if (changed) { - vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; - vsi->back->flags |= I40E_FLAG_FILTER_SYNC; + + if (found) return 0; - } - return -ENOENT; + else + return -ENOENT; } /** @@ -2256,7 +2277,7 @@ static void i40e_vlan_rx_register(struct net_device *netdev, u32 features) **/ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) { - struct i40e_mac_filter *f, *ftmp, *add_f; + struct i40e_mac_filter *f, *ftmp, *add_f, *del_f; /* Locked once because all functions invoked below iterates list*/ spin_lock_bh(&vsi->mac_filter_list_lock); @@ -2290,11 +2311,11 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) * with 0, so we now accept untagged and specified tagged traffic * (and not all tags along with untagged) */ - if (vid > 0) { - if (vsi->netdev && i40e_find_filter(vsi, vsi->netdev->dev_addr, - I40E_VLAN_ANY)) { - i40e_del_filter(vsi, vsi->netdev->dev_addr, - I40E_VLAN_ANY); + if (vid > 0 && vsi->netdev) { + del_f = i40e_find_filter(vsi, vsi->netdev->dev_addr, + I40E_VLAN_ANY); + if (del_f) { + __i40e_del_filter(vsi, del_f); add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, 0); if (!add_f) { dev_info(&vsi->back->pdev->dev, @@ -2311,9 +2332,11 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { if (f->state == I40E_FILTER_REMOVE) continue; - if (!i40e_find_filter(vsi, f->macaddr, I40E_VLAN_ANY)) + del_f = i40e_find_filter(vsi, f->macaddr, + I40E_VLAN_ANY); + if (!del_f) continue; - i40e_del_filter(vsi, f->macaddr, I40E_VLAN_ANY); + __i40e_del_filter(vsi, del_f); add_f = i40e_add_filter(vsi, f->macaddr, 0); if (!add_f) { dev_info(&vsi->back->pdev->dev, @@ -2353,8 +2376,10 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) if (vsi->netdev) i40e_del_filter(vsi, netdev->dev_addr, vid); - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) - i40e_del_filter(vsi, f->macaddr, vid); + list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + if (f->vlan == 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 @@ -2386,7 +2411,8 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) if (!filter_count) { list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { - i40e_del_filter(vsi, f->macaddr, 0); + 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, @@ -9455,7 +9481,7 @@ int i40e_vsi_release(struct i40e_vsi *vsi) /* make sure any remaining filters are marked for deletion */ list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) - i40e_del_filter(vsi, f->macaddr, f->vlan); + __i40e_del_filter(vsi, f); spin_unlock_bh(&vsi->mac_filter_list_lock); From 278e7d0b9d6864a9749b9473a273892aa1528621 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:37 -0700 Subject: [PATCH 08/22] i40e: store MAC/VLAN filters in a hash with the MAC Address as key Replace the mac_filter_list with a static size hash table of 8bits. The primary advantage of this is a decrease in latency of operations related to searching for specific MAC filters, including .set_rx_mode. Using a linked list resulted in several locations which were O(n^2). Using a hash table should give us latency growth closer to O(n*log(n)). Change-ID: I5330bd04053b880e670210933e35830b95948ebb Signed-off-by: Jacob Keller Tested-by: Andrew Bowers --- drivers/net/ethernet/intel/i40e/i40e.h | 24 ++- .../net/ethernet/intel/i40e/i40e_debugfs.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_fcoe.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 198 +++++++++++------- .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 38 ++-- 5 files changed, 161 insertions(+), 107 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index a9c02289049f1..6545550f58913 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -445,6 +446,20 @@ struct i40e_pf { u16 phy_led_val; }; +/** + * i40e_mac_to_hkey - Convert a 6-byte MAC Address to a u64 hash key + * @macaddr: the MAC Address as the base key + * + * Simply copies the address and returns it as a u64 for hashing + **/ +static inline u64 i40e_addr_to_hkey(const u8 *macaddr) +{ + u64 key = 0; + + ether_addr_copy((u8 *)&key, macaddr); + return key; +} + enum i40e_filter_state { I40E_FILTER_INVALID = 0, /* Invalid state */ I40E_FILTER_NEW, /* New, not sent to FW yet */ @@ -454,7 +469,7 @@ enum i40e_filter_state { /* There is no 'removed' state; the filter struct is freed */ }; struct i40e_mac_filter { - struct list_head list; + struct hlist_node hlist; u8 macaddr[ETH_ALEN]; #define I40E_VLAN_ANY -1 s16 vlan; @@ -498,9 +513,10 @@ struct i40e_vsi { #define I40E_VSI_FLAG_VEB_OWNER BIT(1) unsigned long flags; - /* Per VSI lock to protect elements/list (MAC filter) */ - spinlock_t mac_filter_list_lock; - struct list_head mac_filter_list; + /* Per VSI lock to protect elements/hash (MAC filter) */ + spinlock_t mac_filter_hash_lock; + /* Fixed size hash table with 2^8 buckets for MAC filters */ + DECLARE_HASHTABLE(mac_filter_hash, 8); /* VSI stats */ struct rtnl_link_stats64 net_stats; diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c index 8f6ed8d28412b..b8a03a05c4e85 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c @@ -134,7 +134,7 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid) struct rtnl_link_stats64 *nstat; struct i40e_mac_filter *f; struct i40e_vsi *vsi; - int i; + int i, bkt; vsi = i40e_dbg_find_vsi(pf, seid); if (!vsi) { @@ -166,7 +166,7 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid) pf->hw.mac.addr, pf->hw.mac.san_addr, pf->hw.mac.port_addr); - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { dev_info(&pf->pdev->dev, " mac_filter_hash: %pM vid=%d, state %s\n", f->macaddr, f->vlan, diff --git a/drivers/net/ethernet/intel/i40e/i40e_fcoe.c b/drivers/net/ethernet/intel/i40e/i40e_fcoe.c index 0c2a00d666adf..b077ef8b00fa7 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_fcoe.c +++ b/drivers/net/ethernet/intel/i40e/i40e_fcoe.c @@ -1522,12 +1522,12 @@ void i40e_fcoe_config_netdev(struct net_device *netdev, struct i40e_vsi *vsi) * same PCI function. */ netdev->dev_port = 1; - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); i40e_add_filter(vsi, hw->mac.san_addr, 0); i40e_add_filter(vsi, (u8[6]) FC_FCOE_FLOGI_MAC, 0); i40e_add_filter(vsi, FIP_ALL_FCOE_MACS, 0); i40e_add_filter(vsi, FIP_ALL_ENODE_MACS, 0); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* use san mac */ ether_addr_copy(netdev->dev_addr, hw->mac.san_addr); diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 25db8d80b6841..ea4dc9f9c4163 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1152,11 +1152,13 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; + u64 key; if (!vsi || !macaddr) return NULL; - list_for_each_entry(f, &vsi->mac_filter_list, list) { + key = i40e_addr_to_hkey(macaddr); + hash_for_each_possible(vsi->mac_filter_hash, f, hlist, key) { if ((ether_addr_equal(macaddr, f->macaddr)) && (vlan == f->vlan)) return f; @@ -1175,11 +1177,13 @@ static struct i40e_mac_filter *i40e_find_filter(struct i40e_vsi *vsi, struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr) { struct i40e_mac_filter *f; + u64 key; if (!vsi || !macaddr) return NULL; - list_for_each_entry(f, &vsi->mac_filter_list, list) { + key = i40e_addr_to_hkey(macaddr); + hash_for_each_possible(vsi->mac_filter_hash, f, hlist, key) { if ((ether_addr_equal(macaddr, f->macaddr))) return f; } @@ -1195,11 +1199,13 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr) bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) { struct i40e_mac_filter *f; + struct hlist_node *h; + int bkt; /* Only -1 for all the filters denotes not in vlan mode * so we have to go through all the list in order to make sure */ - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->vlan >= 0 || vsi->info.pvid) return true; } @@ -1215,13 +1221,14 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) * * Returns ptr to the filter object or NULL when no memory available. * - * NOTE: This function is expected to be called with mac_filter_list_lock + * NOTE: This function is expected to be called with mac_filter_hash_lock * being held. **/ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, const u8 *macaddr, s16 vlan) { struct i40e_mac_filter *f; + u64 key; if (!vsi || !macaddr) return NULL; @@ -1249,8 +1256,10 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, f->state = I40E_FILTER_FAILED; else f->state = I40E_FILTER_NEW; - INIT_LIST_HEAD(&f->list); - list_add_tail(&f->list, &vsi->mac_filter_list); + INIT_HLIST_NODE(&f->hlist); + + key = i40e_addr_to_hkey(macaddr); + hash_add(vsi->mac_filter_hash, &f->hlist, key); vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; vsi->back->flags |= I40E_FLAG_FILTER_SYNC; @@ -1279,7 +1288,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, * the exact filter you will remove already, such as via i40e_find_filter or * i40e_find_mac. * - * NOTE: This function is expected to be called with mac_filter_list_lock + * NOTE: This function is expected to be called with mac_filter_hash_lock * being held. * ANOTHER NOTE: This function MUST be called from within the context of * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() @@ -1295,7 +1304,7 @@ static void __i40e_del_filter(struct i40e_vsi *vsi, struct i40e_mac_filter *f) /* this one never got added by the FW. Just remove it, * no need to sync anything. */ - list_del(&f->list); + hash_del(&f->hlist); kfree(f); } else { f->state = I40E_FILTER_REMOVE; @@ -1310,7 +1319,7 @@ static void __i40e_del_filter(struct i40e_vsi *vsi, struct i40e_mac_filter *f) * @macaddr: the MAC address * @vlan: the VLAN * - * NOTE: This function is expected to be called with mac_filter_list_lock + * NOTE: This function is expected to be called with mac_filter_hash_lock * being held. * ANOTHER NOTE: This function MUST be called from within the context of * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe() @@ -1342,12 +1351,14 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, const u8 *macaddr) { struct i40e_mac_filter *f, *add = NULL; + struct hlist_node *h; + int bkt; if (vsi->info.pvid) return i40e_add_filter(vsi, macaddr, le16_to_cpu(vsi->info.pvid)); - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->state == I40E_FILTER_REMOVE) continue; add = i40e_add_filter(vsi, macaddr, f->vlan); @@ -1369,12 +1380,14 @@ struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, **/ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr) { - struct i40e_mac_filter *f, *ftmp; + struct i40e_mac_filter *f; + struct hlist_node *h; bool found = false; + int bkt; - WARN(!spin_is_locked(&vsi->mac_filter_list_lock), - "Missing mac_filter_list_lock\n"); - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + WARN(!spin_is_locked(&vsi->mac_filter_hash_lock), + "Missing mac_filter_hash_lock\n"); + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (ether_addr_equal(macaddr, f->macaddr)) { __i40e_del_filter(vsi, f); found = true; @@ -1425,10 +1438,10 @@ static int i40e_set_mac(struct net_device *netdev, void *p) else netdev_info(netdev, "set new mac address %pM\n", addr->sa_data); - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); i40e_del_mac_all_vlan(vsi, netdev->dev_addr); i40e_put_mac_in_vlan(vsi, addr->sa_data); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); ether_addr_copy(netdev->dev_addr, addr->sa_data); if (vsi->type == I40E_VSI_MAIN) { i40e_status ret; @@ -1650,12 +1663,12 @@ static void i40e_set_rx_mode(struct net_device *netdev) struct i40e_netdev_priv *np = netdev_priv(netdev); struct i40e_vsi *vsi = np->vsi; - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); __dev_uc_sync(netdev, i40e_addr_sync, i40e_addr_unsync); __dev_mc_sync(netdev, i40e_addr_sync, i40e_addr_unsync); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* check for other flag changes */ if (vsi->current_netdev_flags != vsi->netdev->flags) { @@ -1678,13 +1691,17 @@ static void i40e_set_rx_mode(struct net_device *netdev) * MAC filter entries from list were slated to be removed from device. **/ static void i40e_undo_del_filter_entries(struct i40e_vsi *vsi, - struct list_head *from) + struct hlist_head *from) { - struct i40e_mac_filter *f, *ftmp; + struct i40e_mac_filter *f; + struct hlist_node *h; + + hlist_for_each_entry_safe(f, h, from, hlist) { + u64 key = i40e_addr_to_hkey(f->macaddr); - list_for_each_entry_safe(f, ftmp, from, list) { /* Move the element back into MAC filter list*/ - list_move_tail(&f->list, &vsi->mac_filter_list); + hlist_del(&f->hlist); + hash_add(vsi->mac_filter_hash, &f->hlist, key); } } @@ -1713,7 +1730,9 @@ i40e_update_filter_state(int count, /* Everything's good, mark all filters active. */ for (i = 0; i < count ; i++) { add_head->state = I40E_FILTER_ACTIVE; - add_head = list_next_entry(add_head, list); + add_head = hlist_entry(add_head->hlist.next, + typeof(struct i40e_mac_filter), + hlist); } } else if (aq_err == I40E_AQ_RC_ENOSPC) { /* Device ran out of filter space. Check the return value @@ -1727,14 +1746,18 @@ i40e_update_filter_state(int count, add_head->state = I40E_FILTER_ACTIVE; retval++; } - add_head = list_next_entry(add_head, list); + add_head = hlist_entry(add_head->hlist.next, + typeof(struct i40e_mac_filter), + hlist); } } else { /* Some other horrible thing happened, fail all filters */ retval = 0; for (i = 0; i < count ; i++) { add_head->state = I40E_FILTER_FAILED; - add_head = list_next_entry(add_head, list); + add_head = hlist_entry(add_head->hlist.next, + typeof(struct i40e_mac_filter), + hlist); } } return retval; @@ -1750,14 +1773,15 @@ i40e_update_filter_state(int count, **/ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) { - struct i40e_mac_filter *f, *ftmp, *add_head = NULL; - struct list_head tmp_add_list, tmp_del_list; + struct i40e_mac_filter *f, *add_head = NULL; + struct hlist_head tmp_add_list, tmp_del_list; struct i40e_hw *hw = &vsi->back->hw; bool promisc_changed = false; char vsi_name[16] = "PF"; int filter_list_len = 0; u32 changed_flags = 0; i40e_status aq_ret = 0; + struct hlist_node *h; int retval = 0; struct i40e_pf *pf; int num_add = 0; @@ -1766,6 +1790,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) u16 cmd_flags; int list_size; int fcnt; + int bkt; /* empty array typed pointers, kcalloc later */ struct i40e_aqc_add_macvlan_element_data *add_list; @@ -1780,8 +1805,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) vsi->current_netdev_flags = vsi->netdev->flags; } - INIT_LIST_HEAD(&tmp_add_list); - INIT_LIST_HEAD(&tmp_del_list); + INIT_HLIST_HEAD(&tmp_add_list); + INIT_HLIST_HEAD(&tmp_del_list); if (vsi->type == I40E_VSI_SRIOV) snprintf(vsi_name, sizeof(vsi_name) - 1, "VF %d", vsi->vf_id); @@ -1791,24 +1816,25 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) if (vsi->flags & I40E_VSI_FLAG_FILTER_CHANGED) { vsi->flags &= ~I40E_VSI_FLAG_FILTER_CHANGED; - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* Create a list of filters to delete. */ - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->state == I40E_FILTER_REMOVE) { /* Move the element into temporary del_list */ - list_move_tail(&f->list, &tmp_del_list); + hash_del(&f->hlist); + hlist_add_head(&f->hlist, &tmp_del_list); vsi->active_filters--; } if (f->state == I40E_FILTER_NEW) { - /* Move the element into temporary add_list */ - list_move_tail(&f->list, &tmp_add_list); + hash_del(&f->hlist); + hlist_add_head(&f->hlist, &tmp_add_list); } } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); } /* Now process 'del_list' outside the lock */ - if (!list_empty(&tmp_del_list)) { + if (!hlist_empty(&tmp_del_list)) { filter_list_len = hw->aq.asq_buf_size / sizeof(struct i40e_aqc_remove_macvlan_element_data); list_size = filter_list_len * @@ -1816,14 +1842,14 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) del_list = kzalloc(list_size, GFP_ATOMIC); if (!del_list) { /* Undo VSI's MAC filter entry element updates */ - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); i40e_undo_del_filter_entries(vsi, &tmp_del_list); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); retval = -ENOMEM; goto out; } - list_for_each_entry_safe(f, ftmp, &tmp_del_list, list) { + hlist_for_each_entry_safe(f, h, &tmp_del_list, hlist) { cmd_flags = 0; /* add to delete list */ @@ -1864,7 +1890,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* Release memory for MAC filter entries which were * synced up with HW. */ - list_del(&f->list); + hlist_del(&f->hlist); kfree(f); } @@ -1891,7 +1917,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) del_list = NULL; } - if (!list_empty(&tmp_add_list)) { + if (!hlist_empty(&tmp_add_list)) { /* Do all the adds now. */ filter_list_len = hw->aq.asq_buf_size / sizeof(struct i40e_aqc_add_macvlan_element_data); @@ -1903,7 +1929,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) goto out; } num_add = 0; - list_for_each_entry(f, &tmp_add_list, list) { + hlist_for_each_entry(f, &tmp_add_list, hlist) { if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state)) { f->state = I40E_FILTER_FAILED; @@ -1974,11 +2000,14 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* Now move all of the filters from the temp add list back to * the VSI's list. */ - spin_lock_bh(&vsi->mac_filter_list_lock); - list_for_each_entry_safe(f, ftmp, &tmp_add_list, list) { - list_move_tail(&f->list, &vsi->mac_filter_list); + spin_lock_bh(&vsi->mac_filter_hash_lock); + hlist_for_each_entry_safe(f, h, &tmp_add_list, hlist) { + u64 key = i40e_addr_to_hkey(f->macaddr); + + hlist_del(&f->hlist); + hash_add(vsi->mac_filter_hash, &f->hlist, key); } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); kfree(add_list); add_list = NULL; } @@ -1990,12 +2019,12 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* See if we have any failed filters. We can't drop out of * promiscuous until these have all been deleted. */ - spin_lock_bh(&vsi->mac_filter_list_lock); - list_for_each_entry(f, &vsi->mac_filter_list, list) { + spin_lock_bh(&vsi->mac_filter_hash_lock); + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { if (f->state == I40E_FILTER_FAILED) failed_count++; } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); if (!failed_count) { dev_info(&pf->pdev->dev, "filter logjam cleared on %s, leaving overflow promiscuous mode\n", @@ -2277,10 +2306,12 @@ static void i40e_vlan_rx_register(struct net_device *netdev, u32 features) **/ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) { - struct i40e_mac_filter *f, *ftmp, *add_f, *del_f; + struct i40e_mac_filter *f, *add_f, *del_f; + struct hlist_node *h; + int bkt; /* Locked once because all functions invoked below iterates list*/ - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); if (vsi->netdev) { add_f = i40e_add_filter(vsi, vsi->netdev->dev_addr, vid); @@ -2288,12 +2319,12 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) dev_info(&vsi->back->pdev->dev, "Could not add vlan filter %d for %pM\n", vid, vsi->netdev->dev_addr); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->state == I40E_FILTER_REMOVE) continue; add_f = i40e_add_filter(vsi, f->macaddr, vid); @@ -2301,7 +2332,7 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) dev_info(&vsi->back->pdev->dev, "Could not add vlan filter %d for %pM\n", vid, f->macaddr); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } @@ -2321,7 +2352,7 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) dev_info(&vsi->back->pdev->dev, "Could not add filter 0 for %pM\n", vsi->netdev->dev_addr); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } @@ -2329,7 +2360,7 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) /* Do not assume that I40E_VLAN_ANY should be reset to VLAN 0 */ if (vid > 0 && !vsi->info.pvid) { - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->state == I40E_FILTER_REMOVE) continue; del_f = i40e_find_filter(vsi, f->macaddr, @@ -2342,13 +2373,13 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) dev_info(&vsi->back->pdev->dev, "Could not add filter 0 for %pM\n", f->macaddr); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* schedule our worker thread which will take care of * applying the new filter changes @@ -2367,16 +2398,18 @@ 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, *ftmp, *add_f; + struct i40e_mac_filter *f, *add_f; + struct hlist_node *h; int filter_count = 0; + int bkt; /* Locked once because all functions invoked below iterates list */ - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); if (vsi->netdev) i40e_del_filter(vsi, netdev->dev_addr, vid); - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { if (f->vlan == vid) __i40e_del_filter(vsi, f); } @@ -2386,7 +2419,7 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) * be replaced with -1. This signifies that we should from now * on accept any traffic (with any tag present, or untagged) */ - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { if (vsi->netdev) { if (f->vlan && ether_addr_equal(netdev->dev_addr, f->macaddr)) @@ -2404,13 +2437,13 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) 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_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } if (!filter_count) { - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + 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); @@ -2418,13 +2451,13 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) 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_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); return -ENOMEM; } } } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* schedule our worker thread which will take care of * applying the new filter changes @@ -7302,7 +7335,7 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type) pf->rss_table_size : 64; vsi->netdev_registered = false; vsi->work_limit = I40E_DEFAULT_IRQ_WORK; - INIT_LIST_HEAD(&vsi->mac_filter_list); + hash_init(vsi->mac_filter_hash); vsi->irqs_ready = false; ret = i40e_set_num_rings_in_vsi(vsi); @@ -7317,7 +7350,7 @@ static int i40e_vsi_mem_alloc(struct i40e_pf *pf, enum i40e_vsi_type type) i40e_vsi_setup_irqhandler(vsi, i40e_msix_clean_rings); /* Initialize VSI lock */ - spin_lock_init(&vsi->mac_filter_list_lock); + spin_lock_init(&vsi->mac_filter_hash_lock); pf->vsi[vsi_idx] = vsi; ret = vsi_idx; goto unlock_pf; @@ -9102,18 +9135,18 @@ static int i40e_config_netdev(struct i40e_vsi *vsi) if (vsi->type == I40E_VSI_MAIN) { SET_NETDEV_DEV(netdev, &pf->pdev->dev); ether_addr_copy(mac_addr, hw->mac.perm_addr); - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); } else { /* relate the VSI_VMDQ name to the VSI_MAIN name */ snprintf(netdev->name, IFNAMSIZ, "%sv%%d", pf->vsi[pf->lan_vsi]->netdev->name); random_ether_addr(mac_addr); - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); } ether_addr_copy(netdev->dev_addr, mac_addr); @@ -9202,7 +9235,9 @@ static int i40e_add_vsi(struct i40e_vsi *vsi) struct i40e_pf *pf = vsi->back; struct i40e_hw *hw = &pf->hw; struct i40e_vsi_context ctxt; - struct i40e_mac_filter *f, *ftmp; + struct i40e_mac_filter *f; + struct hlist_node *h; + int bkt; u8 enabled_tc = 0x1; /* TC0 enabled */ int f_count = 0; @@ -9401,13 +9436,13 @@ static int i40e_add_vsi(struct i40e_vsi *vsi) vsi->active_filters = 0; clear_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state); - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* If macvlan filters already exist, force them to get loaded */ - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) { + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { f->state = I40E_FILTER_NEW; f_count++; } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); if (f_count) { vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; @@ -9437,11 +9472,12 @@ static int i40e_add_vsi(struct i40e_vsi *vsi) **/ int i40e_vsi_release(struct i40e_vsi *vsi) { - struct i40e_mac_filter *f, *ftmp; + struct i40e_mac_filter *f; + struct hlist_node *h; struct i40e_veb *veb = NULL; struct i40e_pf *pf; u16 uplink_seid; - int i, n; + int i, n, bkt; pf = vsi->back; @@ -9471,7 +9507,7 @@ int i40e_vsi_release(struct i40e_vsi *vsi) i40e_vsi_disable_irq(vsi); } - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* clear the sync flag on all filters */ if (vsi->netdev) { @@ -9480,10 +9516,10 @@ int i40e_vsi_release(struct i40e_vsi *vsi) } /* make sure any remaining filters are marked for deletion */ - list_for_each_entry_safe(f, ftmp, &vsi->mac_filter_list, list) + hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) __i40e_del_filter(vsi, f); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); i40e_sync_vsi_filters(vsi); diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 37e6afcd01b0a..2f1f5e3638577 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -686,7 +686,7 @@ static int i40e_alloc_vsi_res(struct i40e_vf *vf, enum i40e_vsi_type type) if (vf->port_vlan_id) i40e_vsi_add_pvid(vsi, vf->port_vlan_id); - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); if (is_valid_ether_addr(vf->default_lan_addr.addr)) { f = i40e_add_filter(vsi, vf->default_lan_addr.addr, vf->port_vlan_id ? @@ -696,7 +696,7 @@ static int i40e_alloc_vsi_res(struct i40e_vf *vf, enum i40e_vsi_type type) "Could not add MAC filter %pM for VF %d\n", vf->default_lan_addr.addr, vf->vf_id); } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); i40e_write_rx_ctl(&pf->hw, I40E_VFQF_HENA1(0, vf->vf_id), (u32)hena); i40e_write_rx_ctl(&pf->hw, I40E_VFQF_HENA1(1, vf->vf_id), @@ -1449,9 +1449,9 @@ static void i40e_vc_reset_vf_msg(struct i40e_vf *vf) static inline int i40e_getnum_vf_vsi_vlan_filters(struct i40e_vsi *vsi) { struct i40e_mac_filter *f; - int num_vlans = 0; + int num_vlans = 0, bkt; - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { if (f->vlan >= 0 && f->vlan <= I40E_MAX_VLANID) num_vlans++; } @@ -1481,6 +1481,7 @@ static int i40e_vc_config_promiscuous_mode_msg(struct i40e_vf *vf, struct i40e_vsi *vsi; bool alluni = false; int aq_err = 0; + int bkt; vsi = i40e_find_vsi_from_id(pf, info->vsi_id); if (!test_bit(I40E_VF_STAT_ACTIVE, &vf->vf_states) || @@ -1507,7 +1508,7 @@ static int i40e_vc_config_promiscuous_mode_msg(struct i40e_vf *vf, vf->port_vlan_id, NULL); } else if (i40e_getnum_vf_vsi_vlan_filters(vsi)) { - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { if (f->vlan < 0 || f->vlan > I40E_MAX_VLANID) continue; aq_ret = i40e_aq_set_vsi_mc_promisc_on_vlan(hw, @@ -1557,7 +1558,7 @@ static int i40e_vc_config_promiscuous_mode_msg(struct i40e_vf *vf, vf->port_vlan_id, NULL); } else if (i40e_getnum_vf_vsi_vlan_filters(vsi)) { - list_for_each_entry(f, &vsi->mac_filter_list, list) { + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) { aq_ret = 0; if (f->vlan >= 0 && f->vlan <= I40E_MAX_VLANID) { aq_ret = @@ -1927,7 +1928,7 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) /* Lock once, because all function inside for loop accesses VSI's * MAC filter list which needs to be protected using same lock. */ - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* add new addresses to the list */ for (i = 0; i < al->num_elements; i++) { @@ -1946,13 +1947,13 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) "Unable to add MAC filter %pM for VF %d\n", al->list[i].addr, vf->vf_id); ret = I40E_ERR_PARAM; - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); goto error_param; } else { vf->num_mac++; } } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* program the updated filter list */ ret = i40e_sync_vsi_filters(vsi); @@ -2001,18 +2002,18 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) } vsi = pf->vsi[vf->lan_vsi_idx]; - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* delete addresses from the list */ for (i = 0; i < al->num_elements; i++) if (i40e_del_mac_all_vlan(vsi, al->list[i].addr)) { ret = I40E_ERR_INVALID_MAC_ADDR; - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); goto error_param; } else { vf->num_mac--; } - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); /* program the updated filter list */ ret = i40e_sync_vsi_filters(vsi); @@ -2687,6 +2688,7 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac) struct i40e_mac_filter *f; struct i40e_vf *vf; int ret = 0; + int bkt; /* validate the request */ if (vf_id >= pf->num_alloc_vfs) { @@ -2713,9 +2715,9 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac) } /* Lock once because below invoked function add/del_filter requires - * mac_filter_list_lock to be held + * mac_filter_hash_lock to be held */ - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); /* delete the temporary mac address */ if (!is_zero_ether_addr(vf->default_lan_addr.addr)) @@ -2725,10 +2727,10 @@ int i40e_ndo_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac) /* Delete all the filters for this VSI - we're going to kill it * anyway. */ - list_for_each_entry(f, &vsi->mac_filter_list, list) + hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) i40e_del_filter(vsi, f->macaddr, f->vlan); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); dev_info(&pf->pdev->dev, "Setting MAC %pM on VF %d\n", mac, vf_id); /* program mac filter */ @@ -2800,9 +2802,9 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id, /* duplicate request, so just return success */ goto error_pvid; - spin_lock_bh(&vsi->mac_filter_list_lock); + spin_lock_bh(&vsi->mac_filter_hash_lock); is_vsi_in_vlan = i40e_is_vsi_in_vlan(vsi); - spin_unlock_bh(&vsi->mac_filter_list_lock); + spin_unlock_bh(&vsi->mac_filter_hash_lock); if (le16_to_cpu(vsi->info.pvid) == 0 && is_vsi_in_vlan) { dev_err(&pf->pdev->dev, From 4a2ce27bb5f291c38d8e49ef16899828289e4d3d Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:38 -0700 Subject: [PATCH 09/22] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters Currently, we fail to correctly restore filters on the temporary add list when we fail to allocate memory either for deletion or addition. Replace calls to "goto out;" with calls to a new location that correctly handles memory allocation failures. Note that it is safe for us to call i40e_undo_filter_entries on the tmp_del_list even after we've deleted filters because at this point it will be empty, so we don't need to separate the logic for add and delete failure. Change-Id: Iee107fd219c6e03e2fd9645c2debf8e8384a8521 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 39 ++++++++++++--------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index ea4dc9f9c4163..35f06ba6bc4e3 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1683,15 +1683,16 @@ static void i40e_set_rx_mode(struct net_device *netdev) } /** - * i40e_undo_del_filter_entries - Undo the changes made to MAC filter entries - * @vsi: pointer to vsi struct + * i40e_undo_filter_entries - Undo the changes made to MAC filter entries + * @vsi: Pointer to VSI struct * @from: Pointer to list which contains MAC filter entries - changes to * those entries needs to be undone. * - * MAC filter entries from list were slated to be removed from device. + * MAC filter entries from list were slated to be sent to firmware, either for + * addition or deletion. **/ -static void i40e_undo_del_filter_entries(struct i40e_vsi *vsi, - struct hlist_head *from) +static void i40e_undo_filter_entries(struct i40e_vsi *vsi, + struct hlist_head *from) { struct i40e_mac_filter *f; struct hlist_node *h; @@ -1840,14 +1841,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) list_size = filter_list_len * sizeof(struct i40e_aqc_remove_macvlan_element_data); del_list = kzalloc(list_size, GFP_ATOMIC); - if (!del_list) { - /* Undo VSI's MAC filter entry element updates */ - spin_lock_bh(&vsi->mac_filter_hash_lock); - i40e_undo_del_filter_entries(vsi, &tmp_del_list); - spin_unlock_bh(&vsi->mac_filter_hash_lock); - retval = -ENOMEM; - goto out; - } + if (!del_list) + goto err_no_memory; hlist_for_each_entry_safe(f, h, &tmp_del_list, hlist) { cmd_flags = 0; @@ -1924,10 +1919,9 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) list_size = filter_list_len * sizeof(struct i40e_aqc_add_macvlan_element_data); add_list = kzalloc(list_size, GFP_ATOMIC); - if (!add_list) { - retval = -ENOMEM; - goto out; - } + if (!add_list) + goto err_no_memory; + num_add = 0; hlist_for_each_entry(f, &tmp_add_list, hlist) { if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, @@ -2152,6 +2146,17 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) clear_bit(__I40E_CONFIG_BUSY, &vsi->state); return retval; + +err_no_memory: + /* Restore elements on the temporary add and delete lists */ + spin_lock_bh(&vsi->mac_filter_hash_lock); + i40e_undo_filter_entries(vsi, &tmp_del_list); + i40e_undo_filter_entries(vsi, &tmp_add_list); + spin_unlock_bh(&vsi->mac_filter_hash_lock); + + vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED; + clear_bit(__I40E_CONFIG_BUSY, &vsi->state); + return -ENOMEM; } /** From 84f5ca6cf4ea1d91fa9ee858ff95211127b0166b Mon Sep 17 00:00:00 2001 From: Alan Brady Date: Wed, 5 Oct 2016 09:30:39 -0700 Subject: [PATCH 10/22] 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 35f06ba6bc4e3..6cf9089531859 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 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) 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 From cbebb85f306f6023ebaa26ca66380571b4b680bf Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:40 -0700 Subject: [PATCH 11/22] i40e: avoid looping to check whether we're in VLAN mode We determine that a VSI is in vlan_mode whenever it has any filters with a VLAN other than -1 (I40E_VLAN_ALL). The previous method of doing so was to perform a loop whenever we needed the check. However, we can notice that only place where filters are added (i40e_add_filter) can change the condition from false to true, and the only place we can return to false is in i40e_vsi_sync_filters_subtask. Thus, we can remove the loop and use a boolean directly. Doing this avoids looping over filters repeatedly especially while we're already inside a loop over all the filters. This should reduce the latency of filter operations throughout the driver. Change-ID: Iafde08df588da2a2ea666997d05e11fad8edc338 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 1 + drivers/net/ethernet/intel/i40e/i40e_main.c | 48 ++++++++++++++++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 6545550f58913..60dbb5b90e1a8 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -517,6 +517,7 @@ struct i40e_vsi { spinlock_t mac_filter_hash_lock; /* Fixed size hash table with 2^8 buckets for MAC filters */ DECLARE_HASHTABLE(mac_filter_hash, 8); + bool has_vlan_filter; /* VSI stats */ struct rtnl_link_stats64 net_stats; diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 6cf9089531859..b4529b5a6af6c 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1198,19 +1198,31 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr) **/ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi) { - struct i40e_mac_filter *f; - struct hlist_node *h; - int bkt; + /* If we have a PVID, always operate in VLAN mode */ + if (vsi->info.pvid) + return true; - /* Only -1 for all the filters denotes not in vlan mode - * so we have to go through all the list in order to make sure + /* We need to operate in VLAN mode whenever we have any filters with + * a VLAN other than I40E_VLAN_ALL. We could check the table each + * time, incurring search cost repeatedly. However, we can notice two + * things: + * + * 1) the only place where we can gain a VLAN filter is in + * i40e_add_filter. + * + * 2) the only place where filters are actually removed is in + * i40e_vsi_sync_filters_subtask. + * + * Thus, we can simply use a boolean value, has_vlan_filters which we + * will set to true when we add a VLAN filter in i40e_add_filter. Then + * we have to perform the full search after deleting filters in + * i40e_vsi_sync_filters_subtask, but we already have to search + * filters here and can perform the check at the same time. This + * results in avoiding embedding a loop for VLAN mode inside another + * loop over all the filters, and should maintain correctness as noted + * above. */ - hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) { - if (f->vlan >= 0 || vsi->info.pvid) - return true; - } - - return false; + return vsi->has_vlan_filter; } /** @@ -1246,6 +1258,12 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, if (!f) return NULL; + /* Update the boolean indicating if we need to function in + * VLAN mode. + */ + if (vlan >= 0) + vsi->has_vlan_filter = true; + ether_addr_copy(f->macaddr, macaddr); f->vlan = vlan; /* If we're in overflow promisc mode, set the state directly @@ -1979,6 +1997,14 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) del_list = NULL; } + /* After finishing notifying firmware of the deleted filters, update + * the cached value of vsi->has_vlan_filter. Note that we are safe to + * use just !!vlan_filters here because if we only have VLAN=0 (that + * is, non_vlan_filters) these will all be converted to VLAN=-1 in the + * logic above already so this value would still be correct. + */ + vsi->has_vlan_filter = !!vlan_filters; + if (!hlist_empty(&tmp_add_list)) { /* Do all the adds now. */ filter_list_len = hw->aq.asq_buf_size / From 0093631966aad046ddf21e157a43765c295e16a3 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:41 -0700 Subject: [PATCH 12/22] i40e: remove duplicate add/delete adminq command code for filters We duplicate some code around adding and deleting filters using the adminq interface. This is prone to errors in case there are bugs. Use functions which extract the logic to their own portion so that we don't duplicate it twice in code. Change-ID: I60d68aeb887976787dec00b23ab386a106e61465 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 156 +++++++++++--------- 1 file changed, 84 insertions(+), 72 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index b4529b5a6af6c..5fe54e5c8efcf 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1782,6 +1782,80 @@ i40e_update_filter_state(int count, return retval; } +/** + * i40e_aqc_del_filters - Request firmware to delete a set of filters + * @vsi: ptr to the VSI + * @vsi_name: name to display in messages + * @list: the list of filters to send to firmware + * @num_del: the number of filters to delete + * @retval: Set to -EIO on failure to delete + * + * Send a request to firmware via AdminQ to delete a set of filters. Uses + * *retval instead of a return value so that success does not force ret_val to + * be set to 0. This ensures that a sequence of calls to this function + * preserve the previous value of *retval on successful delete. + */ +static +void i40e_aqc_del_filters(struct i40e_vsi *vsi, const char *vsi_name, + struct i40e_aqc_remove_macvlan_element_data *list, + int num_del, int *retval) +{ + struct i40e_hw *hw = &vsi->back->hw; + i40e_status aq_ret; + int aq_err; + + aq_ret = i40e_aq_remove_macvlan(hw, vsi->seid, list, num_del, NULL); + aq_err = hw->aq.asq_last_status; + + /* Explicitly ignore and do not report when firmware returns ENOENT */ + if (aq_ret && !(aq_err == I40E_AQ_RC_ENOENT)) { + *retval = -EIO; + dev_info(&vsi->back->pdev->dev, + "ignoring delete macvlan error on %s, err %s, aq_err %s\n", + vsi_name, i40e_stat_str(hw, aq_ret), + i40e_aq_str(hw, aq_err)); + } +} + +/** + * i40e_aqc_add_filters - Request firmware to add a set of filters + * @vsi: ptr to the VSI + * @vsi_name: name to display in messages + * @list: the list of filters to send to firmware + * @add_head: Position in the add hlist + * @num_add: the number of filters to add + * @promisc_change: set to true on exit if promiscuous mode was forced on + * + * Send a request to firmware via AdminQ to add a chunk of filters. Will set + * promisc_changed to true if the firmware has run out of space for more + * filters. + */ +static +void i40e_aqc_add_filters(struct i40e_vsi *vsi, const char *vsi_name, + struct i40e_aqc_add_macvlan_element_data *list, + struct i40e_mac_filter *add_head, + int num_add, bool *promisc_changed) +{ + struct i40e_hw *hw = &vsi->back->hw; + i40e_status aq_ret; + int aq_err, fcnt; + + aq_ret = i40e_aq_add_macvlan(hw, vsi->seid, list, num_add, NULL); + aq_err = hw->aq.asq_last_status; + fcnt = i40e_update_filter_state(num_add, list, add_head, aq_ret); + vsi->active_filters += fcnt; + + if (fcnt != num_add) { + *promisc_changed = true; + set_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state); + vsi->promisc_threshold = (vsi->active_filters * 3) / 4; + dev_warn(&vsi->back->pdev->dev, + "Error %s adding RX filters on %s, promiscuous mode forced on\n", + i40e_aq_str(hw, aq_err), + vsi_name); + } +} + /** * i40e_sync_vsi_filters - Update the VSI filter list to the HW * @vsi: ptr to the VSI @@ -1808,10 +1882,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) int num_add = 0; int num_del = 0; int retval = 0; - int aq_err = 0; u16 cmd_flags; int list_size; - int fcnt; int bkt; /* empty array typed pointers, kcalloc later */ @@ -1948,24 +2020,10 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* flush a full buffer */ if (num_del == filter_list_len) { - aq_ret = i40e_aq_remove_macvlan(hw, vsi->seid, - del_list, - num_del, NULL); - aq_err = hw->aq.asq_last_status; - num_del = 0; + i40e_aqc_del_filters(vsi, vsi_name, del_list, + num_del, &retval); memset(del_list, 0, list_size); - - /* Explicitly ignore and do not report when - * firmware returns ENOENT. - */ - if (aq_ret && !(aq_err == I40E_AQ_RC_ENOENT)) { - retval = -EIO; - dev_info(&pf->pdev->dev, - "ignoring delete macvlan error on %s, err %s, aq_err %s\n", - vsi_name, - i40e_stat_str(hw, aq_ret), - i40e_aq_str(hw, aq_err)); - } + num_del = 0; } /* Release memory for MAC filter entries which were * synced up with HW. @@ -1975,22 +2033,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) } if (num_del) { - aq_ret = i40e_aq_remove_macvlan(hw, vsi->seid, del_list, - num_del, NULL); - aq_err = hw->aq.asq_last_status; - num_del = 0; - - /* Explicitly ignore and do not report when firmware - * returns ENOENT. - */ - if (aq_ret && !(aq_err == I40E_AQ_RC_ENOENT)) { - retval = -EIO; - dev_info(&pf->pdev->dev, - "ignoring delete macvlan error on %s, err %s aq_err %s\n", - vsi_name, - i40e_stat_str(hw, aq_ret), - i40e_aq_str(hw, aq_err)); - } + i40e_aqc_del_filters(vsi, vsi_name, del_list, + num_del, &retval); } kfree(del_list); @@ -2041,48 +2085,16 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi) /* flush a full buffer */ if (num_add == filter_list_len) { - aq_ret = i40e_aq_add_macvlan(hw, vsi->seid, - add_list, num_add, - NULL); - aq_err = hw->aq.asq_last_status; - fcnt = i40e_update_filter_state(num_add, - add_list, - add_head, - aq_ret); - vsi->active_filters += fcnt; - - if (fcnt != num_add) { - promisc_changed = true; - set_bit(__I40E_FILTER_OVERFLOW_PROMISC, - &vsi->state); - vsi->promisc_threshold = - (vsi->active_filters * 3) / 4; - dev_warn(&pf->pdev->dev, - "Error %s adding RX filters on %s, promiscuous mode forced on\n", - i40e_aq_str(hw, aq_err), - vsi_name); - } + i40e_aqc_add_filters(vsi, vsi_name, add_list, + add_head, num_add, + &promisc_changed); memset(add_list, 0, list_size); num_add = 0; } } if (num_add) { - aq_ret = i40e_aq_add_macvlan(hw, vsi->seid, - add_list, num_add, NULL); - aq_err = hw->aq.asq_last_status; - fcnt = i40e_update_filter_state(num_add, add_list, - add_head, aq_ret); - vsi->active_filters += fcnt; - if (fcnt != num_add) { - promisc_changed = true; - set_bit(__I40E_FILTER_OVERFLOW_PROMISC, - &vsi->state); - vsi->promisc_threshold = - (vsi->active_filters * 3) / 4; - dev_warn(&pf->pdev->dev, - "Error %s adding RX filters on %s, promiscuous mode forced on\n", - i40e_aq_str(hw, aq_err), vsi_name); - } + i40e_aqc_add_filters(vsi, vsi_name, add_list, add_head, + num_add, &promisc_changed); } /* Now move all of the filters from the temp add list back to * the VSI's list. From 144ed1763003c6a88a4b788cc5da1d8a1ddf061f Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:42 -0700 Subject: [PATCH 13/22] i40e: correct check for reading TSYNINDX from the receive descriptor When hardware has taken a timestamp for a received packet, it indicates which RXTIME register the timestamp was placed in by some bits in the receive descriptor. It uses 3 bits, one to indicate if the descriptor index is valid (ie: there was a timestamp) and 2 bits to indicate which of the 4 registers to read. However, the driver currently does not check the TSYNVALID bit and only checks the index. It assumes a zero index means no timestamp, and a non zero index means a timestamp occurred. While this appears to be true, it prevents ever reading a timestamp in RXTIME[0], and causes the first timestamp the device captures to be ignored. Fix this by using the TSYNVALID bit correctly as the true indicator of whether the packet has an associated timestamp. Also rename the variable rsyn to tsyn as this is more descriptive and matches the register names. Change-ID: I4437e8f3a3df2c2ddb458b0fb61420f3dafc4c12 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index daade4fe80d69..c9eb6b852d662 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1410,11 +1410,12 @@ void i40e_process_skb_fields(struct i40e_ring *rx_ring, u64 qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len); u32 rx_status = (qword & I40E_RXD_QW1_STATUS_MASK) >> I40E_RXD_QW1_STATUS_SHIFT; - u32 rsyn = (rx_status & I40E_RXD_QW1_STATUS_TSYNINDX_MASK) >> + u32 tsynvalid = rx_status & I40E_RXD_QW1_STATUS_TSYNVALID_MASK; + u32 tsyn = (rx_status & I40E_RXD_QW1_STATUS_TSYNINDX_MASK) >> I40E_RXD_QW1_STATUS_TSYNINDX_SHIFT; - if (unlikely(rsyn)) { - i40e_ptp_rx_hwtstamp(rx_ring->vsi->back, skb, rsyn); + if (unlikely(tsynvalid)) { + i40e_ptp_rx_hwtstamp(rx_ring->vsi->back, skb, tsyn); rx_ring->last_rx_timestamp = jiffies; } From 195512629c956286265dcfeeea2028c42ee0c483 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:43 -0700 Subject: [PATCH 14/22] i40e: use a mutex instead of spinlock in PTP user entry points We need a locking mechanism to protect the hardware SYSTIME register which is split over 2 values, and has internal hardware latching. We can't allow multiple accesses at the same time. However.... The spinlock_t is overkill here, especially use of spin_lock_irqsave, since every PTP access will halt hardirqs. Notice that the only places which need the SYSTIME value are user context and are capable of sleeping. Thus, it is safe to use a mutex here instead of the spinlock. Change-ID: I971761a89b58c6aad953590162e85a327fbba232 Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 2 +- drivers/net/ethernet/intel/i40e/i40e_ptp.c | 20 +++++++------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 60dbb5b90e1a8..21f2bb321cb40 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -430,7 +430,7 @@ struct i40e_pf { struct sk_buff *ptp_tx_skb; struct hwtstamp_config tstamp_config; unsigned long last_rx_ptp_check; - spinlock_t tmreg_lock; /* Used to protect the device time registers. */ + struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */ u64 ptp_base_adj; u32 tx_hwtstamp_timeouts; u32 rx_hwtstamp_cleared; diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c index f1feceab758a5..177b7fb606af6 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c @@ -159,16 +159,15 @@ static int i40e_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) { struct i40e_pf *pf = container_of(ptp, struct i40e_pf, ptp_caps); struct timespec64 now, then; - unsigned long flags; then = ns_to_timespec64(delta); - spin_lock_irqsave(&pf->tmreg_lock, flags); + mutex_lock(&pf->tmreg_lock); i40e_ptp_read(pf, &now); now = timespec64_add(now, then); i40e_ptp_write(pf, (const struct timespec64 *)&now); - spin_unlock_irqrestore(&pf->tmreg_lock, flags); + mutex_unlock(&pf->tmreg_lock); return 0; } @@ -184,11 +183,10 @@ static int i40e_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) static int i40e_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts) { struct i40e_pf *pf = container_of(ptp, struct i40e_pf, ptp_caps); - unsigned long flags; - spin_lock_irqsave(&pf->tmreg_lock, flags); + mutex_lock(&pf->tmreg_lock); i40e_ptp_read(pf, ts); - spin_unlock_irqrestore(&pf->tmreg_lock, flags); + mutex_unlock(&pf->tmreg_lock); return 0; } @@ -205,11 +203,10 @@ static int i40e_ptp_settime(struct ptp_clock_info *ptp, const struct timespec64 *ts) { struct i40e_pf *pf = container_of(ptp, struct i40e_pf, ptp_caps); - unsigned long flags; - spin_lock_irqsave(&pf->tmreg_lock, flags); + mutex_lock(&pf->tmreg_lock); i40e_ptp_write(pf, ts); - spin_unlock_irqrestore(&pf->tmreg_lock, flags); + mutex_unlock(&pf->tmreg_lock); return 0; } @@ -658,10 +655,7 @@ void i40e_ptp_init(struct i40e_pf *pf) return; } - /* we have to initialize the lock first, since we can't control - * when the user will enter the PHC device entry points - */ - spin_lock_init(&pf->tmreg_lock); + mutex_init(&pf->tmreg_lock); /* ensure we have a clock device */ err = i40e_ptp_create_clock(pf); From 124905012db8bdeebf5a7d1ddc841eaadda84a75 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:44 -0700 Subject: [PATCH 15/22] i40e: replace PTP Rx timestamp hang logic The current Rx timestamp hang logic is not very robust because it does not notice a register is hung until all four timestamps have been latched and we wait a full 5 seconds. Replace this logic with a newer Rx hang detection based on storing the jiffies when we first notice a receive timestamp event. We store each register's time separately, along with a flag indicating if it is currently latched. Upon first transitioning to latch, we will update the latch_events[i] jiffies value. This indicates the time we first noticed this event. The watchdog routine will simply check that the either the flag has been cleared, or we have passed at least one second. In this case, it is able to clear the Rx timestamp register under the assumption that it was for a dropped frame. The benefit if this strategy is that we should be able to detect and clear out stalled RXTIME_H registers before we exhaust the supply of 4, and avoid complete stall of Rx timestamp events. Change-ID: Id55458c0cd7a5dd0c951ff2b8ac0b2509364131f Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 4 +- drivers/net/ethernet/intel/i40e/i40e_ptp.c | 117 +++++++++++++------- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 - 4 files changed, 83 insertions(+), 44 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 21f2bb321cb40..9121e46c2ad99 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -429,11 +429,13 @@ struct i40e_pf { struct ptp_clock_info ptp_caps; struct sk_buff *ptp_tx_skb; struct hwtstamp_config tstamp_config; - unsigned long last_rx_ptp_check; struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */ u64 ptp_base_adj; u32 tx_hwtstamp_timeouts; u32 rx_hwtstamp_cleared; + u32 latch_event_flags; + spinlock_t ptp_rx_lock; /* Used to protect Rx timestamp registers. */ + unsigned long latch_events[4]; bool ptp_tx; bool ptp_rx; u16 rss_table_size; /* HW RSS table size */ diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c index 177b7fb606af6..5e2272c9e717e 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c @@ -226,6 +226,47 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp, return -EOPNOTSUPP; } +/** + * i40e_ptp_update_latch_events - Read I40E_PRTTSYN_STAT_1 and latch events + * @pf: the PF data structure + * + * This function reads I40E_PRTTSYN_STAT_1 and updates the corresponding timers + * for noticed latch events. This allows the driver to keep track of the first + * time a latch event was noticed which will be used to help clear out Rx + * timestamps for packets that got dropped or lost. + * + * This function will return the current value of I40E_PRTTSYN_STAT_1 and is + * expected to be called only while under the ptp_rx_lock. + **/ +static u32 i40e_ptp_get_rx_events(struct i40e_pf *pf) +{ + struct i40e_hw *hw = &pf->hw; + u32 prttsyn_stat, new_latch_events; + int i; + + prttsyn_stat = rd32(hw, I40E_PRTTSYN_STAT_1); + new_latch_events = prttsyn_stat & ~pf->latch_event_flags; + + /* Update the jiffies time for any newly latched timestamp. This + * ensures that we store the time that we first discovered a timestamp + * was latched by the hardware. The service task will later determine + * if we should free the latch and drop that timestamp should too much + * time pass. This flow ensures that we only update jiffies for new + * events latched since the last time we checked, and not all events + * currently latched, so that the service task accounting remains + * accurate. + */ + for (i = 0; i < 4; i++) { + if (new_latch_events & BIT(i)) + pf->latch_events[i] = jiffies; + } + + /* Finally, we store the current status of the Rx timestamp latches */ + pf->latch_event_flags = prttsyn_stat; + + return prttsyn_stat; +} + /** * i40e_ptp_rx_hang - Detect error case when Rx timestamp registers are hung * @vsi: The VSI with the rings relevant to 1588 @@ -239,10 +280,7 @@ void i40e_ptp_rx_hang(struct i40e_vsi *vsi) { struct i40e_pf *pf = vsi->back; struct i40e_hw *hw = &pf->hw; - struct i40e_ring *rx_ring; - unsigned long rx_event; - u32 prttsyn_stat; - int n; + int i; /* Since we cannot turn off the Rx timestamp logic if the device is * configured for Tx timestamping, we check if Rx timestamping is @@ -252,42 +290,30 @@ void i40e_ptp_rx_hang(struct i40e_vsi *vsi) if (!(pf->flags & I40E_FLAG_PTP) || !pf->ptp_rx) return; - prttsyn_stat = rd32(hw, I40E_PRTTSYN_STAT_1); + spin_lock_bh(&pf->ptp_rx_lock); - /* Unless all four receive timestamp registers are latched, we are not - * concerned about a possible PTP Rx hang, so just update the timeout - * counter and exit. - */ - if (!(prttsyn_stat & ((I40E_PRTTSYN_STAT_1_RXT0_MASK << - I40E_PRTTSYN_STAT_1_RXT0_SHIFT) | - (I40E_PRTTSYN_STAT_1_RXT1_MASK << - I40E_PRTTSYN_STAT_1_RXT1_SHIFT) | - (I40E_PRTTSYN_STAT_1_RXT2_MASK << - I40E_PRTTSYN_STAT_1_RXT2_SHIFT) | - (I40E_PRTTSYN_STAT_1_RXT3_MASK << - I40E_PRTTSYN_STAT_1_RXT3_SHIFT)))) { - pf->last_rx_ptp_check = jiffies; - return; - } + /* Update current latch times for Rx events */ + i40e_ptp_get_rx_events(pf); - /* Determine the most recent watchdog or rx_timestamp event. */ - rx_event = pf->last_rx_ptp_check; - for (n = 0; n < vsi->num_queue_pairs; n++) { - rx_ring = vsi->rx_rings[n]; - if (time_after(rx_ring->last_rx_timestamp, rx_event)) - rx_event = rx_ring->last_rx_timestamp; + /* Check all the currently latched Rx events and see whether they have + * been latched for over a second. It is assumed that any timestamp + * should have been cleared within this time, or else it was captured + * for a dropped frame that the driver never received. Thus, we will + * clear any timestamp that has been latched for over 1 second. + */ + for (i = 0; i < 4; i++) { + if ((pf->latch_event_flags & BIT(i)) && + time_is_before_jiffies(pf->latch_events[i] + HZ)) { + rd32(hw, I40E_PRTTSYN_RXTIME_H(i)); + pf->latch_event_flags &= ~BIT(i); + pf->rx_hwtstamp_cleared++; + dev_warn(&pf->pdev->dev, + "Clearing a missed Rx timestamp event for RXTIME[%d]\n", + i); + } } - /* Only need to read the high RXSTMP register to clear the lock */ - if (time_is_before_jiffies(rx_event + 5 * HZ)) { - rd32(hw, I40E_PRTTSYN_RXTIME_H(0)); - rd32(hw, I40E_PRTTSYN_RXTIME_H(1)); - rd32(hw, I40E_PRTTSYN_RXTIME_H(2)); - rd32(hw, I40E_PRTTSYN_RXTIME_H(3)); - pf->last_rx_ptp_check = jiffies; - pf->rx_hwtstamp_cleared++; - WARN_ONCE(1, "Detected Rx timestamp register hang\n"); - } + spin_unlock_bh(&pf->ptp_rx_lock); } /** @@ -350,14 +376,25 @@ void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf, struct sk_buff *skb, u8 index) hw = &pf->hw; - prttsyn_stat = rd32(hw, I40E_PRTTSYN_STAT_1); + spin_lock_bh(&pf->ptp_rx_lock); + + /* Get current Rx events and update latch times */ + prttsyn_stat = i40e_ptp_get_rx_events(pf); - if (!(prttsyn_stat & BIT(index))) + /* TODO: Should we warn about missing Rx timestamp event? */ + if (!(prttsyn_stat & BIT(index))) { + spin_unlock_bh(&pf->ptp_rx_lock); return; + } + + /* Clear the latched event since we're about to read its register */ + pf->latch_event_flags &= ~BIT(index); lo = rd32(hw, I40E_PRTTSYN_RXTIME_L(index)); hi = rd32(hw, I40E_PRTTSYN_RXTIME_H(index)); + spin_unlock_bh(&pf->ptp_rx_lock); + ns = (((u64)hi) << 32) | lo; i40e_ptp_convert_to_hwtstamp(skb_hwtstamps(skb), ns); @@ -511,12 +548,15 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf, } /* Clear out all 1588-related registers to clear and unlatch them. */ + spin_lock_bh(&pf->ptp_rx_lock); rd32(hw, I40E_PRTTSYN_STAT_0); rd32(hw, I40E_PRTTSYN_TXTIME_H); rd32(hw, I40E_PRTTSYN_RXTIME_H(0)); rd32(hw, I40E_PRTTSYN_RXTIME_H(1)); rd32(hw, I40E_PRTTSYN_RXTIME_H(2)); rd32(hw, I40E_PRTTSYN_RXTIME_H(3)); + pf->latch_event_flags = 0; + spin_unlock_bh(&pf->ptp_rx_lock); /* Enable/disable the Tx timestamp interrupt based on user input. */ regval = rd32(hw, I40E_PRTTSYN_CTL0); @@ -656,6 +696,7 @@ void i40e_ptp_init(struct i40e_pf *pf) } mutex_init(&pf->tmreg_lock); + spin_lock_init(&pf->ptp_rx_lock); /* ensure we have a clock device */ err = i40e_ptp_create_clock(pf); diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index c9eb6b852d662..783ac4ed9c6db 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1414,10 +1414,8 @@ void i40e_process_skb_fields(struct i40e_ring *rx_ring, u32 tsyn = (rx_status & I40E_RXD_QW1_STATUS_TSYNINDX_MASK) >> I40E_RXD_QW1_STATUS_TSYNINDX_SHIFT; - if (unlikely(tsynvalid)) { + if (unlikely(tsynvalid)) i40e_ptp_rx_hwtstamp(rx_ring->vsi->back, skb, tsyn); - rx_ring->last_rx_timestamp = jiffies; - } i40e_rx_hash(rx_ring, rx_desc, skb, rx_ptype); diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h index 508840585645d..42f04d69c4d19 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h @@ -307,8 +307,6 @@ struct i40e_ring { u8 atr_sample_rate; u8 atr_count; - unsigned long last_rx_timestamp; - bool ring_active; /* is ring online or not */ bool arm_wb; /* do something to arm write back */ u8 packet_stride; From 7d3f04af69bed2db87d6a72d100e182f773e54b7 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 5 Oct 2016 09:30:45 -0700 Subject: [PATCH 16/22] i40evf: avoid an extra msleep while Remove the second call to msleep outside the loop, and move the msleep within the loop as the first step. This guarantees that a single loop will wait the minimum time first, and then after the reset finishes we no longer need an extra msleep. Change-ID: Ib2086f0a142402b614f67846bc091754203a0b9a Signed-off-by: Jacob Keller Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40evf/i40evf_main.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c index bcb1cafdf28a9..db36744c6691f 100644 --- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c +++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c @@ -1746,15 +1746,17 @@ static void i40evf_reset_task(struct work_struct *work) /* wait until the reset is complete and the PF is responding to us */ for (i = 0; i < I40EVF_RESET_WAIT_COUNT; i++) { + /* sleep first to make sure a minimum wait time is met */ + msleep(I40EVF_RESET_WAIT_MS); + reg_val = rd32(hw, I40E_VFGEN_RSTAT) & I40E_VFGEN_RSTAT_VFR_STATE_MASK; if (reg_val == I40E_VFR_VFACTIVE) break; - msleep(I40EVF_RESET_WAIT_MS); } + pci_set_master(adapter->pdev); - /* extra wait to make sure minimum wait is met */ - msleep(I40EVF_RESET_WAIT_MS); + if (i == I40EVF_RESET_WAIT_COUNT) { struct i40evf_mac_filter *ftmp; struct i40evf_vlan_filter *fv, *fvtmp; From 4b8164467b854fbeb7ecbb14cf53b6be9113ef03 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 11 Oct 2016 15:26:53 -0700 Subject: [PATCH 17/22] i40e: Add common function for finding VSI by type This patch adds a common method for finding a VSI by type. The main motivation for doing this is that the Flow Director path actually had two ways of handling this, one stopped on first match and one did not. This patch makes it so that all callers of this function will get the same approach for finding a VSI. Change-ID: Ibf25de8acd8466582520694424aa87da66965fbd Signed-off-by: Alexander Duyck Signed-off-by: Bimmy Pujari Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 19 +++++++++++++++++ .../net/ethernet/intel/i40e/i40e_ethtool.c | 12 +---------- drivers/net/ethernet/intel/i40e/i40e_main.c | 21 ++++++------------- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 5 +---- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 9121e46c2ad99..bb02280bc4c18 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -723,6 +723,25 @@ int i40e_get_rss(struct i40e_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size); void i40e_fill_rss_lut(struct i40e_pf *pf, u8 *lut, u16 rss_table_size, u16 rss_size); struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id); +/** + * i40e_find_vsi_by_type - Find and return Flow Director VSI + * @pf: PF to search for VSI + * @type: Value indicating type of VSI we are looking for + **/ +static inline struct i40e_vsi * +i40e_find_vsi_by_type(struct i40e_pf *pf, u16 type) +{ + int i; + + for (i = 0; i < pf->num_alloc_vsi; i++) { + struct i40e_vsi *vsi = pf->vsi[i]; + + if (vsi && vsi->type == type) + return vsi; + } + + return NULL; +} void i40e_update_stats(struct i40e_vsi *vsi); void i40e_update_eth_stats(struct i40e_vsi *vsi); struct rtnl_link_stats64 *i40e_get_vsi_stats_struct(struct i40e_vsi *vsi); diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c index fb4fb524eab22..962a9bb8a08ea 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c @@ -1768,17 +1768,7 @@ static inline bool i40e_active_vfs(struct i40e_pf *pf) static inline bool i40e_active_vmdqs(struct i40e_pf *pf) { - struct i40e_vsi **vsi = pf->vsi; - int i; - - for (i = 0; i < pf->num_alloc_vsi; i++) { - if (!vsi[i]) - continue; - if (vsi[i]->type == I40E_VSI_VMDQ2) - return true; - } - - return false; + return !!i40e_find_vsi_by_type(pf, I40E_VSI_VMDQ2); } static void i40e_diag_test(struct net_device *netdev, diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 5fe54e5c8efcf..9ec977a425de6 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -6718,7 +6718,6 @@ static int i40e_vsi_clear(struct i40e_vsi *vsi); static void i40e_fdir_sb_setup(struct i40e_pf *pf) { struct i40e_vsi *vsi; - int i; /* quick workaround for an NVM issue that leaves a critical register * uninitialized @@ -6729,6 +6728,7 @@ static void i40e_fdir_sb_setup(struct i40e_pf *pf) 0xeacb7d61, 0xaa4f05b6, 0x9c5c89ed, 0xfc425ddb, 0xa4654832, 0xfc7461d4, 0x8f827619, 0xf5c63c21, 0x95b3a76d}; + int i; for (i = 0; i <= I40E_GLQF_HKEY_MAX_INDEX; i++) wr32(&pf->hw, I40E_GLQF_HKEY(i), hkey[i]); @@ -6738,13 +6738,7 @@ static void i40e_fdir_sb_setup(struct i40e_pf *pf) return; /* find existing VSI and see if it needs configuring */ - vsi = NULL; - for (i = 0; i < pf->num_alloc_vsi; i++) { - if (pf->vsi[i] && pf->vsi[i]->type == I40E_VSI_FDIR) { - vsi = pf->vsi[i]; - break; - } - } + vsi = i40e_find_vsi_by_type(pf, I40E_VSI_FDIR); /* create a new VSI if none exists */ if (!vsi) { @@ -6766,15 +6760,12 @@ static void i40e_fdir_sb_setup(struct i40e_pf *pf) **/ static void i40e_fdir_teardown(struct i40e_pf *pf) { - int i; + struct i40e_vsi *vsi; i40e_fdir_filter_exit(pf); - for (i = 0; i < pf->num_alloc_vsi; i++) { - if (pf->vsi[i] && pf->vsi[i]->type == I40E_VSI_FDIR) { - i40e_vsi_release(pf->vsi[i]); - break; - } - } + vsi = i40e_find_vsi_by_type(pf, I40E_VSI_FDIR); + if (vsi) + i40e_vsi_release(vsi); } /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 783ac4ed9c6db..75b8f5b554e16 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -125,10 +125,7 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data, u16 i; /* find existing FDIR VSI */ - vsi = NULL; - for (i = 0; i < pf->num_alloc_vsi; i++) - if (pf->vsi[i] && pf->vsi[i]->type == I40E_VSI_FDIR) - vsi = pf->vsi[i]; + vsi = i40e_find_vsi_by_type(pf, I40E_VSI_FDIR); if (!vsi) return -ENOENT; From 1dc8b538795fcf92161cc056ceebb852cf375b17 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 11 Oct 2016 15:26:54 -0700 Subject: [PATCH 18/22] i40e: Reorder logic for coalescing RS bits This patch reorders the logic at the end of i40e_tx_map to address the fact that the logic was rather convoluted and much larger than it needed to be. In order to try and coalesce the code paths I have updated some of the comments and repurposed some of the variables in order to reduce unnecessary overhead. This patch does the following: 1. Quit tracking skb->xmit_more with a flag, just max out packet_stride 2. Drop tail_bump and do_rs and instead just use desc_count and td_cmd 3. Pull comments from ixgbe that make need for wmb() more explicit. Change-ID: Ic7da85ec75043c634e87fef958109789bcc6317c Signed-off-by: Alexander Duyck Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 105 +++++++++--------- drivers/net/ethernet/intel/i40e/i40e_txrx.h | 1 - drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 105 +++++++++--------- drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 1 - 4 files changed, 108 insertions(+), 104 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 75b8f5b554e16..5544b509832fe 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -616,7 +616,7 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw) return 0; } -#define WB_STRIDE 0x3 +#define WB_STRIDE 4 /** * i40e_clean_tx_irq - Reclaim resources after transmit completes @@ -732,7 +732,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi, unsigned int j = i40e_get_tx_pending(tx_ring, false); if (budget && - ((j / (WB_STRIDE + 1)) == 0) && (j != 0) && + ((j / WB_STRIDE) == 0) && (j > 0) && !test_bit(__I40E_DOWN, &vsi->state) && (I40E_DESC_UNUSED(tx_ring) != tx_ring->count)) tx_ring->arm_wb = true; @@ -2700,9 +2700,7 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, u32 td_tag = 0; dma_addr_t dma; u16 gso_segs; - u16 desc_count = 0; - bool tail_bump = true; - bool do_rs = false; + u16 desc_count = 1; if (tx_flags & I40E_TX_FLAGS_HW_VLAN) { td_cmd |= I40E_TX_DESC_CMD_IL2TAG1; @@ -2785,8 +2783,7 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, tx_bi = &tx_ring->tx_bi[i]; } - /* set next_to_watch value indicating a packet is present */ - first->next_to_watch = tx_desc; + netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount); i++; if (i == tx_ring->count) @@ -2794,66 +2791,72 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, tx_ring->next_to_use = i; - netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount); i40e_maybe_stop_tx(tx_ring, DESC_NEEDED); + /* write last descriptor with EOP bit */ + td_cmd |= I40E_TX_DESC_CMD_EOP; + + /* We can OR these values together as they both are checked against + * 4 below and at this point desc_count will be used as a boolean value + * after this if/else block. + */ + desc_count |= ++tx_ring->packet_stride; + /* Algorithm to optimize tail and RS bit setting: - * if xmit_more is supported - * if xmit_more is true - * do not update tail and do not mark RS bit. - * if xmit_more is false and last xmit_more was false - * if every packet spanned less than 4 desc - * then set RS bit on 4th packet and update tail - * on every packet - * else - * update tail and set RS bit on every packet. - * if xmit_more is false and last_xmit_more was true - * update tail and set RS bit. + * if queue is stopped + * mark RS bit + * reset packet counter + * else if xmit_more is supported and is true + * advance packet counter to 4 + * reset desc_count to 0 * - * Optimization: wmb to be issued only in case of tail update. - * Also optimize the Descriptor WB path for RS bit with the same - * algorithm. + * if desc_count >= 4 + * mark RS bit + * reset packet counter + * if desc_count > 0 + * update tail * - * Note: If there are less than 4 packets + * Note: If there are less than 4 descriptors * pending and interrupts were disabled the service task will * trigger a force WB. */ - if (skb->xmit_more && - !netif_xmit_stopped(txring_txq(tx_ring))) { - tx_ring->flags |= I40E_TXR_FLAGS_LAST_XMIT_MORE_SET; - tail_bump = false; - } else if (!skb->xmit_more && - !netif_xmit_stopped(txring_txq(tx_ring)) && - (!(tx_ring->flags & I40E_TXR_FLAGS_LAST_XMIT_MORE_SET)) && - (tx_ring->packet_stride < WB_STRIDE) && - (desc_count < WB_STRIDE)) { - tx_ring->packet_stride++; - } else { + if (netif_xmit_stopped(txring_txq(tx_ring))) { + goto do_rs; + } else if (skb->xmit_more) { + /* set stride to arm on next packet and reset desc_count */ + tx_ring->packet_stride = WB_STRIDE; + desc_count = 0; + } else if (desc_count >= WB_STRIDE) { +do_rs: + /* write last descriptor with RS bit set */ + td_cmd |= I40E_TX_DESC_CMD_RS; tx_ring->packet_stride = 0; - tx_ring->flags &= ~I40E_TXR_FLAGS_LAST_XMIT_MORE_SET; - do_rs = true; } - if (do_rs) - tx_ring->packet_stride = 0; tx_desc->cmd_type_offset_bsz = - build_ctob(td_cmd, td_offset, size, td_tag) | - cpu_to_le64((u64)(do_rs ? I40E_TXD_CMD : - I40E_TX_DESC_CMD_EOP) << - I40E_TXD_QW1_CMD_SHIFT); + build_ctob(td_cmd, td_offset, size, td_tag); + + /* Force memory writes to complete before letting h/w know there + * are new descriptors to fetch. + * + * We also use this memory barrier to make certain all of the + * status bits have been updated before next_to_watch is written. + */ + wmb(); + + /* set next_to_watch value indicating a packet is present */ + first->next_to_watch = tx_desc; /* notify HW of packet */ - if (!tail_bump) { - prefetchw(tx_desc + 1); - } else { - /* Force memory writes to complete before letting h/w - * know there are new descriptors to fetch. (Only - * applicable for weak-ordered memory model archs, - * such as IA-64). - */ - wmb(); + if (desc_count) { writel(i, tx_ring->tail); + + /* we need this if more than one processor can write to our tail + * at a time, it synchronizes IO on IA64/Altix systems + */ + mmiowb(); } + return; dma_error: diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h index 42f04d69c4d19..de8550f4e3a4d 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h @@ -313,7 +313,6 @@ struct i40e_ring { u16 flags; #define I40E_TXR_FLAGS_WB_ON_ITR BIT(0) -#define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2) /* stats structs */ struct i40e_queue_stats stats; diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c index e2d362238fd77..c4b174afd253d 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c @@ -150,7 +150,7 @@ u32 i40evf_get_tx_pending(struct i40e_ring *ring, bool in_sw) return 0; } -#define WB_STRIDE 0x3 +#define WB_STRIDE 4 /** * i40e_clean_tx_irq - Reclaim resources after transmit completes @@ -266,7 +266,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi, unsigned int j = i40evf_get_tx_pending(tx_ring, false); if (budget && - ((j / (WB_STRIDE + 1)) == 0) && (j > 0) && + ((j / WB_STRIDE) == 0) && (j > 0) && !test_bit(__I40E_DOWN, &vsi->state) && (I40E_DESC_UNUSED(tx_ring) != tx_ring->count)) tx_ring->arm_wb = true; @@ -1950,9 +1950,7 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, u32 td_tag = 0; dma_addr_t dma; u16 gso_segs; - u16 desc_count = 0; - bool tail_bump = true; - bool do_rs = false; + u16 desc_count = 1; if (tx_flags & I40E_TX_FLAGS_HW_VLAN) { td_cmd |= I40E_TX_DESC_CMD_IL2TAG1; @@ -2035,8 +2033,7 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, tx_bi = &tx_ring->tx_bi[i]; } - /* set next_to_watch value indicating a packet is present */ - first->next_to_watch = tx_desc; + netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount); i++; if (i == tx_ring->count) @@ -2044,66 +2041,72 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb, tx_ring->next_to_use = i; - netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount); i40e_maybe_stop_tx(tx_ring, DESC_NEEDED); + /* write last descriptor with EOP bit */ + td_cmd |= I40E_TX_DESC_CMD_EOP; + + /* We can OR these values together as they both are checked against + * 4 below and at this point desc_count will be used as a boolean value + * after this if/else block. + */ + desc_count |= ++tx_ring->packet_stride; + /* Algorithm to optimize tail and RS bit setting: - * if xmit_more is supported - * if xmit_more is true - * do not update tail and do not mark RS bit. - * if xmit_more is false and last xmit_more was false - * if every packet spanned less than 4 desc - * then set RS bit on 4th packet and update tail - * on every packet - * else - * update tail and set RS bit on every packet. - * if xmit_more is false and last_xmit_more was true - * update tail and set RS bit. + * if queue is stopped + * mark RS bit + * reset packet counter + * else if xmit_more is supported and is true + * advance packet counter to 4 + * reset desc_count to 0 * - * Optimization: wmb to be issued only in case of tail update. - * Also optimize the Descriptor WB path for RS bit with the same - * algorithm. + * if desc_count >= 4 + * mark RS bit + * reset packet counter + * if desc_count > 0 + * update tail * - * Note: If there are less than 4 packets + * Note: If there are less than 4 descriptors * pending and interrupts were disabled the service task will * trigger a force WB. */ - if (skb->xmit_more && - !netif_xmit_stopped(txring_txq(tx_ring))) { - tx_ring->flags |= I40E_TXR_FLAGS_LAST_XMIT_MORE_SET; - tail_bump = false; - } else if (!skb->xmit_more && - !netif_xmit_stopped(txring_txq(tx_ring)) && - (!(tx_ring->flags & I40E_TXR_FLAGS_LAST_XMIT_MORE_SET)) && - (tx_ring->packet_stride < WB_STRIDE) && - (desc_count < WB_STRIDE)) { - tx_ring->packet_stride++; - } else { + if (netif_xmit_stopped(txring_txq(tx_ring))) { + goto do_rs; + } else if (skb->xmit_more) { + /* set stride to arm on next packet and reset desc_count */ + tx_ring->packet_stride = WB_STRIDE; + desc_count = 0; + } else if (desc_count >= WB_STRIDE) { +do_rs: + /* write last descriptor with RS bit set */ + td_cmd |= I40E_TX_DESC_CMD_RS; tx_ring->packet_stride = 0; - tx_ring->flags &= ~I40E_TXR_FLAGS_LAST_XMIT_MORE_SET; - do_rs = true; } - if (do_rs) - tx_ring->packet_stride = 0; tx_desc->cmd_type_offset_bsz = - build_ctob(td_cmd, td_offset, size, td_tag) | - cpu_to_le64((u64)(do_rs ? I40E_TXD_CMD : - I40E_TX_DESC_CMD_EOP) << - I40E_TXD_QW1_CMD_SHIFT); + build_ctob(td_cmd, td_offset, size, td_tag); + + /* Force memory writes to complete before letting h/w know there + * are new descriptors to fetch. + * + * We also use this memory barrier to make certain all of the + * status bits have been updated before next_to_watch is written. + */ + wmb(); + + /* set next_to_watch value indicating a packet is present */ + first->next_to_watch = tx_desc; /* notify HW of packet */ - if (!tail_bump) { - prefetchw(tx_desc + 1); - } else { - /* Force memory writes to complete before letting h/w - * know there are new descriptors to fetch. (Only - * applicable for weak-ordered memory model archs, - * such as IA-64). - */ - wmb(); + if (desc_count) { writel(i, tx_ring->tail); + + /* we need this if more than one processor can write to our tail + * at a time, it synchronizes IO on IA64/Altix systems + */ + mmiowb(); } + return; dma_error: diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h index abcdecabbc560..a586e19cfd1d7 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h @@ -309,7 +309,6 @@ struct i40e_ring { bool ring_active; /* is ring online or not */ bool arm_wb; /* do something to arm write back */ u8 packet_stride; -#define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2) u16 flags; #define I40E_TXR_FLAGS_WB_ON_ITR BIT(0) From 13fd3f9cc3def8b276c7913ae4acbfa2653cb198 Mon Sep 17 00:00:00 2001 From: Mitch Williams Date: Tue, 11 Oct 2016 15:26:55 -0700 Subject: [PATCH 19/22] i40e: clear mac filter count on reset When a VF is reset, it gets a new VSI, so all of its MAC filters go away. Correctly set the number of filters to 0 when freeing VF resources. This corrects a problem with failure to add filters when the VF driver is reloaded. Change-ID: I2acbecf734287b67473bb225293e14b5096acbef Signed-off-by: Mitch Williams Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 2f1f5e3638577..69e184b3f72fb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -811,6 +811,7 @@ static void i40e_free_vf_res(struct i40e_vf *vf) i40e_vsi_release(pf->vsi[vf->lan_vsi_idx]); vf->lan_vsi_idx = 0; vf->lan_vsi_id = 0; + vf->num_mac = 0; } msix_vf = pf->hw.func_caps.num_msix_vectors_vf; From 999b315d39906341fadcb65ee6aba7f5cf38b830 Mon Sep 17 00:00:00 2001 From: Michal Kosiarz Date: Tue, 11 Oct 2016 15:26:56 -0700 Subject: [PATCH 20/22] i40e: Fix for division by zero For some cases when reading from device are incorrect or image is incorrect, this part of code causes crash due to division by zero. Change-ID: I8961029a7a87b0a479995823ef8fcbf6471405e1 Signed-off-by: Michal Kosiarz Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_common.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c index a47594603d69c..98791ba572110 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_common.c +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c @@ -3313,8 +3313,10 @@ static void i40e_parse_discover_capabilities(struct i40e_hw *hw, void *buff, /* partition id is 1-based, and functions are evenly spread * across the ports as partitions */ - hw->partition_id = (hw->pf_id / hw->num_ports) + 1; - hw->num_partitions = num_functions / hw->num_ports; + if (hw->num_ports != 0) { + hw->partition_id = (hw->pf_id / hw->num_ports) + 1; + hw->num_partitions = num_functions / hw->num_ports; + } /* additional HW specific goodies that might * someday be HW version specific From 81fa7c97bebd6e1a52c4e059eeffe18df5b3f11f Mon Sep 17 00:00:00 2001 From: Maciej Sosin Date: Tue, 11 Oct 2016 15:26:57 -0700 Subject: [PATCH 21/22] i40e: Implementation of ERROR state for NVM update state machine This patch adds I40E_NVMUPD_STATE_ERROR state for NVM update. Without this patch driver has no possibility to return NVM image write failure.This state is being set when ARQ rises error. arq_last_status is also updated every time when ARQ event comes, not only on error cases. Change-ID: I67ce43ba22a240773c2821b436e96054db0b7c81 Signed-off-by: Maciej Sosin Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_adminq.c | 4 ++-- drivers/net/ethernet/intel/i40e/i40e_nvm.c | 16 ++++++++++++++++ drivers/net/ethernet/intel/i40e/i40e_type.h | 1 + drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 4 ++-- drivers/net/ethernet/intel/i40evf/i40e_type.h | 1 + 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c index 738b42a44f20b..56fb272989369 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c @@ -964,11 +964,11 @@ i40e_status i40e_clean_arq_element(struct i40e_hw *hw, desc = I40E_ADMINQ_DESC(hw->aq.arq, ntc); desc_idx = ntc; + hw->aq.arq_last_status = + (enum i40e_admin_queue_err)le16_to_cpu(desc->retval); flags = le16_to_cpu(desc->flags); if (flags & I40E_AQ_FLAG_ERR) { ret_code = I40E_ERR_ADMIN_QUEUE_ERROR; - hw->aq.arq_last_status = - (enum i40e_admin_queue_err)le16_to_cpu(desc->retval); i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: Event received with error 0x%X.\n", diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c index 954efe3118dbb..38ee18f111244 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c +++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c @@ -722,9 +722,20 @@ i40e_status i40e_nvmupd_command(struct i40e_hw *hw, *((u16 *)&bytes[2]) = hw->nvm_wait_opcode; } + /* Clear error status on read */ + if (hw->nvmupd_state == I40E_NVMUPD_STATE_ERROR) + hw->nvmupd_state = I40E_NVMUPD_STATE_INIT; + return 0; } + /* Clear status even it is not read and log */ + if (hw->nvmupd_state == I40E_NVMUPD_STATE_ERROR) { + i40e_debug(hw, I40E_DEBUG_NVM, + "Clearing I40E_NVMUPD_STATE_ERROR state without reading\n"); + hw->nvmupd_state = I40E_NVMUPD_STATE_INIT; + } + switch (hw->nvmupd_state) { case I40E_NVMUPD_STATE_INIT: status = i40e_nvmupd_state_init(hw, cmd, bytes, perrno); @@ -1074,6 +1085,11 @@ void i40e_nvmupd_check_wait_event(struct i40e_hw *hw, u16 opcode) } hw->nvm_wait_opcode = 0; + if (hw->aq.arq_last_status) { + hw->nvmupd_state = I40E_NVMUPD_STATE_ERROR; + return; + } + switch (hw->nvmupd_state) { case I40E_NVMUPD_STATE_INIT_WAIT: hw->nvmupd_state = I40E_NVMUPD_STATE_INIT; diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h index bd5f13bef83c7..d9a266041bf15 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_type.h +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h @@ -366,6 +366,7 @@ enum i40e_nvmupd_state { I40E_NVMUPD_STATE_WRITING, I40E_NVMUPD_STATE_INIT_WAIT, I40E_NVMUPD_STATE_WRITE_WAIT, + I40E_NVMUPD_STATE_ERROR }; /* nvm_access definition and its masks/shifts need to be accessible to diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c index 44f7ed7583dd1..96385156b8245 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c +++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c @@ -912,11 +912,11 @@ i40e_status i40evf_clean_arq_element(struct i40e_hw *hw, desc = I40E_ADMINQ_DESC(hw->aq.arq, ntc); desc_idx = ntc; + hw->aq.arq_last_status = + (enum i40e_admin_queue_err)le16_to_cpu(desc->retval); flags = le16_to_cpu(desc->flags); if (flags & I40E_AQ_FLAG_ERR) { ret_code = I40E_ERR_ADMIN_QUEUE_ERROR; - hw->aq.arq_last_status = - (enum i40e_admin_queue_err)le16_to_cpu(desc->retval); i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: Event received with error 0x%X.\n", diff --git a/drivers/net/ethernet/intel/i40evf/i40e_type.h b/drivers/net/ethernet/intel/i40evf/i40e_type.h index 97f96e0d9c4c4..ca7afe59c55f7 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_type.h +++ b/drivers/net/ethernet/intel/i40evf/i40e_type.h @@ -348,6 +348,7 @@ enum i40e_nvmupd_state { I40E_NVMUPD_STATE_WRITING, I40E_NVMUPD_STATE_INIT_WAIT, I40E_NVMUPD_STATE_WRITE_WAIT, + I40E_NVMUPD_STATE_ERROR }; /* nvm_access definition and its masks/shifts need to be accessible to From 3aa7b74dbeedfb32406fec70cfd76d797209e8c9 Mon Sep 17 00:00:00 2001 From: Filip Sadowski Date: Tue, 11 Oct 2016 15:26:58 -0700 Subject: [PATCH 22/22] i40e: removed unreachable code Removed some of unnecessary if statements and unreachable code found by static code analysis tool. The return value of i40e_vsi_control_rings(..., false) is always 0. So, test for non-zero will never be true. The function has been split into "int i40e_vsi_start_rings()" and "void i40e_vsi_stop_rings()" for better understanding. Similarly, the function i40e_vsi_kill_vlan() never fails. So, checking for return value is also unnecessary. Function definition changed to void. The i40e_loopback_test() function is not implemented. The function and all references to loopback testing were removed. Change-ID: Id45cf66f6689ce2bc4e887de13f073e30e8431bd Signed-off-by: Filip Sadowski Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e.h | 5 ++- .../net/ethernet/intel/i40e/i40e_ethtool.c | 18 -------- drivers/net/ethernet/intel/i40e/i40e_main.c | 41 ++++++++++--------- .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 29 ++++--------- 4 files changed, 33 insertions(+), 60 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index bb02280bc4c18..29c23183a0e01 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -773,7 +773,8 @@ void i40e_service_event_schedule(struct i40e_pf *pf); void i40e_notify_client_of_vf_msg(struct i40e_vsi *vsi, u32 vf_id, u8 *msg, u16 len); -int i40e_vsi_control_rings(struct i40e_vsi *vsi, bool enable); +int i40e_vsi_start_rings(struct i40e_vsi *vsi); +void i40e_vsi_stop_rings(struct i40e_vsi *vsi); int i40e_reconfig_rss_queues(struct i40e_pf *pf, int queue_count); struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf, u16 flags, u16 uplink_seid, u16 downlink_seid, u8 enabled_tc); @@ -849,7 +850,7 @@ int i40e_close(struct net_device *netdev); int i40e_vsi_open(struct i40e_vsi *vsi); void i40e_vlan_stripping_disable(struct i40e_vsi *vsi); int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid); -int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid); +void i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid); struct i40e_mac_filter *i40e_put_mac_in_vlan(struct i40e_vsi *vsi, const u8 *macaddr); int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, const u8 *macaddr); diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c index 962a9bb8a08ea..b9e1162d927f3 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c @@ -216,7 +216,6 @@ enum i40e_ethtool_test_id { I40E_ETH_TEST_REG = 0, I40E_ETH_TEST_EEPROM, I40E_ETH_TEST_INTR, - I40E_ETH_TEST_LOOPBACK, I40E_ETH_TEST_LINK, }; @@ -224,7 +223,6 @@ static const char i40e_gstrings_test[][ETH_GSTRING_LEN] = { "Register test (offline)", "Eeprom test (offline)", "Interrupt test (offline)", - "Loopback test (offline)", "Link test (on/offline)" }; @@ -1744,17 +1742,6 @@ static int i40e_intr_test(struct net_device *netdev, u64 *data) return *data; } -static int i40e_loopback_test(struct net_device *netdev, u64 *data) -{ - struct i40e_netdev_priv *np = netdev_priv(netdev); - struct i40e_pf *pf = np->vsi->back; - - netif_info(pf, hw, netdev, "loopback test not implemented\n"); - *data = 0; - - return *data; -} - static inline bool i40e_active_vfs(struct i40e_pf *pf) { struct i40e_vf *vfs = pf->vf; @@ -1790,7 +1777,6 @@ static void i40e_diag_test(struct net_device *netdev, data[I40E_ETH_TEST_REG] = 1; data[I40E_ETH_TEST_EEPROM] = 1; data[I40E_ETH_TEST_INTR] = 1; - data[I40E_ETH_TEST_LOOPBACK] = 1; data[I40E_ETH_TEST_LINK] = 1; eth_test->flags |= ETH_TEST_FL_FAILED; clear_bit(__I40E_TESTING, &pf->state); @@ -1818,9 +1804,6 @@ static void i40e_diag_test(struct net_device *netdev, if (i40e_intr_test(netdev, &data[I40E_ETH_TEST_INTR])) eth_test->flags |= ETH_TEST_FL_FAILED; - if (i40e_loopback_test(netdev, &data[I40E_ETH_TEST_LOOPBACK])) - eth_test->flags |= ETH_TEST_FL_FAILED; - /* run reg test last, a reset is required after it */ if (i40e_reg_test(netdev, &data[I40E_ETH_TEST_REG])) eth_test->flags |= ETH_TEST_FL_FAILED; @@ -1841,7 +1824,6 @@ static void i40e_diag_test(struct net_device *netdev, data[I40E_ETH_TEST_REG] = 0; data[I40E_ETH_TEST_EEPROM] = 0; data[I40E_ETH_TEST_INTR] = 0; - data[I40E_ETH_TEST_LOOPBACK] = 0; } skip_ol_tests: diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 9ec977a425de6..5c6a5ceb8a913 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -2503,10 +2503,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid) * i40e_vsi_kill_vlan - Remove vsi membership for given vlan * @vsi: the vsi being configured * @vid: vlan id to be removed (0 = untagged only , -1 = any) - * - * Return: 0 on success or negative otherwise **/ -int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) +void i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) { struct net_device *netdev = vsi->netdev; struct i40e_mac_filter *f; @@ -2530,7 +2528,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid) * applying the new filter changes */ i40e_service_event_schedule(vsi->back); - return 0; } /** @@ -4017,29 +4014,35 @@ static int i40e_vsi_control_rx(struct i40e_vsi *vsi, bool enable) } /** - * i40e_vsi_control_rings - Start or stop a VSI's rings + * i40e_vsi_start_rings - Start a VSI's rings * @vsi: the VSI being configured - * @enable: start or stop the rings **/ -int i40e_vsi_control_rings(struct i40e_vsi *vsi, bool request) +int i40e_vsi_start_rings(struct i40e_vsi *vsi) { int ret = 0; /* do rx first for enable and last for disable */ - if (request) { - ret = i40e_vsi_control_rx(vsi, request); - if (ret) - return ret; - ret = i40e_vsi_control_tx(vsi, request); - } else { - /* Ignore return value, we need to shutdown whatever we can */ - i40e_vsi_control_tx(vsi, request); - i40e_vsi_control_rx(vsi, request); - } + ret = i40e_vsi_control_rx(vsi, true); + if (ret) + return ret; + ret = i40e_vsi_control_tx(vsi, true); return ret; } +/** + * i40e_vsi_stop_rings - Stop a VSI's rings + * @vsi: the VSI being configured + **/ +void i40e_vsi_stop_rings(struct i40e_vsi *vsi) +{ + /* do rx first for enable and last for disable + * Ignore return value, we need to shutdown whatever we can + */ + i40e_vsi_control_tx(vsi, false); + i40e_vsi_control_rx(vsi, false); +} + /** * i40e_vsi_free_irq - Free the irq association with the OS * @vsi: the VSI being configured @@ -5238,7 +5241,7 @@ static int i40e_up_complete(struct i40e_vsi *vsi) i40e_configure_msi_and_legacy(vsi); /* start rings */ - err = i40e_vsi_control_rings(vsi, true); + err = i40e_vsi_start_rings(vsi); if (err) return err; @@ -5335,7 +5338,7 @@ void i40e_down(struct i40e_vsi *vsi) netif_tx_disable(vsi->netdev); } i40e_vsi_disable_irq(vsi); - i40e_vsi_control_rings(vsi, false); + i40e_vsi_stop_rings(vsi); i40e_napi_disable_all(vsi); for (i = 0; i < vsi->num_queue_pairs; i++) { diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 69e184b3f72fb..53b46553dd8df 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -991,7 +991,7 @@ void i40e_reset_vf(struct i40e_vf *vf, bool flr) if (vf->lan_vsi_idx == 0) goto complete_reset; - i40e_vsi_control_rings(pf->vsi[vf->lan_vsi_idx], false); + i40e_vsi_stop_rings(pf->vsi[vf->lan_vsi_idx]); complete_reset: /* reallocate VF resources to reset the VSI state */ i40e_free_vf_res(vf); @@ -1032,8 +1032,7 @@ void i40e_free_vfs(struct i40e_pf *pf) i40e_notify_client_of_vf_enable(pf, 0); for (i = 0; i < pf->num_alloc_vfs; i++) if (test_bit(I40E_VF_STAT_INIT, &pf->vf[i].vf_states)) - i40e_vsi_control_rings(pf->vsi[pf->vf[i].lan_vsi_idx], - false); + i40e_vsi_stop_rings(pf->vsi[pf->vf[i].lan_vsi_idx]); /* Disable IOV before freeing resources. This lets any VF drivers * running in the host get themselves cleaned up before we yank @@ -1759,7 +1758,7 @@ static int i40e_vc_enable_queues_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) goto error_param; } - if (i40e_vsi_control_rings(pf->vsi[vf->lan_vsi_idx], true)) + if (i40e_vsi_start_rings(pf->vsi[vf->lan_vsi_idx])) aq_ret = I40E_ERR_TIMEOUT; error_param: /* send the response to the VF */ @@ -1798,8 +1797,7 @@ static int i40e_vc_disable_queues_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) goto error_param; } - if (i40e_vsi_control_rings(pf->vsi[vf->lan_vsi_idx], false)) - aq_ret = I40E_ERR_TIMEOUT; + i40e_vsi_stop_rings(pf->vsi[vf->lan_vsi_idx]); error_param: /* send the response to the VF */ @@ -2139,9 +2137,8 @@ static int i40e_vc_remove_vlan_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) } for (i = 0; i < vfl->num_elements; i++) { - int ret = i40e_vsi_kill_vlan(vsi, vfl->vlan_id[i]); - if (!ret) - vf->num_vlan--; + i40e_vsi_kill_vlan(vsi, vfl->vlan_id[i]); + vf->num_vlan--; if (test_bit(I40E_VF_STAT_UC_PROMISC, &vf->vf_states)) i40e_aq_set_vsi_uc_promisc_on_vlan(&pf->hw, vsi->seid, @@ -2153,11 +2150,6 @@ static int i40e_vc_remove_vlan_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) false, vfl->vlan_id[i], NULL); - - if (ret) - dev_err(&pf->pdev->dev, - "Unable to delete VLAN filter %d for VF %d, error %d\n", - vfl->vlan_id[i], vf->vf_id, ret); } error_param: @@ -2835,13 +2827,8 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id, if (vsi->info.pvid) { /* kill old VLAN */ - ret = i40e_vsi_kill_vlan(vsi, (le16_to_cpu(vsi->info.pvid) & - VLAN_VID_MASK)); - if (ret) { - dev_info(&vsi->back->pdev->dev, - "remove VLAN failed, ret=%d, aq_err=%d\n", - ret, pf->hw.aq.asq_last_status); - } + i40e_vsi_kill_vlan(vsi, (le16_to_cpu(vsi->info.pvid) & + VLAN_VID_MASK)); } if (vlan_id || qos) ret = i40e_vsi_add_pvid(vsi, vlanprio);