]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoMEDIUM: mux_h2: Revamp the send path when blocking.
Olivier Houchard [Tue, 11 Sep 2018 16:24:28 +0000 (18:24 +0200)] 
MEDIUM: mux_h2: Revamp the send path when blocking.

Change fctl_list and send_list to be lists of struct wait_list, and nuke
send_wait_list, as it's now redundant.
Make the code responsible for shutr/shutw subscribe to those lists.

6 years agoMINOR: connections: Add a "handle" field to wait_list.
Olivier Houchard [Fri, 31 Aug 2018 15:43:32 +0000 (17:43 +0200)] 
MINOR: connections: Add a "handle" field to wait_list.

Add a new field to struct wait_list, "handle", that can be used by the
entity in charge of subscribing.

6 years agoMEDIUM: stream_interface: Make recv() subscribe when more data is needed.
Olivier Houchard [Fri, 31 Aug 2018 15:29:12 +0000 (17:29 +0200)] 
MEDIUM: stream_interface: Make recv() subscribe when more data is needed.

Refactor the code so that si_cs_recv() subscribes to receive events.

6 years agoMEDIUM: h2: Don't use a wake() method anymore.
Olivier Houchard [Tue, 21 Aug 2018 16:10:44 +0000 (18:10 +0200)] 
MEDIUM: h2: Don't use a wake() method anymore.

Instead of having our wake() method called each time a fd event happens,
just subscribe to recv/send events, and get our tasklet called when that
happens. If any recv/send was possible, the equivalent of what h2_wake_cb()
will be done.

6 years agoMEDIUM: h2: always subscribe to receive if allowed.
Olivier Houchard [Fri, 17 Aug 2018 16:42:48 +0000 (18:42 +0200)] 
MEDIUM: h2: always subscribe to receive if allowed.

Let the connection layer know we're always interested in getting more data,
so that we get scheduled as soon as data is available, instead of relying
on the wake() method.

6 years agoMINOR: h2: Let user of h2_recv() and h2_send() know xfer has been done.
Olivier Houchard [Fri, 17 Aug 2018 16:39:46 +0000 (18:39 +0200)] 
MINOR: h2: Let user of h2_recv() and h2_send() know xfer has been done.

Make h2_recv() and h2_send() return 1 if data has been sent/received, or 0
if it did not. That way the caller will be able to know if more work may
have to be done.

6 years agoMEDIUM: connections: Get rid of the recv() method.
Olivier Houchard [Thu, 9 Aug 2018 11:06:55 +0000 (13:06 +0200)] 
MEDIUM: connections: Get rid of the recv() method.

Remove the recv() method from mux and conn_stream.
The goal is to always receive from the upper layers, instead of waiting
for the connection later. For now, recv() is still called from the wake()
method, but that should change soon.

6 years agoMEDIUM: connections/mux: Add a recv and a send+recv wait list.
Olivier Houchard [Thu, 2 Aug 2018 17:23:05 +0000 (19:23 +0200)] 
MEDIUM: connections/mux: Add a recv and a send+recv wait list.

For struct connection, struct conn_stream, and for the h2 mux, add 2 new
lists, one that handles waiters for recv, and one that handles waiters for
recv and send. That way we can ask to subscribe for either recv or send.

6 years agoMEDIUM: connections: Don't reset the polling flags in conn_fd_handler().
Olivier Houchard [Wed, 12 Sep 2018 15:12:53 +0000 (17:12 +0200)] 
MEDIUM: connections: Don't reset the polling flags in conn_fd_handler().

Resetting the polling flags at the end of conn_fd_handler() shouldn't be
needed anymore, and it will create problem when we won't handle send/recv
from conn_fd_handler() anymore.

6 years agoBUG/MEDIUM: tasks: Don't forget to decrement task_list_size in tasklet_free().
Olivier Houchard [Wed, 12 Sep 2018 12:55:03 +0000 (14:55 +0200)] 
BUG/MEDIUM: tasks: Don't forget to decrement task_list_size in tasklet_free().

In tasklet_free(), if we're currently in the runnable task list, don't
forget to decrement taks_list_size, or it'll end up being to big, and we may
not process tasks in the global runqueue.

6 years agoBUILD: fix build without thread
William Lallemand [Wed, 12 Sep 2018 09:57:19 +0000 (11:57 +0200)] 
BUILD: fix build without thread

Cyril Bonté reported that commit f9cc07c25b broke the build without
thread.

We don't need to initialise tid = 0 in mworker_loop, so we could
completely remove it.

6 years agoBUG/MINOR: h2: report asynchronous end of stream on closed connections
Willy Tarreau [Wed, 12 Sep 2018 07:45:54 +0000 (09:45 +0200)] 
BUG/MINOR: h2: report asynchronous end of stream on closed connections

Christopher noticed that the CS_FL_EOS to CS_FL_REOS conversion was
incomplete : when the connectionis closed, we mark the streams with EOS
instead of REOS, causing the loss of any possibly pending data. At the
moment it's not an issue since H2 is used only with a client, but with
servers it could be a real problem if servers close the connection right
after sending their response.

This patch should be backported to 1.8.

6 years agoBUG/MINOR: server: Crash when setting FQDN via CLI.
Frédéric Lécaille [Tue, 21 Aug 2018 13:04:23 +0000 (15:04 +0200)] 
BUG/MINOR: server: Crash when setting FQDN via CLI.

This patch ensures that a DNS resolution may be launched before
setting a server FQDN via the CLI. Especially, it checks that
resolvers was set.

