]>
Commit | Line | Data |
---|---|---|
81cfed33 GKH |
1 | From 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 Mon Sep 17 00:00:00 2001 |
2 | From: Benjamin Poirier <bpoirier@suse.com> | |
3 | Date: Fri, 21 Jul 2017 11:36:26 -0700 | |
4 | Subject: e1000e: Separate signaling for link check/link up | |
5 | ||
6 | From: Benjamin Poirier <bpoirier@suse.com> | |
7 | ||
8 | commit 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 upstream. | |
9 | ||
10 | Lennart reported the following race condition: | |
11 | ||
12 | \ e1000_watchdog_task | |
13 | \ e1000e_has_link | |
14 | \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link | |
15 | /* link is up */ | |
16 | mac->get_link_status = false; | |
17 | ||
18 | /* interrupt */ | |
19 | \ e1000_msix_other | |
20 | hw->mac.get_link_status = true; | |
21 | ||
22 | link_active = !hw->mac.get_link_status | |
23 | /* link_active is false, wrongly */ | |
24 | ||
25 | This problem arises because the single flag get_link_status is used to | |
26 | signal two different states: link status needs checking and link status is | |
27 | down. | |
28 | ||
29 | Avoid the problem by using the return value of .check_for_link to signal | |
30 | the link status to e1000e_has_link(). | |
31 | ||
32 | Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca> | |
33 | Signed-off-by: Benjamin Poirier <bpoirier@suse.com> | |
34 | Tested-by: Aaron Brown <aaron.f.brown@intel.com> | |
35 | Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> | |
36 | Signed-off-by: Amit Pundir <amit.pundir@linaro.org> | |
37 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
38 | ||
39 | --- | |
40 | drivers/net/ethernet/intel/e1000e/mac.c | 11 ++++++++--- | |
41 | drivers/net/ethernet/intel/e1000e/netdev.c | 2 +- | |
42 | 2 files changed, 9 insertions(+), 4 deletions(-) | |
43 | ||
44 | --- a/drivers/net/ethernet/intel/e1000e/mac.c | |
45 | +++ b/drivers/net/ethernet/intel/e1000e/mac.c | |
46 | @@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e | |
47 | * Checks to see of the link status of the hardware has changed. If a | |
48 | * change in link status has been detected, then we read the PHY registers | |
49 | * to get the current speed/duplex if link exists. | |
50 | + * | |
51 | + * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link | |
52 | + * up). | |
53 | **/ | |
54 | s32 e1000e_check_for_copper_link(struct e1000_hw *hw) | |
55 | { | |
56 | @@ -423,7 +426,7 @@ s32 e1000e_check_for_copper_link(struct | |
57 | * Change or Rx Sequence Error interrupt. | |
58 | */ | |
59 | if (!mac->get_link_status) | |
60 | - return 0; | |
61 | + return 1; | |
62 | ||
63 | /* First we want to see if the MII Status Register reports | |
64 | * link. If so, then we want to get the current speed/duplex | |
65 | @@ -461,10 +464,12 @@ s32 e1000e_check_for_copper_link(struct | |
66 | * different link partner. | |
67 | */ | |
68 | ret_val = e1000e_config_fc_after_link_up(hw); | |
69 | - if (ret_val) | |
70 | + if (ret_val) { | |
71 | e_dbg("Error configuring flow control\n"); | |
72 | + return ret_val; | |
73 | + } | |
74 | ||
75 | - return ret_val; | |
76 | + return 1; | |
77 | } | |
78 | ||
79 | /** | |
80 | --- a/drivers/net/ethernet/intel/e1000e/netdev.c | |
81 | +++ b/drivers/net/ethernet/intel/e1000e/netdev.c | |
82 | @@ -5017,7 +5017,7 @@ static bool e1000e_has_link(struct e1000 | |
83 | case e1000_media_type_copper: | |
84 | if (hw->mac.get_link_status) { | |
85 | ret_val = hw->mac.ops.check_for_link(hw); | |
86 | - link_active = !hw->mac.get_link_status; | |
87 | + link_active = ret_val > 0; | |
88 | } else { | |
89 | link_active = true; | |
90 | } |