[net-next v2 00/15][pull request] 40GbE Intel Wired LAN Driver Updates 2018-01-09

[net-next v2 00/15][pull request] 40GbE Intel Wired LAN Driver Updates 2018-01-09

[Jeff Kirsher]

This series contains updates to i40e and i40evf only.

Alice adds the displaying of priority xon/xoff packet stats, since we
were already keeping track of them.  Based on the recent changes, bump
the driver versions.

Jake changes how the driver determines whether or not the device is
currently up to resolve the possible issue of freeing data structures
and other memory before they have been fully allocated.  Refactored
the driver to simplify the locking behavior and to consistently use
spinlocks instead of an overloaded bit lock to protect MAC and filter
lists.  Created a helper function which can convert the AdminQ link
speed definition into a virtchnl definition.

Colin Ian King cleans up a redundant variable initialization.

Alex cleans up the driver to stop clearing the pending bit array for
each vector manually, since it is prone to dropping an interrupt and
based on the hardware specs, the pending bit array will be cleared
automatically in MSI-X mode.  Cleaned up flags for promiscuous mode to
resolve an issue where enabling & disabling promiscuous mode on a VF
would leave us in a high polling rate for the adminq task.  Cleaned up
code that was prone to race issues.

Jingjing renames pipeline personalization profile (ppp) to dynamic
device personalization (ddp) because it was being confused with the
well known point to point protocol.  Also removed checks for "track_id"
being zero, since it is valid for it to be zero for profiles that do
not have any 'write' commands.

Mariusz introduces a ethtool private flag used for forcing true link
state in order to workaround a firmware link establishment state
machine (LESM). 

v2: cleaned up commit message for patch 12 based on feedback from Sergei
    Shtylyov and Alex Duyck

The following are changes since commit f4803f1b73f877a571be4c8e531dfcf190acc691:
  net: tipc: remove unused hardirq.h
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE

Alexander Duyck (3):
  i40evf: Do not clear MSI-X PBA manually
  i40evf: Clean-up flags for promisc mode to avoid high polling rate
  i40evf: Drop i40evf_fire_sw_int as it is prone to races

Alice Michael (2):
  i40e: display priority_xon and priority_xoff stats
  i40e/i40evf: Bump driver versions

Colin Ian King (1):
  i40e: remove redundant initialization of read_size

Jacob Keller (6):
  i40evf: don't rely on netif_running() outside rtnl_lock()
  i40evf: use spinlock to protect (mac|vlan)_filter_list
  i40evf: release bit locks in reverse order
  i40evf: hold the critical task bit lock while opening
  i40e: update VFs of link state after GET_VF_RESOURCES
  i40e: add helper conversion function for link_speed

Jingjing Wu (2):
  i40e: change ppp name to ddp
  i40e: track id can be 0

Mariusz Stachura (1):
  i40e: link_down_on_close private flag support

 drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h  |  10 +-
 drivers/net/ethernet/intel/i40e/i40e_common.c      |  27 ++-
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   6 +
 drivers/net/ethernet/intel/i40e/i40e_main.c        |  75 +++++++-
 drivers/net/ethernet/intel/i40e/i40e_nvm.c         |   2 +-
 drivers/net/ethernet/intel/i40e/i40e_prototype.h   |  41 ++++-
 drivers/net/ethernet/intel/i40e/i40e_type.h        |  28 +--
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   5 +-
 .../net/ethernet/intel/i40evf/i40e_adminq_cmd.h    |  10 +-
 drivers/net/ethernet/intel/i40evf/i40e_common.c    |  29 ++-
 drivers/net/ethernet/intel/i40evf/i40e_prototype.h |  10 +-
 drivers/net/ethernet/intel/i40evf/i40e_type.h      |  28 +--
 drivers/net/ethernet/intel/i40evf/i40evf.h         |   4 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 196 ++++++++++-----------
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    |  48 ++++-
 15 files changed, 325 insertions(+), 194 deletions(-)

-- 
2.15.1

[net-next v2 01/15] i40e: display priority_xon and priority_xoff stats

[Jeff Kirsher]

From: Alice Michael <alice.michael@intel.com>

Display some more stats that were already being counted, to help users
understand when priority xon/xoff packets are being sent/received

Signed-off-by: Alice Michael <alice.michael@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index cfd788b4fd7a..34173f821fd9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -126,6 +126,10 @@ static const struct i40e_stats i40e_gstrings_stats[] = {
 	I40E_PF_STAT("link_xoff_rx", stats.link_xoff_rx),
 	I40E_PF_STAT("link_xon_tx", stats.link_xon_tx),
 	I40E_PF_STAT("link_xoff_tx", stats.link_xoff_tx),
+	I40E_PF_STAT("priority_xon_rx", stats.priority_xon_rx),
+	I40E_PF_STAT("priority_xoff_rx", stats.priority_xoff_rx),
+	I40E_PF_STAT("priority_xon_tx", stats.priority_xon_tx),
+	I40E_PF_STAT("priority_xoff_tx", stats.priority_xoff_tx),
 	I40E_PF_STAT("rx_size_64", stats.rx_size_64),
 	I40E_PF_STAT("rx_size_127", stats.rx_size_127),
 	I40E_PF_STAT("rx_size_255", stats.rx_size_255),
-- 
2.15.1

[net-next v2 02/15] i40evf: don't rely on netif_running() outside rtnl_lock()

[Jeff Kirsher]

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

In i40evf_reset_task we use netif_running() to determine whether or not
the device is currently up. This allows us to properly free queue memory
and shut down things before we request the hardware reset.

It turns out that we cannot be guaranteed of netif_running() returning
false until the device is fully up, as the kernel core code sets
__LINK_STATE_START prior to calling .ndo_open. Since we're not holding
the rtnl_lock(), it's possible that the driver's i40evf_open handler
function is currently being called while we're resetting.

We can't simply hold the rtnl_lock() while checking netif_running() as
this could cause a deadlock with the i40evf_open() function.
Additionally, we can't avoid the deadlock by holding the rtnl_lock()
over the whole reset path, as this essentially serializes all resets,
and can cause massive delays if we have multiple VFs on a system.

Instead, lets just check our own internal state __I40EVF_RUNNING state
field. This allows us to ensure that the state is correct and is only
set after we've finished bringing the device up.

Without this change we might free data structures about device queues
and other memory before they've been fully allocated.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 7b2a4eba92e2..0b23bf6d7873 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1796,7 +1796,11 @@ static void i40evf_disable_vf(struct i40evf_adapter *adapter)
 
 	adapter->flags |= I40EVF_FLAG_PF_COMMS_FAILED;
 
-	if (netif_running(adapter->netdev)) {
+	/* We don't use netif_running() because it may be true prior to
+	 * ndo_open() returning, so we can't assume it means all our open
+	 * tasks have finished, since we're not holding the rtnl_lock here.
+	 */
+	if (adapter->state == __I40EVF_RUNNING) {
 		set_bit(__I40E_VSI_DOWN, adapter->vsi.state);
 		netif_carrier_off(adapter->netdev);
 		netif_tx_disable(adapter->netdev);
@@ -1854,6 +1858,7 @@ static void i40evf_reset_task(struct work_struct *work)
 	struct i40evf_mac_filter *f;
 	u32 reg_val;
 	int i = 0, err;
+	bool running;
 
 	while (test_and_set_bit(__I40EVF_IN_CLIENT_TASK,
 				&adapter->crit_section))
@@ -1913,7 +1918,13 @@ static void i40evf_reset_task(struct work_struct *work)
 	}
 
 continue_reset:
-	if (netif_running(netdev)) {
+	/* We don't use netif_running() because it may be true prior to
+	 * ndo_open() returning, so we can't assume it means all our open
+	 * tasks have finished, since we're not holding the rtnl_lock here.
+	 */
+	running = (adapter->state == __I40EVF_RUNNING);
+
+	if (running) {
 		netif_carrier_off(netdev);
 		netif_tx_stop_all_queues(netdev);
 		adapter->link_up = false;
@@ -1964,7 +1975,10 @@ static void i40evf_reset_task(struct work_struct *work)
 
 	mod_timer(&adapter->watchdog_timer, jiffies + 2);
 
-	if (netif_running(adapter->netdev)) {
+	/* We were running when the reset started, so we need to restore some
+	 * state here.
+	 */
+	if (running) {
 		/* allocate transmit descriptors */
 		err = i40evf_setup_all_tx_resources(adapter);
 		if (err)
-- 
2.15.1

[net-next v2 03/15] i40evf: use spinlock to protect (mac|vlan)_filter_list

[Jeff Kirsher]

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

Stop overloading the __I40EVF_IN_CRITICAL_TASK bit lock to protect the
mac_filter_list and vlan_filter_list. Instead, implement a spinlock to
protect these two lists, similar to how we protect the hash in the i40e
PF code.

Ensure that every place where we access the list uses the spinlock to
ensure consistency, and stop holding the critical section around blocks
of code which only need access to the macvlan filter lists.

This refactor helps simplify the locking behavior, and is necessary as
a future refactor to the __I40EVF_IN_CRITICAL_TASK would cause
a deadlock otherwise.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf.h         |  4 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 84 +++++++++++-----------
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 42 +++++++++--
 3 files changed, 81 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index de0af521d602..47040ab2e298 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -199,6 +199,9 @@ struct i40evf_adapter {
 	wait_queue_head_t down_waitqueue;
 	struct i40e_q_vector *q_vectors;
 	struct list_head vlan_filter_list;
+	struct list_head mac_filter_list;
+	/* Lock to protect accesses to MAC and VLAN lists */
+	spinlock_t mac_vlan_list_lock;
 	char misc_vector_name[IFNAMSIZ + 9];
 	int num_active_queues;
 	int num_req_queues;
@@ -206,7 +209,6 @@ struct i40evf_adapter {
 	/* TX */
 	struct i40e_ring *tx_rings;
 	u32 tx_timeout_count;
-	struct list_head mac_filter_list;
 	u32 tx_desc_count;
 
 	/* RX */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 0b23bf6d7873..e15ec95260a1 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -706,7 +706,8 @@ static void i40evf_configure_rx(struct i40evf_adapter *adapter)
  * @adapter: board private structure
  * @vlan: vlan tag
  *
- * Returns ptr to the filter object or NULL
+ * Returns ptr to the filter object or NULL. Must be called while holding the
+ * mac_vlan_list_lock.
  **/
 static struct
 i40evf_vlan_filter *i40evf_find_vlan(struct i40evf_adapter *adapter, u16 vlan)
@@ -731,14 +732,8 @@ static struct
 i40evf_vlan_filter *i40evf_add_vlan(struct i40evf_adapter *adapter, u16 vlan)
 {
 	struct i40evf_vlan_filter *f = NULL;
-	int count = 50;
 
-	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
-				&adapter->crit_section)) {
-		udelay(1);
-		if (--count == 0)
-			goto out;
-	}
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
 
 	f = i40evf_find_vlan(adapter, vlan);
 	if (!f) {
@@ -755,8 +750,7 @@ i40evf_vlan_filter *i40evf_add_vlan(struct i40evf_adapter *adapter, u16 vlan)
 	}
 
 clearout:
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
-out:
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 	return f;
 }
 
@@ -768,21 +762,16 @@ i40evf_vlan_filter *i40evf_add_vlan(struct i40evf_adapter *adapter, u16 vlan)
 static void i40evf_del_vlan(struct i40evf_adapter *adapter, u16 vlan)
 {
 	struct i40evf_vlan_filter *f;
-	int count = 50;
 
-	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
-				&adapter->crit_section)) {
-		udelay(1);
-		if (--count == 0)
-			return;
-	}
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
 
 	f = i40evf_find_vlan(adapter, vlan);
 	if (f) {
 		f->remove = true;
 		adapter->aq_required |= I40EVF_FLAG_AQ_DEL_VLAN_FILTER;
 	}
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 }
 
 /**
@@ -824,7 +813,8 @@ static int i40evf_vlan_rx_kill_vid(struct net_device *netdev,
  * @adapter: board private structure
  * @macaddr: the MAC address
  *
- * Returns ptr to the filter object or NULL
+ * Returns ptr to the filter object or NULL. Must be called while holding the
+ * mac_vlan_list_lock.
  **/
 static struct
 i40evf_mac_filter *i40evf_find_filter(struct i40evf_adapter *adapter,
@@ -854,26 +844,17 @@ i40evf_mac_filter *i40evf_add_filter(struct i40evf_adapter *adapter,
 				     u8 *macaddr)
 {
 	struct i40evf_mac_filter *f;
-	int count = 50;
 
 	if (!macaddr)
 		return NULL;
 
-	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
-				&adapter->crit_section)) {
-		udelay(1);
-		if (--count == 0)
-			return NULL;
-	}
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
 
 	f = i40evf_find_filter(adapter, macaddr);
 	if (!f) {
 		f = kzalloc(sizeof(*f), GFP_ATOMIC);
-		if (!f) {
-			clear_bit(__I40EVF_IN_CRITICAL_TASK,
-				  &adapter->crit_section);
-			return NULL;
-		}
+		if (!f)
+			goto clearout;
 
 		ether_addr_copy(f->macaddr, macaddr);
 
@@ -884,7 +865,8 @@ i40evf_mac_filter *i40evf_add_filter(struct i40evf_adapter *adapter,
 		f->remove = false;
 	}
 
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+clearout:
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 	return f;
 }
 
@@ -911,12 +893,16 @@ static int i40evf_set_mac(struct net_device *netdev, void *p)
 	if (adapter->flags & I40EVF_FLAG_ADDR_SET_BY_PF)
 		return -EPERM;
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	f = i40evf_find_filter(adapter, hw->mac.addr);
 	if (f) {
 		f->remove = true;
 		adapter->aq_required |= I40EVF_FLAG_AQ_DEL_MAC_FILTER;
 	}
 
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	f = i40evf_add_filter(adapter, addr->sa_data);
 	if (f) {
 		ether_addr_copy(hw->mac.addr, addr->sa_data);
@@ -937,7 +923,6 @@ static void i40evf_set_rx_mode(struct net_device *netdev)
 	struct netdev_hw_addr *uca;
 	struct netdev_hw_addr *mca;
 	struct netdev_hw_addr *ha;
-	int count = 50;
 
 	/* add addr if not already in the filter list */
 	netdev_for_each_uc_addr(uca, netdev) {
@@ -947,16 +932,8 @@ static void i40evf_set_rx_mode(struct net_device *netdev)
 		i40evf_add_filter(adapter, mca->addr);
 	}
 
-	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
-				&adapter->crit_section)) {
-		udelay(1);
-		if (--count == 0) {
-			dev_err(&adapter->pdev->dev,
-				"Failed to get lock in %s\n", __func__);
-			return;
-		}
-	}
-	/* remove filter if not in netdev list */
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	list_for_each_entry_safe(f, ftmp, &adapter->mac_filter_list, list) {
 		netdev_for_each_mc_addr(mca, netdev)
 			if (ether_addr_equal(mca->addr, f->macaddr))
@@ -995,7 +972,7 @@ static void i40evf_set_rx_mode(struct net_device *netdev)
 		 adapter->flags & I40EVF_FLAG_ALLMULTI_ON)
 		adapter->aq_required |= I40EVF_FLAG_AQ_RELEASE_ALLMULTI;
 
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 }
 
 /**
@@ -1094,6 +1071,8 @@ void i40evf_down(struct i40evf_adapter *adapter)
 	i40evf_napi_disable_all(adapter);
 	i40evf_irq_disable(adapter);
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	/* remove all MAC filters */
 	list_for_each_entry(f, &adapter->mac_filter_list, list) {
 		f->remove = true;
@@ -1102,6 +1081,9 @@ void i40evf_down(struct i40evf_adapter *adapter)
 	list_for_each_entry(f, &adapter->vlan_filter_list, list) {
 		f->remove = true;
 	}
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	if (!(adapter->flags & I40EVF_FLAG_PF_COMMS_FAILED) &&
 	    adapter->state != __I40EVF_RESETTING) {
 		/* cancel any current operation */
@@ -1812,6 +1794,8 @@ static void i40evf_disable_vf(struct i40evf_adapter *adapter)
 		i40evf_free_all_rx_resources(adapter);
 	}
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	/* Delete all of the filters, both MAC and VLAN. */
 	list_for_each_entry_safe(f, ftmp, &adapter->mac_filter_list, list) {
 		list_del(&f->list);
@@ -1823,6 +1807,8 @@ static void i40evf_disable_vf(struct i40evf_adapter *adapter)
 		kfree(fv);
 	}
 
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_free_misc_irq(adapter);
 	i40evf_reset_interrupt_capability(adapter);
 	i40evf_free_queues(adapter);
@@ -1959,6 +1945,8 @@ static void i40evf_reset_task(struct work_struct *work)
 	adapter->aq_required |= I40EVF_FLAG_AQ_GET_CONFIG;
 	adapter->aq_required |= I40EVF_FLAG_AQ_MAP_VECTORS;
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	/* re-add all MAC filters */
 	list_for_each_entry(f, &adapter->mac_filter_list, list) {
 		f->add = true;
@@ -1967,6 +1955,9 @@ static void i40evf_reset_task(struct work_struct *work)
 	list_for_each_entry(vlf, &adapter->vlan_filter_list, list) {
 		vlf->add = true;
 	}
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_MAC_FILTER;
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
 	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
@@ -2957,6 +2948,8 @@ static int i40evf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	mutex_init(&hw->aq.asq_mutex);
 	mutex_init(&hw->aq.arq_mutex);
 
+	spin_lock_init(&adapter->mac_vlan_list_lock);
+
 	INIT_LIST_HEAD(&adapter->mac_filter_list);
 	INIT_LIST_HEAD(&adapter->vlan_filter_list);
 
@@ -3132,6 +3125,7 @@ static void i40evf_remove(struct pci_dev *pdev)
 	i40evf_free_all_rx_resources(adapter);
 	i40evf_free_queues(adapter);
 	kfree(adapter->vf_res);
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
 	/* If we got removed before an up/down sequence, we've got a filter
 	 * hanging out there that we need to get rid of.
 	 */
@@ -3144,6 +3138,8 @@ static void i40evf_remove(struct pci_dev *pdev)
 		kfree(f);
 	}
 
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	free_netdev(netdev);
 
 	pci_disable_pcie_error_reporting(pdev);
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 46c8b8a3907c..2719a057b211 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -433,12 +433,16 @@ void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 			adapter->current_op);
 		return;
 	}
+
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	list_for_each_entry(f, &adapter->mac_filter_list, list) {
 		if (f->add)
 			count++;
 	}
 	if (!count) {
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_MAC_FILTER;
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
 	}
 	adapter->current_op = VIRTCHNL_OP_ADD_ETH_ADDR;
@@ -456,8 +460,10 @@ void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 	}
 
 	veal = kzalloc(len, GFP_KERNEL);
