[Intel-wired-lan] [next PATCH S56 0/8] i40e/i40evf updates

[Intel-wired-lan] [next PATCH S56 0/8] i40e/i40evf updates

[Bimmy Pujari]

Aaron Salter changes code to acquire NVM lock before reads on all
devices.

Henry Tieman adds code to save link FEC info from link up event and
adds code to save more link abilities when using ethtool.

Jacob Keller adds code to not warn every time we clear an Rx timestamp
register, adds code to fix i40e_update_filter_state to skip broadcast
filters and adds code to avoid race condition when sending filters to
firmware for addition.

Mitch Williams removes dead code.

Sudheer Mogilappagari adds bus number info to i40e_bus_info struct.

 drivers/net/ethernet/intel/i40e/i40e.h          |  16 ++
 drivers/net/ethernet/intel/i40e/i40e_client.c   |  29 ++--
 drivers/net/ethernet/intel/i40e/i40e_common.c   |   2 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c  |   3 +
 drivers/net/ethernet/intel/i40e/i40e_main.c     | 190 +++++++++++++++++-------
 drivers/net/ethernet/intel/i40e/i40e_nvm.c      |  12 +-
 drivers/net/ethernet/intel/i40e/i40e_osdep.h    |  12 +-
 drivers/net/ethernet/intel/i40e/i40e_ptp.c      |  21 ++-
 drivers/net/ethernet/intel/i40e/i40e_type.h     |   2 +
 drivers/net/ethernet/intel/i40evf/i40e_type.h   |   2 +
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |   1 +
 11 files changed, 203 insertions(+), 87 deletions(-)

-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 1/8] i40e: Clean up dead code

[Bimmy Pujari]

From: Mitch Williams <mitch.a.williams@intel.com>

The function i40e_client_prepare() can never return an error. So make it
void and quit checking its return value.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Change-ID: I9ff311e2324dde329eb68648efb2c94aaff856db
---
Testing Hints : Found by static code analysis. Not
possible to test, just code cleanup.

 drivers/net/ethernet/intel/i40e/i40e_client.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index f6409f9..337d417 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -653,13 +653,11 @@ static int i40e_client_release(struct i40e_client *client)
  * i40e_client_prepare - prepare client specific resources
  * @client: pointer to the registered client
  *
- * Return 0 on success or < 0 on error
  **/
-static int i40e_client_prepare(struct i40e_client *client)
+static void i40e_client_prepare(struct i40e_client *client)
 {
 	struct i40e_device *ldev;
 	struct i40e_pf *pf;
-	int ret = 0;
 
 	mutex_lock(&i40e_device_mutex);
 	list_for_each_entry(ldev, &i40e_devices, list) {
@@ -669,7 +667,6 @@ static int i40e_client_prepare(struct i40e_client *client)
 		i40e_service_event_schedule(pf);
 	}
 	mutex_unlock(&i40e_device_mutex);
-	return ret;
 }
 
 /**
@@ -926,13 +923,9 @@ int i40e_register_client(struct i40e_client *client)
 	set_bit(__I40E_CLIENT_REGISTERED, &client->state);
 	mutex_unlock(&i40e_client_mutex);
 
-	if (i40e_client_prepare(client)) {
-		ret = -EIO;
-		goto out;
-	}
+	i40e_client_prepare(client);
 
-	pr_info("i40e: Registered client %s with return code %d\n",
-		client->name, ret);
+	pr_info("i40e: Registered client %s\n",	client->name);
 out:
 	return ret;
 }
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 2/8] i40e: Add bus number info to i40e_bus_info struct

[Bimmy Pujari]

From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>

Currently i40e_bus_info has PCI device and function info only and log
messages print device number as bus number. Added field to provide bus
number info and modified log statements to print bus, device and
function information.

Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
Change-ID: I811617cee2714cc0d6bade8d369f57040990756f
---
 drivers/net/ethernet/intel/i40e/i40e_client.c   | 16 +++++++++-------
 drivers/net/ethernet/intel/i40e/i40e_main.c     |  1 +
 drivers/net/ethernet/intel/i40e/i40e_osdep.h    | 12 ++++++------
 drivers/net/ethernet/intel/i40e/i40e_type.h     |  1 +
 drivers/net/ethernet/intel/i40evf/i40e_type.h   |  1 +
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |  1 +
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 337d417..09110d3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -510,9 +510,10 @@ void i40e_client_subtask(struct i40e_pf *pf)
 			continue;
 
 		if (!existing) {
-			dev_info(&pf->pdev->dev, "Added instance of Client %s to PF%d bus=0x%02x func=0x%02x\n",
+			dev_info(&pf->pdev->dev, "Added instance of Client %s to PF%d bus=0x%02x dev=0x%02x func=0x%02x\n",
 				 client->name, pf->hw.pf_id,
-				 pf->hw.bus.device, pf->hw.bus.func);
+				 pf->hw.bus.bus_id, pf->hw.bus.device,
+				 pf->hw.bus.func);
 		}
 
 		mutex_lock(&i40e_client_instance_mutex);
@@ -561,8 +562,9 @@ int i40e_lan_add_device(struct i40e_pf *pf)
 	ldev->pf = pf;
 	INIT_LIST_HEAD(&ldev->list);
 	list_add(&ldev->list, &i40e_devices);
-	dev_info(&pf->pdev->dev, "Added LAN device PF%d bus=0x%02x func=0x%02x\n",
-		 pf->hw.pf_id, pf->hw.bus.device, pf->hw.bus.func);
+	dev_info(&pf->pdev->dev, "Added LAN device PF%d bus=0x%02x dev=0x%02x func=0x%02x\n",
+		 pf->hw.pf_id, pf->hw.bus.bus_id,
+		 pf->hw.bus.device, pf->hw.bus.func);
 
 	/* Since in some cases register may have happened before a device gets
 	 * added, we can schedule a subtask to go initiate the clients if
@@ -590,9 +592,9 @@ int i40e_lan_del_device(struct i40e_pf *pf)
 	mutex_lock(&i40e_device_mutex);
 	list_for_each_entry_safe(ldev, tmp, &i40e_devices, list) {
 		if (ldev->pf == pf) {
-			dev_info(&pf->pdev->dev, "Deleted LAN device PF%d bus=0x%02x func=0x%02x\n",
-				 pf->hw.pf_id, pf->hw.bus.device,
-				 pf->hw.bus.func);
+			dev_info(&pf->pdev->dev, "Deleted LAN device PF%d bus=0x%02x dev=0x%02x func=0x%02x\n",
+				 pf->hw.pf_id, pf->hw.bus.bus_id,
+				 pf->hw.bus.device, pf->hw.bus.func);
 			list_del(&ldev->list);
 			kfree(ldev);
 			ret = 0;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7351361..a0689f1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11006,6 +11006,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	hw->subsystem_device_id = pdev->subsystem_device;
 	hw->bus.device = PCI_SLOT(pdev->devfn);
 	hw->bus.func = PCI_FUNC(pdev->devfn);
+	hw->bus.bus_id = pdev->bus->number;
 	pf->instance = pfs_found;
 
 	/* set up the locks for the AQ, do this only once in probe
diff --git a/drivers/net/ethernet/intel/i40e/i40e_osdep.h b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
index be74bcf..fea81ed 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_osdep.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
@@ -69,12 +69,12 @@ struct i40e_virt_mem {
 #define i40e_allocate_virt_mem(h, m, s) i40e_allocate_virt_mem_d(h, m, s)
 #define i40e_free_virt_mem(h, m) i40e_free_virt_mem_d(h, m)
 
-#define i40e_debug(h, m, s, ...)                                \
-do {                                                            \
-	if (((m) & (h)->debug_mask))                            \
-		pr_info("i40e %02x.%x " s,                      \
-			(h)->bus.device, (h)->bus.func,         \
-			##__VA_ARGS__);                         \
+#define i40e_debug(h, m, s, ...)				\
+do {								\
+	if (((m) & (h)->debug_mask))				\
+		pr_info("i40e %02x:%02x.%x " s,			\
+			(h)->bus.bus_id, (h)->bus.device,	\
+			(h)->bus.func, ##__VA_ARGS__);		\
 } while (0)
 
 typedef enum i40e_status_code i40e_status;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 167244b..be7b611 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -469,6 +469,7 @@ struct i40e_bus_info {
 	u16 func;
 	u16 device;
 	u16 lan_id;
+	u16 bus_id;
 };
 
 /* Flow control (FC) parameters */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_type.h b/drivers/net/ethernet/intel/i40evf/i40e_type.h
