ofproto: Don't destroy mutex before its use.

[sliver-openvswitch.git] / ofproto / ofproto.c

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f16005c..49444c1 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -259,7 +259,6 @@ struct ofport_usage {
 };
 
 /* rule. */
-static void ofproto_rule_destroy__(struct rule *);
 static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
 static bool rule_is_modifiable(const struct rule *rule,
                                enum ofputil_flow_mod_flags flag);
@@ -1019,10 +1018,12 @@ ofproto_port_set_bfd(struct ofproto *ofproto, ofp_port_t ofp_port,
     }
 }
 
-/* Populates 'status' with key value pairs indicating the status of the BFD
- * session on 'ofp_port'.  This information is intended to be populated in the
- * OVS database.  Has no effect if 'ofp_port' is not na OpenFlow port in
- * 'ofproto'. */
+/* Populates 'status' with the status of BFD on 'ofport'.  Returns 0 on
+ * success.  Returns a negative number if there is no status change since
+ * last update.  Returns a positive errno otherwise.  Has no effect if
+ * 'ofp_port' is not an OpenFlow port in 'ofproto'.
+ *
+ * The caller must provide and own '*status'. */
 int
 ofproto_port_get_bfd_status(struct ofproto *ofproto, ofp_port_t ofp_port,
                             struct smap *status)
@@ -1310,9 +1311,9 @@ ofproto_destroy__(struct ofproto *ofproto)
     ovs_assert(list_is_empty(&ofproto->pending));
 
     destroy_rule_executes(ofproto);
-    guarded_list_destroy(&ofproto->rule_executes);
-
     delete_group(ofproto, OFPG_ALL);
+
+    guarded_list_destroy(&ofproto->rule_executes);
     ovs_rwlock_destroy(&ofproto->groups_rwlock);
     hmap_destroy(&ofproto->groups);
 
@@ -1372,7 +1373,8 @@ ofproto_destroy(struct ofproto *p)
     }
 
     p->ofproto_class->destruct(p);
-    ofproto_destroy__(p);
+    /* Destroying rules is deferred, must have 'ofproto' around for them. */
+    ovsrcu_postpone(ofproto_destroy__, p);
 }
 
 /* Destroys the datapath with the respective 'name' and 'type'.  With the Linux
@@ -2642,6 +2644,23 @@ update_mtu(struct ofproto *p, struct ofport *port)
     }
 }
 \f
+static void
+ofproto_rule_destroy__(struct rule *rule)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+    cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
+    rule_actions_destroy(rule_get_actions(rule));
+    ovs_mutex_destroy(&rule->mutex);
+    rule->ofproto->ofproto_class->rule_dealloc(rule);
+}
+
+static void
+rule_destroy_cb(struct rule *rule)
+{
+    rule->ofproto->ofproto_class->rule_destruct(rule);
+    ofproto_rule_destroy__(rule);
+}
+
 void
 ofproto_rule_ref(struct rule *rule)
 {
@@ -2650,25 +2669,20 @@ ofproto_rule_ref(struct rule *rule)
     }
 }
 
+/* Decrements 'rule''s ref_count and schedules 'rule' to be destroyed if the
+ * ref_count reaches 0.
+ *
+ * Use of RCU allows short term use (between RCU quiescent periods) without
+ * keeping a reference.  A reference must be taken if the rule needs to
+ * stay around accross the RCU quiescent periods. */
 void
 ofproto_rule_unref(struct rule *rule)
 {
     if (rule && ovs_refcount_unref(&rule->ref_count) == 1) {
-        rule->ofproto->ofproto_class->rule_destruct(rule);
-        ofproto_rule_destroy__(rule);
+        ovsrcu_postpone(rule_destroy_cb, rule);
     }
 }
 
-static void
-ofproto_rule_destroy__(struct rule *rule)
-    OVS_NO_THREAD_SAFETY_ANALYSIS
-{
-    cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
-    rule_actions_destroy(rule_get_actions(rule));
-    ovs_mutex_destroy(&rule->mutex);
-    rule->ofproto->ofproto_class->rule_dealloc(rule);
-}
-
 static uint32_t get_provider_meter_id(const struct ofproto *,
                                       uint32_t of_meter_id);
 
@@ -3743,20 +3757,22 @@ ofproto_get_netflow_ids(const struct ofproto *ofproto,
     ofproto->ofproto_class->get_netflow_ids(ofproto, engine_type, engine_id);
 }
 
-/* Checks the status of CFM configured on 'ofp_port' within 'ofproto'.  Returns
- * true if the port's CFM status was successfully stored into '*status'.
- * Returns false if the port did not have CFM configured, in which case
- * '*status' is indeterminate.
+/* Checks the status of CFM configured on 'ofp_port' within 'ofproto'.
+ * Returns 0 if the port's CFM status was successfully stored into
+ * '*status'.  Returns positive errno if the port did not have CFM
+ * configured.  Returns negative number if there is no status change
+ * since last update.
  *
- * The caller must provide and owns '*status', and must free 'status->rmps'. */
-bool
+ * The caller must provide and own '*status', and must free 'status->rmps'.
+ * '*status' is indeterminate if the return value is non-zero. */
+int
 ofproto_port_get_cfm_status(const struct ofproto *ofproto, ofp_port_t ofp_port,
                             struct ofproto_cfm_status *status)
 {
     struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
-    return (ofport
-            && ofproto->ofproto_class->get_cfm_status
-            && ofproto->ofproto_class->get_cfm_status(ofport, status));
+    return (ofport && ofproto->ofproto_class->get_cfm_status
+            ? ofproto->ofproto_class->get_cfm_status(ofport, status)
+            : EOPNOTSUPP);
 }
 
 static enum ofperr