Subject: Re: [PATCH next] bonding: speed/duplex update at NETDEV_UP event

From: Stephen Hemminger <[email-masked]>
Date: Fri, 29 Sep 2017 13:08:19 -0700

On Wed, 27 Sep 2017 18:03:49 -0700
Mahesh Bandewar <[email-masked]> wrote:

> From: Mahesh Bandewar <[email-masked]>
> 
> Some NIC drivers don't have correct speed/duplex settings at the
> time they send NETDEV_UP notification and that messes up the
> bonding state. Especially 802.3ad mode which is very sensitive
> to these settings. In the current implementation we invoke
> bond_update_speed_duplex() when we receive NETDEV_UP, however,
> ignore the return value. If the values we get are invalid
> (UNKNOWN), then slave gets removed from the aggregator with
> speed and duplex set to UNKNOWN while link is still marked as UP.
> 
> This patch fixes this scenario. Also 802.3ad mode is sensitive to
> these conditions while other modes are not, so making sure that it
> doesn't change the behavior for other modes.
> 
> Signed-off-by: Mahesh Bandewar <[email-masked]>
> ---
>  drivers/net/bonding/bond_main.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b7313c1d9dcd..177be373966b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3076,7 +3076,16 @@ static int bond_slave_netdev_event(unsigned long event,
>  		break;
>  	case NETDEV_UP:
>  	case NETDEV_CHANGE:
> -		bond_update_speed_duplex(slave);
> +		/* For 802.3ad mode only:
> +		 * Getting invalid Speed/Duplex values here will put slave
> +		 * in weird state. So mark it as link-down for the time
> +		 * being and let link-monitoring (miimon) set it right when
> +		 * correct speeds/duplex are available.
> +		 */
> +		if (bond_update_speed_duplex(slave) &&
> +		    BOND_MODE(bond) == BOND_MODE_8023AD)
> +			slave->link = BOND_LINK_DOWN;
> +
>  		if (BOND_MODE(bond) == BOND_MODE_8023AD)
>  			bond_3ad_adapter_speed_duplex_changed(slave);
>  		/* Fallthrough */

Then fix the drivers. Trying to workaround it here isn't helping.

The problem is that miimon is not required.  Bonding can be purely
event driven.