ofproto: Do straightforward removal of asynchronous flow operations.
authorBen Pfaff <blp@nicira.com>
Wed, 4 Jun 2014 00:12:46 +0000 (17:12 -0700)
committerBen Pfaff <blp@nicira.com>
Thu, 12 Jun 2014 21:23:27 +0000 (14:23 -0700)
Open vSwitch has supported datapaths that cannot update their flow tables
synchronously for many versions.  In that time, I have talked to many
hardware implementers.  None of them has ever mentioned the asynchronous
interface.  Furthermore, the only public hardware implementation of an Open
vSwitch datapath (from Centec), does not use the asynchronous interface.

At the same time, the asynchronous interface makes ofproto hard to read and
hard to understand.  It also makes it hard to maintain and extend.  An
extension in an upcoming commit would be very difficult to implement
asynchronously.

Therefore, this commit begins to remove the asynchronous interface.  This
initial commit does only the most straightforward parts of the removal, the
ones that do not significantly change the structure of the code.  For
example, this commit does not remove the ofoperation or ofopgroup data
structures at the core of the asynchronous interface, but instead reduces
them to a vestigial form: where previously an ofoperation might span
multiple trips through the main loop (if the operation were truly
asynchronous), now it always completes immediately.

The following commit will do more structural changes.  It will also update
all the comments, which are mostly left alone here.

The hope is that this structuring of the asynchronous removal into two
stages will make it easier to understand and review.  If not, the commits
could be squashed.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
ofproto/connmgr.c
ofproto/connmgr.h
ofproto/in-band.c
ofproto/ofproto-dpif.c
ofproto/ofproto-provider.h
ofproto/ofproto.c

