Commit Graph

9267 Commits

Author SHA1 Message Date
Willy Tarreau
f5809cde7a MINOR: threads: make MAX_THREADS configurable at build time
There's some value in being able to limit MAX_THREADS, either to save
precious resources in embedded environments, or to protect certain
deployments against accidently incorrect settings.

With this patch, if MAX_THREADS is defined at build time, it will be
used. However, given that LONGBITS is not a macro but is defined
according to sizeof(long), we can't check the value range at build
time and instead we need to perform the check at early boot time.
However, the compiler is able to optimize away the constant comparisons
and doesn't even emit the check code when values are correct.

The output message regarding threading support was improved to report
the number of threads.
2019-01-26 13:37:48 +01:00
Willy Tarreau
c9a82e48bf MINOR: cfgparse: make the process/thread parser support a maximum value
It was hard-wired to LONGBITS, let's make it configurable depending on the
context (threads, processes).
2019-01-26 13:25:14 +01:00
Tim Duesterhus
4707033932 CLEANUP: h2: Remove debug printf in mux_h2.c
It was introduced by 1915ca273832ba542d72eb0645dd7ccb6d5b945f
and should be backported to 1.9.
2019-01-25 05:22:07 +01:00
Willy Tarreau
1915ca2738 BUG/MINOR: mux-h2: always compare content-length to the sum of DATA frames
This is mandated by RFC7541#8.1.2.6. Till now we didn't have a copy of
the content-length header field. But now that it's already parsed, it's
easy to add the check.

The reg-test was updated to match the new behaviour as the previous one
expected unadvertised data to be silently discarded.

This should be backported to 1.9 along with previous patch (MEDIUM: h2:
always parse and deduplicate the content-length header) after it has got
a bit more exposure.
2019-01-24 19:45:27 +01:00
Willy Tarreau
4790f7c907 MEDIUM: h2: always parse and deduplicate the content-length header
The header used to be parsed only in HTX but not in legacy. And even in
HTX mode, the value was dropped. Let's always parse it and report the
parsed value back so that we'll be able to store it in the streams.
2019-01-24 19:07:26 +01:00
Willy Tarreau
f7a259d46f MINOR: stream: don't wait before retrying after a failed connection reuse
When a connection reuse fails, we must not wait before retrying, as most
likely the issue is related to the reused connection and not to the server
itself.

This should be backported to 1.9, though it depends on previous patches
dealing with SI_ST_CON for connection reuse.
2019-01-24 19:06:43 +01:00
Willy Tarreau
bf66bd1b8b MEDIUM: stream-int: always mark pending outgoing SI_ST_CON
Before the first send() attempt, we should be in SI_ST_CON, not
SI_ST_EST, since we have not yet attempted to send and we are
allowed to retry. This is particularly important with complex
outgoing muxes which can fail during the first send attempt (e.g.
failed stream ID allocation).

It only requires that sess_update_st_con_tcp() knows about this
possibility, as we must not forcefully close a reused connection
when facing an error in this case, this will be handled later.

This may be backported to 1.9 with care after some observation period.
2019-01-24 19:06:43 +01:00
Willy Tarreau
e9634bdc22 MINOR: mux-h2: always consider a server's max-reuse parameter
This parameter allows to limit the number of successive requests sent
on a connection. Let's compare it to the number of streams already sent
on the connection to decide if the connection may still appear in the
idle list or not. This may be used to help certain servers work around
resource leaks, and also helps dealing with the issue of the GOAWAY in
flight which requires to set a usage limit on the client to be reliable.

This must be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
9c538e01c2 MINOR: server: add a max-reuse parameter
Some servers may wish to limit the total number of requests they execute
over a connection because some of their components might leak resources.
In HTTP/1 it was easy, they just had to emit a "connection: close" header
field with the last response. In HTTP/2, it's less easy because the info
is not always shared with the component dealing with the H2 protocol and
it could be harder to advertise a GOAWAY with a stream limit.

This patch provides a solution to this by adding a new "max-reuse" parameter
to the server keyword. This parameter indicates how many times an idle
connection may be reused for new requests. The information is made available
and the underlying muxes will be able to use it at will.

This patch should be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
2c7deddc06 BUG/MEDIUM: backend: never try to attach to a mux having no more stream available
The code dealing with idle connections used to check the number of streams
available on the connection only to unlink the connection from the idle
list. But this still resulted in too many streams reusing the same connection
when they were already attached to it.

