]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: connection: get rid of CO_FL_CURR_* flags
authorWilly Tarreau <w@1wt.eu>
Fri, 17 Jan 2020 16:39:35 +0000 (17:39 +0100)
committerWilly Tarreau <w@1wt.eu>
Fri, 17 Jan 2020 16:45:12 +0000 (17:45 +0100)
These ones used to serve as a set of switches between CO_FL_SOCK_* and
CO_FL_XPRT_*, and now that the SOCK layer is gone, they're always a
copy of the last know CO_FL_XPRT_* ones that is resynchronized before
I/O events by calling conn_refresh_polling_flags(), and that are pushed
back to FDs when detecting changes with conn_xprt_polling_changes().

While these functions are not particularly heavy, what they do is
totally redundant by now because the fd_want_*/fd_stop_*() actions
already perform test-and-set operations to decide to create an entry
or not, so they do the exact same thing that is done by
conn_xprt_polling_changes(). As such it is pointless to call that
one, and given that the only reason to keep CO_FL_CURR_* is to detect
changes there, we can now remove them.

Even if this does only save very few cycles, this removes a significant
complexity that has been responsible for many bugs in the past, including
the last one affecting FreeBSD.

All tests look good, and no performance regressions were observed.

contrib/debug/flags.c
include/proto/connection.h
include/types/connection.h
src/connection.c
src/raw_sock.c
src/ssl_sock.c

index 08ca054f9cff33d8b683568ba514522cc84ca6e0..585dea625577e49a4fda825ff0935a200e0bbb9a 100644 (file)
@@ -139,9 +139,7 @@ void show_conn_flags(unsigned int f)
        SHOW_FLAG(f, CO_FL_WILL_UPDATE);
        SHOW_FLAG(f, CO_FL_XPRT_READY);
        SHOW_FLAG(f, CO_FL_CTRL_READY);
-       SHOW_FLAG(f, CO_FL_CURR_WR_ENA);
        SHOW_FLAG(f, CO_FL_XPRT_WR_ENA);
-       SHOW_FLAG(f, CO_FL_CURR_RD_ENA);
        SHOW_FLAG(f, CO_FL_XPRT_RD_ENA);
 
        if (f) {
index cd6e62d8d34f9bcb3c2f1579b99cc87c6fc76d0c..dd3782273eec91a35dc2df57c5e23bdf0a6dfbac 100644 (file)
@@ -161,55 +161,12 @@ static inline void conn_stop_tracking(struct connection *conn)
 }
 
 /* Update polling on connection <c>'s file descriptor depending on its current
- * state as reported in the connection's CO_FL_CURR_* flags, reports of EAGAIN
- * in CO_FL_WAIT_*, and the upper layer expectations indicated by CO_FL_XPRT_*.
- * The connection flags are updated with the new flags at the end of the
- * operation. Polling is totally disabled if an error was reported.
+ * state as reported in the connection's CO_FL_XPRT_* flags. The connection
+ * flags are updated with the new flags at the end of the operation. Polling
+ * is totally disabled if an error was reported.
  */
 void conn_update_xprt_polling(struct connection *c);
 
