diff mbox

[1/1] ixgbe: synchronize link_up and link_speed of a slave interface

Message ID 1451538295-15823-2-git-send-email-zyjzyj2000@gmail.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Zhu Yanjun Dec. 31, 2015, 5:04 a.m. UTC
From: Zhu Yanjun <yanjun.zhu@windriver.com>

According to the suggestions from Rustad, Mark D, to all the slave 
interfaces, the link_speed and link_up should be synchronized since
the time span between link_up and link_speed will make some virtual
NICs not work well, such as a bonding driver in 802.3ad mode.

Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Kirsher, Jeffrey T Dec. 31, 2015, 5:37 a.m. UTC | #1
On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000@gmail.com wrote:
> From: Zhu Yanjun <yanjun.zhu@windriver.com>
> 
> According to the suggestions from Rustad, Mark D, to all the slave 
> interfaces, the link_speed and link_up should be synchronized since
> the time span between link_up and link_speed will make some virtual
> NICs not work well, such as a bonding driver in 802.3ad mode.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
> ---

Since this is version 5 of your original patch, you should be putting
the change log of this patch here, so that we can follow how you got to
this point.  This helpful for the reviewers (and me, the maintainer).
 For example:
v2: dropped the "else" case of "if" statement, based on feedback from
    Jeff Kirsher
v3: changed the if statement to simply return when the link speed is
    unknown on X540 parts based on feedback from Emil Tantilov
v4: updated code comment and if statement to test for IFF_SLAVE flag
V5: simplified code comment and if statement to only test for IFF_SLAVE
    flag and unknown link speed.

Of course, you can expand on what I have above, I just did a quick
summary of the changes as an example of a change log. 

>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++++
>  1 file changed, 5 insertions(+)
Zhu Yanjun Dec. 31, 2015, 7:11 a.m. UTC | #2
Hi, all

Thanks for the reply from Jeff.

V2: Based on feedback from Jeff Kirsher, it is not appropriate to continue
    in the function ixgbe_watchdog_link_is_up without link_speed since this will
    make some virtual NICs not work well.

V3: Based on feedback from Emil Tantilov, the time span between link_up 
    and link_speed is not important when the X540 NIC acts as an independent 
    interface. 

V4: According to Rustad, Mark D, maybe it is related with copper phy. To make
    fiber phy more robust, synchronize both the link_up and link_speed of a slave
    interface in ixgbe driver.

V5: Based on feedback from Rustad, Mark D, simplify code comment and if statement 
    to only test for IFF_SLAVE flag and unknown link speed.

V6: Based on feedback from Jeff Kirsher, the patch format is adjusted 
    and change log of this patch are added.

Best Regards!
Zhu Yanjun
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..fc461b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,11 @@  static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* For all slave interfaces, wait for the link_speed to be known. */
+	if ((netdev->flags & IFF_SLAVE) &&
+	    (link_speed == IXGBE_LINK_SPEED_UNKNOWN))
+		return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {