[ovs-dev] [PATCH 16/41] ofp-prop: Add support for experimenter properties.

Ben Pfaff blp at ovn.org
Tue Jan 19 07:27:03 UTC 2016


Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/ofp-prop.c |  98 +++++++++++++++++++++++++++++++++++------
 lib/ofp-prop.h |  57 ++++++++++++++++++++----
 lib/ofp-util.c | 134 ++++++++++++---------------------------------------------
 3 files changed, 160 insertions(+), 129 deletions(-)

diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c
index 2d90e1b..83d72ab 100644
--- a/lib/ofp-prop.c
+++ b/lib/ofp-prop.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,8 +21,21 @@
 #include "byte-order.h"
 #include "ofpbuf.h"
 #include "ofp-errors.h"
+#include "openvswitch/vlog.h"
 #include "util.h"
 
+static uint32_t
+ofpprop_type_to_experimenter(uint64_t type)
+{
+    return type >> 32;
+}
+
+static uint32_t
+ofpprop_type_to_exp_id(uint64_t type)
+{
+    return type & UINT32_MAX;
+}
+
 /* 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
@@ -33,7 +46,8 @@
  * you can use ofpprop_pull() for that case. */
 enum ofperr
 ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property,
-               unsigned int alignment, uint16_t *typep)
+               unsigned int alignment, unsigned int min_exp,
+               uint64_t *typep)
 {
     struct ofp_prop_header *oph;
     unsigned int padded_len;
@@ -50,9 +64,31 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property,
         return OFPERR_OFPBPC_BAD_LEN;
     }
 
-    *typep = ntohs(oph->type);
+    uint16_t type = ntohs(oph->type);
+    if (type < min_exp) {
+        *typep = type;
+    } else {
+        struct ofp_prop_experimenter *ope = msg->data;
+        if (len < sizeof *ope) {
+            return OFPERR_OFPBPC_BAD_LEN;
+        }
+
+        if (!ope->experimenter) {
+            /* Reject experimenter 0 because it yields ambiguity with standard
+             * property types. */
+            return OFPERR_OFPBPC_BAD_EXPERIMENTER;
+        }
+
+        *typep = OFPPROP_EXP(ntohl(ope->experimenter), ntohl(ope->exp_type));
+    }
+
     if (property) {
         ofpbuf_use_const(property, msg->data, len);
+        msg->header = msg->data;
+        msg->msg = ((uint8_t *) msg->data
+                    + (type < min_exp
+                       ? sizeof(struct ofp_prop_header)
+                       : sizeof(struct ofp_prop_experimenter)));
     }
     ofpbuf_pull(msg, padded_len);
     return 0;
@@ -66,15 +102,15 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property,
  * This function pulls the property's stated size padded out to a multiple of
  * 8 bytes, which is the common case for OpenFlow properties. */
 enum ofperr
-ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property, uint16_t *typep)
+ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property, uint64_t *typep)
 {
-    return ofpprop_pull__(msg, property, 8, typep);
+    return ofpprop_pull__(msg, property, 8, 0xffff, typep);
 }
 
 /* Adds a property with the given 'type' and 'len'-byte contents 'value' to
  * 'msg', padding the property out to a multiple of 8 bytes. */
 void
