| Message ID | 20150320174638.GA2053@p183.telecom.by |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Alexey Dobriyan <adobriyan@gmail.com> Date: Fri, 20 Mar 2015 20:46:38 +0300 > If you add bonding master as a slave, and then release it, > it will no longer be an IFF_BONDING creating problems like described at > https://bugzilla.kernel.org/show_bug.cgi?id=89541 > > echo +bond1 >/sys/class/net/bonding_masters > echo 1 >/sys/class/net/bond1/bonding/mode > echo +bond2 >/sys/class/net/bonding_masters > echo +bond2 >/sys/class/net/bond1/bonding/slaves > echo -bond2 >/sys/class/net/bond1/bonding/slaves > echo -bond2 >/sys/class/net/bonding_masters > > cat /proc/net/bonding/bond2 # should not exist > [oops] > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> I feel like this has been brought up before and it was stated that some people are actually using things like this. I could be mistaken. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller <davem@davemloft.net> wrote: >From: Alexey Dobriyan <adobriyan@gmail.com> >Date: Fri, 20 Mar 2015 20:46:38 +0300 > >> If you add bonding master as a slave, and then release it, >> it will no longer be an IFF_BONDING creating problems like described at >> https://bugzilla.kernel.org/show_bug.cgi?id=89541 >> >> echo +bond1 >/sys/class/net/bonding_masters >> echo 1 >/sys/class/net/bond1/bonding/mode >> echo +bond2 >/sys/class/net/bonding_masters >> echo +bond2 >/sys/class/net/bond1/bonding/slaves >> echo -bond2 >/sys/class/net/bond1/bonding/slaves >> echo -bond2 >/sys/class/net/bonding_masters >> >> cat /proc/net/bonding/bond2 # should not exist >> [oops] >> >> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > >I feel like this has been brought up before and it was stated that >some people are actually using things like this. > >I could be mistaken. I don't think you are. I did a bit of checking after the discussion last month and found a few relatively recent statements that people were nesting bonds and it was apparently working, e.g., http://www.alexwitherspoon.com/debian-nested-bonded-interfaces/ which, ironically, is exactly the case that would benefit from not nesting the bonds, as 802.3ad would handle multiple aggregators itself. However, there is also this discussion http://lists.openwall.net/netdev/2011/01/22/66 from netdev in 2011 that states that the ingress path of nested bonds does not work, at least for the case described. Perhaps some configurations work and some don't. Let me see if I can run a quick test and see if this actually works for me... -J --- -Jay Vosburgh, jay.vosburgh@canonical.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jay Vosburgh <jay.vosburgh@canonical.com> wrote: >David Miller <davem@davemloft.net> wrote: > >From: Alexey Dobriyan <adobriyan@gmail.com> >>Date: Fri, 20 Mar 2015 20:46:38 +0300 >> >>> If you add bonding master as a slave, and then release it, >>> it will no longer be an IFF_BONDING creating problems like described at >>> https://bugzilla.kernel.org/show_bug.cgi?id=89541 >>> >>> echo +bond1 >/sys/class/net/bonding_masters >>> echo 1 >/sys/class/net/bond1/bonding/mode >>> echo +bond2 >/sys/class/net/bonding_masters >>> echo +bond2 >/sys/class/net/bond1/bonding/slaves >>> echo -bond2 >/sys/class/net/bond1/bonding/slaves >>> echo -bond2 >/sys/class/net/bonding_masters >>> >>> cat /proc/net/bonding/bond2 # should not exist >>> [oops] >>> >>> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> >> >>I feel like this has been brought up before and it was stated that >>some people are actually using things like this. >> >>I could be mistaken. > > I don't think you are. I did a bit of checking after the >discussion last month and found a few relatively recent statements that >people were nesting bonds and it was apparently working, e.g., > >http://www.alexwitherspoon.com/debian-nested-bonded-interfaces/ > > which, ironically, is exactly the case that would benefit from >not nesting the bonds, as 802.3ad would handle multiple aggregators >itself. > > However, there is also this discussion > >http://lists.openwall.net/netdev/2011/01/22/66 > > from netdev in 2011 that states that the ingress path of nested >bonds does not work, at least for the case described. Perhaps some >configurations work and some don't. > > Let me see if I can run a quick test and see if this actually >works for me... I ran a few tests against net-next from a couple of days ago. A simple test of two balance-rr mode bonds nested below an active-backup mode bond appears to function, passing traffic in both directions. I didn't test extensively, but ingress does not appear to be broken as the 2011 netdev discussion indicates. The sequence supplied by Alexey does reveal a bug, in that the bond2 /proc file isn't removed when it should be. In light of the above, however, this will have to be fixed some way other than disallowing nesting. -J --- -Jay Vosburgh, jay.vosburgh@canonical.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1248,6 +1248,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) slave_dev->name); } + if (netif_is_bond_master(slave_dev)) { + netdev_err(bond_dev, "device is bond master\n"); + return -EBUSY; + } + /* already enslaved */ if (slave_dev->flags & IFF_SLAVE) { netdev_dbg(bond_dev, "Error: Device was already enslaved\n");
If you add bonding master as a slave, and then release it, it will no longer be an IFF_BONDING creating problems like described at https://bugzilla.kernel.org/show_bug.cgi?id=89541 echo +bond1 >/sys/class/net/bonding_masters echo 1 >/sys/class/net/bond1/bonding/mode echo +bond2 >/sys/class/net/bonding_masters echo +bond2 >/sys/class/net/bond1/bonding/slaves echo -bond2 >/sys/class/net/bond1/bonding/slaves echo -bond2 >/sys/class/net/bonding_masters cat /proc/net/bonding/bond2 # should not exist [oops] Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- drivers/net/bonding/bond_main.c | 5 +++++ 1 file changed, 5 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html