205 Commits

Author SHA1 Message Date
Frédéric Lécaille
a576c1b0c6 MINOR: quic: Remove a useless test about probing in qc_prep_pkts()
qel->pktns->tx.pto_probe is set to 0 after having prepared a probing
datagram. There is no reason to check this parameter. Furthermore
it is always 0 when the connection does not probe the peer.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:09 +02:00
Frédéric Lécaille
91369cfcd0 MINOR: quic: Display the packet number space flags in traces
Display this information when the encryption level is also displayed.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
9d68c6aaf6 BUG/MINOR: quic: Possible wrapped values used as ACK tree purging limit.
Add two missing checks not to substract too big values from another too little
one. In this case the resulted wrapped huge values could be passed to the function
which has to remove the last range of a tree of ACK ranges as encoded limit size
not to go below, cancelling the ACK ranges deletion. The consequence could be that
no ACK were sent.

Must be backported to 2.6 and 2.7.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
45bf1a82f1 BUG/MEDIUM: quic: Code sanitization about acknowledgements requirements
qc_may_build_pkt() has been modified several times regardless of the conditions
the functions it is supposed to allow to send packets (qc_build_pkt()/qc_do_build_pkt())
really use to finally send packets just after having received others, leading
to contraditions and possible very long loops sending empty packets (PADDING only packets)
because qc_may_build_pkt() could allow qc_build_pkt()/qc_do_build_pkt to build packet,
and the latter did nothing except sending PADDING frames, because from its point
of view they had nothing to send.

For now on, this is the job of qc_may_build_pkt() to decide to if there is
packets to send just after having received others AND to provide this information
to the qc_build_pkt()/qc_do_build_pkt()

Note that the unique case where the acknowledgements are completely ignored is
when the endpoint must probe. But at least this is when sending at most two datagrams!

This commit also fixes the issue reported by Willy about a very low throughput
performance when the client serialized its requests.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
eb3e5171ed MINOR: quic: Add connection flags to traces
This should help in diagnosing issues.

Some adjustments have to be done to avoid deferencing a quic_conn objects from
TRACE_*() calls.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
809bd9fed1 BUG/MINOR: quic: Ignored less than 1ms RTTs
Do not ignore very short RTTs (less than 1ms) before computing the smoothed
RTT initializing it to an "infinite" value (UINT_MAX).

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
fad0e6cf73 MINOR: quic: Add packet loss and maximum cc window to "show quic"
Add the number of packet losts and the maximum congestion control window computed
by the algorithms to "show quic".
Same thing for the traces of existent congestion control algorithms.

Must be backported to 2.7 and 2.6.
2023-04-13 19:20:08 +02:00
Frédéric Lécaille
6fd2576d5e MINOR: quic: Add a trace for packet with an ACK frame
As the ACK frames are not added to the packet list of ack-eliciting frames,
it could not be traced. But there is a flag to identify such packet.
Let's use it to add this information to the traces of TX packets.

Must be backported to 2.6 and 2.7.
2023-04-11 10:47:19 +02:00
Frédéric Lécaille
e47adca432 MINOR: quic: Dump more information at proto level when building packets
This should be helpful to debug issues at without too much traces.

Must be backported to 2.7 and 2.6.
2023-04-11 10:47:19 +02:00
Frédéric Lécaille
c0aaa07aa3 MINOR: quic: Modify qc_try_rm_hp() traces
Dump at proto level the packet information when its header protection was removed.
Remove no more use qpkt_trace variable.

Must be backported to 2.7 and 2.6.
2023-04-11 10:47:19 +02:00
Frédéric Lécaille
68737316ea BUG/MINOR: quic: Wrong packet number space probing before confirmed handshake
It is possible that the handshake was not confirmed and there was no more
packet in flight to probe with. It this case the server must wait for
the client to be unblocked without probing any packet number space contrary
to what was revealed by interop tests as follows:

	[01|quic|2|uic_loss.c:65] TX loss pktns : qc@0x7fac301cd390 pktns=I pp=0
	[01|quic|2|uic_loss.c:67] TX loss pktns : qc@0x7fac301cd390 pktns=H pp=0 tole=-102ms
	[01|quic|2|uic_loss.c:67] TX loss pktns : qc@0x7fac301cd390 pktns=01RTT pp=0 if=1054 tole=-1987ms
	[01|quic|5|uic_loss.c:73] quic_loss_pktns(): leaving : qc@0x7fac301cd390
	[01|quic|5|uic_loss.c:91] quic_pto_pktns(): entering : qc@0x7fac301cd390
	[01|quic|3|ic_loss.c:121] TX PTO handshake not already completed : qc@0x7fac301cd390
	[01|quic|2|ic_loss.c:141] TX PTO : qc@0x7fac301cd390 pktns=I pp=0 dur=83ms
	[01|quic|5|ic_loss.c:142] quic_pto_pktns(): leaving : qc@0x7fac301cd390
	[01|quic|3|c_conn.c:5179] needs to probe Initial packet number space : qc@0x7fac301cd390

