From: Aurelien DARRAGON Date: Wed, 7 May 2025 21:42:04 +0000 (+0200) Subject: MEDIUM: counters: manage shared counters using dedicated helpers X-Git-Tag: v3.3-dev1~14 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b599138842ffe80a66367c6269face7cbbdf4a0f;p=thirdparty%2Fhaproxy.git MEDIUM: counters: manage shared counters using dedicated helpers proxies, listeners and server shared counters are now managed via helpers added in one of the previous commits. When guid is not set (ie: when not yet assigned), shared counters pointer is allocated using calloc() (local memory) and a flag is set on the shared counters struct to know how to manipulate (and free it). Else if guid is set, then it means that the counters may be shared so while for now we don't actually use a shared memory location the API is ready for that. The way it works, for proxies and servers (for which guid is not known during creation), we first call counters_{fe,be}_shared_get with guid not set, which results in local pointer being retrieved (as if we just manually called calloc() to retrieve a pointer). Later (during postparsing) if guid is set we try to upgrade the pointer from local to shared. Lastly, since the memory location for some objects (proxies and servers counters) may change from creation to postparsing, let's update counters->last_change member directly under counters_{fe,be}_shared_get() so we don't miss it. No change of behavior is expected, this is only preparation work. --- diff --git a/include/haproxy/counters-t.h b/include/haproxy/counters-t.h index 28f456ea4..ad7ac9eed 100644 --- a/include/haproxy/counters-t.h +++ b/include/haproxy/counters-t.h @@ -26,11 +26,13 @@ #include #define COUNTERS_SHARED_F_NONE 0x0000 +#define COUNTERS_SHARED_F_LOCAL 0x0001 // shared counter struct is actually process-local // common to fe_counters_shared and be_counters_shared #define COUNTERS_SHARED \ struct { \ uint16_t flags; /* COUNTERS_SHARED_F flags */\ + unsigned long last_change; /* last time, when the state was changed */\ } // for convenience (generic pointer) @@ -74,7 +76,6 @@ struct fe_counters_shared { } http; } p; /* protocol-specific stats */ struct freq_ctr sess_per_sec; /* sessions per second on this server */ - unsigned long last_change; /* last time, when the state was changed */ long long failed_req; /* failed requests (eg: invalid or timeout) */ long long denied_resp; /* blocked responses because of security concerns */ @@ -146,8 +147,6 @@ struct be_counters_shared { long long bytes_in; /* number of bytes transferred from the client to the server */ long long cum_sess; /* cumulated number of accepted connections */ - - unsigned long last_change; /* last time, when the state was changed */ }; /* counters used by servers and backends */ diff --git a/src/cfgparse.c b/src/cfgparse.c index d8ea334f0..d0b86d65a 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -49,6 +49,7 @@ #include #include #include +#include #ifdef USE_CPU_AFFINITY #include #include @@ -4259,7 +4260,7 @@ init_proxies_list_stage2: if (curproxy->options2 & PR_O2_SOCKSTAT) { listener->counters = calloc(1, sizeof(*listener->counters)); if (listener->counters) { - listener->counters->shared = calloc(1, sizeof(*listener->counters->shared)); + listener->counters->shared = counters_fe_shared_get(&listener->guid); if (!listener->counters->shared) { ha_free(&listener->counters); ha_alert("config: %s '%s': out of memory.\n", diff --git a/src/counters.c b/src/counters.c index 41d029350..e9385135a 100644 --- a/src/counters.c +++ b/src/counters.c @@ -19,18 +19,32 @@ */ #include +#include +#include #include +#include /* retrieved shared counters pointer for a given object * hint is expected to reflect the actual type size (fe/be) + * if is not set, then sharing is disabled * Returns the pointer on success or NULL on failure */ -static void *_counters_shared_get(const struct guid_node *guid, size_t size) +static void*_counters_shared_get(const struct guid_node *guid, size_t size) { + struct counters_shared *shared; + uint last_change; + /* no shared memory for now, simply allocate a memory block * for the counters (zero-initialized), ignore guid */ - return calloc(1, size); + shared = calloc(1, size); + if (!shared) + return NULL; + if (!guid->node.key) + shared->flags |= COUNTERS_SHARED_F_LOCAL; + last_change = ns_to_sec(now_ns); + HA_ATOMIC_STORE(&shared->last_change, last_change); + return shared; } /* retrieve shared fe counters pointer for a given object */ diff --git a/src/proxy.c b/src/proxy.c index b39f31621..566f3f616 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -221,8 +222,8 @@ static inline void proxy_free_common(struct proxy *px) ha_free(&px->id); LIST_DEL_INIT(&px->global_list); drop_file_name(&px->conf.file); - ha_free(&px->fe_counters.shared); - ha_free(&px->be_counters.shared); + counters_fe_shared_drop(px->fe_counters.shared); + counters_be_shared_drop(px->be_counters.shared); ha_free(&px->check_command); ha_free(&px->check_path); ha_free(&px->cookie_name); @@ -398,7 +399,7 @@ void deinit_proxy(struct proxy *p) free(l->label); free(l->per_thr); if (l->counters) { - free(l->counters->shared); + counters_fe_shared_drop(l->counters->shared); free(l->counters); } task_destroy(l->rx.rhttp.task); @@ -1710,39 +1711,8 @@ void proxy_unref_defaults(struct proxy *px) */ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char **errmsg) { - uint last_change; - init_new_proxy(px); - last_change = ns_to_sec(now_ns); - /* allocate private memory for shared counters: used as a fallback - * or when sharing is disabled. If sharing is enabled pointers will - * be updated to point to the proper shared memory location during - * proxy postparsing, see proxy_postparse() - */ - if (cap & PR_CAP_FE) { - px->fe_counters.shared = calloc(1, sizeof(*px->fe_counters.shared)); - if (!px->fe_counters.shared) { - memprintf(errmsg, "out of memory"); - goto fail; - } - HA_ATOMIC_STORE(&px->fe_counters.shared->last_change, last_change); - } - if (cap & (PR_CAP_FE|PR_CAP_BE)) { - /* by default stream->be points to stream->fe, thus proxy - * be_counters may be used even if the proxy lacks the backend - * capability - */ - px->be_counters.shared = calloc(1, sizeof(*px->be_counters.shared)); - if (!px->be_counters.shared) { - memprintf(errmsg, "out of memory"); - goto fail; - } - - HA_ATOMIC_STORE(&px->be_counters.shared->last_change, last_change); - } - - if (name) { px->id = strdup(name); if (!px->id) { @@ -1766,8 +1736,8 @@ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char * memprintf(errmsg, "proxy '%s': %s", name, *errmsg); ha_free(&px->id); - ha_free(&px->fe_counters.shared); - ha_free(&px->be_counters.shared); + counters_fe_shared_drop(px->fe_counters.shared); + counters_be_shared_drop(px->be_counters.shared); return 0; } @@ -1799,6 +1769,46 @@ struct proxy *alloc_new_proxy(const char *name, unsigned int cap, char **errmsg) return NULL; } +/* post-check for proxies */ +static int proxy_postcheck(struct proxy *px) +{ + int err_code = ERR_NONE; + + /* allocate private memory for shared counters: used as a fallback + * or when sharing is disabled. If sharing is enabled pointers will + * be updated to point to the proper shared memory location during + * proxy postparsing, see proxy_postparse() + */ + if (px->cap & PR_CAP_FE) { + px->fe_counters.shared = counters_fe_shared_get(&px->guid); + if (!px->fe_counters.shared) { + ha_alert("out of memory while setting up shared counters for %s %s\n", + proxy_type_str(px), px->id); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + } + if (px->cap & (PR_CAP_FE|PR_CAP_BE)) { + /* by default stream->be points to stream->fe, thus proxy + * be_counters may be used even if the proxy lacks the backend + * capability + */ + px->be_counters.shared = counters_be_shared_get(&px->guid); + if (!px->be_counters.shared) { + ha_alert("out of memory while setting up shared counters for %s %s\n", + proxy_type_str(px), px->id); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + + } + + + out: + return err_code; +} +REGISTER_POST_PROXY_CHECK(proxy_postcheck); + /* Copy the proxy settings from to . * Returns 0 on success. * Returns 1 on error. will be allocated with an error description. diff --git a/src/server.c b/src/server.c index 758d7e2a7..7a8d1ba61 100644 --- a/src/server.c +++ b/src/server.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -3028,18 +3029,11 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl struct server *new_server(struct proxy *proxy) { struct server *srv; - struct be_counters_shared *scounters; srv = calloc(1, sizeof *srv); if (!srv) return NULL; - scounters = calloc(1, sizeof(*scounters)); - if (!scounters) { - ha_free(&srv); - return NULL; - } - srv_take(srv); srv->obj_type = OBJ_TYPE_SERVER; @@ -3052,8 +3046,6 @@ struct server *new_server(struct proxy *proxy) srv->rid = 0; /* rid defaults to 0 */ srv->next_state = SRV_ST_RUNNING; /* early server setup */ - srv->counters.shared = scounters; - HA_ATOMIC_STORE(&srv->counters.shared->last_change, ns_to_sec(now_ns)); srv->check.obj_type = OBJ_TYPE_CHECK; srv->check.status = HCHK_STATUS_INI; @@ -3125,7 +3117,7 @@ void srv_free_params(struct server *srv) free(srv->resolvers_id); free(srv->addr_node.key); free(srv->lb_nodes); - free(srv->counters.shared); + counters_be_shared_drop(srv->counters.shared); if (srv->log_target) { deinit_log_target(srv->log_target); free(srv->log_target); @@ -3434,6 +3426,21 @@ int srv_init(struct server *srv) if (err_code & ERR_CODE) goto out; + srv->counters.shared = counters_be_shared_get(&srv->guid); + if (!srv->counters.shared) { + ha_alert("memory error while setting up shared counters for %s/%s server\n", srv->proxy->id, srv->id); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } + + if (srv->flags & SRV_F_DYNAMIC) { + /* A dynamic server is disabled on startup */ + srv->next_admin = SRV_ADMF_FMAINT; + srv->next_state = SRV_ST_STOPPED; + server_recalc_eweight(srv, 0); // relies on srv counters + srv_lb_commit_status(srv); + } + err_code |= init_srv_requeue(srv); if (err_code & ERR_CODE) @@ -3646,14 +3653,8 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg, if (!(parse_flags & SRV_PARSE_DYNAMIC)) { /* Copy default server settings to new server */ srv_settings_cpy(newsrv, &curproxy->defsrv, 0); - } else { + } else 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); - } HA_SPIN_INIT(&newsrv->lock); } else {