ofproto: Support port mods in bundles.
authorJarno Rajahalme <jrajahalme@nicira.com>
Fri, 12 Jun 2015 23:12:56 +0000 (16:12 -0700)
committerJarno Rajahalme <jrajahalme@nicira.com>
Fri, 12 Jun 2015 23:12:56 +0000 (16:12 -0700)
Add support for port mods in an OpenFlow 1.4 bundle, as required for
the minimum support level by the OpenFlow 1.4 specification.  If the
bundle includes port mods, it may not specify the OFPBF_ATOMIC flag.
Port mods and flow mods in a bundle are always applied in order and
the consecutive flow mods between port mods are made available to
lookups atomically.

Note that ovs-ofctl does not support creating bundles with port mods.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
DESIGN.md
NEWS
OPENFLOW-1.1+.md
ofproto/bundles.h
ofproto/ofproto-provider.h
ofproto/ofproto.c

index 4d94c2d..e533b7c 100644 (file)
--- a/DESIGN.md
+++ b/DESIGN.md
@@ -280,11 +280,60 @@ OpenFlow 1.4
 ------------
 
 OpenFlow 1.4 adds the "importance" field to flow_mods, but it does not
-explicitly specify which kinds of flow_mods set the importance.For
+explicitly specify which kinds of flow_mods set the importance.  For
 consistency, Open vSwitch uses the same rule for importance as for
 idle_timeout and hard_timeout, that is, only an "ADD" flow_mod sets
 the importance.  (This issue has been filed with the ONF as EXT-496.)
 
+
+OpenFlow 1.4 Bundles
+====================
+
+Open vSwitch makes all flow table modifications atomically, i.e., any
+datapath packet only sees flow table configurations either before or
+after any change made by any flow_mod.  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.
+
+It should be noted that Open vSwitch caches datapath flows, and that
+the cached flows are NOT flushed immediately when a flow table
+changes.  Instead, the datapath flows are revalidated against the new
+flow table as soon as possible, and usually within one second of the
+modification.  This design amortizes the cost of datapath cache
+flushing across multiple flow table changes, and has a significant
+performance effect during simultaneous heavy flow table churn and high
+traffic load.  This means that different cached datapath flows may
+have been computed based on a different flow table configurations, but
+each of the datapath flows is guaranteed to have been computed over a
+coherent view of the flow tables, as described above.
+
+With OpenFlow 1.4 bundles this atomicity can be extended across an
+arbitrary set of flow_mods.  Bundles are supported for flow_mod and
+port_mod messages only.  For flow_mods, 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.  Port mods may not appear in atomic
+bundles, as port status modifications are not atomic.
+
+To support bundles, ovs-ofctl has a '--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 atomic transaction.  If any of the flow mods
+in a transaction fail, none of them are executed.  All flow mods in a
+bundle appear to datapath lookups simultaneously.
+
+Furthermore, 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 atomic transaction using an
+OpenFlow 1.4 bundle.  Without the '--bundle' option the flow mods are
+executed in order up to the first failing flow_mod, and in case of an
+error the earlier successful flow_mods are not rolled back.
+
+
 OFPT_PACKET_IN
 ==============
 
@@ -844,7 +893,7 @@ not know the MAC address of the local port that is sending the traffic
 or the MAC address of the remote in the guest VM.
 
 With a few notable exceptions below, in-band should work in most
-network setups.  The following are considered "supported' in the
+network setups.  The following are considered "supported" in the
 current implementation:
 
  - Locally Connected.  The switch and remote are on the same
diff --git a/NEWS b/NEWS
index a3eeed5..90d9a29 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -32,11 +32,12 @@ Post-v2.3.0
      commands are now redundant and will be removed in a future
      release.  See ovs-vswitchd(8) for details.
    - OpenFlow:
-     * OpenFlow 1.4 bundles are now supported, but for flow mod
-       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.
+     * OpenFlow 1.4 bundles are now supported for flow mods and port
+       mods.  For flow mods, 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.  Port mods may not appear in
+       atomic bundles, as port status modifications are not atomic.
      * 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.
