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.

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);

src/connection.c

@@ -213,18 +213,27 @@ 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;
 			}
 
+			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:
 	return ret;
 }

src/mux_fcgi.c

@@ -3089,13 +3089,21 @@ 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)) {
+		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. */
 			goto release;

src/mux_h1.c

@@ -4334,13 +4334,21 @@ 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)) {
+		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. */
 			goto release;

src/mux_h2.c

@@ -4993,13 +4993,21 @@ 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)) {
+		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. */
 			goto release;

src/mux_spop.c

@@ -2585,13 +2585,21 @@ 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)) {
+		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. */
 			goto release;

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.
  *

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);
+		struct server *srv = objt_server(conn->target);
 
-		if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
+		/* 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;
 		}