Willy Tarreau [Mon, 9 May 2022 18:07:21 +0000 (20:07 +0200)]
BUILD: debug: work around gcc-12 excessive -Warray-bounds warnings
As was first reported by Ilya in issue #1513, compiling with gcc-12
adds warnings about size 0 around each BUG_ON() call due to the
ABORT_NOW() macro that tries to dereference pointer value 1.
The problem is known, seems to be complex inside gcc and could only
be worked around for now by adjusting a pointer limit so that the
warning still catches NULL derefs in the first page but not other
values commonly used in kernels and boot loaders:
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=91f7d7e1b
It's described in more details here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104657
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103768
And some projects had to work around it using various approaches,
some of which are described in the bugs reports above, plus another
one here:
https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/HLK3BHP2T3FN6FZ46BIPIK3VD5FOU74Z/
In haproxy we can hide it by hiding the pointer in a DISGUISE() macro,
but this forces the pointer to be loaded into a register, so that
register is lost precisely where we want to get the maximum of them.
In our case we purposely use a low-value non-null pointer because:
- it's mandatory that this value fits within an unmapped page and
only the lowest one has this property
- we really want to avoid register loads for the address, as these
will be lost and will complicate the bug analysis, and they tend
to be used for large addresses (i.e. instruction length limit).
- the compiler may decide to optimize away the null deref when it
sees it (seen in the past already)
As such, the current workaround merged in gcc-12 is not effective for
us.
Another approach consists in using pragmas to silently disable
-Warray-bounds and -Wnull-dereference only for this part. The problem
is that pragmas cannot be placed into macros.
The resulting solution consists in defining a forced-inlined function
only to trigger the crash, and surround the dereference with pragmas,
themselves conditionned to gcc >= 5 since older versions don't
understand them (but they don't complain on the dereference at least).
This way the code remains the same even at -O0, without the stack
pointer being modified nor any address register being modified on
common archs (x86 at least). A variation could have been to rely on
__builtin_trap() but it's not everywhere and it behaves differently
on different platforms (undefined opcode or a nasty abort()) while
the segv remains uniform and effective.
This may need to be backported to older releases once users start to
complain about gcc-12 breakage.
Willy Tarreau [Mon, 9 May 2022 08:31:28 +0000 (10:31 +0200)]
BUILD: ssl: work around bogus warning in gcc 12's -Wformat-truncation
As was first reported by Ilya in issue #1513, Gcc 12 incorrectly reports
a possible overflow from the concatenation of two strings whose size was
previously checked to fit:
src/ssl_crtlist.c: In function 'crtlist_parse_file':
src/ssl_crtlist.c:545:58: error: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 1 and 4096 [-Werror=format-truncation=]
545 | snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path);
| ^~
src/ssl_crtlist.c:545:25: note: 'snprintf' output between 2 and 8192 bytes into a destination of size 4097
545 | snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It would be a bit concerning to disable -Wformat-truncation because it
might detect real programming mistakes at other places. The solution
adopted in this patch is absolutely ugly and error-prone, but it works,
it consists in integrating the snprintf() call in the error condition
and to test the result again. Let's hope a smarter compiler will not
warn that this test is absurd since guaranteed by the first condition...
This may have to be backported for those suffering from a compiler upgrade.
Willy Tarreau [Mon, 9 May 2022 17:46:35 +0000 (19:46 +0200)]
BUILD: stats: conditionally mark obsolete stats states as deprecated
The obsolete stats states STAT_ST_* were marked as deprecated with recent
commit 6ef1648dc ("CLEANUP: stats: rename the stats state values an mark
the old ones deprecated"), except that this feature requires gcc 6 and
above. Let's use the macro that depends on this condition instead.
The issue appeared on 2.6-dev9 so no backport is needed.
Willy Tarreau [Mon, 9 May 2022 17:45:06 +0000 (19:45 +0200)]
MINOR: compiler: add a new macro to set an attribute on an enum when possible
Gcc 6 and above support placing an attribute on an enum's value. This
is convenient for marking some values as deprecated. We just need the
macro because older versions fail to parse __attribute__() there.
Willy Tarreau [Sun, 8 May 2022 09:44:15 +0000 (11:44 +0200)]
[RELEASE] Released version 2.6-dev9
Released version 2.6-dev9 with the following main changes :
- MINOR: mux-quic: support full request channel buffer
- BUG/MINOR: h3: fix parsing of unknown frame type with null length
- CLEANUP: backend: make alloc_{bind,dst}_address() idempotent
- MEDIUM: stream: remove the confusing SF_ADDR_SET flag
- MINOR: conn_stream: remove the now unused CS_FL_ADDR_*_SET flags
- CLEANUP: protocol: make sure the connect_* functions always receive a dst
- MINOR: connection: get rid of the CO_FL_ADDR_*_SET flags
- MINOR: session: get rid of the now unused SESS_FL_ADDR_*_SET flags
- CLEANUP: mux: Useless xprt_quic-t.h inclusion
- MINOR: quic: Make the quic_conn be aware of the number of streams
- BUG/MINOR: quic: Dropped retransmitted STREAM frames
- BUG/MINOR: mux_quic: Dropped packet upon retransmission for closed streams
- MEDIUM: httpclient: remove url2sa to use a more flexible parser
- MEDIUM: httpclient: http-request rules for resolving
- MEDIUM: httpclient: allow address and port change for resolving
- CLEANUP: httpclient: remove the comment about resolving
- MINOR: httpclient: handle unix and other socket types in dst
- MINOR: httpclient: rename dash by dot in global option
- MINOR: init: exit() after pre-check upon error
- MINOR: httpclient: cleanup the error handling in init
- MEDIUM: httpclient: hard-error when SSL is configured
- MINOR: httpclient: allow to configure the ca-file
- MINOR: httpclient: configure the resolvers section to use
- MINOR: httpclient: allow ipv4 or ipv6 preference for resolving
- DOC: configuration: httpclient global option
- MINOR: conn-stream: Add mask from flags set by endpoint or app layer
- BUG/MEDIUM: conn-stream: Only keep app layer flags of the endpoint on reset
- BUG/MEDIUM: mux-fcgi: Be sure to never set EOM flag on an empty HTX message
- BUG/MEDIUM: mux-h1: Be able to handle trailers when C-L header was specified
- DOC: config: Update doc for PR/PH session states to warn about rewrite failures
- MINOR: resolvers: cleanup alert/warning in parse-resolve-conf
- MINOR: resolvers: move the resolv.conf parser in parse_resolv_conf()
- MINOR: resolvers: resolvers_new() create a resolvers with default values
- BUILD: debug: unify the definition of ha_backtrace_to_stderr()
- BUG/MINOR: tcp/http: release the expr of set-{src,dst}[-port]
- MEDIUM: resolvers: create a "default" resolvers section at startup
- DOC: resolvers: default resolvers section
- BUG/MINOR: startup: usage() when no -cc arguments
- BUG/MEDIUM: resolvers: make "show resolvers" properly yield
- BUG/MEDIUM: cli: make "show cli sockets" really yield
- BUG/MINOR: proxy/cli: don't enumerate internal proxies on "show backend"
- BUG/MINOR: map/cli: protect the backref list during "show map" errors
- BUG/MINOR: map/cli: make sure patterns don't vanish under "show map"'s init
- BUG/MINOR: ssl/cli: fix "show ssl ca-file/crl-file" not to mix cli+ssl contexts
- BUG/MINOR: ssl/cli: fix "show ssl ca-file <name>" not to mix cli+ssl contexts
- BUG/MINOR: ssl/cli: fix "show ssl crl-file" not to mix cli+ssl contexts
- BUG/MINOR: ssl/cli: fix "show ssl cert" not to mix cli+ssl contexts
- CLEANUP: ssl/cli: do not loop on unknown states in "add ssl crt-list" handler
- MINOR: applet: reserve some generic storage in the applet's context
- CLEANUP: applet: make appctx_new() initialize the whole appctx
- CLEANUP: stream/cli: take the "show sess" context definition out of the appctx
- CLEANUP: stream/cli: stop using appctx->st2 for the dump state
- CLEANUP: stream/cli: remove the unneeded init state from "show sess"
- CLEANUP: stream/cli: remove the unneeded STATE_FIN state from "show sess"
- CLEANUP: stream/cli: remove the now unneeded dump state from "show sess"
- CLEANUP: proxy/cli: take the "show errors" context definition out of the appctx
- CLEANUP: stick-table/cli: take the "show table" context definition out of the appctx
- CLEANUP: stick-table/cli: stop using appctx->st2 for the dump state
- CLEANUP: stick-table/cli: remove the unneeded STATE_INIT for "show table"
- CLEANUP: map/cli: take the "show map" context definition out of the appctx
- CLEANUP: map/cli: stop using cli.i0/i1 to store the generation numbers
- CLEANUP: map/cli: stop using appctx->st2 for the dump state
- CLEANUP: map/cli: always detach the backref from the list after "show map"
- CLEANUP: peers/cli: take the "show peers" context definition out of the appctx
- CLEANUP: peers/cli: stop using appctx->st2 for the dump state
- CLEANUP: peers/cli: remove unneeded state STATE_INIT
- CLEANUP: cli: initialize the whole appctx->ctx, not just the stats part
- CLEANUP: promex: make the applet use its own context
- CLEANUP: promex: stop using appctx->st2
- CLEANUP: stats/cli: take the "show stat" context definition out of the appctx
- CLEANUP: stats/cli: stop using appctx->st2
- CLEANUP: hlua/cli: take the hlua_cli context definition out of the appctx
- CLEANUP: ssl/cli: use a local context for "show cafile"
- CLEANUP: ssl/cli: use a local context for "show crlfile"
- CLEANUP: ssl/cli: use a local context for "show ssl cert"
- CLEANUP: ssl/cli: use a local context for "commit ssl cert"
- CLEANUP: ssl/cli: stop using appctx->st2 for "commit ssl cert"
- CLEANUP: ssl/cli: use a local context for "set ssl cert"
- CLEANUP: ssl/cli: use a local context for "set ssl cafile"
- CLEANUP: ssl/cli: use a local context for "set ssl crlfile"
- CLEANUP: ssl/cli: use a local context for "commit ssl {ca|crl}file"
- CLEANUP: ssl/cli: stop using appctx->st2 for "commit ssl ca/crl"
- CLEANUP: ssl/cli: stop using ctx.cli.i0/i1/p0 for "show tls-keys"
- CLEANUP: ssl/cli: add a new "dump_entries" field to "show_keys_ref"
- CLEANUP: ssl/cli: make "show tlskeys" not use appctx->st2 anymore
- CLEANUP: ssl/cli: make "show ssl ocsp-response" not use cli.p0 anymore
- CLEANUP: ssl/cli: make "{show|dump} ssl crtlist" use its own context
- CLEANUP: ssl/cli: make "add ssl crtlist" use its own context
- CLEANUP: ssl/cli: make "add ssl crtlist" not use st2 anymore
- CLEANUP: dns: stop abusing the sink forwarder's context
- CLEANUP: sink: use the generic context to store the forwarder's context
- CLEANUP: activity/cli: make "show profiling" not use ctx.cli anymore
- CLEANUP: debug/cli: make "debug dev fd" not use ctx.cli anymore
- CLEANUP: debug/cli: make "debug dev memstats" not use ctx.cli anymore
- CLEANUP: ring: pass the ring watch flags to ring_attach_cli(), not in ctx.cli
- CLEANUP: ring/cli: use a locally-defined context instead of using ctx.cli
- CLEANUP: resolvers/cli: make "show resolvers" use a locally-defined context
- CLEANUP: resolvers/cli: remove the unneeded appctx->st2 from "show resolvers"
- CLEANUP: cache/cli: make use of a locally defined context for "show cache"
- CLEANUP: proxy/cli: make use of a locally defined context for "show servers"
- CLEANUP: proxy/cli: get rid of appctx->st2 in "show servers"
- CLEANUP: proxy/cli: make "show backend" only use the generic context
- CLEANUP: cli: make "show fd" use its own context
- CLEANUP: cli: make "show env" use its own context
- CLEANUP: cli: simplify the "show cli sockets" I/O handler
- CLEANUP: cli: make "show cli sockets" use its own context
- CLEANUP: httpclient/cli: use a locally-defined context instead of ctx.cli
- CLEANUP: httpclient: do not use the appctx.ctx anymore
- CLEANUP: peers: do not use appctx.ctx anymore
- CLEANUP: spoe: do not use appctx.ctx anymore
- BUILD: applet: mark the CLI's generic variables as deprecated
- BUILD: applet: mark the appctx's st2 variable as deprecated
- CLEANUP: cache: take the context out of appctx.ctx
- MEDIUM: lua: move the cosocket storage outside of appctx.ctx
- MINOR: lua: move the tcp service storage outside of appctx.ctx
- MINOR: lua: move the http service context out of appctx.ctx
- CLEANUP: cli: move the status print context into its own context
- CLEANUP: stats: rename the stats state values an mark the old ones deprecated
- DOC: internal: document the new cleaner approach to the appctx
- MINOR: tcp: socket translate TCP_KEEPIDLE for macOs equivalent
- DOC: fix typo "ant" for "and" in INSTALL
- CI: dynamically determine actual version of h2spec
David CARLIER [Sun, 1 May 2022 14:29:58 +0000 (15:29 +0100)]
MINOR: tcp: socket translate TCP_KEEPIDLE for macOs equivalent
On Linux the interval before starting to send TCP keep-alive packets
is defined by TCP_KEEPIDLE. MacOS has an equivalent with TCP_KEEPIDLE,
which also uses seconds as a unit, so it's possible to simply remap the
definition of TCP_KEEPIDLE to TCP_KEEPALIVE there and get it to seamlessly
work. The other settings (interval and count) are not present, though.
Willy Tarreau [Fri, 6 May 2022 16:07:53 +0000 (18:07 +0200)]
CLEANUP: stats: rename the stats state values an mark the old ones deprecated
The STAT_ST_* values have been abused by virtually every applet and CLI
keyword handler, and this must not continue as it's a source of bugs and
of overly complicated code.
This patch renames the states to STAT_STATE_*, and keeps the previous
enum while marking each entry as deprecated. This should be sufficient to
catch out-of-tree code that might rely on them and to let them know what
to do with that.
Willy Tarreau [Fri, 6 May 2022 15:16:35 +0000 (17:16 +0200)]
CLEANUP: cli: move the status print context into its own context
Now that the CLI's print context is alone in the appctx, it's possible
to refine the appctx's ctx layout so that the cli part matches exactly
a regular svcctx, and as such move the CLI context into an svcctx like
other applets. External code will still build and work because the
struct cli perfectly maps onto the struct cli_print_ctx that's located
into svc.storage. This is of course only to make a smooth transition
during 2.6 and will disappear immediately after.
A tiny change had to be applied to the opentracing addon which performs
direct accesses to the CLI's err pointer in its own print function. The
rest uses the standard cli_print_* which were the only ones that needed
a small change.
The whole "ctx.cli" struct could be tagged as deprecated so that any
possibly existing external code that relies on it will get build
warnings, and the comments in the struct are pretty clear about the
way to fix it, and the lack of future of this old API.
Willy Tarreau [Fri, 6 May 2022 12:26:10 +0000 (14:26 +0200)]
MINOR: lua: move the http service context out of appctx.ctx
Just like for the TCP service, let's move the context away from
appctx.ctx. A new struct hlua_http_ctx was defined, reserved in
hlua_applet_http_init() and used everywhere else. Similarly, the
task dump code will no more report decoded stack traces in case
these services would be involved. That may be solved later.
Willy Tarreau [Fri, 6 May 2022 12:07:13 +0000 (14:07 +0200)]
MINOR: lua: move the tcp service storage outside of appctx.ctx
The use-service mechanism for Lua in TCP mode relies on the
hlua_tcp storage in appctx->ctx. We can move its definition to
hlua.c and simply use appctx_reserve_svcctx() to reserve and access
the stoage. One tiny side effect is that the task dump used in panics
will not show anymore the Lua call stack in its trace. For this a
better API is needed from the Lua code to expose a function that does
the job from an appctx.
Willy Tarreau [Fri, 6 May 2022 09:57:34 +0000 (11:57 +0200)]
MEDIUM: lua: move the cosocket storage outside of appctx.ctx
The Lua cosockets were using appctx.ctx.hlua_cosocket. Let's move this
to a local definition of "hlua_csk_ctx" in hlua.c, which is allocated
from the appctx by hlua_socket_new(). There's a notable change which is
that, while previously the xref link with the peer was established with
the appctx, it's now in the hlua_csk_ctx. This one must then hold a
pointer to the appctx. The code was adjusted accordingly, and now that
part of the code doesn't use the appctx.ctx anymore.
Willy Tarreau [Fri, 6 May 2022 09:03:39 +0000 (11:03 +0200)]
CLEANUP: cache: take the context out of appctx.ctx
The context was moved to a local definition in the cache code, and
there's nothing specific to the cache anymore in the appctx. The
struct is stored into the appctx's storage area via the svcctx.
Willy Tarreau [Thu, 5 May 2022 18:01:54 +0000 (20:01 +0200)]
BUILD: applet: mark the appctx's st2 variable as deprecated
This one has been misused for a while as well, it's time to deprecate it
since we don't use it anymore. It will be removed in 2.7 and for now is
only marked as deprecated. Since we need to guarantee that it's zeroed
before starting any applet or CLI command, it was moved into an anonymous
union where its sibling is not marked as deprecated so that we can
continue to initialize it without triggering a warning.
If you found this commit after a bisect session you initiated to figure
why you got some build warnings and don't know what to do, have a look
at the code that deals with the "show fd", "show sess" or "show servers"
commands, as it's supposed to be self-explanatory about the tiny changes
to apply to your code to port it. If you find APPLET_MAX_SVCCTX to be
too small for your use case, either kindly ask for a tiny extension
(and try to get your code merged), or just use a pool.
Willy Tarreau [Thu, 5 May 2022 17:43:49 +0000 (19:43 +0200)]
BUILD: applet: mark the CLI's generic variables as deprecated
The generic context variables p0/p1/p2, i0/i1, o0/o1 have been abused
and causing trouble for too long, it's time to remove them now that
they are not used anymore.
However the risk that external code still uses them is not nul and we
had not warned before about their removal. Let's mark them deprecated
in 2.6 and removed in 2.7. This will let external code continue to work
(as well as it could if it misuses them), with a strong encouragement
on updating it.
If you found this commit after a bisect session you initiated to figure
why you got some build warnings and don't know what to do, have a look
at the code that deals with the "show fd", "show env" or "show servers"
commands, as it's supposed to be self-explanatory about the tiny changes
to apply to your code to port it. If you find APPLET_MAX_SVCCTX to be
too small for your use case, either kindly ask for a tiny extension
(and try to get your code merged), or just use a pool.
Willy Tarreau [Thu, 5 May 2022 18:12:01 +0000 (20:12 +0200)]
CLEANUP: httpclient: do not use the appctx.ctx anymore
The httpclient already uses its own pointer and only used to store this
single pointer into the appctx.ctx field. Let's just move it to the
svcctx and remove this entry from the appctx union.
Willy Tarreau [Thu, 5 May 2022 17:38:21 +0000 (19:38 +0200)]
CLEANUP: httpclient/cli: use a locally-defined context instead of ctx.cli
The httpclient's CLI uses ctx.cli.i0 for its flags and .p0 for the client
instance. Let's have a locally defined structure for this so that we don't
need the generic cli variables anymore.
Willy Tarreau [Thu, 5 May 2022 17:11:05 +0000 (19:11 +0200)]
CLEANUP: cli: make "show cli sockets" use its own context
Let's create a show_sock_ctx to store the bind_conf and the listener.
The entry is reserved when entering the I/O handler since there's no
parser here. That's fine because the function doesn't touch the area.
Willy Tarreau [Thu, 5 May 2022 16:52:36 +0000 (18:52 +0200)]
CLEANUP: cli: simplify the "show cli sockets" I/O handler
The code is was a bit convoluted by the use of a state machine around
st2 that is not used since only the STAT_ST_LIST state was used, and
the test of global.cli_fe inside the loop while it ought better be
tested before entering there. Let's get rid of this unneded state and
simplify the code. There's no more need for ->st2 now. The code looks
more changed than it really is due to the reindent caused by the
removal of the switch statement, but "git show -b" shows what really
changed.
Willy Tarreau [Thu, 5 May 2022 15:45:52 +0000 (17:45 +0200)]
CLEANUP: cli: make "show env" use its own context
There is the variable to start from (or environ) and an option to stop
after dumping the first one, just like "show fd". Let's have a small
locally-defined context with these two fields.
Willy Tarreau [Thu, 5 May 2022 15:56:58 +0000 (17:56 +0200)]
CLEANUP: cli: make "show fd" use its own context
The "show fd" command used to rely on cli.i0 for the fd, and st2 just
to decide whether to stop after the first value or not. It could have
been possible to decide to use just a negative integer to dump a single
value, but it's as easy and more durable to declare a two-field struct
show_fd_ctx for this.
Willy Tarreau [Thu, 5 May 2022 15:00:20 +0000 (17:00 +0200)]
CLEANUP: proxy/cli: make use of a locally defined context for "show servers"
The command uses a pointer to the current proxy being dumped, one to the
current server being dumped, an optional ID of the only proxy to dump
(which is in fact used as a boolean), and a flag indicating if we're
doing a "show servers conn" or a "show servers state". Let's move all
this to a struct show_srv_ctx.
Willy Tarreau [Thu, 5 May 2022 14:46:13 +0000 (16:46 +0200)]
CLEANUP: cache/cli: make use of a locally defined context for "show cache"
The command uses a pointer to a cache instance and the next key to dump,
they were in cli.p0/i0 respectively, let's move them to a struct
show_cache_ctx.
Willy Tarreau [Thu, 5 May 2022 14:38:13 +0000 (16:38 +0200)]
CLEANUP: resolvers/cli: remove the unneeded appctx->st2 from "show resolvers"
The command uses this state but _INIT immediately turns to _LIST, which
turns to _FIN at the end without doing anything in that state, thus the
only existing state is _LIST so we don't need to store a state. Let's
just get rid of it.
Willy Tarreau [Thu, 5 May 2022 13:39:02 +0000 (15:39 +0200)]
CLEANUP: resolvers/cli: make "show resolvers" use a locally-defined context
The command was using cli.p0/p1/p2 to select which section to dump, the
current section and the current ns. Let's instead have a locally defined
"show_resolvers_ctx" section for this.
Willy Tarreau [Thu, 5 May 2022 13:29:43 +0000 (15:29 +0200)]
CLEANUP: ring/cli: use a locally-defined context instead of using ctx.cli
The ring code was using ctx.cli.i0/p0/o0 to store its context during CLI
dumps via "show events" or "show errors". Let's use a locally defined
context and drop that.
Willy Tarreau [Thu, 5 May 2022 13:18:57 +0000 (15:18 +0200)]
CLEANUP: ring: pass the ring watch flags to ring_attach_cli(), not in ctx.cli
The ring watch flags (wait, seek end) were dangerously passed via ctx.cli.i0
from "show buf" in sink.c:cli_parse_show_events(), or implicitly reset in
"show errors". That's very unconvenient, difficult to follow, and prone to
short-term breakage.
Let's pass an extra argument to ring_attach_cli() to take these flags, now
defined in ring-t.h as RING_WF_*, and let the function set them itself
where appropriate (still ctx.cli.i0 for now).
Willy Tarreau [Thu, 5 May 2022 12:26:28 +0000 (14:26 +0200)]
CLEANUP: debug/cli: make "debug dev fd" not use ctx.cli anymore
The command only requires to store an int, but it will be useful later
to have a struct to pass extra info such as an "all" flag to dump all
FDs. The new context is now a struct dev_fd_ctx stored in svcctx.
Willy Tarreau [Wed, 4 May 2022 18:42:23 +0000 (20:42 +0200)]
CLEANUP: sink: use the generic context to store the forwarder's context
Instead of having a struct that contains a single pointer in the appctx
context, let's directly use the generic context pointer and get rid of
the now unused sft.ptr entry.
Willy Tarreau [Thu, 5 May 2022 11:48:40 +0000 (13:48 +0200)]
CLEANUP: ssl/cli: make "add ssl crtlist" not use st2 anymore
Several steps are used during the addition of a crtlist to yield during
long operations, and states are used for this. Let's just not use the
st2 anymore and place the state inside the add_crtlist_ctx struct instead.
Willy Tarreau [Thu, 5 May 2022 09:53:23 +0000 (11:53 +0200)]
CLEANUP: ssl/cli: make "{show|dump} ssl crtlist" use its own context
These commands were using cli.i0/p0/p1 and in a not very clean way since
they use the same parser but with different types depending on the I/O
handler. Given there was no explanation about what the variables were
supposed to be, they were named based on best guess and placed into a
new "show_crtlist_ctx" structure.
Willy Tarreau [Wed, 4 May 2022 18:25:05 +0000 (20:25 +0200)]
CLEANUP: ssl/cli: use a local context for "commit ssl {ca|crl}file"
These two commands use distinct parse/release functions but a common
iohandler, thus they need to keep the same context. It was created
under the name "commit_cacrlfile_ctx" and holds a large part of the
pointers (6) and the ca_type field that helps distinguish between
the two commands for the I/O handler. It looks like some of these
fields could have been merged since apparently the CA part only
uses *cafile* and the CRL part *crlfile*, while both old and new
are of type cafile_entry and set only for each type. This could
probably even simplify some parts of the code that tries to use
the correct field.
These fields were the last ones to be migrated thus the appctx's
ssl context could finally be removed.
Willy Tarreau [Wed, 4 May 2022 18:33:03 +0000 (20:33 +0200)]
CLEANUP: ssl/cli: use a local context for "set ssl crlfile"
Just like for "set ssl cafile", the command doesn't really need this
context which doesn't outlive the parsing function but it was there
for a purpose so it's maintained. Only 3 fields were used from the
appctx's ssl context: old_crlfile_entry, new_crlfile_entry, and path.
These ones were reinstantiated into a new "set_crlfile_ctx" struct.
It could have been merged with the one used in "set cafile" if the
fields had been renamed since cafile and crlfile are of the same
type (probably one of them ought to be renamed?).
None of these fields could be dropped as they are still shared with
other commands.
Willy Tarreau [Wed, 4 May 2022 18:12:55 +0000 (20:12 +0200)]
CLEANUP: ssl/cli: use a local context for "set ssl cafile"
Just like for "set ssl cert", the command doesn't really need this
context which doesn't outlive the parsing function but it was there
for a purpose so it's maintained. Only 3 fields were used from the
appctx's ssl context: old_cafile_entry, new_cafile_entry, and path.
These ones were reinstantiated into a new "set_cafile_ctx" struct.
None of them could be dropped as they are still shared with other
commands.
Willy Tarreau [Wed, 4 May 2022 18:05:55 +0000 (20:05 +0200)]
CLEANUP: ssl/cli: use a local context for "set ssl cert"
The command doesn't really need any storage since there's only a parser,
but since it used this context, there might have been plans for extension,
so better continue with a persistent one. Only old_ckchs, new_ckchs, and
path were being used from the appctx's ssl context. There ones moved to
the local definition, and the two former ones were removed from the appctx
since not used anymore.
Willy Tarreau [Wed, 4 May 2022 17:58:00 +0000 (19:58 +0200)]
CLEANUP: ssl/cli: use a local context for "commit ssl cert"
This command only really uses old_ckchs, new_ckchs and next_ckchi
from the appctx's ssl context. The new structure "commit_cert_ctx"
only has these 3 fields, though none could be removed from the shared
ssl context since they're still used by other commands.
Willy Tarreau [Wed, 4 May 2022 17:51:37 +0000 (19:51 +0200)]
CLEANUP: ssl/cli: use a local context for "show ssl cert"
This command only really uses old_ckchs, cur_ckchs and the index
in which the transaction was stored. The new structure "show_cert_ctx"
only has these 3 fields, and the now unused "cur_ckchs" and "index"
could be removed from the shared ssl context.
Willy Tarreau [Wed, 4 May 2022 17:38:57 +0000 (19:38 +0200)]
CLEANUP: ssl/cli: use a local context for "show crlfile"
Now this command doesn't share any context anymore with "show cafile"
nor with the other commands. The previous "cur_cafile_entry" field from
the applet's ssl context was removed as not used anymore. Everything was
moved to show_crlfile_ctx which only has 3 fields.
Willy Tarreau [Wed, 4 May 2022 17:26:59 +0000 (19:26 +0200)]
CLEANUP: ssl/cli: use a local context for "show cafile"
Saying that the layout and usage of the various variables in the ssl
applet context is a mess would be an understatement. It's very hard
to know what command uses what fields, even after having moved away
from the mix of cli and ssl.
Let's extract the parts used by "show cafile" into their own structure.
Only the "show_all" field would be removed from the ssl ctx, the other
fields are still shared with other commands.
Willy Tarreau [Tue, 3 May 2022 16:13:39 +0000 (18:13 +0200)]
CLEANUP: hlua/cli: take the hlua_cli context definition out of the appctx
This context is used by CLI keywords registered by Lua. We can take
it out of the appctx and use the generic command context allocation so
that the appctx doesn't have to declare a specific one anymore. The
context is created during parsing.
Willy Tarreau [Tue, 3 May 2022 15:08:29 +0000 (17:08 +0200)]
CLEANUP: stats/cli: take the "show stat" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing (both in the CLI and HTTP).
The change looks large but it's particularly mechanical. The context
initialization appears in stats.c and http_ana.c. The context is used
in stats.c and resolvers.c since "show stat resolvers" points there.
That's the reason why the definition moved to stats.h. "show info"
and "show stat" continue to share the same state definition for now.
Willy Tarreau [Tue, 3 May 2022 16:05:23 +0000 (18:05 +0200)]
CLEANUP: promex: stop using appctx->st2
Now that we have out own context, there's no need to use the cryptic st2
struct member. It's solely used to carry a field number here, so let's
add this into the context.
Willy Tarreau [Tue, 3 May 2022 15:33:00 +0000 (17:33 +0200)]
CLEANUP: promex: make the applet use its own context
The prometheus applet used to map to the stats context since it was not
convenient to have one's own context, and to reuse the fields with its
own values and enums. The obj1 pointer was used both for proxies and
stick-tables; obj2 was used both for servers and listeners.
This change makes use of the generic command context allocation so that
the there's now a properly typed context for prometheus, defined in the
code itself and independent on the stats or appctx ones. For clarity,
the types are correctly set and there's one proxy, one table, one server
and one listener. Some could be compacted using unions but that's not
necessary since the context is reasonably compact. The stats' st_code
field was used as the object state so the new field name is obj_state.
An attempt was made to change the types to const for what us only visited
but some calls pass through the stats code to retrieve info and that code
uses non-const variables due to internal API limitations (read_freq_ctr()
being used and requiring variable). That could change in the future,
though.
Willy Tarreau [Tue, 3 May 2022 15:02:03 +0000 (17:02 +0200)]
CLEANUP: cli: initialize the whole appctx->ctx, not just the stats part
Historically the CLI was a second access to the stats and we've continued
to initialize only the stats part when initializing the CLI. Let's make
sure we do that on the whole ctx instead. It's probably not more needed
at all nowadays but better stay on the safe side.
Willy Tarreau [Tue, 3 May 2022 12:58:47 +0000 (14:58 +0200)]
CLEANUP: peers/cli: stop using appctx->st2 for the dump state
Let's instead define a 4-state enum solely for this use case, and place
it into the command's context. Note that END and FIN were already
aliases, which is why they were merged.
Willy Tarreau [Tue, 3 May 2022 12:26:31 +0000 (14:26 +0200)]
CLEANUP: peers/cli: take the "show peers" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing. The code also uses st2 which deserves being
addressed in separate commit.
Willy Tarreau [Tue, 3 May 2022 13:42:07 +0000 (15:42 +0200)]
CLEANUP: map/cli: always detach the backref from the list after "show map"
There's no point checking the state before deciding to detach the backref
on "show map", it should always be done if the list is not empty. Note
that being empty guarantees that it's not linked into the list, and
conversely not being empty guarantees that it's in the list, hence the
test doesn't need to be performed under the lock.
Willy Tarreau [Tue, 3 May 2022 12:12:56 +0000 (14:12 +0200)]
CLEANUP: map/cli: stop using cli.i0/i1 to store the generation numbers
The "show map"/"clear map" code used to rely on the cli's i0/i1 fields
to store the generation numbers to work with. That's particularly dirty
because it's done at places where ctx.map is also manipulated while they
are part of the same union, and the reason why this didn't cause trouble
is because cli.i0/i1 are at offset 216/224 while the map parts end at
204, so luckily there was no overlap.
Willy Tarreau [Tue, 3 May 2022 09:54:47 +0000 (11:54 +0200)]
CLEANUP: map/cli: take the "show map" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing. Many commands, including pure parsers, use this
context but that's not a problem as it's designed to be used this way.
Due to this, many lines are changed but that's in fact a replacement of
"appctx->ctx.map" with "ctx->". Note that the code also uses st2 which
deserves being addressed in separate commit.
Willy Tarreau [Tue, 3 May 2022 10:02:36 +0000 (12:02 +0200)]
CLEANUP: stick-table/cli: remove the unneeded STATE_INIT for "show table"
This state is pointless now, it just serves to initialize the initial
table pointer while this can be done more easily in the parser, so let's
do that and drop that state.
Willy Tarreau [Tue, 3 May 2022 09:45:02 +0000 (11:45 +0200)]
CLEANUP: stick-table/cli: stop using appctx->st2 for the dump state
Let's instead define a 4-state enum solely for this use case, and place
it into the command's context. Note that END and FIN were already
aliases, which is why they were merged.
Willy Tarreau [Tue, 3 May 2022 09:35:07 +0000 (11:35 +0200)]
CLEANUP: stick-table/cli: take the "show table" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing. The code also uses st2 which deserves being
addressed in separate commit.
Willy Tarreau [Tue, 3 May 2022 09:24:24 +0000 (11:24 +0200)]
CLEANUP: proxy/cli: take the "show errors" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing.
The code still has room for improvement, such as in the "flags" field
where bits are hard-coded, but they weren't modified.
Willy Tarreau [Tue, 3 May 2022 09:17:35 +0000 (11:17 +0200)]
CLEANUP: stream/cli: remove the now unneeded dump state from "show sess"
The state was a constant, let's remove what remains of the switch/case.
The code from the "case" statement was only reindented as can be checked
with "git show -b".
Willy Tarreau [Tue, 3 May 2022 09:10:19 +0000 (11:10 +0200)]
CLEANUP: stream/cli: remove the unneeded STATE_FIN state from "show sess"
This state is only an alias for "thr >= global.nbthread" which is the
sole condition that indicates the end and reaches it. Let's just drop
this state. There's only the STATE_LIST that's left.
Willy Tarreau [Tue, 3 May 2022 09:05:39 +0000 (11:05 +0200)]
CLEANUP: stream/cli: remove the unneeded init state from "show sess"
This state was only used to preset the list element. Now that we can
guarantee that the context can be properly preset during the parsing
we don't need this state anymore. The first pointer has to be set to
point to the first stream during the initial call which is detected
by the pointer not yet being set (null). Thanks to this we can also
remove one state check on the abort path.
Willy Tarreau [Tue, 3 May 2022 08:49:00 +0000 (10:49 +0200)]
CLEANUP: stream/cli: take the "show sess" context definition out of the appctx
This makes use of the generic command context allocation so that the
appctx doesn't have to declare a specific one anymore. The context is
created during parsing.
Willy Tarreau [Thu, 5 May 2022 17:55:03 +0000 (19:55 +0200)]
CLEANUP: applet: make appctx_new() initialize the whole appctx
Till now, appctx_new() used to allocate an entry from the pool and
pass it through appctx_init() to initialize a debatable part of it that
didn't correspond anymore to the comments, and fill other fields. It's
hard to say what is fully initialized and what is not.
Let's get rid of that, and always zero the initialization (appctx are
not that big anyway, even with the cache there's no difference in
performance), and initialize what remains. this is cleaner and more
resistant to new field additions.
Willy Tarreau [Mon, 2 May 2022 12:57:03 +0000 (14:57 +0200)]
MINOR: applet: reserve some generic storage in the applet's context
Instead of using existing fields and having to put keyword-specific
contexts in the applet definition, let's have the appctx provide a
generic storage area that's currently large enough for existing CLI
commands and small applets, and a function to allocate that storage.
The function will be responsible for verifying that the requested size
fits in the area so that the caller doesn't need to add specific checks;
it is validated during development as this size is static and will
not change at runtime. In addition the caller doesn't even need to
free() the area since it's part of an existing context. For the
caller's convenience, a context pointer "svcctx" for the command is
also provided so that the allocated area can be placed there (or
possibly any other one in case a larger area is needed).
The struct's layout has been temporarily complicated by adding one
level of anonymous union on top of the "ctx" one. This will allow us
to preserve "ctx" during 2.6 for compatibility with possible external
code and get rid of it in 2.7. This explains why the diff extends to
the whole "ctx" union, but a "git show -b" shows that only one extra
layer was added. In order to make both the svcctx pointer and its
storage accessible without further enlarging the appctx structure,
both svcctx and the storage share the same storage as the ctx part.
This is done by having them placed in the union with a protected
overlapping area for svcctx, for which a shadow member is also
present in the storage area:
union {
void* svcctx; // variable accessed by services
struct {
void *shadow; // shadow of svcctx;
char storage[]; // where most services store their data
};
union { // older commands store here and ignore svcctx
...
} ctx;
};
I.e. new applications will use appctx->svcctx while older ones will be
able to continue to use appctx->ctx.*
The whole area (including the pointer's context) is zeroed before any
applet is initialized, and before CLI keyword processor's first invocation,
as it is an important part of the existing keyword processors, which makes
CLI keywords effectively behave like applets.
Willy Tarreau [Thu, 5 May 2022 11:50:46 +0000 (13:50 +0200)]
CLEANUP: ssl/cli: do not loop on unknown states in "add ssl crt-list" handler
The io_handler in "add ssl crt_list" is built around a "while" loop that
only makes forward progress and that doesn't handle its final state as
it's not supposed to be called again once reached. This makes the code
confusing because its construct implies an infinite loop for such a
state (or any other unhandled one). Let's just remove that unneeded loop.
Willy Tarreau [Wed, 4 May 2022 14:11:50 +0000 (16:11 +0200)]
BUG/MINOR: ssl/cli: fix "show ssl cert" not to mix cli+ssl contexts
The "show ssl cert" command mixes some generic pointers from the
"ctx.cli" struct with context-specific ones from "ctx.ssl" while both
are in a union. Amazingly, despite the use of both p0 and i0 to store
respectively a pointer to the current ckchs and a transaction id, there
was no overlap with the other pointers used during these operations,
but should these fields be reordered or slightly updated this will break.
Comments were added above the faulty functions to indicate which fields
they are using.
Willy Tarreau [Wed, 4 May 2022 14:01:24 +0000 (16:01 +0200)]
BUG/MINOR: ssl/cli: fix "show ssl crl-file" not to mix cli+ssl contexts
The "show ssl crl-file" command mixes some generic pointers from the
"ctx.cli" struct with context-specific ones from "ctx.ssl" while both
are in a union. It's fortunate that the p1 pointer in use is located
before the first one used (it overlaps with old_cafile_entry). But
should these fields be reordered or slightly updated this will break.
Willy Tarreau [Wed, 4 May 2022 13:57:30 +0000 (15:57 +0200)]
BUG/MINOR: ssl/cli: fix "show ssl ca-file <name>" not to mix cli+ssl contexts
The "show ssl ca-file <name>" command mixes some generic pointers from
the "ctx.cli" struct and context-specific ones from "ctx.ssl" while both
are in a union. The i0 integer used to store the current ca_index overlaps
with new_crlfile_entry which is thus harmless for now but is at the mercy
of any reordering or addition of these fields. Let's add dedicated fields
into the ssl structure for this.
Comments were added on top of the affected functions to indicate what they
use.
Willy Tarreau [Wed, 4 May 2022 13:47:39 +0000 (15:47 +0200)]
BUG/MINOR: ssl/cli: fix "show ssl ca-file/crl-file" not to mix cli+ssl contexts
The "show ca-file" and "show crl-file" commands mix some generic pointers
from the "ctx.cli" struct and context-specific ones from "ctx.ssl" while
both are in a union. It's fortunate that the p0 pointer in use is located
immediately before the first one used (it overlaps with next_ckchi_link,
and old_cafile_entry is safe). But should these fields be reordered or
slightly updated this will break.
Comments were added on top of the affected functions to indicate what they
use.
Willy Tarreau [Tue, 3 May 2022 13:26:27 +0000 (15:26 +0200)]
BUG/MINOR: map/cli: make sure patterns don't vanish under "show map"'s init
When "show map" initializes itself, it first takes the reference to the
starting point under a lock, then releases it before switching to state
STATE_LIST, and takes the lock again. The problem is that it is possible
for another thread to remove the first element during this unlock/lock
sequence, and make the list run anywhere. This is of course extremely
unlikely but not impossible.
Let's initialize the pointer in the STATE_LIST part under the same lock,
which is simpler and more reliable.
Willy Tarreau [Tue, 3 May 2022 13:19:49 +0000 (15:19 +0200)]
BUG/MINOR: map/cli: protect the backref list during "show map" errors
In case of write error in "show map", the backref is detached but
the list wasn't locked when this is done. The risk is very low but
it may happen that two concurrent "show map" one of which would fail
or one "show map" failing while the same entry is being updated could
cause a crash.
Willy Tarreau [Thu, 5 May 2022 15:10:03 +0000 (17:10 +0200)]
BUG/MINOR: proxy/cli: don't enumerate internal proxies on "show backend"
Commit e7f74623e ("MINOR: stats: don't output internal proxies (PR_CAP_INT)")
in 2.5 ensured we don't dump internal proxies on the stats page, but the
same is needed for "show backend", as since the addition of the HTTP client
it now appears there:
$ socat /tmp/sock1 - <<< "show backend"
# name
<HTTPCLIENT>
Willy Tarreau [Thu, 5 May 2022 16:29:25 +0000 (18:29 +0200)]
BUG/MEDIUM: cli: make "show cli sockets" really yield
This command was introduced in 1.8 with commit eceddf722 ("MEDIUM: cli:
'show cli sockets' list the CLI sockets") but its yielding doesn't work.
Each time it enters, it restarts from the last bind_conf but enumerates
all listening sockets again, thus it loops forever. The risk that it
happens in field is low but it easily triggers on port ranges after
400-500 sockets depending on the length of their addresses:
$ socat /tmp/sock1 - <<< "show cli sockets"
(...)
ipv4@192.168.8.176:30426 operator all
ipv4@192.168.8.176:30427 operator all
ipv4@192.168.8.176:30428 operator all
ipv4@192.168.8.176:30000 operator all
ipv4@192.168.8.176:30001 operator all
ipv4@192.168.8.176:30002 operator all
^C
This patch adds the minimally needed restart point for the listener so
that it can easily be backported. Some more cleanup is needed though.
Willy Tarreau [Thu, 5 May 2022 14:00:45 +0000 (16:00 +0200)]
BUG/MEDIUM: resolvers: make "show resolvers" properly yield
The "show resolvers" command is bogus, it tries to implement a yielding
mechanism except that if it yields it restarts from the beginning, until
it manages to fill the buffer with only line breaks, and faces error -2
that lets it reach the final state and exit.
The risk is low since it requires about 50 name servers to reach that
state, but it's not impossible, especially when using multiple sections.
In addition, the extraneous line breaks, if sent over an interactive
connection, will desynchronize the commands and make the client believe
the end was reached after the first nameserver. This cannot be fixed
separately because that would turn this bug into an infinite loop since
it's the line feed that manages to fill the buffer and stop it.
The fix consists in saving the current resolvers section into ctx.cli.p1
and the current nameserver into ctx.cli.p2.
This should be backported, but that code moved a lot since it was
introduced and has always been bogus. It looks like it has mostly
stabilized in 2.4 with commit c943799c86 so the fix might be backportable
to 2.4 without too much effort.
MEDIUM: resolvers: create a "default" resolvers section at startup
Try to create a "default" resolvers section at startup, but does not
display any error nor warning. This section is initialized using the
/etc/resolv.conf of the system.
This is opportunistic and with no guarantee that it will work (but it should
on most systems).
This is useful for the httpclient as it allows to use the DNS resolver
without any configuration in most of the cases.
The function is called from the httpclient_pre_check() function to
ensure than we tried to create the section before trying to initiate the
httpclient. But it is also called from the resolvers.c to ensure the
section is created when the httpclient init was disabled.
MINOR: resolvers: move the resolv.conf parser in parse_resolv_conf()
Move the resolv.conf parser from the cfg_parse_resolvers so it could be
used separately.
Some changes were made in the memprintf in order to use a char **
instead of a char *. Also the variable is tested before each memprintf
so could skip them if no warnmsg nor errmsg were set.