DOC: config: Clarify the meaning of 'hold' in the 'resolvers' section

This patch improves the 'hold' parameter description in the 'resolvers'
section to make it clearer. It really explains differences between all
status. Thanks to Nick Ramirez for this update.

This patch should solve the issue #1694. It could be backported to all
stable versions.

MEDIUM: mux-quic: Don't expect data from server as long as request is unfinished

As for the H1 and H2 stream, the QUIC stream now states it does not expect
data from the server as long as the request is unfinished. The aim is the
same. We must be sure to not trigger a read timeout on server side if the
client is still uploading data.

From the moment the end of the request is received and forwarded to upper
layer, the QUIC stream reports it expects to receive data from the opposite
endpoint. This re-enables read timeout on the server side.

MEDIUM: mux-h2: Don't expect data from server as long as request is unfinished

As for the H1 stream, the H2 stream now states it does not expect data from
the server as long as the request is unfinished. The aim is the same. We
must be sure to not trigger a read timeout on server side if the client is
still uploading data.

From the moment the end of the request is received and forwarded to upper
layer, the H2 stream reports it expects to receive data from the opposite
endpoint. This re-enables read timeout on the server side.

MEDIUM: mux-h1: Don't expect data from server as long as request is unfinished

On client side, as long as the request is unfinished, the H1 stream states
it does not expect data from the server. It does not mean the server must
not send its response but only it may wait to receive the whole request with
no risk to trigger a read timeout.

When the request is finished, the H1 stream reports it expects to receive
data from the opposite endpoint.

The purpose of this patch is to never report a server timeout on receive if
the client is still uploading data. This way, it is possible to have a
smaller server timeout than the client one.

BUG/MEDIUM: stconn: Report a blocked send if some output data are not consumed

Instead of reporting a blocked send if nothing is send, we do it if some
output data remain blocked after a write attempts or after a call the the
applet's I/O handler. It is mandatory to properly handle write timeouts.

Indeed, if an endpoint is blocked for a while but it partially consumed
output data, no timeout is triggered. It is especially true for
connections. But the same may happen for applet, there is no reason.

Of course, if the endpoint decides to partially consume output data because
it must wait to move on for any reason, it should use the se/applet API
(se/applet_will_consume(), se/applet_wont_consume() and
se/applet_need_more_data()).

This bug was introduced during the channels timeouts refactoring. No
backport is needed.

MINOR: stconn: Report a send activity when endpoint is willing to consume data

When the endpoint (applet or mux) is now willing to consume data while it
said it wouldn't, a send activity is reported. Indeed, the writes was
blocked because of the endpoint. It is now ready to consume outgoing
data. So an send activity must be reported to reset corresponding timers.

Concretly, when the flag SE_FL_WONT_CONSULE is removed, a send activity is
reported.

MEDIUM: stream: Eventually handle stream timeouts when exiting process_stream()

When we exit from process_stream(), if the task is expired, we try to handle
the stream timeouts and we resync the stream-connectors. This avoids a
useless immediate wakeup. It is not really an issue, but it is a small
improvement in edge cases.

MINOR: stream: Handle stream's timeouts in a dedicated function

This will be mandatory to be able to handle stream's timeouts before exiting
process_stream(). So, to not duplicate code, all this stuff is moved in a
dedicated function.

BUG/MINOR: stream: Remove BUG_ON about the task expiration in process_stream()

At the end of process_stream(), A BUG_ON was recently added to abort if we
leave the function with an expired task. However, it may happen if an event
prevents the timeout to be handled but nothing evolved. In this case, the
task expiration is not updated and we expect to catch the timeout on the
immediate task wakeup.

No backport needed.

BUG/MEDIUM: h1-htx: Never copy more than the max data allowed during parsing

A bug during H1 data parsing may lead to copy more data than the maximum
allowed. The bug is an overflow on this max threshold when it is lower than
the size of an htx_blk structure.

At first glance, it means it is possible to not respsect the buffer's
reserve. So it may lead to rewrite errors but it may also block any progress
on the stream if the compression is enabled. In this case, the channel
buffer appears as full and the compression must wait for space to
proceed. Outside of any bug, it is only possible when there are outgoing
data to forward, so the compression filter just waits. Because of this bug,
there is nothing to forward. The buffer is just full of input data. Thus
nothing move and the stream is infinitly blocked.

To fix the bug, we must be sure to be able to create an HTX block of 1 byte
without exceeding the maximum allowed.

This patch should fix the issue #2053. It must be backported as far as 2.5.

BUG/MEDIUM: fd: avoid infinite loops in fd_add_to_fd_list and fd_rm_from_fd_list

With 4d9888c ("CLEANUP: fd: get rid of the __GET_{NEXT,PREV} macros") some
"volatile" keywords were dropped at various assignment places in fd's code.

In fd_add_to_fd_list() and fd_add_to_fd_list(), because of the absence of
the "volatile" keyword: the compiler was able to perform some code
optimizations that prevented prev and next variables from being reloaded
between locking attempts (goto loop).

The result was that fd_add_to_fd_list() and fd_rm_from_fd_list() could enter in
infinite loops, preventing other threads from working further and ultimately
resulting in the watchdog being triggered as described in GH #2011.

To fix this, we made sure to re-audit 4d9888c in order to restore the required
memory barriers / compilers hints to prevent the compiler from mis-optimizing
the code around the fd's locks.
That is: using atomic loads to fetch the prev and next values, and restoring
the "volatile" cast for cur_list.ent variable assignment in fd_rm_from_fd_list()

Big thanks to @xanaxalan for his help and patience and to @wtarreau for his
findings and explanations in regard to compiler's optimizations.

This must be backported in 2.7 with 4d9888c ("CLEANUP: fd: get rid of the
__GET_{NEXT,PREV} macros")

BUILD: thead: Fix several 32 bits compilation issues with uint64_t variables

Cast uint64_t as ullong and difference between two uint64_t as llong.

MINOR: config: add HAPROXY_BRANCH environment variable

This patch adds support from HAPROXY_BRANCH environment variable.
It can be useful is some resources are loaded from different
locations when migrating from one version to another.

Signed-off-by: Sébastien Gross <sgross@haproxy.com>

CLEANUP: ring: remove the now unused ring's offset

Since the previous patch, the ring's offset is not used anymore. The
haring utility remains backward-compatible since it can trust the
buffer element that's at the beginning of the map and which still
contains all the valid data.

MEDIUM: ring: make the offset relative to the head/tail instead of absolute

The ring's offset currently contains a perpetually growing custor which
is the number of bytes written from the start. It's used by readers to
know where to (re)start reading from. It was made absolute because both
the head and the tail can change during writes and we needed a fixed
position to know where the reader was attached. But this is complicated,
error-prone, and limits the ability to reduce the lock's coverage. In
fact what is needed is to know where the reader is currently waiting, if
at all. And this location is exactly where it stored its count, so the
absolute position in the buffer (the seek offset from the first storage
byte) does represent exactly this, as it doesn't move (we don't realign
the buffer), and is stable regardless of how head/tail changes with writes.

