netdev - e1000: LED indicates trouble, and [PATCH] enable/disable of force-link-up

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]

Message-Id: <20081203163636ohashi-h@mail.jp.nec.com>
Date:	Wed, 3 Dec 2008 16:36:36 +0900
From:	ohashi-h@...dnes.nec.co.jp
To:	jeffrey.t.kirsher@...el.com, jesse.brandeburg@...el.com,
	bruce.w.allan@...el.com, peter.p.waskiewicz.jr@...el.com,
	john.ronciak@...el.com, jgarzik@...ox.com
Cc:	netdev@...r.kernel.org, e1000-devel@...ts.sourceforge.net
Subject: e1000: LED indicates trouble, and [PATCH] enable/disable of force-link-up

From: Hiroki Ohashi  <ohashi-h@...dnes.nec.co.jp>

Hi all,

I faced the following situations.
The LED of NIC was indicated, even when the system was not linked
the network (auto-negotiation failed or link partner cannot 
auto-negotiate). 
LED should be indicate only when the network was linked.
And so, it is a problem that LED indicates.

I think the indicating the LED is a problem for maintenance.
I cannot know the condition of the link, because I confirm 
to link up by LED's indication.

We are using 82571EB ethernet controler and serdes interface.

I think this problem is caused by the e1000 device driver
setting LU and SLU bits in the interface and forcing the link
to go up.
I deleted this logic. (the following modification.) 
Then, LED was turned off.

================================
--- e1000_hw.c.org
+++ e1000_hw.c
@@ -3069,6 +3069,7 @@I
             hw->autoneg_failed = 1;
             return 0;
         }
+#if 0
         DEBUGOUT("NOT RXing /C/, disable AutoNeg and force link.\n");
 
         /* Disable auto-negotiation in the TXCW register */
@@ -3078,6 +3079,7 @@
         ctrl = E1000_READ_REG(hw, CTRL);
         ctrl |= (E1000_CTRL_SLU | E1000_CTRL_FD);
         E1000_WRITE_REG(hw, CTRL, ctrl);
+#endif
 
         /* Configure Flow Control after forcing link up. */
         ret_val = e1000_config_fc_after_link_up(hw);
================================


I think the e1000 driver should enable/disable force-link-up
depending on the condition of the link.
But I don't know a good way to do this.
If you have any good ideas, please let me know.

As a temporary idea, I thought of a way of adding a module parameter
to control enable/disable force-link-up.
The patch is as follows.

This patch adds "disable_force_link_up_when_autoneg_failed"
as module parameter. When this is 0, force-link-up is disable.

From: Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>
Signed-off-by: Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>

================================
--- drivers/net/e1000/e1000_hw.h.org
+++ drivers/net/e1000/e1000_hw.h
@@ -1469,6 +1469,7 @@ struct e1000_hw {
 	bool			has_manc2h;
 	bool			rx_needs_kicking;
 	bool			has_smbus;
+	bool			disable_force_link_up_when_autoneg_failed;
 };
--- drivers/net/e1000/e1000_param.c.org
+++ drivers/net/e1000/e1000_param.c
@@ -196,6 +196,13 @@ E1000_PARAM(SmartPowerDownEnable, "Enabl
  */
 E1000_PARAM(KumeranLockLoss, "Enable Kumeran lock loss workaround");
 
+/* Enable Force Link Up When AutoNeg Failed
+ *
+ * Valid Range: 0, 1
+ *
+ * Default Value: 1 (force link up enabled)  */
+E1000_PARAM(EnableForceLinkOnSERDES, "Enable force link up when AutoNeg failed");
+
 struct e1000_option {
 	enum { enable_option, range_option, list_option } type;
 	const char *name;
@@ -534,6 +541,23 @@ void __devinit e1000_check_options(struc
 			adapter->hw.kmrn_lock_loss_workaround_disabled = !opt.def;
 		}
 	}
+	{ /* Enable Force Link Up When AutoNeg Failed */
+		struct e1000_option opt = {
+			.type = enable_option,
+			.name = "Enable Force Link Up When AutoNeg Failed",
+			.err  = "defaulting to Enabled",
+			.def  = OPTION_ENABLED
+		};
+
+		if (num_EnableForceLinkOnSERDES > bd) {
+			unsigned int force_link_up = EnableForceLinkOnSERDES [bd];
+			e1000_validate_option(&force_link_up, &opt, adapter);
+			adapter->hw.disable_force_link_up_when_autoneg_failed = !force_link_up;
+
+		} else {
+			adapter->hw.disable_force_link_up_when_autoneg_failed = !opt.def;
+		}
+	}
 
 	switch (adapter->hw.media_type) {
 	case e1000_media_type_fiber:
--- drivers/net/e1000/e1000_hw.c.org
+++ drivers/net/e1000/e1000_hw.c
@@ -3062,6 +3062,12 @@ s32 e1000_check_for_link(struct e1000_hw
             hw->autoneg_failed = 1;
             return 0;
         }
+
+        if (hw->disable_force_link_up_when_autoneg_failed) {
+            DEBUGOUT("Force Link Up Disabled\n");
+            return -E1000_ERR_PHY;
+        }
+
         DEBUGOUT("NOT RXing /C/, disable AutoNeg and force link.\n");
 
         /* Disable auto-negotiation in the TXCW register */
================================
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html