From: Amaury Denoyelle Date: Wed, 18 Sep 2024 08:32:39 +0000 (+0200) Subject: MEDIUM: quic: decount acknowledged data for MUX txbuf window X-Git-Tag: v3.1-dev10~116 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4ff87db5feb3071c7155f3268d9cdfa6f3882911;p=thirdparty%2Fhaproxy.git MEDIUM: quic: decount acknowledged data for MUX txbuf window 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. --- diff --git a/src/quic_stream.c b/src/quic_stream.c index e2b971c815..896bf1dd15 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -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 . 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 = 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)