Use classifier versioning.

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.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

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 with a single OpenFlow
+     "flow_mod", 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
+       messages only.  Both 'atomic' and 'ordered' bundle flags are
-       'ordered' bundles are trivially supported, as all bundled
+       trivially supported, as all bundled messages are executed in
-       messages are executed in the order they were added to the
+       the order they were added and all flow table modifications are
-       bundle regardless of the presence of the 'ordered' flag.
+       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.
@@ -49,15 +54,15 @@ Post-v2.3.0
    - ovs-ofctl has a new '--bundle' option that makes the flow mod commands
      ('add-flow', 'add-flows', 'mod-flows', 'del-flows', and 'replace-flows')
      use an OpenFlow 1.4 bundle to operate the modifications as a single
-     transaction.  If any of the flow mods in a transaction fail, none of
+     atomic transaction.  If any of the flow mods in a transaction fail, none
-     them are executed.
+     of them are executed.  All flow mods in a bundle appear to datapath
+     lookups simultaneously.
    - ovs-ofctl 'add-flow' and 'add-flows' commands now accept arbitrary flow
      mods as an input by allowing the flow specification to start with an
      explicit 'add', 'modify', 'modify_strict', 'delete', or 'delete_strict'
      keyword.  A missing keyword is treated as 'add', so this is fully
      backwards compatible.  With the new '--bundle' option all the flow mods
-     are executed as a single transaction using the new OpenFlow 1.4 bundles
+     are executed as a single atomic transaction using an OpenFlow 1.4 bundle.
-     support.
    - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
      MD5 is no longer secure and some operating systems have started to disable
      it in OpenSSL.

lib/classifier.c

@@ -202,14 +202,24 @@ cls_rule_init_from_minimatch(struct cls_rule *rule,
     minimatch_clone(CONST_CAST(struct minimatch *, &rule->match), match);
 }
 
+/* Initializes 'dst' as a copy of 'src', but with 'version'.
+ *
+ * The caller must eventually destroy 'dst' with cls_rule_destroy(). */
+void
+cls_rule_clone_in_version(struct cls_rule *dst, const struct cls_rule *src,
+                          long long version)
+{
+    cls_rule_init__(dst, src->priority, version);
+    minimatch_clone(CONST_CAST(struct minimatch *, &dst->match), &src->match);
+}
+
 /* Initializes 'dst' as a copy of 'src'.
  *
  * The caller must eventually destroy 'dst' with cls_rule_destroy(). */
 void
 cls_rule_clone(struct cls_rule *dst, const struct cls_rule *src)
 {
-    cls_rule_init__(dst, src->priority, src->version);
+    cls_rule_clone_in_version(dst, src, src->version);
-    minimatch_clone(CONST_CAST(struct minimatch *, &dst->match), &src->match);
 }
 
 /* Initializes 'dst' with the data in 'src', destroying 'src'.

lib/classifier.h

@@ -367,6 +367,8 @@ void cls_rule_init(struct cls_rule *, const struct match *, int priority,
 void cls_rule_init_from_minimatch(struct cls_rule *, const struct minimatch *,
                                   int priority, long long version);
 void cls_rule_clone(struct cls_rule *, const struct cls_rule *);
+void cls_rule_clone_in_version(struct cls_rule *, const struct cls_rule *,
+        long long version);
 void cls_rule_move(struct cls_rule *dst, struct cls_rule *src);
 void cls_rule_destroy(struct cls_rule *);
 

lib/ofp-actions.c

@@ -3660,7 +3660,24 @@ format_RESUBMIT(const struct ofpact_resubmit *a, struct ds *s)
  * address.  This is not usually the intent in MAC learning; instead, we want
  * the MAC learn entry to expire when no traffic has been sent *from* the
  * learned address.  Use a hard timeout for that.
- */
+ *
+ *
+ * Visibility of Changes
+ * ---------------------
+ *
+ * Prior to Open vSwitch 2.4, any changes made by a "learn" action in a given
+ * flow translation are visible to flow table lookups made later in the flow
+ * translation.  This means that, in the example above, a MAC learned by the
+ * learn action in table 0 would be found in table 1 (if the packet being
+ * processed had the same source and destination MAC address).
+ *
+ * In Open vSwitch 2.4 and later, changes to a flow table (whether to add or
+ * modify a flow) by a "learn" action are visible only for later flow
+ * translations, not for later lookups within the same flow translation.  In
+ * the MAC learning example, a MAC learned by the learn action in table 0 would
+ * not be found in table 1 if the flow translation would resubmit to table 1
+ * after the processing of the learn action, meaning that if this MAC had not
+ * been learned before then the packet would be flooded. */
 struct nx_action_learn {
     ovs_be16 type;              /* OFPAT_VENDOR. */
     ovs_be16 len;               /* At least 24. */

ofproto/bundles.h

@@ -40,9 +40,8 @@ struct ofp_bundle_entry {
     };
 
     /* Used during commit. */
-    struct rule_collection rules;   /* Affected rules. */
+    struct rule_collection old_rules;   /* Affected rules. */
-    struct rule *rule;
+    struct rule_collection new_rules;   /* Affected rules. */
-    bool modify;
 
     /* OpenFlow header and some of the message contents for error reporting. */
     struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];

