MEDIUM: resolvers/dns: split dns.c into dns.c and resolvers.c

This patch splits current dns.c into two files:

The first dns.c contains code related to DNS message exchange over UDP
and in future other TCP. We try to remove depencies to resolving
to make it usable by other stuff as DNS load balancing.

The new resolvers.c inherit of the code specific to the actual
resolvers.

Note:
It was really difficult to obtain a clean diff dur to the amount
of moved code.

Note2:
Counters and stuff related to stats is not cleany separated because
currently counters for both layers are merged and hard to separate
for now.

MEDIUM: resolvers: split resolving and dns message exchange layers.

This patch splits recv and send functions in two layers. the
lowest is responsible of DNS message transactions over
the network. Doing this we could use DNS message layer
for something else than resolving. Load balancing for instance.

This patch also re-works the way to init a nameserver and
introduce the new struct dns_dgram_server to prepare the arrival
of dns_stream_server and the support of DNS over TCP.

The way to retry a send failure of a request because of EAGAIN
was re-worked. Previously there was no control and all "pending"
queries were re-played each time it reaches a EAGAIN. This
patch introduce a ring to stack messages in case of sent
failure. This patch is emptied if poller shows that the
socket is ready again to push messages.

MINOR: resolvers: rework dns stats prototype because specific to resolvers

Counters are currently stored into lowlevel nameservers struct but
most of them are resolving layer data and increased in the upper layer
So this patch renames the prototype used to allocate/dump them with prefix
'resolv' waiting for a clean split.

MINOR: resolvers: replace nameserver's resolver ref by generic parent pointer

This will allow to use nameservers in something else than a resolver
section (load balancing for instance).

MEDIUM: resolvers: move resolvers section parsing from cfgparse.c to dns.c

The resolver section parsing is moved from cfgparse.c to dns.c

MINOR: resolvers: rework prototype suffixes to split resolving and dns.

A lot of prototypes in dns.h are specific to resolvers and must
be renamed to split resolving and DNS layers.

MINOR: resolvers: renames resolvers DNS_UPD_* returncodes to RSLV_UPD_*

This patch renames some #defines prefixes from DNS to RSLV.

MINOR: resolvers: renames resolvers DNS_RESP_* errcodes RSLV_RESP_*

This patch renames some #defines prefixes from DNS to RSLV.

MINOR: resolvers: renames some dns prefixed types using resolv prefix.

@@ -119,8 +119,8 @@ struct act_rule {
-               } dns;                         /* dns resolution */
+               } resolv;                      /* resolving */

