Joseph Lynch [Tue, 12 May 2015 06:25:34 +0000 (23:25 -0700)]
MEDIUM: backend: Allow redispatch on retry intervals
For backend load balancing it sometimes makes sense to redispatch rather
than retrying against the same server. For example, when machines or routers
fail you may not want to waste time retrying against a dead server and
would instead prefer to immediately redispatch against other servers.
This patch allows backend sections to specify that they want to
redispatch on a particular interval. If the interval N is positive the
redispatch occurs on every Nth retry, and if the interval N is negative then
the redispatch occurs on the Nth retry prior to the last retry (-1 is the
default and maintains backwards compatibility). In low latency environments
tuning this setting can save a few hundred milliseconds when backends fail.
Thierry FOURNIER [Tue, 19 May 2015 12:46:23 +0000 (14:46 +0200)]
MINOR: http: split the url_param in two parts
This patch is the part of the body_param fetch. The goal is to have
generic url-encoded parser which can used for parsing the query string
and the body.
Willy Tarreau [Wed, 20 May 2015 08:39:04 +0000 (10:39 +0200)]
BUG/MEDIUM: peers: apply a random reconnection timeout
Commit 9ff95bb ("BUG/MEDIUM: peers: correctly configure the client timeout")
uncovered an old bug in the peers : upon disconnect, we reconnect immediately.
This sometimes results in both ends to do the same thing in parallel causing
a loop of connect/accept/close/close that can last several seconds. The risk
of occurrence of the trouble increases with latency, and is emphasized by the
fact that idle connections are now frequently recycled (after 5s of idle).
In order to avoid this we must apply a random delay before reconnecting.
Fortunately the mechanism already supports a reconnect delay, so here we
compute the random timeout when killing a session. The delay is 50ms plus
a random between 0 and 2 seconds. Ideally an exponential back-off would
be preferred but it's preferable to keep the fix simple.
This bug was reported by Marco Corte.
This fix must be backported to 1.5 since the fix above was backported into
1.5.12.
Willy Tarreau [Thu, 7 May 2015 14:06:18 +0000 (16:06 +0200)]
MEDIUM: http: make url_param iterate over multiple occurrences
There are some situations hwere it's desirable to scan multiple occurrences
of a same parameter name in the query string. This change ensures this can
work, even with an empty name which will then iterate over all parameters.
MEDIUM: Add support for updating TLS ticket keys via socket
Until now, HAproxy needed to be restarted to change the TLS ticket
keys. With this patch, the TLS keys can be updated on a per-file
basis using the admin socket. Two new socket commands have been
introduced: "show tls-keys" and "set ssl tls-keys".
MINOR: Add TLS ticket keys reference and use it in the listener struct
Within the listener struct we need to use a reference to the TLS
ticket keys which binds the actual keys with the filename. This will
make it possible to update the keys through the socket
Willy Tarreau [Wed, 13 May 2015 13:39:48 +0000 (15:39 +0200)]
BUG/MAJOR: checks: break infinite loops when tcp-checks starts with comment
If a tcp-check sequence starts with "comment", then the action is not
matched in the while() loop and the pointer doesn't advance so we face
an endless loop. It is normally detected early except in the case where
very slow checks are performed causing it to trigger after the admin stops
watching.
This bug is 1.6-only and very recent so it didn't have the time to affect
anyone.
Willy Tarreau [Wed, 13 May 2015 10:24:53 +0000 (12:24 +0200)]
BUG/MEDIUM: checks: do not dereference a list as a tcpcheck struct
The method used to skip to next rule in the list is wrong, it assumes
that the list element starts at the same offset as the rule. It happens
to be true on most architectures since the list is the first element for
now but it's definitely wrong. Now the code doesn't crash anymore when
the struct list is moved anywhere else in the struct tcpcheck_rule.
Willy Tarreau [Wed, 13 May 2015 10:08:21 +0000 (12:08 +0200)]
BUG/MAJOR: checks: always check for end of list before proceeding
This is the most important fix of this series. There's a risk of endless
loop and crashes caused by the fact that we go past the head of the list
when skipping to next rule, without checking if it's still a valid element.
Most of the time, the ->action field is checked, which points to the proxy's
check_req pointer (generally NULL), meaning the element is confused with a
TCPCHK_ACT_SEND action.
The situation was accidently made worse with the addition of tcp-check
comment since it also skips list elements. However, since the action that
makes it go forward is TCPCHK_ACT_COMMENT (3), there's little chance to
see this as a valid pointer, except on 64-bit machines where it can match
the end of a check_req string pointer.
This fix heavily depends on previous cleanup and both must be backported
to 1.5 where the bug is present.
Willy Tarreau [Wed, 13 May 2015 09:59:14 +0000 (11:59 +0200)]
CLEANUP: checks: simplify the loop processing of tcp-checks
There is some unobvious redundancy between the various ways we can leave
the loop. Some of them can be factored out. So now we leave the loop when
we can't go further, whether it's caused by reaching the end of the rules
or by a blocking I/O.
Willy Tarreau [Wed, 13 May 2015 09:38:17 +0000 (11:38 +0200)]
BUG/MEDIUM: checks: do not dereference head of a tcp-check at the end
When the end of the list is reached, the current step's action is checked
to know if we must poll or not. Unfortunately, the main reason for going
there is that we walked past the end of list and current_step points to
the head. We cannot dereference ->action since it does not belong to this
structure and can definitely crash if the address is not mapped.
This bug is unlikely to cause a crash since the action appears just after
the list, and corresponds to the "char *check_req" pointer in the proxy
struct, and it seems that we can't go there with current_step being null.
At worst it can cause the check to register for recv events.
This fix needs to be backported to 1.5 since the code is incorrect there
as well.
Willy Tarreau [Wed, 13 May 2015 09:23:01 +0000 (11:23 +0200)]
CLEANUP: checks: fix double usage of cur / current_step in tcp-checks
This cleanup is a preliminary requirement to the upcoming fixes for
the bug that affect tcp-check's improper use of lists. It will have
to be backported to 1.5 though it will not easily apply.
There are two variables pointing to the current rule within the loop,
and either one or the other is used depending on the code blocks,
making it much harder to apply checks to fix the list walking bug.
So first get rid of "cur" and only focus on current_step.
Environment variables were expandables only in adresses.
Now there are expandables everywhere in the configuration file within
double quotes.
This patch breaks compatibility with the previous behavior of
environment variables in adresses, you must enclose adresses with double
quotes to make it work.
Willy Tarreau [Tue, 12 May 2015 09:57:07 +0000 (11:57 +0200)]
BUG/MAJOR: check: fix breakage of inverted tcp-check rules
Recent commit 22b09d2 ("MINOR: include comment in tcpcheck error log")
accidently left a double-step to the next rule in case of an inverted
rule. The effect is that an inverted rule is necessarily skipped and
that we can crash if it was the last rule since we'd use as a rule the
head of the list, thus dereference random memory contents.
tcpcheck error messages include the step id where the error occurs.
In some cases, this is not enough. Now, HAProxy also use the comment
field of the latest tcpcheck rule which has been run.
This commit allows HAProxy to parse a new directive in the tcpcheck
ruleset: 'comment'.
It is used to setup comments on the current tcpcheck rules.
A new field is added into the tcpcheck_rule structure.
This field will host a string used as a comment to describe the rule.
Then this comment can be used in logs to report a more user friendly
message on the step which failed during the tcpcheck ruleset.
MINOR: use an int instead of calling tcpcheck_get_step_id
in src.checks.c, the function tcpcheck_get_step_id is called many times.
In order to save some cpu cycles, I save the result of this function in
an integer.
Willy Tarreau [Tue, 12 May 2015 08:30:12 +0000 (10:30 +0200)]
BUILD/MINOR: ssl: fix build failure introduced by recent patch
Baptiste reported that commit 0a9a2b8 ("MEDIUM: sample change the
prototype of sample-fetches and converters functions") broke the
build of ssl_sock.c when using openssl-1.0.2 because one missed
replacement of sess with smp->sess. No backport is needed.
Thierry FOURNIER [Mon, 11 May 2015 13:25:29 +0000 (15:25 +0200)]
MINOR: sample: fill the struct sample with the options.
Options are relative to the sample. Each sample fetched is associated with
fetch options or fetch flags.
This patch adds the 'opt' vaue in the sample struct. This permits to reduce
the sample-fetch function prototype. In other way, the converters will have
more detail about the origin of the sample.
Thierry FOURNIER [Mon, 11 May 2015 09:54:58 +0000 (11:54 +0200)]
MEDIUM: sample: fill the struct sample with the session, proxy and stream pointers
Some sample analyzer (sample-fetch or converters) needs to known the proxy,
session and stream attached to the sampel. The sample-fetches and the converters
function pointers cannot be called without these 3 pointers filled.
This patch permits to reduce the sample-fetch and the converters called
prototypes, and provides a new mean to add information for this type of
functions.
Willy Tarreau [Mon, 11 May 2015 16:30:33 +0000 (18:30 +0200)]
BUG/MEDIUM: http: don't forward client shutdown without NOLINGER except for tunnels
There's an issue related with shutting down POST transfers or closing the
connection after the end of the upload : the shutdown is forwarded to the
server regardless of the abortonclose option. The problem it causes is that
during a scan, brute force or whatever, it becomes possible that all source
ports are exhausted with all sockets in TIME_WAIT state.
There are multiple issues at once in fact :
- no action is done for the close, it automatically happens at the lower
layers thanks for channel_auto_close(), so we cannot act on NOLINGER ;
- we *do* want to continue to send a clean shutdown in tunnel mode because
some protocols transported over HTTP may need this, regardless of option
abortonclose, thus we can't set the option inconditionally
- for all other modes, we do want to close the dirty way because we're
certain whether we've sent everything or not, and we don't want to eat
all source ports.
The solution is a bit complex and applies to DONE/TUNNEL states :
1) disable automatic close for everything not a tunnel and not just
keep-alive / server-close. Force-close is now covered, as is HTTP/1.0
which implicitly works in force-close mode ;
2) when processing option abortonclose, we know we can disable lingering
if the client has closed and the connection is not in tunnel mode.
Since the last case above leads to a situation where the client side reports
an error, we know the connection will not be reused, so leaving the flag on
the stream-interface is safe. A client closing in the middle of the data
transmission already aborts the transaction so this case is not a problem.
This fix must be backported to 1.5 where the problem was detected.
Willy Tarreau [Fri, 8 May 2015 14:13:42 +0000 (16:13 +0200)]
BUG/MEDIUM: http: fix the http-request capture parser
Due to the code being mostly inspired from the tcp-request parser, it
does some crap because both don't work the same way. The "len" argument
could be mismatched and then the length could be used uninitialized.
Willy Tarreau [Fri, 8 May 2015 13:27:59 +0000 (15:27 +0200)]
MEDIUM: http: add new "capture" action for http-request
This is only possible in frontends of course, but it will finally
make it possible to capture arbitrary http parts, including URL
parameters or parts of the message body.
It's worth noting that an ugly (char **) cast had to be done to
call sample_fetch_string() which is caused by a 5- or 6- levels
of inheritance of this type in the API. Here it's harmless since
the function uses it as a const, but this API madness must be
fixed, starting with the one or two rare functions that modify
the args and inflict this on each and every keyword parser.
(cherry picked from commit 484a4f38460593919a1c1d9a047a043198d69f45)
MEDIUM: cfgparse: introduce weak and strong quoting
This patch introduces quoting which allows to write configuration string
including spaces without escaping them.
Strong (with single quotes) and weak (with double quotes) quoting are
supported. Weak quoting supports escaping and special characters when
strong quoting does not interpret anything.
This patch could break configuration files where ' and " where used.
Willy Tarreau [Mon, 4 May 2015 19:57:58 +0000 (21:57 +0200)]
BUG/MEDIUM: config: properly compute the default number of processes for a proxy
Chad Lavoie reported an interesting regression caused by the latest
updates to automatically detect the processes a peers section runs on.
It turns out that if a config has neither nbproc nor a bind-process
statement and depending on the frontend->backend chaining, it is possible
to evade all bind_proc propagations, resulting in assigning only ~0UL (all
processes, which is 32 or 64) without ever restricting it to nbproc. It
was not visible in backends until they started to reference peers sections
which saw themselves with 64 processes at once.
This patch addresses this by replacing all those ~0UL with nbits(nbproc).
That way all "bind-process" settings *default* to the number of processes
defined in nbproc instead of 32 or 64.
This fix could possibly be backported into 1.5, though there is no indication
that this bug could have any effect there.
Willy Tarreau [Mon, 4 May 2015 16:07:56 +0000 (18:07 +0200)]
BUG/MEDIUM: stats: properly initialize the scope before dumping stats
Issuing a "show sess all" prior to a "show stat" on the CLI results in no
proxy being dumped because the scope_len union member was not properly
reinitialized.
Willy Tarreau [Fri, 1 May 2015 22:46:08 +0000 (00:46 +0200)]
MEDIUM: http: provide 3 fetches for the body
Body processing is still fairly limited, but this is a start. It becomes
possible to apply regex to find contents in order to decide where to route
a request for example. Only the first chunk is parsed for now, and the
response is not yet available (the parsing function must be duplicated for
this).
req.body : binary
This returns the HTTP request's available body as a block of data. It
requires that the request body has been buffered made available using
"option http-buffer-request". In case of chunked-encoded body, currently only
the first chunk is analyzed.
req.body_len : integer
This returns the length of the HTTP request's available body in bytes. It may
be lower than the advertised length if the body is larger than the buffer. It
requires that the request body has been buffered made available using
"option http-buffer-request".
req.body_size : integer
This returns the advertised length of the HTTP request's body in bytes. It
will represent the advertised Content-Length header, or the size of the first
chunk in case of chunked encoding. In order to parse the chunks, it requires
that the request body has been buffered made available using
"option http-buffer-request".
Willy Tarreau [Fri, 1 May 2015 20:42:08 +0000 (22:42 +0200)]
MEDIUM: http: add a new option http-buffer-request
It is sometimes desirable to wait for the body of an HTTP request before
taking a decision. This is what is being done by "balance url_param" for
example. The first use case is to buffer requests from slow clients before
connecting to the server. Another use case consists in taking the routing
decision based on the request body's contents. This option placed in a
frontend or backend forces the HTTP processing to wait until either the whole
body is received, or the request buffer is full, or the first chunk is
complete in case of chunked encoding. It can have undesired side effects with
some applications abusing HTTP by expecting unbufferred transmissions between
the frontend and the backend, so this should definitely not be used by
default.
Note that it would not work for the response because we don't reset the
message state before starting to forward. For the response we need to
1) reset the message state to MSG_100_SENT or BODY , and 2) to reset
body_len in case of chunked encoding to avoid counting it twice.
Willy Tarreau [Fri, 1 May 2015 19:52:31 +0000 (21:52 +0200)]
MEDIUM: stream: move HTTP request body analyser before process_common
Since 1.5, the request body analyser has become independant from any
other element and does not even disturb the message forwarder anymore.
And since it's disabled by default, we can place it before most
analysers so that it's can preempt any other one if an intermediary
one enables it.
Willy Tarreau [Fri, 1 May 2015 22:05:47 +0000 (00:05 +0200)]
BUG/MAJOR: http: prevent risk of reading past end with balance url_param
The get_server_ph_post() function assumes that the buffer is contiguous.
While this is true for all the header part, it is not necessarily true
for the end of data the fit in the reserve. In this case there's a risk
to read past the end of the buffer for a few hundred bytes, and possibly
to crash the process if what follows is not mapped.
The fix consists in truncating the analyzed length to the length of the
contiguous block that follows the headers.
A config workaround for this bug would be to disable balance url_param.
This fix must be backported to 1.5. It seems 1.4 did have the check.
Willy Tarreau [Fri, 1 May 2015 21:05:14 +0000 (23:05 +0200)]
BUG/MEDIUM: http: wait for the exact amount of body bytes in wait_for_request_body
Due to the fact that we were still considering only msg->sov for the
first byte of data after calling http_parse_chunk_size(), we used to
miscompute the input data size and to count the CRLF and the chunk size
as part of the input data. The effect is that it was possible to release
the processing with 3 or 4 missing bytes, especially if they're typed by
hand during debugging sessions. This can cause the stats page to return
some errors in admin mode, and the url_param balance algorithm to fail
to properly hash a body input.
Willy Tarreau [Fri, 1 May 2015 17:15:17 +0000 (19:15 +0200)]
MEDIUM: config: validate that peers sections are bound to exactly one process
If a peers section is bound to no process, it's silently discarded. If its
bound to multiple processes, an error is emitted and the process will not
start.
Willy Tarreau [Fri, 1 May 2015 17:13:41 +0000 (19:13 +0200)]
MEDIUM: init: stop any peers section not bound to the correct process
This will prevent the peers section from remaining in listen state on
the incorrect process. The peers_fe pointer is set to NULL, which will
tell the peers task to commit suicide if it was already scheduled.
Willy Tarreau [Fri, 1 May 2015 16:32:13 +0000 (18:32 +0200)]
MEDIUM: peers: unregister peers that were never started
The peers initialization sequence is a bit complex, they're attached
to stick-tables and initialized very early in the boot process. When
we fork, if some must not start, it's too late to find them. Instead,
simply add a guard in their respective tasks to stop them once they
want to start.
Willy Tarreau [Fri, 1 May 2015 18:02:17 +0000 (20:02 +0200)]
MEDIUM: peers: add the ability to disable a peers section
Sometimes it's very hard to disable the use of peers because an empty
section is not valid, so it is necessary to comment out all references
to the section, and not to forget to restore them in the same state
after the operation.
Let's add a "disabled" keyword just like for proxies. A ->state member
in the peers struct is even present for this purpose but was never used
at all.
Maybe it would make sense to backport this to 1.5 as it's really cumbersome
there.
Willy Tarreau [Fri, 1 May 2015 17:09:08 +0000 (19:09 +0200)]
MEDIUM: config: initialize stick-tables after peers, not before
It's dangerous to initialize stick-tables before peers because they
start a task that cannot be stopped before we know if the peers need
to be disabled and destroyed. Move this after.
Willy Tarreau [Fri, 1 May 2015 17:59:56 +0000 (19:59 +0200)]
BUG/MINOR: config: clear proxy->table.peers.p for disabled proxies
If a table in a disabled proxy references a peers section, the peers
name is not resolved to a pointer to a table, but since it belongs to
a union, it can later be dereferenced. Right now it seems it cannot
happen, but it definitely will after the pending changes.
It doesn't cost anything to backport this into 1.5, it will make gdb
sessions less head-scratching.
Willy Tarreau [Fri, 1 May 2015 13:37:53 +0000 (15:37 +0200)]
MEDIUM: http: add option-ignore-probes to get rid of the floods of 408
Recently some browsers started to implement a "pre-connect" feature
consisting in speculatively connecting to some recently visited web sites
just in case the user would like to visit them. This results in many
connections being established to web sites, which end up in 408 Request
Timeout if the timeout strikes first, or 400 Bad Request when the browser
decides to close them first. These ones pollute the log and feed the error
counters. There was already "option dontlognull" but it's insufficient in
this case. Instead, this option does the following things :
- prevent any 400/408 message from being sent to the client if nothing
was received over a connection before it was closed ;
- prevent any log from being emitted in this situation ;
- prevent any error counter from being incremented
That way the empty connection is silently ignored. Note that it is better
not to use this unless it is clear that it is needed, because it will hide
real problems. The most common reason for not receiving a request and seeing
a 408 is due to an MTU inconsistency between the client and an intermediary
element such as a VPN, which blocks too large packets. These issues are
generally seen with POST requests as well as GET with large cookies. The logs
are often the only way to detect them.
This patch should be backported to 1.5 since it avoids false alerts and
makes it easier to monitor haproxy's status.
Willy Tarreau [Fri, 1 May 2015 11:47:08 +0000 (13:47 +0200)]
MEDIUM: http: disable support for HTTP/0.9 by default
There's not much reason for continuing to accept HTTP/0.9 requests
nowadays except for manual testing. Now we disable support for these
by default, unless option accept-invalid-http-request is specified,
in which case they continue to be upgraded to 1.0.
Willy Tarreau [Fri, 1 May 2015 11:26:00 +0000 (13:26 +0200)]
MEDIUM: http: restrict the HTTP version token to 1 digit as per RFC7230
While RFC2616 used to allow an undeterminate amount of digits for the
major and minor components of the HTTP version, RFC7230 has reduced
that to a single digit for each.
If a server can't properly parse the version string and falls back to 0.9,
it could then send a head-less response whose payload would be taken for
headers, which could confuse downstream agents.
Since there's no more reason for supporting a version scheme that was
never used, let's upgrade to the updated version of the standard. It is
still possible to enforce support for the old behaviour using options
accept-invalid-http-request and accept-invalid-http-response.
It would be wise to backport this to 1.5 as well just in case.
Willy Tarreau [Fri, 1 May 2015 08:09:49 +0000 (10:09 +0200)]
BUG/MEDIUM: http: incorrect transfer-coding in the request is a bad request
The rules related to how to handle a bad transfer-encoding header (one
where "chunked" is not at the final place) have evolved to mandate an
abort when this happens in the request. Previously it was only a close
(which is still valid for the server side).
Willy Tarreau [Fri, 1 May 2015 08:06:30 +0000 (10:06 +0200)]
BUG/MEDIUM: http: do not restrict parsing of transfer-encoding to HTTP/1.1
While Transfer-Encoding is HTTP/1.1, we must still parse it in HTTP/1.0
in case an agent sends it, because it's likely that the other side might
use it as well, causing confusion. This will also result in getting rid
of the Content-Length header in such abnormal situations and in having
a clean connection.
BUG/MEDIUM: http: remove content-length from chunked messages
RFC7230 clarified the behaviour to adopt when facing both a
content-length and a transfer-encoding: chunked in a message. While
haproxy already complied with the method for getting the message
length right, and used to detect improper content-length duplicates,
it still did not remove the content-length header when facing a
transfer-encoding: chunked. Usually it is not a problem since other
agents (clients and servers) are required to parse the message
according to the rules that have been in place since RFC2616 in
1999.
However Régis Leroy reported the existence of at least one such
non-compliant agent so haproxy could be abused to get out of sync
with it on pipelined requests (HTTP request smuggling attack),
it consider part of a payload as a subsequent request.
The best thing to do is then to remove the content-length according
to RFC7230. It used to be in the todo list with a fixme in the code
while waiting for the standard to stabilize, let's apply it now that
it's published.
Thanks to Régis for bringing that subject to our attention.
Simon Horman [Thu, 30 Apr 2015 04:10:34 +0000 (13:10 +0900)]
MEDIUM: Send email alerts when servers are marked as UP or enter the drain state
This is similar to the way email alerts are sent when servers are marked as
DOWN.
Like the log messages corresponding to these state changes the messages
have log level notice. Thus they are suppressed by the default email-alert
level of 'alert'. To allow these messages the email-alert level should
be set to 'notice', 'info' or 'debug'. e.g:
email-alert level notice
"email-alert mailers" and "email-alert to" settings are also required in
order for any email alerts to be sent.
Simon Horman [Thu, 30 Apr 2015 04:10:33 +0000 (13:10 +0900)]
MEDIUM: Lower priority of email alerts for log-health-checks messages
Lower the priority of email alerts for log-health-checks messages from
LOG_NOTICE to LOG_INFO.
This is to allow set-ups with log-health-checks enabled to disable email
for health check state changes while leaving other email alerts enabled.
In order for email alerts to be sent for health check state changes
"log-health-checks" needs to be set and "email-alert level" needs to be 'info'
or lower. "email-alert mailers" and "email-alert to" settings are also
required in order for any email alerts to be sent.
MAJOR: pattern: add LRU-based cache on pattern matching
The principle of this cache is to have a global cache for all pattern
matching operations which rely on lists (reg, sub, dir, dom, ...). The
input data, the expression and a random seed are used as a hashing key.
The cached entries contains a pointer to the expression and a revision
number for that expression so that we don't accidently used obsolete
data after a pattern update or a very unlikely hash collision.
Regarding the risk of collisions, 10k entries at 10k req/s mean 1% risk
of a collision after 60 years, that's already much less than the memory's
reliability in most machines and more durable than most admin's life
expectancy. A collision will result in a valid result to be returned
for a different entry from the same list. If this is not acceptable,
the cache can be disabled using tune.pattern.cache-size.
A test on a file containing 10k small regex showed that the regex
matching was limited to 6k/s instead of 70k with regular strings.
When enabling the LRU cache, the performance was back to 70k/s.
MEDIUM: pattern: add a revision to all pattern expressions
This will be used to detect any change on the pattern list between
two operations, ultimately making it possible to implement a cache
which immediately invalidates obsolete keys after an update. The
revision is simply taken from the timestamp counter to ensure that
even upon a pointer reuse we cannot accidently come back to the
same (expr,revision) tuple.
The xxhash library provides a very fast and excellent hash algorithm
suitable for many purposes. It excels at hashing large blocks but is
also extremely fast on small ones. It's distributed under a 2-clause
BSD license (GPL-compatible) so it can be included here. Updates are
distributed here :
This will be usable to implement some maps/acl caches for heavy datasets
loaded from files (mostly regex-based but in general anything that cannot
be indexed in a tree).
MINOR: tools: provide an rdtsc() function for time comparisons
This one returns a timestamp, either the one from the CPU or from
gettimeofday() in 64-bit format. The purpose is to be able to compare
timestamps on various entities to make it easier to detect updates.
It can also be used for benchmarking in certain situations during
development.
BUG/MEDIUM: check: tcpcheck regression introduced by e16c1b3f
The commit e16c1b3f changed the way the function tcpcheck_get_step_id is
now called (check instead of server).
This change introduced a regression since now this function would return
0 all the time because of:
if (check->current_step)
return 0;
This patch fixes this issue by inversing the test: you want to return 0
only if current_step is not yet set :)
Since appctx are scheduled out of streams, it's pointless to wake up
the task managing the stream to push updates, they won't be seen. In
fact unit tests work because silent sessions are restarted after 5s of
idle and the exchange is correctly scheduled during startup!
So we need to notify the appctx instead. For this we add a pointer to
the appctx in the peer session.
Consecutive to the recent changes brought to applets, peers properly
connect but do not exchange data anymore because the stream interface
is not marked as waiting for data.
BUG/MEDIUM: http: functions set-{path,query,method,uri} breaks the HTTP parser
When one of these functions replaces a part of the query string by
a shorter or longer new one, the header parsing is broken. This is
because the start of the first header is not updated.
In the same way, the total length of the request line is not updated.
I dont see any bug caused by this miss, but I guess than it is better
to store the good length.
BUG/MAJOR: tcp: only call registered actions when they're registered
Commit cc87a11 ("MEDIUM: tcp: add register keyword system.") introduced
the registration of new keywords for TCP rulesets. Unfortunately it
replaced the "accept" action with an unconditionnal call to the rule's
action function, resulting in an immediate segfault when using the
"accept" action in a TCP ruleset.
This bug reported by Baptiste Assmann was introduced in 1.6-dev1, no
backport is needed.
MEDIUM: stream-int: pause the appctx if the task is woken up
If we're going to call the task we don't need to call the appctx anymore
since the task may decide differently in the end and will do the proper
thing using ->update(). This reduces one wake up call per session and
may go down to half in case of high concurrency (scheduling races).
MEDIUM: applet: make the applets only use si_applet_{cant|want|stop}_{get|put}
The applets don't fiddle with SI_FL_WAIT_ROOM anymore, instead they indicate
what they want, possibly that they failed (eg: WAIT_ROOM), and it's done() /
update() which finally updates the WAIT_* flags according to the channels'
and stream interface's states. This solves the issue of the pauses during a
"show sess" without creating busy loops.
MINOR: stream-int: add two flags to indicate an applet's wishes regarding I/O
Currently we have a problem. There are some cases where a sleeping applet
is not woken up (eg: show sess during an injection). The reason is that
the applet is marked WAIT_DATA and is not woken up when WAIT_ROOM leaves,
because we wait for both flags to be cleared in order to call it.
And if we wait for either flag, then we have the opposite situation, which
is that we're not waiting for room in the output buffer so we're spinning
calling the applet to do nothing.
What is missing is an indication of what the applet needs. Since it only
manipulates the WAIT_ROOM/WAIT_DATA which are overwritten later, that cannot
work. In the case of connections, the problem doesn't happen because the
connection maintains these extra states. Ideally we'd need to have similar
states for each appctx and to store those information there. But it would
be overcomplicated given that an applet doesn't exist alone without a
stream-int, so we can safely put these information into the stream int and
make the code simpler.
With this patch we introduce two new flags in the stream interface :
- SI_FL_WANT_PUT : the applet wants to put something into the buffer
- SI_FL_WANT_GET : the applet wants to get something from the buffer
We also have the new functions si_applet_{stop|want|cant}_{get|put}
to make the code look similar to the connection code.
MAJOR: stream: do not allocate request buffers anymore when the left side is an applet
We used to allocate a request buffer so that we could process applets
from process_stream(), and this was causing some trouble because it was
not possible for an analyzer to return an error to an applet, which
we'll need for HTTP/2. Now that we don't call applets anymore from
process_stream() we can simplify this and ensure that a response is
always allocated to process a stream.
MEDIUM: applet: centralize the call to si_applet_done() in the I/O handler
It's much easier to centralize this call into the I/O handler than to
do it everywhere with the risk to miss it. Applets are not allowed to
unregister themselves anyway so their SI is still present and it is
possible to update all the context.
MAJOR: stream: use a regular ->update for all stream interfaces
Now si->update() is used to update any type of stream interface, whether
it's an applet, a connection or even nothing. We don't call si_applet_call()
anymore at the end of the resync and we don't have the risk that the
stream's task is reinserted into the run queue, which makes the code
a bit simpler.
The stream_int_update_applet() function was simplified to ensure that
it remained compatible with this standardized calling convention. It
was almost copy-pasted from the update code dedicated to connections.
Just like for si_applet_done(), it seems that it should be possible to
merge the two functions except that it would require some slow operations,
except maybe if the type of end point is tested inside the update function
itself.
MAJOR: applet: now call si_applet_done() instead of si_update() in I/O handlers
The applet I/O handlers now rely on si_applet_done() which itself decides
to wake up or sleep the appctx. Now it becomes critical that applte handlers
properly call this on every exit path so that the appctx is removed from the
active list after I/O have been handled. One such call was added to the Lua
socket handler. It used to work without it probably because the main task is
woken up by the parent task but now it's needed.
MEDIUM: stream-int: add a new function si_applet_done()
This is the equivalent of si_conn_wake() but for applets. It will be
called after changes to the stream interface are brought by the applet
I/O handler. Ultimately it will release buffers and may be even wake
the stream's task up if some important changes are detected.
It would be nice to be able to merge it with the connection's wake
function since it mostly manipulates the stream interface, but there
are minor differences (such as how to enable/disable polling on a fd
vs applet) and some specificities to applets (eg: don't wake the
applet up until the output is empty) which would require abstract
functions which would slow down everything.
MEDIUM: applet: implement a run queue for active appctx
The new function is called for each round of polling in order to call any
active appctx. For now we pick the stream interface from the appctx's
owner. At the moment there's no appctx queued yet, but we have everything
needed to queue them and remove them.
MEDIUM: applet: make the applet not depend on a stream interface anymore
Now that applet's functions only take an appctx in argument, not a
stream interface. This slightly simplifies the code and will be needed
to take the appctx out of the stream interface.