From: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
To: Jay Vosburgh <jay.vosburgh@canonical•com>,
Nikolay Aleksandrov <razor@blackwall•org>
Cc: netdev@vger•kernel.org, monis@voltaire•com,
gospo@cumulusnetworks•com, vfalico@gmail•com,
davem@davemloft•net
Subject: Re: [PATCH net] bonding: fix bond dev flags after convert to arphrd_ether
Date: Wed, 15 Jul 2015 22:34:19 +0200 [thread overview]
Message-ID: <55A6C3CB.9070605@cumulusnetworks.com> (raw)
In-Reply-To: <1817.1436992353@famine>
On 07/15/2015 10:32 PM, Jay Vosburgh wrote:
> Nikolay Aleksandrov <razor@blackwall•org> wrote:
>
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
>>
>> If a bonding device enslaves devices != arphrd_ether it'll change types
>> and if later these devices are released, it can enslave an arphrd_ether
>> device and switch back calling ether_setup() which resets dev->flags to
>> IFF_BROADCAST|IFF_MULTICAST and clears IFF_MASTER which then could lead
>> to many different bugs. This bug seems to have been there since the
>> introduction of ether_setup() in bond_enslave().
>
> I thought the hack-around for the non-ethernet device problem
> was that, for non-ARPHRD_ETHER devices, the bonding master device is
> automatically destroyed when the last slave is released.
>
> This (well, this sort of thing) originally came up with IPoIB
> devices needing different ops that would disappear if the ipoib module
> was unloaded, so the bond master was deliberately unregistered when the
> last slave was released.
>
> Looking at the code, at first glance this appears to still be
> the case: bond_slave_netdev_event calls bond_release_and_destroy for !=
> ARPHRD_ETHER, which in turn should unregister the bond itself if there
> are no slaves left. Is this no longer working as intended?
>
> -J
>
>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks•com>
>> Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type")
>> ---
>> drivers/net/bonding/bond_main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 317a49480475..8ba119896e55 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1368,6 +1368,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>> bond_setup_by_slave(bond_dev, slave_dev);
>> else {
>> ether_setup(bond_dev);
>> + bond_dev->flags |= IFF_MASTER;
>> bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
>> }
>>
>> --
>> 1.9.3
>>
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical•com
>
Ah yes, the bug is actually that the bond doesn't switch back its type
on failure after the bond_setup_by_slave().
I actually had prepared that fix next. :-)
But you're right, this doesn't need to be fixed if the enslave failure
path is updated to switch back the type on fail.
next prev parent reply other threads:[~2015-07-15 20:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-15 20:09 [PATCH net] bonding: fix bond dev flags after convert to arphrd_ether Nikolay Aleksandrov
2015-07-15 20:32 ` Jay Vosburgh
2015-07-15 20:34 ` Nikolay Aleksandrov [this message]
2015-08-28 2:07 ` Nikolay Aleksandrov
2015-08-28 3:39 ` Jay Vosburgh
2015-08-28 17:38 ` Nikolay Aleksandrov
2015-07-15 20:55 ` Nikolay Aleksandrov
2015-07-20 20:01 ` 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=55A6C3CB.9070605@cumulusnetworks.com \
--to=nikolay@cumulusnetworks$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=gospo@cumulusnetworks$(echo .)com \
--cc=jay.vosburgh@canonical$(echo .)com \
--cc=monis@voltaire$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=razor@blackwall$(echo .)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