sinjuginas/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MAJOR: conn-idle: fix hash indexing issues on idle conns

Browse Source 
Idle connections do not work on 32-bit machines due to an alignment issue
causing the connection nodes to be indexed with their lower 32-bits set to
zero and the higher 32 ones containing the 32 lower bitss of the hash. The
cause is the use of ebmb_node with an aligned data, as on this platform
ebmb_node is only 32-bit aligned, leaving a hole before the following hash
which is a uint64_t:

  $ pahole -C conn_hash_node ./haproxy
  struct conn_hash_node {
        struct ebmb_node           node;                 /*     0    20 */

        /* XXX 4 bytes hole, try to pack */

        int64_t                    hash;                 /*    24     8 */
        struct connection *        conn;                 /*    32     4 */

        /* size: 40, cachelines: 1, members: 3 */
        /* sum members: 32, holes: 1, sum holes: 4 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
  };

Instead, eb64 nodes should be used when it comes to simply storing a
64-bit key, and that is what this patch does.

For backports, a variant consisting in simply marking the "hash" member
with a "packed" attribute on the struct also does the job (tested), and
might be preferable if the fix is difficult to adapt. Only 2.6 and 2.5
are affected by this.

(cherry picked from commit 8522348482)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Willy Tarreau 2022-09-29 20:32:43 +02:00 committed by Christopher Faulet
parent 7fe5c4d5d2
commit c564abd107
10 changed files with 31 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/haproxy/connection-t.h
View File

@ -457,7 +457,7 @@ enum conn_hash_params_t {
#define CONN_HASH_PARAMS_TYPE_COUNT 6
#define CONN_HASH_PAYLOAD_LEN \
(((sizeof(((struct conn_hash_node *)0)->hash)) * 8) - CONN_HASH_PARAMS_TYPE_COUNT)
(((sizeof(((struct conn_hash_node *)0)->node.key)) * 8) - CONN_HASH_PARAMS_TYPE_COUNT)
#define CONN_HASH_GET_PAYLOAD(hash) \
(((hash) << CONN_HASH_PARAMS_TYPE_COUNT) >> CONN_HASH_PARAMS_TYPE_COUNT)
@ -522,8 +522,7 @@ struct connection {
* A connection is identified by a hash generated from its specific parameters
*/
struct conn_hash_node {
struct ebmb_node node;
int64_t hash; /* key for ebmb tree */
struct eb64_node node; /* contains the hashing key */
struct connection *conn; /* connection owner of the node */
};

2
include/haproxy/connection.h
View File

@ -81,7 +81,7 @@ int conn_install_mux_be(struct connection *conn, void *ctx, struct session *sess
const struct mux_ops *force_mux_ops);
int conn_install_mux_chk(struct connection *conn, void *ctx, struct session *sess);
void conn_delete_from_tree(struct ebmb_node *node);
void conn_delete_from_tree(struct eb64_node *node);
void conn_init(struct connection *conn, void *target);
struct connection *conn_new(void *target);

2
include/haproxy/session.h
View File

@ -213,7 +213,7 @@ static inline struct connection *session_get_conn(struct session *sess, void *ta
list_for_each_entry(srv_list, &sess->srv_list, srv_list) {
if (srv_list->target == target) {
list_for_each_entry(srv_conn, &srv_list->conn_list, session_list) {
if ((srv_conn->hash_node && srv_conn->hash_node->hash == hash) &&
if ((srv_conn->hash_node && srv_conn->hash_node->node.key == hash) &&
srv_conn->mux &&
(srv_conn->mux->avail_streams(srv_conn) > 0) &&
!(srv_conn->flags & CO_FL_WAIT_XPRT)) {

9
src/backend.c
View File

@ -1263,9 +1263,8 @@ static struct connection *conn_backend_get(struct stream *s, struct server *srv,
session_add_conn(s->sess, conn, conn->target);
}
else {
ebmb_insert(&srv->per_thr[tid].avail_conns,
&conn->hash_node->node,
sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].avail_conns,
&conn->hash_node->node);
}
}
return conn;
@ -1609,7 +1608,7 @@ skip_reuse:
return SF_ERR_RESOURCE;
}
srv_conn->hash_node->hash = hash;
srv_conn->hash_node->node.key = hash;
}
}
@ -1777,7 +1776,7 @@ skip_reuse:
if (srv && reuse_mode == PR_O_REUSE_ALWS &&
!(srv_conn->flags & CO_FL_PRIVATE) &&
srv_conn->mux->avail_streams(srv_conn) > 0) {
ebmb_insert(&srv->per_thr[tid].avail_conns, &srv_conn->hash_node->node, sizeof(srv_conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].avail_conns, &srv_conn->hash_node->node);
}
else if (srv_conn->flags & CO_FL_PRIVATE ||
(reuse_mode == PR_O_REUSE_SAFE &&

8
src/connection.c
View File

@ -52,9 +52,9 @@ struct mux_stopping_data mux_stopping_data[MAX_THREADS];
/* disables sending of proxy-protocol-v2's LOCAL command */
static int pp2_never_send_local;
void conn_delete_from_tree(struct ebmb_node *node)
void conn_delete_from_tree(struct eb64_node *node)
{
ebmb_delete(node);
eb64_delete(node);
}
int conn_create_mux(struct connection *conn)
@ -83,7 +83,7 @@ int conn_create_mux(struct connection *conn)
*/
if (srv && ((srv->proxy->options & PR_O_REUSE_MASK) == PR_O_REUSE_ALWS) &&
!(conn->flags & CO_FL_PRIVATE) && conn->mux->avail_streams(conn) > 0)
ebmb_insert(&srv->per_thr[tid].avail_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].avail_conns, &conn->hash_node->node);
else if (conn->flags & CO_FL_PRIVATE) {
/* If it fail now, the same will be done in mux->detach() callback */
session_add_conn(sess, conn, conn->target);
@ -165,7 +165,7 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake)
&srv->per_thr[tid].idle_conns;
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
ebmb_insert(root, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(root, &conn->hash_node->node);
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
}
}

9
src/mux_fcgi.c
View File

@ -3069,9 +3069,9 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
if (conn_in_list == CO_FL_SAFE_LIST)
ebmb_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node);
else
ebmb_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node);
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
}
return t;
@ -3689,9 +3689,8 @@ static void fcgi_detach(struct sedesc *sd)
else if (!fconn->conn->hash_node->node.node.leaf_p &&
fcgi_avail_streams(fconn->conn) > 0 && objt_server(fconn->conn->target) &&
!LIST_INLIST(&fconn->conn->session_list)) {
ebmb_insert(&__objt_server(fconn->conn->target)->per_thr[tid].avail_conns,
&fconn->conn->hash_node->node,
sizeof(fconn->conn->hash_node->hash));
eb64_insert(&__objt_server(fconn->conn->target)->per_thr[tid].avail_conns,
&fconn->conn->hash_node->node);
}
}
}

