[ovs-dev] [PATCH V2] bond: Change the way of assigning bond slave for unassigned bond entry.

Ben Pfaff blp at nicira.com
Tue Feb 11 18:14:45 UTC 2014


On Fri, Feb 07, 2014 at 10:46:41AM -0800, Alex Wang wrote:
> Before this commit, ovs randomly selects a slave for unassigned
> bond entry.  If the selected slave is not enabled, the active slave
> is chosen instead.  In this commit, the slave is selected from the
> list of all enabled slaves in a round-robin fashion.  This helps
> improve the consistency of bond behavior when new flows are added.
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>
> 
> ---
> v1->v2:
> - fix a race by moving the test empty list into critical section.

I'm happy with this.  I think that we could improve the lock annotations
and comments, so I'm suggesting to apply the following incremental:

Acked-by: Ben Pfaff <blp at nicira.com>


diff --git a/ofproto/bond.c b/ofproto/bond.c
index 135e713..8adee9d 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -43,6 +43,10 @@
 
 VLOG_DEFINE_THIS_MODULE(bond);
 
+static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER;
+static struct hmap all_bonds__ = HMAP_INITIALIZER(&all_bonds__);
+static struct hmap *const all_bonds OVS_GUARDED_BY(rwlock) = &all_bonds__;
+
 /* Bit-mask for hashing a flow down to a bucket.
  * There are (BOND_MASK + 1) buckets. */
 #define BOND_MASK 0xff
@@ -86,10 +90,13 @@ struct bond {
     /* Slaves. */
     struct hmap slaves;
 
-    /* Enabled slaves. */
-    struct ovs_mutex mutex;     /* Protects 'enabled_slaves' from being */
-                                /* accessed by multiple readers. */
-    struct list enabled_slaves;
+    /* Enabled slaves.
+     *
+     * Any reader or writer of 'enabled_slaves' must hold 'mutex'.
+     * (To prevent the bond_slave from disappearing they must also hold
+     * 'rwlock'.) */
+    struct ovs_mutex mutex OVS_ACQ_AFTER(rwlock);
+    struct list enabled_slaves OVS_GUARDED; /* Contains struct bond_slaves. */
 
     /* Bonding info. */
     enum bond_mode balance;     /* Balancing mode, one of BM_*. */
@@ -112,10 +119,6 @@ struct bond {
     struct ovs_refcount ref_cnt;
 };
 
-static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER;
-static struct hmap all_bonds__ = HMAP_INITIALIZER(&all_bonds__);
-static struct hmap *const all_bonds OVS_GUARDED_BY(rwlock) = &all_bonds__;
-
 static void bond_entry_reset(struct bond *) OVS_REQ_WRLOCK(rwlock);
 static struct bond_slave *bond_slave_lookup(struct bond *, const void *slave_)
     OVS_REQ_RDLOCK(rwlock);



More information about the dev mailing list