aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRoman Arutyunyan <arut@nginx.com>2025-01-03 13:01:06 +0400
committerRoman Arutyunyan <arutyunyan.roman@gmail.com>2025-04-15 19:01:36 +0400
commit38236bf74f3e5728eeea488bef381c61842ac1d2 (patch)
tree8cf01926103f546607e658f426f1377bc2dd4e39
parent53e7e9eb542fb1d3d885bbca03ed1d704aa08f31 (diff)
downloadnginx-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.c2
-rw-r--r--src/event/quic/ngx_event_quic_ack.c54
-rw-r--r--src/event/quic/ngx_event_quic_migration.c2
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);
}