[ovs-dev] [PATCH v3 4/8] Support decoding of NTR selection method

Simon Horman simon.horman at netronome.com
Mon Mar 9 01:10:59 UTC 2015


This is in preparation for supporting group mod and desc reply
messages with an NTR selection method group experimenter property.

Currently decoding always fails as it only allows properties for known
selection methods and no selection methods are known yet. A subsequent
patch will propose a hash selection method.

NTR selection method
Signed-off-by: Simon Horman <simon.horman at netronome.com>

---

v3
* Add check to only permit known selection methods: currently none
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix

v2
* Use list of struct field_array of TLVs rather than OF1.1 match
  for fields field of NTR selection method property
---
 lib/meta-flow.c            |   9 ++
 lib/meta-flow.h            |  10 ++
 lib/nx-match.c             |  49 +++++++++
 lib/nx-match.h             |   2 +
 lib/ofp-util.c             | 249 +++++++++++++++++++++++++++++++++++++++++++--
 lib/ofp-util.h             |  15 +++
 ofproto/ofproto-provider.h |   5 +
 ofproto/ofproto.c          |   7 ++
 8 files changed, 338 insertions(+), 8 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 8058e07..54e7f58 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -2278,3 +2278,12 @@ mf_format_subvalue(const union mf_subvalue *subvalue, struct ds *s)
     }
     ds_put_char(s, '0');
 }
+
+void
+field_array_set(enum mf_field_id id, const union mf_value *value,
+                struct field_array *fa)
+{
+    ovs_assert(id < MFF_N_IDS);
+    bitmap_set1(fa->used.bm, id);
+    fa->value[id] = *value;
+}
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 0e09036..ba87aff 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1574,6 +1574,12 @@ union mf_subvalue {
 };
 BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue));
 
+/* An array of fields with values */
+struct field_array {
+    struct mf_bitmap used;
+    union mf_value value[MFF_N_IDS];
+};
+
 /* Finding mf_fields. */
 const struct mf_field *mf_from_name(const char *name);
 
@@ -1652,4 +1658,8 @@ void mf_format(const struct mf_field *,
                struct ds *);
 void mf_format_subvalue(const union mf_subvalue *subvalue, struct ds *s);
 
+/* Field Arrays. */
+void field_array_set(enum mf_field_id id, const union mf_value *,
+                     struct field_array *);
+
 #endif /* meta-flow.h */
diff --git a/lib/nx-match.c b/lib/nx-match.c
index fd0e0c8..e51c9a3 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -589,6 +589,55 @@ oxm_pull_match_loose(struct ofpbuf *b, struct match *match)
 {
     return oxm_pull_match__(b, false, match);
 }
