[ovs-dev] [PATCH v4 02/12] ovs-ofctl: Add bundle support and unit testing.

Jarno Rajahalme jrajahalme at nicira.com
Wed Jun 10 00:24:09 UTC 2015


All existing ovs-ofctl flow mod commands now take an optional
'--bundle' argument, which executes the flow mods as a single
transaction.  OpenFlow 1.4+ is implicitly assumed when '--bundle' is
specified.

ovs-ofctl 'add-flow' and 'add-flows' commands now accept flow
specifications that start with an optional 'add', 'modify', 'delete',
'modify_strict', or 'delete_strict' keyword, so that arbitrary flow
table modifications may be specified.  For backwards compatibility, a
missing keyword is treated as an 'add'.  With the new '--bundle'
option all the modifications are executed as a single transaction
using an OpenFlow 1.4 bundle.

OpenFlow 1.4 requires bundles to support at least flow and port mods.
This implementation does not yet support port mods in bundles.

Another restriction is that the atomic transactions are not yet
supported.

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 NEWS                        |   19 +++-
 include/openvswitch/vconn.h |    3 +
 lib/ofp-parse.c             |   40 ++++++-
 lib/ofp-parse.h             |    6 +-
 lib/ofp-util.c              |   30 ++++++
 lib/ofp-util.h              |    2 +
 lib/ofp-version-opt.c       |    7 ++
 lib/ofp-version-opt.h       |    1 +
 lib/vconn.c                 |  238 +++++++++++++++++++++++++++++++++++++----
 tests/ofproto-macros.at     |   10 ++
 tests/ofproto.at            |  244 +++++++++++++++++++++++++++++++++++++++++++
 tests/ovs-ofctl.at          |  107 +++++++++++++++++++
 utilities/ovs-ofctl.8.in    |   60 ++++++++---
 utilities/ovs-ofctl.c       |   88 +++++++++++++++-
 14 files changed, 813 insertions(+), 42 deletions(-)

diff --git a/NEWS b/NEWS
index a480607..5bea237 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,6 @@
 Post-v2.3.0
 ---------------------
-   - Added support for SFQ, FQ_CoDel and CoDel qdiscs. 
+   - 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.
    - The MAC learning feature now includes per-port fairness to mitigate
@@ -27,6 +27,11 @@ 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. '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.
      * 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.
@@ -41,6 +46,18 @@ Post-v2.3.0
      * A new Netronome extension to OpenFlow 1.5+ allows control over the
        fields hashed for OpenFlow select groups.  See "selection_method" and
        related options in ovs-ofctl(8) for details.
+   - 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
+     them are executed.
+   - 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
+     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.
diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
index 3b157e1..f8b6655 100644
--- a/include/openvswitch/vconn.h
+++ b/include/openvswitch/vconn.h
@@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **);
 int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
 int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
                                     struct ofpbuf **replyp);
+int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
+                          uint16_t bundle_flags,
+                          void (*error_reporter)(const struct ofp_header *));
 
 void vconn_run(struct vconn *);
 void vconn_run_wait(struct vconn *);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 6125f27..210feed 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -258,6 +258,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
 
     *usable_protocols = OFPUTIL_P_ANY;
 
