From 391e49c4cf2c64a5629dda3884ff38d85d63f41b Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Tue, 15 Jun 2021 10:41:51 +0900 Subject: [PATCH 1/3] set minimum fragments relative to stream-level concurrency --- lib/quicly.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index a8e2017e..5f37a339 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1020,11 +1020,15 @@ static void init_stream_properties(quicly_stream_t *stream, uint32_t initial_max /* Set the number of max ranges to be capable of handling following case: * * every one of the two packets being sent are lost - * * average size of a STREAM frame found in a packet is >= ~512 bytes + * * average size of a STREAM frame found in a packet is >= ~512 bytes, or small STREAM frame is sent for every other stream + * being opened (e.g., sending QPACK encoder/decoder stream frame for each HTTP/3 request) * See also: the doc-comment on `_recv_aux.max_ranges`. */ - if ((stream->_recv_aux.max_ranges = initial_max_stream_data_local / 1024) < 63) - stream->_recv_aux.max_ranges = 63; + uint32_t fragments_minmax = (uint32_t)(stream->conn->super.ctx->transport_params.max_streams_uni + stream->conn->super.ctx->transport_params.max_streams_bidi); + if (fragments_minmax < 63) + fragments_minmax = 63; + if ((stream->_recv_aux.max_ranges = initial_max_stream_data_local / 1024) < fragments_minmax) + stream->_recv_aux.max_ranges = fragments_minmax; } static void dispose_stream_properties(quicly_stream_t *stream) From eb36d245f6b733baa8bad4c7bf82888b5c6fc2b3 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Tue, 15 Jun 2021 10:43:08 +0900 Subject: [PATCH 2/3] relax the state restriction --- lib/sendstate.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/sendstate.c b/lib/sendstate.c index 35ff0e8f..96cde769 100644 --- a/lib/sendstate.c +++ b/lib/sendstate.c @@ -98,8 +98,12 @@ static int check_amount_of_state(quicly_sendstate_t *state) { size_t num_ranges = state->acked.num_ranges + state->pending.num_ranges; - /* bail out if number of gaps are small */ - if (PTLS_LIKELY(num_ranges < 32)) + /* Bail out if number of gaps are small. + * In case of HTTP/3, the worst case is when each HTTP request is received as a separate QUIC packet, and sending a small STREAM + * frame carrying a HPACK encoder / decoder in response. If half of those STREAM frames are lost (note: loss of every other + * packet can happen during slow start), `num_ranges` can become as large as `request_concurrency * 2`, as each gaps will be + * recognized in `acked.num_ranges` and `pending.num_ranges`. */ + if (PTLS_LIKELY(num_ranges < 256)) return 0; /* When there are large number of gaps, make sure that the amount of state retained in quicly is relatively smaller than the @@ -107,8 +111,8 @@ static int check_amount_of_state(quicly_sendstate_t *state) * assumption that the STREAM frames that have been sent are on average at least 512 bytes long, when seeing excess number of * gaps. */ int64_t bytes_buffered = (int64_t)state->size_inflight - (int64_t)state->acked.ranges[0].end; - if ((int64_t)num_ranges * 512 > bytes_buffered) - return QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION; + if ((int64_t)num_ranges * 128 > bytes_buffered) + return QUICLY_ERROR_STATE_EXHAUSTION; return 0; } From 068a21a4a9dd2a69f1f74f944853a5f879c8f02b Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Tue, 15 Jun 2021 14:03:51 +0900 Subject: [PATCH 3/3] clang-format --- lib/quicly.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/quicly.c b/lib/quicly.c index 5f37a339..651aa052 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1024,7 +1024,8 @@ static void init_stream_properties(quicly_stream_t *stream, uint32_t initial_max * being opened (e.g., sending QPACK encoder/decoder stream frame for each HTTP/3 request) * See also: the doc-comment on `_recv_aux.max_ranges`. */ - uint32_t fragments_minmax = (uint32_t)(stream->conn->super.ctx->transport_params.max_streams_uni + stream->conn->super.ctx->transport_params.max_streams_bidi); + uint32_t fragments_minmax = (uint32_t)(stream->conn->super.ctx->transport_params.max_streams_uni + + stream->conn->super.ctx->transport_params.max_streams_bidi); if (fragments_minmax < 63) fragments_minmax = 63; if ((stream->_recv_aux.max_ranges = initial_max_stream_data_local / 1024) < fragments_minmax)