BUG/MEDIUM: tcp: don't poll for write when connect() succeeds
While testing a tcp_fastopen related change, it appeared that in the rare
case where connect() can immediately succeed, we still subscribe to write
notifications on the socket, causing the conn_fd_handler() to immediately
be called and a second call to connect() to be attempted to double-check
the connection.
In fact this issue had already been met with unix sockets (which often
respond immediately) and partially addressed but incorrect so another
patch will follow. But for TCP nothing was done.
The fix consists in removing the WAIT_L4_CONN flag if connect() succeeds
and to subscribe for writes only if some handshakes or L4_CONN are still
needed. In addition in order not to fail raw TCP health checks, we have
to continue to enable polling for data when nothing is scheduled for
leaving and the connection is already established, otherwise the caller
will never be notified.
This fix should be backported to 1.7 and 1.6.
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index d765032..4741651 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -474,10 +474,16 @@
if (global.tune.server_rcvbuf)
setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &global.tune.server_rcvbuf, sizeof(global.tune.server_rcvbuf));
- if ((connect(fd, (struct sockaddr *)&conn->addr.to, get_addr_len(&conn->addr.to)) == -1) &&
- (errno != EINPROGRESS) && (errno != EALREADY) && (errno != EISCONN)) {
-
- if (errno == EAGAIN || errno == EADDRINUSE || errno == EADDRNOTAVAIL) {
+ if (connect(fd, (struct sockaddr *)&conn->addr.to, get_addr_len(&conn->addr.to)) == -1) {
+ if (errno == EINPROGRESS || errno == EALREADY) {
+ /* common case, let's wait for connect status */
+ conn->flags |= CO_FL_WAIT_L4_CONN;
+ }
+ else if (errno == EISCONN) {
+ /* should normally not happen but if so, indicates that it's OK */
+ conn->flags &= ~CO_FL_WAIT_L4_CONN;
+ }
+ else if (errno == EAGAIN || errno == EADDRINUSE || errno == EADDRNOTAVAIL) {
char *msg;
if (errno == EAGAIN || errno == EADDRNOTAVAIL) {
msg = "no free ports";
@@ -514,6 +520,10 @@
return SF_ERR_SRVCL;
}
}
+ else {
+ /* connect() == 0, this is great! */
+ conn->flags &= ~CO_FL_WAIT_L4_CONN;
+ }
conn->flags |= CO_FL_ADDR_TO_SET;
@@ -523,7 +533,6 @@
conn_ctrl_init(conn); /* registers the FD */
fdtab[fd].linger_risk = 1; /* close hard if needed */
- conn_sock_want_send(conn); /* for connect status */
if (conn_xprt_init(conn) < 0) {
conn_force_close(conn);
@@ -531,6 +540,17 @@
return SF_ERR_RESOURCE;
}
+ if (conn->flags & (CO_FL_HANDSHAKE | CO_FL_WAIT_L4_CONN)) {
+ conn_sock_want_send(conn); /* for connect status, proxy protocol or SSL */
+ }
+ else {
+ /* If there's no more handshake, we need to notify the data
+ * layer when the connection is already OK otherwise we'll have
+ * no other opportunity to do it later (eg: health checks).
+ */
+ data = 1;
+ }
+
if (data)
conn_data_want_send(conn); /* prepare to send data if any */