ofproto: Add NXM_NX_TUN_GBP_ID and NXM_NX_TUN_GBP_FLAGS
[cascardo/ovs.git] / lib / ofp-util.c
index d765d03..5437339 100644 (file)
@@ -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.
@@ -32,6 +32,7 @@
 #include "multipath.h"
 #include "netdev.h"
 #include "nx-match.h"
+#include "id-pool.h"
 #include "ofp-actions.h"
 #include "ofp-errors.h"
 #include "ofp-msgs.h"
@@ -41,7 +42,7 @@
 #include "random.h"
 #include "unaligned.h"
 #include "type-props.h"
-#include "vlog.h"
+#include "openvswitch/vlog.h"
 #include "bitmap.h"
 
 VLOG_DEFINE_THIS_MODULE(ofp_util);
@@ -107,7 +108,7 @@ ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property,
     return ofputil_pull_property__(msg, property, 8, typep);
 }
 
-static void PRINTF_FORMAT(2, 3)
+static void OVS_PRINTF_FORMAT(2, 3)
 log_property(bool loose, const char *message, ...)
 {
     enum vlog_level level = loose ? VLL_DBG : VLL_WARN;
@@ -185,7 +186,7 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
 void
 ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
 {
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
 
     /* Initialize most of wc. */
     flow_wildcards_init_catchall(wc);
@@ -1700,6 +1701,11 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
 
         fm->idle_timeout = ntohs(ofm->idle_timeout);
         fm->hard_timeout = ntohs(ofm->hard_timeout);
+        if (oh->version >= OFP14_VERSION && ofm->command == OFPFC_ADD) {
+            fm->importance = ntohs(ofm->importance);
+        } else {
+            fm->importance = 0;
+        }
         fm->buffer_id = ntohl(ofm->buffer_id);
         error = ofputil_port_from_ofp11(ofm->out_port, &fm->out_port);
         if (error) {
@@ -1738,6 +1744,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             fm->new_cookie = ofm->cookie;
             fm->idle_timeout = ntohs(ofm->idle_timeout);
             fm->hard_timeout = ntohs(ofm->hard_timeout);
+            fm->importance = 0;
             fm->buffer_id = ntohl(ofm->buffer_id);
             fm->out_port = u16_to_ofp(ntohs(ofm->out_port));
             fm->out_group = OFPG11_ANY;
@@ -1765,6 +1772,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             fm->new_cookie = nfm->cookie;
             fm->idle_timeout = ntohs(nfm->idle_timeout);
             fm->hard_timeout = ntohs(nfm->hard_timeout);
+            fm->importance = 0;
             fm->buffer_id = ntohl(nfm->buffer_id);
             fm->out_port = u16_to_ofp(ntohs(nfm->out_port));
             fm->out_group = OFPG11_ANY;
@@ -1961,7 +1969,7 @@ ofputil_put_bands(uint16_t n_bands, const struct ofputil_meter_band *mb,
 
 /* Encode a meter stat for 'mc' and append it to 'replies'. */
 void
-ofputil_append_meter_config(struct list *replies,
+ofputil_append_meter_config(struct ovs_list *replies,
                             const struct ofputil_meter_config *mc)
 {
     struct ofpbuf *msg = ofpbuf_from_list(list_back(replies));
@@ -1979,7 +1987,7 @@ ofputil_append_meter_config(struct list *replies,
 
 /* Encode a meter stat for 'ms' and append it to 'replies'. */
 void
-ofputil_append_meter_stats(struct list *replies,
+ofputil_append_meter_stats(struct ovs_list *replies,
                            const struct ofputil_meter_stats *ms)
 {
     struct ofp13_meter_stats *reply;
@@ -2248,6 +2256,11 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
         ofm->out_port = ofputil_port_to_ofp11(fm->out_port);
         ofm->out_group = htonl(fm->out_group);
         ofm->flags = raw_flags;
+        if (version >= OFP14_VERSION && fm->command == OFPFC_ADD) {
+            ofm->importance = htons(fm->importance);
+        } else {
+            ofm->importance = 0;
+        }
         ofputil_put_ofp11_match(msg, &fm->match, protocol);
         ofpacts_put_openflow_instructions(fm->ofpacts, fm->ofpacts_len, msg,
                                           version);
@@ -2834,6 +2847,11 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         fs->duration_nsec = ntohl(ofs->duration_nsec);
         fs->idle_timeout = ntohs(ofs->idle_timeout);
         fs->hard_timeout = ntohs(ofs->hard_timeout);
+        if (oh->version >= OFP14_VERSION) {
+            fs->importance = ntohs(ofs->importance);
+        } else {
+            fs->importance = 0;
+        }
         if (raw == OFPRAW_OFPST13_FLOW_REPLY) {
             error = ofputil_decode_flow_mod_flags(ofs->flags, -1, oh->version,
                                                   &fs->flags);
@@ -2875,6 +2893,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         fs->duration_nsec = ntohl(ofs->duration_nsec);
         fs->idle_timeout = ntohs(ofs->idle_timeout);
         fs->hard_timeout = ntohs(ofs->hard_timeout);
+        fs->importance = 0;
         fs->idle_age = -1;
         fs->hard_age = -1;
         fs->packet_count = ntohll(get_32aligned_be64(&ofs->packet_count));
@@ -2910,6 +2929,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         fs->priority = ntohs(nfs->priority);
         fs->idle_timeout = ntohs(nfs->idle_timeout);
         fs->hard_timeout = ntohs(nfs->hard_timeout);
+        fs->importance = 0;
         fs->idle_age = -1;
         fs->hard_age = -1;
         if (flow_age_extension) {
@@ -2953,7 +2973,7 @@ unknown_to_zero(uint64_t count)
  * have been initialized with ofpmp_init(). */
 void
 ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
-                                struct list *replies)
+                                struct ovs_list *replies)
 {
     struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
     size_t start_ofs = ofpbuf_size(reply);
@@ -2977,6 +2997,11 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
         ofs->priority = htons(fs->priority);
         ofs->idle_timeout = htons(fs->idle_timeout);
         ofs->hard_timeout = htons(fs->hard_timeout);
+        if (version >= OFP14_VERSION) {
+            ofs->importance = htons(fs->importance);
+        } else {
+            ofs->importance = 0;
+        }
         if (raw == OFPRAW_OFPST13_FLOW_REPLY) {
             ofs->flags = ofputil_encode_flow_mod_flags(fs->flags, version);
         } else {
@@ -3273,6 +3298,8 @@ ofputil_decode_packet_in_finish(struct ofputil_packet_in *pin,
     pin->fmd.tun_id = match->flow.tunnel.tun_id;
     pin->fmd.tun_src = match->flow.tunnel.ip_src;
     pin->fmd.tun_dst = match->flow.tunnel.ip_dst;
+    pin->fmd.gbp_id = match->flow.tunnel.gbp_id;
+    pin->fmd.gbp_flags = match->flow.tunnel.gbp_flags;
     pin->fmd.metadata = match->flow.metadata;
     memcpy(pin->fmd.regs, match->flow.regs, sizeof pin->fmd.regs);
     pin->fmd.pkt_mark = match->flow.pkt_mark;
@@ -3398,6 +3425,12 @@ ofputil_packet_in_to_match(const struct ofputil_packet_in *pin,
     if (pin->fmd.tun_dst != htonl(0)) {
         match_set_tun_dst(match, pin->fmd.tun_dst);
     }
+    if (pin->fmd.gbp_id != htons(0)) {
+        match_set_tun_gbp_id(match, pin->fmd.gbp_id);
+    }
+    if (pin->fmd.gbp_flags) {
+        match_set_tun_gbp_flags(match, pin->fmd.gbp_flags);
+    }
     if (pin->fmd.metadata != htonll(0)) {
         match_set_metadata(match, pin->fmd.metadata);
     }
@@ -3502,7 +3535,7 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
         packet_in_size = sizeof (struct ofp12_packet_in);
     } else {
         packet_in_raw = OFPRAW_OFPT13_PACKET_IN;
-        packet_in_version = OFP13_VERSION;
+        packet_in_version = ofputil_protocol_to_ofp_version(protocol);
         packet_in_size = sizeof (struct ofp13_packet_in);
     }
 
@@ -3522,7 +3555,7 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
     opi->pi.total_len = htons(pin->total_len);
     opi->pi.reason = pin->reason;
     opi->pi.table_id = pin->table_id;
-    if (protocol == OFPUTIL_P_OF13_OXM) {
+    if (protocol != OFPUTIL_P_OF12_OXM) {
         opi->cookie = pin->cookie;
     }
 
@@ -3581,6 +3614,12 @@ ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason reason,
         return "action";
     case OFPR_INVALID_TTL:
         return "invalid_ttl";
+    case OFPR_ACTION_SET:
+        return "action_set";
+    case OFPR_GROUP:
+        return "group";
+    case OFPR_PACKET_OUT:
+        return "packet_out";
 
     case OFPR_N_REASONS:
     default:
@@ -4023,7 +4062,7 @@ ofputil_encode_port_desc_stats_request(enum ofp_version ofp_version,
 
 void
 ofputil_append_port_desc_stats_reply(const struct ofputil_phy_port *pp,
-                                     struct list *replies)
+                                     struct ovs_list *replies)
 {
     struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
     size_t start_ofs = ofpbuf_size(reply);
@@ -4046,7 +4085,7 @@ BUILD_ASSERT_DECL((int) OFPUTIL_C_ARP_MATCH_IP == OFPC_ARP_MATCH_IP);
 static uint32_t
 ofputil_capabilities_mask(enum ofp_version ofp_version)
 {
-    /* Handle capabilities whose bit is unique for all Open Flow versions */
+    /* Handle capabilities whose bit is unique for all OpenFlow versions */
     switch (ofp_version) {
     case OFP10_VERSION:
     case OFP11_VERSION:
@@ -4844,7 +4883,7 @@ put_table_instruction_features(
 
 void
 ofputil_append_table_features_reply(const struct ofputil_table_features *tf,
-                                    struct list *replies)
+                                    struct ovs_list *replies)
 {
     struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
     enum ofp_version version = ofpmp_version(replies);
@@ -5778,7 +5817,7 @@ ofputil_encode_flow_monitor_cancel(uint32_t id)
 }
 
 void
-ofputil_start_flow_update(struct list *replies)
+ofputil_start_flow_update(struct ovs_list *replies)
 {
     struct ofpbuf *msg;
 
@@ -5791,7 +5830,7 @@ ofputil_start_flow_update(struct list *replies)
 
 void
 ofputil_append_flow_update(const struct ofputil_flow_update *update,
-                           struct list *replies)
+                           struct ovs_list *replies)
 {
     enum ofp_version version = ofpmp_version(replies);
     struct nx_flow_update_header *nfuh;
@@ -6020,7 +6059,8 @@ ofputil_port_to_ofp11(ofp_port_t ofp10_port)
         OFPUTIL_NAMED_PORT(ALL)                 \
         OFPUTIL_NAMED_PORT(CONTROLLER)          \
         OFPUTIL_NAMED_PORT(LOCAL)               \
-        OFPUTIL_NAMED_PORT(ANY)
+        OFPUTIL_NAMED_PORT(ANY)                 \
+        OFPUTIL_NAMED_PORT(UNSET)
 
 /* For backwards compatibility, so that "none" is recognized as OFPP_ANY */
 #define OFPUTIL_NAMED_PORTS_WITH_NONE           \
@@ -6424,7 +6464,7 @@ ofputil_parse_key_value(char **stringp, char **keyp, char **valuep)
 }
 
 /* Encode a dump ports request for 'port', the encoded message
- * will be for Open Flow version 'ofp_version'. Returns message
+ * will be for OpenFlow version 'ofp_version'. Returns message
  * as a struct ofpbuf. Returns encoded message on success, NULL on error */
 struct ofpbuf *
 ofputil_encode_dump_ports_request(enum ofp_version ofp_version, ofp_port_t port)
@@ -6508,7 +6548,7 @@ ofputil_port_stats_to_ofp13(const struct ofputil_port_stats *ops,
 
 static void
 ofputil_append_ofp14_port_stats(const struct ofputil_port_stats *ops,
-                                struct list *replies)
+                                struct ovs_list *replies)
 {
     struct ofp14_port_stats_prop_ethernet *eth;
     struct ofp14_port_stats *ps14;
@@ -6543,7 +6583,7 @@ ofputil_append_ofp14_port_stats(const struct ofputil_port_stats *ops,
 
 /* Encode a ports stat for 'ops' and append it to 'replies'. */
 void
-ofputil_append_port_stat(struct list *replies,
+ofputil_append_port_stat(struct ovs_list *replies,
                          const struct ofputil_port_stats *ops)
 {
     switch (ofpmp_version(replies)) {
@@ -6837,7 +6877,7 @@ ofputil_decode_port_stats_request(const struct ofp_header *request,
 
 /* Frees all of the "struct ofputil_bucket"s in the 'buckets' list. */
 void
-ofputil_bucket_list_destroy(struct list *buckets)
+ofputil_bucket_list_destroy(struct ovs_list *buckets)
 {
     struct ofputil_bucket *bucket, *next_bucket;
 
@@ -6848,6 +6888,107 @@ ofputil_bucket_list_destroy(struct list *buckets)
     }
 }
 
+/* Clones 'bucket' and its ofpacts data */
+static struct ofputil_bucket *
+ofputil_bucket_clone_data(const struct ofputil_bucket *bucket)
+{
+    struct ofputil_bucket *new;
+
+    new = xmemdup(bucket, sizeof *bucket);
+    new->ofpacts = xmemdup(bucket->ofpacts, bucket->ofpacts_len);
+
+    return new;
+}
+
+/* Clones each of the buckets in the list 'src' appending them
+ * in turn to 'dest' which should be an initialised list.
+ * An exception is that if the pointer value of a bucket in 'src'
+ * matches 'skip' then it is not cloned or appended to 'dest'.
+ * This allows all of 'src' or 'all of 'src' except 'skip' to
+ * be cloned and appended to 'dest'. */
+void
+ofputil_bucket_clone_list(struct ovs_list *dest, const struct ovs_list *src,
+                          const struct ofputil_bucket *skip)
+{
+    struct ofputil_bucket *bucket;
+
+    LIST_FOR_EACH (bucket, list_node, src) {
+        struct ofputil_bucket *new_bucket;
+
+        if (bucket == skip) {
+            continue;
+        }
+
+        new_bucket = ofputil_bucket_clone_data(bucket);
+        list_push_back(dest, &new_bucket->list_node);
+    }
+}
+
+/* Find a bucket in the list 'buckets' whose bucket id is 'bucket_id'
+ * Returns the first bucket found or NULL if no buckets are found. */
+struct ofputil_bucket *
+ofputil_bucket_find(const struct ovs_list *buckets, uint32_t bucket_id)
+{
+    struct ofputil_bucket *bucket;
+
+    if (bucket_id > OFPG15_BUCKET_MAX) {
+        return NULL;
+    }
+
+    LIST_FOR_EACH (bucket, list_node, buckets) {
+        if (bucket->bucket_id == bucket_id) {
+            return bucket;
+        }
+    }
+
+    return NULL;
+}
+
+/* Returns true if more than one bucket in the list 'buckets'
+ * have the same bucket id. Returns false otherwise. */
+bool
+ofputil_bucket_check_duplicate_id(const struct ovs_list *buckets)
+{
+    struct ofputil_bucket *i, *j;
+
+    LIST_FOR_EACH (i, list_node, buckets) {
+        LIST_FOR_EACH_REVERSE (j, list_node, buckets) {
+            if (i == j) {
+                break;
+            }
+            if (i->bucket_id == j->bucket_id) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
+/* Returns the bucket at the front of the list 'buckets'.
+ * Undefined if 'buckets is empty. */
+struct ofputil_bucket *
+ofputil_bucket_list_front(const struct ovs_list *buckets)
+{
+    static struct ofputil_bucket *bucket;
+
+    ASSIGN_CONTAINER(bucket, list_front(buckets), list_node);
+
+    return bucket;
+}
+
+/* Returns the bucket at the back of the list 'buckets'.
+ * Undefined if 'buckets is empty. */
+struct ofputil_bucket *
+ofputil_bucket_list_back(const struct ovs_list *buckets)
+{
+    static struct ofputil_bucket *bucket;
+
+    ASSIGN_CONTAINER(bucket, list_back(buckets), list_node);
+
+    return bucket;
+}
+
 /* Returns an OpenFlow group stats request for OpenFlow version 'ofp_version',
  * that requests stats for group 'group_id'.  (Use OFPG_ALL to request stats
  * for all groups.)
@@ -6981,7 +7122,7 @@ ofputil_group_stats_to_ofp13(const struct ofputil_group_stats *gs,
  * replies already begun in 'replies' and appends it to the list.  'replies'
  * must have originally been initialized with ofpmp_init(). */
 void
-ofputil_append_group_stats(struct list *replies,
+ofputil_append_group_stats(struct ovs_list *replies,
                            const struct ofputil_group_stats *gs)
 {
     size_t bucket_counter_size;
@@ -7188,16 +7329,98 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
     return 0;
 }
 
-/* Appends a group stats reply that contains the data in 'gds' to those already
- * present in the list of ofpbufs in 'replies'.  'replies' should have been
- * initialized with ofpmp_init(). */
-void
-ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds,
-                                struct list *buckets,
-                                struct list *replies)
+static void
+ofputil_put_ofp11_bucket(const struct ofputil_bucket *bucket,
+                         struct ofpbuf *openflow, enum ofp_version ofp_version)
+{
+    struct ofp11_bucket *ob;
+    size_t start;
+
+    start = ofpbuf_size(openflow);
+    ofpbuf_put_zeros(openflow, sizeof *ob);
+    ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
+                                openflow, ofp_version);
+    ob = ofpbuf_at_assert(openflow, start, sizeof *ob);
+    ob->len = htons(ofpbuf_size(openflow) - start);
+    ob->weight = htons(bucket->weight);
+    ob->watch_port = ofputil_port_to_ofp11(bucket->watch_port);
+    ob->watch_group = htonl(bucket->watch_group);
+}
+
+static void
+ofputil_put_ofp15_group_bucket_prop_weight(ovs_be16 weight,
+                                           struct ofpbuf *openflow)
+{
+    size_t start_ofs;
+    struct ofp15_group_bucket_prop_weight *prop;
+
+    start_ofs = start_property(openflow, OFPGBPT15_WEIGHT);
+    ofpbuf_put_zeros(openflow, sizeof *prop - sizeof(struct ofp_prop_header));
+    prop = ofpbuf_at_assert(openflow, start_ofs, sizeof *prop);
+    prop->weight = weight;
+    end_property(openflow, start_ofs);
+}
+
+static void
+ofputil_put_ofp15_group_bucket_prop_watch(ovs_be32 watch, uint16_t type,
+                                          struct ofpbuf *openflow)
+{
+    size_t start_ofs;
+    struct ofp15_group_bucket_prop_watch *prop;
+
+    start_ofs = start_property(openflow, type);
+    ofpbuf_put_zeros(openflow, sizeof *prop - sizeof(struct ofp_prop_header));
+    prop = ofpbuf_at_assert(openflow, start_ofs, sizeof *prop);
+    prop->watch = watch;
+    end_property(openflow, start_ofs);
+}
+
+static void
+ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
+                         uint32_t bucket_id, enum ofp11_group_type group_type,
+                         struct ofpbuf *openflow, enum ofp_version ofp_version)
+{
+    struct ofp15_bucket *ob;
+    size_t start, actions_start, actions_len;
+
+    start = ofpbuf_size(openflow);
+    ofpbuf_put_zeros(openflow, sizeof *ob);
+
+    actions_start = ofpbuf_size(openflow);
+    ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
+                                 openflow, ofp_version);
+    actions_len = ofpbuf_size(openflow) - actions_start;
+
+    if (group_type == OFPGT11_SELECT) {
+        ofputil_put_ofp15_group_bucket_prop_weight(htons(bucket->weight),
+                                                   openflow);
+    }
+    if (bucket->watch_port != OFPP_ANY) {
+        ovs_be32 port = ofputil_port_to_ofp11(bucket->watch_port);
+        ofputil_put_ofp15_group_bucket_prop_watch(port,
+                                                  OFPGBPT15_WATCH_PORT,
+                                                  openflow);
+    }
+    if (bucket->watch_group != OFPG_ANY) {
+        ovs_be32 group = htonl(bucket->watch_group);
+        ofputil_put_ofp15_group_bucket_prop_watch(group,
+                                                  OFPGBPT15_WATCH_GROUP,
+                                                  openflow);
+    }
+
+    ob = ofpbuf_at_assert(openflow, start, sizeof *ob);
+    ob->len = htons(ofpbuf_size(openflow) - start);
+    ob->action_array_len = htons(actions_len);
+    ob->bucket_id = htonl(bucket_id);
+}
+
+static void
+ofputil_append_ofp11_group_desc_reply(const struct ofputil_group_desc *gds,
+                                      const struct ovs_list *buckets,
+                                      struct ovs_list *replies,
+                                      enum ofp_version version)
 {
     struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
-    enum ofp_version version = ofpmp_version(replies);
     struct ofp11_group_desc_stats *ogds;
     struct ofputil_bucket *bucket;
     size_t start_ogds;
@@ -7205,32 +7428,78 @@ ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds,
     start_ogds = ofpbuf_size(reply);
     ofpbuf_put_zeros(reply, sizeof *ogds);
     LIST_FOR_EACH (bucket, list_node, buckets) {
-        struct ofp11_bucket *ob;
-        size_t start_ob;
-
-        start_ob = ofpbuf_size(reply);
-        ofpbuf_put_zeros(reply, sizeof *ob);
-        ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
-                                     reply, version);
-        ob = ofpbuf_at_assert(reply, start_ob, sizeof *ob);
-        ob->len = htons(ofpbuf_size(reply) - start_ob);
-        ob->weight = htons(bucket->weight);
-        ob->watch_port = ofputil_port_to_ofp11(bucket->watch_port);
-        ob->watch_group = htonl(bucket->watch_group);
+        ofputil_put_ofp11_bucket(bucket, reply, version);
+    }
+    ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
+    ogds->length = htons(ofpbuf_size(reply) - start_ogds);
+    ogds->type = gds->type;
+    ogds->group_id = htonl(gds->group_id);
+
+    ofpmp_postappend(replies, start_ogds);
+}
+
+static void
+ofputil_append_ofp15_group_desc_reply(const struct ofputil_group_desc *gds,
+                                      const struct ovs_list *buckets,
+                                      struct ovs_list *replies,
+                                      enum ofp_version version)
+{
+    struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
+    struct ofp15_group_desc_stats *ogds;
+    struct ofputil_bucket *bucket;
+    size_t start_ogds, start_buckets;
+
+    start_ogds = ofpbuf_size(reply);
+    ofpbuf_put_zeros(reply, sizeof *ogds);
+    start_buckets = ofpbuf_size(reply);
+    LIST_FOR_EACH (bucket, list_node, buckets) {
+        ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
+                                 gds->type, reply, version);
     }
     ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
     ogds->length = htons(ofpbuf_size(reply) - start_ogds);
     ogds->type = gds->type;
     ogds->group_id = htonl(gds->group_id);
+    ogds->bucket_list_len =  htons(ofpbuf_size(reply) - start_buckets);
 
     ofpmp_postappend(replies, start_ogds);
 }
 
+/* Appends a group stats reply that contains the data in 'gds' to those already
+ * present in the list of ofpbufs in 'replies'.  'replies' should have been
+ * initialized with ofpmp_init(). */
+void
+ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds,
+                                const struct ovs_list *buckets,
+                                struct ovs_list *replies)
+{
+    enum ofp_version version = ofpmp_version(replies);
+
+    switch (version)
+    {
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+    case OFP13_VERSION:
+    case OFP14_VERSION:
+        ofputil_append_ofp11_group_desc_reply(gds, buckets, replies, version);
+        break;
+
+    case OFP15_VERSION:
+        ofputil_append_ofp15_group_desc_reply(gds, buckets, replies, version);
+        break;
+
+    case OFP10_VERSION:
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
 static enum ofperr
-ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
-                     enum ofp_version version, struct list *buckets)
+ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t buckets_length,
+                           enum ofp_version version, struct ovs_list *buckets)
 {
     struct ofp11_bucket *ob;
+    uint32_t bucket_id = 0;
 
     list_init(buckets);
     while (buckets_length > 0) {
@@ -7279,6 +7548,8 @@ ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
             return OFPERR_OFPGMFC_BAD_WATCH;
         }
         bucket->watch_group = ntohl(ob->watch_group);
+        bucket->bucket_id = bucket_id++;
+
         bucket->ofpacts = ofpbuf_steal_data(&ofpacts);
         bucket->ofpacts_len = ofpbuf_size(&ofpacts);
         list_push_back(buckets, &bucket->list_node);
@@ -7287,19 +7558,176 @@ ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
     return 0;
 }
 
-/* Converts a group description reply in 'msg' into an abstract
- * ofputil_group_desc in 'gd'.
- *
- * Multiple group description replies can be packed into a single OpenFlow
- * message.  Calling this function multiple times for a single 'msg' iterates
- * through the replies.  The caller must initially leave 'msg''s layer pointers
- * null and not modify them between calls.
- *
- * Returns 0 if successful, EOF if no replies were left in this 'msg',
- * otherwise a positive errno value. */
-int
-ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
-                                struct ofpbuf *msg, enum ofp_version version)
+static enum ofperr
+parse_ofp15_group_bucket_prop_weight(const struct ofpbuf *payload,
+                                     ovs_be16 *weight)
+{
+    struct ofp15_group_bucket_prop_weight *prop = ofpbuf_data(payload);
+
+    if (ofpbuf_size(payload) != sizeof *prop) {
+        log_property(false, "OpenFlow bucket weight property length "
+                     "%u is not valid", ofpbuf_size(payload));
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    *weight = prop->weight;
+
+    return 0;
+}
+
+static enum ofperr
+parse_ofp15_group_bucket_prop_watch(const struct ofpbuf *payload,
+                                    ovs_be32 *watch)
+{
+    struct ofp15_group_bucket_prop_watch *prop = ofpbuf_data(payload);
+
+    if (ofpbuf_size(payload) != sizeof *prop) {
+        log_property(false, "OpenFlow bucket watch port or group "
+                     "property length %u is not valid", ofpbuf_size(payload));
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    *watch = prop->watch;
+
+    return 0;
+}
+
+static enum ofperr
+ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length,
+                           enum ofp_version version, struct ovs_list *buckets)
+{
+    struct ofp15_bucket *ob;
+
+    list_init(buckets);
+    while (buckets_length > 0) {
+        struct ofputil_bucket *bucket = NULL;
+        struct ofpbuf ofpacts;
+        enum ofperr err = OFPERR_OFPGMFC_BAD_BUCKET;
+        struct ofpbuf properties;
+        size_t ob_len, actions_len, properties_len;
+        ovs_be32 watch_port = ofputil_port_to_ofp11(OFPP_ANY);
+        ovs_be32 watch_group = htonl(OFPG_ANY);
+        ovs_be16 weight = htons(1);
+
+        ofpbuf_init(&ofpacts, 0);
+
+        ob = ofpbuf_try_pull(msg, sizeof *ob);
+        if (!ob) {
+            VLOG_WARN_RL(&bad_ofmsg_rl, "buckets end with %"PRIuSIZE
+                         " leftover bytes", buckets_length);
+            goto err;
+        }
+
+        ob_len = ntohs(ob->len);
+        actions_len = ntohs(ob->action_array_len);
+
+        if (ob_len < sizeof *ob) {
+            VLOG_WARN_RL(&bad_ofmsg_rl, "OpenFlow message bucket length "
+                         "%"PRIuSIZE" is not valid", ob_len);
+            goto err;
+        } else if (ob_len > buckets_length) {
+            VLOG_WARN_RL(&bad_ofmsg_rl, "OpenFlow message bucket length "
+                         "%"PRIuSIZE" exceeds remaining buckets data size %"
+                         PRIuSIZE, ob_len, buckets_length);
+            goto err;
+        } else if (actions_len > ob_len - sizeof *ob) {
+            VLOG_WARN_RL(&bad_ofmsg_rl, "OpenFlow message bucket actions "
+                         "length %"PRIuSIZE" exceeds remaining bucket "
+                         "data size %"PRIuSIZE, actions_len,
+                         ob_len - sizeof *ob);
+            goto err;
+        }
+        buckets_length -= ob_len;
+
+        err = ofpacts_pull_openflow_actions(msg, actions_len, version,
+                                            &ofpacts);
+        if (err) {
+            goto err;
+        }
+
+        properties_len = ob_len - sizeof *ob - actions_len;
+        ofpbuf_use_const(&properties, ofpbuf_pull(msg, properties_len),
+                         properties_len);
+
+        while (ofpbuf_size(&properties) > 0) {
+            struct ofpbuf payload;
+            uint16_t type;
+
+            err = ofputil_pull_property(&properties, &payload, &type);
+            if (err) {
+                goto err;
+            }
+
+            switch (type) {
+            case OFPGBPT15_WEIGHT:
+                err = parse_ofp15_group_bucket_prop_weight(&payload, &weight);
+                break;
+
+            case OFPGBPT15_WATCH_PORT:
+                err = parse_ofp15_group_bucket_prop_watch(&payload,
+                                                          &watch_port);
+                break;
+
+            case OFPGBPT15_WATCH_GROUP:
+                err = parse_ofp15_group_bucket_prop_watch(&payload,
+                                                          &watch_group);
+                break;
+
+            default:
+                log_property(false, "unknown group bucket property %"PRIu16,
+                             type);
+                err = OFPERR_OFPBPC_BAD_TYPE;
+                break;
+            }
+
+            if (err) {
+                goto err;
+            }
+        }
+
+        bucket = xzalloc(sizeof *bucket);
+
+        bucket->weight = ntohs(weight);
+        err = ofputil_port_from_ofp11(watch_port, &bucket->watch_port);
+        if (err) {
+            err = OFPERR_OFPGMFC_BAD_WATCH;
+            goto err;
+        }
+        bucket->watch_group = ntohl(watch_group);
+        bucket->bucket_id = ntohl(ob->bucket_id);
+        if (bucket->bucket_id > OFPG15_BUCKET_MAX) {
+            VLOG_WARN_RL(&bad_ofmsg_rl, "bucket id (%u) is out of range",
+                         bucket->bucket_id);
+            err = OFPERR_OFPGMFC_BAD_BUCKET;
+            goto err;
+        }
+
+        bucket->ofpacts = ofpbuf_steal_data(&ofpacts);
+        bucket->ofpacts_len = ofpbuf_size(&ofpacts);
+        list_push_back(buckets, &bucket->list_node);
+
+        continue;
+
+    err:
+        free(bucket);
+        ofpbuf_uninit(&ofpacts);
+        ofputil_bucket_list_destroy(buckets);
+        return err;
+    }
+
+    if (ofputil_bucket_check_duplicate_id(buckets)) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "Duplicate bucket id");
+        ofputil_bucket_list_destroy(buckets);
+        return OFPERR_OFPGMFC_BAD_BUCKET;
+    }
+
+    return 0;
+}
+
+static int
+ofputil_decode_ofp11_group_desc_reply(struct ofputil_group_desc *gd,
+                                      struct ofpbuf *msg,
+                                      enum ofp_version version)
 {
     struct ofp11_group_desc_stats *ogds;
     size_t length;
@@ -7328,72 +7756,325 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
         return OFPERR_OFPBRC_BAD_LEN;
     }
 
-    return ofputil_pull_buckets(msg, length - sizeof *ogds, version,
-                                &gd->buckets);
+    return ofputil_pull_ofp11_buckets(msg, length - sizeof *ogds, version,
+                                      &gd->buckets);
 }
 
-/* Converts abstract group mod 'gm' into a message for OpenFlow version
- * 'ofp_version' and returns the message. */
-struct ofpbuf *
-ofputil_encode_group_mod(enum ofp_version ofp_version,
-                         const struct ofputil_group_mod *gm)
+static int
+ofputil_decode_ofp15_group_desc_reply(struct ofputil_group_desc *gd,
+                                      struct ofpbuf *msg,
+                                      enum ofp_version version)
+{
+    struct ofp15_group_desc_stats *ogds;
+    uint16_t length, bucket_list_len;
+
+    if (!msg->frame) {
+        ofpraw_pull_assert(msg);
+    }
+
+    if (!ofpbuf_size(msg)) {
+        return EOF;
+    }
+
+    ogds = ofpbuf_try_pull(msg, sizeof *ogds);
+    if (!ogds) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST11_GROUP_DESC reply has %"PRIu32" "
+                     "leftover bytes at end", ofpbuf_size(msg));
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+    gd->type = ogds->type;
+    gd->group_id = ntohl(ogds->group_id);
+
+    length = ntohs(ogds->length);
+    if (length < sizeof *ogds || length - sizeof *ogds > ofpbuf_size(msg)) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST11_GROUP_DESC reply claims invalid "
+                     "length %u", length);
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+
+    bucket_list_len = ntohs(ogds->bucket_list_len);
+    if (length < bucket_list_len + sizeof *ogds) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST11_GROUP_DESC reply claims invalid "
+                     "bucket list length %u", bucket_list_len);
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+
+    return ofputil_pull_ofp15_buckets(msg, bucket_list_len, version,
+                                      &gd->buckets);
+}
+
+/* Converts a group description reply in 'msg' into an abstract
+ * ofputil_group_desc in 'gd'.
+ *
+ * Multiple group description replies can be packed into a single OpenFlow
+ * message.  Calling this function multiple times for a single 'msg' iterates
+ * through the replies.  The caller must initially leave 'msg''s layer pointers
+ * null and not modify them between calls.
+ *
+ * Returns 0 if successful, EOF if no replies were left in this 'msg',
+ * otherwise a positive errno value. */
+int
+ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
+                                struct ofpbuf *msg, enum ofp_version version)
+{
+    switch (version)
+    {
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+    case OFP13_VERSION:
+    case OFP14_VERSION:
+        return ofputil_decode_ofp11_group_desc_reply(gd, msg, version);
+
+    case OFP15_VERSION:
+        return ofputil_decode_ofp15_group_desc_reply(gd, msg, version);
+
+    case OFP10_VERSION:
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
+static struct ofpbuf *
+ofputil_encode_ofp11_group_mod(enum ofp_version ofp_version,
+                               const struct ofputil_group_mod *gm)
 {
     struct ofpbuf *b;
     struct ofp11_group_mod *ogm;
     size_t start_ogm;
-    size_t start_bucket;
     struct ofputil_bucket *bucket;
-    struct ofp11_bucket *ob;
 
-    switch (ofp_version) {
-    case OFP10_VERSION: {
-        if (gm->command == OFPGC11_ADD) {
-            ovs_fatal(0, "add-group needs OpenFlow 1.1 or later "
-                         "(\'-O OpenFlow11\')");
-        } else if (gm->command == OFPGC11_MODIFY) {
-            ovs_fatal(0, "mod-group needs OpenFlow 1.1 or later "
-                         "(\'-O OpenFlow11\')");
+    b = ofpraw_alloc(OFPRAW_OFPT11_GROUP_MOD, ofp_version, 0);
+    start_ogm = ofpbuf_size(b);
+    ofpbuf_put_zeros(b, sizeof *ogm);
+
+    LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
+        ofputil_put_ofp11_bucket(bucket, b, ofp_version);
+    }
+    ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
+    ogm->command = htons(gm->command);
+    ogm->type = gm->type;
+    ogm->group_id = htonl(gm->group_id);
+
+    return b;
+}
+
+static struct ofpbuf *
+ofputil_encode_ofp15_group_mod(enum ofp_version ofp_version,
+                               const struct ofputil_group_mod *gm)
+{
+    struct ofpbuf *b;
+    struct ofp15_group_mod *ogm;
+    size_t start_ogm;
+    struct ofputil_bucket *bucket;
+    struct id_pool *bucket_ids = NULL;
+
+    b = ofpraw_alloc(OFPRAW_OFPT15_GROUP_MOD, ofp_version, 0);
+    start_ogm = ofpbuf_size(b);
+    ofpbuf_put_zeros(b, sizeof *ogm);
+
+    LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
+        uint32_t bucket_id;
+
+        /* Generate a bucket id if none was supplied */
+        if (bucket->bucket_id > OFPG15_BUCKET_MAX) {
+            if (!bucket_ids) {
+                const struct ofputil_bucket *bkt;
+
+                bucket_ids = id_pool_create(0, OFPG15_BUCKET_MAX + 1);
+
+                /* Mark all bucket_ids that are present in gm
+                 * as used in the pool. */
+                LIST_FOR_EACH_REVERSE (bkt, list_node, &gm->buckets) {
+                    if (bkt == bucket) {
+                        break;
+                    }
+                    if (bkt->bucket_id <= OFPG15_BUCKET_MAX) {
+                        id_pool_add(bucket_ids, bkt->bucket_id);
+                    }
+                }
+            }
+
+            if (!id_pool_alloc_id(bucket_ids, &bucket_id)) {
+                OVS_NOT_REACHED();
+            }
         } else {
-            ovs_fatal(0, "del-groups needs OpenFlow 1.1 or later "
-                         "(\'-O OpenFlow11\')");
+            bucket_id = bucket->bucket_id;
         }
+
+        ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
+    }
+    ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
+    ogm->command = htons(gm->command);
+    ogm->type = gm->type;
+    ogm->group_id = htonl(gm->group_id);
+    ogm->command_bucket_id = htonl(gm->command_bucket_id);
+    ogm->bucket_array_len = htons(ofpbuf_size(b) - start_ogm - sizeof *ogm);
+
+    id_pool_destroy(bucket_ids);
+    return b;
+}
+
+static void
+bad_group_cmd(enum ofp15_group_mod_command cmd) {
+    const char *opt_version;
+    const char *version;
+    const char *cmd_str;
+
+    switch (cmd) {
+    case OFPGC15_ADD:
+    case OFPGC15_MODIFY:
+    case OFPGC15_DELETE:
+        version = "1.1";
+        opt_version = "11";
+        break;
+
+    case OFPGC15_INSERT_BUCKET:
+    case OFPGC15_REMOVE_BUCKET:
+        version = "1.5";
+        opt_version = "15";
+
+    default:
+        OVS_NOT_REACHED();
     }
 
+    switch (cmd) {
+    case OFPGC15_ADD:
+        cmd_str = "add-group";
+        break;
+
+    case OFPGC15_MODIFY:
+        cmd_str = "mod-group";
+        break;
+
+    case OFPGC15_DELETE:
+        cmd_str = "del-group";
+        break;
+
+    case OFPGC15_INSERT_BUCKET:
+        cmd_str = "insert-bucket";
+        break;
+
+    case OFPGC15_REMOVE_BUCKET:
+        cmd_str = "insert-bucket";
+        break;
+
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    ovs_fatal(0, "%s needs OpenFlow %s or later (\'-O OpenFlow%s\')",
+              cmd_str, version, opt_version);
+
+}
+
+/* Converts abstract group mod 'gm' into a message for OpenFlow version
+ * 'ofp_version' and returns the message. */
+struct ofpbuf *
+ofputil_encode_group_mod(enum ofp_version ofp_version,
+                         const struct ofputil_group_mod *gm)
+{
+
+    switch (ofp_version) {
+    case OFP10_VERSION:
+        bad_group_cmd(gm->command);
+
     case OFP11_VERSION:
     case OFP12_VERSION:
     case OFP13_VERSION:
     case OFP14_VERSION:
-    case OFP15_VERSION:
-        b = ofpraw_alloc(OFPRAW_OFPT11_GROUP_MOD, ofp_version, 0);
-        start_ogm = ofpbuf_size(b);
-        ofpbuf_put_zeros(b, sizeof *ogm);
-
-        LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
-            start_bucket = ofpbuf_size(b);
-            ofpbuf_put_zeros(b, sizeof *ob);
-            if (bucket->ofpacts && bucket->ofpacts_len) {
-                ofpacts_put_openflow_actions(bucket->ofpacts,
-                                             bucket->ofpacts_len, b,
-                                             ofp_version);
-            }
-            ob = ofpbuf_at_assert(b, start_bucket, sizeof *ob);
-            ob->len = htons(ofpbuf_size(b) - start_bucket);;
-            ob->weight = htons(bucket->weight);
-            ob->watch_port = ofputil_port_to_ofp11(bucket->watch_port);
-            ob->watch_group = htonl(bucket->watch_group);
+        if (gm->command > OFPGC11_DELETE) {
+            bad_group_cmd(gm->command);
         }
-        ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
-        ogm->command = htons(gm->command);
-        ogm->type = gm->type;
-        ogm->group_id = htonl(gm->group_id);
+        return ofputil_encode_ofp11_group_mod(ofp_version, gm);
 
-        break;
+    case OFP15_VERSION:
+        return ofputil_encode_ofp15_group_mod(ofp_version, gm);
 
     default:
         OVS_NOT_REACHED();
     }
+}
 
-    return b;
+static enum ofperr
+ofputil_pull_ofp11_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version,
+                             struct ofputil_group_mod *gm)
+{
+    const struct ofp11_group_mod *ogm;
+    enum ofperr error;
+
+    ogm = ofpbuf_pull(msg, sizeof *ogm);
+    gm->command = ntohs(ogm->command);
+    gm->type = ogm->type;
+    gm->group_id = ntohl(ogm->group_id);
+    gm->command_bucket_id = OFPG15_BUCKET_ALL;
+
+    error = ofputil_pull_ofp11_buckets(msg, ofpbuf_size(msg), ofp_version,
+                                       &gm->buckets);
+
+    /* OF1.3.5+ prescribes an error when an OFPGC_DELETE includes buckets. */
+    if (!error
+        && ofp_version >= OFP13_VERSION
+        && gm->command == OFPGC11_DELETE
+        && !list_is_empty(&gm->buckets)) {
+        error = OFPERR_OFPGMFC_INVALID_GROUP;
+    }
+
+    return error;
+}
+
+static enum ofperr
+ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version,
+                             struct ofputil_group_mod *gm)
+{
+    const struct ofp15_group_mod *ogm;
+    uint16_t bucket_list_len;
+    enum ofperr error = OFPERR_OFPGMFC_BAD_BUCKET;
+
+    ogm = ofpbuf_pull(msg, sizeof *ogm);
+    gm->command = ntohs(ogm->command);
+    gm->type = ogm->type;
+    gm->group_id = ntohl(ogm->group_id);
+
+    gm->command_bucket_id = ntohl(ogm->command_bucket_id);
+    switch (gm->command) {
+    case OFPGC15_REMOVE_BUCKET:
+        if (gm->command_bucket_id == OFPG15_BUCKET_ALL) {
+            error = 0;
+        }
+        /* Fall through */
+    case OFPGC15_INSERT_BUCKET:
+        if (gm->command_bucket_id <= OFPG15_BUCKET_MAX ||
+            gm->command_bucket_id == OFPG15_BUCKET_FIRST
+            || gm->command_bucket_id == OFPG15_BUCKET_LAST) {
+            error = 0;
+        }
+        break;
+
+    case OFPGC11_ADD:
+    case OFPGC11_MODIFY:
+    case OFPGC11_DELETE:
+    default:
+        if (gm->command_bucket_id == OFPG15_BUCKET_ALL) {
+            error = 0;
+        }
+        break;
+    }
+    if (error) {
+        VLOG_WARN_RL(&bad_ofmsg_rl,
+                     "group command bucket id (%u) is out of range",
+                     gm->command_bucket_id);
+        return OFPERR_OFPGMFC_BAD_BUCKET;
+    }
+
+    bucket_list_len = ntohs(ogm->bucket_array_len);
+    if (bucket_list_len < ofpbuf_size(msg)) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "group has %u trailing bytes",
+                     ofpbuf_size(msg) - bucket_list_len);
+        return OFPERR_OFPGMFC_BAD_BUCKET;
+    }
+
+    return ofputil_pull_ofp15_buckets(msg, bucket_list_len, ofp_version,
+                                      &gm->buckets);
 }
 
 /* Converts OpenFlow group mod message 'oh' into an abstract group mod in
@@ -7402,7 +8083,7 @@ enum ofperr
 ofputil_decode_group_mod(const struct ofp_header *oh,
                          struct ofputil_group_mod *gm)
 {
-    const struct ofp11_group_mod *ogm;
+    enum ofp_version ofp_version = oh->version;
     struct ofpbuf msg;
     struct ofputil_bucket *bucket;
     enum ofperr err;
@@ -7410,16 +8091,57 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
     ofpbuf_use_const(&msg, oh, ntohs(oh->length));
     ofpraw_pull_assert(&msg);
 
-    ogm = ofpbuf_pull(&msg, sizeof *ogm);
-    gm->command = ntohs(ogm->command);
-    gm->type = ogm->type;
-    gm->group_id = ntohl(ogm->group_id);
+    switch (ofp_version)
+    {
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+    case OFP13_VERSION:
+    case OFP14_VERSION:
+        err = ofputil_pull_ofp11_group_mod(&msg, ofp_version, gm);
+        break;
+
+    case OFP15_VERSION:
+        err = ofputil_pull_ofp15_group_mod(&msg, ofp_version, gm);
+        break;
+
+    case OFP10_VERSION:
+    default:
+        OVS_NOT_REACHED();
+    }
 
-    err = ofputil_pull_buckets(&msg, ofpbuf_size(&msg), oh->version, &gm->buckets);
     if (err) {
         return err;
     }
 
+    switch (gm->type) {
+    case OFPGT11_INDIRECT:
+        if (!list_is_singleton(&gm->buckets)) {
+            return OFPERR_OFPGMFC_INVALID_GROUP;
+        }
+        break;
+    case OFPGT11_ALL:
+    case OFPGT11_SELECT:
+    case OFPGT11_FF:
+        break;
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    switch (gm->command) {
+    case OFPGC11_ADD:
+    case OFPGC11_MODIFY:
+    case OFPGC11_DELETE:
+    case OFPGC15_INSERT_BUCKET:
+        break;
+    case OFPGC15_REMOVE_BUCKET:
+        if (!list_is_empty(&gm->buckets)) {
+            return OFPERR_OFPGMFC_BAD_BUCKET;
+        }
+        break;
+    default:
+        OVS_NOT_REACHED();
+    }
+
     LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
         switch (gm->type) {
         case OFPGT11_ALL:
@@ -7476,9 +8198,9 @@ ofputil_decode_queue_stats_request(const struct ofp_header *request,
     }
 }
 
-/* Encode a queue statsrequest for 'oqsr', the encoded message
- * will be fore Open Flow version 'ofp_version'. Returns message
- * as a struct ofpbuf. Returns encoded message on success, NULL on error */
+/* Encode a queue stats request for 'oqsr', the encoded message
+ * will be for OpenFlow version 'ofp_version'. Returns message
+ * as a struct ofpbuf. Returns encoded message on success, NULL on error. */
 struct ofpbuf *
 ofputil_encode_queue_stats_request(enum ofp_version ofp_version,
                                    const struct ofputil_queue_stats_request *oqsr)
@@ -7712,7 +8434,7 @@ ofputil_queue_stats_to_ofp14(const struct ofputil_queue_stats *oqs,
 
 /* Encode a queue stat for 'oqs' and append it to 'replies'. */
 void
-ofputil_append_queue_stat(struct list *replies,
+ofputil_append_queue_stat(struct ovs_list *replies,
                           const struct ofputil_queue_stats *oqs)
 {
     switch (ofpmp_version(replies)) {
@@ -7784,6 +8506,92 @@ ofputil_encode_bundle_ctrl_reply(const struct ofp_header *oh,
     return buf;
 }
 
+/* Return true for bundlable state change requests, false for other messages.
+ */
+static bool
+ofputil_is_bundlable(enum ofptype type)
+{
+    switch (type) {
+        /* Minimum required by OpenFlow 1.4. */
+    case OFPTYPE_PORT_MOD:
+    case OFPTYPE_FLOW_MOD:
+        return true;
+
+        /* Nice to have later. */
+    case OFPTYPE_FLOW_MOD_TABLE_ID:
+    case OFPTYPE_GROUP_MOD:
+    case OFPTYPE_TABLE_MOD:
+    case OFPTYPE_METER_MOD:
+    case OFPTYPE_PACKET_OUT:
+
+        /* Not to be bundlable. */
+    case OFPTYPE_ECHO_REQUEST:
+    case OFPTYPE_FEATURES_REQUEST:
+    case OFPTYPE_GET_CONFIG_REQUEST:
+    case OFPTYPE_SET_CONFIG:
+    case OFPTYPE_BARRIER_REQUEST:
+    case OFPTYPE_ROLE_REQUEST:
+    case OFPTYPE_ECHO_REPLY:
+    case OFPTYPE_SET_FLOW_FORMAT:
+    case OFPTYPE_SET_PACKET_IN_FORMAT:
+    case OFPTYPE_SET_CONTROLLER_ID:
+    case OFPTYPE_FLOW_AGE:
+    case OFPTYPE_FLOW_MONITOR_CANCEL:
+    case OFPTYPE_SET_ASYNC_CONFIG:
+    case OFPTYPE_GET_ASYNC_REQUEST:
+    case OFPTYPE_DESC_STATS_REQUEST:
+    case OFPTYPE_FLOW_STATS_REQUEST:
+    case OFPTYPE_AGGREGATE_STATS_REQUEST:
+    case OFPTYPE_TABLE_STATS_REQUEST:
+    case OFPTYPE_TABLE_FEATURES_STATS_REQUEST:
+    case OFPTYPE_PORT_STATS_REQUEST:
+    case OFPTYPE_QUEUE_STATS_REQUEST:
+    case OFPTYPE_PORT_DESC_STATS_REQUEST:
+    case OFPTYPE_FLOW_MONITOR_STATS_REQUEST:
+    case OFPTYPE_METER_STATS_REQUEST:
+    case OFPTYPE_METER_CONFIG_STATS_REQUEST:
+    case OFPTYPE_METER_FEATURES_STATS_REQUEST:
+    case OFPTYPE_GROUP_STATS_REQUEST:
+    case OFPTYPE_GROUP_DESC_STATS_REQUEST:
+    case OFPTYPE_GROUP_FEATURES_STATS_REQUEST:
+    case OFPTYPE_QUEUE_GET_CONFIG_REQUEST:
+    case OFPTYPE_BUNDLE_CONTROL:
+    case OFPTYPE_BUNDLE_ADD_MESSAGE:
+    case OFPTYPE_HELLO:
+    case OFPTYPE_ERROR:
+    case OFPTYPE_FEATURES_REPLY:
+    case OFPTYPE_GET_CONFIG_REPLY:
+    case OFPTYPE_PACKET_IN:
+    case OFPTYPE_FLOW_REMOVED:
+    case OFPTYPE_PORT_STATUS:
+    case OFPTYPE_BARRIER_REPLY:
+    case OFPTYPE_QUEUE_GET_CONFIG_REPLY:
+    case OFPTYPE_DESC_STATS_REPLY:
+    case OFPTYPE_FLOW_STATS_REPLY:
+    case OFPTYPE_QUEUE_STATS_REPLY:
+    case OFPTYPE_PORT_STATS_REPLY:
+    case OFPTYPE_TABLE_STATS_REPLY:
+    case OFPTYPE_AGGREGATE_STATS_REPLY:
+    case OFPTYPE_PORT_DESC_STATS_REPLY:
+    case OFPTYPE_ROLE_REPLY:
+    case OFPTYPE_FLOW_MONITOR_PAUSED:
+    case OFPTYPE_FLOW_MONITOR_RESUMED:
+    case OFPTYPE_FLOW_MONITOR_STATS_REPLY:
+    case OFPTYPE_GET_ASYNC_REPLY:
+    case OFPTYPE_GROUP_STATS_REPLY:
+    case OFPTYPE_GROUP_DESC_STATS_REPLY:
+    case OFPTYPE_GROUP_FEATURES_STATS_REPLY:
+    case OFPTYPE_METER_STATS_REPLY:
+    case OFPTYPE_METER_CONFIG_STATS_REPLY:
+    case OFPTYPE_METER_FEATURES_STATS_REPLY:
+    case OFPTYPE_TABLE_FEATURES_STATS_REPLY:
+    case OFPTYPE_ROLE_STATUS:
+        break;
+    }
+
+    return false;
+}
+
 enum ofperr
 ofputil_decode_bundle_add(const struct ofp_header *oh,
                           struct ofputil_bundle_add_msg *msg)
@@ -7792,6 +8600,8 @@ ofputil_decode_bundle_add(const struct ofp_header *oh,
     struct ofpbuf b;
     enum ofpraw raw;
     size_t inner_len;
+    enum ofperr error;
+    enum ofptype type;
 
     ofpbuf_use_const(&b, oh, ntohs(oh->length));
     raw = ofpraw_pull_assert(&b);
@@ -7806,6 +8616,21 @@ ofputil_decode_bundle_add(const struct ofp_header *oh,
     if (inner_len < sizeof(struct ofp_header) || inner_len > ofpbuf_size(&b)) {
         return OFPERR_OFPBFC_MSG_BAD_LEN;
     }
+    if (msg->msg->xid != oh->xid) {
+        return OFPERR_OFPBFC_MSG_BAD_XID;
+    }
+
+    /* Reject unbundlable messages. */
+    error = ofptype_decode(&type, msg->msg);
+    if (error) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPT14_BUNDLE_ADD_MESSAGE contained "
+                     "message is unparsable (%s)", ofperr_get_name(error));
+        return OFPERR_OFPBFC_MSG_UNSUP; /* 'error' would be confusing. */
+    }
+
+    if (!ofputil_is_bundlable(type)) {
+        return OFPERR_OFPBFC_MSG_UNSUP;
+    }
 
     return 0;
 }