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

Alex Wang alexw at nicira.com
Fri Feb 7 18:46:41 UTC 2014


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.
---
 ofproto/bond.c |   45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index b4d9487..135e713 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -58,6 +58,7 @@ struct bond_entry {
 /* A bond slave, that is, one of the links comprising a bond. */
 struct bond_slave {
     struct hmap_node hmap_node; /* In struct bond's slaves hmap. */
+    struct list list_node;      /* In struct bond's enabled_slaves list. */
     struct bond *bond;          /* The bond that contains this slave. */
     void *aux;                  /* Client-provided handle for this slave. */
 
@@ -85,6 +86,11 @@ 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;
+
     /* Bonding info. */
     enum bond_mode balance;     /* Balancing mode, one of BM_*. */
     struct bond_slave *active_slave;
@@ -127,6 +133,8 @@ static struct bond_entry *lookup_bond_entry(const struct bond *,
                                             const struct flow *,
                                             uint16_t vlan)
     OVS_REQ_RDLOCK(rwlock);
+static struct bond_slave *get_enabled_slave(struct bond *)
+    OVS_REQ_RDLOCK(rwlock);
 static struct bond_slave *choose_output_slave(const struct bond *,
                                               const struct flow *,
                                               struct flow_wildcards *,
@@ -180,6 +188,8 @@ bond_create(const struct bond_settings *s)
 
     bond = xzalloc(sizeof *bond);
     hmap_init(&bond->slaves);
+    list_init(&bond->enabled_slaves);
+    ovs_mutex_init(&bond->mutex);
     bond->next_fake_iface_update = LLONG_MAX;
     ovs_refcount_init(&bond->ref_cnt);
 
@@ -220,6 +230,7 @@ bond_unref(struct bond *bond)
     }
     hmap_destroy(&bond->slaves);
 
+    ovs_mutex_destroy(&bond->mutex);
     free(bond->hash);
     free(bond->name);
     ovs_refcount_destroy(&bond->ref_cnt);
@@ -1339,6 +1350,13 @@ bond_enable_slave(struct bond_slave *slave, bool enable)
     if (enable != slave->enabled) {
         slave->bond->bond_revalidate = true;
         slave->enabled = enable;
+
+        if (enable) {
+            list_insert(&slave->bond->enabled_slaves, &slave->list_node);
+        } else {
+            list_remove(&slave->list_node);
+        }
+
         VLOG_INFO("interface %s: %s", slave->name,
                   slave->enabled ? "enabled" : "disabled");
     }
@@ -1414,6 +1432,27 @@ lookup_bond_entry(const struct bond *bond, const struct flow *flow,
     return &bond->hash[bond_hash(bond, flow, vlan) & BOND_MASK];
 }
 
+/* Selects and returns an enabled slave from the 'enabled_slaves' list
+ * in a round-robin fashion.  If the 'enabled_slaves' list is empty,
+ * returns NULL. */
+static struct bond_slave *
+get_enabled_slave(struct bond *bond)
+{
+    struct list *node;
+
+    ovs_mutex_lock(&bond->mutex);
+    if (list_is_empty(&bond->enabled_slaves)) {
+        ovs_mutex_unlock(&bond->mutex);
+        return NULL;
+    }
+
+    node = list_pop_front(&bond->enabled_slaves);
+    list_push_back(&bond->enabled_slaves, node);
+    ovs_mutex_unlock(&bond->mutex);
+
+    return CONTAINER_OF(node, struct bond_slave, list_node);
+}
+
 static struct bond_slave *
 choose_output_slave(const struct bond *bond, const struct flow *flow,
                     struct flow_wildcards *wc, uint16_t vlan)
@@ -1451,11 +1490,7 @@ choose_output_slave(const struct bond *bond, const struct flow *flow,
         }
         e = lookup_bond_entry(bond, flow, vlan);
         if (!e->slave || !e->slave->enabled) {
-            e->slave = CONTAINER_OF(hmap_random_node(&bond->slaves),
-                                    struct bond_slave, hmap_node);
-            if (!e->slave->enabled) {
-                e->slave = bond->active_slave;
-            }
+            e->slave = get_enabled_slave(CONST_CAST(struct bond*, bond));
         }
         return e->slave;
 
-- 
1.7.9.5




More information about the dev mailing list