MEDIUM: tcp: make tcp_connect_probe() consider ERR/HUP

Now that we know what pollers can return ERR/HUP, we can take this
into account to save one syscall: with such a poller, if neither are
reported, then we know the connection succeeded and we don't need to
go with getsockopt() nor connect() to validate this. In addition, for
the remaining cases (select() or suspected errors), we'll always go
through the extra connect() attempt and enumerate possible "in progress",
"connected" or "failed" status codes and take action solely based on
this.

This results in one saved syscall on modern pollers, only a second
connect() still being used on select() and the server's address never
being needed anymore.

Note that we cannot safely replace connect() with getsockopt() as the
latter clears the error on the socket without saving it, and health
checks rely on it for their reporting. This would be OK if the error
was saved in the connection itself.

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index b136a602bb948d317434adda5ae9e98183760046..5ed15a985d02fa066bf89487ca67d0d31888f160 100644 (file)
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -641,22 +641,12 @@ int tcp_get_dst(int fd, struct sockaddr *sa, socklen_t salen, int dir)
  * if it fails in a fatal way or needs to poll to go further, otherwise it
  * returns non-zero and removes the CO_FL_WAIT_L4_CONN flag from the connection's
  * flags. In case of error, it sets CO_FL_ERROR and leaves the error code in
- * errno. The error checking is done in two passes in order to limit the number
- * of syscalls in the normal case :
- *   - if POLL_ERR was reported by the poller, we check for a pending error on
- *     the socket before proceeding. If found, it's assigned to errno so that
- *     upper layers can see it.
- *   - otherwise connect() is used to check the connection state again, since
- *     the getsockopt return cannot reliably be used to know if the connection
- *     is still pending or ready. This one may often return an error as well,
- *     since we don't always have POLL_ERR (eg: OSX or cached events).
+ * errno.
  */
 int tcp_connect_probe(struct connection *conn)
 {
        struct sockaddr_storage *addr;
        int fd = conn->handle.fd;
-       socklen_t lskerr;
-       int skerr;
 
        if (conn->flags & CO_FL_ERROR)
                return 0;
@@ -670,24 +660,31 @@ int tcp_connect_probe(struct connection *conn)
        if (!fd_send_ready(fd))
                return 0;
 
-       /* we might be the first witness of FD_POLL_ERR. Note that FD_POLL_HUP
-        * without FD_POLL_IN also indicates a hangup without input data meaning
-        * there was no connection.
+       /* Here we have 2 cases :
+        *  - modern pollers, able to report ERR/HUP. If these ones return any
+        *    of these flags then it's likely a failure, otherwise it possibly
+        *    is a success (i.e. there may have been data received just before
+        *    the error was reported).
+        *  - select, which doesn't report these and with which it's always
+        *    necessary either to try connect() again or to check for SO_ERROR.
+        * In order to simplify everything, we double-check using connect() as
+        * soon as we meet either of these delicate situations. Note that
+        * SO_ERROR would clear the error after reporting it!
         */
-       if (fdtab[fd].ev & FD_POLL_ERR ||
-           (fdtab[fd].ev & (FD_POLL_IN|FD_POLL_HUP)) == FD_POLL_HUP) {
-               skerr = 0;
-               lskerr = sizeof(skerr);
-               getsockopt(fd, SOL_SOCKET, SO_ERROR, &skerr, &lskerr);
-               errno = skerr;
-               if (errno == EAGAIN)
-                       errno = 0;
-               if (errno)
-                       goto out_error;
-       }
-
-       /* Use connect() to check the state of the socket. This has the
-        * advantage of giving us the following info :
+       if (cur_poller.flags & HAP_POLL_F_ERRHUP) {
+               /* modern poller, able to report ERR/HUP */
+               if ((fdtab[fd].ev & (FD_POLL_IN|FD_POLL_ERR|FD_POLL_HUP)) == FD_POLL_IN)
+                       goto done;
+               if ((fdtab[fd].ev & (FD_POLL_OUT|FD_POLL_ERR|FD_POLL_HUP)) == FD_POLL_OUT)
+                       goto done;
+               if (!(fdtab[fd].ev & (FD_POLL_ERR|FD_POLL_HUP)))
+                       goto wait;
+               /* error present, fall through common error check path */
+       }
+
+       /* Use connect() to check the state of the socket. This has the double
+        * advantage of *not* clearing the error (so that health checks can
+        * still use getsockopt(SO_ERROR)) and giving us the following info :
         *  - error
         *  - connecting (EALREADY, EINPROGRESS)
         *  - connected (EISCONN, 0)
@@ -697,18 +694,14 @@ int tcp_connect_probe(struct connection *conn)
                addr = &objt_server(conn->target)->socks4_addr;
 
        if (connect(fd, (const struct sockaddr *)addr, get_addr_len(addr)) == -1) {
-               if (errno == EALREADY || errno == EINPROGRESS) {
-                       __conn_xprt_want_send(conn);
-                       fd_cant_send(fd);
-                       return 0;
-               }
+               if (errno == EALREADY || errno == EINPROGRESS)
+                       goto wait;
 
                if (errno && errno != EISCONN)
                        goto out_error;
-
-               /* otherwise we're connected */
        }
 
+ done:
        /* The FD is ready now, we'll mark the connection as complete and
         * forward the event to the transport layer which will notify the
         * data layer.
@@ -716,6 +709,7 @@ int tcp_connect_probe(struct connection *conn)
        conn->flags &= ~CO_FL_WAIT_L4_CONN;
        fd_may_send(fd);
        fd_cond_recv(fd);
+       errno = 0; // make health checks happy
        return 1;
 
  out_error:
@@ -726,6 +720,11 @@ int tcp_connect_probe(struct connection *conn)
        conn->flags |= CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH;
        __conn_xprt_stop_both(conn);
        return 0;
+
+ wait:
+       __conn_xprt_want_send(conn);
+       fd_cant_send(fd);
+       return 0;
 }
 
 /* XXX: Should probably be elsewhere */