]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoBUG/MEDIUM: mux-h2: do not abort HEADERS frame before decoding them
Willy Tarreau [Wed, 30 Jan 2019 15:58:30 +0000 (16:58 +0100)] 
BUG/MEDIUM: mux-h2: do not abort HEADERS frame before decoding them

If a response HEADERS frame arrives on a closed connection (due to a
client abort sending an RST_STREAM), it's currently immediately rejected
with an RST_STREAM, like any other frame. This is incorrect, as HEADERS
frames must first be decoded to keep the HPACK decoder synchronized,
possibly breaking subsequent responses.

This patch excludes HEADERS/CONTINUATION/PUSH_PROMISE frames from the
central closed state test and leaves to the respective frame parsers
the responsibility to decode the frame then send RST_STREAM.

This fix must be backported to 1.9. 1.8 is not directly impacted since
it doesn't have response HEADERS nor trailers thus cannot recover from
such situations anyway.

6 years agoBUG/MEDIUM: mux-h2: make sure never to send GOAWAY on too old streams
Willy Tarreau [Wed, 30 Jan 2019 18:20:09 +0000 (19:20 +0100)] 
BUG/MEDIUM: mux-h2: make sure never to send GOAWAY on too old streams

The H2 spec requires to send GOAWAY when the client sends a frame after
it has already closed using END_STREAM. Here the corresponding case was
the fallback of a series of tests on the stream state, but it unfortunately
also catches old closed streams which we don't know anymore. Thus any late
packet after we've sent an RST_STREAM will trigger this GOAWAY and break
other streams on the connection.

This can happen when launching two tabs in a browser targetting the same
slow page through an H2-to-H2 proxy, and pressing Escape to stop one of
them. The other one gets an error when the page finally responds (and it
generally retries), and the logs in the middle indicate SD-- flags since
the late response was cancelled.

This patch takes care to only send GOAWAY on streams we still know.

It must be backported to 1.9 and 1.8.

6 years agoBUG/MEDIUM: mux-h2: fix two half-closed to closed transitions
Willy Tarreau [Wed, 30 Jan 2019 18:28:32 +0000 (19:28 +0100)] 
BUG/MEDIUM: mux-h2: fix two half-closed to closed transitions

When receiving a HEADERS or DATA frame with END_STREAM set, we would
inconditionally switch to half-closed(remote). This is wrong because we
could already have been in half-closed(local) and need to switch to closed.
This happens in the following situations :
    - receipt of the end of a client upload after we've already responded
      (e.g. redirects to POST requests)
    - receipt of a response on the backend side after we've already finished
      sending the request (most common case).

This may possibly have caused some streams to stay longer than needed
at the end of a transfer, though this is not apparent in tests.

This must be backported to 1.9 and 1.8.

6 years agoBUG/MEDIUM: mux-h2: wake up flow-controlled streams on initial window update
Willy Tarreau [Wed, 30 Jan 2019 15:11:20 +0000 (16:11 +0100)] 
BUG/MEDIUM: mux-h2: wake up flow-controlled streams on initial window update

When a settings frame updates the initial window, all affected streams's
window is updated as well. However the streams are not put back into the
send list if they were already blocked on flow control. The effect is that
such a stream will only be woken up by a WINDOW_UPDATE message but not by
a SETTINGS changing the initial window size. This can be verified with
h2spec's test http2/6.9.2/1 which occasionally fails without this patch.

It is unclear whether this situation is really met in field, but the
fix is trivial, it consists in adding each unblocked streams to the
wait list as is done for the window updates.

This fix must be backported to 1.9. For 1.8 the patch needs quite
a few adaptations. It's better to copy-paste the code block from
h2c_handle_window_update() adding the stream to the send_list when
its mws is > 0.

6 years agoCLEANUP: mux-h2: remove misleading leftover test on h2s' nullity
Willy Tarreau [Wed, 30 Jan 2019 14:42:44 +0000 (15:42 +0100)] 
CLEANUP: mux-h2: remove misleading leftover test on h2s' nullity

The WINDOW_UPDATE and DATA frame handlers used to still have a check on
h2s to return either h2s_error() or h2c_error(). This is a leftover from
the early code. The h2s cannot be null there anymore as it has already
been dereferenced before reaching these locations.

6 years agoBUG/MINOR: deinit: tcp_rep.inspect_rules not deinit, add to deinit
Kevin Zhu [Wed, 30 Jan 2019 08:01:21 +0000 (16:01 +0800)] 
BUG/MINOR: deinit: tcp_rep.inspect_rules not deinit, add to deinit

It seems like this can be backported as far as 1.5.

