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 <kchai@redhat.com>

diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt
index d7b58521d4b043fa4cc67ad98a6a2d81e77075fe..05d6f148f39549e2b97be191541514fbf1a0d612 100644 (file)
--- 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 (file)
index 336ded3..0000000
--- a/src/crimson/net/Dispatcher.cc
+++ /dev/null
@@ -1,11 +0,0 @@
-#include "auth/Auth.h"
-#include "Dispatcher.h"
-
-namespace ceph::net
-{
-seastar::future<std::unique_ptr<AuthAuthorizer>>
-Dispatcher::ms_get_authorizer(peer_type_t)
-{
-  return seastar::make_ready_future<std::unique_ptr<AuthAuthorizer>>(nullptr);
-}
-}

diff --git a/src/crimson/net/Dispatcher.h b/src/crimson/net/Dispatcher.h
index cbde15499286f4a01b07f130ee3cfbd37d7ed962..8bcc47c917cf9c529067daa9afc12742821dc3cf 100644 (file)
--- a/src/crimson/net/Dispatcher.h
+++ b/src/crimson/net/Dispatcher.h
@@ -53,8 +53,9 @@ class Dispatcher {
                       bufferlist&) {
     return seastar::make_ready_future<msgr_tag_t, bufferlist>(0, bufferlist{});
   }
-  virtual seastar::future<std::unique_ptr<AuthAuthorizer>>
-  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 2907c48687140bdd05bef8b94f4afd94122a8ed9..7f0f4fc4cb93af3abca8fe1bbdbac80a49fdd2ce 100644 (file)
--- 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>(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 62cc77d534776617a674e108ecce01e88ada0a2e..9fd3bdf294362f9541d3ffc165c67a80707bef30 100644 (file)
--- 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<AuthAuthorizer> 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 da4aa269ee330bfe49de165a9121880e1d28591f..dfca51e8cbad0a95a15af7f126d8dd6112d8f350 100644 (file)
--- 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<std::unique_ptr<AuthAuthorizer>>
-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<Dispatcher*>::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<AuthAuthorizer>;
-    return seastar::repeat_until_value([&] () {
-      auto& first = params.first;
-      if (first == params.last) {
-        // just give up
-        return seastar::make_ready_future<std::optional<result_t>>(result_t{});
-      } else {
-        return (*first)->ms_get_authorizer(params.peer_type)
-          .then([&] (auto&& auth)-> std::optional<result_t> {
-          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 c30408361ba9be1e47ff4a32eb5cfa032c530974..8368021c86998cb9c2a035e26680e15bacc2c78d 100644 (file)
--- 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<std::unique_ptr<AuthAuthorizer>>
-  ms_get_authorizer(peer_type_t peer_type) override;
+  AuthAuthorizer* ms_get_authorizer(peer_type_t peer_type) const override;
 };