This bug was not visible before this commit:
     BUG/MINOR: quic: wake up MUX on probing only for 01RTT

This means that before it, one could do bad things (probing the 01RTT packet number
space before the handshake was confirmed).

Must be backported to 2.7 and 2.6.
2023-04-11 10:47:19 +02:00
Amaury Denoyelle
15adc4cc4e MINOR: quic: remove address concatenation to ODCID
Previously, ODCID were concatenated with the client address. This was
done to prevent a collision between two endpoints which used the same
ODCID.

Thanks to the two previous patches, first connection generated CID is
now directly derived from the client ODCID using a hash function which
uses the client source address from the same purpose. Thus, it is now
unneeded to concatenate client address to <odcid> quic-conn member.

This change allows to simplify the quic_cid structure management and
reduce its size which is important as it is embedded several times in
various structures such as quic_conn and quic_rx_packet.

This should be backported up to 2.7.
2023-04-05 11:09:57 +02:00
Amaury Denoyelle
2c98209c1c MINOR: quic: remove ODCID dedicated tree
First connection CID generation has been altered. It is now directly
derived from client ODCID since previous commit :
  commit 162baaff7ab761c411800f4ca6bef45315d7afcb
  MINOR: quic: derive first DCID from client ODCID

This patch removes the ODCID tree which is now unneeded. On connection
lookup via CID, if a DCID is not found the hash derivation is performed
for an INITIAL/0-RTT packet only. In case a client has used multiple
times an ODCID, this will allow to retrieve our generated DCID in the
CID tree without storing the ODCID node.

The impact of this two combined patch is that it may improve slightly
haproxy memory footprint by removing a tree node from quic_conn
structure. The cpu calculation induced by hash derivation should only be
performed only a few times per connection as the client will start to
use our generated CID as soon as it received it.

This should be backported up to 2.7.
2023-04-05 11:07:01 +02:00
Amaury Denoyelle
162baaff7a MINOR: quic: derive first DCID from client ODCID
Change the generation of the first CID of a connection. It is directly
derived from the client ODCID using a 64-bits hash function. Client
address is added to avoid collision between clients which could use the
same ODCID.

For the moment, this change as no functional impact. However, it will be
directly used for the next commit to be able to remove the ODCID tree.

This should be backported up to 2.7.
2023-04-05 11:06:04 +02:00
Frédéric Lécaille
ce5c145df5 BUG/MINOR: quic: Possible crashes in qc_idle_timer_task()
This is due to this commit:

    MINOR: quic: Add trace to debug idle timer task issues

where has been added without having been tested at developer level.
<qc> was dereferenced after having been released by qc_conn_release().

Set qc to NULL value after having been released to forbid its dereferencing.
Add a check for qc->idle_timer_task in the traces added by the mentionned
commit above to prevent its dereferencing if NULL.
Take the opportunity of this patch to modify trace events from
QUIC_EV_CONN_SSLALERT to QUIC_EV_CONN_IDLE_TIMER.

Must be backported to 2.6 and 2.7.
2023-04-05 11:03:20 +02:00
Christopher Faulet
7faac7cf34 MINOR: tree-wide: Simplifiy some tests on SHUT flags by accessing SCs directly
At many places, we simplify the tests on SHUT flags to remove calls to
chn_prod() or chn_cons() function because the corresponding SC is available.
2023-04-05 08:57:06 +02:00
Christopher Faulet
87633c3a11 MEDIUM: tree-wide: Move flags about shut from the channel to the SC
The purpose of this patch is only a one-to-one replacement, as far as
possible.

