Skip to content

Commit 19110cf

Browse files
gobenjiJeff Kirsher
authored andcommitted
e1000e: Separate signaling for link check/link up
Lennart reported the following race condition: \ e1000_watchdog_task \ e1000e_has_link \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link /* link is up */ mac->get_link_status = false; /* interrupt */ \ e1000_msix_other hw->mac.get_link_status = true; link_active = !hw->mac.get_link_status /* link_active is false, wrongly */ This problem arises because the single flag get_link_status is used to signal two different states: link status needs checking and link status is down. Avoid the problem by using the return value of .check_for_link to signal the link status to e1000e_has_link(). Reported-by: Lennart Sorensen <[email protected]> Signed-off-by: Benjamin Poirier <[email protected]> Tested-by: Aaron Brown <[email protected]> Signed-off-by: Jeff Kirsher <[email protected]>
1 parent d3509f8 commit 19110cf

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
410410
* Checks to see of the link status of the hardware has changed. If a
411411
* change in link status has been detected, then we read the PHY registers
412412
* to get the current speed/duplex if link exists.
413+
*
414+
* Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
415+
* up).
413416
**/
414417
s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
415418
{
@@ -423,7 +426,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
423426
* Change or Rx Sequence Error interrupt.
424427
*/
425428
if (!mac->get_link_status)
426-
return 0;
429+
return 1;
427430

428431
/* First we want to see if the MII Status Register reports
429432
* link. If so, then we want to get the current speed/duplex
@@ -461,10 +464,12 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
461464
* different link partner.
462465
*/
463466
ret_val = e1000e_config_fc_after_link_up(hw);
464-
if (ret_val)
467+
if (ret_val) {
465468
e_dbg("Error configuring flow control\n");
469+
return ret_val;
470+
}
466471

467-
return ret_val;
472+
return 1;
468473
}
469474

470475
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5081,7 +5081,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
50815081
case e1000_media_type_copper:
50825082
if (hw->mac.get_link_status) {
50835083
ret_val = hw->mac.ops.check_for_link(hw);
5084-
link_active = !hw->mac.get_link_status;
5084+
link_active = ret_val > 0;
50855085
} else {
50865086
link_active = true;
50875087
}

0 commit comments

Comments
 (0)