[nginx] QUIC: do not increase underutilized congestion window.
Vladimir Homutov
vl at inspert.ru
Fri Apr 25 12:19:23 UTC 2025
On 4/25/25 2:29 PM, Roman Arutyunyan wrote:
> 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?
Yes, I'm able to trigger this reliably with angie as a client (with
cd5e4fa14 merged both on client and server).
>
> On Fri, Apr 25, 2025 at 01:55:11PM +0300, Vladimir Homutov via nginx-devel wrote:
>> missing attachments
>>
>> 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);
>>
>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> https://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> --
> Roman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20250425/23c44948/attachment.htm>
More information about the nginx-devel
mailing list