4
src/mux_h1.c
View File

@ -3189,9 +3189,9 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
if (conn_in_list == CO_FL_SAFE_LIST)
ebmb_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node);
else
ebmb_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node);
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
}
return t;

9
src/mux_h2.c
View File

@ -4047,9 +4047,9 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
if (conn_in_list == CO_FL_SAFE_LIST)
ebmb_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node);
else
ebmb_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node);
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
}
@ -4501,9 +4501,8 @@ static void h2_detach(struct sedesc *sd)
else if (!h2c->conn->hash_node->node.node.leaf_p &&
h2_avail_streams(h2c->conn) > 0 && objt_server(h2c->conn->target) &&
!LIST_INLIST(&h2c->conn->session_list)) {
ebmb_insert(&__objt_server(h2c->conn->target)->per_thr[tid].avail_conns,
&h2c->conn->hash_node->node,
sizeof(h2c->conn->hash_node->hash));
eb64_insert(&__objt_server(h2c->conn->target)->per_thr[tid].avail_conns,
&h2c->conn->hash_node->node);
}
}
}

14
src/server.c
View File

@ -5779,11 +5779,11 @@ void srv_release_conn(struct server *srv, struct connection *conn)
*/
struct connection *srv_lookup_conn(struct eb_root *tree, uint64_t hash)
{
struct ebmb_node *node = NULL;
struct eb64_node *node = NULL;
struct connection *conn = NULL;
struct conn_hash_node *hash_node = NULL;
node = ebmb_lookup(tree, &hash, sizeof(hash_node->hash));
node = eb64_lookup(tree, hash);
if (node) {
hash_node = ebmb_entry(node, struct conn_hash_node, node);
conn = hash_node->conn;
@ -5797,13 +5797,13 @@ struct connection *srv_lookup_conn(struct eb_root *tree, uint64_t hash)
*/
struct connection *srv_lookup_conn_next(struct connection *conn)
{
struct ebmb_node *node = NULL;
struct eb64_node *node = NULL;
struct connection *next_conn = NULL;
struct conn_hash_node *hash_node = NULL;
node = ebmb_next_dup(&conn->hash_node->node);
node = eb64_next_dup(&conn->hash_node->node);
if (node) {
hash_node = ebmb_entry(node, struct conn_hash_node, node);
hash_node = eb64_entry(node, struct conn_hash_node, node);
next_conn = hash_node->conn;
}
@ -5846,11 +5846,11 @@ int srv_add_to_idle_list(struct server *srv, struct connection *conn, int is_saf
if (is_safe) {
conn->flags = (conn->flags & ~CO_FL_LIST_MASK) | CO_FL_SAFE_LIST;
ebmb_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node);
_HA_ATOMIC_INC(&srv->curr_safe_nb);
} else {
conn->flags = (conn->flags & ~CO_FL_LIST_MASK) | CO_FL_IDLE_LIST;
ebmb_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node);
_HA_ATOMIC_INC(&srv->curr_idle_nb);
}
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);

4
src/ssl_sock.c
View File

@ -6535,9 +6535,9 @@ leave:
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
if (conn_in_list == CO_FL_SAFE_LIST)
ebmb_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].safe_conns, &conn->hash_node->node);
else
ebmb_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node, sizeof(conn->hash_node->hash));
eb64_insert(&srv->per_thr[tid].idle_conns, &conn->hash_node->node);
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
}
return t;