On 07/15/2015 10:32 PM, Jay Vosburgh wrote:
> Nikolay Aleksandrov <ra...@blackwall.org> wrote:
>
>> From: Nikolay Aleksandrov <niko...@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 <niko...@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.vosbu...@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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
