guarded-list: New data structure for thread-safe queue.
[cascardo/ovs.git] / ofproto / ofproto-dpif.c
index 229b16c..96cbd7b 100644 (file)
@@ -31,6 +31,7 @@
 #include "dpif.h"
 #include "dynamic-string.h"
 #include "fail-open.h"
+#include "guarded-list.h"
 #include "hmapx.h"
 #include "lacp.h"
 #include "learn.h"
@@ -75,8 +76,6 @@ COVERAGE_DEFINE(subfacet_install_fail);
 COVERAGE_DEFINE(packet_in_overflow);
 COVERAGE_DEFINE(flow_mod_overflow);
 
-#define N_THREADS 16
-
 /* Number of implemented OpenFlow tables. */
 enum { N_TABLES = 255 };
 enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
@@ -85,7 +84,29 @@ BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
 struct flow_miss;
 struct facet;
 
+struct rule_dpif {
+    struct rule up;
+
+    /* These statistics:
+     *
+     *   - Do include packets and bytes from facets that have been deleted or
+     *     whose own statistics have been folded into the rule.
+     *
+     *   - Do include packets and bytes sent "by hand" that were accounted to
+     *     the rule without any facet being involved (this is a rare corner
+     *     case in rule_execute()).
+     *
+     *   - Do not include packet or bytes that can be obtained from any facet's
+     *     packet_count or byte_count member or that can be obtained from the
+     *     datapath by, e.g., dpif_flow_get() for any subfacet.
+     */
+    struct ovs_mutex stats_mutex;
+    uint64_t packet_count OVS_GUARDED;  /* Number of packets received. */
+    uint64_t byte_count OVS_GUARDED;    /* Number of bytes received. */
+};
+
 static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t *bytes);
+static struct rule_dpif *rule_dpif_cast(const struct rule *);
 
 struct ofbundle {
     struct hmap_node hmap_node; /* In struct ofproto's "bundles" hmap. */
@@ -431,6 +452,9 @@ struct dpif_backer {
     /* Number of subfacets added or deleted from 'created' to 'last_minute.' */
     unsigned long long int total_subfacet_add_count;
     unsigned long long int total_subfacet_del_count;
+
+    /* Number of upcall handling threads. */
+    unsigned int n_handler_threads;
 };
 
 /* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -462,9 +486,6 @@ struct ofproto_dpif {
     struct classifier facets;     /* Contains 'struct facet's. */
     long long int consistency_rl;
 
-    /* Support for debugging async flow mods. */
-    struct list completions;
-
     struct netdev_stats stats; /* To account packets generated and consumed in
                                 * userspace. */
 
@@ -488,19 +509,10 @@ struct ofproto_dpif {
     uint64_t n_missed;
 
     /* Work queues. */
-    struct ovs_mutex flow_mod_mutex;
-    struct list flow_mods OVS_GUARDED;
-    size_t n_flow_mods OVS_GUARDED;
-
-    struct ovs_mutex pin_mutex;
-    struct list pins OVS_GUARDED;
-    size_t n_pins OVS_GUARDED;
+    struct guarded_list flow_mods; /* Contains "struct flow_mod"s. */
+    struct guarded_list pins;      /* Contains "struct ofputil_packet_in"s. */
 };
 
-/* Defer flow mod completion until "ovs-appctl ofproto/unclog"?  (Useful only
- * for debugging the asynchronous flow_mod implementation.) */
-static bool clogged;
-
 /* By default, flows in the datapath are wildcarded (megaflows).  They
  * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
 static bool enable_megaflows = true;
@@ -545,36 +557,24 @@ void
 ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
                       struct ofputil_flow_mod *fm)
 {
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    if (ofproto->n_flow_mods > 1024) {
-        ovs_mutex_unlock(&ofproto->flow_mod_mutex);
+    if (!guarded_list_push_back(&ofproto->flow_mods, &fm->list_node, 1024)) {
         COVERAGE_INC(flow_mod_overflow);
         free(fm->ofpacts);
         free(fm);
-        return;
     }
-
-    list_push_back(&ofproto->flow_mods, &fm->list_node);
-    ofproto->n_flow_mods++;
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
 }
 
+/* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
+ * Takes ownership of 'pin' and pin->packet. */
 void
 ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto,
                             struct ofputil_packet_in *pin)
 {
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    if (ofproto->n_pins > 1024) {
-        ovs_mutex_unlock(&ofproto->pin_mutex);
+    if (!guarded_list_push_back(&ofproto->pins, &pin->list_node, 1024)) {
         COVERAGE_INC(packet_in_overflow);
         free(CONST_CAST(void *, pin->packet));
         free(pin);
-        return;
     }
-
-    list_push_back(&ofproto->pins, &pin->list_node);
-    ofproto->n_pins++;
-    ovs_mutex_unlock(&ofproto->pin_mutex);
 }
 \f
 /* Factory functions. */
@@ -700,11 +700,20 @@ type_run(const char *type)
             VLOG_ERR("Failed to enable receiving packets in dpif.");
             return error;
         }
