BUG/MEDIUM: filters: Don't try to init filters for disabled proxies

Configuration is parsed for such proxies but not validated. Concretely, it means
check_config_validity() function does almost nothing for such proxies. Thus, we
must be careful to not initialize filters for disabled proxies because the check
callback function is not called. In fact, to be sure to avoid any trouble,
filters for disabled proxies are released.

This patch fixes a segfault at startup if the SPOE is configured for a disabled
proxy. It must be backported as far as 1.7 (maybe with some adaptations).

BUILD: ssl: use SSL_CTRL_GET_RAW_CIPHERLIST instead of OpenSSL versions

let us use SSL_CTRL_GET_RAW_CIPHERLIST for feature detection instead
of versions

[wla: SSL_CTRL_GET_RAW_CIPHERLIST was introduced by OpenSSL commit
94a209 along with SSL_CIPHER_find. It was removed in boringSSL.]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CLEANUP: pattern: remove unused entry "tree" in pattern.val

This one might have disappeared since patterns were reworked, but the
entry was not removed from the structure, let's do it now.

[RELEASE] Released version 2.3-dev9

Released version 2.3-dev9 with the following main changes :
    - CLEANUP: http_ana: remove unused assignation of `att_beg`
    - BUG/MEDIUM: ssl: OCSP must work with BoringSSL
    - BUG/MINOR: log: fix memory leak on logsrv parse error
    - BUG/MINOR: log: fix risk of null deref on error path
    - BUILD: ssl: more elegant OpenSSL early data support check
    - CI: github actions: update h2spec to 2.6.0
    - BUG/MINOR: cache: Check the return value of http_replace_res_status
    - MINOR: cache: Store the "Last-Modified" date in the cache_entry
    - MINOR: cache: Process the If-Modified-Since header in conditional requests
    - MINOR: cache: Create res.cache_hit and res.cache_name sample fetches
    - MINOR: mux-h2: register a stats module
    - MINOR: mux-h2: add counters instance to h2c
    - MINOR: mux-h2: add stats for received frame types
    - MINOR: mux-h2: report detected error on stats
    - MINOR: mux-h2: count open connections/streams on stats
    - BUG/MINOR: server: fix srv downtime calcul on starting
    - BUG/MINOR: server: fix down_time report for stats
    - BUG/MINOR: lua: initialize sample before using it
    - MINOR: cache: Add Expires header value parsing
    - MINOR: ist: Add a case insensitive istmatch function
    - BUG/MINOR: cache: Manage multiple values in cache-control header value
    - BUG/MINOR: cache: Inverted variables in http_calc_maxage function
    - MINOR: pattern: make pat_ref_append() return the newly added element
    - MINOR: pattern: make pat_ref_add() rely on pat_ref_append()
    - MINOR: pattern: export pat_ref_push()
    - CLEANUP: pattern: use calloc() rather than malloc for structures
    - CLEANUP: pattern: fix spelling/grammatical/copy-paste in comments

CLEANUP: pattern: fix spelling/grammatical/copy-paste in comments

The code is horrible to work with because most functions are documented
with misleading comments resulting from many spelling and grammatical
mistakes, and plenty of remains of copy-paste mentioning arguments that
do not exist and return values that are never set. Too many hours wasted
writing non-working code because of assumptions resulting from this,
let's fix this once for all now!

CLEANUP: pattern: use calloc() rather than malloc for structures

It's particularly difficult to make sure that the various pattern
structures are properly initialized given that they can be allocated
at multiple places and systematically via malloc() instead of calloc(),
thus not even leaving the possibility of default values. Let's adjust
a few of them.

MINOR: pattern: export pat_ref_push()

Strangely this one was marked static inline within the file itself.
Let's export it.

MINOR: pattern: make pat_ref_add() rely on pat_ref_append()

Let's remove unneeded code duplication, both are exactly the same.

MINOR: pattern: make pat_ref_append() return the newly added element

It's more convenient to return the element than to return just 0 or 1,
as the next thing we'll want to do is to act on this element! In addition
it was using variable arguments instead of consts, causing some reuse
constraints which were also addressed. This doesn't change its use as
a boolean, hence why call places were not modified.

BUG/MINOR: cache: Inverted variables in http_calc_maxage function

The maxage and smaxage variables were inadvertently assigned the
Cache-Control s-maxage and max-age values respectively when it should
have been the other way around.

This can be backported on all branches after 1.8 (included).

BUG/MINOR: cache: Manage multiple values in cache-control header value

