missing attachments
commit 0c7c9d6732a5fe3a3208286c8904db1851ac2cba Author: Vladimir Homutov <v...@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);
logs_info.tar.gz
Description: application/gzip
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
