From: Aurelien DARRAGON Date: Mon, 3 Apr 2023 15:40:28 +0000 (+0200) Subject: MINOR: server: change adm_st_chg_cause storage type X-Git-Tag: v2.8-dev8~51 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b1ccd732530d8d9a97e9800131456b9cb09d9f9;p=thirdparty%2Fhaproxy.git MINOR: server: change adm_st_chg_cause storage type Even though it doesn't look like it at first glance, this is more like a cleanup than an actual code improvement: Given that srv->adm_st_chg_cause has been used to exclusively store static strings ever since it was implemented, we make the choice to store it as an enum instead of a fixed-size string within server struct. This will allow to save some space in server struct, and will make it more easily exportable (ie: event handlers) because of the reduced memory footprint during handling and the ability to later get the corresponding human-readable message when it's explicitly needed. --- diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index d465fab58a..003605427d 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -189,6 +189,19 @@ enum srv_log_proto { SRV_LOG_PROTO_OCTET_COUNTING, // TCP frames: MSGLEN SP MSG }; +/* srv administrative change causes */ +enum srv_adm_st_chg_cause { + SRV_ADM_STCHGC_NONE = 0, + SRV_ADM_STCHGC_DNS_NOENT, /* entry removed from srv record */ + SRV_ADM_STCHGC_DNS_NOIP, /* no server ip in the srv record */ + SRV_ADM_STCHGC_DNS_NX, /* resolution spent too much time in NX state */ + SRV_ADM_STCHGC_DNS_TIMEOUT, /* resolution timeout */ + SRV_ADM_STCHGC_DNS_REFUSED, /* query refused by dns server */ + SRV_ADM_STCHGC_DNS_UNSPEC, /* unspecified dns error */ + SRV_ADM_STCHGC_STATS_DISABLE, /* legacy disable from the stats */ + SRV_ADM_STCHGC_STATS_STOP /* legacy stop from the stats */ +}; + struct pid_list { struct list list; pid_t pid; @@ -403,8 +416,8 @@ struct server { long duration; short status, code; char reason[128]; - } op_st_chg; /* operational status change's reason */ - char adm_st_chg_cause[48]; /* administrative status change's cause */ + } op_st_chg; /* operational status change's reason */ + enum srv_adm_st_chg_cause adm_st_chg_cause; /* administrative status change's cause */ event_hdl_sub_list e_subs; /* event_hdl: server's subscribers list (atomically updated) */ diff --git a/include/haproxy/server.h b/include/haproxy/server.h index e5866a5528..f58afd1e87 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -64,6 +64,7 @@ void srv_take(struct server *srv); struct server *srv_drop(struct server *srv); int srv_init_per_thr(struct server *srv); void srv_set_ssl(struct server *s, int use_ssl); +const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause); /* functions related to server name resolution */ int srv_prepare_for_resolution(struct server *srv, const char *hostname); @@ -152,7 +153,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check) * is done. If is non-null, it will be displayed at the end of the log * lines to justify the state change. */ -void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause); +void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause); /* Disables admin flag (among SRV_ADMF_*) on server . This is used to * stop enforcing either maint mode or drain mode. It is not allowed to set more @@ -212,7 +213,7 @@ static inline int server_is_draining(const struct server *s) */ static inline void srv_adm_set_maint(struct server *s) { - srv_set_admin_flag(s, SRV_ADMF_FMAINT, NULL); + srv_set_admin_flag(s, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_NONE); srv_clr_admin_flag(s, SRV_ADMF_FDRAIN); } @@ -221,7 +222,7 @@ static inline void srv_adm_set_maint(struct server *s) */ static inline void srv_adm_set_drain(struct server *s) { - srv_set_admin_flag(s, SRV_ADMF_FDRAIN, NULL); + srv_set_admin_flag(s, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_NONE); srv_clr_admin_flag(s, SRV_ADMF_FMAINT); } diff --git a/src/server.c b/src/server.c index f62a7e3962..9483e8972f 100644 --- a/src/server.c +++ b/src/server.c @@ -104,6 +104,23 @@ struct dict server_key_dict = { .values = EB_ROOT_UNIQUE, }; +static const char *srv_adm_st_chg_cause_str[] = { + [SRV_ADM_STCHGC_NONE] = "", + [SRV_ADM_STCHGC_DNS_NOENT] = "entry removed from SRV record", + [SRV_ADM_STCHGC_DNS_NOIP] = "No IP for server ", + [SRV_ADM_STCHGC_DNS_NX] = "DNS NX status", + [SRV_ADM_STCHGC_DNS_TIMEOUT] = "DNS timeout status", + [SRV_ADM_STCHGC_DNS_REFUSED] = "DNS refused status", + [SRV_ADM_STCHGC_DNS_UNSPEC] = "unspecified DNS error", + [SRV_ADM_STCHGC_STATS_DISABLE] = "'disable' on stats page", + [SRV_ADM_STCHGC_STATS_STOP] = "'stop' on stats page" +}; + +const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause) +{ + return srv_adm_st_chg_cause_str[cause]; +} + int srv_downtime(const struct server *s) { if ((s->cur_state != SRV_ST_STOPPED) || s->last_change >= now.tv_sec) // ignore negative time @@ -1702,7 +1719,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check) * * Must be called with the server lock held. */ -void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause) +void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause) { struct server *srv; @@ -1714,8 +1731,7 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause return; s->next_admin |= mode; - if (cause) - strlcpy2(s->adm_st_chg_cause, cause, sizeof(s->adm_st_chg_cause)); + s->adm_st_chg_cause = cause; /* propagate changes */ srv_update_status(s); @@ -1792,10 +1808,10 @@ static void srv_propagate_admin_state(struct server *srv) for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) { HA_SPIN_LOCK(SERVER_LOCK, &srv2->lock); if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT)) - srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, NULL); + srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, SRV_ADM_STCHGC_NONE); if (srv->next_admin & SRV_ADMF_DRAIN) - srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, NULL); + srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, SRV_ADM_STCHGC_NONE); HA_SPIN_UNLOCK(SERVER_LOCK, &srv2->lock); } } @@ -3510,7 +3526,7 @@ int srvrq_update_srv_status(struct server *s, int has_no_ip) if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, "entry removed from SRV record"); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOENT); return 0; } @@ -3546,8 +3562,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip) if (has_no_ip) { if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, - "No IP for server "); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOIP); return 0; } @@ -3569,7 +3584,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip) if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS NX status"); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NX); return 0; case RSLV_STATUS_TIMEOUT: @@ -3580,7 +3595,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip) if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS timeout status"); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_TIMEOUT); return 0; case RSLV_STATUS_REFUSED: @@ -3591,7 +3606,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip) if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS refused status"); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_REFUSED); return 0; default: @@ -3602,7 +3617,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip) if (s->next_admin & SRV_ADMF_RMAINT) return 1; - srv_set_admin_flag(s, SRV_ADMF_RMAINT, "unspecified DNS error"); + srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_UNSPEC); return 0; } @@ -4021,7 +4036,7 @@ static int srv_iterate_initaddr(struct server *srv) break; case SRV_IADDR_NONE: - srv_set_admin_flag(srv, SRV_ADMF_RMAINT, NULL); + srv_set_admin_flag(srv, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_NONE); if (return_code) { ha_warning("could not resolve address '%s', disabling server.\n", name); @@ -5447,7 +5462,9 @@ static void srv_update_status(struct server *s) chunk_printf(tmptrash, "%sServer %s/%s was DOWN and now enters maintenance%s%s%s", s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id, - *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); + (s->adm_st_chg_cause) ? " (" : "", + srv_adm_st_chg_cause(s->adm_st_chg_cause), + (s->adm_st_chg_cause) ? ")" : ""); srv_append_status(tmptrash, s, NULL, -1, (s->next_admin & SRV_ADMF_FMAINT)); @@ -5486,7 +5503,9 @@ static void srv_update_status(struct server *s) "%sServer %s/%s is going DOWN for maintenance%s%s%s", s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id, - *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); + (s->adm_st_chg_cause) ? " (" : "", + srv_adm_st_chg_cause(s->adm_st_chg_cause), + (s->adm_st_chg_cause) ? ")" : ""); srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FMAINT)); @@ -5669,7 +5688,9 @@ static void srv_update_status(struct server *s) if (tmptrash) { chunk_printf(tmptrash, "%sServer %s/%s enters drain state%s%s%s", s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id, - *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); + (s->adm_st_chg_cause) ? " (" : "", + srv_adm_st_chg_cause(s->adm_st_chg_cause), + (s->adm_st_chg_cause) ? ")" : ""); srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FDRAIN)); @@ -5748,8 +5769,8 @@ static void srv_update_status(struct server *s) } } - /* Re-set log strings to empty */ - *s->adm_st_chg_cause = 0; + /* Re-set adm st change to none */ + s->adm_st_chg_cause = SRV_ADM_STCHGC_NONE; /* explicitly commit state changes (even if it was already applied implicitly * by some lb state change function), so we don't miss anything diff --git a/src/stats.c b/src/stats.c index 1ba4008daa..7bb7f81900 100644 --- a/src/stats.c +++ b/src/stats.c @@ -4170,7 +4170,7 @@ static int stats_process_http_post(struct stconn *sc) if (!(sv->cur_admin & SRV_ADMF_FMAINT)) { altered_servers++; total_servers++; - srv_set_admin_flag(sv, SRV_ADMF_FMAINT, "'disable' on stats page"); + srv_set_admin_flag(sv, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_STATS_DISABLE); } break; case ST_ADM_ACTION_ENABLE: @@ -4182,7 +4182,7 @@ static int stats_process_http_post(struct stconn *sc) break; case ST_ADM_ACTION_STOP: if (!(sv->cur_admin & SRV_ADMF_FDRAIN)) { - srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, "'stop' on stats page"); + srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_STATS_STOP); altered_servers++; total_servers++; }