[ovs-dev] [PATCH v2 19/19] Use classifier versioning.

Jarno Rajahalme jrajahalme at nicira.com
Mon May 18 23:10:28 UTC 2015


Each rule is now added or deleted in a specific tables version.  Flow
tables are versioned with a monotonically increasing 64-bit integer,
where positive values are valid version numbers.

Rule modifications are implemented as an insertion of a new rule and a
deletion of the old rule, both taking place in the same tables
version.  Since concurrent lookups may use different versions, both
the old and new rule must be available for lookups at the same time.

The ofproto provider interface is changed to accomodate the above.  As
rule's actions need not be modified any more, we no longer need
'rule_premodify_actions', nor 'rule_modify_actions'.  'rule_insert'
now takes a pointer to the old rule and adds a flag that tells whether
the old stats should be forwarded to the new rule or not (this
replaces the 'reset_counters' flag of the now removed
'rule_modify_actions').

Versioning all flow table changes has the side effect of making
learned flows visible for future lookups only.  I.e., the upcall that
executes the learn action, will not see the newly learned action in
it's classifier lookups.  Only upcalls that start executing after the
new flow was added will match on it.

Classifier versioning only affects the classifier lookups.  Classifier
iterators and other control functions operating on the classifier will
always see the latest version of the classifier.

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 NEWS                       |   13 +-
 ofproto/bundles.h          |    5 +-
 ofproto/ofproto-dpif.c     |   87 +++--
 ofproto/ofproto-provider.h |   59 +--
 ofproto/ofproto.c          |  927 ++++++++++++++++++++++++--------------------
 tests/ofproto.at           |   42 +-
 tests/ovs-ofctl.at         |   40 +-
 utilities/ovs-ofctl.8.in   |   11 +-
 utilities/ovs-ofctl.c      |    4 +-
 9 files changed, 629 insertions(+), 559 deletions(-)

diff --git a/NEWS b/NEWS
index ac60451..1c52940 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,10 @@
 Post-v2.3.0
 ---------------------
+   - Flow table modifications are now atomic, meaning that each packet
+     now sees a coherent version of the OpenFlow pipeline.  For
+     example, if a controller removes all flows, no packet sees an
+     intermediate version of the OpenFlow pipeline where only some of
+     the flows have been deleted.
    - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
    - Add bash command-line completion support for ovs-vsctl Please check
      utilities/ovs-command-compgen.INSTALL.md for how to use.
@@ -28,10 +33,10 @@ Post-v2.3.0
      release.  See ovs-vswitchd(8) for details.
    - OpenFlow:
      * OpenFlow 1.4 bundles are now supported, but for flow mod
-       messages only. 'atomic' bundles are not yet supported, and
-       'ordered' bundles are trivially supported, as all bundled
-       messages are executed in the order they were added to the
-       bundle regardless of the presence of the 'ordered' flag.
+       messages only. Both 'atomic' and 'ordered' bundle flags are
+       trivially supported, as all bundled messages are executed in
+       the order they were added and all flow table modifications are
+       now atomic to the datapath.
      * IPv6 flow label and neighbor discovery fields are now modifiable.
      * OpenFlow 1.5 extended registers are now supported.
      * The OpenFlow 1.5 actset_output field is now supported.
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 7dbfae0..c87c556 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -41,9 +41,8 @@ struct ofp_bundle_entry {
     struct ofpbuf ofpacts;
 
     /* Used during commit. */
-    struct rule_collection rules;   /* Affected rules. */
-    struct rule *rule;
-    bool modify;
+    struct rule_collection old_rules;   /* Affected rules. */
+    struct rule_collection new_rules;   /* Affected rules. */
 
     /* OpenFlow header and some of the message contents for error reporting. */
     struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 745fd66..26e6f85 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -89,6 +89,11 @@ struct rule_dpif {
     struct ovs_mutex stats_mutex;
     struct dpif_flow_stats stats OVS_GUARDED;
 
+   /* In non-NULL, will point to a new rule (for which a reference is held) to
+    * which all the stats updates should be forwarded. This exists only
+    * transitionally when flows are replaced. */
+    struct rule_dpif *new_rule OVS_GUARDED;
+
     /* If non-zero then the recirculation id that has
      * been allocated for use with this rule.
      * The recirculation id and associated internal flow should
@@ -3676,9 +3681,13 @@ rule_dpif_credit_stats(struct rule_dpif *rule,
                        const struct dpif_flow_stats *stats)
 {
     ovs_mutex_lock(&rule->stats_mutex);
-    rule->stats.n_packets += stats->n_packets;
-    rule->stats.n_bytes += stats->n_bytes;
-    rule->stats.used = MAX(rule->stats.used, stats->used);
+    if (OVS_UNLIKELY(rule->new_rule)) {
+        rule_dpif_credit_stats(rule->new_rule, stats);
+    } else {
+        rule->stats.n_packets += stats->n_packets;
+        rule->stats.n_bytes += stats->n_bytes;
+        rule->stats.used = MAX(rule->stats.used, stats->used);
+    }
     ovs_mutex_unlock(&rule->stats_mutex);
 }
 
@@ -3730,11 +3739,12 @@ rule_set_recirc_id(struct rule *rule_, uint32_t id)
 }
 
 long long
-ofproto_dpif_get_tables_version(struct ofproto_dpif *ofproto)
+ofproto_dpif_get_tables_version(struct ofproto_dpif *ofproto OVS_UNUSED)
 {
     long long version;
 
     atomic_read_relaxed(&ofproto->tables_version, &version);
+
     return version;
 }
 
@@ -3764,12 +3774,12 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, long long version,
     return rule;
 }
 
-/* Look up 'flow' in 'ofproto''s classifier starting from table '*table_id'.
- * Returns the rule that was found, which may be one of the special rules
- * according to packet miss hadling.  If 'may_packet_in' is false, returning of
- * the miss_rule (which issues packet ins for the controller) is avoided.
- * Updates 'wc', if nonnull, to reflect the fields that were used during the
- * lookup.
+/* Look up 'flow' in 'ofproto''s classifier version 'version', starting from
+ * table '*table_id'.  Returns the rule that was found, which may be one of the
+ * special rules according to packet miss hadling.  If 'may_packet_in' is
+ * false, returning of the miss_rule (which issues packet ins for the
+ * controller) is avoided.  Updates 'wc', if nonnull, to reflect the fields
+ * that were used during the lookup.
  *
  * If 'honor_table_miss' is true, the first lookup occurs in '*table_id', but
  * if none is found then the table miss configuration for that table is
@@ -3935,17 +3945,35 @@ rule_construct(struct rule *rule_)
     rule->stats.n_bytes = 0;
     rule->stats.used = rule->up.modified;
     rule->recirc_id = 0;
+    rule->new_rule = NULL;
 
     return 0;
 }
 
-static enum ofperr
-rule_insert(struct rule *rule_)
+static void
+rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_stats)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
+
+    if (old_rule_ && forward_stats) {
+        struct rule_dpif *old_rule = rule_dpif_cast(old_rule_);
+
+        ovs_assert(!old_rule->new_rule);
+
+        /* Take a reference to the new rule, and refer all stats updates from
+         * the old rule to the new rule. */
+        rule_dpif_ref(rule);
+
+        ovs_mutex_lock(&old_rule->stats_mutex);
+        ovs_mutex_lock(&rule->stats_mutex);
+        old_rule->new_rule = rule;       /* Forward future stats. */
+        rule->stats = old_rule->stats;   /* Transfer stats to the new rule. */
+        ovs_mutex_unlock(&rule->stats_mutex);
+        ovs_mutex_unlock(&old_rule->stats_mutex);
+    }
+
     complete_operation(rule);
-    return 0;
 }
 
 static void
@@ -3958,10 +3986,15 @@ rule_delete(struct rule *rule_)
 
 static void
 rule_destruct(struct rule *rule_)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
 
     ovs_mutex_destroy(&rule->stats_mutex);
+    /* Release reference to the new rule, if any. */
+    if (rule->new_rule) {
+        rule_dpif_unref(rule->new_rule);
+    }
     if (rule->recirc_id) {
         recirc_free_id(rule->recirc_id);
     }
@@ -3974,9 +4007,13 @@ rule_get_stats(struct rule *rule_, uint64_t *packets, uint64_t *bytes,
     struct rule_dpif *rule = rule_dpif_cast(rule_);
 
     ovs_mutex_lock(&rule->stats_mutex);
-    *packets = rule->stats.n_packets;
-    *bytes = rule->stats.n_bytes;
-    *used = rule->stats.used;
+    if (OVS_UNLIKELY(rule->new_rule)) {
+        rule_get_stats(&rule->new_rule->up, packets, bytes, used);
+    } else {
+        *packets = rule->stats.n_packets;
+        *bytes = rule->stats.n_bytes;
+        *used = rule->stats.used;
+    }
     ovs_mutex_unlock(&rule->stats_mutex);
 }
 
@@ -3998,22 +4035,6 @@ rule_execute(struct rule *rule, const struct flow *flow,
     return 0;
 }
 
-static void
-rule_modify_actions(struct rule *rule_, bool reset_counters)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    struct rule_dpif *rule = rule_dpif_cast(rule_);
-
-    if (reset_counters) {
-        ovs_mutex_lock(&rule->stats_mutex);
-        rule->stats.n_packets = 0;
-        rule->stats.n_bytes = 0;
-        ovs_mutex_unlock(&rule->stats_mutex);
-    }
-
-    complete_operation(rule);
-}
-
 static struct group_dpif *group_dpif_cast(const struct ofgroup *group)
 {
     return group ? CONTAINER_OF(group, struct group_dpif, up) : NULL;
@@ -5558,8 +5579,6 @@ const struct ofproto_class ofproto_dpif_class = {
     rule_dealloc,
     rule_get_stats,
     rule_execute,
-    NULL,                       /* rule_premodify_actions */
-    rule_modify_actions,
     set_frag_handling,
     packet_out,
     set_netflow,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 8cbbb9f..3a38eec 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -361,7 +361,7 @@ struct rule {
 
     /* OpenFlow actions.  See struct rule_actions for more thread-safety
      * notes. */
-    OVSRCU_TYPE(const struct rule_actions *) actions;
+    const struct rule_actions * const actions;
 
     /* In owning meter's 'rules' list.  An empty list if there is no meter. */
     struct ovs_list meter_list_node OVS_GUARDED_BY(ofproto_mutex);
@@ -385,6 +385,8 @@ struct rule {
 
     /* Must hold 'mutex' for both read/write, 'ofproto_mutex' not needed. */
     long long int modified OVS_GUARDED; /* Time of last modification. */
+
+    long long version OVS_GUARDED; /* Version in which the rule was added. */
 };
 
 void ofproto_rule_ref(struct rule *);
@@ -405,7 +407,7 @@ static inline bool rule_is_hidden(const struct rule *);
  * code that holds 'rule->mutex' (where 'rule' is the rule for which
  * 'rule->actions == actions') or during the RCU active period.
  *
- * All members are immutable: they do not change during the struct's
+ * All members are immutable: they do not change during the rule's
  * lifetime. */
 struct rule_actions {
     /* Flags.
@@ -1124,7 +1126,7 @@ struct ofproto_class {
      * OpenFlow error code), the ofproto base code will uninitialize and
      * deallocate 'rule'.  See "Rule Life Cycle" above for more details.
      *
-     * ->rule_construct() may also:
+     * ->rule_construct() must also:
      *
      *   - Validate that the datapath supports the matching rule in 'rule->cr'
      *     datapath.  For example, if the rule's table does not support
@@ -1133,8 +1135,9 @@ struct ofproto_class {
      *
      *   - Validate that the datapath can correctly implement 'rule->ofpacts'.
      *
-     * Some implementations might need to defer these tasks to ->rule_insert(),
-     * which is also acceptable.
+     * After a successful construction the rest of the rule life cycle calls
+     * may not fail, so ->rule_construct() must also make sure that the rule
+     * can be inserted in to the datapath.
      *
      *
      * Insertion
@@ -1143,11 +1146,10 @@ struct ofproto_class {
      * Following successful construction, the ofproto base case inserts 'rule'
      * into its flow table, then it calls ->rule_insert().  ->rule_insert()
      * must add the new rule to the datapath flow table and return only after
-     * this is complete (whether it succeeds or fails).
-     *
-     * If ->rule_insert() fails, the ofproto base code will remove 'rule' from
-     * the flow table, destruct, uninitialize, and deallocate 'rule'.  See
-     * "Rule Life Cycle" above for more details.
+     * this is complete.  The 'new_rule' may be a duplicate of an 'old_rule'.
+     * In this case the 'old_rule' is non-null, and the implementation should
+     * forward rule statistics from the 'old_rule' to the 'new_rule' if
+     * 'forward_stats' is 'true'.  This may not fail.
      *
      *
      * Deletion
@@ -1169,7 +1171,8 @@ struct ofproto_class {
     struct rule *(*rule_alloc)(void);
     enum ofperr (*rule_construct)(struct rule *rule)
         /* OVS_REQUIRES(ofproto_mutex) */;
-    enum ofperr (*rule_insert)(struct rule *rule)
+    void (*rule_insert)(struct rule *rule, struct rule *old_rule,
+                        bool forward_stats)
         /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_destruct)(struct rule *rule);
@@ -1202,36 +1205,6 @@ struct ofproto_class {
     enum ofperr (*rule_execute)(struct rule *rule, const struct flow *flow,
                                 struct dp_packet *packet);
 
-    /* If the datapath can properly implement changing 'rule''s actions to the
-     * 'ofpacts_len' bytes in 'ofpacts', returns 0.  Otherwise, returns an enum
-     * ofperr indicating why the new actions wouldn't work.
-     *
-     * May be a null pointer if any set of actions is acceptable. */
-    enum ofperr (*rule_premodify_actions)(const struct rule *rule,
-                                          const struct ofpact *ofpacts,
-                                          size_t ofpacts_len)
-        /* OVS_REQUIRES(ofproto_mutex) */;
-
-    /* When ->rule_modify_actions() is called, the caller has already replaced
-     * the OpenFlow actions in 'rule' by a new set.  (If
-     * ->rule_premodify_actions is nonnull, then it was previously called to
-     * verify that the new set of actions is acceptable.)
-     *
-     * ->rule_modify_actions() must:
-     *
-     *   - Update the datapath flow table with the new actions.
-     *
-     *   - Only if 'reset_counters' is true, reset any packet or byte counters
-     *     associated with the rule to zero, so that rule_get_stats() will not
-     *     longer count those packets or bytes.
-     *
-     * Rule modification must not fail.
-     *
-     * ->rule_modify_actions() should not modify any base members of struct
-     * rule. */
-    void (*rule_modify_actions)(struct rule *rule, bool reset_counters)
-        /* OVS_REQUIRES(ofproto_mutex) */;
-
     /* Changes the OpenFlow IP fragment handling policy to 'frag_handling',
      * which takes one of the following values, with the corresponding
      * meanings:
@@ -1785,7 +1758,7 @@ void ofproto_flush_flows(struct ofproto *);
 static inline const struct rule_actions *
 rule_get_actions(const struct rule *rule)
 {
-    return ovsrcu_get(const struct rule_actions *, &rule->actions);
+    return rule->actions;
 }
 
 /* Returns true if 'rule' is an OpenFlow 1.3 "table-miss" rule, false
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index e27bb7e..86214f1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -89,8 +89,6 @@ static void oftable_enable_eviction(struct oftable *,
                                     const struct mf_subfield *fields,
                                     size_t n_fields);
 
-static void oftable_remove_rule(struct rule *rule) OVS_REQUIRES(ofproto_mutex);
-
 /* A set of rules within a single OpenFlow table (oftable) that have the same
  * values for the oftable's eviction_fields.  A rule to be evicted, when one is
  * needed, is taken from the eviction group that contains the greatest number
@@ -256,15 +254,30 @@ struct flow_mod_requester {
 static enum ofperr add_flow(struct ofproto *, struct ofputil_flow_mod *,
                             const struct flow_mod_requester *);
 
-static enum ofperr modify_flow_check__(struct ofproto *,
+
+static enum ofperr replace_rule_create(struct ofproto *,
                                        struct ofputil_flow_mod *,
-                                       const struct rule *)
+                                       struct cls_rule *cr, uint8_t table_id,
+                                       struct rule *old_rule,
+                                       struct rule **new_rule)
+    OVS_REQUIRES(ofproto_mutex);
+
+static void replace_rule_begin(struct ofproto *,
+                               struct rule *old_rule,
+                               struct rule *new_rule,
+                               struct cls_conjunction *, size_t n_conjs)
+    OVS_REQUIRES(ofproto_mutex);
+
+static void replace_rule_revert(struct ofproto *,
+                                struct rule *old_rule, struct rule *new_rule)
     OVS_REQUIRES(ofproto_mutex);
-static void modify_flow__(struct ofproto *, struct ofputil_flow_mod *,
-                          const struct flow_mod_requester *, struct rule *,
-                          struct ovs_list *dead_cookies)
+
+static void replace_rule_finish(struct ofproto *, struct ofputil_flow_mod *,
+                                const struct flow_mod_requester *,
+                                struct rule *old_rule, struct rule *new_rule,
+                                struct ovs_list *dead_cookies)
     OVS_REQUIRES(ofproto_mutex);
-static void delete_flows__(const struct rule_collection *,
+static void delete_flows__(struct rule_collection *,
                            enum ofp_flow_removed_reason,
                            const struct flow_mod_requester *)
     OVS_REQUIRES(ofproto_mutex);
@@ -467,6 +480,14 @@ ofproto_enumerate_names(const char *type, struct sset *names)
     return class ? class->enumerate_names(type, names) : EAFNOSUPPORT;
 }
 
+static void
+ofproto_bump_tables_version(struct ofproto *ofproto)
+{
+    ++ofproto->tables_version;
+    ofproto->ofproto_class->set_tables_version(ofproto,
+                                               ofproto->tables_version);
+}
+
 int
 ofproto_create(const char *datapath_name, const char *datapath_type,
                struct ofproto **ofprotop)
@@ -572,8 +593,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
                               * sizeof(struct meter *));
 
     /* Set the initial tables version. */
-    ofproto->ofproto_class->set_tables_version(ofproto,
-                                               ofproto->tables_version);
+    ofproto_bump_tables_version(ofproto);
 
     *ofprotop = ofproto;
     return 0;
@@ -1440,8 +1460,13 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
      * switch is being deleted and any OpenFlow channels have been or soon will
      * be killed. */
     ovs_mutex_lock(&ofproto_mutex);
-    oftable_remove_rule(rule);
-    ofproto->ofproto_class->rule_delete(rule);
+    if (rule->version > 0) {
+        ovs_assert(!rule->cr.to_be_removed);
+        ovs_assert(classifier_remove(&rule->ofproto->tables[rule->table_id].cls,
+                                     &rule->cr));
+        ofproto_rule_remove__(rule->ofproto, rule);
+        ofproto->ofproto_class->rule_delete(rule);
+    }
     ovs_mutex_unlock(&ofproto_mutex);
 }
 
@@ -1477,7 +1502,6 @@ ofproto_flush__(struct ofproto *ofproto)
             rule_collection_add(&rules, rule);
         }
         delete_flows__(&rules, OFPRR_DELETE, NULL);
-        rule_collection_destroy(&rules);
     }
     /* XXX: Concurrent handler threads may insert new learned flows based on
      * learn actions of the now deleted flows right after we release
@@ -1529,6 +1553,16 @@ ofproto_destroy__(struct ofproto *ofproto)
     ofproto->ofproto_class->dealloc(ofproto);
 }
 
+/* Destroying rules is doubly deferred, must have 'ofproto' around for them.
+ * - 1st we defer the removal of the rules from the classifier
+ * - 2nd we defer the actual destruction of the rules. */
+static void
+ofproto_destroy_defer__(struct ofproto *ofproto)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    ovsrcu_postpone(ofproto_destroy__, ofproto);
+}
+
 void
 ofproto_destroy(struct ofproto *p)
     OVS_EXCLUDED(ofproto_mutex)
