ofproto: Support group mods in bundles.

Allow adding group mods in OpenFlow bundles.  Group mods are executed
atomically with any flow mods in the same bundle.  Mods are executed
in order, so that groups appearing in flow actions need to be inserted
in to the bundle before the dependent flow mods.

ovs-ofctl is enhanced to allow the '--bundle' option with group mod
commands.  add-groups file format is enhanced to allow each line to be
preceded by one of the keywords "add", "modify", "delete",
"add_or_mod", "insert_bucket", or "remove_bucket".

ovs-ofctl also has a new "bundle" command that reads a file in which
each line contains one flow mod or group mod, and then executes them
all as a single atomic bundle transaction.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

2 files changed, 132 insertions, 18 deletions

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 7021b7699..84057c0e9 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -424,15 +424,25 @@ was generated by the switch itself.
 These commands manage the group table in an OpenFlow switch.  In each
 case, \fIgroup\fR specifies a group entry in the format described in
 \fBGroup Syntax\fR, below, and \fIfile\fR is a text file that contains
-zero or more groups in the same syntax, one per line.
+zero or more groups in the same syntax, one per line, and the optional
+\fB\-\-bundle\fR option operates the command as a single atomic
+transation, see option \fB\-\-bundle\fR, below.
 
-.IP "\fBadd\-group \fIswitch group\fR"
-.IQ "\fBadd\-group \fIswitch \fB\- < \fIfile\fR"
-.IQ "\fBadd\-groups \fIswitch file\fR"
+.IP "[\fB\-\-bundle\fR] \fBadd\-group \fIswitch group\fR"
+.IQ "[\fB\-\-bundle\fR] \fBadd\-group \fIswitch \fB\- < \fIfile\fR"
+.IQ "[\fB\-\-bundle\fR] \fBadd\-groups \fIswitch file\fR"
 Add each group entry to \fIswitch\fR's tables.
 .
-.IP "[\fB\-\-may\-create\fR] \fBmod\-group \fIswitch group\fR"
-.IQ "[\fB\-\-may\-create\fR] \fBmod\-group \fIswitch \fB\- < \fIfile\fR"
+Each group specification (e.g., each line in \fIfile\fR) may start
+with \fBadd\fR, \fBmodify\fR, \fBadd_or_mod\fR, \fBdelete\fR,
+\fBinsert_bucket\fR, or \fBremove_bucket\fR keyword to specify whether
+a flow is to be added, modified, or deleted, or whether a group bucket
+is to be added or removed.  For backwards compatibility a group
+specification without one of these keywords is treated as a group add.
+All group mods are executed in the order specified.
+.
+.IP "[\fB\-\-bundle\fR] [\fB\-\-may\-create\fR] \fBmod\-group \fIswitch group\fR"
+.IQ "[\fB\-\-bundle\fR] [\fB\-\-may\-create\fR] \fBmod\-group \fIswitch \fB\- < \fIfile\fR"
 Modify the action buckets in entries from \fIswitch\fR's tables for
 each group entry.  If a specified group does not already exist, then
 without \fB\-\-may\-create\fR, this command has no effect; with
@@ -440,25 +450,45 @@ without \fB\-\-may\-create\fR, this command has no effect; with
 \fB\-\-may\-create\fR option uses an Open vSwitch extension to
 OpenFlow only implemented in Open vSwitch 2.6 and later.
 .
-.IP "\fBdel\-groups \fIswitch\fR"
-.IQ "\fBdel\-groups \fIswitch \fR[\fIgroup\fR]"
-.IQ "\fBdel\-groups \fIswitch \fB\- < \fIfile\fR"
+.IP "[\fB\-\-bundle\fR] \fBdel\-groups \fIswitch\fR"
+.IQ "[\fB\-\-bundle\fR] \fBdel\-groups \fIswitch \fR[\fIgroup\fR]"
+.IQ "[\fB\-\-bundle\fR] \fBdel\-groups \fIswitch \fB\- < \fIfile\fR"
 Deletes entries from \fIswitch\fR's group table.  With only a
 \fIswitch\fR argument, deletes all groups.  Otherwise, deletes the group
 for each group entry.
 .
