ofproto: Only allow indirect groups with one bucket
[cascardo/ovs.git] / lib / ofp-util.c
index 4c2c011..573f38a 100644 (file)
@@ -120,6 +120,36 @@ log_property(bool loose, const char *message, ...)
     }
 }
 
+static size_t
+start_property(struct ofpbuf *msg, uint16_t type)
+{
+    size_t start_ofs = ofpbuf_size(msg);
+    struct ofp_prop_header *oph;
+
+    oph = ofpbuf_put_uninit(msg, sizeof *oph);
+    oph->type = htons(type);
+    oph->len = htons(4);        /* May be updated later by end_property(). */
+    return start_ofs;
+}
+
+static void
+end_property(struct ofpbuf *msg, size_t start_ofs)
+{
+    struct ofp_prop_header *oph;
+
+    oph = ofpbuf_at_assert(msg, start_ofs, sizeof *oph);
+    oph->len = htons(ofpbuf_size(msg) - start_ofs);
+    ofpbuf_padto(msg, ROUND_UP(ofpbuf_size(msg), 8));
+}
+
+static void
+put_bitmap_properties(struct ofpbuf *msg, uint64_t bitmap)
+{
+    for (; bitmap; bitmap = zero_rightmost_1bit(bitmap)) {
+        start_property(msg, rightmost_1bit_idx(bitmap));
+    }
+}
+
 /* Given the wildcard bit count in the least-significant 6 of 'wcbits', returns
  * an IP netmask with a 1 in each bit that must match and a 0 in each bit that
  * is wildcarded.
@@ -1635,12 +1665,6 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             return error;
         }
 
-        error = ofpacts_pull_openflow_instructions(&b, ofpbuf_size(&b), oh->version,
-                                                   ofpacts);
-        if (error) {
-            return error;
-        }
-
         /* Translate the message. */
         fm->priority = ntohs(ofm->priority);
         if (ofm->command == OFPFC_ADD
@@ -1701,13 +1725,6 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             ofputil_match_from_ofp10_match(&ofm->match, &fm->match);
             ofputil_normalize_match(&fm->match);
 
-            /* Now get the actions. */
-            error = ofpacts_pull_openflow_actions(&b, ofpbuf_size(&b), oh->version,
-                                                  ofpacts);
-            if (error) {
-                return error;
-            }
-
             /* OpenFlow 1.0 says that exact-match rules have to have the
              * highest possible priority. */
             fm->priority = (ofm->match.wildcards & htonl(OFPFW10_ALL)
@@ -1736,11 +1753,6 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             if (error) {
                 return error;
             }
-            error = ofpacts_pull_openflow_actions(&b, ofpbuf_size(&b), oh->version,
-                                                  ofpacts);
-            if (error) {
-                return error;
-            }
 
             /* Translate the message. */
             command = ntohs(nfm->command);
@@ -1771,6 +1783,11 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
         }
     }
 
+    error = ofpacts_pull_openflow_instructions(&b, ofpbuf_size(&b),
+                                               oh->version, ofpacts);
+    if (error) {
+        return error;
+    }
     fm->ofpacts = ofpbuf_data(ofpacts);
     fm->ofpacts_len = ofpbuf_size(ofpacts);
 
@@ -2212,7 +2229,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
             || fm->command == OFPFC_ADD) {
             ofm->cookie = fm->new_cookie;
         } else {
-            ofm->cookie = fm->cookie;
+            ofm->cookie = fm->cookie & fm->cookie_mask;
         }
         ofm->cookie_mask = fm->cookie_mask;
         if (fm->table_id != OFPTT_ALL
@@ -2771,6 +2788,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
                                 struct ofpbuf *ofpacts)
 {
     const struct ofp_header *oh;
+    size_t instructions_len;
     enum ofperr error;
     enum ofpraw raw;
 
@@ -2808,13 +2826,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
             VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad match");
             return EINVAL;
         }
-
-        if (ofpacts_pull_openflow_instructions(msg, length - sizeof *ofs -
-                                               padded_match_len, oh->version,
-                                               ofpacts)) {
-            VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
-            return EINVAL;
-        }
+        instructions_len = length - sizeof *ofs - padded_match_len;
 
         fs->priority = ntohs(ofs->priority);
         fs->table_id = ofs->table_id;
@@ -2853,11 +2865,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
                          "length %"PRIuSIZE, length);
             return EINVAL;
         }
-
-        if (ofpacts_pull_openflow_actions(msg, length - sizeof *ofs,
-                                          oh->version, ofpacts)) {
-            return EINVAL;
-        }
+        instructions_len = length - sizeof *ofs;
 
         fs->cookie = get_32aligned_be64(&ofs->cookie);
         ofputil_match_from_ofp10_match(&ofs->match, &fs->match);