index 92ac60d..3f19dff 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_type.h
@@ -442,6 +442,7 @@ struct i40e_bus_info {
 	u16 func;
 	u16 device;
 	u16 lan_id;
+	u16 bus_id;
 };
 
 /* Flow control (FC) parameters */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 729d8fa..920c1cb 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2726,6 +2726,7 @@ static int i40evf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	hw->subsystem_device_id = pdev->subsystem_device;
 	hw->bus.device = PCI_SLOT(pdev->devfn);
 	hw->bus.func = PCI_FUNC(pdev->devfn);
+	hw->bus.bus_id = pdev->bus->number;
 
 	/* set up the locks for the AQ, do this only once in probe
 	 * and destroy them only once in remove
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 3/8] i40e: Save link FEC info from link up event

[Bimmy Pujari]

From: Henry Tieman <henry.w.tieman@intel.com>

Store the FEC status bits from the link up event into the
hw_link_info structure.

Signed-off-by: Henry Tieman <henry.w.tieman@intel.com>
Change-ID: I9a7b256f6dfb0dce89c2f503075d0d383526832e
---
Testing Hints: In support of DCR 2228, this commit stores the FEC
data for later display in the "link up" kernel log message.

 drivers/net/ethernet/intel/i40e/i40e_common.c |  2 ++
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 21 +++++++++++++++++++--
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  1 +
 drivers/net/ethernet/intel/i40evf/i40e_type.h |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 59e766c..549fdca 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -1838,6 +1838,8 @@ i40e_status i40e_aq_get_link_info(struct i40e_hw *hw,
 	hw_link_info->link_speed = (enum i40e_aq_link_speed)resp->link_speed;
 	hw_link_info->link_info = resp->link_info;
 	hw_link_info->an_info = resp->an_info;
+	hw_link_info->fec_info = resp->config & (I40E_AQ_CONFIG_FEC_KR_ENA |
+						 I40E_AQ_CONFIG_FEC_RS_ENA);
 	hw_link_info->ext_info = resp->ext_info;
 	hw_link_info->loopback = resp->loopback;
 	hw_link_info->max_frame_size = le16_to_cpu(resp->max_frame_size);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a0689f1..951976b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5267,6 +5267,8 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 	enum i40e_aq_link_speed new_speed;
 	char *speed = "Unknown";
 	char *fc = "Unknown";
+	char *fec = "";
+	char *an = "";
 
 	new_speed = vsi->back->hw.phy.link_info.link_speed;
 
@@ -5326,8 +5328,23 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 		break;
 	}
 
-	netdev_info(vsi->netdev, "NIC Link is Up %sbps Full Duplex, Flow Control: %s\n",
-		    speed, fc);
+	if (vsi->back->hw.phy.link_info.link_speed == I40E_LINK_SPEED_25GB) {
+		fec = ", FEC: None";
+		an = ", Autoneg: False";
+
+		if (vsi->back->hw.phy.link_info.an_info & I40E_AQ_AN_COMPLETED)
+			an = ", Autoneg: True";
+
+		if (vsi->back->hw.phy.link_info.fec_info &
+		    I40E_AQ_CONFIG_FEC_KR_ENA)
+			fec = ", FEC: CL74 FC-FEC/BASE-R";
+		else if (vsi->back->hw.phy.link_info.fec_info &
+			 I40E_AQ_CONFIG_FEC_RS_ENA)
+			fec = ", FEC: CL108 RS-FEC";
+	}
+
+	netdev_info(vsi->netdev, "NIC Link is Up, %sbps Full Duplex%s%s, Flow Control: %s\n",
+		    speed, fec, an, fc);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index be7b611..df7914e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -184,6 +184,7 @@ struct i40e_link_status {
 	enum i40e_aq_link_speed link_speed;
 	u8 link_info;
 	u8 an_info;
+	u8 fec_info;
 	u8 ext_info;
 	u8 loopback;
 	/* is Link Status Event notification to SW enabled */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_type.h b/drivers/net/ethernet/intel/i40evf/i40e_type.h