-	if (!veal)
+	if (!veal) {
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
+	}
 
 	veal->vsi_id = adapter->vsi_res->vsi_id;
 	veal->num_elements = count;
@@ -472,6 +478,9 @@ void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 	}
 	if (!more)
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_MAC_FILTER;
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_send_pf_msg(adapter, VIRTCHNL_OP_ADD_ETH_ADDR,
 			   (u8 *)veal, len);
 	kfree(veal);
@@ -498,12 +507,16 @@ void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 			adapter->current_op);
 		return;
 	}
+
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	list_for_each_entry(f, &adapter->mac_filter_list, list) {
 		if (f->remove)
 			count++;
 	}
 	if (!count) {
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_MAC_FILTER;
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
 	}
 	adapter->current_op = VIRTCHNL_OP_DEL_ETH_ADDR;
@@ -520,8 +533,10 @@ void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 		more = true;
 	}
 	veal = kzalloc(len, GFP_KERNEL);
-	if (!veal)
+	if (!veal) {
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
+	}
 
 	veal->vsi_id = adapter->vsi_res->vsi_id;
 	veal->num_elements = count;
@@ -537,6 +552,9 @@ void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 	}
 	if (!more)
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_MAC_FILTER;
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_send_pf_msg(adapter, VIRTCHNL_OP_DEL_ETH_ADDR,
 			   (u8 *)veal, len);
 	kfree(veal);
@@ -564,12 +582,15 @@ void i40evf_add_vlans(struct i40evf_adapter *adapter)
 		return;
 	}
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	list_for_each_entry(f, &adapter->vlan_filter_list, list) {
 		if (f->add)
 			count++;
 	}
 	if (!count) {
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
 	}
 	adapter->current_op = VIRTCHNL_OP_ADD_VLAN;
@@ -586,8 +607,10 @@ void i40evf_add_vlans(struct i40evf_adapter *adapter)
 		more = true;
 	}
 	vvfl = kzalloc(len, GFP_KERNEL);
-	if (!vvfl)
+	if (!vvfl) {
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
+	}
 
 	vvfl->vsi_id = adapter->vsi_res->vsi_id;
 	vvfl->num_elements = count;
@@ -602,6 +625,9 @@ void i40evf_add_vlans(struct i40evf_adapter *adapter)
 	}
 	if (!more)
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_send_pf_msg(adapter, VIRTCHNL_OP_ADD_VLAN, (u8 *)vvfl, len);
 	kfree(vvfl);
 }
@@ -628,12 +654,15 @@ void i40evf_del_vlans(struct i40evf_adapter *adapter)
 		return;
 	}
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	list_for_each_entry(f, &adapter->vlan_filter_list, list) {
 		if (f->remove)
 			count++;
 	}
 	if (!count) {
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_VLAN_FILTER;
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
 	}
 	adapter->current_op = VIRTCHNL_OP_DEL_VLAN;
@@ -650,8 +679,10 @@ void i40evf_del_vlans(struct i40evf_adapter *adapter)
 		more = true;
 	}
 	vvfl = kzalloc(len, GFP_KERNEL);
-	if (!vvfl)
+	if (!vvfl) {
+		spin_unlock_bh(&adapter->mac_vlan_list_lock);
 		return;
+	}
 
 	vvfl->vsi_id = adapter->vsi_res->vsi_id;
 	vvfl->num_elements = count;
@@ -667,6 +698,9 @@ void i40evf_del_vlans(struct i40evf_adapter *adapter)
 	}
 	if (!more)
 		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_VLAN_FILTER;
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_send_pf_msg(adapter, VIRTCHNL_OP_DEL_VLAN, (u8 *)vvfl, len);
 	kfree(vvfl);
 }
-- 
2.15.1

[net-next v2 04/15] i40evf: release bit locks in reverse order

[Jeff Kirsher]

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

Although not strictly necessary, it is customary to reverse the order in
which we release locks that we acquire. This helps preserve lock
ordering during future refactors, which can help avoid potential
deadlock situations.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index e15ec95260a1..85e2232af750 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1960,8 +1960,8 @@ static void i40evf_reset_task(struct work_struct *work)
 
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_MAC_FILTER;
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 	clear_bit(__I40EVF_IN_CLIENT_TASK, &adapter->crit_section);
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 	i40evf_misc_irq_enable(adapter);
 
 	mod_timer(&adapter->watchdog_timer, jiffies + 2);
-- 
2.15.1

[net-next v2 05/15] i40evf: hold the critical task bit lock while opening

[Jeff Kirsher]

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

If i40evf_open() is called quickly at the same time as a reset occurs
(such as via ethtool) it is possible for the device to attempt to open
while a reset is in progress. This occurs because the driver was not
holding the critical task bit lock during i40evf_open, nor was it
holding it around the call to i40evf_up_complete() in
i40evf_reset_task().

