tree dd766e0fc404b70108542e4d2bd4b586a8112b84
parent b1918b176e234ddfda6380ccd8e9322523eba194
author Aurelien DARRAGON <adarragon@haproxy.com> 1676361074 +0100
committer Christopher Faulet <cfaulet@haproxy.com> 1690186672 +0200

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

(cherry picked from commit 2370599f966744f09101793b5c3ce5220786363d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 72ccd49d493e6098b9fedae4115cd578189b8d9c)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit e97d84bbbf5e1dc758957e44ddaf13b7a80381ae)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit d39d20db3fe6a67fa59173d8968a1a02924bd2ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>