lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 28 Feb 2018 14:20:06 +0900
From:   Benjamin Poirier <bpoirier@...e.com>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Cc:     Alexander Duyck <alexander.duyck@...il.com>,
        intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] e1000e: Fix link status in case of error.

Before commit 19110cfbb34d ("e1000e: Separate signaling for link check/link
up"), errors which happen after "get_link_status = false" in the copper
check_for_link callbacks would be ignored and the link considered up. After
that commit, any error implies that the link is down. Since all
combinations of link up/down and error/no error are possible, do the same
thing as e1000e_phy_has_link_generic() and return the link status in a
separate variable.

Fixes: 19110cfbb34d ("e1000e: Separate signaling for link check/link up")
Signed-off-by: Benjamin Poirier <bpoirier@...e.com>
---
 drivers/net/ethernet/intel/e1000e/82571.c   |  6 ++++--
 drivers/net/ethernet/intel/e1000e/ethtool.c |  5 +++--
 drivers/net/ethernet/intel/e1000e/hw.h      |  2 +-
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 19 +++++++++++--------
 drivers/net/ethernet/intel/e1000e/mac.c     | 26 +++++++++++++++-----------
 drivers/net/ethernet/intel/e1000e/mac.h     |  6 +++---
 drivers/net/ethernet/intel/e1000e/netdev.c  | 11 +++--------
 7 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/82571.c b/drivers/net/ethernet/intel/e1000e/82571.c
index 6b03c8553e59..980ed89e61ea 100644
--- a/drivers/net/ethernet/intel/e1000e/82571.c
+++ b/drivers/net/ethernet/intel/e1000e/82571.c
@@ -40,7 +40,8 @@
 static s32 e1000_get_phy_id_82571(struct e1000_hw *hw);
 static s32 e1000_setup_copper_link_82571(struct e1000_hw *hw);
 static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw);
-static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw);
+static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw,
+					     bool *unused);
 static s32 e1000_write_nvm_eewr_82571(struct e1000_hw *hw, u16 offset,
 				      u16 words, u16 *data);
 static s32 e1000_fix_nvm_checksum_82571(struct e1000_hw *hw);
@@ -1493,6 +1494,7 @@ static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw)
 /**
  *  e1000_check_for_serdes_link_82571 - Check for link (Serdes)
  *  @hw: pointer to the HW structure
+ *  @unused: unused for serdes links
  *
  *  Reports the link state as up or down.
  *
@@ -1509,7 +1511,7 @@ static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw)
  *  4) forced_up (the link has been forced up, it did not autonegotiate)
  *
  **/