@@ -1565,7 +1599,7 @@ ofproto_destroy(struct ofproto *p)
     connmgr_destroy(p->connmgr);
 
     /* Destroying rules is deferred, must have 'ofproto' around for them. */
-    ovsrcu_postpone(ofproto_destroy__, p);
+    ovsrcu_postpone(ofproto_destroy_defer__, p);
 }
 
 /* Destroys the datapath with the respective 'name' and 'type'.  With the Linux
@@ -2712,62 +2746,6 @@ ofproto_rule_destroy__(struct rule *rule)
     rule->ofproto->ofproto_class->rule_dealloc(rule);
 }
 
-/* Create a new rule based on attributes in 'fm', match in 'cr', and
- * 'table_id'.  Note that the rule is NOT inserted into a any data structures
- * yet.  Takes ownership of 'cr'. */
-static enum ofperr
-ofproto_rule_create(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                    struct cls_rule *cr, uint8_t table_id,
-                    struct rule **rulep)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    struct rule *rule;
-    enum ofperr error;
-
-    /* Allocate new rule. */
-    rule = ofproto->ofproto_class->rule_alloc();
-    if (!rule) {
-        cls_rule_destroy(cr);
-        VLOG_WARN_RL(&rl, "%s: failed to allocate a rule.", ofproto->name);
-        return ENOMEM;
-    }
-
-    /* Initialize base state. */
-    *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->flow_cookie = fm->new_cookie;
-    rule->created = rule->modified = time_msec();
-
-    ovs_mutex_init(&rule->mutex);
-    ovs_mutex_lock(&rule->mutex);
-    rule->idle_timeout = fm->idle_timeout;
-    rule->hard_timeout = fm->hard_timeout;
-    rule->importance = fm->importance;
-    ovs_mutex_unlock(&rule->mutex);
-
-    *CONST_CAST(uint8_t *, &rule->table_id) = table_id;
-    rule->flags = fm->flags & OFPUTIL_FF_STATE;
-    ovsrcu_set_hidden(&rule->actions,
-                      rule_actions_create(fm->ofpacts, fm->ofpacts_len));
-    list_init(&rule->meter_list_node);
-    rule->eviction_group = NULL;
-    list_init(&rule->expirable);
-    rule->monitor_flags = 0;
-    rule->add_seqno = 0;
-    rule->modify_seqno = 0;
-
-    /* Construct rule, initializing derived state. */
-    error = ofproto->ofproto_class->rule_construct(rule);
-    if (error) {
-        ofproto_rule_destroy__(rule);
-        return error;
-    }
-
-    *rulep = rule;
-    return 0;
-}
-
 static void
 rule_destroy_cb(struct rule *rule)
 {
@@ -2806,6 +2784,68 @@ ofproto_rule_unref(struct rule *rule)
     }
 }
 
+static void
+remove_rule_rcu__(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    struct ofproto *ofproto = rule->ofproto;
+    struct oftable *table = &ofproto->tables[rule->table_id];
+
+    ovs_assert(rule->cr.to_be_removed);
+    ovs_assert(classifier_remove(&table->cls, &rule->cr));
+    ofproto->ofproto_class->rule_delete(rule);
+    ofproto_rule_unref(rule);
+}
+
+static void
+remove_rule_rcu(struct rule *rule)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    ovs_mutex_lock(&ofproto_mutex);
+    remove_rule_rcu__(rule);
+    ovs_mutex_unlock(&ofproto_mutex);
+}
+
+/* Removes and deletes rules from a NULL-terminated array of rule pointers. */
+static void
+remove_rules_rcu(struct rule **rules)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    struct rule **orig_rules = rules;
+
+    if (*rules) {
+        struct ofproto *ofproto = rules[0]->ofproto;
+        unsigned long tables[BITMAP_N_LONGS(256)];
+        struct rule *rule;
+        size_t table_id;
+
+        memset(tables, 0, sizeof tables);
+
+        ovs_mutex_lock(&ofproto_mutex);
+        while ((rule = *rules++)) {
+            /* Defer once for each new table.  This defers the subtable cleanup
+             * until later, so that when removing large number of flows the
+             * operation is faster. */
+            if (!bitmap_is_set(tables, rule->table_id)) {
+                struct classifier *cls = &ofproto->tables[rule->table_id].cls;
+
+                bitmap_set1(tables, rule->table_id);
+                classifier_defer(cls);
+            }
+            remove_rule_rcu__(rule);
+        }
+
+        BITMAP_FOR_EACH_1(table_id, 256, tables) {
+            struct classifier *cls = &ofproto->tables[table_id].cls;
+
+            classifier_publish(cls);
+        }
+        ovs_mutex_unlock(&ofproto_mutex);
+    }
+
+    free(orig_rules);
+}
+
 void
 ofproto_group_ref(struct ofgroup *group)
 {
@@ -3041,9 +3081,8 @@ learned_cookies_flush(struct ofproto *ofproto, struct ovs_list *dead_cookies)
                            c->cookie, OVS_BE64_MAX, OFPP_ANY, OFPG_ANY);
         rule_criteria_require_rw(&criteria, false);
         collect_rules_loose(ofproto, &criteria, &rules);
-        delete_flows__(&rules, OFPRR_DELETE, NULL);
         rule_criteria_destroy(&criteria);
-        rule_collection_destroy(&rules);
+        delete_flows__(&rules, OFPRR_DELETE, NULL);
 
         free(c);
     }
@@ -3769,6 +3808,28 @@ rule_collection_unref(struct rule_collection *rules)
     }
 }
 
+/* Returns a NULL-terminated array of rule pointers,
+ * destroys 'rules'. */
+static struct rule **
+rule_collection_detach(struct rule_collection *rules)
+{
+    struct rule **rule_array;
+
+    rule_collection_add(rules, NULL);
+
+    if (rules->rules == rules->stub) {
+        size_t size = rules->n * sizeof *rules->rules;
+
+        rules->rules = xmalloc(size);
+        memcpy(rules->rules, rules->stub, size);
+    }
+
+    rule_array = rules->rules;
+    rule_collection_init(rules);
+
+    return rule_array;
+}
+
 void
 rule_collection_destroy(struct rule_collection *rules)
 {
@@ -3780,6 +3841,20 @@ rule_collection_destroy(struct rule_collection *rules)
     rule_collection_init(rules);
 }
 