We must detect that there is no more room and refrain from using this
connection at all, and instead fall back to the no-reuse case. Ideally
we should try to search among other idle connections, but for a backport
let's stay safe.

This must be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
a80dca8535 BUG/MINOR: mux-h2: refuse to allocate a stream with too high an ID
One of the reasons for the excessive number of aborted requests when a
server sets a limit on the highest stream ID is that we don't check
this limit while allocating a new stream.

This patch does this at two locations :
  - when a backend stream is allocated, we verify that there are still
    IDs left ;
  - when the ID is assigned, we verify that it's not higher than the
    advertised limit.

This should be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
d64a3ebe64 BUG/MINOR: mux-h2: always check the stream ID limit in h2_avail_streams()
This function is used to decide whether to put an idle connection back
into the idle pool. While it considers the limit in number of concurrent
requests, it does not consider the limit in number of streams, so if a
server announces a low limit in a GOAWAY frame, it will be ignored.

However there is a caveat : since we assign the stream IDs when sending
them, we have a number of allocated streams which max_id doesn't take
care of. This can be addressed by adding a new nb_reserved count on each
connection to keep track of the ID-less streams.

This patch makes sure we take care of the remaining number of streams
if such a limit was announced, or of the number of streams before the
highest ID. Now it is possible to accurately know how many streams
can be allocated, and the number of failed outgoing streams has dropped
in half.

This must be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
15c120d251 CLEANUP: server: fix indentation mess on idle connections
Apparently some code was moved around leaving the inner block incorrectly
indented and with the closing brace in the middle of nowhere.
2019-01-24 19:06:43 +01:00
Willy Tarreau
64f6945fec BUG/MINOR: stream: take care of synchronous errors when trying to send
We currently detect a number of situations where we have to immediately
deal with a state change, but we failed to consider the case of the
synchronous error reported on the stream-interface. We definitely do not
want to have to wait for a timeout to handle this one, especially at the
beginning of the connection when it can lead to an immediate retry.

This should be backported to 1.9.
2019-01-24 19:06:43 +01:00
Willy Tarreau
cb923d5001 MINOR: server: make sure pool-max-conn is >= -1
The keyword parser doesn't check the value range, but supported values are
-1 and positive values, thus we should check it.

This can be backported to 1.9.
2019-01-24 16:31:56 +01:00
Willy Tarreau
1e7d444eec BUG/MINOR: hpack: return a compression error on invalid table size updates
RFC7541#6.3 mandates that an error is reported when a dynamic table size
update announces a size larger than the one configured with settings. This
is tested by h2spec using test "hpack/6.3/1".

This must be backported to 1.9 and possibly 1.8 as well.
2019-01-24 15:27:06 +01:00
Willy Tarreau
175cebb38a BUG/MINOR: mux-h2: make it possible to set the error code on an already closed stream
When sending RST_STREAM in response to a frame delivered on an already
closed stream, we used not to be able to update the error code and
deliver an RST_STREAM with a wrong code (e.g. H2_ERR_CANCEL). Let's
always allow to update the code so that RST_STREAM is always sent
with the appropriate error code (most often H2_ERR_STREAM_CLOSED).

This should be backported to 1.9 and possibly to 1.8.
2019-01-24 15:27:06 +01:00
Willy Tarreau
5b4eae33de BUG/MINOR: mux-h2: headers-type frames in HREM are always a connection error
There are incompatible MUST statements in the HTTP/2 specification. Some
require a stream error and others a connection error for the same situation.
As discussed in the thread below, let's always apply the connection error
when relevant (headers-like frame in half-closed(remote)) :

  https://mailarchive.ietf.org/arch/msg/httpbisa/pOIWRBRBdQrw5TDHODZXp8iblcE

This must be backported to 1.9, possibly to 1.8 as well.
2019-01-24 15:27:06 +01:00
Willy Tarreau
113c7a2794 BUG/MINOR: mux-h2: CONTINUATION in closed state must always return GOAWAY
Since we now support CONTINUATION frames, we must take care of properly
aborting the connection when they are sent on a closed stream. By default
we'd get a stream error which is not sufficient since the compression
context is modified and unrecoverable.

