From: Willy Tarreau <w@1wt.eu>
Date: Tue, 14 May 2024 16:54:20 +0000 (+0200)
Subject: BUG/MINOR: ssl_sock: fix xprt_set_used() to properly clear the TASK_F_USR1 bit
X-Git-Tag: v3.0-dev12~40
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=edb99e296d6f4e463523fd64c6cacce2e904df3d;p=thirdparty%2Fhaproxy.git

BUG/MINOR: ssl_sock: fix xprt_set_used() to properly clear the TASK_F_USR1 bit

In 2.4-dev8 with commit 5c7086f6b0 ("MEDIUM: connection: protect idle
conn lists with locks"), the idle conns list started to be protected
using the lock for takeover, and the SSL layer used to always take
that lock. Later in 2.4-dev11, with commit 4149168255 ("MEDIUM: ssl:
implement xprt_set_used and xprt_set_idle to relax context checks"), we
decided to relax this lock using TASK_F_USR1 just as is done in muxes.

However the xprt_set_used() call, that's supposed to clear the flag,
visibly suffered from a copy-paste and kept the OR operation instead of
the AND, resulting in the flag never being released, so that SSL on the
backend continues to take the lock on each and every I/O access even when
the connection is not idle.

The effect is only a reduced performance. This could be backported, but
given the non-zero risk of triggering another bug somewhere, it would
be prudent to wait for this fix to be sufficiently tested in new
versions first.
---

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f8c4291f3b..26cf3b88a2 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6222,7 +6222,7 @@ static void ssl_set_used(struct connection *conn, void *xprt_ctx)
 	if (!ctx || !ctx->wait_event.tasklet)
 		return;
 
-	HA_ATOMIC_OR(&ctx->wait_event.tasklet->state, TASK_F_USR1);
+	HA_ATOMIC_AND(&ctx->wait_event.tasklet->state, ~TASK_F_USR1);
 	if (ctx->xprt)
 		xprt_set_used(conn, ctx->xprt, ctx->xprt_ctx);
 }