All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"gospo@cumulusnetworks.com" <gospo@cumulusnetworks.com>,
	zhuyj <zyjzyj2000@gmail.com>,
	"jiri@mellanox.com" <jiri@mellanox.com>
Subject: Re: bonding reports interface up with 0 Mbps
Date: Thu, 04 Feb 2016 12:19:28 -0800	[thread overview]
Message-ID: <22735.1454617168@famine> (raw)
In-Reply-To: <87618083B2453E4A8714035B62D67992505247BA@FMSMSX105.amr.corp.intel.com>

Tantilov, Emil S <emil.s.tantilov@intel.com> wrote:
[...]
>Sure, I'll give this a try, but I'm not sure this check applies in this case
>as you can see from the trace link is up and carrier is on.

	From code inspection, I see another possible race, although I'm
not sure if it's relevant for this case.  During enslavement, the speed
and duplex are initially set, and then later the link state is checked,
but if link is up, the code presumes the speed and duplex are valid,
which they may not be.

	I think this patch would narrow (but not totally eliminate) this
race:

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 56b560558884..b8b8a24f92d1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1591,6 +1591,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	/* check for initial state */
 	if (bond->params.miimon) {
 		if (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS) {
+			bond_update_speed_duplex(new_slave);
 			if (bond->params.updelay) {
 				bond_set_slave_link_state(new_slave,
 							  BOND_LINK_BACK,


	I'm not sure it's going to be really possible to completely
close all of these races, as the device can change its link state (and
thus what it reports for speed and duplex) asynchronously.  Even in a
NETDEV_UP or NETDEV_CHANGE notifier callback, the link could go down
between the time of the netif_carrier_on call that triggers the notifier
and when the callback runs.

	But, if the link is really flapping here, bonding should get a
notifier for each flap (up or down).  Once it settles down with carrier
up then the speed and duplex should be valid.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

  reply	other threads:[~2016-02-04 20:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-03 23:10 bonding reports interface up with 0 Mbps Tantilov, Emil S
2016-02-04  2:56 ` zhuyj
2016-02-04  5:57 ` Jay Vosburgh
2016-02-04  6:44   ` zhuyj
2016-02-04 15:47   ` Tantilov, Emil S
2016-02-04 20:19     ` Jay Vosburgh [this message]
2016-02-04 20:29 ` Jay Vosburgh
2016-02-05  0:07   ` Tantilov, Emil S
2016-02-05  0:37   ` Jay Vosburgh
2016-02-05  0:43     ` Tantilov, Emil S
2016-02-05  5:19       ` zhuyj
2016-02-05  3:24     ` zhuyj
2016-02-05 16:43     ` Tantilov, Emil S
2016-02-08 16:30     ` Tantilov, Emil S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22735.1454617168@famine \
    --to=jay.vosburgh@canonical.com \
    --cc=emil.s.tantilov@intel.com \
    --cc=gospo@cumulusnetworks.com \
    --cc=jiri@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=zyjzyj2000@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.