From: Willy Tarreau Date: Mon, 10 Jun 2013 17:56:38 +0000 (+0200) Subject: MEDIUM: protocol: implement a "drain" function in protocol layers X-Git-Tag: v1.5-dev19~20 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2b57cb8f30208e4d1d003590e141c9bef03bb279;p=thirdparty%2Fhaproxy.git MEDIUM: protocol: implement a "drain" function in protocol layers Since commit cfd97c6f was merged into 1.5-dev14 (BUG/MEDIUM: checks: prevent TIME_WAITs from appearing also on timeouts), some valid health checks sometimes used to show some TCP resets. For example, this HTTP health check sent to a local server : 19:55:15.742818 IP 127.0.0.1.16568 > 127.0.0.1.8000: S 3355859679:3355859679(0) win 32792 19:55:15.742841 IP 127.0.0.1.8000 > 127.0.0.1.16568: S 1060952566:1060952566(0) ack 3355859680 win 32792 19:55:15.742863 IP 127.0.0.1.16568 > 127.0.0.1.8000: . ack 1 win 257 19:55:15.745402 IP 127.0.0.1.16568 > 127.0.0.1.8000: P 1:23(22) ack 1 win 257 19:55:15.745488 IP 127.0.0.1.8000 > 127.0.0.1.16568: FP 1:146(145) ack 23 win 257 19:55:15.747109 IP 127.0.0.1.16568 > 127.0.0.1.8000: R 23:23(0) ack 147 win 257 After some discussion with Chris Huang-Leaver, it appeared clear that what we want is to only send the RST when we have no other choice, which means when the server has not closed. So we still keep SYN/SYN-ACK/RST for pure TCP checks, but don't want to see an RST emitted as above when the server has already sent the FIN. The solution against this consists in implementing a "drain" function at the protocol layer, which, when defined, causes as much as possible of the input socket buffer to be flushed to make recv() return zero so that we know that the server's FIN was received and ACKed. On Linux, we can make use of MSG_TRUNC on TCP sockets, which has the benefit of draining everything at once without even copying data. On other platforms, we read up to one buffer of data before the close. If recv() manages to get the final zero, we don't disable lingering. Same for hard errors. Otherwise we do. In practice, on HTTP health checks we generally find that the close was pending and is returned upon first recv() call. The network trace becomes cleaner : 19:55:23.650621 IP 127.0.0.1.16561 > 127.0.0.1.8000: S 3982804816:3982804816(0) win 32792 19:55:23.650644 IP 127.0.0.1.8000 > 127.0.0.1.16561: S 4082139313:4082139313(0) ack 3982804817 win 32792 19:55:23.650666 IP 127.0.0.1.16561 > 127.0.0.1.8000: . ack 1 win 257 19:55:23.651615 IP 127.0.0.1.16561 > 127.0.0.1.8000: P 1:23(22) ack 1 win 257 19:55:23.651696 IP 127.0.0.1.8000 > 127.0.0.1.16561: FP 1:146(145) ack 23 win 257 19:55:23.652628 IP 127.0.0.1.16561 > 127.0.0.1.8000: F 23:23(0) ack 147 win 257 19:55:23.652655 IP 127.0.0.1.8000 > 127.0.0.1.16561: . ack 24 win 257 This change should be backported to 1.4 which is where Chris encountered this issue. The code is different, so probably the tcp_drain() function will have to be put in the checks only. --- diff --git a/include/common/compat.h b/include/common/compat.h index 0085a3aa1b..a0764b103a 100644 --- a/include/common/compat.h +++ b/include/common/compat.h @@ -64,6 +64,13 @@ #define MSG_MORE 0 #endif +/* On Linux 2.4 and above, MSG_TRUNC can be used on TCP sockets to drop any + * pending data. Let's rely on NETFILTER to detect if this is supported. + */ +#ifdef NETFILTER +#define MSG_TRUNC_CLEARS_INPUT +#endif + /* Maximum path length, OS-dependant */ #ifndef MAXPATHLEN #define MAXPATHLEN 128 diff --git a/include/proto/proto_tcp.h b/include/proto/proto_tcp.h index 20ae2df71d..07127648f6 100644 --- a/include/proto/proto_tcp.h +++ b/include/proto/proto_tcp.h @@ -34,6 +34,7 @@ int tcp_connect_server(struct connection *conn, int data, int delack); int tcp_connect_probe(struct connection *conn); int tcp_get_src(int fd, struct sockaddr *sa, socklen_t salen, int dir); int tcp_get_dst(int fd, struct sockaddr *sa, socklen_t salen, int dir); +int tcp_drain(int fd); int tcp_inspect_request(struct session *s, struct channel *req, int an_bit); int tcp_inspect_response(struct session *s, struct channel *rep, int an_bit); int tcp_exec_req_rules(struct session *s); diff --git a/include/types/protocol.h b/include/types/protocol.h index 0af2ed8a35..e03692a356 100644 --- a/include/types/protocol.h +++ b/include/types/protocol.h @@ -59,6 +59,7 @@ struct protocol { int (*connect)(struct connection *, int data, int delack); /* connect function if any */ int (*get_src)(int fd, struct sockaddr *, socklen_t, int dir); /* syscall used to retrieve src addr */ int (*get_dst)(int fd, struct sockaddr *, socklen_t, int dir); /* syscall used to retrieve dst addr */ + int (*drain)(int fd); /* indicates whether we can safely close the fd */ struct list listeners; /* list of listeners using this protocol */ int nb_listeners; /* number of listeners */ diff --git a/src/checks.c b/src/checks.c index d1b7a367d4..315ef7a832 100644 --- a/src/checks.c +++ b/src/checks.c @@ -1210,11 +1210,11 @@ static void event_srv_chk_r(struct connection *conn) */ if (conn->xprt && conn->xprt->shutw) conn->xprt->shutw(conn, 0); - if (conn->ctrl) { - if (!(conn->flags & CO_FL_WAIT_RD)) - recv(conn->t.sock.fd, trash.str, trash.size, MSG_NOSIGNAL|MSG_DONTWAIT); - setsockopt(conn->t.sock.fd, SOL_SOCKET, SO_LINGER, - (struct linger *) &nolinger, sizeof(struct linger)); + + if (conn->ctrl && !(conn->flags & CO_FL_SOCK_RD_SH)) { + if (conn->flags & CO_FL_WAIT_RD || !conn->ctrl->drain || !conn->ctrl->drain(conn->t.sock.fd)) + setsockopt(conn->t.sock.fd, SOL_SOCKET, SO_LINGER, + (struct linger *) &nolinger, sizeof(struct linger)); } __conn_data_stop_both(conn); task_wakeup(t, TASK_WOKEN_IO); diff --git a/src/proto_tcp.c b/src/proto_tcp.c index e1b5d8b9ee..049988c4e1 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -77,6 +77,7 @@ static struct protocol proto_tcpv4 = { .enable_all = enable_all_listeners, .get_src = tcp_get_src, .get_dst = tcp_get_dst, + .drain = tcp_drain, .listeners = LIST_HEAD_INIT(proto_tcpv4.listeners), .nb_listeners = 0, }; @@ -98,6 +99,7 @@ static struct protocol proto_tcpv6 = { .enable_all = enable_all_listeners, .get_src = tcp_get_src, .get_dst = tcp_get_dst, + .drain = tcp_drain, .listeners = LIST_HEAD_INIT(proto_tcpv6.listeners), .nb_listeners = 0, }; @@ -513,6 +515,41 @@ int tcp_get_dst(int fd, struct sockaddr *sa, socklen_t salen, int dir) return getsockname(fd, sa, &salen); } +/* Tries to drain any pending incoming data from the socket to reach the + * receive shutdown. Returns non-zero if the shutdown was found, otherwise + * zero. This is useful to decide whether we can close a connection cleanly + * are we must kill it hard. + */ +int tcp_drain(int fd) +{ + int turns = 2; + int len; + + while (turns) { +#ifdef MSG_TRUNC_CLEARS_INPUT + len = recv(fd, NULL, INT_MAX, MSG_DONTWAIT | MSG_NOSIGNAL | MSG_TRUNC); + if (len == -1 && errno == EFAULT) +#endif + len = recv(fd, trash.str, trash.size, MSG_DONTWAIT | MSG_NOSIGNAL); + + if (len == 0) /* cool, shutdown received */ + return 1; + + if (len < 0) { + if (errno == EAGAIN) /* connection not closed yet */ + return 0; + if (errno == EINTR) /* oops, try again */ + continue; + /* other errors indicate a dead connection, fine. */ + return 1; + } + /* OK we read some data, let's try again once */ + turns--; + } + /* some data are still present, give up */ + return 0; +} + /* This is the callback which is set when a connection establishment is pending * and we have nothing to send, or if we have an init function we want to call * once the connection is established. It updates the FD polling status. It diff --git a/src/session.c b/src/session.c index adcfd4e927..7309af00b8 100644 --- a/src/session.c +++ b/src/session.c @@ -157,7 +157,8 @@ int session_accept(struct listener *l, int cfd, struct sockaddr_storage *addr) * - HEALTH mode without HTTP check => just send "OK" * - TCP mode from monitoring address => just close */ - recv(cfd, trash.str, trash.size, MSG_DONTWAIT); + if (l->proto->drain) + l->proto->drain(cfd); if (p->mode == PR_MODE_HTTP || (p->mode == PR_MODE_HEALTH && (p->options2 & PR_O2_CHK_ANY) == PR_O2_HTTP_CHK)) send(cfd, "HTTP/1.0 200 OK\r\n\r\n", 19, MSG_DONTWAIT|MSG_NOSIGNAL|MSG_MORE); diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 8a39dade31..7523246e06 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1096,7 +1096,8 @@ int ssl_sock_handshake(struct connection *conn, unsigned int flag) * TCP sockets. We first try to drain possibly pending * data to avoid this as much as possible. */ - ret = recv(conn->t.sock.fd, trash.str, trash.size, MSG_NOSIGNAL|MSG_DONTWAIT); + if (conn->ctrl && conn->ctrl->drain) + conn->ctrl->drain(conn->t.sock.fd); if (!conn->err_code) conn->err_code = CO_ER_SSL_HANDSHAKE; goto out_error; @@ -1146,7 +1147,8 @@ int ssl_sock_handshake(struct connection *conn, unsigned int flag) * TCP sockets. We first try to drain possibly pending * data to avoid this as much as possible. */ - ret = recv(conn->t.sock.fd, trash.str, trash.size, MSG_NOSIGNAL|MSG_DONTWAIT); + if (conn->ctrl && conn->ctrl->drain) + conn->ctrl->drain(conn->t.sock.fd); if (!conn->err_code) conn->err_code = CO_ER_SSL_HANDSHAKE; goto out_error;