A LEVEL 4 reg testing file is provided.

Thanks to Lukas Tribus for having reported this issue.

Must be backported to 1.8.

6 years agoTESTS: add a python wrapper for sockpair@
William Lallemand [Tue, 11 Sep 2018 14:51:30 +0000 (16:51 +0200)] 
TESTS: add a python wrapper for sockpair@

This is a python wrapper which creates a socketpair and passes it as two
environment variable to haproxy.

It's the easiest way to test the sockpair protocol in haproxy.

6 years agoMEDIUM: protocol: sockpair protocol
William Lallemand [Tue, 11 Sep 2018 14:51:29 +0000 (16:51 +0200)] 
MEDIUM: protocol: sockpair protocol

This protocol is based on the uxst one, but it uses socketpair and FD
passing insteads of a connect()/accept().

The "sockpair@" prefix has been implemented for both bind and server
keywords.

When HAProxy wants to connect through a sockpair@, it creates 2 new
sockets using the socketpair() syscall and pass one of the socket
through the FD specified on the server line.

On the bind side, haproxy will receive the FD, and will use it like it
was the FD of an accept() syscall.

This protocol was designed for internal communication within HAProxy
between the master and the workers, but it's possible to use it
externaly with a wrapper and pass the FD through environment variabls.

6 years agoMEDIUM: protocol: use a custom AF_MAX to help protocol parser
William Lallemand [Tue, 11 Sep 2018 14:51:28 +0000 (16:51 +0200)] 
MEDIUM: protocol: use a custom AF_MAX to help protocol parser

It's possible to have several protocols per family which is a problem
with the current way the protocols are stored.

This allows to register a new protocol in HAProxy which is not a
protocol in the strict socket definition. It will be used to register a
SOCK_STREAM protocol using socketpair().

6 years agoBUG/MAJOR: kqueue: Don't reset the changes number by accident.
Olivier Houchard [Tue, 11 Sep 2018 12:44:51 +0000 (14:44 +0200)] 
BUG/MAJOR: kqueue: Don't reset the changes number by accident.

In _update_fd(), if the fd wasn't polled, and we don't want it to be polled,
we just returned 0, however, we should return changes instead, or all previous
changes will be lost.

This should be backported to 1.8.

6 years agoREORG: http: move some header value processing functions to http.c
Willy Tarreau [Mon, 10 Sep 2018 16:41:28 +0000 (18:41 +0200)] 
REORG: http: move some header value processing functions to http.c

The following functions only deal with header field values and are agnostic
to the HTTP version so they were moved to http.c :

http_header_match2(), find_hdr_value_end(), find_cookie_value_end(),
extract_cookie_value(), parse_qvalue(), http_find_url_param_pos(),
http_find_next_url_param().

Those lacking the "http_" prefix were modified to have it.

6 years agoREORG: http: move the log encoding tables to log.c
Willy Tarreau [Mon, 10 Sep 2018 16:16:53 +0000 (18:16 +0200)] 
REORG: http: move the log encoding tables to log.c

There are 3 tables in proto_http which are used exclusively by logs :
hdr_encode_map[], url_encode_map[] and http_encode_map[]. They indicate
what characters are safe to be emitted in logs depending on the part of
the message where they are placed. Let's move this to log.c, as well as
its initialization. It's worth noting that the rfc5424 map was already
initialized there.

6 years agoREORG: http: move error codes production and processing to http.c
Willy Tarreau [Mon, 10 Sep 2018 16:04:24 +0000 (18:04 +0200)] 
REORG: http: move error codes production and processing to http.c

These error codes and messages are agnostic to the version, even if
they are represented as HTTP/1.0 messages. Ultimately they will have
to be transformed into internal HTTP messages to be used everywhere.

The HTTP/1.1 100 Continue message was turned to an IST and the local
copy in the Lua code was removed.

6 years agoREORG: http: move http_get_path() to http.c
Willy Tarreau [Mon, 10 Sep 2018 15:45:34 +0000 (17:45 +0200)] 
REORG: http: move http_get_path() to http.c

This function is purely HTTP once http_txn is put aside. So the original
one was renamed to http_txn_get_path() and it extracts the relevant offsets
from the txn to pass them to http_get_path(). One benefit of the new version
is that it returns the length at the same time so that allowed to slightly
simplify http_get_path_from_string() which had to look up the end pointer
previously and which is not needed anymore.

6 years agoREORG: http: move the HTTP semantics definitions to http.h/http.c
Willy Tarreau [Mon, 10 Sep 2018 13:38:55 +0000 (15:38 +0200)] 
REORG: http: move the HTTP semantics definitions to http.h/http.c

It's a bit painful to have to deal with HTTP semantics for each protocol
version (H1 and H2), and working on the version-agnostic code further
emphasizes the problem.

This patch creates http.h and http.c which are agnostic to the version
in use, and which borrow a few parts from proto_http and from h1. For
example the once thought h1-specific h1_char_classes array is in fact
dictated by RFC7231 and is used to parse HTTP headers. A few changes
were made to a few files which were including proto_http.h while they
only needed http.h.

Certain string definitions pre-dated the introduction of indirect
strings (ist) so some were used to simplify the definition of the known
HTTP methods. The current lookup code saves 2 kB of a heavily used table
and is faster than the previous table based lookup (typ. 14 ns vs 16
before).

6 years agoMEDIUM: mworker: call per_thread deinit in mworker_reload()
William Lallemand [Tue, 11 Sep 2018 08:06:29 +0000 (10:06 +0200)] 
MEDIUM: mworker: call per_thread deinit in mworker_reload()

