Re: [PATCH net] bonding: fix bond dev flags after convert to arphrd_ether

[Jay Vosburgh <jay.vosburgh@canonical.com>]

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