-        udpif_recv_set(backer->udpif, N_THREADS, backer->recv_set_enable);
+        udpif_recv_set(backer->udpif, n_handler_threads,
+                       backer->recv_set_enable);
         dpif_flow_flush(backer->dpif);
         backer->need_revalidate = REV_RECONFIGURE;
     }
 
+    /* If the n_handler_threads is reconfigured, call udpif_recv_set()
+     * to reset the handler threads. */
+    if (backer->n_handler_threads != n_handler_threads) {
+        udpif_recv_set(backer->udpif, n_handler_threads,
+                       backer->recv_set_enable);
+        backer->n_handler_threads = n_handler_threads;
+    }
+
     if (backer->need_revalidate) {
         struct ofproto_dpif *ofproto;
         struct simap_node *node;
@@ -810,7 +819,7 @@ type_run(const char *type)
             HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
                 int stp_port = ofport->stp_port
                     ? stp_port_no(ofport->stp_port)
-                    : 0;
+                    : -1;
                 xlate_ofport_set(ofproto, ofport->bundle, ofport,
                                  ofport->up.ofp_port, ofport->odp_port,
                                  ofport->up.netdev, ofport->cfm,
@@ -998,7 +1007,6 @@ process_dpif_port_error(struct dpif_backer *backer, int error)
 static int
 dpif_backer_run_fast(struct dpif_backer *backer)
 {
-    udpif_run(backer->udpif);
     handle_upcalls(backer);
 
     return 0;
@@ -1051,6 +1059,8 @@ type_wait(const char *type)
     }
 
     timer_wait(&backer->next_expiration);
+    dpif_wait(backer->dpif);
+    udpif_wait(backer->udpif);
 }
 \f
 /* Basic life-cycle. */
@@ -1209,7 +1219,9 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
         close_dpif_backer(backer);
         return error;
     }
-    udpif_recv_set(backer->udpif, N_THREADS, backer->recv_set_enable);
+    udpif_recv_set(backer->udpif, n_handler_threads,
+                   backer->recv_set_enable);
+    backer->n_handler_threads = n_handler_threads;
 
     backer->max_n_subfacet = 0;
     backer->created = time_msec();
@@ -1232,7 +1244,7 @@ construct(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct shash_node *node, *next;
-    odp_port_t max_ports;
+    uint32_t max_ports;
     int error;
 
     error = open_dpif_backer(ofproto->up.type, &ofproto->backer);
@@ -1241,8 +1253,7 @@ construct(struct ofproto *ofproto_)
     }
 
     max_ports = dpif_get_max_ports(ofproto->backer->dpif);
-    ofproto_init_max_ports(ofproto_, u16_to_ofp(MIN(odp_to_u32(max_ports),
-                                                    ofp_to_u16(OFPP_MAX))));
+    ofproto_init_max_ports(ofproto_, MIN(max_ports, ofp_to_u16(OFPP_MAX)));
 
     ofproto->netflow = NULL;
     ofproto->sflow = NULL;
@@ -1252,24 +1263,13 @@ construct(struct ofproto *ofproto_)
     ofproto->ml = mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME);
     ofproto->mbridge = mbridge_create();
     ofproto->has_bonded_bundles = false;
-    ovs_mutex_init(&ofproto->vsp_mutex, PTHREAD_MUTEX_NORMAL);
+    ovs_mutex_init(&ofproto->vsp_mutex);
 
     classifier_init(&ofproto->facets);
     ofproto->consistency_rl = LLONG_MIN;
 
-    list_init(&ofproto->completions);
-
-    ovs_mutex_init(&ofproto->flow_mod_mutex, PTHREAD_MUTEX_NORMAL);
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    list_init(&ofproto->flow_mods);
-    ofproto->n_flow_mods = 0;
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-
-    ovs_mutex_init(&ofproto->pin_mutex, PTHREAD_MUTEX_NORMAL);
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    list_init(&ofproto->pins);
-    ofproto->n_pins = 0;
-    ovs_mutex_unlock(&ofproto->pin_mutex);
+    guarded_list_init(&ofproto->flow_mods);
+    guarded_list_init(&ofproto->pins);
 
     ofproto_dpif_unixctl_init();
 
@@ -1344,7 +1344,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
 
     if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL,
                                   rulep)) {
-        ovs_rwlock_unlock(&(*rulep)->up.evict);
+        rule_dpif_unref(*rulep);
     } else {
         NOT_REACHED();
     }
@@ -1387,34 +1387,35 @@ add_internal_flows(struct ofproto_dpif *ofproto)
     return error;
 }
 
-static void
-complete_operations(struct ofproto_dpif *ofproto)
-{
-    struct dpif_completion *c, *next;
-
-    LIST_FOR_EACH_SAFE (c, next, list_node, &ofproto->completions) {
-        ofoperation_complete(c->op, 0);
-        list_remove(&c->list_node);
-        free(c);
-    }
-}
-
 static void
 destruct(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct rule_dpif *rule, *next_rule;
-    struct ofputil_flow_mod *pin, *next_pin;
+    struct ofputil_packet_in *pin, *next_pin;
     struct ofputil_flow_mod *fm, *next_fm;
+    struct facet *facet, *next_facet;
+    struct list flow_mods, pins;
+    struct cls_cursor cursor;
     struct oftable *table;
 
+    ovs_rwlock_rdlock(&ofproto->facets.rwlock);
+    cls_cursor_init(&cursor, &ofproto->facets, NULL);
+    ovs_rwlock_unlock(&ofproto->facets.rwlock);
+    CLS_CURSOR_FOR_EACH_SAFE (facet, next_facet, cr, &cursor) {
+        facet_remove(facet);
+    }
+
     ofproto->backer->need_revalidate = REV_RECONFIGURE;
     ovs_rwlock_wrlock(&xlate_rwlock);
     xlate_remove_ofproto(ofproto);
     ovs_rwlock_unlock(&xlate_rwlock);
 
+    /* Discard any flow_miss_batches queued up for 'ofproto', avoiding a
+     * use-after-free error. */
+    udpif_revalidate(ofproto->backer->udpif);
+
     hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
-    complete_operations(ofproto);
 
     OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
         struct cls_cursor cursor;
@@ -1422,30 +1423,26 @@ destruct(struct ofproto *ofproto_)
         ovs_rwlock_wrlock(&table->cls.rwlock);
         cls_cursor_init(&cursor, &table->cls, NULL);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, up.cr, &cursor) {
-            ofproto_rule_destroy(&ofproto->up, &table->cls, &rule->up);
+            ofproto_rule_delete(&ofproto->up, &table->cls, &rule->up);
         }
         ovs_rwlock_unlock(&table->cls.rwlock);
     }
 
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &ofproto->flow_mods) {
+    guarded_list_pop_all(&ofproto->flow_mods, &flow_mods);
+    LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &flow_mods) {
         list_remove(&fm->list_node);
-        ofproto->n_flow_mods--;
         free(fm->ofpacts);
         free(fm);
     }
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-    ovs_mutex_destroy(&ofproto->flow_mod_mutex);
+    guarded_list_destroy(&ofproto->flow_mods);
 
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &ofproto->pins) {
+    guarded_list_pop_all(&ofproto->pins, &pins);
+    LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         list_remove(&pin->list_node);
-        ofproto->n_pins--;
-        free(pin->ofpacts);
+        free(CONST_CAST(void *, pin->packet));
         free(pin);
     }
-    ovs_mutex_unlock(&ofproto->pin_mutex);
-    ovs_mutex_destroy(&ofproto->pin_mutex);
+    guarded_list_destroy(&ofproto->pins);
 
     mbridge_unref(ofproto->mbridge);
 
@@ -1483,17 +1480,7 @@ run_fast(struct ofproto *ofproto_)
         return 0;
     }
 
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    if (ofproto->n_flow_mods) {
-        flow_mods = ofproto->flow_mods;
-        list_moved(&flow_mods);
-        list_init(&ofproto->flow_mods);
-        ofproto->n_flow_mods = 0;
-    } else {
-        list_init(&flow_mods);
-    }
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-
+    guarded_list_pop_all(&ofproto->flow_mods, &flow_mods);
     LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &flow_mods) {
         int error = ofproto_flow_mod(&ofproto->up, fm);
         if (error && !VLOG_DROP_WARN(&rl)) {
@@ -1506,17 +1493,7 @@ run_fast(struct ofproto *ofproto_)
         free(fm);
     }
 
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    if (ofproto->n_pins) {
-        pins = ofproto->pins;
-        list_moved(&pins);
-        list_init(&ofproto->pins);
-        ofproto->n_pins = 0;
-    } else {
-        list_init(&pins);
-    }
-    ovs_mutex_unlock(&ofproto->pin_mutex);
-
+    guarded_list_pop_all(&ofproto->pins, &pins);
     LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         connmgr_send_packet_in(ofproto->up.connmgr, pin);
         list_remove(&pin->list_node);
@@ -1539,10 +1516,6 @@ run(struct ofproto *ofproto_)
     struct ofbundle *bundle;
     int error;
 
-    if (!clogged) {
-        complete_operations(ofproto);
-    }
-
     if (mbridge_need_revalidate(ofproto->mbridge)) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
         ovs_rwlock_wrlock(&ofproto->ml->rwlock);
@@ -1569,6 +1542,9 @@ run(struct ofproto *ofproto_)
     if (ofproto->sflow) {
         dpif_sflow_run(ofproto->sflow);
     }
+    if (ofproto->ipfix) {
+        dpif_ipfix_run(ofproto->ipfix);
+    }
 
     HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
         port_run(ofport);
@@ -1617,19 +1593,16 @@ wait(struct ofproto *ofproto_)
     struct ofport_dpif *ofport;
     struct ofbundle *bundle;
 
-    if (!clogged && !list_is_empty(&ofproto->completions)) {
-        poll_immediate_wake();
-    }
-
     if (ofproto_get_flow_restore_wait()) {
         return;
     }
 
-    dpif_wait(ofproto->backer->dpif);
-    udpif_wait(ofproto->backer->udpif);
     if (ofproto->sflow) {
         dpif_sflow_wait(ofproto->sflow);
     }
+    if (ofproto->ipfix) {
+        dpif_ipfix_wait(ofproto->ipfix);
+    }
     HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
         port_wait(ofport);
     }
@@ -1891,6 +1864,10 @@ port_modified(struct ofport *port_)
         cfm_set_netdev(port->cfm, port->up.netdev);
     }
 