We didn't hold the lock previously because calls to i40evf_down() would
take the bit lock directly, and this would have caused a deadlock.

To avoid this, we'll move the bit lock handling out of i40evf_down() and
into the callers of this function. Additionally, we'll now hold the bit
lock over the entire set of steps when going up or down, to ensure that
we remain consistent.

Ultimately this causes us to serialize the transitions between down and
up properly, and avoid changing status while we're resetting.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 40 +++++++++++++++++++------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 85e2232af750..1b2bbe65c933 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1035,6 +1035,8 @@ static void i40evf_configure(struct i40evf_adapter *adapter)
 /**
  * i40evf_up_complete - Finish the last steps of bringing up a connection
  * @adapter: board private structure
+ *
+ * Expects to be called while holding the __I40EVF_IN_CRITICAL_TASK bit lock.
  **/
 static void i40evf_up_complete(struct i40evf_adapter *adapter)
 {
@@ -1052,6 +1054,8 @@ static void i40evf_up_complete(struct i40evf_adapter *adapter)
 /**
  * i40e_down - Shutdown the connection processing
  * @adapter: board private structure
+ *
+ * Expects to be called while holding the __I40EVF_IN_CRITICAL_TASK bit lock.
  **/
 void i40evf_down(struct i40evf_adapter *adapter)
 {
@@ -1061,10 +1065,6 @@ void i40evf_down(struct i40evf_adapter *adapter)
 	if (adapter->state <= __I40EVF_DOWN_PENDING)
 		return;
 
-	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
-				&adapter->crit_section))
-		usleep_range(500, 1000);
-
 	netif_carrier_off(netdev);
 	netif_tx_disable(netdev);
 	adapter->link_up = false;
@@ -1097,7 +1097,6 @@ void i40evf_down(struct i40evf_adapter *adapter)
 		adapter->aq_required |= I40EVF_FLAG_AQ_DISABLE_QUEUES;
 	}
 
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 	mod_timer_pending(&adapter->watchdog_timer, jiffies + 1);
 }
 
@@ -1960,8 +1959,6 @@ static void i40evf_reset_task(struct work_struct *work)
 
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_MAC_FILTER;
 	adapter->aq_required |= I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
-	clear_bit(__I40EVF_IN_CLIENT_TASK, &adapter->crit_section);
-	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 	i40evf_misc_irq_enable(adapter);
 
 	mod_timer(&adapter->watchdog_timer, jiffies + 2);
@@ -1998,9 +1995,13 @@ static void i40evf_reset_task(struct work_struct *work)
 		adapter->state = __I40EVF_DOWN;
 		wake_up(&adapter->down_waitqueue);
 	}
+	clear_bit(__I40EVF_IN_CLIENT_TASK, &adapter->crit_section);
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 
 	return;
 reset_err:
+	clear_bit(__I40EVF_IN_CLIENT_TASK, &adapter->crit_section);
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 	dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n");
 	i40evf_close(netdev);
 }
@@ -2244,8 +2245,14 @@ static int i40evf_open(struct net_device *netdev)
 		return -EIO;
 	}
 
-	if (adapter->state != __I40EVF_DOWN)
-		return -EBUSY;
+	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
+				&adapter->crit_section))
+		usleep_range(500, 1000);
+
+	if (adapter->state != __I40EVF_DOWN) {
+		err = -EBUSY;
+		goto err_unlock;
+	}
 
 	/* allocate transmit descriptors */
 	err = i40evf_setup_all_tx_resources(adapter);
@@ -2269,6 +2276,8 @@ static int i40evf_open(struct net_device *netdev)
 
 	i40evf_irq_enable(adapter, true);
 
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+
 	return 0;
 
 err_req_irq:
@@ -2278,6 +2287,8 @@ static int i40evf_open(struct net_device *netdev)
 	i40evf_free_all_rx_resources(adapter);
 err_setup_tx:
 	i40evf_free_all_tx_resources(adapter);
+err_unlock:
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 
 	return err;
 }
@@ -2301,6 +2312,9 @@ static int i40evf_close(struct net_device *netdev)
 	if (adapter->state <= __I40EVF_DOWN_PENDING)
 		return 0;
 
+	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
+				&adapter->crit_section))
+		usleep_range(500, 1000);
 
 	set_bit(__I40E_VSI_DOWN, adapter->vsi.state);
 	if (CLIENT_ENABLED(adapter))
@@ -2310,6 +2324,8 @@ static int i40evf_close(struct net_device *netdev)
 	adapter->state = __I40EVF_DOWN_PENDING;
 	i40evf_free_traffic_irqs(adapter);
 
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+
 	/* We explicitly don't free resources here because the hardware is
 	 * still active and can DMA into memory. Resources are cleared in
 	 * i40evf_virtchnl_completion() after we get confirmation from the PF
@@ -2992,6 +3008,10 @@ static int i40evf_suspend(struct pci_dev *pdev, pm_message_t state)
 
 	netif_device_detach(netdev);
 
+	while (test_and_set_bit(__I40EVF_IN_CRITICAL_TASK,
+				&adapter->crit_section))
+		usleep_range(500, 1000);
+
 	if (netif_running(netdev)) {
 		rtnl_lock();
 		i40evf_down(adapter);
@@ -3000,6 +3020,8 @@ static int i40evf_suspend(struct pci_dev *pdev, pm_message_t state)
 	i40evf_free_misc_irq(adapter);
 	i40evf_reset_interrupt_capability(adapter);
 
+	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
+
 	retval = pci_save_state(pdev);
 	if (retval)
 		return retval;
-- 
2.15.1

[net-next v2 06/15] i40e: update VFs of link state after GET_VF_RESOURCES

[Jeff Kirsher]

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

We currently notify a VF of the link state after ENABLE_QUEUES, which is
the last thing a VF does after being configured. Guests may not actually
ENABLE_QUEUES until they get configured, and thus between driver load
and device configuration the VF may show inaccurate link status.

Fix this by also sending the link state after GET_VF_RESOURCES. Although
we could remove the message following ENABLE_QUEUES, it's not that
significant of a loss, so this patch just keeps both to ensure maximum
compatibility with guests on various OSes.

Specifically, without this patch guests running FreeBSD will display
inaccurate link state until the device is brought up. This is mostly
a cosmetic issue but can be confusing to system administrators.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 36cb8e068e85..74e9e5eb2351 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -2749,6 +2749,7 @@ int i40e_vc_process_vf_msg(struct i40e_pf *pf, s16 vf_id, u32 v_opcode,
 		break;
 	case VIRTCHNL_OP_GET_VF_RESOURCES:
 		ret = i40e_vc_get_vf_resources_msg(vf, msg);
+		i40e_vc_notify_vf_link_state(vf);
 		break;
 	case VIRTCHNL_OP_RESET_VF:
 		i40e_vc_reset_vf_msg(vf);
-- 
2.15.1

[net-next v2 07/15] i40e: add helper conversion function for link_speed

[Jeff Kirsher]

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

We introduced the virtchnl interface in order to have an interface for
talking to a virtual device driver which was host-driver agnostic. This
interface has its own definitions, including one for link speed.

The host driver has to talk to the virtchnl interface using these new
definitions in order to remain compatible. Today, the i40e link_speed
enumerations are value-exact matches for the virtchnl interface, so it
was originally decided to simply use a typecast.

However, this is unsafe, and makes it easier for future drivers to
continue this unsafe practice. There is nothing guaranteeing these
values are exact, and the type-cast would hide any compiler warning
which indicates the problem.

Rather than rely on this type cast, introduce a helper function which
can convert the AdminQ link speed definition into a virtchnl
definition. This can then be used by host driver implementations in
order to safely convert to the interface recognized by the virtual
functions.

If the link speed is not able to be represented by the virtchnl
definitions we'll report UNKNOWN which is the safest result.

This will ensure that should the driver specific link_speeds actual bit
definitions change, we do not report them incorrectly according to the
VF.

Additionally, this provides a better pattern for future drivers to copy,
as it is more likely a future device may not use the exact same bit-wise
definition as the current virtchnl interface.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_prototype.h   | 31 ++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  4 +--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index 3bb6659db822..e70bebc4d2a3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -343,6 +343,37 @@ static inline struct i40e_rx_ptype_decoded decode_rx_desc_ptype(u8 ptype)
 	return i40e_ptype_lookup[ptype];
 }
 
+/**
+ * i40e_virtchnl_link_speed - Convert AdminQ link_speed to virtchnl definition
+ * @link_speed: the speed to convert
+ *
+ * Returns the link_speed in terms of the virtchnl interface, for use in
+ * converting link_speed as reported by the AdminQ into the format used for
+ * talking to virtchnl devices. If we can't represent the link speed properly,
+ * report LINK_SPEED_UNKNOWN.
+ **/
+static inline enum virtchnl_link_speed
+i40e_virtchnl_link_speed(enum i40e_aq_link_speed link_speed)
+{
+	switch (link_speed) {
+	case I40E_LINK_SPEED_100MB:
+		return VIRTCHNL_LINK_SPEED_100MB;
+	case I40E_LINK_SPEED_1GB:
+		return VIRTCHNL_LINK_SPEED_1GB;
+	case I40E_LINK_SPEED_10GB:
+		return VIRTCHNL_LINK_SPEED_10GB;
+	case I40E_LINK_SPEED_40GB:
+		return VIRTCHNL_LINK_SPEED_40GB;
+	case I40E_LINK_SPEED_20GB:
+		return VIRTCHNL_LINK_SPEED_20GB;
+	case I40E_LINK_SPEED_25GB:
+		return VIRTCHNL_LINK_SPEED_25GB;
+	case I40E_LINK_SPEED_UNKNOWN:
+	default:
+		return VIRTCHNL_LINK_SPEED_UNKNOWN;
+	}
+}
+
 /* prototype for functions used for SW locks */
 
 /* i40e_common for VF drivers*/
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 74e9e5eb2351..e9309fb9084b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -81,12 +81,12 @@ static void i40e_vc_notify_vf_link_state(struct i40e_vf *vf)
 	if (vf->link_forced) {
 		pfe.event_data.link_event.link_status = vf->link_up;
 		pfe.event_data.link_event.link_speed =
-			(vf->link_up ? I40E_LINK_SPEED_40GB : 0);
+			(vf->link_up ? VIRTCHNL_LINK_SPEED_40GB : 0);
 	} else {
 		pfe.event_data.link_event.link_status =
 			ls->link_info & I40E_AQ_LINK_UP;
 		pfe.event_data.link_event.link_speed =
-			(enum virtchnl_link_speed)ls->link_speed;
+			i40e_virtchnl_link_speed(ls->link_speed);
 	}
 	i40e_aq_send_msg_to_vf(hw, abs_vf_id, VIRTCHNL_OP_EVENT,
 			       0, (u8 *)&pfe, sizeof(pfe), NULL);
-- 
2.15.1

[net-next v2 08/15] i40e/i40evf: Bump driver versions

[Jeff Kirsher]

From: Alice Michael <alice.michael@intel.com>

Bump the i40e driver from 2.1.14 to 2.3.2.

Bump the i40evf driver from 3.0.1 to 3.2.2

Signed-off-by: Alice Michael <alice.michael@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c     | 4 ++--
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 321d8be80871..666d2e7781e1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -47,8 +47,8 @@ static const char i40e_driver_string[] =
 #define DRV_KERN "-k"
 
 #define DRV_VERSION_MAJOR 2
-#define DRV_VERSION_MINOR 1
-#define DRV_VERSION_BUILD 14
+#define DRV_VERSION_MINOR 3
+#define DRV_VERSION_BUILD 2
 #define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
 	     __stringify(DRV_VERSION_MINOR) "." \
 	     __stringify(DRV_VERSION_BUILD)    DRV_KERN
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 1b2bbe65c933..aa50bd9fb760 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -45,8 +45,8 @@ static const char i40evf_driver_string[] =
 #define DRV_KERN "-k"
 
 #define DRV_VERSION_MAJOR 3