+
+/* Verify an array of OXM TLVs treating value of each TLV as a mask,
+ * disallowing masks in each TLV and ignoring pre-requisites. */
+enum ofperr
+oxm_pull_field_array(const void *fields_data, size_t fields_len,
+                     struct field_array *fa)
+{
+    struct ofpbuf b;
+
+    ofpbuf_use_const(&b, fields_data, fields_len);
+    while (b.size) {
+        const uint8_t *pos = b.data;
+        const struct mf_field *field;
+        union mf_value value, mask;
+        enum ofperr error;
+
+        error = nx_pull_match_entry(&b, false, &field, &value, &mask);
+        if (error) {
+            VLOG_DBG_RL(&rl, "error pulling field array field");
+            return error;
+        } else if (!field) {
+            VLOG_DBG_RL(&rl, "unknown field array field");
+            error = OFPERR_OFPBMC_BAD_FIELD;
+        } else if (bitmap_is_set(fa->used.bm, field->id)) {
+            VLOG_DBG_RL(&rl, "duplicate field array field '%s'", field->name);
+            error = OFPERR_OFPBMC_DUP_FIELD;
+        } else if (!mf_is_mask_valid(field, &value)) {
+            VLOG_DBG_RL(&rl, "bad mask in field array field '%s'", field->name);
+            return OFPERR_OFPBMC_BAD_MASK;
+        } else if (!is_all_ones(&mask, field->n_bytes)) {
+            VLOG_DBG_RL(&rl, "mask has a mask in field array field '%s'",
+                        field->name);
+            return OFPERR_OFPBMC_BAD_VALUE;
+        } else {
+            field_array_set(field->id, &value, fa);
+        }
+
+        if (error) {
+            const uint8_t *start = fields_data;
+
+            VLOG_DBG_RL(&rl, "error parsing OXM at offset %"PRIdPTR" "
+                        "within field array (%s)", pos - start,
+                        ofperr_to_string(error));
+            return error;
+        }
+    }
+
+    return 0;
+}
 
 /* nx_put_match() and helpers.
  *
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 9cb6461..0992536 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -55,6 +55,8 @@ enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len,
                                 ovs_be64 *cookie_mask);
 enum ofperr oxm_pull_match(struct ofpbuf *, struct match *);
 enum ofperr oxm_pull_match_loose(struct ofpbuf *, struct match *);
+enum ofperr oxm_pull_field_array(const void *, size_t fields_len,
+                                 struct field_array *);
 int nx_put_match(struct ofpbuf *, const struct match *,
                  ovs_be64 cookie, ovs_be64 cookie_mask);
 int oxm_put_match(struct ofpbuf *, const struct match *, enum ofp_version);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d5927a9..34e6df4 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -38,6 +38,7 @@
 #include "ofp-msgs.h"
 #include "ofp-util.h"
 #include "ofpbuf.h"
+#include "openflow/netronome-ext.h"
 #include "packets.h"
 #include "random.h"
 #include "unaligned.h"
@@ -59,6 +60,14 @@ struct ofp_prop_header {
     ovs_be16 len;
 };
 
+struct ofp_prop_experimenter {
+    ovs_be16 type;          /* OFP*_EXPERIMENTER. */
+    ovs_be16 length;        /* Length in bytes of this property. */
+    ovs_be32 experimenter;  /* Experimenter ID which takes the same form as
+                             * in struct ofp_experimenter_header. */
+    ovs_be32 exp_type;      /* Experimenter defined. */
+};
+
 /* Pulls a property, beginning with struct ofp_prop_header, from the beginning
  * of 'msg'.  Stores the type of the property in '*typep' and, if 'property' is
  * nonnull, the entire property, including the header, in '*property'.  Returns
@@ -7020,6 +7029,13 @@ ofputil_encode_group_stats_request(enum ofp_version ofp_version,
     return request;
 }
 
+void
+ofputil_uninit_group_desc(struct ofputil_group_desc *gd)
+{
+    ofputil_bucket_list_destroy(&gd->buckets);
+    free(&gd->props.fields);
+}
+
 /* Decodes the OpenFlow group description request in 'oh', returning the group
  * whose description is requested, or OFPG_ALL if stats for all groups was
  * requested. */
@@ -7721,6 +7737,199 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length,
     return 0;
 }
 
