Commit Graph

8039 Commits

Author SHA1 Message Date
Frédéric Lécaille
afbfa2a23f REGTEST/MINOR: Add a reg testing file for b406b87 commit. 2018-08-24 16:53:27 +02:00
Willy Tarreau
ad7f0ad1c3 BUG/MEDIUM: mux_pt: dereference the connection with care in mux_pt_wake()
mux_pt_wake() calls data->wake() which can return -1 indicating that the
connection was just destroyed. We need to check for this condition and
immediately exit in this case otherwise we dereference a just freed
connection. Note that this mainly happens on idle connections between
two HTTP requests. It can have random implications between requests as
it may lead a wrong connection's polling to be re-enabled or disabled
for example, especially with threads.

This patch must be backported to 1.8.
2018-08-24 15:48:59 +02:00
Frédéric Lécaille
b083c8316b REGEST/MINOR: Add reg testing files.
Reg testing files for a LUA bug fixed by commit 83ed5d5 ("BUG/MINOR:
lua: Bad HTTP client request duration.")
2018-08-24 14:50:30 +02:00
Frédéric Lécaille
83ed5d58d2 BUG/MINOR: lua: Bad HTTP client request duration.
HTTP LUA applet callback should not update the date on which the HTTP client requests
arrive. This was done just after the LUA applet has completed its job.

This patch simply removes the affected statement. The same fixe has been applied
to TCP LUA applet callback.

To reproduce this issue, as reported by Patrick Hemmer, implement an HTTP LUA applet
which sleeps a bit before replying:

  core.register_service("foo", "http", function(applet)
      core.msleep(100)
      applet:set_status(200)
      applet:start_response()
  end)

This had as a consequence to log %TR field with approximatively the same value as
the LUA sleep time.

Thank you to Patrick Hemmer for having reported this issue.

Must be backported to 1.8, 1.7 and 1.6.
2018-08-24 14:49:30 +02:00
Willy Tarreau
e215bba956 MINOR: connection: make conn_sock_drain() work for all socket families
This patch improves the previous fix by implementing the socket draining
code directly in conn_sock_drain() so that it always applies regardless
of the protocol's family. Thus it gets rid of tcp_drain().
2018-08-24 14:45:46 +02:00
Willy Tarreau
fe5d2ac65f BUG/MEDIUM: unix: provide a ->drain() function
Right now conn_sock_drain() calls the protocol's ->drain() function if
it exists, otherwise it simply tries to disable polling for receiving
on the connection. This doesn't work well anymore since we've implemented
the muxes in 1.8, and it has a side effect with keep-alive backend
connections established over unix sockets. What happens is that if
during the idle time after a request, a connection reports some data,
si_idle_conn_null_cb() is called, which will call conn_sock_drain().
This one sees there's no drain() on unix sockets and will simply disable
polling for data on the connection. But it doesn't do anything on the
conn_stream. Thus while leaving the conn_fd_handler, the mux's polling
is updated and recomputed based on the conn_stream's polling state,
which is still enabled, and nothing changes, so we see the process
use 100% CPU in this case because the FD remains active in the cache.

There are several issues that need to be addressed here. The first and
most important is that we cannot expect some protocols to simply stop
reading data when asked to drain pending data. So this patch make the
unix sockets rely on tcp_drain() since the functions are the same. This
solution is appropriate for backporting, but a better one is desired for
the long term. The second issue is that si_idle_conn_null_cb() shouldn't
drain the connection but the conn_stream.

At the moment we don't have any way to drain a conn_stream, though a flag
on rcv_buf() will do it well. Until we support muxes on the server side
it is not a problem so this part can be addressed later.

This fix must be backported to 1.8.
2018-08-24 14:42:50 +02:00
Frédéric Lécaille
9931634e71 REGTEST/MINOR: Add a new class of regression testing files.
Add LEVEL #4 regression testing files which is dedicated to
VTC files in relation with bugs they help to reproduce.
At the date of this commit, all VTC files are LEVEL 4 VTC files.
2018-08-23 15:47:10 +02:00
Frédéric Lécaille
fe5463ebf2 REGTEST/MINOR: Missing mandatory "ignore_unknown_macro".
Since bbc34e2 varnish commit (for varnishtest), a new "cli"
macro is automatically created for each VTC script to dialog with
the CLI. Consequently, as this macro is unknown from higher level
code for varnishtest, it makes the scripts fail if we
we do not ask varnishtest to disregard the unknown macros.
To prevent this, from now on, for each VTC file for haproxy we MUST add
"feature ignore_unknown_macro" line to do so. This is mandatory
2018-08-23 15:47:07 +02:00
Jens Bissinger
15c64ff4fb DOC: Fix spelling error in configuration doc
Fix spelling error in logging section of configuration doc.
2018-08-23 15:46:38 +02:00
Willy Tarreau
b509232eb8 MINOR: sample: remove impossible tests on negative smp->data.u.str.data
Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can remove
negative size checks.
2018-08-22 05:28:33 +02:00
Willy Tarreau
bba81563cf MINOR: chunk: remove impossible tests on negative chunk->data
Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can remove
negative size checks.
2018-08-22 05:28:32 +02:00
Willy Tarreau
1c913e4232 BUG/MEDIUM: cli/ssl: don't store base64dec() result in the trash's length
By convenience or laziness we used to store base64dec()'s return code
into trash.data and to compare it against 0 to check for conversion
failure, but it's now unsigned since commit 843b7cb ("MEDIUM: chunks:
make the chunk struct's fields match the buffer struct"). Let's clean
this up and test the result itself without storing it first.

No backport is needed.
2018-08-22 05:28:32 +02:00
Willy Tarreau
b406b8708f BUG/MEDIUM: connection: don't store recv() result into trash.data
Cyril Bonté discovered that the proxy protocol randomly fails since
commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields match
the buffer struct"). This is because we used to store recv()'s return
code into trash.data which is now unsigned, so it never compares as
negative against 0. Let's clean this up and test the result itself
without storing it first.

No backport is needed.
2018-08-22 05:28:32 +02:00
Willy Tarreau
2842e05c7c BUG/MEDIUM: map: don't store exp_replace() result in the trash's length
By convenience or laziness we used to store exp_replace()'s return code
into str->data. The result checks applied there compare str->data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct"). Let's clean this up
and test the result itself without storing it first.

No backport is needed.
2018-08-22 05:16:33 +02:00
Willy Tarreau
f6ee9dc616 BUG/MEDIUM: dns: don't store dns_build_query() result in the trash's length
By convenience or laziness we used to store dns_build_query()'s return code
into trash.data. The result checks applied there compare trash.data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct"). Let's clean this up
and test the result itself without storing it first.

No backport is needed.
2018-08-22 05:16:32 +02:00
Willy Tarreau
9c768fdca1 BUG/MEDIUM: http: don't store url_decode() result in the samples's length
By convenience or laziness we used to store url_decode()'s return code
into smp->data.u.str.data. The result checks applied there compare it
to 0 while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make
the chunk struct's fields match the buffer struct "). Let's clean this up
and test the result itself without storing it first.

No backport is needed.
2018-08-22 05:16:32 +02:00
Willy Tarreau
6e27be1a5d BUG/MEDIUM: http: don't store exp_replace() result in the trash's length
By convenience or laziness we used to store exp_replace()'s return code
into trash.data. The result checks applied there compare trash.data to -1
while it's now unsigned since commit 843b7cb ("MEDIUM: chunks: make the
chunk struct's fields match the buffer struct "). Let's clean this up
and test the result itself without storing it first.

No backport is needed.
2018-08-22 05:16:32 +02:00
Willy Tarreau
5f6333caca BUG/MINOR: chunks: do not store -1 into chunk_printf() in case of error
Since commit 843b7cb ("MEDIUM: chunks: make the chunk struct's fields
match the buffer struct") a chunk length is unsigned so we can't reliably
store -1 and check for negative values in the caller. Only one such
location was found in proto_http's http-request auth rules (which cannot
realistically fail).

No backport is needed.
2018-08-22 05:16:31 +02:00
Willy Tarreau
49725a0977 BUG/MEDIUM: check/threads: do not involve the rendez-vous point for status updates
thread_isolate() is currently being called with the server lock held.
This is not acceptable because it prevents other threads from reaching
the rendez-vous point. Now that the LB algos are thread-safe, let's get
rid of this call.

No backport is nedeed.
2018-08-21 19:54:09 +02:00
Willy Tarreau
1b87748ff5 BUG/MEDIUM: lb/threads: always properly lock LB algorithms on maintenance operations
Since commit 3ff577e ("MAJOR: server: make server state changes
synchronous again"), srv_update_status() calls the various maintenance
operations of the LB algorithms (->set_server_up, ->set_server_down,
->update_server_weight()). These ones are called with a single thread
guaranteed by the rendez-vous point, so the fact that they're lacking
some locks has no effect. However we'll need to remove the rendez-vous
point so we have to take care of properly locking all the LB algos.

The comments have been properly updated on the various functions to
mention their locking expectations. All these functions are called
with the server lock held, and all of them now support concurrent
calls by using the lbprm's lock.

This fix doesn't need to be backported at the moment, though if any
check-specific issue surfaced in 1.8, it could make sense to reuse it.
2018-08-21 19:44:53 +02:00
Willy Tarreau
1b13bfd646 BUG/MEDIUM: connection: don't forget to always delete the list's head
During a test it happened that a connection was deleted before the
stream it's attached to, resulting in a crash related to the fix
18a85fe ("BUG/MEDIUM: streams: Don't forget to remove the si from
the wait list.") during the LIST_DEL(). Make sure to always delete
the list's head in this case so that other elements can safely
detach later.

This is purely 1.9, no backport is needed.
2018-08-21 18:33:20 +02:00
Willy Tarreau
deca26c452 BUG/MAJOR: queue/threads: make pendconn_redistribute not lock the server
Since commit 3ff577e ("MAJOR: server: make server state changes
synchronous again"), srv_update_status() is called with the server
lock held. It calls (among others) pendconn_redistribute() which used
to take this lock, causing CPU loops by default, or crashes if build
with -DDEBUG_THREAD. Since this function is not called from any other
place anymore, it doesn't require the lock on its own so let's simply
drop it from there.

No backport is needed, this is 1.9-specific.
2018-08-21 18:11:03 +02:00
Olivier Houchard
80c56790d9 BUG/MEDIUM: stream_interface: Call the wake callback after sending.
If we subscribed to send, and the callback is called, call the wake callback
after, so that process_stream() may be woken up if needed.

This is 1.9-specific, no backport is needed.
2018-08-21 18:06:57 +02:00
Olivier Houchard
fab7c7e91c BUG/MEDIUM: H2: Activate polling after successful h2_snd_buf().
Make sure h2_send() is called after h2_snd_buf() by activating polling.

This is 1.9-specific, no backport is needed.
2018-08-21 18:06:57 +02:00
Olivier Houchard
a6ff035770 BUG/MEDIUM: stream-int: Check if the conn_stream exist in si_cs_io_cb.
It is possible that the conn_stream gets detached from the stream_interface,
and as it subscribed to the wait list, si_cs_io_cb() gets called anyway,
so make sure we have a conn_stream before attempting to send more data.

This is 1.9-specific, no backport is needed.
2018-08-21 18:06:54 +02:00
Olivier Houchard
abedf5f6c3 BUG/MEDIUM: tasklets: Add the thread as active when waking a tasklet.
Set the flag for the current thread in active_threads_mask when waking a
tasklet, or we will never run it if no tasks are available.

This is 1.9-specific, no backport is needed.
2018-08-21 18:06:33 +02:00
Olivier Houchard
18a85fe602 BUG/MEDIUM: streams: Don't forget to remove the si from the wait list.
When freeing the stream, make sure we remove the stream interfaces from the
wait lists, in case it was in there.

This is 1.9-specific, no backport is needed.
2018-08-21 18:06:33 +02:00
Willy Tarreau
3bcc2699ba BUG/MEDIUM: cli/threads: protect some server commands against concurrent operations
The server-specific CLI commands "set weight", "set maxconn",
"disable agent", "enable agent", "disable health", "enable health",
"disable server" and "enable server" were not protected against
concurrent accesses. Now they take the server lock around the
sensitive part.

This patch must be backported to 1.8.
2018-08-21 15:35:31 +02:00
Willy Tarreau
46b7f53ad9 DOC: server/threads: document which functions need to be called with/without locks
At the moment it's totally unclear while reading the server's code which
functions require to be called with the server lock held and which ones
grab it and cannot be called this way. This commit simply inventories
all of them to indicate what is detected depending on how these functions
use the struct server. Only functions used at runtime were checked, those
dedicated to config parsing were skipped. Doing so already has uncovered
a few bugs on some CLI actions.
2018-08-21 14:58:25 +02:00
Willy Tarreau
a275a3710e BUG/MEDIUM: cli/threads: protect all "proxy" commands against concurrent updates
The proxy-related commands like "{enable|disable|shutdown} frontend",
"{enable|disable} dynamic-cookie", "set dynamic-cookie-key" were not
protected against concurrent accesses making their use dangerous with
threads.

This patch must be backported to 1.8.
2018-08-21 14:58:25 +02:00
Willy Tarreau
eeba36b3af BUG/MEDIUM: server: update our local state before propagating changes
Commit 3ff577e ("MAJOR: server: make server state changes synchronous again")
reintroduced synchronous server state changes. However, during the previous
change from synchronous to asynchronous, the server state propagation was
placed at the end of the function to ease the code changes, and the commit
above didn't put it back at its place. This has resulted in propagated
states to be incomplete. For example, making a server leave maintenance
would make it up but would leave its tracking servers down because they
see their tracked server is still down.

Let's just move the status update right to its place. It also adds the
benefit of reporting state changes in the order they appear and not in
reverse.

No backport is needed.
2018-08-21 08:29:25 +02:00
Cyril Bonté
7ee465f1ad BUG/MINOR: lua: fix extra 500ms added to socket timeouts
Since commit #56cc12509, haproxy accepts double values for timeouts. The
value is then converted to  milliseconds before being rounded up and cast
to int. The issue is that to round up the value, a constant value of 0.5
is added to it, but too early in the conversion, resulting in an
additional 500ms to the value. We are talking about a precision of 1ms,
so we can safely get rid of this rounding trick and adjust resulting
timeouts equal to 0 to a minimum of 1ms.

This patch is specific to the 1.9 branch and doesn't require to be
backported.
2018-08-19 22:11:28 +02:00
Cyril Bonté
7bb6345497 BUG/MEDIUM: lua: socket timeouts are not applied
Sachin Shetty reported that socket timeouts set in LUA code have no effect.
Indeed, connect timeout is never modified and is always set to its default,
set to 5 seconds. Currently, this patch will apply the specified timeout
value to the connect timeout.
For the read and write timeouts, the issue is that the timeout is updated but
the expiration dates were not updated.

This patch should be backported up to the 1.6 branch.
2018-08-18 00:23:52 +02:00
Olivier Houchard
6aab737835 MINOR: fd cache: And the thread_mask with all_threads_mask.
When we choose to insert a fd in either the global or the local fd update list,
and the thread_mask against all_threads_mask before checking if it's tid_bit,
that way, if we run with nbthreads==1, we will always use the local list,
which is cheaper than the global one.
2018-08-17 14:50:47 +02:00
Olivier Houchard
19bdf2428d MINOR: tasks: Don't special-case when nbthreads == 1
Instead of checking if nbthreads == 1, just and thread_mask with
all_threads_mask to know if we're supposed to add the task to the local or
the global runqueue.
2018-08-17 14:50:37 +02:00
Willy Tarreau
f7e3955053 DOC: update the layering design notes
Explain the change around cs_recv()/cs_send() and the move of the CS'
rxbuf and txbuf to the mux.
2018-08-17 09:58:29 +02:00
Bertrand Jacquin
a25282bb39 DOC: ssl: Use consistent naming for TLS protocols
In most cases, "TLSv1.x" naming is used across and documentation, lazy
people tend to grep too much and may not find what they are looking for.

Fixing people is hard.
2018-08-16 20:20:26 +02:00
Patrick Hemmer
fabb24f92c DOC: add documentation for prio_class and prio_offset sample fetches.
This adds documentation that was missed as part of 268a707.
2018-08-16 20:19:17 +02:00
Lukas Tribus
c5dd5a500a DOC: dns: explain set server ... fqdn requires resolver
Abhishek Gupta reported on discourse that set server [...] fqdn always
fails. Further investigation showed that this requires the internal
DNS resolver to be configured. Add this requirement to the docs.

Must be backported to 1.8.
2018-08-16 19:54:15 +02:00
Emeric Brun
271022150d BUG/MINOR: map: fix map_regm with backref
Due to a cascade of get_trash_chunk calls the sample is
corrupted when we want to read it.

The fix consist to use a temporary chunk to copy the sample
value and use it.

[wt: for 1.8 and older, a backport was successfully tested here :
 https://www.mail-archive.com/haproxy@formilux.org/msg30694.html]
2018-08-16 19:44:04 +02:00
Emeric Brun
e1b4ed4352 BUG/MEDIUM: ssl: loading dh param from certifile causes unpredictable error.
If the dh parameter is not found, the openssl's error global
stack was not correctly cleared causing unpredictable error
during the following parsing (chain cert parsing for instance).

This patch should be backported in 1.8 (and perhaps 1.7)
2018-08-16 19:36:08 +02:00
Emeric Brun
eb155b6ca6 BUG/MEDIUM: ssl: fix missing error loading a keytype cert from a bundle.
If there was an issue loading a keytype's part of a bundle, the bundle
was implicitly ignored without errors.

This patch should be backported in 1.8 (and perhaps 1.7)
2018-08-16 19:36:06 +02:00
Olivier Houchard
fde2a09a15 BUG/MEDIUM: sessions: Don't use t->state.
In session_expire_embryonic(), don't use t->state, use the "state" argument
instead, as t->state has been cleaned before we're being called.
2018-08-16 19:25:56 +02:00
Olivier Houchard
d8b7a4701d BUG/MEDIUM: tasks: Don't insert in the global rqueue if nbthread == 1
Make sure we don't insert a task in the global run queue if nbthread == 1,
as, as an optimisation, we avoid reading from it if nbthread == 1.
2018-08-16 19:25:46 +02:00
Olivier Houchard
5c110b924e MINOR: checks: Add event_srv_chk_io().
In checks, introduce event_srv_chk_io() as a callback to be called if data
can be sent again, instead of abusing event_srv_chk_w.
2018-08-16 17:29:54 +02:00
Olivier Houchard
29fb89dc5e MINOR: mux_h2: Don't use h2_send() as a callback.
Instead of using h2_send() directly as a callback, introcude h2_io_cb(), that
will call h2_send() if it is possible to send data.
2018-08-16 17:29:54 +02:00
Olivier Houchard
8f0b4c66f5 MINOR: stream_interface: Give stream_interface its own wait_list.
Instead of just using the conn_stream wait_list, give the stream_interface
its own. When the conn_stream will have its own buffers, the stream_interface
may have to wait on it.
2018-08-16 17:29:54 +02:00
Olivier Houchard
91894cbf4c MINOR: stream_interface: Don't use si_cs_send() as a task handler.
Instead of using si_cs_send() as a task handler, define a new function,
si_cs_io_cb(), and give si_cs_send() its original prototype. Right now
si_cs_io_cb() just handles send, but later it'll handle recv() too.
2018-08-16 17:29:54 +02:00
Olivier Houchard
e1c6dbcd70 MINOR: connections/mux: Add the wait reason(s) to wait_list.
Add a new element to the wait_list, that let us know which event(s) we are
waiting on.
2018-08-16 17:29:53 +02:00
Olivier Houchard
5d18718c8f MINOR: tasks: Allow tasklet_wakeup() to wakeup a task.
Modify tasklet_wakeup() so that it handles a task as well, and inserts it
directly into the tasklet list, making it effectively a tasklet.
This should make future developments easier.
2018-08-16 17:29:53 +02:00