[Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

[Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

[Jacob Keller]

The service task reads some registers as part of its normal routine,
even while the interface is down. Normally this is ok. However, during
suspend we have disabled the PCI device. Due to this, registers will
read in the same way as a surprise-remove event. Disable the service
task while we suspend, and re-enable it after we resume. If we don't do
this, the device could be UP when you suspend and come back from resume
as closed (since fm10k closes the device when it gets a surprise
remove).

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 0381c8d192aa..c7ffb2635f92 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1983,6 +1983,16 @@ static int fm10k_resume(struct pci_dev *pdev)
 	if (err)
 		return err;
 
+	/* assume host is not ready, to prevent race with watchdog incase we
+	 * actually don't have connection to the switch
+	 */
+	interface->host_ready = false;
+	fm10k_watchdog_host_not_ready(interface);
+
+	/* clear the service task disable bit to allow service task to start */
+	clear_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+	fm10k_service_event_schedule(interface);
+
 	/* restore SR-IOV interface */
 	fm10k_iov_resume(pdev);
 
@@ -2010,6 +2020,15 @@ static int fm10k_suspend(struct pci_dev *pdev,
 
 	fm10k_iov_suspend(pdev);
 
+	/* the watchdog tasks may read registers, which will appear like a
+	 * surprise-remove event once the PCI device is disabled. This will
+	 * cause us to close the netdevice, so we don't retain the open/closed
+	 * state post-resume. Prevent this by disabling the service task while
+	 * suspended, until we actually resume.
+	 */
+	set_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+	cancel_work_sync(&interface->service_task);
+
 	rtnl_lock();
 
 	if (netif_running(netdev))
-- 
2.4.2

[Intel-wired-lan] [PATCH 2/6] [TRIVIAL] fm10k: remove comment about rtnl_lock around mbx operations

[Jacob Keller]

This comment is no longer true due to a couple of mailbox locking
refactors, and we now don't actually do any rtnl protected operations
directly in the mailbox path. Remove this comment as it is factually
incorrect and confusing.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index c7ffb2635f92..37cd78f3ce24 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -259,8 +259,6 @@ static void fm10k_watchdog_update_host_state(struct fm10k_intfc *interface)
  * @interface: board private structure
  *
  * This function will process both the upstream and downstream mailboxes.
- * It is necessary for us to hold the rtnl_lock while doing this as the
- * mailbox accesses are protected by this lock.
  **/
 static void fm10k_mbx_subtask(struct fm10k_intfc *interface)
 {
@@ -483,7 +481,7 @@ static void fm10k_service_task(struct work_struct *work)
 
 	interface = container_of(work, struct fm10k_intfc, service_task);
 
-	/* tasks always capable of running, but must be rtnl protected */
+	/* tasks run even when interface is down */
 	fm10k_mbx_subtask(interface);
 	fm10k_detach_subtask(interface);
 	fm10k_reset_subtask(interface);
-- 
2.4.2

[Intel-wired-lan] [PATCH 3/6] fm10k: only prevent removal of default VID rules

[Jacob Keller]

This allows us to correctly add a VLAN even if it matches our default
VID. However, we don't want to remove the VID rules once that VLAN is
deleted. Correctly remove the stack layers information of the VLAN, but
then return to forwarding that VID as untagged frames. If we deleted the
VID rules here, we would begin dropping traffic due to VLAN membership
violations.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 99228bf46c12..818bc8b1fa58 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -775,8 +775,8 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
 	if (!set)
 		clear_bit(vid, interface->active_vlans);
 
-	/* if default VLAN is already present do nothing */
-	if (vid == hw->mac.default_vid)
+	/* Do not remove default VID related entries from VLAN and MAC tables */
+	if (!set && vid == hw->mac.default_vid)
 		return 0;
 
 	fm10k_mbx_lock(interface);
-- 
2.4.2

[Intel-wired-lan] [PATCH 4/6] fm10k: reset Tx FIFO head and tail pointers as part of reset_work

[Jacob Keller]

This patch fixes a corner case issue with the PF/VF mailbox code.
Currently, fm10k_mbx_reset_work clears various state about the
mailbox. However, it did not clear the Tx FIFO head/tail pointers. Thus,
when the PF finally re-connects (say due to VF reset), it in-advertently
might send some previous data in the Tx FIFO again. This was discovered
by attempting to perform a static VF MAC address assignment on the PF
device. The VF would receive the message and then initiate a reset.
During this process it disconnects and reconnects to the mailbox. If the
PF doesn't respond fast enough, it may not see the disconnect message,
and only sees a new connect message. In this flow, it does not properly
clear the Tx FIFO pointers. Thus, it will re-transmit the last message
in the Tx FIFO. However, it does not increment the mailbox Tx messages
because it has already reset mbx->pulled and thus does not think it is
actually sending any data. The fix is to simply reset the Tx FIFO
pointers whenever we call fm10k_mbx_reset_work.

As a consequence, we no longer need both fm10k_mbx_fifo_drop_all and calls
to fm10k_mbx_update_max_size with a size of 0.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
index 1a4b52637de9..f9016bf8b5b1 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
@@ -126,18 +126,6 @@ static u16 fm10k_fifo_head_drop(struct fm10k_mbx_fifo *fifo)
 }
 
 /**
- *  fm10k_fifo_drop_all - Drop all messages in FIFO
- *  @fifo: pointer to FIFO
- *
- *  This function resets the head pointer to drop all messages in the FIFO,
- *  and ensure the FIFO is empty.
- **/
-static void fm10k_fifo_drop_all(struct fm10k_mbx_fifo *fifo)
-{
-	fifo->head = fifo->tail;
-}
-
-/**
  *  fm10k_mbx_index_len - Convert a head/tail index into a length value
  *  @mbx: pointer to mailbox
  *  @head: head index
@@ -1054,6 +1042,8 @@ static void fm10k_mbx_reset_work(struct fm10k_mbx_info *mbx)
 	mbx->pulled = 0;
 	mbx->tail_len = 0;
 	mbx->head_len = 0;
+	mbx->tx.tail = 0;
+	mbx->tx.head = 0;
 	mbx->rx.tail = 0;
 	mbx->rx.head = 0;
 }
@@ -1384,7 +1374,6 @@ static void fm10k_mbx_disconnect(struct fm10k_hw *hw,
 	 * drop all left over messages in the FIFO.
 	 */
 	fm10k_mbx_connect_reset(mbx);
-	fm10k_fifo_drop_all(&mbx->tx);
 
 	fm10k_write_reg(hw, mbx->mbmem_reg, 0);
 }
@@ -1725,7 +1714,6 @@ static void fm10k_sm_mbx_disconnect(struct fm10k_hw *hw,
 	mbx->state = FM10K_STATE_CLOSED;
 	mbx->remote = 0;
 	fm10k_mbx_reset_work(mbx);
-	fm10k_mbx_update_max_size(mbx, 0);
 
 	fm10k_write_reg(hw, mbx->mbmem_reg, 0);
 }
-- 
2.4.2

[Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

[Jacob Keller]

This is useful in cases where we connect to a slot at Gen3, but the slot
is behind a bus which only connected at Gen2. This generally only
happens when a PCIe switch is in the sequence of devices, and can be
very confusing when you see slow performance with no obvious cause.

I am aware this patch has a few lines that break 80 characters, but
there does not seem to be a readable way to format them to less than 80
characters. Suggestions welcome.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++--------
 1 file changed, 76 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 37cd78f3ce24..d8ab6cdca456 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc *interface,
 
 static void fm10k_slot_warn(struct fm10k_intfc *interface)
 {
-	struct device *dev = &interface->pdev->dev;
+	enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN;
+	enum pci_bus_speed speed = PCI_SPEED_UNKNOWN;
 	struct fm10k_hw *hw = &interface->hw;
+	int max_gts = 0, expected_gts = 0;
 
-	if (hw->mac.ops.is_slot_appropriate(hw))
+	if (pcie_get_minimum_link(interface->pdev, &speed, &width) ||
+	    speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN) {
+		dev_warn(&interface->pdev->dev,
+			 "Unable to determine PCI Express bandwidth.\n");
 		return;
+	}
 
-	dev_warn(dev,
-		 "For optimal performance, a %s %s slot is recommended.\n",
-		 (hw->bus_caps.width == fm10k_bus_width_pcie_x1 ? "x1" :
-		  hw->bus_caps.width == fm10k_bus_width_pcie_x4 ? "x4" :
-		  "x8"),
-		 (hw->bus_caps.speed == fm10k_bus_speed_2500 ? "2.5GT/s" :
-		  hw->bus_caps.speed == fm10k_bus_speed_5000 ? "5.0GT/s" :
-		  "8.0GT/s"));
-	dev_warn(dev,
-		 "A slot with more lanes and/or higher speed is suggested.\n");
+	switch (speed) {
+	case PCIE_SPEED_2_5GT:
+		/* 8b/10b encoding reduces max throughput by 20% */
+		max_gts = 2 * width;
+		break;
+	case PCIE_SPEED_5_0GT:
+		/* 8b/10b encoding reduces max throughput by 20% */
+		max_gts = 4 * width;
+		break;
+	case PCIE_SPEED_8_0GT:
+		/* 128b/130b encoding has less than 2% impact on throughput */
+		max_gts = 8 * width;
+		break;
+	default:
+		dev_warn(&interface->pdev->dev,
+			 "Unable to determine PCI Express bandwidth.\n");
+		return;
+	}
+
+	dev_info(&interface->pdev->dev,
+		 "PCI Express bandwidth of %dGT/s available\n",
+		 max_gts);
+	dev_info(&interface->pdev->dev,
+		 "(Speed:%s, Width: x%d, Encoding Loss:%s, Payload:%s)\n",
+		 (speed == PCIE_SPEED_8_0GT ? "8.0GT/s" :
+		  speed == PCIE_SPEED_5_0GT ? "5.0GT/s" :
+		  speed == PCIE_SPEED_2_5GT ? "2.5GT/s" :
+		  "Unknown"),
+		 hw->bus.width,
+		 (speed == PCIE_SPEED_2_5GT ? "20%" :
+		  speed == PCIE_SPEED_5_0GT ? "20%" :
+		  speed == PCIE_SPEED_8_0GT ? "<2%" :
+		  "Unknown"),
+		 (hw->bus.payload == fm10k_bus_payload_128 ? "128B" :
+		  hw->bus.payload == fm10k_bus_payload_256 ? "256B" :
+		  hw->bus.payload == fm10k_bus_payload_512 ? "512B" :
+		  "Unknown"));
+
+	switch (hw->bus_caps.speed) {
+	case fm10k_bus_speed_2500:
+		/* 8b/10b encoding reduces max throughput by 20% */
+		expected_gts = 2 * hw->bus_caps.width;
+		break;
+	case fm10k_bus_speed_5000:
+		/* 8b/10b encoding reduces max throughput by 20% */
+		expected_gts = 4 * hw->bus_caps.width;
+		break;
+	case fm10k_bus_speed_8000:
+		/* 128b/130b encoding has less than 2% impact on throughput */
+		expected_gts = 8 * hw->bus_caps.width;
+		break;
+	default:
+		dev_warn(&interface->pdev->dev,
+			 "Unable to determine expected PCI Express bandwidth.\n");
+		return;
+	}
+
+	if (max_gts < expected_gts) {
+		dev_warn(&interface->pdev->dev,
+			 "This device requires %dGT/s of bandwidth for optimal performance.\n",
+			 expected_gts);
+		dev_warn(&interface->pdev->dev,
+			 "A %sslot with x%d lanes is suggested.\n",
+			 (hw->bus_caps.speed == fm10k_bus_speed_2500 ? "2.5GT/s " :
+			  hw->bus_caps.speed == fm10k_bus_speed_5000 ? "5.0GT/s " :
+			  hw->bus_caps.speed == fm10k_bus_speed_8000 ? "8.0GT/s " : ""),
+			 hw->bus_caps.width);
+	}
 }
 
 /**
@@ -1737,7 +1801,6 @@ static int fm10k_probe(struct pci_dev *pdev,
 {
 	struct net_device *netdev;
 	struct fm10k_intfc *interface;
-	struct fm10k_hw *hw;
 	int err;
 
 	err = pci_enable_device_mem(pdev);
@@ -1781,7 +1844,6 @@ static int fm10k_probe(struct pci_dev *pdev,
 
 	interface->netdev = netdev;
 	interface->pdev = pdev;
-	hw = &interface->hw;
 
 	interface->uc_addr = ioremap(pci_resource_start(pdev, 0),
 				     FM10K_UC_ADDR_SIZE);
@@ -1823,21 +1885,6 @@ static int fm10k_probe(struct pci_dev *pdev,
 	/* Register PTP interface */
 	fm10k_ptp_register(interface);
 
-	/* print bus type/speed/width info */
-	dev_info(&pdev->dev, "(PCI Express:%s Width: %s Payload: %s)\n",
-		 (hw->bus.speed == fm10k_bus_speed_8000 ? "8.0GT/s" :
-		  hw->bus.speed == fm10k_bus_speed_5000 ? "5.0GT/s" :
-		  hw->bus.speed == fm10k_bus_speed_2500 ? "2.5GT/s" :
-		  "Unknown"),
-		 (hw->bus.width == fm10k_bus_width_pcie_x8 ? "x8" :
-		  hw->bus.width == fm10k_bus_width_pcie_x4 ? "x4" :
-		  hw->bus.width == fm10k_bus_width_pcie_x1 ? "x1" :
-		  "Unknown"),
-		 (hw->bus.payload == fm10k_bus_payload_128 ? "128B" :
-		  hw->bus.payload == fm10k_bus_payload_256 ? "256B" :
-		  hw->bus.payload == fm10k_bus_payload_512 ? "512B" :
-		  "Unknown"));
-
 	/* print warning for non-optimal configurations */
 	fm10k_slot_warn(interface);
 
-- 
2.4.2

[Intel-wired-lan] [PATCH 6/6] fm10k: update netdev perm_addr during reinit, instead of at up

[Jacob Keller]

Update the netdev permanent address during fm10k_reinit enables the user
to immediately see the new MAC address on the VF even if the device
isn't up. The previous code required that the device by opened before
changes would appear.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 15 ---------------
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c    | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 818bc8b1fa58..b2065cb44edc 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -996,21 +996,6 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface)
 	int xcast_mode;
 	u16 vid, glort;
 
-	/* restore our address if perm_addr is set */
-	if (hw->mac.type == fm10k_mac_vf) {
-		if (is_valid_ether_addr(hw->mac.perm_addr)) {
-			ether_addr_copy(hw->mac.addr, hw->mac.perm_addr);
-			ether_addr_copy(netdev->perm_addr, hw->mac.perm_addr);
-			ether_addr_copy(netdev->dev_addr, hw->mac.perm_addr);
-			netdev->addr_assign_type &= ~NET_ADDR_RANDOM;
-		}
-
-		if (hw->mac.vlan_override)
-			netdev->features &= ~NETIF_F_HW_VLAN_CTAG_RX;
-		else
-			netdev->features |= NETIF_F_HW_VLAN_CTAG_RX;
-	}
-
 	/* record glort for this interface */
 	glort = interface->glort;
 
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index d8ab6cdca456..dc354c74fd58 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -170,6 +170,21 @@ static void fm10k_reinit(struct fm10k_intfc *interface)
 	/* reassociate interrupts */
 	fm10k_mbx_request_irq(interface);
 
+	/* update hardware address for VFs if perm_addr has changed */
+	if (hw->mac.type == fm10k_mac_vf) {
+		if (is_valid_ether_addr(hw->mac.perm_addr)) {
+			ether_addr_copy(hw->mac.addr, hw->mac.perm_addr);
+			ether_addr_copy(netdev->perm_addr, hw->mac.perm_addr);
+			ether_addr_copy(netdev->dev_addr, hw->mac.perm_addr);
+			netdev->addr_assign_type &= ~NET_ADDR_RANDOM;
+		}
+
+		if (hw->mac.vlan_override)
+			netdev->features &= ~NETIF_F_HW_VLAN_CTAG_RX;
+		else
+			netdev->features |= NETIF_F_HW_VLAN_CTAG_RX;
+	}
+
 	/* reset clock */
 	fm10k_ts_reset(interface);
 
-- 
2.4.2

[Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

[Alexander Duyck]

On 06/15/2015 03:00 PM, Jacob Keller wrote:
> This is useful in cases where we connect to a slot at Gen3, but the slot
> is behind a bus which only connected at Gen2. This generally only
> happens when a PCIe switch is in the sequence of devices, and can be
> very confusing when you see slow performance with no obvious cause.
>
> I am aware this patch has a few lines that break 80 characters, but
> there does not seem to be a readable way to format them to less than 80
> characters. Suggestions welcome.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++--------
>   1 file changed, 76 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> index 37cd78f3ce24..d8ab6cdca456 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> @@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc *interface,
>   
>   static void fm10k_slot_warn(struct fm10k_intfc *interface)
>   {
> -	struct device *dev = &interface->pdev->dev;
> +	enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN;
> +	enum pci_bus_speed speed = PCI_SPEED_UNKNOWN;
>   	struct fm10k_hw *hw = &interface->hw;
> +	int max_gts = 0, expected_gts = 0;
>   
> -	if (hw->mac.ops.is_slot_appropriate(hw))

If I am not mistaken I believe this is the only spot that uses the 
is_slot_appropriate function in the upstream driver.  You could just 
drop it from the code if that is the case.  I'll try to get to it in a 
couple of weeks when I get back from vacation otherwise.

- Alex

[Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

[Keller, Jacob E]

On Thu, 2015-06-18 at 19:22 -0700, Alexander Duyck wrote:
> On 06/15/2015 03:00 PM, Jacob Keller wrote:
> > This is useful in cases where we connect to a slot at Gen3, but the 
> > slot
> > is behind a bus which only connected at Gen2. This generally only
> > happens when a PCIe switch is in the sequence of devices, and can 
> > be
> > very confusing when you see slow performance with no obvious cause.
> > 
> > I am aware this patch has a few lines that break 80 characters, but
> > there does not seem to be a readable way to format them to less 
> > than 80
> > characters. Suggestions welcome.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >   drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 
> > +++++++++++++++++++--------
> >   1 file changed, 76 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > index 37cd78f3ce24..d8ab6cdca456 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > @@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc 
> > *interface,
> >   
> >   static void fm10k_slot_warn(struct fm10k_intfc *interface)
> >   {
> > -   struct device *dev = &interface->pdev->dev;
> > +   enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN;
> > +   enum pci_bus_speed speed = PCI_SPEED_UNKNOWN;
> >     struct fm10k_hw *hw = &interface->hw;
> > +   int max_gts = 0, expected_gts = 0;
> >   
> > -   if (hw->mac.ops.is_slot_appropriate(hw))
> 
> If I am not mistaken I believe this is the only spot that uses the 
> is_slot_appropriate function in the upstream driver.  You could just 
> drop it from the code if that is the case.  I'll try to get to it in 
> a 
> couple of weeks when I get back from vacation otherwise.
> 
> - Alex

Makes sense.

Regards,
Jake

[Intel-wired-lan] [PATCH 4/6] fm10k: reset Tx FIFO head and tail pointers as part of reset_work

[Keller, Jacob E]

On Mon, 2015-06-15 at 15:00 -0700, Jacob Keller wrote:
> This patch fixes a corner case issue with the PF/VF mailbox code.
> Currently, fm10k_mbx_reset_work clears various state about the
> mailbox. However, it did not clear the Tx FIFO head/tail pointers. 
> Thus,
> when the PF finally re-connects (say due to VF reset), it in
> -advertently
> might send some previous data in the Tx FIFO again. This was 
> discovered
> by attempting to perform a static VF MAC address assignment on the PF
> device. The VF would receive the message and then initiate a reset.
> During this process it disconnects and reconnects to the mailbox. If 
> the
> PF doesn't respond fast enough, it may not see the disconnect 
> message,
> and only sees a new connect message. In this flow, it does not 
> properly
> clear the Tx FIFO pointers. Thus, it will re-transmit the last 
> message
> in the Tx FIFO. However, it does not increment the mailbox Tx 
> messages
> because it has already reset mbx->pulled and thus does not think it 
> is
> actually sending any data. The fix is to simply reset the Tx FIFO
> pointers whenever we call fm10k_mbx_reset_work.
> 
> As a consequence, we no longer need both fm10k_mbx_fifo_drop_all and 
> calls
> to fm10k_mbx_update_max_size with a size of 0.

This patch will need a revision 2. There is some other issues this
patch creates which we need to properly resolve.

Clearing the Tx FIFO causes us to potentially drop untransmitted work
and thus fail to send repsonses with no error indication returning to
the base driver code.

I am working on a corrected fix now.

Regards,
Jake

[Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

The service task reads some registers as part of its normal routine, even while the interface is down. Normally this is ok. However, during suspend we have disabled the PCI device. Due to this, registers will read in the same way as a surprise-remove event. Disable the service task while we suspend, and re-enable it after we resume. If we don't do this, the device could be UP when you suspend and come back from resume as closed (since fm10k closes the device when it gets a surprise remove).

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 2/6] [TRIVIAL] fm10k: remove comment about rtnl_lock around mbx operations

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 2/6] [TRIVIAL] fm10k: remove comment about rtnl_lock around mbx operations

This comment is no longer true due to a couple of mailbox locking refactors, and we now don't actually do any rtnl protected operations directly in the mailbox path. Remove this comment as it is factually incorrect and confusing.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 3/6] fm10k: only prevent removal of default VID rules

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 3/6] fm10k: only prevent removal of default VID rules

This allows us to correctly add a VLAN even if it matches our default VID. However, we don't want to remove the VID rules once that VLAN is deleted. Correctly remove the stack layers information of the VLAN, but then return to forwarding that VID as untagged frames. If we deleted the VID rules here, we would begin dropping traffic due to VLAN membership violations.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

This is useful in cases where we connect to a slot at Gen3, but the slot is behind a bus which only connected at Gen2. This generally only happens when a PCIe switch is in the sequence of devices, and can be very confusing when you see slow performance with no obvious cause.

I am aware this patch has a few lines that break 80 characters, but there does not seem to be a readable way to format them to less than 80 characters. Suggestions welcome.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++--------
 1 file changed, 76 insertions(+), 29 deletions(-)

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 6/6] fm10k: update netdev perm_addr during reinit, instead of at up

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 6/6] fm10k: update netdev perm_addr during reinit, instead of at up

Update the netdev permanent address during fm10k_reinit enables the user to immediately see the new MAC address on the VF even if the device isn't up. The previous code required that the device by opened before changes would appear.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---


 Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 1/6] fm10k: disable service task during suspend

The service task reads some registers as part of its normal routine, even while the interface is down. Normally this is ok. However, during suspend we have disabled the PCI device. Due to this, registers will read in the same way as a surprise-remove event. Disable the service task while we suspend, and re-enable it after we resume. If we don't do this, the device could be UP when you suspend and come back from resume as closed (since fm10k closes the device when it gets a surprise remove).

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 2/6] [TRIVIAL] fm10k: remove comment about rtnl_lock around mbx operations

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 2/6] [TRIVIAL] fm10k: remove comment about rtnl_lock around mbx operations

