Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30
@ 2020-07-30 17:09 Tony Nguyen
  2020-07-30 17:09 ` [net 1/2] e1000e: continue to init PHY even when failed to disable ULP Tony Nguyen
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Tony Nguyen @ 2020-07-30 17:09 UTC (permalink / raw)
  To: davem; +Cc: Tony Nguyen, netdev, nhorman, sassmann, jeffrey.t.kirsher

This series contains updates to the e1000e and igb drivers.

Aaron Ma allows PHY initialization to continue if ULP disable failed for
e1000e.

Francesco Ruggeri fixes race conditions in igb reset that could cause panics. 

The following are changes since commit 27a2145d6f826d1fad9de06ac541b1016ced3427:
  ibmvnic: Fix IRQ mapping disposal in error path
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue 1GbE

Aaron Ma (1):
  e1000e: continue to init PHY even when failed to disable ULP

Francesco Ruggeri (1):
  igb: reinit_locked() should be called with rtnl_lock

 drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 +---
 drivers/net/ethernet/intel/igb/igb_main.c   | 9 +++++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.26.2


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

* [net 1/2] e1000e: continue to init PHY even when failed to disable ULP
  2020-07-30 17:09 [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Tony Nguyen
@ 2020-07-30 17:09 ` Tony Nguyen
  2020-07-30 17:09 ` [net 2/2] igb: reinit_locked() should be called with rtnl_lock Tony Nguyen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Tony Nguyen @ 2020-07-30 17:09 UTC (permalink / raw)
  To: davem
  Cc: Aaron Ma, netdev, nhorman, sassmann, jeffrey.t.kirsher,
	anthony.l.nguyen, Aaron Brown

From: Aaron Ma <aaron.ma@canonical.com>

After 'commit e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
 for ME systems")',
ThinkPad P14s always failed to disable ULP by ME.
'commit 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")'
break out of init phy:

error log:
[   42.364753] e1000e 0000:00:1f.6 enp0s31f6: Failed to disable ULP
[   42.524626] e1000e 0000:00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[   42.822476] e1000e 0000:00:1f.6 enp0s31f6: Hardware Error

When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.

Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..489bb5b59475 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -301,10 +301,8 @@ static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
 	 */
 	hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
 	ret_val = e1000_disable_ulp_lpt_lp(hw, true);
-	if (ret_val) {
+	if (ret_val)
 		e_warn("Failed to disable ULP\n");
-		goto out;
-	}
 
 	ret_val = hw->phy.ops.acquire(hw);
 	if (ret_val) {
-- 
2.26.2


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

* [net 2/2] igb: reinit_locked() should be called with rtnl_lock
  2020-07-30 17:09 [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Tony Nguyen
  2020-07-30 17:09 ` [net 1/2] e1000e: continue to init PHY even when failed to disable ULP Tony Nguyen