+    if (port->bfd) {
+        bfd_set_netdev(port->bfd, port->up.netdev);
+    }
+
     if (port->is_tunnel && tnl_port_reconfigure(port, port->up.netdev,
                                                 port->odp_port)) {
         ofproto_dpif_cast(port->up.ofproto)->backer->need_revalidate =
@@ -1955,20 +1932,25 @@ set_ipfix(
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct dpif_ipfix *di = ofproto->ipfix;
+    bool has_options = bridge_exporter_options || flow_exporters_options;
 
-    if (bridge_exporter_options || flow_exporters_options) {
-        if (!di) {
-            di = ofproto->ipfix = dpif_ipfix_create();
-        }
+    if (has_options && !di) {
+        di = ofproto->ipfix = dpif_ipfix_create();
+    }
+
+    if (di) {
+        /* Call set_options in any case to cleanly flush the flow
+         * caches in the last exporters that are to be destroyed. */
         dpif_ipfix_set_options(
             di, bridge_exporter_options, flow_exporters_options,
             n_flow_exporters_options);
-    } else {
-        if (di) {
+
+        if (!has_options) {
             dpif_ipfix_unref(di);
             ofproto->ipfix = NULL;
         }
     }
+
     return 0;
 }
 
@@ -2025,7 +2007,8 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
     struct bfd *old;
 
     old = ofport->bfd;
-    ofport->bfd = bfd_configure(old, netdev_get_name(ofport->up.netdev), cfg);
+    ofport->bfd = bfd_configure(old, netdev_get_name(ofport->up.netdev),
+                                cfg, ofport->up.netdev);
     if (ofport->bfd != old) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
     }
@@ -2676,35 +2659,39 @@ static void
 bundle_send_learning_packets(struct ofbundle *bundle)
 {
     struct ofproto_dpif *ofproto = bundle->ofproto;
+    struct ofpbuf *learning_packet;
     int error, n_packets, n_errors;
     struct mac_entry *e;
+    struct list packets;
 
-    error = n_packets = n_errors = 0;
+    list_init(&packets);
     ovs_rwlock_rdlock(&ofproto->ml->rwlock);
     LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
         if (e->port.p != bundle) {
-            struct ofpbuf *learning_packet;
-            struct ofport_dpif *port;
             void *port_void;
-            int ret;
 
-            /* The assignment to "port" is unnecessary but makes "grep"ing for
-             * struct ofport_dpif more effective. */
             learning_packet = bond_compose_learning_packet(bundle->bond,
                                                            e->mac, e->vlan,
                                                            &port_void);
-            port = port_void;
-            ret = send_packet(port, learning_packet);
-            ofpbuf_delete(learning_packet);
-            if (ret) {
-                error = ret;
-                n_errors++;
-            }
-            n_packets++;
+            learning_packet->private_p = port_void;
+            list_push_back(&packets, &learning_packet->list_node);
         }
     }
     ovs_rwlock_unlock(&ofproto->ml->rwlock);
 
+    error = n_packets = n_errors = 0;
+    LIST_FOR_EACH (learning_packet, list_node, &packets) {
+        int ret;
+
+        ret = send_packet(learning_packet->private_p, learning_packet);
+        if (ret) {
+            error = ret;
+            n_errors++;
+        }
+        n_packets++;
+    }
+    ofpbuf_list_delete(&packets);
+
     if (n_errors) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_WARN_RL(&rl, "bond %s: %d errors sending %d gratuitous learning "
@@ -2938,6 +2925,8 @@ port_run(struct ofport_dpif *ofport)
     long long int carrier_seq = netdev_get_carrier_resets(ofport->up.netdev);
     bool carrier_changed = carrier_seq != ofport->carrier_seq;
     bool enable = netdev_get_carrier(ofport->up.netdev);
+    bool cfm_enable = false;
+    bool bfd_enable = false;
 
     ofport->carrier_seq = carrier_seq;
 
@@ -2947,16 +2936,20 @@ port_run(struct ofport_dpif *ofport)
         int cfm_opup = cfm_get_opup(ofport->cfm);
 
         cfm_run(ofport->cfm);
-        enable = enable && !cfm_get_fault(ofport->cfm);
+        cfm_enable = !cfm_get_fault(ofport->cfm);
 
         if (cfm_opup >= 0) {
-            enable = enable && cfm_opup;
+            cfm_enable = cfm_enable && cfm_opup;
         }
     }
 
     if (ofport->bfd) {
         bfd_run(ofport->bfd);
-        enable = enable && bfd_forwarding(ofport->bfd);
+        bfd_enable = bfd_forwarding(ofport->bfd);
+    }
+
+    if (ofport->bfd || ofport->cfm) {
+        enable = enable && (cfm_enable || bfd_enable);
     }
 
     if (ofport->bundle) {
@@ -3073,7 +3066,7 @@ port_del(struct ofproto *ofproto_, ofp_port_t ofp_port)
     sset_find_and_delete(&ofproto->ghost_ports,
                          netdev_get_name(ofport->up.netdev));
     ofproto->backer->need_revalidate = REV_RECONFIGURE;
-    if (!ofport->is_tunnel) {
+    if (!ofport->is_tunnel && !netdev_vport_is_patch(ofport->up.netdev)) {
         error = dpif_port_del(ofproto->backer->dpif, ofport->odp_port);
         if (!error) {
             /* The caller is going to close ofport->up.netdev.  If this is a
@@ -3304,7 +3297,6 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
     facet->byte_count += miss->stats.n_bytes;
     facet->prev_byte_count += miss->stats.n_bytes;
 
-    subfacet = subfacet_create(facet, miss);
     want_path = facet->xout.slow ? SF_SLOW_PATH : SF_FAST_PATH;
 
     /* Don't install the flow if it's the result of the "userspace"
@@ -3921,10 +3913,7 @@ rule_expire(struct rule_dpif *rule)
     long long int now;
     uint8_t reason;
 
-    if (rule->up.pending) {
-        /* We'll have to expire it later. */
-        return;
-    }
+    ovs_assert(!rule->up.pending);
 
     ovs_mutex_lock(&rule->up.timeout_mutex);
     hard_timeout = rule->up.hard_timeout;
@@ -3941,12 +3930,8 @@ rule_expire(struct rule_dpif *rule)
         return;
     }
 
-    if (!ovs_rwlock_trywrlock(&rule->up.evict)) {
-        COVERAGE_INC(ofproto_dpif_expired);
-
-        /* Get rid of the rule. */
-        ofproto_rule_expire(&rule->up, reason);
-    }
+    COVERAGE_INC(ofproto_dpif_expired);
+    ofproto_rule_expire(&rule->up, reason);
 }
 \f
 /* Facets. */
@@ -4143,12 +4128,13 @@ facet_is_controller_flow(struct facet *facet)
         bool is_controller;
 
         rule_dpif_lookup(ofproto, &facet->flow, NULL, &rule);
-        ofpacts_len = rule->up.ofpacts_len;
-        ofpacts = rule->up.ofpacts;
+        ofpacts_len = rule->up.actions->ofpacts_len;
+        ofpacts = rule->up.actions->ofpacts;
         is_controller = ofpacts_len > 0
             && ofpacts->type == OFPACT_CONTROLLER
             && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len);
-        rule_release(rule);
+        rule_dpif_unref(rule);
+
         return is_controller;
     }
     return false;
@@ -4242,7 +4228,7 @@ facet_check_consistency(struct facet *facet)
     rule_dpif_lookup(facet->ofproto, &facet->flow, NULL, &rule);
     xlate_in_init(&xin, facet->ofproto, &facet->flow, rule, 0, NULL);
     xlate_actions(&xin, &xout);
-    rule_release(rule);
+    rule_dpif_unref(rule);
 
     ok = ofpbuf_equal(&facet->xout.odp_actions, &xout.odp_actions)
         && facet->xout.slow == xout.slow;
@@ -4340,7 +4326,7 @@ facet_revalidate(struct facet *facet)
         || memcmp(&facet->xout.wc, &xout.wc, sizeof xout.wc)) {
         facet_remove(facet);
         xlate_out_uninit(&xout);
-        rule_release(new_rule);
+        rule_dpif_unref(new_rule);
         return false;
     }
 
