]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoBUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello error
Remi Tricot-Le Breton [Fri, 7 Jan 2022 16:12:01 +0000 (17:12 +0100)] 
BUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello error

If an error is raised during the ClientHello callback on the server side
(ssl_sock_switchctx_cbk), the servername callback won't be called and
the client's SNI will not be saved in the SSL context. But since we use
the SSL_get_servername function to return this SNI in the ssl_fc_sni
sample fetch, that means that in case of error, such as an SNI mismatch
with a frontend having the strict-sni option enabled, the sample fetch
would not work (making strict-sni related errors hard to debug).

This patch fixes that by storing the SNI as an ex_data in the SSL
context in case the ClientHello callback returns an error. This way the
sample fetch can fallback to getting the SNI this way. It will still
first call the SSL_get_servername function first since it is the proper
way of getting a client's SNI when the handshake succeeded.

In order to avoid memory allocations are runtime into this highly used
runtime function, a new memory pool was created to store those client
SNIs. Its entry size is set to 256 bytes since SNIs can't be longer than
255 characters.

This fixes GitHub #1484.

It can be backported in 2.5.

3 years agoBUG/MEDIUM: mworker: don't use _getsocks in wait mode
William Lallemand [Fri, 7 Jan 2022 17:19:42 +0000 (18:19 +0100)] 
BUG/MEDIUM: mworker: don't use _getsocks in wait mode

Since version 2.5 the master is automatically re-executed in wait-mode
when the config is successfully loaded, puting corner cases of the wait
mode in plain sight.

When using the -x argument and with the right timing, the master will
try to get the FDs again in wait mode even through it's not needed
anymore, which will harm the worker by removing its listeners.

