You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kg...@apache.org on 2020/05/07 14:45:04 UTC

[qpid-dispatch] branch master updated: DISPATCH-1636: extract the peer router's version from the Open frame

This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git


The following commit(s) were added to refs/heads/master by this push:
     new f3dda13  DISPATCH-1636: extract the peer router's version from the Open frame
f3dda13 is described below

commit f3dda1304d77f6f715f1aeb1d432cb8cccb743f9
Author: Kenneth Giusti <kg...@apache.org>
AuthorDate: Mon Apr 27 09:26:11 2020 -0400

    DISPATCH-1636: extract the peer router's version from the Open frame
    
    This closes #730
---
 include/qpid/dispatch/router_core.h   |  28 ++-
 src/router_core/connections.c         |   7 +-
 src/router_core/router_core_private.h |   1 +
 src/router_node.c                     | 380 ++++++++++++++++++----------------
 tests/CMakeLists.txt                  |   1 +
 tests/run_unit_tests.c                |   2 +
 tests/system_tests_edge_router.py     |  46 ++++
 tests/version_test.c                  | 146 +++++++++++++
 8 files changed, 434 insertions(+), 177 deletions(-)

diff --git a/include/qpid/dispatch/router_core.h b/include/qpid/dispatch/router_core.h
index 7339714..ce45950 100644
--- a/include/qpid/dispatch/router_core.h
+++ b/include/qpid/dispatch/router_core.h
@@ -851,6 +851,30 @@ void qdr_query_free(qdr_query_t *query);
 typedef void (*qdr_manage_response_t) (void *context, const qd_amqp_error_t *status, bool more);
 void qdr_manage_handler(qdr_core_t *core, qdr_manage_response_t response_handler);
 