@@ -4372,7 +4358,7 @@ facet_revalidate(struct facet *facet)
     facet->used = MAX(facet->used, new_rule->up.created);
 
     xlate_out_uninit(&xout);
-    rule_release(new_rule);
+    rule_dpif_unref(new_rule);
     return true;
 }
 
@@ -4400,12 +4386,12 @@ flow_push_stats(struct ofproto_dpif *ofproto, struct flow *flow,
     }
 
     rule_dpif_lookup(ofproto, flow, NULL, &rule);
-    rule_credit_stats(rule, stats);
+    rule_dpif_credit_stats(rule, stats);
     xlate_in_init(&xin, ofproto, flow, rule, stats->tcp_flags, NULL);
     xin.resubmit_stats = stats;
     xin.may_learn = may_learn;
     xlate_actions_for_side_effects(&xin);
-    rule_release(rule);
+    rule_dpif_unref(rule);
 }
 
 static void
@@ -4471,14 +4457,50 @@ push_all_stats(void)
 }
 
 void
-rule_credit_stats(struct rule_dpif *rule, const struct dpif_flow_stats *stats)
+rule_dpif_credit_stats(struct rule_dpif *rule,
+                       const struct dpif_flow_stats *stats)
 {
     ovs_mutex_lock(&rule->stats_mutex);
     rule->packet_count += stats->n_packets;
     rule->byte_count += stats->n_bytes;
-    ofproto_rule_update_used(&rule->up, stats->used);
+    rule->up.used = MAX(rule->up.used, stats->used);
     ovs_mutex_unlock(&rule->stats_mutex);
 }
