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 <al...@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 <b...@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);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