6 years agoBUG/MEDIUM: compression: Rewrite strong ETags
Tim Duesterhus [Tue, 29 Jan 2019 15:38:56 +0000 (16:38 +0100)] 
BUG/MEDIUM: compression: Rewrite strong ETags

RFC 7232 section 2.3.3 states:

> Note: Content codings are a property of the representation data,
> so a strong entity-tag for a content-encoded representation has to
> be distinct from the entity tag of an unencoded representation to
> prevent potential conflicts during cache updates and range
> requests.  In contrast, transfer codings (Section 4 of [RFC7230])
> apply only during message transfer and do not result in distinct
> entity-tags.

Thus a strong ETag must be changed when compressing. Usually this is done
by converting it into a weak ETag, which represents a semantically, but not
byte-by-byte identical response. A conversion to a weak ETag still allows
If-None-Match to work.

This should be backported to 1.9 and might be backported to every supported
branch with compression.

6 years agoBUG/MEDIUM: servers: Close the connection if we failed to install the mux.
Olivier Houchard [Tue, 29 Jan 2019 18:11:16 +0000 (19:11 +0100)] 
BUG/MEDIUM: servers: Close the connection if we failed to install the mux.

If we failed to install the mux, just close the connection, or
conn_fd_handler() will be called for the FD, and crash as soon as it attempts
to access the mux' wake method.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: peers: Handle mux creation failure.
Olivier Houchard [Tue, 29 Jan 2019 18:00:33 +0000 (19:00 +0100)] 
BUG/MEDIUM: peers: Handle mux creation failure.

If the mux fails to properly be created by conn_install_mux, fail, instead
of silently ignoring it.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: h2: In h2_send(), stop the loop if we failed to alloc a buf.
Olivier Houchard [Tue, 29 Jan 2019 17:28:36 +0000 (18:28 +0100)] 
BUG/MEDIUM: h2: In h2_send(), stop the loop if we failed to alloc a buf.

In h2_send(), make sure we break the loop if we failed to alloc a buffer,
or we'd end up looping endlessly.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: checks: Don't try to set ALPN if connection failed.
Olivier Houchard [Tue, 29 Jan 2019 15:37:52 +0000 (16:37 +0100)] 
BUG/MEDIUM: checks: Don't try to set ALPN if connection failed.

If we failed to connect, don't attempt to set the ALPN, as we don't have
a SSL context, anyway.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Don't add an incomplete conn to the server idle list.
Olivier Houchard [Tue, 29 Jan 2019 15:05:02 +0000 (16:05 +0100)] 
BUG/MEDIUM: servers: Don't add an incomplete conn to the server idle list.

If we failed to add the connection to the session, only try to add it back
to the server idle list if it has a mux, otherwise the connection is
incomplete and unusable.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Only destroy a conn_stream we just allocated.
Olivier Houchard [Tue, 29 Jan 2019 14:50:38 +0000 (15:50 +0100)] 
BUG/MEDIUM: servers: Only destroy a conn_stream we just allocated.

In connect_server(), if we failed to add the connection to the session,
only destroy the conn_stream if we just allocated it, otherwise it may
have been allocated outside connect_server(), along with a connection which
has its destination address set.
Also use si_release_endpoint() instead of cs_destroy(), to make sure the
stream_interface doesn't reference it anymore.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: checks: Check that conn_install_mux succeeded.
Olivier Houchard [Tue, 29 Jan 2019 14:47:43 +0000 (15:47 +0100)] 
BUG/MEDIUM: checks: Check that conn_install_mux succeeded.

If conn_install_mux failed, then the connection has no mux and won't be
usable, so just give up is on failure instead of ignoring it.

This should be backported to 1.9.

6 years agoCLEANUP: mux-h2: remove two useless but misleading assignments
Willy Tarreau [Tue, 29 Jan 2019 17:51:41 +0000 (18:51 +0100)] 
CLEANUP: mux-h2: remove two useless but misleading assignments

h2c->st0 was assigned to H2_CS_ERROR right after returning from
h2c_error(), which had already done it. It's useless and confusing,
let's remove this.

6 years agoBUG/MEDIUM: mux-h2: only close connection on request frames on closed streams
Willy Tarreau [Tue, 29 Jan 2019 17:33:26 +0000 (18:33 +0100)] 
BUG/MEDIUM: mux-h2: only close connection on request frames on closed streams

A subtle bug was introduced with H2 on the backend. RFC7540 states that
an attempt to create a stream on an ID not higher than the max known is
a connection error. This was translated into rejecting HEADERS frames
for closed streams. But with H2 on the backend, if the client aborts
and causes an RST_STREAM to be emitted, the stream is effectively closed,
and if/once the server responds, it starts by emitting a HEADERS frame
with this ID thus it is interpreted as a connection error.

This test must of course consider the side the mux is installed on and
not take this for a connection error on responses.