CF_SHUTR(_NOW) and CF_SHUTW(_NOW) flags are now carried by the
stream-connecter. CF_ prefix is replaced by SC_FL_ one. Of course, it is not
so simple because at many places, we were testing if a channel was shut for
reads and writes in same time. To do the same, shut for reads must be tested
on one side on the SC and shut for writes on the other side on the opposite
SC. A special care was taken with process_stream(). flags of SCs must be
saved to be able to detect changes, just like for the channels.
2023-04-05 08:57:06 +02:00
Frédéric Lécaille
fdb1494985 BUILD: quic: 32bits compilation issue in cli_io_handler_dump_quic()
Replaced a %zu printf format by %llu for an uint64_t.

Must be backported to 2.7.
2023-04-04 18:24:28 +02:00
Frédéric Lécaille
92f4a7c614 BUG/MINOR: quic: Wrong idle timer expiration (during 20s)
This this commit, this is ->idle_expire of quic_conn struct which must
be taken into an account to display the idel timer task expiration value:

     MEDIUM: quic: Ack delay implementation

Furthermore, this value was always zero until now_ms has wrapped (20 s after
the start time) due to this commit:
     MEDIUM: clock: force internal time to wrap early after boot

Do not rely on the value of now_ms compared to ->idle_expire to display the
difference but use ticks_remain() to compute it.

Must be backported to 2.7 where "show quic" has already been backported.
2023-04-04 18:24:28 +02:00
Frédéric Lécaille
12eca3a727 BUG/MINOR: quic: Unexpected connection closures upon idle timer task execution
This bug arrived with this commit:

      MEDIUM: quic: Ack delay implementation

It is possible that the idle timer task was already in the run queue when its
->expire field was updated calling qc_idle_timer_do_rearm(). To prevent this
task from running in this condition, one must check its ->expire field value
with this condition to run the task if its timer has really expired:

	!tick_is_expired(t->expire, now_ms)

Furthermore, as this task may be directly woken up with a call to task_wakeup()
all, for instance by qc_kill_conn() to kill the connection, one must check this
task has really been woken up when it was in the wait queue and not by a direct
call to task_wakeup() thanks to this test:

	(state & TASK_WOKEN_ANY) == TASK_WOKEN_TIMER

Again, when this condition is not fulfilled, the task must be run.

Must be backported where the commit mentionned above was backported.
2023-04-04 18:24:28 +02:00
Frédéric Lécaille
495968ed51 MINOR: quic: Add trace to debug idle timer task issues
Add TRACE_PROTO() call where this is relevant to debug issues about
qc_idle_timer_task() issues.

Must be backported to 2.6 and 2.7.
2023-04-04 18:24:28 +02:00
Ilya Shipitsin
07be66d21b CLEANUP: assorted typo fixes in the code and comments
This is 35th iteration of typo fixes
2023-04-01 18:33:40 +02:00
Frédéric Lécaille
d721571d26 MEDIUM: quic: Ack delay implementation
Reuse the idle timeout task to delay the acknowledgments. The time of the
idle timer expiration is for now on stored in ->idle_expire. The one to
trigger the acknowledgements is stored in ->ack_expire.
Add QUIC_FL_CONN_ACK_TIMER_FIRED new connection flag to mark a connection
as having its acknowledgement timer been triggered.
Modify qc_may_build_pkt() to prevent the sending of "ack only" packets and
allows the connection to send packet when the ack timer has fired.
It is possible that acks are sent before the ack timer has triggered. In
this case it is cancelled only if ACK frames are really sent.
The idle timer expiration must be set again when the ack timer has been
triggered or when it is cancelled.

Must be backported to 2.7.
2023-03-31 13:41:17 +02:00
Frédéric Lécaille
8f991948f5 MINOR: quic: Traces adjustments at proto level.
Dump variables displayed by TRACE_ENTER() or TRACE_LEAVE() by calls to TRACE_PROTO().
No more variables are displayed by the two former macros. For now on, these information
are accessible from proto level.
Add new calls to TRACE_PROTO() at important locations in relation whith QUIC transport
protocol.
When relevant, try to prefix such traces with TX or RX keyword to identify the
concerned subpart (transmission or reception) of the protocol.

Must be backported to 2.7.
2023-03-31 09:54:59 +02:00
Frédéric Lécaille
deb978149a BUG/MINOR: quic: Missing max_idle_timeout initialization for the connection
This bug was revealed by handshakeloss interop tests (often with quiceh) where one
could see haproxy an Initial packet without TLS ClientHello message (only a padded
PING frame). In this case, as the ->max_idle_timeout was not initialized, the
connection was closed about three seconds later, and haproxy opened a new one with
a new source connection ID upon receipt of the next client Initial packet. As the
interop runner count the number of source connection ID used by the server to check
there were exactly 50 such IDs used by the server, it considered the test as failed.