+/* Schedules postponed removal of rules, destroys 'rules'. */
+static void
+rule_collection_remove_postponed(struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    if (rules->n > 0) {
+        if (rules->n == 1) {
+            ovsrcu_postpone(remove_rule_rcu, rules->rules[0]);
+        } else {
+            ovsrcu_postpone(remove_rules_rcu, rule_collection_detach(rules));
+        }
+    }
+}
+
 /* Checks whether 'rule' matches 'c' and, if so, adds it to 'rules'.  This
  * function verifies most of the criteria in 'c' itself, but the caller must
  * check 'c->cr' itself.
@@ -4291,7 +4366,6 @@ evict_rules_from_table(struct oftable *table, unsigned int extra_space)
         }
     }
     delete_flows__(&rules, OFPRR_EVICTION, NULL);
-    rule_collection_destroy(&rules);
 
     return error;
 }
@@ -4334,26 +4408,18 @@ get_conjunctions(const struct ofputil_flow_mod *fm,
     *n_conjsp = n_conjs;
 }
 
-static void
-set_conjunctions(struct rule *rule, const struct cls_conjunction *conjs,
-                 size_t n_conjs)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    struct cls_rule *cr = CONST_CAST(struct cls_rule *, &rule->cr);
-
-    cls_rule_set_conjunctions(cr, conjs, n_conjs);
-}
-
 static enum ofperr
 add_flow_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-               struct rule **rulep, bool *modify)
+               struct rule **old_rule, struct rule **new_rule)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct oftable *table;
     struct cls_rule cr;
     struct rule *rule;
     uint8_t table_id;
-    enum ofperr error = 0;
+    struct cls_conjunction *conjs;
+    size_t n_conjs;
+    enum ofperr error;
 
     if (!check_table_id(ofproto, fm->table_id)) {
         error = OFPERR_OFPBRC_BAD_TABLE_ID;
@@ -4397,21 +4463,8 @@ add_flow_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
     /* Check for the existence of an identical rule.
      * This will not return rules earlier marked as 'to_be_removed'. */
     rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, &cr));
-    if (rule) {
-        /* Transform "add" into "modify" of an existing identical flow. */
-        cls_rule_destroy(&cr);
-
-        fm->modify_cookie = true;
-        error = modify_flow_check__(ofproto, fm, rule);
-        if (error) {
-            return error;
-        }
-
-        *modify = true;
-    } else {   /* New rule. */
-        struct cls_conjunction *conjs;
-        size_t n_conjs;
-
+    *old_rule = rule;
+    if (!rule) {
         /* Check for overlap, if requested. */
         if (fm->flags & OFPUTIL_FF_CHECK_OVERLAP
             && classifier_rule_overlaps(&table->cls, &cr)) {
@@ -4425,81 +4478,52 @@ add_flow_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
             cls_rule_destroy(&cr);
             return error;
         }
+    } else {
+        fm->modify_cookie = true;
+    }
 
-        /* Allocate new rule. */
-        error = ofproto_rule_create(ofproto, fm, &cr, table - ofproto->tables,
-                                    &rule);
-        if (error) {
-            return error;
-        }
-
-        /* Insert flow to the classifier, so that later flow_mods may relate
-         * to it.  This is reversible, in case later errors require this to
-         * be reverted. */
-        ofproto_rule_insert__(ofproto, rule);
-        /* Make the new rule invisible for classifier lookups. */
-        classifier_defer(&table->cls);
-        get_conjunctions(fm, &conjs, &n_conjs);
-        classifier_insert(&table->cls, CLS_NO_VERSION, &rule->cr, conjs,
-                          n_conjs);
-        free(conjs);
-
-        error = ofproto->ofproto_class->rule_insert(rule);
-        if (error) {
-            oftable_remove_rule(rule);
-            ofproto_rule_unref(rule);
-            return error;
-        }
-
-        *modify = false;
+    /* Allocate new rule. */
+    error = replace_rule_create(ofproto, fm, &cr, table - ofproto->tables,
+                                rule, new_rule);
+    if (error) {
+        return error;
     }
 
-    *rulep = rule;
+    get_conjunctions(fm, &conjs, &n_conjs);
+    replace_rule_begin(ofproto, rule, *new_rule, conjs, n_conjs);
+    free(conjs);
+
     return 0;
 }
 
 /* Revert the effects of add_flow_begin().
- * 'new_rule' must be passed in as NULL, if no new rule was allocated and
- * inserted to the classifier.
  * Note: evictions cannot be reverted. */
 static void
-add_flow_revert(struct ofproto *ofproto, struct rule *new_rule)
+add_flow_revert(struct ofproto *ofproto, struct rule *old_rule,
+                struct rule *new_rule)
     OVS_REQUIRES(ofproto_mutex)
 {
-    /* Old rule was not changed yet, only need to revert a new rule. */
-    if (new_rule) {
-        struct oftable *table = &ofproto->tables[new_rule->table_id];
-
-        ovs_assert(classifier_remove(&table->cls, &new_rule->cr));
-        classifier_publish(&table->cls);
-
-        ofproto_rule_remove__(ofproto, new_rule);
-        ofproto->ofproto_class->rule_delete(new_rule);
-        ofproto_rule_unref(new_rule);
-    }
+    replace_rule_revert(ofproto, old_rule, new_rule);
 }
 
+/* To be called after version bump. */
 static void
 add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                 const struct flow_mod_requester *req,