The effect is that an aborted stream on an outgoing H2 connection, for
example due to a client stopping a transfer with option abortonclose
set, would lead to an abort of all other streams. In the logs, this
appears as one or several CD-- line(s) followed by one or several SD--
lines which are victims.

Thanks to Luke Seelenbinder for reporting this problem and providing
enough elements to help understanding how to reproduce it.

This fix must be backported to 1.9.

6 years agoBUILD/MINOR: peers: shut up a build warning introduced during last cleanup
Willy Tarreau [Tue, 29 Jan 2019 16:45:23 +0000 (17:45 +0100)] 
BUILD/MINOR: peers: shut up a build warning introduced during last cleanup

A new warning appears when building at -O0 since commit 3f0fb9df6 ("MINOR:
peers: move "hello" message treatment code to reduce the size of the I/O
handler."), it is related to the fact that proto_len is initialized from
strlen() which is not a constant. Let's replace it with sizeof-1 instead
and also mark the variable as static since it's useless outside of the file.

6 years agoCLEANUP: peers: factor error handling in peer_treat_definedmsg()
Willy Tarreau [Tue, 29 Jan 2019 10:11:23 +0000 (11:11 +0100)] 
CLEANUP: peers: factor error handling in peer_treat_definedmsg()

This is a trivial code refactoring of similar parsing error code
under a single label.

6 years agoCLEANUP: peers: factor the error handling code in peer_treet_updatemsg()
Willy Tarreau [Tue, 29 Jan 2019 10:08:06 +0000 (11:08 +0100)] 
CLEANUP: peers: factor the error handling code in peer_treet_updatemsg()

The error handling code was extremely repetitive and error-prone due
to the numerous copy-pastes, some involving unlocks or free. Let's
factor this out. The code could be further simplified, but 12 locations
were already cleaned without taking risks.

6 years agoMINOR: peers: move peer initializations code to reduce the size of the I/O handler.
Frédéric Lécaille [Fri, 25 Jan 2019 07:58:41 +0000 (08:58 +0100)] 
MINOR: peers: move peer initializations code to reduce the size of the I/O handler.

Implements two new functions to init peer flags and other stuff after
having accepted or connected them with the peer I/O handler so that
to reduce its size.

May be backported as far as 1.5.

6 years agoMINOR: peers: move "hello" message treatment code to reduce the size of the I/O handler.
Frédéric Lécaille [Fri, 25 Jan 2019 07:30:29 +0000 (08:30 +0100)] 
MINOR: peers: move "hello" message treatment code to reduce the size of the I/O handler.

This patch implements three functions to read and parse the three
line of a "hello" peer protocol message so that to call them from the
peer I/O handler and reduce its size.

May be backported as far as 1.5.

6 years agoCLEANUP: peers: Remove useless statements.
Frédéric Lécaille [Thu, 24 Jan 2019 17:28:44 +0000 (18:28 +0100)] 
CLEANUP: peers: Remove useless statements.

When implementing peer_recv_msg() we added the statements reached with
a "goto imcomplete" at the end of this function. This statements
are executed only when co_getblk() returns something <0. So they
are useless for now on, and may be safely removed. The following
section wich was responsible of sending any peer protocol messages
were reached only when co_getblk() returned 0 (no more message to
read). In this case we replace the "goto impcomplete" statement by
a "goto send_msgs" to reach this only when peer_recv_msg() returns 0.

May be backported as far as 1.5.

6 years agoMINOR: peers: move send code to reduce the size of the I/O handler.
Frédéric Lécaille [Thu, 24 Jan 2019 16:33:48 +0000 (17:33 +0100)] 
MINOR: peers: move send code to reduce the size of the I/O handler.

This patch extracts the code responsible of sending peer protocol
messages from the peer I/O handler to create a new function and to
reduce the size of this handler.

May be backported as far as 1.5.

6 years agoMINOR: peers: move messages treatment code to reduce the size of the I/O handler.
Frédéric Lécaille [Thu, 24 Jan 2019 14:40:11 +0000 (15:40 +0100)] 
MINOR: peers: move messages treatment code to reduce the size of the I/O handler.

Extract the code of the peer I/O handler responsible of treating
any peer protocol message to create peer_treat_awaited_msg() function.
Also rename peer_recv_updatemsg() to peer_treat_updatemsg() as this
function only parse a stick-table update message already received
by peer_recv_msg().

May be backported as far as 1.5.

6 years agoMINOR: peers: move error handling to reduce the size of the I/O handler.
Frédéric Lécaille [Thu, 24 Jan 2019 13:24:05 +0000 (14:24 +0100)] 
MINOR: peers: move error handling to reduce the size of the I/O handler.

Implement new functions to send error and control class stick-table
messages.

May be backported as far as 1.5.

6 years agoCLEANUP: peers: Be more generic.
Frédéric Lécaille [Thu, 24 Jan 2019 09:33:40 +0000 (10:33 +0100)] 
CLEANUP: peers: Be more generic.

Make usage of a C union to pass parameters to all the peer_prepare_*()
functions (more readable).

May be backported as far as 1.5.

6 years agoMINOR: peers: Move high level receive code to reduce the size of I/O handler.
Frédéric Lécaille [Wed, 23 Jan 2019 18:38:11 +0000 (19:38 +0100)] 
MINOR: peers: Move high level receive code to reduce the size of I/O handler.

Implement a new function to read incoming stick-table messages.

May be backported as far as 1.5.

6 years agoMINOR: peers: Move ack, switch and definition receive code to reduce the size of...
Frédéric Lécaille [Wed, 23 Jan 2019 16:31:37 +0000 (17:31 +0100)] 
MINOR: peers: Move ack, switch and definition receive code to reduce the size of the I/O handler.

Implement three new functions to treat peer acks, switch and
definition messages extracting the code from the big swich-case
of the peer I/O handler to give more chances to this latter to be
readable.

May be backported as far as 1.5.

6 years agoMINOR: peers: Move update receive code to reduce the size of the I/O handler.
Frédéric Lécaille [Wed, 23 Jan 2019 10:16:57 +0000 (11:16 +0100)] 
MINOR: peers: Move update receive code to reduce the size of the I/O handler.

This patch implements a new function to treat the stick-table
update messages so that to reduce the size of the peer I/O handler
by ~200 lines.

May be backported as far as 1.5.

6 years agoMEDIUM: peers: synchronizaiton code factorization to reduce the size of the I/O handler.
Frédéric Lécaille [Tue, 22 Jan 2019 21:25:17 +0000 (22:25 +0100)] 
MEDIUM: peers: synchronizaiton code factorization to reduce the size of the I/O handler.

Factorize the code responsible of synchronizing the peers upon startup.

May be backported as far as 1.5.

6 years agoMINOR: peers: Add new functions to send code and reduce the I/O handler.
Frédéric Lécaille [Tue, 22 Jan 2019 16:26:50 +0000 (17:26 +0100)] 
MINOR: peers: Add new functions to send code and reduce the I/O handler.

This patch reduces the size of the peer I/O handler implementing
a new function named peer_send_updatemsg() which uses the already
implement peer_prepare_updatemsg(), then ci_putblk().
Reuse the code used to implement peer_send_(ack|swith)msg() function
especially the more generic function peer_send_msg().

May be backported as far as 1.5.

6 years agoMINOR: peers: send code factorization.
Frédéric Lécaille [Tue, 22 Jan 2019 14:54:53 +0000 (15:54 +0100)] 
MINOR: peers: send code factorization.

Implements peer_send_*msg() functions for switch and ack messages which call the
already defined peer_prepare_*msg() before calling ci_putblk().
These two new functions are used at three places in the peer_io_handler().

May be backported as far as 1.5.

6 years agoCLEANUP: peers: Indentation fixes.
Frédéric Lécaille [Tue, 22 Jan 2019 09:31:39 +0000 (10:31 +0100)] 
CLEANUP: peers: Indentation fixes.

May be backported as far as 1.5.

6 years agoMINOR: peers: Extract some code to be reused.
Frédéric Lécaille [Mon, 21 Jan 2019 12:38:06 +0000 (13:38 +0100)] 
MINOR: peers: Extract some code to be reused.

May be backported as far as 1.5.

6 years agoSCRIPTS: add the issue tracker URL to the announce script
Willy Tarreau [Tue, 29 Jan 2019 05:51:16 +0000 (06:51 +0100)] 
SCRIPTS: add the issue tracker URL to the announce script

This way it's easier for users to follow the status of pending issues
with each release.

6 years agoBUG/MEDIUM: backend: always call si_detach_endpoint() on async connection failure
Willy Tarreau [Mon, 28 Jan 2019 15:33:35 +0000 (16:33 +0100)] 
BUG/MEDIUM: backend: always call si_detach_endpoint() on async connection failure

In case an asynchronous connection (ALPN) succeeds but the mux fails to
attach, we must release the stream interface's endpoint, otherwise we
leave the stream interface with an endpoint pointing to a freed connection
with si_ops == si_conn_ops, and sess_update_st_cer() calls si_shutw() on
it, causing it to crash.

This must be backported to 1.9 only.

6 years agoBUG/MEDIUM: servers: Attempt to reuse an unfinished connection on retry.
Olivier Houchard [Mon, 28 Jan 2019 14:33:15 +0000 (15:33 +0100)] 
BUG/MEDIUM: servers: Attempt to reuse an unfinished connection on retry.

In connect_server(), if the previous connection failed, but had an alpn, no
mux was created, and thus the stream_interface's endpoint would be the
connection. In this case, instead of forgetting about it, and overriding
the stream_interface's endpoint later, try to reuse the connection, or the
connection will still be in the session's connection list, and will reference
to a stream that was probably destroyed.

This should be backported to 1.9.

6 years agoBUG/MINOR: task: fix possibly missed event in inter-thread wakeups
Willy Tarreau [Sun, 27 Jan 2019 16:41:27 +0000 (17:41 +0100)] 
BUG/MINOR: task: fix possibly missed event in inter-thread wakeups

There's a very small but existing uncertainty window when waking another
thread up where it is possible for task_wakeup() not to wake the other
task up because it's still running while this once is in the process of
finishing and loses its TASK_RUNNING flag. In this case the wakeup will
be missed.

The problem is that we have a single flag to store 3 states, since the
transition from running to sleeping isn't atomic. Thus we need to have
another flag to cover this part. This patch introduces TASK_QUEUED to
mention that the task is already in the run queue, running or not. This
bit will be removed while TASK_RUNNING is kept once dequeued, and will
be used when removing TASK_RUNNING to check if the task has been requeued.

It might be possible to slightly improve this but the occurrence rate
is quite low and we don't really need to complexify the scheduler to
optimize for a rare case.

The impact with the current code is very low since we have few inter-
thread wakeups. Most of them are caused by checks killing sessions.

This must be backported to 1.9.

6 years agoBUG/MINOR: spoe: corrected fragmentation string size
Miroslav Zagorac [Sun, 13 Jan 2019 15:55:01 +0000 (16:55 +0100)] 
BUG/MINOR: spoe: corrected fragmentation string size

This patch must be backported to 1.9 and 1.8.

6 years agoBUG/MINOR: mux-h2: do not report available outgoing streams after GOAWAY
Willy Tarreau [Mon, 28 Jan 2019 05:40:19 +0000 (06:40 +0100)] 
BUG/MINOR: mux-h2: do not report available outgoing streams after GOAWAY

The calculation of available outgoing H2 streams was improved by commit
d64a3ebe6 ("BUG/MINOR: mux-h2: always check the stream ID limit in
h2_avail_streams()"), but it still is incorrect because RFC7540#6.8
specifically forbids the creation of new streams after a GOAWAY frame
was received. Thus we must not mark the connection as available anymore
in order to be able to handle a graceful shutdown.

This needs to be backported to 1.9.

6 years agoBUG/MINOR: listener: always fill the source address for accepted socketpairs
Willy Tarreau [Sun, 27 Jan 2019 17:34:12 +0000 (18:34 +0100)] 
BUG/MINOR: listener: always fill the source address for accepted socketpairs

The source address was not set but passed down the chain to the upper
layer's accept() calls. Let's initialize it like other UNIX sockets in
this case. At the moment it should not have any impact since socketpairs
are only usable for the master CLI.

This should be backported to 1.9.

6 years agoDOC: nbthread is no longer experimental.
Willy Tarreau [Sat, 26 Jan 2019 13:20:55 +0000 (14:20 +0100)] 
DOC: nbthread is no longer experimental.

It was mentioned when releasing 1.8 but early bugs have long been
addressed and this comment discourages some users from using threads.

This should be backported to 1.9 and 1.8 now.

6 years agoMINOR: threads: make MAX_THREADS configurable at build time
Willy Tarreau [Sat, 26 Jan 2019 12:35:03 +0000 (13:35 +0100)] 
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.

6 years agoMINOR: cfgparse: make the process/thread parser support a maximum value
Willy Tarreau [Sat, 26 Jan 2019 12:25:14 +0000 (13:25 +0100)] 
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).

6 years agoCLEANUP: h2: Remove debug printf in mux_h2.c
Tim Duesterhus [Thu, 24 Jan 2019 23:56:59 +0000 (00:56 +0100)] 
CLEANUP: h2: Remove debug printf in mux_h2.c

It was introduced by 1915ca273832ba542d72eb0645dd7ccb6d5b945f
and should be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: always compare content-length to the sum of DATA frames
Willy Tarreau [Thu, 24 Jan 2019 10:49:37 +0000 (11:49 +0100)] 
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.

6 years agoMEDIUM: h2: always parse and deduplicate the content-length header
Willy Tarreau [Thu, 24 Jan 2019 10:33:02 +0000 (11:33 +0100)] 
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.

6 years agoMINOR: stream: don't wait before retrying after a failed connection reuse
Willy Tarreau [Wed, 23 Jan 2019 15:29:53 +0000 (16:29 +0100)] 
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.

6 years agoMEDIUM: stream-int: always mark pending outgoing SI_ST_CON
Willy Tarreau [Wed, 23 Jan 2019 14:15:09 +0000 (15:15 +0100)] 
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.

6 years agoMINOR: mux-h2: always consider a server's max-reuse parameter
Willy Tarreau [Wed, 23 Jan 2019 09:25:10 +0000 (10:25 +0100)] 
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.

6 years agoMINOR: server: add a max-reuse parameter
Willy Tarreau [Wed, 23 Jan 2019 09:21:49 +0000 (10:21 +0100)] 
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.

6 years agoBUG/MEDIUM: backend: never try to attach to a mux having no more stream available
Willy Tarreau [Thu, 24 Jan 2019 17:22:19 +0000 (18:22 +0100)] 
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.

6 years agoBUG/MINOR: mux-h2: refuse to allocate a stream with too high an ID
Willy Tarreau [Thu, 24 Jan 2019 16:08:28 +0000 (17:08 +0100)] 
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.

6 years agoBUG/MINOR: mux-h2: always check the stream ID limit in h2_avail_streams()
Willy Tarreau [Wed, 23 Jan 2019 09:22:21 +0000 (10:22 +0100)] 
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.

6 years agoCLEANUP: server: fix indentation mess on idle connections
Willy Tarreau [Wed, 23 Jan 2019 09:41:19 +0000 (10:41 +0100)] 
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.

6 years agoBUG/MINOR: stream: take care of synchronous errors when trying to send
Willy Tarreau [Wed, 23 Jan 2019 14:18:19 +0000 (15:18 +0100)] 
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.

6 years agoMINOR: server: make sure pool-max-conn is >= -1
Willy Tarreau [Wed, 23 Jan 2019 09:39:27 +0000 (10:39 +0100)] 
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.

6 years agoBUG/MINOR: hpack: return a compression error on invalid table size updates
Willy Tarreau [Thu, 24 Jan 2019 09:47:10 +0000 (10:47 +0100)] 
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.

6 years agoBUG/MINOR: mux-h2: make it possible to set the error code on an already closed stream
Willy Tarreau [Thu, 24 Jan 2019 09:02:24 +0000 (10:02 +0100)] 
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.

6 years agoBUG/MINOR: mux-h2: headers-type frames in HREM are always a connection error
Willy Tarreau [Thu, 24 Jan 2019 08:43:32 +0000 (09:43 +0100)] 
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.

6 years agoBUG/MINOR: mux-h2: CONTINUATION in closed state must always return GOAWAY
Willy Tarreau [Thu, 24 Jan 2019 08:36:53 +0000 (09:36 +0100)] 
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).