So, the ->max_idle_timeout of the connection must be at least initialized
to the local "max_idle_timeout" transport parameter value to avoid such
a situation (closing connections too soon) until it is "negotiated" with the
client when receiving its TLS ClientHello message.

Must be backported to 2.7 and 2.6.
2023-03-31 09:54:59 +02:00
Frédéric Lécaille
a3772e1134 MINOR: quic: Add recovery related information to "show quic"
Add ->srtt, ->rtt_var, ->rtt_min and ->pto_count values from ->path->loss
struct to "show quic". Same thing for ->cwnd from ->path struct.

Also take the opportunity of this patch to dump the packet number
space information directly from ->pktns[] array in place of ->els[]
array. Indeed, ->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA] and ->els[QUIC_TLS_ENC_LEVEL_APP]
have the same packet number space.

Must be backported to 2.7 where "show quic" implementation has alredy been
backported.
2023-03-31 09:54:59 +02:00
Frdric Lcaille
9c317b1d35 BUG/MINOR: quic: Missing padding in very short probe packets
This bug arrived with this commit:
   MINOR: quic: Send PING frames when probing Initial packet number space

This may happen when haproxy needs to probe the peer with very short packets
(only one PING frame). In this case, the packet must be padded. There was clearly
a case which was removed by the mentionned commit above. That said, there was
an extra byte which was added to the PADDING frame before the mentionned commit
above. This is no more the case with this patch.

Thank you to @tatsuhiro-t (ngtcp2 manager) for having reported this issue which
was revealed by the keyupdate test (on client side).

Must be backported to 2.7 and 2.6.
2023-03-28 18:26:57 +02:00
Frédéric Lécaille
c425e03b28 BUG/MINOR: quic: Missing STREAM frame type updated
This patch follows this commit which was not sufficient:
  BUG/MINOR: quic: Missing STREAM frame data pointer updates

Indeed, after updating the ->offset field, the bit which informs the
frame builder of its presence must be systematically set.

This bug was revealed by the following BUG_ON() from
quic_build_stream_frame() :
  bug condition "!!(frm->type & 0x04) != !!stream->offset.key" matched at src/quic_frame.c:515

This should fix the last crash occured on github issue #2074.

Must be backported to 2.6 and 2.7.
2023-03-27 16:01:44 +02:00
Amaury Denoyelle
8afe4b88c4 BUG/MINOR: quic: ignore congestion window on probing for MUX wakeup
qc_notify_send() is used to wake up the MUX layer for sending. This
function first ensures that all sending condition are met to avoid to
wake up the MUX for unnecessarily.

One of this condition is to check if there is room in the congestion
window. However, when probe packets must be sent due to a PTO
expiration, RFC 9002 explicitely mentions that the congestion window
must be ignored which was not the case prior to this patch.

This commit fixes this by first setting <pto_probe> of 01RTT packet
space before invoking qc_notify_send(). This ensures that congestion
window won't be checked anymore to wake up the MUX layer until probing
packets are sent.

This commit replaces the following one which was not sufficient :
  commit e25fce03ebe3307bc104d1f81356108e271d2bc3
  BUG/MINOR: quic: Dysfunctional 01RTT packet number space probing

This should be backported up to 2.7.
2023-03-21 14:52:02 +01:00
Amaury Denoyelle
2a19b6e564 BUG/MINOR: quic: wake up MUX on probing only for 01RTT
On PTO probe timeout expiration, a probe packet must be emitted.
quic_pto_pktns() is used to determine for which packet space the timer
has expired. However, if MUX is already subscribed for sending, it is
woken up without checking first if this happened for the 01RTT packet
space.

It is unsure that this is really a bug as in most cases, MUX is
established only after Initial and Handshake packet spaces are removed.
However, the situation is not se clear when 0-RTT is used. For this
reason, adjust the code to explicitely check for the 01RTT packet space
before waking up the MUX layer.

This should be backported up to 2.6. Note that qc_notify_send() does not
exists in 2.6 so it should be replaced by the explicit block checking
(qc->subs && qc->subs->events & SUB_RETRY_SEND).
2023-03-21 14:09:50 +01:00
Frédéric Lécaille
e25fce03eb BUG/MINOR: quic: Dysfunctional 01RTT packet number space probing
This bug arrived with this commit:
   "MINOR: quic: implement qc_notify_send()".
