netdev - Re: Bonding on bond

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D374A8F.2020303@gmail.com>
Date:	Wed, 19 Jan 2011 21:33:19 +0100
From:	Nicolas de Pesloüan 
	<nicolas.2p.debian@...il.com>
To:	Jiri Bohac <jbohac@...e.cz>
CC:	Jay Vosburgh <fubar@...ibm.com>,
	"bonding-devel@...ts.sourceforge.net" 
	<bonding-devel@...ts.sourceforge.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Bonding on bond

Le 19/01/2011 16:49, Jiri Bohac a écrit :
 > On Tue, Jan 18, 2011 at 09:07:20AM +0100, Nicolas de Pesloüan wrote:
 >> Staking bond is not supported. Currently, no setup is know to
 >> require stacking bond.

 > I agree. This question and weird bugreports from people trying
 > this come up over and over. How about this patch?

Why not. Adding this to the documentation should also help.

 > bonding: prohibit enslaving of bonding masters
 >
 > Nested bonding is not supported and will result in strange problems, e.g.:
 > - netif_receive_skb() will not properly change skb->dev to point to the
 >   uppoer-most bonding master
 > - arp monitor will not work (dev->last_rx is only updated by hardware drivers)
 > - accidentally enslaving a bonding master to itself will cause an infinite
 >   recursion in the TX path
 >
 > This patch prevents this by prohibiting a bonding master from being further enslaved.
 >
 > Signed-off-by: Jiri Bohac <jbohac@...e.cz>
 >
 > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
 > index b1025b8..d4d5f42 100644
 > --- a/drivers/net/bonding/bond_main.c
 > +++ b/drivers/net/bonding/bond_main.c
 > @@ -1448,8 +1448,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 >  	}
 >
 >  	/* already enslaved */
 > -	if (slave_dev->flags & IFF_SLAVE) {
 > -		pr_debug("Error, Device was already enslaved\n");
 > +	if (slave_dev->priv_flags & IFF_BONDING) {
 > +		pr_debug("Error, Device already enslaved or a bonding master\n");

Even if it is possible to test for slave and for master with a single condition (IFF_BONDING), I 
suggest to split the tests and the error messages, to give end user the best possible diagnostic.

If the device is already a master, let's say it.
If the device is already enslaved, let's continue to say it. It might even be better to give the 
name of the other master that already own this slave.

 >  		return -EBUSY;
 >  	}
 >
 >
 >
 > --
 > Jiri Bohac <jbohac@...e.cz>
 > SUSE Labs, SUSE CZ
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html