6 years agoMINOR: h2: declare new sets of frame types
Willy Tarreau [Thu, 24 Jan 2019 08:31:40 +0000 (09:31 +0100)] 
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.

6 years agoBUG/MEDIUM: mux-h2: properly abort on trailers decoding errors
Willy Tarreau [Thu, 24 Jan 2019 09:26:47 +0000 (10:26 +0100)] 
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.

6 years agoBUG/MEDIUM: backend: also remove from idle list muxes that have no more room
Willy Tarreau [Wed, 23 Jan 2019 16:33:06 +0000 (17:33 +0100)] 
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.

6 years agoBUG/MAJOR: mux-h2: don't destroy the stream on failed allocation in h2_snd_buf()
Willy Tarreau [Wed, 23 Jan 2019 13:39:41 +0000 (14:39 +0100)] 
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.

6 years agoBUG/MINOR: mux-h1: avoid copying output over itself in zero-copy
Willy Tarreau [Wed, 23 Jan 2019 19:43:53 +0000 (20:43 +0100)] 
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>
6 years agoBUG/MINOR: mux-h1: Apply the reserve on the channel's buffer only
Christopher Faulet [Mon, 21 Jan 2019 10:55:02 +0000 (11:55 +0100)] 
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.

6 years agoBUG/MEDIUM: mux-h2/htx: Respect the channel's reserve
Christopher Faulet [Mon, 21 Jan 2019 10:49:37 +0000 (11:49 +0100)] 
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.