The ->tx.pto_probe variable was no more set when qc_processt_timer() the timer
task for the connection responsible of detecting packet loss and probing upon
PTO expiration leading to interrupted stream transfers. This was revealed by
blackhole interop failed tests where one could see that qc_process_timer()
was wakeup without traces as follows in the log file:
   "needs to probe 01RTT packet number space"

Must be backported to 2.7 and to 2.6 if the commit mentionned above
is backported to 2.6 in the meantime.
2023-03-20 17:50:36 +01:00
Frédéric Lécaille
c664e644eb MINOR: quic: Stop stressing the acknowledgments process (RX ACK frames)
The ACK frame range of packets were handled from the largest to the smallest
packet number, leading to big number of ebtree insertions when the packet are
handled in the inverse way they are sent. This was detected a long time ago
but left in the code to stress our implementation. It is time to be more
efficient and process the packet so that to avoid useless ebtree insertions.

Modify qc_ackrng_pkts() responsible of handling the acknowledged packets from an
ACK frame range of acknowledged packets.

Must be backported to 2.7.
2023-03-20 17:47:12 +01:00
Frdric Lcaille
ca07979b97 BUG/MINOR: quic: Missing STREAM frame data pointer updates
This patch follows this one which was not sufficient:
    "BUG/MINOR: quic: Missing STREAM frame length updates"
Indeed, it is not sufficient to update the ->len and ->offset member
of a STREAM frame to move it forward. The data pointer must also be updated.
This is not done by the STREAM frame builder.

Must be backported to 2.6 and 2.7.
2023-03-17 09:21:18 +01:00
Frdric Lcaille
fc546ab6a7 BUG/MINOR: quic: Missing STREAM frame length updates
Some STREAM frame lengths were not updated before being duplicated, built
of requeued contrary to their ack offsets. This leads haproxy to crash when
receiving acknowledgements for such frames with this thread #1 backtrace:

 Thread 1 (Thread 0x7211b6ffd640 (LWP 986141)):
 #0  ha_crash_now () at include/haproxy/bug.h:52
 No locals.
 #1  b_del (b=<optimized out>, del=<optimized out>) at include/haproxy/buf.h:436
 No locals.
 #2  qc_stream_desc_ack (stream=stream@entry=0x7211b6fd9bc8, offset=offset@entry=53176, len=len@entry=1122) at src/quic_stream.c:111

Thank you to @Tristan971 for having provided such traces which reveal this issue:

[04|quic|5|c_conn.c:1865] qc_requeue_nacked_pkt_tx_frms(): entering : qc@0x72119c22cfe0
[04|quic|5|_frame.c:1179] qc_frm_unref(): entering : qc@0x72119c22cfe0
[04|quic|5|_frame.c:1186] qc_frm_unref(): remove frame reference : qc@0x72119c22cfe0 frm@0x72118863d260 STREAM_F uni=0 fin=1 id=460 off=52957 len=1122 3244
[04|quic|5|_frame.c:1194] qc_frm_unref(): leaving : qc@0x72119c22cfe0
[04|quic|5|c_conn.c:1902] qc_requeue_nacked_pkt_tx_frms(): updated partially acked frame : qc@0x72119c22cfe0 frm@0x72119c472290 STREAM_F uni=0 fin=1 id=460 off=53176 len=1122

Note that haproxy has much more chance to crash if this frame is the last one
(fin bit set). But another condition must be fullfilled to update the ack offset.
A previous STREAM frame from the same stream with the same offset but with less
data must be acknowledged by the peer. This is the condition to update the ack offset.

For others frames without fin bit in the same conditions, I guess the stream may be
truncated because too much data are removed from the stream when they are
acknowledged.

Must be backported to 2.6 and 2.7.
2023-03-16 14:35:19 +01:00
Frédéric Lécaille
be795ceb91 MINOR: quic: Do not stress the peer during retransmissions of lost packets
This issue was revealed by "Multiple streams" QUIC tracker test which very often
fails (locally) with a file of about 1Mbytes (x4 streams). The log of QUIC tracker
revealed that from its point of view, the 4 files were never all received entirely:

 "results" : {
      "stream_0_rec_closed" : true,
      "stream_0_rec_offset" : 1024250,
      "stream_0_snd_closed" : true,
      "stream_0_snd_offset" : 15,
      "stream_12_rec_closed" : false,
      "stream_12_rec_offset" : 72689,
      "stream_12_snd_closed" : true,
      "stream_12_snd_offset" : 15,
      "stream_4_rec_closed" : true,
      "stream_4_rec_offset" : 1024250,
      "stream_4_snd_closed" : true,
      "stream_4_snd_offset" : 15,
      "stream_8_rec_closed" : true,
      "stream_8_rec_offset" : 1024250,
      "stream_8_snd_closed" : true,
      "stream_8_snd_offset" : 15
   },

