From: Willy Tarreau Date: Tue, 10 Feb 2026 06:10:09 +0000 (+0100) Subject: BUG/MEDIUM: lb-chash: always properly initialize lb_nodes with dynamic servers X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64c5d45a269749a30a372420b4d196235dc8d745;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: lb-chash: always properly initialize lb_nodes with dynamic servers An issue was introduced in 3.0 with commit faa8c3e024 ("MEDIUM: lb-chash: Deterministic node hashes based on server address"): the new server_key field and lb_nodes entries initialization were not updated for servers added at run time with "add server": server_key remains zero and the key used in lb_node remains the one depending only on the server's ID. This will cause trouble when adding new servers with consistent hashing, because the hash-key will be ignored until the server's weight changes and the key difference is detected, leading to its recalculation. This is essentially caused by the poorly placed lb_nodes initialization that is specific to lb-chash and had to be replicated in the code dealing with server addition. This commit solves the problem by adding a new ->server_init() function in the lbprm proxy struct, that is called by the server addition code. This also allows to abandon the complex check for LB algos that was placed there for that purpose. For now only lb-chash provides such a function, and calls it as well during initial setup. This way newly added servers always use the correct key now. While it should also theoretically have had an impact on servers added with the "random" algorithm, it's unlikely that the difference between proper server keys and those based on their ID could have had any visible effect. This patch should be backported as far as 3.0. The backport may be eased by a preliminary backport of previous commit "CLEANUP: lb-chash: free lb_nodes from chash's deinit(), not global", though this is not strictly necessary if context is manually adjusted. --- diff --git a/include/haproxy/backend-t.h b/include/haproxy/backend-t.h index 35f882f28..da3c10a5a 100644 --- a/include/haproxy/backend-t.h +++ b/include/haproxy/backend-t.h @@ -192,6 +192,7 @@ struct lbprm { void (*server_requeue)(struct server *); /* function used to place the server where it must be */ void (*proxy_deinit)(struct proxy *); /* to be called when we're destroying the proxy */ void (*server_deinit)(struct server *); /* to be called when we're destroying the server */ + int (*server_init)(struct server *); /* initialize a freshly added server (runtime); <0=fail. */ }; #endif /* _HAPROXY_BACKEND_T_H */ diff --git a/src/lb_chash.c b/src/lb_chash.c index 9e9025458..a852ae07d 100644 --- a/src/lb_chash.c +++ b/src/lb_chash.c @@ -552,6 +552,26 @@ struct server *chash_get_next_server(struct proxy *p, struct server *srvtoavoid) return srv; } +/* Allocates and initializes lb nodes for server . Returns < 0 on error. + * This is called by chash_init_server_tree() as well as from srv_alloc_lb() + * for runtime addition. + */ +int chash_server_init(struct server *srv) +{ + int node; + + srv->lb_nodes = calloc(srv->lb_nodes_tot, sizeof(*srv->lb_nodes)); + if (!srv->lb_nodes) + return -1; + + srv->lb_server_key = chash_compute_server_key(srv); + for (node = 0; node < srv->lb_nodes_tot; node++) { + srv->lb_nodes[node].server = srv; + srv->lb_nodes[node].node.key = chash_compute_node_key(srv, node); + } + return 0; +} + /* Releases the allocated lb_nodes for this server */ void chash_server_deinit(struct server *srv) { @@ -568,11 +588,11 @@ int chash_init_server_tree(struct proxy *p) { struct server *srv; struct eb_root init_head = EB_ROOT; - int node; p->lbprm.set_server_status_up = chash_set_server_status_up; p->lbprm.set_server_status_down = chash_set_server_status_down; p->lbprm.update_server_eweight = chash_update_server_weight; + p->lbprm.server_init = chash_server_init; p->lbprm.server_deinit = chash_server_deinit; p->lbprm.server_take_conn = NULL; p->lbprm.server_drop_conn = NULL; @@ -595,17 +615,11 @@ int chash_init_server_tree(struct proxy *p) srv->lb_tree = (srv->flags & SRV_F_BACKUP) ? &p->lbprm.chash.bck : &p->lbprm.chash.act; srv->lb_nodes_tot = srv->uweight * BE_WEIGHT_SCALE; srv->lb_nodes_now = 0; - srv->lb_nodes = calloc(srv->lb_nodes_tot, - sizeof(*srv->lb_nodes)); - if (!srv->lb_nodes) { + + if (chash_server_init(srv) < 0) { ha_alert("failed to allocate lb_nodes for server %s.\n", srv->id); return -1; } - srv->lb_server_key = chash_compute_server_key(srv); - for (node = 0; node < srv->lb_nodes_tot; node++) { - srv->lb_nodes[node].server = srv; - srv->lb_nodes[node].node.key = chash_compute_node_key(srv, node); - } if (srv_currently_usable(srv)) chash_queue_dequeue_srv(srv); diff --git a/src/server.c b/src/server.c index 2452f0382..61663120c 100644 --- a/src/server.c +++ b/src/server.c @@ -5888,25 +5888,13 @@ static int cli_parse_enable_server(char **args, char *payload, struct appctx *ap */ static int srv_alloc_lb(struct server *sv, struct proxy *be) { - int node; - sv->lb_tree = (sv->flags & SRV_F_BACKUP) ? &be->lbprm.chash.bck : &be->lbprm.chash.act; sv->lb_nodes_tot = sv->uweight * BE_WEIGHT_SCALE; sv->lb_nodes_now = 0; - if (((be->lbprm.algo & (BE_LB_KIND | BE_LB_PARM)) == (BE_LB_KIND_RR | BE_LB_RR_RANDOM)) || - ((be->lbprm.algo & (BE_LB_KIND | BE_LB_HASH_TYPE)) == (BE_LB_KIND_HI | BE_LB_HASH_CONS))) { - sv->lb_nodes = calloc(sv->lb_nodes_tot, sizeof(*sv->lb_nodes)); - - if (!sv->lb_nodes) - return 0; - - for (node = 0; node < sv->lb_nodes_tot; node++) { - sv->lb_nodes[node].server = sv; - sv->lb_nodes[node].node.key = full_hash(sv->puid * SRV_EWGHT_RANGE + node); - } - } + if (be->lbprm.server_init && be->lbprm.server_init(sv) < 0) + return 0; // typically out of memory return 1; }