index 7911406..0b2f0f9 100644 (file)
@@ -148,6 +148,12 @@ parallel in OVS.
     Transactional modification.  OpenFlow 1.4 requires to support
     flow_mods and port_mods in a bundle if bundle is supported.
     (Not related to OVS's 'ofbundle' stuff.)
+    Implemented as an OpenFlow 1.4 feature.  Only flow_mods and
+    port_mods are supported in a bundle.  If the bundle includes port
+    mods, it may not specify the OFPBF_ATOMIC flag.  Nevertheless,
+    port mods and flow mods in a bundle are always applied in order
+    and consecutive flow mods between port mods are made available to
+    lookups atomically.
     [EXT-230]
     [optional for OF1.4+]
 
index 0c7daf2..65717df 100644 (file)
@@ -34,14 +34,17 @@ extern "C" {
 struct ofp_bundle_entry {
     struct ovs_list   node;
     enum ofptype      type;  /* OFPTYPE_FLOW_MOD or OFPTYPE_PORT_MOD. */
+    long long         version;          /* Version in which the changes take
+                                         * effect. */
     union {
         struct ofputil_flow_mod fm;   /* 'fm.ofpacts' must be malloced. */
         struct ofputil_port_mod pm;
     };
 
     /* Used during commit. */
+    struct ofport *port;                /* Affected port. */
     struct rule_collection old_rules;   /* Affected rules. */
-    struct rule_collection new_rules;   /* Affected rules. */
+    struct rule_collection new_rules;   /* Replacement 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))];
@@ -80,6 +83,7 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh)
     struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
 
     entry->type = type;
+    entry->version = 0;
 
     /* Max 64 bytes for error reporting. */
     memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg));
index 0d25f68..527823a 100644 (file)
@@ -93,8 +93,8 @@ struct ofproto {
     long long int eviction_group_timer; /* For rate limited reheapification. */
     struct oftable *tables;
     int n_tables;
-    cls_version_t tables_version;   /* Controls which rules are visible to
-                                     * table lookups. */
+    cls_version_t tables_version;  /* Controls which rules are visible to
+                                    * table lookups. */
 
     /* Rules indexed on their cookie values, in all flow tables. */
     struct hindex cookies OVS_GUARDED_BY(ofproto_mutex);
index cf5fa34..08ba043 100644 (file)
@@ -3433,9 +3433,34 @@ update_port_config(struct ofconn *ofconn, struct ofport *port,
 }
 
 static enum ofperr
-handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+port_mod_start(struct ofconn *ofconn, struct ofputil_port_mod *pm,
+               struct ofport **port)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
+
+    *port = ofproto_get_port(p, pm->port_no);
+    if (!*port) {
+        return OFPERR_OFPPMFC_BAD_PORT;
+    }
+    if (!eth_addr_equals((*port)->pp.hw_addr, pm->hw_addr)) {
+        return OFPERR_OFPPMFC_BAD_HW_ADDR;
+    }
+    return 0;
+}
+
+static void
+port_mod_finish(struct ofconn *ofconn, struct ofputil_port_mod *pm,
+                struct ofport *port)
+{
+    update_port_config(ofconn, port, pm->config, pm->mask);
+    if (pm->advertise) {
+        netdev_set_advertisements(port->netdev, pm->advertise);
+    }
+}
+
+static enum ofperr
+handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+{
     struct ofputil_port_mod pm;
     struct ofport *port;
     enum ofperr error;
@@ -3450,18 +3475,11 @@ handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
         return error;
     }
 
-    port = ofproto_get_port(p, pm.port_no);
-    if (!port) {
-        return OFPERR_OFPPMFC_BAD_PORT;
-    } else if (!eth_addr_equals(port->pp.hw_addr, pm.hw_addr)) {
-        return OFPERR_OFPPMFC_BAD_HW_ADDR;
-    } else {
-        update_port_config(ofconn, port, pm.config, pm.mask);
-        if (pm.advertise) {
-            netdev_set_advertisements(port->netdev, pm.advertise);
-        }
+    error = port_mod_start(ofconn, &pm, &port);
+    if (!error) {
+        port_mod_finish(ofconn, &pm, port);
     }
-    return 0;
+    return error;
 }
 
 static enum ofperr
@@ -6668,17 +6686,16 @@ do_bundle_flow_mod_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
  * possible.  No visible changes were made, so rollback is minimal (remove
  * added invisible rules, restore visibility of rules marked for removal).
  *
- * 3. Bump the version visible to lookups.
- *
- * 4. Finish: Insert replacement rules to the ofproto provider. Remove replaced
- * and deleted rules from ofproto data structures, and Schedule postponed
- * removal of deleted rules from the classifier.  Send notifications, buffered
- * packets, etc.
+ * 3. Finish: Make the changes visible for lookups. Insert replacement rules to
+ * the ofproto provider. Remove replaced and deleted rules from ofproto data
+ * structures, and Schedule postponed removal of deleted rules from the
+ * classifier.  Send notifications, buffered packets, etc.
  */
 static enum ofperr
 do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    cls_version_t visible_version = ofproto->tables_version;
     struct ofp_bundle *bundle;
     struct ofp_bundle_entry *be;
     enum ofperr error;
@@ -6691,23 +6708,42 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
     if (bundle->flags != flags) {
         error = OFPERR_OFPBFC_BAD_FLAGS;
     } else {
+        bool prev_is_port_mod = false;
+
         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. */
-                error = OFPERR_OFPBFC_MSG_FAILED;
+                /* Our port mods are not atomic. */
+                if (flags & OFPBF_ATOMIC) {
+                    error = OFPERR_OFPBFC_MSG_FAILED;
+                } else {
+                    prev_is_port_mod = true;
+                    error = port_mod_start(ofconn, &be->pm, &be->port);
+                }
             } else if (be->type == OFPTYPE_FLOW_MOD) {
+                /* Flow mods between port mods are applied as a single
+                 * version, but the versions are published only after
+                 * we know the commit is successful. */
+                if (prev_is_port_mod) {
+                    ++ofproto->tables_version;
+                }
+                prev_is_port_mod = false;
                 error = do_bundle_flow_mod_start(ofproto, &be->fm, be);
             } else {
                 OVS_NOT_REACHED();
             }
             if (error) {
                 break;
+            } else {
+                /* Store the version in which the changes should take
+                 * effect. */
+                be->version = ofproto->tables_version + 1;
             }
         }