-/* Refresh the connection's polling flags from its file descriptor status.
- * This should be called at the beginning of a connection handler. It does
- * nothing if CO_FL_WILL_UPDATE is present, indicating that an upper caller
- * has already done it.
- */
-static inline void conn_refresh_polling_flags(struct connection *conn)
-{
-       if (conn_ctrl_ready(conn) && !(conn->flags & CO_FL_WILL_UPDATE)) {
-               unsigned int flags = conn->flags;
-
-               flags &= ~(CO_FL_CURR_RD_ENA | CO_FL_CURR_WR_ENA);
-               if (fd_recv_active(conn->handle.fd))
-                       flags |= CO_FL_CURR_RD_ENA;
-               if (fd_send_active(conn->handle.fd))
-                       flags |= CO_FL_CURR_WR_ENA;
-               conn->flags = flags;
-       }
-}
-
-/* inspects c->flags and returns non-zero if XPRT ENA changes from the CURR ENA
- * or if the WAIT flags are set with their respective ENA flags. Additionally,
- * non-zero is also returned if an error was reported on the connection. This
- * function is used quite often and is inlined. In order to proceed optimally
- * with very little code and CPU cycles, the bits are arranged so that a change
- * can be detected by a few left shifts, a xor, and a mask. These operations
- * detect when W&D are both enabled for either direction, when C&D differ for
- * either direction and when Error is set. The trick consists in first keeping
- * only the bits we're interested in, since they don't collide when shifted,
- * and to perform the AND at the end. In practice, the compiler is able to
- * replace the last AND with a TEST in boolean conditions. This results in
- * checks that are done in 4-6 cycles and less than 30 bytes.
- */
-static inline unsigned int conn_xprt_polling_changes(const struct connection *c)
-{
-       unsigned int f = c->flags;
-       f &= CO_FL_XPRT_WR_ENA | CO_FL_XPRT_RD_ENA | CO_FL_CURR_WR_ENA |
-            CO_FL_CURR_RD_ENA | CO_FL_ERROR;
-
-       f = (f ^ (f << 1)) & (CO_FL_CURR_WR_ENA|CO_FL_CURR_RD_ENA);    /* test C ^ D */
-       return f & (CO_FL_CURR_WR_ENA | CO_FL_CURR_RD_ENA | CO_FL_ERROR);
-}
-
 /* Automatically updates polling on connection <c> depending on the XPRT flags.
  * It does nothing if CO_FL_WILL_UPDATE is present, indicating that an upper
  * caller is going to do it again later.
@@ -217,8 +174,7 @@ static inline unsigned int conn_xprt_polling_changes(const struct connection *c)
 static inline void conn_cond_update_xprt_polling(struct connection *c)
 {
        if (!(c->flags & CO_FL_WILL_UPDATE))
-               if (conn_xprt_polling_changes(c))
-                       conn_update_xprt_polling(c);
+               conn_update_xprt_polling(c);
 }
 
 /* Stop all polling on the fd. This might be used when an error is encountered
@@ -228,8 +184,7 @@ static inline void conn_cond_update_xprt_polling(struct connection *c)
  */
 static inline void conn_stop_polling(struct connection *c)
 {
-       c->flags &= ~(CO_FL_CURR_RD_ENA | CO_FL_CURR_WR_ENA |
-                     CO_FL_XPRT_RD_ENA | CO_FL_XPRT_WR_ENA);
+       c->flags &= ~(CO_FL_XPRT_RD_ENA | CO_FL_XPRT_WR_ENA);
        if (!(c->flags & CO_FL_WILL_UPDATE) && conn_ctrl_ready(c))
                fd_stop_both(c->handle.fd);
 }
@@ -245,10 +200,8 @@ static inline void conn_cond_update_polling(struct connection *c)
 {
        if (unlikely(c->flags & CO_FL_ERROR))
                conn_stop_polling(c);
-       else if (!(c->flags & CO_FL_WILL_UPDATE)) {
-               if (conn_xprt_polling_changes(c))
-                       conn_update_xprt_polling(c);
-       }
+       else if (!(c->flags & CO_FL_WILL_UPDATE))
+               conn_update_xprt_polling(c);
 }
 
 /***** Event manipulation primitives for use by DATA I/O callbacks *****/
