]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
4 years agoMEDIUM: connections: Introduce a new XPRT method, start().
Olivier Houchard [Fri, 5 Mar 2021 22:37:48 +0000 (23:37 +0100)] 
MEDIUM: connections: Introduce a new XPRT method, start().

Introduce a new XPRT method, start(). The init() method will now only
initialize whatever is needed for the XPRT to run, but any action the XPRT
has to do before being ready, such as handshakes, will be done in the new
start() method. That way, we will be sure the full stack of xprt will be
initialized before attempting to do anything.
The init() call is also moved to conn_prepare(). There's no longer any reason
to wait for the ctrl to be ready, any action will be deferred until start(),
anyway. This means conn_xprt_init() is no longer needed.

4 years agoMINOR: raw_sock: Add a close method.
Olivier Houchard [Sat, 13 Mar 2021 23:34:49 +0000 (00:34 +0100)] 
MINOR: raw_sock: Add a close method.

Add a close() method, that explicitely cancels any subscription on the
connection, in preparation for future evolutions.

4 years agoCLEANUP: Fix a typo in fix_is_valid description
Christopher Faulet [Thu, 18 Mar 2021 16:40:56 +0000 (17:40 +0100)] 
CLEANUP: Fix a typo in fix_is_valid description

MsgType tag was misspelled.

4 years agoBUG/MINOR: protocol: add missing support of dgram unix socket.
Emeric Brun [Thu, 18 Mar 2021 15:52:17 +0000 (16:52 +0100)] 
BUG/MINOR: protocol: add missing support of dgram unix socket.

The proto "uxdg" (UNIX DGRAM) was not declared, causing an error trying
to put a socket unix on "dgram-bind" into a log-forward section.

This patch introduces the missing "uxdg" protocol by adding proto_uxdg.c
which was fully created based on the code available for the other
protocols.

This patch should be backported to version 2.3 and above.

4 years agoMINOR: server: support keyword proto in 'add server' cli
Amaury Denoyelle [Fri, 12 Mar 2021 17:03:27 +0000 (18:03 +0100)] 
MINOR: server: support keyword proto in 'add server' cli

Allow to specify the mux proto for a dynamic server. It must be
compatible with the backend mode to be accepted. The reg-tests has been
extended for this error case.

4 years agoMINOR: server: enable standard options for dynamic servers
Amaury Denoyelle [Tue, 9 Mar 2021 16:36:23 +0000 (17:36 +0100)] 
MINOR: server: enable standard options for dynamic servers

Enable a subset of server options to be used as keywords on the CLI
command 'add server'. These options are safe and can be applied
flawlessly for a dynamic server.

4 years agoREGTESTS: implement test for 'add server' cli
Amaury Denoyelle [Fri, 12 Mar 2021 09:45:12 +0000 (10:45 +0100)] 
REGTESTS: implement test for 'add server' cli

Write a regtest for the cli command 'add server'. This test will execute
some invalid commands and validates the reported error. A client will
then try to connect to a dynamic server just created and activated.

4 years agoMEDIUM: server: implement 'add server' cli command
Amaury Denoyelle [Mon, 8 Mar 2021 16:13:32 +0000 (17:13 +0100)] 
MEDIUM: server: implement 'add server' cli command

Add a new cli command 'add server'. This command is used to create a new
server at runtime attached on an existing backend. The syntax is the
following one :

$ add server <be_name>/<sv_name> [<kws>...]

This command is only available through experimental mode for the moment.

Currently, no server keywords are supported. They will be activated
individually when deemed properly functional and safe.

Another limitation is put on the backend load-balancing algorithm. The
algorithm must use consistent hashing to guarantee a minimal
reallocation of existing connections on the new server insertion.

4 years agoMINOR: stats: export function to allocate extra proxy counters
Amaury Denoyelle [Thu, 18 Mar 2021 14:48:14 +0000 (15:48 +0100)] 
MINOR: stats: export function to allocate extra proxy counters

Remove static qualifier on stats_allocate_proxy_counters_internal. This
function will be used to allocate extra counters at runtime for dynamic
servers.

4 years agoMINOR: server: prepare parsing for dynamic servers
Amaury Denoyelle [Mon, 8 Mar 2021 16:08:01 +0000 (17:08 +0100)] 
MINOR: server: prepare parsing for dynamic servers

Prepare the server parsing API to support dynamic servers.
- define a new parsing flag to be used for dynamic servers
- each keyword contains a new field dynamic_ok to indicate if it can be
  used for a dynamic server. For now, no keyword are supported.
- do not copy settings from the default server for a new dynamic server.
- a dynamic server is created in a maintenance mode and requires an
  explicit 'enable server' command.
- a new server flag named SRV_F_DYNAMIC is created. This flag is set for
  all servers created at runtime. It might be useful later, for example
  to know if a server can be purged.

4 years agoREORG: server: use flags for parse_server
Amaury Denoyelle [Mon, 8 Mar 2021 15:36:46 +0000 (16:36 +0100)] 
REORG: server: use flags for parse_server

Modify the API of parse_server function. Use flags to describe the type
of the parsed server instead of discrete arguments. These flags can be
used to specify if a server/default-server/server-template is parsed.
Additional parameters are also specified (parsing of the address
required, resolve of a name must be done immediately).

It is now unneeded to use strcmp on args[0] in parse_server. Also, the
calls to parse_server are more explicit thanks to the flags.

4 years agoREORG: server: attach servers in parse_server
Amaury Denoyelle [Mon, 8 Mar 2021 15:35:54 +0000 (16:35 +0100)] 
REORG: server: attach servers in parse_server

Move server linked into proxy backend list outside of _srv_parse_init to
parse_server.

This is groundwork for dynamic servers support. There will be two
differences in case of a dynamic server :
- the server will be attached to the proxy list only at the very end of the
  operations when everything is ok
- the server will be directly attached to the end of the server proxy
  list

4 years agoREORG: server: rename internal functions from parse_server
Amaury Denoyelle [Wed, 17 Mar 2021 13:25:39 +0000 (14:25 +0100)] 
REORG: server: rename internal functions from parse_server

Use a standard convention for the functions used through parse_server.
Use the prefix _srv_parse and specify their private scope in a comment.

4 years agoREORG: server: move alert traces in parse_server
Amaury Denoyelle [Mon, 8 Mar 2021 10:20:52 +0000 (11:20 +0100)] 
REORG: server: move alert traces in parse_server

Move every ha_alert calls in parsing functions into parse_server.
Parsing functions now support a pointer-to-string argument which will be
allocated with an error message if needed via memprintf.

parse_server has then the responsibility to display errors with ha_alert.

This is groundwork for dynamic server. No traces should be printed on
stderr as a response to a cli command. cli_err will replace ha_alert in
this case.

4 years agoREORG: server: split parse_server
Amaury Denoyelle [Mon, 8 Mar 2021 09:29:33 +0000 (10:29 +0100)] 
REORG: server: split parse_server

The huge parse_server function is splitted into two smaller ones.
* _srv_parse_init allocates a new server instance and parses the address
  parameter
* _srv_parse_kw parse the current server keyword

This simplify a bit the parse_server function. Besides, it will be
useful for dynamic server creation.

4 years agoMINOR: server: remove fastinter from mistyped kw list
Amaury Denoyelle [Fri, 12 Mar 2021 15:04:00 +0000 (16:04 +0100)] 
MINOR: server: remove fastinter from mistyped kw list

This keyword is already present in server kw list from checks.c.

4 years agoREORG: server: move keywords in srv_kws
Amaury Denoyelle [Wed, 10 Mar 2021 15:36:02 +0000 (16:36 +0100)] 
REORG: server: move keywords in srv_kws

Move server-keyword hardcoded in parse_server into the srv_kws list of
server.c. Now every server keywords is checked through srv_find_kw. This
has the effect to reduce the size of parse_server. As a side-effect,
common kw list can be reduced.

This change has been made to be able to quickly discard these keywords
in case of a dynamic server.

4 years agoMINOR: cfgparse: always alloc idle conns task
Amaury Denoyelle [Mon, 8 Mar 2021 16:31:39 +0000 (17:31 +0100)] 
MINOR: cfgparse: always alloc idle conns task

The idle conn task is is a global task used to cleanup backend
connections marked for deletion. Previously, it was only only allocated
if at least one server in the configuration has idle connections.

This assumption won't be valid anymore when new servers can be created
at runtime with idle connections. Always allocate the global idle conn
task.

4 years agoREORG: server: add a free server function
Amaury Denoyelle [Tue, 16 Mar 2021 16:20:15 +0000 (17:20 +0100)] 
REORG: server: add a free server function

Create a new server function named free_server. It can be used to
deallocate a server and its member.