However, if it fails, (and it's suppose to, sometimes), the
master will exit with EXIT_FAILURE because it does not have the
MODE_MWORKER flag, but only the MODE_MWORKER_WAIT flag. With the
consequence of killing the workers.

This patch fixes the issue by restricting the use of _getsocks to some
modes.

This patch must be backported in every version supported, even through
the impact should me more harmless in version prior to 2.5.

3 years agoMINOR: quic: Non-optimal use of a TX buffer
Frédéric Lécaille [Fri, 7 Jan 2022 13:32:31 +0000 (14:32 +0100)] 
MINOR: quic: Non-optimal use of a TX buffer

When full, after having reset the writer index, let's reuse the TX buffer in
any case.

3 years agoMINOR: quic: Missing retransmission from qc_prep_fast_retrans()
Frédéric Lécaille [Thu, 6 Jan 2022 16:28:05 +0000 (17:28 +0100)] 
MINOR: quic: Missing retransmission from qc_prep_fast_retrans()

In fact we must look for the first packet with some ack-elicting frame to
in the packet number space tree to retransmit from. Obviously there
may be already retransmit packets which are not deemed as lost and
still present in the packet number space tree for TX packets.

3 years agoMINOR: quic: Only one CRYPTO frame by encryption level
Frédéric Lécaille [Tue, 4 Jan 2022 22:15:40 +0000 (23:15 +0100)] 
MINOR: quic: Only one CRYPTO frame by encryption level

When receiving CRYPTO data from the TLS stack, concatenate the CRYPTO data
to the first allocated CRYPTO frame if present. This reduces by one the number
of handshake packets built for a connection with a standard size certificate.

3 years agoBUILD: makefile: add -Wno-atomic-alignment to work around clang abusive warning
Willy Tarreau [Fri, 7 Jan 2022 13:51:56 +0000 (14:51 +0100)] 
BUILD: makefile: add -Wno-atomic-alignment to work around clang abusive warning

As reported in github issue #1502, clang, when building for i386, will
try to use CMPXCHG8B-based loops for 64-bit atomic operations, and emits
warnings for all 64-bit operands that are not 64-bit aligned, an alignment
that is *not* required by the ABI, that the compiler itself does not
enforce, and that the intel SDM clearly says is not required on this
32-bit platform for this operation. But this is likely an excessive
outcome of the same code being used in 64-bit for CMPXCHG16B which does
require proper alignment. Firefox already gave up on this one 3 years
ago, let's not waste our time arguing and just shut up the warning
instead. It might hide some real bugs in the future but till now
experience showed that overall it's unlikely.

This should be backported to all maintained branches that use 64-bit
atomic ops (e.g. for counters).

Thanks to Brad Smith for reporting it and confirming that shutting the
warning addresses it.

3 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Fri, 7 Jan 2022 09:46:15 +0000 (14:46 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 30th iteration of typo fixes

3 years agoCI: refactor spelling check
Ilya Shipitsin [Fri, 7 Jan 2022 09:42:54 +0000 (14:42 +0500)] 
CI: refactor spelling check

let us switch to codespell github actions instead of invocation from cmdline.
also, "ifset,thrid,strack,ba,chck,hel,unx,mor" added to whitelist, those are
variable names and special terms widely used in HAProxy

3 years agoMINOR: cpuset: switch to sched_setaffinity for FreeBSD 14 and above.
David CARLIER [Thu, 6 Jan 2022 18:53:50 +0000 (18:53 +0000)] 
MINOR: cpuset: switch to sched_setaffinity for FreeBSD 14 and above.

Following up previous update on cpuset-t.h. Ultimately, at some point
 the cpuset_setaffinity code path could be removed.

3 years agoMINOR: proxy: add option idle-close-on-response
William Dauchy [Wed, 5 Jan 2022 21:53:24 +0000 (22:53 +0100)] 
MINOR: proxy: add option idle-close-on-response

Avoid closing idle connections if a soft stop is in progress.

By default, idle connections will be closed during a soft stop. In some
environments, a client talking to the proxy may have prepared some idle
connections in order to send requests later. If there is no proper retry
on write errors, this can result in errors while haproxy is reloading.
Even though a proper implementation should retry on connection/write
errors, this option was introduced to support back compat with haproxy <
v2.4. Indeed before v2.4, we were waiting for a last request to be able
to add a "connection: close" header and advice the client to close the
connection.

In a real life example, this behavior was seen in AWS using the ALB in
front of a haproxy. The end result was ALB sending 502 during haproxy
reloads.
This patch was tested on haproxy v2.4, with a regular reload on the
process, and a constant trend of requests coming in. Before the patch,
we see regular 502 returned to the client; when activating the option,
the 502 disappear.

This patch should help fixing github issue #1506.
In order to unblock some v2.3 to v2.4 migraton, this patch should be
backported up to v2.4 branch.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
[wt: minor edits to the doc to mention other options to care about]
Signed-off-by: Willy Tarreau <w@1wt.eu>
3 years agoMINOR: quic: Re-arm the PTO timer upon datagram receipt
Frédéric Lécaille [Tue, 4 Jan 2022 16:03:11 +0000 (17:03 +0100)] 
MINOR: quic: Re-arm the PTO timer upon datagram receipt

When block by the anti-amplification limit, this is the responsability of the
client to unblock it sending new datagrams. On the server side, even if not
well parsed, such datagrams must trigger the PTO timer arming.

3 years agoMINOR: quic: PTO timer too often reset
Frédéric Lécaille [Tue, 4 Jan 2022 15:59:42 +0000 (16:59 +0100)] 
MINOR: quic: PTO timer too often reset

It must be reset when the anti-amplication was reached but only if the
peer address was not validated.

3 years agoMINOR: quic: Flag asap the connection having reached the anti-amplification limit
Frédéric Lécaille [Tue, 4 Jan 2022 15:57:37 +0000 (16:57 +0100)] 
MINOR: quic: Flag asap the connection having reached the anti-amplification limit

The best location to flag the connection is just after having built the packet
which reached the anti-amplication limit.

3 years agoMINOR: quic: Prepare Handshake packets asap after completed handshake
Frédéric Lécaille [Mon, 3 Jan 2022 16:25:53 +0000 (17:25 +0100)] 
MINOR: quic: Prepare Handshake packets asap after completed handshake

Switch back to QUIC_HS_ST_SERVER_HANDSHAKE state after a completed handshake
if acks must be send.
Also ensure we build post handshake frames only one time without using prev_st
variable and ensure we discard the Handshake packet number space only one time.

3 years agoMINOR: quic: Do not drop secret key but drop the CRYPTO data
Frédéric Lécaille [Mon, 3 Jan 2022 16:00:35 +0000 (17:00 +0100)] 
MINOR: quic: Do not drop secret key but drop the CRYPTO data

We need to be able to decrypt late Handshake packets after the TLS secret
keys have been discarded. If not the peer send Handshake packet which have
not been acknowledged. But for such packets, we discard the CRYPTO data.

3 years agoMINOR: quic: Improve qc_prep_pkts() flexibility
Frédéric Lécaille [Mon, 3 Jan 2022 10:14:30 +0000 (11:14 +0100)] 
MINOR: quic: Improve qc_prep_pkts() flexibility

We want to be able to choose the encryption levels to be used
by qc_prep_pkts() outside of it.

3 years agoMINOR: quic: Comment fix.
Frédéric Lécaille [Fri, 31 Dec 2021 15:37:58 +0000 (16:37 +0100)] 
MINOR: quic: Comment fix.

When we drop a packet with unknown length, this is the entire datagram which
must be skipped.

3 years agoMINOR: quic: Probe several packet number space upon timer expiration
Frédéric Lécaille [Fri, 31 Dec 2021 15:35:28 +0000 (16:35 +0100)] 
MINOR: quic: Probe several packet number space upon timer expiration

When the loss detection timer expires, we SHOULD include new data in
our probing packets (RFC 9002 par 6.2.4. Sending Probe Packets).

3 years agoMINOR: quic: Probe Initial packet number space more often
Frédéric Lécaille [Thu, 30 Dec 2021 22:16:51 +0000 (23:16 +0100)] 
MINOR: quic: Probe Initial packet number space more often

Especially when the PTO expires for Handshake packet number space and when
Initial packets are still flying (for QUIC servers).

3 years agoMINOR: quic: Speeding up Handshake Completion
Frédéric Lécaille [Thu, 30 Dec 2021 15:14:20 +0000 (16:14 +0100)] 
MINOR: quic: Speeding up Handshake Completion

According to RFC 9002 par. 6.2.3. when receving duplicate Initial CRYPTO
data a server may a packet containing non unacknowledged before the PTO
expiry.

3 years agoMINOR: quic: qc_prep_pkts() code moving
Frédéric Lécaille [Wed, 29 Dec 2021 16:18:21 +0000 (17:18 +0100)] 
MINOR: quic: qc_prep_pkts() code moving

Move the switch default case code out of the switch to improve the readibily.

3 years agoMINOR: quic: Useless test in qc_prep_pkts()
Frédéric Lécaille [Wed, 29 Dec 2021 15:00:47 +0000 (16:00 +0100)] 
MINOR: quic: Useless test in qc_prep_pkts()

These tests were there to initiate PTO probing but they are not correct.
Furthermore they may break the PTO probing process and lead to useless packet
building.

3 years agoMINOR: quic: Wrong packet number space trace in qc_prep_pkts()
Frédéric Lécaille [Wed, 29 Dec 2021 14:36:25 +0000 (15:36 +0100)] 
MINOR: quic: Wrong packet number space trace in qc_prep_pkts()

It was always the first packet number space information which was dumped.

3 years agoMINOR: quic: Remove nb_pto_dgrams quic_conn struct member
Frédéric Lécaille [Wed, 29 Dec 2021 11:04:13 +0000 (12:04 +0100)] 
MINOR: quic: Remove nb_pto_dgrams quic_conn struct member

For now on we rely on tx->pto_probe pktns struct member to inform
the packet building function we want to probe.

3 years agoMINOR: quic: Wrong ack_delay compution before calling quic_loss_srtt_update()
Frédéric Lécaille [Tue, 28 Dec 2021 13:27:43 +0000 (14:27 +0100)] 
MINOR: quic: Wrong ack_delay compution before calling quic_loss_srtt_update()

RFC 9002 5.3. Estimating smoothed_rtt and rttvar:
MUST use the lesser of the acknowledgment delay and the peer's max_ack_delay
after the handshake is confirmed.

3 years agoMINOR: quic: Wrong loss time computation in qc_packet_loss_lookup()
Frédéric Lécaille [Mon, 27 Dec 2021 17:15:27 +0000 (18:15 +0100)] 
MINOR: quic: Wrong loss time computation in qc_packet_loss_lookup()

This part as been modified by the RFC since our first implementation.

3 years agoMINOR: quic: Wrong packet number space computation for PTO
Frédéric Lécaille [Mon, 27 Dec 2021 17:13:13 +0000 (18:13 +0100)] 
MINOR: quic: Wrong packet number space computation for PTO

This leaded to make quic_pto_pktns() return 01RTT packet number space
when initiating a probing even if the handshake was not completed!

3 years agoMINOR: quic: Wrong first packet number space computation
Frédéric Lécaille [Mon, 27 Dec 2021 17:09:22 +0000 (18:09 +0100)] 
MINOR: quic: Wrong first packet number space computation

I really do not know where does these inversion come from.

3 years agoMINOR: quic: Add trace about in flight bytes by packet number space
Frédéric Lécaille [Mon, 27 Dec 2021 16:42:51 +0000 (17:42 +0100)] 
MINOR: quic: Add trace about in flight bytes by packet number space

This parameter is useful to diagnose packet loss detection issues.

3 years agoMINOR: quic: Wrong traces after rework
Frédéric Lécaille [Mon, 27 Dec 2021 14:12:09 +0000 (15:12 +0100)] 
MINOR: quic: Wrong traces after rework

TRACE_*() macros must take a quic_conn struct as first argument.

3 years agoBUG/MEDIUM: http-ana: Preserve response's FLT_END analyser on L7 retry
Christopher Faulet [Tue, 4 Jan 2022 09:56:03 +0000 (10:56 +0100)] 
BUG/MEDIUM: http-ana: Preserve response's FLT_END analyser on L7 retry

When a filter is attached on a stream, the FLT_END analyser must not be
removed from the response channel on L7 retry. It is especially important
because CF_FLT_ANALYZE flag is still set. This means the synchronization
between the two sides when the filter ends can be blocked. Depending on the
timing, this can freeze the stream infinitely or lead to a spinning loop.

Note that the synchronization between the two sides at the end of the
analysis was introduced because the stream was reused in HTTP between two
transactions. But, since the HTX was introduced, a new stream is created for
each transaction. So it is probably possible to remove this step for 2.2 and
higher.

This patch must be backported as far as 2.0.

3 years agoBUG/MINOR: cli: fix _getsocks with musl libc
William Lallemand [Mon, 3 Jan 2022 18:43:41 +0000 (19:43 +0100)] 
BUG/MINOR: cli: fix _getsocks with musl libc

In ticket #1413, the transfer of FDs couldn't correctly work on alpine
linux. After a few tests with musl on another distribution it seems to
be a limitation of this libc.

The number of FD that could be sent per sendmsg was set to 253, which
does not seem to work with musl, decreasing it 252 seems to work
better, so lets set this value everywhere since it does not have that
much impact.

This must be backported in every maintained version.

3 years agoBUILD/MINOR: tools: solaris build fix on dladdr.
David Carlier [Fri, 31 Dec 2021 08:15:29 +0000 (08:15 +0000)] 
BUILD/MINOR: tools: solaris build fix on dladdr.

dladdr takes a mutable address on this platform.

3 years agoCI: github actions: update OpenSSL to 3.0.1
Ilya Shipitsin [Sat, 25 Dec 2021 09:01:52 +0000 (14:01 +0500)] 
CI: github actions: update OpenSSL to 3.0.1

OpenSSL-3.0.1 was released on 14 Dec 2021, let's switch to it

3 years agoCLEANUP: assorted typo fixes in the code and comments This is 29th iteration of typo...
Ilya Shipitsin [Sat, 25 Dec 2021 06:45:52 +0000 (11:45 +0500)] 
CLEANUP: assorted typo fixes in the code and comments This is 29th iteration of typo fixes

3 years agoOPTIM: pools: reduce local pool cache size to 512kB
Willy Tarreau [Sun, 2 Jan 2022 18:38:54 +0000 (19:38 +0100)] 
OPTIM: pools: reduce local pool cache size to 512kB

Now that we support batched allocations/releases, it appears that we can
reach the same performance on H2 with shared pools and 256kB thread-local
cache as without shared pools, a fast allocator and 1MB thread-local cache.
With 512kB we're up to 10% faster on highly multiplexed H2 than without the
shared cache. This was tested on a 16-core ARM machine. Thus it's time to
slightly reduce the per-thread memory cost, which may also improve the
performance on machines with smaller L2 caches. It essentially reverts
commit f587003fe ("MINOR: pools: double the local pool cache size to 1 MB").

3 years agoMEDIUM: pools: release cached objects in batches
Willy Tarreau [Sun, 2 Jan 2022 16:53:02 +0000 (17:53 +0100)] 
MEDIUM: pools: release cached objects in batches

With this patch pool_evict_last_items builds clusters of up to
CONFIG_HAP_POOL_CLUSTER_SIZE entries so that accesses to the shared
pools are reduced by CONFIG_HAP_POOL_CLUSTER_SIZE and the inter-
thread contention is reduced by as much..

3 years agoMEDIUM: pools: start to batch eviction from local caches
Willy Tarreau [Sun, 2 Jan 2022 16:24:55 +0000 (17:24 +0100)] 
MEDIUM: pools: start to batch eviction from local caches

Since previous patch we can forcefully evict multiple objects from the
local cache, even when evicting basd on the LRU entries. Let's define
a compile-time configurable setting to batch releasing of objects. For
now we set this value to 8 items per round.

This is marked medium because eviction from the LRU will slightly change
in order to group the last items that are freed within a single cache
instead of accurately scanning only the oldest ones exactly in their
order of appearance. But this is required in order to evolve towards
batched removals.

3 years agoMEDIUM: pools: centralize cache eviction in a common function
Willy Tarreau [Sun, 2 Jan 2022 16:19:14 +0000 (17:19 +0100)] 
MEDIUM: pools: centralize cache eviction in a common function

We currently have two functions to evict cold objects from local caches:
pool_evict_from_local_cache() to evict from a single cache, and
pool_evict_from_local_caches() to evict oldest objects from all caches.

The new function pool_evict_last_items() focuses on scanning oldest
objects from a pool and releasing a predefined number of them, either
to the shared pool or to the system. For now they're evicted one at a
time, but the next step will consist in creating clusters.

3 years agoMINOR: pools: pass the objects count to pool_put_to_shared_cache()
Willy Tarreau [Sun, 2 Jan 2022 14:15:54 +0000 (15:15 +0100)] 
MINOR: pools: pass the objects count to pool_put_to_shared_cache()

This is in order to let the caller build the cluster of items to be
released. For now single items are released hence the count is always
1.

3 years agoMINOR: pools: prepare pool_item to support chained clusters
Willy Tarreau [Sun, 2 Jan 2022 13:35:57 +0000 (14:35 +0100)] 
MINOR: pools: prepare pool_item to support chained clusters

In order to support batched allocations and releases, we'll need to
prepare chains of items linked together and that can be atomically
attached and detached at once. For this we implement a "down" pointer
in each pool_item that points to the other items belonging to the same
group. For now it's always NULL though freeing functions already check
them when trying to release everything.

3 years agoMEDIUM: pool: compute the number of evictable entries once per pool
Willy Tarreau [Sat, 1 Jan 2022 23:27:06 +0000 (00:27 +0100)] 
MEDIUM: pool: compute the number of evictable entries once per pool

In pool_evict_from_local_cache() we used to check for room left in the
pool for each and every object. Now we compute the value before entering
the loop and keep into a local list what has to be released, and call
the OS-specific functions for the other ones.

It should already save some cycles since it's not needed anymore to
recheck for the pool's filling status. But the main expected benefit
comes from the ability to pre-construct a list of all releasable
objects, that will later help with grouping them.

3 years agoMINOR: pool: add a function to estimate how many may be released at once
Willy Tarreau [Sat, 1 Jan 2022 23:21:46 +0000 (00:21 +0100)] 
MINOR: pool: add a function to estimate how many may be released at once

At the moment we count the number of releasable objects to a shared pool
one by one. The way the formula is made allows to pre-compute the number
of available slots, so let's add a function for that so that callers can
do it once before iterating.

This takes into account the average number of entries needed and the
minimum availability per pool. The function is not used yet.

3 years agoMINOR: pool: introduce pool_item to represent shared pool items
Willy Tarreau [Sat, 1 Jan 2022 17:22:20 +0000 (18:22 +0100)] 
MINOR: pool: introduce pool_item to represent shared pool items

In order to support batch allocation from/to shared pools, we'll have to
support a specific representation for pool objects. The new pool_item
structure will be used for this. For now it only contains a "next"
pointer that matches exactly the current storage model. The few functions
that deal with the shared pool entries were adapted to use the new type.
There is no functionality difference at this point.

3 years agoMINOR: pool: check for pool's fullness outside of pool_put_to_shared_cache()
Willy Tarreau [Thu, 30 Dec 2021 16:37:33 +0000 (17:37 +0100)] 
MINOR: pool: check for pool's fullness outside of pool_put_to_shared_cache()

Instead of letting pool_put_to_shared_cache() pass the object to the
underlying OS layer when there's no more room, let's have the caller
check if the pool is full and either call pool_put_to_shared_cache()
or call pool_free_nocache().

Doing this sensibly simplifies the code as this function now only has
to deal with a pool and an item and only for cases where there are
local caches and shared caches. As the code was simplified and the
calls more isolated, the function was moved to pool.c.

Note that it's only called from pool_evict_from_local_cache{,s}() and
that a part of its logic might very well move there when dealing with
batches.

3 years agoMINOR: pool: make pool_is_crowded() always true when no shared pools are used
Willy Tarreau [Sat, 1 Jan 2022 20:00:07 +0000 (21:00 +0100)] 
MINOR: pool: make pool_is_crowded() always true when no shared pools are used

This function is used to know whether the shared pools are full or if we
can store more objects in them. Right now it cannot be used in a generic
way because when shared pools are not used it will return false, letting
one think pools can accept objects. Let's make one variant for each build
model.

3 years agoMINOR: pool: rely on pool_free_nocache() in pool_put_to_shared_cache()
Willy Tarreau [Thu, 30 Dec 2021 16:20:27 +0000 (17:20 +0100)] 
MINOR: pool: rely on pool_free_nocache() in pool_put_to_shared_cache()

At the moment pool_put_to_shared_cache() checks if the pool is crowded,
and if so it does the exact same job as pool_free_nocache(), otherwise
it adds the object there.

This patch rearranges the code so that the function is split in two and
either uses one path or the other, and always relies on pool_free_nocache()
in case we don't want to store the object. This way there will be a common
path with the variant not using the shared cache. The patch is better viewed
using git show -b since a whole block got reindented.

It's worth noting that there is a tiny difference now in the local cache
usage measurement, as the decrement of "used" used to be performed before
checking for pool_is_crowded() instead of being done after. This used to
result in always one less object being kept in the cache than what was
configured in minavail. The rearrangement of the code aligns it with
other call places.

3 years agoCLEANUP: pools: group list updates in pool_get_from_cache()
Willy Tarreau [Sun, 2 Jan 2022 18:34:19 +0000 (19:34 +0100)] 
CLEANUP: pools: group list updates in pool_get_from_cache()

Some changes affect the list element and others affect the pool stats.
Better group them together, as the compiler may not detect certain
possible optimizations after the casts made by the list macros.

3 years agoMINOR: pool: allocate from the shared cache through the local caches
Willy Tarreau [Thu, 30 Dec 2021 16:09:31 +0000 (17:09 +0100)] 
MINOR: pool: allocate from the shared cache through the local caches

One of the thread scaling challenges nowadays for the pools is the
contention on the shared caches. There's never any situation where we
have a shared cache and no local cache anymore, so we can technically
afford to transfer objects from the shared cache to the local cache
before returning them to the user via the regular path. This adds a
little bit more work per object per miss, but will permit batch
processing later.

This patch simply moves pool_get_from_shared_cache() to pool.c under
the new name pool_refill_local_from_shared(), and this function does
not return anything but it places the allocated object at the head of
the local cache.

3 years agoCLEANUP: pools: get rid of the POOL_LINK macro
Willy Tarreau [Sat, 1 Jan 2022 16:10:50 +0000 (17:10 +0100)] 
CLEANUP: pools: get rid of the POOL_LINK macro

The POOL_LINK macro is now only used for debugging, and it still requires
ifdefs around, which needlessly complicates the code. Let's replace it
and the calling code with a new pair of macros: POOL_DEBUG_SET_MARK()
and POOL_DEBUG_CHECK_MARK(), that respectively store and check the pool
pointer in the extra location at the end of the pool. This removes 4
pairs of ifdefs in the middle of the code.

3 years agoCLEANUP: pools: do not use the extra pointer to link shared elements
Willy Tarreau [Fri, 31 Dec 2021 15:00:19 +0000 (16:00 +0100)] 
CLEANUP: pools: do not use the extra pointer to link shared elements

This practice relying on POOL_LINK() dates from the era where there were
no pool caches, but given that the structures are a bit more complex now
and that pool caches do not make use of this feature, it is totally
useless since released elements have already been overwritten, and yet
it complicates the architecture and prevents from making simplifications
and optimizations. Let's just get rid of this feature. The pointer to
the origin pool is preserved though, as it helps detect incorrect frees
and serves as a canary for overflows.

3 years agoDOC: pool: document the purpose of various structures in the code
Willy Tarreau [Thu, 30 Dec 2021 15:35:32 +0000 (16:35 +0100)] 
DOC: pool: document the purpose of various structures in the code

The pools have become complex with the shared pools and the thread-local
caches, and the purpose of certain structures is never easy to grasp.
Let's add a bit of documentation there to save some long and painful
analysis to those touching that area.

3 years agoMINOR: pools: always evict oldest objects first in pool_evict_from_local_cache()
Willy Tarreau [Sun, 2 Jan 2022 11:40:14 +0000 (12:40 +0100)] 
MINOR: pools: always evict oldest objects first in pool_evict_from_local_cache()

For an unknown reason, despite the comment stating that we were evicting
oldest objects first from the local caches, due to the use of LIST_NEXT,
the newest were evicted, since pool_put_to_cache() uses LIST_INSERT().

Some tests on 16 threads show that evicting oldest objects instead can
improve performance by 0.5-1% especially when using shared pools.

3 years agoBUILD/MINOR: cpuset FreeBSD 14 build fix.
David CARLIER [Fri, 31 Dec 2021 05:00:12 +0000 (05:00 +0000)] 
BUILD/MINOR: cpuset FreeBSD 14 build fix.

The 14th release started to introduce api compatibility layer with Linux
for the cpuset part and doing so irrevocably change the CPU* macros as well.

3 years agoREGTESTS: ssl: update of a crt with server deletion
William Lallemand [Thu, 30 Dec 2021 13:57:32 +0000 (14:57 +0100)] 
REGTESTS: ssl: update of a crt with server deletion

This test verifies that a certificate is in a "Unused" state once every
server which uses it are dynamically removed.

3 years agoBUG/MEDIUM: ssl: free the ckch instance linked to a server
William Lallemand [Thu, 30 Dec 2021 13:45:19 +0000 (14:45 +0100)] 
BUG/MEDIUM: ssl: free the ckch instance linked to a server

This patch unlinks and frees the ckch instance linked to a server during
the free of this server.

This could have locked certificates in a "Used" state when removing
servers dynamically from the CLI. And could provoke a segfault once we
try to dynamically update the certificate after that.

This must be backported as far as 2.4.

3 years agoBUG/MINOR: ssl: free the fields in srv->ssl_ctx
William Lallemand [Thu, 30 Dec 2021 10:25:43 +0000 (11:25 +0100)] 
BUG/MINOR: ssl: free the fields in srv->ssl_ctx

A lot of free are missing in ssl_sock_free_srv_ctx(), this could result
in memory leaking when removing dynamically a server via the CLI.

This must be backported in every branches, by removing the fields that
does not exist in the previous branches.

3 years agoREGTESTS: ssl: fix ssl_default_server.vtc
William Lallemand [Wed, 29 Dec 2021 17:16:27 +0000 (18:16 +0100)] 
REGTESTS: ssl: fix ssl_default_server.vtc

Patch 2c776f1 ("BUG/MEDIUM: ssl: initialize correctly ssl w/
default-server") added tests that are not relevant anymore and broke the
reg-test. revert them.

3 years agoBUG/MEDIUM: ssl: initialize correctly ssl w/ default-server
William Lallemand [Tue, 28 Dec 2021 17:47:17 +0000 (18:47 +0100)] 
BUG/MEDIUM: ssl: initialize correctly ssl w/ default-server

This bug was introduced by d817dc73 ("MEDIUM: ssl: Load client
certificates in a ckch for backend servers") in which the creation of
the SSL_CTX for a server was moved to the configuration parser when
using a "crt" keyword instead of being done in ssl_sock_prepare_srv_ctx().

The patch 0498fa40 ("BUG/MINOR: ssl: Default-server configuration ignored by
server") made it worse by setting the same SSL_CTX for every servers
using a default-server. Resulting in any SSL option on a server applied
to every server in its backend.

This patch fixes the issue by reintroducing a string which store the
path of certificate inside the server structure, and loading the
certificate in ssl_sock_prepare_srv_ctx() again.

This is a quick fix to backport, a cleaner way can be achieve by always
creating the SSL_CTX in ssl_sock_prepare_srv_ctx() and splitting
properly the ssl_sock_load_srv_cert() function.

This patch fixes issue #1488.

Must be backported as far as 2.4.

3 years agoMINOR: debug: add support for -dL to dump library names at boot
Willy Tarreau [Tue, 28 Dec 2021 14:43:11 +0000 (15:43 +0100)] 
MINOR: debug: add support for -dL to dump library names at boot

This is a second help to dump loaded library names late at boot, once
external code has already been initialized. The purpose is to provide
a format that makes it easy to pass to "tar" to produce an archive
containing the executable and the list of dependencies. For example
if haproxy is started as "haproxy -f foo.cfg", a config check only
will suffice to quit before starting, "-q" will be used to disable
undesired output messages, and -dL will be use to dump libraries.
This will result in such a command to trivially produce a tarball
of loaded libraries:

   ./haproxy -q -c -dL -f foo.cfg | tar -T - -hzcf archive.tgz

3 years agoMINOR: debug: add ability to dump loaded shared libraries
Willy Tarreau [Tue, 28 Dec 2021 08:57:10 +0000 (09:57 +0100)] 
MINOR: debug: add ability to dump loaded shared libraries

Many times core dumps reported by users who experience trouble are
difficult to exploit due to missing system libraries. Sometimes,
having just a list of loaded libraries and their respective addresses
can already provide some hints about some problems.

This patch makes a step in that direction by adding a new "show libs"
command that will try to enumerate the list of object files that are
loaded in memory, relying on the dynamic linker for this. It may also
be used to detect that some foreign code embarks other undesired libs
(e.g. some external Lua modules).

At the moment it's only supported on glibc when USE_DL is set, but it's
implemented in a way that ought to make it reasonably easy to be extended
to other platforms.

3 years agoMINOR: compat: detect support for dl_iterate_phdr()
Willy Tarreau [Tue, 28 Dec 2021 14:13:12 +0000 (15:13 +0100)] 
MINOR: compat: detect support for dl_iterate_phdr()

We'll use this glibc function to dump loaded libs. It's been
available since glibc-2.2.4, and as it requires dlpi headers defined
in link.h, it implicitly relies on dlfcn, thus we condition it to
USE_DL. Other operating systems or libc might have different
dependencies so let's stick to the bare minimum for now.

3 years agoBUILD: opentracing: display warning in case of using OT_USE_VARS at compile time
Miroslav Zagorac [Mon, 27 Dec 2021 11:44:07 +0000 (12:44 +0100)] 
BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time

Please do not set the OT_USE_VARS configuration variable, as the source
will probably not be able to compile!  For now, this variable can only
be used for experimental purposes, and is not intended for wider use.

For further clarification, please see commit 4cb2c83f4.

Must be backported to 2.5.

3 years agoCI: Github Actions: do not show VTest failures if build failed
Ilya Shipitsin [Sat, 25 Dec 2021 08:53:04 +0000 (13:53 +0500)] 
CI: Github Actions: do not show VTest failures if build failed

this is mostly cleanup, issue is minor. If build failed, VTest execution
tried to be performed as well as VTest result show. This change ignores
those steps if build failed.

3 years agoBUG/MEDIUM: peers: properly skip conn_cur from incoming messages
Willy Tarreau [Fri, 24 Dec 2021 12:38:49 +0000 (13:38 +0100)] 
BUG/MEDIUM: peers: properly skip conn_cur from incoming messages

The approach used for skipping conn_cur in commit db2ab8218 ("MEDIUM:
stick-table: never learn the "conn_cur" value from peers") was wrong,
it only works with simple tables but as soon as frequency counters or
arrays are exchanged after conn_cur, the stream is desynchronized and
incorrect values are read. This is because the fields have a variable
length depending on their types and cannot simply be skipped by a
"continue" statement.

Let's change the approach to make sure we continue to completely parse
these local-only fields, and only drop the value at the moment we're
about to store them, since this is exactly the intent.

A simpler approach could consist in having two sets of stktable_data_ptr()
functions, one for retrieval and one for storage, and to make the store
function return a NULL pointer for local types. For now this doesn't
seem worth the trouble.

This fixes github issue #1497. Thanks to @brenc for the reproducer.

This must be backported to 2.5.

3 years agoBUG/MEDIUM: backend: fix possible sockaddr leak on redispatch
Willy Tarreau [Fri, 24 Dec 2021 10:27:53 +0000 (11:27 +0100)] 
BUG/MEDIUM: backend: fix possible sockaddr leak on redispatch

A subtle change of target address allocation was introduced with commit
68cf3959b ("MINOR: backend: rewrite alloc of stream target address") in
2.4. Prior to this patch, a target address was allocated by function
assign_server_address() only if none was previously allocated. After
the change, the allocation became unconditional. Most of the time it
makes no difference, except when we pass multiple times through
connect_server() with SF_ADDR_SET cleared.

The most obvious fix would be to avoid allocating that address there
when already set, but the root cause is that since introduction of
dynamically allocated addresses, the SF_ADDR_SET flag lies. It can
be cleared during redispatch or during a queue redistribution without
the address being released.

This patch instead gives back all its correct meaning to SF_ADDR_SET
and guarantees that when not set no address is allocated, by freeing
that address at the few places the flag is cleared. The flag could
even be removed so that only the address is checked but that would
require to touch many areas for no benefit.

The easiest way to test it is to send requests to a proxy with l7
retries enabled, which forwards to a server returning 500:

  defaults
    mode http
    timeout client 1s
    timeout server 1s
    timeout connect 1s
    retry-on all-retryable-errors
    retries 1
    option redispatch

  listen proxy
    bind *:5000
    server app 0.0.0.0:5001

  frontend dummy-app
    bind :5001
    http-request return status 500

Issuing "show pools" on the CLI will show that pool "sockaddr" grows
as requests are redispatched, and remains stable with the fix. Even
"ps" will show that the process' RSS grows by ~160B per request.

This fix will need to be backported to 2.4. Note that before 2.5,
there's no strm->si[1].dst, strm->target_addr must be used instead.

This addresses github issue #1499. Special thanks to Daniil Leontiev
for providing a well-documented reproducer.

3 years agoBUG/MINOR: quic: fix potential use of uninit pointer
Amaury Denoyelle [Thu, 23 Dec 2021 15:32:24 +0000 (16:32 +0100)] 
BUG/MINOR: quic: fix potential use of uninit pointer

Properly initialized the ssl_sock_ctx pointer in qc_conn_init. This is
required to avoid to set an undefined pointer in qc.xprt_ctx if argument
*xprt_ctx is NULL.

3 years agoBUG/MINOR: quic: fix potential null dereference
Amaury Denoyelle [Thu, 23 Dec 2021 15:27:56 +0000 (16:27 +0100)] 
BUG/MINOR: quic: fix potential null dereference

This is not a real issue because found_in_dcid can not be set if qc is
NULL.

3 years agoMEDIUM: quic: implement refcount for quic_conn
Amaury Denoyelle [Thu, 23 Dec 2021 09:02:50 +0000 (10:02 +0100)] 
MEDIUM: quic: implement refcount for quic_conn

Implement a refcount on quic_conn instance. By default, the refcount is
0. Two functions are implemented to manipulate it.
* qc_conn_take() which increments the refcount
* qc_conn_drop() which decrements it. If the refcount is 0 *BEFORE*
  the substraction, the instance is freed.

The refcount is incremented on retrieve_qc_conn_from_cid() or when
allocating a new quic_conn in qc_lstnr_pkt_rcv(). It is substracted most
notably by the xprt.close operation and at the end of
qc_lstnr_pkt_rcv(). The increments/decrements should be conducted under
the CID lock to guarantee thread-safety.

3 years agoMINOR: quic: delete timer task on quic_close()
Amaury Denoyelle [Thu, 23 Dec 2021 14:06:56 +0000 (15:06 +0100)] 
MINOR: quic: delete timer task on quic_close()

The timer task is attached to the connection-pinned thread. Only this
thread can delete it. With the future refcount implementation of
quic_conn, every thread can be responsible to remove the quic_conn via
quic_conn_free(). Thus, the timer task deletion is moved from the
calling function quic_close().

3 years agoMINOR: quic: replace usage of ssl_sock_ctx by quic_conn
Amaury Denoyelle [Wed, 22 Dec 2021 10:06:34 +0000 (11:06 +0100)] 
MINOR: quic: replace usage of ssl_sock_ctx by quic_conn

Big refactoring on xprt-quic. A lot of functions were using the
ssl_sock_ctx as argument to only access the related quic_conn. All these
arguments are replaced by a quic_conn parameter.

As a convention, the quic_conn instance is always the first parameter of
these functions.

This commit is part of the rearchitecture of xprt-quic layers and the
separation between xprt and connection instances.

3 years agoMINOR: quic: remove unnecessary if in qc_pkt_may_rm_hp()
Amaury Denoyelle [Thu, 23 Dec 2021 15:02:24 +0000 (16:02 +0100)] 
MINOR: quic: remove unnecessary if in qc_pkt_may_rm_hp()

Remove the shortcut to use the INITIAL encryption level when removing
header protection on first connection packet.

This change is useful for the following change which removes
ssl_sock_ctx in argument lists in favor of the quic_conn instance.

3 years agoMINOR: quic: store ssl_sock_ctx reference into quic_conn
Amaury Denoyelle [Wed, 22 Dec 2021 17:20:38 +0000 (18:20 +0100)] 
MINOR: quic: store ssl_sock_ctx reference into quic_conn

Add a pointer in quic_conn to its related ssl_sock_ctx. This change is
required to avoid to use the connection instance to access it.

This commit is part of the rearchitecture of xprt-quic layers and the
separation between xprt and connection instances. It will be notably
useful when the connection allocation will be delayed.

3 years agoMINOR: quic: remove unnecessary call to free_quic_conn_cids()
Amaury Denoyelle [Thu, 23 Dec 2021 10:30:11 +0000 (11:30 +0100)] 
MINOR: quic: remove unnecessary call to free_quic_conn_cids()

free_quic_conn_cids() was called in quic_build_post_handshake_frames()
if an error occured. However, the only error is an allocation failure of
the CID which does not required to call it.

This change is required for future refcount implementation. The CID lock
will be removed from the free_quic_conn_cids() and to the caller.

3 years agoBUG/MINOR: quic: upgrade rdlock to wrlock for ODCID removal
Amaury Denoyelle [Wed, 22 Dec 2021 10:29:05 +0000 (11:29 +0100)] 
BUG/MINOR: quic: upgrade rdlock to wrlock for ODCID removal

When a quic_conn is found in the DCID tree, it can be removed from the
first ODCID tree. However, this operation must absolutely be run under a
write-lock to avoid race condition. To avoid to use the lock too
frequently, node.leaf_p is checked. This value is set to NULL after
ebmb_delete.

3 years agoREORG: quic: remove qc_ prefix on functions which not used it directly
Amaury Denoyelle [Thu, 23 Dec 2021 09:37:19 +0000 (10:37 +0100)] 
REORG: quic: remove qc_ prefix on functions which not used it directly

The qc_* prefix should be reserved to functions which used a specific
quic_conn instance and are expected to be pinned on the connection
thread.

3 years agoMINOR: quic: Add CONNECTION_CLOSE phrase to trace
Frédéric Lécaille [Thu, 23 Dec 2021 14:19:15 +0000 (15:19 +0100)] 
MINOR: quic: Add CONNECTION_CLOSE phrase to trace

Some applications may send some information about the reason why they decided
to close a connection. Add them to CONNECTION_CLOSE frame traces.
Take the opportunity of this patch to shorten some too long variable names
without any impact.

3 years agoMINOR: quic: Add traces for RX frames (flow control related)
Frédéric Lécaille [Thu, 23 Dec 2021 13:11:25 +0000 (14:11 +0100)] 
MINOR: quic: Add traces for RX frames (flow control related)

Add traces about important frame types to chunk_tx_frm_appendf()
and call this function for any type of frame when parsing a packet.
Move it to quic_frame.c

3 years agoDEBUG: ssl: make sure we never change a servername on established connections
Willy Tarreau [Thu, 23 Dec 2021 10:12:13 +0000 (11:12 +0100)] 
DEBUG: ssl: make sure we never change a servername on established connections

Since this case was already met previously with commit 655dec81b
("BUG/MINOR: backend: do not set sni on connection reuse"), let's make
sure that we don't change reused connection settings. This could be
generalized to most settings that are only in effect before the handshake
in fact (like set_alpn and a few other ones).

3 years agoMINOR: pools: work around possibly slow malloc_trim() during gc
Willy Tarreau [Thu, 23 Dec 2021 08:26:30 +0000 (09:26 +0100)] 
MINOR: pools: work around possibly slow malloc_trim() during gc

During 2.4-dev, support for malloc_trim() was implemented to ease
release of memory in a stopping process. This was found to be quite
effective and later backported to 2.3.7.

Then it was found that sometimes malloc_trim() could take a huge time
to complete it if was competing with other threads still allocating and
releasing memory, reason why it was decided in 2.5-dev to move
malloc_trim() under the thread isolation that was already in place in
the shared pool version of pool_gc() (this was commit 26ed1835).

However, other instances of pool_gc() that used to call malloc_trim()
were not updated since they were not using thread isolation. Currently
we have two other such instances, one for when there is absolutely no
pool and one for when there are only thread-local pools.

Christian Ruppert reported in GH issue #1490 that he's sometimes seeing
and old process die upon reload when upgrading from 2.3 to 2.4, and
that this happens inside malloc_trim(). The problem is that since
2.4-dev11 with commit 0bae07592 we detect modern libc that provide a
faster thread-aware allocator and do not maintain shared pools anymore.
As such we're using again the simpler pool_gc() implementations that do
not use thread isolation around the malloc_trim() call.

All this code was cleaned up recently and the call moved to a new
function trim_all_pools(). This patch implements explicit thread isolation
inside that function so that callers do not have to care about this
anymore. The thread isolation is conditional so that this doesn't affect
the one already in place in the larger version of pool_gc(). This way it
will solve the problem for all callers.

This patch must be backported as far as 2.3. It may possibly require
some adaptations. If trim_all_pools() is not present, copy-pasting the
tests in each version of pool_gc() will have the same effect.

Thanks to Christian for his detailed report and his testing.

3 years agoMINOR: quic: Drop asap Retry or Version Negotiation packets
Frédéric Lécaille [Wed, 22 Dec 2021 19:39:12 +0000 (20:39 +0100)] 
MINOR: quic: Drop asap Retry or Version Negotiation packets

These packet are only sent by servers. We drop them as soon as possible
when we are an haproxy listener.

3 years agoMINOR: quic: xprt traces fixes
Frédéric Lécaille [Wed, 22 Dec 2021 16:40:38 +0000 (17:40 +0100)] 
MINOR: quic: xprt traces fixes

Empty parameters are permitted with TRACE_*() macros. If removed, must
be replaced by NULL.

3 years agoMINOR: quic: Handle the cases of overlapping STREAM frames
Frédéric Lécaille [Wed, 22 Dec 2021 15:13:43 +0000 (16:13 +0100)] 
MINOR: quic: Handle the cases of overlapping STREAM frames

This is the same treatment for bidi and uni STREAM frames. This is a duplication
code which should me remove building a function for both these types of streams.

3 years agoMINOR: quic: Wrong dropped packet skipping
Frédéric Lécaille [Wed, 22 Dec 2021 09:17:01 +0000 (10:17 +0100)] 
MINOR: quic: Wrong dropped packet skipping

There were cases where some dropped packets were not well skipped. This led
the low level QUIC packet parser to continue from wrong packet boundaries.

3 years agoMINOR: quic: unchecked qc_retrieve_conn_from_cid() returned value
Frédéric Lécaille [Tue, 21 Dec 2021 13:48:58 +0000 (14:48 +0100)] 
MINOR: quic: unchecked qc_retrieve_conn_from_cid() returned value

If qc_retrieve_conn_from_cid() did not manage to retrieve the connection
from packet CIDs, we must drop them.

3 years agoMINOR: quic: Add stream IDs to qcs_push_frame() traces
Frédéric Lécaille [Tue, 21 Dec 2021 10:53:33 +0000 (11:53 +0100)] 
MINOR: quic: Add stream IDs to qcs_push_frame() traces

This is only for debug purpose.

3 years agoMINOR: quic: add quic_conn instance in traces for qc_new_conn
Amaury Denoyelle [Tue, 21 Dec 2021 13:51:56 +0000 (14:51 +0100)] 
MINOR: quic: add quic_conn instance in traces for qc_new_conn

The connection instance has been replaced by a quic_conn as first
argument to QUIC traces. It is possible to report the quic_conn instance
in the qc_new_conn(), contrary to the connection which is not
initialized at this stage.

3 years agoMINOR: quic: use quic_conn as argument to traces
Amaury Denoyelle [Tue, 21 Dec 2021 13:29:15 +0000 (14:29 +0100)] 
MINOR: quic: use quic_conn as argument to traces

Replace the connection instance for first argument of trace callback by
a quic_conn instance. The QUIC trace module is properly initialized with
the first argument refering to a quic_conn.

Replace every connection instances in TRACE_* macros invocation in
xprt-quic by its related quic_conn. In some case, the connection is
still used to access the quic_conn. It may cause some problem on the
future when the connection will be completly separated from the xprt
layer.

This commit is part of the rearchitecture of xprt-quic layers and the
separation between xprt and connection instances.

3 years agoMINOR: trace: add quic_conn argument definition
Amaury Denoyelle [Tue, 21 Dec 2021 10:26:02 +0000 (11:26 +0100)] 
MINOR: trace: add quic_conn argument definition

Prepare trace support for quic_conn instances as argument. This will be
used by the xprt-quic layer in replacement of the connection.

This commit is part of the rearchitecture of xprt-quic layers and the
separation between xprt and connection instances.

3 years agoMINOR: quic: add const qualifier for traces function
Amaury Denoyelle [Tue, 21 Dec 2021 13:28:26 +0000 (14:28 +0100)] 
MINOR: quic: add const qualifier for traces function

Add const qualifier on arguments of several dump functions used in the
trace callback. This is required to be able to replace the first trace
argument by a quic_conn instance. The first argument is a const pointer
and so the members accessed through it must also be const.

3 years agoMINOR: quic: add reference to quic_conn in ssl context
Amaury Denoyelle [Tue, 21 Dec 2021 10:41:52 +0000 (11:41 +0100)] 
MINOR: quic: add reference to quic_conn in ssl context

Add a new member in ssl_sock_ctx structure to reference the quic_conn
instance if used in the QUIC stack. This member is initialized during
qc_conn_init().

This is needed to be able to access to the quic_conn without relying on
the connection instance. This commit is part of the rearchitecture of
xprt-quic layers and the separation between xprt and connection
instances.

3 years agoREORG: quic: move mux function outside of xprt
Amaury Denoyelle [Tue, 21 Dec 2021 10:53:10 +0000 (11:53 +0100)] 
REORG: quic: move mux function outside of xprt

Move qcc_get_qcs() function from xprt_quic.c to mux_quic.c. This
function is used to retrieve the qcs instance from a qcc with a stream
id. This clearly belongs to the mux-quic layer.

3 years agoCLEANUP: quic: rename quic_conn instances to qc
Amaury Denoyelle [Tue, 21 Dec 2021 13:45:39 +0000 (14:45 +0100)] 
CLEANUP: quic: rename quic_conn instances to qc

Use the convention of naming quic_conn instance as qc to not confuse it
with a connection instance. The changes occured for qc_parse_pkt_frms(),
qc_build_frms() and qc_do_build_pkt().

3 years agoDOC: fix misspelled keyword "resolve_retries" in resolvers
Thierry Fournier [Wed, 15 Dec 2021 18:03:52 +0000 (19:03 +0100)] 
DOC: fix misspelled keyword "resolve_retries" in resolvers

"resolve_retries" was spelled "resolve_retires".

3 years agoMINOR: quic: Wrong packet refcount handling in qc_pkt_insert()
Frédéric Lécaille [Mon, 20 Dec 2021 13:41:19 +0000 (14:41 +0100)] 
MINOR: quic: Wrong packet refcount handling in qc_pkt_insert()

The QUIC connection I/O handler qc_conn_io_cb() could be called just after
qc_pkt_insert() have inserted a packet in a its tree, and before qc_pkt_insert()
have incremented the reference counter to this packet. As qc_conn_io_cb()
decrement this counter, the packet could be released before qc_pkt_insert()
might increment the counter, leading to possible crashes when trying to do so.
So, let's make qc_pkt_insert() increment this counter before inserting the packet
it is tree. No need to lock anything for that.

3 years agoMINOR: quic: Do not forget STREAM frames received in disorder
Frédéric Lécaille [Mon, 20 Dec 2021 11:02:13 +0000 (12:02 +0100)] 
MINOR: quic: Do not forget STREAM frames received in disorder

Add a function to process all STREAM frames received and ordered
by their offset (qc_treat_rx_strm_frms()) and modify
qc_handle_bidi_strm_frm() consequently.

3 years agoMINOR: quic: Do not expect to receive only one O-RTT packet
Frédéric Lécaille [Fri, 17 Dec 2021 17:24:16 +0000 (18:24 +0100)] 
MINOR: quic: Do not expect to receive only one O-RTT packet

There is nothing about this in the RFC. We must support to receive
several 0-RTT packets before the handshake has completed.

3 years agoMINOR: quic: Stop emptying the RX buffer asap.
Frédéric Lécaille [Mon, 20 Dec 2021 16:12:42 +0000 (17:12 +0100)] 
MINOR: quic: Stop emptying the RX buffer asap.

When a packet is present in the RX buffer at the first place
but without a null reference counter, there is no need to continue
to try to empty the buffer, it is sure the next packet will not
be at the first place!

3 years agoMINOR: quic: Add a function to list remaining RX packets by encryption level
Frédéric Lécaille [Fri, 17 Dec 2021 15:11:54 +0000 (16:11 +0100)] 
MINOR: quic: Add a function to list remaining RX packets by encryption level

This is only to debug some issues which cause the RX buffer saturation
with "Too big packet" traces.

3 years agoMINOR: quic: Increase the RX buffer for each connection
Frédéric Lécaille [Fri, 17 Dec 2021 13:11:00 +0000 (14:11 +0100)] 
MINOR: quic: Increase the RX buffer for each connection

Double this buffer size which reaches 16ko for now on.