[PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zyjzyj2000]

From: Zhu Yanjun <zyjzyj2000@gmail.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..cb9d310 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	       (flow_rx ? "RX" :
 	       (flow_tx ? "TX" : "None"))));
 
-	netif_carrier_on(netdev);
+	/*
+	 * In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 */
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
+		netif_carrier_on(netdev);
+	} else {
+		netif_carrier_on(netdev);
+	}
+
 	ixgbe_check_vf_rate_limit(adapter);
 
 	/* enable transmits */
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zyjzyj2000]

From: Zhu Yanjun <zyjzyj2000@gmail.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..cb9d310 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	       (flow_rx ? "RX" :
 	       (flow_tx ? "TX" : "None"))));
 
-	netif_carrier_on(netdev);
+	/*
+	 * In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 */
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
+		netif_carrier_on(netdev);
+	} else {
+		netif_carrier_on(netdev);
+	}
+
 	ixgbe_check_vf_rate_limit(adapter);
 
 	/* enable transmits */
-- 
1.7.9.5

Re: [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 2615 bytes --]

On Wed, 2015-12-23 at 14:46 +0800, zyjzyj2000@gmail.com wrote:
> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> 
> In X540 NIC, there is a time span between reporting "link on" and
> getting the speed and duplex. To a bonding driver in 802.3ad mode,
> this time span will make it not work well if the time span is big
> enough. The big time span will make bonding driver change the state
> of
> the slave device to up while the speed and duplex of the slave device
> can not be gotten. Later the bonding driver will not have change to
> get the speed and duplex of the slave device. The speed and duplex of
> the slave device are important to a bonding driver in 802.3ad mode.
> 
> To 82599_SFP NIC and other kinds of NICs, this problem does
> not exist. As such, it is necessary for X540 to report"link on" when
> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> 
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16
> +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index aed8d02..cb9d310 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
> ixgbe_adapter *adapter)
>  	       (flow_rx ? "RX" :
>  	       (flow_tx ? "TX" : "None"))));
>  
> -	netif_carrier_on(netdev);
> +	/*
> +	 * In X540 NIC, there is a time span between reporting "link
> on"
> +	 * and getting the speed and duplex. To a bonding driver in
> 802.3ad
> +	 * mode, this time span will make it not work well if the
> time span
> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs,
> this
> +	 * problem does not exist. As such, it is better for X540 to
> report
> +	 * "link on" when the link speed is not
> IXGBE_LINK_SPEED_UNKNOWN.
> +	 */
> +	if ((hw->mac.type == ixgbe_mac_X540) &&
> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> +		netif_carrier_on(netdev);
> +	} else {
> +		netif_carrier_on(netdev);
> +	}
> +
>  	ixgbe_check_vf_rate_limit(adapter);
>  
>  	/* enable transmits */

This patch only adds a needless test before
calling netif_carrier_on(netdev), since the call happens no matter the
branch you take, it appears your running into a timing issue.  So
adding a wait() before calling netif_carrier_on(netdev) will accomplish
the same result and you do not have to add a useless test.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Jeff Kirsher]

On Wed, 2015-12-23 at 14:46 +0800, zyjzyj2000 at gmail.com wrote:
> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> 
> In X540 NIC, there is a time span between reporting "link on" and
> getting the speed and duplex. To a bonding driver in 802.3ad mode,
> this time span will make it not work well if the time span is big
> enough. The big time span will make bonding driver change the state
> of
> the slave device to up while the speed and duplex of the slave device
> can not be gotten. Later the bonding driver will not have change to
> get the speed and duplex of the slave device. The speed and duplex of
> the slave device are important to a bonding driver in 802.3ad mode.
> 
> To 82599_SFP NIC and other kinds of NICs, this problem does
> not exist. As such, it is necessary for X540 to report"link on" when
> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> 
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
> ?drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |???16
> +++++++++++++++-
> ?1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index aed8d02..cb9d310 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
> ixgbe_adapter *adapter)
> ?	???????(flow_rx ? "RX" :
> ?	???????(flow_tx ? "TX" : "None"))));
> ?
> -	netif_carrier_on(netdev);
> +	/*
> +	?* In X540 NIC, there is a time span between reporting "link
> on"
> +	?* and getting the speed and duplex. To a bonding driver in
> 802.3ad
> +	?* mode, this time span will make it not work well if the
> time span
> +	?* is big enough. To 82599_SFP NIC and other kinds of NICs,
> this
> +	?* problem does not exist. As such, it is better for X540 to
> report
> +	?* "link on" when the link speed is not
> IXGBE_LINK_SPEED_UNKNOWN.
> +	?*/
> +	if ((hw->mac.type == ixgbe_mac_X540) &&
> +	????(link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> +		netif_carrier_on(netdev);
> +	} else {
> +		netif_carrier_on(netdev);
> +	}
> +
> ?	ixgbe_check_vf_rate_limit(adapter);
> ?
> ?	/* enable transmits */

This patch only adds a needless test before
calling?netif_carrier_on(netdev), since the call happens no matter the
branch you take, it appears your running into a timing issue. ?So
adding a wait() before calling?netif_carrier_on(netdev) will accomplish
the same result and you do not have to add a useless test.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151223/f955f020/attachment.asc>

Re: [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Sergei Shtylyov]

Hello.

On 12/23/2015 9:46 AM, zyjzyj2000@gmail.com wrote:

> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
> In X540 NIC, there is a time span between reporting "link on" and
> getting the speed and duplex. To a bonding driver in 802.3ad mode,
> this time span will make it not work well if the time span is big
> enough. The big time span will make bonding driver change the state of
> the slave device to up while the speed and duplex of the slave device
> can not be gotten. Later the bonding driver will not have change to
> get the speed and duplex of the slave device. The speed and duplex of
> the slave device are important to a bonding driver in 802.3ad mode.
>
> To 82599_SFP NIC and other kinds of NICs, this problem does
> not exist. As such, it is necessary for X540 to report"link on" when
> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index aed8d02..cb9d310 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
>   	       (flow_rx ? "RX" :
>   	       (flow_tx ? "TX" : "None"))));
>
> -	netif_carrier_on(netdev);
> +	/*
> +	 * In X540 NIC, there is a time span between reporting "link on"
> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
> +	 * mode, this time span will make it not work well if the time span
> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
> +	 * problem does not exist. As such, it is better for X540 to report
> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> +	 */
> +	if ((hw->mac.type == ixgbe_mac_X540) &&
> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> +		netif_carrier_on(netdev);
> +	} else {
> +		netif_carrier_on(netdev);
> +	}
> +

    {} not needed here. And you do the same thing regardless of whether your 
check succeeds or not, this doesn't make sense.

[...]

MBR, Sergei

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Sergei Shtylyov]

Hello.

On 12/23/2015 9:46 AM, zyjzyj2000 at gmail.com wrote:

> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
> In X540 NIC, there is a time span between reporting "link on" and
> getting the speed and duplex. To a bonding driver in 802.3ad mode,
> this time span will make it not work well if the time span is big
> enough. The big time span will make bonding driver change the state of
> the slave device to up while the speed and duplex of the slave device
> can not be gotten. Later the bonding driver will not have change to
> get the speed and duplex of the slave device. The speed and duplex of
> the slave device are important to a bonding driver in 802.3ad mode.
>
> To 82599_SFP NIC and other kinds of NICs, this problem does
> not exist. As such, it is necessary for X540 to report"link on" when
> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index aed8d02..cb9d310 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
>   	       (flow_rx ? "RX" :
>   	       (flow_tx ? "TX" : "None"))));
>
> -	netif_carrier_on(netdev);
> +	/*
> +	 * In X540 NIC, there is a time span between reporting "link on"
> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
> +	 * mode, this time span will make it not work well if the time span
> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
> +	 * problem does not exist. As such, it is better for X540 to report
> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> +	 */
> +	if ((hw->mac.type == ixgbe_mac_X540) &&
> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> +		netif_carrier_on(netdev);
> +	} else {
> +		netif_carrier_on(netdev);
> +	}
> +

    {} not needed here. And you do the same thing regardless of whether your 
check succeeds or not, this doesn't make sense.

[...]

MBR, Sergei

Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>Behalf Of zyjzyj2000@gmail.com
>Sent: Tuesday, December 22, 2015 10:47 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
>In X540 NIC, there is a time span between reporting "link on" and
>getting the speed and duplex. To a bonding driver in 802.3ad mode,
>this time span will make it not work well if the time span is big
>enough. The big time span will make bonding driver change the state of
>the slave device to up while the speed and duplex of the slave device
>can not be gotten. Later the bonding driver will not have change to
>get the speed and duplex of the slave device. The speed and duplex of
>the slave device are important to a bonding driver in 802.3ad mode.
>
>To 82599_SFP NIC and other kinds of NICs, this problem does
>not exist. As such, it is necessary for X540 to report"link on" when
>the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>
>Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>index aed8d02..cb9d310 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>ixgbe_adapter *adapter)
> 	       (flow_rx ? "RX" :
> 	       (flow_tx ? "TX" : "None"))));
>
>-	netif_carrier_on(netdev);
>+	/*
>+	 * In X540 NIC, there is a time span between reporting "link on"
>+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>+	 * mode, this time span will make it not work well if the time span
>+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>+	 * problem does not exist. As such, it is better for X540 to report
>+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>+	 */
>+	if ((hw->mac.type == ixgbe_mac_X540) &&
>+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>+		netif_carrier_on(netdev);
>+	} else {
>+		netif_carrier_on(netdev);
>+	}
>+
> 	ixgbe_check_vf_rate_limit(adapter);
>
> 	/* enable transmits */
>--
>1.7.9.5

NAK

I have already submitted a patch that will address the issue with bonding reporting
unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
http://patchwork.ozlabs.org/patch/552485/

The bonding driver gets the speed from ethtool and this is where the reporting needs
to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a 
certain rate and as such will not be able to detect rapid link changes.

If there is a case where link_speed is unknown when entering this function it's probably
better to just bail rather than have this hack around the netif_carrier_on() especially
after the driver already reported link status change. Rapid link changes can occur between
link partners and not just for X540.

Thanks,
Emil


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>Behalf Of zyjzyj2000 at gmail.com
>Sent: Tuesday, December 22, 2015 10:47 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
>In X540 NIC, there is a time span between reporting "link on" and
>getting the speed and duplex. To a bonding driver in 802.3ad mode,
>this time span will make it not work well if the time span is big
>enough. The big time span will make bonding driver change the state of
>the slave device to up while the speed and duplex of the slave device
>can not be gotten. Later the bonding driver will not have change to
>get the speed and duplex of the slave device. The speed and duplex of
>the slave device are important to a bonding driver in 802.3ad mode.
>
>To 82599_SFP NIC and other kinds of NICs, this problem does
>not exist. As such, it is necessary for X540 to report"link on" when
>the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>
>Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>index aed8d02..cb9d310 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>ixgbe_adapter *adapter)
> 	       (flow_rx ? "RX" :
> 	       (flow_tx ? "TX" : "None"))));
>
>-	netif_carrier_on(netdev);
>+	/*
>+	 * In X540 NIC, there is a time span between reporting "link on"
>+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>+	 * mode, this time span will make it not work well if the time span
>+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>+	 * problem does not exist. As such, it is better for X540 to report
>+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>+	 */
>+	if ((hw->mac.type == ixgbe_mac_X540) &&
>+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>+		netif_carrier_on(netdev);
>+	} else {
>+		netif_carrier_on(netdev);
>+	}
>+
> 	ixgbe_check_vf_rate_limit(adapter);
>
> 	/* enable transmits */
>--
>1.7.9.5

NAK

I have already submitted a patch that will address the issue with bonding reporting
unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
http://patchwork.ozlabs.org/patch/552485/

The bonding driver gets the speed from ethtool and this is where the reporting needs
to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a 
certain rate and as such will not be able to detect rapid link changes.

If there is a case where link_speed is unknown when entering this function it's probably
better to just bail rather than have this hack around the netif_carrier_on() especially
after the driver already reported link status change. Rapid link changes can occur between
link partners and not just for X540.

Thanks,
Emil

Re: [E1000-devel] [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Stephen Hemminger]

On Wed, 23 Dec 2015 15:59:24 +0000
"Tantilov, Emil S" <emil.s.tantilov@intel.com> wrote:

