MEDIUM: conn/muxes/ssl: reinsert BE priv conn into sess on IO completion

When dealing with input/output on a connection related handler, special
care must be taken prior to access the connection if it is considered as
idle, as it could be manipulated by another thread. Thus, connection is
first removed from its idle tree before processing. The connection is
reinserted on processing completion unless it has been freed during it.

Idle private connections are not concerned by this, because takeover is
not applied on them. However, a future patch will implement purging of
these connections along with regular idle ones. As such, it is necessary
to also protect private connections usage now. This is the subject of
this patch and the next one.

With this patch, input/output handlers epilogue of
muxes/SSL/conn_notify_mux() are adjusted. A new code path is able to
deal with a connection attached to a session instead of a server. In
this case, session_reinsert_idle_conn() is used. Contrary to
session_add_conn(), this new function is reserved for idle connections
usage after a temporary removal.

Contrary to _srv_add_idle() used by regular idle connections,
session_reinsert_idle_conn() may fail as an allocation can be required.
If this happens, the connection is immediately destroyed.

This patch has no effect for now. It must be coupled with the next one
which will temporarily remove private idle connections on input/output
handler prologue.

diff --git a/include/haproxy/session.h b/include/haproxy/session.h
index 7a8a733615358601072cc37c8b6086110fc75739..ffa750965034435e1055bfcbd035614fa503992b 100644 (file)
--- a/include/haproxy/session.h
+++ b/include/haproxy/session.h
@@ -44,6 +44,7 @@ void __session_add_glitch_ctr(struct session *sess, uint inc);
 void session_embryonic_build_legacy_err(struct session *sess, struct buffer *out);
 
 int session_add_conn(struct session *sess, struct connection *conn);
+int session_reinsert_idle_conn(struct session *sess, struct connection *conn);
 int session_check_idle_conn(struct session *sess, struct connection *conn);
 struct connection *session_get_conn(struct session *sess, void *target, int64_t hash);
 void session_unown_conn(struct session *sess, struct connection *conn);

