[PATCH 2 of 4] QUIC: always add ACK frame to the queue head
Sergey Kandaurov
pluknet at nginx.com
Thu Aug 10 16:30:55 UTC 2023
> On 10 Aug 2023, at 20:26, Vladimir Homutov via nginx-devel <nginx-devel at 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 <arut at nginx.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Roman Arutyunyan <arut at 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
More information about the nginx-devel
mailing list