BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 26 Jan 2024 13:41:04 +0000 (14:41 +0100)
committerWilly Tarreau <w@1wt.eu>
Wed, 31 Jan 2024 14:44:56 +0000 (15:44 +0100)
commit1d839872bcdaf0c41091065e23f6b70829bfaa57
treef4b6d54d13257b499bdc400cf13f2f05673fee26
parent5afa702cc2d986fd0688ea80b84135f694ba6d17
BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf

QCS instances use qc_stream_desc for data buffering on emission. On
stream reset, its Tx channel is closed earlier than expected. This may
leave unsent data into qc_stream_desc.

Before this patch, these unsent data would remain after QCS freeing.
This prevents the buffer to be released as no ACK reception will remove
them. The buffer is only freed when the whole connection is closed. As
qc_stream_desc buffer is limited per connection, this reduces the buffer
pool for other streams of the same connection. In the worst case if
several streams are resetted, this may completely freeze the transfer of
the remaining connection streams.

This bug was reproduced by reducing the connection buffer pool to a
single buffer instance by using the following global statement :

  tune.quic.frontend.conn-tx-buffers.limit 1.

Then a QUIC client is used which opens a stream for a large enough
object to ensure data are buffered. The client them emits a STOP_SENDING
before reading all data, which forces the corresponding QCS instance to
be resetted. The client then opens a new request but the transfer is
freezed due to this bug.

To fix this, adjust qc_stream_desc API. Add a new argument <final_size>
on qc_stream_desc_release() function. Its value is compared to the
currently buffered offset in latest qc_stream_desc buffer. If
<final_size> is inferior, it means unsent data are present in the
buffer. As such, qc_stream_desc_release() removes them to ensure the
buffer will finally be freed when all ACKs are received. It is also
possible that no data remains immediately, indicating that ACK were
already received. As such, buffer instance is immediately removed by
qc_stream_buf_free().

This must be backported up to 2.6. As this code section is known to
regression, a period of observation could be reserved before
distributing it on LTS releases.

(cherry picked from commit ad6b13d3177945bf6a85d6dc5af80b8e34ea6191)
Signed-off-by: Willy Tarreau <w@1wt.eu>
include/haproxy/quic_stream.h
src/mux_quic.c
src/quic_stream.c
src/quic_tls.c