But this in contradiction with others QUIC tracker logs which confirms that haproxy
has really (re)sent the stream at the suspected offset(stream_12_rec_offset):

       1152085,
       "transport",
       "packet_received",
       {
          "frames" : [
             {
                "frame_type" : "stream",
                "length" : "155",
                "offset" : "72689",
                "stream_id" : "12"
             }
          ],
          "header" : {
             "dcid" : "a14479169ebb9dba",
             "dcil" : "8",
             "packet_number" : "466",
             "packet_size" : 190
          },
          "packet_type" : "1RTT"
       }

When detected as losts, the packets are enlisted, then their frames are
requeued in their packet number space by qc_requeue_nacked_pkt_tx_frms().
This was done using a local list which was spliced to the packet number
frame list. This had as bad effect to retransmit the frames in the inverse
order they have been sent. This is something the QUIC tracker go client
does not like at all!

Removing the frame splicing fixes this issue and allows haproxy to pass the
"Multiple streams" test.

Must be backported to 2.7.
2023-03-08 18:50:45 +01:00
Frédéric Lécaille
cc101cd2aa BUG/MINOR: quic: Wrong RETIRE_CONNECTION_ID sequence number check
This bug arrived with this commit:
     b5a8020e9 MINOR: quic: RETIRE_CONNECTION_ID frame handling (RX)
and was revealed by h3 interop tests with clients like s2n-quic and quic-go
as noticed by Amaury.

Indeed, one must check that the CID matching the sequence number provided by a received
RETIRE_CONNECTION_ID frame does not match the DCID of the packet.
Remove useless ->curr_cid_seq_num member from quic_conn struct.
The sequence number lookup must be done in qc_handle_retire_connection_id_frm()
to check the validity of the RETIRE_CONNECTION_ID frame, it returns the CID to be
retired into <cid_to_retire> variable passed as parameter to this function if
the frame is valid and if the CID was not already retired

Must be backported to 2.7.
2023-03-08 14:53:12 +01:00
Amaury Denoyelle
2d37629222 MINOR: quic: handle new closing list in show quic
A new global quic-conn list has been added by the previous patch. It will
contain every quic-conn in closing or draining state.

Thus, it is now easier to include or skip them on a "show quic" output :
when the default list on the current thread has been browsed entirely,
either we skip to the next thread or we look at the closing list on the
current thread.

This should be backported up to 2.7.
2023-03-08 14:39:48 +01:00
Amaury Denoyelle
efed86c973 MINOR: quic: create a global list dedicated for closing QUIC conns
When a CONNECTION_CLOSE is emitted or received, a QUIC connection enters
respectively in draining or closing state. These states are a loose
equivalent of TCP TIME_WAIT. No data can be exchanged anymore but the
connection is maintained during a certain timer to handle packet
reordering or loss.

A new global list has been defined for QUIC connections in
closing/draining state inside thread_ctx structure. Each time a
connection enters in one of this state, it will be moved from the
default global list to the new closing list.

The objective of this patch is to quickly filter connections on
closing/draining. Most notably, this will be used to wake up these
connections and avoid that haproxy process stopping is delayed by them.

A dedicated function qc_detach_th_ctx_list() has been implemented to
transfer a quic-conn from one list instance to the other. This takes
care of back-references attach to a quic-conn instance in case of a
running "show quic".

This should be backported up to 2.7.
2023-03-08 14:39:48 +01:00
Frédéric Lécaille
5e3201ea77 MINOR: quic: Add transport parameters to "show quic"
Modify quic_transport_params_dump() and others function relative to the
transport parameters value dump from TRACE() to make their output more
compact.
Add call to quic_transport_params_dump() to dump the transport parameters
from "show quic" CLI command.