ofproto/ofproto-dpif.c

@@ -89,6 +89,15 @@ 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.
+    *
+    * Protected by stats_mutex.  If both 'rule->stats_mutex' and
+    * 'rule->new_rule->stats_mutex' must be held together, acquire them in that
+    * order, */
+    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
@@ -3668,9 +3677,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;
+    if (OVS_UNLIKELY(rule->new_rule)) {
-    rule->stats.n_bytes += stats->n_bytes;
+        rule_dpif_credit_stats(rule->new_rule, stats);
-    rule->stats.used = MAX(rule->stats.used, stats->used);
+    } 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);
 }
 
@@ -3722,11 +3735,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;
 }
 
@@ -3756,12 +3770,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'.
+/* Look up 'flow' in 'ofproto''s classifier version 'version', starting from
- * Returns the rule that was found, which may be one of the special rules
+ * table '*table_id'.  Returns the rule that was found, which may be one of the
- * according to packet miss hadling.  If 'may_packet_in' is false, returning of
+ * special rules according to packet miss hadling.  If 'may_packet_in' is
- * the miss_rule (which issues packet ins for the controller) is avoided.
+ * false, returning of the miss_rule (which issues packet ins for the
- * Updates 'wc', if nonnull, to reflect the fields that were used during the
+ * controller) is avoided.  Updates 'wc', if nonnull, to reflect the fields
- * lookup.
+ * 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
@@ -3927,17 +3941,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
+static void
-rule_insert(struct rule *rule_)
+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
@@ -3950,10 +3982,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);
     }
@@ -3966,9 +4003,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;
+    if (OVS_UNLIKELY(rule->new_rule)) {
-    *bytes = rule->stats.n_bytes;
+        rule_get_stats(&rule->new_rule->up, packets, bytes, used);
-    *used = rule->stats.used;
+    } else {
+        *packets = rule->stats.n_packets;
+        *bytes = rule->stats.n_bytes;
+        *used = rule->stats.used;
+    }
     ovs_mutex_unlock(&rule->stats_mutex);
 }
 
@@ -3990,22 +4031,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;
@@ -5550,8 +5575,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,

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.
@@ -323,6 +323,8 @@ struct rule {
     struct ofproto *const ofproto; /* The ofproto that contains this rule. */
     const struct cls_rule cr;      /* In owning ofproto's classifier. */
     const uint8_t table_id;        /* Index in ofproto's 'tables' array. */
+    bool removed;                  /* Rule has been removed from the ofproto
+                                    * data structures. */
 
     /* Protects members marked OVS_GUARDED.
      * Readers only need to hold this mutex.
@@ -361,7 +363,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);
@@ -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(),
+     * After a successful construction the rest of the rule life cycle calls
-     * which is also acceptable.
+     * 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).
+     * 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
-     * If ->rule_insert() fails, the ofproto base code will remove 'rule' from
+     * forward rule statistics from the 'old_rule' to the 'new_rule' if
-     * the flow table, destruct, uninitialize, and deallocate 'rule'.  See
+     * 'forward_stats' is 'true'.  This may not fail.
-     * "Rule Life Cycle" above for more details.
      *
      *
      * 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

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)
 ])
 

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

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,
+\fB\-\-bundle\fR option operates the command as a single atomic
-see option \fB\-\-bundle\fR, below.
+transation, see option \fB\-\-bundle\fR, below.
 .
 .IP "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch flow\fR"
 .IQ "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch \fB\- < \fIfile\fR"
@@ -2397,13 +2397,16 @@ depending on its configuration.
 Uses strict matching when running flow modification commands.
 .
 .IP "\fB\-\-bundle\fR"
-Execute flow mods as an OpenFlow 1.4 bundle transaction.
+Execute flow mods as an OpenFlow 1.4 atomic bundle transaction.
 .RS
 .IP \(bu
 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
+and as a single atomic transaction, meaning that if one of them fails,
-whole transaction fails and none of the changes are made to the
+the whole transaction fails and none of the changes are made to the
-\fIswitch\fR's flow table.
+\fIswitch\fR's flow table, and that each given datapath packet
+traversing the OpenFlow tables sees the flow tables either as before
+the transaction, or after all the flow mods in the bundle have been
+successfully applied.
 .IP \(bu
 The beginning and the end of the flow table modification commands in a
 bundle are delimited with OpenFlow 1.4 bundle control messages, which
@@ -2416,10 +2419,6 @@ Bundles require OpenFlow 1.4 or higher.  An explicit \fB-O
 OpenFlow14\fR option is not needed, but you may need to enable
 OpenFlow 1.4 support for OVS by setting the OVSDB \fIprotocols\fR
 column in the \fIbridge\fR table.
-.IP \(bu
-Current implementation executes all bundles with the 'ordered' flag,
-so that the flow mods are always executed in the order specified.
-Atomic bundles are not yet supported.
 .RE
 .
 .so lib/ofp-version.man

utilities/ovs-ofctl.c

@@ -1223,7 +1223,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);
 }
 
@@ -2700,7 +2700,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);
     }