Stable Archive on lore.kernel.org
 help / color / Atom feed
* [net-next 14/17] igb: Report speed and duplex as unknown when device is runtime suspended
       [not found] <20200523025109.3313635-1-jeffrey.t.kirsher@intel.com>
@ 2020-05-23  2:51 ` Jeff Kirsher
  2020-05-23  2:51 ` [net-next 16/17] e1000e: Disable TSO for buffer overrun workaround Jeff Kirsher
  1 sibling, 0 replies; 2+ messages in thread
From: Jeff Kirsher @ 2020-05-23  2:51 UTC (permalink / raw)
  To: davem
  Cc: Kai-Heng Feng, netdev, nhorman, sassmann, stable,
	Alexander Duyck, Aaron Brown, Jeff Kirsher

From: Kai-Heng Feng <kai.heng.feng@canonical.com>

igb device gets runtime suspended when there's no link partner. We can't
get correct speed under that state:
$ cat /sys/class/net/enp3s0/speed
1000

In addition to that, an error can also be spotted in dmesg:
[  385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost

Since device can only be runtime suspended when there's no link partner,
we can skip reading register and let the following logic set speed and
duplex with correct status.

The more generic approach will be wrap get_link_ksettings() with begin()
and complete() callbacks. However, for this particular issue, begin()
calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
is already hold by upper ethtool layer.

So let's take this approach until the igb_runtime_resume() no longer
needs to hold rtnl_lock.

CC: stable <stable@vger.kernel.org>
Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@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/igb/igb_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 39d3b76a6f5d..2cd003c5ad43 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -143,7 +143,8 @@ static int igb_get_link_ksettings(struct net_device *netdev,
 	u32 speed;
 	u32 supported, advertising;
 
-	status = rd32(E1000_STATUS);
+	status = pm_runtime_suspended(&adapter->pdev->dev) ?
+		 0 : rd32(E1000_STATUS);
 	if (hw->phy.media_type == e1000_media_type_copper) {
 
 		supported = (SUPPORTED_10baseT_Half |
-- 
2.26.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [net-next 16/17] e1000e: Disable TSO for buffer overrun workaround
       [not found] <20200523025109.3313635-1-jeffrey.t.kirsher@intel.com>
  2020-05-23  2:51 ` [net-next 14/17] igb: Report speed and duplex as unknown when device is runtime suspended Jeff Kirsher
@ 2020-05-23  2:51 ` Jeff Kirsher
  1 sibling, 0 replies; 2+ messages in thread
From: Jeff Kirsher @ 2020-05-23  2:51 UTC (permalink / raw)
  To: davem
  Cc: Kai-Heng Feng, netdev, nhorman, sassmann, stable, Aaron Brown,
	Jeff Kirsher

From: Kai-Heng Feng <kai.heng.feng@canonical.com>

Commit b10effb92e27 ("e1000e: fix buffer overrun while the I219 is
processing DMA transactions") imposes roughly 30% performance penalty.

The commit log states that "Disabling TSO eliminates performance loss
for TCP traffic without a noticeable impact on CPU performance", so
let's disable TSO by default to regain the loss.

CC: stable <stable@vger.kernel.org>
Fixes: b10effb92e27 ("e1000e: fix buffer overrun while the I219 is processing DMA transactions")
BugLink: https://bugs.launchpad.net/bugs/1802691
Signed-off-by: Kai-Heng Feng <kai.heng.feng@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/e1000e/netdev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e0b074820b47..66609cf689de 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5294,6 +5294,10 @@ static void e1000_watchdog_task(struct work_struct *work)
 					/* oops */
 					break;
 				}
+				if (hw->mac.type == e1000_pch_spt) {
+					netdev->features &= ~NETIF_F_TSO;
+					netdev->features &= ~NETIF_F_TSO6;
+				}
 			}
 
 			/* enable transmits in the hardware, need to do this
-- 
2.26.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200523025109.3313635-1-jeffrey.t.kirsher@intel.com>
2020-05-23  2:51 ` [net-next 14/17] igb: Report speed and duplex as unknown when device is runtime suspended Jeff Kirsher
2020-05-23  2:51 ` [net-next 16/17] e1000e: Disable TSO for buffer overrun workaround Jeff Kirsher

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git