We need to clean the FDs registered manually in the poller to avoid FD
leaking during a reload of the master.

This patch call the per thread deinit function which close the thread
waker pipe.

6 years agoMEDIUM: threads: close the thread-waker pipe during deinit
William Lallemand [Tue, 11 Sep 2018 08:06:28 +0000 (10:06 +0200)] 
MEDIUM: threads: close the thread-waker pipe during deinit

In order to avoid FD leaking, we close the pipe used to wake the threads
up during per thread deinit.

6 years agoMINOR: mworker: keep and clean the listeners
William Lallemand [Tue, 11 Sep 2018 08:06:27 +0000 (10:06 +0200)] 
MINOR: mworker: keep and clean the listeners

Keep the listeners that should be used in the master process and clean
them in the workers.

6 years agoMEDIUM: mworker: replace the master pipe by socketpairs
William Lallemand [Tue, 11 Sep 2018 08:06:26 +0000 (10:06 +0200)] 
MEDIUM: mworker: replace the master pipe by socketpairs

In order to communicate with the workers, the master pipe has been
replaced by a socketpair() per worker.

The goal is to use these sockets as stats sockets and be able to access
them from the master.

When reloading, the master serialize the information of the workers and
put them in a environment variable. Once the master has been reexecuted
it unserialize that information and it is capable of closing the FDs of
the leaving children.

6 years agoMEDIUM: mworker: master wait mode use its own initialization
William Lallemand [Tue, 11 Sep 2018 08:06:25 +0000 (10:06 +0200)] 
MEDIUM: mworker: master wait mode use its own initialization

The master now use a poll loop, which should be initialized even in wait
mode. We need to init some variables if we didn't success to load the
configuration file.

6 years agoMINOR: mworker: don't deinit the poller fd when in wait mode
William Lallemand [Tue, 11 Sep 2018 08:06:24 +0000 (10:06 +0200)] 
MINOR: mworker: don't deinit the poller fd when in wait mode

If haproxy failed to load its configuration, the process is reexecuted
and it did not init the poller. So we must not try to deinit the poller
before the exec().

6 years agoMEDIUM: startup: unify signal init between daemon and mworker mode
William Lallemand [Tue, 11 Sep 2018 08:06:23 +0000 (10:06 +0200)] 
MEDIUM: startup: unify signal init between daemon and mworker mode

The signals are now unblocked only once the configuration have been
parsed.

6 years agoMEDIUM: mworker: never block SIG{TERM,INT} during reload
William Lallemand [Tue, 11 Sep 2018 08:06:22 +0000 (10:06 +0200)] 
MEDIUM: mworker: never block SIG{TERM,INT} during reload

The master should be able to be killed even if the reload is not
finished.

6 years agoMEDIUM: mworker: block SIGCHLD until the master is ready
William Lallemand [Tue, 11 Sep 2018 08:06:21 +0000 (10:06 +0200)] 
MEDIUM: mworker: block SIGCHLD until the master is ready

With the new way of handling the signals in the master worker, we are
are not staying in a waitpid() loop. Which means that we need to catch the
SIGCHLD signals to call waitpid().

The problem is when the master is reloading, this signal is neither
registered nor blocked so we lost all signals between the restart and
the call to mworker_loop().

This patch blocks the SIGCHLD signals before the reloading and ensure
it's not unblocked before the master registered the SIGCHLD handler.

6 years agoMINOR: mworker: mworker_cleanlisteners() delete the listeners
William Lallemand [Tue, 11 Sep 2018 08:06:20 +0000 (10:06 +0200)] 
MINOR: mworker: mworker_cleanlisteners() delete the listeners

The mworker_cleanlisteners() function now remove the listeners, we don't
need them in the master for now.

6 years agoBUG/MINOR: mworker: no need to stop peers for each proxy
William Lallemand [Tue, 11 Sep 2018 08:06:19 +0000 (10:06 +0200)] 
BUG/MINOR: mworker: no need to stop peers for each proxy

The mworker_cleanlisteners() was cleaning the peers in the proxy loop,
which is useless since we need to stop the peers only once.

6 years agoMEDIUM: mworker: use the haproxy poll loop
William Lallemand [Tue, 11 Sep 2018 08:06:18 +0000 (10:06 +0200)] 
MEDIUM: mworker: use the haproxy poll loop

In order to reorganize the code of the master worker, the mworker_wait()
function which was the main function was split. This function was
handling a wait() loop, but it does not need it anymore since the code
will use the poll loop of haproxy instead.

The function was split in several functions:

- mworker_catch_sigterm() which is a signal handler for SIGTERM ans
SIGUSR1 that sends the signals to the workers
- mworker_catch_sigchld() which is the code handling the leaving of a
child
- mworker_catch_sighup which basically call the mworker_restart()
function
- mworker_loop() which is the function calling the main poll loop in the
master

6 years agoMEDIUM: mworker: remove register/unregister signal functions
William Lallemand [Tue, 11 Sep 2018 08:06:17 +0000 (10:06 +0200)] 
MEDIUM: mworker: remove register/unregister signal functions

Remove the register and unregister signal functions specifics to the
master worker, because that should be done with the generic ones.

6 years agoMEDIUM: snapshot: merge the captured data after the descriptor
Willy Tarreau [Fri, 7 Sep 2018 18:07:17 +0000 (20:07 +0200)] 
MEDIUM: snapshot: merge the captured data after the descriptor