6 years agoBUG/MINOR: proto-htx: Return an error if all headers cannot be received at once
Christopher Faulet [Mon, 21 Jan 2019 10:24:38 +0000 (11:24 +0100)] 
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.

6 years agoDOC: mention the effect of nf_conntrack_tcp_loose on src/dst
Willy Tarreau [Wed, 23 Jan 2019 09:02:15 +0000 (10:02 +0100)] 
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

6 years agoBUG/MEDIUM: ssl: Fix handling of TLS 1.3 KeyUpdate messages
Dirkjan Bussink [Mon, 21 Jan 2019 17:35:03 +0000 (09:35 -0800)] 
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>
6 years agoBUG/MINOR: check: Wake the check task if the check is finished in wake_srv_chk()
Christopher Faulet [Mon, 21 Jan 2019 13:15:50 +0000 (14:15 +0100)] 
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

6 years agoBUG/MINOR: server: don't always trust srv_check_health when loading a server state
Jérôme Magnin [Sun, 20 Jan 2019 10:27:40 +0000 (11:27 +0100)] 
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.

6 years agoBUG/MEDIUM: checks: fix recent regression on agent-check making it crash
Willy Tarreau [Mon, 21 Jan 2019 06:48:26 +0000 (07:48 +0100)] 
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.