@@ -333,7 +286,6 @@ static inline void conn_sock_read0(struct connection *c)
 static inline void conn_sock_shutw(struct connection *c, int clean)
 {
        c->flags |= CO_FL_SOCK_WR_SH;
-       conn_refresh_polling_flags(c);
        __conn_xprt_stop_send(c);
        conn_cond_update_xprt_polling(c);
 
index 3f9fb3cf992ad386cc96ef83cd5ac6e12610a571..d5852aae11601b0e914c5d474267b90c461c9100 100644 (file)
@@ -118,17 +118,16 @@ enum cs_shw_mode {
        CS_SHW_SILENT       = 1,           /* imminent close, don't notify peer */
 };
 
-/* For each direction, we have a CO_FL_{SOCK,DATA}_<DIR>_ENA flag, which
+/* For each direction, we have a CO_FL_XPRT_<DIR>_ENA flag, which
  * indicates if read or write is desired in that direction for the respective
  * layers. The current status corresponding to the current layer being used is
- * remembered in the CO_FL_CURR_<DIR>_ENA flag. The need to poll (ie receipt of
+ * remembered in the CO_FL_XPRT_<DIR>_ENA flag. The need to poll (ie receipt of
  * EAGAIN) is remembered at the file descriptor level so that even when the
  * activity is stopped and restarted, we still remember whether it was needed
  * to poll before attempting the I/O.
  *
- * The CO_FL_CURR_<DIR>_ENA flag is set from the FD status in
- * conn_refresh_polling_flags(). The FD state is updated according to these
- * flags in conn_cond_update_polling().
+ * The FD state is updated according to CO_FL_XPRT_<DIR>_ENA in
+ * conn_cond_update_polling().
  */
 
 /* flags for use in connection->flags */
@@ -136,15 +135,13 @@ enum {
        CO_FL_NONE          = 0x00000000,  /* Just for initialization purposes */
 
        /* Do not change these values without updating conn_*_poll_changes() ! */
-       /* unusued : 0x00000001 */
+       /* unused : 0x00000001 */
        CO_FL_XPRT_RD_ENA   = 0x00000002,  /* receiving data is allowed */
-       CO_FL_CURR_RD_ENA   = 0x00000004,  /* receiving is currently allowed */
-       /* unused : 0x00000008 */
+       /* unused : 0x00000004, 0x00000008 */
 
        /* unused : 0x00000010 */
        CO_FL_XPRT_WR_ENA   = 0x00000020,  /* sending data is desired */
-       CO_FL_CURR_WR_ENA   = 0x00000040,  /* sending is currently desired */
-       /* unused : 0x00000080 */
+       /* unused : 0x00000040, 0x00000080 */
 
        /* These flags indicate whether the Control and Transport layers are initialized */
        CO_FL_CTRL_READY    = 0x00000100, /* FD was registered, fd_delete() needed */
index 3337538c8d729ae824de66ca48778c2e369a54f6..bf55f867f7b64c0524e5e678f9737d236237451d 100644 (file)
@@ -55,7 +55,6 @@ void conn_fd_handler(int fd)
                return;
        }
 
-       conn_refresh_polling_flags(conn);
        conn->flags |= CO_FL_WILL_UPDATE;
 
        flags = conn->flags & ~CO_FL_ERROR; /* ensure to call the wake handler upon error */
@@ -150,10 +149,9 @@ void conn_fd_handler(int fd)
 }
 
 /* Update polling on connection <c>'s file descriptor depending on its current
- * state as reported in the connection's CO_FL_CURR_* flags, reports of EAGAIN
- * in CO_FL_WAIT_*, and the data layer expectations indicated by CO_FL_XPRT_*.
- * The connection flags are updated with the new flags at the end of the
- * operation. Polling is totally disabled if an error was reported.
+ * state as reported in the connection's CO_FL_XPRT_* flags. The connection
+ * flags are updated with the new flags at the end of the operation. Polling
+ * is totally disabled if an error was reported.
  */
 void conn_update_xprt_polling(struct connection *c)
 {
@@ -163,25 +161,16 @@ void conn_update_xprt_polling(struct connection *c)
                return;
 
        /* update read status if needed */
-       if (unlikely((f & (CO_FL_CURR_RD_ENA|CO_FL_XPRT_RD_ENA)) == CO_FL_XPRT_RD_ENA)) {
+       if (f & CO_FL_XPRT_RD_ENA)
                fd_want_recv(c->handle.fd);
-               f |= CO_FL_CURR_RD_ENA;
-       }
-       else if (unlikely((f & (CO_FL_CURR_RD_ENA|CO_FL_XPRT_RD_ENA)) == CO_FL_CURR_RD_ENA)) {
+       else
                fd_stop_recv(c->handle.fd);
-               f &= ~CO_FL_CURR_RD_ENA;
-       }
 
        /* update write status if needed */
-       if (unlikely((f & (CO_FL_CURR_WR_ENA|CO_FL_XPRT_WR_ENA)) == CO_FL_XPRT_WR_ENA)) {
+       if (f & CO_FL_XPRT_WR_ENA)
                fd_want_send(c->handle.fd);
-               f |= CO_FL_CURR_WR_ENA;
-       }
-       else if (unlikely((f & (CO_FL_CURR_WR_ENA|CO_FL_XPRT_WR_ENA)) == CO_FL_CURR_WR_ENA)) {
+       else
                fd_stop_send(c->handle.fd);
-               f &= ~CO_FL_CURR_WR_ENA;
-       }
-       c->flags = f;
 }
 
 /* This is the callback which is set when a connection establishment is pending
index 63c8f6f48c521c1829b1d6372b7762d633102837..91a3593d97422c7cd8bfa1b69fa76056a73d2fcf 100644 (file)
@@ -73,7 +73,6 @@ int raw_sock_to_pipe(struct connection *conn, void *xprt_ctx, struct pipe *pipe,
                return 0;
 
        conn->flags &= ~CO_FL_WAIT_ROOM;
-       conn_refresh_polling_flags(conn);
        errno = 0;
 
        /* Under Linux, if FD_POLL_HUP is set, we have reached the end.
@@ -186,7 +185,6 @@ int raw_sock_from_pipe(struct connection *conn, void *xprt_ctx, struct pipe *pip
        if (!fd_send_ready(conn->handle.fd))
                return 0;
 
-       conn_refresh_polling_flags(conn);
        done = 0;
        while (pipe->data) {
                ret = splice(pipe->cons, NULL, conn->handle.fd, NULL, pipe->data,
@@ -241,7 +239,6 @@ static size_t raw_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
                return 0;
 
        conn->flags &= ~CO_FL_WAIT_ROOM;
-       conn_refresh_polling_flags(conn);
        errno = 0;
 
        if (unlikely(!(fdtab[conn->handle.fd].ev & FD_POLL_IN))) {
@@ -354,7 +351,6 @@ static size_t raw_sock_from_buf(struct connection *conn, void *xprt_ctx, const s
        if (!fd_send_ready(conn->handle.fd))
                return 0;
 
-       conn_refresh_polling_flags(conn);
        done = 0;
        /* send the largest possible block. For this we perform only one call
         * to send() unless the buffer wraps and we exactly fill the first hunk,
@@ -380,10 +376,8 @@ static size_t raw_sock_from_buf(struct connection *conn, void *xprt_ctx, const s
                        /* if the system buffer is full, don't insist */
                        if (ret < try)
                                break;
-                       if (!count) {
+                       if (!count)
                                conn_xprt_stop_send(conn);
-                               conn_refresh_polling_flags(conn);
-                       }
                }
                else if (ret == 0 || errno == EAGAIN || errno == ENOTCONN || errno == EINPROGRESS) {
                        /* nothing written, we need to poll for write first */
index ff8af23959b45f1889bbe123829687c572c06bb2..f2bce219126819b801872980d5b8f174f9d3a8d1 100644 (file)
@@ -6505,8 +6505,6 @@ static size_t ssl_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
        ssize_t ret;
        size_t try, done = 0;
 
-       conn_refresh_polling_flags(conn);
-
        if (!ctx)
                goto out_error;
 
@@ -6631,7 +6629,6 @@ static size_t ssl_sock_from_buf(struct connection *conn, void *xprt_ctx, const s
        size_t try, done;
 
        done = 0;
-       conn_refresh_polling_flags(conn);
 
        if (!ctx)
                goto out_error;