-                struct rule *rule, bool modify)
+                struct rule *old_rule, struct rule *new_rule)
     OVS_REQUIRES(ofproto_mutex)
 {
-    if (modify) {
-        struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
-
-        modify_flow__(ofproto, fm, req, rule, &dead_cookies);
-        learned_cookies_flush(ofproto, &dead_cookies);
-    } else {
-        struct oftable *table = &ofproto->tables[rule->table_id];
-
-        classifier_publish(&table->cls);
+    struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
 
-        learned_cookies_inc(ofproto, rule_get_actions(rule));
+    replace_rule_finish(ofproto, fm, req, old_rule, new_rule, &dead_cookies);
+    learned_cookies_flush(ofproto, &dead_cookies);
 
-        if (minimask_get_vid_mask(&rule->cr.match.mask) == VLAN_VID_MASK) {
+    if (old_rule) {
+        ovsrcu_postpone(remove_rule_rcu, old_rule);
+    } else {
+        if (minimask_get_vid_mask(&new_rule->cr.match.mask) == VLAN_VID_MASK) {
             if (ofproto->vlan_bitmap) {
-                uint16_t vid = miniflow_get_vid(&rule->cr.match.flow);
+                uint16_t vid = miniflow_get_vid(&new_rule->cr.match.flow);
 
                 if (!bitmap_is_set(ofproto->vlan_bitmap, vid)) {
                     bitmap_set1(ofproto->vlan_bitmap, vid);
@@ -4510,12 +4534,12 @@ add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
             }
         }
 
-        ofmonitor_report(ofproto->connmgr, rule, NXFME_ADDED, 0,
+        ofmonitor_report(ofproto->connmgr, new_rule, NXFME_ADDED, 0,
                          req ? req->ofconn : NULL,
                          req ? req->request->xid : 0, NULL);
     }
 
-    send_buffered_packet(req, fm->buffer_id, rule);
+    send_buffered_packet(req, fm->buffer_id, new_rule);
 }
 
 /* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
@@ -4531,13 +4555,13 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
          const struct flow_mod_requester *req)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct rule *rule;
-    bool modify;
+    struct rule *old_rule, *new_rule;
     enum ofperr error;
 
-    error = add_flow_begin(ofproto, fm, &rule, &modify);
+    error = add_flow_begin(ofproto, fm, &old_rule, &new_rule);
     if (!error) {
-        add_flow_finish(ofproto, fm, req, rule, modify);
+        ofproto_bump_tables_version(ofproto);
+        add_flow_finish(ofproto, fm, req, old_rule, new_rule);
     }
 
     return error;
@@ -4545,204 +4569,226 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 
 /* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
 
-/* Checks if the 'rule' can be modified to match 'fm'.
- *
- * Returns 0 on success, otherwise an OpenFlow error code. */
+/* Create a new rule based on attributes in 'fm', match in 'cr', 'table_id',
+ * and 'old_rule'.  Note that the rule is NOT inserted into a any data
+ * structures yet.  Takes ownership of 'cr'. */
 static enum ofperr
-modify_flow_check__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                    const struct rule *rule)
-    OVS_REQUIRES(ofproto_mutex)
+replace_rule_create(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+                    struct cls_rule *cr, uint8_t table_id,
+                    struct rule *old_rule, struct rule **new_rule)
 {
-    if (ofproto->ofproto_class->rule_premodify_actions) {
-        enum ofperr error;
+    struct rule *rule;
+    enum ofperr error;
 
-        error = ofproto->ofproto_class->rule_premodify_actions(
-            rule, fm->ofpacts, fm->ofpacts_len);
-        if (error) {
-            return error;
-        }
+    /* Allocate new rule. */
+    rule = ofproto->ofproto_class->rule_alloc();
+    if (!rule) {
+        cls_rule_destroy(cr);
+        VLOG_WARN_RL(&rl, "%s: failed to allocate a rule.", ofproto->name);
+        return ENOMEM;
     }
 
-    return 0;
-}
+    /* Initialize base state. */
+    *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->flow_cookie = fm->new_cookie;
+    rule->created = rule->modified = time_msec();
 
-/* Checks if the rules listed in 'rules' can have their actions changed to
- * match those in 'fm'.
- *
- * Returns 0 on success, otherwise an OpenFlow error code. */
-static enum ofperr
-modify_flows_check__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                     const struct rule_collection *rules)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    enum ofperr error;
-    size_t i;
+    ovs_mutex_init(&rule->mutex);
+    ovs_mutex_lock(&rule->mutex);
+    rule->idle_timeout = fm->idle_timeout;
+    rule->hard_timeout = fm->hard_timeout;
+    rule->importance = fm->importance;
 
-    if (ofproto->ofproto_class->rule_premodify_actions) {
-        for (i = 0; i < rules->n; i++) {
-            error = modify_flow_check__(ofproto, fm, rules->rules[i]);
-            if (error) {
-                return error;
-            }
+    *CONST_CAST(uint8_t *, &rule->table_id) = table_id;
+    rule->flags = fm->flags & OFPUTIL_FF_STATE;
+    *CONST_CAST(const struct rule_actions **, &rule->actions)
+        = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
+    list_init(&rule->meter_list_node);
+    rule->eviction_group = NULL;
+    list_init(&rule->expirable);
+    rule->monitor_flags = 0;
+    rule->add_seqno = 0;
+    rule->modify_seqno = 0;
+    rule->version = ofproto->tables_version + 1; /* Originally visible in the
+                                                  * next version */
+    /* Copy values from old rule for modify semantics. */
+    if (old_rule) {
+        /*  'fm' says that  */
+        bool change_cookie = (fm->modify_cookie
+                              && fm->new_cookie != OVS_BE64_MAX
+                              && fm->new_cookie != old_rule->flow_cookie);
+
+        ovs_mutex_lock(&old_rule->mutex);
+        if (fm->command != OFPFC_ADD) {
+            rule->idle_timeout = old_rule->idle_timeout;
+            rule->hard_timeout = old_rule->hard_timeout;
+            rule->importance = old_rule->importance;
+            rule->flags = old_rule->flags;
+            rule->created = old_rule->created;
         }
+        if (!change_cookie) {
+            rule->flow_cookie = old_rule->flow_cookie;
+        }
+        ovs_mutex_unlock(&old_rule->mutex);
+    }
+    ovs_mutex_unlock(&rule->mutex);
+
+    /* Construct rule, initializing derived state. */
+    error = ofproto->ofproto_class->rule_construct(rule);
+    if (error) {
+        ofproto_rule_destroy__(rule);
+        return error;
     }
 
+    *new_rule = rule;
     return 0;
 }
 
-/* Modifies the 'rule', changing them to match 'fm'. */
 static void
-modify_flow__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-              const struct flow_mod_requester *req, struct rule *rule,
-              struct ovs_list *dead_cookies)
-    OVS_REQUIRES(ofproto_mutex)
+replace_rule_begin(struct ofproto *ofproto,
+                   struct rule *old_rule, struct rule *new_rule,
+                   struct cls_conjunction *conjs, size_t n_conjs)
 {
-    enum nx_flow_update_event event = fm->command == OFPFC_ADD
-        ? NXFME_ADDED : NXFME_MODIFIED;
-
-    /*  'fm' says that  */
-    bool change_cookie = (fm->modify_cookie
-                          && fm->new_cookie != OVS_BE64_MAX
-                          && fm->new_cookie != rule->flow_cookie);
+    struct oftable *table = &ofproto->tables[new_rule->table_id];
 
-    const struct rule_actions *actions = rule_get_actions(rule);
-    bool change_actions = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
-                                         actions->ofpacts,
-                                         actions->ofpacts_len);
-
-    bool reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
+    if (old_rule) {
+        /* Mark the old rule for removal in the next version. */
+        cls_rule_make_removable_after_version(&old_rule->cr,
+                                              ofproto->tables_version);
+    }
+    /* Insert flow to the classifier, so that later flow_mods may relate
+     * to it.  This is reversible, in case later errors require this to
+     * be reverted. */
+    ofproto_rule_insert__(ofproto, new_rule);
+    /* Make the new rule visible for classifier lookups only from the next
+     * version. */
+    classifier_insert(&table->cls, ofproto->tables_version + 1, &new_rule->cr,
+                      conjs, n_conjs);
+}
 
-    long long int now = time_msec();
+static void replace_rule_revert(struct ofproto *ofproto,
+                                struct rule *old_rule, struct rule *new_rule)
+{
+    struct oftable *table = &ofproto->tables[new_rule->table_id];
 
-    if (change_cookie) {
-        cookies_remove(ofproto, rule);
+    /* Restore the original visibility of the old rule if it
+     * was inserted before this transaction. */
+    if (old_rule && old_rule->version <= ofproto->tables_version) {
+        cls_rule_restore_version(&old_rule->cr, old_rule->version);
     }
 
-    ovs_mutex_lock(&rule->mutex);
-    if (fm->command == OFPFC_ADD) {
-        rule->idle_timeout = fm->idle_timeout;
-        rule->hard_timeout = fm->hard_timeout;
-        rule->importance = fm->importance;
-        rule->flags = fm->flags & OFPUTIL_FF_STATE;
-        rule->created = now;
-    }
-    if (change_cookie) {
-        rule->flow_cookie = fm->new_cookie;
-    }
-    rule->modified = now;
-    ovs_mutex_unlock(&rule->mutex);
+    /* Remove the new rule immediately.  It was never visible to lookups. */
+    ovs_assert(classifier_remove(&table->cls, &new_rule->cr));
 
-    if (change_cookie) {
-        cookies_insert(ofproto, rule);
-    }
-    if (fm->command == OFPFC_ADD) {
-        if (fm->idle_timeout || fm->hard_timeout || fm->importance) {
-            if (!rule->eviction_group) {
-                eviction_group_add_rule(rule);
-            }
-        } else {
-            eviction_group_remove_rule(rule);
-        }
-    }
+    ofproto_rule_remove__(ofproto, new_rule);
+    /* The rule was not inserted to the ofproto provider, so we can
+     * release it without deleting it from the ofproto provider. */
+    ofproto_rule_unref(new_rule);
+}
 
-    if (change_actions) {
-        /* We have to change the actions.  The rule's conjunctive match set
-         * is a function of its actions, so we need to update that too.  The
-         * conjunctive match set is used in the lookup process to figure
-         * which (if any) collection of conjunctive sets the packet matches
-         * with.  However, a rule with conjunction actions is never to be
-         * returned as a classifier lookup result.  To make sure a rule with
-         * conjunction actions is not returned as a lookup result, we update
-         * them in a carefully chosen order:
-         *
-         * - If we're adding a conjunctive match set where there wasn't one
-         *   before, we have to make the conjunctive match set available to
-         *   lookups before the rule's actions are changed, as otherwise
-         *   rule with a conjunction action could be returned as a lookup
-         *   result.
-         *
-         * - To clear some nonempty conjunctive set, we set the rule's
-         *   actions first, so that a lookup can't return a rule with
-         *   conjunction actions.
-         *
-         * - Otherwise, order doesn't matter for changing one nonempty
-         *   conjunctive match set to some other nonempty set, since the
-         *   rule's actions are not seen by the classifier, and hence don't
-         *   matter either before or after the change. */
-        struct cls_conjunction *conjs;
-        size_t n_conjs;
-        get_conjunctions(fm, &conjs, &n_conjs);
+/* Adds the 'new_rule', replacing the 'old_rule'. */
+static void
+replace_rule_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+                    const struct flow_mod_requester *req,
+                    struct rule *old_rule, struct rule *new_rule,
+                    struct ovs_list *dead_cookies)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    bool forward_stats = !(fm->flags & OFPUTIL_FF_RESET_COUNTS);
 
-        if (n_conjs) {
-            set_conjunctions(rule, conjs, n_conjs);
-        }
-        ovsrcu_set(&rule->actions, rule_actions_create(fm->ofpacts,
-                                                           fm->ofpacts_len));
-        if (!conjs) {
-            set_conjunctions(rule, conjs, n_conjs);
-        }
+    /* Insert the new flow to the ofproto provider.  A non-NULL 'old_rule' is a
+     * duplicate rule the 'new_rule' is replacing.  The provider should link
+     * the stats from the old rule to the new one if 'forward_stats' is
+     * 'true'.  The 'old_rule' will be deleted right after this call. */
+    ofproto->ofproto_class->rule_insert(new_rule, old_rule, forward_stats);
+    learned_cookies_inc(ofproto, rule_get_actions(new_rule));
 
-        free(conjs);
-    }
+    if (old_rule) {
+        const struct rule_actions *old_actions = rule_get_actions(old_rule);
 
-    if (change_actions || reset_counters) {
-        ofproto->ofproto_class->rule_modify_actions(rule, reset_counters);
-    }
+        enum nx_flow_update_event event = fm->command == OFPFC_ADD
+            ? NXFME_ADDED : NXFME_MODIFIED;
 
-    if (event != NXFME_MODIFIED || change_actions || change_cookie) {
-        ofmonitor_report(ofproto->connmgr, rule, event, 0,
-                         req ? req->ofconn : NULL, req ? req->request->xid : 0,
-                         change_actions ? actions : NULL);
-    }
+        /*  'fm' says that  */
+        bool change_cookie = (fm->modify_cookie
+                              && fm->new_cookie != OVS_BE64_MAX
+                              && fm->new_cookie != old_rule->flow_cookie);
 
-    if (change_actions) {
-        learned_cookies_inc(ofproto, rule_get_actions(rule));
-        learned_cookies_dec(ofproto, actions, dead_cookies);
-        rule_actions_destroy(actions);
-    }
-}
+        bool change_actions = !ofpacts_equal(fm->ofpacts,
+                                             fm->ofpacts_len,
+                                             old_actions->ofpacts,
+                                             old_actions->ofpacts_len);
 
-/* Modifies the rules listed in 'rules', changing their actions to match those
- * in 'fm'.
- *
- * 'req' is used to retrieve the packet buffer specified in fm->buffer_id,
- * if any. */
-static void
-modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-               const struct flow_mod_requester *req,
-               const struct rule_collection *rules)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
-    size_t i;
+        /* Remove the old rule from data structures.  Removal from the
+         * classifier and the deletion of the rule is RCU postponed by the
+         * caller. */
+        ofproto_rule_remove__(ofproto, old_rule);
+        learned_cookies_dec(ofproto, old_actions, dead_cookies);
 
-    for (i = 0; i < rules->n; i++) {
-        modify_flow__(ofproto, fm, req, rules->rules[i], &dead_cookies);
+        if (event != NXFME_MODIFIED || change_actions || change_cookie) {
+            ofmonitor_report(ofproto->connmgr, new_rule, event, 0,
+                             req ? req->ofconn : NULL,
+                             req ? req->request->xid : 0,
+                             change_actions ? old_actions : NULL);
+        }
     }
-    learned_cookies_flush(ofproto, &dead_cookies);
 }
 
 static enum ofperr
 modify_flows_begin__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                     struct rule_collection *rules)
