[ovs-dev] [PATCH 04/18] ofp-util: Add version bitmap to hello messages

Simon Horman horms at verge.net.au
Thu Oct 18 05:58:04 UTC 2012


Allow encoding and decoding of version bitmap in hello messages
as specified in Open Flow 1.3.1.

Signed-off-by: Simon Horman <horms at verge.net.au>
---
 include/openflow/openflow-common.h |   11 ++++
 lib/ofp-print.c                    |   30 ++++++++-
 lib/ofp-util.c                     |  119 ++++++++++++++++++++++++++++++++++++
 lib/vconn.c                        |   59 ++++++++++++------
 tests/ofp-print.at                 |   19 ++++++
 5 files changed, 217 insertions(+), 21 deletions(-)

diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 0bca0d2..462b2fc 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -334,4 +334,15 @@ enum ofp_group {
     OFPG_ANY        = 0xffffffff   /* Wildcard, for flow stats requests. */
 };
 
+enum ofp_hello_elem_type {
+    OFPHET_VERSIONBITMAP          = 1, /* Bitmap of version supported. */
+};
+
+/* Common header for all Hello Elements */
+struct ofp_hello_elem_header {
+    ovs_be16    type;        /* One of OFPHET_*. */
+    ovs_be16    length;      /* Length in bytes of this element. */
+};
+OFP_ASSERT(sizeof(struct ofp_hello_elem_header) == 4);
+
 #endif /* openflow/openflow-common.h */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 8654783..68b1202 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -901,6 +901,32 @@ ofp_print_error(struct ds *string, enum ofperr error)
 }
 
 static void
+ofp_print_hello(struct ds *string, const struct ofp_header *oh)
+{
+    struct ofputil_hello hello;
+    enum ofperr error;
+    struct ofpbuf b;
+
+    ofpbuf_use_const(&b, oh, ntohs(oh->length));
+    error = ofputil_decode_hello(&hello, &b);
+    if (error) {
+        ofp_print_error(string, error);
+        return;
+    }
+
+    if (hello.version_bitmap.n_bits) {
+        ds_put_cstr(string, "\n version bitmap: ");
+        ofputil_format_version_bitmap(string, &hello.version_bitmap);
+        ofputil_version_bitmap_free_data(&hello.version_bitmap);
+    }
+
+    if (b.size) {
+        ds_put_char(string, '\n');
+        ds_put_hex_dump(string, b.data, b.size, 0, true);
+    }
+}
+
+static void
 ofp_print_error_msg(struct ds *string, const struct ofp_header *oh)
 {
     size_t len = ntohs(oh->length);
@@ -1726,9 +1752,7 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
     ofp_header_to_string__(oh, raw, string);
     switch (ofptype_from_ofpraw(raw)) {
     case OFPTYPE_HELLO:
-        ds_put_char(string, '\n');
-        ds_put_hex_dump(string, oh + 1, ntohs(oh->length) - sizeof *oh,
-                        0, true);
+        ofp_print_hello(string, oh);
         break;
 
     case OFPTYPE_ERROR:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 94354fe..f7111ce 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1093,6 +1093,45 @@ ofputil_version_bitmap_set_range1(struct ofputil_version_bitmap *ovb,
     } while (i-- > start);
 }
 
+static void
+ofputil_version_bitmap_import(struct ofputil_version_bitmap *ovb,
+                              const ovs_be32 *bitmap, size_t n_be32)
+{
+    /* XXX: slow */
+    size_t i = n_be32 - 1;
+
+    do {
+        uint32_t map = ntohl(bitmap[i]);
+        size_t j = sizeof map * CHAR_BIT;
+
+        do {
+            if (map & (1u << j)) {
+                ofputil_version_bitmap_set1(ovb, j + i * sizeof map);
+            }
+        } while (j--);
+    } while (i--);
+}
+
+static void
+ofputil_version_bitmap_export(const struct ofputil_version_bitmap *ovb,
+                              ovs_be32 *bitmap, size_t n_be32)
+{
+    /* XXX: slow */
+    size_t i;
+
+    for (i = 0; i < n_be32; i++) {
+        uint32_t map = 0;
+        size_t j = sizeof map * CHAR_BIT;
+
+        for (j = 0; j < sizeof map * CHAR_BIT; j++) {
+            if (ofputil_version_bitmap_is_set(ovb, j + i * sizeof map)) {
+                map |= (1u << j);
+            }
+            bitmap[i] = htonl(map);
+        }
+    }
+}
+
 /* Test if bit offset is set in ovb. */
 bool
 ofputil_version_bitmap_is_set(const struct ofputil_version_bitmap *ovb,
@@ -1216,6 +1255,86 @@ ofputil_get_allowed_versions_default(void)
     return &ovb;
 }
 
+enum ofperr
+ofputil_decode_hello(struct ofputil_hello *hello, struct ofpbuf *msg)
+{
+    const struct ofp_hello_elem_header *oheh;
+    const struct ofp_header *oh = msg->data;
+
+    ofputil_version_bitmap_init(&hello->version_bitmap);
+    ofpraw_pull_assert(msg);
+    hello->version = oh->version;
+
+    if (msg->size < sizeof *oheh) {
+        return 0;
+    }
+
+    oheh = msg->data;
+    if (oheh->type == htons(OFPHET_VERSIONBITMAP)) {
+        uint16_t elem_len = ntohs(oheh->length);
+        size_t padded_elem_len = ROUND_UP(elem_len, 8);
+        size_t version_len;
+        ovs_be32 *bitmap;
+
+        if (msg->size < padded_elem_len || !elem_len ||
+            elem_len % sizeof *bitmap) {
+            return OFPERR_OFPET_HELLO_FAILED;
+        }
+
+        oheh = ofpbuf_pull(msg, sizeof *oheh);
+        bitmap = ofpbuf_pull(msg, padded_elem_len - sizeof *oheh);
+        version_len = elem_len - sizeof *oheh;
+        if (version_len) {
+            ofputil_version_bitmap_import(&hello->version_bitmap, bitmap,
+                                          version_len / sizeof *bitmap);
+        }
+        return 0;
+    }
+
+    return 0;
+}
+
+static inline bool
+should_send_version_bitmap(const struct ofputil_version_bitmap *ovb,
+                           enum ofp_version ofp_version)
+{
+    size_t i;
+
+    /* No need to send version bitmap if all bits up to ofp_version are one */
+    for (i = OFP10_VERSION; i <= ofp_version; i++) {
+        if (!ofputil_version_bitmap_is_set(ovb, i)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+/* Create an OFPT_HELLO message according to
+ * 'ofp_version' and returns the message. */
+struct ofpbuf *
+ofputil_encode_hello(const struct ofputil_version_bitmap *ovb)
+{
+    enum ofp_version ofp_version = ofputil_version_bitmap_scanr(ovb);
+    struct ofpbuf *msg;
+
+    msg = ofpraw_alloc(OFPRAW_OFPT_HELLO, ofp_version, 0);
+
+    if (should_send_version_bitmap(ovb, ofp_version)) {
+        struct ofp_hello_elem_header *oheh;
+        uint16_t map_len;
+
+        map_len = ROUND_UP(ofp_version, CHAR_BIT * sizeof(ovs_be32)) / CHAR_BIT;
+        oheh = ofpbuf_put_zeros(msg, ROUND_UP(map_len + sizeof *oheh, 8));
+        oheh->type = htons(OFPHET_VERSIONBITMAP);
+        oheh->length = htons(map_len + sizeof *oheh);
+        ofputil_version_bitmap_export(ovb, (ovs_be32 *)(oheh + 1),
+                                      map_len / sizeof(ovs_be32));
+    }
+
+    return msg;
+}
+
 /* Returns an OpenFlow message that, sent on an OpenFlow connection whose
  * protocol is 'current', at least partly transitions the protocol to 'want'.
  * Stores in '*next' the protocol that will be in effect on the OpenFlow
diff --git a/lib/vconn.c b/lib/vconn.c
index 8603ad1..40782bf 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -397,12 +397,12 @@ vcs_connecting(struct vconn *vconn)
 }
 
 static void
-vcs_send_hello(struct vconn *vconn, enum ofp_version max_version)
+vcs_send_hello(struct vconn *vconn)
 {
     struct ofpbuf *b;
     int retval;
 
-    b = ofpraw_alloc(OFPRAW_OFPT_HELLO, max_version, 0);
+    b = ofputil_encode_hello(&vconn->allowed_versions);
     retval = do_send(vconn, b);
     if (!retval) {
         vconn->state = VCS_RECV_HELLO;
@@ -441,42 +441,65 @@ vcs_recv_hello(struct vconn *vconn)
 
     retval = do_recv(vconn, &b);
     if (!retval) {
-        const struct ofp_header *oh = b->data;
         enum ofptype type;
         enum ofperr error;
 
         error = ofptype_decode(&type, b->data);
         if (!error && type == OFPTYPE_HELLO) {
-            if (b->size > sizeof *oh) {
+            struct ofputil_hello hello;
+            struct ofputil_version_bitmap ovb =
+                OFPUTIL_VERSION_BITMAP_INITIALIZER;
+
+            error = ofputil_decode_hello(&hello, b);
+            if (error) {
+                VLOG_WARN_RL(&bad_ofmsg_rl, "%s: ***decode error: %s***\n",
+                             vconn->name, ofperr_get_name(error));
+                return;
+            }
+
+            if (b->size) {
                 struct ds msg = DS_EMPTY_INITIALIZER;
-                ds_put_format(&msg, "%s: extra-long hello:\n", vconn->name);
+                ds_put_format(&msg, "%s: trailing data in hello:\n",
+                              vconn->name);
                 ds_put_hex_dump(&msg, b->data, b->size, 0, true);
                 VLOG_WARN_RL(&bad_ofmsg_rl, "%s", ds_cstr(&msg));
                 ds_destroy(&msg);
             }
 
-            vconn->version =
-                ofputil_version_bitmap_scanr(&vconn->allowed_versions);
-            if (vconn->version == oh->version + 1) {
+            if (!hello.version_bitmap.bitmap) {
+                ofputil_version_bitmap_set_range1(&hello.version_bitmap,
+                                                  OFP10_VERSION,
+                                                  hello.version + 1);
+            }
+
+            ofputil_version_bitmap_clone_data(&hello.version_bitmap, &ovb);
+            ofputil_version_bitmap_and(&vconn->allowed_versions, &ovb);
+            vconn->version = ofputil_version_bitmap_scanr(&ovb);
+
+            if (vconn->version == ovb.n_bits) {
                 struct ds msg = DS_EMPTY_INITIALIZER;
+                ds_put_format(&msg, "%s: we support ", vconn->name);
                 format_versions(&msg, &vconn->allowed_versions);
-                VLOG_WARN_RL(&bad_ofmsg_rl,
-                             "%s: version negotiation failed: we support "
-                             "%s but peer " "supports no later than "
-                             "version 0x%02"PRIx8, vconn->name,
-                             ds_cstr(&msg), oh->version);
+                ds_put_cstr(&msg, ". but peer supports ");
+                format_versions(&msg, &hello.version_bitmap);
+                ds_put_char(&msg, '.');
+                VLOG_WARN_RL(&bad_ofmsg_rl, "%s", ds_cstr(&msg));
                 ds_destroy(&msg);
                 vconn->state = VCS_SEND_ERROR;
             } else {
                 struct ds msg = DS_EMPTY_INITIALIZER;
+                ds_put_format(&msg, "%s: negotiated OpenFlow version 0x%02x "
+                              "(we support ", vconn->name, vconn->version);
                 format_versions(&msg, &vconn->allowed_versions);
-                VLOG_DBG("%s: negotiated OpenFlow version 0x%02x "
-                         "(we support %s, peer no later than "
-                         "version 0x%02"PRIx8")", vconn->name,
-                         vconn->version, ds_cstr(&msg), oh->version);
+                ds_put_cstr(&msg, ". peer supports ");
+                format_versions(&msg, &hello.version_bitmap);
+                ds_put_cstr(&msg, ".)");
+                VLOG_DBG("%s", ds_cstr(&msg));
                 ds_destroy(&msg);
                 vconn->state = VCS_CONNECTED;
             }
+            ofputil_version_bitmap_free_data(&hello.version_bitmap);
+            ofputil_version_bitmap_free_data(&ovb);
             ofpbuf_delete(b);
             return;
         } else {
@@ -540,7 +563,7 @@ vconn_connect(struct vconn *vconn)
             break;
 
         case VCS_SEND_HELLO:
-            vcs_send_hello(vconn, max_version);
+            vcs_send_hello(vconn);
             break;
 
         case VCS_RECV_HELLO:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index f1fdefb..b44f460 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -57,6 +57,25 @@ OFPT_HELLO (xid=0x0):
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_HELLO with version bitmap])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "01 00 00 10 00 00 00 00 00 01 00 08 00 00 00 f1"], [0],
+[dnl
+OFPT_HELLO (xid=0x0):
+ version bitmap: 0x00, 0x04, 0x05, 0x06, 0x07
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_HELLO with version bitmap and extra data])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "01 00 00 13 00 00 00 00 00 01 00 08 00 00 00 f1 61 62 63"], [0],
+[dnl
+OFPT_HELLO (xid=0x0):
+ version bitmap: 0x00, 0x04, 0x05, 0x06, 0x07
+00000000  61 62 63                                        |abc             |
+])
+AT_CLEANUP
+
 dnl OFPT_ERROR tests are in ofp-errors.at.
 
 AT_SETUP([OFPT_ECHO_REQUEST, empty payload])
-- 
1.7.10.4




More information about the dev mailing list