-ofpprop_put(struct ofpbuf *msg, uint16_t type, const void *value, size_t len)
+ofpprop_put(struct ofpbuf *msg, uint64_t type, const void *value, size_t len)
 {
     size_t start_ofs = ofpprop_start(msg, type);
     ofpbuf_put(msg, value, len);
@@ -84,7 +120,7 @@ ofpprop_put(struct ofpbuf *msg, uint16_t type, const void *value, size_t len)
 /* Appends a property to 'msg' whose type is 'type' and whose contents is a
  * series of property headers, one for each 1-bit in 'bitmap'. */
 void
-ofpprop_put_bitmap(struct ofpbuf *msg, uint16_t type, uint64_t bitmap)
+ofpprop_put_bitmap(struct ofpbuf *msg, uint64_t type, uint64_t bitmap)
 {
     size_t start_ofs = ofpprop_start(msg, type);
     for (; bitmap; bitmap = zero_rightmost_1bit(bitmap)) {
@@ -98,14 +134,21 @@ ofpprop_put_bitmap(struct ofpbuf *msg, uint16_t type, uint64_t bitmap)
  * ofpprop_end().  Returns the offset of the beginning of the property (to pass
  * to ofpprop_end() later). */
 size_t
-ofpprop_start(struct ofpbuf *msg, uint16_t type)
+ofpprop_start(struct ofpbuf *msg, uint64_t type)
 {
     size_t start_ofs = msg->size;
-    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 ofpprop_end(). */
+    if (!ofpprop_is_experimenter(type)) {
+        struct ofp_prop_header *oph = ofpbuf_put_uninit(msg, sizeof *oph);
+        oph->type = htons(type);
+        oph->len = htons(4);
+    } else {
+        struct ofp_prop_experimenter *ope
+            = ofpbuf_put_uninit(msg, sizeof *ope);
+        ope->type = htons(0xffff);
+        ope->len = htons(12);
+        ope->experimenter = htonl(ofpprop_type_to_experimenter(type));
+        ope->exp_type = htonl(ofpprop_type_to_exp_id(type));
+    }
     return start_ofs;
 }
 
@@ -123,3 +166,32 @@ ofpprop_end(struct ofpbuf *msg, size_t start_ofs)
     ofpbuf_padto(msg, ROUND_UP(msg->size, 8));
 }
 
+enum ofperr
+ofpprop_unknown(struct vlog_module *module, bool loose, const char *msg,
+                uint64_t type)
+{
+    bool is_experimenter = ofpprop_is_experimenter(type);
+
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+    enum vlog_level level = loose ? VLL_DBG : VLL_WARN;
+    if (!is_experimenter) {
+        vlog_rate_limit(module, level, &rl, "unknown %s property type %"PRId64,
+                        msg, type);
+    } else {
+        vlog_rate_limit(module, level, &rl,
+                        "unknown %s property type for experimenter "
+                        "0x%"PRIx32", exp_id %"PRId32,
+                        msg,
+                        ofpprop_type_to_experimenter(type),
+                        ofpprop_type_to_exp_id(type));
+    }
+
+    /* There's an error OFPBPC_BAD_EXPERIMENTER that we could use for
+     * experimenter IDs that we don't know at all, but that seems like a
+     * difficult distinction and OFPERR_OFPBPC_BAD_EXP_TYPE communicates the
+     * problem quite well. */
+    return (loose ? 0
+            : is_experimenter ? OFPERR_OFPBPC_BAD_EXP_TYPE
+            : OFPERR_OFPBPC_BAD_TYPE);
+}
+
diff --git a/lib/ofp-prop.h b/lib/ofp-prop.h
index 07d9799..fc0c76d 100644
--- a/lib/ofp-prop.h
+++ b/lib/ofp-prop.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,30 +20,64 @@
 /* OpenFlow 1.3+ property support
  * ==============================
  *
- * Several OpenFlow 1.3+ messages use properties that take the common form
- * shown by "struct ofp_prop_header".  This module provides support for
- * serializing and deserializing properties in this format.
+ * Several OpenFlow 1.3+ messages use type-length-value (TLV) properties that
+ * take the common form shown by "struct ofp_prop_header".  This module
+ * provides support for serializing and deserializing properties in this
+ * format.
+ *
+ *
+ * Property types
+ * --------------
+ *
+ * This module uses uint64_t values to identify property types:
+ *
+ *     - OpenFlow assigns 16-bit type values to its own standardized
+ *       properties.  ofpprop uses these values directly in uint64_t.
+ *
+ *     - Vendor-specific "experimenter" properties have a 32-bit "experimenter
+ *       ID" (generally an Ethernet OUI) and a 32-bit experimenter-defined
+ *       "exp_type".  ofpprop encodes these with the experimenter ID in the
+ *       high 32 bits and exp_type in the low 32 bits.  (All existing
+ *       experimenter IDs are nonzero, so this is unambiguous.)  Use
+ *       OFPPROP_EXP to encode these property types.
  */
 
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
 #include "ofp-errors.h"
 #include "openvswitch/types.h"
 
 struct ofpbuf;
+struct vlog_module;
+
+/* Given an OpenFlow experimenter ID (e.g. NX_VENDOR_ID) and exp_type, yields
+ * the code that ofpprop_pull() would use to identify the given experimenter
+ * property. */
+#define OFPPROP_EXP(EXPERIMENTER, EXP_TYPE) \
+    (((uint64_t) (EXPERIMENTER) << 32) | (EXP_TYPE))
+
+/* Returns true if 'type' represents an experimenter property type,
+ * false if it represents a standard property type.*/
+static inline bool
+ofpprop_is_experimenter(uint64_t type)
+{
+    return type > UINT16_MAX;
+}
 
 /* Deserializing properties.  */
 enum ofperr ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property,
-                           unsigned int alignment, uint16_t *typep);
+                           unsigned int alignment, unsigned int min_exp,
+                           uint64_t *typep);
 enum ofperr ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property,
