From 66ba7f3c80ad9d4f518411bb15092ad2531753f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 6 Mar 2017 12:24:12 +0200 Subject: [PATCH] Simplify network socket creation code The socket creation code in mysql_backend.c wasn't MySQL specific and it could be used for all non-blocking network connections. Thus, it makes sense to move it to a common file where other protocol modules can use it. The address resolution code now uses `getaddrinfo` to resolve all addresses instead of manually handling wildcard hosts. This allows the same code to be used for all addresses. --- include/maxscale/utils.h | 10 +- server/core/dcb.c | 17 +-- server/core/utils.c | 131 +++++++++++------- .../MySQL/MySQLBackend/mysql_backend.c | 86 +----------- 4 files changed, 98 insertions(+), 146 deletions(-) diff --git a/include/maxscale/utils.h b/include/maxscale/utils.h index ca32996ec..5aaabd729 100644 --- a/include/maxscale/utils.h +++ b/include/maxscale/utils.h @@ -44,10 +44,10 @@ void utils_end(); * The configuration is passed as string in the `address|port` format. * * @param config The bind address and port separated by a '|' - * @param addr The sockaddr_in6 in which the data is written - * @return 1 on success, 0 on failure + * @param addr The struct sockaddr_storage in which the data is written + * @return True on success, false on failure */ -int parse_bindconfig(const char *, struct sockaddr_in6 *, int *); +bool parse_bindconfig(const char *config, struct sockaddr_storage *addr); /** @@ -55,9 +55,11 @@ int parse_bindconfig(const char *, struct sockaddr_in6 *, int *); * * @param dest Pointer to a struct sockaddr_storage where the configuration is stored * @param host The target host for which the socket is created + * @param port The target port on the host * * @return The opened socket or -1 on failure - */int create_network_socket(struct sockaddr_storage *, char *); + */ +int open_network_socket(struct sockaddr_storage *dest, char *host, uint16_t port); int setnonblocking(int fd); char *gw_strend(register const char *s); diff --git a/server/core/dcb.c b/server/core/dcb.c index dd4d20e9f..e83ba0f3e 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -3109,27 +3109,28 @@ dcb_listen(DCB *listener, const char *config, const char *protocol_name) static int dcb_listen_create_socket_inet(const char *config_bind) { - int listener_socket; - struct sockaddr_in6 server_address = {}; - int one = 1; - int sock_type = 0; + struct sockaddr_storage server_address = {}; - if (!parse_bindconfig(config_bind, &server_address, &sock_type)) + if (!parse_bindconfig(config_bind, &server_address)) { MXS_ERROR("Error in parse_bindconfig for [%s]", config_bind); return -1; } + /** TODO: Move everything before the `bind` call to utils.c */ + /** Create the TCP socket */ - if ((listener_socket = socket(sock_type, SOCK_STREAM, 0)) < 0) + int listener_socket = socket(server_address.ss_family, SOCK_STREAM, 0); + + if (listener_socket < 0) { char errbuf[MXS_STRERROR_BUFLEN]; - MXS_ERROR("Can't create socket: %i, %s", - errno, + MXS_ERROR("Can't create socket: %d, %s", errno, strerror_r(errno, errbuf, sizeof(errbuf))); return -1; } + int one = 1; // socket options if (dcb_set_socket_option(listener_socket, SOL_SOCKET, SO_REUSEADDR, (char *) &one, sizeof(one)) != 0 || dcb_set_socket_option(listener_socket, IPPROTO_TCP, TCP_NODELAY, (char *) &one, sizeof(one)) != 0) diff --git a/server/core/utils.c b/server/core/utils.c index fa3f73ba6..f95f4fc7b 100644 --- a/server/core/utils.c +++ b/server/core/utils.c @@ -37,12 +37,13 @@ #include #include #include - +#include #include #include #include #include +#include #include #include #include @@ -895,42 +896,62 @@ void utils_end() SPINLOCK tmplock = SPINLOCK_INIT; -int create_network_socket(struct sockaddr_storage *dest, char *host) +static bool configure_socket(int so) +{ + int sndbufsize = MXS_BACKEND_SO_SNDBUF; + int rcvbufsize = MXS_BACKEND_SO_RCVBUF; + int one = 1; + + if (setsockopt(so, SOL_SOCKET, SO_SNDBUF, &sndbufsize, sizeof(sndbufsize)) != 0 || + setsockopt(so, SOL_SOCKET, SO_RCVBUF, &rcvbufsize, sizeof(rcvbufsize)) != 0 || + setsockopt(so, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) != 0) + { + char errbuf[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to set socket option: %d, %s.", + errno, strerror_r(errno, errbuf, sizeof(errbuf))); + return false; + } + + return setnonblocking(so) == 0; +} + +static void set_port(struct sockaddr_storage *addr, uint16_t port) +{ + if (addr->ss_family == AF_INET) + { + struct sockaddr_in *ip = (struct sockaddr_in*)addr; + ip->sin_port = htons(port); + } + else if (addr->ss_family == AF_INET6) + { + struct sockaddr_in6 *ip = (struct sockaddr_in6*)addr; + ip->sin6_port = htons(port); + } + else + { + MXS_ERROR("Unknown address family: %d", (int)addr->ss_family); + ss_dassert(false); + } +} + +int open_network_socket(struct sockaddr_storage *dest, char *host, uint16_t port) { #ifdef __USE_POSIX struct addrinfo *ai = NULL, hint = {}; int so, rc; hint.ai_socktype = SOCK_STREAM; hint.ai_family = AF_UNSPEC; + hint.ai_flags = AI_ALL; - if (strcmp(host, "0.0.0.0") == 0 || strcmp(host, "::") == 0) + if ((rc = getaddrinfo(host, NULL, &hint, &ai)) != 0) { - /** All interfaces */ - hint.ai_flags = AI_PASSIVE; - if ((rc = getaddrinfo(host, NULL, &hint, &ai)) != 0) - { - MXS_ERROR("Failed to obtain address for host %s, %s", - host, - gai_strerror(rc)); - - return -1; - } - } - else - { - hint.ai_flags = AI_ALL; - if ((rc = getaddrinfo(host, NULL, &hint, &ai)) != 0) - { - MXS_ERROR("Failed to obtain address for host %s, %s", - host, - gai_strerror(rc)); - - return -1; - } + MXS_ERROR("Failed to obtain address for host %s, %s", + host, gai_strerror(rc)); + return -1; } - /* take the first one */ - if (ai != NULL) + /* Take the first one */ + if (ai) { so = socket(ai->ai_family, SOCK_STREAM, 0); @@ -943,23 +964,33 @@ int create_network_socket(struct sockaddr_storage *dest, char *host) else { memcpy(dest, ai->ai_addr, ai->ai_addrlen); - freeaddrinfo(ai); + set_port(dest, port); + + if (!configure_socket(so)) + { + close(so); + so = -1; + } } + + freeaddrinfo(ai); } + #else #error Only the POSIX networking interface is supported #endif + return so; } -int parse_bindconfig(const char *config, struct sockaddr_in6 *addr, int *sock_type) +bool parse_bindconfig(const char *config, struct sockaddr_storage *addr) { char buf[strlen(config) + 1]; strcpy(buf, config); - *sock_type = strchr(buf, '.') ? AF_INET : AF_INET6; char *port = strrchr(buf, '|'); short pnum; + if (port) { *port = 0; @@ -968,35 +999,35 @@ int parse_bindconfig(const char *config, struct sockaddr_in6 *addr, int *sock_ty } else { + ss_dassert(false); return 0; } - if (!strcmp(buf, "0.0.0.0") || !strcmp(buf, "::")) + struct addrinfo *ai = NULL, hint = {}; + hint.ai_flags = AI_ALL; + hint.ai_family = AF_UNSPEC; + int rc = getaddrinfo(buf, NULL, &hint, &ai); + + if (rc == 0) { - *sock_type = AF_INET6; - addr->sin6_addr = in6addr_any; + if (ai) + { + memcpy(addr, ai->ai_addr, ai->ai_addrlen); + set_port(addr, pnum); + freeaddrinfo(ai); + } + else + { + MXS_ERROR("Failed to find valid network address for '%s'.", config); + rc = -1; + } } else { - if (inet_pton(*sock_type, buf, &addr->sin6_addr) < 1) - { - struct hostent *hp = gethostbyname(buf); - - if (hp) - { - inet_pton(*sock_type, hp->h_addr_list[0], &addr->sin6_addr); - } - else - { - MXS_ERROR("Failed to lookup host '%s'.", buf); - return 0; - } - } + MXS_ERROR("Failed to resolve network address for '%s': %s", config, gai_strerror(rc)); } - addr->sin6_family = *sock_type; - addr->sin6_port = htons(pnum); - return 1; + return rc == 0; } /** diff --git a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c index 907156ec9..a15d3ab3e 100644 --- a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -275,82 +274,16 @@ static int gw_do_connect_to_backend(char *host, int port, int *fd) { struct sockaddr_storage serv_addr = {}; int rv = -1; - int bufsize; /* prepare for connect */ - int so = create_network_socket(&serv_addr, host); + int so = open_network_socket(&serv_addr, host, port); if (so < 0) { - char errbuf[MXS_STRERROR_BUFLEN]; MXS_ERROR("Establishing connection to backend server %s:%d failed.", host, port); return rv; } - /** Configure the destination port */ - if (serv_addr.ss_family == AF_INET) - { - struct sockaddr_in *ip = (struct sockaddr_in*)&serv_addr; - ip->sin_port = htons(port); - } - else - { - ss_dassert(serv_addr.ss_family == AF_INET6); - struct sockaddr_in6 *ip = (struct sockaddr_in6*)&serv_addr; - ip->sin6_port = htons(port); - } - - bufsize = MXS_BACKEND_SO_SNDBUF; - - if (setsockopt(so, SOL_SOCKET, SO_SNDBUF, &bufsize, sizeof(bufsize)) != 0) - { - char errbuf[MXS_STRERROR_BUFLEN]; - MXS_ERROR("Failed to set socket options " - "%s:%d failed.\n\t\t Socket configuration failed " - "due %d, %s.", - host, - port, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))); - /** Close socket */ - close_socket(so); - return rv; - } - bufsize = MXS_BACKEND_SO_RCVBUF; - - if (setsockopt(so, SOL_SOCKET, SO_RCVBUF, &bufsize, sizeof(bufsize)) != 0) - { - char errbuf[MXS_STRERROR_BUFLEN]; - MXS_ERROR("Failed to set socket options " - "%s:%d failed.\n\t\t Socket configuration failed " - "due %d, %s.", - host, - port, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))); - /** Close socket */ - close_socket(so); - return rv; - } - - int one = 1; - if (setsockopt(so, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) != 0) - { - char errbuf[MXS_STRERROR_BUFLEN]; - MXS_ERROR("Failed to set socket options " - "%s:%d failed.\n\t\t Socket configuration failed " - "due %d, %s.", - host, - port, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))); - /** Close socket */ - close_socket(so); - return rv; - } - - /* set socket to as non-blocking here */ - setnonblocking(so); rv = connect(so, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); if (rv != 0) @@ -365,7 +298,7 @@ static int gw_do_connect_to_backend(char *host, int port, int *fd) MXS_ERROR("Failed to connect backend server %s:%d due to: %d, %s.", host, port, errno, strerror_r(errno, errbuf, sizeof(errbuf))); /** Close socket */ - close_socket(so); + close(so); return rv; } } @@ -1817,21 +1750,6 @@ static bool sescmd_response_complete(DCB* dcb) return succp; } -static void inline -close_socket(int sock) -{ - /*< Close newly created socket. */ - if (close(sock) != 0) - { - char errbuf[MXS_STRERROR_BUFLEN]; - MXS_ERROR("Failed to close socket %d due %d, %s.", - sock, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))); - } - -} - /** * Create COM_CHANGE_USER packet and store it to GWBUF *