> >-----Original Message-----
> >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> >Behalf Of zyjzyj2000@gmail.com
> >Sent: Tuesday, December 22, 2015 10:47 PM
> >To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
> >Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
> >A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
> >devel@lists.sourceforge.net
> >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
> >Vincent (Wind River)
> >Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
> >reporting "link on" and getting speed and duplex
> >
> >From: Zhu Yanjun <zyjzyj2000@gmail.com>
> >
> >In X540 NIC, there is a time span between reporting "link on" and
> >getting the speed and duplex. To a bonding driver in 802.3ad mode,
> >this time span will make it not work well if the time span is big
> >enough. The big time span will make bonding driver change the state of
> >the slave device to up while the speed and duplex of the slave device
> >can not be gotten. Later the bonding driver will not have change to
> >get the speed and duplex of the slave device. The speed and duplex of
> >the slave device are important to a bonding driver in 802.3ad mode.
> >
> >To 82599_SFP NIC and other kinds of NICs, this problem does
> >not exist. As such, it is necessary for X540 to report"link on" when
> >the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> >
> >Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> >---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >index aed8d02..cb9d310 100644
> >--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
> >ixgbe_adapter *adapter)
> > 	       (flow_rx ? "RX" :
> > 	       (flow_tx ? "TX" : "None"))));
> >
> >-	netif_carrier_on(netdev);
> >+	/*
> >+	 * In X540 NIC, there is a time span between reporting "link on"
> >+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
> >+	 * mode, this time span will make it not work well if the time span
> >+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
> >+	 * problem does not exist. As such, it is better for X540 to report
> >+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> >+	 */
> >+	if ((hw->mac.type == ixgbe_mac_X540) &&
> >+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> >+		netif_carrier_on(netdev);
> >+	} else {
> >+		netif_carrier_on(netdev);
> >+	}
> >+
> > 	ixgbe_check_vf_rate_limit(adapter);
> >
> > 	/* enable transmits */
> >--
> >1.7.9.5  
> 
> NAK
> 
> I have already submitted a patch that will address the issue with bonding reporting
> unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
> http://patchwork.ozlabs.org/patch/552485/
> 
> The bonding driver gets the speed from ethtool and this is where the reporting needs
> to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a 
> certain rate and as such will not be able to detect rapid link changes.

Kernel has netdev notifiers, these should really always be used rather than
polling. The polling stuff was added back in ancient kernel days when netdev
notifiers didn't work because most drivers were broken.

[Intel-wired-lan] [E1000-devel] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Stephen Hemminger]

On Wed, 23 Dec 2015 15:59:24 +0000
"Tantilov, Emil S" <emil.s.tantilov@intel.com> wrote:

> >-----Original Message-----
> >From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> >Behalf Of zyjzyj2000 at gmail.com
> >Sent: Tuesday, December 22, 2015 10:47 PM
> >To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
> >Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
> >A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
> >devel at lists.sourceforge.net
> >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
> >Vincent (Wind River)
> >Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
> >reporting "link on" and getting speed and duplex
> >
> >From: Zhu Yanjun <zyjzyj2000@gmail.com>
> >
> >In X540 NIC, there is a time span between reporting "link on" and
> >getting the speed and duplex. To a bonding driver in 802.3ad mode,
> >this time span will make it not work well if the time span is big
> >enough. The big time span will make bonding driver change the state of
> >the slave device to up while the speed and duplex of the slave device
> >can not be gotten. Later the bonding driver will not have change to
> >get the speed and duplex of the slave device. The speed and duplex of
> >the slave device are important to a bonding driver in 802.3ad mode.
> >
> >To 82599_SFP NIC and other kinds of NICs, this problem does
> >not exist. As such, it is necessary for X540 to report"link on" when
> >the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> >
> >Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> >---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >index aed8d02..cb9d310 100644
> >--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
> >ixgbe_adapter *adapter)
> > 	       (flow_rx ? "RX" :
> > 	       (flow_tx ? "TX" : "None"))));
> >
> >-	netif_carrier_on(netdev);
> >+	/*
> >+	 * In X540 NIC, there is a time span between reporting "link on"
> >+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
> >+	 * mode, this time span will make it not work well if the time span
> >+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
> >+	 * problem does not exist. As such, it is better for X540 to report
> >+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
> >+	 */
> >+	if ((hw->mac.type == ixgbe_mac_X540) &&
> >+	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
> >+		netif_carrier_on(netdev);
> >+	} else {
> >+		netif_carrier_on(netdev);
> >+	}
> >+
> > 	ixgbe_check_vf_rate_limit(adapter);
> >
> > 	/* enable transmits */
> >--
> >1.7.9.5  
> 
> NAK
> 
> I have already submitted a patch that will address the issue with bonding reporting
> unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
> http://patchwork.ozlabs.org/patch/552485/
> 
> The bonding driver gets the speed from ethtool and this is where the reporting needs
> to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a 
> certain rate and as such will not be able to detect rapid link changes.

Kernel has netdev notifiers, these should really always be used rather than
polling. The polling stuff was added back in ancient kernel days when netdev
notifiers didn't work because most drivers were broken.

Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zhuyj]

On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>> Behalf Of zyjzyj2000@gmail.com
>> Sent: Tuesday, December 22, 2015 10:47 PM
>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>> devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>> reporting "link on" and getting speed and duplex
>>
>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>
>> In X540 NIC, there is a time span between reporting "link on" and
>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>> this time span will make it not work well if the time span is big
>> enough. The big time span will make bonding driver change the state of
>> the slave device to up while the speed and duplex of the slave device
>> can not be gotten. Later the bonding driver will not have change to
>> get the speed and duplex of the slave device. The speed and duplex of
>> the slave device are important to a bonding driver in 802.3ad mode.
>>
>> To 82599_SFP NIC and other kinds of NICs, this problem does
>> not exist. As such, it is necessary for X540 to report"link on" when
>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>
>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index aed8d02..cb9d310 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>> ixgbe_adapter *adapter)
>> 	       (flow_rx ? "RX" :
>> 	       (flow_tx ? "TX" : "None"))));
>>
>> -	netif_carrier_on(netdev);
>> +	/*
>> +	 * In X540 NIC, there is a time span between reporting "link on"
>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>> +	 * mode, this time span will make it not work well if the time span
>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>> +	 * problem does not exist. As such, it is better for X540 to report
>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>> +	 */
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>> +		netif_carrier_on(netdev);
>> +	} else {
>> +		netif_carrier_on(netdev);
>> +	}
>> +
>> 	ixgbe_check_vf_rate_limit(adapter);
>>
>> 	/* enable transmits */
>> --
>> 1.7.9.5
> NAK
>
> I have already submitted a patch that will address the issue with bonding reporting
> unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
> http://patchwork.ozlabs.org/patch/552485/
>
> The bonding driver gets the speed from ethtool and this is where the reporting needs
> to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a
> certain rate and as such will not be able to detect rapid link changes.
Thanks for your reply. The root cause is different from my problem. My 
problem is that
"link up" is prior to "speed and duplex". That is, the physical NIC 
reports "link up" while
the speed is unknown at the same time. We can run "ethtool ethx" to 
confirm it.

Any way, thanks a lot.

Zhu Yanjun
>
> If there is a case where link_speed is unknown when entering this function it's probably
> better to just bail rather than have this hack around the netif_carrier_on() especially
> after the driver already reported link status change. Rapid link changes can occur between
> link partners and not just for X540.
>
> Thanks,
> Emil
>

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zhuyj]

On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>> Behalf Of zyjzyj2000 at gmail.com
>> Sent: Tuesday, December 22, 2015 10:47 PM
>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>> devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>> reporting "link on" and getting speed and duplex
>>
>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>
>> In X540 NIC, there is a time span between reporting "link on" and
>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>> this time span will make it not work well if the time span is big
>> enough. The big time span will make bonding driver change the state of
>> the slave device to up while the speed and duplex of the slave device
>> can not be gotten. Later the bonding driver will not have change to
>> get the speed and duplex of the slave device. The speed and duplex of
>> the slave device are important to a bonding driver in 802.3ad mode.
>>
>> To 82599_SFP NIC and other kinds of NICs, this problem does
>> not exist. As such, it is necessary for X540 to report"link on" when
>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>
>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index aed8d02..cb9d310 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>> ixgbe_adapter *adapter)
>> 	       (flow_rx ? "RX" :
>> 	       (flow_tx ? "TX" : "None"))));
>>
>> -	netif_carrier_on(netdev);
>> +	/*
>> +	 * In X540 NIC, there is a time span between reporting "link on"
>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>> +	 * mode, this time span will make it not work well if the time span
>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>> +	 * problem does not exist. As such, it is better for X540 to report
>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>> +	 */
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>> +		netif_carrier_on(netdev);
>> +	} else {
>> +		netif_carrier_on(netdev);
>> +	}
>> +
>> 	ixgbe_check_vf_rate_limit(adapter);
>>
>> 	/* enable transmits */
>> --
>> 1.7.9.5
> NAK
>
> I have already submitted a patch that will address the issue with bonding reporting
> unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps:
> http://patchwork.ozlabs.org/patch/552485/
>
> The bonding driver gets the speed from ethtool and this is where the reporting needs
> to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a
> certain rate and as such will not be able to detect rapid link changes.
Thanks for your reply. The root cause is different from my problem. My 
problem is that
"link up" is prior to "speed and duplex". That is, the physical NIC 
reports "link up" while
the speed is unknown at the same time. We can run "ethtool ethx" to 
confirm it.

Any way, thanks a lot.

Zhu Yanjun
>
> If there is a case where link_speed is unknown when entering this function it's probably
> better to just bail rather than have this hack around the netif_carrier_on() especially
> after the driver already reported link status change. Rapid link changes can occur between
> link partners and not just for X540.
>
> Thanks,
> Emil
>

[V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[zyjzyj2000]

Hi, Jeff

Thanks for your reply.

Changes:
Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready.

Best Regards!
Zhu Yanjun

[Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[zyjzyj2000]

Hi, Jeff

Thanks for your reply.

Changes:
Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready.

Best Regards!
Zhu Yanjun

[PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: yzhu1 <yzhu1@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..2b58a33 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: yzhu1 <yzhu1@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..2b58a33 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

Re: [V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[zhuyj]

[-- Attachment #1.1: Type: text/plain, Size: 5366 bytes --]

On 12/24/2015 11:12 AM, zyjzyj2000@gmail.com wrote:
> Hi, Jeff
>
> Thanks for your reply.
>
> Changes:
> Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready.
>
> 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
>
Hi, Jeff

The following logs are from the customer.  From these logs, there is a 
time span between link_up and link_speed.
This time span sometimes will make bonding in 802.3ad mode not work well.

What I have done in the patch is to prevent this time span in the ixgbe 
driver.

Best Regards!
Zhu Yanjun

Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: no
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: 10000Mb/s
     Duplex: Full
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[zhuyj]

On 12/24/2015 11:12 AM, zyjzyj2000 at gmail.com wrote:
> Hi, Jeff
>
> Thanks for your reply.
>
> Changes:
> Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready.
>
> Best Regards!
> Zhu Yanjun
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Hi, Jeff

The following logs are from the customer.  From these logs, there is a 
time span between link_up and link_speed.
This time span sometimes will make bonding in 802.3ad mode not work well.

What I have done in the patch is to prevent this time span in the ixgbe 
driver.

Best Regards!
Zhu Yanjun

Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: no
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: Unknown!
     Duplex: Unknown! (255)
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
Settings for eth0:
     Supported ports: [ TP ]
     Supported link modes:   100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Supported pause frame use: No
     Supports auto-negotiation: Yes
     Advertised link modes:  100baseT/Full
                             1000baseT/Full
                             10000baseT/Full
     Advertised pause frame use: No
     Advertised auto-negotiation: Yes
     Speed: 10000Mb/s
     Duplex: Full
     Port: Twisted Pair
     PHYAD: 0
     Transceiver: external
     Auto-negotiation: on
     MDI-X: Unknown
     Supports Wake-on: d
     Wake-on: d
     Current message level: 0x00000007 (7)
                    drv probe link
     Link detected: yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151224/ca9d7c56/attachment.html>

RE: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Wednesday, December 23, 2015 6:28 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org]
>On
>>> Behalf Of zyjzyj2000@gmail.com
>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>Mitch
>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>> devel@lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>> reporting "link on" and getting speed and duplex
>>>
>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>
>>> In X540 NIC, there is a time span between reporting "link on" and
>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>> this time span will make it not work well if the time span is big
>>> enough. The big time span will make bonding driver change the state of
>>> the slave device to up while the speed and duplex of the slave device
>>> can not be gotten. Later the bonding driver will not have change to
>>> get the speed and duplex of the slave device. The speed and duplex of
>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>
>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>> not exist. As such, it is necessary for X540 to report"link on" when
>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>
>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index aed8d02..cb9d310 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>> ixgbe_adapter *adapter)
>>> 	       (flow_rx ? "RX" :
>>> 	       (flow_tx ? "TX" : "None"))));
>>>
>>> -	netif_carrier_on(netdev);
>>> +	/*
>>> +	 * In X540 NIC, there is a time span between reporting "link on"
>>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>>> +	 * mode, this time span will make it not work well if the time span
>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>>> +	 * problem does not exist. As such, it is better for X540 to report
>>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>> +	 */
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>> +		netif_carrier_on(netdev);
>>> +	} else {
>>> +		netif_carrier_on(netdev);
>>> +	}
>>> +
>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>
>>> 	/* enable transmits */
>>> --
>>> 1.7.9.5
>> NAK
>>
>> I have already submitted a patch that will address the issue with bonding
>reporting
>> unknown speed (in /proc/bonding/bondX) after the link is established due
>to link flaps:
>> http://patchwork.ozlabs.org/patch/552485/
>>
>> The bonding driver gets the speed from ethtool and this is where the
>reporting needs
>> to be fixed. The issue is that the bonding driver polls for
>netif_carrier_ok() at a
>> certain rate and as such will not be able to detect rapid link changes.
>Thanks for your reply. The root cause is different from my problem. My
>problem is that
>"link up" is prior to "speed and duplex". That is, the physical NIC
>reports "link up" while

