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

Takashi Yamamoto yamamoto at midokura.com
Thu Oct 15 04:35:29 UTC 2015


hi,

On Wed, Jun 10, 2015 at 9:24 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> 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):

can you explain why this flow-stats reply is expected to be empty?
i'm seeing a test failure where it containing flows added in the
previous add-flows
on my environment.  do you have any idea?

> +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
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list