[nginx] QUIC: prevent spurious congestion control recovery mode.

[noreply at nginx.com]

details:   https://github.com/nginx/nginx/commit/38236bf74f3e5728eeea488bef381c61842ac1d2
branches:  master
commit:    38236bf74f3e5728eeea488bef381c61842ac1d2
user:      Roman Arutyunyan <arut at nginx.com>
date:      Fri, 3 Jan 2025 13:01:06 +0400
description:
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.

---
 src/event/quic/ngx_event_quic.c           |  2 +-
 src/event/quic/ngx_event_quic_ack.c       | 54 ++++++++++++++++++++++++-------
 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);
     }