The "link up" event is a result of an LSC interrupt, the speed is 
determined as result of that interrupt by checking the LINKS register.
If the LINKS register reports link as unknown then that is the actual state 
of the PHY - meaning the device is re-negotiating the speed for some reason.

>the speed is unknown at the same time. We can run "ethtool ethx" to
>confirm it.

Prior to my patch the ethtool call will read the LINKS register which can show
speed as unknown due to a link flap (for example). You are seeing the momentary 
state of the device.

If you are still seeing the bond reporting "unknown" speed after the patch I pointed
out  please file a bug either through e1000.sf.net or via Intel support and provide
detailed information about the bonding setup, the type of the link partner (switch 
model etc) and full dmesg from the failed scenario along with the output from
/proc/bonding/bond0

Thanks,
Emil

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>Sent: Wednesday, December 23, 2015 6:28 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
>On
>>> Behalf Of zyjzyj2000 at gmail.com
>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>Mitch
>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>> devel at lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>> reporting "link on" and getting speed and duplex
>>>
>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>
>>> In X540 NIC, there is a time span between reporting "link on" and
>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>> this time span will make it not work well if the time span is big
>>> enough. The big time span will make bonding driver change the state of
>>> the slave device to up while the speed and duplex of the slave device
>>> can not be gotten. Later the bonding driver will not have change to
>>> get the speed and duplex of the slave device. The speed and duplex of
>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>
>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>> not exist. As such, it is necessary for X540 to report"link on" when
>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>
>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index aed8d02..cb9d310 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>> ixgbe_adapter *adapter)
>>> 	       (flow_rx ? "RX" :
>>> 	       (flow_tx ? "TX" : "None"))));
>>>
>>> -	netif_carrier_on(netdev);
>>> +	/*
>>> +	 * In X540 NIC, there is a time span between reporting "link on"
>>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>>> +	 * mode, this time span will make it not work well if the time span
>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>>> +	 * problem does not exist. As such, it is better for X540 to report
>>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>> +	 */
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>> +		netif_carrier_on(netdev);
>>> +	} else {
>>> +		netif_carrier_on(netdev);
>>> +	}
>>> +
>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>
>>> 	/* enable transmits */
>>> --
>>> 1.7.9.5
>> NAK
>>
>> I have already submitted a patch that will address the issue with bonding
>reporting
>> unknown speed (in /proc/bonding/bondX) after the link is established due
>to link flaps:
>> http://patchwork.ozlabs.org/patch/552485/
>>
>> The bonding driver gets the speed from ethtool and this is where the
>reporting needs
>> to be fixed. The issue is that the bonding driver polls for
>netif_carrier_ok() at a
>> certain rate and as such will not be able to detect rapid link changes.
>Thanks for your reply. The root cause is different from my problem. My
>problem is that
>"link up" is prior to "speed and duplex". That is, the physical NIC
>reports "link up" while

The "link up" event is a result of an LSC interrupt, the speed is 
determined as result of that interrupt by checking the LINKS register.
If the LINKS register reports link as unknown then that is the actual state 
of the PHY - meaning the device is re-negotiating the speed for some reason.

>the speed is unknown at the same time. We can run "ethtool ethx" to
>confirm it.

Prior to my patch the ethtool call will read the LINKS register which can show
speed as unknown due to a link flap (for example). You are seeing the momentary 
state of the device.

If you are still seeing the bond reporting "unknown" speed after the patch I pointed
out  please file a bug either through e1000.sf.net or via Intel support and provide
detailed information about the bonding setup, the type of the link partner (switch 
model etc) and full dmesg from the failed scenario along with the output from
/proc/bonding/bond0

Thanks,
Emil

RE: [Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>Behalf Of zhuyj
>Sent: Wednesday, December 23, 2015 9:11 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and
>
>On 12/24/2015 11:12 AM, zyjzyj2000@gmail.com <mailto:zyjzyj2000@gmail.com>
>wrote:
>
>
>
>	Hi, Jeff
>
>	Thanks for your reply.
>
>	Changes:
>	Since there is a time span between link_up and link_speed to X540
>NIC, it is not appropriate to continue in the function
>ixgbe_watchdog_link_is_up if only link_up is ready.
>
>	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
><mailto:majordomo@vger.kernel.org>
>	More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>Hi, Jeff
>
>The following logs are from the customer.  From these logs, there is a time
>span between link_up and link_speed.
>This time span sometimes will make bonding in 802.3ad mode not work well.
>
>What I have done in the patch is to prevent this time span in the ixgbe
>driver.
>
>Best Regards!
>Zhu Yanjun
>
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: no
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: 10000Mb/s
>    Duplex: Full
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes

Can you provide the related dmesg output? 
What did the driver report in the "Link Up" messages?

Thanks,
Emil

[Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize reporting "link on" and

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>Behalf Of zhuyj
>Sent: Wednesday, December 23, 2015 9:11 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and
>
>On 12/24/2015 11:12 AM, zyjzyj2000 at gmail.com <mailto:zyjzyj2000@gmail.com>
>wrote:
>
>
>
>	Hi, Jeff
>
>	Thanks for your reply.
>
>	Changes:
>	Since there is a time span between link_up and link_speed to X540
>NIC, it is not appropriate to continue in the function
>ixgbe_watchdog_link_is_up if only link_up is ready.
>
>	Best Regards!
>	Zhu Yanjun
>	--
>	To unsubscribe from this list: send the line "unsubscribe netdev" in
>	the body of a message to majordomo at vger.kernel.org
><mailto:majordomo@vger.kernel.org>
>	More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>Hi, Jeff
>
>The following logs are from the customer.  From these logs, there is a time
>span between link_up and link_speed.
>This time span sometimes will make bonding in 802.3ad mode not work well.
>
>What I have done in the patch is to prevent this time span in the ixgbe
>driver.
>
>Best Regards!
>Zhu Yanjun
>
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: no
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: Unknown!
>    Duplex: Unknown! (255)
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes
>Settings for eth0:
>    Supported ports: [ TP ]
>    Supported link modes:   100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Supported pause frame use: No
>    Supports auto-negotiation: Yes
>    Advertised link modes:  100baseT/Full
>                            1000baseT/Full
>                            10000baseT/Full
>    Advertised pause frame use: No
>    Advertised auto-negotiation: Yes
>    Speed: 10000Mb/s
>    Duplex: Full
>    Port: Twisted Pair
>    PHYAD: 0
>    Transceiver: external
>    Auto-negotiation: on
>    MDI-X: Unknown
>    Supports Wake-on: d
>    Wake-on: d
>    Current message level: 0x00000007 (7)
>                   drv probe link
>    Link detected: yes

Can you provide the related dmesg output? 
What did the driver report in the "Link Up" messages?

Thanks,
Emil

Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zhuyj]

On 12/24/2015 01:58 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>> Sent: Wednesday, December 23, 2015 6:28 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>> reporting "link on" and getting speed and duplex
>>
>> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org]
>> On
>>>> Behalf Of zyjzyj2000@gmail.com
>>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>> Mitch
>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>> devel@lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>>> reporting "link on" and getting speed and duplex
>>>>
>>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>
>>>> In X540 NIC, there is a time span between reporting "link on" and
>>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>>> this time span will make it not work well if the time span is big
>>>> enough. The big time span will make bonding driver change the state of
>>>> the slave device to up while the speed and duplex of the slave device
>>>> can not be gotten. Later the bonding driver will not have change to
>>>> get the speed and duplex of the slave device. The speed and duplex of
>>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>>
>>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>>> not exist. As such, it is necessary for X540 to report"link on" when
>>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>>
>>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index aed8d02..cb9d310 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>>> ixgbe_adapter *adapter)
>>>> 	       (flow_rx ? "RX" :
>>>> 	       (flow_tx ? "TX" : "None"))));
>>>>
>>>> -	netif_carrier_on(netdev);
>>>> +	/*
>>>> +	 * In X540 NIC, there is a time span between reporting "link on"
>>>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>>>> +	 * mode, this time span will make it not work well if the time span
>>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>>>> +	 * problem does not exist. As such, it is better for X540 to report
>>>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>> +	 */
>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>>> +		netif_carrier_on(netdev);
>>>> +	} else {
>>>> +		netif_carrier_on(netdev);
>>>> +	}
>>>> +
>>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>>
>>>> 	/* enable transmits */
>>>> --
>>>> 1.7.9.5
>>> NAK
>>>
>>> I have already submitted a patch that will address the issue with bonding
>> reporting
>>> unknown speed (in /proc/bonding/bondX) after the link is established due
>> to link flaps:
>>> http://patchwork.ozlabs.org/patch/552485/
>>>
>>> The bonding driver gets the speed from ethtool and this is where the
>> reporting needs
>>> to be fixed. The issue is that the bonding driver polls for
>> netif_carrier_ok() at a
>>> certain rate and as such will not be able to detect rapid link changes.
>> Thanks for your reply. The root cause is different from my problem. My
>> problem is that
>> "link up" is prior to "speed and duplex". That is, the physical NIC
>> reports "link up" while
> The "link up" event is a result of an LSC interrupt, the speed is
> determined as result of that interrupt by checking the LINKS register.
Hi,

Sorry. I do not agree with you. Please see the followings for details.

/**
  * ixgbe_watchdog_update_link - update the link status
  * @adapter: pointer to the device adapter structure
  * @link_speed: pointer to a u32 to store the link_speed
  **/
static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter)

 From this function, link_up and link_speed is from watchdo poll.

Thanks for your reply.

Zhu Yanjun
> If the LINKS register reports link as unknown then that is the actual state
> of the PHY - meaning the device is re-negotiating the speed for some reason.
>
>> the speed is unknown at the same time. We can run "ethtool ethx" to
>> confirm it.
> Prior to my patch the ethtool call will read the LINKS register which can show
> speed as unknown due to a link flap (for example). You are seeing the momentary
> state of the device.
>
> If you are still seeing the bond reporting "unknown" speed after the patch I pointed
> out  please file a bug either through e1000.sf.net or via Intel support and provide
> detailed information about the bonding setup, the type of the link partner (switch
> model etc) and full dmesg from the failed scenario along with the output from
> /proc/bonding/bond0
>
> Thanks,
> Emil
>


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[zhuyj]

On 12/24/2015 01:58 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>> Sent: Wednesday, December 23, 2015 6:28 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>> reporting "link on" and getting speed and duplex
>>
>> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
>> On
>>>> Behalf Of zyjzyj2000 at gmail.com
>>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>> Mitch
>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>> devel at lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>>> reporting "link on" and getting speed and duplex
>>>>
>>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>
>>>> In X540 NIC, there is a time span between reporting "link on" and
>>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>>> this time span will make it not work well if the time span is big
>>>> enough. The big time span will make bonding driver change the state of
>>>> the slave device to up while the speed and duplex of the slave device
>>>> can not be gotten. Later the bonding driver will not have change to
>>>> get the speed and duplex of the slave device. The speed and duplex of
>>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>>
>>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>>> not exist. As such, it is necessary for X540 to report"link on" when
>>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>>
>>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index aed8d02..cb9d310 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>>> ixgbe_adapter *adapter)
>>>> 	       (flow_rx ? "RX" :
>>>> 	       (flow_tx ? "TX" : "None"))));
>>>>
>>>> -	netif_carrier_on(netdev);
>>>> +	/*
>>>> +	 * In X540 NIC, there is a time span between reporting "link on"
>>>> +	 * and getting the speed and duplex. To a bonding driver in 802.3ad
>>>> +	 * mode, this time span will make it not work well if the time span
>>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
>>>> +	 * problem does not exist. As such, it is better for X540 to report
>>>> +	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>> +	 */
>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>>> +		netif_carrier_on(netdev);
>>>> +	} else {
>>>> +		netif_carrier_on(netdev);
>>>> +	}
>>>> +
>>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>>
>>>> 	/* enable transmits */
>>>> --
>>>> 1.7.9.5
>>> NAK
>>>
>>> I have already submitted a patch that will address the issue with bonding
>> reporting
>>> unknown speed (in /proc/bonding/bondX) after the link is established due
>> to link flaps:
>>> http://patchwork.ozlabs.org/patch/552485/
>>>
>>> The bonding driver gets the speed from ethtool and this is where the
>> reporting needs
>>> to be fixed. The issue is that the bonding driver polls for
>> netif_carrier_ok() at a
>>> certain rate and as such will not be able to detect rapid link changes.
>> Thanks for your reply. The root cause is different from my problem. My
>> problem is that
>> "link up" is prior to "speed and duplex". That is, the physical NIC
>> reports "link up" while
> The "link up" event is a result of an LSC interrupt, the speed is
> determined as result of that interrupt by checking the LINKS register.
Hi,

Sorry. I do not agree with you. Please see the followings for details.

/**
  * ixgbe_watchdog_update_link - update the link status
  * @adapter: pointer to the device adapter structure
  * @link_speed: pointer to a u32 to store the link_speed
  **/
static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter)

 From this function, link_up and link_speed is from watchdo poll.

Thanks for your reply.

Zhu Yanjun
> If the LINKS register reports link as unknown then that is the actual state
> of the PHY - meaning the device is re-negotiating the speed for some reason.
>
>> the speed is unknown at the same time. We can run "ethtool ethx" to
>> confirm it.
> Prior to my patch the ethtool call will read the LINKS register which can show
> speed as unknown due to a link flap (for example). You are seeing the momentary
> state of the device.
>
> If you are still seeing the bond reporting "unknown" speed after the patch I pointed
> out  please file a bug either through e1000.sf.net or via Intel support and provide
> detailed information about the bonding setup, the type of the link partner (switch
> model etc) and full dmesg from the failed scenario along with the output from
> /proc/bonding/bond0
>
> Thanks,
> Emil
>

RE: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Wednesday, December 23, 2015 10:24 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>On 12/24/2015 01:58 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>> Sent: Wednesday, December 23, 2015 6:28 PM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>> reporting "link on" and getting speed and duplex
>>>
>>> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>bounces@lists.osuosl.org]
>>> On
>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>> Mitch
>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>> devel@lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>>>> reporting "link on" and getting speed and duplex
>>>>>
>>>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>>
>>>>> In X540 NIC, there is a time span between reporting "link on" and
>>>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>>>> this time span will make it not work well if the time span is big
>>>>> enough. The big time span will make bonding driver change the state of
>>>>> the slave device to up while the speed and duplex of the slave device
>>>>> can not be gotten. Later the bonding driver will not have change to
>>>>> get the speed and duplex of the slave device. The speed and duplex of
>>>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>>>
>>>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>>>> not exist. As such, it is necessary for X540 to report"link on" when
>>>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>>>
>>>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>> ---
>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> index aed8d02..cb9d310 100644
>>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>>>> ixgbe_adapter *adapter)
>>>>> 	       (flow_rx ? "RX" :
>>>>> 	       (flow_tx ? "TX" : "None"))));
>>>>>
>>>>> -	netif_carrier_on(netdev);
>>>>> +	/*
>>>>> +	 * In X540 NIC, there is a time span between reporting "link
>on"
>>>>> +	 * and getting the speed and duplex. To a bonding driver in
>802.3ad
>>>>> +	 * mode, this time span will make it not work well if the time
>span
>>>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs,
>this
>>>>> +	 * problem does not exist. As such, it is better for X540 to
>report
>>>>> +	 * "link on" when the link speed is not
>IXGBE_LINK_SPEED_UNKNOWN.
>>>>> +	 */
>>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>>>> +		netif_carrier_on(netdev);
>>>>> +	} else {
>>>>> +		netif_carrier_on(netdev);
>>>>> +	}
>>>>> +
>>>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>>>
>>>>> 	/* enable transmits */
>>>>> --
>>>>> 1.7.9.5
>>>> NAK
>>>>
>>>> I have already submitted a patch that will address the issue with
>bonding
>>> reporting
>>>> unknown speed (in /proc/bonding/bondX) after the link is established
>due
>>> to link flaps:
>>>> http://patchwork.ozlabs.org/patch/552485/
>>>>
>>>> The bonding driver gets the speed from ethtool and this is where the
>>> reporting needs
>>>> to be fixed. The issue is that the bonding driver polls for
>>> netif_carrier_ok() at a
>>>> certain rate and as such will not be able to detect rapid link changes.
>>> Thanks for your reply. The root cause is different from my problem. My
>>> problem is that
>>> "link up" is prior to "speed and duplex". That is, the physical NIC
>>> reports "link up" while
>> The "link up" event is a result of an LSC interrupt, the speed is
>> determined as result of that interrupt by checking the LINKS register.
>Hi,
>
>Sorry. I do not agree with you. Please see the followings for details.
>
>/**
>  * ixgbe_watchdog_update_link - update the link status
>  * @adapter: pointer to the device adapter structure
>  * @link_speed: pointer to a u32 to store the link_speed
>  **/
>static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter)
>
> From this function, link_up and link_speed is from watchdo poll.
>
>Thanks for your reply.

ixgbe_watchdog_update_link() only runs when the IXGBE_FLAG_NEED_LINK_UPDATE is
set which can happen in 2 situations - LSC is received or the interface is 
brought up. At that point the driver will call hw->mac.ops.check_link() which
sets adapter->link_up based on the LINKS register and if we have link_up then
we update the link status:

6669     if (adapter->link_up)
6670         ixgbe_watchdog_link_is_up(adapter);

The thing is if the LINKS.LINK_UP bit was set then at that point we also have a
speed which is contrary to what you are suggesting. If you want to debug your
issue you have to monitor the LINKS register.

That is why I asked for the dmesg output - if your claim is correct then the 
"Link Up" event in dmesg will be reporting "unknown speed" and if not then we 
are not dealing with delayed speed, but a simple link flap. 

Thanks,
Emil

[Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize reporting "link on" and getting speed and duplex

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>Sent: Wednesday, December 23, 2015 10:24 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>reporting "link on" and getting speed and duplex
>
>On 12/24/2015 01:58 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>> Sent: Wednesday, December 23, 2015 6:28 PM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>> reporting "link on" and getting speed and duplex
>>>
>>> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>bounces at lists.osuosl.org]
>>> On
>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>> Sent: Tuesday, December 22, 2015 10:47 PM
>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>> Mitch
>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>> devel at lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize
>>>>> reporting "link on" and getting speed and duplex
>>>>>
>>>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>>
>>>>> In X540 NIC, there is a time span between reporting "link on" and
>>>>> getting the speed and duplex. To a bonding driver in 802.3ad mode,
>>>>> this time span will make it not work well if the time span is big
>>>>> enough. The big time span will make bonding driver change the state of
>>>>> the slave device to up while the speed and duplex of the slave device
>>>>> can not be gotten. Later the bonding driver will not have change to
>>>>> get the speed and duplex of the slave device. The speed and duplex of
>>>>> the slave device are important to a bonding driver in 802.3ad mode.
>>>>>
>>>>> To 82599_SFP NIC and other kinds of NICs, this problem does
>>>>> not exist. As such, it is necessary for X540 to report"link on" when
>>>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
>>>>>
>>>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>>> ---
>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 +++++++++++++++-
>>>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> index aed8d02..cb9d310 100644
>>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct
>>>>> ixgbe_adapter *adapter)
>>>>> 	       (flow_rx ? "RX" :
>>>>> 	       (flow_tx ? "TX" : "None"))));
>>>>>
>>>>> -	netif_carrier_on(netdev);
>>>>> +	/*
>>>>> +	 * In X540 NIC, there is a time span between reporting "link
>on"
>>>>> +	 * and getting the speed and duplex. To a bonding driver in
>802.3ad
>>>>> +	 * mode, this time span will make it not work well if the time
>span
>>>>> +	 * is big enough. To 82599_SFP NIC and other kinds of NICs,
>this
>>>>> +	 * problem does not exist. As such, it is better for X540 to
>report
>>>>> +	 * "link on" when the link speed is not
>IXGBE_LINK_SPEED_UNKNOWN.
>>>>> +	 */
>>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>>> +	    (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) {
>>>>> +		netif_carrier_on(netdev);
>>>>> +	} else {
>>>>> +		netif_carrier_on(netdev);
>>>>> +	}
>>>>> +
>>>>> 	ixgbe_check_vf_rate_limit(adapter);
>>>>>
>>>>> 	/* enable transmits */
>>>>> --
>>>>> 1.7.9.5
>>>> NAK
>>>>
>>>> I have already submitted a patch that will address the issue with
>bonding
>>> reporting
>>>> unknown speed (in /proc/bonding/bondX) after the link is established
>due
>>> to link flaps:
>>>> http://patchwork.ozlabs.org/patch/552485/
>>>>
>>>> The bonding driver gets the speed from ethtool and this is where the
>>> reporting needs
>>>> to be fixed. The issue is that the bonding driver polls for
>>> netif_carrier_ok() at a
>>>> certain rate and as such will not be able to detect rapid link changes.
>>> Thanks for your reply. The root cause is different from my problem. My
>>> problem is that
>>> "link up" is prior to "speed and duplex". That is, the physical NIC
>>> reports "link up" while
>> The "link up" event is a result of an LSC interrupt, the speed is
>> determined as result of that interrupt by checking the LINKS register.
>Hi,
>
>Sorry. I do not agree with you. Please see the followings for details.
>
>/**
>  * ixgbe_watchdog_update_link - update the link status
>  * @adapter: pointer to the device adapter structure
>  * @link_speed: pointer to a u32 to store the link_speed
>  **/
>static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter)
>
> From this function, link_up and link_speed is from watchdo poll.
>
>Thanks for your reply.

ixgbe_watchdog_update_link() only runs when the IXGBE_FLAG_NEED_LINK_UPDATE is
set which can happen in 2 situations - LSC is received or the interface is 
brought up. At that point the driver will call hw->mac.ops.check_link() which
sets adapter->link_up based on the LINKS register and if we have link_up then
we update the link status:

6669     if (adapter->link_up)
6670         ixgbe_watchdog_link_is_up(adapter);

The thing is if the LINKS.LINK_UP bit was set then at that point we also have a
speed which is contrary to what you are suggesting. If you want to debug your
issue you have to monitor the LINKS register.

That is why I asked for the dmesg output - if your claim is correct then the 
"Link Up" event in dmesg will be reporting "unknown speed" and if not then we 
are not dealing with delayed speed, but a simple link flap. 

Thanks,
Emil

[PATCH V3] ixgbe: force to synchronize link_up and speed as a slave

[zyjzyj2000]

Hi, all

During the discussion with Emil, I think the time span between link_up 
and link_speed is not important when the X540 NIC acts as an independent 
interface. As such, a new patch is made to restrict synchronization 
between link_up and link_speed.

Any reply is appreciated.

Zhu Yanjun

[Intel-wired-lan] [PATCH V3] ixgbe: force to synchronize link_up and speed as a slave

[zyjzyj2000]

Hi, all

During the discussion with Emil, I think the time span between link_up 
and link_speed is not important when the X540 NIC acts as an independent 
interface. As such, a new patch is made to restrict synchronization 
between link_up and link_speed.

Any reply is appreciated.

Zhu Yanjun

[PATCH 1/2] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: yzhu1 <yzhu1@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..ace21b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 1/2] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: yzhu1 <yzhu1@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..ace21b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

[PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

When the X540 NIC acts as a slave of some virtual NICs, it is very
important to synchronize link_up and link_speed, such as a bonding
driver in 802.3ad mode. When X540 NIC acts as an independent interface,
it is not necessary to synchronize link_up and link_speed. That is,
the time span between link_up and link_speed is acceptable.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ace21b9..1bb6056 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * time. To X540 NIC, there is a time span between link_up and
 	 * link_speed. As such, only continue if link_up and link_speed are
 	 * ready to X540 NIC.
+	 * The time span between link_up and link_speed is very important
+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
+	 * independent interface, it is not necessary to synchronize link_up
+	 * and link_speed.
+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
 	 */
-	if (hw->mac.type == ixgbe_mac_X540)
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (netdev->flags & IFF_SLAVE))
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

When the X540 NIC acts as a slave of some virtual NICs, it is very
important to synchronize link_up and link_speed, such as a bonding
driver in 802.3ad mode. When X540 NIC acts as an independent interface,
it is not necessary to synchronize link_up and link_speed. That is,
the time span between link_up and link_speed is acceptable.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ace21b9..1bb6056 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * time. To X540 NIC, there is a time span between link_up and
 	 * link_speed. As such, only continue if link_up and link_speed are
 	 * ready to X540 NIC.
+	 * The time span between link_up and link_speed is very important
+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
+	 * independent interface, it is not necessary to synchronize link_up
+	 * and link_speed.
+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
 	 */
-	if (hw->mac.type == ixgbe_mac_X540)
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (netdev->flags & IFF_SLAVE))
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>Behalf Of zyjzyj2000@gmail.com
>Sent: Monday, December 28, 2015 6:32 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of
>link_up and speed
>
>From: Zhu Yanjun <yanjun.zhu@windriver.com>
>
>When the X540 NIC acts as a slave of some virtual NICs, it is very
>important to synchronize link_up and link_speed, such as a bonding
>driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>it is not necessary to synchronize link_up and link_speed. That is,
>the time span between link_up and link_speed is acceptable.

What exactly do you mean by "time span between link_up and link_speed"?
Where is it you think the de-synchronization occurs?

>Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>index ace21b9..1bb6056 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>ixgbe_adapter *adapter)
> 	 * time. To X540 NIC, there is a time span between link_up and
> 	 * link_speed. As such, only continue if link_up and link_speed are
> 	 * ready to X540 NIC.
>+	 * The time span between link_up and link_speed is very important
>+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>+	 * independent interface, it is not necessary to synchronize link_up
>+	 * and link_speed.
>+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>UNKNOWN)

This is a patch on top of your previous patch which I don't think was applied, 
so this is not going to apply cleanly.

> 	 */
>-	if (hw->mac.type == ixgbe_mac_X540)
>+	if ((hw->mac.type == ixgbe_mac_X540) &&
>+	    (netdev->flags & IFF_SLAVE))
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
assume that you also have a dmesg that shows:
"NIC Link is Up unknown speed"

by the interface you use in the bond?

Thanks,
Emil


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>Behalf Of zyjzyj2000 at gmail.com
>Sent: Monday, December 28, 2015 6:32 PM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of
>link_up and speed
>
>From: Zhu Yanjun <yanjun.zhu@windriver.com>
>
>When the X540 NIC acts as a slave of some virtual NICs, it is very
>important to synchronize link_up and link_speed, such as a bonding
>driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>it is not necessary to synchronize link_up and link_speed. That is,
>the time span between link_up and link_speed is acceptable.

What exactly do you mean by "time span between link_up and link_speed"?
Where is it you think the de-synchronization occurs?

>Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>index ace21b9..1bb6056 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>ixgbe_adapter *adapter)
> 	 * time. To X540 NIC, there is a time span between link_up and
> 	 * link_speed. As such, only continue if link_up and link_speed are
> 	 * ready to X540 NIC.
>+	 * The time span between link_up and link_speed is very important
>+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>+	 * independent interface, it is not necessary to synchronize link_up
>+	 * and link_speed.
>+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>UNKNOWN)

This is a patch on top of your previous patch which I don't think was applied, 
so this is not going to apply cleanly.

> 	 */
>-	if (hw->mac.type == ixgbe_mac_X540)
>+	if ((hw->mac.type == ixgbe_mac_X540) &&
>+	    (netdev->flags & IFF_SLAVE))
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
assume that you also have a dmesg that shows:
"NIC Link is Up unknown speed"

by the interface you use in the bond?

Thanks,
Emil

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Rustad, Mark D]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

Emil S <emil.s.tantilov@intel.com> wrote:

>>  */
>> -	if (hw->mac.type == ixgbe_mac_X540)
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (netdev->flags & IFF_SLAVE))
>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>> 			return;
> 
> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
> assume that you also have a dmesg that shows:
> "NIC Link is Up unknown speed"
> 
> by the interface you use in the bond?

It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540.

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Rustad, Mark D]

Emil S <emil.s.tantilov@intel.com> wrote:

>>  */
>> -	if (hw->mac.type == ixgbe_mac_X540)
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (netdev->flags & IFF_SLAVE))
>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>> 			return;
> 
> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
> assume that you also have a dmesg that shows:
> "NIC Link is Up unknown speed"
> 
> by the interface you use in the bond?

It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540.

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151229/775e8321/attachment-0001.asc>

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>> Behalf Of zyjzyj2000@gmail.com
>> Sent: Monday, December 28, 2015 6:32 PM
>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>> devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of
>> link_up and speed
>>
>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>
>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>> important to synchronize link_up and link_speed, such as a bonding
>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>> it is not necessary to synchronize link_up and link_speed. That is,
>> the time span between link_up and link_speed is acceptable.
> What exactly do you mean by "time span between link_up and link_speed"?

In the previous mail, I show you some ethtool logs. In these logs, there 
is some
time with NIC up while speed is unknown. I think this "some time" is 
time span between
link_up and link_speed. Please see the previous mail for details.

> Where is it you think the de-synchronization occurs?

When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in 
netdevice struct.
Before we enter this function, we check IFF_SLAVE flag. If this flag is 
set, we continue to check
link_speed. If not, this function is executed whether this link_speed is 
unknown or not.

>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index ace21b9..1bb6056 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>> ixgbe_adapter *adapter)
>> 	 * time. To X540 NIC, there is a time span between link_up and
>> 	 * link_speed. As such, only continue if link_up and link_speed are
>> 	 * ready to X540 NIC.
>> +	 * The time span between link_up and link_speed is very important
>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>> +	 * independent interface, it is not necessary to synchronize link_up
>> +	 * and link_speed.
>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>> UNKNOWN)
> This is a patch on top of your previous patch which I don't think was applied,
> so this is not going to apply cleanly.
>
>> 	 */
>> -	if (hw->mac.type == ixgbe_mac_X540)
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (netdev->flags & IFF_SLAVE))
>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>> 			return;
> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
> assume that you also have a dmesg that shows:
> "NIC Link is Up unknown speed"
>
> by the interface you use in the bond?
Sure. There is a dmesg log from the customer.
"
...
2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link 
status definitely up for interface eth0, 0 Mbps full duplex.
...
"

Thanks a lot.
Zhu Yanjun
>
> Thanks,
> Emil
>


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>> Behalf Of zyjzyj2000 at gmail.com
>> Sent: Monday, December 28, 2015 6:32 PM
>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch
>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>> devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of
>> link_up and speed
>>
>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>
>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>> important to synchronize link_up and link_speed, such as a bonding
>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>> it is not necessary to synchronize link_up and link_speed. That is,
>> the time span between link_up and link_speed is acceptable.
> What exactly do you mean by "time span between link_up and link_speed"?

In the previous mail, I show you some ethtool logs. In these logs, there 
is some
time with NIC up while speed is unknown. I think this "some time" is 
time span between
link_up and link_speed. Please see the previous mail for details.

> Where is it you think the de-synchronization occurs?

When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in 
netdevice struct.
Before we enter this function, we check IFF_SLAVE flag. If this flag is 
set, we continue to check
link_speed. If not, this function is executed whether this link_speed is 
unknown or not.

>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index ace21b9..1bb6056 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>> ixgbe_adapter *adapter)
>> 	 * time. To X540 NIC, there is a time span between link_up and
>> 	 * link_speed. As such, only continue if link_up and link_speed are
>> 	 * ready to X540 NIC.
>> +	 * The time span between link_up and link_speed is very important
>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>> +	 * independent interface, it is not necessary to synchronize link_up
>> +	 * and link_speed.
>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>> UNKNOWN)
> This is a patch on top of your previous patch which I don't think was applied,
> so this is not going to apply cleanly.
>
>> 	 */
>> -	if (hw->mac.type == ixgbe_mac_X540)
>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>> +	    (netdev->flags & IFF_SLAVE))
>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>> 			return;
> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
> assume that you also have a dmesg that shows:
> "NIC Link is Up unknown speed"
>
> by the interface you use in the bond?
Sure. There is a dmesg log from the customer.
"
...
2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link 
status definitely up for interface eth0, 0 Mbps full duplex.
...
"

Thanks a lot.
Zhu Yanjun
>
> Thanks,
> Emil
>

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 03:17 AM, Rustad, Mark D wrote:
> Emil S <emil.s.tantilov@intel.com> wrote:
>
>>>   */
>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (netdev->flags & IFF_SLAVE))
>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>> 			return;
>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
>> assume that you also have a dmesg that shows:
>> "NIC Link is Up unknown speed"
>>
>> by the interface you use in the bond?
> It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540.
>
> --
> Mark Rustad, Networking Division, Intel Corporation
Hi, Mark

Thanks for your suggestions.
The following is the feedback from the customer.
"
...
We observing this issue on x540T interfaces (8086:1528), but not on 
82599_SFP (8086:10FB).
...
"
To narrow this problem, I restrict mac.type to ixgbe_mac_X540. I agree 
with you. Maybe this problem is
related with the copper phy. But I only have X540 NIC to test. So it is 
difficult for me to confirm whether this
problem occurs on other mac type or not, such as X550.

I will consider your suggestions in the latest patch.

Thanks again.

Zhu Yanjun

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 03:17 AM, Rustad, Mark D wrote:
> Emil S <emil.s.tantilov@intel.com> wrote:
>
>>>   */
>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (netdev->flags & IFF_SLAVE))
>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>> 			return;
>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would
>> assume that you also have a dmesg that shows:
>> "NIC Link is Up unknown speed"
>>
>> by the interface you use in the bond?
> It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540.
>
> --
> Mark Rustad, Networking Division, Intel Corporation
Hi, Mark

Thanks for your suggestions.
The following is the feedback from the customer.
"
...
We observing this issue on x540T interfaces (8086:1528), but not on 
82599_SFP (8086:10FB).
...
"
To narrow this problem, I restrict mac.type to ixgbe_mac_X540. I agree 
with you. Maybe this problem is
related with the copper phy. But I only have X540 NIC to test. So it is 
difficult for me to confirm whether this
problem occurs on other mac type or not, such as X550.

I will consider your suggestions in the latest patch.

Thanks again.

Zhu Yanjun

RE: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Tuesday, December 29, 2015 6:49 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org]
>On
>>> Behalf Of zyjzyj2000@gmail.com
>>> Sent: Monday, December 28, 2015 6:32 PM
>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>Mitch
>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>> devel@lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of
>>> link_up and speed
>>>
>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>
>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>> important to synchronize link_up and link_speed, such as a bonding
>>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>>> it is not necessary to synchronize link_up and link_speed. That is,
>>> the time span between link_up and link_speed is acceptable.
>> What exactly do you mean by "time span between link_up and link_speed"?
>
>In the previous mail, I show you some ethtool logs. In these logs, there
>is some
>time with NIC up while speed is unknown. I think this "some time" is
>time span between
>link_up and link_speed. Please see the previous mail for details.

Was this when reporting the link state from check_link() (reading the LINKS
register) or reporting the adapter->link_speed?

>> Where is it you think the de-synchronization occurs?
>
>When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>netdevice struct.
>Before we enter this function, we check IFF_SLAVE flag. If this flag is
>set, we continue to check
>link_speed. If not, this function is executed whether this link_speed is
>unknown or not.

I can already see this in your patch. I was asking about the reason why your 
change is needed.

>>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index ace21b9..1bb6056 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>>> ixgbe_adapter *adapter)
>>> 	 * time. To X540 NIC, there is a time span between link_up and
>>> 	 * link_speed. As such, only continue if link_up and link_speed are
>>> 	 * ready to X540 NIC.
>>> +	 * The time span between link_up and link_speed is very important
>>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>>> +	 * independent interface, it is not necessary to synchronize link_up
>>> +	 * and link_speed.
>>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>>> UNKNOWN)
>> This is a patch on top of your previous patch which I don't think was
>applied,
>> so this is not going to apply cleanly.
>>
>>> 	 */
>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (netdev->flags & IFF_SLAVE))
>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>> 			return;
>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then
>I would
>> assume that you also have a dmesg that shows:
>> "NIC Link is Up unknown speed"
>>
>> by the interface you use in the bond?
>Sure. There is a dmesg log from the customer.
>"
>...
>2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link
>status definitely up for interface eth0, 0 Mbps full duplex.

This message is from the bonding driver not from ixgbe.

In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up()
if that condition was true then you should also see "unknown link" being reported by ixgbe.

Thanks,
Emil

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>Sent: Tuesday, December 29, 2015 6:49 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
>On
>>> Behalf Of zyjzyj2000 at gmail.com
>>> Sent: Monday, December 28, 2015 6:32 PM
>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>Mitch
>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>> devel at lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of
>>> link_up and speed
>>>
>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>
>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>> important to synchronize link_up and link_speed, such as a bonding
>>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>>> it is not necessary to synchronize link_up and link_speed. That is,
>>> the time span between link_up and link_speed is acceptable.
>> What exactly do you mean by "time span between link_up and link_speed"?
>
>In the previous mail, I show you some ethtool logs. In these logs, there
>is some
>time with NIC up while speed is unknown. I think this "some time" is
>time span between
>link_up and link_speed. Please see the previous mail for details.

Was this when reporting the link state from check_link() (reading the LINKS
register) or reporting the adapter->link_speed?

>> Where is it you think the de-synchronization occurs?
>
>When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>netdevice struct.
>Before we enter this function, we check IFF_SLAVE flag. If this flag is
>set, we continue to check
>link_speed. If not, this function is executed whether this link_speed is
>unknown or not.

I can already see this in your patch. I was asking about the reason why your 
change is needed.

>>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index ace21b9..1bb6056 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>>> ixgbe_adapter *adapter)
>>> 	 * time. To X540 NIC, there is a time span between link_up and
>>> 	 * link_speed. As such, only continue if link_up and link_speed are
>>> 	 * ready to X540 NIC.
>>> +	 * The time span between link_up and link_speed is very important
>>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>>> +	 * independent interface, it is not necessary to synchronize link_up
>>> +	 * and link_speed.
>>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>>> UNKNOWN)
>> This is a patch on top of your previous patch which I don't think was
>applied,
>> so this is not going to apply cleanly.
>>
>>> 	 */
>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>> +	    (netdev->flags & IFF_SLAVE))
>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>> 			return;
>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then
>I would
>> assume that you also have a dmesg that shows:
>> "NIC Link is Up unknown speed"
>>
>> by the interface you use in the bond?
>Sure. There is a dmesg log from the customer.
>"
>...
>2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link
>status definitely up for interface eth0, 0 Mbps full duplex.

This message is from the bonding driver not from ixgbe.

In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up()
if that condition was true then you should also see "unknown link" being reported by ixgbe.

Thanks,
Emil

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>> Sent: Tuesday, December 29, 2015 6:49 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org]
>> On
>>>> Behalf Of zyjzyj2000@gmail.com
>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>> Mitch
>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>> devel@lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of
>>>> link_up and speed
>>>>
>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>
>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>> important to synchronize link_up and link_speed, such as a bonding
>>>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>> the time span between link_up and link_speed is acceptable.
>>> What exactly do you mean by "time span between link_up and link_speed"?
>> In the previous mail, I show you some ethtool logs. In these logs, there
>> is some
>> time with NIC up while speed is unknown. I think this "some time" is
>> time span between
>> link_up and link_speed. Please see the previous mail for details.
> Was this when reporting the link state from check_link() (reading the LINKS
> register) or reporting the adapter->link_speed?
>
>>> Where is it you think the de-synchronization occurs?
>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>> netdevice struct.
>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>> set, we continue to check
>> link_speed. If not, this function is executed whether this link_speed is
>> unknown or not.
> I can already see this in your patch. I was asking about the reason why your
> change is needed.

an extreme example, let us assume this scenario:

An ixgbe NIC directly connects to another NIC (let us call it NIC-a). 
And auto-negotiate is off while no static speed is set in the 2 NICs. 
These 2 NICs acts as 2 independent interfaces. As such, at this time, 
there is no speed in the both 2 NICs. That is, link_speed is unknown.

When the user run "ifconfig or ethtool", NIC-a will show "Link detected: 
yes" while ixgbe NIC will show "Link detected: no" if the flag IFF_SLAVE 
is not set.

NIC-a stands for most NIC, such as e1000, e1000e and so on.

Best Regards!
Zhu Yanjun
>
>>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index ace21b9..1bb6056 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>>>> ixgbe_adapter *adapter)
>>>> 	 * time. To X540 NIC, there is a time span between link_up and
>>>> 	 * link_speed. As such, only continue if link_up and link_speed are
>>>> 	 * ready to X540 NIC.
>>>> +	 * The time span between link_up and link_speed is very important
>>>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>>>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>>>> +	 * independent interface, it is not necessary to synchronize link_up
>>>> +	 * and link_speed.
>>>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>>>> UNKNOWN)
>>> This is a patch on top of your previous patch which I don't think was
>> applied,
>>> so this is not going to apply cleanly.
>>>
>>>> 	 */
>>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>> +	    (netdev->flags & IFF_SLAVE))
>>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>>> 			return;
>>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then
>> I would
>>> assume that you also have a dmesg that shows:
>>> "NIC Link is Up unknown speed"
>>>
>>> by the interface you use in the bond?
>> Sure. There is a dmesg log from the customer.
>> "
>> ...
>> 2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link
>> status definitely up for interface eth0, 0 Mbps full duplex.
> This message is from the bonding driver not from ixgbe.
>
> In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up()
> if that condition was true then you should also see "unknown link" being reported by ixgbe.
>
> Thanks,
> Emil
>


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>> Sent: Tuesday, December 29, 2015 6:49 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
>> On
>>>> Behalf Of zyjzyj2000 at gmail.com
>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>> Mitch
>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>> devel at lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of
>>>> link_up and speed
>>>>
>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>
>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>> important to synchronize link_up and link_speed, such as a bonding
>>>> driver in 802.3ad mode. When X540 NIC acts as an independent interface,
>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>> the time span between link_up and link_speed is acceptable.
>>> What exactly do you mean by "time span between link_up and link_speed"?
>> In the previous mail, I show you some ethtool logs. In these logs, there
>> is some
>> time with NIC up while speed is unknown. I think this "some time" is
>> time span between
>> link_up and link_speed. Please see the previous mail for details.
> Was this when reporting the link state from check_link() (reading the LINKS
> register) or reporting the adapter->link_speed?
>
>>> Where is it you think the de-synchronization occurs?
>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>> netdevice struct.
>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>> set, we continue to check
>> link_speed. If not, this function is executed whether this link_speed is
>> unknown or not.
> I can already see this in your patch. I was asking about the reason why your
> change is needed.

an extreme example, let us assume this scenario:

An ixgbe NIC directly connects to another NIC (let us call it NIC-a). 
And auto-negotiate is off while no static speed is set in the 2 NICs. 
These 2 NICs acts as 2 independent interfaces. As such, at this time, 
there is no speed in the both 2 NICs. That is, link_speed is unknown.

When the user run "ifconfig or ethtool", NIC-a will show "Link detected: 
yes" while ixgbe NIC will show "Link detected: no" if the flag IFF_SLAVE 
is not set.

NIC-a stands for most NIC, such as e1000, e1000e and so on.

Best Regards!
Zhu Yanjun
>
>>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++++++-
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index ace21b9..1bb6056 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct
>>>> ixgbe_adapter *adapter)
>>>> 	 * time. To X540 NIC, there is a time span between link_up and
>>>> 	 * link_speed. As such, only continue if link_up and link_speed are
>>>> 	 * ready to X540 NIC.
>>>> +	 * The time span between link_up and link_speed is very important
>>>> +	 * when the X540 NIC acts as a slave in some virtual NICs, such as
>>>> +	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
>>>> +	 * independent interface, it is not necessary to synchronize link_up
>>>> +	 * and link_speed.
>>>> +	 * In the end, not continue if (X540 NIC && SLAVE && link_speed
>>>> UNKNOWN)
>>> This is a patch on top of your previous patch which I don't think was
>> applied,
>>> so this is not going to apply cleanly.
>>>
>>>> 	 */
>>>> -	if (hw->mac.type == ixgbe_mac_X540)
>>>> +	if ((hw->mac.type == ixgbe_mac_X540) &&
>>>> +	    (netdev->flags & IFF_SLAVE))
>>>> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
>>>> 			return;
>>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then
>> I would
>>> assume that you also have a dmesg that shows:
>>> "NIC Link is Up unknown speed"
>>>
>>> by the interface you use in the bond?
>> Sure. There is a dmesg log from the customer.
>> "
>> ...
>> 2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link
>> status definitely up for interface eth0, 0 Mbps full duplex.
> This message is from the bonding driver not from ixgbe.
>
> In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up()
> if that condition was true then you should also see "unknown link" being reported by ixgbe.
>
> Thanks,
> Emil
>

[PATCH V4] ixgbe: synchronize the link_speed and link_up of a slave interface

[zyjzyj2000]

Hi, all

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.

Best Regards!
Zhu Yanjun

[Intel-wired-lan] [PATCH V4] ixgbe: synchronize the link_speed and link_up of a slave interface

[zyjzyj2000]

Hi, all

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.

Best Regards!
Zhu Yanjun

[PATCH 1/3] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..ace21b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 1/3] ixgbe: force to synchronize reporting "link on" and getting speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

In X540 NIC, there is a time span between reporting "link on" and
getting the speed and duplex. To a bonding driver in 802.3ad mode,
this time span will make it not work well if the time span is big
enough. The big time span will make bonding driver change the state of
the slave device to up while the speed and duplex of the slave device
can not be gotten. Later the bonding driver will not have change to
get the speed and duplex of the slave device. The speed and duplex of
the slave device are important to a bonding driver in 802.3ad mode.

To 82599_SFP NIC and other kinds of NICs, this problem does
not exist. As such, it is necessary for X540 to report"link on" when
the link speed is not IXGBE_LINK_SPEED_UNKNOWN.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aed8d02..ace21b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6426,6 +6426,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	if (netif_carrier_ok(netdev))
 		return;
 
+	/* In X540 NIC, there is a time span between reporting "link on"
+	 * and getting the speed and duplex. To a bonding driver in 802.3ad
+	 * mode, this time span will make it not work well if the time span
+	 * is big enough. To 82599_SFP NIC and other kinds of NICs, this
+	 * problem does not exist. As such, it is better for X540 to report
+	 * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN.
+	 * To other NICs, the link_up and link_speed are gotten at the same
+	 * time. To X540 NIC, there is a time span between link_up and
+	 * link_speed. As such, only continue if link_up and link_speed are
+	 * ready to X540 NIC.
+	 */
+	if (hw->mac.type == ixgbe_mac_X540)
+		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
+			return;
+
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
 
 	switch (hw->mac.type) {
-- 
1.7.9.5

[PATCH 2/3] ixgbe: restrict synchronization of link_up and speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

When the X540 NIC acts as a slave of some virtual NICs, it is very
important to synchronize link_up and link_speed, such as a bonding
driver in 802.3ad mode. When X540 NIC acts as an independent interface,
it is not necessary to synchronize link_up and link_speed. That is,
the time span between link_up and link_speed is acceptable.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ace21b9..1bb6056 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * time. To X540 NIC, there is a time span between link_up and
 	 * link_speed. As such, only continue if link_up and link_speed are
 	 * ready to X540 NIC.
+	 * The time span between link_up and link_speed is very important
+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
+	 * independent interface, it is not necessary to synchronize link_up
+	 * and link_speed.
+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
 	 */
-	if (hw->mac.type == ixgbe_mac_X540)
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (netdev->flags & IFF_SLAVE))
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 2/3] ixgbe: restrict synchronization of link_up and speed

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

When the X540 NIC acts as a slave of some virtual NICs, it is very
important to synchronize link_up and link_speed, such as a bonding
driver in 802.3ad mode. When X540 NIC acts as an independent interface,
it is not necessary to synchronize link_up and link_speed. That is,
the time span between link_up and link_speed is acceptable.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ace21b9..1bb6056 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * time. To X540 NIC, there is a time span between link_up and
 	 * link_speed. As such, only continue if link_up and link_speed are
 	 * ready to X540 NIC.
+	 * The time span between link_up and link_speed is very important
+	 * when the X540 NIC acts as a slave in some virtual NICs, such as
+	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
+	 * independent interface, it is not necessary to synchronize link_up
+	 * and link_speed.
+	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
 	 */
-	if (hw->mac.type == ixgbe_mac_X540)
+	if ((hw->mac.type == ixgbe_mac_X540) &&
+	    (netdev->flags & IFF_SLAVE))
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

[PATCH 3/3] ixgbe: synchronize the link_speed and link_up of a slave interface

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

According to the suggestion from Rustad, Mark D, this behavior perhaps
is more related to the copper phy. But to make fiber phy more robust,
to all the interfaces as a slave interface, the link_speed and link_up
is synchronized.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1bb6056..ce47639 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
 	 * independent interface, it is not necessary to synchronize link_up
 	 * and link_speed.
-	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
+	 * According to the suggestion from Rustad, Mark D, this behavior
+	 * perhaps is related to the copper phy. To make fiber phy more robust,
+	 * To all the interfaces as a slave, the link_speed is checked.
+	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)
 	 */
-	if ((hw->mac.type == ixgbe_mac_X540) &&
-	    (netdev->flags & IFF_SLAVE))
+	if (netdev->flags & IFF_SLAVE)
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

[Intel-wired-lan] [PATCH 3/3] ixgbe: synchronize the link_speed and link_up of a slave interface

[zyjzyj2000]

From: Zhu Yanjun <yanjun.zhu@windriver.com>

According to the suggestion from Rustad, Mark D, this behavior perhaps
is more related to the copper phy. But to make fiber phy more robust,
to all the interfaces as a slave interface, the link_speed and link_up
is synchronized.

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

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1bb6056..ce47639 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
 	 * independent interface, it is not necessary to synchronize link_up
 	 * and link_speed.
-	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
+	 * According to the suggestion from Rustad, Mark D, this behavior
+	 * perhaps is related to the copper phy. To make fiber phy more robust,
+	 * To all the interfaces as a slave, the link_speed is checked.
+	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)
 	 */
-	if ((hw->mac.type == ixgbe_mac_X540) &&
-	    (netdev->flags & IFF_SLAVE))
+	if (netdev->flags & IFF_SLAVE)
 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
 			return;
 
-- 
1.7.9.5

RE: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Wednesday, December 30, 2015 12:20 AM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>> of link_up and speed
>>>
>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>bounces@lists.osuosl.org]
>>> On
>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>> Mitch
>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>> devel@lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>>> of
>>>>> link_up and speed
>>>>>
>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>
>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>interface,
>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>> the time span between link_up and link_speed is acceptable.
>>>> What exactly do you mean by "time span between link_up and link_speed"?
>>> In the previous mail, I show you some ethtool logs. In these logs, there
>>> is some
>>> time with NIC up while speed is unknown. I think this "some time" is
>>> time span between
>>> link_up and link_speed. Please see the previous mail for details.
>> Was this when reporting the link state from check_link() (reading the
>LINKS
>> register) or reporting the adapter->link_speed?
>>
>>>> Where is it you think the de-synchronization occurs?
>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>> netdevice struct.
>>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>>> set, we continue to check
>>> link_speed. If not, this function is executed whether this link_speed is
>>> unknown or not.
>> I can already see this in your patch. I was asking about the reason why
>>your change is needed.
>
>an extreme example, let us assume this scenario:

Is this the scenario you are trying to fix?

>An ixgbe NIC directly connects to another NIC (let us call it NIC-a).
>And auto-negotiate is off while no static speed is set in the 2 NICs.

The ixgbe driver does not support disabling auto-negotiation directly.
The only time this is true is when the advertised speed is restricted,
so the above scenario is not possible (you either have autoneg or 
advertised speed set) with the current driver.

Is this example in theory or do you have your interface configured this
way somehow? 

Thanks,
Emil

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>Sent: Wednesday, December 30, 2015 12:20 AM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>> of link_up and speed
>>>
>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>bounces at lists.osuosl.org]
>>> On
>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>> Mitch
>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>> devel at lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>>> of
>>>>> link_up and speed
>>>>>
>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>
>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>interface,
>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>> the time span between link_up and link_speed is acceptable.
>>>> What exactly do you mean by "time span between link_up and link_speed"?
>>> In the previous mail, I show you some ethtool logs. In these logs, there
>>> is some
>>> time with NIC up while speed is unknown. I think this "some time" is
>>> time span between
>>> link_up and link_speed. Please see the previous mail for details.
>> Was this when reporting the link state from check_link() (reading the
>LINKS
>> register) or reporting the adapter->link_speed?
>>
>>>> Where is it you think the de-synchronization occurs?
>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>> netdevice struct.
>>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>>> set, we continue to check
>>> link_speed. If not, this function is executed whether this link_speed is
>>> unknown or not.
>> I can already see this in your patch. I was asking about the reason why
>>your change is needed.
>
>an extreme example, let us assume this scenario:

Is this the scenario you are trying to fix?

>An ixgbe NIC directly connects to another NIC (let us call it NIC-a).
>And auto-negotiate is off while no static speed is set in the 2 NICs.

The ixgbe driver does not support disabling auto-negotiation directly.
The only time this is true is when the advertised speed is restricted,
so the above scenario is not possible (you either have autoneg or 
advertised speed set) with the current driver.

Is this example in theory or do you have your interface configured this
way somehow? 

Thanks,
Emil

Re: [PATCH 3/3] ixgbe: synchronize the link_speed and link_up of a slave interface

[Rustad, Mark D]

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

zyjzyj2000@gmail.com wrote:

> From: Zhu Yanjun <yanjun.zhu@windriver.com>
> 
> According to the suggestion from Rustad, Mark D, this behavior perhaps
> is more related to the copper phy. But to make fiber phy more robust,
> to all the interfaces as a slave interface, the link_speed and link_up
> is synchronized.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1bb6056..ce47639 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
> 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
> 	 * independent interface, it is not necessary to synchronize link_up
> 	 * and link_speed.
> -	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
> +	 * According to the suggestion from Rustad, Mark D, this behavior
> +	 * perhaps is related to the copper phy. To make fiber phy more robust,
> +	 * To all the interfaces as a slave, the link_speed is checked.
> +	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)

There is no need to make reference to my suggestion in the comment, especially since that is in the commit message. Please simplify your comment above to be something like:
	 * For all slave interfaces, wait for the link_speed to be known.

> 	 */
> -	if ((hw->mac.type == ixgbe_mac_X540) &&
> -	    (netdev->flags & IFF_SLAVE))
> +	if (netdev->flags & IFF_SLAVE)
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

The above would be better as:
	if ((netdev->flags & IFF_SLAVE) &&
	    link_speed == IXGBE_LINK_SPEED_UNKNOWN)
		return;

Please do not send a series of patches - it just adds needless confusion and is a bisect hazard. Just send a single patch with the desired change as a V5.

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

[Intel-wired-lan] [PATCH 3/3] ixgbe: synchronize the link_speed and link_up of a slave interface

[Rustad, Mark D]

zyjzyj2000 at gmail.com wrote:

> From: Zhu Yanjun <yanjun.zhu@windriver.com>
> 
> According to the suggestion from Rustad, Mark D, this behavior perhaps
> is more related to the copper phy. But to make fiber phy more robust,
> to all the interfaces as a slave interface, the link_speed and link_up
> is synchronized.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1bb6056..ce47639 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
> 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
> 	 * independent interface, it is not necessary to synchronize link_up
> 	 * and link_speed.
> -	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
> +	 * According to the suggestion from Rustad, Mark D, this behavior
> +	 * perhaps is related to the copper phy. To make fiber phy more robust,
> +	 * To all the interfaces as a slave, the link_speed is checked.
> +	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)

There is no need to make reference to my suggestion in the comment, especially since that is in the commit message. Please simplify your comment above to be something like:
	 * For all slave interfaces, wait for the link_speed to be known.

> 	 */
> -	if ((hw->mac.type == ixgbe_mac_X540) &&
> -	    (netdev->flags & IFF_SLAVE))
> +	if (netdev->flags & IFF_SLAVE)
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

The above would be better as:
	if ((netdev->flags & IFF_SLAVE) &&
	    link_speed == IXGBE_LINK_SPEED_UNKNOWN)
		return;

Please do not send a series of patches - it just adds needless confusion and is a bisect hazard. Just send a single patch with the desired change as a V5.

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151230/5415a7a9/attachment.asc>

[PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[zyjzyj2000]

Hi, all

Thanks for the suggestions from Rustad, Mark D.
According to his suggestions, the logs and source code are simplified.

Zhu Yanjun

[Intel-wired-lan] [PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[zyjzyj2000]

Hi, all

Thanks for the suggestions from Rustad, Mark D.
According to his suggestions, the logs and source code are simplified.

Zhu Yanjun

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

[zyjzyj2000]

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(+)

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) {
-- 
1.9.1

[Intel-wired-lan] [PATCH 1/1] ixgbe: synchronize link_up and link_speed of a slave interface

[zyjzyj2000]

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(+)

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) {
-- 
1.9.1

Re: [PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000@gmail.com wrote:
> Thanks for the suggestions from Rustad, Mark D.
> According to his suggestions, the logs and source code are
> simplified.

I find it funny that this email (no patch) is got the correct subject,
yet the updated patch you sent does not.  The subject line for this
email should have been used for the updated patch you sent out.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Intel-wired-lan] [PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[Jeff Kirsher]

On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000 at gmail.com wrote:
> Thanks for the suggestions from Rustad, Mark D.
> According to his suggestions, the logs and source code are
> simplified.

I find it funny that this email (no patch) is got the correct subject,
yet the updated patch you sent does not. ?The subject line for this
email should have been used for the updated patch you sent out.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151230/110ef944/attachment.asc>

Re: [PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[zhuyj]

On 12/31/2015 01:17 PM, Jeff Kirsher wrote:
> On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000@gmail.com wrote:
>> Thanks for the suggestions from Rustad, Mark D.
>> According to his suggestions, the logs and source code are
>> simplified.
> I find it funny that this email (no patch) is got the correct subject,
> yet the updated patch you sent does not.  The subject line for this
> email should have been used for the updated patch you sent out.
Thanks for your reply.
I will resend the patch soon.

Best Regards!
Zhu Yanjun

[Intel-wired-lan] [PATCH V5] ixgbe: synchronize link_up and link_speed of a slave

[zhuyj]

On 12/31/2015 01:17 PM, Jeff Kirsher wrote:
> On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000 at gmail.com wrote:
>> Thanks for the suggestions from Rustad, Mark D.
>> According to his suggestions, the logs and source code are
>> simplified.
> I find it funny that this email (no patch) is got the correct subject,
> yet the updated patch you sent does not.  The subject line for this
> email should have been used for the updated patch you sent out.
Thanks for your reply.
I will resend the patch soon.

Best Regards!
Zhu Yanjun

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

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

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(+)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Intel-wired-lan] [PATCH 1/1] ixgbe: synchronize link_up and link_speed of a slave interface

[Jeff Kirsher]

On Thu, 2015-12-31 at 13:04 +0800, zyjzyj2000 at 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(+)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151230/dd49f916/attachment-0001.asc>

[PATCH V6] ixgbe: synchronize link_up and link_speed of a slave

[zyjzyj2000]

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

[Intel-wired-lan] [PATCH V6] ixgbe: synchronize link_up and link_speed of a slave

[zyjzyj2000]

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

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

[zyjzyj2000]

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(+)

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) {
-- 
1.9.1

[Intel-wired-lan] [PATCH V6 1/1] ixgbe: synchronize link_up and link_speed of a slave interface

[zyjzyj2000]

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(+)

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) {
-- 
1.9.1

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>> Sent: Wednesday, December 30, 2015 12:20 AM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>> bounces@lists.osuosl.org]
>>>> On
>>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>>> Mitch
>>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>>> devel@lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>>>> of
>>>>>> link_up and speed
>>>>>>
>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>
>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>> interface,
>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>> the time span between link_up and link_speed is acceptable.
>>>>> What exactly do you mean by "time span between link_up and link_speed"?
>>>> In the previous mail, I show you some ethtool logs. In these logs, there
>>>> is some
>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>> time span between
>>>> link_up and link_speed. Please see the previous mail for details.
>>> Was this when reporting the link state from check_link() (reading the
>> LINKS
>>> register) or reporting the adapter->link_speed?
>>>
>>>>> Where is it you think the de-synchronization occurs?
>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>> netdevice struct.
>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>>>> set, we continue to check
>>>> link_speed. If not, this function is executed whether this link_speed is
>>>> unknown or not.
>>> I can already see this in your patch. I was asking about the reason why
>>> your change is needed.
>> an extreme example, let us assume this scenario:
> Is this the scenario you are trying to fix?
Sure. If IFF_SLAVE is checked, this scenario will not happen.

Zhu Yanjun
>
>> An ixgbe NIC directly connects to another NIC (let us call it NIC-a).
>> And auto-negotiate is off while no static speed is set in the 2 NICs.
> The ixgbe driver does not support disabling auto-negotiation directly.
> The only time this is true is when the advertised speed is restricted,
> so the above scenario is not possible (you either have autoneg or
> advertised speed set) with the current driver.
>
> Is this example in theory or do you have your interface configured this
> way somehow?
>
> Thanks,
> Emil
>
>


------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>> Sent: Wednesday, December 30, 2015 12:20 AM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>> bounces at lists.osuosl.org]
>>>> On
>>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams,
>>>> Mitch
>>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>>> devel at lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>>>> of
>>>>>> link_up and speed
>>>>>>
>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>
>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>> interface,
>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>> the time span between link_up and link_speed is acceptable.
>>>>> What exactly do you mean by "time span between link_up and link_speed"?
>>>> In the previous mail, I show you some ethtool logs. In these logs, there
>>>> is some
>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>> time span between
>>>> link_up and link_speed. Please see the previous mail for details.
>>> Was this when reporting the link state from check_link() (reading the
>> LINKS
>>> register) or reporting the adapter->link_speed?
>>>
>>>>> Where is it you think the de-synchronization occurs?
>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>> netdevice struct.
>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag is
>>>> set, we continue to check
>>>> link_speed. If not, this function is executed whether this link_speed is
>>>> unknown or not.
>>> I can already see this in your patch. I was asking about the reason why
>>> your change is needed.
>> an extreme example, let us assume this scenario:
> Is this the scenario you are trying to fix?
Sure. If IFF_SLAVE is checked, this scenario will not happen.

Zhu Yanjun
>
>> An ixgbe NIC directly connects to another NIC (let us call it NIC-a).
>> And auto-negotiate is off while no static speed is set in the 2 NICs.
> The ixgbe driver does not support disabling auto-negotiation directly.
> The only time this is true is when the advertised speed is restricted,
> so the above scenario is not possible (you either have autoneg or
> advertised speed set) with the current driver.
>
> Is this example in theory or do you have your interface configured this
> way somehow?
>
> Thanks,
> Emil
>
>

RE: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Tuesday, January 05, 2016 9:42 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>> of link_up and speed
>>>
>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>Ronciak,
>>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>> synchronization
>>>>> of link_up and speed
>>>>>
>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>> bounces@lists.osuosl.org]
>>>>> On
>>>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>Williams,
>>>>> Mitch
>>>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>>>> devel@lists.sourceforge.net
>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>> Bourg,
>>>>>>> Vincent (Wind River)
>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>>>> of
>>>>>>> link_up and speed
>>>>>>>
>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>
>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>> interface,
>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>> What exactly do you mean by "time span between link_up and
>link_speed"?
>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>there
>>>>> is some
>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>> time span between
>>>>> link_up and link_speed. Please see the previous mail for details.
>>>> Was this when reporting the link state from check_link() (reading the
>>> LINKS
>>>> register) or reporting the adapter->link_speed?
>>>>
>>>>>> Where is it you think the de-synchronization occurs?
>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>> netdevice struct.
>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>is
>>>>> set, we continue to check
>>>>> link_speed. If not, this function is executed whether this link_speed
>is
>>>>> unknown or not.
>>>> I can already see this in your patch. I was asking about the reason why
>>>> your change is needed.
>>> an extreme example, let us assume this scenario:
>> Is this the scenario you are trying to fix?
>Sure. If IFF_SLAVE is checked, this scenario will not happen.

I already explained why this is not a valid scenario, but if you were able
to set it up somehow I'd like to know how you did it

If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
be an issue regardless of whether the interface is a part of a bond or not, 
but you haven't provided any proof that this is the case. Do you have a
dmesg log that shows ixgbe reporting unknown speed?

Was your patch tested by the customer that reported this issue?

Thanks,
Emil

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[Tantilov, Emil S]

>-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>Sent: Tuesday, January 05, 2016 9:42 PM
>To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>Vincent (Wind River)
>Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>of link_up and speed
>
>On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>Bourg,
>>> Vincent (Wind River)
>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>> of link_up and speed
>>>
>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>> -----Original Message-----
>>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>Ronciak,
>>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>> Bourg,
>>>>> Vincent (Wind River)
>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>> synchronization
>>>>> of link_up and speed
>>>>>
>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>> bounces at lists.osuosl.org]
>>>>> On
>>>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>Williams,
>>>>> Mitch
>>>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>>>> devel at lists.sourceforge.net
>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>> Bourg,
>>>>>>> Vincent (Wind River)
>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>synchronization
>>>>> of
>>>>>>> link_up and speed
>>>>>>>
>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>
>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>> interface,
>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>> What exactly do you mean by "time span between link_up and
>link_speed"?
>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>there
>>>>> is some
>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>> time span between
>>>>> link_up and link_speed. Please see the previous mail for details.
>>>> Was this when reporting the link state from check_link() (reading the
>>> LINKS
>>>> register) or reporting the adapter->link_speed?
>>>>
>>>>>> Where is it you think the de-synchronization occurs?
>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>> netdevice struct.
>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>is
>>>>> set, we continue to check
>>>>> link_speed. If not, this function is executed whether this link_speed
>is
>>>>> unknown or not.
>>>> I can already see this in your patch. I was asking about the reason why
>>>> your change is needed.
>>> an extreme example, let us assume this scenario:
>> Is this the scenario you are trying to fix?
>Sure. If IFF_SLAVE is checked, this scenario will not happen.

I already explained why this is not a valid scenario, but if you were able
to set it up somehow I'd like to know how you did it

If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
be an issue regardless of whether the interface is a part of a bond or not, 
but you haven't provided any proof that this is the case. Do you have a
dmesg log that shows ixgbe reporting unknown speed?

Was your patch tested by the customer that reported this issue?

Thanks,
Emil

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 01/06/2016 11:30 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>> Sent: Tuesday, January 05, 2016 9:42 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>> Ronciak,
>>>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>>> synchronization
>>>>>> of link_up and speed
>>>>>>
>>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>>> bounces@lists.osuosl.org]
>>>>>> On
>>>>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>> Williams,
>>>>>> Mitch
>>>>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>>>>> devel@lists.sourceforge.net
>>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>>> Bourg,
>>>>>>>> Vincent (Wind River)
>>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>>>> of
>>>>>>>> link_up and speed
>>>>>>>>
>>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>>
>>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>>> interface,
>>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>>> What exactly do you mean by "time span between link_up and
>> link_speed"?
>>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>> there
>>>>>> is some
>>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>>> time span between
>>>>>> link_up and link_speed. Please see the previous mail for details.
>>>>> Was this when reporting the link state from check_link() (reading the
>>>> LINKS
>>>>> register) or reporting the adapter->link_speed?
>>>>>
>>>>>>> Where is it you think the de-synchronization occurs?
>>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>>> netdevice struct.
>>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>> is
>>>>>> set, we continue to check
>>>>>> link_speed. If not, this function is executed whether this link_speed
>> is
>>>>>> unknown or not.
>>>>> I can already see this in your patch. I was asking about the reason why
>>>>> your change is needed.
>>>> an extreme example, let us assume this scenario:
>>> Is this the scenario you are trying to fix?
>> Sure. If IFF_SLAVE is checked, this scenario will not happen.
> I already explained why this is not a valid scenario, but if you were able
> to set it up somehow I'd like to know how you did it
If it is not a valid scenario, maybe there is something wrong with NIC 
driver/hardware.
We should pay attention to it.

Zhu Yanjun

>
> If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
> be an issue regardless of whether the interface is a part of a bond or not,
> but you haven't provided any proof that this is the case. Do you have a
> dmesg log that shows ixgbe reporting unknown speed?
>
> Was your patch tested by the customer that reported this issue?
>
> Thanks,
> Emil
>

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 01/06/2016 11:30 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>> Sent: Tuesday, January 05, 2016 9:42 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>> Ronciak,
>>>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>>> synchronization
>>>>>> of link_up and speed
>>>>>>
>>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>>> bounces at lists.osuosl.org]
>>>>>> On
>>>>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>> Williams,
>>>>>> Mitch
>>>>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>>>>> devel at lists.sourceforge.net
>>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>>> Bourg,
>>>>>>>> Vincent (Wind River)
>>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>>>> of
>>>>>>>> link_up and speed
>>>>>>>>
>>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>>
>>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>>> interface,
>>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>>> What exactly do you mean by "time span between link_up and
>> link_speed"?
>>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>> there
>>>>>> is some
>>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>>> time span between
>>>>>> link_up and link_speed. Please see the previous mail for details.
>>>>> Was this when reporting the link state from check_link() (reading the
>>>> LINKS
>>>>> register) or reporting the adapter->link_speed?
>>>>>
>>>>>>> Where is it you think the de-synchronization occurs?
>>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>>> netdevice struct.
>>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>> is
>>>>>> set, we continue to check
>>>>>> link_speed. If not, this function is executed whether this link_speed
>> is
>>>>>> unknown or not.
>>>>> I can already see this in your patch. I was asking about the reason why
>>>>> your change is needed.
>>>> an extreme example, let us assume this scenario:
>>> Is this the scenario you are trying to fix?
>> Sure. If IFF_SLAVE is checked, this scenario will not happen.
> I already explained why this is not a valid scenario, but if you were able
> to set it up somehow I'd like to know how you did it
If it is not a valid scenario, maybe there is something wrong with NIC 
driver/hardware.
We should pay attention to it.

Zhu Yanjun

>
> If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
> be an issue regardless of whether the interface is a part of a bond or not,
> but you haven't provided any proof that this is the case. Do you have a
> dmesg log that shows ixgbe reporting unknown speed?
>
> Was your patch tested by the customer that reported this issue?
>
> Thanks,
> Emil
>

Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 01/06/2016 11:30 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>> Sent: Tuesday, January 05, 2016 9:42 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com]
>>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>> Ronciak,
>>>>>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org;
>>>>>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>>> synchronization
>>>>>> of link_up and speed
>>>>>>
>>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>>> bounces@lists.osuosl.org]
>>>>>> On
>>>>>>>> Behalf Of zyjzyj2000@gmail.com
>>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>> Williams,
>>>>>> Mitch
>>>>>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000-
>>>>>>>> devel@lists.sourceforge.net
>>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>>> Bourg,
>>>>>>>> Vincent (Wind River)
>>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>>>> of
>>>>>>>> link_up and speed
>>>>>>>>
>>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>>
>>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>>> interface,
>>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>>> What exactly do you mean by "time span between link_up and
>> link_speed"?
>>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>> there
>>>>>> is some
>>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>>> time span between
>>>>>> link_up and link_speed. Please see the previous mail for details.
>>>>> Was this when reporting the link state from check_link() (reading the
>>>> LINKS
>>>>> register) or reporting the adapter->link_speed?
>>>>>
>>>>>>> Where is it you think the de-synchronization occurs?
>>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>>> netdevice struct.
>>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>> is
>>>>>> set, we continue to check
>>>>>> link_speed. If not, this function is executed whether this link_speed
>> is
>>>>>> unknown or not.
>>>>> I can already see this in your patch. I was asking about the reason why
>>>>> your change is needed.
>>>> an extreme example, let us assume this scenario:
>>> Is this the scenario you are trying to fix?
>> Sure. If IFF_SLAVE is checked, this scenario will not happen.
> I already explained why this is not a valid scenario, but if you were able
> to set it up somehow I'd like to know how you did it
Sorry. Why can we not disable auto-negotiate on ixgbe NIC ? To be 
honest, I am interested in it.
This is related with hardware or driver?

Thanks a lot.
Zhu Yanjun
>
> If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
> be an issue regardless of whether the interface is a part of a bond or not,
> but you haven't provided any proof that this is the case. Do you have a
> dmesg log that shows ixgbe reporting unknown speed?
>
> Was your patch tested by the customer that reported this issue?
>
> Thanks,
> Emil
>
>

[Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization of link_up and speed

[zhuyj]

On 01/06/2016 11:30 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>> Sent: Tuesday, January 05, 2016 9:42 PM
>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg,
>> Vincent (Wind River)
>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict synchronization
>> of link_up and speed
>>
>> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>> Sent: Wednesday, December 30, 2015 12:20 AM
>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak,
>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>> Bourg,
>>>> Vincent (Wind River)
>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>> of link_up and speed
>>>>
>>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote:
>>>>>> -----Original Message-----
>>>>>> From: zhuyj [mailto:zyjzyj2000 at gmail.com]
>>>>>> Sent: Tuesday, December 29, 2015 6:49 PM
>>>>>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson,
>>>>>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W;
>> Ronciak,
>>>>>> John; Williams, Mitch A; intel-wired-lan at lists.osuosl.org;
>>>>>> netdev at vger.kernel.org; e1000-devel at lists.sourceforge.net
>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>> Bourg,
>>>>>> Vincent (Wind River)
>>>>>> Subject: Re: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>>>> synchronization
>>>>>> of link_up and speed
>>>>>>
>>>>>> On 12/30/2015 12:18 AM, Tantilov, Emil S wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Intel-wired-lan [mailto:intel-wired-lan-
>>>> bounces at lists.osuosl.org]
>>>>>> On
>>>>>>>> Behalf Of zyjzyj2000 at gmail.com
>>>>>>>> Sent: Monday, December 28, 2015 6:32 PM
>>>>>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
>>>>>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John;
>> Williams,
>>>>>> Mitch
>>>>>>>> A; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; e1000-
>>>>>>>> devel at lists.sourceforge.net
>>>>>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River);
>>>>>> Bourg,
>>>>>>>> Vincent (Wind River)
>>>>>>>> Subject: [Intel-wired-lan] [PATCH 2/2] ixgbe: restrict
>> synchronization
>>>>>> of
>>>>>>>> link_up and speed
>>>>>>>>
>>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>>>>>>>
>>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very
>>>>>>>> important to synchronize link_up and link_speed, such as a bonding
>>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent
>>>> interface,
>>>>>>>> it is not necessary to synchronize link_up and link_speed. That is,
>>>>>>>> the time span between link_up and link_speed is acceptable.
>>>>>>> What exactly do you mean by "time span between link_up and
>> link_speed"?
>>>>>> In the previous mail, I show you some ethtool logs. In these logs,
>> there
>>>>>> is some
>>>>>> time with NIC up while speed is unknown. I think this "some time" is
>>>>>> time span between
>>>>>> link_up and link_speed. Please see the previous mail for details.
>>>>> Was this when reporting the link state from check_link() (reading the
>>>> LINKS
>>>>> register) or reporting the adapter->link_speed?
>>>>>
>>>>>>> Where is it you think the de-synchronization occurs?
>>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in
>>>>>> netdevice struct.
>>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag
>> is
>>>>>> set, we continue to check
>>>>>> link_speed. If not, this function is executed whether this link_speed
>> is
>>>>>> unknown or not.
>>>>> I can already see this in your patch. I was asking about the reason why
>>>>> your change is needed.
>>>> an extreme example, let us assume this scenario:
>>> Is this the scenario you are trying to fix?
>> Sure. If IFF_SLAVE is checked, this scenario will not happen.
> I already explained why this is not a valid scenario, but if you were able
> to set it up somehow I'd like to know how you did it
Sorry. Why can we not disable auto-negotiate on ixgbe NIC ? To be 
honest, I am interested in it.
This is related with hardware or driver?

Thanks a lot.
Zhu Yanjun
>
> If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would
> be an issue regardless of whether the interface is a part of a bond or not,
> but you haven't provided any proof that this is the case. Do you have a
> dmesg log that shows ixgbe reporting unknown speed?
>
> Was your patch tested by the customer that reported this issue?
>
> Thanks,
> Emil
>
>