From 8a31783b647f3bef171a7faf074da2b1563f8a80 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 27 Feb 2024 15:33:59 +0100 Subject: [PATCH] BUG/MEDIUM: server: fix dynamic servers initial settings Contrary to static servers, dynamic servers does not initialize their settings from a default server instance. As such, _srv_parse_init() was responsible to set a set of minimal values to have a correct behavior. However, some settings were not properly initialized. This caused dynamic servers to not behave as static ones without explicit parameters. Currently, the main issue detected is connection reuse which was completely impossible. This is due to incorrect pool_purge_delay and max_reuse settings incompatible with srv_add_to_idle_list(). To fix the connection reuse, but also more generally to ensure dynamic servers are aligned with other server instances, define a new function srv_settings_init(). This is used to set initial values for both default servers and dynamic servers. For static servers, srv_settings_cpy() is kept instead, using their default server as reference. This patch could have unexpected effects on dynamic servers behavior as it restored proper initial settings. Previously, they were set to 0 via calloc() invocation from new_server(). This should be backported up to 2.6, after a brief period of observation. --- include/haproxy/server.h | 1 + src/proxy.c | 24 +------------------ src/server.c | 51 +++++++++++++++++++++++++++++++--------- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 1dcde1b30..18b4d3874 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -45,6 +45,7 @@ extern struct dict server_key_dict; int srv_downtime(const struct server *s); int srv_lastsession(const struct server *s); int srv_getinter(const struct check *check); +void srv_settings_init(struct server *srv); void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl); int parse_server(const char *file, int linenum, char **args, struct proxy *curproxy, const struct proxy *defproxy, int parse_flags); int srv_update_addr(struct server *s, void *ip, int ip_sin_family, struct server_inetaddr_updater updater); diff --git a/src/proxy.c b/src/proxy.c index 4436ccfa1..ba15b3b9c 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -1398,29 +1398,7 @@ void proxy_preset_defaults(struct proxy *defproxy) defproxy->options2 |= PR_O2_INDEPSTR; defproxy->max_out_conns = MAX_SRV_LIST; - defproxy->defsrv.check.inter = DEF_CHKINTR; - defproxy->defsrv.check.fastinter = 0; - defproxy->defsrv.check.downinter = 0; - defproxy->defsrv.agent.inter = DEF_CHKINTR; - defproxy->defsrv.agent.fastinter = 0; - defproxy->defsrv.agent.downinter = 0; - defproxy->defsrv.check.rise = DEF_RISETIME; - defproxy->defsrv.check.fall = DEF_FALLTIME; - defproxy->defsrv.agent.rise = DEF_AGENT_RISETIME; - defproxy->defsrv.agent.fall = DEF_AGENT_FALLTIME; - defproxy->defsrv.check.port = 0; - defproxy->defsrv.agent.port = 0; - defproxy->defsrv.maxqueue = 0; - defproxy->defsrv.minconn = 0; - defproxy->defsrv.maxconn = 0; - defproxy->defsrv.max_reuse = -1; - defproxy->defsrv.max_idle_conns = -1; - defproxy->defsrv.pool_purge_delay = 5000; - defproxy->defsrv.slowstart = 0; - defproxy->defsrv.onerror = DEF_HANA_ONERR; - defproxy->defsrv.consecutive_errors_limit = DEF_HANA_ERRLIMIT; - defproxy->defsrv.uweight = defproxy->defsrv.iweight = 1; - LIST_INIT(&defproxy->defsrv.pp_tlvs); + srv_settings_init(&defproxy->defsrv); defproxy->email_alert.level = LOG_ALERT; defproxy->load_server_state_from_file = PR_SRV_STATE_FILE_UNSPEC; diff --git a/src/server.c b/src/server.c index 197600ebd..fa3cd80f5 100644 --- a/src/server.c +++ b/src/server.c @@ -2594,6 +2594,45 @@ int srv_prepare_for_resolution(struct server *srv, const char *hostname) return -1; } +/* Initialize default values for . Used both for dynamic servers and + * default servers. The latter are not initialized via new_server(), hence this + * function purpose. For static servers, srv_settings_cpy() is used instead + * reusing their default server instance. + */ +void srv_settings_init(struct server *srv) +{ + srv->check.inter = DEF_CHKINTR; + srv->check.fastinter = 0; + srv->check.downinter = 0; + srv->check.rise = DEF_RISETIME; + srv->check.fall = DEF_FALLTIME; + srv->check.port = 0; + + srv->agent.inter = DEF_CHKINTR; + srv->agent.fastinter = 0; + srv->agent.downinter = 0; + srv->agent.rise = DEF_AGENT_RISETIME; + srv->agent.fall = DEF_AGENT_FALLTIME; + srv->agent.port = 0; + + srv->maxqueue = 0; + srv->minconn = 0; + srv->maxconn = 0; + + srv->max_reuse = -1; + srv->max_idle_conns = -1; + srv->pool_purge_delay = 5000; + + srv->slowstart = 0; + + srv->onerror = DEF_HANA_ONERR; + srv->consecutive_errors_limit = DEF_HANA_ERRLIMIT; + + srv->uweight = srv->iweight = 1; + + LIST_INIT(&srv->pp_tlvs); +} + /* * Copy server settings to server allocating * everything needed. @@ -3279,22 +3318,12 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg, /* Copy default server settings to new server */ srv_settings_cpy(newsrv, &curproxy->defsrv, 0); } else { - /* Initialize dynamic server weight to 1 */ - newsrv->uweight = newsrv->iweight = 1; + srv_settings_init(newsrv); /* A dynamic server is disabled on startup */ newsrv->next_admin = SRV_ADMF_FMAINT; newsrv->next_state = SRV_ST_STOPPED; server_recalc_eweight(newsrv, 0); - - /* Set default values for checks */ - newsrv->check.inter = DEF_CHKINTR; - newsrv->check.rise = DEF_RISETIME; - newsrv->check.fall = DEF_FALLTIME; - - newsrv->agent.inter = DEF_CHKINTR; - newsrv->agent.rise = DEF_AGENT_RISETIME; - newsrv->agent.fall = DEF_AGENT_FALLTIME; } HA_SPIN_INIT(&newsrv->lock); }