From 2370599f966744f09101793b5c3ce5220786363d Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Tue, 14 Feb 2023 08:51:14 +0100 Subject: [PATCH] MINOR: listener: make sure we don't pause/resume bypassed listeners Some listeners are kept in LI_ASSIGNED state but are not supposed to be started since they were bypassed on initial startup (eg: in protocol_bind_all() or in enable_listener()...) Introduce the LI_F_FINALIZED flag: when the variable is non zero it means that the listener made it past the LI_LISTEN state (finalized) at least once so we can safely pause / resume. This way we won't risk starting a previously bypassed listener which never made it that far and thus was not expected to be lazy-started by accident. As listener_pause() and listener_resume() are currently partially broken, such unexpected lazy-start won't happen. But we're trying to restore pause() and resume() behavior so this patch will be required before going any further. We had to re-introduce listeners 'flags' struct member since it was recently moved into bind_conf struct. But here we do have a legitimate need for these listener-only flags. This should only be backported if explicitly required by another commit. -- Backport notes: -> 2.4 and 2.5: The 2-bytes hole we're using in the current patch does not apply, let's use the 4-byte hole located under the 'option' field. Replace this: |@@ -226,7 +226,8 @@ struct li_per_thread { | struct listener { | enum obj_type obj_type; /* object type = OBJ_TYPE_LISTENER */ | enum li_state state; /* state: NEW, INIT, ASSIGNED, LISTEN, READY, FULL */ |- /* 2-byte hole here */ |+ uint16_t flags; /* listener flags: LI_F_* */ | int luid; /* listener universally unique ID, used for SNMP */ | int nbconn; /* current number of connections on this listener */ | unsigned int thr_idx; /* thread indexes for queue distribution : (t2<<16)+t1 */ By this: |@@ -209,6 +209,8 @@ struct listener { | short int nice; /* nice value to assign to the instantiated tasks */ | int luid; /* listener universally unique ID, used for SNMP */ | int options; /* socket options : LI_O_* */ |+ uint16_t flags; /* listener flags: LI_F_* */ |+ /* 2-bytes hole here */ | __decl_thread(HA_RWLOCK_T lock); | | struct fe_counters *counters; /* statistics counters */ -> 2.4 only: We need to adjust some contextual lines. Replace this: |@@ -477,7 +478,7 @@ int pause_listener(struct listener *l, int lpx, int lli) | if (!lli) | HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); | |- if (l->state <= LI_PAUSED) |+ if (!(l->flags & LI_F_FINALIZED) || l->state <= LI_PAUSED) | goto end; | | if (l->rx.proto->suspend) By this: |@@ -477,7 +478,7 @@ int pause_listener(struct listener *l, int lpx, int lli) | !(proc_mask(l->rx.settings->bind_proc) & pid_bit)) | goto end; | |- if (l->state <= LI_PAUSED) |+ if (!(l->flags & LI_F_FINALIZED) || l->state <= LI_PAUSED) | goto end; | | if (l->rx.proto->suspend) And this: |@@ -535,7 +536,7 @@ int resume_listener(struct listener *l, int lpx, int lli) | if (MT_LIST_INLIST(&l->wait_queue)) | goto end; | |- if (l->state == LI_READY) |+ if (!(l->flags & LI_F_FINALIZED) || l->state == LI_READY) | goto end; | | if (l->rx.proto->resume) By this: |@@ -535,7 +536,7 @@ int resume_listener(struct listener *l, int lpx, int lli) | !(proc_mask(l->rx.settings->bind_proc) & pid_bit)) | goto end; | |- if (l->state == LI_READY) |+ if (!(l->flags & LI_F_FINALIZED) || l->state == LI_READY) | goto end; | | if (l->rx.proto->resume) -> 2.6 and 2.7 only: struct listener 'flags' member still exists, let's use it. Remove this from the current patch: |@@ -226,7 +226,8 @@ struct li_per_thread { | struct listener { | enum obj_type obj_type; /* object type = OBJ_TYPE_LISTENER */ | enum li_state state; /* state: NEW, INIT, ASSIGNED, LISTEN, READY, FULL */ |- /* 2-byte hole here */ |+ uint16_t flags; /* listener flags: LI_F_* */ | int luid; /* listener universally unique ID, used for SNMP */ | int nbconn; /* current number of connections on this listener */ | unsigned int thr_idx; /* thread indexes for queue distribution : (t2<<16)+t1 */ Then, replace this: |@@ -251,6 +250,9 @@ struct listener { | EXTRA_COUNTERS(extra_counters); | }; | |+/* 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 */ |+ | /* 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 | * function pointer can be NULL if not implemented. The function also has an By this: |@@ -221,6 +221,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 */ | | /* 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 | 5 ++++- src/listener.c | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h index 1fe25f59d..d9a32e09a 100644 --- a/include/haproxy/listener-t.h +++ b/include/haproxy/listener-t.h @@ -226,7 +226,7 @@ struct li_per_thread { struct listener { enum obj_type obj_type; /* object type = OBJ_TYPE_LISTENER */ enum li_state state; /* state: NEW, INIT, ASSIGNED, LISTEN, READY, FULL */ - /* 2-byte hole here */ + uint16_t flags; /* listener flags: LI_F_* */ int luid; /* listener universally unique ID, used for SNMP */ int nbconn; /* current number of connections on this listener */ unsigned int thr_idx; /* thread indexes for queue distribution : (t2<<16)+t1 */ @@ -251,6 +251,9 @@ struct listener { EXTRA_COUNTERS(extra_counters); }; +/* 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 */ + /* 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 * function pointer can be NULL if not implemented. The function also has an diff --git a/src/listener.c b/src/listener.c index 08dd5ecad..25b22e13c 100644 --- a/src/listener.c +++ b/src/listener.c @@ -288,6 +288,7 @@ void listener_set_state(struct listener *l, enum li_state st) case LI_LIMITED: BUG_ON(l->rx.fd == -1); _HA_ATOMIC_INC(&px->li_ready); + l->flags |= LI_F_FINALIZED; break; } } @@ -477,7 +478,7 @@ int pause_listener(struct listener *l, int lpx, int lli) if (!lli) HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); - if (l->state <= LI_PAUSED) + if (!(l->flags & LI_F_FINALIZED) || l->state <= LI_PAUSED) goto end; if (l->rx.proto->suspend) @@ -535,7 +536,7 @@ int resume_listener(struct listener *l, int lpx, int lli) if (MT_LIST_INLIST(&l->wait_queue)) goto end; - if (l->state == LI_READY) + if (!(l->flags & LI_F_FINALIZED) || l->state == LI_READY) goto end; if (l->rx.proto->resume)