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.

include/haproxy/counters-t.h

@@ -26,11 +26,13 @@
 #include <haproxy/freq_ctr-t.h>
 
 #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 */

src/cfgparse.c

@@ -49,6 +49,7 @@
 #include <haproxy/check.h>
 #include <haproxy/chunk.h>
 #include <haproxy/clock.h>
+#include <haproxy/counters.h>
 #ifdef USE_CPU_AFFINITY
 #include <haproxy/cpuset.h>
 #include <haproxy/cpu_topo.h>
@@ -4259,7 +4260,7 @@ int check_config_validity()
 			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",

src/counters.c

@@ -19,18 +19,32 @@
  */
 
 #include <stdlib.h>
+#include <haproxy/atomic.h>
+#include <haproxy/clock.h>
 #include <haproxy/counters.h>
+#include <haproxy/time.h>
 
 /* retrieved shared counters pointer for a given <guid> object
  * <size> hint is expected to reflect the actual type size (fe/be)
+ * if <guid> 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 <guid> object */

src/proxy.c

@@ -25,6 +25,7 @@
 #include <haproxy/capture-t.h>
 #include <haproxy/cfgparse.h>
 #include <haproxy/cli.h>
+#include <haproxy/counters.h>
 #include <haproxy/errors.h>
 #include <haproxy/fd.h>
 #include <haproxy/filters.h>
@@ -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 <defproxy> to <curproxy>.
  * Returns 0 on success.
  * Returns 1 on error. <errmsg> will be allocated with an error description.

src/server.c

@@ -25,6 +25,7 @@
 #include <haproxy/check.h>
 #include <haproxy/cli.h>
 #include <haproxy/connection.h>
+#include <haproxy/counters.h>
 #include <haproxy/dict-t.h>
 #include <haproxy/errors.h>
 #include <haproxy/global.h>
@@ -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 {