More info in this discussion :

   https://mailarchive.ietf.org/arch/msg/httpbisa/azZ1jiOkvM3xrpH4jX-Q72KoH00

This needs to be backported to 1.9 and possibly to 1.8 (less important there).
2019-01-24 15:27:06 +01:00
Willy Tarreau
71c3811589 MINOR: h2: declare new sets of frame types
This patch adds H2_FT_HDR_MASK to group all frame types carrying headers
information, and H2_FT_LATE_MASK to group frame types allowed to arrive
after a stream was closed.
2019-01-24 15:27:06 +01:00
Willy Tarreau
31e846a071 BUG/MEDIUM: mux-h2: properly abort on trailers decoding errors
There was an incomplete test in h2c_frt_handle_headers() resulting
in negative return values from h2c_decode_headers() not being taken
as errors. The effect is that the stream is then aborted on timeout
only.

This fix must be backported to 1.9.
2019-01-24 15:27:06 +01:00
Willy Tarreau
5ce6337254 BUG/MEDIUM: backend: also remove from idle list muxes that have no more room
The current test consists in removing muxes which report that they're going
to assign their last available stream, but a mux may already be saturated
without having passed in this situation at all. This is what happens with
mux_h2 when receiving a GOAWAY frame informing the mux about the ID of the
last stream the other end is willing to process. The limit suddenly changes
from near infinite to 0. Currently what happens is that such a mux remains
in the idle list for a long time and refuses all new streams. Now at least
it will only fail a single stream in a retryable way. A future improvement
should consist in trying to pick another connection from the idle list.

This fix must be backported to 1.9.
2019-01-24 13:53:06 +01:00
Willy Tarreau
759ca1eacc BUG/MAJOR: mux-h2: don't destroy the stream on failed allocation in h2_snd_buf()
In case we cannot allocate a stream ID for an outgoing stream, the stream
will be aborted. The problem is that we also release it and it will be
destroyed again by the application detecting the error, leading to a NULL
dereference in h2_shutr() and h2_shutw(). Let's only mark the error on the
CS and let the rest of the code handle the close.

This should be backported to 1.9.
2019-01-24 13:52:10 +01:00
Willy Tarreau
b57af617c0 BUG/MINOR: mux-h1: avoid copying output over itself in zero-copy
It's almost funny but one side effect of the latest zero-copy changes made
to mux-h1 resulted in the temporary buffer being copied over itself at the
exact same location. This has no impact except slowing down operations and
irritating valgrind. The cause is an incorrect pointer check after the
alignment optimizations were made.

This needs to be backported to 1.9.

Reported-by: Tim Duesterhus <tim@bastelstu.be>
2019-01-23 20:43:53 +01:00
Christopher Faulet
afe57846bf BUG/MINOR: mux-h1: Apply the reserve on the channel's buffer only
There is no reason to use the reserve to limit the amount of data of the input
buffer that we can parse at a time. The only important thing is to keep the
reserve free in the channel's buffer.

This patch should be backported to 1.9.
2019-01-23 11:27:34 +01:00
Christopher Faulet
a413e958fd BUG/MEDIUM: mux-h2/htx: Respect the channel's reserve
When data are pushed in the channel's buffer, in h2_rcv_buf(), the mux-h2 must
respect the reserve if the flag CO_RFL_KEEP_RSV is set. In HTX, because the
stream-interface always sees the buffer as full, there is no other way to know
the reserve must be respected.

This patch must be backported to 1.9.
2019-01-23 11:27:34 +01:00
Christopher Faulet
dcd8c5eed4 BUG/MINOR: proto-htx: Return an error if all headers cannot be received at once
When an HTX stream is waiting for a request or a response, it reports an error
(400 for the request or 502 for the response) if a parsing error is reported by
the mux (HTX_FL_PARSING_ERROR). The mux-h1 uses this error, among other things,
when the headers are too big to be analyzed at once. But the mux-h2 doesn't. So
the stream must also report an error if the multiplexer is unable to emit all
headers at once. The multiplexers must always emit all the headers at once
otherwise it is an error.

There are 2 ways to detect this error:

  * The end-of-headers marker was not received yet _AND_ the HTX message is not
    empty.

  * The end-of-headers marker was not received yet _AND_ the multiplexer have
    some data to emit but it is waiting for more space in the channel's buffer.

