RE: [PATCH net-next v2 4/4] bonding: allow carrier and link status to determine link state

From: "Banerjee, Debabrata" <dbanerje@akamai.com>
To: 'David Miller' <davem@davemloft.net>,
	"jay.vosburgh@canonical.com" <jay.vosburgh@canonical.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"vfalico@gmail.com" <vfalico@gmail.com>,
	"andy@greyhouse.net" <andy@greyhouse.net>
Subject: RE: [PATCH net-next v2 4/4] bonding: allow carrier and link status to determine link state
Date: Wed, 16 May 2018 16:54:40 +0000	[thread overview]
Message-ID: <27902b745c4240b4a7968f1a148b4340@usma1ex-dag1mb2.msg.corp.akamai.com> (raw)
In-Reply-To: <20180516.122847.2004339616613745427.davem@davemloft.net>

> From: David Miller [mailto:davem@davemloft.net]
> From: Jay Vosburgh <jay.vosburgh@canonical.com>
> Date: Wed, 16 May 2018 18:11:08 +0200
> 
> > Debabrata Banerjee <dbanerje@akamai.com> wrote:
> >
> >>In a mixed environment it may be difficult to tell if your hardware
> >>support carrier, if it does not it can always report true. With a new
> >>use_carrier option of 2, we can check both carrier and link status
> >>sequentially, instead of one or the other
> >
> > 	Your reply in the prior discussion suggests to me that there is a bug
> > somewhere else (perhaps in bonding, perhaps in the network driver),
> > and this is just papering over it.  As I said, I don't believe that an
> > additional hack to bonding is the appropriate way to resolve this.
> 
> Sorry this crossed with my review and application of this series, my bad.
> 
> Debabrata please give a specific example of a case where the new "both"
> mode actually is needed.
> 
> Thank you.

See output of /proc/net/bonding/bond0 below, same content as the prior email. bnx2x driver, on ith1: "MII Status: up" is directly from netif_carrier_ok(bond->dev) , but speed and duplex are unknown, which come from the same call as would be used from bond_mii_monitor(), __ethtool_get_link_ksettings(slave_dev, &ecmd). Yes it's possible this is because of bugs in the drivers themselves, but without being able to readily reproduce the state below we can't debug it, nor do we know which combination of hardware and drivers are reliable at any given point in time. We can say though that if this had been set to "both", ith1 would have been correctly removed from the bond. That said if this is still controversial I can resubmit the series without this patch.

Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)

Bonding Mode: adaptive load balancing
Primary Slave: None
Currently Active Slave: ith1
MII Status: up
MII Polling Interval (ms): 100
Up Delay (ms): 0
Down Delay (ms): 0

Slave Interface: ith0
MII Status: up
Speed: 10000 Mbps
Duplex: full
Link Failure Count: 1
Permanent HW addr: 00:e0:81:e5:0e:16
Slave queue ID: 0

Slave Interface: ith1
MII Status: up
Speed: Unknown
Duplex: Unknown
Link Failure Count: 0
Permanent HW addr: 00:e0:81:e5:0e:18
Slave queue ID: 0