+
         if (error) {
             /* Send error referring to the original message. */
             if (error) {
@@ -6720,24 +6756,38 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
                 if (be->type == OFPTYPE_FLOW_MOD) {
                     do_bundle_flow_mod_revert(ofproto, &be->fm, be);
                 }
+                /* Nothing needs to be reverted for a port mod. */
             }
         } else {
-            /* 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) {
+                /* Bump the lookup version to the one of the current message.
+                 * This makes all the changes in the bundle at this version
+                 * visible to lookups at once. */
+                if (visible_version < be->version) {
+                    visible_version = be->version;
+                    ofproto->ofproto_class->set_tables_version(
+                        ofproto, visible_version);
+                }
                 if (be->type == OFPTYPE_FLOW_MOD) {
                     struct flow_mod_requester req = { ofconn, be->ofp_msg };
 
                     do_bundle_flow_mod_finish(ofproto, &be->fm, &req, be);
+                } else if (be->type == OFPTYPE_PORT_MOD) {
+                    /* Perform the actual port mod. This is not atomic, i.e.,
+                     * the effects will be immediately seen by upcall
+                     * processing regardless of the lookup version.  It should
+                     * be noted that port configuration changes can originate
+                     * also from OVSDB changes asynchronously to all upcall
+                     * processing. */
+                    port_mod_finish(ofconn, &be->pm, be->port);
                 }
             }
         }
+
+        /* Reset the tables_version. */
+        ofproto->tables_version = visible_version;
+
         ofmonitor_flush(ofproto->connmgr);
         ovs_mutex_unlock(&ofproto_mutex);