Note the mux-h2 is buggy for now when HTX is enabled. It does not respect the
reserve. So there is no way to hit this bug.

This patch must be backported to 1.9.
2019-01-23 11:27:34 +01:00
Willy Tarreau
64ded3db2c DOC: mention the effect of nf_conntrack_tcp_loose on src/dst
On rare occasions the logs may report inverted src/dst when using
conntrack with this sysctl. Add a mention for it in the doc. More
info here :

     https://www.spinics.net/lists/netdev/msg544878.html
2019-01-23 10:02:15 +01:00
Dirkjan Bussink
526894ff39 BUG/MEDIUM: ssl: Fix handling of TLS 1.3 KeyUpdate messages
In OpenSSL 1.1.1 TLS 1.3 KeyUpdate messages will trigger the callback
that is used to verify renegotiation is disabled. This means that these
KeyUpdate messages fail. In OpenSSL 1.1.1 a better mechanism is
available with the SSL_OP_NO_RENEGOTIATION flag that disables any TLS
1.2 and earlier negotiation.

So if this SSL_OP_NO_RENEGOTIATION flag is available, instead of having
a manual check, trust OpenSSL and disable the check. This means that TLS
1.3 KeyUpdate messages will work properly.

Reported-By: Adam Langley <agl@imperialviolet.org>
2019-01-23 09:51:22 +01:00
Christopher Faulet
774c486cec BUG/MINOR: check: Wake the check task if the check is finished in wake_srv_chk()
With tcp-check, the result of the check is set by the function tcpcheck_main()
from the I/O layer. So it is important to wake up the check task to handle the
result and finish the check. Otherwise, we will wait the task timeout to handle
the result of a tcp-check, delaying the next check by as much.

This patch also fixes a problem about email alerts reported by PiBa-NL (Pieter)
on the ML [1] on all versions since the 1.6. So this patch must be backported
from 1.9 to 1.6.

[1] https://www.mail-archive.com/haproxy@formilux.org/msg32190.html
2019-01-22 07:01:15 +01:00
Jérôme Magnin
f57afa453a BUG/MINOR: server: don't always trust srv_check_health when loading a server state
When we load health values from a server state file, make sure what we assign
to srv->check.health actually matches the state we restore.

