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
This commit is contained in:
Aurelien DARRAGON 2023-02-14 08:51:14 +01:00 committed by Christopher Faulet
parent f5d98938ad
commit 2370599f96
2 changed files with 7 additions and 3 deletions

View File

@ -226,7 +226,7 @@ struct li_per_thread {
struct listener { struct listener {
enum obj_type obj_type; /* object type = OBJ_TYPE_LISTENER */ enum obj_type obj_type; /* object type = OBJ_TYPE_LISTENER */
enum li_state state; /* state: NEW, INIT, ASSIGNED, LISTEN, READY, FULL */ 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 luid; /* listener universally unique ID, used for SNMP */
int nbconn; /* current number of connections on this listener */ int nbconn; /* current number of connections on this listener */
unsigned int thr_idx; /* thread indexes for queue distribution : (t2<<16)+t1 */ unsigned int thr_idx; /* thread indexes for queue distribution : (t2<<16)+t1 */
@ -251,6 +251,9 @@ struct listener {
EXTRA_COUNTERS(extra_counters); 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 /* 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 * 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 * function pointer can be NULL if not implemented. The function also has an

View File

@ -288,6 +288,7 @@ void listener_set_state(struct listener *l, enum li_state st)
case LI_LIMITED: case LI_LIMITED:
BUG_ON(l->rx.fd == -1); BUG_ON(l->rx.fd == -1);
_HA_ATOMIC_INC(&px->li_ready); _HA_ATOMIC_INC(&px->li_ready);
l->flags |= LI_F_FINALIZED;
break; break;
} }
} }
@ -477,7 +478,7 @@ int pause_listener(struct listener *l, int lpx, int lli)
if (!lli) if (!lli)
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
if (l->state <= LI_PAUSED) if (!(l->flags & LI_F_FINALIZED) || l->state <= LI_PAUSED)
goto end; goto end;
if (l->rx.proto->suspend) 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)) if (MT_LIST_INLIST(&l->wait_queue))
goto end; goto end;
if (l->state == LI_READY) if (!(l->flags & LI_F_FINALIZED) || l->state == LI_READY)
goto end; goto end;
if (l->rx.proto->resume) if (l->rx.proto->resume)