Must be backported to 2.7.
2023-03-08 08:50:54 +01:00
Frédéric Lécaille
ece86e64c4 MINOR: quic: Add spin bit support
Add QUIC_FL_RX_PACKET_SPIN_BIT new RX packet flag to mark an RX packet as having
the spin bit set. Idem for the connection with QUIC_FL_CONN_SPIN_BIT flag.
Implement qc_handle_spin_bit() to set/unset QUIC_FL_CONN_SPIN_BIT for the connection
as soon as a packet number could be deciphered.
Modify quic_build_packet_short_header() to set the spin bit when building
a short packet header.

Validated by quic-tracker spin bit test.

Must be backported to 2.7.
2023-03-08 08:50:54 +01:00
Frédéric Lécaille
433af7fad9 MINOR: quic: Useless TLS context allocations in qc_do_rm_hp()
These allocations are definitively useless.

Must be backported to 2.7.
2023-03-08 08:50:54 +01:00
Frédéric Lécaille
8ac8a8778d MINOR: quic: RETIRE_CONNECTION_ID frame handling (RX)
Add ->curr_cid_seq_num new quic_conn struct frame to store the connection
ID sequence number currently used by the connection.
Implement qc_handle_retire_connection_id_frm() to handle this RX frame.
Implement qc_retire_connection_seq_num() to remove a connection ID from its
sequence number.
Implement qc_build_new_connection_id_frm to allocate a new NEW_CONNECTION_ID
frame from a CID.
Modify qc_parse_pkt_frms() which parses the frames of an RX packet to handle
the case of the RETIRE_CONNECTION_ID frame.

Must be backported to 2.7.
2023-03-08 08:50:54 +01:00
Frédéric Lécaille
b4c5471425 MINOR: quic: Store the next connection IDs sequence number in the connection
Add ->next_cid_seq_num new member to quic_conn struct to store the next
connection ID to be used to alloacated a connection ID.
It is initialized to 0 from qc_new_conn() which initializes a connection.
Modify new_quic_cid() to use this variable each time it is called without
giving the possibility to the caller to pass the sequence number for the
connection to be allocated.

Modify quic_build_post_handshake_frames() to use ->next_cid_seq_num
when building NEW_CONNECTION_ID frames after the hanshake has been completed.
Limit the number of connection IDs provided to the peer to the minimum
between 4 and the value it sent with active_connection_id_limit transport
parameter. This includes the connection ID used by the connection to send
this new connection IDs.

Must be backported to 2.7.
2023-03-08 08:50:54 +01:00
Amaury Denoyelle
315a4f6ae5 BUG/MEDIUM: quic: do not crash when handling STREAM on released MUX
The MUX instance is released before its quic-conn counterpart. On
termination, a H3 GOAWAY is emitted to prevent the client to open new
streams for this connection.

The quic-conn instance will stay alive until all opened streams data are
acknowledged. If the client tries to open a new stream during this
interval despite the GOAWAY, quic-conn is responsible to request its
immediate closure with a STOP_SENDING + RESET_STREAM.

This behavior was already implemented but the received packet with the
new STREAM was never acknowledged. This was fixed with the following
commit :
  commit 156a89aef8c63910502b266251dc34f648a99fae
  BUG/MINOR: quic: acknowledge STREAM frame even if MUX is released

However, this patch introduces a regression as it did not skip the call
to qc_handle_strm_frm() despite the MUX instance being released. This
can cause a segfault when using qcc_get_qcs() on a released MUX
instance. To fix this, add a missing break statement which will skip
qc_handle_strm_frm() when the MUX instance is not initialized.