diff --git a/src/connection.c b/src/connection.c
index 6dbca2c12f1deff0afd5b4ebd5c96612ec0f5256..0299532b34b923c1b2df4d69a5b45c20f9e9e63a 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -213,16 +213,25 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake)
                        goto done;
 
                if (conn_in_list) {
-                       if (srv->cur_admin & SRV_ADMF_MAINT) {
+                       if (srv && (srv->cur_admin & SRV_ADMF_MAINT)) {
                                /* Do not store an idle conn if server in maintenance. */
                                conn->mux->destroy(conn->ctx);
                                ret = -1;
                                goto done;
                        }
 
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       conn->mux->destroy(conn->ctx);
+                                       ret = -1;
+                               }
+                       }
+                       else {
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
        }
  done:

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index c0778f44c4a33a65380f2ca9516d85cd9d96ffc6..7afbaf3e7440e833f2e576723b7fa4e6d3842b11 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -3089,12 +3089,20 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
                t = NULL;
 
        if (!ret && conn_in_list) {
-               struct server *srv = __objt_server(conn->target);
+               struct server *srv = objt_server(conn->target);
 
-               if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       goto release;
+                               }
+                       }
+                       else {
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
                else {
                        /* Do not store an idle conn if server in maintenance. */

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 86ca866b4b84e262da39fe89d817d12cac761b65..bf17dfcb6efc7a8c2e34ba1d0f1f1309679a2dad 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -4334,12 +4334,20 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
                t = NULL;
 
        if (!ret && conn_in_list) {
-               struct server *srv = __objt_server(conn->target);
+               struct server *srv = objt_server(conn->target);
 
-               if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       goto release;
+                               }
+                       }
+                       else {
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
                else {
                        /* Do not store an idle conn if server in maintenance. */

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 57fea0b5a2c286c6f4d6114b289a34898d21df1f..84f30469d5777cf02d6580dbc5eea8bd1a4d1d9f 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -4993,12 +4993,20 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
                h2c->next_tasklet = NULL;
 
        if (!ret && conn_in_list) {
-               struct server *srv = __objt_server(conn->target);
+               struct server *srv = objt_server(conn->target);
 
-               if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       goto release;
+                               }
+                       }
+                       else {
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
                else {
                        /* Do not store an idle conn if server in maintenance. */

diff --git a/src/mux_spop.c b/src/mux_spop.c
index 4ba8dabcad3563047194fdd3cba3ae2db8ea97a0..d167e522da2627b0b8537ae785f42527c3f77a1f 100644 (file)
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -2585,12 +2585,20 @@ static struct task *spop_io_cb(struct task *t, void *ctx, unsigned int state)
                t = NULL;
 
        if (!ret && conn_in_list) {
-               struct server *srv = __objt_server(conn->target);
+               struct server *srv = objt_server(conn->target);
 
-               if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       goto release;
+                               }
+                       }
+                       else {
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
                else {
                        /* Do not store an idle conn if server in maintenance. */

diff --git a/src/session.c b/src/session.c
index 89f81fde8073eba8d350a97fa0eb511f1b549dbf..803a60d2d472c963a4a9d55b5d23b9fa7e83f7f7 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -697,6 +697,38 @@ int session_add_conn(struct session *sess, struct connection *conn)
        return ret;
 }
 
+/* Reinsert a backend connection <conn> into <sess> session. This function is
+ * reserved for idle conns which were previously temporarily removed from
+ * session to protect it against other threads.
+ *
+ * Returns true on success else false.
+ */
+int session_reinsert_idle_conn(struct session *sess, struct connection *conn)
+{
+       struct sess_priv_conns *pconns;
+       int ret = 0;
+
+       /* This function is reserved for idle private connections. */
+       BUG_ON(!(conn->flags & CO_FL_SESS_IDLE));
+
+       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+
+       pconns = sess_get_sess_conns(sess, conn->target);
+       if (!pconns) {
+               pconns = sess_alloc_sess_conns(sess, conn->target);
+               if (!pconns)
+                       goto out;
+       }
+
+       LIST_APPEND(&pconns->conn_list, &conn->sess_el);
+       ++sess->idle_conns;
+       ret = 1;
+
+ out:
+       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+       return ret;
+}
+
 /* Check that session <sess> is able to keep idle connection <conn>. This must
  * be called each time a connection stored in a session becomes idle.
  *

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 9342bd6d124fdab79fa99b8299aa4c1d940765b3..fffa1802c5b59e0024fa905c19515d44c690e750 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6478,19 +6478,29 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
 #endif
 leave:
        if (!ret && conn_in_list) {
-               struct server *srv = __objt_server(conn->target);
-
-               if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
-                       TRACE_DEVEL("adding conn back to idle list", SSL_EV_CONN_IO_CB, conn);
-                       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-                       _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
-                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               struct server *srv = objt_server(conn->target);
+
+               /* Connection is idle which means MUX layer is already initialized. */
+               BUG_ON(!conn->mux);
+
+               if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
+                       if (conn->flags & CO_FL_SESS_IDLE) {
+                               TRACE_DEVEL("adding conn back to session list", SSL_EV_CONN_IO_CB, conn);
+                               if (!session_reinsert_idle_conn(conn->owner, conn)) {
+                                       /* session add conn failure */
+                                       conn->mux->destroy(conn->ctx);
+                                       t = NULL;
+                               }
+                       }
+                       else {
+                               TRACE_DEVEL("adding conn back to idle list", SSL_EV_CONN_IO_CB, conn);
+                               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                               _srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
+                               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       }
                }
                else {
                        /* Do not store an idle conn if server in maintenance. */
-
-                       /* Connection is idle which means MUX layer is already initialized. */
-                       BUG_ON(!conn->mux);
                        conn->mux->destroy(conn->ctx);
                        t = NULL;
                }