This comment is no longer true due to a couple of mailbox locking refactors, and we now don't actually do any rtnl protected operations directly in the mailbox path. Remove this comment as it is factually incorrect and confusing.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 3/6] fm10k: only prevent removal of default VID rules

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 3/6] fm10k: only prevent removal of default VID rules

This allows us to correctly add a VLAN even if it matches our default VID. However, we don't want to remove the VID rules once that VLAN is deleted. Correctly remove the stack layers information of the VLAN, but then return to forwarding that VID as untagged frames. If we deleted the VID rules here, we would begin dropping traffic due to VLAN membership violations.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 
Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link

This is useful in cases where we connect to a slot at Gen3, but the slot is behind a bus which only connected at Gen2. This generally only happens when a PCIe switch is in the sequence of devices, and can be very confusing when you see slow performance with no obvious cause.

I am aware this patch has a few lines that break 80 characters, but there does not seem to be a readable way to format them to less than 80 characters. Suggestions welcome.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---

Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH 6/6] fm10k: update netdev perm_addr during reinit, instead of at up

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, June 15, 2015 3:01 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 6/6] fm10k: update netdev perm_addr during reinit, instead of at up

Update the netdev permanent address during fm10k_reinit enables the user to immediately see the new MAC address on the VF even if the device isn't up. The previous code required that the device by opened before changes would appear.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 
Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan