-----Original Message-----
From: zhuyj [mailto:zyjzyj2000@gmail.com]
Sent: Wednesday, January 06, 2016 7:34 PM
To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh
Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org;
Shteinbock, Boris (Wind River)
Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode

On 01/07/2016 10:43 AM, Tantilov, Emil S wrote:

quoted

quoted

-----Original Message-----
From: zhuyj [mailto:zyjzyj2000@gmail.com]
Sent: Tuesday, January 05, 2016 7:05 PM
To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh
Cc: vfalico@gmail.com; gospo@cumulusnetworks.com;

netdev@vger.kernel.org;

quoted

quoted

Shteinbock, Boris (Wind River)
Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode

On 01/06/2016 09:26 AM, Tantilov, Emil S wrote:

quoted

quoted

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-

owner@vger.kernel.org]

quoted

quoted

On

quoted

quoted

Behalf Of zhuyj
Sent: Monday, December 28, 2015 1:19 AM
To: Michal Kubecek; Jay Vosburgh
Cc: vfalico@gmail.com; gospo@cumulusnetworks.com;

netdev@vger.kernel.org;

quoted

quoted

Shteinbock, Boris (Wind River)
Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode

On 12/28/2015 04:43 PM, Michal Kubecek wrote:

quoted

On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote:

quoted

[off-list ref] wrote:

quoted

In 802.3ad mode, the speed and duplex is needed. But in some NIC,
there is a time span between NIC up state and getting speed and

duplex.

quoted

quoted

quoted

quoted

quoted

As such, sometimes a slave in 802.3ad mode is in up state without
speed and duplex. This will make bonding in 802.3ad mode can not
work well.
To make bonding driver be compatible with more NICs, it is
necessary to restrict the up state in 802.3ad mode.

	What device is this?  It seems a bit odd that an Ethernet

device

quoted

quoted

quoted

quoted

quoted

quoted

can be carrier up but not have the duplex and speed available.

...

quoted

	In general, though, bonding expects a speed or duplex change to
be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would
propagate to the 802.3ad logic.

	If the device here is going carrier up prior to having speed or
duplex available, then maybe it should call netdev_state_change()

when

quoted

quoted

quoted

quoted

quoted

quoted

the duplex and speed are available, or delay calling

netif_carrier_on().

quoted

quoted

quoted

I have encountered this problem (NIC having carrier on before being

able

quoted

quoted

quoted

to detect speed/duplex and driver not notifying when speed/duplex
becomes available) with netxen cards earlier. But it was eventually
fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event
handling.") so this example rather supports what you said.

                                                             Michal

Kubecek

quoted

quoted

Thanks a lot.
I checked the commit 9d01412ae76f ("netxen: Fix link event
handling."). The symptoms are the same with mine.

The root cause is different. In my problem, the root cause is that

LINKS

quoted

quoted

quoted

quoted

register[]  can not provide link_up and link_speed at the same time.
There is a time span between link_up and link_speed.

The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are

updated

quoted

simultaneously. Do you have any proof to show the delay you are

referring

quoted

quoted

to

quoted

as I am sure our HW engineers would like to know about it.

Sorry. I can not reproduce this problem locally. What I have is the
feedback from the customer.

So you are assuming that there is a delay due to the issue you are

seeing?

Sure. Before I get the further feedback from the customer, I can not
make further conclusion.
My patch is based on the feedback from the customer.

Your patch is throwing an RTNL assertion warning:

RTNL: assertion failed at net/core/ethtool.c (357)

Looks like you may need to hold an RTNL lock for the slave before calling
bond_update_speed_duplex(), though I am not sure if it's a good idea in
general. 

Thanks,
Emil