Allow encoding and decoding of version bitmap in hello messages
as specified in Open Flow 1.3.1.
Signed-off-by: Simon Horman <ho...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
