Skip to content

Commit da2b4ae

Browse files
gobenjigregkh
authored andcommitted
e1000e: Fix link check race condition
commit e2710db upstream. Alex reported the following race condition: /* link goes up... interrupt... schedule watchdog */ \ e1000_watchdog_task \ e1000e_has_link \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link \ e1000e_phy_has_link_generic(..., &link) link = true /* link goes down... interrupt */ \ e1000_msix_other hw->mac.get_link_status = true /* link is up */ mac->get_link_status = false link_active = true /* link_active is true, wrongly, and stays so because * get_link_status is false */ Avoid this problem by making sure that we don't set get_link_status = false after having checked the link. It seems this problem has been present since the introduction of e1000e. Link: https://lkml.org/lkml/2018/1/29/338 Reported-by: Alexander Duyck <alexander.duyck@gmail.com> Signed-off-by: Benjamin Poirier <bpoirier@suse.com> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com> Tested-by: Aaron Brown <aaron.f.brown@intel.com> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Cc: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent dc2aa33 commit da2b4ae

2 files changed

Lines changed: 24 additions & 21 deletions

File tree

drivers/net/ethernet/intel/e1000e/ich8lan.c

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,19 +1383,20 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
13831383
*/
13841384
if (!mac->get_link_status)
13851385
return 0;
1386+
mac->get_link_status = false;
13861387

13871388
/* First we want to see if the MII Status Register reports
13881389
* link. If so, then we want to get the current speed/duplex
13891390
* of the PHY.
13901391
*/
13911392
ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
13921393
if (ret_val)
1393-
return ret_val;
1394+
goto out;
13941395

13951396
if (hw->mac.type == e1000_pchlan) {
13961397
ret_val = e1000_k1_gig_workaround_hv(hw, link);
13971398
if (ret_val)
1398-
return ret_val;
1399+
goto out;
13991400
}
14001401

14011402
/* When connected at 10Mbps half-duplex, some parts are excessively
@@ -1428,7 +1429,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
14281429

14291430
ret_val = hw->phy.ops.acquire(hw);
14301431
if (ret_val)
1431-
return ret_val;
1432+
goto out;
14321433

14331434
if (hw->mac.type == e1000_pch2lan)
14341435
emi_addr = I82579_RX_CONFIG;
@@ -1450,7 +1451,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
14501451
hw->phy.ops.release(hw);
14511452

14521453
if (ret_val)
1453-
return ret_val;
1454+
goto out;
14541455

14551456
if (hw->mac.type >= e1000_pch_spt) {
14561457
u16 data;
@@ -1459,14 +1460,14 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
14591460
if (speed == SPEED_1000) {
14601461
ret_val = hw->phy.ops.acquire(hw);
14611462
if (ret_val)
1462-
return ret_val;
1463+
goto out;
14631464

14641465
ret_val = e1e_rphy_locked(hw,
14651466
PHY_REG(776, 20),
14661467
&data);
14671468
if (ret_val) {
14681469
hw->phy.ops.release(hw);
1469-
return ret_val;
1470+
goto out;
14701471
}
14711472

14721473
ptr_gap = (data & (0x3FF << 2)) >> 2;
@@ -1480,18 +1481,18 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
14801481
}
14811482
hw->phy.ops.release(hw);
14821483
if (ret_val)
1483-
return ret_val;
1484+
goto out;
14841485
} else {
14851486
ret_val = hw->phy.ops.acquire(hw);
14861487
if (ret_val)
1487-
return ret_val;
1488+
goto out;
14881489

14891490
ret_val = e1e_wphy_locked(hw,
14901491
PHY_REG(776, 20),
14911492
0xC023);
14921493
hw->phy.ops.release(hw);
14931494
if (ret_val)
1494-
return ret_val;
1495+
goto out;
14951496

14961497
}
14971498
}
@@ -1518,15 +1519,15 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
15181519
(hw->adapter->pdev->device == E1000_DEV_ID_PCH_I218_V3)) {
15191520
ret_val = e1000_k1_workaround_lpt_lp(hw, link);
15201521
if (ret_val)
1521-
return ret_val;
1522+
goto out;
15221523
}
15231524
if (hw->mac.type >= e1000_pch_lpt) {
15241525
/* Set platform power management values for
15251526
* Latency Tolerance Reporting (LTR)
15261527
*/
15271528
ret_val = e1000_platform_pm_pch_lpt(hw, link);
15281529
if (ret_val)
1529-
return ret_val;
1530+
goto out;
15301531
}
15311532

15321533
/* Clear link partner's EEE ability */
@@ -1549,9 +1550,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
15491550
}
15501551

15511552
if (!link)
1552-
return 0; /* No link detected */
1553-
1554-
mac->get_link_status = false;
1553+
goto out;
15551554

15561555
switch (hw->mac.type) {
15571556
case e1000_pch2lan:
@@ -1617,6 +1616,10 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
16171616
e_dbg("Error configuring flow control\n");
16181617

16191618
return ret_val;
1619+
1620+
out:
1621+
mac->get_link_status = true;
1622+
return ret_val;
16201623
}
16211624

16221625
static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)

drivers/net/ethernet/intel/e1000e/mac.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -424,19 +424,15 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
424424
*/
425425
if (!mac->get_link_status)
426426
return 0;
427+
mac->get_link_status = false;
427428

428429
/* First we want to see if the MII Status Register reports
429430
* link. If so, then we want to get the current speed/duplex
430431
* of the PHY.
431432
*/
432433
ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
433-
if (ret_val)
434-
return ret_val;
435-
436-
if (!link)
437-
return 0; /* No link detected */
438-
439-
mac->get_link_status = false;
434+
if (ret_val || !link)
435+
goto out;
440436

441437
/* Check if there was DownShift, must be checked
442438
* immediately after link-up
@@ -465,6 +461,10 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
465461
e_dbg("Error configuring flow control\n");
466462

467463
return ret_val;
464+
465+
out:
466+
mac->get_link_status = true;
467+
return ret_val;
468468
}
469469

470470
/**

0 commit comments

Comments
 (0)