index e0a097f..b53b9f0 100644 (file)
@@ -76,11 +76,6 @@ struct ofconn {
     enum ofputil_protocol protocol; /* Current protocol variant. */
     enum nx_packet_in_format packet_in_format; /* OFPT_PACKET_IN format. */
 
-    /* Asynchronous flow table operation support. */
-    struct list opgroups;       /* Contains pending "ofopgroups", if any. */
-    struct ofpbuf *blocked;     /* Postponed OpenFlow message, if any. */
-    bool retry;                 /* True if 'blocked' is ready to try again. */
-
     /* OFPT_PACKET_IN related data. */
     struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */
 #define N_SCHEDULERS 2
@@ -153,9 +148,9 @@ static void ofconn_reconfigure(struct ofconn *,
                                const struct ofproto_controller *);
 
 static void ofconn_run(struct ofconn *,
-                       bool (*handle_openflow)(struct ofconn *,
+                       void (*handle_openflow)(struct ofconn *,
                                                const struct ofpbuf *ofp_msg));
-static void ofconn_wait(struct ofconn *, bool handling_openflow);
+static void ofconn_wait(struct ofconn *);
 
 static void ofconn_log_flow_mods(struct ofconn *);
 
@@ -304,23 +299,13 @@ connmgr_destroy(struct connmgr *mgr)
     free(mgr);
 }
 
-/* Does all of the periodic maintenance required by 'mgr'.
- *
- * If 'handle_openflow' is nonnull, calls 'handle_openflow' for each message
- * received on an OpenFlow connection, passing along the OpenFlow connection
- * itself and the message that was sent.  If 'handle_openflow' returns true,
- * the message is considered to be fully processed.  If 'handle_openflow'
- * returns false, the message is considered not to have been processed at all;
- * it will be stored and re-presented to 'handle_openflow' following the next
- * call to connmgr_retry().  'handle_openflow' must not modify or free the
- * message.
- *
- * If 'handle_openflow' is NULL, no OpenFlow messages will be processed and
- * other activities that could affect the flow table (in-band processing,
- * fail-open processing) are suppressed too. */
+/* Does all of the periodic maintenance required by 'mgr'.  Calls
+ * 'handle_openflow' for each message received on an OpenFlow connection,
+ * passing along the OpenFlow connection itself and the message that was sent.
+ * 'handle_openflow' must not modify or free the message. */
 void
 connmgr_run(struct connmgr *mgr,
-            bool (*handle_openflow)(struct ofconn *,
+            void (*handle_openflow)(struct ofconn *,
                                     const struct ofpbuf *ofp_msg))
     OVS_EXCLUDED(ofproto_mutex)
 {
@@ -328,7 +313,7 @@ connmgr_run(struct connmgr *mgr,
     struct ofservice *ofservice;
     size_t i;
 
-    if (handle_openflow && mgr->in_band) {
+    if (mgr->in_band) {
         if (!in_band_run(mgr->in_band)) {
             in_band_destroy(mgr->in_band);
             mgr->in_band = NULL;
@@ -342,7 +327,7 @@ connmgr_run(struct connmgr *mgr,
 
     /* Fail-open maintenance.  Do this after processing the ofconns since
      * fail-open checks the status of the controller rconn. */
-    if (handle_openflow && mgr->fail_open) {
+    if (mgr->fail_open) {
         fail_open_run(mgr->fail_open);
     }
 
@@ -387,26 +372,22 @@ connmgr_run(struct connmgr *mgr,
     }
 }
 
-/* Causes the poll loop to wake up when connmgr_run() needs to run.
- *
- * If 'handling_openflow' is true, arriving OpenFlow messages and other
- * activities that affect the flow table will wake up the poll loop.  If
- * 'handling_openflow' is false, they will not. */
+/* Causes the poll loop to wake up when connmgr_run() needs to run. */
 void
-connmgr_wait(struct connmgr *mgr, bool handling_openflow)
+connmgr_wait(struct connmgr *mgr)
 {
     struct ofservice *ofservice;
     struct ofconn *ofconn;
     size_t i;
 
     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
-        ofconn_wait(ofconn, handling_openflow);
+        ofconn_wait(ofconn);
     }
     ofmonitor_wait(mgr);
-    if (handling_openflow && mgr->in_band) {
+    if (mgr->in_band) {
         in_band_wait(mgr->in_band);
     }
-    if (handling_openflow && mgr->fail_open) {
+    if (mgr->fail_open) {
         fail_open_wait(mgr->fail_open);
     }
     HMAP_FOR_EACH (ofservice, node, &mgr->services) {
@@ -447,19 +428,6 @@ ofconn_get_ofproto(const struct ofconn *ofconn)
 {
     return ofconn->connmgr->ofproto;
 }
-
-/* If processing of OpenFlow messages was blocked on any 'mgr' ofconns by
- * returning false to the 'handle_openflow' callback to connmgr_run(), this
- * re-enables them. */
-void
-connmgr_retry(struct connmgr *mgr)
-{
-    struct ofconn *ofconn;
-
-    LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
-        ofconn->retry = true;
-    }
-}
 \f
 /* OpenFlow configuration. */
 
@@ -1162,28 +1130,6 @@ ofconn_report_flow_mod(struct ofconn *ofconn,
     ofconn->last_op = now;
 }
 
-/* Returns true if 'ofconn' has any pending opgroups. */
-bool
-ofconn_has_pending_opgroups(const struct ofconn *ofconn)
-{
-    return !list_is_empty(&ofconn->opgroups);
-}
-
-/* Adds 'ofconn_node' to 'ofconn''s list of pending opgroups.
- *
- * If 'ofconn' is destroyed or its connection drops, then 'ofconn' will remove
- * 'ofconn_node' from the list and re-initialize it with list_init().  The
- * client may, therefore, use list_is_empty(ofconn_node) to determine whether
- * 'ofconn_node' is still associated with an active ofconn.
- *
- * The client may also remove ofconn_node from the list itself, with
- * list_remove(). */
-void
-ofconn_add_opgroup(struct ofconn *ofconn, struct list *ofconn_node)
-{
-    list_push_back(&ofconn->opgroups, ofconn_node);
-}
-
 struct hmap *
 ofconn_get_bundles(struct ofconn *ofconn)
 {
@@ -1212,8 +1158,6 @@ ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type,
     ofconn->type = type;
     ofconn->enable_async_msgs = enable_async_msgs;
 
-    list_init(&ofconn->opgroups);
-
     hmap_init(&ofconn->monitors);
     list_init(&ofconn->updates);
 
@@ -1239,18 +1183,6 @@ ofconn_flush(struct ofconn *ofconn)
     ofconn_set_protocol(ofconn, OFPUTIL_P_NONE);
     ofconn->packet_in_format = NXPIF_OPENFLOW10;
 
-    /* Disassociate 'ofconn' from all of the ofopgroups that it initiated that
-     * have not yet completed.  (Those ofopgroups will still run to completion
-     * in the usual way, but any errors that they run into will not be reported
-     * on any OpenFlow channel.)
-     *
-     * Also discard any blocked operation on 'ofconn'. */
-    while (!list_is_empty(&ofconn->opgroups)) {
-        list_init(list_pop_front(&ofconn->opgroups));
-    }
-    ofpbuf_delete(ofconn->blocked);
-    ofconn->blocked = NULL;
-
     rconn_packet_counter_destroy(ofconn->packet_in_counter);
     ofconn->packet_in_counter = rconn_packet_counter_create();
     for (i = 0; i < N_SCHEDULERS; i++) {
@@ -1368,12 +1300,12 @@ static bool
 ofconn_may_recv(const struct ofconn *ofconn)
 {
     int count = rconn_packet_counter_n_packets(ofconn->reply_counter);
-    return (!ofconn->blocked || ofconn->retry) && count < OFCONN_REPLY_MAX;
+    return count < OFCONN_REPLY_MAX;
 }
 
 static void
 ofconn_run(struct ofconn *ofconn,
-           bool (*handle_openflow)(struct ofconn *,
+           void (*handle_openflow)(struct ofconn *,
                                    const struct ofpbuf *ofp_msg))
 {
     struct connmgr *mgr = ofconn->connmgr;
@@ -1388,31 +1320,20 @@ ofconn_run(struct ofconn *ofconn,
 
     rconn_run(ofconn->rconn);
 
-    if (handle_openflow) {
-        /* Limit the number of iterations to avoid starving other tasks. */
-        for (i = 0; i < 50 && ofconn_may_recv(ofconn); i++) {
-            struct ofpbuf *of_msg;
-
-            of_msg = (ofconn->blocked
-                      ? ofconn->blocked
-                      : rconn_recv(ofconn->rconn));
-            if (!of_msg) {
-                break;
-            }
-            if (mgr->fail_open) {
-                fail_open_maybe_recover(mgr->fail_open);
-            }
+    /* Limit the number of iterations to avoid starving other tasks. */
+    for (i = 0; i < 50 && ofconn_may_recv(ofconn); i++) {
+        struct ofpbuf *of_msg = rconn_recv(ofconn->rconn);
+        if (!of_msg) {
+            break;
+        }
 
-            if (handle_openflow(ofconn, of_msg)) {
-                ofpbuf_delete(of_msg);
-                ofconn->blocked = NULL;
-            } else {
-                ofconn->blocked = of_msg;
-                ofconn->retry = false;
-            }
+        if (mgr->fail_open) {
+            fail_open_maybe_recover(mgr->fail_open);
         }
-    }
 
+        handle_openflow(ofconn, of_msg);
+        ofpbuf_delete(of_msg);
+    }
 
     if (time_msec() >= ofconn->next_op_report) {
         ofconn_log_flow_mods(ofconn);
@@ -1428,7 +1349,7 @@ ofconn_run(struct ofconn *ofconn,
 }
 
 static void
-ofconn_wait(struct ofconn *ofconn, bool handling_openflow)
+ofconn_wait(struct ofconn *ofconn)
 {
     int i;
 
@@ -1436,7 +1357,7 @@ ofconn_wait(struct ofconn *ofconn, bool handling_openflow)
         pinsched_wait(ofconn->schedulers[i]);
     }
     rconn_run_wait(ofconn->rconn);
-    if (handling_openflow && ofconn_may_recv(ofconn)) {
+    if (ofconn_may_recv(ofconn)) {
         rconn_recv_wait(ofconn->rconn);
     }
     if (ofconn->next_op_report != LLONG_MAX) {
@@ -2189,7 +2110,6 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
         HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) {
             if (m->flags & update
                 && (m->table_id == 0xff || m->table_id == rule->table_id)
-                && ofoperation_has_out_port(rule->pending, m->out_port)
                 && cls_rule_is_loose_match(&rule->cr, &m->match)) {
                 flags |= m->flags;
             }
index b6fc90b..75a4c06 100644 (file)
@@ -29,7 +29,6 @@
 
 struct nlattr;
 struct ofconn;
-struct ofopgroup;
 struct rule;
 struct simap;
 struct sset;
@@ -96,9 +95,9 @@ struct connmgr *connmgr_create(struct ofproto *ofproto,
 void connmgr_destroy(struct connmgr *);
 
 void connmgr_run(struct connmgr *,
-                 bool (*handle_openflow)(struct ofconn *,
+                 void (*handle_openflow)(struct ofconn *,
                                          const struct ofpbuf *ofp_msg));
-void connmgr_wait(struct connmgr *, bool handling_openflow);
+void connmgr_wait(struct connmgr *);
 
 void connmgr_get_memory_usage(const struct connmgr *, struct simap *usage);
 
@@ -156,9 +155,6 @@ void ofconn_send_error(const struct ofconn *, const struct ofp_header *request,
 enum ofperr ofconn_pktbuf_retrieve(struct ofconn *, uint32_t id,
                                    struct ofpbuf **bufferp, ofp_port_t *in_port);
 
-bool ofconn_has_pending_opgroups(const struct ofconn *);
-void ofconn_add_opgroup(struct ofconn *, struct list *);
-
 struct hmap *ofconn_get_bundles(struct ofconn *ofconn);
 
 /* Logging flow_mod summaries. */
index 42b3efb..cbe0d1c 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -386,13 +386,9 @@ in_band_run(struct in_band *ib)
             break;
 
         case DEL:
-            if (ofproto_delete_flow(ib->ofproto,
-                                    &rule->match, rule->priority)) {
-                /* ofproto doesn't have the rule anymore so there's no reason
-                 * for us to track it any longer. */
-                hmap_remove(&ib->rules, &rule->hmap_node);
-                free(rule);
-            }
+            ofproto_delete_flow(ib->ofproto, &rule->match, rule->priority);
+            hmap_remove(&ib->rules, &rule->hmap_node);
+            free(rule);
             break;
         }
     }
index 69eee4b..e6d88bc 100644 (file)
@@ -3053,8 +3053,6 @@ rule_expire(struct rule_dpif *rule)
     long long int now = time_msec();
     int reason = -1;
 
-    ovs_assert(!rule->up.pending);
-
     hard_timeout = rule->up.hard_timeout;
     idle_timeout = rule->up.idle_timeout;
 
@@ -3437,7 +3435,6 @@ complete_operation(struct rule_dpif *rule)
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
 
     ofproto->backer->need_revalidate = REV_FLOW_TABLE;
-    ofoperation_complete(rule->up.pending, 0);
 }
 
 static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
@@ -3473,12 +3470,13 @@ rule_construct(struct rule *rule_)
     return 0;
 }
 
-static void
+static enum ofperr
 rule_insert(struct rule *rule_)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
     complete_operation(rule);
+    return 0;
 }
 
 static void
index 97e5cdd..d61fc7c 100644 (file)
@@ -54,6 +54,7 @@ struct match;
 struct ofputil_flow_mod;
 struct bfd_cfg;
 struct meter;
+struct ofoperation;
 
 extern struct ovs_mutex ofproto_mutex;
 
@@ -109,21 +110,9 @@ struct ofproto {
     /* OpenFlow connections. */
     struct connmgr *connmgr;
 
-    /* Flow table operation tracking.
-     *
-     * 'state' is meaningful only within ofproto.c, one of the enum
-     * ofproto_state constants defined there.
-     *
-     * 'pending' is the list of "struct ofopgroup"s currently pending.
-     *
-     * 'n_pending' is the number of elements in 'pending'.
-     *
-     * 'deletions' contains pending ofoperations of type OFOPERATION_DELETE,
-     * indexed on its rule's flow.*/
+    /* 'meaningful only within ofproto.c, one of the enum ofproto_state
+     * constants defined there. */
     int state;
-    struct list pending OVS_GUARDED_BY(ofproto_mutex);
-    unsigned int n_pending OVS_GUARDED_BY(ofproto_mutex);
-    struct hmap deletions OVS_GUARDED_BY(ofproto_mutex);
 
     /* Delayed rule executions.
      *
@@ -350,9 +339,6 @@ struct rule {
      * reference. */
     struct ovs_refcount ref_count;
 
-    /* Operation now in progress, if nonnull. */
-    struct ofoperation *pending OVS_GUARDED_BY(ofproto_mutex);
-
     /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with
      * a flow.. */
     ovs_be64 flow_cookie OVS_GUARDED;
@@ -470,9 +456,6 @@ void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
 
 void ofoperation_complete(struct ofoperation *, enum ofperr);
 
-bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port)
-    OVS_REQUIRES(ofproto_mutex);
-
 /* A group within a "struct ofproto".
  *
  * With few exceptions, ofproto implementations may look at these fields but
@@ -1254,7 +1237,8 @@ struct ofproto_class {
     struct rule *(*rule_alloc)(void);
     enum ofperr (*rule_construct)(struct rule *rule)
         /* OVS_REQUIRES(ofproto_mutex) */;
-    void (*rule_insert)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
+    enum ofperr (*rule_insert)(struct rule *rule)
+        /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_destruct)(struct rule *rule);
     void (*rule_dealloc)(struct rule *rule);
@@ -1702,25 +1686,13 @@ extern const struct ofproto_class ofproto_dpif_class;
 int ofproto_class_register(const struct ofproto_class *);
 int ofproto_class_unregister(const struct ofproto_class *);
 
-/* ofproto_flow_mod() returns this value if the flow_mod could not be processed
- * because it overlaps with an ongoing flow table operation that has not yet
- * completed.  The caller should retry the operation later.
- *
- * ofproto.c also uses this value internally for additional (similar) purposes.
- *
- * This particular value is a good choice because it is large, so that it does
- * not collide with any errno value, but not large enough to collide with an
- * OFPERR_* value. */
-enum { OFPROTO_POSTPONE = 1 << 16 };
-BUILD_ASSERT_DECL(OFPROTO_POSTPONE < OFPERR_OFS);
-
 int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *)
     OVS_EXCLUDED(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *,
                       unsigned int priority,
                       const struct ofpact *ofpacts, size_t ofpacts_len)
     OVS_EXCLUDED(ofproto_mutex);
-bool ofproto_delete_flow(struct ofproto *,
+void ofproto_delete_flow(struct ofproto *,
                          const struct match *, unsigned int priority)
     OVS_EXCLUDED(ofproto_mutex);
 void ofproto_flush_flows(struct ofproto *);
index 44b92f4..be4a35d 100644 (file)
@@ -97,9 +97,7 @@ enum ofoperation_type {
  * assigned to groups but will not have an associated ofconn. */
 struct ofopgroup {
     struct ofproto *ofproto;    /* Owning ofproto. */
-    struct list ofproto_node;   /* In ofproto's "pending" list. */
     struct list ops;            /* List of "struct ofoperation"s. */
-    int n_running;              /* Number of ops still pending. */
 
     /* Data needed to send OpenFlow reply on failure or to send a buffered
      * packet on success.
@@ -309,7 +307,7 @@ static bool ofproto_group_exists(const struct ofproto *ofproto,
                                  uint32_t group_id)
     OVS_EXCLUDED(ofproto->groups_rwlock);
 static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *);
-static bool handle_openflow(struct ofconn *, const struct ofpbuf *);
+static void handle_openflow(struct ofconn *, const struct ofpbuf *);
 static enum ofperr handle_flow_mod__(struct ofproto *,
                                      struct ofputil_flow_mod *,
                                      const struct flow_mod_requester *)
@@ -550,10 +548,6 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     hindex_init(&ofproto->cookies);
     list_init(&ofproto->expirable);
     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->vlan_bitmap = NULL;
     ofproto->vlans_changed = false;
@@ -1251,15 +1245,12 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops)
 }
 
 static void
-ofproto_rule_delete__(struct ofproto *ofproto, struct rule *rule,
-                      uint8_t reason)
+ofproto_rule_delete__(struct rule *rule, uint8_t reason)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct ofopgroup *group;
 
-    ovs_assert(!rule->pending);
-
-    group = ofopgroup_create_unattached(ofproto);
+    group = ofopgroup_create_unattached(rule->ofproto);
     delete_flow__(rule, group, reason);
     ofopgroup_submit(group);
 }
@@ -1278,14 +1269,14 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
     OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofopgroup *group;
+    struct ofoperation *op;
 
     ovs_mutex_lock(&ofproto_mutex);
-    ovs_assert(!rule->pending);
-
     group = ofopgroup_create_unattached(ofproto);
-    ofoperation_create(group, rule, OFOPERATION_DELETE, OFPRR_DELETE);
+    op = ofoperation_create(group, rule, OFOPERATION_DELETE, OFPRR_DELETE);
     oftable_remove_rule__(ofproto, rule);
     ofproto->ofproto_class->rule_delete(rule);
+    ofoperation_complete(op, 0);
     ofopgroup_submit(group);
 
     ovs_mutex_unlock(&ofproto_mutex);
@@ -1314,9 +1305,7 @@ ofproto_flush__(struct ofproto *ofproto)
         cls_cursor_init(&cursor, &table->cls, NULL);
         fat_rwlock_unlock(&table->cls.rwlock);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
-            if (!rule->pending) {
-                ofproto_rule_delete__(ofproto, rule, OFPRR_DELETE);
-            }
+            ofproto_rule_delete__(rule, OFPRR_DELETE);
         }
     }
     ovs_mutex_unlock(&ofproto_mutex);
@@ -1330,8 +1319,6 @@ ofproto_destroy__(struct ofproto *ofproto)
 {
     struct oftable *table;
 
-    ovs_assert(list_is_empty(&ofproto->pending));
-
     destroy_rule_executes(ofproto);
     delete_group(ofproto, OFPG_ALL);
 
@@ -1359,8 +1346,6 @@ ofproto_destroy__(struct ofproto *ofproto)
     }
     free(ofproto->tables);
 
-    hmap_destroy(&ofproto->deletions);
-
     ovs_assert(hindex_is_empty(&ofproto->cookies));
     hindex_destroy(&ofproto->cookies);
 
@@ -1458,19 +1443,6 @@ ofproto_type_wait(const char *datapath_type)
     }
 }
 
-static bool
-any_pending_ops(const struct ofproto *p)
-    OVS_EXCLUDED(ofproto_mutex)
-{
-    bool b;
-
-    ovs_mutex_lock(&ofproto_mutex);
-    b = !list_is_empty(&p->pending);
-    ovs_mutex_unlock(&ofproto_mutex);
-
-    return b;
-}
-
 int
 ofproto_run(struct ofproto *p)
 {
@@ -1568,18 +1540,14 @@ ofproto_run(struct ofproto *p)
     case S_EVICT:
         connmgr_run(p->connmgr, NULL);
         ofproto_evict(p);
-        if (!any_pending_ops(p)) {
-            p->state = S_OPENFLOW;
-        }
+        p->state = S_OPENFLOW;
         break;
 
     case S_FLUSH:
         connmgr_run(p->connmgr, NULL);
         ofproto_flush__(p);
-        if (!any_pending_ops(p)) {
-            connmgr_flushed(p->connmgr);
-            p->state = S_OPENFLOW;
-        }
+        connmgr_flushed(p->connmgr);
+        p->state = S_OPENFLOW;
         break;
 
     default:
@@ -1600,15 +1568,13 @@ ofproto_wait(struct ofproto *p)
 
     switch (p->state) {
     case S_OPENFLOW:
-        connmgr_wait(p->connmgr, true);
+        connmgr_wait(p->connmgr);
         break;
 
     case S_EVICT:
     case S_FLUSH:
-        connmgr_wait(p->connmgr, false);
-        if (!any_pending_ops(p)) {
-            poll_immediate_wake();
-        }
+        connmgr_wait(p->connmgr);
+        poll_immediate_wake();
         break;
     }
 }
@@ -1629,11 +1595,6 @@ ofproto_get_memory_usage(const struct ofproto *ofproto, struct simap *usage)
 
     simap_increase(usage, "ports", hmap_count(&ofproto->ports));
 
-    ovs_mutex_lock(&ofproto_mutex);
-    simap_increase(usage, "ops",
-                   ofproto->n_pending + hmap_count(&ofproto->deletions));
-    ovs_mutex_unlock(&ofproto_mutex);
-
     n_rules = 0;
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
         fat_rwlock_rdlock(&table->cls.rwlock);
@@ -2006,7 +1967,7 @@ ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
  * ofproto's table 0 and, if it finds one, deletes it.
  *
  * This is a helper function for in-band control and fail-open. */
-bool
+void
 ofproto_delete_flow(struct ofproto *ofproto,
                     const struct match *target, unsigned int priority)
     OVS_EXCLUDED(ofproto_mutex)
@@ -2020,15 +1981,13 @@ ofproto_delete_flow(struct ofproto *ofproto,
     rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target,
                                                             priority));
     fat_rwlock_unlock(&cls->rwlock);
-    if (!rule) {
-        return true;
+    if (rule) {
+        /* Execute 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.  */
+        simple_flow_mod(ofproto, target, priority, NULL, 0,
+                        OFPFC_DELETE_STRICT);
     }
-
-    /* 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.  */
-    return simple_flow_mod(ofproto, target, priority, NULL, 0,
-                           OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE;
 }
 
 /* Starts the process of deleting all of the flows from all of ofproto's flow
@@ -2707,30 +2666,6 @@ ofproto_rule_has_out_group(const struct rule *rule, uint32_t group_id)
     }
 }
 
-/* Returns true if a rule related to 'op' has an OpenFlow OFPAT_OUTPUT or
- * OFPAT_ENQUEUE action that outputs to 'out_port'. */
-bool
-ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    if (ofproto_rule_has_out_port(op->rule, out_port)) {
-        return true;
-    }
-
-    switch (op->type) {
-    case OFOPERATION_ADD:
-    case OFOPERATION_DELETE:
-        return false;
-
-    case OFOPERATION_MODIFY:
-    case OFOPERATION_REPLACE:
-        return ofpacts_output_to_port(op->actions->ofpacts,
-                                      op->actions->ofpacts_len, out_port);
-    }
-
-    OVS_NOT_REACHED();
-}
-
 static void
 rule_execute_destroy(struct rule_execute *e)
 {
@@ -3452,11 +3387,8 @@ rule_collection_destroy(struct rule_collection *rules)
  * check 'c->cr' itself.
  *
  * Increments '*n_readonly' if 'rule' wasn't added because it's read-only (and
- * 'c' only includes modifiable rules).
- *
- * Returns 0 ordinarily, but OFPROTO_POSTPONE if we would otherwise collect a
- * rule that has a pending operation. */
-static enum ofperr
+ * 'c' only includes modifiable rules). */
+static void
 collect_rule(struct rule *rule, const struct rule_criteria *c,
              struct rule_collection *rules, size_t *n_readonly)
     OVS_REQUIRES(ofproto_mutex)
@@ -3466,10 +3398,6 @@ collect_rule(struct rule *rule, const struct rule_criteria *c,
         && ofproto_rule_has_out_group(rule, c->out_group)
         && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask)
         && (!rule_is_hidden(rule) || c->include_hidden)) {
-        if (rule->pending) {
-            return OFPROTO_POSTPONE;
-        }
-
         /* Rule matches all the criteria... */
         if (rule_is_modifiable(rule, 0) || c->include_readonly) {
             /* ...add it. */
@@ -3479,7 +3407,6 @@ collect_rule(struct rule *rule, const struct rule_criteria *c,
             ++*n_readonly;
         }
     }
-    return 0;
 }
 
 /* Searches 'ofproto' for rules that match the criteria in 'criteria'.  Matches
@@ -3512,10 +3439,7 @@ collect_rules_loose(struct ofproto *ofproto,
                                    hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
             if (cls_rule_is_loose_match(&rule->cr, &criteria->cr.match)) {
-                error = collect_rule(rule, criteria, rules, &n_readonly);
-                if (error) {
-                    break;
-                }
+                collect_rule(rule, criteria, rules, &n_readonly);
             }
         }
     } else {
@@ -3526,10 +3450,7 @@ collect_rules_loose(struct ofproto *ofproto,
             fat_rwlock_rdlock(&table->cls.rwlock);
             cls_cursor_init(&cursor, &table->cls, &criteria->cr);
             CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-                error = collect_rule(rule, criteria, rules, &n_readonly);
-                if (error) {
-                    break;
-                }
+                collect_rule(rule, criteria, rules, &n_readonly);
             }
             fat_rwlock_unlock(&table->cls.rwlock);
         }
@@ -3577,10 +3498,7 @@ collect_rules_strict(struct ofproto *ofproto,
                                    hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
             if (cls_rule_equal(&rule->cr, &criteria->cr)) {
-                error = collect_rule(rule, criteria, rules, &n_readonly);
-                if (error) {
-                    break;
-                }
+                collect_rule(rule, criteria, rules, &n_readonly);
             }
         }
     } else {
@@ -3592,15 +3510,17 @@ collect_rules_strict(struct ofproto *ofproto,
                                           &table->cls, &criteria->cr));
             fat_rwlock_unlock(&table->cls.rwlock);
             if (rule) {
-                error = collect_rule(rule, criteria, rules, &n_readonly);
-                if (error) {
-                    break;
-                }
+                collect_rule(rule, criteria, rules, &n_readonly);
             }
         }
     }
 
 exit:
+    if (!error && !rules->n && n_readonly) {
+        /* We didn't find any rules to modify.  We did find some read-only
+         * rules that we're not allowed to modify, so report that. */
+        error = OFPERR_OFPBRC_EPERM;
+    }
     if (error) {
         rule_collection_destroy(rules);
     }
@@ -3945,28 +3865,6 @@ handle_queue_stats_request(struct ofconn *ofconn,
     return error;
 }
 
-static bool
-is_flow_deletion_pending(const struct ofproto *ofproto,
-                         const struct cls_rule *cls_rule,
-                         uint8_t table_id)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    if (!hmap_is_empty(&ofproto->deletions)) {
-        struct ofoperation *op;
-
-        HMAP_FOR_EACH_WITH_HASH (op, hmap_node,
-                                 cls_rule_hash(cls_rule, table_id),
-                                 &ofproto->deletions) {
-            if (op->rule->table_id == table_id
-                && cls_rule_equal(cls_rule, &op->rule->cr)) {
-                return true;
-            }
-        }
-    }
-
-    return false;
-}
-
 static bool
 should_evict_a_rule(struct oftable *table, unsigned int extra_space)
     OVS_REQUIRES(ofproto_mutex)
@@ -3985,8 +3883,6 @@ evict_rules_from_table(struct ofproto *ofproto, struct oftable *table,
 
         if (!choose_rule_to_evict(table, &rule)) {
             return OFPERR_OFPFMFC_TABLE_FULL;
-        } else if (rule->pending) {
-            return OFPROTO_POSTPONE;
         } else {
             struct ofopgroup *group = ofopgroup_create_unattached(ofproto);
             delete_flow__(rule, group, OFPRR_EVICTION);
@@ -4016,6 +3912,7 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 {
     const struct rule_actions *actions;
     struct ofopgroup *group;
+    struct ofoperation *op;
     struct oftable *table;
     struct cls_rule cr;
     struct rule *rule;
@@ -4070,8 +3967,6 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
         cls_rule_destroy(&cr);
         if (!rule_is_modifiable(rule, fm->flags)) {
             return OFPERR_OFPBRC_EPERM;
-        } else if (rule->pending) {
-            return OFPROTO_POSTPONE;
         } else {
             struct rule_collection rules;
 
@@ -4085,12 +3980,6 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
         }
     }
 
-    /* Serialize against pending deletion. */
-    if (is_flow_deletion_pending(ofproto, &cr, table_id)) {
-        cls_rule_destroy(&cr);
-        return OFPROTO_POSTPONE;
-    }
-
     /* Check for overlap, if requested. */
     if (fm->flags & OFPUTIL_FF_CHECK_OVERLAP) {
         bool overlaps;
@@ -4125,7 +4014,6 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
     *CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto;
     cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), &cr);
     ovs_refcount_init(&rule->ref_count);
-    rule->pending = NULL;
     rule->flow_cookie = fm->new_cookie;
     rule->created = rule->modified = time_msec();
 
@@ -4167,8 +4055,9 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
     fat_rwlock_unlock(&table->cls.rwlock);
 
     group = ofopgroup_create(ofproto, fm->buffer_id, req);
-    ofoperation_create(group, rule, OFOPERATION_ADD, 0);
-    ofproto->ofproto_class->rule_insert(rule);
+    op = ofoperation_create(group, rule, OFOPERATION_ADD, 0);
+    error = ofproto->ofproto_class->rule_insert(rule);
+    ofoperation_complete(op, error);
     ofopgroup_submit(group);
 
     return error;
@@ -4255,9 +4144,8 @@ modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
             ovsrcu_set(&rule->actions, new_actions);
 
             ofproto->ofproto_class->rule_modify_actions(rule, reset_counters);
-        } else {
-            ofoperation_complete(op, 0);
         }
+        ofoperation_complete(op, 0);
     }
     ofopgroup_submit(group);
 
@@ -4470,12 +4358,10 @@ void
 ofproto_rule_expire(struct rule *rule, uint8_t reason)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct ofproto *ofproto = rule->ofproto;
-
     ovs_assert(reason == OFPRR_HARD_TIMEOUT || reason == OFPRR_IDLE_TIMEOUT
                || reason == OFPRR_DELETE || reason == OFPRR_GROUP_DELETE);
 
-    ofproto_rule_delete__(ofproto, rule, reason);
+    ofproto_rule_delete__(rule, reason);
 }
 
 /* Reduces '*timeout' to no more than 'max'.  A value of zero in either case
@@ -4615,11 +4501,6 @@ handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
     }
 
     if (request.role != OFPCR12_ROLE_NOCHANGE) {
-        if (ofconn_get_role(ofconn) != request.role
-            && ofconn_has_pending_opgroups(ofconn)) {
-            return OFPROTO_POSTPONE;
-        }
-
         if (request.have_generation_id
             && !ofconn_set_master_election_id(ofconn, request.generation_id)) {
                 return OFPERR_OFPRRFC_STALE;
@@ -4665,12 +4546,8 @@ handle_nxt_set_flow_format(struct ofconn *ofconn, const struct ofp_header *oh)
 
     cur = ofconn_get_protocol(ofconn);
     next = ofputil_protocol_set_base(cur, next_base);
-    if (cur != next && ofconn_has_pending_opgroups(ofconn)) {
-        /* Avoid sending async messages in surprising protocol. */
-        return OFPROTO_POSTPONE;
-    }
-
     ofconn_set_protocol(ofconn, next);
+
     return 0;
 }
 
@@ -4686,12 +4563,6 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn,
         return OFPERR_OFPBRC_EPERM;
     }
 
-    if (format != ofconn_get_packet_in_format(ofconn)
-        && ofconn_has_pending_opgroups(ofconn)) {
-        /* Avoid sending async message in surprsing packet in format. */
-        return OFPROTO_POSTPONE;
-    }
-
     ofconn_set_packet_in_format(ofconn, format);
     return 0;
 }
@@ -4764,10 +4635,6 @@ handle_barrier_request(struct ofconn *ofconn, const struct ofp_header *oh)
 {
     struct ofpbuf *buf;
 
-    if (ofconn_has_pending_opgroups(ofconn)) {
-        return OFPROTO_POSTPONE;
-    }
-
     buf = ofpraw_alloc_reply((oh->version == OFP10_VERSION
                               ? OFPRAW_OFPT10_BARRIER_REPLY
                               : OFPRAW_OFPT11_BARRIER_REPLY), oh, 0);
@@ -4781,17 +4648,10 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
                                     struct list *msgs)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct ofoperation *op = rule->pending;
     const struct rule_actions *actions;
     struct ofputil_flow_update fu;
     struct match match;
 
-    if (op && op->type == OFOPERATION_ADD) {
-        /* We'll report the final flow when the operation completes.  Reporting
-         * it now would cause a duplicate report later. */
-        return;
-    }
-
     fu.event = (flags & (NXFMF_INITIAL | NXFMF_ADD)
                 ? NXFME_ADDED : NXFME_MODIFIED);
     fu.reason = 0;
@@ -4805,30 +4665,7 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
     fu.match = &match;
     fu.priority = rule->cr.priority;
 
-    if (!(flags & NXFMF_ACTIONS)) {
-        actions = NULL;
-    } else if (!op) {
-        actions = rule_get_actions(rule);
-    } 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. */
-        switch (op->type) {
-        case OFOPERATION_ADD:
-            OVS_NOT_REACHED();
-
-        case OFOPERATION_MODIFY:
-        case OFOPERATION_REPLACE:
-            actions = op->actions ? op->actions : rule_get_actions(rule);
-            break;
-
-        case OFOPERATION_DELETE:
-            actions = rule_get_actions(rule);
-            break;
-
-        default:
-            OVS_NOT_REACHED();
-        }
-    }
+    actions = flags & NXFMF_ACTIONS ? rule_get_actions(rule) : NULL;
     fu.ofpacts = actions ? actions->ofpacts : NULL;
     fu.ofpacts_len = actions ? actions->ofpacts_len : 0;
 
@@ -4866,9 +4703,7 @@ ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m,
         return;
     }
 
-    if (!(rule->pending
-          ? ofoperation_has_out_port(rule->pending, m->out_port)
-          : ofproto_rule_has_out_port(rule, m->out_port))) {
+    if (!ofproto_rule_has_out_port(rule, m->out_port)) {
         return;
     }
 
@@ -4901,7 +4736,6 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m,
     OVS_REQUIRES(ofproto_mutex)
 {
     const struct ofproto *ofproto = ofconn_get_ofproto(m->ofconn);
-    const struct ofoperation *op;
     const struct oftable *table;
     struct cls_rule target;
 
@@ -4913,22 +4747,10 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m,
         fat_rwlock_rdlock(&table->cls.rwlock);
         cls_cursor_init(&cursor, &table->cls, &target);
         CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-            ovs_assert(!rule->pending); /* XXX */
             ofproto_collect_ofmonitor_refresh_rule(m, rule, seqno, rules);
         }
         fat_rwlock_unlock(&table->cls.rwlock);
     }
-
-    HMAP_FOR_EACH (op, hmap_node, &ofproto->deletions) {
-        struct rule *rule = op->rule;
-
-        if (((m->table_id == 0xff
-              ? !(ofproto->tables[rule->table_id].flags & OFTABLE_HIDDEN)
-              : m->table_id == rule->table_id))
-            && cls_rule_is_loose_match(&rule->cr, &target.match)) {
-            ofproto_collect_ofmonitor_refresh_rule(m, rule, seqno, rules);
-        }
-    }
     cls_rule_destroy(&target);
 }
 
@@ -5211,10 +5033,6 @@ handle_delete_meter(struct ofconn *ofconn, struct ofputil_meter_mod *mm)
             struct rule *rule;
 
             LIST_FOR_EACH (rule, meter_list_node, &meter->rules) {
-                if (rule->pending) {
-                    error = OFPROTO_POSTPONE;
-                    goto exit;
-                }
                 rule_collection_add(&rules, rule);
             }
         }
@@ -5226,7 +5044,6 @@ handle_delete_meter(struct ofconn *ofconn, struct ofputil_meter_mod *mm)
     /* Delete the meters. */
     meter_delete(ofproto, first, last);
 
-exit:
     ovs_mutex_unlock(&ofproto_mutex);
     rule_collection_destroy(&rules);
 
@@ -6145,16 +5962,15 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     }
 }
 
-static bool
+static void
 handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg)
     OVS_EXCLUDED(ofproto_mutex)
 {
     int error = handle_openflow__(ofconn, ofp_msg);
-    if (error && error != OFPROTO_POSTPONE) {
+    if (error) {
         ofconn_send_error(ofconn, ofpbuf_data(ofp_msg), error);
     }
     COVERAGE_INC(ofproto_recv_openflow);
-    return error != OFPROTO_POSTPONE;
 }
 \f
 /* Asynchronous operations. */
@@ -6170,7 +5986,6 @@ ofopgroup_create_unattached(struct ofproto *ofproto)
 {
     struct ofopgroup *group = xzalloc(sizeof *group);
     group->ofproto = ofproto;
-    list_init(&group->ofproto_node);
     list_init(&group->ops);
     list_init(&group->ofconn_node);
     return group;
@@ -6199,7 +6014,6 @@ ofopgroup_create(struct ofproto *ofproto, uint32_t buffer_id,
 
         ovs_assert(ofconn_get_ofproto(req->ofconn) == ofproto);
 
-        ofconn_add_opgroup(req->ofconn, &group->ofconn_node);
         group->ofconn = req->ofconn;
         group->request = xmemdup(req->request, MIN(request_len, 64));
         group->buffer_id = buffer_id;
@@ -6217,12 +6031,7 @@ static void
 ofopgroup_submit(struct ofopgroup *group)
     OVS_REQUIRES(ofproto_mutex)
 {
-    if (!group->n_running) {
-        ofopgroup_complete(group);
-    } else {
-        list_push_back(&group->ofproto->pending, &group->ofproto_node);
-        group->ofproto->n_pending++;
-    }
+    ofopgroup_complete(group);
 }
 
 static void
@@ -6237,8 +6046,6 @@ ofopgroup_complete(struct ofopgroup *group)
     struct ofoperation *op, *next_op;
     int error;
 
-    ovs_assert(!group->n_running);
-
     error = 0;
     LIST_FOR_EACH (op, group_node, &group->ops) {
         if (op->error) {
@@ -6326,8 +6133,6 @@ ofopgroup_complete(struct ofopgroup *group)
                              op->reason, abbrev_ofconn, abbrev_xid);
         }
 
-        rule->pending = NULL;
-
         ovs_assert(!op->error || op->type == OFOPERATION_ADD);
         switch (op->type) {
         case OFOPERATION_ADD:
@@ -6379,17 +6184,8 @@ ofopgroup_complete(struct ofopgroup *group)
 
     ofmonitor_flush(ofproto->connmgr);
 
-    if (!list_is_empty(&group->ofproto_node)) {
-        ovs_assert(ofproto->n_pending > 0);
-        ofproto->n_pending--;
-        list_remove(&group->ofproto_node);
-    }
-    if (!list_is_empty(&group->ofconn_node)) {
-        list_remove(&group->ofconn_node);
-        if (error) {
-            ofconn_send_error(group->ofconn, group->request, error);
-        }
-        connmgr_retry(ofproto->connmgr);
+    if (error) {
+        ofconn_send_error(group->ofconn, group->request, error);
     }
     free(group->request);
     free(group);
@@ -6409,12 +6205,9 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule,
                    enum ofp_flow_removed_reason reason)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct ofproto *ofproto = group->ofproto;
     struct ofoperation *op;
 
-    ovs_assert(!rule->pending);
-
-    op = rule->pending = xzalloc(sizeof *op);
+    op = xzalloc(sizeof *op);
     op->group = group;
     list_push_back(&group->ops, &op->group_node);
     op->rule = rule;
@@ -6427,13 +6220,6 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule,
     ovs_mutex_unlock(&rule->mutex);
     op->flags = rule->flags;
 
-    group->n_running++;
-
-    if (type == OFOPERATION_DELETE) {
-        hmap_insert(&ofproto->deletions, &op->hmap_node,
-                    cls_rule_hash(&rule->cr, rule->table_id));
-    }
-
     return op;
 }
 
@@ -6441,15 +6227,6 @@ static void
 ofoperation_destroy(struct ofoperation *op)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct ofopgroup *group = op->group;
-
-    if (op->rule) {
-        op->rule->pending = NULL;
-    }
-    if (op->type == OFOPERATION_DELETE) {
-        hmap_remove(&group->ofproto->deletions, &op->hmap_node);
-    }
-    list_remove(&op->group_node);
     rule_actions_destroy(op->actions);
     free(op);
 }
@@ -6472,22 +6249,8 @@ ofoperation_destroy(struct ofoperation *op)
 void
 ofoperation_complete(struct ofoperation *op, enum ofperr error)
 {
-    struct ofopgroup *group = op->group;
-
-    ovs_assert(group->n_running > 0);
     ovs_assert(!error || op->type == OFOPERATION_ADD);
-
     op->error = error;
-    if (!--group->n_running && !list_is_empty(&group->ofproto_node)) {
-        /* This function can be called from ->rule_construct(), in which case
-         * ofproto_mutex is held, or it can be called from ->run(), in which
-         * case ofproto_mutex is not held.  But only in the latter case can we
-         * arrive here, so we can safely take ofproto_mutex now. */
-        ovs_mutex_lock(&ofproto_mutex);
-        ovs_assert(op->rule->pending == op);
-        ofopgroup_complete(group);
-        ovs_mutex_unlock(&ofproto_mutex);
-    }
 }
 \f
 static uint64_t