+typedef struct {
+    uint16_t major;
+    uint16_t minor;
+    uint16_t patch;
+    uint16_t flags;
+#define QDR_ROUTER_VERSION_SNAPSHOT 0x0100
+#define QDR_ROUTER_VERSION_RC       0x0200  // lower byte == RC #
+#define QDR_ROUTER_VERSION_RC_MASK  0x00FF
+} qdr_router_version_t;
+
+// version >= (Major, Minor, Patch)
+#define QDR_ROUTER_VERSION_AT_LEAST(V, MAJOR, MINOR, PATCH)                       \
+    ((V).major > (MAJOR) || ((V).major == (MAJOR)                                 \
+                             && ((V).minor > (MINOR) || ((V).minor == (MINOR)     \
+                                                         && (V).patch >= (PATCH)) \
+                             )                                                    \
+                        )                                                         \
+    )
+
+// version < (Major, Minor, Patch)
+#define QDR_ROUTER_VERSION_LESS_THAN(V, MAJOR, MINOR, PATCH)    \
+    (!QDR_ROUTER_VERSION_AT_LEAST(V, MAJOR, MINOR, PATCH))
+
+
 qdr_connection_info_t *qdr_connection_info(bool             is_encrypted,
                                            bool             is_authenticated,
                                            bool             opened,
@@ -863,7 +887,9 @@ qdr_connection_info_t *qdr_connection_info(bool             is_encrypted,
                                            const char      *container,
                                            pn_data_t       *connection_properties,
                                            int              ssl_ssf,
-                                           bool             ssl);
+                                           bool             ssl,
+                                           // set if remote is a qdrouter
+                                           const qdr_router_version_t *version);
 
 
 typedef struct {
diff --git a/src/router_core/connections.c b/src/router_core/connections.c
index e2b0eb1..4c0e73b 100644
--- a/src/router_core/connections.c
+++ b/src/router_core/connections.c
@@ -161,7 +161,8 @@ qdr_connection_info_t *qdr_connection_info(bool             is_encrypted,
                                            const char      *container,
                                            pn_data_t       *connection_properties,
                                            int              ssl_ssf,
-                                           bool             ssl)
+                                           bool             ssl,
+                                           const qdr_router_version_t *version)
 {
     qdr_connection_info_t *connection_info = new_qdr_connection_info_t();
     ZERO(connection_info);
@@ -190,6 +191,10 @@ qdr_connection_info_t *qdr_connection_info(bool             is_encrypted,
     connection_info->ssl_ssf = ssl_ssf;
     connection_info->ssl     = ssl;
 
+    if (version) {  // only if peer is a router
+        connection_info->version = *version;
+    }
+
     return connection_info;
 }
 
diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h
index 5f358d0..e3757c4 100644
--- a/src/router_core/router_core_private.h
+++ b/src/router_core/router_core_private.h
@@ -636,6 +636,7 @@ struct qdr_connection_info_t {
     pn_data_t                  *connection_properties;
     bool                        ssl;
     int                         ssl_ssf; //ssl strength factor
+    qdr_router_version_t        version; // if role is router or edge
 };
 
 ALLOC_DECLARE(qdr_connection_info_t);
diff --git a/src/router_node.c b/src/router_node.c
index 664b5b1..b2dd1d5 100644
--- a/src/router_node.c
+++ b/src/router_node.c
@@ -44,6 +44,7 @@ static char *direct_prefix;
 static char *node_id;
 
 static void deferred_AMQP_rx_handler(void *context, bool discard);
+static bool parse_failover_property_list(qd_router_t *router, qd_connection_t *conn, pn_data_t *props);
 
 //==============================================================================
 // Functions to handle the linkage between proton deliveries and qdr deliveries
@@ -997,11 +998,12 @@ static void AMQP_opened_handler(qd_router_t *router, qd_connection_t *conn, bool
 {
     qdr_connection_role_t  role = 0;
     int                    cost = 1;
-    int                    remote_cost = 1;
     int                    link_capacity = 1;
     const char            *name = 0;
     bool                   multi_tenant = false;
     const char            *vhost = 0;
+    qdr_router_version_t   rversion = {0};
+    bool                   rversion_found = false;
     uint64_t               connection_id = qd_connection_connection_id(conn);
     pn_connection_t       *pn_conn = qd_connection_pn(conn);
     pn_transport_t *tport = 0;
@@ -1042,192 +1044,73 @@ static void AMQP_opened_handler(qd_router_t *router, qd_connection_t *conn, bool
     qd_router_connection_get_config(conn, &role, &cost, &name, &multi_tenant,
                                     &conn->strip_annotations_in, &conn->strip_annotations_out, &link_capacity);
 
+    // if connection properties are present parse out any important data
+    //
     pn_data_t *props = pn_conn ? pn_connection_remote_properties(pn_conn) : 0;
-
-    if (role == QDR_ROLE_INTER_ROUTER || role == QDR_ROLE_EDGE_CONNECTION) {
-        //
-        // Check the remote properties for an inter-router cost value.
-        //
-        if (props) {
-            pn_data_rewind(props);
-            pn_data_next(props);
-            if (props && pn_data_type(props) == PN_MAP) {
-                pn_data_enter(props);
-                while (pn_data_next(props)) {
-                    if (pn_data_type(props) == PN_SYMBOL) {
-                        pn_bytes_t sym = pn_data_get_symbol(props);
-                        if (sym.size == strlen(QD_CONNECTION_PROPERTY_COST_KEY) &&
-                            strcmp(sym.start, QD_CONNECTION_PROPERTY_COST_KEY) == 0) {
-                            pn_data_next(props);
-                            if (pn_data_type(props) == PN_INT)
-                                remote_cost = pn_data_get_int(props);
-                            break;
-                        }
-                    }
-                }
-            }
-        }
-
-        //
-        // Use the larger of the local and remote costs for this connection
-        //
-        if (remote_cost > cost)
-            cost = remote_cost;
-    }
-
-    bool found_failover = false;
-
     if (props) {
+        const bool is_router = (role == QDR_ROLE_INTER_ROUTER || role == QDR_ROLE_EDGE_CONNECTION);
         pn_data_rewind(props);
-        pn_data_next(props);
-        if (props && pn_data_type(props) == PN_MAP) {
+        if (pn_data_next(props) && pn_data_type(props) == PN_MAP) {
+            const size_t num_items = pn_data_get_map(props);
+            int props_found = 0;  // once all props found exit loop
             pn_data_enter(props);
-
-            //
-            // We are attempting to find a connection property called failover-server-list which is a list of failover host names and ports..
-            // failover-server-list looks something like this
-            //      :"failover-server-list"=[{:"network-host"="some-host", :port="35000"}, {:"network-host"="localhost", :port="25000"}]
-            // There are three cases here -
-            // 1. The failover-server-list is present but the content of the list is empty in which case we scrub the failover list except we keep the original connector information and current connection information.
-            // 2. If the failover list contains one or more maps that contain failover connection information, that information will be appended to the list which already contains the original connection information
-            //    and the current connection information. Any other failover information left over from the previous connection is deleted
-            // 3. If the failover-server-list is not present at all in the connection properties, the failover list we maintain in untoched.
-            //
-            while (pn_data_next(props)) {
-                if (pn_data_type(props) == PN_SYMBOL) {
-                    pn_bytes_t sym = pn_data_get_symbol(props);
-                    if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_LIST_KEY) &&
-                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_LIST_KEY) == 0) {
-                        found_failover = true;
+            for (int i = 0; i < num_items / 2 && props_found < 3; ++i) {
+                if (!pn_data_next(props)) break;
+                if (pn_data_type(props) != PN_SYMBOL) break;  // invalid properties map
+                pn_bytes_t key = pn_data_get_symbol(props);
+
+                if (key.size == strlen(QD_CONNECTION_PROPERTY_COST_KEY) &&
+                    strncmp(key.start, QD_CONNECTION_PROPERTY_COST_KEY, key.size) == 0) {
+                    props_found += 1;
+                    if (!pn_data_next(props)) break;
+                    if (is_router) {
+                        if (pn_data_type(props) == PN_INT) {
+                            const int remote_cost = (int) pn_data_get_int(props);
+                            if (remote_cost > cost)
+                                cost = remote_cost;
+                        }
                     }
-                }
-                else if (pn_data_type(props) == PN_LIST && found_failover) {
-                    size_t list_num_items = pn_data_get_list(props);
-
-                    if (list_num_items > 0) {
-
-                        save_original_and_current_conn_info(conn);
-
-                        pn_data_enter(props); // enter list
-
-                        for (int i=0; i < list_num_items; i++) {
-                            pn_data_next(props);// this is the first element of the list, a map.
-                            if (props && pn_data_type(props) == PN_MAP) {
-
-                                size_t map_num_items = pn_data_get_map(props);
-                                pn_data_enter(props);
-
-                                qd_failover_item_t *item = NEW(qd_failover_item_t);
-                                ZERO(item);
-
-                                // We have found a map with the connection information. Step thru the map contents and create qd_failover_item_t
-
-                                for (int j=0; j < map_num_items/2; j++) {
-
-                                    pn_data_next(props);
-                                    if (pn_data_type(props) == PN_SYMBOL) {
-                                        pn_bytes_t sym = pn_data_get_symbol(props);
-                                        if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_NETHOST_KEY) &&
-                                                                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_NETHOST_KEY) == 0) {
-                                            pn_data_next(props);
-                                            if (pn_data_type(props) == PN_STRING) {
-                                                item->host = strdup(pn_data_get_string(props).start);
-                                            }
-                                        }
-                                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_PORT_KEY) &&
-                                                                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_PORT_KEY) == 0) {
-                                            pn_data_next(props);
-                                            item->port = qdpn_data_as_string(props);
-
-                                        }
-                                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_SCHEME_KEY) &&
-                                                                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_SCHEME_KEY) == 0) {
-                                            pn_data_next(props);
-                                            if (pn_data_type(props) == PN_STRING) {
-                                                item->scheme = strdup(pn_data_get_string(props).start);
-                                            }
-
-                                        }
-                                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_HOSTNAME_KEY) &&
-                                                                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_HOSTNAME_KEY) == 0) {
-                                            pn_data_next(props);
-                                            if (pn_data_type(props) == PN_STRING) {
-                                                item->hostname = strdup(pn_data_get_string(props).start);
-                                            }
-                                        }
-                                    }
-                                }
-
-                                int host_length = strlen(item->host);
-                                //
-                                // We will not even bother inserting the item if there is no host available.
-                                //
-                                if (host_length != 0) {
-                                    if (item->scheme == 0)
-                                        item->scheme = strdup("amqp");
-                                    if (item->port == 0)
-                                        item->port = strdup("5672");
-
-                                    int hplen = strlen(item->host) + strlen(item->port) + 2;
-                                    item->host_port = malloc(hplen);
-                                    snprintf(item->host_port, hplen, "%s:%s", item->host, item->port);
-
-                                    //
-                                    // Iterate through failover list items and sets insert_tail to true
-                                    // when list has just original connector's host and port or when new
-                                    // reported host and port is not yet part of the current list.
-                                    //
-                                    bool insert_tail = false;
-                                    if ( DEQ_SIZE(conn->connector->conn_info_list) == 1 ) {
-                                        insert_tail = true;
-                                    } else {
-                                        qd_failover_item_t *conn_item = DEQ_HEAD(conn->connector->conn_info_list);
-                                        insert_tail = true;
-                                        while ( conn_item ) {
-                                            if ( !strcmp(conn_item->host_port, item->host_port) ) {
-                                                insert_tail = false;
-                                                break;
-                                            }
-                                            conn_item = DEQ_NEXT(conn_item);
-                                        }
-                                    }
-
-                                    // Only inserts if not yet part of failover list
-                                    if ( insert_tail ) {
-                                        DEQ_INSERT_TAIL(conn->connector->conn_info_list, item);
-                                        qd_log(router->log_source, QD_LOG_DEBUG, "Added %s as backup host", item->host_port);
-                                    }
-                                    else {
-                                        free(item->scheme);
-                                        free(item->host);
-                                        free(item->port);
-                                        free(item->hostname);
-                                        free(item->host_port);
-                                        free(item);
-                                    }
-
-                                }
-                                else {
-                                        free(item->scheme);
-                                        free(item->host);
-                                        free(item->port);
-                                        free(item->hostname);
-                                        free(item->host_port);
-                                        free(item);
-                                }
+
+                } else if (key.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_LIST_KEY) &&
+                           strncmp(key.start, QD_CONNECTION_PROPERTY_FAILOVER_LIST_KEY, key.size) == 0) {
+                    props_found += 1;
+                    if (!pn_data_next(props)) break;
+                    parse_failover_property_list(router, conn, props);
+
+                } else if (key.size == strlen(QD_CONNECTION_PROPERTY_VERSION_KEY)
+                           && strncmp(key.start, QD_CONNECTION_PROPERTY_VERSION_KEY, key.size) == 0) {
+                    props_found += 1;
+                    if (!pn_data_next(props)) break;
+                    if (is_router) {
+                        pn_bytes_t vdata = pn_data_get_string(props);
+                        if (vdata.size < 64) {  // > strlen("u16.u16.u16-SNAPSHOT")
+                            char vstr[64];
+                            memcpy(vstr, vdata.start, vdata.size);
+                            vstr[vdata.size] = 0;
+                            int rc = sscanf(vstr, "%"SCNu16".%"SCNu16".%"SCNu16,
+                                            &rversion.major,
+                                            &rversion.minor,
+                                            &rversion.patch);
+                            if (strstr(vstr, "SNAPSHOT")) {
+                                rversion.flags = QDR_ROUTER_VERSION_SNAPSHOT;
+                            }
+                            rversion_found = rc == 3;
+                            if (rversion_found) {
+                                qd_log(router->log_source, QD_LOG_DEBUG, "[C%"PRIu64"] Peer router version: %u.%u.%u%s",
+                                       connection_id, rversion.major, rversion.minor, rversion.patch, (rversion.flags) ? "-SNAPSHOT" : "");
                             }
-                            pn_data_exit(props);
                         }
-                    } // list_num_items > 0
-                    else {
-                        save_original_and_current_conn_info(conn);
-
                     }
+
+                } else {
+                    // skip this key
+                    if (!pn_data_next(props)) break;
                 }
             }
         }
     }
 