This patch modifies this so that the application code now uses this
representation instead. The most noticeable change is the initialization,
where we've kept ~0 as a marker to go to the end, and it's now set to
the tail offset instead of trying to resolve the current write offset
against the current ring's position.

The offset was also used at the end of the consuming loop, to detect
if a new write had happened between the lock being released and taken
again, so as to wake the consumer(s) up again. For this we used to
take a copy of the ring->ofs before unlocking and comparing with the
new value read in the next lock. Since it's not possible to write past
the current reader's location, there's no risk of complete rollover, so
it's sufficient to check if the tail has changed.

Note that the change also has an impact on the haring consumer which
needs to adapt as well. But that's good in fact because it will rely
on one less variable, and will use offsets relative to the buffer's
head, and the change remains backward-compatible.

BUG/MINOR: ring: do not realign ring contents on resize

If a ring is resized, we must not zero its head since the contents
are preserved in-situ. Till now it used to work because we only resize
during boot and we emit very few data (if at all) during boot. But this
can change in the future.

This can be backported to 2.2 though no older version should notice a
difference.

BUILD: quic: 32-bits compilation issue with %zu in quic_rx_pkts_del()

This issue arrived with this commit:
    1dbeb35f8 MINOR: quic: Add new traces about by connection RX buffer handling