@@ -2874,7 +2882,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         fs->flags = 0;
     } else if (raw == OFPRAW_NXST_FLOW_REPLY) {
         const struct nx_flow_stats *nfs;
-        size_t match_len, actions_len, length;
+        size_t match_len, length;
 
         nfs = ofpbuf_try_pull(msg, sizeof *nfs);
         if (!nfs) {
@@ -2893,12 +2901,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL)) {
             return EINVAL;
         }
-
-        actions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
-        if (ofpacts_pull_openflow_actions(msg, actions_len, oh->version,
-                                          ofpacts)) {
-            return EINVAL;
-        }
+        instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
 
         fs->cookie = nfs->cookie;
         fs->table_id = nfs->table_id;
@@ -2924,6 +2927,11 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         OVS_NOT_REACHED();
     }
 
+    if (ofpacts_pull_openflow_instructions(msg, instructions_len, oh->version,
+                                           ofpacts)) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
+        return EINVAL;
+    }
     fs->ofpacts = ofpbuf_data(ofpacts);
     fs->ofpacts_len = ofpbuf_size(ofpacts);
 
@@ -4552,38 +4560,6 @@ parse_table_features_next_table(struct ofpbuf *payload,
     return 0;
 }
 
-static enum ofperr
-parse_oxm(struct ofpbuf *b, bool loose,
-          const struct mf_field **fieldp, bool *hasmask)
-{
-    ovs_be32 *oxmp;
-    uint32_t oxm;
-
-    oxmp = ofpbuf_try_pull(b, sizeof *oxmp);
-    if (!oxmp) {
-        return OFPERR_OFPBPC_BAD_LEN;
-    }
-    oxm = ntohl(*oxmp);
-
-    /* Determine '*hasmask'.  If 'oxm' is masked, convert it to the equivalent
-     * unmasked version, because the table of OXM fields we support only has
-     * masked versions of fields that we support with masks, but we should be
-     * able to parse the masked versions of those here. */
-    *hasmask = NXM_HASMASK(oxm);
-    if (*hasmask) {
-        if (NXM_LENGTH(oxm) & 1) {
-            return OFPERR_OFPBPC_BAD_VALUE;
-        }
-        oxm = NXM_HEADER(NXM_VENDOR(oxm), NXM_FIELD(oxm), NXM_LENGTH(oxm) / 2);
-    }
-
-    *fieldp = mf_from_nxm_header(oxm);
-    if (!*fieldp) {
-        log_property(loose, "unknown OXM field %#"PRIx32, ntohl(*oxmp));
-    }
-    return *fieldp ? 0 : OFPERR_OFPBMC_BAD_FIELD;
-}
-
 static enum ofperr
 parse_oxms(struct ofpbuf *payload, bool loose,
            struct mf_bitmap *exactp, struct mf_bitmap *maskedp)