If an HTTP request or response had a "Cache-Control" header that had
multiple comma-separated subparts in its value (like "max-age=1,
no-store" for instance), we did not process the values correctly and
only parsed the first one. That made us store some HTTP responses in the
cache when they were explicitely uncacheable.
This patch replaces the way the values are parsed by an http_find_header
loop that manages every sub part of the value independently.

This patch should be backported to 2.2 and 2.1. The bug also exists on
previous versions but since the sources changed, a new commit will have
to be created.

[wla: This patch requires bb4582c ("MINOR: ist: Add a case insensitive
istmatch function"). Backporting for < 2.1 is not a requirement since it
works well enough for most cases, it was a known limitation of the
implementation of non-htx version too]

MINOR: ist: Add a case insensitive istmatch function

Add a helper function that checks if a string starts with another string
while ignoring case.

MINOR: cache: Add Expires header value parsing

When no Cache-Control max-age or s-maxage information is present in a
cached response, we need to parse the Expires header value (RFC 7234#5.3).
An invalid Expires date value or a date earlier than the reception date
will make the cache_entry stale upon creation.
For now, the Cache-Control and Expires headers are parsed after the
insertion of the response in the cache so even if the parsing of the
Expires results in an already stale entry, the entry will exist in the
cache.

BUG/MINOR: lua: initialize sample before using it

Memset the sample before using it through hlua_lua2smp. This function is
ORing the smp.flags, so this field need to be cleared before its use.
This was reported by a coverity warning.

Fixes the github issue #929.
This bug can be backported up to 1.8.

BUG/MINOR: server: fix down_time report for stats

Adjust condition used to report down_time for statistics. There was a
tiny probabilty to have a negative downtime if last_change was superior
to now. If this is the case, return only down_time.

This bug can backported up to 1.8.

BUG/MINOR: server: fix srv downtime calcul on starting

When a server is up after a failure, its downtime was reset to 0 on the
statistics. This is due to a wrong condition that causes srv.down_time
to never be set. Fix this by updating down_time each time the server is in
STARTING state.

Fixes the github issue #920.
This bug can be backported up to 1.8.

MINOR: mux-h2: count open connections/streams on stats

Implement as a gauge h2 counters for currently open connections and
streams. The counters are decremented when closing the stream or the
connection.

MINOR: mux-h2: report detected error on stats

Implement counters for h2 protocol error on connection or stream level.
Also count the total number of rst_stream and goaway frames sent by the
mux in response to a detected error.

MINOR: mux-h2: add stats for received frame types

Implement counters for h2 frame received based on their type for
HEADERS, DATA, SETTINGS, RST_STREAM and GOAWAY.

MINOR: mux-h2: add counters instance to h2c

Add pointer to counters as a member for h2c structure. This pointer is
initialized on h2_init function. This is useful to quickly access and
manipulate the counters inside every h2 functions.

MINOR: mux-h2: register a stats module

Use statistics API to register a new stats module generating counters
on h2 module. The counters are attached to frontend/backend instances.

MINOR: cache: Create res.cache_hit and res.cache_name sample fetches

Res.cache_hit sample fetch returns a boolean which is true when the HTTP
response was built out of a cache. The cache's name is returned by the
res.cache_name sample_fetch.

This resolves GitHub issue #900.

MINOR: cache: Process the If-Modified-Since header in conditional requests

If a client sends a conditional request containing an If-Modified-Since
header (and no If-None-Match header), we try to compare the date with
the one stored in the cache entry (coming either from a Last-Modified
head, or a Date header, or corresponding to the first response's
reception time). If the request's date is earlier than the stored one,
we send a "304 Not Modified" response back. Otherwise, the stored is sent
(through a 200 OK response).

This resolves GitHub issue #821.

MINOR: cache: Store the "Last-Modified" date in the cache_entry

In order to manage "If-Modified-Since" requests, we need to keep a
reference time for our cache entries (to which the conditional request's
date will be compared).
This reference is either extracted from the "Last-Modified" header, or
the "Date" header, or the reception time of the response (in decreasing
order of priority).
The date values are converted into seconds since epoch in order to ease
comparisons and to limit storage space.

BUG/MINOR: cache: Check the return value of http_replace_res_status

Send the full body if the status `304` cannot be applied. This should be the
most graceful failure.

Specific for 2.3, no backport needed.

CI: github actions: update h2spec to 2.6.0

BUILD: ssl: more elegant OpenSSL early data support check

BorinSSL pretends to be 1.1.1 version of OpenSSL. It messes some
version based feature presense checks. For example, OpenSSL specific
early data support.

Let us change that feature detction to SSL_READ_EARLY_DATA_SUCCESS
macro check instead of version comparision.

BUG/MINOR: log: fix risk of null deref on error path

Previous commit ae32ac74db ("BUG/MINOR: log: fix memory leak on logsrv
parse error") addressed one issue and introduced another one, the logsrv
pointer may also be null at the end of the function so we must test it
before deciding to dereference it.

This should be backported along with the patch above to 2.2.

BUG/MINOR: log: fix memory leak on logsrv parse error

In case of parsing error on logsrv, we can leave parse_logsrv() without
releasing logsrv->ring_name or smp_rgs. Let's free them on the error path.
This should fix issue #926 detected by Coverity.

The impact is only a tiny leak just before reporting a fatal error, so it
will essentially annoy valgrind.

This can be backported to 2.0 (just drop the ring part).

BUG/MEDIUM: ssl: OCSP must work with BoringSSL

It's a regression from b3201a3e "BUG/MINOR: disable dynamic OCSP load
with BoringSSL". The origin bug is link to 76b4a12 "BUG/MEDIUM: ssl:
memory leak of ocsp data at SSL_CTX_free()": ssl_sock_free_ocsp()
shoud be in #ifndef OPENSSL_IS_BORINGSSL.
To avoid long #ifdef for small code, the BoringSSL part for ocsp load
is isolated in a simple #ifdef.

This must be backported in 2.2 and 2.1

CLEANUP: http_ana: remove unused assignation of `att_beg`

`att_beg` is assigned to `next` at the end of the `for` loop, but is
assigned to `prev` at the beginning of the loop, which is itself
assigned to `next` after each loop. So it represents a double
assignation for the same value. Also `att_beg` is not used after the end
of the loop.

this is a partial fix for github issue #923, all the others could
probably be marked as intentional to protect future changes.

no backport needed.

Signed-off-by: William Dauchy <wdauchy@gmail.com>

[RELEASE] Released version 2.3-dev8

Released version 2.3-dev8 with the following main changes :
    - MINOR: backend: replace the lbprm lock with an rwlock
    - MINOR: lb/map: use seek lock and read locks where appropriate
    - MINOR: lb/leastconn: only take a read lock in fwlc_get_next_server()
    - MINOR: lb/first: use a read lock in fas_get_next_server()
    - MINOR: lb/chash: use a read lock in chash_get_server_hash()
    - BUG/MINOR: disable dynamic OCSP load with BoringSSL
    - BUILD: ssl: make BoringSSL use its own version numbers
    - CLEANUP: threads: don't register an initcall when not debugging
    - MINOR: threads: change lock_t to an unsigned int
    - CLEANUP: tree-wide: reorder a few structures to plug some holes around locks
    - CLEANUP: task: remove the unused and mishandled global_rqueue_size
    - BUG/MEDIUM: connection: Never cleanup server lists when freeing private conns
    - MEDIUM: config: report that "nbproc" is deprecated
    - BUG/MINOR: listener: close before free in `listener_accept`
    - MINOR: ssl: 'ssl-load-extra-del-ext' removes the certificate extension
    - BUG/MINOR: queue: properly report redistributed connections
    - CONTRIB: tcploop: remove unused local variables in tcp_pause()
    - BUILD: makefile: add entries to build common debugging tools
    - BUG/MEDIUM: server: support changing the slowstart value from state-file
    - MINOR: http: Add `enum etag_type http_get_etag_type(const struct ist)`
    - MINOR: http: Add etag comparison function
    - MEDIUM: cache: Store the ETag information in the cache_entry
    - MEDIUM: cache: Add support for 'If-None-Match' request header
    - REGTEST: cache: Add if-none-match test case
    - CLEANUP: compression: Make use of http_get_etag_type()
    - BUG/MINOR: http-ana: Don't send payload for internal responses to HEAD requests
    - BUG/MAJOR: mux-h2: Don't try to send data if we know it is no longer possible
    - MINOR: threads/debug: only report used lock stats
    - MINOR: threads/debug: only report lock stats for used operations
    - MINOR: proxy; replace the spinlock with an rwlock
    - MINOR: server: read-lock the cookie during srv_set_dyncookie()
    - MINOR: proxy/cli: only take a read lock in "show errors"
    - OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued
    - MINOR: queue: split __pendconn_unlink() in per-srv and per-prx
    - MINOR: queue: reduce the locked area in pendconn_add()
    - OPTIM: queue: make the nbpend counters atomic
    - OPTIM: queue: decrement the nbpend and totpend counters outside of the lock
    - MINOR: leastconn: take the queue length into account when queuing servers
    - MEDIUM: fwlc: re-enable per-server queuing up to maxqueue
    - Revert "OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued"
    - MINOR: stats: support the "up" output modifier for "show stat"
    - MINOR: stats: also support a "no-maint" show stat modifier
    - MINOR: stats: indicate the number of servers in a backend's status
    - MEDIUM: ssl: ssl-load-extra-del-ext work only with .crt
    - REGTEST: ssl: test "set ssl cert" with separate key / crt
    - DOC: management: apply the "show stat" modifiers to "show stat", not "show info"
    - MINOR: stats: report server's user-configured weight next to effective weight
    - CI: travis-ci: switch to Ubuntu 20.04
    - CONTRIB: release-estimator: Add release estimating tool
    - BUG/MEDIUM: queue: fix unsafe proxy pointer when counting nbpend
    - BUG/MINOR: extcheck: add missing checks on extchk_setenv()

BUG/MINOR: extcheck: add missing checks on extchk_setenv()

Issue #910 reports that we fail to check a few extchk_setenv() in the
child process. These are mostly harmless, but instead of counting on
the external check script to fail the dirty way, better fail cleanly
when detecting the failure.

This could probably be backported to all stable branches.

BUG/MEDIUM: queue: fix unsafe proxy pointer when counting nbpend

As reported by Coverity in issue #917, commit 96bca33 ("OPTIM: queue:
decrement the nbpend and totpend counters outside of the lock")
introduced a bug when moving the increments outside of the loop,
because we can't always rely on the pendconn "p" here as it may
be null. We can retrieve the proxy pointer directly from s->proxy
instead. The same is true for pendconn_redistribute(), though the
last "p" pointer there was still valid. This patch fixes both.

No backport is needed, this was introduced just before 2.3-dev8.

CONTRIB: release-estimator: Add release estimating tool

This tool monitors the HAProxy stable branches and calculates a proposed
release date for the next minor release based on the bug fixes that are in
the queue.

Print only:
    ./release-estimator.py --print

Send email:
    ./release-estimator.py --send-mail --from-email from@domain.local --to-email to@domain.local

See contrib/release-estimator/README.md for details.

CI: travis-ci: switch to Ubuntu 20.04

we were blocked by https://github.com/vtest/VTest/issues/20
issue is resolved, let us update to focal

MINOR: stats: report server's user-configured weight next to effective weight

The "weight" column on the stats page is somewhat confusing when using
slowstart becaue it reports the effective weight, without being really
explicit about it. In some situations the user-configured weight is more
relevant (especially with long slowstarts where it's important to know
if the configured weight is correct).

This adds a new uweight stat which reports a server's user-configured
weight, and in a backend it receives the sum of all servers' uweights.
In addition it adds the mention of "effective" in a few descriptions
for the "weight" column (help and doc).

As a result, the list of servers in a backend is now always scanned
when dumping the stats. But this is not a problem given that these
servers are already scanned anyway and for way heavier processing.

DOC: management: apply the "show stat" modifiers to "show stat", not "show info"

By mistake I added the "up" then "maint" output modifiers to the "show info"
block instead of the "show stat" one in the two previous commits 65141ffc4
("MINOR: stats: support the "up" output modifier for "show stat"") and
3e3203670 ("MINOR: stats: also support a "no-maint" show stat modifier").

No backport is needed.

REGTEST: ssl: test "set ssl cert" with separate key / crt

This reg-test tests the "set ssl cert" command the same way the
set_ssl_cert.vtc does, but with separate key/crt files and with the
ssl-load-extra-del-ext.

It introduces new key/.crt files that contains the same pair as the
existing .pem.

MEDIUM: ssl: ssl-load-extra-del-ext work only with .crt

In order to be compatible with the "set ssl cert" command of the CLI,
this patch restrict the ssl-load-extra-del-ext to files with a ".crt"
extension in the configuration.

Related to issue #785.

Should be backported where 8e8581e ("MINOR: ssl: 'ssl-load-extra-del-ext'
removes the certificate extension") was backported.

MINOR: stats: indicate the number of servers in a backend's status

When dumping the stats page (or the CSV output), when many states are
mixed, it's hard to figure the number of up servers. But when showing
only the "up" servers or hiding the "maint" servers, there's no way to
know how many servers are configured, which is problematic when trying
to update server-templates.

What this patch does, for dumps in "up" or "no-maint" modes, is to add
after the backend's "UP" or "DOWN" state "(%d/%d)" indicating the number
of servers seen as UP to the total number of servers in the backend. As
such, seeing "UP (33/39)" immediately tells that there are 6 servers that
are not listed when using "up", or will let the client figure how many
servers are left once deducted the number of non-maintenance ones. It's
not done on default dumps so as not to disturb existing tools, which
already have all the information they need in the dump.

MINOR: stats: also support a "no-maint" show stat modifier

"no-maint" is a bit similar to "up" except that it will only hide
servers that are in maintenance (or disabled in the configuration), and
not those that are enabled but failed a check. One benefit here is to
significantly reduce the output of the "show stat" command when using
large server-templates containing entries that are not yet provisioned.

Note that the prometheus exporter also has such an option which does
the exact same.

MINOR: stats: support the "up" output modifier for "show stat"

We already had it on the HTTP interface but it was not accessible on the
CLI. It can be very convenient to hide servers which are down, do not
resolve, or are in maintenance.

Revert "OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued"

This reverts commit b7ba1d901174cb1193033f7d967987ef74e89856. Actually
this test had already been removed in the past by commit fac0f645d
("BUG/MEDIUM: queue: make pendconn_cond_unlink() really thread-safe"),
but the condition to reproduce the bug mentioned there was not clear.

Now after analysis and a certain dose of code cleanup, things start to
appear more obvious. what happens is that if we check the presence of
the node in the tree without taking the lock, we can see the NULL at
the instant the node is being unlinked by another thread in
pendconn_process_next_strm() as part of __pendconn_unlink_prx() or
__pendconn_unlink_srv(). Till now there is no issue except that the
pendconn is not removed from the queue during this operation and that
the task is scheduled to be woken up by pendconn_process_next_strm()
with the stream being added to the list of the server's active
connections by __stream_add_srv_conn(). The first thread finishes
faster and gets back to stream_free() faster than the second one
sets the srv_conn on the stream, so stream_free() skips the s->srv_conn
test and doesn't try to dequeue the freshly queued entry. At the
very least a barrier would be needed there but we can't afford to
free the stream while it's being queued. So there's no other solution
than making sure that either __pendconn_unlink_prx() or
pendconn_cond_unlink() get the entry but never both, which is why the
lock is required around the test. A possible solution would be to set
p->target before unlinking the entry and using it to complete the test.
This would leave no dead period where the pendconn is not seen as
attached.

It is possible, yet extremely difficult, to reproduce this bug, which
was first noticed in bug #880. Running 100 servers with maxconn 1 and
maxqueue 1 on leastconn and a connect timeout of 30ms under 16 threads
with DEBUG_UAF, with a traffic making the backend's queue oscillate
around zero (typically using 250 connections with a local httpterm
server) may rarely manage to trigger a use-after-free.

No backport is needed.

MEDIUM: fwlc: re-enable per-server queuing up to maxqueue

Leastconn has the nice propery of being able to sort servers by their
current usage. It's really a shame to force all requests into the backend
queue when the algo would be able to also consider their current queue.

In order not to change existing behavior but extend it, this patch allows
leastconn to elect servers which are already full if they have an explicitly
configured maxqueue setting above zero and their queue hasn't reached that
threshold. This will significantly reduce the pressure in the backend queue
when queuing a lot with lots of servers.

A test on 8 threads with 100 servers configured with maxconn 1 jumped
from 165krps to 330krps with maxqueue 15 with this patch.

This partially undoes commit 82cd5c13a ("OPTIM: backend: skip LB when we
know the backend is full") but allows to scale much better even by setting
a single-digit maxqueue value. Some better heuristics could be used to
maintain the behavior of the bypass in the patch above, consisting in
keeping it if it's known that there is no server with a configured
maxqueue in the farm (or in the backend).

MINOR: leastconn: take the queue length into account when queuing servers

When servers are queued into the leastconn tree, it's important to also
consider their queue length. There could be some servers with lots of
queued requests that we don't want to hammer with extra connections. In
order not to add extra stress to the LB algorithm, we don't update the
value when adding to the queue, only when updating the connection count
(i.e. picking from the queue or releasing a connection). This will be
sufficient to significantly improve the fairness in such situations.

OPTIM: queue: decrement the nbpend and totpend counters outside of the lock

We don't need to do that inside the lock. However since the operation
used to be done in deep functions, we have to make it resurface closer
to visible parts. It remains reasonably self-contained in queue.c so
that's not that big of a deal. Some places (redistribute) could benefit
from a single operation for all counts at once. Others like
pendconn_process_next_strm() are still called with both locks held but
now it will be possible to change this.

OPTIM: queue: make the nbpend counters atomic

Instead of incrementing, decrementing them and updating their max under
the lock, make them atomic and keep them out of the lock as much as
possible. For __pendconn_unlink_* it would be wide to decide to move
these counters outside of the function, inside the callers so that a
single atomic op can be done per counter even for groups of operations.

MINOR: queue: reduce the locked area in pendconn_add()

Similarly to previous changes, we know if we're dealing with a server
or proxy lock so let's directly lock at the finest possible places
there. It's worth noting that a part of the operation consisting in
an increment and update of a max could be done outside of the lock
using atomic ops and a CAS.

MINOR: queue: split __pendconn_unlink() in per-srv and per-prx

The function is called with the lock held and does too many tests for
things that are already known from its callers. Let's split it in two
so that its callers call either the per-server or per-proxy function
depending on where the element is (since they had to determine it
prior to taking the lock).

OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued

On connection error processing, we can see massive storms of calls to
pendconn_cond_unlink() to release a possible place in the queue. For
example, in issue #908, on average half of the threads are caught in
this function via back_try_conn_req() consecutive to a synchronous
error. However we wait until grabbing the lock to know if the pendconn
is effectively in a queue, which is expensive for many cases. We know
the transition may only happen from in-queue to out-of-queue so it's safe
to first run a preliminary check to see if it's worth going further. This
will allow to avoid the cost of locking for most requests. This should
not change anything for those completing correctly as they're already
run through pendconn_free() which doesn't call pendconn_cond_unlink()
unless deemed necessary.

MINOR: proxy/cli: only take a read lock in "show errors"

There's no point having an exclusive lock here, nothing is modified.

MINOR: server: read-lock the cookie during srv_set_dyncookie()

No need to use an exclusive lock on the proxy anymore when reading its
setting, a read lock is enough. A few other places continue to use a
write-lock when modifying simple flags only in order to let this
function see a consistent value all along. This might be changed in
the future using barriers and local copies.

MINOR: proxy; replace the spinlock with an rwlock

This is an anticipation of finer grained locking for the queues. For now
all lock places take a write lock so that there is no difference at all
with previous code.

MINOR: threads/debug: only report lock stats for used operations

In addition to the previous simplification, most locks don't use the
seek or read lock (e.g. spinlocks etc) so let's split the dump into
distinct operations (write/seek/read) and only report those which
were used. Now the output size is roughly divided by 5 compared
to previous ones.

MINOR: threads/debug: only report used lock stats

The lock stats are very verbose and more than half of them are used in
a typical test, making it hard to spot the sought values. Let's simply
report "not used" for those which have not been called at all.

BUG/MAJOR: mux-h2: Don't try to send data if we know it is no longer possible

In h2_send(), if we are in a state where we know it is no longer possible to
send data, we must exit the sending loop to avoid any possiblity to loop
forever. It may happen if the mbuf ring is released while the H2_CF_MUX_MFULL
flag is still set. Here is a possible scenario to trigger the bug :

  1) The mbuf ring is full because we are unable to send data. The
     H2_CF_MUX_MFULL flag is set on the H2 connection.

  2) At this stage, the task timeout expires because the H2 connection is
     blocked. We enter in h2_timeout_task() function. Because the mbuf ring is
     full, we cannot send the GOAWAY frame. Thus the H2_CF_GOAWAY_FAILED flag is
     set. The H2 connection is not released yet because there is still a stream
     attached. Here we leave h2_timeout_task() function.

  3) A bit later, the H2 connection is woken up. If h2_process(), nothing is
     performed by the first attempt to send data, in h2_send(). Then, because
     the H2_CF_GOAWAY_FAILED flag is set, the mbuf ring is released. But the
     H2_CF_MUX_MFULL flag is still there. At this step a second attempt to send
     data is performed.

  4) In h2_send(), we try to send data in a loop. To exist this loop, done
     variable must be set to 1. Because the H2_CF_MUX_MFULL flag is set, we
     don't call h2_process_mux() and done is not updated. Because the mbuf ring
     is now empty, nothing is sent and the H2_CF_MUX_MFULL flag is never
     removed. Now, we loop forever... waiting for the watchdog.

To fix the bug, we now exit the loop if one of these conditions is true :

  - The H2_CF_GOAWAY_FAILED flag is set on the H2 connection
  - The CO_FL_SOCK_WR_SH flag is set on the underlying connection
  - The H2 connection is in the H2_CS_ERROR2 state

This patch should fix the issue #912 and most probably #875. It must be
backported as far as the 1.8.

BUG/MINOR: http-ana: Don't send payload for internal responses to HEAD requests

When an internal response is returned to a client, the message payload must be
skipped if it is a reply to a HEAD request. The payload is removed from the HTX
message just before the message forwarding.

This bugs has been around for a long time. It was already there in the pre-HTX
versions. In legacy HTTP mode, internal errors are not parsed. So this bug
cannot be easily fixed. Thus, this patch should only be backported in all HTX
versions, as far as 2.0. However, the code has significantly changed in the
2.2. Thus in the 2.1 and 2.0, the patch must be entirely reworked.

CLEANUP: compression: Make use of http_get_etag_type()

This commit makes the compressor use http_get_etag_type to validate the
ETag instead of using an ad-hoc condition.

REGTEST: cache: Add if-none-match test case

Test that if-none-match header is properly taken into account and that
when the conditions are fulfilled, a "304 Not Modified" response can be
sent to the client.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>

MEDIUM: cache: Add support for 'If-None-Match' request header

Partial support of conditional HTTP requests. This commit adds the
support of the 'If-None-Match' header (see RFC 7232#3.2).
When a client specifies a list of ETags through one or more
'If-None-Match' headers, they are all compared to the one that might have
been stored in the corresponding http cache entry until one of them
matches.
If a match happens, a specific "304 Not Modified" response is
sent instead of the cached data. This response has all the stored
headers but no other data (see RFC 7232#4.1). Otherwise, the whole cached data
is sent.
Although unlikely in a GET/HEAD request, the "If-None-Match: *" syntax is
valid and also receives a "304 Not Modified" response (RFC 7434#4.3.2).

This resolves a part of GitHub issue #821.

MEDIUM: cache: Store the ETag information in the cache_entry

When sent by a server for a given resource, the ETag header is
stored in the coresponding cache entry (as any other header). So in
order to perform future ETag comparisons (for subsequent conditional
HTTP requests), we keep the length of the ETag and its offset
relative to the start of the cache_entry.
If no ETag header exists, the length and offset are zero.

MINOR: http: Add etag comparison function

Add a function that compares two etags that might be of different types.
If any of them is weak, the 'W/' prefix is discarded and a strict string
comparison is performed.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>

MINOR: http: Add `enum etag_type http_get_etag_type(const struct ist)`

http_get_etag_type returns whether a given `etag` is a strong, weak, or invalid
ETag.

BUG/MEDIUM: server: support changing the slowstart value from state-file

If the slowstart value in a state file implies the latest state change
is within the slowstart period, we end up calling srv_update_status()
to reschedule the server's state change but its task is not yet
allocated and remains null, causing a crash on startup.

Make sure srv_update_status() supports being called with partially
initialized servers which do not yet have a task. If the task has to
be scheduled, it will necessarily happen after initialization since
it will result from a state change.

This should be backported wherever server-state is present.

BUILD: makefile: add entries to build common debugging tools

A few tools in contrib/ such as halog, flags, poll and tcploop are
occasionally useful at least to developers, and some of them such as
halog or flags can occasionally break due to some changes in the include
files. As reported in issue #907, their inability to inherit the global
build options also causes some warnings related to some specificities
of the main include files. Let's just add entries in the main makefile
to build them.

CONTRIB: tcploop: remove unused local variables in tcp_pause()

Building with -Wall shows that "pollfd" and "ret" are not used. Silly
copy-paste...

BUG/MINOR: queue: properly report redistributed connections

In commit 5cd4bbd7a ("BUG/MAJOR: threads/queue: Fix thread-safety issues
on the queues management") the counter of transferred connections was
accidently lost, so that when a server goes down with connections in its
queue, it will always be reported that 0 connection were transferred.

This should be backported as far as 1.8 since the patch above was
backported there.

MINOR: ssl: 'ssl-load-extra-del-ext' removes the certificate extension

In issue #785, users are reporting that it's not convenient to load a
".crt.key" when the configuration contains a ".crt".

This option allows to remove the extension of the certificate before
trying to load any extra SSL file (.key, .ocsp, .sctl, .issuer etc.)

The patch changes a little bit the way ssl_sock_load_files_into_ckch()
looks for the file.

BUG/MINOR: listener: close before free in `listener_accept`

safer to close handle before the object is put back in the global pool.

this was introduced by commit 9378bbe0bef4005155d ("MEDIUM: listener:
use protocol->accept_conn() to accept a connection")

this should fix github issue #902

no backport needed.

Signed-off-by: William Dauchy <wdauchy@gmail.com>

MEDIUM: config: report that "nbproc" is deprecated

As previously discussed, nbproc usage is bad, deprecated, and scheduled
for removal in 2.5.

If "nbproc" is found with more than one process while nbthread is not
set, a warning will be emitted encouraging to remove it or to migrate
to nbthread instead. This makes sure the user has an opportunity to
both see the message and silence it.

BUG/MEDIUM: connection: Never cleanup server lists when freeing private conns

When a connection is released, depending on its state, it may be detached from
the session and it may be removed from the server lists. The first case may
happen for private or unsharable active connections. The second one should only
be performed for idle or available connections. We never try to remove a
connection from the server list if it is attached to a session. But it is also
important to never try to remove a private connecion from the server lists, even
if it is not attached to a session. Otherwise, the curr_used_conn server counter
is decremented once too often.

This bug was introduced by the commit 04a24c5ea ("MINOR: connection: don't check
priv flag on free"). It is related to the issue #881. It only affects the 2.3,
no backport is needed.

CLEANUP: task: remove the unused and mishandled global_rqueue_size

This counter is only updated and never used, and in addition it's done
without any atomicity so it's very unlikely to be correct on multi-CPU
systems! Let's just remove it since it's not used.

CLEANUP: tree-wide: reorder a few structures to plug some holes around locks

A few structures were slightly rearranged in order to plug some holes
left around the locks. Sizes ranging from 8 to 32 bytes could be saved
depending on the structures. No performance difference was noticed (none
was expected there), though memory usage might be slightly reduced in
some rare cases.

MINOR: threads: change lock_t to an unsigned int

We don't need to waste the size of a long for the locks: with the plocks,
even an unsigned short would offer enough room for up to 126 threads! Let's
use an unsigned int which will be easier to place in certain structures
and will more conveniently plug some holes, and Atomic ops are at least
as fast on 32-bit as on 64-bit. This will not change anything for 32-bit
platforms.

CLEANUP: threads: don't register an initcall when not debugging

It's a bit overkill to register an initcall to call a function to set
a lock to zero when not debugging, let's just declare the lock as
pre-initialized to zero.

BUILD: ssl: make BoringSSL use its own version numbers

BoringSSL is a fork of OpenSSL 1.1.0, however in
49e9f67d8b7cbeb3953b5548ad1009d15947a523 it has changed version to 1.1.1.

Should fix issue #895.

This must be backported to 2.2, 2.1, 2.0, 1.8

BUG/MINOR: disable dynamic OCSP load with BoringSSL

it was accidently enabled on BoringSSL while
actually it is not supported

wla: Fix part of the issue mentionned in #895.
It fixes build of boringSSL versions prior to commit
https://boringssl.googlesource.com/boringssl/+/49e9f67d8b7cbeb3953b5548ad1009d15947a523

Must be backported in 2.2.

Signed-off-by: William Lallemand <wlallemand@haproxy.org>

MINOR: lb/chash: use a read lock in chash_get_server_hash()

When using a low hash-balance-factor value, it's possible to loop
many times trying to find the best server. Figures in the order of
100-300 times were observed for 1000 servers with a factor of 101
(which seems a bit excessive for such a large farm). Given that
there's nothing in that function that prevents multiple threads
from working in parallel, let's switch to a read lock. Tests on
8 threads show roughly a 2% performance increase with this.

MINOR: lb/first: use a read lock in fas_get_next_server()

The "first" algorithm creates a lot of contention because all threads
focus on the same server by definition (the first available one). By
turning the exclusive lock to a read lock in fas_get_next_server(),
the request rate increases by 16% for 8 threads when many servers are
getting close to their maxconn.

MINOR: lb/leastconn: only take a read lock in fwlc_get_next_server()

This function doesn't change the tree, it only looks for the first
usable server, so let's do that under a read lock to limit the
situations like the ones described in issue #881 where finding a
usable server when dealing with lots of saturated ones can be
expensive. At least threads will now be able to look up in
parallel.

It's interesting to note that s->served is not incremented during the
server choice, nor is the server repositionned. So right now already,
nothing prevents multiple threads from picking the same server. This
will not cause a significant imbalance anyway given that the server
will automatically be repositionned at the right place, but this might
be something to improve in the future if it doesn't come with too high
a cost.

It also looks like the way a server's weight is updated could be
revisited so that the write lock gets tighter at the expense of a
short part of inconsistency between weights and servers still present
in the tree.

MINOR: lb/map: use seek lock and read locks where appropriate

- map_get_server_hash() doesn't need a write lock since it only
  reads the array, let's only use a read lock here.

- map_get_server_rr() only needs exclusivity to adjust the rr_idx
  while looking for its entry. Since this one is not used by
  map_get_server_hash(), let's turn this lock to a seek lock that
  doesn't block reads.

With 8 threads, no significant performance difference was noticed
given that lookups are usually instant with this LB algo so the
lock contention is rare.

MINOR: backend: replace the lbprm lock with an rwlock

It was previously a spinlock, and it happens that a number of LB algos
only lock it for lookups, without performing any modification. Let's
first turn it to an rwlock and w-lock it everywhere. This is strictly
identical.

It was carefully checked that every HA_SPIN_LOCK() was turned to
HA_RWLOCK_WRLOCK() and that HA_SPIN_UNLOCK() was turned to
HA_RWLOCK_WRUNLOCK() on this lock. _INIT and _DESTROY were updated too.

[RELEASE] Released version 2.3-dev7

Released version 2.3-dev7 with the following main changes :
    - CI: travis-ci: replace not defined SSL_LIB, SSL_INC for BotringSSL builds
    - BUG/MINOR: init: only keep rlim_fd_cur if max is unlimited
    - BUG/MINOR: mux-h2: do not stop outgoing connections on stopping
    - MINOR: fd: report an error message when failing initial allocations
    - MINOR: proto-tcp: make use of connect(AF_UNSPEC) for the pause
    - MINOR: sock: add sock_accept_conn() to test a listening socket
    - MINOR: protocol: make proto_tcp & proto_uxst report listening sockets
    - MINOR: sockpair: implement the .rx_listening function
    - CLEANUP: tcp: make use of sock_accept_conn() where relevant
    - CLEANUP: unix: make use of sock_accept_conn() where relevant
    - BUG/MINOR: listener: detect and handle shared sockets stopped in other processes
    - CONTRIB: tcploop: implement a disconnect operation 'D'
    - CLEANUP: protocol: intitialize all of the sockaddr when disconnecting
    - BUG/MEDIUM: deinit: check fdtab before fdtab[fd].owner
    - BUG/MINOR: connection: fix loop iter on connection takeover
    - BUG/MEDIUM: connection: fix srv idle count on conn takeover
    - MINOR: connection: improve list api usage
    - MINOR: mux/connection: add a new mux flag for HOL risk
    - MINOR: connection: don't check priv flag on free
    - MEDIUM: backend: add new conn to session if mux marked as HOL blocking
    - MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking
    - MEDIUM: h2: remove conn from session on detach
    - MEDIUM: fcgi: remove conn from session on detach
    - DOC: Describe reuse safe for HOL handling
    - MEDIUM: proxy: remove obsolete "mode health"
    - MEDIUM: proxy: remove obsolete "monitor-net"
    - CLEANUP: protocol: remove the ->drain() function
    - CLEANUP: fd: finally get rid of fd_done_recv()
    - MINOR: connection: make sockaddr_alloc() take the address to be copied
    - MEDIUM: listener: allocate the connection before queuing a new connection
    - MINOR: session: simplify error path in session_accept_fd()
    - MINOR: connection: add new error codes for accept_conn()
    - MINOR: sock: rename sock_accept_conn() to sock_accepting_conn()
    - MINOR: protocol: add a new function accept_conn()
    - MINOR: sock: implement sock_accept_conn() to accept a connection
    - MINOR: sockpair: implement sockpair_accept_conn() to accept a connection
    - MEDIUM: listener: use protocol->accept_conn() to accept a connection
    - MEDIUM: listener: remove the second pass of fd manipulation at the end
    - MINOR: protocol: add a default I/O callback and put it into the receiver
    - MINOR: log: set the UDP receiver's I/O handler in the receiver
    - MINOR: protocol: register the receiver's I/O handler and not the protocol's
    - CLEANUP: protocol: remove the now unused <handler> field of proto_fam->bind()
    - DOC: improve the documentation for "option nolinger"
    - BUG/MEDIUM: proxy: properly stop backends
    - BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once
    - MINOR: threads: augment rwlock debugging stats to report seek lock stats
    - MINOR: threads: add the transitions to/from the seek state
    - MEDIUM: task: use an upgradable seek lock when scanning the wait queue
    - BUILD: listener: avoir a build warning when threads are disabled
    - BUG/MINOR: peers: Possible unexpected peer seesion reset after collisions.
    - MINOR: ssl: add volatile flags to ssl samples
    - MEDIUM: backend: reuse connection if using a static sni
    - BUG/MEDIUM: spoe: Unset variable instead of set it if no data provided
    - BUG/MEDIUM: mux-h1: Get the session from the H1S when capturing bad messages
    - BUG/MEDIUM: lb: Always lock the server when calling server_{take,drop}_conn
    - DOC: fix typo in MAX_SESS_STKCTR

DOC: fix typo in MAX_SESS_STKCTR

MAX_SESS_STKCTR is spelled wrongly a couple of times
in the configuration docs (K and C are swapped). This patch
fixes the typos.

BUG/MEDIUM: lb: Always lock the server when calling server_{take,drop}_conn

The server lock must be held when server_take_conn() and server_drop_conn()
lbprm callback functions are called. It is a documented prerequisite but it is
not always performed. It only affects leastconn and fas lb algorithm. Others
don't use these callback functions.

A race condition on the next pending effecive weight (next_eweight) may be
encountered with the leastconn lb algorithm. An agent check may set it to 0
while fwlc_srv_reposition() is called. The server is locked during the
next_eweight update. But because the server lock is not acquired when
fwlc_srv_reposition() is called, we may use it to recompute the server key,
leading to a division by 0.

This patch must be backported as far as 1.8.

BUG/MEDIUM: mux-h1: Get the session from the H1S when capturing bad messages

It is not guaranteed that the backend connection has an owner. It is set when
the connection is created. But when the connection is moved in a server idle
list, the connection owner is set to NULL and may never be set again. On the
other hand, when a mux is created or when a CS is attached, the session is
always defined. The H1 stream always keep a reference on it when it is
created. Thus, when a bad message is captured we should not rely on the
connection owner to retrieve the session. Instead we should get it from the H1
stream.

BUG/MEDIUM: spoe: Unset variable instead of set it if no data provided

If an agent try to set a variable with the NULL data type, an unset is perform
instead to avoid undefined behaviors. Once decoded, such data are translated to
a sample with the type SMP_T_ANY. It is unexpected in HAProxy. When a variable
is set with such sample, no data are attached to the variable. Thus, when the
variable is retrieved later in the transaction, the sample data are
uninitialized, leading to undefined behaviors depending on how it is used. For
instance, it leads to a crash if the debug converter is used on such variable.

This patch should fix the issue #855. It must be backported as far as 1.8.

MEDIUM: backend: reuse connection if using a static sni

Detect if the sni used a constant value and if so, allow to reuse this
connection for later sessions. Use a combination of SMP_USE_INTRN +
!SMP_F_VOLATILE to consider a sample as a constant value.

This features has been requested on github issue #371.

MINOR: ssl: add volatile flags to ssl samples

The ssl samples are not constant over time and change according to the
session. Add the flag SMP_F_VOL_SESS to indicate this.

BUG/MINOR: peers: Possible unexpected peer seesion reset after collisions.

During a peers session collision (two peer sessions opened on both side) we must
mark the peer the session of which will be shutdown as alive, if not ->reconnect
timer will be set with a wrong value if the synchro task expires after the peer
has been reconnected. This possibly leads to unexpected deconnections during handshakes.
Furthermore, this patch cancels any heartbeat tranmimission when a reconnection
is prepared.

BUILD: listener: avoir a build warning when threads are disabled

It's just a __decl_thread() that appeared before the last variable.

MEDIUM: task: use an upgradable seek lock when scanning the wait queue

Right now when running a configuration with many global timers (e.g. many
health checks), there is a lot of contention on the global wait queue
lock because all threads queue up in front of it to scan it.

With 2000 servers checked every 10 milliseconds (200k checks per second),
after 23 seconds running on 8 threads, the lock stats were this high:

  Stats about Lock TASK_WQ:
      write lock  : 9872564
      write unlock: 9872564 (0)
      wait time for write     : 9208.409 msec
      wait time for write/lock: 932.727 nsec
      read lock   : 240367
      read unlock : 240367 (0)
      wait time for read      : 149.025 msec
      wait time for read/lock : 619.991 nsec

i.e. ~5% of the total runtime spent waiting on this specific lock.

With upgradable locks we don't need to work like this anymore. We
can just try to upgade the read lock to a seek lock before scanning
the queue, then upgrade the seek lock to a write lock for each element
we want to delete there and immediately downgrade it to a seek lock.

The benefit is double:
  - all other threads which need to call next_expired_task() before
    polling won't wait anymore since the seek lock is compatible with
    the read lock ;

  - all other threads competing on trying to grab this lock will fail
    on the upgrade attempt from read to seek, and will let the current
    lock owner finish collecting expired entries.

Doing only this has reduced the wake_expired_tasks() CPU usage in a
very large servers test from 2.15% to 1.04% as reported by perf top,
and increased by 3% the health check rate (all threads being saturated).

This is expected to help against (and possibly solve) the problem
described in issue #875.

MINOR: threads: add the transitions to/from the seek state

Since our locks are based on progressive locks, we support the upgradable
seek lock that is compatible with readers and upgradable to a write lock.
The main purpose is to take it while seeking down a tree for modification
while other threads may seek the same tree for an input (e.g. compute the
next event date).

The newly supported operations are:

  HA_RWLOCK_SKLOCK(lbl,l)         pl_take_s(l)      /* N --> S */
  HA_RWLOCK_SKTOWR(lbl,l)         pl_stow(l)        /* S --> W */
  HA_RWLOCK_WRTOSK(lbl,l)         pl_wtos(l)        /* W --> S */
  HA_RWLOCK_SKTORD(lbl,l)         pl_stor(l)        /* S --> R */
  HA_RWLOCK_WRTORD(lbl,l)         pl_wtor(l)        /* W --> R */
  HA_RWLOCK_SKUNLOCK(lbl,l)       pl_drop_s(l)      /* S --> N */
  HA_RWLOCK_TRYSKLOCK(lbl,l)      (!pl_try_s(l))    /* N -?> S */
  HA_RWLOCK_TRYRDTOSK(lbl,l)      (!pl_try_rtos(l)) /* R -?> S */

Existing code paths are left unaffected so this patch doesn't affect
any running code.

MINOR: threads: augment rwlock debugging stats to report seek lock stats

We currently use only read and write lock operations with rwlocks, but
ours also support upgradable seek locks for which we do not report any
stats. Let's add them now when DEBUG_THREAD is enabled.

BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once

There is a theorical problem in the wait queue, which is that with many
threads, one could spend a lot of time looping on the newly expired tasks,
causing a lot of contention on the global wq_lock and on the global
rq_lock. This initially sounds bening, but if another thread does just
a task_schedule() or task_queue(), it might end up waiting for a long
time on this lock, and this wait time will count on its execution budget,
degrading the end user's experience and possibly risking to trigger the
watchdog if that lasts too long.

The simplest (and backportable) solution here consists in bounding the
number of expired tasks that may be picked from the global wait queue at
once by a thread, given that all other ones will do it as well anyway.

We don't need to pick more than global.tune.runqueue_depth tasks at once
as we won't process more, so this counter is updated for both the local
and the global queues: threads with more local expired tasks will pick
less global tasks and conversely, keeping the load balanced between all
threads. This will guarantee a much lower latency if/when wakeup storms
happen (e.g. hundreds of thousands of synchronized health checks).

Note that some crashes have been witnessed with 1/4 of the threads in
wake_expired_tasks() and, while the issue might or might not be related,
not having reasonable bounds here definitely justifies why we can spend
so much time there.

This patch should be backported, probably as far as 2.0 (maybe with
some adaptations).

BUG/MEDIUM: proxy: properly stop backends

The proxy stopping mechanism was changed with commit 322b9b94e ("MEDIUM:
proxy: make stop_proxy() now use stop_listener()") so that it's now
entirely driven by the listeners. One thing was forgotten though, which
is that pure backends will not stop anymore since they don't have any
listener, and that it's necessary to stop them in order to stop the
health checks.

No backport is needed.

DOC: improve the documentation for "option nolinger"

This reminds the different issues caused by option nolinger, as discussed
in issue #896.

CLEANUP: protocol: remove the now unused <handler> field of proto_fam->bind()

We don't need to specify the handler anymore since it's set in the
receiver. Let's remove this argument from the function and clean up
the remains of code that were still setting it.

MINOR: protocol: register the receiver's I/O handler and not the protocol's

Now we define a new sock_accept_iocb() for socket-based stream protocols
and use it as a wrapper for listener_accept() which now takes a listener
and not an FD anymore. This will allow the receiver's I/O cb to be
redefined during registration, and more specifically to get rid of the
hard-coded hacks in protocol_bind_all() made for syslog.

The previous ->accept() callback in the protocol was removed since it
doesn't have anything to do with accept() anymore but is more generic.
A few places where listener_accept() was compared against the FD's IO
callback for debugging purposes on the CLI were updated.