+static void
+ofputil_init_group_properties(struct ofputil_group_props *gp)
+{
+    memset(gp, 0, sizeof *gp);
+}
+
+static enum ofperr
+parse_group_prop_ntr_selection_method(struct ofpbuf *payload,
+                                      enum ofp11_group_type group_type,
+                                      enum ofp15_group_mod_command group_cmd,
+                                      struct ofputil_group_props *gp)
+{
+    struct ntr_group_prop_selection_method *prop = payload->data;
+    size_t fields_len, method_len;
+    enum ofperr error;
+
+    switch (group_type) {
+    case OFPGT11_SELECT:
+        break;
+    case OFPGT11_ALL:
+    case OFPGT11_INDIRECT:
+    case OFPGT11_FF:
+        log_property(false, "ntr selection method property is only allowed "
+                     "for select groups");
+        return OFPERR_OFPBPC_BAD_VALUE;
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    switch (group_cmd) {
+    case OFPGC15_ADD:
+    case OFPGC15_MODIFY:
+        break;
+    case OFPGC15_DELETE:
+    case OFPGC15_INSERT_BUCKET:
+    case OFPGC15_REMOVE_BUCKET:
+        log_property(false, "ntr selection method property is only allowed "
+                     "for add and delete group modifications");
+        return OFPERR_OFPBPC_BAD_VALUE;
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    if (payload->size < sizeof *prop) {
+        log_property(false, "ntr selection method property length "
+                     "%u is not valid", payload->size);
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    method_len = strnlen(prop->selection_method, NTR_MAX_SELECTION_METHOD_LEN);
+
+    if (method_len == NTR_MAX_SELECTION_METHOD_LEN) {
+        log_property(false, "ntr selection method is not null terminated");
+        return OFPERR_OFPBPC_BAD_VALUE;
+    }
+
+    /* Only allow selection method property if the selection_method field
+     * matches a suported method. As no methods are currently supported
+     * this check is a no-op that always fails. As selection methods are
+     * added they should be checked against the selection_method field
+     * here. */
+    log_property(false, "ntr selection method '%s' is not supported",
+                 prop->selection_method);
+    return OFPERR_OFPBPC_BAD_VALUE;
+
+    strcpy(gp->selection_method, prop->selection_method);
+    gp->selection_method_param = ntohll(prop->selection_method_param);
+
+    if (!method_len && gp->selection_method_param) {
+        log_property(false, "ntr selection method parameter is non-zero but "
+                     "selection method is empty");
+        return OFPERR_OFPBPC_BAD_VALUE;
+    }
+
+    ofpbuf_pull(payload, sizeof *prop);
+
+    fields_len = ntohs(prop->length) - sizeof *prop;
+    if (!method_len && fields_len) {
+        log_property(false, "ntr selection method parameter is zero "
+                     "but fields are provided");
+        return OFPERR_OFPBPC_BAD_VALUE;
+    }
+
+    error = oxm_pull_field_array(payload->data, fields_len,
+                                 &gp->fields);
+    if (error) {
+        log_property(false, "ntr selection method fields are invalid");
+        return error;
+    }
+
+    return 0;
+}
+
+static enum ofperr
+parse_group_prop_ntr(struct ofpbuf *payload, uint32_t exp_type,
+                     enum ofp11_group_type group_type,
+                     enum ofp15_group_mod_command group_cmd,
+                     struct ofputil_group_props *gp)
+{
+    enum ofperr error;
+
+    switch (exp_type) {
+    case NTRT_SELECTION_METHOD:
+        error = parse_group_prop_ntr_selection_method(payload, group_type,
+                                                      group_cmd, gp);
+        break;
+
+    default:
+        log_property(false, "unknown group property ntr experimenter type "
+                     "%"PRIu32, exp_type);
+        error = OFPERR_OFPBPC_BAD_TYPE;
+        break;
+    }
+
+    return error;
+}
+
+static enum ofperr
+parse_ofp15_group_prop_exp(struct ofpbuf *payload,
+                           enum ofp11_group_type group_type,
+                           enum ofp15_group_mod_command group_cmd,
+                           struct ofputil_group_props *gp)
+{
+    struct ofp_prop_experimenter *prop = payload->data;
+    uint16_t experimenter;
+    uint32_t exp_type;
+    enum ofperr error;
+
+    if (payload->size < sizeof *prop) {
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    experimenter = ntohl(prop->experimenter);
+    exp_type = ntohl(prop->exp_type);
+
+    switch (experimenter) {
+    case NTR_VENDOR_ID:
+        error = parse_group_prop_ntr(payload, exp_type, group_type,
+                                     group_cmd, gp);
+        break;
+
+    default:
+        log_property(false, "unknown group property experimenter %"PRIu16,
+                     experimenter);
+        error = OFPERR_OFPBPC_BAD_EXPERIMENTER;
+        break;
+    }
+
+    return error;
+}
+
+static enum ofperr
+parse_ofp15_group_properties(struct ofpbuf *msg,
+                             enum ofp11_group_type group_type,
+                             enum ofp15_group_mod_command group_cmd,
+                             struct ofputil_group_props *gp,
+                             size_t properties_len)
+{
+    struct ofpbuf properties;
+
+    ofpbuf_use_const(&properties, ofpbuf_pull(msg, properties_len),
+                     properties_len);
+
+    while (properties.size > 0) {
+        struct ofpbuf payload;
+        enum ofperr error;
+        uint16_t type;
+
+        error = ofputil_pull_property(&properties, &payload, &type);
+        if (error) {
+            return error;
+        }
+
+        switch (type) {
+        case OFPGPT15_EXPERIMENTER:
+            error = parse_ofp15_group_prop_exp(&payload, group_type,
+                                               group_cmd, gp);
+            break;
+
+        default:
+            log_property(false, "unknown group property %"PRIu16, type);
+            error = OFPERR_OFPBPC_BAD_TYPE;
+            break;
+        }
+
+        if (error) {
+            return error;
+        }
+    }
+
+    return 0;
+}
+
 static int
 ofputil_decode_ofp11_group_desc_reply(struct ofputil_group_desc *gd,
                                       struct ofpbuf *msg,
@@ -7764,6 +7973,7 @@ ofputil_decode_ofp15_group_desc_reply(struct ofputil_group_desc *gd,
 {
     struct ofp15_group_desc_stats *ogds;
     uint16_t length, bucket_list_len;
+    int error;
 
     if (!msg->header) {
         ofpraw_pull_assert(msg);
@@ -7795,9 +8005,22 @@ ofputil_decode_ofp15_group_desc_reply(struct ofputil_group_desc *gd,
                      "bucket list length %u", bucket_list_len);
         return OFPERR_OFPBRC_BAD_LEN;
     }
+    error = ofputil_pull_ofp15_buckets(msg, bucket_list_len, version,
+                                       &gd->buckets);
+    if (error) {
+        return error;
+    }
 
-    return ofputil_pull_ofp15_buckets(msg, bucket_list_len, version,
-                                      &gd->buckets);
+    /* By definition group desc messages don't have a group mod command.
+     * However, parse_group_prop_ntr_selection_method() checks to make sure
+     * that the command is OFPGC15_ADD or OFPGC15_DELETE to guard
+     * against group mod messages with other commands supplying
+     * a NTR selection method group experimenter property.
+     * Such properties are valid for group desc replies so
+     * claim that the group mod command is OFPGC15_ADD to
+     * satisfy the check in parse_group_prop_ntr_selection_method() */
+    return parse_ofp15_group_properties(msg, gd->type, OFPGC15_ADD, &gd->props,
+                                        msg->size);
 }
 
 /* Converts a group description reply in 'msg' into an abstract
@@ -7814,6 +8037,8 @@ int
 ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
                                 struct ofpbuf *msg, enum ofp_version version)
 {
+    ofputil_init_group_properties(&gd->props);
+
     switch (version)
     {
     case OFP11_VERSION:
@@ -7831,6 +8056,12 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
     }
 }
 
+void
+ofputil_uninit_group_mod(struct ofputil_group_mod *gm)
+{
+    ofputil_bucket_list_destroy(&gm->buckets);
+}
+
 static struct ofpbuf *
 ofputil_encode_ofp11_group_mod(enum ofp_version ofp_version,
                                const struct ofputil_group_mod *gm)
@@ -8064,14 +8295,14 @@ ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version,
     }
 
     bucket_list_len = ntohs(ogm->bucket_array_len);
-    if (bucket_list_len < msg->size) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "group has %u trailing bytes",
-                     msg->size - bucket_list_len);
-        return OFPERR_OFPGMFC_BAD_BUCKET;
+    error = ofputil_pull_ofp15_buckets(msg, bucket_list_len, ofp_version,
+                                       &gm->buckets);
+    if (error) {
+        return error;
     }
 
-    return ofputil_pull_ofp15_buckets(msg, bucket_list_len, ofp_version,
-                                      &gm->buckets);
+    return parse_ofp15_group_properties(msg, gm->type, gm->command, &gm->props,
+                                        msg->size);
 }
 
 /* Converts OpenFlow group mod message 'oh' into an abstract group mod in
@@ -8088,6 +8319,8 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
     ofpbuf_use_const(&msg, oh, ntohs(oh->length));
     ofpraw_pull_assert(&msg);
 
+    ofputil_init_group_properties(&gm->props);
+
     switch (ofp_version)
     {
     case OFP11_VERSION:
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index df4d044..ee3f1be 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -27,6 +27,7 @@
 #include "match.h"
 #include "meta-flow.h"
 #include "netdev.h"
+#include "openflow/netronome-ext.h"
 #include "openflow/nicira-ext.h"
 #include "openvswitch/types.h"
 #include "type-props.h"
@@ -217,6 +218,8 @@ void ofputil_match_to_ofp10_match(const struct match *, struct ofp10_match *);
 /* Work with ofp11_match. */
 enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, struct match *,
                                      uint16_t *padded_match_len);
+enum ofperr ofputil_pull_ofp11_mask(struct ofpbuf *, struct match *,
+                                    struct mf_bitmap *bm);
 enum ofperr ofputil_match_from_ofp11_match(const struct ofp11_match *,
                                            struct match *);
 int ofputil_put_ofp11_match(struct ofpbuf *, const struct match *,
@@ -994,6 +997,14 @@ struct ofputil_bucket {
 };
 
 /* Protocol-independent group_mod. */
+struct ofputil_group_props {
+    /* NTR selection method */
+    char selection_method[NTR_MAX_SELECTION_METHOD_LEN];
+    uint64_t selection_method_param;
+    struct field_array fields;
+};
+
+/* Protocol-independent group_mod. */
 struct ofputil_group_mod {
     uint16_t command;             /* One of OFPGC15_*. */
     uint8_t type;                 /* One of OFPGT11_*. */
@@ -1003,6 +1014,7 @@ struct ofputil_group_mod {
                                    * OFPGC15_REMOVE_BUCKET commands
                                    * execution.*/
     struct ovs_list buckets;      /* Contains "struct ofputil_bucket"s. */
+    struct ofputil_group_props props; /* Group properties. */
 };
 
 /* Group stats reply, independent of protocol. */
@@ -1032,6 +1044,7 @@ struct ofputil_group_desc {
     uint8_t type;               /* One of OFPGT_*. */
     uint32_t group_id;          /* Group identifier. */
     struct ovs_list buckets;    /* Contains "struct ofputil_bucket"s. */
+    struct ofputil_group_props props; /* Group properties. */
 };
 
 void ofputil_bucket_list_destroy(struct ovs_list *buckets);
@@ -1062,6 +1075,7 @@ struct ofpbuf *ofputil_encode_group_features_reply(
     const struct ofputil_group_features *, const struct ofp_header *request);
 void ofputil_decode_group_features_reply(const struct ofp_header *,
                                          struct ofputil_group_features *);
+void ofputil_uninit_group_mod(struct ofputil_group_mod *gm);
 struct ofpbuf *ofputil_encode_group_mod(enum ofp_version ofp_version,
                                         const struct ofputil_group_mod *gm);
 
@@ -1071,6 +1085,7 @@ enum ofperr ofputil_decode_group_mod(const struct ofp_header *,
 int ofputil_decode_group_stats_reply(struct ofpbuf *,
                                      struct ofputil_group_stats *);
 
+void ofputil_uninit_group_desc(struct ofputil_group_desc *gd);
 uint32_t ofputil_decode_group_desc_request(const struct ofp_header *);
 struct ofpbuf *ofputil_encode_group_desc_request(enum ofp_version,
                                                  uint32_t group_id);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7208541..05a2dfe 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -499,6 +499,11 @@ struct ofgroup {
 
     struct ovs_list buckets;        /* Contains "struct ofputil_bucket"s. */
     const uint32_t n_buckets;
+
+    /* NTR selection method */
+    const char selection_method[NTR_MAX_SELECTION_METHOD_LEN];
+    const uint64_t selection_method_param;
+    const struct field_array fields;   /* List of other elements in the array */
 };
 
 bool ofproto_group_lookup(const struct ofproto *ofproto, uint32_t group_id,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 07a1f5d..08eed78 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5929,6 +5929,13 @@ init_group(struct ofproto *ofproto, struct ofputil_group_mod *gm,
     *CONST_CAST(uint32_t *, &(*ofgroup)->n_buckets) =
         list_size(&(*ofgroup)->buckets);
 
+    memcpy(CONST_CAST(char *, (*ofgroup)->selection_method),
+           gm->props.selection_method, NTR_MAX_SELECTION_METHOD_LEN);
+    *CONST_CAST(uint64_t *, &(*ofgroup)->selection_method_param) =
+        gm->props.selection_method_param;
+    memcpy(CONST_CAST(struct field_array *, &(*ofgroup)->fields),
+           &gm->props.fields, sizeof gm->props.fields);
+
     /* Construct called BEFORE any locks are held. */
     error = ofproto->ofproto_class->group_construct(*ofgroup);
     if (error) {
-- 
2.1.4




More information about the dev mailing list