From: Sage Weil Date: Sun, 9 Feb 2020 00:44:51 +0000 (-0600) Subject: msg/Policy: limit unregistered anon connections to mon X-Git-Tag: v15.1.1~403^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=507d213cc453ed86ab38619590f710f33245c652;p=ceph.git 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 --- diff --git a/src/ceph_mon.cc b/src/ceph_mon.cc index f46607ed0b25..117a94dc55cc 100644 --- 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 5d13ffb868aa..9f5ea32a0bd9 100644 --- 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 4e890a195c31..c0620da1d452 100644 --- 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 fe2158289a50..921c8d516623 100644 --- 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