]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
10 years agoMEDIUM: Support sending email alerts
Simon Horman [Fri, 30 Jan 2015 02:23:00 +0000 (11:23 +0900)] 
MEDIUM: Support sending email alerts

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Allow configuration of email alerts
Simon Horman [Fri, 30 Jan 2015 02:22:59 +0000 (11:22 +0900)] 
MEDIUM: Allow configuration of email alerts

This currently does nothing beyond parsing the configuration
and storing in the proxy as there is no implementation of email alerts.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Add parsing of mailers section
Simon Horman [Fri, 30 Jan 2015 02:22:58 +0000 (11:22 +0900)] 
MEDIUM: Add parsing of mailers section

As mailer and mailers structures and allow parsing of
a mailers section into those structures.

These structures will subsequently be freed as it is
not yet possible to use reference them in the configuration.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Attach tcpcheck_rules to check
Simon Horman [Fri, 30 Jan 2015 02:22:57 +0000 (11:22 +0900)] 
MEDIUM: Attach tcpcheck_rules to check

This is to allow checks to be established whose tcpcheck_rules
are not those of its proxy.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Move proto and addr fields struct check
Simon Horman [Fri, 30 Jan 2015 02:22:56 +0000 (11:22 +0900)] 
MEDIUM: Move proto and addr fields struct check

The motivation for this is to make checks more independent of each
other to allow further reuse of their infrastructure.

For nowserver->check and server->agent still always use the same values
for the addr and proto fields so this patch should not introduce any
behavioural changes.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Add free_check() helper
Simon Horman [Fri, 30 Jan 2015 02:22:55 +0000 (11:22 +0900)] 
MEDIUM: Add free_check() helper

Add free_check() helper to free the memory allocated by init_check().

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Refactor init_check and move to checks.c
Simon Horman [Fri, 30 Jan 2015 02:22:54 +0000 (11:22 +0900)] 
MEDIUM: Refactor init_check and move to checks.c

Refactor init_check so that an error string is returned
rather than alerts being printed by it. Also
init_check to checks.c and provide a prototype to allow
it to be used from multiple C files.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: Remove connect_chk
Simon Horman [Fri, 30 Jan 2015 02:22:53 +0000 (11:22 +0900)] 
MEDIUM: Remove connect_chk

Remove connect_chk and instead call connect_proc_chk()
and connect_conn_chk(). There no longer seems to be any
value in having a wrapper function here.

Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoBUG/MINOR: stats:Fix incorrect printf type.
Warren Turkal [Tue, 27 Jan 2015 23:04:16 +0000 (15:04 -0800)] 
BUG/MINOR: stats:Fix incorrect printf type.

The value is defined in include/types/global.h to be an unsigned int.
The type format in the printf is for a signed int. This eventually wraps
around.

WT: This bug was introduced in 1.5.

10 years agoBUG/MINOR: http: abort request processing on filter failure
Willy Tarreau [Fri, 30 Jan 2015 19:58:58 +0000 (20:58 +0100)] 
BUG/MINOR: http: abort request processing on filter failure