This should be backported as far as 1.6.
2019-01-21 11:09:03 +01:00
Willy Tarreau
1ba32032ef BUG/MEDIUM: checks: fix recent regression on agent-check making it crash
In order to address the mailers issues, we needed to store the proxy
into the checks struct, which was done by commit c98aa1f18 ("MINOR:
checks: Store the proxy in checks."). However this one did it only for
the health checks and not for the agent checks, resulting in an immediate
crash when the agent is enabled on a random config like this one :

  listen agent
      bind :8000
      server s1 255.255.255.255:1 agent-check agent-port 1

Thanks to Seri Kim for reporting it and providing a reproducer in
issue #20. This fix must be backported to 1.9.
2019-01-21 07:48:26 +01:00
Uman Shahzad
da7eeedf38 BUG/MINOR: startup: certain goto paths in init_pollers fail to free
If we fail to initialize pollers due to fdtab/fdinfo/polled_mask
not getting allocated, we free any of those that were allocated
and exit. However the ordering was incorrect, and there was an old
unused and unreachable "fail_cache" path as well, which needs to
be taken when no poller works.

This was introduced with this commit during 1.9-dev :
   cb92f5c ("MINOR: pollers: move polled_mask outside of struct fdtab.")

It needs to be backported to 1.9 only.
2019-01-21 04:48:48 +01:00
Frédéric Lécaille
2f167b3543 DOC: peers: SSL/TLS documentation for "peers" 2019-01-18 14:26:21 +01:00
Frédéric Lécaille
355b2033ec MINOR: cfgparse: SSL/TLS binding in "peers" sections.
Make "bind" keywork be supported in "peers" sections.
All "bind" settings are supported on this line.
Add "default-bind" option to parse the binding options excepted the bind address.
Do not parse anymore the bind address for local peers on "server" lines.
Do not use anymore list_for_each_entry() to set the "peers" section
listener parameters because there is only one listener by "peers" section.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
1055e687a2 MINOR: peers: Make outgoing connection to SSL/TLS peers work.
This patch adds pointer to a struct server to peer structure which
is initialized after having parsed a remote "peer" line.

After having parsed all peers section we run ->prepare_srv to initialize
all SSL/TLS stuff of remote perr (or server).

Remaining thing to do to completely support peer protocol over SSL/TLS:
make "bind" keyword be supported in "peers" sections to make SSL/TLS
incoming connections to local peers work.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
c06b5d4f74 MINOR: cfgparse: Make "peer" lines be parsed as "server" lines.
With this patch "default-server" lines are supported in "peers" sections
to setup the default settings of peers which are from now setup
when parsing both "peer" and "server" lines.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
9492c4ecdb MINOR: cfgparse: Simplication.
Make init_peers_frontend() be callable without having to check if
there is something to do or not.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
91694d51f7 MINOR: cfgparse: Rework peers frontend init.
Even if not already the case, we suppose that the frontend "peers" section
may have been already initialized outside of "peer" line, we seperate
their initializations from their binding initializations.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
4ba5198899 MINOR: cfgparse: Useless frontend initialization in "peers" sections.
Use ->local "peers" struct member to flag a "peers" section frontend
has being initialized. This is to be able to initialize the frontend
of "peers" sections on lines different from "peer" lines.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
16e491004b CLEANUP: cfgparse: Code reindentation.
May help the series of patches to be reviewed.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
6617e769bf CLEANUP: cfgparse: Return asap from cfg_parse_peers().
Avoid useless code indentation.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Frédéric Lécaille
1825103fbe MINOR: cfgparse: Extract some code to be re-used.
Create init_peers_frontend() function to allocate and initialize
the frontend of "peers" sections (->peers_fe) so that to reuse it later.

May be backported to 1.5 and newer.
2019-01-18 14:26:21 +01:00
Lukas Tribus
6583dca675 DOC: add github issue templates
See issue #2

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
2019-01-17 22:53:55 +01:00
Olivier Houchard
f24502ba46 BUG/MEDIUM: connections: Add the CO_FL_CONNECTED flag if a send succeeded.
If a send succeeded, add the CO_FL_CONNECTED flag, the send may have been
called by the upper layers before we even realized we were connected, and we
may even read the response before we get the information, and si_cs_recv()
has to know if we were connected or not.

This should be backported to 1.9.
2019-01-17 19:18:20 +01:00
Olivier Houchard
09a0f03994 BUG/MEDIUM: servers: Make assign_tproxy_address work when ALPN is set.
If an ALPN is set on the server line, then when we reach assign_tproxy_address,
the stream_interface's endpoint will be a connection, not a conn_stream,
so make sure assign_tproxy_address() handles both cases.

This should be backported to 1.9.
2019-01-17 19:18:20 +01:00
PiBa-NL
b07e7b4dc1 REGTEST: checks basic stats webpage functionality
This regtest verifies that the stats webpage can be used to change a
server state to maintenance or drain, and that filtering the page scope
will result in a filtered page.
2019-01-17 11:32:12 +01:00
Jérôme Magnin
35e53a6a08 DOC: add a missing space in the documentation for bc_http_major
add a missing space between sample fetch name and colon, and make
haproxy-dconv happier.
2019-01-17 10:28:58 +01:00
Christopher Faulet
ed7a066b45 BUG/MEDIUM: stats: Get the right scope pointer depending on HTX is used or not
For HTX streams, the scope pointer is relative to the URI in the start-line. But
for streams using the legacy HTTP representation, the scope pointer is relative
to the beginning of output data in the channel's buffer. So we must be careful
to use the right one depending on the HTX is used or not.

Because the start-line is used to get de scope pointer, it is important to keep
it after the parsing of post paramters. So now, instead of removing blocks when
read in the function stats_process_http_post(), we just move on next, leaving it
in the HTX message.

Thanks to Pieter (PiBa-NL) to report this bug.

This patch must be backported to 1.9.
2019-01-16 17:27:49 +01:00
Ben51Degrees
daa356bd7d BUG: 51d: Changes to the buffer API in 1.9 were not applied to the 51Degrees code.
The code using the deprecated 'buf->p' has been updated to use
'ci_head(buf)' as described in section 5 of
'doc/internals/buffer-api.txt'.
A compile time switch on 'BUF_NULL' has also been added to enable the
same source code to be used with pre and post API change HAProxy.

This should be backported to 1.9, and is compatible with previous
versions.
2019-01-16 17:26:14 +01:00