diff options
author | Roman Arutyunyan <arut@nginx.com> | 2025-01-03 13:01:06 +0400 |
---|---|---|
committer | Roman Arutyunyan <arutyunyan.roman@gmail.com> | 2025-04-15 19:01:36 +0400 |
commit | 38236bf74f3e5728eeea488bef381c61842ac1d2 (patch) | |
tree | 8cf01926103f546607e658f426f1377bc2dd4e39 | |
parent | 53e7e9eb542fb1d3d885bbca03ed1d704aa08f31 (diff) | |
download | nginx-38236bf74f3e5728eeea488bef381c61842ac1d2.tar.gz nginx-38236bf74f3e5728eeea488bef381c61842ac1d2.zip |
QUIC: prevent spurious congestion control recovery mode.
Since recovery_start field was initialized with ngx_current_msec, all
congestion events that happened within the same millisecond or cycle
iteration, were treated as in recovery mode.
Also, when handling persistent congestion, initializing recovery_start
with ngx_current_msec resulted in treating all sent packets as in recovery
mode, which violates RFC 9002, see example in Appendix B.8.
While here, also fixed recovery_start wrap protection. Previously it used
2 * max_idle_timeout time frame for all sent frames, which is not a
reliable protection since max_idle_timeout is unrelated to congestion
control. Now recovery_start <= now condition is enforced. Note that
recovery_start wrap is highly unlikely and can only occur on a
32-bit system if there are no congestion events for 24 days.
-rw-r--r-- | src/event/quic/ngx_event_quic.c | 2 | ||||
-rw-r--r-- | src/event/quic/ngx_event_quic_ack.c | 54 | ||||
-rw-r--r-- | src/event/quic/ngx_event_quic_migration.c | 2 |
3 files changed, 44 insertions, 14 deletions
diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c index 70d9748bd..11497a6d7 100644 --- a/src/event/quic/ngx_event_quic.c +++ b/src/event/quic/ngx_event_quic.c @@ -312,7 +312,7 @@ ngx_quic_new_connection(ngx_connection_t *c, ngx_quic_conf_t *conf, ngx_max(2 * NGX_QUIC_MIN_INITIAL_SIZE, 14720)); qc->congestion.ssthresh = (size_t) -1; - qc->congestion.recovery_start = ngx_current_msec; + qc->congestion.recovery_start = ngx_current_msec - 1; if (pkt->validated && pkt->retried) { qc->tp.retry_scid.len = pkt->dcid.len; diff --git a/src/event/quic/ngx_event_quic_ack.c b/src/event/quic/ngx_event_quic_ack.c index 4616e7053..29c5bfed1 100644 --- a/src/event/quic/ngx_event_quic_ack.c +++ b/src/event/quic/ngx_event_quic_ack.c @@ -41,6 +41,7 @@ static ngx_int_t ngx_quic_detect_lost(ngx_connection_t *c, ngx_quic_ack_stat_t *st); static ngx_msec_t ngx_quic_pcg_duration(ngx_connection_t *c); static void ngx_quic_persistent_congestion(ngx_connection_t *c); +static ngx_msec_t ngx_quic_oldest_sent_packet(ngx_connection_t *c); static void ngx_quic_congestion_lost(ngx_connection_t *c, ngx_quic_frame_t *frame); static void ngx_quic_lost_handler(ngx_event_t *ev); @@ -335,6 +336,14 @@ ngx_quic_congestion_ack(ngx_connection_t *c, ngx_quic_frame_t *f) cg->in_flight -= f->plen; + /* prevent recovery_start from wrapping */ + + timer = now - cg->recovery_start; + + if ((ngx_msec_int_t) timer < 0) { + cg->recovery_start = ngx_quic_oldest_sent_packet(c) - 1; + } + timer = f->send_time - cg->recovery_start; if ((ngx_msec_int_t) timer <= 0) { @@ -360,14 +369,6 @@ ngx_quic_congestion_ack(ngx_connection_t *c, ngx_quic_frame_t *f) now, cg->window, cg->in_flight); } - /* prevent recovery_start from wrapping */ - - timer = cg->recovery_start - now + qc->tp.max_idle_timeout * 2; - - if ((ngx_msec_int_t) timer < 0) { - cg->recovery_start = now - qc->tp.max_idle_timeout * 2; - } - done: if (blocked && cg->in_flight < cg->window) { @@ -543,19 +544,48 @@ ngx_quic_pcg_duration(ngx_connection_t *c) static void ngx_quic_persistent_congestion(ngx_connection_t *c) { - ngx_msec_t now; ngx_quic_congestion_t *cg; ngx_quic_connection_t *qc; qc = ngx_quic_get_connection(c); cg = &qc->congestion; - now = ngx_current_msec; - cg->recovery_start = now; + cg->recovery_start = ngx_quic_oldest_sent_packet(c) - 1; cg->window = qc->path->mtu * 2; ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0, - "quic congestion persistent t:%M win:%uz", now, cg->window); + "quic congestion persistent t:%M win:%uz", + ngx_current_msec, cg->window); +} + + +static ngx_msec_t +ngx_quic_oldest_sent_packet(ngx_connection_t *c) +{ + ngx_msec_t oldest; + ngx_uint_t i; + ngx_queue_t *q; + ngx_quic_frame_t *start; + ngx_quic_send_ctx_t *ctx; + ngx_quic_connection_t *qc; + + qc = ngx_quic_get_connection(c); + oldest = ngx_current_msec; + + for (i = 0; i < NGX_QUIC_SEND_CTX_LAST; i++) { + ctx = &qc->send_ctx[i]; + + if (!ngx_queue_empty(&ctx->sent)) { + q = ngx_queue_head(&ctx->sent); + start = ngx_queue_data(q, ngx_quic_frame_t, queue); + + if ((ngx_msec_int_t) (start->send_time - oldest) < 0) { + oldest = start->send_time; + } + } + } + + return oldest; } diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c index ac22b1327..3caae88e5 100644 --- a/src/event/quic/ngx_event_quic_migration.c +++ b/src/event/quic/ngx_event_quic_migration.c @@ -186,7 +186,7 @@ valid: ngx_max(2 * NGX_QUIC_MIN_INITIAL_SIZE, 14720)); qc->congestion.ssthresh = (size_t) -1; - qc->congestion.recovery_start = ngx_current_msec; + qc->congestion.recovery_start = ngx_current_msec - 1; ngx_quic_init_rtt(qc); } |