-#define DRV_VERSION_MINOR 0
-#define DRV_VERSION_BUILD 1
+#define DRV_VERSION_MINOR 2
+#define DRV_VERSION_BUILD 2
 #define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
 	     __stringify(DRV_VERSION_MINOR) "." \
 	     __stringify(DRV_VERSION_BUILD) \
-- 
2.15.1

[net-next v2 09/15] i40e: remove redundant initialization of read_size

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

Variable read_size is initialized and this value is never read, it is
instead set inside the do-loop, hence the initialization is redundant
and can be removed. Cleans up clang warning:

drivers/net/ethernet/intel/i40e/i40e_nvm.c:390:6: warning: Value stored
to 'read_size' during its initialization is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_nvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
index 7689c2ee0d46..425713fb72e5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
@@ -389,7 +389,7 @@ static i40e_status i40e_read_nvm_buffer_aq(struct i40e_hw *hw, u16 offset,
 					   u16 *words, u16 *data)
 {
 	i40e_status ret_code;
-	u16 read_size = *words;
+	u16 read_size;
 	bool last_cmd = false;
 	u16 words_read = 0;
 	u16 i = 0;
-- 
2.15.1

[net-next v2 10/15] i40evf: Do not clear MSI-X PBA manually

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

We should not be clearing the pending bit array for each vector manually.
The documentation for the hardware states that when in MSI-X mode the
pending bit array will be cleared automatically. Us clearing it ourselves
just results in multiple opportunities for us to drop an interrupt.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index aa50bd9fb760..5fff62a45323 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -276,8 +276,7 @@ void i40evf_irq_enable_queues(struct i40evf_adapter *adapter, u32 mask)
 		if (mask & BIT(i - 1)) {
 			wr32(hw, I40E_VFINT_DYN_CTLN1(i - 1),
 			     I40E_VFINT_DYN_CTLN1_INTENA_MASK |
-			     I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK |
-			     I40E_VFINT_DYN_CTLN1_CLEARPBA_MASK);
+			     I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK);
 		}
 	}
 }
@@ -296,16 +295,14 @@ static void i40evf_fire_sw_int(struct i40evf_adapter *adapter, u32 mask)
 	if (mask & 1) {
 		dyn_ctl = rd32(hw, I40E_VFINT_DYN_CTL01);
 		dyn_ctl |= I40E_VFINT_DYN_CTLN1_SWINT_TRIG_MASK |
-			   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK |
-			   I40E_VFINT_DYN_CTLN1_CLEARPBA_MASK;
+			   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK;
 		wr32(hw, I40E_VFINT_DYN_CTL01, dyn_ctl);
 	}
 	for (i = 1; i < adapter->num_msix_vectors; i++) {
 		if (mask & BIT(i)) {
 			dyn_ctl = rd32(hw, I40E_VFINT_DYN_CTLN1(i - 1));
 			dyn_ctl |= I40E_VFINT_DYN_CTLN1_SWINT_TRIG_MASK |
-				   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK |
-				   I40E_VFINT_DYN_CTLN1_CLEARPBA_MASK;
+				   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK;
 			wr32(hw, I40E_VFINT_DYN_CTLN1(i - 1), dyn_ctl);
 		}
 	}
@@ -337,15 +334,10 @@ static irqreturn_t i40evf_msix_aq(int irq, void *data)
 	struct net_device *netdev = data;
 	struct i40evf_adapter *adapter = netdev_priv(netdev);
 	struct i40e_hw *hw = &adapter->hw;
-	u32 val;
 
 	/* handle non-queue interrupts, these reads clear the registers */
-	val = rd32(hw, I40E_VFINT_ICR01);
-	val = rd32(hw, I40E_VFINT_ICR0_ENA1);
-
-	val = rd32(hw, I40E_VFINT_DYN_CTL01) |
-	      I40E_VFINT_DYN_CTL01_CLEARPBA_MASK;
-	wr32(hw, I40E_VFINT_DYN_CTL01, val);
+	rd32(hw, I40E_VFINT_ICR01);
+	rd32(hw, I40E_VFINT_ICR0_ENA1);
 
 	/* schedule work on the private workqueue */
 	schedule_work(&adapter->adminq_task);
-- 
2.15.1

[net-next v2 11/15] i40evf: Clean-up flags for promisc mode to avoid high polling rate

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

If you enabled and disabled promiscuous mode on a VF you could easily put
it into a state where it would start firing interrupts on all queues at a
rate of 50+ interrupts per second even though there was no traffic present.
The issue seems to have been a stray admin queue feature flag set that was
leaving us in a high polling rate for the adminq task.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 2719a057b211..feb95b62a077 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -739,8 +739,10 @@ void i40evf_set_promiscuous(struct i40evf_adapter *adapter, int flags)
 	}
 
 	if (!flags) {
-		adapter->flags &= ~I40EVF_FLAG_PROMISC_ON;
-		adapter->aq_required &= ~I40EVF_FLAG_AQ_RELEASE_PROMISC;
+		adapter->flags &= ~(I40EVF_FLAG_PROMISC_ON |
+				    I40EVF_FLAG_ALLMULTI_ON);
+		adapter->aq_required &= ~(I40EVF_FLAG_AQ_RELEASE_PROMISC |
+					  I40EVF_FLAG_AQ_RELEASE_ALLMULTI);
 		dev_info(&adapter->pdev->dev, "Leaving promiscuous mode\n");
 	}
 
-- 
2.15.1

[net-next v2 12/15] i40evf: Drop i40evf_fire_sw_int as it is prone to races

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

Having the interrupts firing while we are polling causes extra overhead and
isn't needed for most systems out there. If an interrupt is lost us
experiencing a 2s latency spike before recovering is still not acceptable
and masks the issue. We are better off just identifying systems that lose
interrupts and instead enable workarounds for those systems.

To that end I am dropping the code that was strobing the interrupts as
there is a narrow window where having them enabled can actually cause
race issues anyway where a few stray packets might get misses if the
interrupt is re-enabled and fires before we call napi_complete.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 34 -------------------------
 1 file changed, 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 5fff62a45323..f92587aba3c7 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -281,33 +281,6 @@ void i40evf_irq_enable_queues(struct i40evf_adapter *adapter, u32 mask)
 	}
 }
 
-/**
- * i40evf_fire_sw_int - Generate SW interrupt for specified vectors
- * @adapter: board private structure
- * @mask: bitmap of vectors to trigger
- **/
-static void i40evf_fire_sw_int(struct i40evf_adapter *adapter, u32 mask)
-{
-	struct i40e_hw *hw = &adapter->hw;
-	int i;
-	u32 dyn_ctl;
-
-	if (mask & 1) {
-		dyn_ctl = rd32(hw, I40E_VFINT_DYN_CTL01);
-		dyn_ctl |= I40E_VFINT_DYN_CTLN1_SWINT_TRIG_MASK |
-			   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK;
-		wr32(hw, I40E_VFINT_DYN_CTL01, dyn_ctl);
-	}
-	for (i = 1; i < adapter->num_msix_vectors; i++) {
-		if (mask & BIT(i)) {
-			dyn_ctl = rd32(hw, I40E_VFINT_DYN_CTLN1(i - 1));
-			dyn_ctl |= I40E_VFINT_DYN_CTLN1_SWINT_TRIG_MASK |
-				   I40E_VFINT_DYN_CTLN1_ITR_INDX_MASK;
-			wr32(hw, I40E_VFINT_DYN_CTLN1(i - 1), dyn_ctl);
-		}
-	}
-}
-
 /**
  * i40evf_irq_enable - Enable default interrupt generation settings
  * @adapter: board private structure
@@ -1743,13 +1716,6 @@ static void i40evf_watchdog_task(struct work_struct *work)
 	if (adapter->state == __I40EVF_RUNNING)
 		i40evf_request_stats(adapter);
 watchdog_done:
-	if (adapter->state == __I40EVF_RUNNING) {
-		i40evf_irq_enable_queues(adapter, ~0);
-		i40evf_fire_sw_int(adapter, 0xFF);
-	} else {
-		i40evf_fire_sw_int(adapter, 0x1);
-	}
-
 	clear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);
 restart_watchdog:
 	if (adapter->state == __I40EVF_REMOVE)
-- 
2.15.1

[net-next v2 13/15] i40e: change ppp name to ddp

[Jeff Kirsher]

From: Jingjing Wu <jingjing.wu@intel.com>

PPP name was going to be confusing since PPP already means point
to point protocol. It is decided to change pipeline personalization
profile(ppp) to dynamic device personalization(ddp).

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h  | 10 ++++----
 drivers/net/ethernet/intel/i40e/i40e_common.c      | 22 ++++++++---------
 drivers/net/ethernet/intel/i40e/i40e_prototype.h   | 10 ++++----
 drivers/net/ethernet/intel/i40e/i40e_type.h        | 28 +++++++++++-----------
 .../net/ethernet/intel/i40evf/i40e_adminq_cmd.h    | 10 ++++----
 drivers/net/ethernet/intel/i40evf/i40e_common.c    | 24 +++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_prototype.h | 10 ++++----
 drivers/net/ethernet/intel/i40evf/i40e_type.h      | 28 +++++++++++-----------
 8 files changed, 73 insertions(+), 69 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index b0188b8f91ba..c5776340517c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -198,7 +198,7 @@ enum i40e_admin_queue_opc {
 	i40e_aqc_opc_add_mirror_rule	= 0x0260,
 	i40e_aqc_opc_delete_mirror_rule	= 0x0261,
 
-	/* Pipeline Personalization Profile */
+	/* Dynamic Device Personalization */
 	i40e_aqc_opc_write_personalization_profile	= 0x0270,
 	i40e_aqc_opc_get_personalization_profile_list	= 0x0271,
 
@@ -1594,7 +1594,7 @@ struct i40e_aqc_add_delete_mirror_rule_completion {
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_add_delete_mirror_rule_completion);
 