+
+bool
+rule_dpif_fail_open(const struct rule_dpif *rule)
+{
+    return rule->up.cr.priority == FAIL_OPEN_PRIORITY;
+}
+
+ovs_be64
+rule_dpif_get_flow_cookie(const struct rule_dpif *rule)
+{
+    return rule->up.flow_cookie;
+}
+
+void
+rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
+                     uint16_t hard_timeout)
+{
+    ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout);
+}
+
+/* Returns 'rule''s actions.  The caller owns a reference on the returned
+ * actions and must eventually release it (with rule_actions_unref()) to avoid
+ * a memory leak. */
+struct rule_actions *
+rule_dpif_get_actions(const struct rule_dpif *rule)
+{
+    struct rule_actions *actions;
+
+    ovs_rwlock_rdlock(&rule->up.rwlock);
+    actions = rule->up.actions;
+    rule_actions_ref(actions);
+    ovs_rwlock_unlock(&rule->up.rwlock);
+
+    return actions;
+}
 \f
 /* Subfacets. */
 
@@ -4747,16 +4769,14 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
                      flow->in_port.ofp_port);
     }
 
-    *rule = choose_miss_rule(port ? port->up.pp.config : 0, ofproto->miss_rule,
-                             ofproto->no_packet_in_rule);
-    ovs_rwlock_rdlock(&(*rule)->up.evict);
+    choose_miss_rule(port ? port->up.pp.config : 0, ofproto->miss_rule,
+                     ofproto->no_packet_in_rule, rule);
 }
 
 bool
 rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
                           const struct flow *flow, struct flow_wildcards *wc,
                           uint8_t table_id, struct rule_dpif **rule)
-    OVS_ACQ_RDLOCK((*rule)->up.evict)
 {
     struct cls_rule *cls_rule;
     struct classifier *cls;
@@ -4791,11 +4811,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
     }
 
     *rule = rule_dpif_cast(rule_from_cls_rule(cls_rule));
-    if (*rule && ovs_rwlock_tryrdlock(&(*rule)->up.evict)) {
-        /* The rule is in the process of being removed.  Best we can do is
-         * pretend it isn't there. */
-        *rule = NULL;
-    }
+    rule_dpif_ref(*rule);
     ovs_rwlock_unlock(&cls->rwlock);
 
     return *rule != NULL;
@@ -4804,18 +4820,27 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
 /* Given a port configuration (specified as zero if there's no port), chooses
  * which of 'miss_rule' and 'no_packet_in_rule' should be used in case of a
  * flow table miss. */
-struct rule_dpif *
+void
 choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule,
-                 struct rule_dpif *no_packet_in_rule)
+                 struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule)
+{
+    *rule = config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule;
+    rule_dpif_ref(*rule);
+}
+
+void
+rule_dpif_ref(struct rule_dpif *rule)
 {
-    return config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule;
+    if (rule) {
+        ofproto_rule_ref(&rule->up);
+    }
 }
 
 void
-rule_release(struct rule_dpif *rule)
+rule_dpif_unref(struct rule_dpif *rule)
 {
     if (rule) {
-        ovs_rwlock_unlock(&rule->up.evict);
+        ofproto_rule_unref(&rule->up);
     }
 }
 
@@ -4825,13 +4850,12 @@ complete_operation(struct rule_dpif *rule)
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
 
     ofproto->backer->need_revalidate = REV_FLOW_TABLE;
-    if (clogged) {
-        struct dpif_completion *c = xmalloc(sizeof *c);
-        c->op = rule->up.pending;
-        list_push_back(&ofproto->completions, &c->list_node);
-    } else {
-        ofoperation_complete(rule->up.pending, 0);
-    }
+    ofoperation_complete(rule->up.pending, 0);
+}
+
+static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
+{
+    return rule ? CONTAINER_OF(rule, struct rule_dpif, up) : NULL;
 }
 
 static struct rule *