6 years agoBUG/MINOR: startup: certain goto paths in init_pollers fail to free
Uman Shahzad [Thu, 17 Jan 2019 08:21:39 +0000 (08:21 +0000)] 
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.

6 years agoDOC: peers: SSL/TLS documentation for "peers"
Frédéric Lécaille [Fri, 11 Jan 2019 13:13:54 +0000 (14:13 +0100)] 
DOC: peers: SSL/TLS documentation for "peers"

6 years agoMINOR: cfgparse: SSL/TLS binding in "peers" sections.
Frédéric Lécaille [Fri, 11 Jan 2019 13:06:12 +0000 (14:06 +0100)] 
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.

6 years agoMINOR: peers: Make outgoing connection to SSL/TLS peers work.
Frédéric Lécaille [Thu, 26 Apr 2018 12:35:21 +0000 (14:35 +0200)] 
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.

6 years agoMINOR: cfgparse: Make "peer" lines be parsed as "server" lines.
Frédéric Lécaille [Thu, 26 Apr 2018 08:06:41 +0000 (10:06 +0200)] 
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.

6 years agoMINOR: cfgparse: Simplication.
Frédéric Lécaille [Fri, 11 Jan 2019 10:47:12 +0000 (11:47 +0100)] 
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.

6 years agoMINOR: cfgparse: Rework peers frontend init.
Frédéric Lécaille [Fri, 11 Jan 2019 10:43:53 +0000 (11:43 +0100)] 
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.

