From f22d433ad600608da3525a4946c7bdef170abd78 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 4 Mar 2019 13:01:50 +0800 Subject: [PATCH] crimson/{net,osd}: make ms_get_authorizer() sync the authorizer manager does not perform (significant) i/o for building an authorizer. see CephXTicketHandler::build_authorizer(). what it does is but read random bytes using getentropy(3) which uses getrandom(2). getrandom(2) could potentially block if the system just boots and does not have enough randomness. but i think it's safe to assume that we have enough entrophy when crimson-osd starts. Signed-off-by: Kefu Chai --- src/crimson/CMakeLists.txt | 1 - src/crimson/net/Dispatcher.cc | 11 -------- src/crimson/net/Dispatcher.h | 5 ++-- src/crimson/net/SocketConnection.cc | 34 +++++++++++------------ src/crimson/net/SocketConnection.h | 2 +- src/crimson/osd/chained_dispatchers.cc | 37 +++++++------------------- src/crimson/osd/chained_dispatchers.h | 3 +-- 7 files changed, 30 insertions(+), 63 deletions(-) delete mode 100644 src/crimson/net/Dispatcher.cc diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt index d7b58521d4b..05d6f148f39 100644 --- a/src/crimson/CMakeLists.txt +++ b/src/crimson/CMakeLists.txt @@ -116,7 +116,6 @@ set(crimson_mon_srcs mon/MonClient.cc ${PROJECT_SOURCE_DIR}/src/mon/MonSub.cc) set(crimson_net_srcs - net/Dispatcher.cc net/Errors.cc net/Messenger.cc net/SocketConnection.cc diff --git a/src/crimson/net/Dispatcher.cc b/src/crimson/net/Dispatcher.cc deleted file mode 100644 index 336ded38f0f..00000000000 --- a/src/crimson/net/Dispatcher.cc +++ /dev/null @@ -1,11 +0,0 @@ -#include "auth/Auth.h" -#include "Dispatcher.h" - -namespace ceph::net -{ -seastar::future> -Dispatcher::ms_get_authorizer(peer_type_t) -{ - return seastar::make_ready_future>(nullptr); -} -} diff --git a/src/crimson/net/Dispatcher.h b/src/crimson/net/Dispatcher.h index cbde1549928..8bcc47c917c 100644 --- a/src/crimson/net/Dispatcher.h +++ b/src/crimson/net/Dispatcher.h @@ -53,8 +53,9 @@ class Dispatcher { bufferlist&) { return seastar::make_ready_future(0, bufferlist{}); } - virtual seastar::future> - ms_get_authorizer(peer_type_t); + virtual AuthAuthorizer* ms_get_authorizer(peer_type_t) const { + return nullptr; + } // get the local dispatcher shard if it is accessed by another core virtual Dispatcher* get_local_shard() { diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc index 2907c486871..7f0f4fc4cb9 100644 --- a/src/crimson/net/SocketConnection.cc +++ b/src/crimson/net/SocketConnection.cc @@ -716,7 +716,7 @@ SocketConnection::handle_connect_reply(msgr_tag_t tag) h.authorizer->session_key, features)); } - h.authorizer.reset(); + h.authorizer = nullptr; return seastar::make_ready_future(stop_t::yes); }); break; @@ -758,23 +758,21 @@ SocketConnection::repeat_connect() // this is fyi, actually, server decides! h.connect.flags = policy.lossy ? CEPH_MSG_CONNECT_LOSSY : 0; - return dispatcher.ms_get_authorizer(peer_type) - .then([this](auto&& auth) { - h.authorizer = std::move(auth); - bufferlist bl; - if (h.authorizer) { - h.connect.authorizer_protocol = h.authorizer->protocol; - h.connect.authorizer_len = h.authorizer->bl.length(); - bl.append(create_static(h.connect)); - bl.append(h.authorizer->bl); - } else { - h.connect.authorizer_protocol = 0; - h.connect.authorizer_len = 0; - bl.append(create_static(h.connect)); - }; - return socket->write_flush(std::move(bl)); - }).then([this] { - // read the reply + h.authorizer = dispatcher.ms_get_authorizer(peer_type); + bufferlist bl; + if (h.authorizer) { + h.connect.authorizer_protocol = h.authorizer->protocol; + h.connect.authorizer_len = h.authorizer->bl.length(); + bl.append(create_static(h.connect)); + bl.append(h.authorizer->bl); + } else { + h.connect.authorizer_protocol = 0; + h.connect.authorizer_len = 0; + bl.append(create_static(h.connect)); + } + return socket->write_flush(std::move(bl)) + .then([this] { + // read the reply return socket->read(sizeof(h.reply)); }).then([this] (bufferlist bl) { auto p = bl.cbegin(); diff --git a/src/crimson/net/SocketConnection.h b/src/crimson/net/SocketConnection.h index 62cc77d5347..9fd3bdf2943 100644 --- a/src/crimson/net/SocketConnection.h +++ b/src/crimson/net/SocketConnection.h @@ -69,7 +69,7 @@ class SocketConnection : public Connection { struct Handshake { ceph_msg_connect connect; ceph_msg_connect_reply reply; - std::unique_ptr authorizer; + AuthAuthorizer* authorizer = nullptr; std::chrono::milliseconds backoff; uint32_t connect_seq = 0; uint32_t peer_global_seq = 0; diff --git a/src/crimson/osd/chained_dispatchers.cc b/src/crimson/osd/chained_dispatchers.cc index da4aa269ee3..dfca51e8cba 100644 --- a/src/crimson/osd/chained_dispatchers.cc +++ b/src/crimson/osd/chained_dispatchers.cc @@ -38,35 +38,16 @@ ChainedDispatchers::ms_handle_remote_reset(ceph::net::ConnectionRef conn) { }); } -seastar::future> -ChainedDispatchers::ms_get_authorizer(peer_type_t peer_type) +AuthAuthorizer* +ChainedDispatchers::ms_get_authorizer(peer_type_t peer_type) const { // since dispatcher returns a nullptr if it does not have the authorizer, // let's use the chain-of-responsibility pattern here. - struct Params { - peer_type_t peer_type; - std::deque::iterator first, last; - } params = {peer_type, dispatchers.begin(), dispatchers.end()}; - return seastar::do_with(Params{params}, [this] (Params& params) { - using result_t = std::unique_ptr; - return seastar::repeat_until_value([&] () { - auto& first = params.first; - if (first == params.last) { - // just give up - return seastar::make_ready_future>(result_t{}); - } else { - return (*first)->ms_get_authorizer(params.peer_type) - .then([&] (auto&& auth)-> std::optional { - if (auth) { - // hooray! - return std::move(auth); - } else { - // try next one - ++first; - return {}; - } - }); - } - }); - }); + for (auto dispatcher : dispatchers) { + if (auto auth = dispatcher->ms_get_authorizer(peer_type); auth) { + return auth; + } + } + // just give up + return nullptr; } diff --git a/src/crimson/osd/chained_dispatchers.h b/src/crimson/osd/chained_dispatchers.h index c30408361ba..8368021c869 100644 --- a/src/crimson/osd/chained_dispatchers.h +++ b/src/crimson/osd/chained_dispatchers.h @@ -27,6 +27,5 @@ public: seastar::future<> ms_handle_connect(ceph::net::ConnectionRef conn) override; seastar::future<> ms_handle_reset(ceph::net::ConnectionRef conn) override; seastar::future<> ms_handle_remote_reset(ceph::net::ConnectionRef conn) override; - seastar::future> - ms_get_authorizer(peer_type_t peer_type) override; + AuthAuthorizer* ms_get_authorizer(peer_type_t peer_type) const override; }; -- 2.39.5