Willy Tarreau [Fri, 25 Nov 2016 15:39:17 +0000 (16:39 +0100)]
[RELEASE] Released version 1.7.0
Released version 1.7.0 with the following main changes :
- SCRIPTS: make publish-release also copy the new SPOE doc
- BUILD: http: include types/sample.h in proto_http.h
- BUILD: debug/flags: remove test for SF_COMP_READY
- CONTRIB: debug/flags: add check for SF_ERR_CHK_PORT
- MINOR: lua: add function which return true if the channel is full.
- MINOR: lua: add ip addresses and network manipulation function
- CONTRIB: tcploop: scriptable TCP I/O for debugging purposes
- CONTRIB: tcploop: implement fork()
- CONTRIB: tcploop: implement logging when called with -v
- CONTRIB: tcploop: update the usage output
- CONTRIB: tcploop: support sending plain strings
- CONTRIB: tcploop: don't report failed send() or recv()
- CONTRIB: tcploop: add basic loops via a jump instruction
- BUG/MEDIUM: channel: bad unlikely macro
- CLEANUP: lua: move comment
- CLEANUP: lua: control executed twice
- BUG/MEDIUM: ssl: Store certificate filename in a variable
- BUG/MINOR: ssl: Print correct filename when error occurs reading OCSP
- CLEANUP: ssl: Remove goto after return dead code
- CLEANUP: ssl: Fix bind keywords name in comments
- DOC: ssl: Use correct wording for ca-sign-pass
- CLEANUP: lua: avoid directly calling getsockname/getpeername()
- BUG/MINOR: stick-table: handle out-of-memory condition gracefully
- MINOR: cli: add private pointer and release function
- MEDIUM: lua: Add cli handler for Lua
- BUG/MEDIUM: connection: check the control layer before stopping polling
- DEBUG: connection: mark the closed FDs with a value that is easier to detect
- BUG/MEDIUM: stick-table: fix regression caused by recent fix for out-of-memory
- BUG/MINOR: cli: properly decrement ref count on tables during failed dumps
- BUG/MEDIUM: lua: In some case, the return of sample-fetche is ignored
- MINOR: filters: Add check_timeouts callback to handle timers expiration on streams
- MINOR: spoe: Add 'timeout processing' option to limit time to process an event
- MINOR: spoe: Remove useless 'timeout ack' option
- MINOR: spoe: Add 'option continue-on-error' statement in spoe-agent section
- MINOR: spoe: Add "maxconnrate" and "maxerrrate" statements
- MINOR: spoe: Add "option set-on-error" statement
- MINOR: stats: correct documentation of process ID for typed output
- BUILD: contrib: fix ip6range build on Centos 7
- BUILD: fix build on Solaris 10/11
- BUG/MINOR: cli: fix pointer size when reporting data/transport layer name
- BUG/MINOR: cli: dequeue from the proxy when changing a maxconn
- BUG/MINOR: cli: wake up the CLI's task after a timeout update
- MINOR: connection: add a few functions to report the data and xprt layers' names
- MINOR: connection: add names for transport and data layers
- REORG: cli: split dumpstats.c in src/cli.c and src/stats.c
- REORG: cli: split dumpstats.h in stats.h and cli.h
- REORG: cli: move ssl CLI functions to ssl_sock.c
- REORG: cli: move map and acl code to map.c
- REORG: cli: move show stat resolvers to dns.c
- MINOR: cli: create new function cli_has_level() to validate permissions
- MINOR: server: create new function cli_find_server() to find a server
- MINOR: proxy: create new function cli_find_frontend() to find a frontend
- REORG: cli: move 'set server' to server.c
- REORG: cli: move 'show pools' to memory.c
- REORG: cli: move 'show servers' to proxy.c
- REORG: cli: move 'show sess' to stream.c
- REORG: cli: move 'show backend' to proxy.c
- REORG: cli: move get/set weight to server.c
- REORG: cli: move "show stat" to stats.c
- REORG: cli: move "show info" to stats.c
- REORG: cli: move dump_text(), dump_text_line(), and dump_binary() to standard.c
- REORG: cli: move table dump/clear/set to stick_table.c
- REORG: cli: move "show errors" out of cli.c
- REORG: cli: make "show env" also use the generic keyword registration
- REORG: cli: move "set timeout" to its own handler
- REORG: cli: move "clear counters" to stats.c
- REORG: cli: move "set maxconn global" to its own handler
- REORG: cli: move "set maxconn server" to server.c
- REORG: cli: move "set maxconn frontend" to proxy.c
- REORG: cli: move "shutdown sessions server" to stream.c
- REORG: cli: move "shutdown session" to stream.c
- REORG: cli: move "shutdown frontend" to proxy.c
- REORG: cli: move "{enable|disable} frontend" to proxy.c
- REORG: cli: move "{enable|disable} server" to server.c
- REORG: cli: move "{enable|disable} health" to server.c
- REORG: cli: move "{enable|disable} agent" to server.c
- REORG: cli: move the "set rate-limit" functions to their own parser
- CLEANUP: cli: rename STAT_CLI_* to CLI_ST_*
- CLEANUP: cli: simplify the request parser a little bit
- CLEANUP: cli: remove assignments to st0 and st2 in keyword parsers
- BUILD: server: remove a build warning introduced by latest series
- BUG/MINOR: log-format: uncatched memory allocation functions
- CLEANUP: log-format: useless file and line in json converter
- CLEANUP/MINOR: log-format: unexport functions parse_logformat_var_args() and parse_logformat_var()
- CLEANUP: log-format: fix return code of the function parse_logformat_var()
- CLEANUP: log-format: fix return code of function parse_logformat_var_args()
- CLEANUP: log-format: remove unused arguments
- MEDIUM: log-format: strict parsing and enable fail
- MEDIUM: log-format/conf: take into account the parse_logformat_string() return code
- BUILD: ssl: make the SSL layer build again with openssl 0.9.8
- BUILD: vars: remove a build warning on vars.c
- MINOR: lua: add utility function for check boolean argument
- MINOR: lua: Add tokenize function.
- BUG/MINOR: conf: calloc untested
- MINOR: http/conf: store the use_backend configuration file and line for logs
- MEDIUM: log-format: Use standard HAProxy log system to report errors
- CLEANUP: sample: report "converter" instead of "conv method" in error messages
- BUG: spoe: Fix parsing of SPOE actions in ACK frames
- MINOR: cli: make "show stat" support a proxy name
- MINOR: cli: make "show errors" support a proxy name
- MINOR: cli: make "show errors" capable of dumping only request or response
- BUG/MINOR: freq-ctr: make swrate_add() support larger values
- CLEANUP: counters: move from 3 types to 2 types
- CLEANUP: cfgparse: cascade the warnif_misplaced_* rules
- REORG: tcp-rules: move tcp rules processing to their own file
- REORG: stkctr: move all the stick counters processing to stick-tables.c
- DOC: update the roadmap file with the latest changes
Willy Tarreau [Fri, 25 Nov 2016 15:10:05 +0000 (16:10 +0100)]
REORG: stkctr: move all the stick counters processing to stick-tables.c
Historically we used to have the stick counters processing put into
session.c which became stream.c. But a big part of it is now in
stick-table.c (eg: converters) but despite this we still have all
the sample fetch functions in stream.c
These parts do not depend on the stream anymore, so let's move the
remaining chunks to stick-table.c and have cleaner files.
What remains in stream.c is everything needed to attach/detach
trackers to the stream and to update the counters while the stream
is being processed.
Willy Tarreau [Fri, 25 Nov 2016 14:49:32 +0000 (15:49 +0100)]
REORG: tcp-rules: move tcp rules processing to their own file
There's no more reason to keep tcp rules processing inside proto_tcp.c
given that there is nothing in common there except these 3 letters : tcp.
The tcp rules are in fact connection, session and content processing rules.
Let's move them to "tcp-rules" and let them live their life there.
Willy Tarreau [Fri, 25 Nov 2016 14:16:12 +0000 (15:16 +0100)]
CLEANUP: cfgparse: cascade the warnif_misplaced_* rules
There are 8 functions each repeating what another does and adding one
extra test. We used to have some copy-paste issues in the past due to
this. Instead we now make them simply rely on the previous one and add
the final test. It's much better and much safer. The functions could
be moved to inlines but they're used at a few other locations only,
it didn't make much sense in the end.
Willy Tarreau [Fri, 25 Nov 2016 13:44:52 +0000 (14:44 +0100)]
CLEANUP: counters: move from 3 types to 2 types
We used to have 3 types of counters with a huge overlap :
- listener counters : stats collected for each bind line
- proxy counters : union of the frontend and backend counters
- server counters : stats collected per server
It happens that quite a good part was common between listeners and
proxies due to the frontend counters being updated at the two locations,
and that similarly the server and proxy counters were overlapping and
being updated together.
This patch cleans this up to propose only two types of counters :
- fe_counters: used by frontends and listeners, related to
incoming connections activity
- be_counters: used by backends and servers, related to outgoing
connections activity
This allowed to remove some non-sensical counters from both parts. For
frontends, the following entries were removed :
For backends, this ones was removed : intercepted_req.
While doing this it was discovered that we used to incorrectly report
intercepted_req for backends in the HTML stats, which was always zero
since it's never updated.
Also it revealed a few inconsistencies (which were not fixed as they
are harmless). For example, backends count connections (cum_conn)
instead of sessions while servers count sessions and not connections.
Over the long term, some extra cleanups may be performed by having
some counters update functions touching both the server and backend
at the same time, as well as both the frontend and listener, to
ensure that all sides have all their stats properly filled. The stats
dump will also be able to factor the dump functions by counter types.
Willy Tarreau [Fri, 25 Nov 2016 10:55:10 +0000 (11:55 +0100)]
BUG/MINOR: freq-ctr: make swrate_add() support larger values
Reinhard Vicinus reported that the reported average response times cannot
be larger than 16s due to the double multiply being performed by
swrate_add() which causes an overflow very quickly. Indeed, with N=512,
the highest average value is 16448.
One solution proposed by Reinhard is to turn to long long, but this
involves 64x64 multiplies and 64->32 divides, which are extremely
expensive on 32-bit platforms.
There is in fact another way to avoid the overflow without using larger
integers, it consists in avoiding the multiply using the fact that
x*(n-1)/N = x-(x/N).
Now it becomes possible to store average values as large as 8.4 millions,
which is around 2h18mn.
Interestingly, this improvement also makes the code cheaper to execute
both on 32 and on 64 bit platforms :
Willy Tarreau [Fri, 25 Nov 2016 08:16:37 +0000 (09:16 +0100)]
MINOR: cli: make "show errors" capable of dumping only request or response
When dealing with many proxies, it's hard to spot response errors because
all internet-facing frontends constantly receive attacks. This patch now
makes it possible to demand that only request or response errors are dumped
by appending "request" or "reponse" to the show errors command.
MEDIUM: log-format: Use standard HAProxy log system to report errors
The function log format emit its own error message using Alert(). This
patch replaces this behavior and uses the standard HAProxy error system
(with memprintf).
The benefits are:
- cleaning the log system
- the logformat can ignore the caller (actually the caller must set
a flag designing the caller function).
- Make the usage of the logformat function easy for future components.
Willy Tarreau [Thu, 24 Nov 2016 20:23:28 +0000 (21:23 +0100)]
BUILD: vars: remove a build warning on vars.c
gcc 3.4.6 noticed a possibly unitialized variable in vars.c, and while it
cannot happen the way the function is used, it's surprizing that newer
versions did not report it.
Willy Tarreau [Thu, 24 Nov 2016 19:07:11 +0000 (20:07 +0100)]
BUILD: ssl: make the SSL layer build again with openssl 0.9.8
Commit 1866d6d ("MEDIUM: ssl: Add support for OpenSSL 1.1.0")
introduced support for openssl 1.1.0 and temporarily broke 0.9.8.
In the end the port was not very hard given that the only cause of
build failures were functions supposedly absent from 0.9.8 that in
fact did exist.
Thus, adding a new #if to move these functions for versions older
than 0.9.8 was enough to fix the trouble. It received very light
testing, basically only an SSL bridge decrypting and re-encrypting
traffic, and checking that everything looks right. That said, the
functions specific to 0.9.8 here compared to 1.0.x are only
SSL_SESSION_set1_id_context(), EVP_PKEY_base_id(), and
X509_PUBKEY_get0_param().
MEDIUM: log-format/conf: take into account the parse_logformat_string() return code
This patch takes into account the return code of the parse_logformat_string()
function. Now the configuration parser will fail if the log_format is not
strict.
The log-format function parse_logformat_string() takes file and line
for building parsing logs. These two parameters are embedded in the
struct proxy curproxy, which is the current parsing context.
CLEANUP/MINOR: log-format: unexport functions parse_logformat_var_args() and parse_logformat_var()
Remove export of the fucntion parse_logformat_var_args() and
parse_logformat_var(). These functions are a part of the
logformat parser, and this export is useless.
CLEANUP: log-format: useless file and line in json converter
The caller must log location information, so this information is
provided two times in the log line. The error log is like this:
[ALERT] 327/011513 (14291) : parsing [o3.conf:38]: 'http-response
set-header': Sample fetch <method,json(rrr)> failed with : invalid
args in conv method 'json' : Unexpected input code type at file
'o3.conf', line 38. Allowed value are 'ascii', 'utf8', 'utf8s',
'utf8p' and 'utf8ps'.
This patch removes the second location indication, the the same error
becomes:
[ALERT] 327/011637 (14367) : parsing [o3.conf:38]: 'http-response
set-header': Sample fetch <method,json(rrr)> failed with : invalid
args in conv method 'json' : Unexpected input code type. Allowed
value are 'ascii', 'utf8', 'utf8s', 'utf8p' and 'utf8ps'.
Willy Tarreau [Thu, 24 Nov 2016 15:23:38 +0000 (16:23 +0100)]
CLEANUP: cli: simplify the request parser a little bit
stats_sock_parse_request() was renamed cli_parse_request(). It now takes
an appctx instead of a stream interface, and presets ->st2 to 0 so that
most handlers will not have to set it anymore. The io_handler is set by
default to the keyword's IO handler so that the parser can simply change
it without having to rewrite the new state.
Willy Tarreau [Thu, 24 Nov 2016 13:51:17 +0000 (14:51 +0100)]
REORG: cli: move the "set rate-limit" functions to their own parser
All 4 rate-limit settings were handled at once given that exactly the
same checks are performed on them. In case of missing or incorrect
argument, the detailed supported options are printed with their use
case.
This was the last specific entry in the CLI parser, some additional
cleanup may still be done.
Willy Tarreau [Thu, 24 Nov 2016 11:56:01 +0000 (12:56 +0100)]
REORG: cli: move "{enable|disable} agent" to server.c
Also mention that "set server" is preferred now. Note that these
were the last enable/disable commands in cli.c. Also remove the
now unused expect_server_admin() function.
Willy Tarreau [Wed, 23 Nov 2016 15:50:48 +0000 (16:50 +0100)]
REORG: cli: move "shutdown sessions server" to stream.c
It could be argued that it's between server, stream and session but
at least due to the fact that it operates on streams, its best place
is in stream.c.
Willy Tarreau [Tue, 22 Nov 2016 19:21:23 +0000 (20:21 +0100)]
REORG: cli: make "show env" also use the generic keyword registration
This way we don't have any more state specific to a given yieldable
command. The other commands should be easier to move as they only
involve a parser.
Willy Tarreau [Tue, 22 Nov 2016 18:48:51 +0000 (19:48 +0100)]
REORG: cli: move "show errors" out of cli.c
It really belongs to proto_http.c since it's a dump for HTTP request
and response errors. Note that it's possible that some parts do not
need to be exported anymore since it really is the only place where
errors are manipulated.
Willy Tarreau [Tue, 22 Nov 2016 17:00:53 +0000 (18:00 +0100)]
REORG: cli: move table dump/clear/set to stick_table.c
The table dump code was a horrible mess, with common parts interleaved
all the way to deal with the various actions (set/clear/show). A few
error messages were still incorrect, as the "set" operation did not
update them so they would still report "unknown action" (now fixed).
The action was now passed as a private argument to the CLI keyword
which itself is copied into the appctx private field. It's just an
int cast to a pointer.
Some minor issues were noticed while doing this, for example when dumping
an entry by key, if the key doesn't exist, nothing is printed, not even
the table's header. It's unclear whether this was intentional but it
doesn't really match what is done for data-based dumps. It was left
unchanged for now so that a later fix can be backported if needed.
Enum entries STAT_CLI_O_TAB, STAT_CLI_O_CLR and STAT_CLI_O_SET were
removed.
Willy Tarreau [Tue, 22 Nov 2016 15:36:53 +0000 (16:36 +0100)]
REORG: cli: move "show info" to stats.c
Move the "show info" command to stats.c using the CLI keyword API
to register it on the CLI. The stats_dump_info_to_buffer() function
is now static again. Note, we don't need proto_ssl anymore in cli.c.
Willy Tarreau [Tue, 22 Nov 2016 15:18:05 +0000 (16:18 +0100)]
REORG: cli: move "show stat" to stats.c
Move the "show stat" command to stats.c using the CLI keyword API
to register it on the CLI. The stats_dump_stat_to_buffer() function
is now static again.
Willy Tarreau [Thu, 24 Nov 2016 11:02:29 +0000 (12:02 +0100)]
MINOR: proxy: create new function cli_find_frontend() to find a frontend
Several CLI commands require a frontend, so let's have a function to
look this one up and prepare the appropriate error message and the
appctx's state in case of failure.
Willy Tarreau [Wed, 23 Nov 2016 16:15:08 +0000 (17:15 +0100)]
MINOR: server: create new function cli_find_server() to find a server
Several CLI commands require a server, so let's have a function to
look this one up and prepare the appropriate error message and the
appctx's state in case of failure.
Move map and acl CLI functions to map.c and use the cli keyword API to
register actions on the CLI. Then remove the now unused individual
"add" and "del" keywords.
REORG: cli: split dumpstats.h in stats.h and cli.h
proto/dumpstats.h has been split in 4 files:
* proto/cli.h contains protypes for the CLI
* proto/stats.h contains prototypes for the stats
* types/cli.h contains definition for the CLI
* types/stats.h contains definition for the stats
Willy Tarreau [Wed, 23 Nov 2016 17:00:08 +0000 (18:00 +0100)]
MINOR: connection: add a few functions to report the data and xprt layers' names
These functions will be needed by "show sess" on the CLI, let's make them
globally available. It's important to note that due to the fact that we
still do not set the data and transport layers' names in the structures,
we still have to rely on some exports just to match the pointers. This is
ugly but is preferable to adding many includes since the short-term goal
is to get rid of these tests by having proper names in place.
Willy Tarreau [Thu, 24 Nov 2016 14:35:16 +0000 (15:35 +0100)]
BUG/MINOR: cli: wake up the CLI's task after a timeout update
When the CLI's timeout is reduced, nothing was done to take the task
up to update it. In the past it used to run inside process_stream()
so it used to be refreshed. This is not the case anymore since we have
the appctx so the task needs to be woken up in order to recompute the
new expiration date.
Willy Tarreau [Thu, 24 Nov 2016 14:25:39 +0000 (15:25 +0100)]
BUG/MINOR: cli: dequeue from the proxy when changing a maxconn
The "set maxconn frontend" statement on the CLI tries to dequeue possibly
pending requests, but due to a copy-paste error, they're dequeued on the
CLI's frontend instead of the one being changed.
The impact is very minor as it only means that possibly pending connections
will still have to wait for a previous one to complete before being accepted
when a limit is raised.
Willy Tarreau [Thu, 24 Nov 2016 14:21:26 +0000 (15:21 +0100)]
BUG/MINOR: cli: fix pointer size when reporting data/transport layer name
In dumpstats.c we have get_conn_xprt_name() and get_conn_data_name() to
report the name of the data and transport layers used on a connection.
But when the name is not known, its pointer is reported instead. But the
static char used to report the pointer is too small as it doesn't leave
room for '0x'. Fortunately all subsystems are known so we never trigger
this case.
Willy Tarreau [Tue, 22 Nov 2016 10:50:51 +0000 (11:50 +0100)]
BUILD: contrib: fix ip6range build on Centos 7
Jarno Huuskonen reported that ip6range doesn't build anymore on
Centos 7 (and possibly other distros) due to "in6_u" not being known.
Using s6_addr32 instead of in6_u.u6_addr32 apparently works fine, and
it's also what the Lua code uses so it should be OK.
It defines the variable to set when an error occurred during an event
processing. It will only be set when an error occurred in the scope of the
transaction. As for all other variables define by the SPOE, it will be
prefixed. So, if your variable name is "error" and your prefix is "my_spoe_pfx",
the variable will be "txn.my_spoe_pfx.error".
When set, the variable is the boolean "true". Note that if "option
continue-on-error" is set, the variable is not automatically removed between
events processing.
MINOR: spoe: Add "maxconnrate" and "maxerrrate" statements
"maxconnrate" is the maximum number of connections per second. The SPOE will
stop to open new connections if the maximum is reached and will wait to acquire
an existing one.
"maxerrrate" is the maximum number of errors per second. The SPOE will stop its
processing if the maximum is reached.
These options replace hardcoded macros MAX_NEW_SPOE_APPLETS and
MAX_NEW_SPOE_APPLET_ERRS. We use it to limit SPOE activity, especially when
servers are down..
MINOR: spoe: Add 'option continue-on-error' statement in spoe-agent section
By default, for a specific stream, when an abnormal/unexpected error occurs, the
SPOE is disabled for all the transaction. So if you have several events
configured, such error on an event will disabled all followings. For TCP
streams, this will disable the SPOE for the whole session. For HTTP streams,
this will disable it for the transaction (request and response).
To bypass this behaviour, you can set 'continue-on-error' option in 'spoe-agent'
section. With this option, only the current event will be ignored.
MINOR: spoe: Add 'timeout processing' option to limit time to process an event
It is a way to set the maximum time to wait for a stream to process an event,
i.e to acquire a stream to talk with an agent, to encode all messages, to send
the NOTIFY frame, to receive the corrsponding acknowledgement and to process all
actions. It is applied on the stream that handle the client and the server
sessions.
BUG/MEDIUM: lua: In some case, the return of sample-fetche is ignored
When:
- A Lua action return data and close the channel. The request status
is set to HTTP_MSG_CLOSED for the request and HTTP_MSG_DONE for the
response.
- HAProxy sets the state HTTP_MSG_ERROR. I don't known why, because
there are many line which sets this state.
- A Lua sample-fetch is executed, typically for building the log
line.
- When the Lua sample fetch exits, a control of the data is
executed. If HAProxy is currently parsing the request, the request
is aborted in order to prevent a segfault or sending corrupted
data.
This ast control is executed comparing the state HTTP_MSG_BODY. When
this state is reached, the request is parsed and no error are
possible. When the state is < than HTTP_MSG_BODY, the parser is
running.
Unfortunately, the code HTTP_MSG_ERROR is just < HTTP_MSG_BODY. When
we are in error, we want to terminate the execution of Lua without
error.
Willy Tarreau [Fri, 18 Nov 2016 18:17:40 +0000 (19:17 +0100)]
BUG/MINOR: cli: properly decrement ref count on tables during failed dumps
Gernot Pörner reported some constant leak of ref counts for stick tables
entries. It happens that this leak was not at all in the regular traffic
path but on the "show table" path. An extra ref count was taken during
the dump if the output had to be paused, and it was released upon clean
termination or an error detected in the I/O handler. But the release
handler didn't do it, while it used to properly do it for the sessions
dump.
Willy Tarreau [Fri, 18 Nov 2016 17:21:39 +0000 (18:21 +0100)]
BUG/MEDIUM: stick-table: fix regression caused by recent fix for out-of-memory
Commit ef8f4fe ("BUG/MINOR: stick-table: handle out-of-memory condition
gracefully") unfortunately got trapped by a pointer operation. Replacing
ts = poll_alloc() + size;
with :
ts = poll_alloc();
ts += size;
Doesn't give the same result because pool_alloc() is void while ts is a
struct stksess*. So now we don't access the same places, which is visible
in certain stick-table scenarios causing a crash.
Willy Tarreau [Thu, 17 Nov 2016 13:22:52 +0000 (14:22 +0100)]
DEBUG: connection: mark the closed FDs with a value that is easier to detect
Setting an FD to -1 when closed isn't the most easily noticeable thing
to do when we're chasing accidental reuse of a stale file descriptor.
Instead set it to that large a negative value that it will overflow the
fdtab and provide an analysable core at the moment the issue happens.
Care was taken to ensure it doesn't overflow nor change sign on 32-bit
machines when multiplied by fdtab, and that it also remains negative for
the various checks that exist. The value equals 0xFDDEADFD which happens
to be easily spotted in a debugger.
Willy Tarreau [Thu, 17 Nov 2016 11:05:13 +0000 (12:05 +0100)]
BUG/MEDIUM: connection: check the control layer before stopping polling
The bug described in commit 568743a ("BUG/MEDIUM: stream-int: completely
detach connection on connect error") was not a stream-interface layer bug
but a connection layer bug. There was exactly one place in the code where
we could change a file descriptor's status without first checking whether
it is valid or not, it was in conn_stop_polling(). This one is called when
the polling status is changed after an update, and calls fd_stop_both even
if we had already closed the file descriptor :
The problem with the previous fix above is that it break the http_proxy mode
and possibly even some Lua parts and peers to a certain extent ; all outgoing
connections where the target address is initially copied into the outgoing
connection which experience a retry would use a random outgoing address after
the retry because closing and detaching the connection causes the target
address to be lost. This was attempted to be addressed by commit 0857d7a
("BUG/MAJOR: stream: properly mark the server address as unset on connect
retry") but it used to only solve the most visible effect and not the root
cause.
Prior to this fix, it was possible to cause this config to keep CLOSE_WAIT
for as long as it takes to expire a client or server timeout (note the
missing client timeout) :
listen test
mode http
bind :8002
server s1 127.0.0.1:8001
$ tcploop 8001 L0 W N20 A R P100 S:"HTTP/1.1 200 OK\r\nContent-length: 0\r\n\r\n" &
$ tcploop 8002 N200 C T W S:"GET / HTTP/1.0\r\n\r\n" O P10000 K
With this patch, these CLOSE_WAIT properly vanish when both processes leave.
This commit reverts the two fixes above and replaces them with the proper
fix in connection.h. It must be backported to 1.6 and 1.5. Thanks to
Robson Roberto Souza Peixoto for providing very detailed traces showing
some obvious inconsistencies leading to finding this bug.
MINOR: cli: add private pointer and release function
This pointer will be used for storing private context. With this,
the same executed function can handle more than one keyword. This
will be very useful for creation Lua cli bindings.
The release function is called when the command is terminated (give
back the hand to the prompt) or when the session is broken (timeout
or client closed).
In case `pool_alloc2()` returns NULL, propagate the condition to the
caller. This could happen when limiting the amount of memory available
for HAProxy with `-m`.
Bertrand Jacquin [Sun, 13 Nov 2016 16:37:13 +0000 (16:37 +0000)]
BUG/MEDIUM: ssl: Store certificate filename in a variable
Before this change, trash is being used to create certificate filename
to read in care Mutli-Cert are in used. But then ssl_sock_load_ocsp()
modify trash leading to potential wrong information given in later error
message.
This also blocks any further use of certificate filename for other
usage, like ongoing patch to support Certificate Transparency handling
in Multi-Cert bundle.
The unlikely macro doesn't take in acount the condition, but only one
variable.
Must be backported in 1.6
[wt: with gcc 3.x, unlikely(x) is defined as __builtin_expect((x) != 0, 0)
so the condition is wrong for negative numbers, which correspond to the
case where bi_getblk_nc() has reached the end of the buffer and the
channel is already closed. With gcc 4.x, the output is cast to unsigned long
so the <=0 will not match negative values either. This is only used in Lua
for now so that may explain why it hasn't hit yet]
Willy Tarreau [Sat, 12 Nov 2016 17:54:20 +0000 (18:54 +0100)]
CONTRIB: tcploop: add basic loops via a jump instruction
This one jumps back to the oldest post-fork and post-accept action,
so it allows to recv(), pause() and send() in loops after a fork()
and an accept() for example. This is handy for bugs that reproduce
once in a while or to keep idle connections working.