-struct dns_options {
+struct resolv_options {

MINOR: resolvers: renames some resolvers specific types to not use dns prefix

This patch applies those changes on names:

-struct dns_resolution {
+struct resolv_resolution {

-struct dns_requester {
+struct resolv_requester {

-struct dns_srvrq {
+struct resolv_srvrq {

@@ -185,12 +185,12 @@ struct stream {

        struct {
-               struct dns_requester *dns_requester;
+               struct resolv_requester *requester;
...
-       } dns_ctx;
+       } resolv_ctx;

MINOR: resolvers: renames type dns_resolvers to resolvers.

It also renames 'dns_resolvers' head list to sec_resolvers
to avoid conflicts with local variables 'resolvers'.

MINOR: resolvers: renames some resolvers internal types and removes dns prefix

Some types are specific to resolver code and a renamed using
the 'resolv' prefix instead 'dns'.

-struct dns_query_item {
+struct resolv_query_item {

-struct dns_answer_item {
+struct resolv_answer_item {

-struct dns_response_packet {
+struct resolv_response {

BUG/MINOR: resolvers: fix attribute packed struct for dns

This patch adds the attribute packed on struct dns_question
because it is directly memcpy to network building a response.

This patch also removes the commented line:
//     struct list options;       /* list of option records */

because it is also used directly using memcpy to build a request
and must not contain host data.

BUG/MINOR: dns: add missing sent counter and parent id to dns counters.

Resolv callbacks are also updated to rely on counters and not on
nameservers.
"show stat domain dns" will now show the parent id (i.e. resolvers
section name).

CLEANUP: channel: fix comment in ci_putblk.

The comment is outdated and refer to an old code.

Should be backported until branch 1.5

MINOR: ring: adds new ring_init function.

Adds the new ring_init function to initialize
a pre-allocated ring struct using the given
memory area.

MINOR: tcp: add support for defer-accept on FreeBSD.

FreeBSD has a kernel feature (accf) and a sockopt flag similar to the
Linux's TCP_DEFER_ACCEPT to filter incoming data upon ACK. The main
difference is the filter needs to be placed when the socket actually
listens.

DOC: tune: explain the origin of block size for ssl.cachesize

A user could eventually ask himself where those 200 bytes block size are
coming from. This patch tries to better explain the origin in case
people are curious or want to double check the reality.

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

MINOR: cfgparse: implement a simple if/elif/else/endif macro block handler

Very often, especially since reg-tests, it would be desirable to be able
to conditionally comment out a config block, such as removing an SSL
binding when SSL is disabled, or enabling HTX only for certain versions,
etc.

This patch introduces a very simple nested block management which takes
".if", ".elif", ".else" and ".endif" directives to take or ignore a block.

For now the conditions are limited to empty string or "0" for false versus
a non-nul integer for true, which already suffices to test environment
variables. Still, it needs to be a bit more advanced with defines, versions
etc.

A set of ".notice", ".warning" and ".alert" statements are provided to
emit messages, often in order to provide advice about how to fix certain
conditions.

MINOR: peers/cli: do not dump the peers dictionaries by default on "show peers"

The "show peers" output has become huge due to the dictionaries making it
less readable. Now this feature has reached a certain level of maturity
which doesn't warrant to dump it all the time, given that it was essentially
needed by developers. Let's make it optional, and disabled by default, only
when "show peers dict" is requested. The default output reminds about the
command. The output has been divided by 5 :

  $ socat - /tmp/sock1  <<< "show peers dict" | wc -l
  125
  $ socat - /tmp/sock1  <<< "show peers" | wc -l
  26

It could be useful to backport this to recent stable versions.

CLEANUP: server: Remove useless "filepath" variable in apply_server_state()

This variable is now only used to point on the local server-state file. When
the server-state is global, it is unused. So, we now use "localfilepath"
instead. Thus, the "filepath" variable can safely be removed.

BUG/MINOR: server: Don't call fopen() with server-state filepath set to NULL

When a local server-state file is loaded, if its name is too long, the error
is not properly handled, resulting to a call to fopen() with the "filepath"
variable set to NULL. To fix the bug, when this error occurs, we jump to the
next proxy, via a "continue" statement. And we take case to set "filepath"
variable after the error handling to be sure.

This patch should fix the issue #1111. It must be backported as far as 1.6.

CLEANUP: tcpcheck: Remove a useless test on port variable

When a connect rule is evaluated a test is performed on the "port" variable
while it is set to 0 just on the line just above. Just remove this useless
test to make ccpcheck happy.

This patch fixes the issue #1113.

MINOR: http: add baseq sample fetch

Symetrical to path/pathq, baseq returns the concatenation of
the Host header and the path including the query string.

MEDIUM: cfgparse: allow a proxy to designate the defaults section to use

Now it becomes possible to specify "from foo" on a frontend/listen/backend
or even on a "defaults" line, to mention that defaults section "foo" needs
to be used to preset the proxy's settings.

When not set, the last section remains used. In case the designated name
is found at multiple places, it is rejected and an error indicates two
occurrences of the same name. Similarly, if the section name is found,
its name must only use valid characters. This allows multiple named
defaults section to continue to coexist without the risk that they will
cause trouble by accident.

When it comes to "defaults" relying on another defaults, what happens is
just that a new defaults section is created from the designated one. This
will make it possible for example to reuse some settings such as log-format
like below:

    defaults tcp-clear
        log stdout local0 info
        log-format "%ci:%cp/%b/%si:%sp %ST %ts %U/%B %{+Q}r"

    defaults tcp-ssl
        log stdout local0 info
        log-format "%ci:%cp/%b/%si:%sp %ST %ts %U/%B %{+Q}r ssl=%sslv"

    defaults http-clear from tcp-clear
        mode http

    defaults http-ssl from tcp-ssl
        mode http

    frontend fe1 from http-clear
        bind :8001

    frontend fe2 from http-ssl
        bind :8002

A small corner case remains in the error detection, if a second defaults
section appears with the same name after the point where it was used, and
nobody references it, the duplicate will not be detected. This could be
addressed by performing the syntactic checks in check_config_validity(),
and by postponing the freeing of the defaults, after tagging a defaults
section as explicitly looked up by another section. This doesn't seem
that important at the moment though.

MEDIUM: proxy: store the default proxies in a tree by name

Now default proxies are stored into a dedicated tree, sorted by name.
Only unnamed entries are not kept upon new section creation. The very
first call to cfg_parse_listen() will automatically allocate a dummy
defaults section which corresponds to the previous static one, since
the code requires to have one at a few places.

The first immediately visible benefit is that it allows to reuse
alloc_new_proxy() to allocate a defaults section instead of doing it by
hand. And the secret goal is to allow to keep multiple named defaults
section in memory to reuse them from various proxies.

MINOR: proxy: support storing defaults sections into their own tree

Now we'll have a tree of named defaults sections. The regular insertion
and lookup functions take care of the capability in order to select the
appropriate tree. A new function proxy_destroy_defaults() removes a
proxy from this tree and frees it entirely.

MINOR: proxy: also store the name for a defaults section

There's an optional name, but till now it was not even saved into the
structure, let's keep it.

MINOR: cfgparse: use a pointer to the current default proxy

In order to make the default proxy configurable, we'll need to have a
pointer to it which might differ from &defproxy. cfg_parse_listen()
now gets curr_defproxy for this.

MINOR: cfgparse: check PR_CAP_DEF instead of comparing poiner against defproxy

We want to get rid of this defproxy, let's now simply check the proxy's
capabilities instead of comparing its pointer to the known default one.

MINOR: proxy: add a new capability PR_CAP_DEF

In order to more easily distinguish a default proxy from a standard one,
let's introduce a new capability PR_CAP_DEF.

MINOR: cfgparse: move defproxy to cfgparse-listen as a static

We don't want to expose this one anymore as we'll soon keep multiple
default proxies. Let's move it inside the parser which is the only
place which still uses it, and initialize it on the fly once needed
instead of doing it at boot time.

BUG/MINOR: server: parse_server() must take a const for the defproxy

The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.

BUG/MINOR: tcpcheck: proxy_parse_*check*() must take a const for the defproxy

The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.

BUG/MINOR: extcheck: proxy_parse_extcheck() must take a const for the defproxy

The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.

MINOR: proxy: always properly reset the just freed default instance pointers

In proxy_free_defaults(); none of the free() calls was followed by a
pointer reset. Not only it's hard to figure if one of them is duplicated,
but this code started to call other functions which might or might not
rely on such just freed pointers. Let's reset them as they should be to
make sure there will never be any case of use-after-free. The 3 functions
called there were inspected and are all unaffected by this so this remains
safe to do right now.

MINOR: proxy: move the defproxy freeing code to proxy.c

This used to be open-coded in cfgparse-listen.c when facing a "defaults"
keyword. Let's move this into proxy_free_defaults(). This code is ugly and
doesn't even reset the just freed pointers. Let's not change this yet.

This code should probably be merged with a generic proxy deinit function
called from deinit(). However there's a catch on uri_auth which cannot be
freed because it might be used by one or several proxies. We definitely
need refcounts there!

MEDIUM: proxy: only take defaults when a default proxy is passed.

The proxy initialization code relies on three phases, allocation,
pre-initialization, and assignments from defaults. This last part is
entirely taken from the defaults proxy when arguments are set. This
sensibly complexifies the initialization code as it requires to always
have a default proxy.

This patch instead first applies the original default settings on a
proxy, and then uses those from a default proxy only if one such is
used. This will allow to initialize a proxy out of any default proxy
while still using valid defaults. A careful inspection of the function
showed that only 4 fields used to be set regardless of the default
proxy, and those were moved to init_new_proxy() where they ought to
have been in the first place.

REORG: proxy: centralize the proxy allocation code into alloc_new_proxy()

This new function takes over the old open-coding that used to be done
for too long in cfg_parse_listen() and it now does everything at once
in a proxy-centric function. The function does all the job of allocating
the structure, initializing it, presetting its defaults from the default
proxy and checking for errors. The code was almost unchanged except for
defproxy being passed as a pointer, and the error message being passed
using memprintf().

This change will be needed to ease reuse of multiple default proxies,
or to create dynamic backends in a distant future.

REORG: move init_default_instance() to proxy.c and pass it the defproxy pointer

init_default_instance() was still left in cfgparse.c which is not the
best place to pre-initialize a proxy. Let's place it in proxy.c just
after init_new_proxy(), take this opportunity for renaming it to
proxy_preset_defaults() and taking out init_new_proxy() from it, and
let's pass it the pointer to the default proxy to be initialized instead
of implicitly assuming defproxy. We'll soon be able to exploit this.
Only two call places had to be updated.

BUILD: proxy: add missing compression-t.h to proxy-t.h

struct comp is used in struct proxy but never declared prior to this
so depending on where proxy.h is included, touching the <comp> field
can break the build.

BUG/MINOR: tcpheck: the source list must be a const in dup_tcpcheck_var()

This is just an API bug but it's annoying when trying to tidy the code.
The source list passed in argument must be a const and not a variable,
as it's typically the list head from a default proxy and must obviously
not be modified by the function. No backport is needed as it only impacts
new code.

BUG/MINOR: http-htx: defpx must be a const in proxy_dup_default_conf_errors()

This is just an API bug but it's annoying when trying to tidy the code.
The default proxy passed in argument must be a const and not a variable.
No backport is needed as it only impacts new code.

BUG/MINOR: cfgparse: do not mention "addr:port" as supported on proxy lines

The very old error message indicating that a proxy name is mandatory
still had a reference to the optional addr:port argument while this one
is explicitly rejected a few lines later since at least 1.9.

This is harmless but confusing. This can be backported to 2.0.

BUG/MINOR: stats: revert the change on ST_CONVDONE

In 2.1, commit ee4f5f83d ("MINOR: stats: get rid of the ST_CONVDONE flag")
introduced a subtle bug. By testing curproxy against defproxy in
check_config_validity(), it tried to eliminate the need for a flag
to indicate that stats authentication rules were already compiled,
but by doing so it left the issue opened for the case where a new
defaults section appears after the two proxies sharing the first
one:

      defaults
          mode http
          stats auth foo:bar

      listen l1
          bind :8080

      listen l2
          bind :8181

      defaults
          # just to break above

This config results in:
  [ALERT] 042/113725 (3121) : proxy 'f2': stats 'auth'/'realm' and 'http-request' can't be used at the same time.
  [ALERT] 042/113725 (3121) : Fatal errors found in configuration.

Removing the last defaults remains OK. It turns out that the cleanups
that followed that patch render it useless, so the best fix is to revert
the change (with the up-to-date flags instead). The flag was marked as
belonging to the config. It's not exact but it's the closest to the
reality, as it's not there to configure the behavior but ti mention
that the config parser did its job.

This could be backported as far as 2.1, but in practice it looks like
nobody ever hit it.

BUG/MEDIUM: config: don't pick unset values from last defaults section

Since commit 1.3.14 with commit 1fa3126ec ("[MEDIUM] introduce separation
between contimeout, and tarpit + queue"), check_config_validity() looks
at the last defaults section to update all proxies' queue and tarpit
timeouts if they were not set!

This was apparently an attempt to properly set them on the fallback values,
except that the fallback values were taken from the default proxy before
looking at the current proxy itself. The worst part of it is that it might
have randomly worked by accident for some configurations when there was a
single defaults section, but has certainly caused too short queue
expirations once another defaults section was added later in the file with
these explicitly defined.

Let's remove the defproxy part and keep only the curproxy ones. This could
be backported everywhere, the bug has been there for 13 years.

CLEANUP: deinit: release global and per-proxy server-state variables on deinit

The global server-state base directory and file name are now released on
deinit, as well as per-proxy server-state file name.

BUG/MINOR: server: Fix server-state-file-name directive

Since the beginning, this directive is documented to accept an optional file
name. But it should also be possible to use it without any argument to use
the backend name as file name. However, when no argument is provided, an
error is reported during the configuration parsing requesting an argument, a
file name or "use-backend-name". And This last special argument is not
documented.

So, to respect the documentation and to avoid configuration breakages, all
modes are now supported. If this directive is called with no argument or
with "use-backend-name", the backend name is use as file name for the
server-state file. Otherwise, the provided string is used.

In addition, we take care to release any previously allocated file name in
case this directive is defines multiple times in the same backend. And an
error is reported if more than one argument are defined. Finally, the
documentation is updated accordingly. Sections supporting this directive are
also mentioned.

This patch should be backported as far as 1.6.

MINOR: server: enhance error precision when applying server state

server health checks and agent parameters are written the same way as
others to be able to enahcne code reuse: basically we make use of
parsing and assignment at the same place. It makes it difficult for
error handling to know whether srv object was modified partially or not.
The problem was already present with SRV resolution though.

I was a bit puzzled about the approach to take to be honest, and I did
not wanted to go into a full refactor, so I assumed it was ok to simply
notify whether the line was failed or partially applied.

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

MEDIUM: server: support {check,agent}_addr, agent_port in server state

logical followup from cli commands addition, so that the state server
file stays compatible with the changes made at runtime; use previously
added helper to load server attributes.

also alloc a specific chunk to avoid mixing with other called functions
using it

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

MEDIUM: server: add server-states version 2

Even if it is possibly too much work for the current usage, it makes
sure we don't break states file from v2.3 to v2.4; indeed, since v2.3,
we introduced two new fields, so we put them aside to guarantee we can
easily reload from a version 1.
The diff seems huge but there is no specific change apart from:
- introduce v2 where it is needed (parsing, update)
- move away from switch/case in update to be able to reuse code
- move srv lock to the whole function to make it easier

this patch confirm how painful it is to maintain this functionality.

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

MEDIUM: cli: add agent-port command

this patch allows to set agent port at runtime. In order to align with
both `addr` and `check-addr` commands, also add the possibility to
optionnaly set port on `agent-addr` command. This led to a small
refactor in order to use the same function for both `agent-addr` and
`agent-port` commands.

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

MEDIUM: cli: add check-addr command

this patch allows to set server health check address at runtime. In
order to align with `addr` command, also allow to set port optionnaly.
This led to a small refactor in order to use the same function for both
`check-addr` and `check-port` commands.
for `check-port`, we however don't permit the change anymore if checks
are not enabled on the server.

This command becomes more and more useful for people having a consul
like architecture:
- the backend server is located on a container with its own IP
- the health checks are done the consul instance located on the host
  with the host IP

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

CI: cirrus: update FreeBSD image to 12.2

we already tried to run FreeBSD-stable. it is pain,
so we use FreeBSD releases, we need to keep packages and release in sync.

let us update to released FreeBSD-12.2

REGTESTS: fix sni used in http_reuse_conn_hash for libressl 3.3.0

libressl 3.3.0 is stricter on the sni field and fails if it contains
illegal characters such as the underscore. Replace sni field with proper
name to pass the test on the CI environment.

REGTESTS: deactivate a failed test on CI in http_reuse_conn_hash

The last request with proxy protocol fails on the CI, reporting
first_http_request as 1. Currently this is not reproducible on my
local machine.

REGTESTS: adjust http_reuse_conn_hash requirements

This test is only valid for 2.4+ as the tested feature was introduced in
this version. Besides, it can only run with ssl support to test sni
reuse.

BUG/MINOR: backend: fix compilation without ssl

sni_smp/sni_hash are reported as unused on compilation without
USE_OPENSL and may cause compilation failure

This does not need to be backported.

MINOR: doc: update http reuse for new eligilible connections

Update the doc to remove entries on http-reuse marking private
connection for specific source address or sni.

MINOR: reg-tests: test http-reuse with proxy protocol

Complete the http-reuse test with the proxy protocol usage. A server
connection can be reused if the proxy protocol fields are the same.

MINOR: connection: use proxy protocol as parameter for srv conn hash

Use the proxy protocol frame if proxy protocol is activated on the
server line. Do not add anymore these connections in the private list.
If some requests are made with the same proxy fields, they can reuse
the idle connection.

The reg-tests proxy_protocol_send_unique_id must be adapted has it
relied on the side effect behavior that every requests from a same
connection reused a private server connection. Now, a new connection is
created as expected if the proxy protocol fields differ.

MINOR: connection: use src addr as parameter for srv conn hash

The source address is used as an input to the the server connection hash. The
address and port are used as separate hash inputs. Do not add anymore these
connections in the private list.

This parameter is set only if used in the transparent-proxy mode.

MINOR: backend: rewrite alloc of connection src address

This commit is similar to
"MINOR: backend: rewrite alloc of stream target address" but with source
address.

MINOR: reg-test: test http-reuse with specific dst addr

Add a reg-test for connection reuse with a specific destination address.
It is based on the set-dst-port http rule.

MINOR: connection: use dst addr as parameter for srv conn hash

The destination address is used as an input to the server connection hash. The
address and port are used as separated hash inputs. Note that they are not used
when statically specified on the server line. This is only useful for dynamic
destination address.

This is typically used when the server address is dynamically set via the
set-dst action. The address and port are separated hash parameters.

Most notably, it should fixed set-dst use case (cf github issue #947).

MINOR: backend: rewrite alloc of stream target address

Change the API of the function used to allocate the stream target
address. This is done in order to be able to allocate the destination
address and use it to reuse a connection sharing with the same address.
In particular, the flag stream SF_ADDR_SET is now set outside of the
function.

MINOR: reg-tests: test http-reuse with sni

Add a reg-test for connection reuse with the sni parameter. Ensure that
only connections that use the same sni are reused.

MINOR: connection: use sni as parameter for srv conn hash

The sni parameter is an input to the server connection hash. Do not add
anymore connections with dynamic sni in the private list. Thus, it is
now possible to reuse a server connection if they use the same sni.

MINOR: backend: compare conn hash for session conn reuse

Compare the connection hash when reusing a connection from the session.
This ensures that a private connection is reused only if it shares the
same set of parameters.

MINOR: connection: use the srv pointer for the srv conn hash

The pointer of the target server is used as a first parameter for the
server connection hash calcul. This prevents the hash to be null when no
specific parameters are present, and can serve as a simple defense
against an attacker trying to reuse a non-conform connection.

MINOR: connection: prepare hash calcul for server conns

This is a preliminary work for the calcul of the backend connection
hash. A structure conn_hash_params is the input for the operation,
containing the various specific parameters of a connection.

The high bits of the hash will reflect the parameters present as input.
A set of macros is written to manipulate the connection hash and extract
the parameters/payload.

MINOR: backend: search conn in idle tree after safe on always reuse

With http-reuse always, if no matching safe connection is found, check
in idle tree for a matching one. This is needed because now idle
connections can be differentiated from each other.

If only the safe tree was checked because not empty, but did not contain
a matching connection, we could miss matching entry in idle tree.

MINOR: backend: search conn in idle/safe trees after available

If no matching connection is found on available, check on idle/safe
trees for a matching one. This is needed because now idle connections
can be differentiated from each other.

If only the available list was checked because not empty, but did not
contain a matching connection, we could miss matching entries in idle or
safe trees.

MEDIUM: connection: replace idle conn lists by eb trees

The server idle/safe/available connection lists are replaced with ebmb-
trees. This is used to store backend connections, with the new field
connection hash as the key. The hash is a 8-bytes size field, used to
reflect specific connection parameters.

This is a preliminary work to be able to reuse connection with SNI,
explicit src/dst address or PROXY protocol.

MEDIUM: connection: protect idle conn lists with locks

This is a preparation work for connection reuse with sni/proxy
protocol/specific src-dst addresses.

Protect every access to idle conn lists with a lock. This is currently
strictly not needed because the access to the list are made with atomic
operations. However, to be able to reuse connection with specific
parameters, the list storage will be converted to eb-trees. As this
structure does not have atomic operation, it is mandatory to protect it
with a lock.

For this, the takeover lock is reused. Its role was to protect during
connection takeover. As it is now extended to general idle conns usage,
it is renamed to idle_conns_lock. A new lock section is also
instantiated named IDLE_CONNS_LOCK to isolate its impact on performance.

BUG/MINOR: backend: hold correctly lock when killing idle conn

The wrong lock seems to be held when trying to remove another thread
connection if max fd limit has been reached (locking the current thread
instead of the target thread lock).

This could be backported up to 2.0.

CLEANUP: queue: Remove useless tests on p or pp in pendconn_process_next_strm()

This patch removes unecessary tests on p or pp pointers in
pendconn_process_next_strm() function. This should make cppcheck happy and
avoid false report of null pointer dereference.

This patch should fix the issue #1036.

CLEANUP: remove unused variable assigned found by Coverity

this is pure cleanup, no need to backport

2116        if ((end - 1) == (payload + strlen(PAYLOAD_PATTERN))) {
2117                /* if the payload pattern is at the end */
2118                s->pcli_flags |= PCLI_F_PAYLOAD;
    CID 1399833 (#1 of 1): Unused value (UNUSED_VALUE)assigned_value: Assigning value from reql to ret here, but that stored value is overwritten before it can be used.
2119                ret = reql;
2120        }

This patch fixes the issue #1048.

BUG/MINOR: tools: Fix a memory leak on error path in parse_dotted_uints()

When an invalid character is found during parsing in parse_dotted_uints()
function, the allocated array of uint must be released. This patch fixes a
memory leak on error path during the configuration parsing.

This patch should fix the issue #1106. It should be backported as far as
2.0. Note that, for 2.1 and 2.0, the function is in src/standard.c

CLEANUP: muxes: Remove useless calls to b_realign_if_empty()

In H1, H2 and FCGI muxes, b_realign_if_empty() is called to reset the head
of an empty buffer before setting it a specific value to permit the
zero-copy. Thus, we can remove call to b_realign_if_empty().

BUG/MINOR: server: re-align state file fields number

Since commit 3169471964fdc49963e63f68c1fd88686821a0c4 ("MINOR: Add
server port field to server state file.") max_fields was not increased
on version number 1. So this patch aims to fix it. This should be
backported as far as v1.8, but the numbering should be adpated depending
on the version: simply increase the field by 1.

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

REGTESTS: Add a script to test payload skipping for bodyless HTTP responses

This script is used to be sure unexpected payload in bodyless HTTP responses
is properly ignored on outgoing side, both by H1 and H2 muxes.

MINOR: mux-h1: Be sure EOM flag is set when processing end of outgoing message

When a message is sent, an extra check is performed when the parser is
switch to MSG_DONE state to be sure the EOM flag is really set. This flag is
quite new and replaces the EOM block. Thus, this test is a safeguard waiting
for a proper refactoring of the outgoing side.

BUG/MEDIUM: mux-h2: Add EOT block when EOM flag is set on an empty HTX message

In the H2 mux, when a empty DATA frame is used to finish a message, just to
set the ES flag, we now only set the EOM flag on the HTX message. However,
if the HTX message is empty, this event will not be properly handled on the
other side because there is no effective data to handle. Thus, it is
interpreted as an abort by the H1 mux.

It is in part caused by the current H1 mux design but also because there is
no way to emit empty HTX block (NOOP HTX block) or to wakeup a mux for send
when there is no data to finish some internal processing.

Thus, for now, to work around this limitation, an EOT HTX block is added by
the H2 mux if a EOM flag is added on an empty HTX message. This case is only
possible when an empty DATA frame with the ES flag is received.

This fix is specific for 2.4. No backport needed.

BUG/MINOR: mux-h1: Don't blindly skip EOT block for non-chunked messages

In HTTP/2, we may have trailers for messages with a Content-length
header. Thus, when the H2 mux receives a HEADERS frame at the end of a
message, it always emits TLR and EOT HTX blocks. On the H1 mux, if this
happens, these blocks are just skipped because we cannot emit trailers for a
non-chunked message. But the EOT HTX block must not be blindly
ignored. Indeed, there is no longer EOM HTX block to mark the end of the
message. Thus the EOT block, when found, is the end of the message. So we
must handle it to swith in MSG_DONE state.

This fix is specific for 2.4. No backport needed.

BUG/MINOR: mux-h1: Fix data skipping for bodyless responses

When payload is received for a bodyless response, for instance a response to
a HEAD request, it is silently skipped. Unfortunately, when this happens,
the end of the message is not properly handled. The response remains in the
MSG_DATA state (or MSG_TRAILERS if the message is chunked). In addition,
when a zero-copy is possible, the data are not removed from the channel
buffer and the H1 connection is killed because an error is then triggered.

To fix the bug, the zero-copy is disabled for bodyless responses. It is not
a problem because there is no copy at all. And the last block (DATA or EOT)
is now properly handled.

This bug was introduced by the commit e5596bf53 ("MEDIUM: mux-h1: Don't emit
any payload for bodyless responses").

This fix is specific for 2.4. No backport needed.

BUG/MEDIUM: mux-h1: Always set CS_FL_EOI for response in MSG_DONE state

During the message parsing, if in MSG_DONE state, the CS_FL_EOI flag must
always be set on the conn-stream if following conditions are met :

  * It is a response or
  * It is a request but not a protocol upgrade nor a CONNECT.

For now, there is no test on the message type (request or response). Thus
the CS_FL_EOI flag is not set for a response with a "Connection: upgrade"
header but not a 101 response.

This bug was introduced by the commit 3e1748bbf ("BUG/MINOR: mux-h1: Don't
set CS_FL_EOI too early for protocol upgrade requests"). It was backported
as far as 2.0. Thus, this patch must also be backported as far as 2.0.

BUG/MINOR: http-ana: Don't increment HTTP error counter on internal errors

If internal error is reported by the mux during HTTP request parsing, the
HTTP error counter should not be incremented. It should only be incremented
on parsing error to reflect errors caused by clients.

This patch must be backported as far as 2.0. During the backport, the same
must be performed for 408-request-time-out errors.

BUG/MINOR: mux-h1: Don't increment HTTP error counter for 408/500/501 errors

The HTTP error counter reflects the number of errors caused by
clients. Thus, In the H1 mux, it should only be increment on parsing errors.

This fix is specific for 2.4. No backport needed.

MINOR: ssl: add SSL_SERVER_LOCK label in threads.h

Amaury reported that the commit 3ce6eed ("MEDIUM: ssl: add a rwlock for
SSL server session cache") introduced some warning during compilation:

    include/haproxy/thread.h|411 col 2| warning: enumeration value 'SSL_SERVER_LOCK' not handled in switch [-Wswitch]

This patch fix the issue by adding the right entry in the switch block.

Must be backported where 3ce6eed is backported. (2.4 only for now)

MINOR: stick-tables/counters: add http_fail_cnt and http_fail_rate data types

Historically we've been counting lots of client-triggered events in stick
tables to help detect misbehaving ones, but we've been missing the same on
the server side, and there's been repeated requests for being able to count
the server errors per URL in order to precisely monitor the quality of
service or even to avoid routing requests to certain dead services, which
is also called "circuit breaking" nowadays.

This commit introduces http_fail_cnt and http_fail_rate, which work like
http_err_cnt and http_err_rate in that they respectively count events and
their frequency, but they only consider server-side issues such as network
errors, unparsable and truncated responses, and 5xx status codes other
than 501 and 505 (since these ones are usually triggered by the client).
Note that retryable errors are purposely not accounted for, so that only
what the client really sees is considered.

With this it becomes very simple to put some protective measures in place
to perform a redirect or return an excuse page when the error rate goes
beyond a certain threshold for a given URL, and give more chances to the
server to recover from this condition. Typically it could look like this
to bypass a URL causing more than 10 requests per second:

  stick-table type string len 80 size 4k expire 1m store http_fail_rate(1m)
  http-request track-sc0 base       # track host+path, ignore query string
  http-request return status 503 content-type text/html \
      lf-file excuse.html if { sc0_http_fail_rate gt 10 }

A more advanced mechanism using gpt0 could even implement high/low rates
to disable/enable the service.

Reg-test converteers_ref_cnt_never_dec.vtc was updated to test it.

BUG/MINOR: freq_ctr: fix a wrong delay calculation in next_event_delay()

The sleep time calculation in next_event_delay() was wrong because it
was dividing 999 by the number of pending events, and was directly
responsible for an observation made a long time ago that listeners
would eat all the CPU when hammered while globally rate-limited,
because the more the queued events, the least it would wait, and would
ignore the configured frequency to compute the delay.

This was addressed in various ways in listeners through the switch to
the FULL state and the wakeup of manage_global_listener_queue() that
avoids this fast loop, but the calculation made there remained wrong
nevertheless. It's even visible with this patch that the accept
frequency is much more accurate at low values now; for example,
configuring a maxconrate of 10 would give between 8.99 and 11.0 cps
before this patch and between 9.99 and 10.0 with it.

Better fix it now in case it's reused anywhere else and causes confusion
again. It maybe be backported but is probably not worth it.

BUG/MINOR: intops: fix mul32hi()'s off-by-one

mul32hi() multiples a constant a with a variable b from 0 to 0xffffffff
and shifts the result by 32 bits. It's visible that it's always impossible
to reach the constant a this way because the product always misses exactly
one unit of a to be preserved. And this cannot be corrected by the caller
either as adding one to the output will only shift the output range, and
it's not possible to pass 2^32 on the ratio <b>. The right approach is to
add "a" after the multiplication so that the input range is always
preserved for all ratio values from 0 to 0xffffffff:

     (a=0x00000000 * b=0x00000000 + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000000 * b=0x00000001 + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000000 * b=0xffffffff + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000001 * b=0x00000000 + a=0x00000001) >> 32 = 0x00000000
     (a=0x00000001 * b=0x00000001 + a=0x00000001) >> 32 = 0x00000000
     (a=0x00000001 * b=0xffffffff + a=0x00000001) >> 32 = 0x00000001
     (a=0xffffffff * b=0x00000000 + a=0xffffffff) >> 32 = 0x00000000
     (a=0xffffffff * b=0x00000001 + a=0xffffffff) >> 32 = 0x00000001
     (a=0xffffffff * b=0xffffffff + a=0xffffffff) >> 32 = 0xffffffff

This is only used in freq_ctr calculations and the slightly lower value
is unlikely to have ever been noticed by anyone. This may be backported
though it is not important.

MEDIUM: ssl: add a rwlock for SSL server session cache

When adding the server side support for certificate update over the CLI
we encountered a design problem with the SSL session cache which was not
locked.

Indeed, once a certificate is updated we need to flush the cache, but we
also need to ensure that the cache is not used during the update.
To prevent the use of the cache during an update, this patch introduce a
rwlock for the SSL server session cache.

In the SSL session part this patch only lock in read, even if it writes.
The reason behind this, is that in the session part, there is one cache
storage per thread so it is not a problem to write in the cache from
several threads. The problem is only when trying to write in the cache
from the CLI (which could be on any thread) when a session is trying to
access the cache. So there is a write lock in the CLI part to prevent
simultaneous access by a session and the CLI.

This patch also remove the thread_isolate attempt which is eating too
much CPU time and was not protecting from the use of a free ptr in the
session.

BUILD: ssl: guard SSL_CTX_set_msg_callback with SSL_CTRL_SET_MSG_CALLBACK macro

both SSL_CTX_set_msg_callback and SSL_CTRL_SET_MSG_CALLBACK defined since
ea262260469e49149cb10b25a87dfd6ad3fbb4ba, we can safely switch to that guard
instead of OpenSSL version

CLEANUP: tools: typo in `strl2irc` mention

`str2irc` does not exist

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

CLEANUP: check: fix some typo in comments

a few obvious english typo in comments, some of which introduced by
myself quite recently

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

CLEANUP: assorted typo fixes in the code and comments

This is 17th iteration of typo fixes

MEDIUM: contrib/prometheus-exporter: export base stick table stats

I saw some people falling back to unix socket to collect some data they
could not find in prometheus exporter. One of them is base info from
stick tables (used/size).
I do not plan to extend it more for now; keys are quite a mess to
handle.

This should resolve github issue #1008.

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

MINOR: contrib/prometheus-exporter: use stats desc when possible followup

Remove remaining descrition which are common to stats.c.

This patch is a followup of commit
82b2ce2f967d967139adb7afab064416fadad615 ("MINOR:
contrib/prometheus-exporter: use stats desc when possible"). I probably
messed up with one of my rebase because I'm pretty sure I removed them
at some point, but who knows what happened.

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