From 73fd12e928918ae2291a9483ebb04dc8272358bb Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 18 Aug 2025 15:19:18 +0200 Subject: [PATCH] MEDIUM: conn/muxes/ssl: remove BE priv idle conn from sess on IO This is a direct follow-up of previous patch which adjust idle private connections access via input/output handlers. This patch implement the handlers prologue part. Now, private idle connections require a similar treatment with non-private idle connections. Thus, private conns are removed temporarily from its session under protection of idle_conns lock. As locking usage is already performed in input/output handler, session_unown_conn() cannot be called. Thus, a new function session_detach_idle_conn() is implemented in session module, which performs basically the same operation but relies on external locking. --- include/haproxy/session.h | 1 + src/connection.c | 10 ++++++++-- src/mux_fcgi.c | 15 ++++++++++++--- src/mux_h1.c | 13 ++++++++++--- src/mux_h2.c | 15 ++++++++++++--- src/mux_spop.c | 15 ++++++++++++--- src/session.c | 37 +++++++++++++++++++++++++++++++++++++ src/ssl_sock.c | 18 +++++++++++++++--- 8 files changed, 107 insertions(+), 17 deletions(-) diff --git a/include/haproxy/session.h b/include/haproxy/session.h index ffa750965..7014a8637 100644 --- a/include/haproxy/session.h +++ b/include/haproxy/session.h @@ -48,6 +48,7 @@ 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); +int session_detach_idle_conn(struct session *sess, struct connection *conn); /* Remove the refcount from the session to the tracked counters, and clear the * pointer to ensure this is only performed once. The caller is responsible for diff --git a/src/connection.c b/src/connection.c index 0299532b3..d1faf6f2a 100644 --- a/src/connection.c +++ b/src/connection.c @@ -199,12 +199,18 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake) ((conn->flags ^ old_flags) & CO_FL_NOTIFY_DONE) || ((old_flags & CO_FL_WAIT_XPRT) && !(conn->flags & CO_FL_WAIT_XPRT))) && conn->mux && conn->mux->wake) { - uint conn_in_list = conn->flags & CO_FL_LIST_MASK; + uint conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); struct server *srv = objt_server(conn->target); if (conn_in_list) { HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); - conn_delete_from_tree(conn); + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c index 7afbaf3e7..09c5f6156 100644 --- a/src/mux_fcgi.c +++ b/src/mux_fcgi.c @@ -3061,9 +3061,16 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state) conn = fconn->conn; TRACE_POINT(FCGI_EV_FCONN_WAKE, conn); - conn_in_list = conn->flags & CO_FL_LIST_MASK; - if (conn_in_list) - conn_delete_from_tree(conn); + conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); + if (conn_in_list) { + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } + } HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } else { @@ -3322,6 +3329,8 @@ struct task *fcgi_timeout_task(struct task *t, void *context, unsigned int state */ if (fconn->conn->flags & CO_FL_LIST_MASK) conn_delete_from_tree(fconn->conn); + else if (fconn->conn->flags & CO_FL_SESS_IDLE) + session_detach_idle_conn(fconn->conn->owner, fconn->conn); HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); diff --git a/src/mux_h1.c b/src/mux_h1.c index bf17dfcb6..a12535a73 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -4306,9 +4306,16 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state) /* Remove the connection from the list, to be sure nobody attempts * to use it while we handle the I/O events */ - conn_in_list = conn->flags & CO_FL_LIST_MASK; - if (conn_in_list) - conn_delete_from_tree(conn); + conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); + if (conn_in_list) { + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } + } HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } else { diff --git a/src/mux_h2.c b/src/mux_h2.c index 84f30469d..e98ecd21e 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -4962,9 +4962,16 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state) /* Remove the connection from the list, to be sure nobody attempts * to use it while we handle the I/O events */ - conn_in_list = conn->flags & CO_FL_LIST_MASK; - if (conn_in_list) - conn_delete_from_tree(conn); + conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); + if (conn_in_list) { + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } + } HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } else { @@ -5245,6 +5252,8 @@ struct task *h2_timeout_task(struct task *t, void *context, unsigned int state) */ if (h2c->conn->flags & CO_FL_LIST_MASK) conn_delete_from_tree(h2c->conn); + else if (h2c->conn->flags & CO_FL_SESS_IDLE) + session_detach_idle_conn(h2c->conn->owner, h2c->conn); HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); diff --git a/src/mux_spop.c b/src/mux_spop.c index d167e522d..37a9f35d2 100644 --- a/src/mux_spop.c +++ b/src/mux_spop.c @@ -2557,9 +2557,16 @@ static struct task *spop_io_cb(struct task *t, void *ctx, unsigned int state) conn = spop_conn->conn; TRACE_POINT(SPOP_EV_SPOP_CONN_WAKE, conn); - conn_in_list = conn->flags & CO_FL_LIST_MASK; - if (conn_in_list) - conn_delete_from_tree(conn); + conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); + if (conn_in_list) { + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } + } HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } else { @@ -2792,6 +2799,8 @@ static struct task *spop_timeout_task(struct task *t, void *context, unsigned in */ if (spop_conn->conn->flags & CO_FL_LIST_MASK) conn_delete_from_tree(spop_conn->conn); + else if (spop_conn->conn->flags & CO_FL_SESS_IDLE) + session_detach_idle_conn(spop_conn->conn->owner, spop_conn->conn); HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); diff --git a/src/session.c b/src/session.c index 803a60d2d..cad66ddc6 100644 --- a/src/session.c +++ b/src/session.c @@ -837,6 +837,43 @@ void session_unown_conn(struct session *sess, struct connection *conn) HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } +/* Remove connection from session. Contrary to + * session_unown_conn(), this function is not protected by a lock, so the + * caller is responsible to properly use idle_conns_lock prior to calling it. + * + * Another notable difference is that member of is not resetted. + * This is a convenience as this function usage is generally coupled with a + * following session_reinsert_idle_conn(). + * + * Must be called with idle_conns_lock held. + * + * Returns true on connection removal, false if it was already not stored. + */ +int session_detach_idle_conn(struct session *sess, struct connection *conn) +{ + struct sess_priv_conns *pconns; + + if (!LIST_INLIST(&conn->sess_el)) + return 0; + + /* This function is reserved for idle private connections. */ + BUG_ON(!(conn->flags & CO_FL_SESS_IDLE)); + + --sess->idle_conns; + LIST_DEL_INIT(&conn->sess_el); + + pconns = sess_get_sess_conns(sess, conn->target); + BUG_ON(!pconns); /* if conn is attached to session, its sess_conn must exists. */ + if (LIST_ISEMPTY(&pconns->conn_list)) { + /* Remove sess_conn element as no connection left in it. */ + LIST_DELETE(&pconns->sess_el); + MT_LIST_DELETE(&pconns->srv_el); + pool_free(pool_head_sess_priv_conns, pconns); + } + + return 1; +} + /* * Local variables: diff --git a/src/ssl_sock.c b/src/ssl_sock.c index fffa1802c..c1dd67c53 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -6403,9 +6403,21 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state) } ctx = context; conn = ctx->conn; - conn_in_list = conn->flags & CO_FL_LIST_MASK; - if (conn_in_list) - conn_delete_from_tree(conn); + + /* Remove the connection from the list, to be sure nobody attempts + * to use it while we handle the I/O events + */ + conn_in_list = conn->flags & (CO_FL_LIST_MASK|CO_FL_SESS_IDLE); + if (conn_in_list) { + if (conn->flags & CO_FL_SESS_IDLE) { + if (!session_detach_idle_conn(conn->owner, conn)) + conn_in_list = 0; + } + else { + conn_delete_from_tree(conn); + } + } + HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock); } else { ctx = context;