Commit c600204 ("BUG/MEDIUM: regex: fix risk of buffer overrun in
exp_replace()") added a control of failure on the response headers,
but forgot to check for the error during request processing. So if
the filters fail to apply, we could keep the request. It might
cause some headers to silently fail to be added for example. Note
that it's tagged MINOR because a standard configuration cannot make
this case happen.

The fix should be backported to 1.5 and 1.4 though.

10 years agoBUG/MINOR: checks: prevent http keep-alive with http-check expect
Cyril Bonté [Thu, 29 Jan 2015 23:07:07 +0000 (00:07 +0100)] 
BUG/MINOR: checks: prevent http keep-alive with http-check expect

Sébastien Rohaut reported that string negation in http-check expect didn't
work as expected.

The misbehaviour is caused by responses with HTTP keep-alive. When the
condition is not met, haproxy awaits more data until the buffer is full or the
connection is closed, resulting in a check timeout when "timeout check" is
lower than the keep-alive timeout on the server side.

In order to avoid the issue, when a "http-check expect" is used, haproxy will
ask the server to disable keep-alive by automatically appending a
"Connection: close" header to the request.

10 years agoBUG/MINOR: http: fix incorrect header value offset in replace-hdr/replace-value
Willy Tarreau [Thu, 29 Jan 2015 13:01:34 +0000 (14:01 +0100)] 
BUG/MINOR: http: fix incorrect header value offset in replace-hdr/replace-value

The two http-req/http-resp actions "replace-hdr" and "replace-value" were
expecting exactly one space after the colon, which is wrong. It was causing
the first char not to be seen/modified when no space was present, and empty
headers not to be modified either. Instead of using name->len+2, we must use
ctx->val which points to the first character of the value even if there is
no value.

This fix must be backported into 1.5.

10 years agoMEDIUM: init: continue to enforce SYSTEM_MAXCONN with auto settings if set
Willy Tarreau [Wed, 28 Jan 2015 18:03:21 +0000 (19:03 +0100)] 
MEDIUM: init: continue to enforce SYSTEM_MAXCONN with auto settings if set

Commit d025648 ("MAJOR: init: automatically set maxconn and/or maxsslconn
when possible") resulted in a case where if enough memory is available,
a maxconn value larger than SYSTEM_MAXCONN could be computed, resulting
in possibly overflowing other systems resources (eg: kernel socket buffers,
conntrack entries, etc). Let's bound any automatic maxconn to SYSTEM_MAXCONN
if it is defined. Note that the value is set to DEFAULT_MAXCONN since
SYSTEM_MAXCONN forces DEFAULT_MAXCONN, thus it is not an error.

10 years agoBUG/MINOR: parse: check the validity of size string in a more strict way
Godbach [Wed, 28 Jan 2015 09:36:16 +0000 (17:36 +0800)] 
BUG/MINOR: parse: check the validity of size string in a more strict way

If a stick table is defined as below:
stick-table type ip size 50ka expire 300s

HAProxy will stop parsing size after passing through "50k" and return the value
directly. But such format string of size should not be valid. The patch checks
the next character to report error if any.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoMEDIUM: samples: provide basic arithmetic and bitwise operators
Willy Tarreau [Tue, 27 Jan 2015 14:12:13 +0000 (15:12 +0100)] 
MEDIUM: samples: provide basic arithmetic and bitwise operators

This commit introduces a new category of converters. They are bitwise and
arithmetic operators which support performing basic operations on integers.
Some bitwise operations are supported (and, or, xor, cpl) and some arithmetic
operations are supported (add, sub, mul, div, mod, neg). Some comparators
are provided (odd, even, not, bool) which make it possible to report a match
without having to write an ACL.

The detailed list of new operators as they appear in the doc is :

add(<value>)
  Adds <value> to the input value of type unsigned integer, and returns the
  result as an unsigned integer.

and(<value>)
  Performs a bitwise "AND" between <value> and the input value of type unsigned
  integer, and returns the result as an unsigned integer.

bool
  Returns a boolean TRUE if the input value of type unsigned integer is
  non-null, otherwise returns FALSE. Used in conjunction with and(), it can be
  used to report true/false for bit testing on input values (eg: verify the
  presence of a flag).

cpl
  Takes the input value of type unsigned integer, applies a twos-complement
  (flips all bits) and returns the result as an unsigned integer.

div(<value>)
  Divides the input value of type unsigned integer by <value>, and returns the
  result as an unsigned integer. If <value> is null, the largest unsigned
  integer is returned (typically 2^32-1).

even
  Returns a boolean TRUE if the input value of type unsigned integer is even
  otherwise returns FALSE. It is functionally equivalent to "not,and(1),bool".

mod(<value>)
  Divides the input value of type unsigned integer by <value>, and returns the
  remainder as an unsigned integer. If <value> is null, then zero is returned.

mul(<value>)
  Multiplies the input value of type unsigned integer by <value>, and returns
  the product as an unsigned integer. In case of overflow, the higher bits are
  lost, leading to seemingly strange values.

neg
  Takes the input value of type unsigned integer, computes the opposite value,
  and returns the remainder as an unsigned integer. 0 is identity. This
  operator is provided for reversed subtracts : in order to subtract the input
  from a constant, simply perform a "neg,add(value)".

not
  Returns a boolean FALSE if the input value of type unsigned integer is
  non-null, otherwise returns TRUE. Used in conjunction with and(), it can be
  used to report true/false for bit testing on input values (eg: verify the
  absence of a flag).

odd
  Returns a boolean TRUE if the input value of type unsigned integer is odd
  otherwise returns FALSE. It is functionally equivalent to "and(1),bool".

or(<value>)
  Performs a bitwise "OR" between <value> and the input value of type unsigned
  integer, and returns the result as an unsigned integer.

sub(<value>)
  Subtracts <value> from the input value of type unsigned integer, and returns
  the result as an unsigned integer. Note: in order to subtract the input from
  a constant, simply perform a "neg,add(value)".

xor(<value>)
  Performs a bitwise "XOR" (exclusive OR) between <value> and the input value
  of type unsigned integer, and returns the result as an unsigned integer.

10 years agoMINOR: ssl: load certificates in alphabetical order
Cyril Bonté [Sat, 24 Jan 2015 23:16:08 +0000 (00:16 +0100)] 
MINOR: ssl: load certificates in alphabetical order

As reported by Raphaël Enrici, certificates loaded from a directory are loaded
in a non predictive order. If no certificate was first loaded from a file, it
can result in different behaviours when haproxy is used in cluster.
We can also imagine other cases which weren't met yet.

Instead of using readdir(), we can use scandir() and sort files alphabetically.
This will ensure a predictive behaviour.

This patch should also be backported to 1.5.

10 years agoDOC: fix missing closing brackend on regsub
Willy Tarreau [Fri, 23 Jan 2015 19:39:28 +0000 (20:39 +0100)] 
DOC: fix missing closing brackend on regsub

"]" was missing for <flags>.

10 years agoMEDIUM: http: implement http-request set-{method,path,query,uri}
Willy Tarreau [Thu, 22 Jan 2015 19:46:11 +0000 (20:46 +0100)] 
MEDIUM: http: implement http-request set-{method,path,query,uri}

This commit implements the following new actions :

- "set-method" rewrites the request method with the result of the
  evaluation of format string <fmt>. There should be very few valid reasons
  for having to do so as this is more likely to break something than to fix
  it.

- "set-path" rewrites the request path with the result of the evaluation of
  format string <fmt>. The query string, if any, is left intact. If a
  scheme and authority is found before the path, they are left intact as
  well. If the request doesn't have a path ("*"), this one is replaced with
  the format. This can be used to prepend a directory component in front of
  a path for example. See also "set-query" and "set-uri".

  Example :
      # prepend the host name before the path
      http-request set-path /%[hdr(host)]%[path]

- "set-query" rewrites the request's query string which appears after the
  first question mark ("?") with the result of the evaluation of format
  string <fmt>. The part prior to the question mark is left intact. If the
  request doesn't contain a question mark and the new value is not empty,
  then one is added at the end of the URI, followed by the new value. If
  a question mark was present, it will never be removed even if the value
  is empty. This can be used to add or remove parameters from the query
  string. See also "set-query" and "set-uri".

  Example :
      # replace "%3D" with "=" in the query string
      http-request set-query %[query,regsub(%3D,=,g)]

- "set-uri" rewrites the request URI with the result of the evaluation of
  format string <fmt>. The scheme, authority, path and query string are all
  replaced at once. This can be used to rewrite hosts in front of proxies,
  or to perform complex modifications to the URI such as moving parts
  between the path and the query string. See also "set-path" and
  "set-query".

All of them are handled by the same parser and the same exec function,
which is why they're merged all together. For once, instead of adding
even more entries to the huge switch/case, we used the new facility to
register action keywords. A number of the existing ones should probably
move there as well.

10 years agoBUG/MINOR: sample: fix case sensitivity for the regsub converter
Willy Tarreau [Fri, 23 Jan 2015 19:23:17 +0000 (20:23 +0100)] 
BUG/MINOR: sample: fix case sensitivity for the regsub converter

Two commits ago in 7eda849 ("MEDIUM: samples: add a regsub converter to
perform regex-based transformations"), I got caught for the second time
with the inverted case sensitivity usage of regex_comp(). So by default
it is case insensitive and passing the "i" flag makes it case sensitive.
I forgot to recheck that case before committing the cleanup. No harm
anyway, nobody had the time to use it.

10 years agoMEDIUM/BUG: Only explicitly report "DOWN (agent)" if the agent health is zero
Simon Horman [Wed, 12 Nov 2014 06:55:54 +0000 (15:55 +0900)] 
MEDIUM/BUG: Only explicitly report "DOWN (agent)" if the agent health is zero

Make check check used to report explicitly report "DOWN (agent)" slightly
more restrictive such that it only triggers if the agent health is zero.

This avoids the following problem.

1. Backend is started disabled, agent check is is enabled
2. Backend is stabled using set server vip/rip state ready
3. Health is marked as down using set server vip/rip health down

   At this point the http stats page will report "DOWN (agent)"
   but the backend being down has nothing to do with the agent check

This problem appears to have been introduced by cf2924bc2537bb08c
("MEDIUM: stats: report down caused by agent prior to reporting up").

Note that "DOWN (agent)" may also be reported by a more generic conditional
which immediately follows the code changed by this patch.

Reported-by: Mark Brooks <mark@loadbalancer.org>
Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoBUG/MEDIUM: Do not set agent health to zero if server is disabled in config
Simon Horman [Wed, 12 Nov 2014 06:55:53 +0000 (15:55 +0900)] 
BUG/MEDIUM: Do not set agent health to zero if server is disabled in config

disable starts a server in the disabled state, however setting the health
of an agent implies that the agent is disabled as well as the server.

This is a problem because the state of the agent is not restored if
the state of the server is subsequently updated leading to an
unexpected state.

For example, if a server is started disabled and then the server
state is set to ready then without this change show stat indicates
that the server is "DOWN (agent)" when it is expected that the server
would be UP if its (non-agent) health check passes.

Reported-by: Mark Brooks <mark@loadbalancer.org>
Signed-off-by: Simon Horman <horms@verge.net.au>
10 years agoMEDIUM: samples: add a regsub converter to perform regex-based transformations
Willy Tarreau [Tue, 20 Jan 2015 18:47:06 +0000 (19:47 +0100)] 
MEDIUM: samples: add a regsub converter to perform regex-based transformations

We can now replace matching regex parts with a string, a la sed. Note
that there are at least 3 different behaviours for existing sed
implementations when matching 0-length strings. Here is the result
of the following operation on each implementationt tested :

  echo 'xzxyz' | sed -e 's/x*y*/A/g'

  GNU sed 4.2.1       => AzAzA
  Perl's sed 5.16.1   => AAzAAzA
  Busybox v1.11.2 sed => AzAz

The psed behaviour was adopted because it causes the least exceptions
in the code and seems logical from a certain perspective :

  - "x"  matches x*y*  => add "A" and skip "x"
  - "z"  matches x*y*  => add "A" and keep "z", not part of the match
  - "xy" matches x*y*  => add "A" and skip "xy"
  - "z"  matches x*y*  => add "A" and keep "z", not part of the match
  - ""   matches x*y*  => add "A" and stop here

Anyway, given the incompatibilities between implementations, it's unlikely
that some processing will rely on this behaviour.

There currently is one big limitation : the configuration parser makes it
impossible to pass commas or closing parenthesis (or even closing brackets
in log formats). But that's still quite usable to replace certain characters
or character sequences. It will become more complete once the config parser
is reworked.

10 years agoMEDIUM: regex: add support for passing regex flags to regex_exec_match()
Willy Tarreau [Wed, 21 Jan 2015 12:39:42 +0000 (13:39 +0100)] 
MEDIUM: regex: add support for passing regex flags to regex_exec_match()

This function (and its sister regex_exec_match2()) abstract the regex
execution but make it impossible to pass flags to the regex engine.
Currently we don't use them but we'll need to support REG_NOTBOL soon
(to indicate that we're not at the beginning of a line). So let's add
support for this flag and update the API accordingly.

10 years agoMINOR: args: implement a new arg type for regex : ARGT_REG
Willy Tarreau [Mon, 19 Jan 2015 18:00:58 +0000 (19:00 +0100)] 
MINOR: args: implement a new arg type for regex : ARGT_REG

This one will be used when a regex is expected. It is automatically
resolved after the parsing and compiled into a regex. Some optional
flags are supported in the type-specific flags that should be set by
the optional arg checker. One is used during the regex compilation :
ARGF_REG_ICASE to ignore case.

10 years agoMINOR: args: add type-specific flags for each arg in a list
Willy Tarreau [Wed, 21 Jan 2015 14:51:47 +0000 (15:51 +0100)] 
MINOR: args: add type-specific flags for each arg in a list

These flags are meant to be used by arg checkers to pass out-of-band
information related to some args. A typical use is to indicate how a
regex is expected to be compiled/matched based on other arguments.
These flags are initialized to zero by default and it is up to the args
checkers to set them if needed.

10 years agoMEDIUM: args: increase arg type to 5 bits and limit arg count to 5
Willy Tarreau [Mon, 19 Jan 2015 17:54:49 +0000 (18:54 +0100)] 
MEDIUM: args: increase arg type to 5 bits and limit arg count to 5

We'll soon need to add new argument types, and we don't use the current
limit of 7 arguments, so let's increase the arg type size to 5 bits and
reduce the arg count to 5 (3 max are used today).

10 years agoMEDIUM: args: use #define to specify the number of bits used by arg types and counts
Willy Tarreau [Mon, 19 Jan 2015 17:44:07 +0000 (18:44 +0100)] 
MEDIUM: args: use #define to specify the number of bits used by arg types and counts

This is in order to add new types. This patch does not change anything
else. Two remaining (harmless) occurrences of a count of 8 instead of 7
were fixed by this patch : empty_arg_list[] and the for() loop counting
args.

10 years agoBUG/MEDIUM: http: make http-request set-header compute the string before removal
Willy Tarreau [Wed, 21 Jan 2015 19:39:27 +0000 (20:39 +0100)] 
BUG/MEDIUM: http: make http-request set-header compute the string before removal

The way http-request/response set-header works is stupid. For a naive
reuse of the del-header code, it removes all occurrences of the header
to be set before computing the new format string. This makes it almost
unusable because it is not possible to append values to an existing
header without first copying them to a dummy header, performing the
copy back and removing the dummy header.

Instead, let's share the same code as add-header and perform the optional
removal after the string is computed. That way it becomes possible to
write things like :

   http-request set-header X-Forwarded-For %[hdr(X-Forwarded-For)],%[src]

Note that this change is not expected to have any undesirable impact on
existing configs since if they rely on the bogus behaviour, they don't
work as they always retrieve an empty string.

This fix must be backported to 1.5 to stop the spreadth of ugly configs.

10 years agoBUG/MINOR: args: add missing entry for ARGT_MAP in arg_type_names
Willy Tarreau [Mon, 19 Jan 2015 17:58:20 +0000 (18:58 +0100)] 
BUG/MINOR: args: add missing entry for ARGT_MAP in arg_type_names

This type is currently not used in an argument so it's harmles. But
better correctly fill the name.

10 years agoMEDIUM: backend: add the crc32 hash algorithm for load balancing
Willy Tarreau [Tue, 20 Jan 2015 18:44:50 +0000 (19:44 +0100)] 
MEDIUM: backend: add the crc32 hash algorithm for load balancing

Since we have it available, let's make it usable for load balancing,
it comes at no cost except 3 lines of documentation.

10 years agoMINOR: samples: provide a "crc32" converter
Willy Tarreau [Tue, 20 Jan 2015 18:35:24 +0000 (19:35 +0100)] 
MINOR: samples: provide a "crc32" converter

This converter hashes a binary input sample into an unsigned 32-bit quantity
using the CRC32 hash function. Optionally, it is possible to apply a full
avalanche hash function to the output if the optional <avalanche> argument
equals 1. This converter uses the same functions as used by the various hash-
based load balancing algorithms, so it will provide exactly the same results.
It is provided for compatibility with other software which want a CRC32 to be
computed on some input keys, so it follows the most common implementation as
found in Ethernet, Gzip, PNG, etc... It is slower than the other algorithms
but may provide a better or at least less predictable distribution.

10 years agoMINOR: hash: add new function hash_crc32
Willy Tarreau [Tue, 20 Jan 2015 18:17:09 +0000 (19:17 +0100)] 
MINOR: hash: add new function hash_crc32

This function will be used to perform CRC32 computations. This one wa
loosely inspired from crc32b found here, and focuses on size and speed
at the same time :

    http://www.hackersdelight.org/hdcodetxt/crc.c.txt

Much faster table-based versions exist but are pointless for our usage
here, this hash already sustains gigabit speed which is far faster than
what we'd ever need. Better preserve the CPU's cache instead.

10 years agoMINOR: http: add a new fetch "query" to extract the request's query string
Willy Tarreau [Mon, 19 Jan 2015 14:06:26 +0000 (15:06 +0100)] 
MINOR: http: add a new fetch "query" to extract the request's query string

This fetch extracts the request's query string, which starts after the first
question mark. If no question mark is present, this fetch returns nothing. If
a question mark is present but nothing follows, it returns an empty string.
This means it's possible to easily know whether a query string is present
using the "found" matching method. This fetch is the completemnt of "path"
which stops before the question mark.

10 years agoMAJOR: init: automatically set maxconn and/or maxsslconn when possible
Willy Tarreau [Thu, 15 Jan 2015 20:45:22 +0000 (21:45 +0100)] 
MAJOR: init: automatically set maxconn and/or maxsslconn when possible

If a memory size limit is enforced using "-n" on the command line and
one or both of maxconn / maxsslconn are not set, instead of using the
build-time values, haproxy now computes the number of sessions that can
be allocated depending on a number of parameters among which :

  - global.maxconn (if set)
  - global.maxsslconn (if set)
  - maxzlibmem
  - tune.ssl.cachesize
  - presence of SSL in at least one frontend (bind lines)
  - presence of SSL in at least one backend (server lines)
  - tune.bufsize
  - tune.cookie_len

The purpose is to ensure that not haproxy will not run out of memory
when maxing out all parameters. If neither maxconn nor maxsslconn are
used, it will consider that 100% of the sessions involve SSL on sides
where it's supported. That means that it will typically optimize maxconn
for SSL offloading or SSL bridging on all connections. This generally
means that the simple act of enabling SSL in a frontend or in a backend
will significantly reduce the global maxconn but in exchange of that, it
will guarantee that it will not fail.

All metrics may be enforced using #defines to accomodate variations in
SSL libraries or various allocation sizes.

10 years agoMINOR: global: report information about the cost of SSL connections
Willy Tarreau [Thu, 15 Jan 2015 20:34:39 +0000 (21:34 +0100)] 
MINOR: global: report information about the cost of SSL connections

An SSL connection takes some memory when it exists and during handshakes.
We measured up to 16kB for an established endpoint, and up to 76 extra kB
during a handshake. The SSL layer stores these values into the global
struct during initialization. If other SSL libs are used, it's easy to
change these values. Anyway they'll only be used as gross estimates in
order to guess the max number of SSL conns that can be established when
memory is constrained and the limit is not set.

10 years agoMINOR: global: always export some SSL-specific metrics
Willy Tarreau [Thu, 15 Jan 2015 20:32:40 +0000 (21:32 +0100)] 
MINOR: global: always export some SSL-specific metrics

We'll need to know the number of SSL connections, their use and their
cost soon. In order to avoid getting tons of ifdefs everywhere, always
export SSL information in the global section. We add two flags to know
whether or not SSL is used in a frontend and in a backend.

10 years agoMINOR: tools: add new round_2dig() function to round integers
Willy Tarreau [Thu, 15 Jan 2015 17:43:49 +0000 (18:43 +0100)] 
MINOR: tools: add new round_2dig() function to round integers

This function rounds down an integer to the closest value having only
2 significant digits.

10 years agoBUG/MAJOR: log: don't try to emit a log if no logger is set
Willy Tarreau [Thu, 15 Jan 2015 15:29:53 +0000 (16:29 +0100)] 
BUG/MAJOR: log: don't try to emit a log if no logger is set

send_log() calls update_hdr() to build a log header. It may happen
that no logger is defined at all but that we try to send a log anyway
(eg: upon startup). This results in a segfault when building the log
header because logline was never allocated.

This bug was revealed by the recent log-tag changes because the logline
is dereferenced after the call to snprintf(). So in 1.5 on most platforms
it has no impact because snprintf() will ignore NULL, but not necessarily
on all platforms.

The fix needs to be backported to 1.5.

10 years agoMINOR: channel: rename bi_erase() to channel_truncate()
Willy Tarreau [Wed, 14 Jan 2015 19:32:59 +0000 (20:32 +0100)] 
MINOR: channel: rename bi_erase() to channel_truncate()

It applies to the channel and it doesn't erase outgoing data, only
pending unread data, which is strictly equivalent to what recv()
does with MSG_TRUNC, so that new name is more accurate and intuitive.

10 years agoMINOR: channel: rename bi_avail() to channel_recv_max()
Willy Tarreau [Wed, 14 Jan 2015 19:25:34 +0000 (20:25 +0100)] 
MINOR: channel: rename bi_avail() to channel_recv_max()

This name more accurately reminds that it applies to a channel and not
to a buffer, and that what is returned may be used as a max number of
bytes to pass to recv().

10 years agoMINOR: channel: rename buffer_max_len() to channel_recv_limit()
Willy Tarreau [Wed, 14 Jan 2015 19:21:43 +0000 (20:21 +0100)] 
MINOR: channel: rename buffer_max_len() to channel_recv_limit()

Buffer_max_len() is ambiguous and misleading since it considers the
channel. The new name more accurately designates the size limit for
received data.

10 years agoMINOR: channel: rename buffer_reserved() to channel_reserved()
Willy Tarreau [Wed, 14 Jan 2015 19:16:52 +0000 (20:16 +0100)] 
MINOR: channel: rename buffer_reserved() to channel_reserved()

This applies to the channel, not the buffer, so let's fix this name.
Warning, the function's name happens to be the same as the old one
which was mistakenly used during 1.5.

10 years agoMINOR: channel: rename channel_full() to !channel_may_recv()
Willy Tarreau [Tue, 13 Jan 2015 19:20:10 +0000 (20:20 +0100)] 
MINOR: channel: rename channel_full() to !channel_may_recv()

This function's name was poorly chosen and is confusing to the point of
being suspiciously used at some places. The operations it does always
consider the ability to forward pending input data before receiving new
data. This is not obvious at all, especially at some places where it was
used when consuming outgoing data to know if the buffer has any chance
to ever get the missing data. The code needs to be re-audited with that
in mind. Care must be taken with existing code since the polarity of the
function was switched with the renaming.

10 years agoCLEANUP: channel: rename channel_reserved -> channel_is_rewritable
Willy Tarreau [Tue, 13 Jan 2015 13:39:16 +0000 (14:39 +0100)] 
CLEANUP: channel: rename channel_reserved -> channel_is_rewritable

channel_reserved is confusingly named. It is used to know whether or
not the rewrite area is left intact for situations where we want to
ensure we can use it before proceeding. Let's rename it to fix this
confusion.

10 years agoBUG/MEDIUM: channel: don't schedule data in transit for leaving until connected
Willy Tarreau [Wed, 14 Jan 2015 15:08:45 +0000 (16:08 +0100)] 
BUG/MEDIUM: channel: don't schedule data in transit for leaving until connected

Option http-send-name-header is still hurting. If a POST request has to be
redispatched when this option is used, and the next server's name is larger
than the initial one, and the POST body fills the buffer, it becomes
impossible to rewrite the server's name in the buffer when redispatching.
In 1.4, this is worse, the process may crash because of a negative size
computation for the memmove().

The only solution to fix this is to refrain from eating the reserve before
we're certain that we won't modify the buffer anymore. And the condition for
that is that the connection is established.

This patch introduces "channel_may_send()" which helps to detect whether it's
safe to eat the reserve or not. This condition is used by channel_in_transit()
introduced by recent patches.

This patch series must be backported into 1.5, and a simpler version must be
backported into 1.4 where fixing the bug is much easier since there were no
channels by then. Note that in 1.4 the severity is major.

10 years agoMEDIUM: channel: make bi_avail() use channel_in_transit()
Willy Tarreau [Wed, 14 Jan 2015 14:56:50 +0000 (15:56 +0100)] 
MEDIUM: channel: make bi_avail() use channel_in_transit()

This ensures that we rely on a sane computation for the buffer size.

10 years agoMEDIUM: channel: make buffer_reserved() use channel_in_transit()
Willy Tarreau [Wed, 14 Jan 2015 13:07:13 +0000 (14:07 +0100)] 
MEDIUM: channel: make buffer_reserved() use channel_in_transit()

This ensures that we rely on a sane computation for the buffer size.

10 years agoMINOR: channel: add channel_in_transit()
Willy Tarreau [Tue, 13 Jan 2015 19:09:54 +0000 (20:09 +0100)] 
MINOR: channel: add channel_in_transit()

This function returns the amount of bytes in transit in a channel's buffer,
which is the amount of outgoing data plus the amount of incoming data bound
to the forward limit.

10 years agoBUG/MINOR: channel: compare to_forward with buf->i, not buf->size
Willy Tarreau [Tue, 13 Jan 2015 18:07:23 +0000 (19:07 +0100)] 
BUG/MINOR: channel: compare to_forward with buf->i, not buf->size

We know that all incoming data are going to be purged if to_forward
is greater than them, not only if greater than the buffer size. This
buf has no direct impact on this version, but it participates to some
bugs affecting http-send-name-header since 1.4. This fix will have to
be backported down to 1.4 albeit in a different form.

10 years agoBUG/MEDIUM: channel: fix possible integer overflow on reserved size computation
Willy Tarreau [Thu, 8 Jan 2015 10:34:55 +0000 (11:34 +0100)] 
BUG/MEDIUM: channel: fix possible integer overflow on reserved size computation

The buffer_max_len() function is subject to an integer overflow in this
calculus :

    int ret = global.tune.maxrewrite - chn->to_forward - chn->buf->o;

  - chn->to_forward may be up to 2^31 - 1
  - chn->buf->o may be up to chn->buf->size
  - global.tune.maxrewrite is by definition smaller than chn->buf->size

Thus here we can subtract (2^31 + buf->o) (highly negative) from something
slightly positive, and result in ret being larger than expected.

Fortunately in 1.5 and 1.6, this is only used by bi_avail() which itself
is used by applets which do not set high values for to_forward so this
problem does not happen there. However in 1.4 the equivalent computation
was used to limit the size of a read and can result in a read overflow
when combined with the nasty http-send-name-header feature.

This fix must be backported to 1.5 and 1.4.

10 years agoMINOR: config: extend the default max hostname length to 64 and beyond
Willy Tarreau [Wed, 14 Jan 2015 10:48:58 +0000 (11:48 +0100)] 
MINOR: config: extend the default max hostname length to 64 and beyond

Some users reported that the default max hostname length of 32 is too
short in some environments. This patch does two things :

  - it relies on the system's max hostname length as found in MAXHOSTNAMELEN
    if it is set. This is the most logical thing to do as the system libs
    generally present the appropriate value supported by the system. This
    value is 64 on Linux and 256 on Solaris, to give a few examples.

  - otherwise it defaults to 64

It is still possible to override this value by defining MAX_HOSTNAME_LEN at
build time. After some observation time, this patch may be backported to
1.5 if it does not cause any build issue, as it is harmless and may help
some users.

10 years agoBUG/MEDIUM: http: fix header removal when previous header ends with pure LF
Willy Tarreau [Wed, 7 Jan 2015 16:23:50 +0000 (17:23 +0100)] 
BUG/MEDIUM: http: fix header removal when previous header ends with pure LF

In 1.4-dev7, a header removal mechanism was introduced with commit 68085d8
("[MINOR] http: add http_remove_header2() to remove a header value."). Due
to a typo in the function, the beginning of the headers gets desynchronized
if the header preceeding the deleted one ends with an LF/CRLF combination
different form the one of the removed header. The reason is that while
rewinding the pointer, we go back by a number of bytes taking into account
the LF/CRLF status of the removed header instead of the previous one. The
case where it fails is in http-request del-header/set-header where the
multiple occurrences of a header are present and their LF/CRLF ending
differs from the preceeding header. The loop then stops because no more
headers are found given that the names and length do not match.

Another point to take into consideration is that removing headers using
a loop of http_find_header2() and this function is inefficient since we
remove values one at a time while it could be simpler and faster to
remove full header lines. This is something that should be addressed
separately.

This fix must be backported to 1.5 and 1.4. Note that http-send-name-header
relies on this function as well so it could be possible that some of the
issues encountered with it in 1.4 come from this bug.

10 years agoMINOR: logs: add a new per-proxy "log-tag" directive
Willy Tarreau [Wed, 7 Jan 2015 14:03:42 +0000 (15:03 +0100)] 
MINOR: logs: add a new per-proxy "log-tag" directive

This is equivalent to what was done in commit 48936af ("[MINOR] log:
ability to override the syslog tag") but this time instead of doing
this globally, it does it per proxy. The purpose is to be able to use
a separate log tag for various proxies (eg: make it easier to route
log messages depending on the customer).

10 years agoDOC: add missing entry for log-format and clarify the text
Willy Tarreau [Wed, 7 Jan 2015 13:55:17 +0000 (14:55 +0100)] 
DOC: add missing entry for log-format and clarify the text

There was no "log-format" entry in the keyword index! This should be
backported to 1.5.

10 years agoBUG/MEDIUM: backend: correctly detect the domain when use_domain_only is used
Cyril Bonté [Sun, 4 Jan 2015 14:17:36 +0000 (15:17 +0100)] 
BUG/MEDIUM: backend: correctly detect the domain when use_domain_only is used

balance hdr(<name>) provides on option 'use_domain_only' to match only the
domain part in a header (designed for the Host header).

Olivier Fredj reported that the hashes were not the same for
'subdomain.domain.tld' and 'domain.tld'.

This is because the pointer was rewinded one step to far, resulting in a hash
calculated against wrong values :
- '.domai' for 'subdomain.domain.tld'
- ' domai' for 'domain.tld' (beginning with the space in the header line)

Another special case is when no dot can be found in the header : the hash will
be calculated against an empty string.

The patch addresses both cases : 'domain' will be used to compute the hash for
'subdomain.domain.tld', 'domain.tld' and 'domain' (using the whole header value
for the last case).

The fix must be backported to haproxy 1.5 and 1.4.

10 years agoCLEANUP: session: remove session_from_task()
Willy Tarreau [Sun, 28 Dec 2014 11:19:57 +0000 (12:19 +0100)] 
CLEANUP: session: remove session_from_task()

Since commit 3dd6a25 ("MINOR: stream-int: retrieve session pointer from
stream-int"), we can get the session from the task, so let's get rid of
this less obvious function.

10 years agoDOC: checks: environment variables used by "external-check command"
Cyril Bonté [Sat, 27 Dec 2014 21:28:39 +0000 (22:28 +0100)] 
DOC: checks: environment variables used by "external-check command"

Add some documentation about the environment variables available with
"external-check command". Currently, only one of them is dynamically updated
on each check : HAPROXY_SERVER_CURCONN.

10 years agoMINOR: checks: update dynamic environment variables in external checks
Cyril Bonté [Sat, 27 Dec 2014 21:28:38 +0000 (22:28 +0100)] 
MINOR: checks: update dynamic environment variables in external checks

commit 9ede66b0 introduced an environment variable (HAPROXY_SERVER_CURCONN) that
was supposed to be dynamically updated, but it was set only once, during its
initialization.

Most of the code provided in this previous patch has been rewritten in order to
easily update the environment variables without reallocating memory during each
check.

Now, HAPROXY_SERVER_CURCONN will contain the current number of connections on
the server at the time of the check.

10 years agoOPTIM: stream-int: try to send pending spliced data
Willy Tarreau [Tue, 9 Dec 2014 18:56:47 +0000 (19:56 +0100)] 
OPTIM: stream-int: try to send pending spliced data

This is the equivalent of eb9fd51 ("OPTIM: stream_sock: reduce the amount
of in-flight spliced data") whose purpose is to try to immediately send
spliced data if available.

10 years agoMEDIUM: stream-int: support splicing from applets
Willy Tarreau [Tue, 9 Dec 2014 18:47:54 +0000 (19:47 +0100)] 
MEDIUM: stream-int: support splicing from applets

If we want to splice from applets, we must check the pipe before clearing
SI_FL_WAIT_ROOM.

10 years agoMEDIUM: channel: implement a zero-copy buffer transfer
Willy Tarreau [Mon, 8 Dec 2014 17:14:53 +0000 (18:14 +0100)] 
MEDIUM: channel: implement a zero-copy buffer transfer

bi_swpbuf() swaps the buffer passed in argument with the one attached to
the channel, but only if this last one is empty. The idea is to avoid a
copy when buffers can simply be swapped.

10 years agoMINOR: config: implement global setting tune.buffers.limit
Willy Tarreau [Tue, 23 Dec 2014 21:52:37 +0000 (22:52 +0100)] 
MINOR: config: implement global setting tune.buffers.limit

This setting is used to limit memory usage without causing the alloc
failures caused by "-m". Unexpectedly, tests have shown a performance
boost of up to about 18% on HTTP traffic when limiting the number of
buffers to about 10% of the amount of concurrent connections.

tune.buffers.limit <number>
  Sets a hard limit on the number of buffers which may be allocated per process.
  The default value is zero which means unlimited. The minimum non-zero value
  will always be greater than "tune.buffers.reserve" and should ideally always
  be about twice as large. Forcing this value can be particularly useful to
  limit the amount of memory a process may take, while retaining a sane
  behaviour. When this limit is reached, sessions which need a buffer wait for
  another one to be released by another session. Since buffers are dynamically
  allocated and released, the waiting time is very short and not perceptible
  provided that limits remain reasonable. In fact sometimes reducing the limit
  may even increase performance by increasing the CPU cache's efficiency. Tests
  have shown good results on average HTTP traffic with a limit to 1/10 of the
  expected global maxconn setting, which also significantly reduces memory
  usage. The memory savings come from the fact that a number of connections
  will not allocate 2*tune.bufsize. It is best not to touch this value unless
  advised to do so by an haproxy core developer.

10 years agoMINOR: config: implement global setting tune.buffers.reserve
Willy Tarreau [Tue, 23 Dec 2014 21:40:40 +0000 (22:40 +0100)] 
MINOR: config: implement global setting tune.buffers.reserve

Used in conjunction with the dynamic buffer allocator.

tune.buffers.reserve <number>
  Sets the number of buffers which are pre-allocated and reserved for use only
  during memory shortage conditions resulting in failed memory allocations. The
  minimum value is 2 and is also the default. There is no reason a user would
  want to change this value, it's mostly aimed at haproxy core developers.

10 years agoMAJOR: session: only wake up as many sessions as available buffers permit
Willy Tarreau [Thu, 27 Nov 2014 00:11:56 +0000 (01:11 +0100)] 
MAJOR: session: only wake up as many sessions as available buffers permit

We've already experimented with three wake up algorithms when releasing
buffers : the first naive one used to wake up far too many sessions,
causing many of them not to get any buffer. The second approach which
was still in use prior to this patch consisted in waking up either 1
or 2 sessions depending on the number of FDs we had released. And this
was still inaccurate. The third one tried to cover the accuracy issues
of the second and took into consideration the number of FDs the sessions
would be willing to use, but most of the time we ended up waking up too
many of them for nothing, or deadlocking by lack of buffers.

This patch completely removes the need to allocate two buffers at once.
Instead it splits allocations into critical and non-critical ones and
implements a reserve in the pool for this. The deadlock situation happens
when all buffers are be allocated for requests pending in a maxconn-limited
server queue, because then there's no more way to allocate buffers for
responses, and these responses are critical to release the servers's
connection in order to release the pending requests. In fact maxconn on
a server creates a dependence between sessions and particularly between
oldest session's responses and latest session's requests. Thus, it is
mandatory to get a free buffer for a response in order to release a
server connection which will permit to release a request buffer.

Since we definitely have non-symmetrical buffers, we need to implement
this logic in the buffer allocation mechanism. What this commit does is
implement a reserve of buffers which can only be allocated for responses
and that will never be allocated for requests. This is made possible by
the requester indicating how much margin it wants to leave after the
allocation succeeds. Thus it is a cooperative allocation mechanism : the
requester (process_session() in general) prefers not to get a buffer in
order to respect other's need for response buffers. The session management
code always knows if a buffer will be used for requests or responses, so
that is not difficult :

  - either there's an applet on the initiator side and we really need
    the request buffer (since currently the applet is called in the
    context of the session)

  - or we have a connection and we really need the response buffer (in
    order to support building and sending an error message back)

This reserve ensures that we don't take all allocatable buffers for
requests waiting in a queue. The downside is that all the extra buffers
are really allocated to ensure they can be allocated. But with small
values it is not an issue.

With this change, we don't observe any more deadlocks even when running
with maxconn 1 on a server under severely constrained memory conditions.

The code becomes a bit tricky, it relies on the scheduler's run queue to
estimate how many sessions are already expected to run so that it doesn't
wake up everyone with too few resources. A better solution would probably
consist in having two queues, one for urgent requests and one for normal
requests. A failed allocation for a session dealing with an error, a
connection event, or the need for a response (or request when there's an
applet on the left) would go to the urgent request queue, while other
requests would go to the other queue. Urgent requests would be served
from 1 entry in the pool, while the regular ones would be served only
according to the reserve. Despite not yet having this, it works
remarkably well.

This mechanism is quite efficient, we don't perform too many wake up calls
anymore. For 1 million sessions elapsed during massive memory contention,
we observe about 4.5M calls to process_session() compared to 4.0M without
memory constraints. Previously we used to observe up to 16M calls, which
rougly means 12M failures.

During a test run under high memory constraints (limit enforced to 27 MB
instead of the 58 MB normally needed), performance used to drop by 53% prior
to this patch. Now with this patch instead it *increases* by about 1.5%.

The best effect of this change is that by limiting the memory usage to about
2/3 to 3/4 of what is needed by default, it's possible to increase performance
by up to about 18% mainly due to the fact that pools are reused more often
and remain hot in the CPU cache (observed on regular HTTP traffic with 20k
objects, buffers.limit = maxconn/10, buffers.reserve = limit/2).

Below is an example of scenario which used to cause a deadlock previously :
  - connection is received
  - two buffers are allocated in process_session() then released
  - one is allocated when receiving an HTTP request
  - the second buffer is allocated then released in process_session()
    for request parsing then connection establishment.
  - poll() says we can send, so the request buffer is sent and released
  - process session gets notified that the connection is now established
    and allocates two buffers then releases them
  - all other sessions do the same till one cannot get the request buffer
    without hitting the margin
  - and now the server responds. stream_interface allocates the response
    buffer and manages to get it since it's higher priority being for a
    response.
  - but process_session() cannot allocate the request buffer anymore

  => We could end up with all buffers used by responses so that none may
     be allocated for a request in process_session().

When the applet processing leaves the session context, the test will have
to be changed so that we always allocate a response buffer regardless of
the left side (eg: H2->H1 gateway). A final improvement would consists in
being able to only retry the failed I/O operation without waking up a
task, but to date all experiments to achieve this have proven not to be
reliable enough.

10 years agoMINOR: stats: report a "waiting" flags for sessions
Willy Tarreau [Wed, 26 Nov 2014 17:05:38 +0000 (18:05 +0100)] 
MINOR: stats: report a "waiting" flags for sessions

This flag indicates if the session is still waiting for some memory.

10 years agoMAJOR: session: only allocate buffers when needed
Willy Tarreau [Tue, 25 Nov 2014 18:46:36 +0000 (19:46 +0100)] 
MAJOR: session: only allocate buffers when needed

A session doesn't need buffers all the time, especially when they're
empty. With this patch, we don't allocate buffers anymore when the
session is initialized, we only allocate them in two cases :

  - during process_session()
  - during I/O operations

During process_session(), we try hard to allocate both buffers at once
so that we know for sure that a started operation can complete. Indeed,
a previous version of this patch used to allocate one buffer at a time,
but it can result in a deadlock when all buffers are allocated for
requests for example, and there's no buffer left to emit error responses.
Here, if any of the buffers cannot be allocated, the whole operation is
cancelled and the session is added at the tail of the buffer wait queue.

At the end of process_session(), a call to session_release_buffers() is
done so that we can offer unused buffers to other sessions waiting for
them.

For I/O operations, we only need to allocate a buffer on the Rx path.
For this, we only allocate a single buffer but ensure that at least two
are available to avoid the deadlock situation. In case buffers are not
available, SI_FL_WAIT_ROOM is set on the stream interface and the session
is queued. Unused buffers resulting either from a successful send() or
from an unused read buffer are offered to pending sessions during the
->wake() callback.

10 years agoMAJOR: session: implement a wait-queue for sessions who need a buffer
Willy Tarreau [Tue, 25 Nov 2014 20:10:35 +0000 (21:10 +0100)] 
MAJOR: session: implement a wait-queue for sessions who need a buffer

When a session_alloc_buffers() fails to allocate one or two buffers,
it subscribes the session to buffer_wq, and waits for another session
to release buffers. It's then removed from the queue and woken up with
TASK_WAKE_RES, and can attempt its allocation again.

We decide to try to wake as many waiters as we release buffers so
that if we release 2 and two waiters need only once, they both have
their chance. We must never come to the situation where we don't wake
enough tasks up.

It's common to release buffers after the completion of an I/O callback,
which can happen even if the I/O could not be performed due to half a
failure on memory allocation. In this situation, we don't want to move
out of the wait queue the session that was just added, otherwise it
will never get any buffer. Thus, we only force ourselves out of the
queue when freeing the session.

Note: at the moment, since session_alloc_buffers() is not used, no task
is subscribed to the wait queue.

10 years agoMEDIUM: session: implement a basic atomic buffer allocator
Willy Tarreau [Tue, 25 Nov 2014 18:46:36 +0000 (19:46 +0100)] 
MEDIUM: session: implement a basic atomic buffer allocator

This patch introduces session_alloc_recv_buffer(), session_alloc_buffers()
and session_release_buffers() whose purpose will be to allocate missing
buffers and release unneeded ones around the process_session() and during
I/O operations.

I/O callbacks only need a single buffer for recv operations and none
for send. However we still want to ensure that we don't pick the last
buffer. That's what session_alloc_recv_buffer() is for.

This allocator is atomic in that it always ensures we can get 2 buffers
or fails. Here, if any of the buffers is not ready and cannot be
allocated, the operation is cancelled. The purpose is to guarantee that
we don't enter into the deadlock where all buffers are allocated by the
same size of all sessions.

A queue will have to be implemented for failed allocations. For now
they're just reported as failures.

10 years agoMEDIUM: buffer: implement b_alloc_margin()
Willy Tarreau [Tue, 2 Dec 2014 12:54:01 +0000 (13:54 +0100)] 
MEDIUM: buffer: implement b_alloc_margin()

This function is used to allocate a buffer and ensure that we leave
some margin after it in the pool. The function is not obvious. While
we allocate only one buffer, we want to ensure that at least two remain
available after our allocation. The purpose is to ensure we'll never
enter a deadlock where all sessions allocate exactly one buffer, and
none of them will be able to allocate the second buffer needed to build
a response in order to release the first one.

We also take care of remaining fast in the the fast path by first
checking whether or not there is enough margin, in which case we only
rely on b_alloc_fast() which is guaranteed to succeed. Otherwise we
take the slow path using pool_refill_alloc().

10 years agoMINOR: buffer: implement b_alloc_fast()
Willy Tarreau [Mon, 8 Dec 2014 15:37:26 +0000 (16:37 +0100)] 
MINOR: buffer: implement b_alloc_fast()

This function allocates a buffer and replaces *buf with this buffer. If
no memory is available, &buf_wanted is used instead. No control is made
to check if *buf already pointed to another buffer. The allocated buffer
is returned, or NULL in case no memory is available. The difference with
b_alloc() is that this function only picks from the pool and never calls
malloc(), so it can fail even if some memory is available. It is the
caller's job to refill the buffer pool if needed.

10 years agoMINOR: session: group buffer allocations together
Willy Tarreau [Tue, 25 Nov 2014 18:54:11 +0000 (19:54 +0100)] 
MINOR: session: group buffer allocations together

We'll soon want to release buffers together upon failure so we need to
allocate them after the channels. Let's change this now. There's no
impact on the behaviour, only the error path is unrolled slightly
differently. The same was done in peers.

10 years agoMEDIUM: channel: do not report full when buf_empty is present on a channel
Willy Tarreau [Fri, 28 Nov 2014 19:54:13 +0000 (20:54 +0100)] 
MEDIUM: channel: do not report full when buf_empty is present on a channel

Till now we'd consider a buffer full even if it had size==0 due to pointing
to buf.size. Now we change this : if buf_wanted is present, it means that we
have already tried to allocate a buffer but failed. Thus the buffer must be
considered full so that we stop trying to poll for reads on it. Otherwise if
it's empty, it's buf_empty and we report !full since we may allocate it on
the fly.

10 years agoMEDIUM: buffer: add a new buf_wanted dummy buffer to report failed allocations
Willy Tarreau [Mon, 24 Nov 2014 10:55:08 +0000 (11:55 +0100)] 
MEDIUM: buffer: add a new buf_wanted dummy buffer to report failed allocations

Doing so ensures that even when no memory is available, we leave the
channel in a sane condition. There's a special case in proto_http.c
regarding the compression, we simply pre-allocate the tmpbuf to point
to the dummy buffer. Not reusing &buf_empty for this allows the rest
of the code to differenciate an empty buffer that's not used from an
empty buffer that results from a failed allocation which has the same
semantics as a buffer full.

10 years agoMEDIUM: buffer: always assign a dummy empty buffer to channels
Willy Tarreau [Mon, 24 Nov 2014 10:39:34 +0000 (11:39 +0100)] 
MEDIUM: buffer: always assign a dummy empty buffer to channels

Channels are now created with a valid pointer to a buffer before the
buffer is allocated. This buffer is a global one called "buf_empty" and
of size zero. Thus it prevents any activity from being performed on
the buffer and still ensures that chn->buf may always be dereferenced.
b_free() also resets the buffer to &buf_empty, and was split into
b_drop() which does not reset the buffer.

10 years agoMINOR: buffer: only use b_free to release buffers
Willy Tarreau [Tue, 25 Nov 2014 18:45:11 +0000 (19:45 +0100)] 
MINOR: buffer: only use b_free to release buffers

We don't call pool_free2(pool2_buffers) anymore, we only call b_free()
to do the job. This ensures that we can start to centralize the releasing
of buffers.

10 years agoMINOR: buffer: move buffer initialization after channel initialization
Willy Tarreau [Mon, 24 Nov 2014 10:36:57 +0000 (11:36 +0100)] 
MINOR: buffer: move buffer initialization after channel initialization

It's not clean to initialize the buffer before the channel since it
dereferences one pointer in the channel. Also we'll want to let the
channel pre-initialize the buffer, so let's ensure that the channel
is always initialized prior to the buffers.

10 years agoMEDIUM: buffer: use b_alloc() to allocate and initialize a buffer
Willy Tarreau [Mon, 24 Nov 2014 10:30:16 +0000 (11:30 +0100)] 
MEDIUM: buffer: use b_alloc() to allocate and initialize a buffer

b_alloc() now allocates a buffer and initializes it to the size specified
in the pool minus the size of the struct buffer itself. This ensures that
callers do not need to care about buffer details anymore. Also this never
applies memory poisonning, which is slow and useless on buffers.

10 years agoMINOR: buffer: reset a buffer in b_reset() and not channel_init()
Willy Tarreau [Mon, 24 Nov 2014 09:54:47 +0000 (10:54 +0100)] 
MINOR: buffer: reset a buffer in b_reset() and not channel_init()

We'll soon need to be able to switch buffers without touching the
channel, so let's move buffer initialization out of channel_init().
We had the same in compressoin.c.

10 years agoMINOR: stream-int: retrieve session pointer from stream-int
Willy Tarreau [Fri, 28 Nov 2014 17:30:25 +0000 (18:30 +0100)] 
MINOR: stream-int: retrieve session pointer from stream-int

sess_from_si() does this via the owner (struct task). It works because
all stream ints belong to a task nowadays.

10 years agoMEDIUM: memory: improve pool_refill_alloc() to pass a refill count
Willy Tarreau [Wed, 3 Dec 2014 14:25:28 +0000 (15:25 +0100)] 
MEDIUM: memory: improve pool_refill_alloc() to pass a refill count

Till now this function would only allocate one entry at a time. But with
dynamic buffers we'll like to allocate the number of missing entries to
properly refill the pool.

Let's modify it to take a minimum amount of available entries. This means
that when we know we need at least a number of available entries, we can
ask to allocate all of them at once. It also ensures that we don't move
the pointers back and forth between the caller and the pool, and that we
don't call pool_gc2() for each failed malloc. Instead, it's called only
once and the malloc is only allowed to fail once.

10 years agoMINOR: memory: cut pool allocator in 3 layers
Willy Tarreau [Mon, 8 Dec 2014 15:35:23 +0000 (16:35 +0100)] 
MINOR: memory: cut pool allocator in 3 layers

pool_alloc2() used to pick the entry from the pool, fall back to
pool_refill_alloc(), and to perform the poisonning itself, which
pool_refill_alloc() was also doing. While this led to optimal
code size, it imposes memory poisonning on the buffers as well,
which is extremely slow on large buffers.

This patch cuts the allocator in 3 layers :
  - a layer to pick the first entry from the pool without falling back to
    pool_refill_alloc() : pool_get_first()
  - a layer to allocate a dirty area by falling back to pool_refill_alloc()
    but never performing the poisonning : pool_alloc_dirty()
  - pool_alloc2() which calls the latter and optionally poisons the area

No functional changes were made.

10 years agoCLEANUP: memory: replace macros pool_alloc2/pool_free2 with functions
Willy Tarreau [Tue, 23 Dec 2014 13:13:16 +0000 (14:13 +0100)] 
CLEANUP: memory: replace macros pool_alloc2/pool_free2 with functions

Using inline functions here makes the code more readable and reduces its
size by about 2 kB.

10 years agoCLEANUP: memory: remove dead code
Willy Tarreau [Tue, 23 Dec 2014 12:51:28 +0000 (13:51 +0100)] 
CLEANUP: memory: remove dead code

The very old pool managment code has not been used for the last 7 years
and is still polluting the file. Get rid of it now.

10 years agoCLEANUP: lists: remove dead code
Willy Tarreau [Tue, 23 Dec 2014 12:58:43 +0000 (13:58 +0100)] 
CLEANUP: lists: remove dead code

Remove the code dealing with the old dual-linked lists imported from
librt that has remained unused for the last 8 years. Now everything
uses the linux-like circular lists instead.

10 years agoBUG/MEDIUM: compression: correctly report zlib_mem
Willy Tarreau [Wed, 24 Dec 2014 17:07:55 +0000 (18:07 +0100)] 
BUG/MEDIUM: compression: correctly report zlib_mem

In zlib we track memory usage. The problem is that the way alloc_zlib()
and free_zlib() account for memory is different, resulting in variations
that can lead to negative zlib_mem being reported. The alloc() function
uses the requested size while the free() function uses the pool size. The
difference can happen when pools are shared with other pools of similar
size. The net effect is that zlib_mem can be reported negative with a
slowly decreasing count, and over the long term the limit will not be
enforced anymore.

The fix is simple : let's use the pool size in both cases, which is also
the exact value when it comes to memory usage.

This fix must be backported to 1.5.

10 years agoBUG/MAJOR: namespaces: conn->target is not necessarily a server
Willy Tarreau [Wed, 24 Dec 2014 12:47:55 +0000 (13:47 +0100)] 
BUG/MAJOR: namespaces: conn->target is not necessarily a server

create_server_socket() used to dereference objt_server(conn->target),
but if the target is not a server (eg: a proxy) then it's NULL and we
get a segfault. This can be reproduced with a proxy using "dispatch"
with no server, even when namespaces are disabled, because that code
is not #ifdef'd. The fix consists in first checking if the target is
a server.

This fix does not need to be backported, this is 1.6-only.

10 years agoBUG/MEDIUM: memory: fix freeing logic in pool_gc2()
Willy Tarreau [Mon, 22 Dec 2014 20:40:55 +0000 (21:40 +0100)] 
BUG/MEDIUM: memory: fix freeing logic in pool_gc2()

There's a long-standing bug in pool_gc2(). It tries to protect the pool
against releasing of too many entries but the formula is wrong as it
compares allocated to minavail instead of (allocated-used) to minavail.
Under memory contention, it ends up releasing more than what is granted
by minavail and causes trouble to the dynamic buffer allocator.

This bug is in fact major by itself, but since minavail has never been
used till now, there is no impact at least in mainline. A backport to
1.5 is desired anyway in case any future backport or out-of-tree patch
relies on this.

10 years agoBUG/MAJOR: stream-int: properly check the memory allocation return
Willy Tarreau [Mon, 22 Dec 2014 18:34:00 +0000 (19:34 +0100)] 
BUG/MAJOR: stream-int: properly check the memory allocation return

In stream_int_register_handler(), we call si_alloc_appctx(si) but as a
mistake, instead of checking the return value for a NULL, we test <si>.
This bug was discovered under extreme memory contention (memory for only
two buffers with 500 connections waiting) and after 3 million failed
connections. While it was very hard to produce it, the fix is tagged
major because in theory it could happen when haproxy runs with a very
low "-m" setting preventing from allocating just the few bytes needed
for an appctx. But most users will never be able to trigger it. The
fix was confirmed to address the bug.

This fix must be backported to 1.5.

10 years agoBUG/MAJOR: ns: HAProxy segfault if the cli_conn is not from a network connection
Thierry FOURNIER [Fri, 19 Dec 2014 12:37:11 +0000 (13:37 +0100)] 
BUG/MAJOR: ns: HAProxy segfault if the cli_conn is not from a network connection

The path "MAJOR: namespace: add Linux network namespace support" doesn't
permit to use internal data producer like a "peers synchronisation"
system. The result is a segfault when the internal application starts.

This patch fix the commit b3e54fe387c7c1ea750f39d3029672d640c499f9
It is introduced in 1.6dev version, it doesn't need to be backported.

10 years agoMINOR: map/acl/dumpstats: remove the "Done." message
Thierry FOURNIER [Thu, 18 Dec 2014 14:28:01 +0000 (15:28 +0100)] 
MINOR: map/acl/dumpstats: remove the "Done." message

By convention, the HAProxy CLI doesn't return message if the opration
is sucessfully done. The MAP and ACL returns the "Done." message, an
its noise the output during big MAP or ACL injection.

10 years agoBUG/MEDIUM: config: do not propagate processes between stopped processes
Willy Tarreau [Thu, 18 Dec 2014 13:00:43 +0000 (14:00 +0100)] 
BUG/MEDIUM: config: do not propagate processes between stopped processes

Immo Goltz reported a case of segfault while parsing the config where
we try to propagate processes across stopped frontends (those with a
"disabled" statement). The fix is trivial. The workaround consists in
commenting out these frontends, although not always easy.

This fix must be backported to 1.5.

10 years agoBUG/MINOR: config: fix typo in condition when propagating process binding
Willy Tarreau [Thu, 18 Dec 2014 12:56:26 +0000 (13:56 +0100)] 
BUG/MINOR: config: fix typo in condition when propagating process binding

propagate_processes() has a typo in a condition :

if (!from->cap & PR_CAP_FE)
return;

The return is never taken because each proxy has at least one capability
so !from->cap always evaluates to zero. Most of the time the caller already
checks that <from> is a frontend. In the cases where it's not tested
(use_backend, reqsetbe), the rules have been checked for the context to
be a frontend as well, so in the end it had no nasty side effect.

This should be backported to 1.5.

10 years agoBUG/MINOR: parse: refer curproxy instead of proxy
Godbach [Thu, 18 Dec 2014 07:44:58 +0000 (15:44 +0800)] 
BUG/MINOR: parse: refer curproxy instead of proxy

Since during parsing stage, curproxy always represents a proxy to be operated,
it should be a mistake by referring proxy.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoBUG/MINOR: http: fix typo: "401 Unauthorized" => "407 Unauthorized"
Godbach [Wed, 17 Dec 2014 08:32:05 +0000 (16:32 +0800)] 
BUG/MINOR: http: fix typo: "401 Unauthorized" => "407 Unauthorized"

401 Unauthorized => 407 Unauthorized

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoCLEANUP: epoll: epoll_events should be allocated according to global.tune.maxpollevents
Godbach [Wed, 17 Dec 2014 08:14:26 +0000 (16:14 +0800)] 
CLEANUP: epoll: epoll_events should be allocated according to global.tune.maxpollevents

Willy: commit f2e8ee2b introduced an optimization in the old speculative epoll
code, which implemented its own event cache. It was needed to store that many
events (it was bound to maxsock/4 btw). Now the event cache lives on its own
and we don't need this anymore. And since events are allocated on the kernel
side, we only need to allocate the events we want to return.

As a result, absmaxevents will be not used anymore. Just remove the definition
and the comment of it, replace it with global.tune.maxpollevents. It is also an
optimization of memory usage for large amounts of sockets.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoDOC: httplog does not support 'no'
PiBa-NL [Thu, 11 Dec 2014 20:31:54 +0000 (21:31 +0100)] 
DOC: httplog does not support 'no'

10 years agoBUG/MEDIUM: sample: fix random number upper-bound
Vincent Bernat [Wed, 10 Dec 2014 09:31:37 +0000 (10:31 +0100)] 
BUG/MEDIUM: sample: fix random number upper-bound

random() will generate a number between 0 and RAND_MAX. POSIX mandates
RAND_MAX to be at least 32767. GNU libc uses (1<<31 - 1) as
RAND_MAX.

In smp_fetch_rand(), a reduction is done with a multiply and shift to
avoid skewing the results. However, the shift was always 32 and hence
the numbers were not distributed uniformly in the specified range. We
fix that by dividing by RAND_MAX+1. gcc is smart enough to turn that
into a shift:

    0x000000000046ecc8 <+40>:    shr    $0x1f,%rax

10 years agoDOC: fix a few typos
Godbach [Wed, 10 Dec 2014 02:21:30 +0000 (10:21 +0800)] 
DOC: fix a few typos

include/types/proto_http.h: hwen -> when
include/types/server.h: SRV_ST_DOWN -> SRV_ST_STOPPED
src/backend.c: prefer-current-server -> prefer-last-server

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoBUILD: ssl: use OPENSSL_NO_OCSP to detect OCSP support
Lukas Tribus [Tue, 9 Dec 2014 15:32:51 +0000 (16:32 +0100)] 
BUILD: ssl: use OPENSSL_NO_OCSP to detect OCSP support

Since commit 656c5fa7e859 ("BUILD: ssl: disable OCSP when using
boringssl) the OCSP code is bypassed when OPENSSL_IS_BORINGSSL
is defined. The correct thing to do here is to use OPENSSL_NO_OCSP
instead, which is defined for this exact purpose in
openssl/opensslfeatures.h.

This makes haproxy forward compatible if boringssl ever introduces
full OCSP support with the additional benefit that it links fine
against a OCSP-disabled openssl.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUG/MEDIUM: tcp-checks: disable quick-ack unless next rule is an expect
Willy Tarreau [Mon, 8 Dec 2014 11:11:28 +0000 (12:11 +0100)] 
BUG/MEDIUM: tcp-checks: disable quick-ack unless next rule is an expect

Using "option tcp-checks" without any rule is different from not using
it at all in that checks are sent with the TCP quick ack mode enabled,
causing servers to log incoming port probes.

This commit fixes this behaviour by disabling quick-ack on tcp-checks
unless the next rule exists and is an expect. All combinations were
tested and now the behaviour is as expected : basic port probes are
now doing a SYN-SYN/ACK-RST sequence.

This fix must be backported to 1.5.