BUG/MINOR: connection: rearrange union list members

A connection can be stored in several lists, thus there is several
attach points in struct connection. Depending on its proxy side, either
frontend or backend, a single connection will only access some of them
during its lifetime.

As an optimization, these attach points are organized in a union.
However, this repartition was not correctly achieved along
frontend/backend side delimitation.

Furthermore, reverse HTTP has recently been introduced. With this
feature, a connection can migrate from frontend to backend side or vice
versa. As such, it becomes even more tedious to ensure that these
members are always accessed in a safe way.

This commit rearrange these fields. First, union is now clearly splitted
between frontend and backend only elements. Next, backend elements are
initialized with conn_backend_init(), which is already used during
connection reversal on an edge endpoint. A new function
conn_frontend_init() serves to initialize the other members, called both
on connection first instantiation and on reversal on a dialer endpoint.

This model is much cleaner and should prevent any access to fields from
the wrong side.

Currently, there is no known case of wrong access in the existing code
base. However, this cleanup is considered an improvement which must be
backported up to 3.0 to remove any possible undefined behavior.

diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index ab0231642e53c06e3df1e905807b845d63bb5d45..d216e49a846ed7104a2bd44ce87a5e22bf578c2a 100644 (file)
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -622,12 +622,14 @@ struct connection {
        /* second cache line */
        struct wait_event *subs; /* Task to wake when awaited events are ready */
        union {
-               struct list    idle_list; /* list element for idle connection in server idle list */
-               struct mt_list toremove_list; /* list element when idle connection is ready to be purged */
-       };
-       union {
-               struct list sess_el;       /* used by private backend conns, list elem into session */
-               struct list stopping_list; /* used by frontend conns, attach point in mux stopping list */
+               /* Backend connections only */
+               struct {
+                       struct mt_list toremove_list; /* list element when idle connection is ready to be purged */
+                       struct list    idle_list;     /* list element for idle connection in server idle list */
+                       struct list    sess_el;       /* used by private connections, list elem into session */
+               };
+               /* Frontend connections only */
+               struct list stopping_list; /* attach point in mux stopping list */
        };
        union conn_handle handle;     /* connection handle at the socket layer */
        const struct netns_entry *proxy_netns;

diff --git a/src/connection.c b/src/connection.c
index 9c5a0fe7aad85f6bc98c6ebac6bc6c557a06cb4d..ea4a0b27b1cf31a8c88bae9f467374d98d2708ce 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -467,11 +467,6 @@ void conn_init(struct connection *conn, void *target)
        conn->target = target;
        conn->destroy_cb = NULL;
        conn->proxy_netns = NULL;
-       MT_LIST_INIT(&conn->toremove_list);
-       if (conn_is_back(conn))
-               LIST_INIT(&conn->sess_el);
-       else
-               LIST_INIT(&conn->stopping_list);
        LIST_INIT(&conn->tlv_list);
        conn->subs = NULL;
        conn->src = NULL;
@@ -488,6 +483,10 @@ void conn_init(struct connection *conn, void *target)
  */
 static int conn_backend_init(struct connection *conn)
 {
+       LIST_INIT(&conn->idle_list);
+       LIST_INIT(&conn->sess_el);
+       MT_LIST_INIT(&conn->toremove_list);
+
        if (!sockaddr_alloc(&conn->dst, 0, 0))
                return 1;
 
@@ -498,6 +497,12 @@ static int conn_backend_init(struct connection *conn)
        return 0;
 }
 
+/* Initialize members used only on frontend connections. */
+static void conn_frontend_init(struct connection *conn)
+{
+       LIST_INIT(&conn->stopping_list);
+}
+
 /* Release connection elements reserved for backend side usage. It also takes
  * care to detach it if linked to a session or a server instance.
  *
@@ -553,6 +558,9 @@ struct connection *conn_new(void *target)
                        return NULL;
                }
        }
+       else {
+               conn_frontend_init(conn);
+       }
 
        return conn;
 }
@@ -2967,6 +2975,7 @@ int conn_reverse(struct connection *conn)
 
                conn_backend_deinit(conn);
 
+               conn_frontend_init(conn);
                conn->target = &l->obj_type;
                conn->flags |= CO_FL_ACT_REVERSING;
                task_wakeup(l->rx.rhttp.task, TASK_WOKEN_RES);