From: Sage Weil <sage@redhat.com>
Date: Tue, 5 Feb 2019 11:08:00 +0000 (-0600)
Subject: msg/async: very protocol type when looking up existing connections
X-Git-Tag: v14.1.0~184^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cb0e7e0281ad7754669ea17a7c52efcd56393b21;p=ceph.git

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 5a115cb57c56..8eecb7c2e5fc 100644
--- 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 d97099964d8d..fe05dead7b30 100644
--- 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 cb3745d6b46d..8a96a2748699 100644
--- 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();