@@ -4852,20 +4876,32 @@ static enum ofperr
 rule_construct(struct rule *rule_)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
-    ovs_mutex_init(&rule->stats_mutex, PTHREAD_MUTEX_NORMAL);
+    ovs_mutex_init(&rule->stats_mutex);
     ovs_mutex_lock(&rule->stats_mutex);
     rule->packet_count = 0;
     rule->byte_count = 0;
     ovs_mutex_unlock(&rule->stats_mutex);
-    complete_operation(rule);
     return 0;
 }
 
 static void
-rule_destruct(struct rule *rule_)
+rule_insert(struct rule *rule_)
+{
+    struct rule_dpif *rule = rule_dpif_cast(rule_);
+    complete_operation(rule);
+}
+
+static void
+rule_delete(struct rule *rule_)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
     complete_operation(rule);
+}
+
+static void
+rule_destruct(struct rule *rule_)
+{
+    struct rule_dpif *rule = rule_dpif_cast(rule_);
     ovs_mutex_destroy(&rule->stats_mutex);
 }
 
@@ -4898,7 +4934,7 @@ rule_dpif_execute(struct rule_dpif *rule, const struct flow *flow,
     struct xlate_in xin;
 
     dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
-    rule_credit_stats(rule, &stats);
+    rule_dpif_credit_stats(rule, &stats);
 
     xlate_in_init(&xin, ofproto, flow, rule, stats.tcp_flags, packet);
     xin.resubmit_stats = &stats;
@@ -4920,10 +4956,17 @@ rule_execute(struct rule *rule, const struct flow *flow,
 }
 
 static void
-rule_modify_actions(struct rule *rule_)
+rule_modify_actions(struct rule *rule_, bool reset_counters)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
 
+    if (reset_counters) {
+        ovs_mutex_lock(&rule->stats_mutex);
+        rule->packet_count = 0;
+        rule->byte_count = 0;
+        ovs_mutex_unlock(&rule->stats_mutex);
+    }
+
     complete_operation(rule);
 }
 \f
@@ -5241,7 +5284,8 @@ trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
 
     ds_put_char_multiple(result, '\t', level);
     ds_put_cstr(result, "OpenFlow ");
-    ofpacts_format(rule->up.ofpacts, rule->up.ofpacts_len, result);
+    ofpacts_format(rule->up.actions->ofpacts, rule->up.actions->ofpacts_len,
+                   result);
     ds_put_char(result, '\n');
 }
 
@@ -5512,23 +5556,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
         xlate_out_uninit(&trace.xout);
     }
 
-    rule_release(rule);
-}
-
-static void
-ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-                  const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-    clogged = true;
-    unixctl_command_reply(conn, NULL);
-}
-
-static void
-ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-                    const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-    clogged = false;
-    unixctl_command_reply(conn, NULL);
+    rule_dpif_unref(rule);
 }
 
 /* Runs a self-check of flow translations in 'ofproto'.  Appends a message to
@@ -5947,10 +5975,6 @@ ofproto_dpif_unixctl_init(void)
                              ofproto_unixctl_fdb_flush, NULL);
     unixctl_command_register("fdb/show", "bridge", 1, 1,
                              ofproto_unixctl_fdb_show, NULL);
-    unixctl_command_register("ofproto/clog", "", 0, 0,
-                             ofproto_dpif_clog, NULL);
-    unixctl_command_register("ofproto/unclog", "", 0, 0,
-                             ofproto_dpif_unclog, NULL);
     unixctl_command_register("ofproto/self-check", "[bridge]", 0, 1,
                              ofproto_dpif_self_check, NULL);
     unixctl_command_register("dpif/dump-dps", "", 0, 0,
@@ -6308,6 +6332,8 @@ const struct ofproto_class ofproto_dpif_class = {
     NULL,                       /* rule_choose_table */
     rule_alloc,
     rule_construct,
+    rule_insert,
+    rule_delete,
     rule_destruct,
     rule_dealloc,
     rule_get_stats,