+    if (command == -2) {
+        size_t len;
+
+        string += strspn(string, " \t\r\n");   /* Skip white space. */
+        len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
+
+        if (!strncmp(string, "add", len)) {
+            command = OFPFC_ADD;
+        } else if (!strncmp(string, "delete", len)) {
+            command = OFPFC_DELETE;
+        } else if (!strncmp(string, "delete_strict", len)) {
+            command = OFPFC_DELETE_STRICT;
+        } else if (!strncmp(string, "modify", len)) {
+            command = OFPFC_MODIFY;
+        } else if (!strncmp(string, "modify_strict", len)) {
+            command = OFPFC_MODIFY_STRICT;
+        } else {
+            len = 0;
+            command = OFPFC_ADD;
+        }
+        string += len;
+    }
+
     switch (command) {
     case -1:
         fields = F_OUT_PORT;
@@ -486,6 +509,10 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
  * constant for 'command'.  To parse syntax for an OFPST_FLOW or
  * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for 'command'.
  *
+ * If 'command' is given as -2, 'str_' may begin with a command name ("add",
+ * "modify", "delete", "modify_strict", or "delete_strict").  A missing command
+ * name is treated as "add".
+ *
  * Returns NULL if successful, otherwise a malloc()'d string describing the
  * error.  The caller is responsible for freeing the returned string. */
 char * OVS_WARN_UNUSED_RESULT
@@ -818,14 +845,19 @@ parse_flow_monitor_request(struct ofputil_flow_monitor_request *fmr,
 /* Parses 'string' as an OFPT_FLOW_MOD or NXT_FLOW_MOD with command 'command'
  * (one of OFPFC_*) into 'fm'.
  *
+ * If 'command' is given as -2, 'string' may begin with a command name ("add",
+ * "modify", "delete", "modify_strict", or "delete_strict").  A missing command
+ * name is treated as "add".
+ *
  * Returns NULL if successful, otherwise a malloc()'d string describing the
  * error.  The caller is responsible for freeing the returned string. */
 char * OVS_WARN_UNUSED_RESULT
 parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string,
-                       uint16_t command,
+                       int command,
                        enum ofputil_protocol *usable_protocols)
 {
     char *error = parse_ofp_str(fm, command, string, usable_protocols);
+
     if (!error) {
         /* Normalize a copy of the match.  This ensures that non-normalized
          * flows get logged but doesn't affect what gets sent to the switch, so
@@ -883,10 +915,14 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm, const char *table_id,
  * type (one of OFPFC_*).  Stores each flow_mod in '*fm', an array allocated
  * on the caller's behalf, and the number of flow_mods in '*n_fms'.
  *
+ * If 'command' is given as -2, each line may start with a command name
+ * ("add", "modify", "delete", "modify_strict", or "delete_strict").  A missing
+ * command name is treated as "add".
+ *
  * Returns NULL if successful, otherwise a malloc()'d string describing the
  * error.  The caller is responsible for freeing the returned string. */
 char * OVS_WARN_UNUSED_RESULT
-parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
+parse_ofp_flow_mod_file(const char *file_name, int command,
                         struct ofputil_flow_mod **fms, size_t *n_fms,
                         enum ofputil_protocol *usable_protocols)
 {
diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h
index db30f43..f112603 100644
--- a/lib/ofp-parse.h
+++ b/lib/ofp-parse.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 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.
@@ -42,7 +42,7 @@ char *parse_ofp_str(struct ofputil_flow_mod *, int command, const char *str_,
     OVS_WARN_UNUSED_RESULT;
 
 char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
-                             uint16_t command,
+                             int command,
                              enum ofputil_protocol *usable_protocols)
     OVS_WARN_UNUSED_RESULT;
 
@@ -51,7 +51,7 @@ char *parse_ofp_table_mod(struct ofputil_table_mod *,
                           enum ofputil_protocol *usable_protocols)
     OVS_WARN_UNUSED_RESULT;
 
-char *parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
+char *parse_ofp_flow_mod_file(const char *file_name, int command,
                               struct ofputil_flow_mod **fms, size_t *n_fms,
                               enum ofputil_protocol *usable_protocols)
     OVS_WARN_UNUSED_RESULT;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9004b8d..89359c1 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -8696,6 +8696,36 @@ ofputil_decode_bundle_ctrl(const struct ofp_header *oh,
 }
 
 struct ofpbuf *
+ofputil_encode_bundle_ctrl_request(enum ofp_version ofp_version,
+                                   struct ofputil_bundle_ctrl_msg *bc)
+{
+    struct ofpbuf *request;
+    struct ofp14_bundle_ctrl_msg *m;
+
+    switch (ofp_version) {
+    case OFP10_VERSION:
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+    case OFP13_VERSION:
+        ovs_fatal(0, "bundles need OpenFlow 1.4 or later "
+                     "(\'-O OpenFlow14\')");
+    case OFP14_VERSION:
+    case OFP15_VERSION:
+        request = ofpraw_alloc(OFPRAW_OFPT14_BUNDLE_CONTROL, ofp_version, 0);
+        m = ofpbuf_put_zeros(request, sizeof *m);
+
+        m->bundle_id = htonl(bc->bundle_id);
+        m->type = htons(bc->type);
+        m->flags = htons(bc->flags);
+        break;
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    return request;
+}
+
+struct ofpbuf *
 ofputil_encode_bundle_ctrl_reply(const struct ofp_header *oh,
                                  struct ofputil_bundle_ctrl_msg *msg)
 {
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 549f118..596c2e2 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -1114,6 +1114,8 @@ enum ofptype;
 enum ofperr ofputil_decode_bundle_ctrl(const struct ofp_header *,
                                        struct ofputil_bundle_ctrl_msg *);
 
+struct ofpbuf *ofputil_encode_bundle_ctrl_request(enum ofp_version,
+                                                  struct ofputil_bundle_ctrl_msg *);
 struct ofpbuf *ofputil_encode_bundle_ctrl_reply(const struct ofp_header *,
                                                 struct ofputil_bundle_ctrl_msg *);
 
diff --git a/lib/ofp-version-opt.c b/lib/ofp-version-opt.c
index 46fb45a..1cf57e1 100644
--- a/lib/ofp-version-opt.c
+++ b/lib/ofp-version-opt.c
@@ -27,6 +27,13 @@ mask_allowed_ofp_versions(uint32_t bitmap)
 }
 
 void
+add_allowed_ofp_versions(uint32_t bitmap)
+{
+    assert_single_threaded();
+    allowed_versions |= bitmap;
+}
+
+void
 ofp_version_usage(void)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
diff --git a/lib/ofp-version-opt.h b/lib/ofp-version-opt.h
index 6bf5eed..82b4ccc 100644
--- a/lib/ofp-version-opt.h
+++ b/lib/ofp-version-opt.h
@@ -21,6 +21,7 @@
 uint32_t get_allowed_ofp_versions(void);
 void set_allowed_ofp_versions(const char *string);
 void mask_allowed_ofp_versions(uint32_t);
+void add_allowed_ofp_versions(uint32_t);
 void ofp_version_usage(void);
 
 #endif
diff --git a/lib/vconn.c b/lib/vconn.c
index c033b48..fbe16b3 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 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.
@@ -744,18 +744,15 @@ vconn_recv_block(struct vconn *vconn, struct ofpbuf **msgp)
     return retval;
 }
 
-/* Waits until a message with a transaction ID matching 'xid' is received on
- * 'vconn'.  Returns 0 if successful, in which case the reply is stored in
- * '*replyp' for the caller to examine and free.  Otherwise returns a positive
- * errno value, or EOF, and sets '*replyp' to null.
- *
- * 'request' is always destroyed, regardless of the return value. */
-int
-vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp)
+static int
+vconn_recv_xid__(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp,
+                 void (*error_reporter)(const struct ofp_header *))
 {
     for (;;) {
         ovs_be32 recv_xid;
         struct ofpbuf *reply;
+        const struct ofp_header *oh;
+        enum ofptype type;
         int error;
 
         error = vconn_recv_block(vconn, &reply);
@@ -763,19 +760,54 @@ vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp)
             *replyp = NULL;
             return error;
         }
-        recv_xid = ((struct ofp_header *) reply->data)->xid;
+        oh = reply->data;
+        recv_xid = oh->xid;
         if (xid == recv_xid) {
             *replyp = reply;
             return 0;
         }
 
-        VLOG_DBG_RL(&bad_ofmsg_rl, "%s: received reply with xid %08"PRIx32
-                    " != expected %08"PRIx32,
-                    vconn->name, ntohl(recv_xid), ntohl(xid));
+        error = ofptype_decode(&type, oh);
+        if (!error && type == OFPTYPE_ERROR && error_reporter) {
+            error_reporter(oh);
+        } else {
+            VLOG_DBG_RL(&bad_ofmsg_rl, "%s: received reply with xid %08"PRIx32
+                        " != expected %08"PRIx32,
+                        vconn->name, ntohl(recv_xid), ntohl(xid));
+        }
         ofpbuf_delete(reply);
     }
 }
 
+/* Waits until a message with a transaction ID matching 'xid' is received on
+ * 'vconn'.  Returns 0 if successful, in which case the reply is stored in
+ * '*replyp' for the caller to examine and free.  Otherwise returns a positive
+ * errno value, or EOF, and sets '*replyp' to null.
+ *
+ * 'request' is always destroyed, regardless of the return value. */
+int
+vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp)
+{
+    return vconn_recv_xid__(vconn, xid, replyp, NULL);
+}
+
+static int
+vconn_transact__(struct vconn *vconn, struct ofpbuf *request,
+                 struct ofpbuf **replyp,
+                 void (*error_reporter)(const struct ofp_header *))
+{
+    ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid;
+    int error;
+
+    *replyp = NULL;
+    error = vconn_send_block(vconn, request);
+    if (error) {
+        ofpbuf_delete(request);
+    }
+    return error ? error : vconn_recv_xid__(vconn, send_xid, replyp,
+                                            error_reporter);
+}
+
 /* Sends 'request' to 'vconn' and blocks until it receives a reply with a
  * matching transaction ID.  Returns 0 if successful, in which case the reply
  * is stored in '*replyp' for the caller to examine and free.  Otherwise
@@ -790,15 +822,7 @@ int
 vconn_transact(struct vconn *vconn, struct ofpbuf *request,
                struct ofpbuf **replyp)
 {
-    ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid;
-    int error;
-
-    *replyp = NULL;
-    error = vconn_send_block(vconn, request);
-    if (error) {
-        ofpbuf_delete(request);
-    }
-    return error ? error : vconn_recv_xid(vconn, send_xid, replyp);
+    return vconn_transact__(vconn, request, replyp, NULL);
 }
 
 /* Sends 'request' followed by a barrier request to 'vconn', then blocks until
@@ -897,6 +921,176 @@ vconn_transact_multiple_noreply(struct vconn *vconn, struct ovs_list *requests,
     return 0;
 }
 
+static enum ofperr
+vconn_bundle_reply_validate(struct ofpbuf *reply, enum ofp_version version,
+                            struct ofputil_bundle_ctrl_msg *request,
+                            void (*error_reporter)(const struct ofp_header *))
+{
+    const struct ofp_header *oh;
+    enum ofptype type;
+    enum ofperr error;
+    struct ofputil_bundle_ctrl_msg rbc;
+
+    oh = reply->data;
+    if (oh->version != version) {
+        return OFPERR_OFPBRC_BAD_VERSION;
+    }
+
+    error = ofptype_decode(&type, oh);
+    if (error) {
+        return error;
+    }
+
+    if (type == OFPTYPE_ERROR) {
+        error_reporter(oh);
+        error = ofperr_decode_msg(oh, NULL);
+        return error;
+    }
+    if (type != OFPTYPE_BUNDLE_CONTROL) {
+        return OFPERR_OFPBRC_BAD_TYPE;
+    }
+
+    error = ofputil_decode_bundle_ctrl(oh, &rbc);
+    if (error) {
+        return error;
+    }
+
+    if (rbc.bundle_id != request->bundle_id) {
+        return OFPERR_OFPBFC_BAD_ID;
+    }
+
+    if (rbc.type != request->type + 1) {
+        return OFPERR_OFPBFC_BAD_TYPE;
+    }
+
+    return 0;
+}
+
+static int
+vconn_bundle_control_transact(struct vconn *vconn,
+                              struct ofputil_bundle_ctrl_msg *bc,
+                              uint16_t type,
+                              void (*error_reporter)(const struct ofp_header *))
+{
+    struct ofpbuf *request, *reply;
+    int error;
+    enum ofperr ofperr;
+
+    bc->type = type;
+    request = ofputil_encode_bundle_ctrl_request(vconn->version, bc);
+    ofpmsg_update_length(request);
+    error = vconn_transact__(vconn, request, &reply, error_reporter);
+    if (error) {
+        return error;
+    }
+
+    ofperr = vconn_bundle_reply_validate(reply, vconn->version, bc,
+                                         error_reporter);
+    if (ofperr) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "Bundle %s failed (%s).",
+                     type == OFPBCT_OPEN_REQUEST ? "open"
+                     : type == OFPBCT_CLOSE_REQUEST ? "close"
+                     : type == OFPBCT_COMMIT_REQUEST ? "commit"
+                     : type == OFPBCT_DISCARD_REQUEST ? "discard"
+                     : "control message",
+                     ofperr_to_string(ofperr));
+    }
+    ofpbuf_delete(reply);
+
+    return ofperr ? EPROTO : 0;
+}
+
+/* Checks if error responses can be received on
+ * 'vconn'. */
+static void
+vconn_recv_error(struct vconn *vconn,
+                 void (*error_reporter)(const struct ofp_header *))
+{
+    struct ofpbuf *reply;
+    int error;
+
+    error = vconn_recv(vconn, &reply);
+    if (!error) {
+        const struct ofp_header *oh;
+        enum ofptype type;
+        enum ofperr ofperr;
+
+        oh = reply->data;
+        ofperr = ofptype_decode(&type, oh);
+        if (!ofperr && type == OFPTYPE_ERROR) {
+            error_reporter(oh);
+        } else {
+            VLOG_DBG_RL(&bad_ofmsg_rl,
+                        "%s: received unexpected reply with xid %08"PRIx32,
+                        vconn->name, ntohl(oh->xid));
+        }
+        ofpbuf_delete(reply);
+    }
+}
+
+static int
+vconn_bundle_add_msg(struct vconn *vconn, struct ofputil_bundle_ctrl_msg *bc,
+                     struct ofpbuf *msg,
+                     void (*error_reporter)(const struct ofp_header *))
+{
+    struct ofputil_bundle_add_msg bam;
+    struct ofpbuf *request;
+    int error;
+
+    bam.bundle_id = bc->bundle_id;
+    bam.flags = bc->flags;
+    bam.msg = msg->data;
+
+    request = ofputil_encode_bundle_add(vconn->version, &bam);
+    ofpmsg_update_length(request);
+
+    error = vconn_send_block(vconn, request);
+    if (!error) {
+        /* Check for an error return, so that the socket buffer does not become
+         * full of errors. */
+        vconn_recv_error(vconn, error_reporter);
+    }
+    return error;
+}
+
+int
+vconn_bundle_transact(struct vconn *vconn, struct ovs_list *requests,
+                      uint16_t flags,
+                      void (*error_reporter)(const struct ofp_header *))
+{
+    struct ofputil_bundle_ctrl_msg bc;
+    struct ofpbuf *request;
+    int error;
+
+    memset(&bc, 0, sizeof bc);
+    bc.flags = flags;
+    error = vconn_bundle_control_transact(vconn, &bc, OFPBCT_OPEN_REQUEST,
+                                          error_reporter);
+    if (error) {
+        return error;
+    }
+
+    LIST_FOR_EACH (request, list_node, requests) {
+        error = vconn_bundle_add_msg(vconn, &bc, request, error_reporter);
+        if (error) {
+            break;
+        }
+    }
+
+    if (!error) {
+        error = vconn_bundle_control_transact(vconn, &bc,
+                                              OFPBCT_COMMIT_REQUEST,
+                                              error_reporter);
+    } else {
+        /* Do not overwrite the error code from vconn_bundle_add_msg().
+         * Any error in discard should be either reported or logged, so it
+         * should not get lost. */
+        vconn_bundle_control_transact(vconn, &bc, OFPBCT_DISCARD_REQUEST,
+                                      error_reporter);
+    }
+    return error;
+}
+
 void
 vconn_wait(struct vconn *vconn, enum vconn_wait_type wait)
 {
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index fd915ef..65bfe40 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -15,6 +15,16 @@ s/ hard_age=[0-9]*,//
 '
 }
 
