[ovs-dev] [of1.5 v2 05/20] ofp-util: Generalize functions for parsing OF1.3+ properties.

Ben Pfaff blp at nicira.com
Sat May 10 02:40:21 UTC 2014


The main effect is to move these functions a little earlier in the file.

Also, OpenFlow 1.4 changed the table-features specific error codes to new
values that apply to all property sets, so this commit updates the error
code names and adds the appropriate OpenFlow 1.4+ codes.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-errors.h |  46 +++++++++++++++++-----
 lib/ofp-util.c   | 115 +++++++++++++++++++++++++++++++------------------------
 2 files changed, 103 insertions(+), 58 deletions(-)

diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index b1bcf7cc..2895138 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -545,17 +545,45 @@ enum ofperr {
     /* OF1.3+(13,1).  Invalid metadata mask. */
     OFPERR_OFPTFFC_BAD_METADATA,
 
-    /* OF1.3+(13,2).  Unknown property type. */
-    OFPERR_OFPTFFC_BAD_TYPE,
+    /* OF1.3+(13,5).  Permissions error. */
+    OFPERR_OFPTFFC_EPERM,
 
-    /* OF1.3+(13,3).  Length problem in properties. */
-    OFPERR_OFPTFFC_BAD_LEN,
+/* ## ------------------ ## */
+/* ## OFPET_BAD_PROPERTY ## */
+/* ## ------------------ ## */
 
-    /* OF1.3+(13,4).  Unsupported property value. */
-    OFPERR_OFPTFFC_BAD_ARGUMENT,
+    /* OF1.3(13,2), OF1.4+(14,0).  Unknown property type.
+     *
+     * [Known as OFPTFFC_BAD_TYPE in OF1.3.] */
+    OFPERR_OFPBPC_BAD_TYPE,
 
-    /* OF1.3+(13,5).  Permissions error. */
-    OFPERR_OFPTFFC_EPERM,
+    /* OF1.3(13,3), OF1.4+(14,1).  Length problem in property.
+     *
+     * [Known as OFPTFFC_BAD_LEN in OF1.3.] */
+    OFPERR_OFPBPC_BAD_LEN,
+
+    /* OF1.3(13,4), OF1.4+(14,2).  Unsupported property value.
+     *
+     * [Known as OFPTFFC_BAD_ARGUMENT in OF1.3.] */
+    OFPERR_OFPBPC_BAD_VALUE,
+
+    /* OF1.4+(14,3).  Can't handle this many properties. */
+    OFPERR_OFPBPC_TOO_MANY,
+
+    /* OF1.4+(14,4).  A property type was duplicated. */
+    OFPERR_OFPBPC_DUP_TYPE,
+
+    /* OF1.4+(14,5).  Unknown experimenter id specified. */
+    OFPERR_OFPBPC_BAD_EXPERIMENTER,
+
+    /* OF1.4+(14,6).  Unknown exp_type for experimenter id. */
+    OFPERR_OFPBPC_BAD_EXP_TYPE,
+
+    /* OF1.4+(14,7).  Unknown value for experimenter id. */
+    OFPERR_OFPBPC_BAD_EXP_VALUE,
+
+    /* OF1.4+(14,8).  Permissions error. */
+    OFPERR_OFPBPC_EPERM,
 
 /* ## -------------------- ## */
 /* ## OFPET_BUNDLE_FAILED  ## */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1ba743f..59667e6 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -50,6 +50,53 @@ VLOG_DEFINE_THIS_MODULE(ofp_util);
  * in the peer and so there's not much point in showing a lot of them. */
 static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
+struct ofp_prop_header {
+    ovs_be16 type;
+    ovs_be16 len;
+};
+
+/* 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
+ * 0 if successful, otherwise an error code. */
+static enum ofperr
+ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property,
+                      uint16_t *typep)
+{
+    struct ofp_prop_header *oph;
+    unsigned int len;
+
+    if (ofpbuf_size(msg) < sizeof *oph) {
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    oph = ofpbuf_data(msg);
+    len = ntohs(oph->len);
+    if (len < sizeof *oph || ROUND_UP(len, 8) > ofpbuf_size(msg)) {
+        return OFPERR_OFPBPC_BAD_LEN;
+    }
+
+    *typep = ntohs(oph->type);
+    if (property) {
+        ofpbuf_use_const(property, ofpbuf_data(msg), len);
+    }
+    ofpbuf_pull(msg, ROUND_UP(len, 8));
+    return 0;
+}
+
+
+static void PRINTF_FORMAT(2, 3)
+log_property(bool loose, const char *message, ...)
+{
+    enum vlog_level level = loose ? VLL_DBG : VLL_WARN;
+    if (!vlog_should_drop(THIS_MODULE, level, &bad_ofmsg_rl)) {
+        va_list args;
+
+        va_start(args, message);
+        vlog_valist(THIS_MODULE, level, message, args);
+        va_end(args);
+    }
+}
 /* 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.
@@ -4208,48 +4255,17 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
     return b;
 }
 
-struct ofp_prop_header {
-    ovs_be16 type;
-    ovs_be16 len;
-};
-
 static enum ofperr
-ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *payload,
-                      uint16_t *typep)
-{
-    struct ofp_prop_header *oph;
-    unsigned int len;
-
-    if (ofpbuf_size(msg) < sizeof *oph) {
-        return OFPERR_OFPTFFC_BAD_LEN;
-    }
-
-    oph = ofpbuf_data(msg);
-    len = ntohs(oph->len);
-    if (len < sizeof *oph || ROUND_UP(len, 8) > ofpbuf_size(msg)) {
-        return OFPERR_OFPTFFC_BAD_LEN;
-    }
-
-    *typep = ntohs(oph->type);
-    if (payload) {
-        ofpbuf_use_const(payload, ofpbuf_data(msg), len);
-        ofpbuf_pull(payload, sizeof *oph);
-    }
-    ofpbuf_pull(msg, ROUND_UP(len, 8));
-    return 0;
-}
-
-static void PRINTF_FORMAT(2, 3)
-log_property(bool loose, const char *message, ...)
+pull_table_feature_property(struct ofpbuf *msg, struct ofpbuf *payload,
+                        uint16_t *typep)
 {
-    enum vlog_level level = loose ? VLL_DBG : VLL_WARN;
-    if (!vlog_should_drop(THIS_MODULE, level, &bad_ofmsg_rl)) {
-        va_list args;
+    enum ofperr error;
 
-        va_start(args, message);
-        vlog_valist(THIS_MODULE, level, message, args);
-        va_end(args);
+    error = ofputil_pull_property(msg, payload, typep);
+    if (payload && !error) {
+        ofpbuf_pull(payload, sizeof(struct ofp_prop_header));
     }
+    return error;
 }
 
 static enum ofperr
@@ -4259,7 +4275,7 @@ parse_table_ids(struct ofpbuf *payload, uint32_t *ids)
 
     *ids = 0;
     while (ofpbuf_size(payload) > 0) {
-        enum ofperr error = ofputil_pull_property(payload, NULL, &type);
+        enum ofperr error = pull_table_feature_property(payload, NULL, &type);
         if (error) {
             return error;
         }
@@ -4279,7 +4295,7 @@ parse_instruction_ids(struct ofpbuf *payload, bool loose, uint32_t *insts)
         enum ofperr error;
         uint16_t ofpit;
 
-        error = ofputil_pull_property(payload, NULL, &ofpit);
+        error = pull_table_feature_property(payload, NULL, &ofpit);
         if (error) {
             return error;
         }
@@ -4304,7 +4320,7 @@ parse_table_features_next_table(struct ofpbuf *payload,
     for (i = 0; i < ofpbuf_size(payload); i++) {
         uint8_t id = ((const uint8_t *) ofpbuf_data(payload))[i];
         if (id >= 255) {
-            return OFPERR_OFPTFFC_BAD_ARGUMENT;
+            return OFPERR_OFPBPC_BAD_VALUE;
         }
         bitmap_set1(next_tables, id);
     }
@@ -4320,7 +4336,7 @@ parse_oxm(struct ofpbuf *b, bool loose,
 
     oxmp = ofpbuf_try_pull(b, sizeof *oxmp);
     if (!oxmp) {
-        return OFPERR_OFPTFFC_BAD_LEN;
+        return OFPERR_OFPBPC_BAD_LEN;
     }
     oxm = ntohl(*oxmp);
 
@@ -4331,7 +4347,7 @@ parse_oxm(struct ofpbuf *b, bool loose,
     *hasmask = NXM_HASMASK(oxm);
     if (*hasmask) {
         if (NXM_LENGTH(oxm) & 1) {
-            return OFPERR_OFPTFFC_BAD_ARGUMENT;
+            return OFPERR_OFPBPC_BAD_VALUE;
         }
         oxm = NXM_HEADER(NXM_VENDOR(oxm), NXM_FIELD(oxm), NXM_LENGTH(oxm) / 2);
     }
@@ -4411,13 +4427,13 @@ ofputil_decode_table_features(struct ofpbuf *msg,
     }
 
     if (ofpbuf_size(msg) < sizeof *otf) {
-        return OFPERR_OFPTFFC_BAD_LEN;
+        return OFPERR_OFPBPC_BAD_LEN;
     }
 
     otf = ofpbuf_data(msg);
     len = ntohs(otf->length);
     if (len < sizeof *otf || len % 8 || len > ofpbuf_size(msg)) {
-        return OFPERR_OFPTFFC_BAD_LEN;
+        return OFPERR_OFPBPC_BAD_LEN;
     }
     ofpbuf_pull(msg, sizeof *otf);
 
@@ -4437,7 +4453,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
         enum ofperr error;
         uint16_t type;
 
-        error = ofputil_pull_property(msg, &payload, &type);
+        error = pull_table_feature_property(msg, &payload, &type);
         if (error) {
             return error;
         }
@@ -4500,9 +4516,10 @@ ofputil_decode_table_features(struct ofpbuf *msg,
 
         case OFPTFPT13_EXPERIMENTER:
         case OFPTFPT13_EXPERIMENTER_MISS:
-            log_property(loose,
-                         "unknown table features experimenter property");
-            error = loose ? 0 : OFPERR_OFPTFFC_BAD_TYPE;
+        default:
+            log_property(loose, "unknown table features property %"PRIu16,
+                         type);
+            error = loose ? 0 : OFPERR_OFPBPC_BAD_TYPE;
             break;
         }
         if (error) {
-- 
1.9.1




More information about the dev mailing list