Hi all; I'm hoping someone can point me in the right direction.  I have
a Broadcom NetXen II BCM5708S network card (bnx2) and a Broadcom NetXen
5714S network card (tg3).  If I use either one by itself, it works fine.
However, I want to bond them as active-active, and I can't use mode=4
because there are other devices on the network which don't support it.
So, I create the bond interface with:

        # modprobe bonding mode=6 miimon=200 xmit_hash_policy=layer2
        
        Ethernet Channel Bonding Driver: v3.3.0 (June 10, 2008)
        bonding: xor_mode param is irrelevant in mode adaptive load balancing
        bonding: In ALB mode you might experience client disconnections upon reconnection of a link if the bonding module updelay parameter (0 msec) is incompatible with the forwarding delay time of the switch

This seems to work fine.  Then I bring up the interface with ifconfig
and I get:

        bond0     Link encap:Ethernet  HWaddr 00:00:00:00:00:00  
                  inet addr:10.0.9.46  Bcast:10.0.15.255  Mask:255.255.240.0
                  UP BROADCAST MASTER MULTICAST  MTU:1500  Metric:1
                  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
                  TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
                  collisions:0 txqueuelen:0 
                  RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

Then I enslave one of my ethernet cards (it doesn't appear to matter
which one I enslave first), and that works fine as well:

        # ifenslave bond0 eth2
        bnx2: eth2: using MSI
        bonding: bond0: enslaving eth2 as an active interface with a down link.
        bnx2: eth2 NIC SerDes Link is Up, 1000 Mbps full duplex, receive & transmit flow control ON
        bonding: bond0: link status definitely up for interface eth2.
        bonding: bond0: making interface eth2 the new active one.
        bonding: bond0: first active interface up!
        
        # ifconfig eth2
        eth2      Link encap:Ethernet  HWaddr ...
[ message continues ]

Sorry for the top-post, but I just wanted to add: the system has two
NetXen II interfaces and two NetXen interfaces.  I've now tried bonding
all combinations of these interfaces, and regardless of the order they
all fail when the second interface is bonded.

As another data point, if I change the bonding to mode=4 instead, then I
don't get any kernel failures (but of course the bonding doesn't work
properly as the switch is not configured for this).

Is anyone else able to use mode=6 with the bonding driver, or is that
mode just non-functional?  Is it something particular to these Broadcom
drivers?

I'm still pretty stumped here and I'd really love some pointers...
thanks!


-- 
Paul Smith <psmith@mad-scientist.net>

--

[ message continues ]

The only help I can give you right now is that on my test system it doesn't crash.

	Bonding Driver: v3.5.0 (November 4, 2008)
	2x Broadcom NetXtreme II BCM5708

The kernel itself is stock 2.6.29 and I'm running on an x86_64 box, not exactly
embedded :)

-Brian
--

[ message continues ]

Paul Smith <paul@mad-scientist.net> wrote:
[...]

	I think I know what's going on.  I believe this patch will
resolve things, but I won't be able to test it until tomorrow.  If you
want to test this, great; if you want to wait, that's fine too.

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 8dc6fbb..b22467a 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1708,10 +1708,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
  * Called with RTNL
  */
 int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
-	__releases(&bond->curr_slave_lock)
-	__releases(&bond->lock)
 	__acquires(&bond->lock)
-	__acquires(&bond->curr_slave_lock)
+	__releases(&bond->lock)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct sockaddr *sa = addr;
@@ -1747,9 +1745,6 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
 		}
 	}
 
-	write_unlock_bh(&bond->curr_slave_lock);
-	read_unlock(&bond->lock);
-
 	if (swap_slave) {
 		alb_swap_mac_addr(bond, swap_slave, bond->curr_active_slave);
 		alb_fasten_mac_swap(bond, swap_slave, bond->curr_active_slave);
@@ -1757,16 +1752,17 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
 		alb_set_slave_mac_addr(bond->curr_active_slave, bond_dev->dev_addr,
 				       bond->alb_info.rlb_enabled);
 
-		alb_send_learning_packets(bond->curr_active_slave, bond_dev->dev_addr);
+		read_lock(&bond->lock);
+		alb_send_learning_packets(bond->curr_active_slave,
+					  bond_dev->dev_addr);
 		if (bond->alb_info.rlb_enabled) {
 			/* inform clients mac address has changed */
-			rlb_req_update_slave_clients(bond, bond->curr_active_slave);
+			rlb_req_update_slave_clients(bond,
+						     bond->curr_active_slave);
 		}
+		read_unlock(&bond->lock);
 	}
 
-	read_lock(&bond->lock);
-	write_lock_bh(&bond->curr_slave_lock);
-
 	return 0;
 }
 



	-J

---
	-Jay Vosburgh, IBM Linux ...
[ message continues ]

From: Jay Vosburgh <fubar@us.ibm.com>

Jay, thanks for working on this.

Let me know when you have a final version of this fix for me
to include.
--

[ message continues ]

I tested this; it works great.  All my systems came up fine with this

-- 
Paul Smith <paul@mad-scientist.net>
--

[ message continues ]

Hi Jay; as I mentioned last night this patch is working fine for me so
far.

However, looking at the rest of this function it seems to me that there
are other locking issues, at least based on the documentation in the
header file:

 * Here are the locking policies for the two bonding locks:
 *
 * 1) Get bond->lock when reading/writing slave list.
 * 2) Get bond->curr_slave_lock when reading/writing bond->curr_active_slave.
 *    (It is unnecessary when the write-lock is put with bond->lock.)
 * 3) When we lock with bond->curr_slave_lock, we must lock with bond->lock
 *    beforehand.

For example, don't you need to hold bond->curr_slave_lock at least
around the "if (!bond->curr_active_slave)"?  What about around the
"bond_for_each_slave" loop?

Many of the other functions, later, also seem to work with
bond->curr_active_slave and they don't take this lock.

Unless I'm missing something, I think there are still more problems in
the locking in bond_alb_set_mac_address().


-- 
Paul Smith <paul@mad-scientist.net>

--

[ message continues ]

The various MAC manipulating functions are either called under
RTNL (as bond_alb_set_mac_address is) or take pains to acquire RTNL
before doing anything with the MAC.  Also, the slave list and
curr_active_slave are mutexed by RTNL, so those inspections should be
safe.

	I'm reasonably sure that the curr_slave_lock is superfluous
(which wasn't the case when it was originally introduced), but I haven't
had a chance to validate this.  The locking has changed from what's
documented in the header file; RTNL wasn't used for this when that was
written.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
--

[ message continues ]

OK, sounds good.  I'll let you know if I observe any other odd behavior
with the bonding driver.

Thanks for the great support!  Cheers!

-- 
Paul Smith <paul@mad-scientist.net>

--

[ message continues ]