]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: quic: decount acknowledged data for MUX txbuf window
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 18 Sep 2024 08:32:39 +0000 (10:32 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 4 Oct 2024 15:31:26 +0000 (17:31 +0200)
Recently, a new allocation mechanism was implemented for Tx buffers used
by QUIC MUX. Now, underlying congestion window size is used to determine
if it is still possible or not to allocate a new buffer when necessary.

This mechanism has render the QUIC stack more flexible. However, it also
has brought some performance degradation, with transfer time longer in
certain environment. It was first discovered on the measurement results
of the interop. It can also easily be reproduced using the following
ngtcp2-client example which forces a very small congestion window due to
frequent loss :

 $ ngtcp2-client -q --no-quic-dump --no-http-dump --exit-on-all-streams-close -r 0.1 127.0.0.1 20443 "https://[::]:20443/?s=10m"

This performance decrease is caused by the allocator which is now too
strict. It may cause buffer underrun frequently at the MUX layer when
the congestion window is too small, as new buffers cannot be allocated
until the current one is fully acknowledged. This resuls in transfers
with very bad throughput utilisation. The objective of this new serie of
patches is to relax some restrictions to permit QUIC MUX to allocate new
buffers more quickly, while preserving the initial limitation based on
congestion window size.

An interesting method for this is to notify QUIC MUX about newly
available room on individual ACK reception, without waiting for the full
bffer acknowledgement. This is easily implemented by adding a new
notify_room invokation in QUIC streamdesc layer on ACK reception.
However, ACK reception are handled in-order at the stream level. Out of
order ACKs are buffered and are not decounted for now. This will be
implemented in a future commit.

Note that for a single buffer instance, data can in parallel be written
by QUIC MUX and removed on ACK reception. This could cause room
notification to QUIC MUX layer to report invalid values. As such, ACK
reception are only accounted for released buffers. This ensures that
such buffers won't received any new data. In the same time, buffer room
is notified on release operation as it does not need acknowledgement.

This commit has permit to improve performance for the ngtcp2-client
scenario above. However, it is not yet sufficient enough for interop
goodput test.

src/quic_stream.c

index e2b971c815548b8d10da5779dbdd5b228a944258..896bf1dd152935e7cb98c4191a034b080fce4f46 100644 (file)
@@ -23,18 +23,23 @@ static void qc_stream_buf_free(struct qc_stream_desc *stream,
                                struct qc_stream_buf **stream_buf)
 {
        struct buffer *buf = &(*stream_buf)->buf;
-       uint64_t free_size;
+       uint64_t room;
 
        /* Caller is responsible to remove buffered ACK frames before destroying a buffer instance. */
        BUG_ON(!eb_is_empty(&(*stream_buf)->acked_frms));
 
        eb64_delete(&(*stream_buf)->offset_node);
 
-       /* Reset current buf ptr if deleted instance is the same one. */
-       if (*stream_buf == stream->buf)
+       if (*stream_buf == stream->buf) {
+               /* Reset current buffer ptr. */
                stream->buf = NULL;
+               room = b_size(buf);
+       }
+       else {
+               /* For released buffer, acked data were already notified. */
+               room = b_data(buf);
+       }
 
-       free_size = b_size(buf);
        if ((*stream_buf)->sbuf) {
                pool_free(pool_head_sbuf, buf->area);
        }
@@ -46,8 +51,8 @@ static void qc_stream_buf_free(struct qc_stream_desc *stream,
        *stream_buf = NULL;
 
        /* notify MUX about available buffers. */
-       if (stream->notify_room)
-               stream->notify_room(stream, free_size);
+       if (stream->notify_room && room)
+               stream->notify_room(stream, room);
 }
 
 /* Allocate a new stream descriptor with id <id>. The caller is responsible to
@@ -108,6 +113,7 @@ void qc_stream_desc_release(struct qc_stream_desc *stream,
        stream->flags |= QC_SD_FL_RELEASE;
        stream->ctx = new_ctx;
 
+       /* Release active buffer if still present on streamdesc release. */
        if (stream->buf) {
                struct qc_stream_buf *stream_buf = stream->buf;
                struct buffer *buf = &stream_buf->buf;
@@ -121,10 +127,14 @@ void qc_stream_desc_release(struct qc_stream_desc *stream,
                if (final_size < tail_offset)
                        b_sub(buf, tail_offset - final_size);
 
-               if (!b_data(buf))
+               if (!b_data(buf)) {
+                       /* This will ensure notify_room is triggered. */
                        qc_stream_buf_free(stream, &stream_buf);
+               }
+               else if (stream->notify_room && b_room(buf)) {
+                       stream->notify_room(stream, b_room(buf));
+               }
 
-               /* A released stream does not use <stream.buf>. */
                stream->buf = NULL;
        }
 
@@ -153,6 +163,10 @@ static struct qc_stream_buf *qc_stream_buf_ack(struct qc_stream_buf *buf,
                const uint64_t diff = offset + len - stream->ack_offset;
                b_del(&buf->buf, diff);
                stream->ack_offset += diff;
+
+               /* notify room from acked data if buffer has been released. */
+               if (stream->notify_room && buf != stream->buf)
+                       stream->notify_room(stream, diff);
        }
 
        if (fin) {
@@ -407,11 +421,18 @@ struct buffer *qc_stream_buf_realloc(struct qc_stream_desc *stream)
  */
 void qc_stream_buf_release(struct qc_stream_desc *stream)
 {
+       uint64_t room;
+
        /* current buffer already released */
        BUG_ON(!stream->buf);
 
+       room = b_room(&stream->buf->buf);
        stream->buf = NULL;
        stream->buf_offset = 0;
+
+       /* Released buffer won't receive any new data. Reports non consumed space as free room. */
+       if (stream->notify_room && room)
+               stream->notify_room(stream, room);
 }
 
 static int create_sbuf_pool(void)