and revealed by the GH CI as follows:

   src/quic_conn.c: In function â\80\98quic_rx_pkts_delâ\80\99:
    include/haproxy/trace.h:134:65: error: format â\80\98%zuâ\80\99 expects argument of type â\80\98size_tâ\80\99,
    but argument 6 has type â\80\98uint64_tâ\80\99 {aka â\80\98long long unsigned intâ\80\99} [-Werror=format=]
    _msg_len = snprintf(_msg, sizeof(_msg), (fmt), ##args);

Replace all %zu printf integer format by %llu.

Must be backported to 2.7 where the previous is supposed to be backported.

MINOR: haproxy: always protocol unbind on startup error path

In haproxy startup, all init error paths after the protocol bind step
cautiously call protocol_unbind_all() before exiting except one that was
conditional. We're not making an exception to the rule and we now properly
call protocol_unbind_all() as well.

No backport needed as this patch is unnoticeable.

MINOR: proto_ux: ability to dump ABNS names in error messages

In sock_unix_bind_receiver(), uxst_bind_listener() and uxdg_bind_listener(),
properly dump ABNS socket names by leveraging sa2str() function which does the
hard work for us.

UNIX sockets are reported as is (unchanged) while ABNS UNIX sockets
are prefixed with 'abns@' to match the syntax used in config file.
(they where previously showing as empty strings because of the leading
NULL-byte that was not properly handled in this case)

This is only a minor debug improvement, however it could be useful to
backport it up to 2.4.
[for 2.4: you should replace "%s [%s]" by "%s for [%s]" for uxst and uxgd if
you wan't the patch to apply properly]

MEDIUM: proto_ux: properly suspend named UNIX listeners

When a listener is suspended, we expect that it may not process any data for
the time it is suspended.

Yet for named UNIX socket, as the suspend operation is a no-op at the proto
level, recv events on the socket may still be processed by the polling loop.

This is quite disturbing as someone may rely on a paused proxy being harmless,
which is true for all protos except for named UNIX sockets.

To fix this behavior, we explicitely disable io recv events when suspending a
named UNIX socket listener (we call disable() method on the listener).

The io recv events will automatically be restored when the listener is resumed
since the l->enable() method is called at the end of the resume() operation.

This could be backported up to 2.4 after a reasonable observation
period to make sure that this change doesn't cause unwanted side-effects.

BUG/MINOR: sock_unix: match finalname with tempname in sock_unix_addrcmp()

In sock_unix_addrcmp(), named UNIX sockets paths are manually compared in
order to properly handle tempname paths (ending with ".XXXX.tmp") that result
from the 2-step bind implemented in sock_unix_bind_receiver().

However, this logic does not take into account "final" path names (without the
".XXXX.tmp" suffix).

Example:
  /tmp/test did not match with /tmp/test.1288.tmp prior to this patch

Indeed, depending on how the socket addr is retrieved, the same socket
could be designated either by its tempname or finalname.
socket addr is normally stored with its finalname within a receiver, but a
call to getsockname() on the same socket will return the tempname that was
used for the bind() call (sock_get_old_sockets() depends on getsockname()).

This causes sock_find_compatible_fd() to malfunction with named UNIX
sockets (ie: haproxy -x CLI option).

To fix this, we slightly modify the check around the temp suffix in
sock_unix_addrcmp(): we perform the suffix check even if one of the
paths is lacking the temp suffix (with proper precautions).

Now the function is able to match:
  - finalname x finalname
  - tempname  x tempname
  - finalname x tempname

That is: /tmp/test == /tmp/test.1288.tmp == /tmp/test.X.tmp

It should be backported up to 2.4

BUG/MEDIUM: listener/proxy: fix listeners notify for proxy resume

In 58651b42f ("MEDIUM: listener/proxy: make the listeners notify about
proxy pause/resume") we introduced the logic for pause/resume notify using
li_ready for pause and li_paused for resume.

Unfortunately, relying on li_paused for resume doesn't work reliably if we
resume a listener which is only made of receivers that are completely stopped.
For example, this could happen with receivers that don't support the
LI_PAUSED state like ABNS sockets.

This is especially true since pause_listener() was renamed to
suspend_listener() to better reflect its actual behavior in
("MINOR: listener:  pause_listener() becomes suspend_listener())

To fix this, we now rely on the li_suspended state in resume_listener() to make
sure that suspend_listener() and resume_listener() notify messages are
consistent to each other:

"Proxy pause" is triggered when there are no more ready listeners.
"Proxy resume" is triggered when there are no more suspended listeners.

Also, we make use of the new PR_FL_PAUSED proxy flag to make sure we don't
report the same event twice.

This could be backported up to 2.4 after a reasonable observation
period to make sure that this change doesn't cause unwanted side-effects.

--
Backport notes:

This commit depends on:
 - "MINOR: listener: pause_listener() becomes suspend_listener()"

-> 2.4 only, as "MINOR: proxy/listener: support for additional PAUSED state"
was not backported:

Replace this:

    |+       if (px && !(px->flags & PR_FL_PAUSED) && !px->li_ready) {
    |                /* PROXY_LOCK is required */
    |                proxy_cond_pause(px);
    |                ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);

By this:

    |+       if (px && !px->li_ready) {
    |                ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
    |                send_log(px, LOG_WARNING, "Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
    |        }

And this:

    |+       if (px && (px->flags & PR_FL_PAUSED) && !px->li_suspended) {
    |                /* PROXY_LOCK is required */
    |                proxy_cond_resume(px);
    |                ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);

By this:

    |+       if (px && !px->li_suspended) {
    |                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);
    |        }

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

BUG/MEDIUM: resume from LI_ASSIGNED in default_resume_listener()

Since fc974887c ("MEDIUM: protocol: explicitly start the receiver before
the listener"), resume from LI_ASSIGNED state does not work anymore.

This is because the binding part has been divided into 2 distinct steps
since: first bind(), then listen().

This new logic was properly implemented in startup sequence
through protocol_bind_all() but wasn't properly reported in
default_resume_listener() function.

Fixing default_resume_listener() to comply with the new logic.

This should help ABNS sockets to properly rebind in resume_listener()
after they have been stopped by pause_listener():
See Redmine:4475 for more context.

This commit depends on:
 - "MINOR: listener: workaround for closing a tiny race between resume_listener() and stopping"
 - "MINOR: listener: make sure we don't pause/resume bypassed listeners"

This could be backported up to 2.4 after a reasonable observation period to
make sure that this change doesn't cause unwanted side-effects.

BUG/MINOR: listener: fix resume_listener() resume return value handling

In resume_listener(), proto->resume() errors were not properly handled:
the function kept flowing down as if no errors were detected.

Instead, we're performing an early return when such errors are detected to
prevent undefined behaviors.

This could be backported up to 2.4.

--
Backport notes:

This commit depends on:
 - "MINOR: listener: make sure we don't pause/resume bypassed listeners"

-> 2.4 ... 2.7:

Replace this:

    |        if (l->bind_conf->maxconn && l->nbconn >= l->bind_conf->maxconn) {
    |                l->rx.proto->disable(l);

By this:

    |        if (l->maxconn && l->nbconn >= l->maxconn) {
    |                l->rx.proto->disable(l);

BUG/MEDIUM: listener: fix pause_listener() suspend return value handling

Legacy suspend() return value handling in pause_listener() has been altered
over the time.

First with fb76bd5ca ("BUG/MEDIUM: listeners: correctly report pause() errors")
Then with e03204c8e ("MEDIUM: listeners: implement protocol level
->suspend/resume() calls")

We aim to restore original function behavior and comply with resume_listener()
function description.
This is required for resume_listener() and pause_listener() to work as a whole

Now, it is made explicit that pause_listener() may stop a listener if the
listener doesn't support the LI_PAUSED state (depending on the protocol
family, ie: ABNS sockets), in this case l->state will be set to LI_ASSIGNED
and this won't be considered as an error.

This could be backported up to 2.4 after a reasonable observation period
to make sure that this change doesn't cause unwanted side-effects.

--
Backport notes:

This commit depends on:
 - "MINOR: listener: make sure we don't pause/resume bypassed listeners"

-> 2.4: manual change required because "MINOR: proxy/listener: support
for additional PAUSED state" was not backported: the contextual patch
lines don't match.

Replace this:

    |        if (px && !px->li_ready) {
    |                /* PROXY_LOCK is required */

By this:

    |        if (px && !px->li_ready) {
    |               ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);

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

MINOR: listener: workaround for closing a tiny race between resume_listener() and stopping

This is an alternative fix that tries to address the same issue as
d1ebee177 ("BUG/MINOR: listener: close tiny race between
resume_listener() and stopping") while allowing resume_listener() to be
more versatile.

Indeed, because of the previous fix, resume_listener() is not able to
rebind stopped listeners, and this breaks the original behavior that is
documented in the function description:

 "If the listener was only in the assigned
  state, it's totally rebound. This can happen if a pause() has completely
  stopped it. If the resume fails, 0 is returned and an error might be
  displayed."

With relax_listener(), we now make sure to check l->state under the
listener lock so we don't call resume_listener() when the conditions are not
met.

As such, concurrently stopped listeners may not be rebound using
relax_listener().

Note: the documented race can't happen since 1b927eb3c ("MEDIUM: proto: stop
protocols under thread isolation during soft stop"), but older versions are
concerned as 1b927eb3c was not marked for backports.
Moreover, the patch also prevents the race between protocol_pause_all() and
resuming from LIMITED or FULL states.

This commit depends on:
  - "MINOR: listener: add relax_listener() function"

This should be backported with d1ebee177 up to 2.4
(d1ebee177 is marked to be backported for all stable versions but the current
patch does not apply for versions < 2.4)

MINOR: listener: add relax_listener() function

There is a need for a small difference between resuming and relaxing
a listener.

When resuming, we expect that the listener may completely resume, this includes
unpausing or rebinding if required.
Resuming a listener is a best-effort operation: no matter the current state,
try our best to bring the listener up to the LI_READY state.

There are some cases where we only want to "relax" listeners that were
previously restricted using limit_listener() or listener_full() functions.
Here we don't want to ressucitate listeners, we're simply interested in
cancelling out the previous restriction.

To this day, listener_resume() on a unbound listener is broken, that's why
the need for this wasn't felt yet.

But we're trying to restore historical listener_resume() behavior, so we better
prepare for this by introducing an explicit relax_listener() function that
only does what is expected in such cases.

This commit depends on:
 - "MINOR: listener/api: add lli hint to listener functions"

MINOR: listener/api: add lli hint to listener functions

Add listener lock hint (AKA lli) to (stop/resume/pause)_listener() functions.
All these functions implicitely take the listener lock when they are called:
It could be useful to be able to call them while already holding the lock, so
we're adding lli hint to make them take the lock only when it is missing.

This should only be backported if explicitly required by another commit
--

-> 2.4 and 2.5 common backport notes:

These 2 commits need to be backported first:
 - 187396e34 "CLEANUP: listener: function comment typo in stop_listener()"
 - a57786e87 "BUG/MINOR: listener: null pointer dereference suspected by
   coverity"

-> 2.4 special backport notes:

In addition to the previously mentionned dependencies, the patch needs to be
slightly adapted to match the corresponding contextual lines:

Replace this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if (l->state <= LI_PAUSED)
    |                goto end;

By this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if ((global.mode & (MODE_DAEMON | MODE_MWORKER)) &&
    |            !(proc_mask(l->rx.settings->bind_proc) & pid_bit))

Replace this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |-                       stop_listener(listener, 0, 1);
    |+                       stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
    | }

By this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |                        if (!listener->bind_conf->frontend->grace)
    |-                               stop_listener(listener, 0, 1);
    |+                               stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

Replace this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) {
    |                /* might be just a backend */

By this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!p->disabled && !p->li_ready) {
    |                /* might be just a backend */

MINOR: proto_uxst: add resume method

resume method was not explicitly defined for uxst protocol family.
Here we can safely use the default_resume_listener, just like the uxdg family.

This could be backported up to 2.4.

BUG/MINOR: protocol: fix minor memory leak in protocol_bind_all()

In protocol_bind_all() (involved in startup sequence):
We only free errmsg (set by fam->bind() attempt) when we make use of it.
But this could lead to some memory leaks because there are some cases
where we ignore the error message (e.g: verbose=0 with ERR_WARN messages).

As long as errmsg is set, we should always free it.

As mentioned earlier, this really is a minor leak because it can only occur on
specific conditions (error paths) during the startup phase.

This may be backported up to 2.4.

--
Backport notes:

-> 2.4 only:

Replace this:

    |                                        ha_warning("Binding [%s:%d] for %s %s: %s\n",
    |                                                   listener->bind_conf->file, listener->bind_conf->line,
    |                                                   proxy_type_str(px), px->id, errmsg);

By this:

    |                                else if (lerr & ERR_WARN)
    |                                        ha_warning("Starting %s %s: %s\n",
    |                                                   proxy_type_str(px), px->id, errmsg);

BUG/MINOR: proto_ux: report correct error when bind_listener fails

In uxst_bind_listener() and uxdg_bind_listener(), when the function
fails because the listener is not bound, both function are setting
the error message but don't set the err status before returning.

Because of this, such error is not properly handled by the upper functions.

Making sure this error is properly catched by returning a composition of
ERR_FATAL and ERR_ALERT.

This could be backported up to 2.4.

MINOR: stconn: Add functions to set/clear SE_FL_EXP_NO_DATA flag from endpoint

se_expect_data() and se_expect_no_data() should be used from the endpoint to
inform upper layer it expects data or not from the opposite endpoint.

REGTESTS: cache: Use rxresphdrs to only get headers for 304 responses

304 responses contains "Content-length" or "Transfer-encoding"
headers. rxresp action expects to get a payload in this case, even if 304
reponses must not have any payload. A workaround was added to remove these
headers from the 304 responses. However, a better solution is to only get
the response headers from clients using rxresphdrs action.

If a payload is erroneously added in these reponses, the scripts will fail
the same way. So it is safe.

MINOR: stconn: Remove half-closed timeout

The half-closed timeout is now directly retrieved from the proxy
settings. There is no longer usage for the .hcto field in the stconn
structure. So let's remove it.

MINOR: stconn: Set half-close timeout using proxy settings

We now directly use the proxy settings to set the half-close timeout of a
stream-connector. The function sc_set_hcto() must be used to do so. This
timeout is only set when a shutw is performed. So it is not really a big
deal to use a dedicated function to do so.

CLEANUP: stconn: Remove old read and write expiration dates

Old read and write expiration dates are no longer used. Thus we can safely
remove them.

MINOR: stconn: Always report READ/WRITE event on shutr/shutw

It was done by hand by callers when a shutdown for read or write was
performed. It is now always handled by the functions performing the
shutdown. This way the callers don't take care of it. This will avoid some
bugs.

MINOR: stream: Use relative expiration date in trace messages

Expiration dates in trace messages are now relative to now_ms. It is fairly
easier to read traces this way. And an expired date is now negative. So, it
is also easy to detect when a timeout was reached.

MINOR: stream: Report rex/wex value using the sedesc date in trace messages

Becasue read and write timeout are now detected using .lra and .fsb fields
of the stream-endpoint descriptor, it is better to also use these fields to
report read and write expiration date in trace messages. Especially because
old rex and wex fields will be removed.

MINOR: stream: Dump the task expiration date in trace messages

The expiration date of the stream's task is now diplayed in the trace
messages. This will help to track changes in the stream traces.

MAJOR: stream: Use SE descriptor date to detect read/write timeouts

We stop to use the channel's expiration dates to detect read and write
timeouts on the channels. We now rely on the stream-endpoint descriptor to
do so. All the stuff is handled in process_stream().

The stream relies on 2 helper functions to know if the receives or sends may
expire: sc_rcv_may_expire() and sc_snd_may_expire().

MINOR: applet/stconn: Add a SE flag to specify an endpoint does not expect data

An endpoint should now set SE_FL_EXP_NO_DATA flag if it does not expect any
data from the opposite endpoint. This way, the stream will be able to
disable any read timeout on the opposite endpoint. Applets should use
applet_expect_no_data() and applet_expect_data() functions to set or clear
the flag. For now, only dns and sink forwarder applets are concerned.

MEDIUM: stconn: Add two date to track successful reads and blocked sends

The stream endpoint descriptor now owns two date, lra (last read activity) and
fsb (first send blocked).

The first one is updated every time a read activity is reported, including data
received from the endpoint, successful connect, end of input and shutdown for
reads. A read activity is also reported when receives are unblocked. It will be
used to detect read timeouts.

The other one is updated when no data can be sent to the endpoint and reset
when some data are sent. It is the date of the first send blocked by the
endpoint. It will be used to detect write timeouts.

Helper functions are added to report read/send activity and to retrieve lra/fsb
date.

MEDIUM: stconn: Replace read and write timeouts by a unique I/O timeout

Read and write timeouts (.rto and .wto) are now replaced by an unique
timeout, call .ioto. Since the recent refactoring on channel's timeouts,
both use the same value, the client timeout on client side and the server
timeout on the server side. Thus, this part may be simplified. Now it
represents the I/O timeout.

MEDIUM: stconn: Don't requeue the stream's task after I/O

After I/O handling, in sc_notify(), the stream's task is no longer
requeue. The stream may be woken up. But its task is not requeue. It is
useless nowadays and only avoids a call to process_stream() for edge
cases. It is not really a big deal if the stream is woken up for nothing
because its task expired. At worst, it will be responsible to compute its
new expiration date.

MEDIUM: channel/stconn: Move rex/wex timer from the channel to the sedesc

These timers are related to the I/O. Thus it is logical to move them into
the SE descriptor. The patch is a bit huge but it is just a
replacement. However it is error-prone.

From the stconn or the stream, helper functions are used to get, set or
reset these timers. This simplify the timers manipulations.

MINOR: channel/stconn: Move rto/wto from the channel to the stconn

Read and write timeouts concerns the I/O. Thus, it is logical to move it into
the stconn. At the end, the stream is responsible to detect the timeouts. So
it is logcial to have these values in the stconn and not in the SE
descriptor. But it may change depending on the recfactoring.

So, now:
  * scf->rto is used instead of req->rto
  * scf->wto is used instead of res->wto
  * scb->rto is used instead of res->rto
  * scb->wto is used instead of req->wto

DEBUG: stream/trace: Add sedesc flags in trace messages

In trace messages, when info about stream-connector are dumped, the
stream-endpoint descriptor flags are now also dumped.

MAJOR: channel: Remove flags to report READ or WRITE errors

This patch removes CF_READ_ERROR and CF_WRITE_ERROR flags. We now rely on
SE_FL_ERR_PENDING and SE_FL_ERROR flags. SE_FL_ERR_PENDING is used for write
errors and SE_FL_ERROR for read or unrecoverable errors.

When a connection error is reported, SE_FL_ERROR and SE_FL_EOS are now set and a
read event and a write event are reported to be sure the stream will properly
process the error. At the stream-connector level, it is similar. When an error
is reported during a send, a write event is triggered. On the read side, nothing
more is performed because an error at this stage is enough to wake the stream
up.

A major change is brought with this patch. We stop to check flags of the
ooposite channel to report abort or timeout. It also means when an read or
write error is reported on a side, we no longer update the other side. Thus
a read error on the server side does no long lead to a write error on the
client side. This should ease errors report.

MEDIUM: channel: Remove CF_READ_NOEXP flag

This flag was introduced in 1.3 to fix a design issue. It was untouch since
then but there is no reason to still have this trick. Note it could be good
to review what happens in HTTP with the server is waiting for the end of the
request. It could be good to be sure a client timeout is always reported.

BUG/MEDIUM: httpclient/lua: fix a race between lua GC and hlua_ctx_destroy

In bb581423b ("BUG/MEDIUM: httpclient/lua: crash when the lua task timeout
before the httpclient"), a new logic was implemented to make sure that
when a lua ctx destroyed, related httpclients are correctly destroyed too
to prevent a such httpclients from being resuscitated on a destroyed lua ctx.

This was implemented by adding a list of httpclients within the lua ctx,
and a new function, hlua_httpclient_destroy_all(), that is called under
hlua_ctx_destroy() and runs through the httpclients list in the lua context
to properly terminate them.

This was done with the assumption that no concurrent Lua garbage collection
cycles could occur on the same ressources, which seems OK since the "lua"
context is about to be freed and is not explicitly being used by other threads.

But when 'lua-load' is used, the main lua stack is shared between multiple
OS threads, which means that all lua ctx in the process are linked to the
same parent stack.
Yet it seems that lua GC, which can be triggered automatically under
lua_resume() or manually through lua_gc(), does not limit itself to the
"coroutine" stack (the stack referenced in lua->T) when performing the cleanup,
but is able to perform some cleanup on the main stack plus coroutines stacks
that were created under the same main stack (via lua_newthread()) as well.

This can be explained by the fact that lua_newthread() coroutines are not meant
to be thread-safe by design.
Source: http://lua-users.org/lists/lua-l/2011-07/msg00072.html (lua co-author)

It did not cause other issues so far because most of the time when using
'lua-load', the global lua lock is taken when performing critical operations
that are known to interfere with the main stack.
But here in hlua_httpclient_destroy_all(), we don't run under the global lock.

Now that we properly understand the issue, the fix is pretty trivial:

We could simply guard the hlua_httpclient_destroy_all() under the global
lua lock, this would work but it could increase the contention over the
global lock.

Instead, we switched 'lua->hc_list' which was introduced with bb581423b
from simple list to mt_list so that concurrent accesses between
hlua_httpclient_destroy_all and hlua_httpclient_gc() are properly handled.

The issue was reported by @Mark11122 on Github #2037.

This must be backported with bb581423b ("BUG/MEDIUM: httpclient/lua: crash
when the lua task timeout before the httpclient") as far as 2.5.

BUG/MINOR: lua/httpclient: missing free in hlua_httpclient_send()

In hlua_httpclient_send(), we replace hc->req.url with a new url.
But we forgot to free the original url that was allocated in
hlua_httpclient_new() or in the previous httpclient_send() call.

Because of this, each httpclient request performed under lua scripts would
result in a small leak. When stress-testing a lua action which uses httpclient,
the leak is clearly visible since we're leaking severals Mbytes per minute.

This bug was discovered by chance when trying to reproduce GH issue #2037.

It must be backported up to 2.5

MINOR: compiler: add a TOSTR() macro to turn a value into a string

Pretty often we have to emit a value (setting, limit etc) in an error
message, and this value is known at compile-time, and just doing this
forces to use a printf format such as "%d". Let's have a simple macro
to turn any other macro or value into a string that can be concatenated
with the rest of the string around. This simplifies error messages
production on the CLI for example.

BUG/MINOR: cache: Check cache entry is complete in case of Vary

Before looking for a secondary cache entry for a given request we
checked that the first entry was complete, which might prevent us from
using a valid entry if the first one with the same primary key is not
full yet.
Likewise, if the primary entry is complete but not the secondary entry
we try to use, we might end up using a partial entry from the cache as
a response.

This bug was raised in GitHub #2048.
It can be backported up to branch 2.4.

BUG/MINOR: cache: Cache response even if request has "no-cache" directive

Since commit cc9bf2e5f "MEDIUM: cache: Change caching conditions"
responses that do not have an explicit expiration time are not cached
anymore. But this mechanism wrongly used the TX_CACHE_IGNORE flag
instead of the TX_CACHEABLE one. The effect this had is that a cacheable
response that corresponded to a request having a "Cache-Control:
no-cache" for instance would not be cached.
Contrary to what was said in the other commit message, the "checkcache"
option should not be impacted by the use of the TX_CACHEABLE flag
instead of the TX_CACHE_IGNORE one. The response is indeed considered as
not cacheable if it has no expiration time, regardless of the presence
of a cookie in the response.

This should fix GitHub issue #2048.
This patch can be backported up to branch 2.4.

MINOR: startup: HAPROXY_STARTUP_VERSION contains the version used to start

HAPROXY_STARTUP_VERSION: contains the version used to start, in
master-worker mode this is the version which was used to start the
master, even after updating the binary and reloading.

This patch could be backported in every version since it is useful when
debugging.

BUG/MEDIUM: mworker: don't register mworker_accept_wrapper() when master FD is wrong

This patch handles the case where the fd could be -1 when proc_self was
lost for some reason (environment variable corrupted or upgrade from < 1.9).

This could result in a out of bound array access fdtab[-1] and would crash.

Must be backported in every maintained versions.

BUG/MEDIUM: mworker: prevent inconsistent reload when upgrading from old versions

Previous versions ( < 1.9 ) of the master-worker process didn't had the
"HAPROXY_PROCESSES" environment variable which contains the list of
processes, fd etc.

The part which describes the master is created at first startup so if
you started the master with an old version you would never have
it.

Since patch 68836740 ("MINOR: mworker: implement a reload failure
counter"), the failedreloads member of the proc_self structure for the
master is set to 0. However if this structure does not exist, it will
result in a NULL dereference and crash the master.

This patch fixes the issue by creating the proc_self structure for the
master when it does not exist. It also shows a warning which states to
restart the master if that is the case, because we can't guarantee that
it will be working correctly.

This MUST be backported as far as 2.5, and could be backported in every
other stable branches.

BUG/MINOR: mworker: stop doing strtok directly from the env

When parsing the HAPROXY_PROCESSES environement variable, strtok was
done directly from the ptr resulting from getenv(), which replaces the ;
by \0, showing confusing environment variables when debugging in /proc
or in a corefile.

Example:

(gdb) x/39s *environ
[...]
0x7fff6935af64: "HAPROXY_PROCESSES=|type=w"
0x7fff6935af7e: "fd=3"
0x7fff6935af83: "pid=4444"
0x7fff6935af8d: "rpid=1"
0x7fff6935af94: "reloads=0"
0x7fff6935af9e: "timestamp=1676338060"
0x7fff6935afb3: "id="
0x7fff6935afb7: "version=2.4.0-8076da-1010+11"

This patch fixes the issue by doing a strdup on the variable.

Could be backported in previous versions (mworker_proc_to_env_list
exists since 1.9)

REGTESTS: Fix ssl_errors.vtc script to wait for connections close

In this scripts, several clients perform a requests and exit because an SSL
error is expected and thus no response is sent. However, we must explicitly
wait for the connection close, via an "expect_close" statement.  Otherwise,
depending on the timing, HAProxy may detect the client abort before any
connection attempt on the server side and no SSL error is reported, making
the script to fail.

REGTESTS: Skip http_splicing.vtc script if fast-forward is disabled

If "-dF" command line argument is passed to haproxy to execute the script,
by sepcifying HAPROXY_ARGS variable, http_splicing.vtc is now skipped.
Without this patch, the script fails when the fast-forward is disabled.

MINOR: cfgcond: Implement enabled condition expression

Implement a way to test if some options are enabled at run-time. For now,
following options may be detected:

  POLL, EPOLL, KQUEUE, EVPORTS, SPLICE, GETADDRINFO, REUSEPORT,
  FAST-FORWARD, SERVER-SSL-VERIFY-NONE

These options are those that can be disabled on the command line. This way
it is possible, from a reg-test for instance, to know if a feature is
supported or not :

  feature cmd "$HAPROXY_PROGRAM -cc '!(globa.tune & GTUNE_NO_FAST_FWD)'"

MINOR: cfgcond: Implement strstr condition expression

Implement a way to match a substring in a string. The strstr expresionn can
now be used to do so.

DOC: config: Add the missing tune.fail-alloc option from global listing

This global option is documented but it is not in the list of supported
options for the global section. So let's add it.

This patch could be backported to all stable versions.

BUG/MINOR: haproxy: Fix option to disable the fast-forward

The option was renamed to only permit to disable the fast-forward. First
there is no reason to enable it because it is the default behavior. Then it
introduced a bug because there is no way to be sure the command line has
precedence over the configuration this way. So, the option is now named
"tune.disable-fast-forward" and does not support any argument. And of
course, the commande line option "-dF" has now precedence over the
configuration.

No backport needed.

MINOR: proxy: Only consider backend httpclose option for server connections

For server connections, both the frontend and backend were considered to
enable the httpclose option. However, it is ambiguous because on client side
only the frontend is considerd. In addition for 2 frontends, one with the
option enabled and not for the other, the HTTP connection mode may differ
while it is a backend setting.

Thus, now, for the server side, only the backend is considered. Of course,
if the option is set for a listener, the option will be enabled if the
listener is the backend's connection.

DOC: config: Fix description of options about HTTP connection modes

Since the HTX, the decription of options about HTTP connection modes is
wrong. In fact, it is worst, all the documentation about HTTP connection
mode is wrong. But only options will be updated for now to be backported.

So, documentation of "option httpclose", "option "http-keep-alive", "option
http-server-close" and "option "http-pretend-keepalive" was reviewed. First,
it is specify these options only concern HTT/1.x connections. Then, the
descriptions were updated to reflect the HTX implementation.

The main changes concerns the fact that server connections are no longer
attached to client connections. The connection mode on one side does not
affect the connection mode on the other side. It is especially true for
t"option httpclose". For client connections, only the frontend option is
considered and for server ones, both frontend and backend options are
considered.

This patch should be backported as far as 2.2.

DEBUG: stream: Add a BUG_ON to never exit process_stream with an expired task

We must never exit for the stream processing function with an expired
task. Otherwise, we are pretty sure this will ends with a spinning loop. It
is really better to abort as far as possible and with the original buggy
state. This will ease the debug sessions.

BUG/MEDIUM: quic: Missing TX buffer draining from qc_send_ppkts()

If the TX buffer (->tx.buf) attached to the connection is not drained, there
are chances that this will be detected by qc_txb_release() which triggers
a BUG_ON_HOT() when this is the case as follows

[00|quic|2|c_conn.c:3477] UDP port unreachable : qc@0x5584f18d6d50 pto_count=0 cwnd=6816 ppif=1046 pif=1046
[00|quic|5|ic_conn.c:749] qc_kill_conn(): entering : qc@0x5584f18d6d50
[00|quic|5|ic_conn.c:752] qc_kill_conn(): leaving : qc@0x5584f18d6d50
[00|quic|5|c_conn.c:3532] qc_send_ppkts(): leaving : qc@0x5584f18d6d50 pto_count=0 cwnd=6816 ppif=1046 pif=1046

FATAL: bug condition "buf && b_data(buf)" matched at src/quic_conn.c:3098

Consume the remaining data in the TX buffer calling b_del().

This bug arrived with this commit:
    a2c62c314 MINOR: quic: Kill the connections on ICMP (port unreachable) packet receipt

Takes also the opportunity of this patch to modify the comments for qc_send_ppkts()
which should have arrived with a2c62c314 commit.

Must be backported to 2.7 where this latter commit is supposed to be backported.

MINOR: mux-h2/traces: add a missing TRACE_LEAVE() in h2s_frt_handle_headers()

Traces from this function would miss a TRACE_LEAVE() on the success path,
which had for consequences, 1) that it was difficult to figure where the
function was left, and 2) that we never had the allocated stream ID
clearly visible (actually the one returned by h2c_frt_stream_new() is
the right one but it's not obvious).

This can be backported to 2.7 and 2.6.

MINOR: mux-h2/traces: do not log h2s pointer for dummy streams

Functions which are called with dummy streams pass it down the traces
and that leads to somewhat confusing "h2s=0x1234568(0,IDL)" for example
while the nature of the called function makes this stream useless at that
place. Better not report a random pointer, especially since it always
requires to look at the code before remembering how this should be
interpreted.

Now what we're doing is that the idle stream only prints "h2s=IDL" which
is shorter and doesn't report a pointer, closed stream do not report
anything since the stream ID 0 already implies it, and other ones are
reported normally.

This could be backported to 2.7 and 2.6 as it improves traces legibility.

MEDIUM: quic: trigger fast connection closing on process stopping

With previous commit, quic-conn are now handled as jobs to prevent the
termination of haproxy process. This ensures that QUIC connections are
closed when all data are acknowledged by the client and there is no more
active streams.

The quic-conn layer emits a CONNECTION_CLOSE once the MUX has been
released and all streams are acknowledged. Then, the timer is scheduled
to definitely free the connection after the idle timeout period. This
allows to treat late-arriving packets.

Adjust this procedure to deactivate this timer when process stopping is
in progress. In this case, quic-conn timer is set to expire immediately
to free the quic-conn instance as soon as possible. This allows to
quickly close haproxy process.

This should be backported up to 2.7.

MINOR: quic: mark quic-conn as jobs on socket allocation

To prevent data loss for QUIC connections, haproxy global variable jobs
is incremented each time a quic-conn socket is allocated. This allows
the QUIC connection to terminate all its transfer operation during proxy
soft-stop. Without this patch, the process will be terminated without
waiting for QUIC connections.

Note that this is done in qc_alloc_fd(). This means only QUIC connection
with their owned socket will properly support soft-stop. In the other
case, the connection will be interrupted abruptly as before. Similarly,
jobs decrement is conducted in qc_release_fd().

This should be backported up to 2.7.

MEDIUM: mux-quic: properly implement soft-stop

Properly implement support for haproxy soft-stop on QUIC MUX. This code
is similar to H2 MUX :

* on timeout refresh, if stop-stop in progress, schedule the timeout to
  expire with regards to the close-spread-end window.

* after input/output processing, if soft-stop in progress, shutdown the
  connection. This is randomly spread by close-spread-end window. In the
  case of H3 connection, a GOAWAY is emitted and the connection is kept
  until all data are sent for opened streams. If the client tries to use
  new streams, they are rejected in conformance with the GOAWAY
  specification.

This ensures that MUX is able to forward all content properly before
closing the connection. The lower quic-conn layer is then responsible
for retransmission and should be closed when all data are acknowledged.
This will be implemented in the next commit to fully support soft-stop
for QUIC connections.

This should be backported up to 2.7.

MINOR: mux-quic: implement client-fin timeout

Implement client-fin timeout for MUX quic. This timeout is used once an
applicative layer shutdown has been called. In HTTP/3, this corresponds
to the emission of a GOAWAY.

This should be backported up to 2.7.

MINOR: mux-quic: define qc_process()

Define a new function qc_process(). This function will regroup several
internal operation which should be called both on I/O tasklet and wake()
callback. For the moment, only streams purge is conducted there.

This patch is useful to support haproxy soft stop. This should be
backported up to 2.7.

MINOR: mux-quic: define qc_shutdown()

Factorize shutdown operation in a dedicated function qc_shutdown(). This
will allow to call it from multiple places. A new flag QC_CF_APP_SHUT is
also defined to ensure it will only be executed once even if called
multiple times per connection.

This commit will be useful to properly support haproxy soft stop.
This should be backported up to 2.7.

MEDIUM: h3: enforce GOAWAY by resetting higher unhandled stream

When a GOAWAY has been emitted, an ID is announced to represent handled
streams. H3 RFC suggests that higher streams should be resetted with the
error code H3_REQUEST_CANCELLED. This allows the peer to replay requests
on another connection.

For the moment, the impact of this change is limitted as GOAWAY is only
used on connection shutdown just before the MUX is freed. However, for
soft-stop support, a GOAWAY can be emitted in anticipation while keeping
the MUX to finish the active streams. In this case, new streams opened
by the client are resetted.

As a consequence of this change, app_ops.attach() operation has been
delayed at the very end of qcs_new(). This ensure that all qcs members
are initialized to support RESET_STREAM sending.

This should be backported up to 2.7.

BUG/MINOR: h3: prevent hypothetical demux failure on int overflow

h3s stores the current demux frame type and length as a state info. It
should be big enough to store a QUIC variable-length integer which is
the maximum H3 frame type and size.

Without this patch, there is a risk of integer overflow if H3 frame size
is bigger than INT_MAX. This can typically causes demux state mismatch
and demux frame error. However, no occurence has been found yet of this
bug with the current implementation.

This should be backported up to 2.6.

BUG/MINOR: quic: acknowledge STREAM frame even if MUX is released

When the MUX is freed, the quic-conn layer may stay active until all
streams acknowledgment are processed. In this interval, if a new stream
is opened by the client, the quic-conn is thus now responsible to handle
it. This is done by the emission of a STOP_SENDING + RESET_STREAM.

Prior to this patch, the received packet was not acknowledged. This is
undesirable if the quic-conn is able to properly reject the request as
this can lead to unneeded retransmission from the client.

This must be backported up to 2.6.

BUG/MINOR: quic: also send RESET_STREAM if MUX released

When the MUX is freed, the quic-conn layer may stay active until all
streams acknowledgment are processed. In this interval, if a new stream
is opened by the client, the quic-conn is thus now responsible to handle
it. This is done by the emission of a STOP_SENDING.

This process has been completed to also emit a RESET_STREAM with the
same error code H3_REQUEST_REJECTED. This is done to conform with the H3
specification to invite the client to retry its request on a new
connection.

This should be backported up to 2.6.

MINOR: quic: adjust request reject when MUX is already freed

When the MUX is freed, the quic-conn layer may stay active until all
streams acknowledgment are processed. In this interval, if a new stream
is opened by the client, the quic-conn is thus now responsible to handle
it. This is done by the emission of a STOP_SENDING.

This process is closely related to HTTP/3 protocol despite being handled
by the quic-conn layer. This highlights a flaw in our QUIC architecture
which should be adjusted. To reflect this situation, the function
qc_stop_sending_frm_enqueue() is renamed qc_h3_request_reject(). Also,
internal H3 treatment such as uni-directional bypass has been moved
inside the function.

This commit is only a refactor. However, bug fix on next patches will
rely on it so it should be backported up to 2.6.

BUG/MINOR: quic: Missing padding for short packets

This was revealed by Amaury when setting tune.quic.frontend.max-streams-bidi to 8
and asking a client to open 12 streams. haproxy has to send short packets
with little MAX_STREAMS frames encoded with 2 bytes. In addition to a packet number
encoded with only one byte. In the case <len_frms> is the length of the encoded
frames to be added to the packet plus the length of the packet number.

Ensure the length of the packet is at least QUIC_PACKET_PN_MAXLEN adding a PADDING
frame wich (QUIC_PACKET_PN_MAXLEN - <len_frms>) as size. For instance with
a two bytes MAX_STREAMS frames and a one byte packet number length, this adds
one byte of padding.

See https://datatracker.ietf.org/doc/html/rfc9001#name-header-protection-sample.

Must be backported to 2.7 and 2.6.

BUG/MINOR: quic: Do not drop too small datagrams with Initial packets

When receiving an Initial packet a peer must drop it if the datagram is smaller
than 1200. Before this patch, this is the entire datagram which was dropped.

In such a case, drop the packet after having parsed its length.

Must be backported to 2.6 and 2.7

BUG/MINOR: quic: Wrong initialization for io_cb_wakeup boolean

This bug arrives with this commit:
    982896961 MINOR: quic: split and rename qc_lstnr_pkt_rcv()
The first block of code consists in possibly setting this variable to true.
But it was already initialized to true before entering this code section.
Should be initialized to false.

Also take the opportunity to remove an unused "err" label.

Must be backported to 2.6 and 2.7.

BUG/MINOR: quic: Do not probe with too little Initial packets

Before probing the Initial packet number space, verify that we can at least
sent 1200 bytes by datagram. This may not be the case due to the amplification limit.

Must be backported to 2.6 and 2.7.

MINOR: quic: Add <pto_count> to the traces

This may be useful to diagnose issues in relation with QUIC recovery.

Must be backported to 2.7.

MINOR: quic: Add a trace to identify connections which sent Initial packet.

This should help in diagnosing issues revealed by the interop runner which counts
the number of handshakes from the number of Initial packets sent by the server.

Must be backported to 2.7.

BUG/MINOR: quic: Missing call to task_queue() in qc_idle_timer_do_rearm()

The aim of this function is to rearm the idle timer. The ->expire
field of the timer task was updated without being requeued.
Some connection could be unexpectedly terminated.

Must be backported to 2.6 and 2.7.

MINOR: quic: Make qc_dgrams_retransmit() return a status.

This is very helpful during retranmission when receiving ICMP port unreachable
errors after the peer has left. This is the unique case at prevent where
qc_send_hdshk_pkts() or qc_send_app_probing() may fail (when they call
qc_send_ppkts() which fails with ECONNREFUSED as errno).

Also make the callers qc_dgrams_retransmit() stop their packet process. This
is the case of quic_conn_app_io_cb() and quic_conn_io_cb().

This modifications stops definitively any packet processing when receiving
ICMP port unreachable errors.

Must be backported to 2.7.

MINOR: quic: Add traces to qc_kill_conn()

Very minor modification to help in debugging issues.

Must be backported to 2.7.

MINOR: quic: Kill the connections on ICMP (port unreachable) packet receipt

The send*() syscall which are responsible of such ICMP packets reception
fails with ECONNREFUSED as errno.

  man(7) udp
  ECONNREFUSED
      No receiver was associated with the destination address.
      This might be caused by a previous packet sent over the socket.

We must kill asap the underlying connection.

Must be backported to 2.7.

MINOR: quic: Simplication for qc_set_timer()

There is no reason to run code for nothing if the timer task has been
released when entering qc_set_timer().

Must be backported to 2.7.

BUG/MINOR: quic: Really cancel the connection timer from qc_set_timer()

The ->expire field of the timer task to be cancelled was not reset
to TICK_ETERNITY.

Must be backported to 2.6 and 2.7.

MINOR: quic: Move code to wakeup the timer task to avoid anti-amplication deadlock

This code was there because the timer task was not running on the same thread
as the one which parse the QUIC packets. Now that this is no more the case,
we can wake up this task directly.

Must be backported to 2.7.

MINOR: quic: Add new traces about by connection RX buffer handling

Move quic_rx_pkts_del() out of quic_conn.h to make it benefit from the TRACE API.
Add traces which already already helped in diagnosing an issue encountered with
ngtcp2 which sent too much 1RTT packets before the handshake completion. This
has been fixed here after having discussed with Tasuhiro on QUIC dev slack:

https://github.com/ngtcp2/ngtcp2/pull/663

Must be backported to 2.7.

BUG/MINOR: quic: Possible unexpected counter incrementation on send*() errors

Some counters could potentially be incremented even if send*() syscall returned
no error when ret >= 0 and ret != sz. This could be the case for instance if
a first call to send*() returned -1 with errno set to EINTR (or any previous syscall
which set errno to a non-null value) and if the next call to send*() returned
something positive and smaller than <sz>.

Must be backported to 2.7 and 2.6.

MINOR: h3: add traces on decode_qcs callback

Add traces inside h3_decode_qcs(). Every error path has now its
dedicated trace which should simplify debugging. Each early returns has
been converted to a goto invocation.

To complete the demux tracing, demux frame type and length are now
printed using the h3s instance whenever its possible on trace
invocation. A new internal value H3_FT_UNINIT is used as a frame type to
mark demuxing as inactive.

This should be backported up to 2.7.