+# Filter (multiline) vconn debug messages from ovs-vswitchd.log.
+# Use with ofctl_strip()
+print_vconn_debug () { awk -F\| < ovs-vswitchd.log '
+BEGIN { prt=0 }
+/\|vconn\|DBG\|/ { sub(/[ \t]*$/, ""); print $3 "|" $4 "|" $5; prt=1; next }
+$4 != "" { prt=0; next }
+prt==1 { sub(/[ \t]*$/, ""); print $0 }
+'
+}
+
 # parse_listening_port [SERVER]
 #
 # Parses the TCP or SSL port on which a server is listening from the
diff --git a/tests/ofproto.at b/tests/ofproto.at
index be1b298..1fa5b2d 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -3450,3 +3450,247 @@ OFPT_BARRIER_REPLY (OF1.4):
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ofproto - bundle with multiple flow mods (OpenFlow 1.4)])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-appctl vlog/set vconn:dbg])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+
+AT_DATA([flows.txt], [dnl
+add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=1
+add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=2
+add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=3
+add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=4
+delete
+add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=5
+add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=6
+add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=7
+delete in_port=2 dl_src=00:88:99:aa:bb:cc
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:5
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:6
+NXST_FLOW reply:
+])
+
+AT_DATA([flows.txt], [dnl
+modify actions=drop
+modify_strict in_port=2 dl_src=00:77:88:99:aa:bb actions=7
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:7
+NXST_FLOW reply:
+])
+
+# Adding an existing flow acts as a modify, and delete_strict also works.
+AT_DATA([flows.txt], [dnl
+add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=8
+delete_strict in_port=2 dl_src=00:66:77:88:99:aa
+add in_port=2 dl_src=00:66:77:88:99:aa actions=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:8
+ in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+NXST_FLOW reply:
+])
+
+dnl Check logs for OpenFlow trace
+# Prevent race.
+OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success): NXST_FLOW reply" ovs-vswitchd.log | wc -l` -ge 3])
+AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl
+vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+vconn|DBG|unix: received: OFPT_HELLO:
+ version bitmap: 0x01
+vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 and earlier, peer supports version 0x01)
+vconn|DBG|unix: received: OFPT_FLOW_MOD: DEL actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST:
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY:
+vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+vconn|DBG|unix: received: OFPT_HELLO:
+ version bitmap: 0x01
+vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 and earlier, peer supports version 0x01)
+vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST:
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY:
+vconn|DBG|unix: received: NXST_FLOW request:
+vconn|DBG|unix: sent (Success): NXST_FLOW reply:
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:5
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:6
+vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+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
+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
+OFPT_FLOW_MOD (OF1.4): MOD actions=drop
+vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
+ bundle_id=0 flags=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
+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):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+vconn|DBG|unix: received: OFPT_HELLO:
+ version bitmap: 0x01
+vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 and earlier, peer supports version 0x01)
+vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST:
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY:
+vconn|DBG|unix: received: NXST_FLOW request:
+vconn|DBG|unix: sent (Success): NXST_FLOW reply:
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:7
+vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+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
+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
+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
+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
+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
+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):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+vconn|DBG|unix: received: OFPT_HELLO:
+ version bitmap: 0x01
+vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 and earlier, peer supports version 0x01)
+vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST:
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY:
+vconn|DBG|unix: received: NXST_FLOW request: 
+vconn|DBG|unix: sent (Success): NXST_FLOW reply:
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:8
+ in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto - failing bundle commit (OpenFlow 1.4)])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-ofctl del-flows br0])
+
+ovs-ofctl add-flows br0 - <<EOF
+idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=11
+idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=22
+idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=33
+EOF
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:11
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:22
+ idle_timeout=70, in_port=2,dl_src=00:88:99:aa:bb:cc actions=output:33
+NXST_FLOW reply:
+])
+
+# last line uses illegal table number (OVS internal table)
+AT_DATA([flows.txt], [dnl
+add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=1
+add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=2
+add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=3
+modify idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=4
+delete
+add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=5
+add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=6
+add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=7
+delete in_port=2 dl_src=00:88:99:aa:bb:cc
+add table=254 actions=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt 2>&1 | sed '/|WARN|/d
+s/unix:.*br0\.mgmt/unix:br0.mgmt/'], [0], [dnl
+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
+ovs-ofctl: talking to unix:br0.mgmt (Protocol error)
+])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:11
+ idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:22
+ idle_timeout=70, in_port=2,dl_src=00:88:99:aa:bb:cc actions=output:33
+NXST_FLOW reply:
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 1e12827..b7db9bb 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2813,3 +2813,110 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLO
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ovs-ofctl replace-flows with --bundle])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-appctl vlog/set vconn:dbg])
+
+dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle. For more details refer "ovs-ofctl rule with importance" test case.
+for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt
+AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt])
+
+dnl Replace some flows in the bridge.
+for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10`,actions=drop"; done > replace-flows.txt
+AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt])
+
+dnl Dump them and compare the dump flows output against the expected output.
+for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then importance=`expr $i + 10`; else importance=$i; fi; echo " importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLOW/d' | sort],
+  [0], [expout])
+
+dnl Check logs for OpenFlow trace
+# Prevent race.
+OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success): OFPST_FLOW reply" ovs-vswitchd.log | wc -l` -ge 2])
+# AT_CHECK([sed -n "s/^.*\(|vconn|DBG|.*xid=.*:\).*$/\1/p" ovs-vswitchd.log], [0], [dnl
+AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl
+vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+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: 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
+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
+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
+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
+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
+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
+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):
+ version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
+vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
+ version bitmap: 0x05
+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
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index c667aa4..2c6a073 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -296,29 +296,39 @@ Print meter features.
 .
 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, and \fIfile\fR is a text file that contains
-zero or more flows in the same syntax, one per line.
-.
-.IP "\fBadd\-flow \fIswitch flow\fR"
-.IQ "\fBadd\-flow \fIswitch \fB\- < \fIfile\fR"
-.IQ "\fBadd\-flows \fIswitch file\fR"
+\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 option \fB\-\-bundle\fR, below.
+.
+.IP "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch flow\fR"
+.IQ "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch \fB\- < \fIfile\fR"
+.IQ "[\fB\-\-bundle\fR] \fBadd\-flows \fIswitch file\fR"
 Add each flow entry to \fIswitch\fR's tables.
 .
-.IP "[\fB\-\-strict\fR] \fBmod\-flows \fIswitch flow\fR"
-.IQ "[\fB\-\-strict\fR] \fBmod\-flows \fIswitch \fB\- < \fIfile\fR"
+Each flow specification (e.g., each line in \fIfile\fR) may start with
+\fBadd\fR, \fBmodify\fR, \fBdelete\fR, \fBmodify_strict\fR, or
+\fBdelete_strict\fR keyword to specify whether a flow is to be added,
+modified, or deleted, and whether the modify or delete is strict or
+not.  For backwards compatibility a flow specification without one of
+these keywords is treated as a flow add.  All flow mods are executed
+in the order specified.
+.
+.IP "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBmod\-flows \fIswitch flow\fR"
+.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBmod\-flows \fIswitch \fB\- < \fIfile\fR"
 Modify the actions in entries from \fIswitch\fR's tables that match
 the specified flows.  With \fB\-\-strict\fR, wildcards are not treated
 as active for matching purposes.
 .
