haload is a client-side HTTP benchmarking tool designed to manage
concurrent HTTP streams.
This patch defines the hldstream C structure, which serves as the
core object to represent a haload HTTP stream for all the HTTP protocol.
It will be used by the upcoming haload module to handle specialized
stream contexts.
haload is the successor to the h1load HTTP benchmarking tool.
This patch adds haload stream definitions as arguments for the TRACE API.
These will be used by the upcoming haload module, which will handle
hldstream struct objects instead of regular stream structs.
Introduce the new <no_listener_mode> global variable to define a new operating mode
for haproxy. This variable can be set to 1 to allow haproxy to start without
any listeners. Without such a setting, haproxy refuses to start without listener.
During the initialization cycle, setting this variable to 1 ensures that the
lack of configured listeners is no longer treated as a fatal error. This allows
programs based on haproxy source code to initialize the stack and use its
features even without a frontend. This will be the case for haload.
Add a new lightweight hbuf API to buffer formatted strings, similar to the
existing buffer API (struct buffer), extracting the code which already does this
in haterm_init.c. This is used by haterm to build its configuration in memory
(fileless mode). And this will be used by haload to do the same thing.
Update haterm to use this new API.
Note: hstream_str_buf_append() has been renamed to hbuf_str_append().
port_range was never freed. That used not to be a problem, but now that
we can dynamically add and remove servers, it becomes one, as that leads
to a memory leak each time a server with a "source" directive is destroyed.
However, just adding a free() is not enough. We have to add a refcount,
because the server is not the only one with a reference to it. We may
also have one in fdinfo, so that we know which port to release when we
finally close the fd.
So add a refcount, and make sure to call port_range_release() when a
server is destroyed.
This should be backported up to 3.0.
Document the new 'set server <b>/<s> name <newname>' CLI command in
management.txt.
The documentation states the two preconditions that gate the operation
(server must be in maintenance, server's name must not be statically
referenced via use-server / track / ARGT_SRV), notes that the command
is not gated by a per-backend opt-in directive (parity with 'add
server' / 'del server'), and mentions the EVENT_HDL_SUB_SERVER_NAME
event published on successful rename so Lua and other event consumers
know to subscribe to it.
Tests cover:
- error cases: missing name, not-in-maintenance, invalid chars
(rejected by invalid_char()), duplicate name in the same backend,
name-referenced server (use-server target, track target)
- same-name rename as a no-op success
- successful rename with verification via 'show servers state'
- old name no longer resolves after rename
- round-trip rename back to original name
- traffic still works after rename round-trip
The use-server and tracked-server cases exercise the SRV_F_NAME_REFD
gating added in the preceding patch. Servers pinned only via resolvers
(SRV_F_NON_PURGEABLE without SRV_F_NAME_REFD) remain renamable; that
positive case is not exercised here as it would require a real DNS
resolver in the test environment.
Add the ability to rename a HAProxy server at runtime via the CLI:
set server <backend>/<server> name <newname>
This is useful in slot-based dynamic scaling setups where servers are
pre-allocated with generic names (e.g. srv001, srv002) but the operator
wants the names to reflect the current workload (e.g. pod name or
IP:port) for observability and server-state-file consistency.
The implementation:
- validates the new name: non-empty, passes invalid_char() check
(allows [A-Za-z0-9_:.-]), and fits in the event data name field
- requires the server to be administratively in maintenance mode
(same precondition as 'del server')
- rejects the rename if the server has SRV_F_NAME_REFD set (use-server
target, track target, sample-fetch ARGT_SRV referent) - keeps the
running state consistent with the configuration text
- re-indexes the server in the name tree under thread_isolate(),
mirroring the locking pattern used by 'add server' / 'del server'
- publishes a new EVENT_HDL_SUB_SERVER_NAME event with the old and
new names so downstream consumers (logs, observability backends)
can track the rename
- frees the old name immediately under thread isolation: srv_name
sample consumers (ACLs, log formats, ...) act on the fetched pointer
within the current task and do not retain it across wake-ups, so
no extra deferred-free machinery is needed
There is no opt-in directive: like 'add server' and 'del server', the
operation is gated by the server's properties rather than by a
per-backend toggle. This avoids the runtime-surprise failure mode
where an operator discovers at the CLI that renaming is forbidden by
a missing 'option server-rename' rather than by an actual structural
reference.
This feature was discussed in:
https://github.com/haproxy/haproxy/issues/952
Until now, every form of "this server is referenced by something in the
running config" was collapsed onto a single flag, SRV_F_NON_PURGEABLE,
which prevents the server from being removed via 'del server'. This
catches everything but conflates two distinct properties:
- the server object itself is pinned by another runtime structure
(e.g. DNS resolution attached to it), versus
- the server's *name* is referenced statically (use-server rules,
track chains, sample-fetch arguments of type ARGT_SRV)
These differ for any operation that touches the name but not the
object identity, e.g. the runtime rename feature added next. Removing
a name-referenced server is still forbidden (the rule text would
dangle), but renaming such a server should also be forbidden for the
same reason - while renaming a resolver-pinned server is fine, since
the resolver holds the object pointer and doesn't care about the name.
Introduce SRV_F_NAME_REFD for the name-reference case and move the
three name-based setters (sample.c ARGT_SRV resolution, proxy.c
use-server resolution, server.c track chain setup) from
SRV_F_NON_PURGEABLE to SRV_F_NAME_REFD. The resolvers.c call site
keeps SRV_F_NON_PURGEABLE since it is the object-pinned case.
Adjust 'del server' to check both flags so the set of servers it
refuses to remove is unchanged: same observable behavior, just a
richer internal taxonomy.
A subsequent patch introducing 'set server name' will gate on
SRV_F_NAME_REFD only.
smp_fetch_srv_name() stored a raw pointer to srv->id in the sample
without setting SMP_F_CONST. Every other sibling id-pointer fetch
(smp_fetch_be_name on px->id, smp_fetch_fe_name on fe->id, the SSL
helpers using OBJ_nid2sn() / SSL_get_cipher_name(), etc.) correctly
sets SMP_F_CONST to prevent in-place mutation by converters such as
,upper / ,lower / ,regsub.
Without SMP_F_CONST, an expression like srv_name,lower would write
into srv->id for the lifetime of the process. In practice this has
gone unnoticed because srv->id is a private allocation that is never
read back by name, but the bug is real and the divergence from the
other id fetches is unintentional.
This becomes more important with the introduction of runtime server
renaming (next patch in series): SMP_F_CONST ensures that callers go
through smp_make_rw() / smp_dup() before mutating, isolating the
sample's bytes from the server's id storage.
This is a stand-alone fix and should be backported.
srv_settings_init() sets agent.rise but forgets agent.health, while
srv_settings_cpy() sets both. check.health is fixed up later when the
server's admin state is updated at startup, but nothing does the same
for agent.health.
This used to be harmless because servers were always set up through
srv_settings_cpy(). But since 49a619aca ("MEDIUM: proxy: no longer
allocate the default-server entry by default") the defsrv pointer is
NULL when a proxy has no "default-server" line, and srv_settings_cpy()
then falls back to srv_settings_init(). So a server whose agent-check is
declared entirely on its "server" line ends up with agent.health == 0,
which is below agent.rise.
The wrong value only bites when the server has to come back up. While it
stays up nobody notices agent.health is 0, but as soon as the regular
health check fails and recovers, agent.health is still 0 (below rise) and
check_notify_success() won't bring the server back up. The agent never
sends an explicit "up", which is the only thing that raises agent.health,
so the server stays down for good. Moving the agent settings to a
"default-server" line works around it.
Just initialise agent.health in srv_settings_init() like
srv_settings_cpy() already does.
This should be backported to 3.3 and 3.4.
Similar to the previous patch, complete HTTP/0.9 user traces by logging
received HTX headers on request (BE side) or response (FE). This is only
for debugging purpose : the final HTTP/0.9 content does not contain any
of these.
Add a user trace when HTTP/0.9 response is either emitted (FE side) or
received (BE). The status code is displayed despite not being present in
the HTTP/0.9 response.
Add traces to log the start-line of received (FE side) or sent (BE)
requests.
This uses a similar pattern with already supported HTTP/3 header traces.
However, this only requires minimal trace verbosity. This is because
these traces will be mostly useful for QUIC interop testing. However it
is probably not desirable to use advanced verbosity in this context to
avoid increasing the traces output.
Add user traces in qcm_init() and qcc_release(). This is useful to be
able to quickly account connection allocation/release without using the
developer trace level.
In a recent patch, dedicated HTTP/3 traces have been added to log the
transfered HTTP content, with the start/status line and headers as well.
This patch adjusts these traces, correcting "qcc" typo to "qcs". It also
now correctly pass qcc and qcs as argument, which is used for trace
follow.
No need to backport unless HTTP/3 header traces are picked to previous
releases.
On the backend side, HTTP/0.9 transcoder is responsible to convert a HTX
request into a HTTP start line. In particular, path is generated from
the HTX request URI.
However, an absolute URI was not converted correctly in a HTTP/0.9
simple path. This occurs notably in most cases when using HTTP/2 or 3 on
the frontend side.
This issue was detected when running QUIC interop. Some servers
implementation such as picoquic would reject these requests as they are
considered invalid.
To adjust this, extract the path component from HTX uri using
http_uri_parser API.
This should be backported up to 3.3 as this is a QUIC backend fix.
The below patch has implemented support for response wrapping in HTTP/0.9
transcoder, similar to what is already performed in HTTP/3.
1e144c488c
BUG/MINOR: hq-interop: support response buffer wrapping
However, some bits were incorrectly written and the transcoding would
not be able to handle all of the wrapping data in one pass, despite no
crash possible. This patch fixes these, so that a wrapping response can
be handled in a single pass by the HTTP/0.9 transcoder if there is
enough room in the output HTX buffer.
This should fix github issue #3430.
This should be backported up to 3.3.
The current httpclient implementation does not initialize its server
with the options from the global section: ciphers, ciphersuites and
various SSL options are always the default of the SSL library.
This patch changes the behavior and apply the ssl-default-server-*
keywords to the httpclient SSL server.
When freeing a QUIC stream (qcs), the receive application buffer
(qcs->rx.app_buf) was not released if it still contained data or
had been allocated. This led to a memory leak over time as streams
were opened and closed.
Fix this by explicitly freeing qcs->rx.app_buf via b_free() in
qcs_free() if its size is non-zero, and call offer_buffers() to
notify the buffer pool.
This should be backported as far as 2.6.
When removing support for USE_OT=1 I only saw the tests but not the
build matrix, and this now causes build failures on the CI for tests
with "all features". Let's remove it there as well as the checks for
the OT cache and libs.
Fix segfault when using HTTP/3 header traces on the frontend side. This
occured because headers list was dumped prior to the insertion of the
end marker.
This issue is introduced by the following patch :
commit 00c081b5f3
MINOR: h3: trace HTTP headers on FE side
No need to backport, unless HTTP/3 header traces are picked to previous
releases.
OpenTracing support has long been best-effort and was deprecated in 3.3
with removal planned in 3.5. Let's clean it up now.
This commit removes addons/ot, the build script, ARGC_OT, USE_OT and
OT_* variables in the Makefile, and replaces the config section with a
mention for the OpenTelemetry filter instead.
For more info, see GH issues #1640 and #2782, as well as the wiki's
"breaking changes" page.
In tcp_connect_server(), uxst_connect_server(), and quic_connect_server(),
we can no longer see obj_type(conn->target) == OBJ_TYPE_PROXY so let's
drop that code. This implies that srv may no longer be NULL so we can
drop these checks as well.
Since a connection's target may no longer be a proxy and is necessarily
a server, let's simplify such checks. This is essentially in mux install
code and in the debugging code.
"show fd" supports various flags, one of which is 'd' for "dispatch",
which also catches "transparent", in fact, connections whose target is
a proxy. Since these can no longer happen, let's remove that. The 'b'
and 's' flags are now aliases of each other for simplicity.
These ones were deprecated in 3.3-dev2 with commits 5c15ba5eff ("MEDIUM:
proxy: mark the "dispatch" directive as deprecated") and e93f3ea3f8
("MEDIUM: proxy: deprecate the "transparent" and "option transparent"
directives"), and were planned for removal in 3.5. See also:
https://github.com/orgs/haproxy/discussions/2921
as well as the wiki page about breaking changes.
They've lived their lives and always cause internal limitations
(exceptions between connecting to server or connecting to proxy), and
are even confusing to some extents (especially "transparent" which users
often get wrong).
This commit removes the ability to configure them, tests based on them
and all the doc related to them. The keywords remain detected by the
parser and indicate how to proceed instead.
It's likely that other deeper parts will be changed as well (e.g.
conn->target will no longer be of OBJ_TYPE_PROXY). This will be done
over the long term.
It's already possible to report that some options are not supported due
to build options by passing 0 instead of PR_CAP_* in the option's cap
field. Let's extend that by passing a non-zero value in the val field,
where the 3rd byte will be the major version and the 4th one the minor.
In this case haproxy will now indicate that support for that option was
removed in that version.
Released version 3.5-dev1 with the following main changes :
- BUG/MEDIUM: check: Skip tcpcheck post-config for external checks
- BUG/MEDIUM: check: Ignore small-buffer option when starting an external check
- MINOR: check: Don't dump buffers state in check traces for external checks
- BUG/MEDIUM: server/checks: Support healtcheck keyword on default-server lines
- BUG/MEDIUM: mux_quic: prevent risk of infinite loop on recv
- OPTIM: mux_quic: remove QCS from recv_list on reset
- BUG/MINOR: mux_quic: do not interrupt recv on error/incomplete data
- BUG/MINOR: tcpcheck: Override external check if healthcheck section is set
- REGTESTS: checks: Add script for external healthchecks
- BUG/MEDIUM: regex: initialize the match array earlier during boot
- BUG/MEDIUM: threads: Fiw build when using no thread
- BUG/MEDIUM: xprt_qmux: implement ->get_ssl_sock_ctx() to get the SSL laye
- CLEANUP: sessions: simplify the sess_priv_conns pool name
- MINOR: pools: reject creation of pools containing invalid chars in their name
- BUG/MINOR: acl: report "ACL" not "map" in ACL ID lookup failures
- MINOR: memprof: make in_memprof a bitfield instead of a counter
- MINOR: memprof: be careful to account allocations only once
- BUG/MEDIUM: checks: Dequeue checks on purge
- MINOR: servers: Add a back-pointer to the server in srv_per_thread
- MEDIUM: servers: Move to a per-thread idle connection cleanup task
- REGTESTS: Fix log matching in healthcheck-section.vtc
- BUG/MINOR: quic: fix Initial length value in sent packets
- BUILD: servers: Fix build with -std=gnu89
- BUG/MEDIUM: acme: stuck ACME task when authz is already "valid"
- MINOR: acme: introduce acme_challenge_ready() for reuse outside the CLI
- MINOR: h3: extend trace verbosity
- MINOR: h3: trace HTTP headers on FE side
- MINOR: h3: trace HTTP headers on BE side
- BUILD: h3: fix compilation with USE_TRACE=0
- MINOR: lua: add REGISTER_HLUA_STATE_INIT() to register state init callbacks
- MEDIUM: lua: move longjmp annotation macros to hlua.h
- MINOR: acme/lua: implement ACME.challenge_ready() Lua function
- BUG/MEDIUM: ktls: defer enabling TLS ULP on a socket until connected
- MINOR: errors: add ha_diag_notice() to report diag-level notifications
- BUG/MINOR: cpu-topo: use ha_diag_notice() to report thread creations
- MINOR: acme: publish ACME_NEWCERT event via event_hdl
- MINOR: acme: publish ACME_DEPLOY event via event_hdl
- EXAMPLES: lua/acme: add a dns-01 handler for Gandi LiveDNS API
- DOC: acme: add mentions of lua features
- MINOR: tasks: Introduce __task_set_state_and_tid
- MINOR: tasks: Add __task_get_new_tid_field()
- MINOR: tasks: Introduce __task_get_current_owner
- MINOR: tasks: Use __task_get_current_owner() in task_kill.
- MINOR: tasks: Start using __task_set_state_and_tid()
- MEDIUM: tasks: Remove the per-thread group wait queue
- MINOR: tasks: Use __task_set_state_and_tid() in task_instant_wakeup()
- MINOR: tasks: Remove wq_lock and the per-thread group wait queues
- MEDIUM: tasks: Redispatch shared tasks when the thread is loaded
- BUG/MEDIUM: h3: Properly handle PUSH_PROMISE on backend connections
- BUG/MINOR: server: fix add server with consistent hash balancing
- MINOR: lua: export hlua_pusherror() and check_args()
- REORG: httpclient/lua: move the lua httpclient code to http_client.c
- MEDIUM: httpclient/lua: allow multiple requests from a single core.httpclient() instance
- MEDIUM: httpclient: set res.status to 0 upon SF_ERR_MASK
- DOC: httpclient: document status 0 on internal error
- DEBUG: stconn: Add a BUG_ON on shut flags when the endpoint is shut
- BUG/MINOR: http-ana: Remove a debugging memset on redirect
- BUG/MEDIUM: http-ana: Don't ignore L7 retry errors
- BUG/MINOR: mux-h1: Properly resolve file path for 'h1-case-adjust-file'
- BUG/MINOR: quic: fix rxbuf settings on backend side
- EXAMPLES: lua/acme: fix acme-gandi-livedns.lua configuration example
- BUG/MEDIUM: ssl: Don't free the early data buffer too early
- BUG/MINOR: hpack-tbl: add missing NULL check after hpack_dht_defrag()
- BUG/MEDIUM: mux_quic: fix freeze transfer after QCS rxbuf realign
- BUG/MEDIUM: http-act: Make a copy of the sample expr in (set/add)-headers-bin
- BUG/MEDIUM: mux-fcgi: fix uint16_t overflow in drl += drp
- OPTIM: mux-fcgi: Reorganise fcgi_conn structure to fill some holes
- BUG/MINOR: hq-interop: reject too big content
- MINOR: hq_interop: do not rely on stream layer for HTX stline encoding
- BUG/MINOR: hq-interop: prevent reset if missing content-length
- BUG/MEDIUM: hlua: Properly report EOS when http applet exits
- BUG/MINOR: hq-interop: support full demux buf on large response
- BUG/MINOR: hq-interop: support response buffer wrapping
- DOC: sched: Document the wait queue modifications
- DOC: lua: remove incorrect init tags
- BUG/MEDIUM: h3: increment unknown request payload length
- REGTESTS: quic: test H3 request without content-length
- DEBUG: cli: relax tid check in "debug dev task" for recent sched changes
- MINOR: debug: add "print" to "debug dev sched"
- BUG/MINOR: poller: fix wait time calculation that is always 1 extra ms
- BUILD: quic_pacing: add missing includes for api and activity in the file
- MINOR: task: move the profiling checks to the called functions not callers
- MINOR: task: add a new explicitly local tasklet wakeup function
- MINOR: task: make tasklet_wakeup() explicitly call _tasklet_wakeup_here()
- MINOR: task: make task_instant_wakeup() explicitly call _tasklet_wakeup_here()
- MEDIUM: task: make __tasklet_wakeup_on() only accept non-local threads
- MEDIUM: task: add a new flag TASK_RT to permit a task to skip the priority queue
- MINOR: debug: add "rt=1" to "debug dev task" to tune the RT flag
- BUG/MEDIUM: mux-fcgi: Truly drain outgoing HTX data when the stream is closed
- BUG/MEDIUM: mux-h2: Truly drain outgoing HTX data when the stream is closed
- BUG/MEDIUM: mux-spop: Truly drain outgoing data when the stream is closed
- BUG/MEDIUM: mux-quic: Drain the given amount of data in qcs_http_reset_buf()
- CLEANUP: task: remove duplicated code in __tasklet_wakeup_after()
- BUILD: task: silence a build warning with threads disabled
- MINOR: task: do not try to redistribute the WQ when single-threaded
- MEDIUM: task: add a new tasklet class for real-time: TL_RT
This adds new class TL_RT, which is processed before other queues for
one (and only one) tasklet featuring the TASK_RT flag. This is meant to
process real time wakeups under load with even less latency. We only
process one entry to make sure it will not be abused for unimportant
stuff, and if tune.sched.low-latency is set, we also avoid picking more
tasks from the current run queues and looping after the first call to
run_tasks_from_list().
Measurements under a load of 10k concurrent conns injection at 10 Gbps
(~58k 20kB objects/s) on 4 threads and with task profiling enabled shows
that the average wakeup latency for wakeups every 10ms dropped from 220
microseconds to 1.8 microsecond, and even ~550 nanoseconds when
tune.sched.low-latency is set, or 400 times less.
The doc was updated, including the schematics.
When running with nbthread=1, we still try to redistribute once, it
fails (new_tid=tid) and leaves the loop. That's just a waste for no
reason. Let's condition the redispatch to the presence of at least
another thread.
The compiler doesn't know that a random value based on global.nbthread
is necessarily smaller than MAX_THREADS, and when picking a random
thread number while single-threaded it complains that new_tid 1 is
out of bounds for the array. In fact all this is dead code in this
case.
Let's tell it about it to silence the warning.
In the case where the task is first inserted (!head), the code is
exactly __tasklet_wakeup_here(), so let's rely on this one. The
profiling and rq_total parts are already handled there so let's
move them to the head!=NULL branch.
Name of qcs_http_reset_buf() function is confusing. But the comment is
clear. In this function, a given amount of HTX data must be cleared from the
buffer. However, concretely, the whole buffer was always reset. Most of time
it is equivalent but it could be possible to keep unsent data in the
buffer. For instance, when a filter is registered on the data forwarding
stage.
So, instead of calling htx_reset(), htx_drain() must be used.
This patch must be backported to all supported version.
After the H2 and the FCGI multiplexers, it is the third mux concerned by
this issue.
When we try to send data to the server and the stream is closed (in error,
in half-closed state or fully closed), remaining data must be drained. This
way the upper stream is able to properly handle the stream close.
However, there was a bug here. The mux claimed to have consumed these data
without draining them from the buffer. The issue was never reported on the
SPOP multiplexer. But, in theory, the same than for the FCGI multiplexer is
possible.
This patch must be backported as far as 3.2.
It is the same bug than the previous one on the FCGI mux.
When we try to send data to the server and the stream is closed (in error,
in half-closed state or fully closed), remaining data must be drained. This
way the upper stream is able to properly handle the stream close.
However, there was a bug here. The mux claimed to have consumed these data
without draining them from the buffer. The issue was never reported on the
H2 multiplexer. But, in theory, the same than for the FCGI multiplexer is
possible.
Tihs patch must be backported to all supported versions.
When we try to send data to the server and the stream is closed (in error,
in half-closed state or fully closed), remaining data must be drained. This
way the upper stream is able to properly handle the stream close.
However, there was a bug here. The mux claimed to have consumed these data
without draining them from the buffer. So the upper stream will try to send
these data in loop. Because of this bug, it is possible to trigger the
watchdog with a bogus stream.
This patch should fix the issue #3425. It must be backported to all
supported versions.
For some very rare tasks that need to be woken up at an exact date (right
now the only known use case is haload's periodic stats collection), it's
currently difficult to guarantee the wake up date on a heavily loaded
run queue.
This patch introduces TASK_RT for real-time tasks. Right now, all it does
is modify __task_wakeup() to immediately switch to __tasklet_wakeup_*()
and effectively bypass the priority-based run queue. Doing it here has
the benefit of making sure that it automatically applies to tasks found
in the wait queue, and that it will also work for _task_drop_running().
For now nothing uses it. The doc was updated.
The ambiguity in usage for __tasklet_wakeup_on() is now gone. All known
callers that used to be able to pass a negative value now call
__tasklet_wakeup_here(), and remaining ones always pass an explicit
thread number. This means that we can remove the "if (thr<0)" branch,
but still leave a BUG_ON_HOT() to catch any possibly missed case. The
comment around tasklet_wakeup_on() not supporting remotely waking a
tasklet whose tid<0 was also removed since it was addressed long ago.
This patch moves the tid check upper in the chain, in task_instant_wakeup()
so as to branch to _tasklet_wakeup_here() for run-anywhere tasks, or
_tasklet_wakeup_on() for designated threads.
At this point there is no longer any direct caller of __tasklet_wakeup_on()
passing a negative thread value.
This patch moves the tid check upper in the chain, in tasklet_wakeup()
so as to branch to _tasklet_wakeup_here() for run-anywhere tasklets, or
_tasklet_wakeup_on() for designated threads. The tid is retrieved via
__task_get_current_owner() so that the call remains compatible with
tasklets that would have a super-negative tid due to being tasks used
as tasklets.
The current tasklet_wakeup() call relies on tasklet_wakeup_on(tl->tid),
which was already quite ambiguous till now due to the sole reliance on
tid being negative or not to decide to run locally, but it no longer
works correctly if used to wake tasks up since the new set of possible
negative values for ->tid (particularly if some code calls
__tasklet_wakeup_on() on a task as is done in task_instant_wakeup()).
The problem is that it is not possible in the current API to explicitly
say that we want a task/tasklet to run locally or remotely without having
to play games with a thread number. The chosen approach to address this
is to change tasklet_wakeup_on() to always be remote and have
tasklet_wakeup_here() which will always be local, with tasklet_wakeup()
choosing one or the other depending on the tid, for backwards compat
only.
This patch implements tasklet_wakeup_here() to __tasklet_wakeup_here()
that reimplement the part of __tasklet_wakeup_on() that used to deal
with the local thread only (negative tid). No other change was made.
For now it remains unused.
The doc was updated.
The checks on TH_FL_TASK_PROFILING that are used to decide whether or not
to set t->wake_date from now_mono_time() used to be made in callers of
__tasklet_wakeup_on() and __tasklet_wakeup_after(), but not only this
needlessly inflates code by placing this in every caller (~4kB), it also
renders the design fragile since each caller needs to blindly copy-paste
that statement.
Let's move the operation in the callees instead. As a bonus, it allows
to check the flag on the target thread and not on the calling thread
(which was arguably a bug though without a noticeable effect since for
now profiling is for all threads or none).
quic_pacing.c is missing a number of include files that it got by chance
through task.h, resulting in build breakage as soon as that one gets
cleaned up. Let's add api.h and activity.h that are needed. No backport
is needed.
In 1.3.11, 19 years ago, commit bdefc513a0 ("[BUG] fix null timeouts in
*poll-based pollers") addressed an issue where some wakeup times could
sometimes be rounded to less than one millisecond (by then they were
calculated on timeval), and would make the poller wake up too early and
loop with a timeout of zero. The solution used by then consisted in
always adding 1 to the wait delay so that poll() was never called with
a null timeout.
Nowadays our internal wakeup delays are in milliseconds so we cannot wake
too early, all the timeout calculation was moved to compute_poll_timeout()
which has a specific check for expired next wakeup event, so we cannot
even have a null timeout as a result of a real delay calculation by
accident. Yet, it's clearly visible with strace thats a task created
with an interval of 10ms results in a poll timeout of 11ms, causing some
small time drift in periodic wakeups.
Let's just now drop this "+1" which is no longer needed nor relevant and
only causes wrong delays to be calculated. Now creating a time-printing
task results in correct delays passed to poll() and measured intervals
around:
- ~10.3ms interval for 10ms
- ~100.5ms for 100ms
- ~1001ms for 1000ms
E.g:
$ socat - /tmp/sock1 <<< "expert-mode on;debug dev sched task print=1 inter=10 count=1"
(...)
17:58:05.191885 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=44590744}) = 0
17:58:05.191919 epoll_wait(4, [], 200, 10) = 0
17:58:05.202215 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=44601494}) = 0
17:58:05.202237 write(1, "task 0x3aeeb080: time_ms=3553"..., 42task 0x3aeeb080: time_ms=355304053.757383
) = 42
17:58:05.202253 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=44610199}) = 0
17:58:05.202265 epoll_wait(4, [], 200, 10) = 0
17:58:05.212579 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=44631754}) = 0
17:58:05.212639 write(1, "task 0x3aeeb080: time_ms=3553"..., 42task 0x3aeeb080: time_ms=355304064.157626
) = 42
These delays with longer sleeps are entirely on the system side, most
likely due to the CPU switching to low-power for such long delays (tests
run on a laptop).
There is no reason to backport this fix, though it shouldn't hurt either.
Passing "print=1" for a periodic task will cause it to print the exact
monotonic time at each wakeup to stdout and do nothing else. This is
convenient to watch wakeup delay drifts.
Since commit 0988b9c773 ("MEDIUM: tasks: Remove the per-thread group
wait queue") in 3.5-dev, a task's tid may be as negative as -MAX_THREAD-1
and not just -1, so we must accept this when trying to check if a pointer
looks like a valid task.
No backport is needed.