6 years agoMINOR: cfgparse: Useless frontend initialization in "peers" sections.
Frédéric Lécaille [Wed, 25 Apr 2018 13:32:18 +0000 (15:32 +0200)] 
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.

6 years agoCLEANUP: cfgparse: Code reindentation.
Frédéric Lécaille [Fri, 11 Jan 2019 10:27:16 +0000 (11:27 +0100)] 
CLEANUP: cfgparse: Code reindentation.

May help the series of patches to be reviewed.

May be backported to 1.5 and newer.

6 years agoCLEANUP: cfgparse: Return asap from cfg_parse_peers().
Frédéric Lécaille [Wed, 25 Apr 2018 13:13:38 +0000 (15:13 +0200)] 
CLEANUP: cfgparse: Return asap from cfg_parse_peers().

Avoid useless code indentation.

May be backported to 1.5 and newer.

6 years agoMINOR: cfgparse: Extract some code to be re-used.
Frédéric Lécaille [Fri, 11 Jan 2019 10:07:15 +0000 (11:07 +0100)] 
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.

6 years agoDOC: add github issue templates
Lukas Tribus [Thu, 17 Jan 2019 17:02:18 +0000 (18:02 +0100)] 
DOC: add github issue templates

See issue #2

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
6 years agoBUG/MEDIUM: connections: Add the CO_FL_CONNECTED flag if a send succeeded.
Olivier Houchard [Thu, 17 Jan 2019 18:09:11 +0000 (19:09 +0100)] 
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.

6 years agoBUG/MEDIUM: servers: Make assign_tproxy_address work when ALPN is set.
Olivier Houchard [Thu, 17 Jan 2019 14:59:13 +0000 (15:59 +0100)] 
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.

6 years agoREGTEST: checks basic stats webpage functionality
PiBa-NL [Sat, 12 Jan 2019 20:57:48 +0000 (21:57 +0100)] 
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.

6 years agoDOC: add a missing space in the documentation for bc_http_major
Jérôme Magnin [Wed, 16 Jan 2019 13:38:37 +0000 (14:38 +0100)] 
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.

6 years agoBUG/MEDIUM: stats: Get the right scope pointer depending on HTX is used or not
Christopher Faulet [Mon, 14 Jan 2019 10:07:34 +0000 (11:07 +0100)] 
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.

6 years agoBUG: 51d: Changes to the buffer API in 1.9 were not applied to the 51Degrees code.
Ben51Degrees [Wed, 16 Jan 2019 10:19:15 +0000 (10:19 +0000)] 
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.

