From 3791fdded764d39beeabc4b3b69b9dd5f5dcbe2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sat, 1 Dec 2018 20:31:48 +0200 Subject: [PATCH] MXS-2196: Pass client DCB to MXS_PROTOCOL::accept By doing the actual accepting of the new DCB in the core, the protocol modules can only do the actual protocol level work. This removes some of the redundant code that was in the protocol modules. --- include/maxscale/protocol.h | 9 +- server/core/dcb.cc | 8 +- server/modules/protocol/CDC/cdc.cc | 92 +++++++++---------- server/modules/protocol/HTTPD/httpd.cc | 37 +++----- .../MySQL/mariadbclient/mysql_client.cc | 13 +-- .../modules/protocol/maxscaled/maxscaled.cc | 69 +++++++------- server/modules/protocol/telnetd/telnetd.cc | 50 +++++----- 7 files changed, 122 insertions(+), 156 deletions(-) diff --git a/include/maxscale/protocol.h b/include/maxscale/protocol.h index 48935e0b1..c0942cde6 100644 --- a/include/maxscale/protocol.h +++ b/include/maxscale/protocol.h @@ -23,11 +23,6 @@ #include #include -#include - -class Listener; -using SListener = std::shared_ptr; - MXS_BEGIN_DECLS struct DCB; @@ -94,13 +89,13 @@ typedef struct mxs_protocol /** * Accept a connection, only for client side protocol modules * - * @param dcb The listener DCB + * @param dcb The client DCB * * @return 1 on success, 0 on error * * @note Currently the return value is ignored */ - int32_t (* accept)(const SListener& listener); + int32_t (* accept)(DCB* client_dcb); /** * Connect to a server, only for backend side protocol modules diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 5963c3013..c3c51e0da 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2999,8 +2999,12 @@ static uint32_t dcb_process_poll_events(DCB* dcb, uint32_t events) if (dcb_session_check(dcb, "accept")) { - DCB_EH_NOTICE("Calling dcb->func.accept(%p)", dcb); - dcb->func.accept(dcb->listener); + DCB* client_dcb; + + while ((client_dcb = dcb_accept(dcb->listener))) + { + dcb->func.accept(client_dcb); + } } } else diff --git a/server/modules/protocol/CDC/cdc.cc b/server/modules/protocol/CDC/cdc.cc index 0ae38144a..145cff2f1 100644 --- a/server/modules/protocol/CDC/cdc.cc +++ b/server/modules/protocol/CDC/cdc.cc @@ -48,7 +48,7 @@ static int cdc_write_event(DCB* dcb); static int cdc_write(DCB* dcb, GWBUF* queue); static int cdc_error(DCB* dcb); static int cdc_hangup(DCB* dcb); -static int cdc_accept(const SListener& listener); +static int cdc_accept(DCB*); static int cdc_close(DCB* dcb); static CDC_protocol* cdc_protocol_init(DCB* dcb); static void cdc_protocol_done(DCB* dcb); @@ -280,60 +280,52 @@ static int cdc_hangup(DCB* dcb) * * @param dcb The descriptor control block */ -static int cdc_accept(const SListener& listener) +static int cdc_accept(DCB* client_dcb) { - int n_connect = 0; - DCB* client_dcb; + CDC_session* client_data = NULL; + CDC_protocol* protocol = NULL; - while ((client_dcb = dcb_accept(listener)) != NULL) + /* allocating CDC protocol */ + protocol = cdc_protocol_init(client_dcb); + if (protocol == NULL) { - CDC_session* client_data = NULL; - CDC_protocol* protocol = NULL; - - /* allocating CDC protocol */ - protocol = cdc_protocol_init(client_dcb); - if (protocol == NULL) - { - client_dcb->protocol = NULL; - dcb_close(client_dcb); - continue; - } - - client_dcb->protocol = (CDC_protocol*) protocol; - - /* Dummy session */ - client_dcb->session = session_set_dummy(client_dcb); - - if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) - { - dcb_close(client_dcb); - continue; - } - - /* - * create the session data for CDC - * this coud be done in anothe routine, let's keep it here for now - */ - client_data = (CDC_session*) MXS_CALLOC(1, sizeof(CDC_session)); - if (client_data == NULL) - { - dcb_close(client_dcb); - continue; - } - - client_dcb->data = client_data; - - /* client protocol state change to CDC_STATE_WAIT_FOR_AUTH */ - protocol->state = CDC_STATE_WAIT_FOR_AUTH; - - MXS_NOTICE("%s: new connection from [%s]", - client_dcb->service->name, - client_dcb->remote != NULL ? client_dcb->remote : ""); - - n_connect++; + client_dcb->protocol = NULL; + dcb_close(client_dcb); + return 0; } - return n_connect; + client_dcb->protocol = (CDC_protocol*) protocol; + + /* Dummy session */ + client_dcb->session = session_set_dummy(client_dcb); + + if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) + { + dcb_close(client_dcb); + return 0; + } + + /* + * create the session data for CDC + * this coud be done in anothe routine, let's keep it here for now + */ + client_data = (CDC_session*) MXS_CALLOC(1, sizeof(CDC_session)); + if (client_data == NULL) + { + dcb_close(client_dcb); + return 0; + } + + client_dcb->data = client_data; + + /* client protocol state change to CDC_STATE_WAIT_FOR_AUTH */ + protocol->state = CDC_STATE_WAIT_FOR_AUTH; + + MXS_NOTICE("%s: new connection from [%s]", + client_dcb->service->name, + client_dcb->remote != NULL ? client_dcb->remote : ""); + + return 1; } /** diff --git a/server/modules/protocol/HTTPD/httpd.cc b/server/modules/protocol/HTTPD/httpd.cc index c176a9d9e..f6891d279 100644 --- a/server/modules/protocol/HTTPD/httpd.cc +++ b/server/modules/protocol/HTTPD/httpd.cc @@ -50,7 +50,7 @@ static int httpd_write_event(DCB* dcb); static int httpd_write(DCB* dcb, GWBUF* queue); static int httpd_error(DCB* dcb); static int httpd_hangup(DCB* dcb); -static int httpd_accept(const SListener& listener); +static int httpd_accept(DCB*); static int httpd_close(DCB* dcb); static int httpd_get_line(int sock, char* buf, int size); static void httpd_send_headers(DCB* dcb, int final, bool auth_ok); @@ -358,33 +358,26 @@ static int httpd_hangup(DCB* dcb) * * @param listener The descriptor control block */ -static int httpd_accept(const SListener& listener) +static int httpd_accept(DCB* client_dcb) { - int n_connect = 0; - DCB* client_dcb; + HTTPD_session* client_data = NULL; - while ((client_dcb = dcb_accept(listener)) != NULL) + /* create the session data for HTTPD */ + if ((client_data = (HTTPD_session*)MXS_CALLOC(1, sizeof(HTTPD_session))) == NULL) { - HTTPD_session* client_data = NULL; + dcb_close(client_dcb); + return 0; + } + client_dcb->data = client_data; - /* create the session data for HTTPD */ - if ((client_data = (HTTPD_session*)MXS_CALLOC(1, sizeof(HTTPD_session))) == NULL) - { - dcb_close(client_dcb); - continue; - } - client_dcb->data = client_data; - - client_dcb->session = session_alloc(listener->service(), client_dcb); - if (NULL == client_dcb->session || poll_add_dcb(client_dcb) == -1) - { - dcb_close(client_dcb); - continue; - } - n_connect++; + client_dcb->session = session_alloc(client_dcb->service, client_dcb); + if (NULL == client_dcb->session || poll_add_dcb(client_dcb) == -1) + { + dcb_close(client_dcb); + return 0; } - return n_connect; + return 1; } /** diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index abccf486d..0654e9558 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -58,7 +58,7 @@ static void process_finish(void); static int thread_init(void); static void thread_finish(void); -static int gw_MySQLAccept(const SListener& listener); +static int gw_MySQLAccept(DCB*); static int gw_read_client_event(DCB* dcb); static int gw_write_client_event(DCB* dcb); static int gw_MySQLWrite_client(DCB* dcb, GWBUF* queue); @@ -1390,16 +1390,9 @@ return_1: * @return 0 in success, 1 in failure * */ -int gw_MySQLAccept(const SListener& listener) +int gw_MySQLAccept(DCB* client_dcb) { - DCB* client_dcb; - - while ((client_dcb = dcb_accept(listener)) != NULL) - { - gw_process_one_new_client(client_dcb); - } /**< while client_dcb != NULL */ - - /* Must have broken out of while loop or received NULL client_dcb */ + gw_process_one_new_client(client_dcb); return 1; } diff --git a/server/modules/protocol/maxscaled/maxscaled.cc b/server/modules/protocol/maxscaled/maxscaled.cc index 022dabab2..e68d7152f 100644 --- a/server/modules/protocol/maxscaled/maxscaled.cc +++ b/server/modules/protocol/maxscaled/maxscaled.cc @@ -61,7 +61,7 @@ static int maxscaled_write_event(DCB* dcb); static int maxscaled_write(DCB* dcb, GWBUF* queue); static int maxscaled_error(DCB* dcb); static int maxscaled_hangup(DCB* dcb); -static int maxscaled_accept(const SListener& listener); +static int maxscaled_accept(DCB*); static int maxscaled_close(DCB* dcb); static char* mxsd_default_auth(); @@ -345,48 +345,43 @@ static int maxscaled_hangup(DCB* dcb) * @param dcb The descriptor control block * @return The number of new connections created */ -static int maxscaled_accept(const SListener& listener) +static int maxscaled_accept(DCB* client_dcb) { - int n_connect = 0; - DCB* client_dcb; socklen_t len = sizeof(struct ucred); struct ucred ucred; - while ((client_dcb = dcb_accept(listener)) != NULL) + MAXSCALED* maxscaled_protocol = (MAXSCALED*)calloc(1, sizeof(MAXSCALED)); + + if (!maxscaled_protocol) { - MAXSCALED* maxscaled_protocol = (MAXSCALED*)calloc(1, sizeof(MAXSCALED)); - - if (!maxscaled_protocol) - { - dcb_close(client_dcb); - continue; - } - - maxscaled_protocol->username = NULL; - maxscaled_protocol->state = MAXSCALED_STATE_LOGIN; - - bool authenticated = false; - - if (!authenticate_socket(maxscaled_protocol, client_dcb)) - { - dcb_close(client_dcb); - free(maxscaled_protocol); - continue; - } - - pthread_mutex_init(&maxscaled_protocol->lock, NULL); - client_dcb->protocol = (void*)maxscaled_protocol; - - client_dcb->session = session_alloc(listener->service(), client_dcb); - - if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) - { - dcb_close(client_dcb); - continue; - } - n_connect++; + dcb_close(client_dcb); + return 0; } - return n_connect; + + maxscaled_protocol->username = NULL; + maxscaled_protocol->state = MAXSCALED_STATE_LOGIN; + + bool authenticated = false; + + if (!authenticate_socket(maxscaled_protocol, client_dcb)) + { + dcb_close(client_dcb); + free(maxscaled_protocol); + return 0; + } + + pthread_mutex_init(&maxscaled_protocol->lock, NULL); + client_dcb->protocol = (void*)maxscaled_protocol; + + client_dcb->session = session_alloc(client_dcb->service, client_dcb); + + if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) + { + dcb_close(client_dcb); + return 0; + } + + return 1; } /** diff --git a/server/modules/protocol/telnetd/telnetd.cc b/server/modules/protocol/telnetd/telnetd.cc index 65acb9715..af1c725cf 100644 --- a/server/modules/protocol/telnetd/telnetd.cc +++ b/server/modules/protocol/telnetd/telnetd.cc @@ -63,7 +63,7 @@ static int telnetd_write_event(DCB* dcb); static int telnetd_write(DCB* dcb, GWBUF* queue); static int telnetd_error(DCB* dcb); static int telnetd_hangup(DCB* dcb); -static int telnetd_accept(const SListener& listener); +static int telnetd_accept(DCB*); static int telnetd_close(DCB* dcb); static char* telnetd_default_auth(); @@ -276,37 +276,31 @@ static int telnetd_hangup(DCB* dcb) * @param listener The descriptor control block * @return The number of new connections created */ -static int telnetd_accept(const SListener& listener) +static int telnetd_accept(DCB* client_dcb) { - int n_connect = 0; - DCB* client_dcb; + TELNETD* telnetd_protocol = NULL; - while ((client_dcb = dcb_accept(listener)) != NULL) + if ((telnetd_protocol = (TELNETD*)MXS_CALLOC(1, sizeof(TELNETD))) == NULL) { - TELNETD* telnetd_protocol = NULL; - - if ((telnetd_protocol = (TELNETD*)MXS_CALLOC(1, sizeof(TELNETD))) == NULL) - { - dcb_close(client_dcb); - continue; - } - telnetd_protocol->state = TELNETD_STATE_LOGIN; - telnetd_protocol->username = NULL; - client_dcb->protocol = (void*)telnetd_protocol; - - client_dcb->session = session_alloc(listener->service(), client_dcb); - if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) - { - dcb_close(client_dcb); - continue; - } - - ssl_authenticate_client(client_dcb, client_dcb->authfunc.connectssl(client_dcb)); - - n_connect++; - dcb_printf(client_dcb, "MaxScale login: "); + dcb_close(client_dcb); + return 0; } - return n_connect; + telnetd_protocol->state = TELNETD_STATE_LOGIN; + telnetd_protocol->username = NULL; + client_dcb->protocol = (void*)telnetd_protocol; + + client_dcb->session = session_alloc(client_dcb->service, client_dcb); + if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) + { + dcb_close(client_dcb); + return 0; + } + + ssl_authenticate_client(client_dcb, client_dcb->authfunc.connectssl(client_dcb)); + + dcb_printf(client_dcb, "MaxScale login: "); + + return 1; } /**