-                         uint16_t *typep);
+                         uint64_t *typep);
 
 /* Serializing properties. */
-void ofpprop_put(struct ofpbuf *, uint16_t type,
+void ofpprop_put(struct ofpbuf *, uint64_t type,
                  const void *value, size_t len);
-void ofpprop_put_bitmap(struct ofpbuf *, uint16_t type, uint64_t bitmap);
+void ofpprop_put_bitmap(struct ofpbuf *, uint64_t type, uint64_t bitmap);
 
-size_t ofpprop_start(struct ofpbuf *, uint16_t type);
+size_t ofpprop_start(struct ofpbuf *, uint64_t type);
 void ofpprop_end(struct ofpbuf *, size_t start_ofs);
 
 /* Logging errors while deserializing properties.
@@ -68,4 +102,9 @@ void ofpprop_end(struct ofpbuf *, size_t start_ofs);
 #define OFPPROP_LOG(RL, LOOSE, ...)                         \
     VLOG_RL(RL, (LOOSE) ? VLL_DBG : VLL_WARN, __VA_ARGS__)
 
+enum ofperr ofpprop_unknown(struct vlog_module *, bool loose, const char *msg,
+                            uint64_t type);
+#define OFPPROP_UNKNOWN(LOOSE, MSG, TYPE) \
+    ofpprop_unknown(THIS_MODULE, LOOSE, MSG, TYPE)
+
 #endif /* ofp-prop.h */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 05cc6d1..449f383 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3785,7 +3785,7 @@ ofputil_pull_ofp14_port(struct ofputil_phy_port *pp, struct ofpbuf *msg)
     while (properties.size > 0) {
         struct ofpbuf payload;
         enum ofperr error;
-        uint16_t type;
+        uint64_t type;
 
         error = ofpprop_pull(&properties, &payload, &type);
         if (error) {
@@ -3798,9 +3798,7 @@ ofputil_pull_ofp14_port(struct ofputil_phy_port *pp, struct ofpbuf *msg)
             break;
 
         default:
-            OFPPROP_LOG(&bad_ofmsg_rl, true,
-                        "unknown port property %"PRIu16, type);
-            error = 0;
+            error = OFPPROP_UNKNOWN(true, "port", type);
             break;
         }
 
@@ -4390,7 +4388,7 @@ ofputil_decode_port_mod(const struct ofp_header *oh,
         while (b.size > 0) {
             struct ofpbuf property;
             enum ofperr error;
-            uint16_t type;
+            uint64_t type;
 
             error = ofpprop_pull(&b, &property, &type);
             if (error) {
@@ -4403,15 +4401,7 @@ ofputil_decode_port_mod(const struct ofp_header *oh,
                 break;
 
             default:
-                OFPPROP_LOG(&bad_ofmsg_rl, loose,
-                            "unknown port_mod property %"PRIu16, type);
-                if (loose) {
-                    error = 0;
-                } else if (type == OFPPMPT14_EXPERIMENTER) {
-                    error = OFPERR_OFPBPC_BAD_EXPERIMENTER;
-                } else {
-                    error = OFPERR_OFPBRC_BAD_TYPE;
-                }
+                error = OFPPROP_UNKNOWN(loose, "port_mod", type);
                 break;
             }
 
@@ -4496,13 +4486,13 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
 
 static enum ofperr
 pull_table_feature_property(struct ofpbuf *msg, struct ofpbuf *payload,
-                            uint16_t *typep)
+                            uint64_t *typep)
 {
     enum ofperr error;
 
     error = ofpprop_pull(msg, payload, typep);
     if (payload && !error) {
-        ofpbuf_pull(payload, sizeof(struct ofp_prop_header));
+        ofpbuf_pull(payload, (uint8_t *)msg->msg - (uint8_t *)msg->header);
     }
     return error;
 }
@@ -4514,10 +4504,10 @@ parse_action_bitmap(struct ofpbuf *payload, enum ofp_version ofp_version,
     uint32_t types = 0;
 
     while (payload->size > 0) {
-        uint16_t type;
         enum ofperr error;
+        uint64_t type;
 
-        error = ofpprop_pull__(payload, NULL, 1, &type);
+        error = ofpprop_pull__(payload, NULL, 1, 0x10000, &type);
         if (error) {
             return error;
         }
@@ -4537,7 +4527,7 @@ parse_instruction_ids(struct ofpbuf *payload, bool loose, uint32_t *insts)
     while (payload->size > 0) {
         enum ovs_instruction_type inst;
         enum ofperr error;
-        uint16_t ofpit;
+        uint64_t ofpit;
 
         /* OF1.3 and OF1.4 aren't clear about padding in the instruction IDs.
          * It seems clear that they aren't padded to 8 bytes, though, because
@@ -4548,7 +4538,7 @@ parse_instruction_ids(struct ofpbuf *payload, bool loose, uint32_t *insts)
          *
          * Anyway, we just assume they're all glommed together on byte
          * boundaries. */
-        error = ofpprop_pull__(payload, NULL, 1, &ofpit);
+        error = ofpprop_pull__(payload, NULL, 1, 0x10000, &ofpit);
         if (error) {
             return error;
         }
@@ -4682,7 +4672,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
     while (properties.size > 0) {
         struct ofpbuf payload;
         enum ofperr error;
-        uint16_t type;
+        uint64_t type;
 
         error = pull_table_feature_property(&properties, &payload, &type);
         if (error) {
@@ -4752,9 +4742,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
         case OFPTFPT13_EXPERIMENTER:
         case OFPTFPT13_EXPERIMENTER_MISS:
         default:
-            OFPPROP_LOG(&bad_ofmsg_rl, loose,
-                        "unknown table features property %"PRIu16, type);
-            error = loose ? 0 : OFPERR_OFPBPC_BAD_TYPE;
+            error = OFPPROP_UNKNOWN(loose, "table features", type);
             break;
         }
         if (error) {
@@ -4977,7 +4965,7 @@ ofputil_decode_table_desc(struct ofpbuf *msg,
     while (properties.size > 0) {
         struct ofpbuf payload;
         enum ofperr error;
-        uint16_t type;
+        uint64_t type;
 
         error = ofpprop_pull(&properties, &payload, &type);
         if (error) {
@@ -4994,9 +4982,7 @@ ofputil_decode_table_desc(struct ofpbuf *msg,
             break;
 
         default:
-            OFPPROP_LOG(&bad_ofmsg_rl, true,
-                        "unknown table_desc property %"PRIu16, type);
-            error = 0;
+            error = OFPPROP_UNKNOWN(true, "table_desc", type);
             break;
         }
 
@@ -5266,7 +5252,7 @@ ofputil_decode_table_mod(const struct ofp_header *oh,
         while (b.size > 0) {
             struct ofpbuf property;
             enum ofperr error;
-            uint16_t type;
+            uint64_t type;
 
             error = ofpprop_pull(&b, &property, &type);
             if (error) {
@@ -7187,7 +7173,7 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops,
     while (properties.size > 0) {
         struct ofpbuf payload;
         enum ofperr error;
-        uint16_t type;
+        uint64_t type;
 
         error = ofpprop_pull(&properties, &payload, &type);
         if (error) {
@@ -7200,9 +7186,7 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops,
             break;
 
         default:
-            OFPPROP_LOG(&bad_ofmsg_rl, true,
-                        "unknown port stats property %"PRIu16, type);
-            error = 0;
+            error = OFPPROP_UNKNOWN(true, "port stats", type);
             break;
         }
 
@@ -8128,7 +8112,7 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length,
 
         while (properties.size > 0) {
             struct ofpbuf payload;
-            uint16_t type;
+            uint64_t type;
 
             err = ofpprop_pull(&properties, &payload, &type);
             if (err) {
@@ -8151,9 +8135,7 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length,
                 break;
 
             default:
-                OFPPROP_LOG(&bad_ofmsg_rl, false,
-                            "unknown group bucket property %"PRIu16, type);
-                err = OFPERR_OFPBPC_BAD_TYPE;
+                err = OFPPROP_UNKNOWN(false, "group bucket", type);
                 break;
             }
 
@@ -8295,67 +8277,6 @@ parse_group_prop_ntr_selection_method(struct ofpbuf *payload,
 }
 
 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:
-        OFPPROP_LOG(&bad_ofmsg_rl, 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:
-    case NTR_COMPAT_VENDOR_ID:
-        error = parse_group_prop_ntr(payload, exp_type, group_type,
-                                     group_cmd, gp);
-        break;
-
-    default:
-        OFPPROP_LOG(&bad_ofmsg_rl, 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,
@@ -8370,7 +8291,7 @@ parse_ofp15_group_properties(struct ofpbuf *msg,
     while (properties.size > 0) {
         struct ofpbuf payload;
         enum ofperr error;
-        uint16_t type;
+        uint64_t type;
 
         error = ofpprop_pull(&properties, &payload, &type);
         if (error) {
@@ -8378,15 +8299,14 @@ parse_ofp15_group_properties(struct ofpbuf *msg,
         }
 
         switch (type) {
-        case OFPGPT15_EXPERIMENTER:
-            error = parse_ofp15_group_prop_exp(&payload, group_type,
-                                               group_cmd, gp);
+        case OFPPROP_EXP(NTR_VENDOR_ID, NTRT_SELECTION_METHOD):
+        case OFPPROP_EXP(NTR_COMPAT_VENDOR_ID, NTRT_SELECTION_METHOD):
+            error = parse_group_prop_ntr_selection_method(&payload, group_type,
+                                                          group_cmd, gp);
             break;
 
         default:
-            OFPPROP_LOG(&bad_ofmsg_rl, false,
-                        "unknown group property %"PRIu16, type);
-            error = OFPERR_OFPBPC_BAD_TYPE;
+            error = OFPPROP_UNKNOWN(false, "group", type);
             break;
         }
 
@@ -9600,9 +9520,9 @@ ofputil_decode_set_async_config(const struct ofp_header *oh,
             struct ofp14_async_config_prop_reasons *msg;
             struct ofpbuf property;
             enum ofperr error;
-            uint16_t type;
+            uint64_t type;
 
-            error = ofpprop_pull(&b, &property, &type);
+            error = ofpprop_pull__(&b, &property, 8, 0xfffe, &type);
             if (error) {
                 return error;
             }
-- 
2.1.3




More information about the dev mailing list