msg/Policy: limit unregistered anon connections to mon

When we were fixing 'tell mon' we changed the messenger to allow multiple
lossy connections to the same server.  This was needed for the mon, and
assumed to be harmless for everyone else.  However, for the OSD, it can
lead to out-of-order requests, as observed in this bug:
	https://tracker.ceph.com/issues/42328

Fix by reverting this behavior change except for the mon:

- Introduce a stateless_anon_server policy that sets
  register_lossy_clients = false

- Effectively revert the behavior change in c48a29b9edde3c6d3cd34252d202885e2e064fe0
  so that lossy clients *are* registered for stateless_server policy.

- Use the state_anon_server policy for the mon, which was the one place
  where we needed unregistered connections (for 'tell mon.x' to work).

Fixes: https://tracker.ceph.com/issues/42328
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/ceph_mon.cc b/src/ceph_mon.cc
index f46607ed0b25abc4a23e55016a43bf879ca5850c..117a94dc55ccbf90013f21be132f2d3655445454 100644 (file)
--- a/src/ceph_mon.cc
+++ b/src/ceph_mon.cc
@@ -765,17 +765,17 @@ int main(int argc, const char **argv)
   msgr->set_cluster_protocol(CEPH_MON_PROTOCOL);
   msgr->set_default_send_priority(CEPH_MSG_PRIO_HIGH);
 
-  msgr->set_default_policy(Messenger::Policy::stateless_server(0));
+  msgr->set_default_policy(Messenger::Policy::stateless_anon_server(0));
   msgr->set_policy(entity_name_t::TYPE_MON,
                    Messenger::Policy::lossless_peer_reuse(
                     CEPH_FEATURE_SERVER_LUMINOUS));
   msgr->set_policy(entity_name_t::TYPE_OSD,
-                   Messenger::Policy::stateless_server(
+                   Messenger::Policy::stateless_anon_server(
                     CEPH_FEATURE_SERVER_LUMINOUS));
   msgr->set_policy(entity_name_t::TYPE_CLIENT,
-                   Messenger::Policy::stateless_server(0));
+                   Messenger::Policy::stateless_anon_server(0));
   msgr->set_policy(entity_name_t::TYPE_MDS,
-                   Messenger::Policy::stateless_server(0));
+                   Messenger::Policy::stateless_anon_server(0));
 
   // throttle client traffic
   Throttle *client_throttler = new Throttle(g_ceph_context, "mon_client_bytes",

diff --git a/src/msg/Policy.h b/src/msg/Policy.h
index 5d13ffb868aa23d01ce1f8dbe811ef1a006c04fd..9f5ea32a0bd954756495c53615cb7ce9c7cdddf0 100644 (file)
--- a/src/msg/Policy.h
+++ b/src/msg/Policy.h
@@ -25,6 +25,14 @@ struct Policy {
   bool standby;
   /// If true, we will try to detect session resets
   bool resetcheck;
+
+  /// Server: register lossy client connections.
+  bool register_lossy_clients = true;
+  // The net result of this is that a given client can only have one
+  // open connection with the server.  If a new connection is made,
+  // the old (registered) one is closed by the messenger during the accept
+  // process.
+  
   /**
    *  The throttler is used to limit how much data is held by Messages from
    *  the associated Connection(s). When reading in a new Message, the Messenger
@@ -45,8 +53,9 @@ struct Policy {
       features_supported(CEPH_FEATURES_SUPPORTED_DEFAULT),
       features_required(0) {}
 private:
-  Policy(bool l, bool s, bool st, bool r, uint64_t req)
+  Policy(bool l, bool s, bool st, bool r, bool rlc, uint64_t req)
     : lossy(l), server(s), standby(st), resetcheck(r),
+      register_lossy_clients(rlc),
       throttler_bytes(NULL),
       throttler_messages(NULL),
       features_supported(CEPH_FEATURES_SUPPORTED_DEFAULT),
@@ -54,22 +63,25 @@ private:
   
 public:
   static Policy stateful_server(uint64_t req) {
-    return Policy(false, true, true, true, req);
+    return Policy(false, true, true, true, true, req);
   }
   static Policy stateless_server(uint64_t req) {
-    return Policy(true, true, false, false, req);
+    return Policy(true, true, false, false, true, req);
+  }
+  static Policy stateless_anon_server(uint64_t req) {
+    return Policy(true, true, false, false, false, req);
   }
   static Policy lossless_peer(uint64_t req) {
-    return Policy(false, false, true, false, req);
+    return Policy(false, false, true, false, true, req);
   }
   static Policy lossless_peer_reuse(uint64_t req) {
-    return Policy(false, false, true, true, req);
+    return Policy(false, false, true, true, true, req);
   }
   static Policy lossy_client(uint64_t req) {
-    return Policy(true, false, false, false, req);
+    return Policy(true, false, false, false, true, req);
   }
   static Policy lossless_client(uint64_t req) {
-    return Policy(false, false, false, true, req);
+    return Policy(false, false, false, true, true, req);
   }
 };
 

diff --git a/src/msg/async/ProtocolV1.cc b/src/msg/async/ProtocolV1.cc
index 4e890a195c3197502d031bed4d27790f6079ca46..c0620da1d452855d50a28c162dada513f11073bc 100644 (file)
--- a/src/msg/async/ProtocolV1.cc
+++ b/src/msg/async/ProtocolV1.cc
@@ -2062,7 +2062,8 @@ CtPtr ProtocolV1::handle_connect_message_2() {
   ldout(cct, 10) << __func__ << " accept setting up session_security." << dendl;
 
   if (connection->policy.server &&
-      connection->policy.lossy) {
+      connection->policy.lossy &&
+      !connection->policy.register_lossy_clients) {
     // incoming lossy client, no need to register this connection
     // new session
     ldout(cct, 10) << __func__ << " accept new session" << dendl;

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index fe2158289a50b2e623de83f52d6bf58ea8af28c8..921c8d516623efdf10c946699c5c3507d4ac458a 100644 (file)
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -2369,7 +2369,8 @@ CtPtr ProtocolV2::handle_client_ident(ceph::bufferlist &payload)
   peer_global_seq = client_ident.global_seq();
 
   if (connection->policy.server &&
-      connection->policy.lossy) {
+      connection->policy.lossy &&
+      connection->policy.register_lossy_clients) {
     // incoming lossy client, no need to register this connection
   } else {
     // Looks good so far, let's check if there is already an existing connection