+
     if (multi_tenant)
         vhost = pn_connection_remote_hostname(pn_conn);
 
@@ -1262,7 +1145,8 @@ static void AMQP_opened_handler(qd_router_t *router, qd_connection_t *conn, bool
                                                                  container,
                                                                  props,
                                                                  ssl_ssf,
-                                                                 is_ssl);
+                                                                 is_ssl,
+                                                                 (rversion_found) ? &rversion : 0);
 
     qdr_connection_opened(router->router_core, inbound, role, cost, connection_id, name,
                           pn_connection_remote_container(pn_conn),
@@ -1296,6 +1180,152 @@ static void AMQP_opened_handler(qd_router_t *router, qd_connection_t *conn, bool
            authenticated ? mech : "no", (char*) user, container, props_str);
 }
 
+
+// We are attempting to find a connection property called failover-server-list which is a list of failover host names and ports..
+// failover-server-list looks something like this
+//      :"failover-server-list"=[{:"network-host"="some-host", :port="35000"}, {:"network-host"="localhost", :port="25000"}]
+// There are three cases here -
+// 1. The failover-server-list is present but the content of the list is empty in which case we scrub the failover list except we keep the original connector information and current connection information.
+// 2. If the failover list contains one or more maps that contain failover connection information, that information will be appended to the list which already contains the original connection information
+//    and the current connection information. Any other failover information left over from the previous connection is deleted
+// 3. If the failover-server-list is not present at all in the connection properties, the failover list we maintain in untoched.
+//
+// props should be pointing at the value that corresponds to the QD_CONNECTION_PROPERTY_FAILOVER_LIST_KEY
+// returns true if failover list properly parsed.
+//
+static bool parse_failover_property_list(qd_router_t *router, qd_connection_t *conn, pn_data_t *props)
+{
+    bool found_failover = false;
+
+    if (pn_data_type(props) != PN_LIST)
+        return false;
+
+    size_t list_num_items = pn_data_get_list(props);
+
+    if (list_num_items > 0) {
+
+        save_original_and_current_conn_info(conn);
+
+        pn_data_enter(props); // enter list
+
+        for (int i=0; i < list_num_items; i++) {
+            pn_data_next(props);// this is the first element of the list, a map.
+            if (props && pn_data_type(props) == PN_MAP) {
+
+                size_t map_num_items = pn_data_get_map(props);
+                pn_data_enter(props);
+
+                qd_failover_item_t *item = NEW(qd_failover_item_t);
+                ZERO(item);
+
+                // We have found a map with the connection information. Step thru the map contents and create qd_failover_item_t
+
+                for (int j=0; j < map_num_items/2; j++) {
+
+                    pn_data_next(props);
+                    if (pn_data_type(props) == PN_SYMBOL) {
+                        pn_bytes_t sym = pn_data_get_symbol(props);
+                        if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_NETHOST_KEY) &&
+                            strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_NETHOST_KEY) == 0) {
+                            pn_data_next(props);
+                            if (pn_data_type(props) == PN_STRING) {
+                                item->host = strdup(pn_data_get_string(props).start);
+                            }
+                        }
+                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_PORT_KEY) &&
+                                 strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_PORT_KEY) == 0) {
+                            pn_data_next(props);
+                            item->port = qdpn_data_as_string(props);
+
+                        }
+                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_SCHEME_KEY) &&
+                                 strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_SCHEME_KEY) == 0) {
+                            pn_data_next(props);
+                            if (pn_data_type(props) == PN_STRING) {
+                                item->scheme = strdup(pn_data_get_string(props).start);
+                            }
+
+                        }
+                        else if (sym.size == strlen(QD_CONNECTION_PROPERTY_FAILOVER_HOSTNAME_KEY) &&
+                                 strcmp(sym.start, QD_CONNECTION_PROPERTY_FAILOVER_HOSTNAME_KEY) == 0) {
+                            pn_data_next(props);
+                            if (pn_data_type(props) == PN_STRING) {
+                                item->hostname = strdup(pn_data_get_string(props).start);
+                            }
+                        }
+                    }
+                }
+
+                int host_length = strlen(item->host);
+                //
+                // We will not even bother inserting the item if there is no host available.
+                //
+                if (host_length != 0) {
+                    if (item->scheme == 0)
+                        item->scheme = strdup("amqp");
+                    if (item->port == 0)
+                        item->port = strdup("5672");
+
+                    int hplen = strlen(item->host) + strlen(item->port) + 2;
+                    item->host_port = malloc(hplen);
+                    snprintf(item->host_port, hplen, "%s:%s", item->host, item->port);
+
+                    //
+                    // Iterate through failover list items and sets insert_tail to true
+                    // when list has just original connector's host and port or when new
+                    // reported host and port is not yet part of the current list.
+                    //
+                    bool insert_tail = false;
+                    if ( DEQ_SIZE(conn->connector->conn_info_list) == 1 ) {
+                        insert_tail = true;
+                    } else {
+                        qd_failover_item_t *conn_item = DEQ_HEAD(conn->connector->conn_info_list);
+                        insert_tail = true;
+                        while ( conn_item ) {
+                            if ( !strcmp(conn_item->host_port, item->host_port) ) {
+                                insert_tail = false;
+                                break;
+                            }
+                            conn_item = DEQ_NEXT(conn_item);
+                        }
+                    }
+
+                    // Only inserts if not yet part of failover list
+                    if ( insert_tail ) {
+                        DEQ_INSERT_TAIL(conn->connector->conn_info_list, item);
+                        qd_log(router->log_source, QD_LOG_DEBUG, "Added %s as backup host", item->host_port);
+                        found_failover = true;
+                    }
+                    else {
+                        free(item->scheme);
+                        free(item->host);
+                        free(item->port);
+                        free(item->hostname);
+                        free(item->host_port);
+                        free(item);
+                    }
+
+                }
+                else {
+                    free(item->scheme);
+                    free(item->host);
+                    free(item->port);
+                    free(item->hostname);
+                    free(item->host_port);
+                    free(item);
+                }
+            }
+            pn_data_exit(props);
+        }
+    } // list_num_items > 0
+    else {
+        save_original_and_current_conn_info(conn);
+    }
+
+    return found_failover;
+}
+
+
 static int AMQP_inbound_opened_handler(void *type_context, qd_connection_t *conn, void *context)
 {
     qd_router_t *router = (qd_router_t*) type_context;
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 041ee26..9fdd1aa 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -36,6 +36,7 @@ set(unit_test_SOURCES
     parse_tree_tests.c
     proton_utils_tests.c
     alloc_test.c
+    version_test.c
     )
 
 add_executable(unit_tests ${unit_test_SOURCES})
diff --git a/tests/run_unit_tests.c b/tests/run_unit_tests.c
index a944b43..08c75e3 100644
--- a/tests/run_unit_tests.c
+++ b/tests/run_unit_tests.c
@@ -31,6 +31,7 @@ int policy_tests(void);
 int failoverlist_tests(void);
 int parse_tree_tests(void);
 int proton_utils_tests(void);
+int version_tests(void);
 
 int main(int argc, char** argv)
 {
@@ -63,6 +64,7 @@ int main(int argc, char** argv)
     result += parse_tree_tests();
     result += proton_utils_tests();
     result += core_timer_tests();
+    result += version_tests();
 
     qd_dispatch_free(qd);       // dispatch_free last.
 
diff --git a/tests/system_tests_edge_router.py b/tests/system_tests_edge_router.py
index dd256a0..d546a2f 100644
--- a/tests/system_tests_edge_router.py
+++ b/tests/system_tests_edge_router.py
@@ -22,6 +22,7 @@ from __future__ import division
 from __future__ import absolute_import
 from __future__ import print_function
 
+from os import path
 from time import sleep
 from threading import Event
 from threading import Timer
@@ -1640,6 +1641,51 @@ class LinkRouteProxyTest(TestCase):
             tr.queue.get(timeout=TIMEOUT)
         tr.stop()
 
+    def test_000_check_peer_version_info(self):
+        """
+        Not a link proxy test - ensure router correctly parses peer version
+        numbers advertised in the incoming @open frame
+        """
+        lines = None
+        with open(path.join(self.INT_A.outdir, "INT.A.log")) as inta_log:
+            lines = [l for l in inta_log.read().split("\n")
+                     if "] Connection Opened: " in l
+                     or "] Peer router version: " in l]
+
+        self.assertTrue(lines is not None)
+
+        for peer in ['INT.B', 'EA1']:
+
+            conn_id = None
+            open_version = None
+            parsed_version = None
+
+            # extract the version string from the incoming @open
+            for l in lines:
+                ls = l.split()
+                if "container_id=%s" % peer in ls:
+                    conn_id = ls[5]
+                    for f in ls:
+                        index = f.find(':version=')
+                        if index >= 0:
+                            open_version = f[index + len(':version='):].strip("\",")
+                            break;
+                    break;
+
+            self.assertTrue(conn_id is not None)
+            self.assertTrue(open_version is not None)
+
+            # find the debug log message where the router logs the parsed peers
+            # version on the given connection
+            for l in lines:
+                ls = l.split()
+                if ls[5] == conn_id and "version:" in ls:
+                    parsed_version = ls[9]
+                    break;
+
+            self.assertTrue(parsed_version is not None)
+            self.assertEqual(open_version, parsed_version)
+
     def test_01_immedate_detach_reattach(self):
         if self.skip [ 'test_01' ] :
             self.skipTest ( "Test skipped during development." )
diff --git a/tests/version_test.c b/tests/version_test.c
new file mode 100644
index 0000000..494c411
--- /dev/null
+++ b/tests/version_test.c
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include "test_case.h"
+#include <qpid/dispatch/router_core.h>
+
+typedef struct {
+    qdr_router_version_t version;
+    uint16_t major;
+    uint16_t minor;
+    uint16_t patch;
+    bool   expected;
+} test_data_t;
+
+static const test_data_t data[] = {
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 10,
+        .minor = 11,
+        .patch = 12,
+        .expected = true,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 10,
+        .minor = 11,
+        .patch = 13,
+        .expected = false,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 10,
+        .minor = 12,
+        .patch = 0,
+        .expected = false,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 11,
+        .minor = 0,
+        .patch = 0,
+        .expected = false,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 10,
+        .minor = 11,
+        .patch = 11,
+        .expected = true,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 10,
+        .minor = 10,
+        .patch = 13,
+        .expected = true,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 9,
+        .minor = 12,
+        .patch = 13,
+        .expected = true,
+    },
+    {
+        .version = {.major = 10,
+                    .minor = 11,
+                    .patch = 12},
+        .major = 0,
+        .minor = 0,
+        .patch = 0,
+        .expected = true,
+    },
+};
+const int data_count = (sizeof(data)/sizeof(data[0]));
+
+static char buffer[100];
+
+static char *test_version_compare(void *context)
+{
+
+    const test_data_t *p = data;
+    for (int i = 0; i < data_count; ++i) {
+
+        if (QDR_ROUTER_VERSION_AT_LEAST(p->version, p->major, p->minor, p->patch) != p->expected) {
+            snprintf(buffer, sizeof(buffer), "At least failed: %u.%u.%u / %u.%u.%u e=%s\n",
+                     p->version.major, p->version.minor, p->version.patch,
+                     p->major, p->minor, p->patch, p->expected ? "true" : "false");
+            return buffer;
+        }
+        if (QDR_ROUTER_VERSION_LESS_THAN(p->version, p->major, p->minor, p->patch) == p->expected) {
+            snprintf(buffer, sizeof(buffer), "Less than failed: %u.%u.%u / %u.%u.%u e=%s\n",
+                     p->version.major, p->version.minor, p->version.patch,
+                     p->major, p->minor, p->patch, p->expected ? "true" : "false");
+            return buffer;
+        }
+        ++p;
+    }
+
+    return 0;
+}
+
+
+int version_tests()
+{
+    int result = 0;
+    char *test_group = "version_tests";
+
+    TEST_CASE(test_version_compare, 0);
+
+    return result;
+}
+


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org