Instead of having a separate area for the captured data, we now have a
contigous block made of the descriptor and the data. At the moment, since
the area is dynamically allocated, we can adjust its size to what is
needed, but the idea is to quickly switch to a pool and an LRU list.

6 years agoMEDIUM: snapshots: dynamically allocate the snapshots
Willy Tarreau [Fri, 7 Sep 2018 17:02:32 +0000 (19:02 +0200)] 
MEDIUM: snapshots: dynamically allocate the snapshots

Now upon error we dynamically allocate the snapshot instead of overwriting
it. This way there is no more memory wasted in the proxy to hold the two
error snapshot descriptors. Also an appreciable side effect of this is that
the proxy's lock is only taken during the pointer swap, no more while copying
the buffer's contents. This saves 480 bytes of memory per proxy.

6 years agoBUG/MEDIUM: snapshot: take the proxy's lock while dumping errors
Willy Tarreau [Fri, 7 Sep 2018 17:55:44 +0000 (19:55 +0200)] 
BUG/MEDIUM: snapshot: take the proxy's lock while dumping errors

The proxy's lock it held while filling the error but not while dumping
it, so it's possible to dereference pointers being replaced, typically
server pointers. The risk is very low and unlikely but not inexistent.

Since "show errors" is rarely used in parallel, let's simply grab the
proxy's lock while dumping. Ideally we should use an R/W lock here but
it will not make any difference.

This patch must be backported to 1.8, but the code is in proto_http.c
there, though mostly similar.

6 years agoREORG: cli: move the "show errors" handler from http to proxy
Willy Tarreau [Fri, 7 Sep 2018 16:34:24 +0000 (18:34 +0200)] 
REORG: cli: move the "show errors" handler from http to proxy

There's nothing HTTP-specific there anymore at all, let's move this
to the proxy where it belongs.

6 years agoMINOR: http: remove the pointer to the error snapshot in http_capture_bad_message()
Willy Tarreau [Fri, 7 Sep 2018 16:01:03 +0000 (18:01 +0200)] 
MINOR: http: remove the pointer to the error snapshot in http_capture_bad_message()

It's not needed anymore as we know the side thanks to the channel. This
will allow the proxy generic code to better manage the error snapshots.

6 years agoMINOR: http: make the HTTP error capture rely on the generic proxy code
Willy Tarreau [Fri, 7 Sep 2018 13:47:35 +0000 (15:47 +0200)] 
MINOR: http: make the HTTP error capture rely on the generic proxy code

Now that we have a generic error capture function, let's simplify
http_capture_bad_message() to make use of it. At this point the API
is not changed at all, but it could be further simplified.

6 years agoMINOR: proxy: add a new generic proxy_capture_error()
Willy Tarreau [Fri, 7 Sep 2018 15:43:26 +0000 (17:43 +0200)] 
MINOR: proxy: add a new generic proxy_capture_error()

This function now captures an error regardless of its side and protocol.
The caller must pass a number of elements and may pass a protocol-specific
structure and a callback to display it. Later this function may deal with
more advanced allocation techniques to avoid allocating as many buffers
as proxies.

6 years agoMEDIUM: snapshot: implement a show() callback and use it for HTTP
Willy Tarreau [Fri, 7 Sep 2018 12:01:39 +0000 (14:01 +0200)] 
MEDIUM: snapshot: implement a show() callback and use it for HTTP

The HTTP dumps are now configurable in the code : "show errors" now
calls a protocol-specific function to emit the decoded output. For
now only HTTP is implemented.

6 years agoMEDIUM: snapshot: start to reorder the HTTP snapshot output a little bit
Willy Tarreau [Fri, 7 Sep 2018 11:49:44 +0000 (13:49 +0200)] 
MEDIUM: snapshot: start to reorder the HTTP snapshot output a little bit

The output of "show errors" was slightly reordered to split the HTTP part
in a single chunk_appendf() call. The useless buffer total input was
replaced to report the buffer's start offset, which is the offset in the
stream of the first input byte (thus not counting output). Also it was
the opportunity to stop calling the stream "session".

6 years agoMINOR: snapshot: split the error snapshots into common and proto-specific parts
Willy Tarreau [Thu, 6 Sep 2018 17:41:22 +0000 (19:41 +0200)] 
MINOR: snapshot: split the error snapshots into common and proto-specific parts

The idea will be to make the error snapshot feature accessible to other
protocols than just HTTP. This patch only introduces an "http_snapshot"
structure and renames a few fields to make things more explicit. The
HTTP part was installed inside a union so that we can easily add more
protocols in the future.

6 years agoMINOR: snapshot: restart on the event ID and not the stream ID
Willy Tarreau [Fri, 7 Sep 2018 09:51:51 +0000 (11:51 +0200)] 
MINOR: snapshot: restart on the event ID and not the stream ID

The snapshots have the ability to restart a partial dump and they use
the stream ID as the restart point. Since it's purely HTTP, let's use
the event ID instead.

6 years agoBUG/MINOR: http/threads: atomically increment the error snapshot ID
Willy Tarreau [Fri, 7 Sep 2018 09:29:59 +0000 (11:29 +0200)] 
BUG/MINOR: http/threads: atomically increment the error snapshot ID

Let's use an atomic increment for the error snapshot, as we'd rather
not assign the same ID to two errors happening in parallel. It's very
unlikely that it will ever happen though.

This patch must be backported to 1.8 with the other one it relies on
("MINOR: thread: implement HA_ATOMIC_XADD()").