This commit was reproduced using a short timeout client and sending
several requests with delay between them by using a modified aioquic. It
produces a crash with the following backtrace :
 #0  0x000055555594d261 in __eb64_lookup (x=4, root=0x7ffff4091f60) at include/import/eb64tree.h:132
 #1  eb64_lookup (root=0x7ffff4091f60, x=4) at src/eb64tree.c:37
 #2  0x000055555563fc66 in qcc_get_qcs (qcc=0x7ffff4091dc0, id=4, receive_only=1, send_only=0, out=0x7ffff780ca70) at src/mux_quic.c:668
 #3  0x0000555555641e1a in qcc_recv (qcc=0x7ffff4091dc0, id=4, len=40, offset=0, fin=1 '\001', data=0x7ffff40c4fef "\001&") at src/mux_quic.c:974
 #4  0x0000555555619d28 in qc_handle_strm_frm (pkt=0x7ffff4088e60, strm_frm=0x7ffff780cf50, qc=0x7ffff7cef000, fin=1 '\001') at src/quic_conn.c:2515
 #5  0x000055555561d677 in qc_parse_pkt_frms (qc=0x7ffff7cef000, pkt=0x7ffff4088e60, qel=0x7ffff7cef6c0) at src/quic_conn.c:3050
 #6  0x00005555556230aa in qc_treat_rx_pkts (qc=0x7ffff7cef000, cur_el=0x7ffff7cef6c0, next_el=0x0) at src/quic_conn.c:4214
 #7  0x0000555555625fee in quic_conn_app_io_cb (t=0x7ffff40c1fa0, context=0x7ffff7cef000, state=32848) at src/quic_conn.c:4640
 #8  0x00005555558a676d in run_tasks_from_lists (budgets=0x7ffff780d470) at src/task.c:596
 #9  0x00005555558a725b in process_runnable_tasks () at src/task.c:876
 #10 0x00005555558522ba in run_poll_loop () at src/haproxy.c:2945
 #11 0x00005555558529ac in run_thread_poll_loop (data=0x555555d14440 <ha_thread_info+64>) at src/haproxy.c:3141
 #12 0x00007ffff789ebb5 in ?? () from /usr/lib/libc.so.6
 #13 0x00007ffff7920d90 in ?? () from /usr/lib/libc.so.6

This should fix github issue #2067.

This must be backported up to 2.6.
2023-03-06 13:39:40 +01:00
Frédéric Lécaille
ec93721fb0 MINOR: quic: Send PING frames when probing Initial packet number space
In very very rare cases, it is possible the Initial packet number space
must be probed even if it there is no more in flight CRYPTO frames.
In such cases, a PING frame is sent into an Initial packet. As this
packet is ack-eliciting, it must be padded by the server. qc_do_build_pkt()
is modified to do so.

Take the opportunity of this patch to modify the trace for TX frames to
easily distinguished them from other frame relative traces.

Must be backported to 2.7.
2023-03-03 19:12:26 +01:00
Frédéric Lécaille
a65b71f89f BUG/MINOR: quic: Missing detections of amplification limit reached
Mark the connection as limited by the anti-amplification limit when trying to
probe the peer.
Wakeup the connection PTO/dectection loss timer as soon as a datagram is
received. This was done only when the datagram was dropped.
This fixes deadlock issues revealed by some interop runner tests.

Must be backported to 2.7 and 2.6.
2023-03-03 19:12:26 +01:00
Frédéric Lécaille
e6359b649b BUG/MINOR: quic: Do not resend already acked frames
Some frames are marked as already acknowledged from duplicated packets
whose the original packet has been acknowledged. There is no need
to resend such packets or frames.

Implement qc_pkt_with_only_acked_frms() to detect packet with only
already acknowledged frames inside and use it from qc_prep_fast_retrans()
which selects the packet to be retransmitted.

Must be backported to 2.6 and 2.7.
2023-03-03 19:12:26 +01:00
Frédéric Lécaille
21564be4a2 BUG/MINOR: quic: Ensure not to retransmit packets with no ack-eliciting frames
Even if there is a check in callers of qc_prep_hdshk_fast_retrans() and
qc_prep_fast_retrans() to prevent retransmissions of packets with no ack-eliciting
frames, these two functions should pay attention not do to that especially if
someone decides to modify their implementations in the future.

Must be backported to 2.6 and 2.7.
2023-03-03 19:12:26 +01:00
Frédéric Lécaille
b3562a3815 BUG/MINOR: quic: Remove force_ack for Initial,Handshake packets
This is an old bug which arrived in this commit due to a misinterpretation
of the RFC I guess where the desired effect was to acknowledge all the
handshake packets:

    77ac6f566 BUG/MINOR: quic: Missing acknowledgments for trailing packets

This had as bad effect to acknowledge all the handshake packets even the
ones which are not ack-eliciting.

Must be backported to 2.7 and 2.6.
2023-03-03 19:12:26 +01:00
Frédéric Lécaille
51a7caf921 MINOR: quic: Add traces about QUIC TLS key update
Dump the secret used to derive the next one during a key update initiated by the
client and dump the resulted new secret and the new key and iv to be used to
decryption Application level packets.

Also add a trace when the key update is supposed to be initiated on haproxy side.

This has already helped in diagnosing an issue evealed by the key update interop
test with xquic as client.

Must be backported to 2.7.
2023-03-03 19:12:26 +01:00