ofproto: Drop 'expirable_mutex' in favor of new global 'ofproto_mutex'.

This is the first step toward making a global lock that protects everything
needed for updating the flow table.  This commit starts out by merging one
lock into the new one, and later commits will continue that process.

The mutex is initially a recursive one, because I wasn't sure that there
were no nested acquisitions at this stage, but a later commit will change
it to a regular error-checking mutex once it's settled down a bit.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 96cbd7b..11630aa 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3642,12 +3642,12 @@ expire(struct dpif_backer *backer)
 
         /* Expire OpenFlow flows whose idle_timeout or hard_timeout
          * has passed. */
-        ovs_mutex_lock(&ofproto->up.expirable_mutex);
+        ovs_mutex_lock(&ofproto_mutex);
         LIST_FOR_EACH_SAFE (rule, next_rule, expirable,
                             &ofproto->up.expirable) {
             rule_expire(rule_dpif_cast(rule));
         }
-        ovs_mutex_unlock(&ofproto->up.expirable_mutex);
+        ovs_mutex_unlock(&ofproto_mutex);
 
         /* All outstanding data in existing flows has been accounted, so it's a
          * good time to do bond rebalancing. */

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 8d0e14f..06ba70e 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -40,6 +40,8 @@ struct ofputil_flow_mod;
 struct bfd_cfg;
 struct meter;
 
+extern struct ovs_mutex ofproto_mutex;
+
 /* An OpenFlow switch.
  *
  * With few exceptions, ofproto implementations may look at these fields but
@@ -77,11 +79,8 @@ struct ofproto {
 
     struct hindex cookies;      /* Rules indexed on their cookie values. */
 
-    /* Optimisation for flow expiry.
-     * These flows should all be present in tables. */
-    struct ovs_mutex expirable_mutex;
-    struct list expirable OVS_GUARDED; /* Expirable 'struct rule"s in all
-                                          tables. */
+    /* List of expirable flows, in all flow tables. */
+    struct list expirable OVS_GUARDED_BY(ofproto_mutex);
 
     /* Meter table.
      * OpenFlow meters start at 1.  To avoid confusion we leave the first
@@ -268,9 +267,9 @@ struct rule {
     uint64_t add_seqno;         /* Sequence number when added. */
     uint64_t modify_seqno;      /* Sequence number when changed. */
 
-    /* Optimisation for flow expiry. */
-    struct list expirable;      /* In ofproto's 'expirable' list if this rule
-                                 * is expirable, otherwise empty. */
+    /* Optimisation for flow expiry.  In ofproto's 'expirable' list if this
+     * rule is expirable, otherwise empty. */
+    struct list expirable OVS_GUARDED_BY(ofproto_mutex);
 };
 
 void ofproto_rule_ref(struct rule *);
@@ -336,7 +335,7 @@ void ofproto_rule_delete(struct ofproto *, struct classifier *cls,
                          struct rule *) OVS_REQ_WRLOCK(cls->rwlock);
 void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
                                   uint16_t hard_timeout)
-    OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex);
+    OVS_EXCLUDED(ofproto_mutex, rule->timeout_mutex);
 
 bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 685bfa6..4e1a1c7 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -275,6 +275,8 @@ static const struct ofproto_class **ofproto_classes;
 static size_t n_ofproto_classes;
 static size_t allocated_ofproto_classes;
 
+struct ovs_mutex ofproto_mutex;
+
 unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
 unsigned n_handler_threads;
 enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
@@ -305,6 +307,8 @@ ofproto_init(const struct shash *iface_hints)
     struct shash_node *node;
     size_t i;
 
+    ovs_mutex_init_recursive(&ofproto_mutex);
+
     ofproto_class_register(&ofproto_dpif_class);
 
     /* Make a local copy, since we don't own 'iface_hints' elements. */
@@ -460,6 +464,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     }
 
     /* Initialize. */
+    ovs_mutex_lock(&ofproto_mutex);
     memset(ofproto, 0, sizeof *ofproto);
     ofproto->ofproto_class = class;
     ofproto->name = xstrdup(datapath_name);
@@ -484,7 +489,6 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->n_tables = 0;
     hindex_init(&ofproto->cookies);
     list_init(&ofproto->expirable);
-    ovs_mutex_init_recursive(&ofproto->expirable_mutex);
     ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
     ofproto->state = S_OPENFLOW;
     list_init(&ofproto->pending);
@@ -498,6 +502,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->vlan_bitmap = NULL;
     ofproto->vlans_changed = false;
     ofproto->min_mtu = INT_MAX;
+    ovs_mutex_unlock(&ofproto_mutex);
 
     error = ofproto->ofproto_class->construct(ofproto);
     if (error) {
@@ -1212,7 +1217,6 @@ ofproto_destroy__(struct ofproto *ofproto)
 
     free(ofproto->vlan_bitmap);
 
-    ovs_mutex_destroy(&ofproto->expirable_mutex);
     ofproto->ofproto_class->dealloc(ofproto);
 }
 
@@ -4090,17 +4094,17 @@ reduce_timeout(uint16_t max, uint16_t *timeout)
 void
 ofproto_rule_reduce_timeouts(struct rule *rule,
                              uint16_t idle_timeout, uint16_t hard_timeout)
-    OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex)
+    OVS_EXCLUDED(ofproto_mutex, rule->timeout_mutex)
 {
     if (!idle_timeout && !hard_timeout) {
         return;
     }
 
-    ovs_mutex_lock(&rule->ofproto->expirable_mutex);
+    ovs_mutex_lock(&ofproto_mutex);
     if (list_is_empty(&rule->expirable)) {
         list_insert(&rule->ofproto->expirable, &rule->expirable);
     }
-    ovs_mutex_unlock(&rule->ofproto->expirable_mutex);
+    ovs_mutex_unlock(&ofproto_mutex);
 
     ovs_mutex_lock(&rule->timeout_mutex);
     reduce_timeout(idle_timeout, &rule->idle_timeout);
@@ -5854,11 +5858,11 @@ oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls,
     classifier_remove(cls, &rule->cr);
     cookies_remove(ofproto, rule);
     eviction_group_remove_rule(rule);
-    ovs_mutex_lock(&ofproto->expirable_mutex);
+    ovs_mutex_lock(&ofproto_mutex);
     if (!list_is_empty(&rule->expirable)) {
         list_remove(&rule->expirable);
     }
-    ovs_mutex_unlock(&ofproto->expirable_mutex);
+    ovs_mutex_unlock(&ofproto_mutex);
     if (!list_is_empty(&rule->meter_list_node)) {
         list_remove(&rule->meter_list_node);
         list_init(&rule->meter_list_node);
@@ -5891,9 +5895,9 @@ oftable_insert_rule(struct rule *rule)
     ovs_mutex_unlock(&rule->timeout_mutex);
 
     if (may_expire) {
-        ovs_mutex_lock(&ofproto->expirable_mutex);
+        ovs_mutex_lock(&ofproto_mutex);
         list_insert(&ofproto->expirable, &rule->expirable);
-        ovs_mutex_unlock(&ofproto->expirable_mutex);
+        ovs_mutex_unlock(&ofproto_mutex);
     }
     cookies_insert(ofproto, rule);