6 years agoBUG/MINOR: stick_table: Prevent conn_cur from underflowing
Tim Duesterhus [Thu, 3 Jan 2019 23:11:59 +0000 (00:11 +0100)] 
BUG/MINOR: stick_table: Prevent conn_cur from underflowing

When using the peers feature a race condition could prevent
a connection from being properly counted. When this connection
exits it is being "uncounted" nonetheless, leading to a possible
underflow (-1) of the conn_curr stick table entry in the following
scenario :

  - Connect to peer A     (A=1, B=0)
  - Peer A sends 1 to B   (A=1, B=1)
  - Kill connection to A  (A=0, B=1)
  - Connect to peer B     (A=0, B=2)
  - Peer A sends 0 to B   (A=0, B=0)
  - Peer B sends 0/2 to A (A=?, B=0)
  - Kill connection to B  (A=?, B=-1)
  - Peer B sends -1 to A  (A=-1, B=-1)

This fix may be backported to all supported branches.

6 years agoBUILD/MEDIUM: da: Necessary code changes for new buffer API.
David Carlier [Tue, 15 Jan 2019 12:09:00 +0000 (12:09 +0000)] 
BUILD/MEDIUM: da: Necessary code changes for new buffer API.

The most significant change from 1.8 to >=1.9 is the buffer
data structure, using the new field and fixing along side
a little hidden compilation warning.

This must be backported to 1.9.

6 years agoMINOR: backend: make the random algorithm support a number of draws
Willy Tarreau [Mon, 14 Jan 2019 17:14:27 +0000 (18:14 +0100)] 
MINOR: backend: make the random algorithm support a number of draws

When an argument <draws> is present, it must be an integer value one
or greater, indicating the number of draws before selecting the least
loaded of these servers. It was indeed demonstrated that picking the
least loaded of two servers is enough to significantly improve the
fairness of the algorithm, by always avoiding to pick the most loaded
server within a farm and getting rid of any bias that could be induced
by the unfair distribution of the consistent list. Higher values N will
take away N-1 of the highest loaded servers at the expense of performance.
With very high values, the algorithm will converge towards the leastconn's
result but much slower. The default value is 2, which generally shows very
good distribution and performance. This algorithm is also known as the
Power of Two Random Choices and is described here :

http://www.eecs.harvard.edu/~michaelm/postscripts/handbook2001.pdf

6 years agoMEDIUM: backend: move all LB algo parameters into an union
Willy Tarreau [Mon, 14 Jan 2019 15:55:42 +0000 (16:55 +0100)] 
MEDIUM: backend: move all LB algo parameters into an union

Since all of them are exclusive, let's move them to an union instead
of eating memory with the sum of all of them. We're using a transparent
union to limit the code changes.

Doing so reduces the struct lbprm from 392 bytes to 372, and thanks
to these changes, the struct proxy is now down to 6480 bytes vs 6624
before the changes (144 bytes saved per proxy).

6 years agoMINOR: backend: move hash_balance_factor out of chash
Willy Tarreau [Mon, 14 Jan 2019 15:50:58 +0000 (16:50 +0100)] 
MINOR: backend: move hash_balance_factor out of chash

This one is a proxy option which can be inherited from defaults even
if the LB algo changes. Move it out of the lb_chash struct so that we
don't need to keep anything separate between these structs. This will
allow us to merge them into an union later. It even takes less room
now as it fills a hole and removes another one.

6 years agoMINOR: backend: remap the balance uri settings to lbprm.arg_opt{1,2,3}
Willy Tarreau [Mon, 14 Jan 2019 15:14:15 +0000 (16:14 +0100)] 
MINOR: backend: remap the balance uri settings to lbprm.arg_opt{1,2,3}

The algo-specific settings move from the proxy to the LB algo this way :
  - uri_whole => arg_opt1
  - uri_len_limit => arg_opt2
  - uri_dirs_depth1 => arg_opt3

6 years agoMINOR: backend: make the header hash use arg_opt1 for use_domain_only
Willy Tarreau [Mon, 14 Jan 2019 15:04:54 +0000 (16:04 +0100)] 
MINOR: backend: make the header hash use arg_opt1 for use_domain_only

This is only a boolean extra arg. Let's map it to arg_opt1 and remove
hh_match_domain from struct proxy.

6 years agoMINOR: backend: add new fields in lbprm to store more LB options
Willy Tarreau [Mon, 14 Jan 2019 15:04:01 +0000 (16:04 +0100)] 
MINOR: backend: add new fields in lbprm to store more LB options

Some algorithms require a few extra options (up to 3). Let's provide
some room in lbprm to store them, and make sure they're passed from
defaults to backends.