auth: drop the RWLock in AuthClientHandler

the `RWLock` in `AuthClientHandler` was introduced in #1636. back to
then, we were accessing `MonClient::auth` directly, see
https://github.com/ceph/ceph/commit/a2eb6ae3fb57b09efdd4d7baac6871ca8dd8e79f#diff-fa6c2eba8356ae1442d1bf749beacfdfL6209
. and in #11128, we added a helper method offering accessing to
`MonClient::auth`. this method guards the access to `auth` using
`monc_lock`.

and there is no other users outside of `MonClient` using
`AuthClientHandler`. and all accesses to `MonClient::auth` are now
protected by `monc_lock`.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/auth/AuthClientHandler.cc b/src/auth/AuthClientHandler.cc
index 320d2faba4e153968811ca597590d819567f0364..1ac96a50c4f60e3b43f06ad657c8ee340c995cd3 100644 (file)
--- a/src/auth/AuthClientHandler.cc
+++ b/src/auth/AuthClientHandler.cc
@@ -21,19 +21,23 @@
 
 
 template<ceph::LockPolicy lp>
-AuthClientHandler<lp>*
-AuthClientHandler<lp>::create(CephContext *cct, int proto,
-                             RotatingKeyRing<lp> *rkeys)
+AuthClientHandler*
+AuthClientHandler::create(CephContext *cct, int proto,
+                         RotatingKeyRing<lp> *rkeys)
 {
   switch (proto) {
   case CEPH_AUTH_CEPHX:
     return new CephxClientHandler<lp>(cct, rkeys);
   case CEPH_AUTH_NONE:
-    return new AuthNoneClientHandler<lp>(cct, rkeys);
+    return new AuthNoneClientHandler{cct};
   default:
     return NULL;
   }
 }
 
 // explicitly instantiate only the classes we need
-template class AuthClientHandler<ceph::LockPolicy::MUTEX>;
+template AuthClientHandler*
+AuthClientHandler::create<ceph::LockPolicy::MUTEX>(
+  CephContext *cct,
+  int proto,
+  RotatingKeyRing<ceph::LockPolicy::MUTEX> *rkeys);

diff --git a/src/auth/AuthClientHandler.h b/src/auth/AuthClientHandler.h
index 140dbcc63b4d8b9a795a3551b2c4587e761cc9e6..a395c326838f32a0991901f2a0a7d595371490fb 100644 (file)
--- a/src/auth/AuthClientHandler.h
+++ b/src/auth/AuthClientHandler.h
@@ -18,13 +18,11 @@
 
 #include "auth/Auth.h"
 #include "common/lock_policy.h"
-#include "common/lock_shared_mutex.h"
 
 class CephContext;
 struct MAuthReply;
 template<ceph::LockPolicy> class RotatingKeyRing;
 
-template<ceph::LockPolicy lock_policy>
 class AuthClientHandler {
 protected:
   CephContext *cct;
@@ -33,19 +31,16 @@ protected:
   uint32_t want;
   uint32_t have;
   uint32_t need;
-  mutable SharedMutexT<lock_policy> lock;
 
 public:
   explicit AuthClientHandler(CephContext *cct_)
-    : cct(cct_), global_id(0), want(CEPH_ENTITY_TYPE_AUTH), have(0), need(0),
-      lock{SharedMutex<lock_policy>::create("AuthClientHandler::lock")}
+    : cct(cct_), global_id(0), want(CEPH_ENTITY_TYPE_AUTH), have(0), need(0)
   {}
   virtual ~AuthClientHandler() {}
 
   void init(const EntityName& n) { name = n; }
   
   void set_want_keys(__u32 keys) {
-    std::unique_lock l{lock};
     want = keys | CEPH_ENTITY_TYPE_AUTH;
     validate_tickets();
   }
@@ -64,7 +59,8 @@ public:
 
   virtual void set_global_id(uint64_t id) = 0;
 
-  static AuthClientHandler<lock_policy>*
+  template<ceph::LockPolicy lock_policy>
+  static AuthClientHandler*
   create(CephContext *cct, int proto, RotatingKeyRing<lock_policy> *rkeys);
 protected:
   virtual void validate_tickets() = 0;

diff --git a/src/auth/cephx/CephxClientHandler.cc b/src/auth/cephx/CephxClientHandler.cc
index ea0b67cebbb56c3f91d935b34308042ae212968f..1748fa27e543f5f7667909f4569f869d66384b8d 100644 (file)
--- a/src/auth/cephx/CephxClientHandler.cc
+++ b/src/auth/cephx/CephxClientHandler.cc
@@ -32,8 +32,6 @@ int CephxClientHandler<lp>::build_request(bufferlist& bl) const
 {
   ldout(cct, 10) << "build_request" << dendl;
 
-  std::shared_lock l{lock};
-
   if (need & CEPH_ENTITY_TYPE_AUTH) {
     /* authenticate */
     CephXRequestHeader header;
@@ -112,7 +110,6 @@ template<LockPolicy lp>
 int CephxClientHandler<lp>::handle_response(int ret, bufferlist::const_iterator& indata)
 {
   ldout(cct, 10) << "handle_response ret = " << ret << dendl;
-  std::unique_lock l{lock};
   
   if (ret < 0)
     return ret; // hrm!
@@ -206,7 +203,6 @@ int CephxClientHandler<lp>::handle_response(int ret, bufferlist::const_iterator&
 template<LockPolicy lp>
 AuthAuthorizer *CephxClientHandler<lp>::build_authorizer(uint32_t service_id) const
 {
-  std::shared_lock l{lock};
   ldout(cct, 10) << "build_authorizer for service " << ceph_entity_type_name(service_id) << dendl;
   return tickets.build_authorizer(service_id);
 }
@@ -225,7 +221,6 @@ bool CephxClientHandler<lp>::build_rotating_request(bufferlist& bl) const
 template<LockPolicy lp>
 void CephxClientHandler<lp>::prepare_build_request()
 {
-  std::unique_lock l{lock};
   ldout(cct, 10) << "validate_tickets: want=" << want << " need=" << need
                 << " have=" << have << dendl;
   validate_tickets();
@@ -245,7 +240,6 @@ void CephxClientHandler<lp>::validate_tickets()
 template<LockPolicy lp>
 bool CephxClientHandler<lp>::need_tickets()
 {
-  std::unique_lock l{lock};
   validate_tickets();
 
   ldout(cct, 20) << "need_tickets: want=" << want

diff --git a/src/auth/cephx/CephxClientHandler.h b/src/auth/cephx/CephxClientHandler.h
index eb6055c739ee5260e4647c9577f30bb63a95011f..7a91e0322c07c858af0bcc4dcd24521c34e3291b 100644 (file)
--- a/src/auth/cephx/CephxClientHandler.h
+++ b/src/auth/cephx/CephxClientHandler.h
@@ -23,7 +23,7 @@ class CephContext;
 class KeyRing;
 
 template<LockPolicy lock_policy>
-class CephxClientHandler : public AuthClientHandler<lock_policy> {
+class CephxClientHandler : public AuthClientHandler {
   bool starting;
 
   /* envelope protocol parameters */
@@ -35,17 +35,10 @@ class CephxClientHandler : public AuthClientHandler<lock_policy> {
   RotatingKeyRing<lock_policy> *rotating_secrets;
   KeyRing *keyring;
 
-  using AuthClientHandler<lock_policy>::cct;
-  using AuthClientHandler<lock_policy>::global_id;
-  using AuthClientHandler<lock_policy>::want;
-  using AuthClientHandler<lock_policy>::have;
-  using AuthClientHandler<lock_policy>::need;
-  using AuthClientHandler<lock_policy>::lock;
-
 public:
   CephxClientHandler(CephContext *cct_,
                     RotatingKeyRing<lock_policy> *rsecrets)
-    : AuthClientHandler<lock_policy>(cct_),
+    : AuthClientHandler(cct_),
       starting(false),
       server_challenge(0),
       tickets(cct_),
@@ -57,7 +50,6 @@ public:
   }
 
   void reset() override {
-    std::unique_lock l{lock};
     starting = true;
     server_challenge = 0;
   }
@@ -73,7 +65,6 @@ public:
   bool need_tickets() override;
 
   void set_global_id(uint64_t id) override {
-    std::unique_lock l{lock};
     global_id = id;
     tickets.global_id = id;
   }

diff --git a/src/auth/none/AuthNoneClientHandler.h b/src/auth/none/AuthNoneClientHandler.h
index 32a410ab76daa2123b580fd767dd9a183aaf5364..c74acb6ec3856e38202b9a34b53c09e6bcd3c14a 100644 (file)
--- a/src/auth/none/AuthNoneClientHandler.h
+++ b/src/auth/none/AuthNoneClientHandler.h
@@ -20,16 +20,11 @@
 #include "common/ceph_context.h"
 #include "common/config.h"
 
-template<LockPolicy lock_policy>
-class AuthNoneClientHandler : public AuthClientHandler<lock_policy> {
-  using AuthClientHandler<lock_policy>::cct;
-  using AuthClientHandler<lock_policy>::global_id;
-  using AuthClientHandler<lock_policy>::lock;
+class AuthNoneClientHandler : public AuthClientHandler {
 
 public:
-  AuthNoneClientHandler(CephContext *cct_,
-                       RotatingKeyRing<lock_policy> *rkeys)
-    : AuthClientHandler<lock_policy>(cct_) {}
+  AuthNoneClientHandler(CephContext *cct_)
+    : AuthClientHandler(cct_) {}
 
   void reset() override { }
 
@@ -41,7 +36,6 @@ public:
   int get_protocol() const override { return CEPH_AUTH_NONE; }
   
   AuthAuthorizer *build_authorizer(uint32_t service_id) const override {
-    std::shared_lock l{lock};
     AuthNoneAuthorizer *auth = new AuthNoneAuthorizer();
     if (auth) {
       auth->build_authorizer(cct->_conf->name, global_id);
@@ -52,7 +46,6 @@ public:
   bool need_tickets() override { return false; }
 
   void set_global_id(uint64_t id) override {
-    std::unique_lock l{lock};
     global_id = id;
   }
 private:

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index 09a86749faef39d8b9fdfe9e4b2d669b44aeb25c..56baab2385897ac1349685b4d6f8802d08855b11 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -1278,7 +1278,7 @@ int MonConnection::_negotiate(MAuthReply *m,
   }
 
   auth.reset(
-    AuthClientHandler<LockPolicy::MUTEX>::create(cct,m->protocol, keyring));
+    AuthClientHandler::create(cct,m->protocol, keyring));
   if (!auth) {
     ldout(cct, 10) << "no handler for protocol " << m->protocol << dendl;
     if (m->result == -ENOTSUP) {

diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index bffd2d916e67c83a150d52e8bfa161cb81253b34..e359f96bd3751486a0c918438748e44137b957d9 100644 (file)
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -34,7 +34,7 @@ class MAuthRotating;
 class LogClient;
 class AuthAuthorizer;
 class AuthMethodList;
-template<LockPolicy> class AuthClientHandler;
+class AuthClientHandler;
 class KeyRing;
 template<LockPolicy> class RotatingKeyRing;
 
@@ -118,7 +118,7 @@ public:
   ConnectionRef get_con() {
     return con;
   }
-  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>>& get_auth() {
+  std::unique_ptr<AuthClientHandler>& get_auth() {
     return auth;
   }
 
@@ -139,7 +139,7 @@ private:
   State state = State::NONE;
   ConnectionRef con;
 
-  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>> auth;
+  std::unique_ptr<AuthClientHandler> auth;
   uint64_t global_id;
 };
 
@@ -192,7 +192,7 @@ private:
   bool got_config = false;
 
   // authenticate
-  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>> auth;
+  std::unique_ptr<AuthClientHandler> auth;
   uint32_t want_keys = 0;
   uint64_t global_id = 0;
   Cond auth_cond;

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 8da36176c0a6232a4d83876fb1bbc515f62a6c4d..4771c9f549cd1fd6be5d0f3d6aa0f8339babe824 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -5713,7 +5713,7 @@ bool Monitor::ms_get_authorizer(int service_id, AuthAuthorizer **authorizer,
   if (!auth_cluster_required.is_supported_auth(CEPH_AUTH_CEPHX)) {
     // auth_none
     dout(20) << __func__ << " building auth_none authorizer" << dendl;
-    AuthNoneClientHandler<LockPolicy::MUTEX> handler(g_ceph_context, nullptr);
+    AuthNoneClientHandler handler{g_ceph_context};
     handler.set_global_id(0);
     *authorizer = handler.build_authorizer(service_id);
     return true;