* [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 related [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 related [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