@@ -4596,7 +4572,7 @@ parse_oxms(struct ofpbuf *payload, bool loose,
         enum ofperr error;
         bool hasmask;
 
-        error = parse_oxm(payload, loose, &field, &hasmask);
+        error = nx_pull_header(payload, &field, &hasmask);
         if (!error) {
             bitmap_set1(hasmask ? masked.bm : exact.bm, field->id);
         } else if (error != OFPERR_OFPBMC_BAD_FIELD || !loose) {
@@ -4760,15 +4736,17 @@ ofputil_decode_table_features(struct ofpbuf *msg,
 
     /* Fix inconsistencies:
      *
-     *     - Turn off 'mask' and 'wildcard' bits that are not in 'match',
-     *       because a field must be matchable to be masked or wildcarded.
+     *     - Turn on 'match' bits that are set in 'mask', because maskable
+     *       fields are matchable.
      *
      *     - Turn on 'wildcard' bits that are set in 'mask', because a field
-     *       that is arbitrarily maskable can be wildcarded entirely. */
-    bitmap_and(tf->mask.bm, tf->match.bm, MFF_N_IDS);
-    bitmap_and(tf->wildcard.bm, tf->match.bm, MFF_N_IDS);
-
+     *       that is arbitrarily maskable can be wildcarded entirely.
+     *
+     *     - Turn off 'wildcard' bits that are not in 'match', because a field
+     *       must be matchable for it to be meaningfully wildcarded. */
+    bitmap_or(tf->match.bm, tf->mask.bm, MFF_N_IDS);
     bitmap_or(tf->wildcard.bm, tf->mask.bm, MFF_N_IDS);
+    bitmap_and(tf->wildcard.bm, tf->match.bm, MFF_N_IDS);
 
     return 0;
 }
@@ -4799,6 +4777,101 @@ ofputil_encode_table_features_request(enum ofp_version ofp_version)
     return request;
 }
 
+static void
+put_fields_property(struct ofpbuf *reply,
+                    const struct mf_bitmap *fields,
+                    const struct mf_bitmap *masks,
+                    enum ofp13_table_feature_prop_type property,
+                    enum ofp_version version)
+{
+    size_t start_ofs;
+    int field;
+
+    start_ofs = start_property(reply, property);
+    BITMAP_FOR_EACH_1 (field, MFF_N_IDS, fields->bm) {
+        nx_put_header(reply, field, version,
+                      masks && bitmap_is_set(masks->bm, field));
+    }
+    end_property(reply, start_ofs);
+}
+
+static void
+put_table_action_features(struct ofpbuf *reply,
+                          const struct ofputil_table_action_features *taf,
+                          enum ofp13_table_feature_prop_type actions_type,
+                          enum ofp13_table_feature_prop_type set_fields_type,
+                          int miss_offset, enum ofp_version version)
+{
+    size_t start_ofs;
+
+    start_ofs = start_property(reply, actions_type + miss_offset);
+    put_bitmap_properties(reply,
+                          ntohl(ofpact_bitmap_to_openflow(taf->ofpacts,
+                                                          version)));
+    end_property(reply, start_ofs);
+
+    put_fields_property(reply, &taf->set_fields, NULL,
+                        set_fields_type + miss_offset, version);
+}
+
+static void
+put_table_instruction_features(
+    struct ofpbuf *reply, const struct ofputil_table_instruction_features *tif,
+    int miss_offset, enum ofp_version version)
+{
+    size_t start_ofs;
+    uint8_t table_id;
+
+    start_ofs = start_property(reply, OFPTFPT13_INSTRUCTIONS + miss_offset);
+    put_bitmap_properties(reply,
+                          ntohl(ovsinst_bitmap_to_openflow(tif->instructions,
+                                                           version)));
+    end_property(reply, start_ofs);
+
+    start_ofs = start_property(reply, OFPTFPT13_NEXT_TABLES + miss_offset);
+    BITMAP_FOR_EACH_1 (table_id, 255, tif->next) {
+        ofpbuf_put(reply, &table_id, 1);
+    }
+    end_property(reply, start_ofs);
+
+    put_table_action_features(reply, &tif->write,
+                              OFPTFPT13_WRITE_ACTIONS,
+                              OFPTFPT13_WRITE_SETFIELD, miss_offset, version);
+    put_table_action_features(reply, &tif->apply,
+                              OFPTFPT13_APPLY_ACTIONS,
+                              OFPTFPT13_APPLY_SETFIELD, miss_offset, version);
+}
+
+void
+ofputil_append_table_features_reply(const struct ofputil_table_features *tf,
+                                    struct list *replies)
+{
+    struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
+    enum ofp_version version = ofpmp_version(replies);
+    size_t start_ofs = ofpbuf_size(reply);
+    struct ofp13_table_features *otf;
+
+    otf = ofpbuf_put_zeros(reply, sizeof *otf);
+    otf->table_id = tf->table_id;
+    ovs_strlcpy(otf->name, tf->name, sizeof otf->name);
+    otf->metadata_match = tf->metadata_match;
+    otf->metadata_write = tf->metadata_write;
+    otf->config = ofputil_table_miss_to_config(tf->miss_config, version);
+    otf->max_entries = htonl(tf->max_entries);
+
+    put_table_instruction_features(reply, &tf->nonmiss, 0, version);
+    put_table_instruction_features(reply, &tf->miss, 1, version);
+
+    put_fields_property(reply, &tf->match, &tf->mask,
+                        OFPTFPT13_MATCH, version);
+    put_fields_property(reply, &tf->wildcard, NULL,
+                        OFPTFPT13_WILDCARDS, version);
+
+    otf = ofpbuf_at_assert(reply, start_ofs, sizeof *otf);
+    otf->length = htons(ofpbuf_size(reply) - start_ofs);
+    ofpmp_postappend(replies, start_ofs);
+}
+
 /* ofputil_table_mod */
 
 /* Given 'config', taken from an OpenFlow 'version' message that specifies
@@ -5245,46 +5318,6 @@ ofputil_put_ofp11_table_stats(const struct ofputil_table_stats *stats,
     out->matched_count = htonll(stats->matched_count);
 }
 
-static ovs_be64
-mf_bitmap_to_oxm_bitmap(const struct mf_bitmap *fields,
-                        enum ofp_version version)
-{
-    uint64_t oxm_bitmap = 0;
-    int i;
-
-    BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fields->bm) {
-        uint32_t oxm = mf_oxm_header(i, version);
-        uint32_t vendor = NXM_VENDOR(oxm);
-        int field = NXM_FIELD(oxm);
-
-        if (vendor == OFPXMC12_OPENFLOW_BASIC && field < 64) {
-            oxm_bitmap |= UINT64_C(1) << field;
-        }
-    }
-    return htonll(oxm_bitmap);
-}
-
-static struct mf_bitmap
-mf_bitmap_from_oxm_bitmap(ovs_be64 oxm_bitmap, enum ofp_version version)
-{
-    struct mf_bitmap fields = MF_BITMAP_INITIALIZER;
-
-    for (enum mf_field_id id = 0; id < MFF_N_IDS; id++) {
-        const struct mf_field *f = mf_from_id(id);
-        uint32_t oxm = f->oxm_header;
-        uint32_t vendor = NXM_VENDOR(oxm);
-        int field = NXM_FIELD(oxm);
-
-        if (version >= f->oxm_version
-            && vendor == OFPXMC12_OPENFLOW_BASIC
-            && field < 64
-            && oxm_bitmap & htonll(UINT64_C(1) << field)) {
-            bitmap_set1(fields.bm, id);
-        }
-    }
-    return fields;
-}
-
 static void
 ofputil_put_ofp12_table_stats(const struct ofputil_table_stats *stats,
                               const struct ofputil_table_features *features,
@@ -5295,16 +5328,16 @@ ofputil_put_ofp12_table_stats(const struct ofputil_table_stats *stats,
     out = ofpbuf_put_zeros(buf, sizeof *out);
     out->table_id = features->table_id;
     ovs_strlcpy(out->name, features->name, sizeof out->name);
-    out->match = mf_bitmap_to_oxm_bitmap(&features->match, OFP12_VERSION);
-    out->wildcards = mf_bitmap_to_oxm_bitmap(&features->wildcard,
+    out->match = oxm_bitmap_from_mf_bitmap(&features->match, OFP12_VERSION);
+    out->wildcards = oxm_bitmap_from_mf_bitmap(&features->wildcard,
                                              OFP12_VERSION);
     out->write_actions = ofpact_bitmap_to_openflow(
         features->nonmiss.write.ofpacts, OFP12_VERSION);
     out->apply_actions = ofpact_bitmap_to_openflow(
         features->nonmiss.apply.ofpacts, OFP12_VERSION);
-    out->write_setfields = mf_bitmap_to_oxm_bitmap(
+    out->write_setfields = oxm_bitmap_from_mf_bitmap(
         &features->nonmiss.write.set_fields, OFP12_VERSION);
-    out->apply_setfields = mf_bitmap_to_oxm_bitmap(
+    out->apply_setfields = oxm_bitmap_from_mf_bitmap(
         &features->nonmiss.apply.set_fields, OFP12_VERSION);
     out->metadata_match = features->metadata_match;
     out->metadata_write = features->metadata_write;
@@ -5457,15 +5490,15 @@ ofputil_decode_ofp12_table_stats(struct ofpbuf *msg,
         ots->write_actions, OFP12_VERSION);
     features->nonmiss.apply.ofpacts = ofpact_bitmap_from_openflow(
         ots->apply_actions, OFP12_VERSION);
-    features->nonmiss.write.set_fields = mf_bitmap_from_oxm_bitmap(
+    features->nonmiss.write.set_fields = oxm_bitmap_to_mf_bitmap(
         ots->write_setfields, OFP12_VERSION);
-    features->nonmiss.apply.set_fields = mf_bitmap_from_oxm_bitmap(
+    features->nonmiss.apply.set_fields = oxm_bitmap_to_mf_bitmap(
         ots->apply_setfields, OFP12_VERSION);
     features->miss = features->nonmiss;
 
-    features->match = mf_bitmap_from_oxm_bitmap(ots->match, OFP12_VERSION);
-    features->wildcard = mf_bitmap_from_oxm_bitmap(ots->wildcards,
-                                                   OFP12_VERSION);
+    features->match = oxm_bitmap_to_mf_bitmap(ots->match, OFP12_VERSION);
+    features->wildcard = oxm_bitmap_to_mf_bitmap(ots->wildcards,
+                                                 OFP12_VERSION);
     bitmap_or(features->match.bm, features->wildcard.bm, MFF_N_IDS);
 
     stats->table_id = ots->table_id;
@@ -6187,121 +6220,6 @@ ofputil_pull_phy_port(enum ofp_version ofp_version, struct ofpbuf *b,
     }
 }
 
-/* ofp-util.def lists the mapping from names to action. */
-static const char *const names[OFPUTIL_N_ACTIONS] = {
-    NULL,
-#define OFPAT10_ACTION(ENUM, STRUCT, NAME)             NAME,
-#define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) NAME,
-#define OFPAT13_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) NAME,
-#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)   NAME,
-#include "ofp-util.def"
-};
-
-/* Returns the 'enum ofputil_action_code' corresponding to 'name' (e.g. if
- * 'name' is "output" then the return value is OFPUTIL_OFPAT10_OUTPUT), or -1
- * if 'name' is not the name of any action. */
-int
-ofputil_action_code_from_name(const char *name)
-{
-    const char *const *p;
-
-    for (p = names; p < &names[ARRAY_SIZE(names)]; p++) {
-        if (*p && !strcasecmp(name, *p)) {
-            return p - names;
-        }
-    }
-    return -1;
-}
-
-/* Returns name corresponding to the 'enum ofputil_action_code',
- * or "Unkonwn action", if the name is not available. */
-const char *
-ofputil_action_name_from_code(enum ofputil_action_code code)
-{
-    return code < (int)OFPUTIL_N_ACTIONS && names[code] ? names[code]
-        : "Unknown action";
-}
-
-enum ofputil_action_code
-ofputil_action_code_from_ofp13_action(enum ofp13_action_type type)
-{
-    switch (type) {
-
-#define OFPAT13_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)  \
-    case ENUM:                                          \
-        return OFPUTIL_##ENUM;
-#include "ofp-util.def"
-
-    default:
-        return OFPUTIL_ACTION_INVALID;
-    }
-}
-
-/* Appends an action of the type specified by 'code' to 'buf' and returns the
- * action.  Initializes the parts of 'action' that identify it as having type
- * <ENUM> and length 'sizeof *action' and zeros the rest.  For actions that
- * have variable length, the length used and cleared is that of struct
- * <STRUCT>.  */
-void *
-ofputil_put_action(enum ofputil_action_code code, struct ofpbuf *buf)
-{
-    switch (code) {
-    case OFPUTIL_ACTION_INVALID:
-#define OFPAT13_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
-#include "ofp-util.def"
-        OVS_NOT_REACHED();
-
-#define OFPAT10_ACTION(ENUM, STRUCT, NAME)                  \
-    case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
-#define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)      \
-    case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
-#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)        \
-    case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
-#include "ofp-util.def"
-    }
-    OVS_NOT_REACHED();
-}
-
-#define OFPAT10_ACTION(ENUM, STRUCT, NAME)                        \
-    void                                                        \
-    ofputil_init_##ENUM(struct STRUCT *s)                       \
-    {                                                           \
-        memset(s, 0, sizeof *s);                                \
-        s->type = htons(ENUM);                                  \
-        s->len = htons(sizeof *s);                              \
-    }                                                           \
-                                                                \
-    struct STRUCT *                                             \
-    ofputil_put_##ENUM(struct ofpbuf *buf)                      \
-    {                                                           \
-        struct STRUCT *s = ofpbuf_put_uninit(buf, sizeof *s);   \
-        ofputil_init_##ENUM(s);                                 \
-        return s;                                               \
-    }
-#define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) \
-    OFPAT10_ACTION(ENUM, STRUCT, NAME)
-#define OFPAT13_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) \
-    OFPAT10_ACTION(ENUM, STRUCT, NAME)
-#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)            \
-    void                                                        \
-    ofputil_init_##ENUM(struct STRUCT *s)                       \
-    {                                                           \
-        memset(s, 0, sizeof *s);                                \
-        s->type = htons(OFPAT10_VENDOR);                        \
-        s->len = htons(sizeof *s);                              \
-        s->vendor = htonl(NX_VENDOR_ID);                        \
-        s->subtype = htons(ENUM);                               \
-    }                                                           \
-                                                                \
-    struct STRUCT *                                             \
-    ofputil_put_##ENUM(struct ofpbuf *buf)                      \
-    {                                                           \
-        struct STRUCT *s = ofpbuf_put_uninit(buf, sizeof *s);   \
-        ofputil_init_##ENUM(s);                                 \
-        return s;                                               \
-    }
-#include "ofp-util.def"
-
 static void
 ofputil_normalize_match__(struct match *match, bool may_log)
 {
@@ -7502,6 +7420,20 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
         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();
+    }
+
     LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
         switch (gm->type) {
         case OFPGT11_ALL: