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

Simon Horman horms at verge.net.au
Fri Oct 26 02:35:51 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>

---

v3
* As suggested by Ben Pfaff
  - Skip unknown hello elements
  - Use bitmap directly in the form of a uint32_t
    instead of creating a struct ofp_hello

v2
* Initial post
---
 include/openflow/openflow-common.h |   11 ++++
 lib/ofp-print.c                    |   27 ++++++++-
 lib/ofp-util.c                     |  117 ++++++++++++++++++++++++++++++++++++
 lib/ofp-util.h                     |    4 ++
 lib/vconn.c                        |   47 +++++++++------
 tests/ofp-print.at                 |   23 +++++++
 6 files changed, 209 insertions(+), 20 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..b4f0028 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -901,6 +901,29 @@ ofp_print_error(struct ds *string, enum ofperr error)
 }
 
 static void
+ofp_print_hello(struct ds *string, const struct ofp_header *oh)
+{
+    enum ofperr error;
+    struct ofpbuf b;
+    uint32_t allowed_versions;
+
+    ofpbuf_use_const(&b, oh, ntohs(oh->length));
+    error = ofputil_decode_hello(&allowed_versions, &b);
+    if (error) {
+        ofp_print_error(string, error);
+        return;
+    }
+
+    ds_put_cstr(string, "\n version bitmap: ");
+    ofputil_format_version_bitmap(string, allowed_versions);
+
+    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 +1749,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 cc70017..b415128 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1135,6 +1135,123 @@ ofputil_format_version_bitmap_names(struct ds *msg, uint32_t bitmap)
     ofputil_format_version_bitmap__(msg, bitmap, ofputil_format_version_name);
 }
 
+static enum ofperr
+ofputil_decode_hello_bitmap(uint32_t *allowed_versions, struct ofpbuf *msg)
+{
+    const struct ofp_hello_elem_header *oheh = msg->data;
+    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) {
+        /* N.B: Only use the first 32bit element of the bitmap
+        * as that is all the current implementation supports.
+        * Subsequent elements are ignored which should have
+        * no effect on session negotiation until Open vSwtich
+        * supports wire-protocol versions greater than 31.
+        */
+        *allowed_versions = ntohl(bitmap[0]);
+    }
+
+    return 0;
+}
+
+enum ofperr
+ofputil_decode_hello(uint32_t *allowed_versions, struct ofpbuf *msg)
+{
+    const struct ofp_header *oh = msg->data;
+    enum ofp_version ofp_version = oh->version;
+    bool have_version_bitmap = false;
+
+    ofpraw_pull_assert(msg);
+    *allowed_versions = 0;
+
+    while (1) {
+        const struct ofp_hello_elem_header *oheh = msg->data;
+
+        if (msg->size < sizeof *oheh) {
+            break;
+        }
+
+        if (oheh->type == htons(OFPHET_VERSIONBITMAP)) {
+            enum ofperr err;
+
+            have_version_bitmap = true;
+            err = ofputil_decode_hello_bitmap(allowed_versions, msg);
+            if (err) {
+                return err;
+            }
+        } else {
+            size_t padded_elem_len = ROUND_UP(ntohs(oheh->length), 8);
+
+            if (!padded_elem_len || msg->size < padded_elem_len) {
+                break;
+            }
+
+            /* Skip unknown element */
+            ofpbuf_pull(msg, padded_elem_len);
+        }
+    }
+
+    if (!have_version_bitmap) {
+        /* Construct bitmap based on version in OpenFlow header */
+        *allowed_versions = ofputil_version_bitmap_set_range1(OFP10_VERSION,
+                                                              ofp_version);
+    }
+
+    return 0;
+}
+
+static inline bool
+should_send_version_bitmap(uint32_t allowed_versions,
+                           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(allowed_versions, i)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+/* Create an OFPT_HELLO message according to
+ * 'ofp_version' and returns the message. */
+struct ofpbuf *
+ofputil_encode_hello(uint32_t allowed_versions)
+{
+    enum ofp_version ofp_version;
+    struct ofpbuf *msg;
+
+    ofp_version = ofputil_version_bitmap_scanr(allowed_versions);
+    msg = ofpraw_alloc(OFPRAW_OFPT_HELLO, ofp_version, 0);
+
+    if (should_send_version_bitmap(allowed_versions, ofp_version)) {
+        struct ofp_hello_elem_header *oheh;
+        uint16_t map_len;
+
+        map_len = VERSION_BITMAP_W / 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);
+        *(ovs_be32 *)(oheh + 1) = htonl(allowed_versions);
+    }
+
+    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/ofp-util.h b/lib/ofp-util.h
index 4838e20..f6a69af 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -150,6 +150,10 @@ enum ofputil_protocol ofputil_protocols_from_string(const char *s);
 const char *ofputil_version_to_string(enum ofp_version ofp_version);
 uint32_t ofputil_versions_from_string(const char *s);
 
+enum ofperr ofputil_decode_hello(uint32_t *allowed_versions,
+                                 struct ofpbuf *msg);
+struct ofpbuf *ofputil_encode_hello(uint32_t version_bitmap);
+
 struct ofpbuf *ofputil_encode_set_protocol(enum ofputil_protocol current,
                                            enum ofputil_protocol want,
                                            enum ofputil_protocol *next);
diff --git a/lib/vconn.c b/lib/vconn.c
index db293c5..e548c20 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -395,12 +395,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;
@@ -439,39 +439,52 @@ 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) {
+            uint32_t peer_allowed_versions;
+            uint32_t allowed_versions;
+
+            error = ofputil_decode_hello(&peer_allowed_versions, 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);
+            allowed_versions = peer_allowed_versions & vconn->allowed_versions;
+            vconn->version = ofputil_version_bitmap_scanr(allowed_versions);
+
             if (vconn->version == VERSION_BITMAP_W) {
                 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, peer_allowed_versions);
+                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, peer_allowed_versions);
+                ds_put_cstr(&msg, ".)");
+                VLOG_DBG("%s", ds_cstr(&msg));
                 ds_destroy(&msg);
                 vconn->state = VCS_CONNECTED;
             }
@@ -537,7 +550,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 6133fff..ee26e75 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -45,6 +45,7 @@ AT_SETUP([OFPT_HELLO - ordinary])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print 0100000800000000], [0], [dnl
 OFPT_HELLO (xid=0x0):
+ version bitmap: 0x01
 ])
 AT_CLEANUP
 
@@ -53,10 +54,32 @@ AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print 0100001300000000657874726120646174610a], [0],
 [dnl
 OFPT_HELLO (xid=0x0):
+ version bitmap: 0x01
 00000000  65 78 74 72 61 20 64 61-74 61 0a                |extra data.     |
 ])
 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 1b 00 00 00 00 ff ff 00 06 01 02 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