-.IP "\fBdel\-flows \fIswitch\fR"
-.IQ "[\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fR[\fIflow\fR]"
-.IQ "[\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fB\- < \fIfile\fR"
+.IP "[\fB\-\-bundle\fR] \fBdel\-flows \fIswitch\fR"
+.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fR[\fIflow\fR]"
+.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fB\- < \fIfile\fR"
 Deletes entries from \fIswitch\fR's flow table.  With only a
 \fIswitch\fR argument, deletes all flows.  Otherwise, deletes flow
 entries that match the specified flows.  With \fB\-\-strict\fR,
 wildcards are not treated as active for matching purposes.
 .
-.IP "[\fB\-\-readd\fR] \fBreplace\-flows \fIswitch file\fR"
+.IP "[\fB\-\-bundle\fR] [\fB\-\-readd\fR] \fBreplace\-flows \fIswitch file\fR"
 Reads flow entries from \fIfile\fR (or \fBstdin\fR if \fIfile\fR is
 \fB\-\fR) and queries the flow table from \fIswitch\fR.  Then it fixes
 up any differences, adding flows from \fIflow\fR that are missing on
@@ -2386,6 +2396,32 @@ depending on its configuration.
 \fB\-\-strict\fR
 Uses strict matching when running flow modification commands.
 .
