]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoBUG/MEDIUM: server: initialize the orphaned conns lists and tasks at the end
Willy Tarreau [Thu, 7 Feb 2019 13:59:29 +0000 (14:59 +0100)] 
BUG/MEDIUM: server: initialize the orphaned conns lists and tasks at the end

This also depends on the nbthread count, so it must only be performed after
parsing the whole config file. As a side effect, this removes some code
duplication between servers and server-templates.

This must be backported to 1.9.

6 years agoBUG/MEDIUM: server: initialize the idle conns list after parsing the config
Willy Tarreau [Thu, 7 Feb 2019 13:46:29 +0000 (14:46 +0100)] 
BUG/MEDIUM: server: initialize the idle conns list after parsing the config

The idle conns lists are sized according to the number of threads. As such
they cannot be initialized during the parsing since nbthread can be set
later, as revealed by this simple config which randomly crashes when used.
Let's do this at the end instead.

    listen proxy
        bind :4445
        mode http
        timeout client 10s
        timeout server 10s
        timeout connect 10s
        http-reuse always
        server s1 127.0.0.1:8000

    global
        nbthread 8

This fix must be backported to 1.9 and 1.8.

6 years agoBUG/MEDIUM: spoe: initialization depending on nbthread must be done last
Willy Tarreau [Thu, 7 Feb 2019 12:40:33 +0000 (13:40 +0100)] 
BUG/MEDIUM: spoe: initialization depending on nbthread must be done last

The agent used to be configured depending on global.nbthread while nbthread
may be set after the agent is parsed. Let's move this part to the spoe_check()
function to make sure nbthread is always correct and arrays are appropriately
sized.

This fix must be backported to 1.9 and 1.8.

6 years agoBUG/MINOR: lua: initialize the correct idle conn lists for the SSL sockets
Willy Tarreau [Thu, 7 Feb 2019 13:48:24 +0000 (14:48 +0100)] 
BUG/MINOR: lua: initialize the correct idle conn lists for the SSL sockets

Commit 40a007cf2 ("MEDIUM: threads/server: Make connection list
(priv/idle/safe) thread-safe") made a copy-paste error when initializing
the Lua sockets, as the TCP one was initialized twice. Fortunately it has
no impact because the pointers are set to NULL after a memset(0) and are
not changed in between.

This must be backported to 1.9 and 1.8.

6 years agoBUG/MINOR: spoe: do not assume agent->rt is valid on exit
Willy Tarreau [Thu, 7 Feb 2019 13:22:52 +0000 (14:22 +0100)] 
BUG/MINOR: spoe: do not assume agent->rt is valid on exit

As reported by Christopher, we may call spoe_release_agent() when leaving
after an allocation failure or a config parse error. We must not assume
agent->rt is valid there as the allocation could have failed.

This should be backported to 1.9 and 1.8.

6 years agoDOC: ssl: Stop documenting ciphers example to use
Bertrand Jacquin [Sun, 3 Feb 2019 18:48:49 +0000 (18:48 +0000)] 
DOC: ssl: Stop documenting ciphers example to use

Since TLS ciphers are not well understand, it is very common pratice to
copy and paste parameters from documentation and use them as-is. Since RC4
should not be used anymore, it is wiser to link users to up to date
documnetation from Mozilla to avoid unsafe configuration in the wild.

Clarify the location of man pages for OpenSSL when missing.

6 years agoDOC: ssl: Clarify when pre TLSv1.3 cipher can be used
Bertrand Jacquin [Sun, 3 Feb 2019 18:35:25 +0000 (18:35 +0000)] 
DOC: ssl: Clarify when pre TLSv1.3 cipher can be used

This is mainly driven by the fact TLSv1.3 will have a successor at some
point.

6 years agoBUG/MINOR: config: make sure to count the error on incorrect track-sc/stick rules
Willy Tarreau [Wed, 6 Feb 2019 09:25:07 +0000 (10:25 +0100)] 
BUG/MINOR: config: make sure to count the error on incorrect track-sc/stick rules

When commit 151e1ca98 ("BUG/MAJOR: config: verify that targets of track-sc
and stick rules are present") added a check for some process inconsistencies
between rules and their stick tables, some errors resulted in a "return 0"
statement, which is taken as "no error" in some cases. Let's fix this.

This must be backported to all versions using the above commit.

6 years agoBUG/MAJOR: htx/backend: Make all tests on HTTP messages compatible with HTX
Christopher Faulet [Mon, 4 Feb 2019 11:02:18 +0000 (12:02 +0100)] 
BUG/MAJOR: htx/backend: Make all tests on HTTP messages compatible with HTX

A piece of code about the HTX was lost this summer, after the "big merge"
(htx/http2/connection layer refactoring). I forgot to keep HTX changes in the
functions connect_server() and assign_server(). So, this patch fixes "uri",
"url_param" and "hdr" LB algorithms when the HTX is enabled. It also fixes
evaluation of the "sni" expression on server lines.

This issue was reported on github. See issue #32.

This patch must be backported in 1.9.

6 years agoBUG/MAJOR: spoe: verify that backends used by SPOE cover all their callers' processes
Willy Tarreau [Tue, 5 Feb 2019 12:37:19 +0000 (13:37 +0100)] 
BUG/MAJOR: spoe: verify that backends used by SPOE cover all their callers' processes

When a filter is installed on a proxy and references spoe, we must be
absolutely certain that the whole chain is valid on a given process
when running in multi-process mode. The problem here is that if a proxy
1 runs on process 1, referencing an SPOE agent itself based on a backend
running on process 2, this last one will be completely deinited on
process 1, and will thus cause random crashes when it gets messages
from this proess.

This patch makes sure that the whole chain is valid on all of the caller's
processes.

This fix must be backported to all spoe-enabled maintained versions. It
may potentially disrupt configurations which already randomly crash.
There hardly is any intermediary solution though, such configurations
need to be fixed.

6 years agoBUG/MAJOR: config: verify that targets of track-sc and stick rules are present
Willy Tarreau [Tue, 5 Feb 2019 10:38:38 +0000 (11:38 +0100)] 
BUG/MAJOR: config: verify that targets of track-sc and stick rules are present

Stick and track-sc rules may optionally designate a table in a different
proxy. In this case, a number of verifications are made such as validating
that this proxy actually exists. However, in multi-process mode, the target
table might indeed exist but not be bound to the set of processes the rules
will execute on. This will definitely result in a random behaviour especially
if these tables do require peer synchronization, because some tasks will be
started to try to synchronize form uninitialized areas.

The typical issue looks like this :

    peers my-peers
         peer foo ...

    listen proxy
         bind-process 1
         stick on src table ip
         ...

    backend ip
         bind-process 2
         stick-table type ip size 1k peers my-peers

While it appears obvious that the example above will not work, there are
less obvious situations, such as having bind-process in a defaults section
and having a larger set of processes for the referencing proxy than the
referenced one.

The present patch adds checks for such situations by verifying that all
processes from the referencing proxy are present on the other one in all
track-sc* and stick-* rules, and in sample fetch / converters referencing
another table so that sc_inc_gpc0() and similar are safe as well.

This fix must be backported to all maintained versions. It may potentially
disrupt configurations which already randomly crash. There hardly is any
intermediary solution though, such configurations need to be fixed.

6 years agoBUG/MINOR: task: close a tiny race in the inter-thread wakeup
Willy Tarreau [Mon, 4 Feb 2019 09:26:53 +0000 (10:26 +0100)] 
BUG/MINOR: task: close a tiny race in the inter-thread wakeup

__task_wakeup() takes care of a small race that exists between threads,
but it uses a store barrier that is not sufficient since apparently the
state read after clearing the leaf_p pointer sometimes is incorrect. This
results in missed wakeups between threads competing at a high rate. Let's
use a full barrier instead to serialize the operations.

This may be backported to 1.9 though it's extremely unlikely that this
bug will ever manifest itself there.

6 years agoBUG/MINOR: compression: properly report compression stats in HTX mode
Willy Tarreau [Mon, 4 Feb 2019 10:48:03 +0000 (11:48 +0100)] 
BUG/MINOR: compression: properly report compression stats in HTX mode

When HTX support was added to HTTP compression, a set of counters was missed,
namely comp_in and comp_byp, resulting in no stats being available for compression.

This must be backported to 1.9.

6 years agoMINOR: threads: make use of thread_mask() to simplify some thread calculations
Willy Tarreau [Sat, 2 Feb 2019 17:00:17 +0000 (18:00 +0100)] 
MINOR: threads: make use of thread_mask() to simplify some thread calculations

By doing so it's visible that some fd_insert() calls were relying on
MAX_THREADS while all_threads_mask should have been more suitable.

6 years agoMINOR: config: simplify bind_proc processing using proc_mask()
Willy Tarreau [Sat, 2 Feb 2019 16:39:53 +0000 (17:39 +0100)] 
MINOR: config: simplify bind_proc processing using proc_mask()

At a number of places we used to have null tests on bind_proc for
listeners and proxies. Let's simplify all these tests by always
having the proper bits reported via proc_mask().

6 years agoMINOR: global: add proc_mask() and thread_mask()
Willy Tarreau [Sat, 2 Feb 2019 16:22:19 +0000 (17:22 +0100)] 
MINOR: global: add proc_mask() and thread_mask()

These two functions return either all_{proc,threads}_mask, or the argument.
This is used to default to all_proc_mask or all_threads_mask when not set
on bind_conf or proxies.

6 years agoMINOR: config: keep an all_proc_mask like we have all_threads_mask
Willy Tarreau [Sat, 2 Feb 2019 16:11:28 +0000 (17:11 +0100)] 
MINOR: config: keep an all_proc_mask like we have all_threads_mask

This simplifies some mask comparisons at various places where
nbits(global.nbproc) was used.

6 years agoMINOR: tools: improve the popcount() operation
Willy Tarreau [Sat, 2 Feb 2019 19:17:31 +0000 (20:17 +0100)] 
MINOR: tools: improve the popcount() operation

We'll call popcount() more often so better use a parallel method
than an iterative one. One optimal design is proposed at the site
below. It requires a fast multiplication though, but even without
it will still be faster than the iterative one, and all relevant
64 bit platforms do have a multiply unit.

     https://graphics.stanford.edu/~seander/bithacks.html

6 years agoOPTIM: listener: optimize cache-line packing for struct listener
Willy Tarreau [Sun, 3 Feb 2019 09:28:24 +0000 (10:28 +0100)] 
OPTIM: listener: optimize cache-line packing for struct listener

Some unused fields were placed early and some important ones were on
the second cache line. Let's move the proto_list and name closer to
the end of the structure to bring accept() and default_target() into
the first cache line.

6 years agoCLEANUP: threads: use nbits to calculate the thread mask
Willy Tarreau [Sat, 2 Feb 2019 16:05:03 +0000 (17:05 +0100)] 
CLEANUP: threads: use nbits to calculate the thread mask

It's pointless to do arithmetics by hand, we have a function for this.

6 years agoCLEANUP: threads: fix misleading comment about all_threads_mask
Willy Tarreau [Sat, 2 Feb 2019 16:03:41 +0000 (17:03 +0100)] 
CLEANUP: threads: fix misleading comment about all_threads_mask

This variable changed a bit after 1.8, it's never zero anymore.

6 years agoBUG/MINOR: config: fix bind line thread mask validation
Willy Tarreau [Sat, 2 Feb 2019 16:46:24 +0000 (17:46 +0100)] 
BUG/MINOR: config: fix bind line thread mask validation

When no nbproc is specified, a computation leads to reading bind_thread[-1]
before checking if the thread mask is valid for a bind conf. It may either
report a false warning and compute a wrong mask, or miss some incorrect
configs.

This must be backported to 1.9 and possibly 1.8.

6 years agoBUG/MINOR: threads: fix the process range of thread masks
Willy Tarreau [Sat, 2 Feb 2019 12:18:01 +0000 (13:18 +0100)] 
BUG/MINOR: threads: fix the process range of thread masks

Commit 421f02e ("MINOR: threads: add a MAX_THREADS define instead of
LONGBITS") used a MAX_THREADS macros to fix threads limits. However,
one change was wrong as it affected the upper bound of the process
loop when setting threads masks. No backport is needed.

6 years agoBUG/MEDIUM: stream: Don't forget to free s->unique_id in stream_free().
Olivier Houchard [Fri, 1 Feb 2019 17:10:46 +0000 (18:10 +0100)] 
BUG/MEDIUM: stream: Don't forget to free s->unique_id in stream_free().

In stream_free(), free s->unique_id. We may still have one, because it's
allocated in log.c::strm_log() no matter what, even if it's a TCP connection
and thus it won't get free'd by http_end_txn().
Failure to do so leads to a memory leak.

This should probably be backported to all maintained branches.

6 years agoBUG/MEDIUM: mux-h2: always set :authority on request output
Willy Tarreau [Fri, 1 Feb 2019 15:13:59 +0000 (16:13 +0100)] 
BUG/MEDIUM: mux-h2: always set :authority on request output

PiBa-NL reported that some servers don't fall back to the Host header when
:authority is absent. After studying all the combinations of Host and
:authority, it appears that we always have to send the latter, hence we
never need the former. In case of CONNECT method, the authority is retrieved
from the URI part, otherwise it's extracted from the Host field.

The tricky part is that we have to scan all headers for the Host header
before dumping other headers. This is due to the fact that we must emit
pseudo headers before other ones. One improvement could possibly be made
later in the request parser to search and emit the Host header immediately
if authority was not found. This would cost nothing on the vast marjority
of requests and make the lookup faster on output since Host would appear
first.

This fix must be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h2: always omit :scheme and :path for the CONNECT method
Willy Tarreau [Fri, 1 Feb 2019 14:51:59 +0000 (15:51 +0100)] 
BUG/MEDIUM: mux-h2: always omit :scheme and :path for the CONNECT method

This is mandated by RFC7540 #8.3, these pseudo-headers must not be emitted
with the CONNECT method.

This must be backported to 1.9.

6 years agoBUG/MINOR: backend: check srv_conn before dereferencing it
Willy Tarreau [Fri, 1 Feb 2019 15:38:48 +0000 (16:38 +0100)] 
BUG/MINOR: backend: check srv_conn before dereferencing it

Commit 3c4e19f42 ("BUG/MEDIUM: backend: always release the previous
connection into its own target srv_list") introduced a valid warning
about a null-deref risk since we didn't check conn_new()'s return value.

This patch must be backported to 1.9 with the patch above.

6 years agoBUG/MINOR: tune.fail-alloc: Don't forget to initialize ret.
Olivier Houchard [Fri, 1 Feb 2019 15:28:04 +0000 (16:28 +0100)] 
BUG/MINOR: tune.fail-alloc: Don't forget to initialize ret.

In mem_should_fail(), if we don't want to fail the allocation, either
because mem_fail_rate is 0, or because we're still initializing, don't
forget to initialize ret, or we may return a non-zero value, and fail
an allocation we didn't want to fail.
This should only affect users that compile with DEBUG_FAIL_ALLOC.

6 years agoBUG/MEDIUM: htx: check the HTX compatibility in dynamic use-backend rules
Willy Tarreau [Fri, 1 Feb 2019 14:06:09 +0000 (15:06 +0100)] 
BUG/MEDIUM: htx: check the HTX compatibility in dynamic use-backend rules

I would have sworn it was done, probably we lost it during the refactoring.
If a frontend is in HTX and the backend not (and conersely), this is
normally detected at config parsing time unless the rule is dynamic. In
this case we must abort with an error 500. The logs will report "RR"
(resource issue while processing request) with the frontend and the
backend assigned, so that it's possible to figure what was attempted.

This must be backported to 1.9.

6 years agoBUG/MEDIUM: backend: always release the previous connection into its own target srv_list
Willy Tarreau [Fri, 1 Feb 2019 10:54:23 +0000 (11:54 +0100)] 
BUG/MEDIUM: backend: always release the previous connection into its own target srv_list

There was a bug reported in issue #19 regarding the fact that haproxy
could mis-route requests to the wrong server. It turns out that when
switching to another server, the old connection was put back into the
srv_list corresponding to the stream's target instead of this connection's
target. Thus if this connection was later picked, it was pointing to the
wrong server.

The patch fixes this and also clarifies the assignment to srv_conn->target
so that it's clear we don't change it when picking it from the srv_list.

This must be backported to 1.9 only.

6 years agoMINOR: debug: Add an option that causes random allocation failures.
Olivier Houchard [Tue, 29 Jan 2019 14:20:16 +0000 (15:20 +0100)] 
MINOR: debug: Add an option that causes random allocation failures.

When compiling with DEBUG_FAIL_ALLOC, add a new option, tune.fail-alloc,
that gives the percentage of chances an allocation fails.
This is useful to check that allocation failures are always handled
gracefully.

6 years agoMINOR: muxes: Don't bother to LIST_DEL(&conn->list) before calling conn_free().
Olivier Houchard [Fri, 18 Jan 2019 16:25:29 +0000 (17:25 +0100)] 
MINOR: muxes: Don't bother to LIST_DEL(&conn->list) before calling conn_free().

conn_free() already removes the connection from any idle list, so there's no
need to do it in the mux code, just before calling conn_free().

6 years agoMINOR: xref: Add missing barriers.
Olivier Houchard [Fri, 18 Jan 2019 16:21:32 +0000 (17:21 +0100)] 
MINOR: xref: Add missing barriers.

Add a few missing barriers in the xref code, it's unlikely to be a problem
for x86, but may be on architectures with weak memory ordering.

6 years agoBUG/MEDIUM: mux-h2: properly consider the peer's advertised max-concurrent-streams
Willy Tarreau [Thu, 31 Jan 2019 09:42:05 +0000 (10:42 +0100)] 
BUG/MEDIUM: mux-h2: properly consider the peer's advertised max-concurrent-streams

Till now we used to only rely on tune.h2.max-concurrent-streams but if
a peer advertises a lower limit this can cause streams to be reset or
even the conection to be killed. Let's respect the peer's value for
outgoing streams.

This patch should be backported to 1.9, though it depends on the following
ones :
    BUG/MINOR: server: fix logic flaw in idle connection list management
    MINOR: mux-h2: max-concurrent-streams should be unsigned
    MINOR: mux-h2: make sure to only check concurrency limit on the frontend
    MINOR: mux-h2: learn and store the peer's advertised MAX_CONCURRENT_STREAMS setting

6 years agoMINOR: mux-h2: learn and store the peer's advertised MAX_CONCURRENT_STREAMS setting
Willy Tarreau [Thu, 31 Jan 2019 09:34:07 +0000 (10:34 +0100)] 
MINOR: mux-h2: learn and store the peer's advertised MAX_CONCURRENT_STREAMS setting

We used not to take it into account because we only used the configured
parameter everywhere. This patch makes sure we can actually learn the
value advertised by the peer. We still enforce our own limit on top of
it however, to make sure we can actually limit resources or stream
concurrency in case of suboptimal server settings.

6 years agoMINOR: mux-h2: make sure to only check concurrency limit on the frontend
Willy Tarreau [Thu, 31 Jan 2019 09:31:51 +0000 (10:31 +0100)] 
MINOR: mux-h2: make sure to only check concurrency limit on the frontend

h2_has_too_many_cs() was renamed to h2_frt_has_too_many_cs() to make it
clear it's only used to throttle the frontend connection, and the call
places were adjusted to only call this code on a front connection. In
practice it was already the case since the H2_CF_DEM_TOOMANY flag is
only set there. But now the ambiguity is removed.

6 years agoMINOR: mux-h2: max-concurrent-streams should be unsigned
Willy Tarreau [Thu, 31 Jan 2019 09:39:51 +0000 (10:39 +0100)] 
MINOR: mux-h2: max-concurrent-streams should be unsigned

We compare it to other unsigned values, let's make it unsigned as well.

6 years agoBUG/MINOR: server: fix logic flaw in idle connection list management
Willy Tarreau [Sat, 26 Jan 2019 11:19:01 +0000 (12:19 +0100)] 
BUG/MINOR: server: fix logic flaw in idle connection list management

With variable connection limits, it's not possible to accurately determine
whether the mux is still in use by comparing usage and max to be equal due
to the fact that one determines the capacity and the other one takes care
of the context. This can cause some connections to be dropped before they
reach their stream ID limit.

It seems it could also cause some connections to be terminated with
streams still alive if the limit was reduced to match the newly computed
avail_streams() value, though this cannot yet happen with existing muxes.

Instead let's switch to usage reports and simply check whether connections
are both unused and available before adding them to the idle list.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h2: do not close the connection on aborted streams
Willy Tarreau [Thu, 31 Jan 2019 18:12:48 +0000 (19:12 +0100)] 
BUG/MEDIUM: mux-h2: do not close the connection on aborted streams

We used to rely on a hint that a shutw() or shutr() without data is an
indication that the upper layer had performed a tcp-request content reject
and really wanted to kill the connection, but sadly there is another
situation where this happens, which is failed keep-alive request to a
server. In this case the upper layer stream silently closes to let the
client retry. In our case this had the side effect of killing all the
connection.

Instead of relying on such hints, let's address the problem differently
and rely on information passed by the upper layers about the intent to
kill the connection. During shutr/shutw, this is detected because the
flag CS_FL_KILL_CONN is set on the connstream. Then only in this case
we send a GOAWAY(ENHANCE_YOUR_CALM), otherwise we only send the reset.

This makes sure that failed backend requests only fail frontend requests
and not the whole connections anymore.

This fix relies on the two previous patches adding SI_FL_KILL_CONN and
CS_FL_KILL_CONN as well as the fix for the connection close, and it must
be backported to 1.9 and 1.8, though the code in 1.8 could slightly differ
(cs is always valid) :

  BUG/MEDIUM: mux-h2: wait for the mux buffer to be empty before closing the connection
  MINOR: stream-int: add a new flag to mention that we want the connection to be killed
  MINOR: connstream: have a new flag CS_FL_KILL_CONN to kill a connection

6 years agoMINOR: connstream: have a new flag CS_FL_KILL_CONN to kill a connection
Willy Tarreau [Thu, 31 Jan 2019 18:09:59 +0000 (19:09 +0100)] 
MINOR: connstream: have a new flag CS_FL_KILL_CONN to kill a connection

This is the equivalent of SI_FL_KILL_CONN but for the connstreams. It
will be set by the stream-interface during the various shutdown
operations.

6 years agoMINOR: stream-int: add a new flag to mention that we want the connection to be killed
Willy Tarreau [Thu, 31 Jan 2019 18:02:43 +0000 (19:02 +0100)] 
MINOR: stream-int: add a new flag to mention that we want the connection to be killed

The new flag SI_FL_KILL_CONN is now set by the rare actions which
deliberately want the whole connection (and not just the stream) to be
killed. This is only used for "tcp-request content reject",
"tcp-response content reject", "tcp-response content close" and
"http-request reject". The purpose is to desambiguate the close from
a regular shutdown. This will be used by the next patches.

6 years agoBUG/MEDIUM: mux-h2: wait for the mux buffer to be empty before closing the connection
Willy Tarreau [Thu, 31 Jan 2019 17:48:20 +0000 (18:48 +0100)] 
BUG/MEDIUM: mux-h2: wait for the mux buffer to be empty before closing the connection

When finishing to respond on a stream, a shutw() is called (resulting
in either an end of stream or RST), then h2_detach() is called, and may
decide to kill the connection is a number of conditions are satisfied.
Actually one of these conditions is that a GOAWAY frame was already sent
or attempted to be sent. This one is wrong, because it can happen in at
least these two situations :
  - a shutw() sends a GOAWAY to obey tcp-request content reject
  - a graceful shutdown is pending

In both cases, the connection will be aborted with the mux buffer holding
some data. In case of a strong abort the client will not see the GOAWAY or
RST and might want to try again, which is counter-productive. In case of
the graceful shutdown, it could result in truncated data. It looks like a
valid candidate for the issue reported here :

    https://www.mail-archive.com/haproxy@formilux.org/msg32433.html

A backport to 1.9 and 1.8 is necessary.

6 years agoBUG/MINOR: stream: don't close the front connection when facing a backend error
Willy Tarreau [Thu, 31 Jan 2019 17:58:06 +0000 (18:58 +0100)] 
BUG/MINOR: stream: don't close the front connection when facing a backend error

In 1.5-dev13, a bug was introduced by commit e3224e870 ("BUG/MINOR:
session: ensure that we don't retry connection if some data were sent").
If a connection error is reported after some data were sent (and lost),
we used to accidently mark the front connection as being in error instead
of only the back one because the two direction flags were applied to the
same channel. This case is extremely rare with raw connections but can
happen a bit more often with multiplexed streams. This will result in
the error not being correctly reported to the client.

This patch can be backported to all supported versions.

6 years agoBUG/MEDIUM: connections: Don't forget to remove CO_FL_SESS_IDLE.
Olivier Houchard [Thu, 31 Jan 2019 18:31:19 +0000 (19:31 +0100)] 
BUG/MEDIUM: connections: Don't forget to remove CO_FL_SESS_IDLE.

If we're adding a connection to the server orphan idle list, don't forget
to remove the CO_FL_SESS_IDLE flag, or we will assume later it's still
attached to a session.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h1: Don't add "transfer-encoding" if message-body is forbidden
Christopher Faulet [Wed, 30 Jan 2019 20:55:21 +0000 (21:55 +0100)] 
BUG/MEDIUM: mux-h1: Don't add "transfer-encoding" if message-body is forbidden

When a HTTP/1.1 or above response is emitted to a client, if the flag
H1_MF_XFER_LEN is set whereas H1_MF_CLEN and H1_MF_CHNK are not, the header
"transfer-encoding" is added. It is a way to make HTX chunking consistent with
H2. But we must exclude all cases where the message-body is explicitly forbidden
by the RFC:

 * for all 1XX, 204 and 304 responses
 * for any responses to HEAD requests
 * for 200 responses to CONNECT requests

For these 3 cases, the flag H1_MF_XFER_LEN is set but H1_MF_CLEN and H1_MF_CHNK
not. And the header "transfer-encoding" must not be added.

See issue #27 on github for details about the bug.

This patch must be backported in 1.9.

6 years agoBUG/MEDIUM: peers: Peer addresses parsing broken.
Frédéric Lécaille [Thu, 31 Jan 2019 05:48:16 +0000 (06:48 +0100)] 
BUG/MEDIUM: peers: Peer addresses parsing broken.

This bug was introduced by 355b203 commit which prevented the peer
addresses to be parsed for the local peer of a "peers" section.
When adding "parse_addr" boolean parameter to parse_server(), this commit
missed the case where the syntax with "peer" keyword should still be
supported in addition to the new syntax with "server"+"bind" keyword.

May be backported as fas as 1.5.

6 years agoMINOR: mux-h2: consistently rely on the htx variable to detect the mode
Willy Tarreau [Thu, 31 Jan 2019 06:23:00 +0000 (07:23 +0100)] 
MINOR: mux-h2: consistently rely on the htx variable to detect the mode

In h2_frt_transfer_data(), we support both HTX and legacy modes. The
HTX mode is detected from the proxy option and sets a valid pointer
into the htx variable. Better rely on this variable in all the function
rather than testing the option again. This way the code is clearer and
even the compiler knows this pointer is valid when it's dereferenced.

This should be backported to 1.9 if the b_is_null() patch is backported.

6 years agoMINOR: htx: never check for null htx pointer in htx_is_{,not_}empty()
Willy Tarreau [Thu, 31 Jan 2019 06:31:18 +0000 (07:31 +0100)] 
MINOR: htx: never check for null htx pointer in htx_is_{,not_}empty()

The previous patch clarifies the fact that the htx pointer is never null
along all the code. This test for a null will never match, didn't catch
the pointer 1 before the fix for b_is_null(), but it confuses the compiler
letting it think that any dereferences made to this pointer after this
test could actually mean we're dereferencing a null. Let's now drop this
test. This saves us from having to add impossible tests everywhere to
avoid the warning.

This should be backported to 1.9 if the b_is_null() patch is backported.

6 years agoDOC: htx: make it clear that htxbuf() and htx_from_buf() always return valid pointers
Willy Tarreau [Thu, 31 Jan 2019 06:21:42 +0000 (07:21 +0100)] 
DOC: htx: make it clear that htxbuf() and htx_from_buf() always return valid pointers

Update the comments above htxbuf() and htx_from_buf() to make it clear
that they always return valid htx pointers so that callers know they do
not have to test them. This is only true after the fix on b_is_null()
which was the only known corner case.

This should be backported to 1.9 if the b_is_null() patch is backported.

6 years agoBUG/MEDIUM: buffer: Make sure b_is_null handles buffers waiting for allocation.
Olivier Houchard [Tue, 29 Jan 2019 18:10:02 +0000 (19:10 +0100)] 
BUG/MEDIUM: buffer: Make sure b_is_null handles buffers waiting for allocation.

In b_is_null(), make sure we return 1 if the buffer is waiting for its
allocation, as users assume there's memory allocated if b_is_null() returns
0.

The indirect impact of not having this was that htxbuf() would not match
b_is_null() for a buffer waiting for an allocation, and would thus return
the value 1 for the htx pointer, causing various crashes under low memory
condition.

Note that this patch makes gcc versions 6 and above report two null-deref
warnings in proto_htx.c since htx_is_empty() continues to check for a null
pointer without knowing that this is protected by the test on b_is_null().
This is addressed by the following patches.

This should be backported to 1.9.

6 years agoDOC: compression: Update the reasons for disabled compression
Tim Duesterhus [Wed, 30 Jan 2019 22:46:04 +0000 (23:46 +0100)] 
DOC: compression: Update the reasons for disabled compression

- Update the list of status codes to include 201 - 203.
- Remove the fact about the temporary workaround for chunked responses
  (this is verified using reg-test compression/h00000.vtc).
- Add malformed ETags

see b229f018eedef4d18571ce6da23d8e153249a836

This commit should be backported together with b229f018eedef4d18571ce6da23d8e153249a836
the changes should be correct until 1.7 at the very least, possibly older.

6 years agoBUG/MINOR: mux-h2: make sure request trailers on aborted streams don't break the...
Willy Tarreau [Wed, 30 Jan 2019 10:44:07 +0000 (11:44 +0100)] 
BUG/MINOR: mux-h2: make sure request trailers on aborted streams don't break the connection

We used to respond a connection error in case we received a trailers
frame on a closed stream, but it's a problem to do this if the error
was caused by a reset because the sender has not yet received it and
is just a victim of the timing. Thus we must not close the connection
in this case.

This patch may be backported to 1.9 but then it requires the following
previous ones :
   MINOR: h2: add a generic frame checker
   MEDIUM: mux-h2: check the frame validity before considering the stream state
   CLEANUP: mux-h2: remove stream ID and frame length checks from the frame parsers

6 years agoCLEANUP: mux-h2: remove stream ID and frame length checks from the frame parsers
Willy Tarreau [Wed, 30 Jan 2019 14:39:55 +0000 (15:39 +0100)] 
CLEANUP: mux-h2: remove stream ID and frame length checks from the frame parsers

It's not convenient to have such structural checks mixed with the ones
related to the stream state. Let's remove all these basic tests that are
already covered once for all when reading the frame header.

6 years agoMEDIUM: mux-h2: check the frame validity before considering the stream state
Willy Tarreau [Wed, 30 Jan 2019 14:11:03 +0000 (15:11 +0100)] 
MEDIUM: mux-h2: check the frame validity before considering the stream state

There are some uneasy situation where it's difficult to validate a frame's
format without being in an appropriate state. This patch makes sure that
each frame passes through h2_frame_check() before being checked in the
context of the stream's state. This makes sure we can always return a GOAWAY
for protocol violations even if we can't process the frame.

6 years agoMINOR: h2: add a generic frame checker
Willy Tarreau [Wed, 30 Jan 2019 14:09:21 +0000 (15:09 +0100)] 
MINOR: h2: add a generic frame checker

The new function h2_frame_check() checks the protocol limits for the
received frame (length, ID, direction) and returns a verdict made of
a connection error code. The purpose is to be able to validate any
frame regardless of the state and the ability to call the frame handler,
and to emit a GOAWAY early in this case.

6 years agoBUG/MINOR: mux-h2: make sure response HEADERS are not received in other states than...
Willy Tarreau [Wed, 30 Jan 2019 15:55:48 +0000 (16:55 +0100)] 
BUG/MINOR: mux-h2: make sure response HEADERS are not received in other states than OPEN and HLOC

RFC7540#5.1 states that these are the only states allowing any frame
type. For response HEADERS frames, we cannot accept that they are
delivered on idle streams of course, so we're left with these two
states only. It is important to test this so that we can remove the
generic CLOSE_STREAM test for such frames in the main loop.

This must be backported to 1.9 (1.8 doesn't have response HEADERS).

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).