msg/async: very protocol type when looking up existing connections

Since we register client connections as any:, we may have either a ProtocolV1 or V2
connection.  This happens when clients have an imprecise mon search list and connect
to the same mon via both v1 and v2, for example when you do something like

 ceph -m 'v2:127.0.0.1:40648/0,v1:127.0.0.1:40649/0' -s

If we do encounter the other protocol type than what we expect, just mark it down and
proceed.  This is only a temporarily case that happens during mon discovery, the client
is always prepared to retry, and it doesn't actually matter which one succeeds since
it will return a monmap and the client will adapt accordingly.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/msg/async/Protocol.h b/src/msg/async/Protocol.h
index 5a115cb57c562b3d43f4fcf99905a757b9efe910..8eecb7c2e5fcaac9f39de5fadf26bda54957a67d 100644 (file)
--- a/src/msg/async/Protocol.h
+++ b/src/msg/async/Protocol.h
@@ -73,8 +73,9 @@ public:
 class AsyncMessenger;
 
 class Protocol {
+public:
+  const int proto_type;
 protected:
-  int proto_type;
   AsyncConnection *connection;
   AsyncMessenger *messenger;
   CephContext *cct;

diff --git a/src/msg/async/ProtocolV1.cc b/src/msg/async/ProtocolV1.cc
index d97099964d8d8a98c4b5f3abb8ab407a244137d4..fe05dead7b304e81a95ab1969cc3061846f4308a 100644 (file)
--- a/src/msg/async/ProtocolV1.cc
+++ b/src/msg/async/ProtocolV1.cc
@@ -1963,6 +1963,13 @@ CtPtr ProtocolV1::handle_connect_message_2() {
   if (existing == connection) {
     existing = nullptr;
   }
+  if (existing && existing->protocol->proto_type != 1) {
+    ldout(cct,1) << __func__ << " existing " << existing << " proto "
+                << existing->protocol.get() << " version is "
+                << existing->protocol->proto_type << ", marking down" << dendl;
+    existing->mark_down();
+    existing = nullptr;
+  }
 
   if (existing) {
     // There is no possible that existing connection will acquire this
@@ -1970,15 +1977,11 @@ CtPtr ProtocolV1::handle_connect_message_2() {
     existing->lock.lock();  // skip lockdep check (we are locking a second
                             // AsyncConnection here)
 
-    ProtocolV1 *exproto = dynamic_cast<ProtocolV1 *>(existing->protocol.get());
     ldout(cct,10) << __func__ << " existing=" << existing << " exproto="
-                 << exproto << dendl;
-    assert(exproto->proto_type == 1);
-
-    if (!exproto) {
-      ldout(cct, 1) << __func__ << " existing=" << existing << dendl;
-      ceph_assert(false);
-    }
+                 << existing->protocol.get() << dendl;
+    ProtocolV1 *exproto = dynamic_cast<ProtocolV1 *>(existing->protocol.get());
+    ceph_assert(exproto);
+    ceph_assert(exproto->proto_type == 1);
 
     if (exproto->state == CLOSED) {
       ldout(cct, 1) << __func__ << " existing " << existing

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index cb3745d6b46d152d8daae9154eea305c0ed09b9b..8a96a27486992a8610b06ca8de2e66ac647fb739 100644 (file)
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -2627,6 +2627,15 @@ CtPtr ProtocolV2::handle_client_ident(char *payload, uint32_t length) {
   connection->lock.unlock();
   AsyncConnectionRef existing = messenger->lookup_conn(*connection->peer_addrs);
 
+  if (existing &&
+      existing->protocol->proto_type != 2) {
+    ldout(cct,1) << __func__ << " existing " << existing << " proto "
+                << existing->protocol.get() << " version is "
+                << existing->protocol->proto_type << ", marking down" << dendl;
+    existing->mark_down();
+    existing = nullptr;
+  }
+
   connection->inject_delay();
 
   connection->lock.lock();
@@ -2665,6 +2674,15 @@ CtPtr ProtocolV2::handle_reconnect(char *payload, uint32_t length) {
   connection->lock.unlock();
   AsyncConnectionRef existing = messenger->lookup_conn(*connection->peer_addrs);
 
+  if (existing &&
+      existing->protocol->proto_type != 2) {
+    ldout(cct,1) << __func__ << " existing " << existing << " proto "
+                << existing->protocol.get() << " version is "
+                << existing->protocol->proto_type << ", marking down" << dendl;
+    existing->mark_down();
+    existing = nullptr;
+  }
+
   connection->inject_delay();
 
   connection->lock.lock();