-.IP "\fBinsert\-buckets \fIswitch group\fR"
-.IQ "\fBinsert\-buckets \fIswitch \fB\- < \fIfile\fR"
+.IP "[\fB\-\-bundle\fR] \fBinsert\-buckets \fIswitch group\fR"
+.IQ "[\fB\-\-bundle\fR] \fBinsert\-buckets \fIswitch \fB\- < \fIfile\fR"
 Add buckets to an existing group present in the \fIswitch\fR's group table.
 If no \fIcommand_bucket_id\fR is present in the group specification then all
 buckets of the group are removed.
 .
-.IP "\fBremove\-buckets \fIswitch group\fR"
-.IQ "\fBremove\-buckets \fIswitch \fB\- < \fIfile\fR"
+.IP "[\fB\-\-bundle\fR] \fBremove\-buckets \fIswitch group\fR"
+.IQ "[\fB\-\-bundle\fR] \fBremove\-buckets \fIswitch \fB\- < \fIfile\fR"
 Remove buckets to an existing group present in the \fIswitch\fR's group table.
 If no \fIcommand_bucket_id\fR is present in the group specification then all
 buckets of the group are removed.
 .
+.SS OpenFlow Switch Bundle Command
+.
+Transactional updates to both flow and group tables can be made with
+the \fBbundle\fR command.  \fIfile\fR is a text file that contains
+zero or more flows and groups in either \fBFlow Syntax\fR or \fBGroup
+Syntax\fR, each line preceded by either \fBflow\fR or \fBgroup\fR
+keyword.  The \fBflow\fR keyword may be optionally followed by one of
+the keywords \fBadd\fR, \fBmodify\fR, \fBmodify_strict\fR,
+\fBdelete\fR, or \fBdelete_strict\fR, of which the \fBadd\fR is
+assumed if a bare \fBflow\fR is given.  Similarly, the \fBgroup\fR
+keyword may be optionally followed by one of the keywords \fBadd\fR,
+\fBmodify\fR, \fBadd_or_mod\fR, \fBdelete\fR, \fBinsert_bucket\fR, or
+\fBremove_bucket\fR, of which the \fBadd\fR is assumed if a bare
+\fBgroup\fR is given.
+.
+.IP "\fBbundle \fIswitch file\fR"
+Execute all flow and group mods in \fIfile\fR as a single atomic
+transaction against \fIswitch\fR's tables.  All bundled mods are
+executed in the order specified.
+.
 .SS "OpenFlow Switch Tunnel TLV Table Commands"
 .
 Open vSwitch maintains a mapping table between tunnel option TLVs (defined
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 124e88495..27c5d3b47 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2505,6 +2505,36 @@ ofctl_dump_ipfix_flow(struct ovs_cmdl_context *ctx)
 }
 
 static void
+bundle_group_mod__(const char *remote, struct ofputil_group_mod *gms,
+                   size_t n_gms, enum ofputil_protocol usable_protocols)
+{
+    enum ofputil_protocol protocol;
+    enum ofp_version version;
+    struct vconn *vconn;
+    struct ovs_list requests;
+    size_t i;
+
+    ovs_list_init(&requests);
+
+    /* Bundles need OpenFlow 1.3+. */
+    usable_protocols &= OFPUTIL_P_OF13_UP;
+    protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols);
+    version = ofputil_protocol_to_ofp_version(protocol);
+
+    for (i = 0; i < n_gms; i++) {
+        struct ofputil_group_mod *gm = &gms[i];
+        struct ofpbuf *request = ofputil_encode_group_mod(version, gm);
+
+        ovs_list_push_back(&requests, &request->list_node);
+        ofputil_uninit_group_mod(gm);
+    }
+
+    bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
+    ofpbuf_list_delete(&requests);
+    vconn_close(vconn);
+}
+
+static void
 ofctl_group_mod__(const char *remote, struct ofputil_group_mod *gms,
                   size_t n_gms, enum ofputil_protocol usable_protocols)
 {
@@ -2516,30 +2546,38 @@ ofctl_group_mod__(const char *remote, struct ofputil_group_mod *gms,
     struct vconn *vconn;
     size_t i;
 
+    if (bundle) {
+        bundle_group_mod__(remote, gms, n_gms, usable_protocols);
+        return;
+    }
+
     protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols);
     version = ofputil_protocol_to_ofp_version(protocol);
 
     for (i = 0; i < n_gms; i++) {
         gm = &gms[i];
         request = ofputil_encode_group_mod(version, gm);
-        if (request) {
-            transact_noreply(vconn, request);
-        }
+        transact_noreply(vconn, request);
         ofputil_uninit_group_mod(gm);
     }
 
     vconn_close(vconn);
 }
 
