* [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates
@ 2016-10-05 16:30 Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 01/15] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
` (15 more replies)
0 siblings, 16 replies; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
Alan Brady adds code to fix mac filters when removing vlans.
Jacob Keller provides several patches to refactor handling of MAC
filters and fix several issues, and improve code design. This
includes removal of unnecessary tracking of VF and Netdev filters,
as well as removal of a faulty debugfs command. Additionally he
provides patches to fix an issue with PTP Rx timestamps, updating
the logic for handling timestamps on discarded frames.
drivers/net/ethernet/intel/i40e/i40e.h | 50 +-
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 92 +-
drivers/net/ethernet/intel/i40e/i40e_fcoe.c | 12 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 949 +++++++++++----------
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 137 +--
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 +-
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 -
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 57 +-
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 8 +-
9 files changed, 669 insertions(+), 647 deletions(-)
--
2.4.11
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 01/15] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync Bimmy Pujari
` (14 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Idf916dd2a1159b1188ddbab5bef6b85ea6bf27d9
---
Testing-hints:
Should test add and remove of various filters with high frequency.
Ensure that debugfs cannot add or remove filters any more.
drivers/net/ethernet/intel/i40e/i40e.h | 19 +-
drivers/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 ++++++---------------
drivers/net/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 5a6f851..8445591 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 0354632..8f6ed8d 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 <uplink_seid> <vsi_seid>\n");
dev_info(&pf->pdev->dev, " del relay <relay_seid>\n");
- dev_info(&pf->pdev->dev, " add macaddr <vsi_seid> <aa:bb:cc:dd:ee:ff> [vlan]\n");
- dev_info(&pf->pdev->dev, " del macaddr <vsi_seid> <aa:bb:cc:dd:ee:ff> [vlan]\n");
dev_info(&pf->pdev->dev, " add pvid <vsi_seid> <vid>\n");
dev_info(&pf->pdev->dev, " del pvid <vsi_seid>\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 58e6c15..0c2a00d 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 1527ccf..09f855f 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 @@ add_filter_out:
* @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 @@ add_filter_out:
* 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);
}
@@ -2316,17 +2242,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",
@@ -2337,7 +2258,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",
@@ -2353,13 +2274,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",
@@ -2373,13 +2292,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",
@@ -2410,20 +2326,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
@@ -2431,7 +2343,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++;
@@ -2441,10 +2353,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",
@@ -2456,9 +2367,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",
@@ -9147,7 +9057,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 */
@@ -9156,7 +9066,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);
}
@@ -9512,8 +9422,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 54b8ee2..dccc9f8 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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 01/15] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 19:23 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan Bimmy Pujari
` (13 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-Id: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474
---
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 8445591..a9c0228 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 09f855f..eb05ca8 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;
@@ -1568,6 +1569,52 @@ static void i40e_vsi_setup_queue_map(struct i40e_vsi *vsi,
}
/**
+ * 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 */
@@ -9421,8 +9427,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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 01/15] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 19:24 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 04/15] i40e: refactor i40e_put_mac_in_vlan to avoid changing f->vlan Bimmy Pujari
` (12 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474
---
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 eb05ca8..5908f7d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1208,63 +1208,6 @@ 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
- *
- * 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
* @macaddr: the MAC address
@@ -1365,6 +1308,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
* @p: pointer to an address structure
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 04/15] i40e: refactor i40e_put_mac_in_vlan to avoid changing f->vlan
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (2 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters Bimmy Pujari
` (11 subsequent siblings)
15 siblings, 0 replies; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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 implicitely
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I2325dfd843aec76d89fb0d7cb0e7c4f290a34840
---
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 5908f7d..9a18c67 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;
}
/**
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (3 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 04/15] i40e: refactor i40e_put_mac_in_vlan to avoid changing f->vlan Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 19:28 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable Bimmy Pujari
` (10 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-Id: I7962154013ce02fe950584690aeeb3ed853d0086
---
Testing-hints:
1. Load driver and bring link up
modprobe i40e
ip link set enp130s0 up
2. Add a VLAN
ip link add link enp130s0 enp130s0.4000 type vlan id 4000
ip link set enp130s0.4000 up
3. Dump filters:
echo "dump filters" > /sys/kernel/debug/i40e/0000:82:00.0/command
dmesg | tail -n40
Note that without this patch you should see some filters repeated with
both VLAN 0 and VLAN -1, when we expect only seeing filters on VLAN 0.
It may take multiple steps to reproduce as it might be a race
condition, though I did not have trouble reproducing the issue.
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 9a18c67..d365652 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;
@@ -2264,6 +2266,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,
@@ -2298,6 +2302,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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (4 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 19:30 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key Bimmy Pujari
` (9 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I75fabe0f53bf73f56b80d342e5fdcfcc28f4d3eb
---
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 d365652..5692390 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;
@@ -1308,6 +1305,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
* @macaddr: the mac address to be filtered
@@ -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;
}
/**
@@ -2249,7 +2270,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);
@@ -2283,11 +2304,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,
@@ -2304,9 +2325,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,
@@ -2346,8 +2369,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
@@ -2379,7 +2404,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,
@@ -9442,7 +9468,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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (5 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-06 20:31 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters Bimmy Pujari
` (8 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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)).
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I5330bd04053b880e670210933e35830b95948ebb
---
drivers/net/ethernet/intel/i40e/i40e.h | 24 ++-
drivers/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 ++++++++++++---------
drivers/net/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 a9c0228..6545550 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -39,6 +39,7 @@
#include <linux/iommu.h>
#include <linux/slab.h>
#include <linux/list.h>
+#include <linux/hashtable.h>
#include <linux/string.h>
#include <linux/in.h>
#include <linux/ip.h>
@@ -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 8f6ed8d..b8a03a0 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 0c2a00d..b077ef8 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 5692390..cce1e18 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",
@@ -2270,10 +2299,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);
@@ -2281,12 +2312,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);
@@ -2294,7 +2325,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;
}
}
@@ -2314,7 +2345,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;
}
}
@@ -2322,7 +2353,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,
@@ -2335,13 +2366,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
@@ -2360,16 +2391,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);
}
@@ -2379,7 +2412,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))
@@ -2397,13 +2430,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);
@@ -2411,13 +2444,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
@@ -7295,7 +7328,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);
@@ -7310,7 +7343,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;
@@ -9094,18 +9127,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);
@@ -9189,7 +9222,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;
@@ -9388,13 +9423,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;
@@ -9424,11 +9459,12 @@ err:
**/
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;
@@ -9458,7 +9494,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) {
@@ -9467,10 +9503,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 dccc9f8..df917b1 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,
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (6 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 19:32 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans Bimmy Pujari
` (7 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-Id: Iee107fd219c6e03e2fd9645c2debf8e8384a8521
---
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 cce1e18..2071727 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,
@@ -2140,6 +2134,17 @@ out:
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;
}
/**
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (7 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-06 20:41 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode Bimmy Pujari
` (6 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Alan Brady <alan.brady@intel.com>
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 accomdating 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.
Signed-off-by: Alan Brady <alan.brady@intel.com>
Change-ID: Ib38fe6034b356eee9a0e20b8a9eeed5ff2debcd9
---
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 2071727..21725dc 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);
}
@@ -2138,6 +2205,7 @@ out:
err_no_memory:
/* Restore elements on the temporary add and delete lists */
spin_lock_bh(&vsi->mac_filter_hash_lock);
+err_no_memory_locked:
i40e_undo_filter_entries(vsi, &tmp_del_list);
i40e_undo_filter_entries(vsi, &tmp_add_list);
spin_unlock_bh(&vsi->mac_filter_hash_lock);
@@ -2396,9 +2464,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 */
@@ -2412,49 +2479,6 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
__i40e_del_filter(vsi, f);
}
- /* go through all the filters for this VSI and if there is only
- * vid == 0 it means there are no other filters, so vid 0 must
- * be replaced with -1. This signifies that we should from now
- * on accept any traffic (with any tag present, or untagged)
- */
- hash_for_each(vsi->mac_filter_hash, bkt, f, hlist) {
- if (vsi->netdev) {
- if (f->vlan &&
- ether_addr_equal(netdev->dev_addr, f->macaddr))
- filter_count++;
- }
-
- if (f->vlan)
- filter_count++;
- }
-
- if (!filter_count && vsi->netdev) {
- i40e_del_filter(vsi, netdev->dev_addr, 0);
- f = i40e_add_filter(vsi, netdev->dev_addr, I40E_VLAN_ANY);
- if (!f) {
- dev_info(&vsi->back->pdev->dev,
- "Could not add filter %d for %pM\n",
- I40E_VLAN_ANY, netdev->dev_addr);
- spin_unlock_bh(&vsi->mac_filter_hash_lock);
- return -ENOMEM;
- }
- }
-
- if (!filter_count) {
- hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
- if (!f->vlan)
- __i40e_del_filter(vsi, f);
- add_f = i40e_add_filter(vsi, f->macaddr, I40E_VLAN_ANY);
- if (!add_f) {
- dev_info(&vsi->back->pdev->dev,
- "Could not add filter %d for %pM\n",
- I40E_VLAN_ANY, f->macaddr);
- spin_unlock_bh(&vsi->mac_filter_hash_lock);
- return -ENOMEM;
- }
- }
- }
-
spin_unlock_bh(&vsi->mac_filter_hash_lock);
/* schedule our worker thread which will take care of
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (8 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-06 20:43 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters Bimmy Pujari
` (5 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Iafde08df588da2a2ea666997d05e11fad8edc338
---
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 6545550..60dbb5b 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 21725dc..327ffbf 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 /
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (9 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 16:27 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor Bimmy Pujari
` (4 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I60d68aeb887976787dec00b23ab386a106e61465
---
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 327ffbf..4f14b55 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1783,6 +1783,80 @@ i40e_update_filter_state(int count,
}
/**
+ * 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.
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (10 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 16:33 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points Bimmy Pujari
` (3 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I4437e8f3a3df2c2ddb458b0fb61420f3dafc4c12
---
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 daade4fe..c9eb6b8 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;
}
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (11 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 16:34 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic Bimmy Pujari
` (2 subsequent siblings)
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I971761a89b58c6aad953590162e85a327fbba232
---
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 60dbb5b..21f2bb3 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 f1fecea..177b7fb 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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (12 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 16:39 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [PATCH] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Id55458c0cd7a5dd0c951ff2b8ac0b2509364131f
---
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 21f2bb3..9121e46 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 177b7fb..5e2272c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -227,6 +227,47 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
}
/**
+ * 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 c9eb6b8..783ac4e 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 5088405..42f04d6 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;
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (13 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
2016-10-07 16:40 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [PATCH] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
15 siblings, 1 reply; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Ib2086f0a142402b614f67846bc091754203a0b9a
---
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 2099517..8b37556 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;
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [PATCH] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
` (14 preceding siblings ...)
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while Bimmy Pujari
@ 2016-10-05 16:30 ` Bimmy Pujari
15 siblings, 0 replies; 30+ messages in thread
From: Bimmy Pujari @ 2016-10-05 16:30 UTC (permalink / raw)
To: intel-wired-lan
From: Jacob Keller <jacob.e.keller@intel.com>
DO NOT SEND THIS PATCH UPSTREAM IF THIS LINE IS STILL HERE!
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. This simplifies the
code and removes a couple of subtle bugs caused by interaction with
debugFS. It also removes some complex code in i40e_add_filter and
i40e_del_filter which was confusing.
Testing-hints:
Should test add and remove of various filters, as well as add/remove
via debugFS. Filters should now behave correctly when added and
removed via debugFS regardless of ordering with other filters in
regular netdev operation.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Abodunrin, Akeem G <akeem.g.abodunrin@intel.com>
Reviewed-by: Maharajan, Pandi <pandi.maharajan@intel.com>
Change-ID: Id9b637a4157999ea29e91142e82b287b183e3eac
---
drivers/net/ethernet/intel/i40e/i40e.h | 18 ++++--------
drivers/net/ethernet/intel/i40e/i40e_main.c | 34 ++++------------------
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 19 +++++-------
3 files changed, 19 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 5a6f851..1f8fc8d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -459,8 +459,6 @@ struct i40e_mac_filter {
#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 +721,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 +813,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_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d05c85f..a459ac8 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;
@@ -1173,14 +1170,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;
@@ -1291,8 +1285,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,8 +1292,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,
- bool is_vf, bool is_netdev)
+ u8 *macaddr, s16 vlan)
{
struct i40e_mac_filter *f;
int changed = false;
@@ -1316,7 +1307,7 @@ 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)
@@ -1337,20 +1328,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
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++;
- }
+ f->counter++;
if (changed) {
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
@@ -1366,8 +1344,6 @@ add_filter_out:
* @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.
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 54b8ee2..dccc9f8 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);
--
2.4.11
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key Bimmy Pujari
@ 2016-10-06 20:31 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-06 20:31 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN
> filters in a hash with the MAC Address as key
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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)).
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I5330bd04053b880e670210933e35830b95948ebb
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 24 ++-
> drivers/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 ++++++++++++--------
> -
> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 38 ++--
> 5 files changed, 161 insertions(+), 107 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans Bimmy Pujari
@ 2016-10-06 20:41 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-06 20:41 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Brady, Alan <alan.brady@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters
> when removing vlans
>
> From: Alan Brady <alan.brady@intel.com>
>
> 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 accomdating
> 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.
>
> Signed-off-by: Alan Brady <alan.brady@intel.com>
> Change-ID: Ib38fe6034b356eee9a0e20b8a9eeed5ff2debcd9
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 120 +++++++++++++++++----
> -------
> 1 file changed, 72 insertions(+), 48 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode Bimmy Pujari
@ 2016-10-06 20:43 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-06 20:43 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to
> check whether we're in vlan mode
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Iafde08df588da2a2ea666997d05e11fad8edc338
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 1 +
> drivers/net/ethernet/intel/i40e/i40e_main.c | 48
> ++++++++++++++++++++++-------
> 2 files changed, 38 insertions(+), 11 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters Bimmy Pujari
@ 2016-10-07 16:27 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 16:27 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate
> add/delete adminq command code for filters
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I60d68aeb887976787dec00b23ab386a106e61465
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 156 +++++++++++++++-------
> ------
> 1 file changed, 84 insertions(+), 72 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor Bimmy Pujari
@ 2016-10-07 16:33 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 16:33 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for
> reading TSYNINDX from the receive descriptor
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I4437e8f3a3df2c2ddb458b0fb61420f3dafc4c12
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points Bimmy Pujari
@ 2016-10-07 16:34 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 16:34 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex
> instead of spinlock in PTP user entry points
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I971761a89b58c6aad953590162e85a327fbba232
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
> drivers/net/ethernet/intel/i40e/i40e_ptp.c | 20 +++++++-------------
> 2 files changed, 8 insertions(+), 14 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic Bimmy Pujari
@ 2016-10-07 16:39 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 16:39 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx
> timestamp hang logic
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Id55458c0cd7a5dd0c951ff2b8ac0b2509364131f
> ---
> 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(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while Bimmy Pujari
@ 2016-10-07 16:40 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 16:40 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra
> msleep while
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Ib2086f0a142402b614f67846bc091754203a0b9a
> ---
> drivers/net/ethernet/intel/i40evf/i40evf_main.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync Bimmy Pujari
@ 2016-10-07 19:23 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 19:23 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of
> __dev_uc_sync and __dev_mc_sync
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-Id: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 10 +--
> drivers/net/ethernet/intel/i40e/i40e_main.c | 113 ++++++++++++++++-----
> -------
> 2 files changed, 69 insertions(+), 54 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan Bimmy Pujari
@ 2016-10-07 19:24 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 19:24 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move
> i40e_put_mac_in_vlan and i40e_del_mac_all_vlan
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I2ca7fd4e10c0c07ed2291db1ea41bf5987fc6474
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 113 ++++++++++++++---------
> -----
> 1 file changed, 56 insertions(+), 57 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters Bimmy Pujari
@ 2016-10-07 19:28 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 19:28 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all
> MAC/VLAN filters, ignore removed filters
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-Id: I7962154013ce02fe950584690aeeb3ed853d0086
> ---
> Testing-hints:
> 1. Load driver and bring link up
> modprobe i40e
> ip link set enp130s0 up
> 2. Add a VLAN
> ip link add link enp130s0 enp130s0.4000 type vlan id 4000
> ip link set enp130s0.4000 up
> 3. Dump filters:
> echo "dump filters" > /sys/kernel/debug/i40e/0000:82:00.0/command
> dmesg | tail -n40
>
> Note that without this patch you should see some filters repeated with
> both VLAN 0 and VLAN -1, when we expect only seeing filters on VLAN 0.
> It may take multiple steps to reproduce as it might be a race
> condition, though I did not have trouble reproducing the issue.
>
> drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Works as described
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable Bimmy Pujari
@ 2016-10-07 19:30 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 19:30 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement
> __i40e_del_filter and use where applicable
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I75fabe0f53bf73f56b80d342e5fdcfcc28f4d3eb
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 92 ++++++++++++++++++----
> -------
> 1 file changed, 59 insertions(+), 33 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters Bimmy Pujari
@ 2016-10-07 19:32 ` Bowers, AndrewX
0 siblings, 0 replies; 30+ messages in thread
From: Bowers, AndrewX @ 2016-10-07 19:32 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Wednesday, October 05, 2016 9:31 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup
> on allocation failure in i40e_sync_vsi_filters
>
> From: Jacob Keller <jacob.e.keller@intel.com>
>
> 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-Id: Iee107fd219c6e03e2fd9645c2debf8e8384a8521
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 39 ++++++++++++++++-------
> ------
> 1 file changed, 22 insertions(+), 17 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2016-10-07 19:32 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-05 16:30 [Intel-wired-lan] [next PATCH S49-V2 00/15] i40e-i40evf updates Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 01/15] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 02/15] i40e: make use of __dev_uc_sync and __dev_mc_sync Bimmy Pujari
2016-10-07 19:23 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 03/15] i40e: move i40e_put_mac_in_vlan and i40e_del_mac_all_vlan Bimmy Pujari
2016-10-07 19:24 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 04/15] i40e: refactor i40e_put_mac_in_vlan to avoid changing f->vlan Bimmy Pujari
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 05/15] i40e: When searching all MAC/VLAN filters, ignore removed filters Bimmy Pujari
2016-10-07 19:28 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 06/15] i40e: implement __i40e_del_filter and use where applicable Bimmy Pujari
2016-10-07 19:30 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 07/15] i40e: store MAC/VLAN filters in a hash with the MAC Address as key Bimmy Pujari
2016-10-06 20:31 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 08/15] i40e: properly cleanup on allocation failure in i40e_sync_vsi_filters Bimmy Pujari
2016-10-07 19:32 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 09/15] i40e: fix mac filters when removing vlans Bimmy Pujari
2016-10-06 20:41 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 10/15] i40e: avoid looping to check whether we're in vlan mode Bimmy Pujari
2016-10-06 20:43 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 11/15] i40e: remove duplicate add/delete adminq command code for filters Bimmy Pujari
2016-10-07 16:27 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 12/15] i40e: correct check for reading TSYNINDX from the receive descriptor Bimmy Pujari
2016-10-07 16:33 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 13/15] i40e: use a mutex instead of spinlock in PTP user entry points Bimmy Pujari
2016-10-07 16:34 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 14/15] i40e: replace PTP Rx timestamp hang logic Bimmy Pujari
2016-10-07 16:39 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [next PATCH S49-V2 15/15] i40evf: avoid an extra msleep while Bimmy Pujari
2016-10-07 16:40 ` Bowers, AndrewX
2016-10-05 16:30 ` [Intel-wired-lan] [PATCH] i40e: drop is_vf and is_netdev fields in struct i40e_mac_filter Bimmy Pujari
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.