[net-next] bonding: don't allow the master to become its slave

Submitted by Jay Vosburgh on Aug. 9, 2012, 7:23 p.m.


Message ID 21070.1344540211@death.nxdomain
State RFC
Delegated to: David Miller
Headers show

Commit Message

Jay Vosburgh Aug. 9, 2012, 7:23 p.m.
Ben Hutchings <bhutchings@solarflare.com> wrote:

>On Thu, 2012-08-09 at 15:30 -0300, Flavio Leitner wrote:
>> It doesn't make any sense to allow the master to become
>> its slave. That creates a loop of events causing a crash.
>What if there are other intermediate devices, e.g. the slave is a VLAN
>sub-device of the bond?  And doesn't team also have this problem?
>I think a more general check for such loops might be required.

	I thought we had disallowed any nesting of bonds at all, but I
checked the netdev archives, and it appears we discussed it (and agreed
it didn't work), but it kind of petered out.


	In any event, I think a patch like the following would get all
cases (double enslavement or enslavement of any bonding master) in one

	This is basically the same logic that Jiri Bohac originally
proposed in the discussion I mention above, although this patch moves
the test further up and combines the master and slave tests into one.

	Comments?  I haven't tested this at all, but I think the logic
is correct.  I don't think having two separate tests to get special
"master" and "slave" error cases is worthwhile.


	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.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

Patch hide | download patch | download mbox

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6fae5f3..d14651c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1505,18 +1505,17 @@  int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	int link_reporting;
 	int res = 0;
+	if (slave_dev->priv_flags & IFF_BONDING) {
+		pr_debug("Error, Device was already enslaved\n");
+		return -EBUSY;
+	}
 	if (!bond->params.use_carrier && slave_dev->ethtool_ops == NULL &&
 		slave_ops->ndo_do_ioctl == NULL) {
 		pr_warning("%s: Warning: no link monitoring support for %s\n",
 			   bond_dev->name, slave_dev->name);
-	/* already enslaved */
-	if (slave_dev->flags & IFF_SLAVE) {
-		pr_debug("Error, Device was already enslaved\n");
-		return -EBUSY;
-	}
 	/* vlan challenged mutual exclusion */
 	/* no need to lock since we're protected by rtnl_lock */
 	if (slave_dev->features & NETIF_F_VLAN_CHALLENGED) {