4 years agoMINOR: cli: implement experimental-mode
Amaury Denoyelle [Thu, 18 Mar 2021 14:32:53 +0000 (15:32 +0100)] 
MINOR: cli: implement experimental-mode

Experimental mode is similar to expert-mode. It can be used to access to
features still in development.

4 years agoMINOR: mworker/cli: alert the user if we enabled a master CLI but not the master...
Eric Salama [Tue, 16 Mar 2021 14:11:17 +0000 (15:11 +0100)] 
MINOR: mworker/cli: alert the user if we enabled a master CLI but not the master-worker mode

Declaring a master CLI socket without activating the master-worker mode
is likely a user error, so we issue a warning.

This patch can be backported as far as 1.8.

4 years agoMINOR/BUG: mworker/cli: do not use the unix_bind prefix for the master CLI socket
Eric Salama [Tue, 16 Mar 2021 14:12:17 +0000 (15:12 +0100)] 
MINOR/BUG: mworker/cli: do not use the unix_bind prefix for the master CLI socket

If the configuration file contains a 'unix-bind prefix' directive, and
if we use the -S option and specify a UNIX socket path, the path of the
socket will be prepended with the value of the unix-bind prefix.

For instance, if we have 'unix-bind prefix /tmp/sockets/' and we use
'-S /tmp/master-socket' on the command line, we will get this error:

Starting proxy MASTER:
cannot bind UNIX socket (No such file or directory) [/tmp/sockets/tmp/master-socket]

So this patch adds an exception, and will ignore the unix-bind prefix
for the master CLI socket.

This patch can be backported as far as 1.9.

4 years agoMINOR: freq_ctr/threads: relax when failing to update a sliding window value
Willy Tarreau [Wed, 17 Mar 2021 18:22:03 +0000 (19:22 +0100)] 
MINOR: freq_ctr/threads: relax when failing to update a sliding window value

The swrate_add* functions would sping fast on a failed CAS, better place
a cpu_relax() call there to reduce contention if any.

4 years agoBUG/MINOR: freq_ctr/threads: make use of the last updated global time
Willy Tarreau [Wed, 17 Mar 2021 18:10:23 +0000 (19:10 +0100)] 
BUG/MINOR: freq_ctr/threads: make use of the last updated global time

The freq counters were using the thread's own time as the start of the
current period. The problem is that in case of contention, it was
occasionally possible to perform non-monotonic updates on the edge of
the next second, because if the upfront thread updates a counter first,
it causes a rotation, then the second thread loses the race from its
older time, and tries again, and detects a different time again, but
in the past so it only updates the counter, then a third thread on the
new date would detect a change again, thus provoking a rotation again.

The effect was triple:
  - rare loss of stored values during certain transitions from one
    period to the next one, causing counters to report 0
  - half of the threads forced to go through the slow path every second
  - difficult convergence when using many threads where the CAS can fail
    a lot and we can observe N(N-1) attempts for N threads to complete

This patch fixes this issue in two ways:
  - first, it now makes use og the monotonic global_now value which also
    happens to be volatile and to carry the latest known time; this way
    time will never jump backwards anymore and only the first thread
    updates it on transition, the other ones do not need to.

  - second, re-read the time in the loop after each failure, because
    if the date changed in the counter, it means that one thread knows
    a more recent one and we need to update. In this case if it matches
    the new current second, the fast path is usable.

This patch relies on previous patch "MINOR: time: export the global_now
variable" and must be backported as far as 1.8.

4 years agoMINOR: time: export the global_now variable
Willy Tarreau [Wed, 17 Mar 2021 17:52:18 +0000 (18:52 +0100)] 
MINOR: time: export the global_now variable

This is the process-wide monotonic time that is used to update each
thread's own time. It may be required at a few places where a strictly
monotonic clock is required such as freq_ctr. It will be have to be
backported as a dependency of a forthcoming fix.

4 years agoBUG/MINOR: resolvers: Add missing case-insensitive comparisons of DNS hostnames
Christopher Faulet [Tue, 16 Mar 2021 10:21:04 +0000 (11:21 +0100)] 
BUG/MINOR: resolvers: Add missing case-insensitive comparisons of DNS hostnames