@ 2020-07-30 17:09 ` Tony Nguyen
  2020-07-30 23:27 ` [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Jakub Kicinski
  2020-07-31 23:52 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Tony Nguyen @ 2020-07-30 17:09 UTC (permalink / raw)
  To: davem
  Cc: Francesco Ruggeri, netdev, nhorman, sassmann, jeffrey.t.kirsher,
	anthony.l.nguyen, Aaron Brown

From: Francesco Ruggeri <fruggeri@arista.com>

We observed two panics involving races with igb_reset_task.
The first panic is caused by this race condition:

	kworker			reboot -f

	igb_reset_task
	igb_reinit_locked
	igb_down
	napi_synchronize
				__igb_shutdown
				igb_clear_interrupt_scheme
				igb_free_q_vectors
				igb_free_q_vector
				adapter->q_vector[v_idx] = NULL;
	napi_disable
	Panics trying to access
	adapter->q_vector[v_idx].napi_state

The second panic (a divide error) is caused by this race:

kworker		reboot -f	tx packet

igb_reset_task
		__igb_shutdown
		rtnl_lock()
		...
		igb_clear_interrupt_scheme
		igb_free_q_vectors
		adapter->num_tx_queues = 0
		...
		rtnl_unlock()
rtnl_lock()
igb_reinit_locked
igb_down
igb_up
netif_tx_start_all_queues
				dev_hard_start_xmit
				igb_xmit_frame
				igb_tx_queue_mapping
				Panics on
				r_idx % adapter->num_tx_queues

This commit applies to igb_reset_task the same changes that
were applied to ixgbe in commit 2f90b8657ec9 ("ixgbe: this patch
adds support for DCB to the kernel and ixgbe driver"),
commit 8f4c5c9fb87a ("ixgbe: reinit_locked() should be called with
rtnl_lock") and commit 88adce4ea8f9 ("ixgbe: fix possible race in
reset subtask").

Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8bb3db2cbd41..6e5861bfb0fa 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6224,9 +6224,18 @@ static void igb_reset_task(struct work_struct *work)
 	struct igb_adapter *adapter;
 	adapter = container_of(work, struct igb_adapter, reset_task);
 
+	rtnl_lock();
+	/* If we're already down or resetting, just bail */
+	if (test_bit(__IGB_DOWN, &adapter->state) ||
+	    test_bit(__IGB_RESETTING, &adapter->state)) {
+		rtnl_unlock();
+		return;
+	}
+
 	igb_dump(adapter);
 	netdev_err(adapter->netdev, "Reset adapter\n");
 	igb_reinit_locked(adapter);
+	rtnl_unlock();
 }
 
 /**
-- 
2.26.2


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

* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30
  2020-07-30 17:09 [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Tony Nguyen
  2020-07-30 17:09 ` [net 1/2] e1000e: continue to init PHY even when failed to disable ULP Tony Nguyen
  2020-07-30 17:09 ` [net 2/2] igb: reinit_locked() should be called with rtnl_lock Tony Nguyen
@ 2020-07-30 23:27 ` Jakub Kicinski
  2020-07-31 15:33   ` Nguyen, Anthony L
  2020-07-31 23:52 ` David Miller
  3 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2020-07-30 23:27 UTC (permalink / raw)
  To: Tony Nguyen; +Cc: davem, netdev, nhorman, sassmann, jeffrey.t.kirsher

On Thu, 30 Jul 2020 10:09:36 -0700 Tony Nguyen wrote:
> This series contains updates to the e1000e and igb drivers.
> 
> Aaron Ma allows PHY initialization to continue if ULP disable failed for
> e1000e.
> 
> Francesco Ruggeri fixes race conditions in igb reset that could cause panics. 

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

In the future please try to add Fixes tags on all net submissions
(patch 2). Also - are similar fixes for other Intel drivers in the
works?

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

* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30
  2020-07-30 23:27 ` [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Jakub Kicinski
@ 2020-07-31 15:33   ` Nguyen, Anthony L
  0 siblings, 0 replies; 6+ messages in thread
From: Nguyen, Anthony L @ 2020-07-31 15:33 UTC (permalink / raw)
  To: kuba; +Cc: nhorman, davem, netdev, Kirsher, Jeffrey T, sassmann

On Thu, 2020-07-30 at 16:27 -0700, Jakub Kicinski wrote:
> On Thu, 30 Jul 2020 10:09:36 -0700 Tony Nguyen wrote:
> > This series contains updates to the e1000e and igb drivers.
> > 
> > Aaron Ma allows PHY initialization to continue if ULP disable
> > failed for
> > e1000e.
> > 
> > Francesco Ruggeri fixes race conditions in igb reset that could
> > cause panics. 
> 
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> 
> In the future please try to add Fixes tags on all net submissions
> (patch 2). 

Will do.

> Also - are similar fixes for other Intel drivers in the
> works?

Jeff said he had someone looking into it. I'll track down who's working
on it.

Thanks,
Tony

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

* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30
  2020-07-30 17:09 [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Tony Nguyen
                   ` (2 preceding siblings ...)
  2020-07-30 23:27 ` [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Jakub Kicinski
@ 2020-07-31 23:52 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-07-31 23:52 UTC (permalink / raw)
  To: anthony.l.nguyen; +Cc: netdev, nhorman, sassmann, jeffrey.t.kirsher

From: Tony Nguyen <anthony.l.nguyen@intel.com>
Date: Thu, 30 Jul 2020 10:09:36 -0700

> This series contains updates to the e1000e and igb drivers.
> 
> Aaron Ma allows PHY initialization to continue if ULP disable failed for
> e1000e.
> 
> Francesco Ruggeri fixes race conditions in igb reset that could cause panics. 
 ...
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue 1GbE

Pulled, thank you.

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 17:09 [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Tony Nguyen
2020-07-30 17:09 ` [net 1/2] e1000e: continue to init PHY even when failed to disable ULP Tony Nguyen
2020-07-30 17:09 ` [net 2/2] igb: reinit_locked() should be called with rtnl_lock Tony Nguyen
2020-07-30 23:27 ` [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30 Jakub Kicinski
2020-07-31 15:33   ` Nguyen, Anthony L
2020-07-31 23:52 ` David Miller

Netdev Archive on lore.kernel.org

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

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

Example config snippet for mirrors

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


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