ofproto: Drop 'expirable_mutex' in favor of new global 'ofproto_mutex'.
[sliver-openvswitch.git] / ofproto / ofproto.c
index f84510f..9abf678 100644 (file)
@@ -124,9 +124,7 @@ struct ofoperation {
 
     /* OFOPERATION_MODIFY, OFOPERATION_REPLACE: The old actions, if the actions
      * are changing. */
-    struct ofpact *ofpacts;
-    size_t ofpacts_len;
-    uint32_t meter_id;
+    struct rule_actions *actions;
 
     /* OFOPERATION_DELETE. */
     enum ofp_flow_removed_reason reason; /* Reason flow was removed. */
@@ -222,6 +220,17 @@ static void rule_criteria_init(struct rule_criteria *, uint8_t table_id,
                                ofp_port_t out_port, uint32_t out_group);
 static void rule_criteria_destroy(struct rule_criteria *);
 
+/* A packet that needs to be passed to rule_execute(). */
+struct rule_execute {
+    struct list list_node;      /* In struct ofproto's "rule_executes" list. */
+    struct rule *rule;          /* Owns a reference to the rule. */
+    ofp_port_t in_port;
+    struct ofpbuf *packet;      /* Owns the packet. */
+};
+
+static void run_rule_executes(struct ofproto *);
+static void destroy_rule_executes(struct ofproto *);
+
 /* ofport. */
 static void ofport_destroy__(struct ofport *);
 static void ofport_destroy(struct ofport *);
@@ -231,7 +240,6 @@ static int init_ports(struct ofproto *);
 static void reinit_ports(struct ofproto *);
 
 /* rule. */
-static void ofproto_rule_destroy(struct 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 *);
@@ -270,6 +278,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;
@@ -300,6 +310,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. */
@@ -455,6 +467,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);
@@ -479,12 +492,12 @@ 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);
     ofproto->n_pending = 0;
     hmap_init(&ofproto->deletions);
+    guarded_list_init(&ofproto->rule_executes);
     ofproto->n_add = ofproto->n_delete = ofproto->n_modify = 0;
     ofproto->first_op = ofproto->last_op = LLONG_MIN;
     ofproto->next_op_report = LLONG_MAX;
@@ -494,6 +507,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->min_mtu = INT_MAX;
     ovs_rwlock_init(&ofproto->groups_rwlock);
     hmap_init(&ofproto->groups);
+    ovs_mutex_unlock(&ofproto_mutex);
 
     error = ofproto->ofproto_class->construct(ofproto);
     if (error) {
@@ -1183,6 +1197,9 @@ ofproto_destroy__(struct ofproto *ofproto)
     ovs_assert(list_is_empty(&ofproto->pending));
     ovs_assert(!ofproto->n_pending);
 
+    destroy_rule_executes(ofproto);
+    guarded_list_destroy(&ofproto->rule_executes);
+
     delete_group(ofproto, OFPG_ALL);
     ovs_rwlock_destroy(&ofproto->groups_rwlock);
     hmap_destroy(&ofproto->groups);
@@ -1211,7 +1228,6 @@ ofproto_destroy__(struct ofproto *ofproto)
 
     free(ofproto->vlan_bitmap);
 
-    ovs_mutex_destroy(&ofproto->expirable_mutex);
     ofproto->ofproto_class->dealloc(ofproto);
 }
 
@@ -1326,6 +1342,8 @@ ofproto_run(struct ofproto *p)
         VLOG_ERR_RL(&rl, "%s: run failed (%s)", p->name, ovs_strerror(error));
     }
 
+    run_rule_executes(p);
+
     /* Restore the eviction group heap invariant occasionally. */
     if (p->eviction_group_timer < time_msec()) {
         size_t i;
@@ -1776,20 +1794,29 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
                  const struct ofpact *ofpacts, size_t ofpacts_len)
 {
     const struct rule *rule;
+    bool must_add;
 
     /* First do a cheap check whether the rule we're looking for already exists
      * with the actions that we want.  If it does, then we're done. */
     ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
     rule = rule_from_cls_rule(classifier_find_match_exactly(
                                   &ofproto->tables[0].cls, match, priority));
+    if (rule) {
+        ovs_rwlock_rdlock(&rule->rwlock);
+        must_add = !ofpacts_equal(rule->actions->ofpacts,
+                                  rule->actions->ofpacts_len,
+                                  ofpacts, ofpacts_len);
+        ovs_rwlock_unlock(&rule->rwlock);
+    } else {
+        must_add = true;
+    }
     ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
 
     /* If there's no such rule or the rule doesn't have the actions we want,
      * fall back to a executing a full flow mod.  We can't optimize this at
      * all because we didn't take enough locks above to ensure that the flow
      * table didn't already change beneath us.  */
-    if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
-                                ofpacts, ofpacts_len)) {
+    if (must_add) {
         simple_flow_mod(ofproto, match, priority, ofpacts, ofpacts_len,
                         OFPFC_MODIFY_STRICT);
     }
@@ -2322,12 +2349,30 @@ update_mtu(struct ofproto *p, struct ofport *port)
     }
 }
 \f
