Skip to content

Commit 8de0807

Browse files
a-denoyellea-denoyelle
committed
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.
1 parent 9574867 commit 8de0807

File tree

8 files changed

+118
-34
lines changed

8 files changed

+118
-34
lines changed

‎include/haproxy/session.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ void __session_add_glitch_ctr(struct session *sess, uint inc);
4444
void session_embryonic_build_legacy_err(struct session *sess, struct buffer *out);
4545

4646
int session_add_conn(struct session *sess, struct connection *conn);
47+
int session_reinsert_idle_conn(struct session *sess, struct connection *conn);
4748
int session_check_idle_conn(struct session *sess, struct connection *conn);
4849
struct connection *session_get_conn(struct session *sess, void *target, int64_t hash);
4950
void session_unown_conn(struct session *sess, struct connection *conn);

‎src/connection.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,25 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake)
213213
goto done;
214214

215215
if (conn_in_list) {
216-
if (srv->cur_admin & SRV_ADMF_MAINT) {
216+
if (srv && (srv->cur_admin & SRV_ADMF_MAINT)) {
217217
/* Do not store an idle conn if server in maintenance. */
218218
conn->mux->destroy(conn->ctx);
219219
ret = -1;
220220
goto done;
221221
}
222222

223-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
224-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
225-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
223+
if (conn->flags & CO_FL_SESS_IDLE) {
224+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
225+
/* session add conn failure */
226+
conn->mux->destroy(conn->ctx);
227+
ret = -1;
228+
}
229+
}
230+
else {
231+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
232+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
233+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
234+
}
226235
}
227236
}
228237
done:

‎src/mux_fcgi.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3089,12 +3089,20 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
30893089
t = NULL;
30903090

30913091
if (!ret && conn_in_list) {
3092-
struct server *srv = __objt_server(conn->target);
3092+
struct server *srv = objt_server(conn->target);
30933093

3094-
if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
3095-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
3096-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
3097-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
3094+
if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
3095+
if (conn->flags & CO_FL_SESS_IDLE) {
3096+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
3097+
/* session add conn failure */
3098+
goto release;
3099+
}
3100+
}
3101+
else {
3102+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
3103+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
3104+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
3105+
}
30983106
}
30993107
else {
31003108
/* Do not store an idle conn if server in maintenance. */

‎src/mux_h1.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4334,12 +4334,20 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
43344334
t = NULL;
43354335

43364336
if (!ret && conn_in_list) {
4337-
struct server *srv = __objt_server(conn->target);
4337+
struct server *srv = objt_server(conn->target);
43384338

4339-
if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
4340-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
4341-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
4342-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
4339+
if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
4340+
if (conn->flags & CO_FL_SESS_IDLE) {
4341+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
4342+
/* session add conn failure */
4343+
goto release;
4344+
}
4345+
}
4346+
else {
4347+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
4348+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
4349+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
4350+
}
43434351
}
43444352
else {
43454353
/* Do not store an idle conn if server in maintenance. */

‎src/mux_h2.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4993,12 +4993,20 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
49934993
h2c->next_tasklet = NULL;
49944994

49954995
if (!ret && conn_in_list) {
4996-
struct server *srv = __objt_server(conn->target);
4996+
struct server *srv = objt_server(conn->target);
49974997

4998-
if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
4999-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
5000-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
5001-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
4998+
if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
4999+
if (conn->flags & CO_FL_SESS_IDLE) {
5000+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
5001+
/* session add conn failure */
5002+
goto release;
5003+
}
5004+
}
5005+
else {
5006+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
5007+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
5008+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
5009+
}
50025010
}
50035011
else {
50045012
/* Do not store an idle conn if server in maintenance. */

‎src/mux_spop.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2585,12 +2585,20 @@ static struct task *spop_io_cb(struct task *t, void *ctx, unsigned int state)
25852585
t = NULL;
25862586

25872587
if (!ret && conn_in_list) {
2588-
struct server *srv = __objt_server(conn->target);
2588+
struct server *srv = objt_server(conn->target);
25892589

2590-
if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
2591-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
2592-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
2593-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
2590+
if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
2591+
if (conn->flags & CO_FL_SESS_IDLE) {
2592+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
2593+
/* session add conn failure */
2594+
goto release;
2595+
}
2596+
}
2597+
else {
2598+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
2599+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
2600+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
2601+
}
25942602
}
25952603
else {
25962604
/* Do not store an idle conn if server in maintenance. */

‎src/session.c‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,38 @@ int session_add_conn(struct session *sess, struct connection *conn)
697697
return ret;
698698
}
699699

700+
/* Reinsert a backend connection <conn> into <sess> session. This function is
701+
* reserved for idle conns which were previously temporarily removed from
702+
* session to protect it against other threads.
703+
*
704+
* Returns true on success else false.
705+
*/
706+
int session_reinsert_idle_conn(struct session *sess, struct connection *conn)
707+
{
708+
struct sess_priv_conns *pconns;
709+
int ret = 0;
710+
711+
/* This function is reserved for idle private connections. */
712+
BUG_ON(!(conn->flags & CO_FL_SESS_IDLE));
713+
714+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
715+
716+
pconns = sess_get_sess_conns(sess, conn->target);
717+
if (!pconns) {
718+
pconns = sess_alloc_sess_conns(sess, conn->target);
719+
if (!pconns)
720+
goto out;
721+
}
722+
723+
LIST_APPEND(&pconns->conn_list, &conn->sess_el);
724+
++sess->idle_conns;
725+
ret = 1;
726+
727+
out:
728+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
729+
return ret;
730+
}
731+
700732
/* Check that session <sess> is able to keep idle connection <conn>. This must
701733
* be called each time a connection stored in a session becomes idle.
702734
*

‎src/ssl_sock.c‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6478,19 +6478,29 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
64786478
#endif
64796479
leave:
64806480
if (!ret && conn_in_list) {
6481-
struct server *srv = __objt_server(conn->target);
6482-
6483-
if (!(srv->cur_admin & SRV_ADMF_MAINT)) {
6484-
TRACE_DEVEL("adding conn back to idle list", SSL_EV_CONN_IO_CB, conn);
6485-
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
6486-
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
6487-
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
6481+
struct server *srv = objt_server(conn->target);
6482+
6483+
/* Connection is idle which means MUX layer is already initialized. */
6484+
BUG_ON(!conn->mux);
6485+
6486+
if (!srv || !(srv->cur_admin & SRV_ADMF_MAINT)) {
6487+
if (conn->flags & CO_FL_SESS_IDLE) {
6488+
TRACE_DEVEL("adding conn back to session list", SSL_EV_CONN_IO_CB, conn);
6489+
if (!session_reinsert_idle_conn(conn->owner, conn)) {
6490+
/* session add conn failure */
6491+
conn->mux->destroy(conn->ctx);
6492+
t = NULL;
6493+
}
6494+
}
6495+
else {
6496+
TRACE_DEVEL("adding conn back to idle list", SSL_EV_CONN_IO_CB, conn);
6497+
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
6498+
_srv_add_idle(srv, conn, conn_in_list == CO_FL_SAFE_LIST);
6499+
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
6500+
}
64886501
}
64896502
else {
64906503
/* Do not store an idle conn if server in maintenance. */
6491-
6492-
/* Connection is idle which means MUX layer is already initialized. */
6493-
BUG_ON(!conn->mux);
64946504
conn->mux->destroy(conn->ctx);
64956505
t = NULL;
64966506
}

0 commit comments

Comments
 (0)