[ovs-dev] [lacp bridge 2/4] bond: Give bridge control over LACP module.

Ben Pfaff blp at nicira.com
Fri Apr 15 17:29:14 UTC 2011


On Thu, Apr 14, 2011 at 07:09:36PM -0700, Ethan Jackson wrote:
> Before this patch, the bonding code had taken over responsibility
> for running the LACP module.  However, the bonding code only needs
> the LACP module for some basic status queries.  LACP and bonding
> are actually logically parallel modules and do not really have a
> parent child relationship.  Furthermore, we need to be able to run
> LACP on non-bonded interfaces which the existing approach
> prevented.  This patch gives control of the LACP module back to the
> bridge.

This adds a blank line at the beginning of port_run().

I don't see anything that destroys the lacp object when a port is
destroyed.

I think you missed adding one "->aux":

diff --git a/lib/bond.c b/lib/bond.c
index b1ca5d5..3ec1ba2 100644
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -1483,7 +1483,7 @@ bond_choose_slave(const struct bond *bond)
     best = NULL;
     HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
         if (slave->delay_expires != LLONG_MAX
-            && lacp_slave_may_enable(bond->lacp, slave)
+            && lacp_slave_may_enable(bond->lacp, slave->aux)
             && (!best || slave->delay_expires < best->delay_expires))
	     {
             best = slave;
         }

Otherwise looks good.  I'd be concerned about keeping the "struct lacp
*" pointer in bond except that you're taking care of that in later
patches.



More information about the dev mailing list