+                     struct rule_collection *old_rules,
+                     struct rule_collection *new_rules)
     OVS_REQUIRES(ofproto_mutex)
 {
     enum ofperr error;
 
-    if (rules->n > 0) {
-        error = modify_flows_check__(ofproto, fm, rules);
+    rule_collection_init(new_rules);
+
+    if (old_rules->n > 0) {
+        struct cls_conjunction *conjs;
+        size_t n_conjs;
+        size_t i;
+
+        /* Create a new 'modified' rule for each old rule. */
+        for (i = 0; i < old_rules->n; i++) {
+            struct rule *old_rule = old_rules->rules[i];
+            struct rule *new_rule;
+            struct cls_rule cr;
+
+            cls_rule_clone(&cr, &old_rule->cr);
+            error = replace_rule_create(ofproto, fm, &cr, old_rule->table_id,
+                                        old_rule, &new_rule);
+            if (!error) {
+                rule_collection_add(new_rules, new_rule);
+            } else {
+                rule_collection_unref(new_rules);
+                rule_collection_destroy(new_rules);
+                return error;
+            }
+        }
+        ovs_assert(new_rules->n == old_rules->n);
+
+        get_conjunctions(fm, &conjs, &n_conjs);
+        for (i = 0; i < old_rules->n; i++) {
+            replace_rule_begin(ofproto, old_rules->rules[i],
+                               new_rules->rules[i], conjs, n_conjs);
+        }
+        free(conjs);
     } else if (!(fm->cookie_mask != htonll(0)
                  || fm->new_cookie == OVS_BE64_MAX)) {
-        bool modify;
-
-        error = add_flow_begin(ofproto, fm, &rules->rules[0], &modify);
+        /* No match, add a new flow. */
+        error = add_flow_begin(ofproto, fm, &old_rules->rules[0],
+                               &new_rules->rules[0]);
         if (!error) {
-            ovs_assert(!modify);
+            ovs_assert(!old_rules->rules[0]);
         }
+        new_rules->n = 1;
     } else {
-        rules->rules[0] = NULL;
         error = 0;
     }
+
     return error;
 }
 
@@ -4753,7 +4799,8 @@ modify_flows_begin__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
  * if any. */
 static enum ofperr
 modify_flows_begin_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                         struct rule_collection *rules)
+                         struct rule_collection *old_rules,
+                         struct rule_collection *new_rules)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_criteria criteria;
@@ -4763,43 +4810,62 @@ modify_flows_begin_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                        fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
     rule_criteria_require_rw(&criteria,
                              (fm->flags & OFPUTIL_FF_NO_READONLY) != 0);
-    error = collect_rules_loose(ofproto, &criteria, rules);
+    error = collect_rules_loose(ofproto, &criteria, old_rules);
     rule_criteria_destroy(&criteria);
 
     if (!error) {
-        error = modify_flows_begin__(ofproto, fm, rules);
+        error = modify_flows_begin__(ofproto, fm, old_rules, new_rules);
     }
 
     if (error) {
-        rule_collection_destroy(rules);
+        rule_collection_destroy(old_rules);
     }
     return error;
 }
 
 static void
-modify_flows_revert(struct ofproto *ofproto, struct rule_collection *rules)
+modify_flows_revert(struct ofproto *ofproto, struct rule_collection *old_rules,
+                    struct rule_collection *new_rules)
     OVS_REQUIRES(ofproto_mutex)
 {
-    /* Old rules were not changed yet, only need to revert a new rule. */
-    if (rules->n == 0 && rules->rules[0] != NULL) {
-        add_flow_revert(ofproto, rules->rules[0]);
+    /* Old rules were not changed yet, only need to revert new rules. */
+    if (old_rules->n == 0 && new_rules->n == 1) {
+        add_flow_revert(ofproto, new_rules->rules[0], NULL);
+    } else if (old_rules->n > 0) {
+        for (size_t i = 0; i < old_rules->n; i++) {
+            replace_rule_revert(ofproto, old_rules->rules[i],
+                                new_rules->rules[i]);
+        }
+        rule_collection_destroy(new_rules);
+        rule_collection_destroy(old_rules);
     }
-    rule_collection_destroy(rules);
 }
 
 static void
 modify_flows_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                     const struct flow_mod_requester *req,
-                    struct rule_collection *rules)
+                    struct rule_collection *old_rules,
+                    struct rule_collection *new_rules)
     OVS_REQUIRES(ofproto_mutex)
 {
-    if (rules->n > 0) {
-        modify_flows__(ofproto, fm, req, rules);
-        send_buffered_packet(req, fm->buffer_id, rules->rules[0]);
-    } else if (rules->rules[0] != NULL) {
-        add_flow_finish(ofproto, fm, req, rules->rules[0], false);
+    if (old_rules->n == 0 && new_rules->n == 1) {
+        add_flow_finish(ofproto, fm, req, old_rules->rules[0],
+                        new_rules->rules[0]);
+    } else if (old_rules->n > 0) {
+        struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
+
+        ovs_assert(new_rules->n == old_rules->n);
+
+        for (size_t i = 0; i < old_rules->n; i++) {
+            replace_rule_finish(ofproto, fm, req, old_rules->rules[i],
+                                new_rules->rules[i], &dead_cookies);
+        }
+        learned_cookies_flush(ofproto, &dead_cookies);
+        rule_collection_remove_postponed(old_rules);
+
+        send_buffered_packet(req, fm->buffer_id, new_rules->rules[0]);
+        rule_collection_destroy(new_rules);
     }
-    rule_collection_destroy(rules);
 }
 
 static enum ofperr
@@ -4807,12 +4873,13 @@ modify_flows_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                    const struct flow_mod_requester *req)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct rule_collection rules;
+    struct rule_collection old_rules, new_rules;
     enum ofperr error;
 
-    error = modify_flows_begin_loose(ofproto, fm, &rules);
+    error = modify_flows_begin_loose(ofproto, fm, &old_rules, &new_rules);
     if (!error) {
-        modify_flows_finish(ofproto, fm, req, &rules);
+        ofproto_bump_tables_version(ofproto);
+        modify_flows_finish(ofproto, fm, req, &old_rules, &new_rules);
     }
 
     return error;
@@ -4822,7 +4889,8 @@ modify_flows_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
  * code on failure. */
 static enum ofperr
 modify_flow_begin_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
-                         struct rule_collection *rules)
+                         struct rule_collection *old_rules,
+                         struct rule_collection *new_rules)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_criteria criteria;
@@ -4832,16 +4900,16 @@ modify_flow_begin_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                        fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
     rule_criteria_require_rw(&criteria,
                              (fm->flags & OFPUTIL_FF_NO_READONLY) != 0);
-    error = collect_rules_strict(ofproto, &criteria, rules);
+    error = collect_rules_strict(ofproto, &criteria, old_rules);
     rule_criteria_destroy(&criteria);
 
     if (!error) {
         /* collect_rules_strict() can return max 1 rule. */
-        error = modify_flows_begin__(ofproto, fm, rules);
+        error = modify_flows_begin__(ofproto, fm, old_rules, new_rules);
     }
 
     if (error) {
-        rule_collection_destroy(rules);
+        rule_collection_destroy(old_rules);
     }
     return error;
 }
