From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jay Vosburgh Subject: Re: [PATCH net] bonding: fix bond dev flags after convert to arphrd_ether Date: Wed, 15 Jul 2015 13:32:33 -0700 Message-ID: <1817.1436992353@famine> References: <1436990983-1406-1-git-send-email-razor@blackwall.org> Cc: netdev@vger.kernel.org, monis@voltaire.com, gospo@cumulusnetworks.com, vfalico@gmail.com, davem@davemloft.net, Nikolay Aleksandrov To: Nikolay Aleksandrov Return-path: Received: from youngberry.canonical.com ([91.189.89.112]:51668 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753239AbbGOUcj (ORCPT ); Wed, 15 Jul 2015 16:32:39 -0400 In-reply-to: <1436990983-1406-1-git-send-email-razor@blackwall.org> Sender: netdev-owner@vger.kernel.org List-ID: Nikolay Aleksandrov wrote: >From: Nikolay Aleksandrov > >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 >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