DNS hostname comparisons were fixed to be case-insensitive (see b17b88487
"BUG/MEDIUM: dns: Consider the fact that dns answers are
case-insensitive"). However 2 comparisons are still case-sensitive.

This patch must be backported as far as 1.8.

4 years agoMINOR: cfgparse/proxy: also support spelling fixes on options
Willy Tarreau [Mon, 15 Mar 2021 10:11:55 +0000 (11:11 +0100)] 
MINOR: cfgparse/proxy: also support spelling fixes on options

Some are not always easy to spot with "chk" vs "check" or hyphens at
some places and not at others. Now entering "option http-close" properly
suggests "httpclose" and "option tcp-chk" suggests "tcp-check". There's
no need to consider the proxy's capabilities, what matters is to figure
what related word the user tried to spell, and there are not that many
options anyway.

4 years agoMINOR: cli: limit spelling suggestions to 5
Willy Tarreau [Mon, 15 Mar 2021 09:38:04 +0000 (10:38 +0100)] 
MINOR: cli: limit spelling suggestions to 5

There's no need to suggest up to 10 entries for matching keywords,
most of the times 5 are plenty, and will be more readable.

4 years agoMINOR: cli: sort the suggestions by order of relevance
Willy Tarreau [Mon, 15 Mar 2021 09:35:04 +0000 (10:35 +0100)] 
MINOR: cli: sort the suggestions by order of relevance

Now the suggested keywords are sorted with the most relevant ones first
instead of scanning them all in registration order and only dumping the
proposed ones:

- "tra"
   trace <module> [cmd [args...]] : manage live tracing
   operator       : lower the level of the current CLI session to operator
   user           : lower the level of the current CLI session to user
   show trace [<module>] : show live tracing state

- "pool"
   show pools     : report information about the memory pools usage
   add acl        : add acl entry
   del map        : delete map entry
   user           : lower the level of the current CLI session to user
   del acl        : delete acl entry

- "sh ta"
   show stat      : report counters for each proxy and server [desc|json|no-maint|typed|up]*
   show tasks     : show running tasks
   set table [id] : update or create a table entry's data
   show table [id]: report table usage stats or dump this table's contents
   trace <module> [cmd [args...]] : manage live tracing

- "sh state"
   show stat      : report counters for each proxy and server [desc|json|no-maint|typed|up]*
   set table [id] : update or create a table entry's data
   show table [id]: report table usage stats or dump this table's contents
   show servers state [id]: dump volatile server information (for backend <id>)
   show sess [id] : report the list of current sessions or dump this session

4 years agoMINOR: cli: improve fuzzy matching to work on all remaining words at once
Willy Tarreau [Mon, 15 Mar 2021 09:00:29 +0000 (10:00 +0100)] 
MINOR: cli: improve fuzzy matching to work on all remaining words at once

Till now the fuzzy matching would only work on the same number of words,
but this doesn't account for commands like "show servers conn" which
involve 3 words and were not proposed when entering only "show conn".
Let's improve the situation by building the two fingerprints separately
for the correct keyword sequence and the entered one, then compare them.
This can result in slightly larger variations due to the different string
lengths but is easily compensated for. Thanks to this, we can now see
"show servers conn" when entering "show conn", and the following choices
are relevant to correct typos:

- "show foo"
   show sess [id] : report the list of current sessions or dump this session
   show info      : report information about the running process [desc|json|typed]*
   show env [var] : dump environment variables known to the process
   show fd [num] : dump list of file descriptors in use
   show pools     : report information about the memory pools usage

- "show stuff"
   show sess [id] : report the list of current sessions or dump this session
   show info      : report information about the running process [desc|json|typed]*
   show stat      : report counters for each proxy and server [desc|json|no-maint|typed|up]*
   show fd [num] : dump list of file descriptors in use
   show tasks     : show running tasks

- "show stafe"
   show sess [id] : report the list of current sessions or dump this session
   show stat      : report counters for each proxy and server [desc|json|no-maint|typed|up]*
   show fd [num] : dump list of file descriptors in use
   show table [id]: report table usage stats or dump this table's contents
   show tasks     : show running tasks

- "show state"
   show stat      : report counters for each proxy and server [desc|json|no-maint|typed|up]*
   show servers state [id]: dump volatile server information (for backend <id>)

It's still visible that the shorter ones continue to easily match, such
as "show sess" not having much in common with "show foo" but what matters
is that the best candidates are definitely relevant. Probably that listing
them in match order would further help.

4 years agoMINOR: tools: do not sum squares of differences for word fingerprints
Willy Tarreau [Mon, 15 Mar 2021 08:44:53 +0000 (09:44 +0100)] 
MINOR: tools: do not sum squares of differences for word fingerprints

While sums of squares usually give excellent results in fixed-sise
patterns, they don't work well to compare different sized ones such
as when some sub-words are missing, because a word such as "server"
contains "er" twice, which will rsult in an extra distance of at
least 4 for just this e->r transition compared to another one missing
it. This is one of the main reasons why "show conn" only proposes
"show info" on the CLI. Maybe an improved approach consisting in
using squares only for exact same lengths would work, but it would
still make it difficult to spot reversed characters.

4 years agoMINOR: tools: improve word fingerprinting by counting presence
Willy Tarreau [Mon, 15 Mar 2021 08:34:27 +0000 (09:34 +0100)] 
MINOR: tools: improve word fingerprinting by counting presence

The distance between two words can be high due to a sub-word being missing
and in this case it happens that other totally unrealted words are proposed
because their average score looks lower thanks to being shorter. Here we're
introducing the notion of presence of each character so that word sequences
that contain existing sub-words are favored against the shorter ones having
nothing in common. In addition we do not distinguish being/end from a
regular delimitor anymore. That made it harder to spot inverted words.

4 years agoBUG/MINOR: cfgparse: use the GLOBAL not LISTEN keywords list for spell checking
Willy Tarreau [Mon, 15 Mar 2021 08:12:41 +0000 (09:12 +0100)] 
BUG/MINOR: cfgparse: use the GLOBAL not LISTEN keywords list for spell checking

In commit a0e8eb8ca ("MINOR: cfgparse: suggest correct spelling for
unknown words in global section") we got the ability to locate a better
matching word in case of error. But it mistakenly used the CFG_LISTEN
class of words instead of CFG_GLOBAL, resulting in proposing unsuitable
matches in addition to the long hard-coded list. Now, "tune.dh-param"
correctly proposes "tune.ssl.default-dh-param".

No backport is needed.

4 years agoBUG/MEDIUM: cli: fix "help" crashing since recent spelling fixes
Willy Tarreau [Sat, 13 Mar 2021 11:25:43 +0000 (12:25 +0100)] 
BUG/MEDIUM: cli: fix "help" crashing since recent spelling fixes

I somehow managed to re-break the "help" command in b736458bf ("MEDIUM:
cli: apply spelling fixes for known commands before listing them")
after fixing it once. A null-deref happens when checking the args
early in the processing.

No backport is needed as this was introduced in 2.4-dev12.

4 years ago[RELEASE] Released version 2.4-dev12 v2.4-dev12
Willy Tarreau [Sat, 13 Mar 2021 10:48:28 +0000 (11:48 +0100)] 
[RELEASE] Released version 2.4-dev12

Released version 2.4-dev12 with the following main changes :
    - CLEANUP: connection: Use `VAR_ARRAY` in `struct tlv` definition
    - CLEANUP: connection: Remove useless test for NULL before calling `pool_free()`
    - CLEANUP: connection: Use istptr / istlen for proxy_unique_id
    - MINOR: connection: Use a `struct ist` to store proxy_authority
    - CLEANUP: connection: Consistently use `struct ist` to process all TLV types
    - BUILD: task: fix build at -O0 with threads disabled
    - BUILD: bug: refine HA_LINK_ERROR() to only be used on gcc and derivatives
    - CLEANUP: config: make the cfg_keyword parsers take a const for the defproxy
    - BUILD: connection: do not use VAR_ARRAY in struct tlv
    - BUG/MEDIUM: session: NULL dereference possible when accessing the listener
    - MINOR: build: force CC to set a return code when probing options
    - CLEANUP: stream: rename a few remaining occurrences of "stream *sess"
    - BUG/MEDIUM: resolvers: handle huge responses over tcp servers.
    - CLEANUP: config: also address the cfg_keyword API change in the compression code
    - BUG/MEDIUM: ssl: properly remove the TASK_HEAVY flag at end of handshake
    - BUG/MINOR: sample: Rename SenderComID/TargetComID to SenderCompID/TargetCompID
    - MINOR: task: give the scheduler a bit more flexibility in the runqueue size
    - OPTIM: task: automatically adjust the default runqueue-depth to the threads
    - BUG/MINOR: connection: Missing QUIC initialization
    - BUG/MEDIUM: stick-tables: fix ref counter in table entry using multiple http tracksc.
    - BUILD: atomic/arm64: force the register pairs to use in __ha_cas_dw()
    - BUG/MEDIUM: filters: Set CF_FL_ANALYZE on channels when filters are attached
    - BUG/MINOR: tcpcheck: Update .health threshold of agent inside an agent-check
    - BUG/MINOR: proxy/session: Be sure to have a listener to increment its counters
    - BUG/MINOR: tcpcheck: Fix double free on error path when parsing tcp/http-check
    - BUG/MINOR: server-state: properly handle the case where the base is not set
    - BUG/MINOR: server-state: use the argument, not the global state
    - CLEANUP: tcp-rules: add missing actions in the tcp-request error message
    - CLEANUP: vars: make the error message clearer on missing arguments for set-var
    - CLEANUP: http-rules: remove the unexpected comma before the list of action keywords
    - CLEANUP: actions: the keyword must always be const from the rule
    - MINOR: tools: add simple word fingerprinting to find similar-looking words
    - MINOR: cfgparse: add cfg_find_best_match() to suggest an existing word
    - MINOR: cfgparse: suggest correct spelling for unknown words in proxy sections
    - MINOR: cfgparse: suggest correct spelling for unknown words in global section
    - MINOR: cfgparse/server: try to fix spelling mistakes on server lines
    - MINOR: cfgparse/bind: suggest correct spelling for unknown bind keywords
    - MINOR: actions: add a function to suggest an action ressembling a given word
    - MINOR: http-rules: suggest approaching action names on mismatch
    - MINOR: tcp-rules: suggest approaching action names on mismatch
    - BUG/MINOR: cfgparse/server: increment the extra keyword counter one at a time
    - Revert "BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record"
    - BUG/MINOR: resolvers: Consider server to have no IP on DNS resolution error
    - BUG/MINOR: resolvers: Reset server address on DNS error only on status change
    - BUG/MINOR: resolvers: Unlink DNS resolution to set RMAINT on SRV resolution
    - BUG/MEDIUM: resolvers: Don't set an address-less server as UP
    - BUG/MEDIUM: resolvers: Fix the loop looking for an existing ADD item
    - MINOR: resolvers: new function find_srvrq_answer_record()
    - BUG/MINOR; resolvers: Ignore DNS resolution for expired SRV item
    - BUG/MEDIUM: resolvers: Trigger a DNS resolution if an ADD item is obsolete
    - MINOR: resolvers: Use a function to remove answers attached to a resolution
    - MINOR: resolvers: Purge answer items when a SRV resolution triggers an error
    - MINOR: resolvers: Add function to change the srv status based on SRV resolution
    - MINOR: resolvers: Directly call srvrq_update_srv_state() when possible
    - BUG/MEDIUM: resolvers: Don't release resolution from a requester callbacks
    - BUG/MEDIUM: resolvers: Skip DNS resolution at startup if SRV resolution is set
    - MINOR: resolvers: Use milliseconds for cached items in resolver responses
    - MINOR: resolvers: Don't try to match immediatly renewed ADD items
    - CLEANUP: resolvers: Use ha_free() in srvrq_resolution_error_cb()
    - CLEANUP: resolvers: Perform unsafe loop on requester list when possible
    - BUG/MINOR: cli: make sure "help", "prompt", "quit" are enabled at master level
    - CLEANUP: cli: fix misleading comment and better indent the access level flags
    - MINOR: cli: set the ACCESS_MASTER* bits on the master bind_conf
    - MINOR: cli: test the appctx level for master access instead of comparing pointers
    - MINOR: cli: print the error message in the parser function itself
    - MINOR: cli: filter the list of commands to the matching part
    - MEDIUM: cli: apply spelling fixes for known commands before listing them
    - MINOR: tools: add the ability to update a word fingerprint
    - MINOR: cli: apply the fuzzy matching on the whole command instead of words
    - CLEANUP: cli: rename MAX_STATS_ARGS to MAX_CLI_ARGS
    - CLEANUP: cli: rename the last few "stats_" to "cli_"
    - CLEANUP: task: make sure tasklet handlers always indicate their statuses
    - CLEANUP: assorted typo fixes in the code and comments

4 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Fri, 12 Mar 2021 16:53:34 +0000 (21:53 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 20th iteration of typo fixes

4 years agoCLEANUP: task: make sure tasklet handlers always indicate their statuses
Willy Tarreau [Sat, 13 Mar 2021 10:30:19 +0000 (11:30 +0100)] 
CLEANUP: task: make sure tasklet handlers always indicate their statuses

When tasklets were derived from tasks, there was no immediate need for
the scheduler to know their status after execution, and in a spirit of
simplicity they just started to always return NULL. The problem is that
it simply prevents the scheduler from 1) accounting their execution time,
and 2) keeping track of their current execution status. Indeed, a remote
wake-up could very well end up manipulating a tasklet that's currently
being executed. And this is the reason why those handlers have to take
the idle lock before checking their context.

In 2.5 we'll take care of making tasklets and tasks work more similarly,
but trouble is to be expected if we continue to propagate the trend of
returning NULL everywhere, especially if some fixes relying on a stricter
model later need to be backported. For this reason this patch updates all
known tasklet handlers to make them return NULL only when the tasklet was
freed. It has no effect for now and isn't even guaranteed to always be
100% safe but it puts the code into the right direction for this.

4 years agoCLEANUP: cli: rename the last few "stats_" to "cli_"
Willy Tarreau [Sat, 13 Mar 2021 10:00:33 +0000 (11:00 +0100)] 
CLEANUP: cli: rename the last few "stats_" to "cli_"

There were still a very small list of functions, variables and fields
called "stats_" while they were really purely CLI-centric. There's the
frontend called "stats_fe" in the global section, which instantiates a
"cli_applet" called "<CLI>" so it was renamed "cli_fe".

The "alloc_stats_fe" function cas renamed to "cli_alloc_fe" which also
better matches the naming convention of all cli-specific functions.

Finally the "stats_permission_denied_msg" used to return an error on
the CLI was renamed "cli_permission_denied_msg".

Now there's no more "stats_something" that designates the CLI.

4 years agoCLEANUP: cli: rename MAX_STATS_ARGS to MAX_CLI_ARGS
Willy Tarreau [Sat, 13 Mar 2021 09:59:23 +0000 (10:59 +0100)] 
CLEANUP: cli: rename MAX_STATS_ARGS to MAX_CLI_ARGS

This is the number of args accepted on a command received on the CLI,
is has long been totally independent of stats and should not carry
this misleading "stats" name anymore.

4 years agoMINOR: cli: apply the fuzzy matching on the whole command instead of words
Willy Tarreau [Fri, 12 Mar 2021 18:01:59 +0000 (19:01 +0100)] 
MINOR: cli: apply the fuzzy matching on the whole command instead of words

Now instead of comparing words at an exact position, we build a fingerprint
made of all of them, so that we can check for them in any position. For
example, "show conn serv" finds "show servers conn" and that "set servers
maxconn" proposes both "set server" and "set maxconn servers".

4 years agoMINOR: tools: add the ability to update a word fingerprint
Willy Tarreau [Fri, 12 Mar 2021 17:59:31 +0000 (18:59 +0100)] 
MINOR: tools: add the ability to update a word fingerprint

Instead of making a new one from scratch, let's support not wiping the
existing fingerprint and updating it, and to do the same char by char.
The word-by-word one will still result in multiple beginnings and ends,
but that will accurately translate word boundaries. The char-based one
has more flexibility and requires that the caller maintains the previous
char to indicate the transition, which also allows to insert delimiters
for example.

4 years agoMEDIUM: cli: apply spelling fixes for known commands before listing them
Willy Tarreau [Fri, 12 Mar 2021 17:24:46 +0000 (18:24 +0100)] 
MEDIUM: cli: apply spelling fixes for known commands before listing them

Entering "show tls" would still emit 35 entries. By measuring the distance
between all unknown words and the candidates, we can sort them and pick the
10 most likely candidates. This works reasonably well, as now "show tls"
only proposes "show tls-keys", "show threads", "show pools" and "show tasks".

If the distance is still too high or if a word is missing, the whole
prefix list continues to be dumped, thus "show" alone will still report
the entire list of commands beginning with "show".

It's still impossible to skip a word, for example "show conn" will not
propose "show servers conn" because the distance is calculated for each
word individually. Some changes to the distance calculation to support
updating an existing map could easily address this. But this is already
a great improvement.

4 years agoMINOR: cli: filter the list of commands to the matching part
Willy Tarreau [Fri, 12 Mar 2021 16:13:28 +0000 (17:13 +0100)] 
MINOR: cli: filter the list of commands to the matching part

The error message on the CLI has become unreadable due to the long list
and it's not even sorted, making it even harder to figure the right
command.

This patch starts by looking if some of the words match something known,
and if so, will limit the listing only to those commands that start like
the current one. The "help", "prompt" and "quit" commands are always
shown to help the user try something else. Now thanks to this, typing
"add" or "del" will only list "add acl", "add map" and not 50 lines
anymore.

As a small bonus, we won't print "Unknown command" anymore in response
to the "help" command.

4 years agoMINOR: cli: print the error message in the parser function itself
Willy Tarreau [Fri, 12 Mar 2021 14:57:47 +0000 (15:57 +0100)] 
MINOR: cli: print the error message in the parser function itself

By doing so we can report more accurate information about what's wrong.
As a first step, we already distinguish the case of expert-only commands
from other ones.

4 years agoMINOR: cli: test the appctx level for master access instead of comparing pointers
Willy Tarreau [Fri, 12 Mar 2021 14:20:39 +0000 (15:20 +0100)] 
MINOR: cli: test the appctx level for master access instead of comparing pointers

Now that the appctx contains the master level, it greatly simplifies
all the tests, as we can simply verify that keyword levels match the
effective level without having to cheat with applet pointers. This
also allows to fold the expert test in them.

4 years agoMINOR: cli: set the ACCESS_MASTER* bits on the master bind_conf
Willy Tarreau [Fri, 12 Mar 2021 14:00:57 +0000 (15:00 +0100)] 
MINOR: cli: set the ACCESS_MASTER* bits on the master bind_conf

Right now the code is a bit hackish, it tests for the keyword's level
flags but checks the applet's origin to compare the bits. Let's start
by properly setting the ACCESS_MASTER_ONLY and ACCESS_MASTER flags on
the master CLI's bind_conf so that they are automatically present
all the time.

4 years agoCLEANUP: cli: fix misleading comment and better indent the access level flags
Willy Tarreau [Fri, 12 Mar 2021 13:56:44 +0000 (14:56 +0100)] 
CLEANUP: cli: fix misleading comment and better indent the access level flags

It was mentioned that ACCESS_MASTER_ONLY as for workers only instead of
master-only. And it wasn't clear that all ACCESS_* would belong to the
same thing.

4 years agoBUG/MINOR: cli: make sure "help", "prompt", "quit" are enabled at master level
Willy Tarreau [Fri, 12 Mar 2021 16:03:33 +0000 (17:03 +0100)] 
BUG/MINOR: cli: make sure "help", "prompt", "quit" are enabled at master level

These 3 commands are functionally valid both in master and worker CLIs.
However, while they do have a valid handler, they are not permitted by
the code and work partially by chance in the master:
  - "prompt" and "quit" are intercepted by the request analyser
  - "help" triggers an error, which results in displaying the error
    message

Let's make sure they are permitted so that we don't count errors there and
that we can report appropriate help.

This bug has always been there but it doesn't have any functional effect
at the moment since "help" can only show the error message. As such, there
is no need to backport it.

4 years agoCLEANUP: resolvers: Perform unsafe loop on requester list when possible
Christopher Faulet [Thu, 11 Mar 2021 17:19:41 +0000 (18:19 +0100)] 
CLEANUP: resolvers: Perform unsafe loop on requester list when possible

When answer list of a response is checked, it is useless to perform a safe
loop on the requester list.

4 years agoCLEANUP: resolvers: Use ha_free() in srvrq_resolution_error_cb()
Christopher Faulet [Wed, 10 Mar 2021 14:51:13 +0000 (15:51 +0100)] 
CLEANUP: resolvers: Use ha_free() in srvrq_resolution_error_cb()

Two occurrences to "free(A);A=NULL;" may be replaced by a call to ha_free()
in the srvrq_resolution_error_cb() function.

4 years agoMINOR: resolvers: Don't try to match immediatly renewed ADD items
Christopher Faulet [Fri, 12 Mar 2021 15:42:45 +0000 (16:42 +0100)] 
MINOR: resolvers: Don't try to match immediatly renewed ADD items

The loop looking for existing ADD items to renew their last_seen must ignore
the items already renewed in the same loop. To do so, we rely on the
last_seen time. because it is now based on now_ms, it is safe.

Doing so avoid to match several time the same ADD item when the same IP
address is found in several ADD item. This reduces the number of extra DNS
resolutions.

This patch depends on "MINOR: resolvers: Use milliseconds for cached items
in resolver responses". Both may be backported as far as 2.2 if necessary.

4 years agoMINOR: resolvers: Use milliseconds for cached items in resolver responses
Christopher Faulet [Thu, 11 Mar 2021 08:36:05 +0000 (09:36 +0100)] 
MINOR: resolvers: Use milliseconds for cached items in resolver responses

The last time when an item was seen in a resolver responses is now stored in
milliseconds instead of seconds. This avoid some corner-cases at the
edges. This also simplifies time comparisons.

4 years agoBUG/MEDIUM: resolvers: Skip DNS resolution at startup if SRV resolution is set
Christopher Faulet [Fri, 12 Mar 2021 09:23:05 +0000 (10:23 +0100)] 
BUG/MEDIUM: resolvers: Skip DNS resolution at startup if SRV resolution is set

At startup, if a SRV resolution is set for a server, no DNS resolution is
created. We must wait the first SRV resolution to know if it must be
triggered. It is important to do so for two reasons.

First, during a "classical" startup, a server based on a SRV resolution has
no hostname. Thus the created DNS resolution is useless. Best waiting the
first SRV resolution. It is not really a bug at this stage, it is just
useless.

Second, in the same situation, if the server state is loaded from a file,
its hosname will be set a bit later. Thus, if there is no additionnal record
for this server, because there is already a DNS resolution, it inhibits any
new DNS resolution. But there is no hostname attached to the existing DNS
resolution. So no resolution is performed at all for this server.

To avoid any problem, it is fairly easier to handle this special case during
startup. But this means we must be prepared to have no "resolv_requester"
field for a server at runtime.

This patch must be backported as far as 2.2.

4 years agoBUG/MEDIUM: resolvers: Don't release resolution from a requester callbacks
Christopher Faulet [Thu, 11 Mar 2021 17:09:53 +0000 (18:09 +0100)] 
BUG/MEDIUM: resolvers: Don't release resolution from a requester callbacks

Another way to say it: "Safely unlink requester from a requester callbacks".

Requester callbacks must never try to unlink a requester from a resolution, for
the current requester or another one. First, these callback functions are called
in a loop on a request list, not necessarily safe. Thus unlink resolution at
this place, may be unsafe. And it is useless to try to make these loops safe
because, all this stuff is placed in a loop on a resolution list. Unlink a
requester may lead to release a resolution if it is the last requester.

However, the unkink is necessary because we cannot reset the server state
(hostname and IP) with some pending DNS resolution on it. So, to workaround
this issue, we introduce the "safe" unlink. It is only performed from a
requester callback. In this case, the unlink function never releases the
resolution, it only reset it if necessary. And when a resolution is found
with an empty requester list, it is released.

This patch depends on the following commits :

 * MINOR: resolvers: Purge answer items when a SRV resolution triggers an error
 * MINOR: resolvers: Use a function to remove answers attached to a resolution
 * MINOR: resolvers: Directly call srvrq_update_srv_state() when possible
 * MINOR: resolvers: Add function to change the srv status based on SRV resolution

All the series must be backported as far as 2.2. It fixes a regression
introduced by the commit b4badf720 ("BUG/MINOR: resolvers: new callback to
properly handle SRV record errors").

don't release resolution from requester cb

4 years agoMINOR: resolvers: Directly call srvrq_update_srv_state() when possible
Christopher Faulet [Thu, 11 Mar 2021 17:06:23 +0000 (18:06 +0100)] 
MINOR: resolvers: Directly call srvrq_update_srv_state() when possible

When the server status must be updated from the result of a SRV resolution,
we can directly call srvrq_update_srv_state(). It is simpler and this avoid
a test on the server DNS resolution.

This patch is mandatory for the next commit. It also rely on "MINOR:
resolvers: Directly call srvrq_update_srv_state() when possible".

4 years agoMINOR: resolvers: Add function to change the srv status based on SRV resolution
Christopher Faulet [Thu, 11 Mar 2021 17:03:21 +0000 (18:03 +0100)] 
MINOR: resolvers: Add function to change the srv status based on SRV resolution

srvrq_update_srv_status() update the server status based on result of SRV
resolution. For now, it is only used from snr_update_srv_status() when
appropriate.

4 years agoMINOR: resolvers: Purge answer items when a SRV resolution triggers an error
Christopher Faulet [Wed, 10 Mar 2021 14:46:46 +0000 (15:46 +0100)] 
MINOR: resolvers: Purge answer items when a SRV resolution triggers an error

When a SRV request trigger an error, if we decide to handle the error
because last_valid duration is expired, the answer list may be purged. All
items are considered as obsolete.

4 years agoMINOR: resolvers: Use a function to remove answers attached to a resolution
Christopher Faulet [Wed, 10 Mar 2021 13:40:39 +0000 (14:40 +0100)] 
MINOR: resolvers: Use a function to remove answers attached to a resolution

resolv_purge_resolution_answer_records() must be used to removed all answers
attached to a resolution. For now, it is only used when a resolution is
released.

4 years agoBUG/MEDIUM: resolvers: Trigger a DNS resolution if an ADD item is obsolete
Christopher Faulet [Wed, 10 Mar 2021 17:38:37 +0000 (18:38 +0100)] 
BUG/MEDIUM: resolvers: Trigger a DNS resolution if an ADD item is obsolete

When a ADD item attached to a SRV item is removed because it is obsolete, we
must trigger a DNS resolution to be sure the hostname still resolves or
not. There is no other way to be the entry is still valid. And we cannot set
the server in RMAINT immediatly, because a DNS server may be inconsitent and
may stop to add some additionnal records.

The opposite is also true. If a valid ADD item is still attached to a SRV
item, any DNS resolution must be stopped. There is no reason to perform
extra resolution in this case.

This patch must be backported as far as 2.2.

4 years agoBUG/MINOR; resolvers: Ignore DNS resolution for expired SRV item
Christopher Faulet [Wed, 10 Mar 2021 14:07:27 +0000 (15:07 +0100)] 
BUG/MINOR; resolvers: Ignore DNS resolution for expired SRV item

If no ADD item is found for a SRV item in a SRV response, a DNS resolution
is triggered. When it succeeds, we must be sure the SRV item is still
alive. Otherwise the DNS resolution must be ignored.

This patch depends on the commit "MINOR: resolvers: Move last_seen time of
an ADD into its corresponding SRV item". Both must be backported as far as
2.2.

4 years agoMINOR: resolvers: new function find_srvrq_answer_record()
Baptiste Assmann [Wed, 10 Mar 2021 11:22:18 +0000 (12:22 +0100)] 
MINOR: resolvers: new function find_srvrq_answer_record()

This function search for a SRV answer item associated to a requester
whose type is server.
This is mainly useful to "link" a server to its SRV record when no
additional record were found to configure the IP address.

This patch is required by a bug fix.

4 years agoBUG/MEDIUM: resolvers: Fix the loop looking for an existing ADD item
Christopher Faulet [Wed, 10 Mar 2021 14:19:57 +0000 (15:19 +0100)] 
BUG/MEDIUM: resolvers: Fix the loop looking for an existing ADD item

For each ADD item found in a SRV response, we try to find a corresponding
ADD item already attached to an existing SRV item. If found, the ADD
last_seen time is updated, otherwise we try to find a SRV item with no ADD
to attached the new one.

However, the loop is buggy. Instead of comparing 2 ADD items, it compares
the new ADD item with the SRV item. Because of this bug, we are unable to
renew last_seen time of existing ADD.

This patch must be backported as far as 2.2.

4 years agoBUG/MEDIUM: resolvers: Don't set an address-less server as UP
Christopher Faulet [Wed, 10 Mar 2021 14:34:52 +0000 (15:34 +0100)] 
BUG/MEDIUM: resolvers: Don't set an address-less server as UP

when a server status is updated based on a SRV item, it is always set to UP,
regardless it has an IP address defined or not. For instance, if only a SRV
item is received, with no additional record, only the server hostname is
defined. We must wait to have an IP address to set the server as UP.

This patch must be backported as far as 2.2.

4 years agoBUG/MINOR: resolvers: Unlink DNS resolution to set RMAINT on SRV resolution
Christopher Faulet [Wed, 10 Mar 2021 20:33:21 +0000 (21:33 +0100)] 
BUG/MINOR: resolvers: Unlink DNS resolution to set RMAINT on SRV resolution

When a server is set in RMAINT becaues of a SRV resolution failure, the
server DNS resolution, if any, must be unlink first. It is mandatory to
handle the change in the context of a SRV resolution.

This patch must be backported as far as 2.2.

4 years agoBUG/MINOR: resolvers: Reset server address on DNS error only on status change
Christopher Faulet [Wed, 10 Mar 2021 19:31:40 +0000 (20:31 +0100)] 
BUG/MINOR: resolvers: Reset server address on DNS error only on status change

When a DNS resolution error is detected, in snr_resolution_error_cb(), the
server address must be reset only if the server status has changed. It this
case, it means the server is set to RMAINT. Thus the server address may by
reset.

This patch fixes a bug introduced by commit d127ffa9f ("BUG/MEDIUM:
resolvers: Reset address for unresolved servers"). It must be backported as
far as 2.0.

4 years agoBUG/MINOR: resolvers: Consider server to have no IP on DNS resolution error
Christopher Faulet [Wed, 10 Mar 2021 14:39:16 +0000 (15:39 +0100)] 
BUG/MINOR: resolvers: Consider server to have no IP on DNS resolution error

When an error is received for a DNS resolution, for instance a NXDOMAIN
error, the server must be considered to have no address when its status is
updated, not the opposite.

Concretly, because this parameter is not used on error path in
snr_update_srv_status(), there is no impact.

This patch must be backported as far as 1.8.

4 years agoRevert "BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record"
Christopher Faulet [Wed, 10 Mar 2021 14:54:14 +0000 (15:54 +0100)] 
Revert "BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record"

This reverts commit a331a1e8eb2ad4750711a477ca3e22d940495faf.

This commit fixes a real bug, but it also reveals some hidden bugs, mostly
because of some design issues. Thus, in itself, it create more problem than
it solves. So revert it for now. All known bugs will be addressed in next
commits.

This patch should be backported as far as 2.2.

4 years agoBUG/MINOR: cfgparse/server: increment the extra keyword counter one at a time
Willy Tarreau [Fri, 12 Mar 2021 13:47:10 +0000 (14:47 +0100)] 
BUG/MINOR: cfgparse/server: increment the extra keyword counter one at a time

This was introduced in previous commit 49c2b45c1 ("MINOR: cfgparse/server:
try to fix spelling mistakes on server lines"), the loop was changed but
the increment left. No backport is needed.

4 years agoMINOR: tcp-rules: suggest approaching action names on mismatch
Willy Tarreau [Fri, 12 Mar 2021 12:46:10 +0000 (13:46 +0100)] 
MINOR: tcp-rules: suggest approaching action names on mismatch

This adds support for action_suggest() in tcp-request and tcp-response
rules so as to propose the closest match in case of misspelling.

4 years agoMINOR: http-rules: suggest approaching action names on mismatch
Willy Tarreau [Fri, 12 Mar 2021 11:01:34 +0000 (12:01 +0100)] 
MINOR: http-rules: suggest approaching action names on mismatch

This adds support for action_suggest() in http-request, http-response
and http-after-response rulesets. For example:

   parsing [/dev/stdin:2]: 'http-request' expects (...), but got 'del-hdr'. Did you mean 'del-header' maybe ?

4 years agoMINOR: actions: add a function to suggest an action ressembling a given word
Willy Tarreau [Fri, 12 Mar 2021 10:59:24 +0000 (11:59 +0100)] 
MINOR: actions: add a function to suggest an action ressembling a given word

action_suggest() will return a pointer to an action whose keyword more or
less ressembles the passed argument. It also accepts to be more tolerant
against prefixes (since actions taking arguments are handled as prefixes).
This will be used to suggest approaching words.

4 years agoMINOR: cfgparse/bind: suggest correct spelling for unknown bind keywords
Willy Tarreau [Fri, 12 Mar 2021 09:14:07 +0000 (10:14 +0100)] 
MINOR: cfgparse/bind: suggest correct spelling for unknown bind keywords

Just like with the server keywords, now's the turn of "bind" keywords.
The difference is that 100% of the bind keywords are registered, thus
we do not need the list of extra keywords.

There are multiple bind line parsers today, all were updated:
  - peers
  - log
  - dgram-bind
  - cli

$ printf "listen f\nbind :8000 tcut\n" | ./haproxy -c -f /dev/stdin
[NOTICE] 070/101358 (25146) : haproxy version is 2.4-dev11-7b8787-26
[NOTICE] 070/101358 (25146) : path to executable is ./haproxy
[ALERT] 070/101358 (25146) : parsing [/dev/stdin:2] : 'bind :8000' unknown keyword 'tcut'; did you mean 'tcp-ut' maybe ?
[ALERT] 070/101358 (25146) : Error(s) found in configuration file : /dev/stdin
[ALERT] 070/101358 (25146) : Fatal errors found in configuration.

4 years agoMINOR: cfgparse/server: try to fix spelling mistakes on server lines
Willy Tarreau [Fri, 12 Mar 2021 08:58:04 +0000 (09:58 +0100)] 
MINOR: cfgparse/server: try to fix spelling mistakes on server lines

Let's apply the fuzzy match to server keywords so that we can avoid
dumping the huge list of supported keywords each time there is a spelling
mistake, and suggest proper spelling instead:

  $ printf "listen f\nserver s 0 sendpx-v2\n" | ./haproxy -c -f /dev/stdin
  [NOTICE] 070/095718 (24152) : haproxy version is 2.4-dev11-caa6e3-25
  [NOTICE] 070/095718 (24152) : path to executable is ./haproxy
  [ALERT] 070/095718 (24152) : parsing [/dev/stdin:2] : 'server s' unknown keyword 'sendpx-v2'; did you mean 'send-proxy-v2' maybe ?
  [ALERT] 070/095718 (24152) : Error(s) found in configuration file : /dev/stdin
  [ALERT] 070/095718 (24152) : Fatal errors found in configuration.

4 years agoMINOR: cfgparse: suggest correct spelling for unknown words in global section
Willy Tarreau [Fri, 12 Mar 2021 08:30:14 +0000 (09:30 +0100)] 
MINOR: cfgparse: suggest correct spelling for unknown words in global section

The global section also knows a large number of keywords that are not
referenced in any list, so this needed them to be specifically listed.
It becomes particularly handy now because some tunables are never easy
to remember, but now it works remarkably well:

  $ printf "global\nsched.queue_depth\n" | ./haproxy -c -f /dev/stdin
  [NOTICE] 070/093007 (23457) : haproxy version is 2.4-dev11-dd8ee5-24
  [NOTICE] 070/093007 (23457) : path to executable is ./haproxy
  [ALERT] 070/093007 (23457) : parsing [/dev/stdin:2] : unknown keyword 'sched.queue_depth' in 'global' section; did you mean 'tune.runqueue-depth' maybe ?
  [ALERT] 070/093007 (23457) : Error(s) found in configuration file : /dev/stdin
  [ALERT] 070/093007 (23457) : Fatal errors found in configuration.

4 years agoMINOR: cfgparse: suggest correct spelling for unknown words in proxy sections
Willy Tarreau [Fri, 12 Mar 2021 08:14:19 +0000 (09:14 +0100)] 
MINOR: cfgparse: suggest correct spelling for unknown words in proxy sections

Let's start by the largest keyword list, the listeners. Many keywords were
still not part of a list, so a common_kw_list array was added to list the
not enumerated ones. Now for example, typing "tmout" properly suggests
"timeout":

  $ printf "frontend f\ntmout client 10s\n" | ./haproxy -c -f /dev/stdin
  [NOTICE] 070/091355 (22545) : haproxy version is 2.4-dev11-3b728a-21
  [NOTICE] 070/091355 (22545) : path to executable is ./haproxy
  [ALERT] 070/091355 (22545) : parsing [/dev/stdin:2] : unknown keyword 'tmout' in 'frontend' section; did you mean 'timeout' maybe ?
  [ALERT] 070/091355 (22545) : Error(s) found in configuration file : /dev/stdin
  [ALERT] 070/091355 (22545) : Fatal errors found in configuration.

4 years agoMINOR: cfgparse: add cfg_find_best_match() to suggest an existing word
Willy Tarreau [Fri, 12 Mar 2021 08:08:04 +0000 (09:08 +0100)] 
MINOR: cfgparse: add cfg_find_best_match() to suggest an existing word

Instead of just reporting "unknown keyword", let's provide a function which
will look through a list of registered keywords for a similar-looking word
to the one that wasn't matched. This will help callers suggest correct
spelling. Also, given that a large part of the config parser still relies
on a long chain of strcmp(), we'll need to be able to pass extra candidates.
Thus the function supports an optional extra list for this purpose.

4 years agoMINOR: tools: add simple word fingerprinting to find similar-looking words
Willy Tarreau [Fri, 12 Mar 2021 08:01:52 +0000 (09:01 +0100)] 
MINOR: tools: add simple word fingerprinting to find similar-looking words

This introduces two functions, one which creates a fingerprint of a word,
and one which computes a distance between two words fingerprints. The
fingerprint is made by counting the transitions between one character and
another one. Here we consider the 26 alphabetic letters regardless of
their case, then any digit as a digit, and anything else as "other". We
also consider the first and last locations as transitions from begin to
first char, and last char to end. The distance is simply the sum of the
squares of the differences between two fingerprints. This way, doubling/
missing a letter has the same cost, however some repeated transitions
such as "e"->"r" like in "server" are very unlikely to match against
situations where they do not exist. This is a naive approach but it seems
to work sufficiently well for now. It may be refined in the future if
needed.

4 years agoCLEANUP: actions: the keyword must always be const from the rule
Willy Tarreau [Fri, 12 Mar 2021 10:12:38 +0000 (11:12 +0100)] 
CLEANUP: actions: the keyword must always be const from the rule

There's no reason for a rule to want to modify an action keyword, let's
make sure it is always const.

4 years agoCLEANUP: http-rules: remove the unexpected comma before the list of action keywords
Willy Tarreau [Fri, 12 Mar 2021 10:37:05 +0000 (11:37 +0100)] 
CLEANUP: http-rules: remove the unexpected comma before the list of action keywords

The error message for http-request and http-response starts with a comma
that very likely is a leftover from a previous list construct. Let's remove
it: "'http-request' expects , 'wait-for-handshake', 'use-service' ...".

4 years agoCLEANUP: vars: make the error message clearer on missing arguments for set-var
Willy Tarreau [Fri, 12 Mar 2021 10:53:19 +0000 (11:53 +0100)] 
CLEANUP: vars: make the error message clearer on missing arguments for set-var

The error message after "http-response set-var" isn't very clear:

  [ALERT] 070/115043 (30526) : parsing [/dev/stdin:2] : error detected in proxy 'f' while parsing 'http-response set-var' rule : invalid variable 'set-var'. Expects 'set-var(<var-name>)' or 'unset-var(<var-name>)'.

Let's change it to this instead:

  [ALERT] 070/115608 (30799) : parsing [/dev/stdin:2] : error detected in proxy 'f' while parsing 'http-response set-var' rule : invalid or incomplete action 'set-var'. Expects 'set-var(<var-name>)' or 'unset-var(<var-name>)'.

With a wrong action name, it also works better (it's handled as a prefix
due to the opening parenthesis):

  [ALERT] 070/115608 (30799) : parsing [/dev/stdin:2] : error detected in proxy 'f' while parsing 'http-response set-varxxx' rule : invalid or incomplete action 'set-varxxx'. Expects 'set-var(<var-name>)' or 'unset-var(<var-name>)'.

4 years agoCLEANUP: tcp-rules: add missing actions in the tcp-request error message
Willy Tarreau [Fri, 12 Mar 2021 12:42:43 +0000 (13:42 +0100)] 
CLEANUP: tcp-rules: add missing actions in the tcp-request error message

The tcp-request error message only mentions "accept", "reject" and
track-sc*, but there are a few other ones that were missing, so let's
add them.

This could be backported, though it's not likely that it will help anyone
with an existing config.

4 years agoBUG/MINOR: server-state: use the argument, not the global state
Willy Tarreau [Fri, 12 Mar 2021 13:09:10 +0000 (14:09 +0100)] 
BUG/MINOR: server-state: use the argument, not the global state

The refactoring in commit 131b07be3 ("MEDIUM: server: Refactor
apply_server_state() to make it more readable") also had a copy-paste
error resulting in using global.server_state_file instead of the
function's argument, which easily crashes with a conf having a
state file in a backend and no global state file.

In addition, let's simplify the code and get rid of strcpy() which
almost certainly will break the build on OpenBSD.

This was introduced in 2.4-dev10, no backport is needed.

4 years agoBUG/MINOR: server-state: properly handle the case where the base is not set
Willy Tarreau [Fri, 12 Mar 2021 12:57:19 +0000 (13:57 +0100)] 
BUG/MINOR: server-state: properly handle the case where the base is not set

The refactoring in commit 131b07be3 ("MEDIUM: server: Refactor
apply_server_state() to make it more readable") made the global
server_state_base be dereferenced before being checked, resulting
in a crash on certain files.

This happened in 2.4-dev10, no backport is needed.

4 years agoBUG/MINOR: tcpcheck: Fix double free on error path when parsing tcp/http-check
Christopher Faulet [Fri, 12 Mar 2021 11:00:14 +0000 (12:00 +0100)] 
BUG/MINOR: tcpcheck: Fix double free on error path when parsing tcp/http-check

When a "tcp-check" or a "http-check" rule is parsed, we try to get the
previous rule in the ruleset to get its index. We must take care to reset
the pointer on this rule in case an error is triggered later on the
parsing. Otherwise, the same rule may be released twice. For instance, it
happens with such line :

    http-check meth GET uri / ## note there is no "send" parameter

This patch must be backported as far as 2.2.

4 years agoBUG/MINOR: proxy/session: Be sure to have a listener to increment its counters
Christopher Faulet [Fri, 12 Mar 2021 08:16:27 +0000 (09:16 +0100)] 
BUG/MINOR: proxy/session: Be sure to have a listener to increment its counters

It is possible to have a session without a listener. It happens for applets
on the client side. Thus all accesses to the listener info from the session
must be guarded. It was the purpose of the commit 36119de18 ("BUG/MEDIUM:
session: NULL dereference possible when accessing the listener"). However,
some tests on the session's listener existence are missing in proxy_inc_*
functions.

This patch should fix the issues #1171, #1172, #1173, #1174 and #1175. It
must be backported with the above commit as far as 1.8.

4 years agoBUG/MINOR: tcpcheck: Update .health threshold of agent inside an agent-check
Christopher Faulet [Fri, 12 Mar 2021 08:06:07 +0000 (09:06 +0100)] 
BUG/MINOR: tcpcheck: Update .health threshold of agent inside an agent-check

If an agent-check is configured for a server, When the response is parsed,
the .health threshold of the agent must be updated on up/down/stopped/fail
command and not the threshold of the health-check. Otherwise, the
agent-check will compete with the health-check and may mark a DOWN server as
UP.

This patch should fix the issue #1176. It must be backported as far as 2.2.

4 years agoBUG/MEDIUM: filters: Set CF_FL_ANALYZE on channels when filters are attached
Christopher Faulet [Mon, 8 Mar 2021 12:40:30 +0000 (13:40 +0100)] 
BUG/MEDIUM: filters: Set CF_FL_ANALYZE on channels when filters are attached

CF_FL_ANALYZE flag is used to know a channel is filtered. It is important to
synchronize request and response channels when the filtering ends.

However, it is possible to call all request analyzers before starting the
filtering on the response channel. This means flt_end_analyze() may be
called for the request channel before flt_start_analyze() on the response
channel. Thus because CF_FL_ANALYZE flag is not set on the response channel,
we consider the filtering is finished on both sides. The consequence is that
flt_end_analyze() is not called for the response and backend filters are
unregistered before their execution on the response channel.

It is possible to encounter this bug on TCP frontend or CONNECT request on
HTTP frontend if the client shutdown is reveiced with the first read.

To fix this bug, CF_FL_ANALYZE is set when filters are attached to the
stream. It means, on the request channel when the stream is created, in
flt_stream_start(). And on both channels when the backend is set, in
flt_set_stream_backend().

This patch must be backported as far as 1.7.

4 years agoBUILD: atomic/arm64: force the register pairs to use in __ha_cas_dw()
Willy Tarreau [Fri, 12 Mar 2021 05:06:14 +0000 (06:06 +0100)] 
BUILD: atomic/arm64: force the register pairs to use in __ha_cas_dw()

Since commit f8fb4f75f ("MINOR: atomic: implement a more efficient arm64
__ha_cas_dw() using pairs"), on some modern arm64 (armv8.1+) compiled
with -march=armv8.1-a under gcc-7.5.0, a build error may appear on ev_poll.o :

  /tmp/ccHD2lN8.s:1771: Error: reg pair must start from even reg at operand 1 -- `casp x27,x28,x22,x23,[x12]'
  Makefile:927: recipe for target 'src/ev_poll.o' failed

It appears that the compiler cannot always assign register pairs there
for a structure made of two u64. It was possibly later addressed since
gcc-9.3 never caused this, but there's no trivially available info on
the subject in the changelogs. Unsuprizingly, using a u128 instead does
fix this, but it significantly inflates the code (+4kB for just 6 places,
very likely that it loaded some extra stubs) and the comparison is ugly,
involving two slower conditional jumps instead of a single one and a
conditional comparison. For example, ha_random64() grew from 144 bytes
to 232.

However, simply forcing the base register does work pretty well, and
makes the code even cleaner and more efficient by further reducing it
by about 4.5kB, possibly because it helps the compiler to pick suitable
registers for the pair there. And the perf on 64-cores looks steadily
0.5% above the previous one, so let's do this.

Note that the commit above was backported to 2.3 to fix scalability
issues on AWS Graviton2 platform, so this one will need to be as well.

4 years agoBUG/MEDIUM: stick-tables: fix ref counter in table entry using multiple http tracksc.
Emeric Brun [Wed, 10 Mar 2021 15:58:03 +0000 (16:58 +0100)] 
BUG/MEDIUM: stick-tables: fix ref counter in table entry using multiple http tracksc.

Setting multiple http-request track-scX rules generates entries
which never expires.

If there was already an entry registered by a previous http rule
'stream_track_stkctr(&s->stkctr[rule->action], t, ts)' didn't
register the new 'ts' into the stkctr. And function is left
with no reference on 'ts' whereas refcount had been increased
by the '_get_entry'

The patch applies the same policy as the one showed on tcp track
rules and if there is successive rules the track counter keep the
first entry registered in the counter and nothing more is computed.

After validation this should be backported in all versions.

4 years agoBUG/MINOR: connection: Missing QUIC initialization
Frédéric Lécaille [Wed, 10 Mar 2021 10:51:38 +0000 (11:51 +0100)] 
BUG/MINOR: connection: Missing QUIC initialization

The QUIC connection struct connection member was not initialized. This may
make randomly haproxy handle TLS connections as QUIC ones only when QUIC support
is enabled leading to such OpenSSL errors (captured from a reg test output, TLS
Client-Hello callback failed):

    OpenSSL error[0x10000085] OPENSSL_internal: CONNECTION_REJECTED
    OpenSSL error[0x10000410] OPENSSL_internal: SSLV3_ALERT_HANDSHAKE_FAILURE
    OpenSSL error[0x1000009a] OPENSSL_internal: HANDSHAKE_FAILURE_ON_CLIENT_HELLO

This patch should fix #1168 github issue.

4 years agoOPTIM: task: automatically adjust the default runqueue-depth to the threads
Willy Tarreau [Wed, 10 Mar 2021 10:06:26 +0000 (11:06 +0100)] 
OPTIM: task: automatically adjust the default runqueue-depth to the threads

The recent default runqueue size reduction appeared to have significantly
lowered performance on low-thread count configs. Testing various values
runqueue values on different workloads under thread counts ranging from
1 to 64, it appeared that lower values are more optimal for high thread
counts and conversely. It could even be drawn that the optimal value for
various workloads sits around 280/sqrt(nbthread), and probably has to do
with both the L3 cache usage and how to optimally interlace the threads'
activity to minimize contention. This is much easier to optimally
configure, so let's do this by default now.

4 years agoMINOR: task: give the scheduler a bit more flexibility in the runqueue size
Willy Tarreau [Wed, 10 Mar 2021 08:26:24 +0000 (09:26 +0100)] 
MINOR: task: give the scheduler a bit more flexibility in the runqueue size

Instead of setting a hard-limit on runqueue-depth and keeping it short
to maintain fairness, let's allow the scheduler to automatically cut
the existing one in two equal halves if its size is between the configured
size and its double. This will allow to increase the default value while
keeping a low latency.

4 years agoBUG/MINOR: sample: Rename SenderComID/TargetComID to SenderCompID/TargetCompID
Daniel Corbett [Wed, 10 Mar 2021 04:00:34 +0000 (23:00 -0500)] 
BUG/MINOR: sample: Rename SenderComID/TargetComID to SenderCompID/TargetCompID

The recently introduced Financial Information eXchange (FIX)
converters have some hard coded tags based on the specification that
were misspelled. Specifically, SenderComID and TargetComID should
be SenderCompID and TargetCompID according to the specification [1][2].

This patch updates all references, which includes the converters
themselves, the regression test, and the documentation.

[1] https://fiximate.fixtrading.org/en/FIX.5.0SP2_EP264/tag49.html
[2] https://fiximate.fixtrading.org/en/FIX.5.0SP2_EP264/tag56.html

4 years agoBUG/MEDIUM: ssl: properly remove the TASK_HEAVY flag at end of handshake
Willy Tarreau [Tue, 9 Mar 2021 16:58:02 +0000 (17:58 +0100)] 
BUG/MEDIUM: ssl: properly remove the TASK_HEAVY flag at end of handshake

Emeric found that SSL+keepalive traffic had dropped quite a bit in the
recent changes, which could be bisected to recent commit 9205ab31d
("MINOR: ssl: mark the SSL handshake tasklet as heavy"). Indeed, a
first incarnation of this commit made use of the TASK_SELF_WAKING
flag but the last version directly used TASK_HEAVY, but it would still
continue to remove the already absent TASK_SELF_WAKING one instead of
TASK_HEAVY. As such, the SSL traffic remained processed with low
granularity.

No backport is needed as this is only 2.4.

4 years agoCLEANUP: config: also address the cfg_keyword API change in the compression code
Willy Tarreau [Tue, 9 Mar 2021 15:55:18 +0000 (16:55 +0100)] 
CLEANUP: config: also address the cfg_keyword API change in the compression code

The tests were made on slz and the zlib parsers for memlevel and windowsize
managed to escape the change made by commit 018251667 ("CLEANUP: config: make
the cfg_keyword parsers take a const for the defproxy"). This is now fixed.

4 years agoBUG/MEDIUM: resolvers: handle huge responses over tcp servers.
Emeric Brun [Mon, 8 Mar 2021 15:41:29 +0000 (16:41 +0100)] 
BUG/MEDIUM: resolvers: handle huge responses over tcp servers.

Parameter "accepted_payload_size" is currently considered regardless
the used nameserver is using TCP or UDP. It remains mandatory to annouce
such capability to support e-dns, so a value have to be announced also
in TCP. Maximum DNS message size in TCP is limited by protocol to 65535
and so for UDP (65507) if system supports such UDP messages. But
the maximum value for this option was arbitrary forced to 8192.

This patch change this maximum to 65535 to allow user to set bigger value
for UDP if its system supports. It also sets accepted_payload_size
in TCP allowing to retrieve huge responses if the configuration uses
TCP nameservers.

The request announcing the accepted_payload_size capability is currently
built at resolvers level and is common to all used nameservers of the
section regardess transport protocol used. A further patch should be
made to at least specify a different payload size depending of the
transport, and perhaps could be forced to 65535 in case of TCP and
maximum would be forced back to 65507 matching UDP max.

This patch is appliable since 2.4 version

4 years agoCLEANUP: stream: rename a few remaining occurrences of "stream *sess"
Willy Tarreau [Tue, 9 Mar 2021 14:43:32 +0000 (15:43 +0100)] 
CLEANUP: stream: rename a few remaining occurrences of "stream *sess"

These are some leftovers from the ancient code where they were still
called sessions, but these areas in the code remain confusing due to
this naming. They were now called "strm" which will not even affect
indenting nor alignment.

4 years agoMINOR: build: force CC to set a return code when probing options
Bertrand Jacquin [Sat, 6 Mar 2021 20:25:46 +0000 (20:25 +0000)] 
MINOR: build: force CC to set a return code when probing options

gcc returns non zero code if an option is not supported (tested
from 6.5 to 10.2).

  $ gcc -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
  1

clang always return 0 if an option in not recognized unless
-Werror is also passed, preventing a correct probing of options
supported by the compiler (tested with clang 6.0.1 to 11.1.0).

  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
  0
  $ clang -Werror -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
  1

Please note today this is not visible since clang 11 exit with SIGABRT
or with return code 1 on older version due to bad file descriptor from
file descriptor handling

  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null 2>&0 ; echo $?
  Aborted (core dumped)
  134
  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null ; echo $?
  warning: unknown warning option '-Wfoobar'; did you mean '-Wformat'? [-Wunknown-warning-option]
  1 warning generated.
  0
  $ clang-11 -Werror -Wfoobar -E -xc - -o /dev/null < /dev/null ; echo $?
  error: unknown warning option '-Wfoobar'; did you mean '-Wformat'? [-Werror,-Wunknown-warning-option]
  1

This specific issue is being tracked with clang upstream in https://bugs.llvm.org/show_bug.cgi?id=49463

4 years agoBUG/MEDIUM: session: NULL dereference possible when accessing the listener
William Lallemand [Mon, 8 Mar 2021 14:26:48 +0000 (15:26 +0100)] 
BUG/MEDIUM: session: NULL dereference possible when accessing the listener

When implementing a client applet, a NULL dereference was encountered on
the error path which increment the counters.

Indeed, the counters incremented are the one in the listener which does
not exist in the case of client applets, so in sess->listener->counters,
listener is NULL.

This patch fixes the access to the listener structure when accessing
from a sesssion, most of the access are the counters in error paths.

Must be backported as far as 1.8.

4 years agoBUILD: connection: do not use VAR_ARRAY in struct tlv
Willy Tarreau [Tue, 9 Mar 2021 09:15:16 +0000 (10:15 +0100)] 
BUILD: connection: do not use VAR_ARRAY in struct tlv

It was brought by commit c44b8de99 ("CLEANUP: connection: Use `VAR_ARRAY`
in `struct tlv` definition") but breaks the build with clang. Actually it
had already been done 6 months ago by commit 4987a4744 ("CLEANUP: tree-wide:
use VAR_ARRAY instead of [0] in various definitions") then reverted by
commit 441b6c31e ("BUILD: connection: fix build on clang after the VAR_ARRAY
cleanup") which explained the same thing but didn't place a comment in the
code to justify this (in short it's just an end of struct marker).