-/* Pipeline Personalization Profile */
+/* Dynamic Device Personalization */
 struct i40e_aqc_write_personalization_profile {
 	u8      flags;
 	u8      reserved[3];
@@ -1605,7 +1605,7 @@ struct i40e_aqc_write_personalization_profile {
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_write_personalization_profile);
 
-struct i40e_aqc_write_ppp_resp {
+struct i40e_aqc_write_ddp_resp {
 	__le32 error_offset;
 	__le32 error_info;
 	__le32 addr_high;
@@ -1614,8 +1614,8 @@ struct i40e_aqc_write_ppp_resp {
 
 struct i40e_aqc_get_applied_profiles {
 	u8      flags;
-#define I40E_AQC_GET_PPP_GET_CONF	0x1
-#define I40E_AQC_GET_PPP_GET_RDPU_CONF	0x2
+#define I40E_AQC_GET_DDP_GET_CONF	0x1
+#define I40E_AQC_GET_DDP_GET_RDPU_CONF	0x2
 	u8      rsv[3];
 	__le32  reserved;
 	__le32  addr_high;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 095965f268bd..a196c3d305ab 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -5236,7 +5236,7 @@ i40e_status i40e_aq_get_phy_register(struct i40e_hw *hw,
 }
 
 /**
- * i40e_aq_write_ppp - Write pipeline personalization profile (ppp)
+ * i40e_aq_write_ddp - Write dynamic device personalization (ddp)
  * @hw: pointer to the hw struct
  * @buff: command buffer (size in bytes = buff_size)
  * @buff_size: buffer size in bytes
@@ -5246,7 +5246,7 @@ i40e_status i40e_aq_get_phy_register(struct i40e_hw *hw,
  * @cmd_details: pointer to command details structure or NULL
  **/
 enum
-i40e_status_code i40e_aq_write_ppp(struct i40e_hw *hw, void *buff,
+i40e_status_code i40e_aq_write_ddp(struct i40e_hw *hw, void *buff,
 				   u16 buff_size, u32 track_id,
 				   u32 *error_offset, u32 *error_info,
 				   struct i40e_asq_cmd_details *cmd_details)
@@ -5255,7 +5255,7 @@ i40e_status_code i40e_aq_write_ppp(struct i40e_hw *hw, void *buff,
 	struct i40e_aqc_write_personalization_profile *cmd =
 		(struct i40e_aqc_write_personalization_profile *)
 		&desc.params.raw;
-	struct i40e_aqc_write_ppp_resp *resp;
+	struct i40e_aqc_write_ddp_resp *resp;
 	i40e_status status;
 
 	i40e_fill_default_direct_cmd_desc(&desc,
@@ -5271,7 +5271,7 @@ i40e_status_code i40e_aq_write_ppp(struct i40e_hw *hw, void *buff,
 
 	status = i40e_asq_send_command(hw, &desc, buff, buff_size, cmd_details);
 	if (!status) {
-		resp = (struct i40e_aqc_write_ppp_resp *)&desc.params.raw;
+		resp = (struct i40e_aqc_write_ddp_resp *)&desc.params.raw;
 		if (error_offset)
 			*error_offset = le32_to_cpu(resp->error_offset);
 		if (error_info)
@@ -5282,14 +5282,14 @@ i40e_status_code i40e_aq_write_ppp(struct i40e_hw *hw, void *buff,
 }
 
 /**
- * i40e_aq_get_ppp_list - Read pipeline personalization profile (ppp)
+ * i40e_aq_get_ddp_list - Read dynamic device personalization (ddp)
  * @hw: pointer to the hw struct
  * @buff: command buffer (size in bytes = buff_size)
  * @buff_size: buffer size in bytes
  * @cmd_details: pointer to command details structure or NULL
  **/
 enum
-i40e_status_code i40e_aq_get_ppp_list(struct i40e_hw *hw, void *buff,
+i40e_status_code i40e_aq_get_ddp_list(struct i40e_hw *hw, void *buff,
 				      u16 buff_size, u8 flags,
 				      struct i40e_asq_cmd_details *cmd_details)
 {
@@ -5378,7 +5378,7 @@ i40e_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 				break;
 	}
 	if (i == dev_cnt) {
-		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Device doesn't support PPP");
+		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Device doesn't support DDP");
 		return I40E_ERR_DEVICE_NOT_SUPPORTED;
 	}
 
@@ -5397,7 +5397,7 @@ i40e_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 			sizeof(struct i40e_profile_section_header);
 
 		/* Write profile */
-		status = i40e_aq_write_ppp(hw, (void *)sec, (u16)section_size,
+		status = i40e_aq_write_ddp(hw, (void *)sec, (u16)section_size,
 					   track_id, &offset, &info, NULL);
 		if (status) {
 			i40e_debug(hw, I40E_DEBUG_PACKAGE,
@@ -5439,10 +5439,10 @@ i40e_add_pinfo_to_list(struct i40e_hw *hw,
 					     sec->section.offset);
 	pinfo->track_id = track_id;
 	pinfo->version = profile->version;
-	pinfo->op = I40E_PPP_ADD_TRACKID;
-	memcpy(pinfo->name, profile->name, I40E_PPP_NAME_SIZE);
+	pinfo->op = I40E_DDP_ADD_TRACKID;
+	memcpy(pinfo->name, profile->name, I40E_DDP_NAME_SIZE);
 
-	status = i40e_aq_write_ppp(hw, (void *)sec, sec->data_end,
+	status = i40e_aq_write_ddp(hw, (void *)sec, sec->data_end,
 				   track_id, &offset, &info, NULL);
 
 	return status;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index e70bebc4d2a3..b3cc89cc3a86 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -431,13 +431,15 @@ i40e_status i40e_write_phy_register(struct i40e_hw *hw, u8 page, u16 reg,
 u8 i40e_get_phy_address(struct i40e_hw *hw, u8 dev_num);
 i40e_status i40e_blink_phy_link_led(struct i40e_hw *hw,
 				    u32 time, u32 interval);
-i40e_status i40e_aq_write_ppp(struct i40e_hw *hw, void *buff,
+i40e_status i40e_aq_write_ddp(struct i40e_hw *hw, void *buff,
 			      u16 buff_size, u32 track_id,
 			      u32 *error_offset, u32 *error_info,
-			      struct i40e_asq_cmd_details *cmd_details);
-i40e_status i40e_aq_get_ppp_list(struct i40e_hw *hw, void *buff,
+			      struct i40e_asq_cmd_details *
+			      cmd_details);
+i40e_status i40e_aq_get_ddp_list(struct i40e_hw *hw, void *buff,
 				 u16 buff_size, u8 flags,
-				 struct i40e_asq_cmd_details *cmd_details);
+				 struct i40e_asq_cmd_details *
+				 cmd_details);
 struct i40e_generic_seg_header *
 i40e_find_segment_in_package(u32 segment_type,
 			     struct i40e_package_header *pkg_header);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 0e8568719b4e..5a708c363d99 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -1502,19 +1502,19 @@ struct i40e_lldp_variables {
 #define I40E_FLEX_57_SHIFT		6
 #define I40E_FLEX_57_MASK		(0x1ULL << I40E_FLEX_57_SHIFT)
 
-/* Version format for PPP */
-struct i40e_ppp_version {
+/* Version format for Dynamic Device Personalization(DDP) */
+struct i40e_ddp_version {
 	u8 major;
 	u8 minor;
 	u8 update;
 	u8 draft;
 };
 
-#define I40E_PPP_NAME_SIZE	32
+#define I40E_DDP_NAME_SIZE	32
 
 /* Package header */
 struct i40e_package_header {
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 segment_count;
 	u32 segment_offset[1];
 };
@@ -1526,16 +1526,16 @@ struct i40e_generic_seg_header {
 #define SEGMENT_TYPE_I40E	0x00000011
 #define SEGMENT_TYPE_X722	0x00000012
 	u32 type;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 size;
-	char name[I40E_PPP_NAME_SIZE];
+	char name[I40E_DDP_NAME_SIZE];
 };
 
 struct i40e_metadata_segment {
 	struct i40e_generic_seg_header header;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 track_id;
-	char name[I40E_PPP_NAME_SIZE];
+	char name[I40E_DDP_NAME_SIZE];
 };
 
 struct i40e_device_id_entry {
@@ -1545,8 +1545,8 @@ struct i40e_device_id_entry {
 
 struct i40e_profile_segment {
 	struct i40e_generic_seg_header header;
-	struct i40e_ppp_version version;
-	char name[I40E_PPP_NAME_SIZE];
+	struct i40e_ddp_version version;
+	char name[I40E_DDP_NAME_SIZE];
 	u32 device_table_count;
 	struct i40e_device_id_entry device_table[1];
 };
@@ -1573,11 +1573,11 @@ struct i40e_profile_section_header {
 
 struct i40e_profile_info {
 	u32 track_id;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u8 op;
-#define I40E_PPP_ADD_TRACKID		0x01
-#define I40E_PPP_REMOVE_TRACKID	0x02
+#define I40E_DDP_ADD_TRACKID		0x01
+#define I40E_DDP_REMOVE_TRACKID	0x02
 	u8 reserved[7];
-	u8 name[I40E_PPP_NAME_SIZE];
+	u8 name[I40E_DDP_NAME_SIZE];
 };
 #endif /* _I40E_TYPE_H_ */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
index 06b04572c518..435a112d09f5 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
@@ -198,7 +198,7 @@ enum i40e_admin_queue_opc {
 	i40e_aqc_opc_add_mirror_rule	= 0x0260,
 	i40e_aqc_opc_delete_mirror_rule	= 0x0261,
 
-	/* Pipeline Personalization Profile */
+	/* Dynamic Device Personalization */
 	i40e_aqc_opc_write_personalization_profile	= 0x0270,
 	i40e_aqc_opc_get_personalization_profile_list	= 0x0271,
 
@@ -1562,7 +1562,7 @@ struct i40e_aqc_add_delete_mirror_rule_completion {
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_add_delete_mirror_rule_completion);
 
-/* Pipeline Personalization Profile */
+/* Dynamic Device Personalization */
 struct i40e_aqc_write_personalization_profile {
 	u8      flags;
 	u8      reserved[3];
@@ -1573,7 +1573,7 @@ struct i40e_aqc_write_personalization_profile {
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_write_personalization_profile);
 
-struct i40e_aqc_write_ppp_resp {
+struct i40e_aqc_write_ddp_resp {
 	__le32 error_offset;
 	__le32 error_info;
 	__le32 addr_high;
@@ -1582,8 +1582,8 @@ struct i40e_aqc_write_ppp_resp {
 
 struct i40e_aqc_get_applied_profiles {
 	u8      flags;
-#define I40E_AQC_GET_PPP_GET_CONF	0x1
-#define I40E_AQC_GET_PPP_GET_RDPU_CONF	0x2
+#define I40E_AQC_GET_DDP_GET_CONF	0x1
+#define I40E_AQC_GET_DDP_GET_RDPU_CONF	0x2
 	u8      rsv[3];
 	__le32  reserved;
 	__le32  addr_high;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index 7d70bf69b249..c1e473219957 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -1202,7 +1202,7 @@ i40e_status i40e_vf_reset(struct i40e_hw *hw)
 }
 
 /**
- * i40evf_aq_write_ppp - Write pipeline personalization profile (ppp)
+ * i40evf_aq_write_ddp - Write dynamic device personalization (ddp)
  * @hw: pointer to the hw struct
  * @buff: command buffer (size in bytes = buff_size)
  * @buff_size: buffer size in bytes
@@ -1212,7 +1212,7 @@ i40e_status i40e_vf_reset(struct i40e_hw *hw)
  * @cmd_details: pointer to command details structure or NULL
  **/
 enum
-i40e_status_code i40evf_aq_write_ppp(struct i40e_hw *hw, void *buff,
+i40e_status_code i40evf_aq_write_ddp(struct i40e_hw *hw, void *buff,
 				     u16 buff_size, u32 track_id,
 				     u32 *error_offset, u32 *error_info,
 				     struct i40e_asq_cmd_details *cmd_details)
@@ -1221,7 +1221,7 @@ i40e_status_code i40evf_aq_write_ppp(struct i40e_hw *hw, void *buff,
 	struct i40e_aqc_write_personalization_profile *cmd =
 		(struct i40e_aqc_write_personalization_profile *)
 		&desc.params.raw;
-	struct i40e_aqc_write_ppp_resp *resp;
+	struct i40e_aqc_write_ddp_resp *resp;
 	i40e_status status;
 
 	i40evf_fill_default_direct_cmd_desc(&desc,
@@ -1237,7 +1237,7 @@ i40e_status_code i40evf_aq_write_ppp(struct i40e_hw *hw, void *buff,
 
 	status = i40evf_asq_send_command(hw, &desc, buff, buff_size, cmd_details);
 	if (!status) {
-		resp = (struct i40e_aqc_write_ppp_resp *)&desc.params.raw;
+		resp = (struct i40e_aqc_write_ddp_resp *)&desc.params.raw;
 		if (error_offset)
 			*error_offset = le32_to_cpu(resp->error_offset);
 		if (error_info)
@@ -1248,16 +1248,16 @@ i40e_status_code i40evf_aq_write_ppp(struct i40e_hw *hw, void *buff,
 }
 
 /**
- * i40evf_aq_get_ppp_list - Read pipeline personalization profile (ppp)
+ * i40evf_aq_get_ddp_list - Read dynamic device personalization (ddp)
  * @hw: pointer to the hw struct
  * @buff: command buffer (size in bytes = buff_size)
  * @buff_size: buffer size in bytes
  * @cmd_details: pointer to command details structure or NULL
  **/
 enum
-i40e_status_code i40evf_aq_get_ppp_list(struct i40e_hw *hw, void *buff,
+i40e_status_code i40evf_aq_get_ddp_list(struct i40e_hw *hw, void *buff,
 					u16 buff_size, u8 flags,
-				      struct i40e_asq_cmd_details *cmd_details)
+				       struct i40e_asq_cmd_details *cmd_details)
 {
 	struct i40e_aq_desc desc;
 	struct i40e_aqc_get_applied_profiles *cmd =
@@ -1344,7 +1344,7 @@ i40evf_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 				break;
 	}
 	if (i == dev_cnt) {
-		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Device doesn't support PPP");
+		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Device doesn't support DDP");
 		return I40E_ERR_DEVICE_NOT_SUPPORTED;
 	}
 
@@ -1363,7 +1363,7 @@ i40evf_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 			sizeof(struct i40e_profile_section_header);
 
 		/* Write profile */
-		status = i40evf_aq_write_ppp(hw, (void *)sec, (u16)section_size,
+		status = i40evf_aq_write_ddp(hw, (void *)sec, (u16)section_size,
 					     track_id, &offset, &info, NULL);
 		if (status) {
 			i40e_debug(hw, I40E_DEBUG_PACKAGE,
@@ -1405,10 +1405,10 @@ i40evf_add_pinfo_to_list(struct i40e_hw *hw,
 					     sec->section.offset);
 	pinfo->track_id = track_id;
 	pinfo->version = profile->version;
-	pinfo->op = I40E_PPP_ADD_TRACKID;
-	memcpy(pinfo->name, profile->name, I40E_PPP_NAME_SIZE);
+	pinfo->op = I40E_DDP_ADD_TRACKID;
+	memcpy(pinfo->name, profile->name, I40E_DDP_NAME_SIZE);
 
-	status = i40evf_aq_write_ppp(hw, (void *)sec, sec->data_end,
+	status = i40evf_aq_write_ddp(hw, (void *)sec, sec->data_end,
 				     track_id, &offset, &info, NULL);
 	return status;
 }
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_prototype.h b/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
index b624b5994075..47c429931a57 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
@@ -131,13 +131,15 @@ i40e_status i40e_write_phy_register(struct i40e_hw *hw, u8 page, u16 reg,
 u8 i40e_get_phy_address(struct i40e_hw *hw, u8 dev_num);
 i40e_status i40e_blink_phy_link_led(struct i40e_hw *hw,
 				    u32 time, u32 interval);
-i40e_status i40evf_aq_write_ppp(struct i40e_hw *hw, void *buff,
+i40e_status i40evf_aq_write_ddp(struct i40e_hw *hw, void *buff,
 				u16 buff_size, u32 track_id,
 				u32 *error_offset, u32 *error_info,
-				struct i40e_asq_cmd_details *cmd_details);
-i40e_status i40evf_aq_get_ppp_list(struct i40e_hw *hw, void *buff,
+				struct i40e_asq_cmd_details *
+				cmd_details);
+i40e_status i40evf_aq_get_ddp_list(struct i40e_hw *hw, void *buff,
 				   u16 buff_size, u8 flags,
-				   struct i40e_asq_cmd_details *cmd_details);
+				   struct i40e_asq_cmd_details *
+				   cmd_details);
 struct i40e_generic_seg_header *
 i40evf_find_segment_in_package(u32 segment_type,
 			       struct i40e_package_header *pkg_header);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_type.h b/drivers/net/ethernet/intel/i40evf/i40e_type.h
index 213b773dfad6..6afc31616e04 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_type.h
@@ -1422,19 +1422,19 @@ enum i40e_reset_type {
 #define I40E_FD_INSET_FLEX_WORD57_MASK		(0x1ULL << \
 					I40E_FD_INSET_FLEX_WORD57_SHIFT)
 
-/* Version format for PPP */
-struct i40e_ppp_version {
+/* Version format for Dynamic Device Personalization(DDP) */
+struct i40e_ddp_version {
 	u8 major;
 	u8 minor;
 	u8 update;
 	u8 draft;
 };
 
-#define I40E_PPP_NAME_SIZE	32
+#define I40E_DDP_NAME_SIZE	32
 
 /* Package header */
 struct i40e_package_header {
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 segment_count;
 	u32 segment_offset[1];
 };
@@ -1446,16 +1446,16 @@ struct i40e_generic_seg_header {
 #define SEGMENT_TYPE_I40E	0x00000011
 #define SEGMENT_TYPE_X722	0x00000012
 	u32 type;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 size;
-	char name[I40E_PPP_NAME_SIZE];
+	char name[I40E_DDP_NAME_SIZE];
 };
 
 struct i40e_metadata_segment {
 	struct i40e_generic_seg_header header;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u32 track_id;
-	char name[I40E_PPP_NAME_SIZE];
+	char name[I40E_DDP_NAME_SIZE];
 };
 
 struct i40e_device_id_entry {
@@ -1465,8 +1465,8 @@ struct i40e_device_id_entry {
 
 struct i40e_profile_segment {
 	struct i40e_generic_seg_header header;
-	struct i40e_ppp_version version;
-	char name[I40E_PPP_NAME_SIZE];
+	struct i40e_ddp_version version;
+	char name[I40E_DDP_NAME_SIZE];
 	u32 device_table_count;
 	struct i40e_device_id_entry device_table[1];
 };
@@ -1493,11 +1493,11 @@ struct i40e_profile_section_header {
 
 struct i40e_profile_info {
 	u32 track_id;
-	struct i40e_ppp_version version;
+	struct i40e_ddp_version version;
 	u8 op;
-#define I40E_PPP_ADD_TRACKID		0x01
-#define I40E_PPP_REMOVE_TRACKID	0x02
+#define I40E_DDP_ADD_TRACKID		0x01
+#define I40E_DDP_REMOVE_TRACKID	0x02
 	u8 reserved[7];
-	u8 name[I40E_PPP_NAME_SIZE];
+	u8 name[I40E_DDP_NAME_SIZE];
 };
 #endif /* _I40E_TYPE_H_ */
-- 
2.15.1

[net-next v2 14/15] i40e: track id can be 0

[Jeff Kirsher]

From: Jingjing Wu <jingjing.wu@intel.com>

track_id == 0 is valid for “read only” profiles when
profile does not have any “write” commands.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c   | 5 -----
 drivers/net/ethernet/intel/i40evf/i40e_common.c | 5 -----
 2 files changed, 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index a196c3d305ab..40c5f7628aa1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -5364,11 +5364,6 @@ i40e_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 	u32 offset = 0, info = 0;
 	u32 i;
 
-	if (!track_id) {
-		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Track_id can't be 0.");
-		return I40E_NOT_SUPPORTED;
-	}
-
 	dev_cnt = profile->device_table_count;
 
 	for (i = 0; i < dev_cnt; i++) {
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index c1e473219957..a94648429a5b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -1330,11 +1330,6 @@ i40evf_write_profile(struct i40e_hw *hw, struct i40e_profile_segment *profile,
 	u32 offset = 0, info = 0;
 	u32 i;
 
-	if (!track_id) {
-		i40e_debug(hw, I40E_DEBUG_PACKAGE, "Track_id can't be 0.");
-		return I40E_NOT_SUPPORTED;
-	}
-
 	dev_cnt = profile->device_table_count;
 
 	for (i = 0; i < dev_cnt; i++) {
-- 
2.15.1

[net-next v2 15/15] i40e: link_down_on_close private flag support

[Jeff Kirsher]

From: Mariusz Stachura <mariusz.stachura@intel.com>

This patch introduces new ethtool private flag used for
forcing true link state. Function i40e_force_link_state that implements
this functionality was added, it sets phy_type = 0 in order to
work-around firmware's LESM. False positive error messages were
suppressed.

Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  2 +
 drivers/net/ethernet/intel/i40e/i40e_main.c    | 71 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 34173f821fd9..50908f343221 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -230,6 +230,8 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
 	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
 	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
 	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 0),
+	I40E_PRIV_FLAG("link-down-on-close",
+		       I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED, 0),
 	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
 	I40E_PRIV_FLAG("disable-source-pruning",
 		       I40E_FLAG_SOURCE_PRUNING_DISABLED, 0),
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 666d2e7781e1..41fbccd41641 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6600,6 +6600,72 @@ int i40e_up(struct i40e_vsi *vsi)
 	return err;
 }
 
+/**
+ * i40e_force_link_state - Force the link status
+ * @pf: board private structure
+ * @is_up: whether the link state should be forced up or down
+ **/
+static void i40e_force_link_state(struct i40e_pf *pf, bool is_up)
+{
+	struct i40e_aq_get_phy_abilities_resp abilities;
+	struct i40e_aq_set_phy_config config = {0};
+	struct i40e_hw *hw = &pf->hw;
+	enum i40e_aq_phy_type cnt;
+	u64 mask = 0;
+	i40e_status err;
+
+	/* Get the current phy config */
+	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
+					   NULL);
+	if (err)
+		dev_dbg(&pf->pdev->dev,
+			"failed to get phy cap., ret =  %s last_status =  %s\n",
+			i40e_stat_str(hw, err),
+			i40e_aq_str(hw, hw->aq.asq_last_status));
+
+	/* If link needs to go up, but was not forced to go down,
+	 * no need for a flap
+	 */
+	if (is_up && abilities.phy_type != 0)
+		return;
+
+	/* To force link we need to set bits for all supported PHY types,
+	 * but there are now more than 32, so we need to split the bitmap
+	 * across two fields.
+	 */
+	for (cnt = I40E_PHY_TYPE_SGMII; cnt < I40E_PHY_TYPE_MAX; cnt++)
+		mask |= (1ULL << cnt);
+
+	config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
+	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
+	/* Copy the old settings, except of phy_type */
+	config.abilities = abilities.abilities;
+	config.link_speed = abilities.link_speed;
+	config.eee_capability = abilities.eee_capability;
+	config.eeer = abilities.eeer_val;
+	config.low_power_ctrl = abilities.d3_lpan;
+	err = i40e_aq_set_phy_config(hw, &config, NULL);
+
+	if (err)
+		dev_dbg(&pf->pdev->dev,
+			"set phy config ret =  %s last_status =  %s\n",
+			i40e_stat_str(&pf->hw, err),
+			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
+
+	/* Update the link info */
+	err = i40e_update_link_info(hw);
+	if (err) {
+		/* Wait a little bit (on 40G cards it sometimes takes a really
+		 * long time for link to come back from the atomic reset)
+		 * and try once more
+		 */
+		msleep(1000);
+		i40e_update_link_info(hw);
+	}
+
+	i40e_aq_set_link_restart_an(hw, true, NULL);
+}
+
 /**
  * i40e_down - Shutdown the connection processing
  * @vsi: the VSI being stopped
@@ -6617,6 +6683,9 @@ void i40e_down(struct i40e_vsi *vsi)
 	}
 	i40e_vsi_disable_irq(vsi);
 	i40e_vsi_stop_rings(vsi);
+	if (vsi->type == I40E_VSI_MAIN &&
+	    vsi->back->flags & I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED)
+		i40e_force_link_state(vsi->back, false);
 	i40e_napi_disable_all(vsi);
 
 	for (i = 0; i < vsi->num_queue_pairs; i++) {
@@ -7578,6 +7647,8 @@ int i40e_open(struct net_device *netdev)
 
 	netif_carrier_off(netdev);
 
+	i40e_force_link_state(pf, true);
+
 	err = i40e_vsi_open(vsi);
 	if (err)
 		return err;
-- 
2.15.1

Re: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Jakub Kicinski]

On Tue,  9 Jan 2018 08:19:30 -0800, Jeff Kirsher wrote:
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> This patch introduces new ethtool private flag used for
> forcing true link state. Function i40e_force_link_state that implements
> this functionality was added, it sets phy_type = 0 in order to
> work-around firmware's LESM. False positive error messages were
> suppressed.

Hi, could you say a bit more about how this works?  Last week I've been
actually trying to use a Fortville connected by a DAC to another card
to test link state on that other.  ifconfig down does not bring the
link down on the Fortville, apparently.  BMC is configured to only use
LOM, no PXE etc.

With this patch I assume I will be able to force the link down?  What
is keeping the link up otherwise?  You mention phy_type, is this
phy-specific?  Or are you using the phy_type purely as a way of fooling
FW to do what you want?

Few nits below..

> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  2 +
>  drivers/net/ethernet/intel/i40e/i40e_main.c    | 71 ++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 34173f821fd9..50908f343221 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -230,6 +230,8 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
>  	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
>  	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
>  	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 0),
> +	I40E_PRIV_FLAG("link-down-on-close",
> +		       I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED, 0),
>  	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
>  	I40E_PRIV_FLAG("disable-source-pruning",
>  		       I40E_FLAG_SOURCE_PRUNING_DISABLED, 0),
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 666d2e7781e1..41fbccd41641 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -6600,6 +6600,72 @@ int i40e_up(struct i40e_vsi *vsi)
>  	return err;
>  }
>  
> +/**
> + * i40e_force_link_state - Force the link status
> + * @pf: board private structure
> + * @is_up: whether the link state should be forced up or down
> + **/
> +static void i40e_force_link_state(struct i40e_pf *pf, bool is_up)
> +{
> +	struct i40e_aq_get_phy_abilities_resp abilities;
> +	struct i40e_aq_set_phy_config config = {0};
> +	struct i40e_hw *hw = &pf->hw;
> +	enum i40e_aq_phy_type cnt;
> +	u64 mask = 0;
> +	i40e_status err;

nit: reverse christmas tree

> +	/* Get the current phy config */
> +	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
> +					   NULL);

Are abilities guaranteed to be filled in by this function?  Otherwise
you will use uninitialized stack memory.

> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"failed to get phy cap., ret =  %s last_status =  %s\n",
> +			i40e_stat_str(hw, err),
> +			i40e_aq_str(hw, hw->aq.asq_last_status));

so getting 'abilities' can fail here...

> +	/* If link needs to go up, but was not forced to go down,
> +	 * no need for a flap
> +	 */
> +	if (is_up && abilities.phy_type != 0)
> +		return;
> +
> +	/* To force link we need to set bits for all supported PHY types,
> +	 * but there are now more than 32, so we need to split the bitmap
> +	 * across two fields.
> +	 */
> +	for (cnt = I40E_PHY_TYPE_SGMII; cnt < I40E_PHY_TYPE_MAX; cnt++)
> +		mask |= (1ULL << cnt);

nit: looks like a GENMASK_ULL() coded as a loop

> +	config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
> +	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
> +	/* Copy the old settings, except of phy_type */
> +	config.abilities = abilities.abilities;
> +	config.link_speed = abilities.link_speed;
> +	config.eee_capability = abilities.eee_capability;
> +	config.eeer = abilities.eeer_val;
> +	config.low_power_ctrl = abilities.d3_lpan;

... but then here you use the values.  Can this not lead to trouble?

> +	err = i40e_aq_set_phy_config(hw, &config, NULL);
> +
> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"set phy config ret =  %s last_status =  %s\n",
> +			i40e_stat_str(&pf->hw, err),
> +			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));

IMHO ignoring errors on close/down is fine in this case, but if the
link is forced down and then subsequent open can't bring it up the open
should fail.  dev_dbg() won't even show up in logs on 99.9% of machines.

Perhaps I'm misunderstanding what this patch does.

> +	/* Update the link info */
> +	err = i40e_update_link_info(hw);
> +	if (err) {
> +		/* Wait a little bit (on 40G cards it sometimes takes a really
> +		 * long time for link to come back from the atomic reset)
> +		 * and try once more
> +		 */
> +		msleep(1000);
> +		i40e_update_link_info(hw);
> +	}
> +
> +	i40e_aq_set_link_restart_an(hw, true, NULL);
> +}
> +
>  /**
>   * i40e_down - Shutdown the connection processing
>   * @vsi: the VSI being stopped
> @@ -6617,6 +6683,9 @@ void i40e_down(struct i40e_vsi *vsi)
>  	}
>  	i40e_vsi_disable_irq(vsi);
>  	i40e_vsi_stop_rings(vsi);
> +	if (vsi->type == I40E_VSI_MAIN &&
> +	    vsi->back->flags & I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED)
> +		i40e_force_link_state(vsi->back, false);
>  	i40e_napi_disable_all(vsi);
>  
>  	for (i = 0; i < vsi->num_queue_pairs; i++) {
> @@ -7578,6 +7647,8 @@ int i40e_open(struct net_device *netdev)
>  
>  	netif_carrier_off(netdev);
>  
> +	i40e_force_link_state(pf, true);
> +
>  	err = i40e_vsi_open(vsi);
>  	if (err)
>  		return err;

RE: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Stachura, Mariusz]

Hey Kuba,

Did you set private flag before doing ifconfig IFNAME down?

"ethtool --set-priv-flags enp5s0f0 link-down-on-close on"
"ifconfig enp5s0f0 down"

And the link partner should see this port as physically downed.

Setting phy_type to 0 is, as you said, a way of fooling FW. I'll try to address some of your comments in my next mail.

Many thanks and best regards,
Mariusz

-----Original Message-----
From: Jakub Kicinski [mailto:kubakici@wp.pl] 
Sent: Wednesday, January 10, 2018 7:41 AM
To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: davem@davemloft.net; Stachura, Mariusz <mariusz.stachura@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com; Williams, Mitch A <mitch.a.williams@intel.com>
Subject: Re: [net-next v2 15/15] i40e: link_down_on_close private flag support

On Tue,  9 Jan 2018 08:19:30 -0800, Jeff Kirsher wrote:
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> This patch introduces new ethtool private flag used for forcing true 
> link state. Function i40e_force_link_state that implements this 
> functionality was added, it sets phy_type = 0 in order to work-around 
> firmware's LESM. False positive error messages were suppressed.

Hi, could you say a bit more about how this works?  Last week I've been actually trying to use a Fortville connected by a DAC to another card to test link state on that other.  ifconfig down does not bring the link down on the Fortville, apparently.  BMC is configured to only use LOM, no PXE etc.

With this patch I assume I will be able to force the link down?  What is keeping the link up otherwise?  You mention phy_type, is this phy-specific?  Or are you using the phy_type purely as a way of fooling FW to do what you want?

Few nits below..

> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  2 +
>  drivers/net/ethernet/intel/i40e/i40e_main.c    | 71 ++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 34173f821fd9..50908f343221 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -230,6 +230,8 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
>  	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
>  	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
>  	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 
> 0),
> +	I40E_PRIV_FLAG("link-down-on-close",
> +		       I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED, 0),
>  	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
>  	I40E_PRIV_FLAG("disable-source-pruning",
>  		       I40E_FLAG_SOURCE_PRUNING_DISABLED, 0), diff --git 
> a/drivers/net/ethernet/intel/i40e/i40e_main.c 
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 666d2e7781e1..41fbccd41641 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -6600,6 +6600,72 @@ int i40e_up(struct i40e_vsi *vsi)
>  	return err;
>  }
>  
> +/**
> + * i40e_force_link_state - Force the link status
> + * @pf: board private structure
> + * @is_up: whether the link state should be forced up or down  **/ 
> +static void i40e_force_link_state(struct i40e_pf *pf, bool is_up) {
> +	struct i40e_aq_get_phy_abilities_resp abilities;
> +	struct i40e_aq_set_phy_config config = {0};
> +	struct i40e_hw *hw = &pf->hw;
> +	enum i40e_aq_phy_type cnt;
> +	u64 mask = 0;
> +	i40e_status err;

nit: reverse christmas tree

> +	/* Get the current phy config */
> +	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
> +					   NULL);

Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.

> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"failed to get phy cap., ret =  %s last_status =  %s\n",
> +			i40e_stat_str(hw, err),
> +			i40e_aq_str(hw, hw->aq.asq_last_status));

so getting 'abilities' can fail here...

> +	/* If link needs to go up, but was not forced to go down,
> +	 * no need for a flap
> +	 */
> +	if (is_up && abilities.phy_type != 0)
> +		return;
> +
> +	/* To force link we need to set bits for all supported PHY types,
> +	 * but there are now more than 32, so we need to split the bitmap
> +	 * across two fields.
> +	 */
> +	for (cnt = I40E_PHY_TYPE_SGMII; cnt < I40E_PHY_TYPE_MAX; cnt++)
> +		mask |= (1ULL << cnt);

nit: looks like a GENMASK_ULL() coded as a loop

> +	config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
> +	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
> +	/* Copy the old settings, except of phy_type */
> +	config.abilities = abilities.abilities;
> +	config.link_speed = abilities.link_speed;
> +	config.eee_capability = abilities.eee_capability;
> +	config.eeer = abilities.eeer_val;
> +	config.low_power_ctrl = abilities.d3_lpan;

... but then here you use the values.  Can this not lead to trouble?

> +	err = i40e_aq_set_phy_config(hw, &config, NULL);
> +
> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"set phy config ret =  %s last_status =  %s\n",
> +			i40e_stat_str(&pf->hw, err),
> +			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));

IMHO ignoring errors on close/down is fine in this case, but if the link is forced down and then subsequent open can't bring it up the open should fail.  dev_dbg() won't even show up in logs on 99.9% of machines.

Perhaps I'm misunderstanding what this patch does.

> +	/* Update the link info */
> +	err = i40e_update_link_info(hw);
> +	if (err) {
> +		/* Wait a little bit (on 40G cards it sometimes takes a really
> +		 * long time for link to come back from the atomic reset)
> +		 * and try once more
> +		 */
> +		msleep(1000);
> +		i40e_update_link_info(hw);
> +	}
> +
> +	i40e_aq_set_link_restart_an(hw, true, NULL); }
> +
>  /**
>   * i40e_down - Shutdown the connection processing
>   * @vsi: the VSI being stopped
> @@ -6617,6 +6683,9 @@ void i40e_down(struct i40e_vsi *vsi)
>  	}
>  	i40e_vsi_disable_irq(vsi);
>  	i40e_vsi_stop_rings(vsi);
> +	if (vsi->type == I40E_VSI_MAIN &&
> +	    vsi->back->flags & I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED)
> +		i40e_force_link_state(vsi->back, false);
>  	i40e_napi_disable_all(vsi);
>  
>  	for (i = 0; i < vsi->num_queue_pairs; i++) { @@ -7578,6 +7647,8 @@ 
> int i40e_open(struct net_device *netdev)
>  
>  	netif_carrier_off(netdev);
>  
> +	i40e_force_link_state(pf, true);
> +
>  	err = i40e_vsi_open(vsi);
>  	if (err)
>  		return err;

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [net-next v2 00/15][pull request] 40GbE Intel Wired LAN Driver Updates 2018-01-09

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue,  9 Jan 2018 08:19:15 -0800

> This series contains updates to i40e and i40evf only.
 ...
> Mariusz introduces a ethtool private flag used for forcing true link
> state in order to workaround a firmware link establishment state
> machine (LESM). 
> 
> v2: cleaned up commit message for patch 12 based on feedback from Sergei
>     Shtylyov and Alex Duyck

Looks like patch #15 needs some work, see Jakub's feedback.

RE: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Stachura, Mariusz]

Hello Kuba,

First of all sorry for a long response.

Did you have a chance to try it using instruction I've sent?

As for review:
1) nit: reverse christmas tree
    I'm fixing it.
2) Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.
    Good point, I'm changing dev_dbg to dev_err and returning immediately after printing it
3) nit: looks like a GENMASK_ULL() coded as a loop
    You are right, but I just realized this is not what I wanted :). I wanted to include all enum values, however not all values between enums. In example, there is a gap between:
	I40E_PHY_TYPE_UNSUPPORTED		= 0xF,
	I40E_PHY_TYPE_100BASE_TX		= 0x11,
    And I did not want to include 0x10. I am adding #define that inludes all ORed enums (in i40e_adminq_cmd.h, just after enum i40e_aq_phy_type definition). I will assign this define to the mask, instead of using GENMASK_ULL.

Thank you once again!
Mariusz
    

-----Original Message-----
From: Stachura, Mariusz 
Sent: Wednesday, January 10, 2018 10:33 AM
To: Jakub Kicinski <kubakici@wp.pl>; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: davem@davemloft.net; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com; Williams, Mitch A <mitch.a.williams@intel.com>
Subject: RE: [net-next v2 15/15] i40e: link_down_on_close private flag support

Hey Kuba,

Did you set private flag before doing ifconfig IFNAME down?

"ethtool --set-priv-flags enp5s0f0 link-down-on-close on"
"ifconfig enp5s0f0 down"

And the link partner should see this port as physically downed.

Setting phy_type to 0 is, as you said, a way of fooling FW. I'll try to address some of your comments in my next mail.

Many thanks and best regards,
Mariusz

-----Original Message-----
From: Jakub Kicinski [mailto:kubakici@wp.pl]
Sent: Wednesday, January 10, 2018 7:41 AM
To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: davem@davemloft.net; Stachura, Mariusz <mariusz.stachura@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com; Williams, Mitch A <mitch.a.williams@intel.com>
Subject: Re: [net-next v2 15/15] i40e: link_down_on_close private flag support

On Tue,  9 Jan 2018 08:19:30 -0800, Jeff Kirsher wrote:
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> This patch introduces new ethtool private flag used for forcing true 
> link state. Function i40e_force_link_state that implements this 
> functionality was added, it sets phy_type = 0 in order to work-around 
> firmware's LESM. False positive error messages were suppressed.

Hi, could you say a bit more about how this works?  Last week I've been actually trying to use a Fortville connected by a DAC to another card to test link state on that other.  ifconfig down does not bring the link down on the Fortville, apparently.  BMC is configured to only use LOM, no PXE etc.

With this patch I assume I will be able to force the link down?  What is keeping the link up otherwise?  You mention phy_type, is this phy-specific?  Or are you using the phy_type purely as a way of fooling FW to do what you want?

Few nits below..

> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  2 +
>  drivers/net/ethernet/intel/i40e/i40e_main.c    | 71 ++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 34173f821fd9..50908f343221 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -230,6 +230,8 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
>  	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
>  	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
>  	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 
> 0),
> +	I40E_PRIV_FLAG("link-down-on-close",
> +		       I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED, 0),
>  	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
>  	I40E_PRIV_FLAG("disable-source-pruning",
>  		       I40E_FLAG_SOURCE_PRUNING_DISABLED, 0), diff --git 
> a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 666d2e7781e1..41fbccd41641 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -6600,6 +6600,72 @@ int i40e_up(struct i40e_vsi *vsi)
>  	return err;
>  }
>  
> +/**
> + * i40e_force_link_state - Force the link status
> + * @pf: board private structure
> + * @is_up: whether the link state should be forced up or down  **/ 
> +static void i40e_force_link_state(struct i40e_pf *pf, bool is_up) {
> +	struct i40e_aq_get_phy_abilities_resp abilities;
> +	struct i40e_aq_set_phy_config config = {0};
> +	struct i40e_hw *hw = &pf->hw;
> +	enum i40e_aq_phy_type cnt;
> +	u64 mask = 0;
> +	i40e_status err;

nit: reverse christmas tree

> +	/* Get the current phy config */
> +	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
> +					   NULL);

Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.

> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"failed to get phy cap., ret =  %s last_status =  %s\n",
> +			i40e_stat_str(hw, err),
> +			i40e_aq_str(hw, hw->aq.asq_last_status));

so getting 'abilities' can fail here...

> +	/* If link needs to go up, but was not forced to go down,
> +	 * no need for a flap
> +	 */
> +	if (is_up && abilities.phy_type != 0)
> +		return;
> +
> +	/* To force link we need to set bits for all supported PHY types,
> +	 * but there are now more than 32, so we need to split the bitmap
> +	 * across two fields.
> +	 */
> +	for (cnt = I40E_PHY_TYPE_SGMII; cnt < I40E_PHY_TYPE_MAX; cnt++)
> +		mask |= (1ULL << cnt);

nit: looks like a GENMASK_ULL() coded as a loop

> +	config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
> +	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
> +	/* Copy the old settings, except of phy_type */
> +	config.abilities = abilities.abilities;
> +	config.link_speed = abilities.link_speed;
> +	config.eee_capability = abilities.eee_capability;
> +	config.eeer = abilities.eeer_val;
> +	config.low_power_ctrl = abilities.d3_lpan;

... but then here you use the values.  Can this not lead to trouble?

> +	err = i40e_aq_set_phy_config(hw, &config, NULL);
> +
> +	if (err)
> +		dev_dbg(&pf->pdev->dev,
> +			"set phy config ret =  %s last_status =  %s\n",
> +			i40e_stat_str(&pf->hw, err),
> +			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));

IMHO ignoring errors on close/down is fine in this case, but if the link is forced down and then subsequent open can't bring it up the open should fail.  dev_dbg() won't even show up in logs on 99.9% of machines.

Perhaps I'm misunderstanding what this patch does.

> +	/* Update the link info */
> +	err = i40e_update_link_info(hw);
> +	if (err) {
> +		/* Wait a little bit (on 40G cards it sometimes takes a really
> +		 * long time for link to come back from the atomic reset)
> +		 * and try once more
> +		 */
> +		msleep(1000);
> +		i40e_update_link_info(hw);
> +	}
> +
> +	i40e_aq_set_link_restart_an(hw, true, NULL); }
> +
>  /**
>   * i40e_down - Shutdown the connection processing
>   * @vsi: the VSI being stopped
> @@ -6617,6 +6683,9 @@ void i40e_down(struct i40e_vsi *vsi)
>  	}
>  	i40e_vsi_disable_irq(vsi);
>  	i40e_vsi_stop_rings(vsi);
> +	if (vsi->type == I40E_VSI_MAIN &&
> +	    vsi->back->flags & I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED)
> +		i40e_force_link_state(vsi->back, false);
>  	i40e_napi_disable_all(vsi);
>  
>  	for (i = 0; i < vsi->num_queue_pairs; i++) { @@ -7578,6 +7647,8 @@ 
> int i40e_open(struct net_device *netdev)
>  
>  	netif_carrier_off(netdev);
>  
> +	i40e_force_link_state(pf, true);
> +
>  	err = i40e_vsi_open(vsi);
>  	if (err)
>  		return err;

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Jakub Kicinski]

On Tue, 23 Jan 2018 10:55:33 +0000, Stachura, Mariusz wrote:
> Hello Kuba,

Hi Mariusz!

> First of all sorry for a long response.
> 
> Did you have a chance to try it using instruction I've sent?

Unfortunately, I can't take the machine with Fortville down ATM to swap
kernels.  Is this patch already applied to your out-of-tree packaged
driver?  Could I try that?

> As for review:
> 1) nit: reverse christmas tree
>     I'm fixing it.
> 2) Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.
>     Good point, I'm changing dev_dbg to dev_err and returning immediately after printing it
> 3) nit: looks like a GENMASK_ULL() coded as a loop
>     You are right, but I just realized this is not what I wanted :). I wanted to include all enum values, however not all values between enums. In example, there is a gap between:
> 	I40E_PHY_TYPE_UNSUPPORTED		= 0xF,
> 	I40E_PHY_TYPE_100BASE_TX		= 0x11,
>     And I did not want to include 0x10. I am adding #define that inludes all ORed enums (in i40e_adminq_cmd.h, just after enum i40e_aq_phy_type definition). I will assign this define to the mask, instead of using GENMASK_ULL.

What about the error handling?  That was my main worry.  That the
communication with FW will fail on open and the link will remain
disabled.

RE: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Stachura, Mariusz]

>On Tue, 23 Jan 2018 10:55:33 +0000, Stachura, Mariusz wrote:
>> Hello Kuba,
>
>Hi Mariusz!
>
>> First of all sorry for a long response.
>> 
>> Did you have a chance to try it using instruction I've sent?
>
>Unfortunately, I can't take the machine with Fortville down ATM to swap kernels.  Is this patch already applied to your out-of-tree packaged driver?  Could I try that?
>
>> As for review:
>> 1) nit: reverse christmas tree
>>     I'm fixing it.
>> 2) Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.
>>     Good point, I'm changing dev_dbg to dev_err and returning 
>> immediately after printing it
>> 3) nit: looks like a GENMASK_ULL() coded as a loop
>>     You are right, but I just realized this is not what I wanted :). I wanted to include all enum values, however not all values between enums. In example, there is a gap between:
>> 	I40E_PHY_TYPE_UNSUPPORTED		= 0xF,
>> 	I40E_PHY_TYPE_100BASE_TX		= 0x11,
>>     And I did not want to include 0x10. I am adding #define that inludes all ORed enums (in i40e_adminq_cmd.h, just after enum i40e_aq_phy_type definition). I will assign this define to the mask, instead of using GENMASK_ULL.
>
>What about the error handling?  That was my main worry.  That the communication with FW will fail on open and the link will remain disabled.

Hi again,

As I checked, this ( https://sourceforge.net/projects/e1000/files/i40e%20stable/2.4.3/i40e-2.4.3.tar.gz/download ) version had this patch applied, feel free to give it a shot.

Error handling was also addressed. i40e_open checks i40e_force_link_state and returns EBUSY if get/set capabilities was not successful.

Best regards,
Mariusz Stachura
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [net-next v2 15/15] i40e: link_down_on_close private flag support

[Jakub Kicinski]

On Wed, 24 Jan 2018 13:54:16 +0000, Stachura, Mariusz wrote:
> >On Tue, 23 Jan 2018 10:55:33 +0000, Stachura, Mariusz wrote:  
> >> As for review:
> >> 1) nit: reverse christmas tree
> >>     I'm fixing it.
> >> 2) Are abilities guaranteed to be filled in by this function?  Otherwise you will use uninitialized stack memory.
> >>     Good point, I'm changing dev_dbg to dev_err and returning 
> >> immediately after printing it
> >> 3) nit: looks like a GENMASK_ULL() coded as a loop
> >>     You are right, but I just realized this is not what I wanted :). I wanted to include all enum values, however not all values between enums. In example, there is a gap between:
> >> 	I40E_PHY_TYPE_UNSUPPORTED		= 0xF,
> >> 	I40E_PHY_TYPE_100BASE_TX		= 0x11,
> >>     And I did not want to include 0x10. I am adding #define that inludes all ORed enums (in i40e_adminq_cmd.h, just after enum i40e_aq_phy_type definition). I will assign this define to the mask, instead of using GENMASK_ULL.  
> >
> >What about the error handling?  That was my main worry.  That the communication with FW will fail on open and the link will remain disabled.  
> 
> Hi again,
> 
> As I checked, this
> ( https://sourceforge.net/projects/e1000/files/i40e%20stable/2.4.3/i40e-2.4.3.tar.gz/download )
> version had this patch applied, feel free to give it a shot.

Cool thanks, I tried that and the link does go down as expected (y)

> Error handling was also addressed. i40e_open checks
> i40e_force_link_state and returns EBUSY if get/set capabilities was
> not successful.