[nginx] QUIC: do not increase underutilized congestion window.

[Vladimir Homutov]

missing attachments

-------------- next part --------------
commit 0c7c9d6732a5fe3a3208286c8904db1851ac2cba
Author: Vladimir Homutov <vl at inspert.ru>
Date:   Fri Apr 25 13:35:00 2025 +0300

    QUIC: fixed possible deadlock with ACKs not sent due to congestion.
    
    The commit cd5e4fa1446dff86fafc3b6ffcc11afd527a024f (QUIC: do not increase
    underutilized congestion window) lead to increased possibility of deadlock,
    caused by the fact that quic does not output any packets in congestion.
    
    If both client and server follow the same logic, it is possible that both
    ends are in the same state: waiting for ACKs to increase window, while
    being unable to send even ACK to unblock the other side.
    
    Since packets that contain ACK frames only are not congestion controlled,
    we are definitely allowed to send them, since we also need to respect
    the max_ack_delay.  Currently, the max_ack_delay may be triggered and
    push handler is called, but the output does not send anything, because
    window is closed, thus nothing is sent.
    
    The patch allows to send ACK-only packets in case when the window is closed.
    Probably, this needs to be attached to the actual trigger of max_ack_delay
    timer, but it looks like suggested changes is goed enough for practical reasons.
    
    Also, RFC 9000 13.2.1 says:
        Since packets containing only ACK frames are not congestion controlled,
        an endpoint MUST NOT send more than one such packet in response to
        receiving an ack-eliciting packet.
    
    Probably, this also needs to be accounted.

diff --git a/src/event/quic/ngx_event_quic_output.c b/src/event/quic/ngx_event_quic_output.c
index a92a539f3..4e51518c0 100644
--- a/src/event/quic/ngx_event_quic_output.c
+++ b/src/event/quic/ngx_event_quic_output.c
@@ -55,7 +55,8 @@ static ssize_t ngx_quic_send_segments(ngx_connection_t *c, u_char *buf,
     size_t len, struct sockaddr *sockaddr, socklen_t socklen, size_t segment);
 #endif
 static ssize_t ngx_quic_output_packet(ngx_connection_t *c,
-    ngx_quic_send_ctx_t *ctx, u_char *data, size_t max, size_t min);
+    ngx_quic_send_ctx_t *ctx, u_char *data, size_t max, size_t min,
+    ngx_uint_t ack_only);
 static void ngx_quic_init_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
     ngx_quic_header_t *pkt, ngx_quic_path_t *path);
 static ngx_uint_t ngx_quic_get_padding_level(ngx_connection_t *c);
@@ -116,7 +117,7 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
     ssize_t                 n;
     u_char                 *p;
     uint64_t                preserved_pnum[NGX_QUIC_SEND_CTX_LAST];
-    ngx_uint_t              i, pad;
+    ngx_uint_t              i, pad, ack_only;
     ngx_quic_path_t        *path;
     ngx_quic_send_ctx_t    *ctx;
     ngx_quic_congestion_t  *cg;
@@ -131,7 +132,9 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
     ngx_memzero(preserved_pnum, sizeof(preserved_pnum));
 #endif
 
-    while (cg->in_flight < cg->window) {
+    ack_only = (cg->in_flight >= cg->window);
+
+    while ((cg->in_flight < cg->window) || ack_only) {
 
         p = dst;
 
@@ -158,7 +161,7 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
                 return NGX_OK;
             }
 
-            n = ngx_quic_output_packet(c, ctx, p, len, min);
+            n = ngx_quic_output_packet(c, ctx, p, len, min, ack_only);
             if (n == NGX_ERROR) {
                 return NGX_ERROR;
             }
@@ -186,6 +189,9 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
 
         ngx_quic_commit_send(c);
 
+        /* send pure acks just once */
+        ack_only = 0;
+
         path->sent += len;
     }
 
@@ -331,7 +337,7 @@ ngx_quic_create_segments(ngx_connection_t *c)
     size_t                  len, segsize;
     ssize_t                 n;
     u_char                 *p, *end;
-    ngx_uint_t              nseg, level;
+    ngx_uint_t              nseg, level, ack_only;
     ngx_quic_path_t        *path;
     ngx_quic_send_ctx_t    *ctx;
     ngx_quic_congestion_t  *cg;
@@ -358,13 +364,15 @@ ngx_quic_create_segments(ngx_connection_t *c)
     level = ctx - qc->send_ctx;
     preserved_pnum[level] = ctx->pnum;
 
+    ack_only = (cg->in_flight >= cg->window);
+
     for ( ;; ) {
 
         len = ngx_min(segsize, (size_t) (end - p));
 
-        if (len && cg->in_flight + (p - dst) < cg->window) {
+        if (len && ((cg->in_flight + (p - dst) < cg->window) || ack_only)) {
 
-            n = ngx_quic_output_packet(c, ctx, p, len, len);
+            n = ngx_quic_output_packet(c, ctx, p, len, len, ack_only);
             if (n == NGX_ERROR) {
                 return NGX_ERROR;
             }
@@ -397,6 +405,9 @@ ngx_quic_create_segments(ngx_connection_t *c)
 
             ngx_quic_commit_send(c);
 
+            /* send pure acks just once */
+            ack_only = 0;
+
             path->sent += n;
 
             p = dst;
@@ -521,7 +532,7 @@ ngx_quic_get_padding_level(ngx_connection_t *c)
 
 static ssize_t
 ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
-    u_char *data, size_t max, size_t min)
+    u_char *data, size_t max, size_t min, ngx_uint_t ack_only)
 {
     size_t                  len, pad, min_payload, max_payload;
     u_char                 *p;
@@ -589,6 +600,12 @@ ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
             break;
         }
 
+        if (ack_only
+            && (f->type != NGX_QUIC_FT_ACK && f->type != NGX_QUIC_FT_ACK_ECN))
+        {
+            continue;
+        }
+
         if (len + f->len > max_payload) {
             rc = ngx_quic_split_frame(c, f, max_payload - len);
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: logs_info.tar.gz
Type: application/gzip
Size: 24054 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20250425/054fe34b/attachment-0001.bin>