From d3fc982cd788e8b244c974eefa8658d38e51c9ed Mon Sep 17 00:00:00 2001 From: Valentine Krasnobaeva Date: Tue, 23 Apr 2024 23:37:43 +0200 Subject: [PATCH] MEDIUM: proto: make common fd checks in sock_create_server_socket quic_connect_server(), tcp_connect_server(), uxst_connect_server() duplicate same code to check different ERRNOs, that socket() and setns() may return. They also duplicate some runtime condition checks, applied to the obtained server socket fd. So, in order to remove these duplications and to improve code readability, let's encapsulate socket() and setns() ERRNOs handling in sock_handle_system_err(). It must be called just before fd's runtime condition checks, which we also move in sock_create_server_socket by the same reason. --- include/haproxy/sock.h | 2 +- src/proto_quic.c | 65 +++------------------------- src/proto_tcp.c | 64 +++------------------------- src/proto_uxst.c | 64 +++------------------------- src/sock.c | 96 ++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 108 insertions(+), 183 deletions(-) diff --git a/include/haproxy/sock.h b/include/haproxy/sock.h index b8bd731b0..ba8e9d021 100644 --- a/include/haproxy/sock.h +++ b/include/haproxy/sock.h @@ -30,7 +30,7 @@ #include #include -int sock_create_server_socket(struct connection *conn); +int sock_create_server_socket(struct connection *conn, struct proxy *be); void sock_enable(struct receiver *rx); void sock_disable(struct receiver *rx); void sock_unbind(struct receiver *rx); diff --git a/src/proto_quic.c b/src/proto_quic.c index d7559e737..c616f14e8 100644 --- a/src/proto_quic.c +++ b/src/proto_quic.c @@ -301,67 +301,12 @@ int quic_connect_server(struct connection *conn, int flags) return SF_ERR_INTERNAL; } - fd = conn->handle.fd = sock_create_server_socket(conn); - - if (fd == -1) { - qfprintf(stderr, "Cannot get a server socket.\n"); - - if (errno == ENFILE) { - conn->err_code = CO_ER_SYS_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system FD limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EMFILE) { - conn->err_code = CO_ER_PROC_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached process FD limit (maxsock=%d). Please check 'ulimit-n' and restart.\n", - be->id, global.maxsock); - } - else if (errno == ENOBUFS || errno == ENOMEM) { - conn->err_code = CO_ER_SYS_MEMLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system memory limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT) { - conn->err_code = CO_ER_NOPROTO; - } - else - conn->err_code = CO_ER_SOCK_ERR; - - /* this is a resource error */ - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - - if (fd >= global.maxsock) { - /* do not log anything there, it's a normal condition when this option - * is used to serialize connections to a server ! - */ - ha_alert("socket(): not enough free sockets. Raise -n argument. Giving up.\n"); - close(fd); - conn->err_code = CO_ER_CONF_FDLIM; - conn->flags |= CO_FL_ERROR; - return SF_ERR_PRXCOND; /* it is a configuration limit */ - } - - if (fd_set_nonblock(fd) == -1) { - qfprintf(stderr,"Cannot set client socket to non blocking mode.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } - - if (master == 1 && fd_set_cloexec(fd) == -1) { - ha_alert("Cannot set CLOEXEC on client socket.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } + /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ + fd = conn->handle.fd = sock_create_server_socket(conn, be); + if (fd & SF_ERR_MASK) + return fd; + /* FD is ok, perform protocol specific settings */ /* allow specific binding : * - server-specific at first * - proxy-specific next diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 45ce27f11..b1a369871 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -298,68 +298,14 @@ int tcp_connect_server(struct connection *conn, int flags) return SF_ERR_INTERNAL; } - fd = conn->handle.fd = sock_create_server_socket(conn); - if (fd == -1) { - qfprintf(stderr, "Cannot get a server socket.\n"); - if (errno == ENFILE) { - conn->err_code = CO_ER_SYS_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system FD limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EMFILE) { - conn->err_code = CO_ER_PROC_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached process FD limit (maxsock=%d). Please check 'ulimit-n' and restart.\n", - be->id, global.maxsock); - } - else if (errno == ENOBUFS || errno == ENOMEM) { - conn->err_code = CO_ER_SYS_MEMLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system memory limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT) { - conn->err_code = CO_ER_NOPROTO; - } - else - conn->err_code = CO_ER_SOCK_ERR; - - /* this is a resource error */ - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - - if (fd >= global.maxsock) { - /* do not log anything there, it's a normal condition when this option - * is used to serialize connections to a server ! - */ - ha_alert("socket(): not enough free sockets. Raise -n argument. Giving up.\n"); - close(fd); - conn->err_code = CO_ER_CONF_FDLIM; - conn->flags |= CO_FL_ERROR; - return SF_ERR_PRXCOND; /* it is a configuration limit */ - } - - if (fd_set_nonblock(fd) == -1 || - (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) == -1)) { - qfprintf(stderr,"Cannot set client socket to non blocking mode.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } - - if (master == 1 && fd_set_cloexec(fd) == -1) { - ha_alert("Cannot set CLOEXEC on client socket.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } + /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ + fd = conn->handle.fd = sock_create_server_socket(conn, be); + if ((fd & SF_ERR_MASK) == fd) + return fd; + /* FD is OK, continue with protocol specific settings */ if (be->options & PR_O_TCP_SRV_KA) { setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &one, sizeof(one)); diff --git a/src/proto_uxst.c b/src/proto_uxst.c index cd584aa9a..45953d3b1 100644 --- a/src/proto_uxst.c +++ b/src/proto_uxst.c @@ -239,66 +239,12 @@ static int uxst_connect_server(struct connection *conn, int flags) return SF_ERR_INTERNAL; } - fd = conn->handle.fd = sock_create_server_socket(conn); - if (fd == -1) { - qfprintf(stderr, "Cannot get a server socket.\n"); - - if (errno == ENFILE) { - conn->err_code = CO_ER_SYS_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system FD limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EMFILE) { - conn->err_code = CO_ER_PROC_FDLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached process FD limit (maxsock=%d). Please check 'ulimit-n' and restart.\n", - be->id, global.maxsock); - } - else if (errno == ENOBUFS || errno == ENOMEM) { - conn->err_code = CO_ER_SYS_MEMLIM; - send_log(be, LOG_EMERG, - "Proxy %s reached system memory limit (maxsock=%d). Please check system tunables.\n", - be->id, global.maxsock); - } - else if (errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT) { - conn->err_code = CO_ER_NOPROTO; - } - else - conn->err_code = CO_ER_SOCK_ERR; - - /* this is a resource error */ - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - - if (fd >= global.maxsock) { - /* do not log anything there, it's a normal condition when this option - * is used to serialize connections to a server ! - */ - ha_alert("socket(): not enough free sockets. Raise -n argument. Giving up.\n"); - close(fd); - conn->err_code = CO_ER_CONF_FDLIM; - conn->flags |= CO_FL_ERROR; - return SF_ERR_PRXCOND; /* it is a configuration limit */ - } - - if (fd_set_nonblock(fd) == -1) { - qfprintf(stderr,"Cannot set client socket to non blocking mode.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } - - if (master == 1 && fd_set_cloexec(fd) == -1) { - ha_alert("Cannot set CLOEXEC on client socket.\n"); - close(fd); - conn->err_code = CO_ER_SOCK_ERR; - conn->flags |= CO_FL_ERROR; - return SF_ERR_INTERNAL; - } + /* perform common checks on obtained socket FD, return appropriate Stream Error Flag in case of failure */ + fd = conn->handle.fd = sock_create_server_socket(conn, be); + if (fd & SF_ERR_MASK) + return fd; + /* FD is ok, continue with protocol specific settings */ if (global.tune.server_sndbuf) setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &global.tune.server_sndbuf, sizeof(global.tune.server_sndbuf)); diff --git a/src/sock.c b/src/sock.c index 215fddb51..a13450591 100644 --- a/src/sock.c +++ b/src/sock.c @@ -195,13 +195,63 @@ struct connection *sock_accept_conn(struct listener *l, int *status) goto done; } +/* Common code to handle in one place different ERRNOs, that socket() et setns() + * may return + */ +static int sock_handle_system_err(struct connection *conn, struct proxy *be) +{ + qfprintf(stderr, "Cannot get a server socket.\n"); + + conn->flags |= CO_FL_ERROR; + conn->err_code = CO_ER_SOCK_ERR; + + switch(errno) { + case ENFILE: + conn->err_code = CO_ER_SYS_FDLIM; + send_log(be, LOG_EMERG, + "Proxy %s reached system FD limit (maxsock=%d). " + "Please check system tunables.\n", be->id, global.maxsock); + + return SF_ERR_RESOURCE; + + case EMFILE: + conn->err_code = CO_ER_PROC_FDLIM; + send_log(be, LOG_EMERG, + "Proxy %s reached process FD limit (maxsock=%d). " + "Please check 'ulimit-n' and restart.\n", be->id, global.maxsock); + + return SF_ERR_RESOURCE; + + case ENOBUFS: + case ENOMEM: + conn->err_code = CO_ER_SYS_MEMLIM; + send_log(be, LOG_EMERG, + "Proxy %s reached system memory limit (maxsock=%d). " + "Please check system tunables.\n", be->id, global.maxsock); + + return SF_ERR_RESOURCE; + + case EAFNOSUPPORT: + case EPROTONOSUPPORT: + conn->err_code = CO_ER_NOPROTO; + break; + + default: + send_log(be, LOG_EMERG, + "Proxy %s cannot create a server socket: %s\n", + be->id, strerror(errno)); + } + + return SF_ERR_INTERNAL; +} + /* Create a socket to connect to the server in conn->dst (which MUST be valid), * using the configured namespace if needed, or the one passed by the proxy * protocol if required to do so. It then calls socket() or socketat(). On * success, checks if mark or tos sockopts need to be set on the file handle. * Returns the FD or error code. */ -int sock_create_server_socket(struct connection *conn) +int sock_create_server_socket(struct connection *conn, struct proxy *be) { const struct netns_entry *ns = NULL; int sock_fd; @@ -215,14 +265,52 @@ int sock_create_server_socket(struct connection *conn) } #endif sock_fd = my_socketat(ns, conn->dst->ss_family, SOCK_STREAM, 0); - if (sock_fd == -1) - goto end; + + /* at first, handle common to all proto families system limits and permission related errors */ + if (sock_fd == -1) { + return sock_handle_system_err(conn, be); + } + + /* now perform some runtime condition checks */ + if (sock_fd >= global.maxsock) { + /* do not log anything there, it's a normal condition when this option + * is used to serialize connections to a server ! + */ + ha_alert("socket(): not enough free sockets. Raise -n argument. Giving up.\n"); + send_log(be, LOG_EMERG, "socket(): not enough free sockets. Raise -n argument. Giving up.\n"); + close(sock_fd); + conn->err_code = CO_ER_CONF_FDLIM; + conn->flags |= CO_FL_ERROR; + + return SF_ERR_PRXCOND; /* it is a configuration limit */ + } + + if (fd_set_nonblock(sock_fd) == -1 || + ((conn->ctrl->sock_prot == IPPROTO_TCP) && (setsockopt(sock_fd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) == -1))) { + qfprintf(stderr,"Cannot set client socket to non blocking mode.\n"); + send_log(be, LOG_EMERG, "Cannot set client socket to non blocking mode.\n"); + close(sock_fd); + conn->err_code = CO_ER_SOCK_ERR; + conn->flags |= CO_FL_ERROR; + + return SF_ERR_INTERNAL; + } + + if (master == 1 && fd_set_cloexec(sock_fd) == -1) { + ha_alert("Cannot set CLOEXEC on client socket.\n"); + send_log(be, LOG_EMERG, "Cannot set CLOEXEC on client socket.\n"); + close(sock_fd); + conn->err_code = CO_ER_SOCK_ERR; + conn->flags |= CO_FL_ERROR; + + return SF_ERR_INTERNAL; + } + if (conn->flags & CO_FL_OPT_MARK) sock_set_mark(sock_fd, conn->ctrl->fam->sock_family, conn->mark); if (conn->flags & CO_FL_OPT_TOS) sock_set_tos(sock_fd, conn->dst, conn->tos); - end: return sock_fd; }