From d3ffba4512087a8d78cfb31416135ee4765bbb41 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Mon, 13 Feb 2023 17:45:08 +0100 Subject: [PATCH] MINOR: listener: pause_listener() becomes suspend_listener() We are simply renaming pause_listener() to suspend_listener() to prevent confusion around listener pausing. A suspended listener can be in two differents valid states: - LI_PAUSED: the listener is effectively paused, it will unpause on resume_listener() - LI_ASSIGNED (not bound): the listener does not support the LI_PAUSED state, so it was unbound to satisfy the suspend request, it will correcly re-bind on resume_listener() Besides that, we add the LI_F_SUSPENDED flag to mark suspended listeners in suspend_listener() and unmark them in resume_listener(). We're also adding li_suspend proxy variable to track the number of currently suspended listeners: That is, the number of listeners that were suspended through suspend_listener() and that are either in LI_PAUSED or LI_ASSIGNED state. Counter is increased on successful suspend in suspend_listener() and it is decreased on successful resume in resume_listener() -- Backport notes: -> 2.4 only, as "MINOR: proxy/listener: support for additional PAUSED state" was not backported: Replace this: | /* PROXY_LOCK is require | proxy_cond_resume(px); By this: | ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id); | send_log(px, LOG_WARNING, "Resumed %s %s.\n", proxy_cap_str(px->cap), px->id); -> 2.6 and 2.7 only, as "MINOR: listener: make sure we don't pause/resume" was custom patched: Replace this: |@@ -253,6 +253,7 @@ struct listener { | | /* listener flags (16 bits) */ | #define LI_F_FINALIZED 0x0001 /* listener made it to the READY||LIMITED||FULL state at least once, may be suspended/resumed safely */ |+#define LI_F_SUSPENDED 0x0002 /* listener has been suspended using suspend_listener(), it is either is LI_PAUSED or LI_ASSIGNED state */ | | /* Descriptor for a "bind" keyword. The ->parse() function returns 0 in case of | * success, or a combination of ERR_* flags if an error is encountered. The By this: |@@ -222,6 +222,7 @@ struct li_per_thread { | | #define LI_F_QUIC_LISTENER 0x00000001 /* listener uses proto quic */ | #define LI_F_FINALIZED 0x00000002 /* listener made it to the READY||LIMITED||FULL state at least once, may be suspended/resumed safely */ |+#define LI_F_SUSPENDED 0x00000004 /* listener has been suspended using suspend_listener(), it is either is LI_PAUSED or LI_ASSIGNED state */ | | /* The listener will be directly referenced by the fdtab[] which holds its | * socket. The listener provides the protocol-specific accept() function to --- include/haproxy/listener-t.h | 1 + include/haproxy/listener.h | 4 +++- include/haproxy/proxy-t.h | 1 + src/listener.c | 18 +++++++++++++----- src/protocol.c | 8 ++++---- src/proxy.c | 2 +- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h index d9a32e09a..58dce2fe5 100644 --- a/include/haproxy/listener-t.h +++ b/include/haproxy/listener-t.h @@ -253,6 +253,7 @@ struct listener { /* listener flags (16 bits) */ #define LI_F_FINALIZED 0x0001 /* listener made it to the READY||LIMITED||FULL state at least once, may be suspended/resumed safely */ +#define LI_F_SUSPENDED 0x0002 /* listener has been suspended using suspend_listener(), it is either is LI_PAUSED or LI_ASSIGNED state */ /* Descriptor for a "bind" keyword. The ->parse() function returns 0 in case of * success, or a combination of ERR_* flags if an error is encountered. The diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h index 001737239..d7d6bf9e8 100644 --- a/include/haproxy/listener.h +++ b/include/haproxy/listener.h @@ -43,11 +43,13 @@ void listener_set_state(struct listener *l, enum li_state st); * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * is disabled. It normally returns non-zero, unless an error is reported. * It will need to operate under the proxy's lock and the listener's lock. + * suspend() may totally stop a listener if it doesn't support the PAUSED + * state, in which case state will be set to ASSIGNED. * The caller is responsible for indicating in lpx, lli whether the respective * locks are already held (non-zero) or not (zero) so that the function pick * the missing ones, in this order. */ -int pause_listener(struct listener *l, int lpx, int lli); +int suspend_listener(struct listener *l, int lpx, int lli); /* This function tries to resume a temporarily disabled listener. * The resulting state will either be LI_READY or LI_FULL. 0 is returned diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h index 3db2d3e18..50c84889d 100644 --- a/include/haproxy/proxy-t.h +++ b/include/haproxy/proxy-t.h @@ -403,6 +403,7 @@ struct proxy { unsigned int li_paused; /* total number of listeners paused (LI_PAUSED) */ unsigned int li_bound; /* total number of listeners ready (LI_LISTEN) */ unsigned int li_ready; /* total number of listeners ready (>=LI_READY) */ + unsigned int li_suspended; /* total number of listeners suspended (could be paused or unbound) */ /* warning: these structs are huge, keep them at the bottom */ struct sockaddr_storage dispatch_addr; /* the default address to connect to */ diff --git a/src/listener.c b/src/listener.c index dfd1fd9cf..ca13a7196 100644 --- a/src/listener.c +++ b/src/listener.c @@ -480,14 +480,14 @@ int default_resume_listener(struct listener *l) * closes upon SHUT_WR and refuses to rebind. So a common validation path * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * is disabled. It normally returns non-zero, unless an error is reported. - * pause() may totally stop a listener if it doesn't support the PAUSED state, - * in which case state will be set to ASSIGNED. + * suspend() may totally stop a listener if it doesn't support the PAUSED + * state, in which case state will be set to ASSIGNED. * It will need to operate under the proxy's lock and the listener's lock. * The caller is responsible for indicating in lpx, lli whether the respective * locks are already held (non-zero) or not (zero) so that the function pick * the missing ones, in this order. */ -int pause_listener(struct listener *l, int lpx, int lli) +int suspend_listener(struct listener *l, int lpx, int lli) { struct proxy *px = l->bind_conf->frontend; int ret = 1; @@ -518,6 +518,10 @@ int pause_listener(struct listener *l, int lpx, int lli) */ listener_set_state(l, ((ret) ? LI_PAUSED : LI_ASSIGNED)); + if (px && !(l->flags & LI_F_SUSPENDED)) + px->li_suspended++; + l->flags |= LI_F_SUSPENDED; + /* at this point, everything is under control, no error should be * returned to calling function */ @@ -545,7 +549,7 @@ int pause_listener(struct listener *l, int lpx, int lli) * or LI_FULL. 0 is returned in case of failure to resume (eg: dead socket). * Listeners bound to a different process are not woken up unless we're in * foreground mode, and are ignored. If the listener was only in the assigned - * state, it's totally rebound. This can happen if a pause() has completely + * state, it's totally rebound. This can happen if a suspend() has completely * stopped it. If the resume fails, 0 is returned and an error might be * displayed. * It will need to operate under the proxy's lock and the listener's lock. @@ -590,6 +594,10 @@ int resume_listener(struct listener *l, int lpx, int lli) listener_set_state(l, LI_READY); done: + if (px && (l->flags & LI_F_SUSPENDED)) + px->li_suspended--; + l->flags &= ~LI_F_SUSPENDED; + if (was_paused && !px->li_paused) { /* PROXY_LOCK is required */ proxy_cond_resume(px); @@ -1275,7 +1283,7 @@ void listener_accept(struct listener *l) * Let's put it to pause in this case. */ if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) { - pause_listener(l, 0, 0); + suspend_listener(l, 0, 0); goto end; } diff --git a/src/protocol.c b/src/protocol.c index 71bfa83f5..c627d5c73 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -174,10 +174,10 @@ void protocol_stop_now(void) HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); } -/* pauses all listeners of all registered protocols. This is typically +/* suspends all listeners of all registered protocols. This is typically * used on SIG_TTOU to release all listening sockets for the time needed to - * try to bind a new process. The listeners enter LI_PAUSED. It returns - * ERR_NONE, with ERR_FATAL on failure. + * try to bind a new process. The listeners enter LI_PAUSED or LI_ASSIGNED. + * It returns ERR_NONE, with ERR_FATAL on failure. */ int protocol_pause_all(void) { @@ -189,7 +189,7 @@ int protocol_pause_all(void) HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); list_for_each_entry(proto, &protocols, list) { list_for_each_entry(listener, &proto->receivers, rx.proto_list) - if (!pause_listener(listener, 0, 0)) + if (!suspend_listener(listener, 0, 0)) err |= ERR_FATAL; } HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); diff --git a/src/proxy.c b/src/proxy.c index fd5c49e03..93c7620a4 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -2286,7 +2286,7 @@ int pause_proxy(struct proxy *p) goto end; list_for_each_entry(l, &p->conf.listeners, by_fe) - pause_listener(l, 1, 0); + suspend_listener(l, 1, 0); if (p->li_ready) { ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id);