<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 4/25/25 2:29 PM, Roman Arutyunyan
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20250425112913.nxpwbtr6mn3olaxg@N00W24XTQX">
<pre wrap="" class="moz-quote-pre">Hi Vladimir,
Thanks for your patch, I will look into this. The issue indeed seems possible
and needs to be addressed. The mentiobned change cd5e4fa14 is correct though
since without it CWND used to skyrocket while being underutilized. And this
certainly consealed other issues.
Do you have a reliable way to trigger this?</pre>
</blockquote>
Yes, I'm able to trigger this reliably with angie as a client (with
<span style="white-space: pre-wrap">cd5e4fa14</span> merged both on
client and server).<br>
<blockquote type="cite"
cite="mid:20250425112913.nxpwbtr6mn3olaxg@N00W24XTQX">
<pre wrap="" class="moz-quote-pre">
On Fri, Apr 25, 2025 at 01:55:11PM +0300, Vladimir Homutov via nginx-devel wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">missing attachments
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">commit 0c7c9d6732a5fe3a3208286c8904db1851ac2cba
Author: Vladimir Homutov <a class="moz-txt-link-rfc2396E" href="mailto:vl@inspert.ru"><vl@inspert.ru></a>
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);
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">_______________________________________________
nginx-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a>
<a class="moz-txt-link-freetext" href="https://mailman.nginx.org/mailman/listinfo/nginx-devel">https://mailman.nginx.org/mailman/listinfo/nginx-devel</a>
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
--
Roman
</pre>
</blockquote>
</body>
</html>