-
 static void
-ofctl_group_mod_file(int argc OVS_UNUSED, char *argv[], uint16_t command)
+ofctl_group_mod_file(int argc OVS_UNUSED, char *argv[], int command)
 {
     struct ofputil_group_mod *gms = NULL;
     enum ofputil_protocol usable_protocols;
     size_t n_gms = 0;
     char *error;
 
+    if (command == OFPGC11_ADD) {
+        /* Allow the file to specify a mix of commands.  If none specified at
+         * the beginning of any given line, then the default is OFPGC11_ADD, so
+         * this is backwards compatible. */
+        command = -2;
+    }
     error = parse_ofp_group_mod_file(argv[2], command, &gms, &n_gms,
                                      &usable_protocols);
     if (error) {
@@ -2674,6 +2712,48 @@ ofctl_dump_group_features(struct ovs_cmdl_context *ctx)
 }
 
 static void
+ofctl_bundle(struct ovs_cmdl_context *ctx)
+{
+    enum ofputil_protocol protocol, usable_protocols;
+    struct ofputil_bundle_msg *bms;
+    struct ovs_list requests;
+    struct vconn *vconn;
+    size_t n_bms;
+    char *error;
+
+    error = parse_ofp_bundle_file(ctx->argv[2], &bms, &n_bms,
+                                  &usable_protocols);
+    if (error) {
+        ovs_fatal(0, "%s", error);
+    }
+
+    /* Implicit OpenFlow 1.4. */
+    if (!(get_allowed_ofp_versions() &
+          ofputil_protocols_to_version_bitmap(OFPUTIL_P_OF13_UP))) {
+
+        /* Add implicit allowance for OpenFlow 1.4. */
+        add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap(
+                                     OFPUTIL_P_OF14_OXM));
+        /* Remove all versions that do not support bundles. */
+        mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap(
+                                     OFPUTIL_P_OF13_UP));
+        allowed_protocols = ofputil_protocols_from_version_bitmap(
+                                     get_allowed_ofp_versions());
+    }
+
+    /* Bundles need OpenFlow 1.3+. */
+    usable_protocols &= OFPUTIL_P_OF13_UP;
+    protocol = open_vconn_for_flow_mod(ctx->argv[1], &vconn, usable_protocols);
+
+    ovs_list_init(&requests);
+    ofputil_encode_bundle_msgs(bms, n_bms, &requests, protocol);
+    bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
+    ofpbuf_list_delete(&requests);
+
+    vconn_close(vconn);
+}
+
+static void
 ofctl_tlv_mod(struct ovs_cmdl_context *ctx, uint16_t command)
 {
     enum ofputil_protocol usable_protocols;
@@ -4139,6 +4219,10 @@ static const struct ovs_cmdl_command all_commands[] = {
       1, 2, ofctl_dump_group_stats },
     { "dump-group-features", "switch",
       1, 1, ofctl_dump_group_features },
+
+    { "bundle", "switch file",
+      2, 2, ofctl_bundle },
+
     { "add-tlv-map", "switch map",
       2, 2, ofctl_add_tlv_map },
     { "del-tlv-map", "switch [map]",