> On 10 Aug 2023, at 20:26, Vladimir Homutov via nginx-devel > <nginx-devel@nginx.org> wrote: > > On Thu, Aug 10, 2023 at 08:02:06PM +0400, Sergey Kandaurov wrote: >> >>> On 27 Jul 2023, at 16:42, Roman Arutyunyan <a...@nginx.com> wrote: >>> >>> # HG changeset patch >>> # User Roman Arutyunyan <a...@nginx.com> >>> # Date 1690461509 -14400 >>> # Thu Jul 27 16:38:29 2023 +0400 >>> # Node ID 0d12ada84c168c62e9bae847af2725641da583d0 >>> # Parent 2fd16fc76920ef0b8ea2fa64858934e38c4477c5 >>> QUIC: always add ACK frame to the queue head. >>> >>> Previously it was added to the tail as all other frames. However, if the >>> amount of queued data is large, it could delay the delivery of ACK, which >>> could trigger frames retransmissions and slow down the connection. >>> >>> diff --git a/src/event/quic/ngx_event_quic_output.c >>> b/src/event/quic/ngx_event_quic_output.c >>> --- a/src/event/quic/ngx_event_quic_output.c >>> +++ b/src/event/quic/ngx_event_quic_output.c >>> @@ -1175,7 +1175,9 @@ ngx_quic_send_ack(ngx_connection_t *c, n >>> frame->u.ack.range_count = ctx->nranges; >>> frame->u.ack.first_range = ctx->first_range; >>> >>> - ngx_quic_queue_frame(qc, frame); >>> + ngx_queue_insert_head(&ctx->frames, &frame->queue); >>> + >>> + frame->len = ngx_quic_create_frame(NULL, frame); >>> >>> return NGX_OK; >>> } >> >> place frame->len first, to other frame assignments? >> >> Otherwise, looks good. > > The ngx_quic_queue_frame() functions also posts the push event. > > Most likely it will be set anyway, but formally the change missing it.
Yes, post is missed now, thanks for noting. I believe Roman did so for a reason, because ngx_quic_send_ack() is called from send data path, so I think this should be safe. -- Sergey Kandaurov _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