+.IP "\fB\-\-bundle\fR"
+Execute flow mods as an OpenFlow 1.4 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
+whole transaction fails and none of the changes are made to the
+\fIswitch\fR's flow table.
+.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
+makes it possible to stream the included commands without explicit
+OpenFlow barriers, which are otherwise used after each flow table
+modification command.  This may make large modifications execute
+faster as a bundle.
+.IP \(bu
+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
 .
 .IP "\fB\-F \fIformat\fR[\fB,\fIformat\fR...]"
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 54a5bb8..be0f149 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -67,6 +67,12 @@
 
 VLOG_DEFINE_THIS_MODULE(ofctl);
 
+/* --bundle: Use OpenFlow 1.4 bundle for making the flow table change atomic.
+ * NOTE: Also the flow mod will use OpenFlow 1.4, so the semantics may be
+ * different (see the comment in parse_options() for details).
+ */
+static bool bundle = false;
+
 /* --strict: Use strict matching for flow mod commands?  Additionally governs
  * use of nx_pull_match() instead of nx_pull_match_loose() in parse-nx-match.
  */
@@ -159,6 +165,7 @@ parse_options(int argc, char *argv[])
         OPT_SORT,
         OPT_RSORT,
         OPT_UNIXCTL,
+        OPT_BUNDLE,
         DAEMON_OPTION_ENUMS,
         OFP_VERSION_OPTION_ENUMS,
         VLOG_OPTION_ENUMS
@@ -176,6 +183,7 @@ parse_options(int argc, char *argv[])
         {"unixctl",     required_argument, NULL, OPT_UNIXCTL},
         {"help", no_argument, NULL, 'h'},
         {"option", no_argument, NULL, 'o'},
+        {"bundle", no_argument, NULL, OPT_BUNDLE},
         DAEMON_LONG_OPTIONS,
         OFP_VERSION_LONG_OPTIONS,
         VLOG_LONG_OPTIONS,
@@ -249,6 +257,10 @@ parse_options(int argc, char *argv[])
             ovs_cmdl_print_options(long_options);
             exit(EXIT_SUCCESS);
 
+        case OPT_BUNDLE:
+            bundle = true;
+            break;
+
         case OPT_STRICT:
             strict = true;
             break;
@@ -293,6 +305,12 @@ parse_options(int argc, char *argv[])
 
     free(short_options);
 
+    /* Implicit OpenFlow 1.4 with the '--bundle' option. */
+    if (bundle) {
+        /* Add implicit allowance for OpenFlow 1.4. */
+        add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap(
+                                     OFPUTIL_P_OF14_OXM));
+    }
     versions = get_allowed_ofp_versions();
     version_protocols = ofputil_protocols_from_version_bitmap(versions);
     if (!(allowed_protocols & version_protocols)) {
@@ -602,6 +620,26 @@ transact_multiple_noreply(struct vconn *vconn, struct ovs_list *requests)
     ofpbuf_delete(reply);
 }
 