@@ -4851,12 +4919,13 @@ modify_flow_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
                    const struct flow_mod_requester *req)
     OVS_REQUIRES(ofproto_mutex)
 {
-    struct rule_collection rules;
+    struct rule_collection old_rules, new_rules;
     enum ofperr error;
 
-    error = modify_flow_begin_strict(ofproto, fm, &rules);
+    error = modify_flow_begin_strict(ofproto, fm, &old_rules, &new_rules);
     if (!error) {
-        modify_flows_finish(ofproto, fm, req, &rules);
+        ofproto_bump_tables_version(ofproto);
+        modify_flows_finish(ofproto, fm, req, &old_rules, &new_rules);
     }
 
     return error;
@@ -4865,50 +4934,59 @@ modify_flow_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 
 /* OFPFC_DELETE implementation. */
 
-/* Deletes the rules listed in 'rules'. */
 static void
-delete_flows__(const struct rule_collection *rules,
-               enum ofp_flow_removed_reason reason,
-               const struct flow_mod_requester *req)
+delete_flows_begin__(struct ofproto *ofproto,
+                     const struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    for (size_t i = 0; i < rules->n; i++) {
+        cls_rule_make_removable_after_version(&rules->rules[i]->cr,
+                                              ofproto->tables_version);
+    }
+}
+
+static void
+delete_flows_finish__(struct ofproto *ofproto,
+                      struct rule_collection *rules,
+                      enum ofp_flow_removed_reason reason,
+                      const struct flow_mod_requester *req)
     OVS_REQUIRES(ofproto_mutex)
 {
     if (rules->n) {
         struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
-        struct ofproto *ofproto = rules->rules[0]->ofproto;
-        struct rule *rule, *next;
-        uint8_t prev_table = UINT8_MAX;
-        size_t i;
 
-        for (i = 0, next = rules->rules[0];
-             rule = next, next = (++i < rules->n) ? rules->rules[i] : NULL,
-                 rule; prev_table = rule->table_id) {
-            struct classifier *cls = &ofproto->tables[rule->table_id].cls;
-            uint8_t next_table = next ? next->table_id : UINT8_MAX;
+        for (size_t i = 0; i < rules->n; i++) {
+            struct rule *rule = rules->rules[i];
 
             ofproto_rule_send_removed(rule, reason);
-
             ofmonitor_report(ofproto->connmgr, rule, NXFME_DELETED, reason,
                              req ? req->ofconn : NULL,
                              req ? req->request->xid : 0, NULL);
-
-            /* Defer once for each new table. */
-            if (rule->table_id != prev_table) {
-                classifier_defer(cls);
-            }
-            classifier_remove(cls, &rule->cr);
-            if (next_table != rule->table_id) {
-                classifier_publish(cls);
-            }
             ofproto_rule_remove__(ofproto, rule);
-
-            ofproto->ofproto_class->rule_delete(rule);
-
             learned_cookies_dec(ofproto, rule_get_actions(rule),
                                 &dead_cookies);
-
-            ofproto_rule_unref(rule);
         }
+        rule_collection_remove_postponed(rules);
+
         learned_cookies_flush(ofproto, &dead_cookies);
+    }
+}
+
+/* Deletes the rules listed in 'rules'.
+ * The deleted rules will become invisible to the lookups in the next version.
+ * Destroys 'rules'. */
+static void
+delete_flows__(struct rule_collection *rules,
+               enum ofp_flow_removed_reason reason,
+               const struct flow_mod_requester *req)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    if (rules->n) {
+        struct ofproto *ofproto = rules->rules[0]->ofproto;
+
+        delete_flows_begin__(ofproto, rules);
+        ofproto_bump_tables_version(ofproto);
+        delete_flows_finish__(ofproto, rules, reason, req);
         ofmonitor_flush(ofproto->connmgr);
     }
 }
@@ -4932,36 +5010,37 @@ delete_flows_begin_loose(struct ofproto *ofproto,
     rule_criteria_destroy(&criteria);
 
     if (!error) {
-        for (size_t i = 0; i < rules->n; i++) {
-            struct rule *rule = rules->rules[i];
-
-            CONST_CAST(struct cls_rule *, &rule->cr)->to_be_removed = true;
-        }
+        delete_flows_begin__(ofproto, rules);
     }
 
     return error;
 }
 
 static void
-delete_flows_revert(struct rule_collection *rules)
+delete_flows_revert(struct ofproto *ofproto,
+                    struct rule_collection *rules)
     OVS_REQUIRES(ofproto_mutex)
 {
     for (size_t i = 0; i < rules->n; i++) {
         struct rule *rule = rules->rules[i];
 
-        CONST_CAST(struct cls_rule *, &rule->cr)->to_be_removed = false;
+        /* Restore the original visibility of the old rule if it
+         * was inserted before this transaction. */
+        if (rule->version <= ofproto->tables_version) {
+            cls_rule_restore_version(&rule->cr, rule->version);
+        }
     }
     rule_collection_destroy(rules);
 }
 
 static void
-delete_flows_finish(const struct ofputil_flow_mod *fm,
+delete_flows_finish(struct ofproto *ofproto,
+                    const struct ofputil_flow_mod *fm,
                     const struct flow_mod_requester *req,
                     struct rule_collection *rules)
     OVS_REQUIRES(ofproto_mutex)
 {
-    delete_flows__(rules, fm->delete_reason, req);
-    rule_collection_destroy(rules);
+    delete_flows_finish__(ofproto, rules, fm->delete_reason, req);
 }
 
 static enum ofperr
@@ -4975,7 +5054,8 @@ delete_flows_loose(struct ofproto *ofproto,
 
     error = delete_flows_begin_loose(ofproto, fm, &rules);
     if (!error) {
-        delete_flows_finish(fm, req, &rules);
+        ofproto_bump_tables_version(ofproto);
+        delete_flows_finish(ofproto, fm, req, &rules);
     }
 
     return error;
@@ -5000,11 +5080,7 @@ delete_flow_begin_strict(struct ofproto *ofproto,
     rule_criteria_destroy(&criteria);
 
     if (!error) {
-        for (size_t i = 0; i < rules->n; i++) {
-            struct rule *rule = rules->rules[i];
-
-            CONST_CAST(struct cls_rule *, &rule->cr)->to_be_removed = true;
-        }
+        delete_flows_begin__(ofproto, rules);
     }
 
     return error;
@@ -5020,7 +5096,8 @@ delete_flow_strict(struct ofproto *ofproto, const struct ofputil_flow_mod *fm,
 
     error = delete_flow_begin_strict(ofproto, fm, &rules);
     if (!error) {
-        delete_flows_finish(fm, req, &rules);
+        ofproto_bump_tables_version(ofproto);
+        delete_flows_finish(ofproto, fm, req, &rules);
     }
 
     return error;
@@ -5758,7 +5835,6 @@ handle_delete_meter(struct ofconn *ofconn, struct ofputil_meter_mod *mm)
     meter_delete(ofproto, first, last);
 
     ovs_mutex_unlock(&ofproto_mutex);
-    rule_collection_destroy(&rules);
 
     return error;
 }
@@ -6559,23 +6635,26 @@ do_bundle_flow_mod_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 
     switch (fm->command) {
     case OFPFC_ADD:
-        error = add_flow_begin(ofproto, fm, &be->rule, &be->modify);
+        error = add_flow_begin(ofproto, fm, &be->old_rules.stub[0],
+                               &be->new_rules.stub[0]);
         break;
 
     case OFPFC_MODIFY:
-        error = modify_flows_begin_loose(ofproto, fm, &be->rules);
+        error = modify_flows_begin_loose(ofproto, fm, &be->old_rules,
+                                         &be->new_rules);
         break;
 
     case OFPFC_MODIFY_STRICT:
-        error = modify_flow_begin_strict(ofproto, fm, &be->rules);
+        error = modify_flow_begin_strict(ofproto, fm, &be->old_rules,
+                                         &be->new_rules);
         break;
 
     case OFPFC_DELETE:
-        error = delete_flows_begin_loose(ofproto, fm, &be->rules);
+        error = delete_flows_begin_loose(ofproto, fm, &be->old_rules);
         break;
 
     case OFPFC_DELETE_STRICT:
-        error = delete_flow_begin_strict(ofproto, fm, &be->rules);
+        error = delete_flow_begin_strict(ofproto, fm, &be->old_rules);
         break;
 
     default:
@@ -6598,17 +6677,17 @@ do_bundle_flow_mod_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 {
     switch (fm->command) {
     case OFPFC_ADD:
-        add_flow_revert(ofproto, be->modify ? NULL : be->rule);
+        add_flow_revert(ofproto, be->old_rules.stub[0], be->new_rules.stub[0]);
         break;
 
     case OFPFC_MODIFY:
     case OFPFC_MODIFY_STRICT:
-        modify_flows_revert(ofproto, &be->rules);
+        modify_flows_revert(ofproto, &be->old_rules, &be->new_rules);
         break;
 
     case OFPFC_DELETE:
     case OFPFC_DELETE_STRICT:
-        delete_flows_revert(&be->rules);
+        delete_flows_revert(ofproto, &be->old_rules);
         break;
 
     default:
@@ -6624,17 +6703,18 @@ do_bundle_flow_mod_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 {
     switch (fm->command) {
     case OFPFC_ADD:
-        add_flow_finish(ofproto, fm, req, be->rule, be->modify);
+        add_flow_finish(ofproto, fm, req, be->old_rules.stub[0],
+                        be->new_rules.stub[0]);
         break;
 
     case OFPFC_MODIFY:
     case OFPFC_MODIFY_STRICT:
-        modify_flows_finish(ofproto, fm, req, &be->rules);
+        modify_flows_finish(ofproto, fm, req, &be->old_rules, &be->new_rules);
         break;
 
     case OFPFC_DELETE:
     case OFPFC_DELETE_STRICT:
-        delete_flows_finish(fm, req, &be->rules);
+        delete_flows_finish(ofproto, fm, req, &be->old_rules);
         break;
 
     default:
@@ -6644,23 +6724,20 @@ do_bundle_flow_mod_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
 
 /* Commit phases (all while locking ofproto_mutex):
  *
- * 1. Gather resources - do not send any events or notifications.
- *
- * add: Check conflicts, check for a displaced flow. If no displaced flow
- *      exists, add the new flow, but mark it as "invisible".
- * mod: Collect affected flows, Do not modify yet.
- * del: Collect affected flows, Do not delete yet.
+ * 1. Begin: Gather resources and make changes visible in the next version.
+ *           - Mark affected rules as 'to be deleted' in the next version.
+ *           - Create new replacement rules, make visible in the next
+ *             version.
+ *           - Do not send any events or notifications.
  *
- * 2a. Fail if any errors are found.  After this point no errors are possible.
- * No visible changes were made, so rollback is minimal (remove added invisible
- * flows, revert 'to_be_removed' status of flows).
+ * 2. Revert: Fail if any errors are found.  After this point no errors are
+ * possible.  No visible changes were made, so rollback is minimal (remove
+ * added invisible flows, revert 'to_be_removed' status of flows).
  *
- * 2b. Commit the changes
+ * 3. Bump the version visible to the lookups.
  *
- * add: if have displaced flow, modify it, otherwise mark the new flow as
- *      "visible".
- * mod: Modify the collected flows.
- * del: Delete the collected flows.
+ * 4. Finish: Make the changes visible also in all future versions. Send
+ * notifications, buffered packets, etc.
  */
 static enum ofperr
 do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
@@ -6680,6 +6757,8 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
     } else {
         error = 0;
         ovs_mutex_lock(&ofproto_mutex);
+
+        /* 1. Begin. */
         LIST_FOR_EACH (be, node, &bundle->msg_list) {
             if (be->type == OFPTYPE_PORT_MOD) {
                 /* Not supported yet. */
@@ -6700,14 +6779,21 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
                 error = OFPERR_OFPBFC_MSG_FAILED;
             }
 
-            /* Revert all previous entires. */
+            /* 2. Revert.  Undo all the changes made above. */
             LIST_FOR_EACH_REVERSE_CONTINUE(be, node, &bundle->msg_list) {
                 if (be->type == OFPTYPE_FLOW_MOD) {
                     do_bundle_flow_mod_revert(ofproto, &be->fm, be);
                 }
             }
         } else {
-            /* Finish the changes. */
+            /* 3. Bump the version.  This makes all the changes in the bundle
+             * visible to the lookups at once.  For this to work an upcall must
+             * read the tables_version once at the beginning and keep using the
+             * same version number for the whole duration of the upcall
+             * processing. */
+            ofproto_bump_tables_version(ofproto);
+
+            /* 4. Finish. */
             LIST_FOR_EACH (be, node, &bundle->msg_list) {
                 if (be->type == OFPTYPE_FLOW_MOD) {
                     struct flow_mod_requester req = { ofconn, be->ofp_msg };
@@ -6743,10 +6829,6 @@ handle_bundle_control(struct ofconn *ofconn, const struct ofp_header *oh)
     if (error) {
         return error;
     }
-    /* Atomic updates not supported yet. */
-    if (bctrl.flags & OFPBF_ATOMIC) {
-        return OFPERR_OFPBFC_BAD_FLAGS;
-    }
     reply.flags = 0;
     reply.bundle_id = bctrl.bundle_id;
 
@@ -7464,6 +7546,8 @@ static void
 ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
     OVS_REQUIRES(ofproto_mutex)
 {
+    rule->version = -1;  /* Marks removal from the ofproto structures. */
+
     cookies_remove(ofproto, rule);
 
     eviction_group_remove_rule(rule);
@@ -7475,17 +7559,6 @@ ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
         list_init(&rule->meter_list_node);
     }
 }
-
-static void
-oftable_remove_rule(struct rule *rule)
-    OVS_REQUIRES(ofproto_mutex)
-{
-    struct classifier *cls = &rule->ofproto->tables[rule->table_id].cls;
-
-    if (classifier_remove(cls, &rule->cr)) {
-        ofproto_rule_remove__(rule->ofproto, rule);
-    }
-}
 
 /* unixctl commands. */
 
diff --git a/tests/ofproto.at b/tests/ofproto.at
index e6ede1a..2aa9422 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -3526,38 +3526,38 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
  version bitmap: 0x01, 0x05
 vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05)
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=OPEN_REQUEST flags=ordered
+ bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=OPEN_REPLY flags=0
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 actions=output:1
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 actions=output:2
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 actions=output:3
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 actions=output:4
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): DEL table:255 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 actions=output:5
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 actions=output:6
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 actions=output:7
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): DEL table:255 in_port=2,dl_src=00:88:99:aa:bb:cc actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REPLY flags=0
 vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
@@ -3578,17 +3578,17 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
  version bitmap: 0x01, 0x05
 vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05)
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=OPEN_REQUEST flags=ordered
+ bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=OPEN_REPLY flags=0
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): MOD actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): MOD_STRICT in_port=2,dl_src=00:77:88:99:aa:bb actions=output:7
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REPLY flags=0
 vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
@@ -3609,20 +3609,20 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
  version bitmap: 0x01, 0x05
 vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05)
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=OPEN_REQUEST flags=ordered
+ bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=OPEN_REPLY flags=0
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 actions=output:8
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 in_port=2,dl_src=00:66:77:88:99:aa actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REPLY flags=0
 vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
@@ -3681,7 +3681,7 @@ OFPT_ERROR (OF1.4) (xid=0xb): OFPBRC_EPERM
 OFPT_FLOW_MOD (OF1.4) (xid=0xb): ADD table:254 actions=drop
 OFPT_ERROR (OF1.4) (xid=0xd): OFPBFC_MSG_FAILED
 OFPT_BUNDLE_CONTROL (OF1.4) (xid=0xd):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 ovs-ofctl: talking to unix:br0.mgmt (Protocol error)
 ])
 
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index d2164bb..5f1c330 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2843,35 +2843,35 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
  version bitmap: 0x01, 0x05
 vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05)
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=OPEN_REQUEST flags=ordered
+ bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=OPEN_REPLY flags=0
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:1 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=2 importance:2 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:3 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=4 importance:4 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:5 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=6 importance:6 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:7 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=8 importance:8 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REPLY flags=0
 vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
@@ -2882,23 +2882,23 @@ vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and ea
 vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
 vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=OPEN_REQUEST flags=ordered
+ bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=OPEN_REPLY flags=0
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:13 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:15 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
- bundle_id=0 flags=ordered
+ bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:17 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
- bundle_id=0 type=COMMIT_REQUEST flags=ordered
+ bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REPLY flags=0
 vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
@@ -2908,14 +2908,14 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
 vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05)
 vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
 vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4):