index 3f19dff..16bb880 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_type.h
@@ -158,6 +158,7 @@ struct i40e_link_status {
 	enum i40e_aq_link_speed link_speed;
 	u8 link_info;
 	u8 an_info;
+	u8 fec_info;
 	u8 ext_info;
 	u8 loopback;
 	/* is Link Status Event notification to SW enabled */
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 4/8] i40e: don't warn every time we clear an Rx timestamp register

[Bimmy Pujari]

From: Jacob Keller <jacob.e.keller@intel.com>

The intent of this message was to indicate to a user that we might have
missed a timestamp event for a valid packet. The original method of
detecting the missed events relied on waiting until all 4 registers were
filled.

A recent commit d55458c0cd7a5 ("i40e: replace PTP Rx timestamp hang
logic") replaced this logic with much better detection
scheme that could detect a stalled Rx timestamp register even when other
registers were still functional.

The new logic means that a message will be displayed almost as soon as
a timestamp for a dropped frame occurs. This new logic highlights that
the hardware will attempt timestamp for frames which it later decides to
drop. The most prominent example is when a multicast PTP frame is
received on a multicast address that we are not subscribed to.

Because the hardware initiates the Rx timestamp as soon as possible, it
will latch an RXTIME register, but then drop the packet.

This results in users being confused by the message as they are not
expecting to see dropped timestamp messages unless their application
also indicates that timestamps were missing.

Resolve this by reducing the severity and frequency of the displayed
message. We now only print the message if 3 or 4 of the RXTIME registers
are stalled and get cleared within the same watchdog event. This ensures
that the common case does not constantly display the message.
Additionally, since the message is likely not as meaningful to most
users, reduce the message to a dev_dbg instead of a dev_warn.

Users can still get a count of the number of timestamps dropped by
reading the ethtool statistics value, if necessary.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: I35494442226a444c418dfb4f91a3070d06c8435c
---
 drivers/net/ethernet/intel/i40e/i40e_ptp.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 9e49ffa..2caee35 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -280,7 +280,7 @@ void i40e_ptp_rx_hang(struct i40e_vsi *vsi)
 {
 	struct i40e_pf *pf = vsi->back;
 	struct i40e_hw *hw = &pf->hw;
-	int i;
+	unsigned int i, cleared = 0;
 
 	/* Since we cannot turn off the Rx timestamp logic if the device is
 	 * configured for Tx timestamping, we check if Rx timestamping is
@@ -306,14 +306,25 @@ void i40e_ptp_rx_hang(struct i40e_vsi *vsi)
 		    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);
+			cleared++;
 		}
 	}
 
 	spin_unlock_bh(&pf->ptp_rx_lock);
+
+	/* Log a warning if more than 2 timestamps got dropped in the same
+	 * check. We don't want to warn about all drops because it can occur
+	 * in normal scenarios such as PTP frames on multicast addresses we
+	 * aren't listening to. However, administrator should know if this is
+	 * the reason packets aren't receiving timestamps.
+	 */
+	if (cleared > 2)
+		dev_dbg(&pf->pdev->dev,
+			"Dropped %d missed RXTIME timestamp events\n",
+			cleared);
+
+	/* Finally, update the rx_hwtstamp_cleared counter */
+	pf->rx_hwtstamp_cleared += cleared;
 }
 
 /**
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 5/8] i40e: allow i40e_update_filter_state to skip broadcast filters

[Bimmy Pujari]

From: Jacob Keller <jacob.e.keller@intel.com>

Fix a bug where we modified the mac_filter_hash while outside a lock,
when handling addition of broadcast filters.

Normally, we add filters to firmware by batching the additions into
lists and issuing 1 update for every few filters. Broadcast filters are
handled differently, by instead setting the broadcast promiscuous mode
flags. In order to make sure the 1<->1 mapping of filters in our
addition array lined up with filters in the hlist tmp_add_list, we had
to remove the filter and move it back to the main hash. However, we
didn't do this under lock, which could cause consistency problems for
the list.

Fix this by updating i40e_update_filter_state logic so that it knows to
avoid broadcast filters. This ensures that we don't have to remove the
filter separately, and can put it back using the normal flow.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Id288fade80b3e3a9a54b68cc249188cb95147518
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 37 ++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 951976b..827676e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1849,6 +1849,31 @@ static void i40e_undo_filter_entries(struct i40e_vsi *vsi,
 }
 
 /**
+ * i40e_next_entry - Get the next non-broadcast filter from a list
+ * @f: pointer to filter in list
+ *
+ * Returns the next non-broadcast filter in the list. Required so that we
+ * ignore broadcast filters within the list, since these are not handled via
+ * the normal firmware update path.
+ */
+static struct i40e_mac_filter *i40e_next_filter(struct i40e_mac_filter *f)
+{
+	while (f) {
+		f = hlist_entry(f->hlist.next,
+				typeof(struct i40e_mac_filter),
+				hlist);
+
+		/* keep going if we found a broadcast filter */
+		if (f && is_broadcast_ether_addr(f->macaddr))
+			continue;
+
+		break;
+	}
+
+	return f;
+}
+
+/**
  * i40e_update_filter_state - Update filter state based on return data
  * from firmware
  * @count: Number of filters added
@@ -1880,9 +1905,9 @@ i40e_update_filter_state(int count,
 			retval++;
 		}
 
-		add_head = hlist_entry(add_head->hlist.next,
-				       typeof(struct i40e_mac_filter),
-				       hlist);
+		add_head = i40e_next_filter(add_head);
+		if (!add_head)
+			break;
 	}
 
 	return retval;
@@ -2101,7 +2126,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 			cmd_flags = 0;
 
 			/* handle broadcast filters by updating the broadcast
-			 * promiscuous flag instead of deleting a MAC filter.
+			 * promiscuous flag and release filter list.
 			 */
 			if (is_broadcast_ether_addr(f->macaddr)) {
 				i40e_aqc_broadcast_filter(vsi, vsi_name, f);
@@ -2170,11 +2195,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 			 * promiscuous flag instead of adding a MAC filter.
 			 */
 			if (is_broadcast_ether_addr(f->macaddr)) {
-				u64 key = i40e_addr_to_hkey(f->macaddr);
 				i40e_aqc_broadcast_filter(vsi, vsi_name, f);
-
-				hlist_del(&f->hlist);
-				hash_add(vsi->mac_filter_hash, &f->hlist, key);
 				continue;
 			}
 
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 6/8] i40e: avoid race condition when sending filters to firmware for addition

[Bimmy Pujari]

From: Jacob Keller <jacob.e.keller@intel.com>

Refactor how we add new filters to firmware to avoid a race condition
that can occur due to removing filters from the hash temporarily.

To understand the race condition, suppose that you have a number of MAC
filters, but have not yet added any VLANs. Now, add two VLANs in rapid
succession. A possible resulting flow would look something like the
following:

(1) lock hash for add VLAN
(2) add the new MAC/VLAN combos for each current MAC filter
(3) unlock hash
(4) lock hash for filter sync
(5) notice that we have a VLAN, so prepare to update all MAC filters
    with VLAN=-1 to be VLAN=0.
(6) move NEW and REMOVE filters to temporary list
(7) unlock hash
(8) lock hash for add VLAN
(9) add new MAC/VLAN combos. Notice that no MAC filters are currently in
    the hash list, so we don't add any VLANs <--- BUG!
(10) unlock hash
(11) sync the temporary lists to firmware
(12) lock hash for post-sync
(13) move the temporary elements back to the main list
....

Because we take filters out of the main hash into temporary lists, we
introduce a narrow window where it is possible that other callers to the
list will not see some of the filters which were previously added but
have not yet been finalized. This results in sometimes dropping VLAN
additions, and could also result in failing to add a MAC address on the
newly added VLAN.

One obvious way to avoid this race condition would be to lock the entire
firmware process. Unfortunately this does not work because adminq
firmware commands take a mutex which results in a sleep while atomic
BUG(). So, we can't use the simplest approach.

An alternative approach is to simply not remove the filters from the
hash list while adding. Instead, add an i40e_new_mac_filter structure
which we will use to track added filters. This avoids the need to remove
the filter from the hash list. We'll store a pointer to the original
i40e_mac_filter, along with our own copy of the state.

We won't update the state directly, so as to avoid race with other code
that may modify the state while under the lock. We are safe to read
f->macaddr and f->vlan since these only change in two locations. The
first is on filter creation, which must have already occurred. The
second is inside i40e_correct_vlan_filters which was previously run
after creation of this object and can't be run again until after. Thus,
we should be safe to read the MAC address and VLAN while outside the
lock.

We also aren't going to run into a use-after-free issue because the only
place where we free filters is when they are marked FAILED or when we
remove them inside the sync subtask. Since the subtask has its own
critical flag to prevent duplicate runs, we know this won't happen. We
also know that the only location to transition a filter from NEW to
FAILED is inside the subtask also, so we aren't worried about that
either.

Use the wrapper i40e_new_mac_filter for additions, and once we've
finalized the addition to firmware, we will update the filter state
inside a lock, and then free the wrapper structure.

In order to avoid a possible race condition with filter deletion, we
won't update the original filter state unless it is still
I40E_FILTER_NEW when we finish the firmware sync.

This approach is more complex, but avoids race conditions related to
filters being temporarily removed from the list. We do not need the same
behavior for deletion because we always unconditionally removed the
filters from the list regardless of the firmware status.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-Id: I14b74bc2301f8e69433fbe77ebca532db20c5317
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  16 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c | 147 ++++++++++++++++++----------
 2 files changed, 112 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index eb01dac..0f2bf241 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -506,6 +506,22 @@ struct i40e_mac_filter {
 	enum i40e_filter_state state;
 };
 
+/* Wrapper structure to keep track of filters while we are preparing to send
+ * firmware commands. We cannot send firmware commands while holding a
+ * spinlock, since it might sleep. To avoid this, we wrap the added filters in
+ * a separate structure, which will track the state change and update the real
+ * filter while under lock. We can't simply hold the filters in a separate
+ * list, as this opens a window for a race condition when adding new MAC
+ * addresses to all VLANs, or when adding new VLANs to all MAC addresses.
+ */
+struct i40e_new_mac_filter {
+	struct hlist_node hlist;
+	struct i40e_mac_filter *f;
+
+	/* Track future changes to state separately */
+	enum i40e_filter_state state;
+};
+
 struct i40e_veb {
 	struct i40e_pf *pf;
 	u16 idx;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 827676e..69a51a4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1261,6 +1261,7 @@ static int i40e_correct_mac_vlan_filters(struct i40e_vsi *vsi,
 					 int vlan_filters)
 {
 	struct i40e_mac_filter *f, *add_head;
+	struct i40e_new_mac_filter *new;
 	struct hlist_node *h;
 	int bkt, new_vlan;
 
@@ -1279,13 +1280,13 @@ static int i40e_correct_mac_vlan_filters(struct i40e_vsi *vsi,
 	 */
 
 	/* Update the filters about to be added in place */
-	hlist_for_each_entry(f, tmp_add_list, hlist) {
-		if (vsi->info.pvid && f->vlan != vsi->info.pvid)
-			f->vlan = vsi->info.pvid;
-		else if (vlan_filters && f->vlan == I40E_VLAN_ANY)
-			f->vlan = 0;
-		else if (!vlan_filters && f->vlan == 0)
-			f->vlan = I40E_VLAN_ANY;
+	hlist_for_each_entry(new, tmp_add_list, hlist) {
+		if (vsi->info.pvid && new->f->vlan != vsi->info.pvid)
+			new->f->vlan = vsi->info.pvid;
+		else if (vlan_filters && new->f->vlan == I40E_VLAN_ANY)
+			new->f->vlan = 0;
+		else if (!vlan_filters && new->f->vlan == 0)
+			new->f->vlan = I40E_VLAN_ANY;
 	}
 
 	/* Update the remaining active filters */
@@ -1311,9 +1312,16 @@ static int i40e_correct_mac_vlan_filters(struct i40e_vsi *vsi,
 			if (!add_head)
 				return -ENOMEM;
 
-			/* Put the replacement filter into the add list */
-			hash_del(&add_head->hlist);
-			hlist_add_head(&add_head->hlist, tmp_add_list);
+			/* Create a temporary i40e_new_mac_filter */
+			new = kzalloc(sizeof(*new), GFP_ATOMIC);
+			if (!new)
+				return -ENOMEM;
+
+			new->f = add_head;
+			new->state = add_head->state;
+
+			/* Add the new filter to the tmp list */
+			hlist_add_head(&new->hlist, tmp_add_list);
 
 			/* Put the original filter into the delete list */
 			f->state = I40E_FILTER_REMOVE;
@@ -1825,16 +1833,15 @@ static void i40e_set_rx_mode(struct net_device *netdev)
 }
 
 /**
- * i40e_undo_filter_entries - Undo the changes made to MAC filter entries
+ * i40e_undo_del_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 sent to firmware, either for
- * addition or deletion.
+ * MAC filter entries from this list were slated for deletion.
  **/
-static void i40e_undo_filter_entries(struct i40e_vsi *vsi,
-				     struct hlist_head *from)
+static void i40e_undo_del_filter_entries(struct i40e_vsi *vsi,
+					 struct hlist_head *from)
 {
 	struct i40e_mac_filter *f;
 	struct hlist_node *h;
@@ -1849,28 +1856,50 @@ static void i40e_undo_filter_entries(struct i40e_vsi *vsi,
 }
 
 /**
+ * i40e_undo_add_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 this list were slated for addition.
+ **/
+static void i40e_undo_add_filter_entries(struct i40e_vsi *vsi,
+					 struct hlist_head *from)
+{
+	struct i40e_new_mac_filter *new;
+	struct hlist_node *h;
+
+	hlist_for_each_entry_safe(new, h, from, hlist) {
+		/* We can simply free the wrapper structure */
+		hlist_del(&new->hlist);
+		kfree(new);
+	}
+}
+
+/**
  * i40e_next_entry - Get the next non-broadcast filter from a list
- * @f: pointer to filter in list
+ * @next: pointer to filter in list
  *
  * Returns the next non-broadcast filter in the list. Required so that we
  * ignore broadcast filters within the list, since these are not handled via
  * the normal firmware update path.
  */
-static struct i40e_mac_filter *i40e_next_filter(struct i40e_mac_filter *f)
+static
+struct i40e_new_mac_filter *i40e_next_filter(struct i40e_new_mac_filter *next)
 {
-	while (f) {
-		f = hlist_entry(f->hlist.next,
-				typeof(struct i40e_mac_filter),
-				hlist);
+	while (next) {
+		next = hlist_entry(next->hlist.next,
+				   typeof(struct i40e_new_mac_filter),
+				   hlist);
 
 		/* keep going if we found a broadcast filter */
-		if (f && is_broadcast_ether_addr(f->macaddr))
+		if (next && is_broadcast_ether_addr(next->f->macaddr))
 			continue;
 
 		break;
 	}
 
-	return f;
+	return next;
 }
 
 /**
@@ -1886,7 +1915,7 @@ static struct i40e_mac_filter *i40e_next_filter(struct i40e_mac_filter *f)
 static int
 i40e_update_filter_state(int count,
 			 struct i40e_aqc_add_macvlan_element_data *add_list,
-			 struct i40e_mac_filter *add_head)
+			 struct i40e_new_mac_filter *add_head)
 {
 	int retval = 0;
 	int i;
@@ -1964,7 +1993,7 @@ void i40e_aqc_del_filters(struct i40e_vsi *vsi, const char *vsi_name,
 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,
+			  struct i40e_new_mac_filter *add_head,
 			  int num_add, bool *promisc_changed)
 {
 	struct i40e_hw *hw = &vsi->back->hw;
@@ -1992,10 +2021,12 @@ void i40e_aqc_add_filters(struct i40e_vsi *vsi, const char *vsi_name,
  * This function sets or clears the promiscuous broadcast flags for VLAN
  * filters in order to properly receive broadcast frames. Assumes that only
  * broadcast filters are passed.
+ *
+ * Returns status indicating success or failure;
  **/
-static
-void i40e_aqc_broadcast_filter(struct i40e_vsi *vsi, const char *vsi_name,
-			       struct i40e_mac_filter *f)
+static i40e_status
+i40e_aqc_broadcast_filter(struct i40e_vsi *vsi, const char *vsi_name,
+			  struct i40e_mac_filter *f)
 {
 	bool enable = f->state == I40E_FILTER_NEW;
 	struct i40e_hw *hw = &vsi->back->hw;
@@ -2014,15 +2045,13 @@ void i40e_aqc_broadcast_filter(struct i40e_vsi *vsi, const char *vsi_name,
 							    NULL);
 	}
 
-	if (aq_ret) {
+	if (aq_ret)
 		dev_warn(&vsi->back->pdev->dev,
 			 "Error %s setting broadcast promiscuous mode on %s\n",
 			 i40e_aq_str(hw, hw->aq.asq_last_status),
 			 vsi_name);
-		f->state = I40E_FILTER_FAILED;
-	} else if (enable) {
-		f->state = I40E_FILTER_ACTIVE;
-	}
+
+	return aq_ret;
 }
 
 /**
@@ -2036,7 +2065,8 @@ void i40e_aqc_broadcast_filter(struct i40e_vsi *vsi, const char *vsi_name,
 int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 {
 	struct hlist_head tmp_add_list, tmp_del_list;
-	struct i40e_mac_filter *f, *add_head = NULL;
+	struct i40e_mac_filter *f;
+	struct i40e_new_mac_filter *new, *add_head = NULL;
 	struct i40e_hw *hw = &vsi->back->hw;
 	unsigned int failed_filters = 0;
 	unsigned int vlan_filters = 0;
@@ -2090,8 +2120,17 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 				continue;
 			}
 			if (f->state == I40E_FILTER_NEW) {
-				hash_del(&f->hlist);
-				hlist_add_head(&f->hlist, &tmp_add_list);
+				/* Create a temporary i40e_new_mac_filter */
+				new = kzalloc(sizeof(*new), GFP_ATOMIC);
+				if (!new)
+					goto err_no_memory_locked;
+
+				/* Store pointer to the real filter */
+				new->f = f;
+				new->state = f->state;
+
+				/* Add it to the hash list */
+				hlist_add_head(&new->hlist, &tmp_add_list);
 			}
 
 			/* Count the number of active (current and new) VLAN
@@ -2184,32 +2223,37 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 			goto err_no_memory;
 
 		num_add = 0;
-		hlist_for_each_entry_safe(f, h, &tmp_add_list, hlist) {
+		hlist_for_each_entry_safe(new, h, &tmp_add_list, hlist) {
 			if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
 				     &vsi->state)) {
-				f->state = I40E_FILTER_FAILED;
+				new->state = I40E_FILTER_FAILED;
 				continue;
 			}
 
 			/* handle broadcast filters by updating the broadcast
 			 * promiscuous flag instead of adding a MAC filter.
 			 */
-			if (is_broadcast_ether_addr(f->macaddr)) {
-				i40e_aqc_broadcast_filter(vsi, vsi_name, f);
+			if (is_broadcast_ether_addr(new->f->macaddr)) {
+				if (i40e_aqc_broadcast_filter(vsi, vsi_name,
+							      new->f))
+					new->state = I40E_FILTER_FAILED;
+				else
+					new->state = I40E_FILTER_ACTIVE;
 				continue;
 			}
 
 			/* add to add array */
 			if (num_add == 0)
-				add_head = f;
+				add_head = new;
 			cmd_flags = 0;
-			ether_addr_copy(add_list[num_add].mac_addr, f->macaddr);
-			if (f->vlan == I40E_VLAN_ANY) {
+			ether_addr_copy(add_list[num_add].mac_addr,
+					new->f->macaddr);
+			if (new->f->vlan == I40E_VLAN_ANY) {
 				add_list[num_add].vlan_tag = 0;
 				cmd_flags |= I40E_AQC_MACVLAN_ADD_IGNORE_VLAN;
 			} else {
 				add_list[num_add].vlan_tag =
-					cpu_to_le16((u16)(f->vlan));
+					cpu_to_le16((u16)(new->f->vlan));
 			}
 			add_list[num_add].queue_number = 0;
 			/* set invalid match method for later detection */
@@ -2236,11 +2280,12 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 		 * the VSI's 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);
+		hlist_for_each_entry_safe(new, h, &tmp_add_list, hlist) {
+			/* Only update the state if we're still NEW */
+			if (new->f->state == I40E_FILTER_NEW)
+				new->f->state = new->state;
+			hlist_del(&new->hlist);
+			kfree(new);
 		}
 		spin_unlock_bh(&vsi->mac_filter_hash_lock);
 		kfree(add_list);
@@ -2389,8 +2434,8 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 	/* 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);
+	i40e_undo_del_filter_entries(vsi, &tmp_del_list);
+	i40e_undo_add_filter_entries(vsi, &tmp_add_list);
 	spin_unlock_bh(&vsi->mac_filter_hash_lock);
 
 	vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 7/8] i40e: Save more link abilities when using ethtool

[Bimmy Pujari]

From: Henry Tieman <henry.w.tieman@intel.com>

Ethtool support needs to save more PHY information. The
added information includes FEC capabilities and 25G link
types. Without this change it is possible to lose 25G or
FEC settings by using ethtool.

Signed-off-by: Henry Tieman <henry.w.tieman@intel.com>
Change-ID: Ie42255b1e901ffbf9583b8c46466a54894114280
---
Testing Hints: Found by code inspection. Test by changing
link speed from 25G to 10G and back to 25G using ethtool.

 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index f9f71fb..dece0d6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -803,9 +803,12 @@ static int i40e_set_settings(struct net_device *netdev,
 	if (change || (abilities.link_speed != config.link_speed)) {
 		/* copy over the rest of the abilities */
 		config.phy_type = abilities.phy_type;
+		config.phy_type_ext = abilities.phy_type_ext;
 		config.eee_capability = abilities.eee_capability;
 		config.eeer = abilities.eeer_val;
 		config.low_power_ctrl = abilities.d3_lpan;
+		config.fec_config = abilities.fec_cfg_curr_mod_ext_info &
+				    I40E_AQ_PHY_FEC_CONFIG_MASK;
 
 		/* save the requested speeds */
 		hw->phy.link_info.requested_speeds = config.link_speed;
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 8/8] Acquire NVM lock before reads on all devices

[Bimmy Pujari]

From: Aaron Salter <aaron.k.salter@intel.com>

Acquire NVM lock before reads on all devices.  Previously, locks were
only used for X722 and later.  Fixes an issue where simultaneous X710
NVM accesses were interfering with each other.

Signed-off-by: Aaron Salter <aaron.k.salter@intel.com>
Change-ID: If570bb7acf958cef58725ec2a2011cead6f80638
---
 drivers/net/ethernet/intel/i40e/i40e_nvm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
index 38ee18f..800bd55 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
@@ -292,14 +292,14 @@ i40e_status i40e_read_nvm_word(struct i40e_hw *hw, u16 offset,
 {
 	enum i40e_status_code ret_code = 0;
 
-	if (hw->flags & I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE) {
-		ret_code = i40e_acquire_nvm(hw, I40E_RESOURCE_READ);
-		if (!ret_code) {
+	ret_code = i40e_acquire_nvm(hw, I40E_RESOURCE_READ);
+	if (!ret_code) {
+		if (hw->flags & I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE) {
 			ret_code = i40e_read_nvm_word_aq(hw, offset, data);
-			i40e_release_nvm(hw);
+		} else {
+			ret_code = i40e_read_nvm_word_srctl(hw, offset, data);
 		}
-	} else {
-		ret_code = i40e_read_nvm_word_srctl(hw, offset, data);
+		i40e_release_nvm(hw);
 	}
 	return ret_code;
 }
-- 
2.4.11

[Intel-wired-lan] [next PATCH S56 1/8] i40e: Clean up dead code

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S56 1/8] i40e: Clean up dead code
> 
> From: Mitch Williams <mitch.a.williams@intel.com>
> 
> The function i40e_client_prepare() can never return an error. So make it void
> and quit checking its return value.
> 
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Change-ID: I9ff311e2324dde329eb68648efb2c94aaff856db
> ---
> Testing Hints : Found by static code analysis. Not possible to test, just code
> cleanup.
> 
>  drivers/net/ethernet/intel/i40e/i40e_client.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 2/8] i40e: Add bus number info to i40e_bus_info struct

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S56 2/8] i40e: Add bus number info to
> i40e_bus_info struct
> 
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> 
> Currently i40e_bus_info has PCI device and function info only and log
> messages print device number as bus number. Added field to provide bus
> number info and modified log statements to print bus, device and function
> information.
> 
> Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> Change-ID: I811617cee2714cc0d6bade8d369f57040990756f
> ---
>  drivers/net/ethernet/intel/i40e/i40e_client.c   | 16 +++++++++-------
>  drivers/net/ethernet/intel/i40e/i40e_main.c     |  1 +
>  drivers/net/ethernet/intel/i40e/i40e_osdep.h    | 12 ++++++------
>  drivers/net/ethernet/intel/i40e/i40e_type.h     |  1 +
>  drivers/net/ethernet/intel/i40evf/i40e_type.h   |  1 +
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c |  1 +
>  6 files changed, 19 insertions(+), 13 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 3/8] i40e: Save link FEC info from link up event

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Tieman, Henry W <henry.w.tieman@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S56 3/8] i40e: Save link FEC info from
> link up event
> 
> From: Henry Tieman <henry.w.tieman@intel.com>
> 
> Store the FEC status bits from the link up event into the hw_link_info
> structure.
> 
> Signed-off-by: Henry Tieman <henry.w.tieman@intel.com>
> Change-ID: I9a7b256f6dfb0dce89c2f503075d0d383526832e
> ---
> Testing Hints: In support of DCR 2228, this commit stores the FEC data for
> later display in the "link up" kernel log message.
> 
>  drivers/net/ethernet/intel/i40e/i40e_common.c |  2 ++
>  drivers/net/ethernet/intel/i40e/i40e_main.c   | 21 +++++++++++++++++++-
> -
>  drivers/net/ethernet/intel/i40e/i40e_type.h   |  1 +
>  drivers/net/ethernet/intel/i40evf/i40e_type.h |  1 +
>  4 files changed, 23 insertions(+), 2 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 4/8] i40e: don't warn every time we clear an Rx timestamp register

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S56 4/8] i40e: don't warn every time
> we clear an Rx timestamp register
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The intent of this message was to indicate to a user that we might have
> missed a timestamp event for a valid packet. The original method of
> detecting the missed events relied on waiting until all 4 registers were filled.
> 
> A recent commit d55458c0cd7a5 ("i40e: replace PTP Rx timestamp hang
> logic") replaced this logic with much better detection scheme that could
> detect a stalled Rx timestamp register even when other registers were still
> functional.
> 
> The new logic means that a message will be displayed almost as soon as a
> timestamp for a dropped frame occurs. This new logic highlights that the
> hardware will attempt timestamp for frames which it later decides to drop.
> The most prominent example is when a multicast PTP frame is received on a
> multicast address that we are not subscribed to.
> 
> Because the hardware initiates the Rx timestamp as soon as possible, it will
> latch an RXTIME register, but then drop the packet.
> 
> This results in users being confused by the message as they are not
> expecting to see dropped timestamp messages unless their application also
> indicates that timestamps were missing.
> 
> Resolve this by reducing the severity and frequency of the displayed
> message. We now only print the message if 3 or 4 of the RXTIME registers are
> stalled and get cleared within the same watchdog event. This ensures that
> the common case does not constantly display the message.
> Additionally, since the message is likely not as meaningful to most users,
> reduce the message to a dev_dbg instead of a dev_warn.
> 
> Users can still get a count of the number of timestamps dropped by reading
> the ethtool statistics value, if necessary.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: I35494442226a444c418dfb4f91a3070d06c8435c
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 5/8] i40e: allow i40e_update_filter_state to skip broadcast filters

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S56 5/8] i40e: allow
> i40e_update_filter_state to skip broadcast filters
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Fix a bug where we modified the mac_filter_hash while outside a lock, when
> handling addition of broadcast filters.
> 
> Normally, we add filters to firmware by batching the additions into lists and
> issuing 1 update for every few filters. Broadcast filters are handled
> differently, by instead setting the broadcast promiscuous mode flags. In
> order to make sure the 1<->1 mapping of filters in our addition array lined up
> with filters in the hlist tmp_add_list, we had to remove the filter and move it
> back to the main hash. However, we didn't do this under lock, which could
> cause consistency problems for the list.
> 
> Fix this by updating i40e_update_filter_state logic so that it knows to avoid
> broadcast filters. This ensures that we don't have to remove the filter
> separately, and can put it back using the normal flow.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Id288fade80b3e3a9a54b68cc249188cb95147518
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 37
> ++++++++++++++++++++++-------
>  1 file changed, 29 insertions(+), 8 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 6/8] i40e: avoid race condition when sending filters to firmware for addition

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S56 6/8] i40e: avoid race condition
> when sending filters to firmware for addition
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Refactor how we add new filters to firmware to avoid a race condition that
> can occur due to removing filters from the hash temporarily.
> 
> To understand the race condition, suppose that you have a number of MAC
> filters, but have not yet added any VLANs. Now, add two VLANs in rapid
> succession. A possible resulting flow would look something like the
> following:
> 
> (1) lock hash for add VLAN
> (2) add the new MAC/VLAN combos for each current MAC filter
> (3) unlock hash
> (4) lock hash for filter sync
> (5) notice that we have a VLAN, so prepare to update all MAC filters
>     with VLAN=-1 to be VLAN=0.
> (6) move NEW and REMOVE filters to temporary list
> (7) unlock hash
> (8) lock hash for add VLAN
> (9) add new MAC/VLAN combos. Notice that no MAC filters are currently in
>     the hash list, so we don't add any VLANs <--- BUG!
> (10) unlock hash
> (11) sync the temporary lists to firmware
> (12) lock hash for post-sync
> (13) move the temporary elements back to the main list ....
> 
> Because we take filters out of the main hash into temporary lists, we
> introduce a narrow window where it is possible that other callers to the list
> will not see some of the filters which were previously added but have not yet
> been finalized. This results in sometimes dropping VLAN additions, and could
> also result in failing to add a MAC address on the newly added VLAN.
> 
> One obvious way to avoid this race condition would be to lock the entire
> firmware process. Unfortunately this does not work because adminq
> firmware commands take a mutex which results in a sleep while atomic
> BUG(). So, we can't use the simplest approach.
> 
> An alternative approach is to simply not remove the filters from the hash list
> while adding. Instead, add an i40e_new_mac_filter structure which we will
> use to track added filters. This avoids the need to remove the filter from the
> hash list. We'll store a pointer to the original i40e_mac_filter, along with our
> own copy of the state.
> 
> We won't update the state directly, so as to avoid race with other code that
> may modify the state while under the lock. We are safe to read
> f->macaddr and f->vlan since these only change in two locations. The
> first is on filter creation, which must have already occurred. The second is
> inside i40e_correct_vlan_filters which was previously run after creation of
> this object and can't be run again until after. Thus, we should be safe to read
> the MAC address and VLAN while outside the lock.
> 
> We also aren't going to run into a use-after-free issue because the only place
> where we free filters is when they are marked FAILED or when we remove
> them inside the sync subtask. Since the subtask has its own critical flag to
> prevent duplicate runs, we know this won't happen. We also know that the
> only location to transition a filter from NEW to FAILED is inside the subtask
> also, so we aren't worried about that either.
> 
> Use the wrapper i40e_new_mac_filter for additions, and once we've
> finalized the addition to firmware, we will update the filter state inside a lock,
> and then free the wrapper structure.
> 
> In order to avoid a possible race condition with filter deletion, we won't
> update the original filter state unless it is still I40E_FILTER_NEW when we
> finish the firmware sync.
> 
> This approach is more complex, but avoids race conditions related to filters
> being temporarily removed from the list. We do not need the same behavior
> for deletion because we always unconditionally removed the filters from the
> list regardless of the firmware status.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-Id: I14b74bc2301f8e69433fbe77ebca532db20c5317
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h      |  16 +++
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 147 ++++++++++++++++++--
> --------
>  2 files changed, 112 insertions(+), 51 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 8/8] Acquire NVM lock before reads on all devices

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Salter, Aaron K <aaron.k.salter@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S56 8/8] Acquire NVM lock before
> reads on all devices
> 
> From: Aaron Salter <aaron.k.salter@intel.com>
> 
> Acquire NVM lock before reads on all devices.  Previously, locks were only
> used for X722 and later.  Fixes an issue where simultaneous X710 NVM
> accesses were interfering with each other.
> 
> Signed-off-by: Aaron Salter <aaron.k.salter@intel.com>
> Change-ID: If570bb7acf958cef58725ec2a2011cead6f80638
> ---
>  drivers/net/ethernet/intel/i40e/i40e_nvm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S56 7/8] i40e: Save more link abilities when using ethtool

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Friday, December 02, 2016 12:33 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Tieman, Henry W <henry.w.tieman@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S56 7/8] i40e: Save more link abilities
> when using ethtool
> 
> From: Henry Tieman <henry.w.tieman@intel.com>
> 
> Ethtool support needs to save more PHY information. The added information
> includes FEC capabilities and 25G link types. Without this change it is possible
> to lose 25G or FEC settings by using ethtool.
> 
> Signed-off-by: Henry Tieman <henry.w.tieman@intel.com>
> Change-ID: Ie42255b1e901ffbf9583b8c46466a54894114280
> ---
> Testing Hints: Found by code inspection. Test by changing link speed from
> 25G to 10G and back to 25G using ethtool.
> 
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 +++
>  1 file changed, 3 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>