[PATCH] QUIC: optimized immediate close

[Sergey Kandaurov]

> On 2 May 2023, at 17:58, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1683035693 -14400
> #      Tue May 02 17:54:53 2023 +0400
> # Branch quic
> # Node ID 0ae438bff8e8788f6972b2f8268194b571d728c0
> # Parent  558cdf07793de4108527ed6757fd3864e9694b93
> QUIC: optimized immediate close.
> 
> Previously, before sending CONNECTION_CLOSE to client, all pending frames
> were sent.  This is redundant and could prevent CONNECTION_CLOSE from being
> sent due to congestion control.  Now pending frames are freed and
> CONNECTION_CLOSE is sent without congestion control, as advised by RFC 9002:
> 
>  Packets containing frames besides ACK or CONNECTION_CLOSE frames
>  count toward congestion control limits and are considered to be in flight.
> 
> diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> --- a/src/event/quic/ngx_event_quic.c
> +++ b/src/event/quic/ngx_event_quic.c
> @@ -482,6 +482,7 @@ ngx_quic_close_connection(ngx_connection
> 
>         /* drop packets from retransmit queues, no ack is expected */
>         for (i = 0; i < NGX_QUIC_SEND_CTX_LAST; i++) {
> +            ngx_quic_free_frames(c, &qc->send_ctx[i].frames);
>             ngx_quic_free_frames(c, &qc->send_ctx[i].sent);
>         }
> 
> 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
> @@ -882,7 +882,7 @@ ngx_quic_send_stateless_reset(ngx_connec
> ngx_int_t
> ngx_quic_send_cc(ngx_connection_t *c)
> {
> -    ngx_quic_frame_t       *frame;
> +    ngx_quic_frame_t        frame;
>     ngx_quic_connection_t  *qc;
> 
>     qc = ngx_quic_get_connection(c);
> @@ -898,27 +898,22 @@ ngx_quic_send_cc(ngx_connection_t *c)
>         return NGX_OK;
>     }
> 
> -    frame = ngx_quic_alloc_frame(c);
> -    if (frame == NULL) {
> -        return NGX_ERROR;
> -    }
> +    ngx_memzero(&frame, sizeof(ngx_quic_frame_t));
> 
> -    frame->level = qc->error_level;
> -    frame->type = qc->error_app ? NGX_QUIC_FT_CONNECTION_CLOSE_APP
> -                                : NGX_QUIC_FT_CONNECTION_CLOSE;
> -    frame->u.close.error_code = qc->error;
> -    frame->u.close.frame_type = qc->error_ftype;
> +    frame.level = qc->error_level;
> +    frame.type = qc->error_app ? NGX_QUIC_FT_CONNECTION_CLOSE_APP
> +                               : NGX_QUIC_FT_CONNECTION_CLOSE;
> +    frame.u.close.error_code = qc->error;
> +    frame.u.close.frame_type = qc->error_ftype;
> 
>     if (qc->error_reason) {
> -        frame->u.close.reason.len = ngx_strlen(qc->error_reason);
> -        frame->u.close.reason.data = (u_char *) qc->error_reason;
> +        frame.u.close.reason.len = ngx_strlen(qc->error_reason);
> +        frame.u.close.reason.data = (u_char *) qc->error_reason;
>     }
> 
> -    ngx_quic_queue_frame(qc, frame);
> -
>     qc->last_cc = ngx_current_msec;
> 
> -    return ngx_quic_output(c);
> +    return ngx_quic_frame_sendto(c, &frame, 0, qc->path);
> }
> 

It should be fine as long as we don't have pending frames
neither on idle timeout (I expect we don't) nor on immediate close.
Immediate close (rc == NGX_OK) are close handler, stateless reset,
and receiving CONNECTION_CLOSE where we don't have to send anything.
Immediate close (rc == NGX_ERROR) is something we expect to be
sort of fatal errors where we should normally cease sending.
To sum up, I think the change is good.

-- 
Sergey Kandaurov