-static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw)
+static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw, bool *unused)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	u32 rxcw;
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 003cbd605799..1946ddae06c0 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1753,6 +1753,7 @@ static int e1000_loopback_test(struct e1000_adapter *adapter, u64 *data)
 static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	bool link_status;
 
 	*data = 0;
 	if (hw->phy.media_type == e1000_media_type_internal_serdes) {
@@ -1764,7 +1765,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
 		 * could take as long as 2-3 minutes
 		 */
 		do {
-			hw->mac.ops.check_for_link(hw);
+			hw->mac.ops.check_for_link(hw, NULL);
 			if (hw->mac.serdes_has_link)
 				return *data;
 			msleep(20);
@@ -1772,7 +1773,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
 
 		*data = 1;
 	} else {
-		hw->mac.ops.check_for_link(hw);
+		hw->mac.ops.check_for_link(hw, &link_status);
 		if (hw->mac.autoneg)
 			/* On some Phy/switch combinations, link establishment
 			 * can take a few seconds more than expected.
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index d803b1a12349..4dff6df469bb 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -472,7 +472,7 @@ struct e1000_mac_operations {
 	s32  (*id_led_init)(struct e1000_hw *);
 	s32  (*blink_led)(struct e1000_hw *);
 	bool (*check_mng_mode)(struct e1000_hw *);
-	s32  (*check_for_link)(struct e1000_hw *);
+	s32  (*check_for_link)(struct e1000_hw *, bool *);
 	s32  (*cleanup_led)(struct e1000_hw *);
 	void (*clear_hw_cntrs)(struct e1000_hw *);
 	void (*clear_vfta)(struct e1000_hw *);
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index ff308b05d68c..3d25255289ff 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1363,15 +1363,14 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
 /**
  *  e1000_check_for_copper_link_ich8lan - Check for link (Copper)
  *  @hw: pointer to the HW structure
+ *  @link_status: pointer to whether the link is up or not
  *
  *  Checks to see of the link status of the hardware has changed.  If a
  *  change in link status has been detected, then we read the PHY registers
  *  to get the current speed/duplex if link exists.
- *
- *  Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
- *  up).
  **/
-static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
+static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw,
+					       bool *link_status)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	s32 ret_val, tipg_reg = 0;
@@ -1384,8 +1383,11 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
 	 * get_link_status flag is set upon receiving a Link Status
 	 * Change or Rx Sequence Error interrupt.
 	 */
-	if (!mac->get_link_status)
-		return 1;
+	if (!mac->get_link_status) {
+		*link_status = true;
+		return 0;
+	}
+	*link_status = false;
 
 	/* First we want to see if the MII Status Register reports
 	 * link.  If so, then we want to get the current speed/duplex
@@ -1554,6 +1556,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
 	if (!link)
 		return 0;	/* No link detected */
 
+	*link_status = true;
 	mac->get_link_status = false;
 
 	switch (hw->mac.type) {
@@ -1602,7 +1605,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
 	 * we have already determined whether we have link or not.
 	 */
 	if (!mac->autoneg)
-		return 1;
+		return 0;
 
 	/* Auto-Neg is enabled.  Auto Speed Detection takes care
 	 * of MAC speed/duplex configuration.  So we only need to
@@ -1621,7 +1624,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
 		return ret_val;
 	}
 
-	return 1;
+	return 0;
 }
 
 static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c
index db735644b312..5a5e219fc864 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.c
+++ b/drivers/net/ethernet/intel/e1000e/mac.c
@@ -406,15 +406,13 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
 /**
  *  e1000e_check_for_copper_link - Check for link (Copper)
  *  @hw: pointer to the HW structure
+ *  @link_status: pointer to whether the link is up or not
  *
  *  Checks to see of the link status of the hardware has changed.  If a
  *  change in link status has been detected, then we read the PHY registers
  *  to get the current speed/duplex if link exists.
- *
- *  Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
- *  up).
  **/
-s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
+s32 e1000e_check_for_copper_link(struct e1000_hw *hw, bool *link_status)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	s32 ret_val;
@@ -425,8 +423,11 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
 	 * get_link_status flag is set upon receiving a Link Status
 	 * Change or Rx Sequence Error interrupt.
 	 */
-	if (!mac->get_link_status)
-		return 1;
+	if (!mac->get_link_status) {
+		*link_status = true;
+		return 0;
+	}
+	*link_status = false;
 
 	/* First we want to see if the MII Status Register reports
 	 * link.  If so, then we want to get the current speed/duplex
@@ -439,6 +440,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
 	if (!link)
 		return 0;	/* No link detected */
 
+	*link_status = true;
 	mac->get_link_status = false;
 
 	/* Check if there was DownShift, must be checked
@@ -450,7 +452,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
 	 * we have already determined whether we have link or not.
 	 */
 	if (!mac->autoneg)
-		return 1;
+		return 0;
 
 	/* Auto-Neg is enabled.  Auto Speed Detection takes care
 	 * of MAC speed/duplex configuration.  So we only need to
@@ -469,17 +471,18 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
 		return ret_val;
 	}
 
-	return 1;
+	return 0;
 }
 
 /**
  *  e1000e_check_for_fiber_link - Check for link (Fiber)
  *  @hw: pointer to the HW structure
+ *  @unused: unused for fiber links
  *
  *  Checks for link up on the hardware.  If link is not up and we have
  *  a signal, then we need to force link up.
  **/
-s32 e1000e_check_for_fiber_link(struct e1000_hw *hw)
+s32 e1000e_check_for_fiber_link(struct e1000_hw *hw, bool *unused)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	u32 rxcw;
@@ -540,11 +543,12 @@ s32 e1000e_check_for_fiber_link(struct e1000_hw *hw)
 /**
  *  e1000e_check_for_serdes_link - Check for link (Serdes)
  *  @hw: pointer to the HW structure
+ *  @unused: unused for serdes links
  *
  *  Checks for link up on the hardware.  If link is not up and we have
  *  a signal, then we need to force link up.
  **/
-s32 e1000e_check_for_serdes_link(struct e1000_hw *hw)
+s32 e1000e_check_for_serdes_link(struct e1000_hw *hw, bool *unused)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	u32 rxcw;
@@ -833,7 +837,7 @@ static s32 e1000_poll_fiber_serdes_link_generic(struct e1000_hw *hw)
 		 * link up if we detect a signal. This will allow us to
 		 * communicate with non-autonegotiating link partners.
 		 */
-		ret_val = mac->ops.check_for_link(hw);
+		ret_val = mac->ops.check_for_link(hw, NULL);
 		if (ret_val) {
 			e_dbg("Error while checking for link\n");
 			return ret_val;
diff --git a/drivers/net/ethernet/intel/e1000e/mac.h b/drivers/net/ethernet/intel/e1000e/mac.h
index 8284618af9ff..74299bf1a5bb 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.h
+++ b/drivers/net/ethernet/intel/e1000e/mac.h
@@ -23,9 +23,9 @@
 #define _E1000E_MAC_H_
 
 s32 e1000e_blink_led_generic(struct e1000_hw *hw);
-s32 e1000e_check_for_copper_link(struct e1000_hw *hw);
-s32 e1000e_check_for_fiber_link(struct e1000_hw *hw);
-s32 e1000e_check_for_serdes_link(struct e1000_hw *hw);
+s32 e1000e_check_for_copper_link(struct e1000_hw *hw, bool *link_status);
+s32 e1000e_check_for_fiber_link(struct e1000_hw *hw, bool *unused);
+s32 e1000e_check_for_serdes_link(struct e1000_hw *hw, bool *unused);
 s32 e1000e_cleanup_led_generic(struct e1000_hw *hw);
 s32 e1000e_config_fc_after_link_up(struct e1000_hw *hw);
 s32 e1000e_disable_pcie_master(struct e1000_hw *hw);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9fd4050a91ca..548250108dc5 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5088,19 +5088,14 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
 	 */
 	switch (hw->phy.media_type) {
 	case e1000_media_type_copper:
-		if (hw->mac.get_link_status) {
-			ret_val = hw->mac.ops.check_for_link(hw);
-			link_active = ret_val > 0;
-		} else {
-			link_active = true;
-		}
+		ret_val = hw->mac.ops.check_for_link(hw, &link_active);
 		break;
 	case e1000_media_type_fiber:
-		ret_val = hw->mac.ops.check_for_link(hw);
+		ret_val = hw->mac.ops.check_for_link(hw, NULL);
 		link_active = !!(er32(STATUS) & E1000_STATUS_LU);
 		break;
 	case e1000_media_type_internal_serdes:
-		ret_val = hw->mac.ops.check_for_link(hw);
+		ret_val = hw->mac.ops.check_for_link(hw, NULL);
 		link_active = hw->mac.serdes_has_link;
 		break;
 	default:
-- 
2.16.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