From: Stephen Hemminger <stephen@networkplumber•org>
To: Mahesh Bandewar <mahesh@bandewar•net>
Cc: Jay Vosburgh <j.vosburgh@gmail•com>,
Andy Gospodarek <andy@greyhouse•net>,
Veaceslav Falico <vfalico@gmail•com>,
David Miller <davem@davemloft•net>,
Netdev <netdev@vger•kernel.org>,
Mahesh Bandewar <maheshb@google•com>
Subject: Re: [PATCH next] bonding: speed/duplex update at NETDEV_UP event
Date: Fri, 29 Sep 2017 13:08:19 -0700 [thread overview]
Message-ID: <20170929130819.74879e99@xeon-e3> (raw)
In-Reply-To: <20170928010349.8988-1-mahesh@bandewar.net>
On Wed, 27 Sep 2017 18:03:49 -0700
Mahesh Bandewar <mahesh@bandewar•net> wrote:
> From: Mahesh Bandewar <maheshb@google•com>
>
> 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 <maheshb@google•com>
> ---
> 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.
next prev parent reply other threads:[~2017-09-29 20:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 1:03 [PATCH next] bonding: speed/duplex update at NETDEV_UP event Mahesh Bandewar
2017-09-29 20:08 ` Stephen Hemminger [this message]
2017-09-29 22:02 ` Mahesh Bandewar (महेश बंडेवार)
2017-10-03 21:32 ` David Miller
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=20170929130819.74879e99@xeon-e3 \
--to=stephen@networkplumber$(echo .)org \
--cc=andy@greyhouse$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=j.vosburgh@gmail$(echo .)com \
--cc=mahesh@bandewar$(echo .)net \
--cc=maheshb@google$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=vfalico@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox