From 9c679edea638e1b1e0594f867a222a7c9de69116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 30 Nov 2018 12:41:12 +0200 Subject: [PATCH] MXS-2196: Preload DCB entry points in Listener By loading the entry points required by a DCB when the Listener is created, the extra cost of finding the module is removed. It also simplifies DCB creation by removing the possibility of all failures to load modules at DCB creation time. --- include/maxscale/listener.hh | 12 ++++++++ server/core/dcb.cc | 33 +++----------------- server/core/listener.cc | 59 +++++++++++++++--------------------- 3 files changed, 40 insertions(+), 64 deletions(-) diff --git a/include/maxscale/listener.hh b/include/maxscale/listener.hh index 2538b1f10..44fbb7abb 100644 --- a/include/maxscale/listener.hh +++ b/include/maxscale/listener.hh @@ -124,6 +124,16 @@ public: */ const char* protocol() const; + /** + * The protocol module entry points + */ + const MXS_PROTOCOL& protocol_func() const; + + /** + * The authenticator module entry points + */ + const MXS_AUTHENTICATOR& auth_func() const; + /** * The authenticator instance */ @@ -180,6 +190,8 @@ private: struct users* m_users; /**< The user data for this listener */ SERVICE* m_service; /**< The service which used by this listener */ std::atomic m_active; /**< True if the port has not been deleted */ + MXS_PROTOCOL m_proto_func; /**< Preloaded protocol functions */ + MXS_AUTHENTICATOR m_auth_func; /**< Preloaded authenticator functions */ /** * Creates a new listener that points to a service diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 081145ada..083a06a8a 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2390,11 +2390,8 @@ int dcb_connect_SSL(DCB* dcb) DCB* dcb_accept(DCB* dcb) { DCB* client_dcb = NULL; - MXS_PROTOCOL* protocol_funcs = &dcb->func; int c_sock; - int sendbuf; struct sockaddr_storage client_conn; - socklen_t optlen = sizeof(sendbuf); if ((c_sock = dcb_accept_one_connection(dcb, (struct sockaddr*)&client_conn)) >= 0) { @@ -2402,7 +2399,7 @@ DCB* dcb_accept(DCB* dcb) configure_network_socket(c_sock, client_conn.ss_family); - client_dcb = dcb_alloc(DCB_ROLE_CLIENT_HANDLER, dcb->listener, dcb->service); + client_dcb = dcb_alloc(DCB_ROLE_CLIENT_HANDLER, dcb->listener, dcb->listener->service()); if (client_dcb == NULL) { @@ -2411,10 +2408,7 @@ DCB* dcb_accept(DCB* dcb) } else { - const char* authenticator_name = "NullAuthDeny"; - MXS_AUTHENTICATOR* authfuncs; - - client_dcb->service = dcb->session->service; + client_dcb->service = dcb->listener->service(); client_dcb->session = session_set_dummy(client_dcb); client_dcb->fd = c_sock; @@ -2452,27 +2446,8 @@ DCB* dcb_accept(DCB* dcb) } } - memcpy(&client_dcb->func, protocol_funcs, sizeof(MXS_PROTOCOL)); - if (*dcb->listener->authenticator()) - { - authenticator_name = dcb->listener->authenticator(); - } - else if (client_dcb->func.auth_default != NULL) - { - authenticator_name = client_dcb->func.auth_default(); - } - if ((authfuncs = (MXS_AUTHENTICATOR*)load_module(authenticator_name, - MODULE_AUTHENTICATOR)) == NULL) - { - if ((authfuncs = (MXS_AUTHENTICATOR*)load_module("NullAuthDeny", - MODULE_AUTHENTICATOR)) == NULL) - { - MXS_ERROR("Failed to load authenticator module '%s'", authenticator_name); - dcb_close(client_dcb); - return NULL; - } - } - memcpy(&(client_dcb->authfunc), authfuncs, sizeof(MXS_AUTHENTICATOR)); + client_dcb->func = dcb->listener->protocol_func(); + client_dcb->authfunc = dcb->listener->auth_func(); /** Allocate DCB specific authentication data */ if (client_dcb->authfunc.create diff --git a/server/core/listener.cc b/server/core/listener.cc index 22de48b9e..a2c1cad75 100644 --- a/server/core/listener.cc +++ b/server/core/listener.cc @@ -57,6 +57,8 @@ Listener::Listener(SERVICE* service, const std::string& name, const std::string& , m_listener(nullptr) , m_users(nullptr) , m_service(service) + , m_proto_func(*(MXS_PROTOCOL*)load_module(protocol.c_str(), MODULE_PROTOCOL)) + , m_auth_func(*(MXS_AUTHENTICATOR*)load_module(authenticator.c_str(), MODULE_AUTHENTICATOR)) { } @@ -103,11 +105,19 @@ SListener Listener::create(SERVICE* service, return nullptr; } + // Add protocol and authenticator capabilities from the listener + const MXS_MODULE* proto_mod = get_module(protocol.c_str(), MODULE_PROTOCOL); + const MXS_MODULE* auth_mod = get_module(auth, MODULE_AUTHENTICATOR); + mxb_assert(proto_mod && auth_mod); + SListener listener(new(std::nothrow) Listener(service, name, address, port, protocol, auth, auth_options, auth_instance, ssl)); if (listener) { + // Note: This isn't good: we modify the service from a listener and the service itself should do this. + service->capabilities |= proto_mod->module_capabilities | auth_mod->module_capabilities; + std::lock_guard guard(listener_lock); all_listeners.push_back(listener); } @@ -593,9 +603,9 @@ json_t* Listener::to_json() const json_object_set_new(attr, CN_STATE, json_string(state())); json_object_set_new(attr, CN_PARAMETERS, param); - if (m_listener->authfunc.diagnostic_json) + if (m_auth_func.diagnostic_json) { - json_t* diag = m_listener->authfunc.diagnostic_json(this); + json_t* diag = m_auth_func.diagnostic_json(this); if (diag) { @@ -641,6 +651,16 @@ const char* Listener::protocol() const return m_protocol.c_str(); } +const MXS_PROTOCOL& Listener::protocol_func() const +{ + return m_proto_func; +} + +const MXS_AUTHENTICATOR& Listener::auth_func() const +{ + return m_auth_func; +} + void* Listener::auth_instance() const { return m_auth_instance; @@ -719,39 +739,8 @@ bool Listener::listen() return false; } - MXS_PROTOCOL* funcs = (MXS_PROTOCOL*)load_module(protocol(), MODULE_PROTOCOL); - - if (!funcs) - { - MXS_ERROR("Unable to load protocol module %s. Listener for service %s not started.", - protocol(), m_service->name); - return false; - } - - memcpy(&(m_listener->func), funcs, sizeof(MXS_PROTOCOL)); - - if (m_authenticator.empty()) - { - m_authenticator = m_listener->func.auth_default(); - } - - MXS_AUTHENTICATOR* authfuncs = (MXS_AUTHENTICATOR*)load_module(authenticator(), MODULE_AUTHENTICATOR); - - if (!authfuncs) - { - MXS_ERROR("Failed to load authenticator module '%s' for listener '%s'", authenticator(), name()); - return false; - } - - // Add protocol and authenticator capabilities from the listener - const MXS_MODULE* proto_mod = get_module(protocol(), MODULE_PROTOCOL); - const MXS_MODULE* auth_mod = get_module(authenticator(), MODULE_AUTHENTICATOR); - mxb_assert(proto_mod && auth_mod); - - // Note: This isn't good: we modify the service from a listener and the service itself should do this. - m_service->capabilities |= proto_mod->module_capabilities | auth_mod->module_capabilities; - - memcpy(&m_listener->authfunc, authfuncs, sizeof(MXS_AUTHENTICATOR)); + m_listener->func = m_proto_func; + m_listener->authfunc = m_auth_func; /** * Normally, we'd allocate the DCB specific authentication data. As the