From: Olivier Houchard Date: Wed, 14 Jan 2026 10:37:49 +0000 (+0100) Subject: MEDIUM: counters: mostly revert da813ae4d7cb77137ed X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7f4b053b26429754df3094001e375b81cee6895d;p=thirdparty%2Fhaproxy.git MEDIUM: counters: mostly revert da813ae4d7cb77137ed Contrarily to what was previously believed, there are corner cases where the counters may not be allocated, and we may want to make them optional at a later date, so we have to check if those counters are there. However, just checking that shared.tg is non-NULL is enough, we can then assume that shared.tg[tgid - 1] has properly been allocated too. Also modify the various COUNTER_SHARED_* macros to make sure they check for that too. --- diff --git a/include/haproxy/backend.h b/include/haproxy/backend.h index 47ecfff33..bdb457e33 100644 --- a/include/haproxy/backend.h +++ b/include/haproxy/backend.h @@ -88,7 +88,8 @@ static inline int be_usable_srv(struct proxy *be) /* set the time of last session on the backend */ static inline void be_set_sess_last(struct proxy *be) { - HA_ATOMIC_STORE(&be->be_counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns)); + if (be->be_counters.shared.tg) + HA_ATOMIC_STORE(&be->be_counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns)); } /* This function returns non-zero if the designated server will be diff --git a/include/haproxy/counters.h b/include/haproxy/counters.h index 89855ce1c..a6a68623d 100644 --- a/include/haproxy/counters.h +++ b/include/haproxy/counters.h @@ -43,11 +43,13 @@ void counters_be_shared_drop(struct be_counters_shared *counters); */ #define COUNTERS_SHARED_LAST_OFFSET(scounters, type, offset) \ ({ \ - unsigned long last = HA_ATOMIC_LOAD((type *)((char *)scounters[0] + offset));\ + unsigned long last = 0; \ unsigned long now_seconds = ns_to_sec(now_ns); \ int it; \ \ - for (it = 1; (it < global.nbtgroups && scounters[it]); it++) { \ + if (scounters) \ + last = HA_ATOMIC_LOAD((type *)((char *)scounters[0] + offset));\ + for (it = 1; (it < global.nbtgroups && scounters); it++) { \ unsigned long cur = HA_ATOMIC_LOAD((type *)((char *)scounters[it] + offset));\ if ((now_seconds - cur) < (now_seconds - last)) \ last = cur; \ @@ -74,7 +76,7 @@ void counters_be_shared_drop(struct be_counters_shared *counters); uint64_t __ret = 0; \ int it; \ \ - for (it = 0; (it < global.nbtgroups && scounters[it]); it++) \ + for (it = 0; (it < global.nbtgroups && scounters); it++) \ __ret += rfunc((type *)((char *)scounters[it] + offset)); \ __ret; \ }) @@ -94,7 +96,7 @@ void counters_be_shared_drop(struct be_counters_shared *counters); uint64_t __ret = 0; \ int it; \ \ - for (it = 0; (it < global.nbtgroups && scounters[it]); it++) \ + for (it = 0; (it < global.nbtgroups && scounters); it++) \ __ret += rfunc(&scounters[it]->elem, arg1, arg2); \ __ret; \ }) diff --git a/include/haproxy/proxy.h b/include/haproxy/proxy.h index 34274756b..a4ba4be58 100644 --- a/include/haproxy/proxy.h +++ b/include/haproxy/proxy.h @@ -166,10 +166,11 @@ static inline int proxy_abrt_close(const struct proxy *px) /* increase the number of cumulated connections received on the designated frontend */ static inline void proxy_inc_fe_conn_ctr(struct listener *l, struct proxy *fe) { - _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_conn); - update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->conn_per_sec, 1); - - if (l && l->counters) + if (fe->fe_counters.shared.tg) { + _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_conn); + update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->conn_per_sec, 1); + } + if (l && l->counters && l->counters->shared.tg) _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_conn); HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.cps_max, update_freq_ctr(&fe->fe_counters._conn_per_sec, 1)); @@ -178,10 +179,11 @@ static inline void proxy_inc_fe_conn_ctr(struct listener *l, struct proxy *fe) /* increase the number of cumulated connections accepted by the designated frontend */ static inline void proxy_inc_fe_sess_ctr(struct listener *l, struct proxy *fe) { - _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess); - update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->sess_per_sec, 1); - - if (l && l->counters) + if (fe->fe_counters.shared.tg) { + _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess); + update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->sess_per_sec, 1); + } + if (l && l->counters && l->counters->shared.tg) _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_sess); HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.sps_max, update_freq_ctr(&fe->fe_counters._sess_per_sec, 1)); @@ -197,17 +199,19 @@ static inline void proxy_inc_fe_cum_sess_ver_ctr(struct listener *l, struct prox http_ver > sizeof(fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver) / sizeof(*fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver)) return; - _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]); - if (l && l->counters) + if (fe->fe_counters.shared.tg) + _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]); + if (l && l->counters && l->counters->shared.tg && l->counters->shared.tg[tgid - 1]) _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]); } /* increase the number of cumulated streams on the designated backend */ static inline void proxy_inc_be_ctr(struct proxy *be) { - _HA_ATOMIC_INC(&be->be_counters.shared.tg[tgid - 1]->cum_sess); - update_freq_ctr(&be->be_counters.shared.tg[tgid - 1]->sess_per_sec, 1); - + if (be->be_counters.shared.tg) { + _HA_ATOMIC_INC(&be->be_counters.shared.tg[tgid - 1]->cum_sess); + update_freq_ctr(&be->be_counters.shared.tg[tgid - 1]->sess_per_sec, 1); + } HA_ATOMIC_UPDATE_MAX(&be->be_counters.sps_max, update_freq_ctr(&be->be_counters._sess_per_sec, 1)); } @@ -222,10 +226,11 @@ static inline void proxy_inc_fe_req_ctr(struct listener *l, struct proxy *fe, if (http_ver >= sizeof(fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req) / sizeof(*fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req)) return; - _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req[http_ver]); - update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->req_per_sec, 1); - - if (l && l->counters) + if (fe->fe_counters.shared.tg) { + _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req[http_ver]); + update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->req_per_sec, 1); + } + if (l && l->counters && l->counters->shared.tg) _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->p.http.cum_req[http_ver]); HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.p.http.rps_max, update_freq_ctr(&fe->fe_counters.p.http._req_per_sec, 1)); diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 3dd438dd7..05446e98f 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -207,9 +207,10 @@ static inline void server_index_id(struct proxy *px, struct server *srv) /* increase the number of cumulated streams on the designated server */ static inline void srv_inc_sess_ctr(struct server *s) { - _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->cum_sess); - update_freq_ctr(&s->counters.shared.tg[tgid - 1]->sess_per_sec, 1); - + if (s->counters.shared.tg) { + _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->cum_sess); + update_freq_ctr(&s->counters.shared.tg[tgid - 1]->sess_per_sec, 1); + } HA_ATOMIC_UPDATE_MAX(&s->counters.sps_max, update_freq_ctr(&s->counters._sess_per_sec, 1)); } @@ -217,7 +218,8 @@ static inline void srv_inc_sess_ctr(struct server *s) /* set the time of last session on the designated server */ static inline void srv_set_sess_last(struct server *s) { - HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns)); + if (s->counters.shared.tg) + HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns)); } /* returns the current server throttle rate between 0 and 100% */ diff --git a/src/backend.c b/src/backend.c index 54787f5ed..3f00e1d19 100644 --- a/src/backend.c +++ b/src/backend.c @@ -823,7 +823,8 @@ int assign_server(struct stream *s) else if (srv != prev_srv) { if (s->be_tgcounters) _HA_ATOMIC_INC(&s->be_tgcounters->cum_lbconn); - _HA_ATOMIC_INC(&srv->counters.shared.tg[tgid - 1]->cum_lbconn); + if (srv->counters.shared.tg) + _HA_ATOMIC_INC(&srv->counters.shared.tg[tgid - 1]->cum_lbconn); } stream_set_srv_target(s, srv); } @@ -997,11 +998,13 @@ int assign_server_and_queue(struct stream *s) s->txn->flags |= TX_CK_DOWN; } s->flags |= SF_REDISP; - _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->redispatches); + if (prev_srv->counters.shared.tg) + _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->redispatches); if (s->be_tgcounters) _HA_ATOMIC_INC(&s->be_tgcounters->redispatches); } else { - _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->retries); + if (prev_srv->counters.shared.tg) + _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->retries); if (s->be_tgcounters) _HA_ATOMIC_INC(&s->be_tgcounters->retries); } diff --git a/src/cache.c b/src/cache.c index 67edf1889..5fd254059 100644 --- a/src/cache.c +++ b/src/cache.c @@ -2133,10 +2133,12 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p return ACT_RET_CONT; if (px == strm_fe(s)) { - _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_lookups); + if (px->fe_counters.shared.tg) + _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_lookups); } else { - _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_lookups); + if (px->be_counters.shared.tg) + _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_lookups); } cache_tree = get_cache_tree_from_hash(cache, read_u32(s->txn->cache_hash)); @@ -2224,10 +2226,12 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p should_send_notmodified_response(cache, htxbuf(&s->req.buf), res); if (px == strm_fe(s)) { - _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_hits); + if (px->fe_counters.shared.tg) + _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_hits); } else { - _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_hits); + if (px->be_counters.shared.tg) + _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_hits); } return ACT_RET_CONT; } else { diff --git a/src/check.c b/src/check.c index 0102f0c36..574700b2b 100644 --- a/src/check.c +++ b/src/check.c @@ -513,7 +513,8 @@ void set_server_check_status(struct check *check, short status, const char *desc if ((!(check->state & CHK_ST_AGENT) || (check->status >= HCHK_STATUS_L57DATA)) && (check->health > 0)) { - _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_checks); + if (s->counters.shared.tg) + _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_checks); report = 1; check->health--; if (check->health < check->rise) @@ -740,7 +741,8 @@ void __health_adjust(struct server *s, short status) HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); HA_ATOMIC_STORE(&s->consecutive_errors, 0); - _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_hana); + if (s->counters.shared.tg) + _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_hana); if (s->check.fastinter) { /* timer might need to be advanced, it might also already be diff --git a/src/server.c b/src/server.c index 9a2c3b852..024b03244 100644 --- a/src/server.c +++ b/src/server.c @@ -7159,7 +7159,8 @@ static void srv_update_status(struct server *s, int type, int cause) } else if (s->cur_state == SRV_ST_STOPPED) { /* server was up and is currently down */ - HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->down_trans); + if (s->counters.shared.tg) + HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->down_trans); _srv_event_hdl_publish(EVENT_HDL_SUB_SERVER_DOWN, cb_data.common, s); } @@ -7173,7 +7174,8 @@ static void srv_update_status(struct server *s, int type, int cause) } s->last_change = ns_to_sec(now_ns); - HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_state_change, s->last_change); + if (s->counters.shared.tg) + HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_state_change, s->last_change); /* publish the state change */ _srv_event_hdl_prepare_state(&cb_data.state, @@ -7193,7 +7195,8 @@ static void srv_update_status(struct server *s, int type, int cause) if (last_change < ns_to_sec(now_ns)) // ignore negative times s->proxy->down_time += ns_to_sec(now_ns) - last_change; s->proxy->last_change = ns_to_sec(now_ns); - HA_ATOMIC_STORE(&s->proxy->be_counters.shared.tg[tgid - 1]->last_state_change, s->proxy->last_change); + if (s->proxy->be_counters.shared.tg) + HA_ATOMIC_STORE(&s->proxy->be_counters.shared.tg[tgid - 1]->last_state_change, s->proxy->last_change); } }