-static void
-ofproto_rule_destroy(struct rule *rule)
+void
+ofproto_rule_ref(struct rule *rule)
 {
     if (rule) {
-        rule->ofproto->ofproto_class->rule_destruct(rule);
-        ofproto_rule_destroy__(rule);
+        unsigned int orig;
+
+        atomic_add(&rule->ref_count, 1, &orig);
+        ovs_assert(orig != 0);
+    }
+}
+
+void
+ofproto_rule_unref(struct rule *rule)
+{
+    if (rule) {
+        unsigned int orig;
+
+        atomic_sub(&rule->ref_count, 1, &orig);
+        if (orig == 1) {
+            rule->ofproto->ofproto_class->rule_destruct(rule);
+            ofproto_rule_destroy__(rule);
+        } else {
+            ovs_assert(orig != 0);
+        }
     }
 }
 
@@ -2335,19 +2380,64 @@ static void
 ofproto_rule_destroy__(struct rule *rule)
 {
     cls_rule_destroy(&rule->cr);
-    free(rule->ofpacts);
+    rule_actions_unref(rule->actions);
     ovs_mutex_destroy(&rule->timeout_mutex);
     ovs_rwlock_destroy(&rule->rwlock);
     rule->ofproto->ofproto_class->rule_dealloc(rule);
 }
 
+/* Creates and returns a new 'struct rule_actions', with a ref_count of 1,
+ * whose actions are a copy of from the 'ofpacts_len' bytes of 'ofpacts'. */
+struct rule_actions *
+rule_actions_create(const struct ofpact *ofpacts, size_t ofpacts_len)
+{
+    struct rule_actions *actions;
+
+    actions = xmalloc(sizeof *actions);
+    atomic_init(&actions->ref_count, 1);
+    actions->ofpacts = xmemdup(ofpacts, ofpacts_len);
+    actions->ofpacts_len = ofpacts_len;
+    actions->meter_id = ofpacts_get_meter(ofpacts, ofpacts_len);
+    return actions;
+}
+
+/* Increments 'actions''s ref_count. */
+void
+rule_actions_ref(struct rule_actions *actions)
+{
+    if (actions) {
+        unsigned int orig;
+
+        atomic_add(&actions->ref_count, 1, &orig);
+        ovs_assert(orig != 0);
+    }
+}
+
+/* Decrements 'actions''s ref_count and frees 'actions' if the ref_count
+ * reaches 0. */
+void
+rule_actions_unref(struct rule_actions *actions)
+{
+    if (actions) {
+        unsigned int orig;
+
+        atomic_sub(&actions->ref_count, 1, &orig);
+        if (orig == 1) {
+            free(actions);
+        } else {
+            ovs_assert(orig != 0);
+        }
+    }
+}
+
 /* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
  * that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
 bool
 ofproto_rule_has_out_port(const struct rule *rule, ofp_port_t port)
 {
     return (port == OFPP_ANY
-            || ofpacts_output_to_port(rule->ofpacts, rule->ofpacts_len, port));
+            || ofpacts_output_to_port(rule->actions->ofpacts,
+                                      rule->actions->ofpacts_len, port));
 }
 
 /* Returns true if 'rule' has group and equals group_id. */
@@ -2355,7 +2445,8 @@ bool
 ofproto_rule_has_out_group(const struct rule *rule, uint32_t group_id)
 {
     return (group_id == OFPG11_ANY
-            || ofpacts_output_to_group(rule->ofpacts, rule->ofpacts_len, group_id));
+            || ofpacts_output_to_group(rule->actions->ofpacts,
+                                       rule->actions->ofpacts_len, group_id));
 }
 
 /* Returns true if a rule related to 'op' has an OpenFlow OFPAT_OUTPUT or
@@ -2374,28 +2465,55 @@ ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port)
 
     case OFOPERATION_MODIFY:
     case OFOPERATION_REPLACE:
-        return ofpacts_output_to_port(op->ofpacts, op->ofpacts_len, out_port);
+        return ofpacts_output_to_port(op->actions->ofpacts,
+                                      op->actions->ofpacts_len, out_port);
     }
 
     NOT_REACHED();
 }
 
-/* Executes the actions indicated by 'rule' on 'packet' and credits 'rule''s
- * statistics appropriately.
- *
- * 'packet' doesn't necessarily have to match 'rule'.  'rule' will be credited
- * with statistics for 'packet' either way.
- *
- * Takes ownership of 'packet'. */
-static int
-rule_execute(struct rule *rule, ofp_port_t in_port, struct ofpbuf *packet)
+static void
+rule_execute_destroy(struct rule_execute *e)
 {
-    struct flow flow;
-    union flow_in_port in_port_;
+    ofproto_rule_unref(e->rule);
+    list_remove(&e->list_node);
+    free(e);
+}
+
+/* Executes all "rule_execute" operations queued up in ofproto->rule_executes,
+ * by passing them to the ofproto provider. */
+static void
+run_rule_executes(struct ofproto *ofproto)
+{
+    struct rule_execute *e, *next;
+    struct list executes;
+
+    guarded_list_pop_all(&ofproto->rule_executes, &executes);
+    LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
+        union flow_in_port in_port_;
+        struct flow flow;
+
+        in_port_.ofp_port = e->in_port;
+        flow_extract(e->packet, 0, 0, NULL, &in_port_, &flow);
+        ofproto->ofproto_class->rule_execute(e->rule, &flow, e->packet);
+
+        rule_execute_destroy(e);
+    }
+}
+
+/* Destroys and discards all "rule_execute" operations queued up in
+ * ofproto->rule_executes. */
+static void
+destroy_rule_executes(struct ofproto *ofproto)
+{
+    struct rule_execute *e, *next;
+    struct list executes;
 
-    in_port_.ofp_port = in_port;
-    flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
-    return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet);
+    guarded_list_pop_all(&ofproto->rule_executes, &executes);
+    LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
+        ofpbuf_delete(e->packet);
+        rule_execute_destroy(e);
+    }
 }
 
 /* Returns true if 'rule' should be hidden from the controller.
@@ -3213,8 +3331,8 @@ handle_flow_stats_request(struct ofconn *ofconn,
         fs.hard_age = age_secs(now - rule->modified);
         ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
                                                &fs.byte_count);
-        fs.ofpacts = rule->ofpacts;
-        fs.ofpacts_len = rule->ofpacts_len;
+        fs.ofpacts = rule->actions->ofpacts;
+        fs.ofpacts_len = rule->actions->ofpacts_len;
 
         ovs_mutex_lock(&rule->timeout_mutex);
         fs.idle_timeout = rule->idle_timeout;
@@ -3250,7 +3368,8 @@ flow_stats_ds(struct rule *rule, struct ds *results)
     ds_put_format(results, "n_bytes=%"PRIu64", ", byte_count);
     cls_rule_format(&rule->cr, results);
     ds_put_char(results, ',');
-    ofpacts_format(rule->ofpacts, rule->ofpacts_len, results);
+    ofpacts_format(rule->actions->ofpacts, rule->actions->ofpacts_len,
+                   results);
     ds_put_cstr(results, "\n");
 }
 
@@ -3637,6 +3756,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     /* Initialize base state. */
     rule->ofproto = ofproto;
     cls_rule_move(&rule->cr, &cr);
+    atomic_init(&rule->ref_count, 1);
     rule->pending = NULL;
     rule->flow_cookie = fm->new_cookie;
     rule->created = rule->modified = rule->used = time_msec();
@@ -3649,10 +3769,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 
     rule->table_id = table - ofproto->tables;
     rule->flags = fm->flags & OFPUTIL_FF_STATE;
-
-    rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
-    rule->ofpacts_len = fm->ofpacts_len;
-    rule->meter_id = ofpacts_get_meter(rule->ofpacts, rule->ofpacts_len);
+    rule->actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
     list_init(&rule->meter_list_node);
     rule->eviction_group = NULL;
     list_init(&rule->expirable);
@@ -3724,7 +3841,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         }
 
         actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
-                                         rule->ofpacts, rule->ofpacts_len);
+                                         rule->actions->ofpacts,
+                                         rule->actions->ofpacts_len);
 
         op = ofoperation_create(group, rule, type, 0);
 
@@ -3749,17 +3867,15 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
 
         reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
         if (actions_changed || reset_counters) {
-            op->ofpacts = rule->ofpacts;
-            op->ofpacts_len = rule->ofpacts_len;
-            op->meter_id = rule->meter_id;
+            struct rule_actions *new_actions;
+
+            op->actions = rule->actions;
+            new_actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
 
             ovs_rwlock_wrlock(&rule->rwlock);
-            rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
-            rule->ofpacts_len = fm->ofpacts_len;
+            rule->actions = new_actions;
             ovs_rwlock_unlock(&rule->rwlock);
 
-            rule->meter_id = ofpacts_get_meter(rule->ofpacts,
-                                               rule->ofpacts_len);
             rule->ofproto->ofproto_class->rule_modify_actions(rule,
                                                               reset_counters);
         } else {
@@ -3998,17 +4114,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);
@@ -4077,35 +4193,46 @@ static enum ofperr
 handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
                   struct ofputil_flow_mod *fm, const struct ofp_header *oh)
 {
-    if (ofproto->n_pending >= 50) {
-        ovs_assert(!list_is_empty(&ofproto->pending));
-        return OFPROTO_POSTPONE;
-    }
+    enum ofperr error;
 
-    switch (fm->command) {
-    case OFPFC_ADD:
-        return add_flow(ofproto, ofconn, fm, oh);
+    if (ofproto->n_pending < 50) {
+        switch (fm->command) {
+        case OFPFC_ADD:
+            error = add_flow(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_MODIFY:
-        return modify_flows_loose(ofproto, ofconn, fm, oh);
+        case OFPFC_MODIFY:
+            error = modify_flows_loose(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_MODIFY_STRICT:
-        return modify_flow_strict(ofproto, ofconn, fm, oh);
+        case OFPFC_MODIFY_STRICT:
+            error = modify_flow_strict(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_DELETE:
-        return delete_flows_loose(ofproto, ofconn, fm, oh);
+        case OFPFC_DELETE:
+            error = delete_flows_loose(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_DELETE_STRICT:
-        return delete_flow_strict(ofproto, ofconn, fm, oh);
+        case OFPFC_DELETE_STRICT:
+            error = delete_flow_strict(ofproto, ofconn, fm, oh);
+            break;
 
-    default:
-        if (fm->command > 0xff) {
-            VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but "
-                         "flow_mod_table_id extension is not enabled",
-                         ofproto->name);
+        default:
+            if (fm->command > 0xff) {
+                VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but "
+                             "flow_mod_table_id extension is not enabled",
+                             ofproto->name);
+            }
+            error = OFPERR_OFPFMFC_BAD_COMMAND;
+            break;
         }
-        return OFPERR_OFPFMFC_BAD_COMMAND;
+    } else {
+        ovs_assert(!list_is_empty(&ofproto->pending));
+        error = OFPROTO_POSTPONE;
     }
+
+    run_rule_executes(ofproto);
+    return error;
 }
 
 static enum ofperr
@@ -4288,6 +4415,7 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
                                     struct list *msgs)
 {
     struct ofoperation *op = rule->pending;
+    const struct rule_actions *actions;
     struct ofputil_flow_update fu;
     struct match match;
 
@@ -4309,12 +4437,11 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
     minimatch_expand(&rule->cr.match, &match);
     fu.match = &match;
     fu.priority = rule->cr.priority;
+
     if (!(flags & NXFMF_ACTIONS)) {
-        fu.ofpacts = NULL;
-        fu.ofpacts_len = 0;
+        actions = NULL;
     } else if (!op) {
-        fu.ofpacts = rule->ofpacts;
-        fu.ofpacts_len = rule->ofpacts_len;
+        actions = rule->actions;
     } else {
         /* An operation is in progress.  Use the previous version of the flow's
          * actions, so that when the operation commits we report the change. */
@@ -4324,24 +4451,19 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
 
         case OFOPERATION_MODIFY:
         case OFOPERATION_REPLACE:
-            if (op->ofpacts) {
-                fu.ofpacts = op->ofpacts;
-                fu.ofpacts_len = op->ofpacts_len;
-            } else {
-                fu.ofpacts = rule->ofpacts;
-                fu.ofpacts_len = rule->ofpacts_len;
-            }
+            actions = op->actions ? op->actions : rule->actions;
             break;
 
         case OFOPERATION_DELETE:
-            fu.ofpacts = rule->ofpacts;
-            fu.ofpacts_len = rule->ofpacts_len;
+            actions = rule->actions;
             break;
 
         default:
             NOT_REACHED();
         }
     }
+    fu.ofpacts = actions ? actions->ofpacts : NULL;
+    fu.ofpacts_len = actions ? actions->ofpacts_len : 0;
 
     if (list_is_empty(msgs)) {
         ofputil_start_flow_update(msgs);
@@ -5547,8 +5669,23 @@ ofopgroup_complete(struct ofopgroup *group)
                 error = ofconn_pktbuf_retrieve(group->ofconn, group->buffer_id,
                                                &packet, &in_port);
                 if (packet) {
+                    struct rule_execute *re;
+
                     ovs_assert(!error);
-                    error = rule_execute(op->rule, in_port, packet);
+
+                    ofproto_rule_ref(op->rule);
+
+                    re = xmalloc(sizeof *re);
+                    re->rule = op->rule;
+                    re->in_port = in_port;
+                    re->packet = packet;
+
+                    if (!guarded_list_push_back(&ofproto->rule_executes,
+                                                &re->list_node, 1024)) {
+                        ofproto_rule_unref(op->rule);
+                        ofpbuf_delete(re->packet);
+                        free(re);
+                    }
                 }
                 break;
             }
@@ -5576,7 +5713,7 @@ ofopgroup_complete(struct ofopgroup *group)
         if (!(op->error
               || ofproto_rule_is_hidden(rule)
               || (op->type == OFOPERATION_MODIFY
-                  && op->ofpacts
+                  && op->actions
                   && rule->flow_cookie == op->flow_cookie))) {
             /* Check that we can just cast from ofoperation_type to
              * nx_flow_update_event. */
@@ -5626,13 +5763,13 @@ ofopgroup_complete(struct ofopgroup *group)
             } else {
                 ovs_rwlock_wrlock(&rule->rwlock);
                 oftable_remove_rule(rule);
-                ofproto_rule_destroy(rule);
+                ofproto_rule_unref(rule);
             }
             break;
 
         case OFOPERATION_DELETE:
             ovs_assert(!op->error);
-            ofproto_rule_destroy(rule);
+            ofproto_rule_unref(rule);
             op->rule = NULL;
             break;
 
@@ -5651,16 +5788,16 @@ ofopgroup_complete(struct ofopgroup *group)
                 rule->idle_timeout = op->idle_timeout;
                 rule->hard_timeout = op->hard_timeout;
                 ovs_mutex_unlock(&rule->timeout_mutex);
-                if (op->ofpacts) {
-                    free(rule->ofpacts);
+                if (op->actions) {
+                    struct rule_actions *old_actions;
 
                     ovs_rwlock_wrlock(&rule->rwlock);
-                    rule->ofpacts = op->ofpacts;
-                    rule->ofpacts_len = op->ofpacts_len;
+                    old_actions = rule->actions;
+                    rule->actions = op->actions;
                     ovs_rwlock_unlock(&rule->rwlock);
 
-                    op->ofpacts = NULL;
-                    op->ofpacts_len = 0;
+                    op->actions = NULL;
+                    rule_actions_unref(old_actions);
                 }
                 rule->flags = op->flags;
             }
@@ -5744,7 +5881,7 @@ ofoperation_destroy(struct ofoperation *op)
         hmap_remove(&group->ofproto->deletions, &op->hmap_node);
     }
     list_remove(&op->group_node);
-    free(op->ofpacts);
+    rule_actions_unref(op->actions);
     free(op);
 }
 
@@ -6200,11 +6337,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);
@@ -6237,13 +6374,14 @@ 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);
-    if (rule->meter_id) {
-        struct meter *meter = ofproto->meters[rule->meter_id];
+
+    if (rule->actions->meter_id) {
+        struct meter *meter = ofproto->meters[rule->actions->meter_id];
         list_insert(&meter->rules, &rule->meter_list_node);
     }
     ovs_rwlock_wrlock(&table->cls.rwlock);