+static void
+bundle_error_reporter(const struct ofp_header *oh)
+{
+    ofp_print(stderr, oh, ntohs(oh->length), verbosity + 1);
+    fflush(stderr);
+}
+
+static void
+bundle_transact(struct vconn *vconn, struct ovs_list *requests, uint16_t flags)
+{
+    struct ofpbuf *request;
+
+    LIST_FOR_EACH (request, list_node, requests) {
+        ofpmsg_update_length(request);
+    }
+
+    run(vconn_bundle_transact(vconn, requests, flags, bundle_error_reporter),
+        "talking to %s", vconn_get_name(vconn));
+}
+
 /* Sends 'request', which should be a request that only has a reply if an error
  * occurs, and waits for it to succeed or fail.  If an error does occur, prints
  * it and exits with an error.
@@ -1175,6 +1213,10 @@ open_vconn_for_flow_mod(const char *remote, struct vconn **vconnp,
 }
 
 static void
+bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
+                        size_t n_fms, enum ofputil_protocol);
+
+static void
 ofctl_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
                  size_t n_fms, enum ofputil_protocol usable_protocols)
 {
@@ -1182,6 +1224,11 @@ ofctl_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
     struct vconn *vconn;
     size_t i;
 
+    if (bundle) {
+        bundle_flow_mod__(remote, fms, n_fms, usable_protocols);
+        return;
+    }
+
     protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols);
 
     for (i = 0; i < n_fms; i++) {
@@ -1194,13 +1241,19 @@ ofctl_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
 }
 
 static void
-ofctl_flow_mod_file(int argc OVS_UNUSED, char *argv[], uint16_t command)
+ofctl_flow_mod_file(int argc OVS_UNUSED, char *argv[], int command)
 {
     enum ofputil_protocol usable_protocols;
     struct ofputil_flow_mod *fms = NULL;
     size_t n_fms = 0;
     char *error;
 
+    if (command == OFPFC_ADD) {
+        /* Allow the file to specify a mix of commands.  If none specified at
+         * the beginning of any given line, then the default is OFPFC_ADD, so
+         * this is backwards compatible. */
+        command = -2;
+    }
     error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms,
                                     &usable_protocols);
     if (error) {
@@ -2262,6 +2315,33 @@ ofctl_dump_group_features(struct ovs_cmdl_context *ctx)
 }
 
 static void
+bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
+                  size_t n_fms, enum ofputil_protocol usable_protocols)
+{
+    enum ofputil_protocol protocol;
+    struct vconn *vconn;
+    struct ovs_list requests;
+    size_t i;
+
+    list_init(&requests);
+
+    /* Bundles need OpenFlow 1.4+. */
+    usable_protocols &= OFPUTIL_P_OF14_UP;
+    protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols);
+
+    for (i = 0; i < n_fms; i++) {
+        struct ofputil_flow_mod *fm = &fms[i];
+        struct ofpbuf *request = ofputil_encode_flow_mod(fm, protocol);
+
+        list_push_back(&requests, &request->list_node);
+        free(CONST_CAST(struct ofpact *, fm->ofpacts));
+    }
+
+    bundle_transact(vconn, &requests, OFPBF_ORDERED);
+    vconn_close(vconn);
+}
+
+static void
 ofctl_help(struct ovs_cmdl_context *ctx OVS_UNUSED)
 {
     usage();
@@ -2636,7 +2716,11 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx)
             fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol, &requests);
         }
     }
-    transact_multiple_noreply(vconn, &requests);
+    if (bundle) {
+        bundle_transact(vconn, &requests, OFPBF_ORDERED);
+    } else {
+        transact_multiple_noreply(vconn, &requests);
+    }
     vconn_close(vconn);
 
     fte_free_all(&cls);
-- 
1.7.10.4




More information about the dev mailing list