6 years agoBUG/MINOR: dns: check and link servers' resolvers right after config parsing
Baptiste Assmann [Fri, 10 Aug 2018 08:56:38 +0000 (10:56 +0200)] 
BUG/MINOR: dns: check and link servers' resolvers right after config parsing

On the Mailing list, Marcos Moreno reported that haproxy configuration
validation (through "haproxy -c cfgfile") does not detect when a
resolvers section does not exist for a server.
That said, this checking is done after HAProxy has started up.

The problem is that this can create production issue, since init
script can't detect the problem before starting / reloading HAProxy.

To fix this issue, this patch registers the function which validates DNS
configuration validity and run it right after configuration parsing is
finished (through cfg_register_postparser()).
Thanks to it, now "haproxy -c cfgfile" will fail when a server
points to a non-existing resolvers section (or any other validation made
by the function above).

Backport status: 1.8

6 years agoMINOR: log: One const should be enough.
Olivier Houchard [Thu, 6 Sep 2018 16:14:09 +0000 (18:14 +0200)] 
MINOR: log: One const should be enough.

"const const" doesn't bring much more constness, so only use one.

6 years agoMINOR: connection: add new function conn_is_back()
Willy Tarreau [Thu, 6 Sep 2018 12:52:21 +0000 (14:52 +0200)] 
MINOR: connection: add new function conn_is_back()

This function returns true if the connection is a backend connection
and false if it's a frontend connection.

6 years agoMINOR: connection: add new function conn_get_proxy()
Willy Tarreau [Thu, 6 Sep 2018 09:48:44 +0000 (11:48 +0200)] 
MINOR: connection: add new function conn_get_proxy()

This function returns the proxy associated to a connection. For front
connections it returns the frontend, and for back connections it
returns the backend. This will be used to retrieve some configuration
parameters from within a mux.

6 years agoMINOR: connection: make the initialization more consistent
Willy Tarreau [Thu, 6 Sep 2018 09:45:30 +0000 (11:45 +0200)] 
MINOR: connection: make the initialization more consistent

Sometimes a connection is prepared before the target is set, sometimes
after. There's no real rule since the few functions involved operate on
different and independent fields. Soon we'll benefit from knowing the
target at the connection layer, in order to figure the associated proxy
and retrieve the various parameters (timeouts etc). This patch slightly
reorders a few calls to conn_prepare() so that we can make sure that the
target is always known to the mux.

6 years agoBUG/MINOR: h1: fix buffer shift after realignment
Willy Tarreau [Thu, 6 Sep 2018 08:48:15 +0000 (10:48 +0200)] 
BUG/MINOR: h1: fix buffer shift after realignment

Commit 5e74b0b ("MEDIUM: h1: port to new buffer API.") introduced a
minor bug by which a buffer's head could stay shifted by the amount
of removed CRLF if it started with empty lines. This would cause the
second request (or response) not to work until it would receive a few
extra characters. This most only impacts requests sent by hand though.

This is purely 1.9, no backport is needed.

6 years agoMEDIUM: h2: produce some logs on early errors that prevent streams from being created
Willy Tarreau [Wed, 5 Sep 2018 17:55:58 +0000 (19:55 +0200)] 
MEDIUM: h2: produce some logs on early errors that prevent streams from being created

The h2 mux currently lacks some basic transparency. Some errors cause the
connection to be aborted but they couldn't be reported. With this patch,
almost all situations where an error will cause a stream or connection to
be aborted without the ability for an existing stream to report it will be
reported in the logs. This at least provides a solution to monitor the
activity and abnormal traffic.

6 years agoMINOR: log: provide a function to emit a log for a session
Willy Tarreau [Wed, 5 Sep 2018 17:51:10 +0000 (19:51 +0200)] 
MINOR: log: provide a function to emit a log for a session

The new function sess_log() only needs a session to emit a log. It will
ignore the parts that depend on the stream. It is usable to emit a log
to report early errors in muxes. These ones will typically mention
"<BADREQ>" for the request and 0 for the HTTP status code.

6 years agoMEDIUM: log: make sess_build_logline() support being called with no stream
Willy Tarreau [Wed, 5 Sep 2018 14:55:15 +0000 (16:55 +0200)] 
MEDIUM: log: make sess_build_logline() support being called with no stream

Till now it was impossible to emit logs from the lower layers only because
a stream was mandatory. From now on it will at least be possible to emit a
log to report a bad request or some timings for example. When the stream
is null, sess_build_logline() will use default values and will extract the
timing information from the session just like stream_new() does, so the
resulting log line is perfectly valid.

The termination state will indicate a proxy error during the request phase
since it is the only realistic use for such a call with no stream.

6 years agoMINOR: log: use zero as the request counter if there is no stream
Willy Tarreau [Wed, 5 Sep 2018 13:52:59 +0000 (15:52 +0200)] 
MINOR: log: use zero as the request counter if there is no stream

When s==NULL we don't have any assigned request counter. Ideally we
should proceed exactly like when a stream is initialized and assign
a unique value. For now we only place it into a local variable.

6 years agoMINOR: log: keep a copy of s->flags early to avoid a dereference
Willy Tarreau [Wed, 5 Sep 2018 13:51:28 +0000 (15:51 +0200)] 
MINOR: log: keep a copy of s->flags early to avoid a dereference

By placing s->flags into a local variable we'll be able to force it new
values when s is NULL.

6 years agoMINOR: log: use NULL for the unique_id if there is no stream
Willy Tarreau [Wed, 5 Sep 2018 13:49:01 +0000 (15:49 +0200)] 
MINOR: log: use NULL for the unique_id if there is no stream

Now s->unique_id is used as NULL (not set) if s==NULL.

6 years agoMINOR: log: don't check the stream-int's conn_retries if the stream is NULL
Willy Tarreau [Tue, 4 Sep 2018 17:21:44 +0000 (19:21 +0200)] 
MINOR: log: don't check the stream-int's conn_retries if the stream is NULL

Let's simply forget the conn_retries when there is no stream since we
haven't tried to connect yet.

6 years agoMINOR: log: be sure not to dereference a null stream for a target
Willy Tarreau [Wed, 5 Sep 2018 13:30:16 +0000 (15:30 +0200)] 
MINOR: log: be sure not to dereference a null stream for a target

The supported targets are either a server or an applet, so both are
NULL if the stream is NULL.

6 years agoMINOR: log: do not dereference a null stream to access captures
Willy Tarreau [Wed, 5 Sep 2018 13:28:07 +0000 (15:28 +0200)] 
MINOR: log: do not dereference a null stream to access captures

If the stream is null, let's simply not check captures. That's already
done if there is no capture.

6 years agoMINOR: log: keep a copy of the backend connection early in sess_build_logline()
Willy Tarreau [Wed, 5 Sep 2018 13:24:56 +0000 (15:24 +0200)] 
MINOR: log: keep a copy of the backend connection early in sess_build_logline()

This way we can avoid dereferencing a possibly inexisting stream.

6 years agoCLEANUP: log: make the low_level lf_{ip,port,text,text_len} functions take consts
Willy Tarreau [Wed, 5 Sep 2018 13:23:10 +0000 (15:23 +0200)] 
CLEANUP: log: make the low_level lf_{ip,port,text,text_len} functions take consts

These ones were abusively relying on variables making it hard to integrate
with const arguments.

6 years agoMINOR: log: don't unconditionally pick log info from s->logs
Willy Tarreau [Wed, 5 Sep 2018 13:16:23 +0000 (15:16 +0200)] 
MINOR: log: don't unconditionally pick log info from s->logs

We'll soon support s==NULL so let's use an intermediary variable for the
logs structure. For now it only points to s->logs but will support a local
variable as an alternative later.

6 years agoMINOR: log: make sess_build_logline() not dereference a NULL stream for txn
Willy Tarreau [Wed, 5 Sep 2018 13:10:35 +0000 (15:10 +0200)] 
MINOR: log: make sess_build_logline() not dereference a NULL stream for txn

If the stream is NULL, the txn is NULL as well. This condition is already
handled everywhere else.

6 years agoMINOR: log: make the backend fall back to the frontend when there's no stream
Willy Tarreau [Wed, 5 Sep 2018 13:07:15 +0000 (15:07 +0200)] 
MINOR: log: make the backend fall back to the frontend when there's no stream

This is already what happens before the backend is assigned, except that
now we don't need to dereference a NULL stream to figure this.

6 years agoMINOR: log: move the log code to sess_build_logline() to add extra arguments
Willy Tarreau [Wed, 5 Sep 2018 12:58:15 +0000 (14:58 +0200)] 
MINOR: log: move the log code to sess_build_logline() to add extra arguments

The current build_logline() can only be used with valid streams, which
means it is not suitable for use from muxes. We start by moving it into
another more generic function which takes the session as an argument,
to avoid complexifying all the internal API for jsut a few use cases.
This new function is not supposed to be called directly from outside so
we'll be able to instrument it to support several calling conventions.

For now the behaviour and conditions remain unchanged.

6 years agoBUG/MAJOR: buffer: fix incorrect check in __b_putblk()
Willy Tarreau [Wed, 5 Sep 2018 17:00:20 +0000 (19:00 +0200)] 
BUG/MAJOR: buffer: fix incorrect check in __b_putblk()

This function was split in two at commit f7d0447 ("MINOR: buffers:
split b_putblk() into __b_putblk()") but it's wrong, the first half's
length is not adjusted to the requested size so it copies more than
desired.

This is purely 1.9-specific, no backport is needed.

6 years agoBUG/MEDIUM: h2: fix risk of memory leak on malformated wrapped frames
Willy Tarreau [Wed, 5 Sep 2018 16:30:05 +0000 (18:30 +0200)] 
BUG/MEDIUM: h2: fix risk of memory leak on malformated wrapped frames

While parsing a headers frame, if the frame is wrapped in the buffer
and needs to be unwrapped, it will be duplicated before being processed.
But if it contains certain combinations of invalid flags, the parser
returns without releasing the temporary buffer leading to a memory
leak.

This fix needs to be backported to 1.8.

6 years agoBUG/MEDIUM: session: fix reporting of handshake processing time in the logs
Willy Tarreau [Wed, 5 Sep 2018 09:56:48 +0000 (11:56 +0200)] 
BUG/MEDIUM: session: fix reporting of handshake processing time in the logs

The handshake processing time used to be stored per stream, which was
valid when there was exactly one stream per session. With H2 and
multiplexing it's not the case anymore and the reported handshake times
are wrong in the logs as it's computed between the TCP accept() and the
stream creation. Let's first move the handshake where it belongs, which
is the session.

However, this is not enough because we don't want to report an excessive
idle time either for H2 (since many requests use the connection).

So the solution used here is to have the stream retrieve sess->tv_accept
and the handshake duration when the stream is created, and let the mux
immediately reset them. This way, the handshake time becomes zero for the
second and subsequent requests in H2 (which was already the case in H1),
and the idle time exactly counts how long the connection remained unused
while it could be used, so in H1 it runs from the end of the previous
response and in H2 it runs from the end of the previous request since the
channel is already available.

This patch will need to be backported to 1.8.

6 years agoBUG/MINOR: stream: use atomic increments for the request counter
Willy Tarreau [Wed, 5 Sep 2018 14:21:29 +0000 (16:21 +0200)] 
BUG/MINOR: stream: use atomic increments for the request counter

The request counter is incremented when creating a new stream and when
resetting a stream, preparing for a new request. Unfortunately during
the thread migration this was missed, leading to non-atomic increments
in case threads are in use. The most visible side effect is that two
requests may have the same ID from time to time in the logs. However
the SPOE also uses this ID to route responses back to the stream so it
may also lead to occasional spurious SPOE timeouts.

Note that it still doesn't guarantee temporal unicity in the stream
identifiers since a long and a short connection could technically use
the same ID. The likeliness that this happens at the same time is almost
null (roughly threads*runqueue_depth/2^32 that it happens in the same
poll loop), but it will have to be addressed later anyway.

This patch must be backported to 1.8 with the other one it relies on
("MINOR: thread: implement HA_ATOMIC_XADD()").

6 years agoMINOR: thread: implement HA_ATOMIC_XADD()
Willy Tarreau [Wed, 5 Sep 2018 14:11:03 +0000 (16:11 +0200)] 
MINOR: thread: implement HA_ATOMIC_XADD()

We've been missing it several times and now we'll need it to increment
a request counter. Let's do it once for all.

This patch will need to be backported to 1.8 with the associated fix.

6 years agoMINOR: tools: make date2str_log() take some consts
Willy Tarreau [Tue, 4 Sep 2018 17:08:48 +0000 (19:08 +0200)] 
MINOR: tools: make date2str_log() take some consts

The "tm" and "date" field are not modified, they can be const instead
of forcing their callers to use vars.

6 years agoBUG/MEDIUM: ECC cert should work with TLS < v1.2 and openssl >= 1.1.1
Emmanuel Hocdet [Mon, 3 Sep 2018 14:29:16 +0000 (16:29 +0200)] 
BUG/MEDIUM: ECC cert should work with TLS < v1.2 and openssl >= 1.1.1

With openssl >= 1.1.1 and boringssl multi-cert is natively supported.
ECDSA/RSA selection is done and work correctly with TLS >= v1.2.
TLS < v1.2 have no TLSEXT_TYPE_signature_algorithms extension: ECC
certificate can't be selected, and handshake fail if no RSA cert
is present. Safe ECC certificate selection without client announcement
can be very tricky (browser compatibilty). The safer approach is to
select ECDSA certificate if no other certificate matches, like it is
with openssl < 1.1.1: certificate selection is only done via the SNI.

Thanks to Lukas Tribus for reporting this and analysing the problem.

This patch should be backported to 1.8

6 years agoBUG/MEDIUM: dns/server: fix incomatibility between SRV resolution and server state...
Baptiste Assmann [Mon, 2 Jul 2018 15:00:54 +0000 (17:00 +0200)] 
BUG/MEDIUM: dns/server: fix incomatibility between SRV resolution and server state file

Server state file has no indication that a server is currently managed
by a DNS SRV resolution.
And thus, both feature (DNS SRV resolution and server state), when used
together, does not provide the expected behavior: a smooth experience...

This patch introduce the "SRV record name" in the server state file and
loads and applies it if found and wherever required.

This patch applies to haproxy-dev branch only. For backport, a specific patch
is provided for 1.8.

6 years agoREGTEST/MINOR: lua: Add reg testing files for 70d318c.
Frédéric Lécaille [Tue, 4 Sep 2018 13:58:14 +0000 (15:58 +0200)] 
REGTEST/MINOR: lua: Add reg testing files for 70d318c.

6 years agoBUG/MEDIUM: hlua: Don't call RESET_SAFE_LJMP if SET_SAFE_LJMP returns 0.
Olivier Houchard [Mon, 27 Aug 2018 10:59:14 +0000 (12:59 +0200)] 
BUG/MEDIUM: hlua: Don't call RESET_SAFE_LJMP if SET_SAFE_LJMP returns 0.

If SET_SAFE_LJMP returns 0, the spinlock is already unlocked, and lua_atpanic
is already set back to hlua_panic_safe, so there's no need to call
RESET_SAFE_LJMP.

This should be MFC'd into 1.8.

6 years agoREGTEST/MINOR: Add a reg testing file for 3e60b11.
Frédéric Lécaille [Fri, 31 Aug 2018 09:05:39 +0000 (11:05 +0200)] 
REGTEST/MINOR: Add a reg testing file for 3e60b11.

Thank you to Daniel for having worked on this one.

6 years agoBUG/MAJOR: thread: lua: Wrong SSL context initialization.
Frédéric Lécaille [Wed, 29 Aug 2018 11:46:24 +0000 (13:46 +0200)] 
BUG/MAJOR: thread: lua: Wrong SSL context initialization.

When calling ->prepare_srv() callback for SSL server which
depends on global "nbthread" value, this latter was not already parsed,
so equal to 1 default value. This lead to bad memory accesses.

Thank you to Pieter (PiBa-NL) for having reported this issue and
for having provided a very helpful reg testing file to reproduce
this issue (reg-test/lua/b00002.*).

Must be backported to 1.8.

6 years agoBUG/MEDIUM: stream_interface: try to call si_cs_send() earlier.
Olivier Houchard [Tue, 28 Aug 2018 17:37:41 +0000 (19:37 +0200)] 
BUG/MEDIUM: stream_interface: try to call si_cs_send() earlier.

Call si_cs_send() at the beginning of si_cs_wake_cb(), instead of from
stream_int_notify-), so that if we get a connection error while trying to
send, the stream_interface will get SI_FL_ERR, the associated task will
be woken up, and the connection will be properly destroyed.

No backport needed.

6 years agoMINOR: checks: Call wake_srv_chk() when we can finally send data.
Olivier Houchard [Tue, 28 Aug 2018 17:36:18 +0000 (19:36 +0200)] 
MINOR: checks: Call wake_srv_chk() when we can finally send data.

Instead of calling __event_srv_chk_w, call wake_srv_chk(), which will then
either call tcpcheck_main() or __event_srv_chk_w().
Also make tcpcheck_main() subscribe if it can't send.

6 years agoBUG/MEDIUM: hlua: Make sure we drain the output buffer when done.
Olivier Houchard [Tue, 28 Aug 2018 12:41:31 +0000 (14:41 +0200)] 
BUG/MEDIUM: hlua: Make sure we drain the output buffer when done.

In hlua_applet_tcp_fct(), drain the output buffer when the applet is done
running, every time we're called.
Overwise, there's a race condition, and the output buffer could be filled
after the applet ran, and as it is never cleared, the stream interface
will never be destroyed.

This should be backported to 1.8 and 1.7.

6 years agoMINOR: Add srv_conn_free sample fetch
Patrick Hemmer [Thu, 14 Jun 2018 22:01:35 +0000 (18:01 -0400)] 
MINOR: Add srv_conn_free sample fetch

This adds the 'srv_conn_free([<backend>/]<server>)' sample fetch. This fetch
provides the number of available connections on the designated server.

6 years agoMINOR: add be_conn_free sample fetch
Patrick Hemmer [Thu, 14 Jun 2018 21:10:27 +0000 (17:10 -0400)] 
MINOR: add be_conn_free sample fetch

This adds the sample fetch 'be_conn_free([<backend>])'. This sample fetch
provides the total number of unused connections across available servers in the
specified backend.

6 years agoBUG/MEDIUM: lua: reset lua transaction between http requests
Patrick Hemmer [Wed, 22 Aug 2018 14:02:00 +0000 (10:02 -0400)] 
BUG/MEDIUM: lua: reset lua transaction between http requests

Previously LUA code would maintain the transaction state between http
requests, resulting in things like txn:get_priv() retrieving data from
a previous request. This addresses the issue by ensuring the LUA state
is reset between requests.

Co-authored-by: Tim Düsterhus <tim@bastelstu.be>
6 years agoREGTEST/MINOR: Add a reg testing file for b406b87 commit.
Frédéric Lécaille [Fri, 24 Aug 2018 14:14:28 +0000 (16:14 +0200)] 
REGTEST/MINOR: Add a reg testing file for b406b87 commit.

6 years agoBUG/MEDIUM: mux_pt: dereference the connection with care in mux_pt_wake()
Willy Tarreau [Fri, 24 Aug 2018 13:48:59 +0000 (15:48 +0200)] 
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.

6 years agoREGEST/MINOR: Add reg testing files.
Frédéric Lécaille [Thu, 23 Aug 2018 16:06:35 +0000 (18:06 +0200)] 
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.")

6 years agoBUG/MINOR: lua: Bad HTTP client request duration.
Frédéric Lécaille [Wed, 18 Jul 2018 12:25:26 +0000 (14:25 +0200)] 
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.

6 years agoMINOR: connection: make conn_sock_drain() work for all socket families
Willy Tarreau [Fri, 24 Aug 2018 12:31:53 +0000 (14:31 +0200)] 
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().

6 years agoBUG/MEDIUM: unix: provide a ->drain() function
Willy Tarreau [Fri, 24 Aug 2018 12:31:53 +0000 (14:31 +0200)] 
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.

6 years agoREGTEST/MINOR: Add a new class of regression testing files.
Frédéric Lécaille [Wed, 22 Aug 2018 08:41:33 +0000 (10:41 +0200)] 
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.

6 years agoREGTEST/MINOR: Missing mandatory "ignore_unknown_macro".
Frédéric Lécaille [Tue, 21 Aug 2018 13:29:24 +0000 (15:29 +0200)] 
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

6 years agoDOC: Fix spelling error in configuration doc
Jens Bissinger [Thu, 23 Aug 2018 12:11:27 +0000 (14:11 +0200)] 
DOC: Fix spelling error in configuration doc

Fix spelling error in logging section of configuration doc.

6 years agoMINOR: sample: remove impossible tests on negative smp->data.u.str.data
Willy Tarreau [Wed, 22 Aug 2018 03:07:14 +0000 (05:07 +0200)] 
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.

6 years agoMINOR: chunk: remove impossible tests on negative chunk->data
Willy Tarreau [Wed, 22 Aug 2018 02:59:48 +0000 (04:59 +0200)] 
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.

6 years agoBUG/MEDIUM: cli/ssl: don't store base64dec() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 03:26:57 +0000 (05:26 +0200)] 
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.

6 years agoBUG/MEDIUM: connection: don't store recv() result into trash.data
Willy Tarreau [Wed, 22 Aug 2018 03:20:32 +0000 (05:20 +0200)] 
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.

6 years agoBUG/MEDIUM: map: don't store exp_replace() result in the trash's length
Willy Tarreau [Wed, 22 Aug 2018 02:55:43 +0000 (04:55 +0200)] 
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.