- importance=11, dl_vlan=1 actions=drop
  importance=2, dl_vlan=2 actions=drop
- importance=13, dl_vlan=3 actions=drop
  importance=4, dl_vlan=4 actions=drop
- importance=15, dl_vlan=5 actions=drop
  importance=6, dl_vlan=6 actions=drop
- importance=17, dl_vlan=7 actions=drop
  importance=8, dl_vlan=8 actions=drop
+ importance=11, dl_vlan=1 actions=drop
+ importance=13, dl_vlan=3 actions=drop
+ importance=15, dl_vlan=5 actions=drop
+ importance=17, dl_vlan=7 actions=drop
 ])
 
 OVS_VSWITCHD_STOP
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 1805a64..1f116c8 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -298,8 +298,8 @@ These commands manage the flow table in an OpenFlow switch.  In each
 case, \fIflow\fR specifies a flow entry in the format described in
 \fBFlow Syntax\fR, below, \fIfile\fR is a text file that contains zero
 or more flows in the same syntax, one per line, and the optional
-\fB\-\-bundle\fR option operates the command as a single transation,
-see command \fBbundle\fR, below.
+\fB\-\-bundle\fR option operates the command as a single atomic
+transation, see command \fBbundle\fR, below.
 .
 .IP "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch flow\fR"
 .IQ "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch \fB\- < \fIfile\fR"
@@ -352,9 +352,10 @@ The following applies to the flow table modification commands with the
 .RS
 .IP -
 Within a bundle, all flow mods are processed in the order they appear
-and as a single transaction, meaning that if one of them fails, the
-whole transaction fails and none of the changes are made to the
-\fIswitch\fR's flow table.  However, the \fIswitch\fR may perform
+and as a single atomic transaction, meaning that if one of them fails,
+the whole transaction fails and none of the changes are made to the
+\fIswitch\fR's flow table, and that no packet will see an intermediate
+version to the flow table.  However, the \fIswitch\fR may perform
 resource management, such as evicting flows, that may not be unrolled
 even when the transaction fails.
 .
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 9754a64..4bcc43f 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2337,7 +2337,7 @@ bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
         free(CONST_CAST(struct ofpact *, fm->ofpacts));
     }
 
-    bundle_transact(vconn, &requests, OFPBF_ORDERED);
+    bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
     vconn_close(vconn);
 }
 
@@ -2735,7 +2735,7 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx)
         }
     }
     if (bundle) {
-        bundle_transact(vconn, &requests, OFPBF_ORDERED);
+        bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
     } else {
         transact_multiple_noreply(vconn, &requests);
     }
-- 
1.7.10.4




More information about the dev mailing list