[PATCH 5 of 5] QUIC: ignore blocked status in congestion event handlers

[Sergey Kandaurov]

> On 1 Aug 2023, at 11:45, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1690874694 -14400
> #      Tue Aug 01 11:24:54 2023 +0400
> # Node ID 80df0852e7ed58631025398694da6dd4dab42611
> # Parent  cd0ef56b0f1afaa54d7d2756dad2182628445e04
> QUIC: ignore blocked status in congestion event handlers.
> 
> Sometimes, while congestion window allows to send more bytes, the next frame
> still cannot be sent since it's too big.  When this happens, push event is not
> triggered from congestion ack/loss event handlers which may delay packet send.
> 
> Now the blocked status is ignored and push event is always posted when
> congestion window grows bigger.

It would be nice to provide an example.
In my tests of a simple download without packet lost, I don't see that
additionally posted push events do any useful work, while the number
of posted events raised 2x.  Further, I failed to see how the change
would help, because push events are posted then directly from
ngx_quic_handle_ack_frame_range() and ngx_quic_resend_frames().

> 
> diff --git a/src/event/quic/ngx_event_quic_ack.c b/src/event/quic/ngx_event_quic_ack.c
> --- a/src/event/quic/ngx_event_quic_ack.c
> +++ b/src/event/quic/ngx_event_quic_ack.c
> @@ -307,7 +307,6 @@ ngx_quic_handle_ack_frame_range(ngx_conn
> void
> ngx_quic_congestion_ack(ngx_connection_t *c, ngx_quic_frame_t *f)
> {
> -    ngx_uint_t              blocked;
>     ngx_msec_t              timer;
>     ngx_quic_congestion_t  *cg;
>     ngx_quic_connection_t  *qc;
> @@ -319,8 +318,6 @@ ngx_quic_congestion_ack(ngx_connection_t
>     qc = ngx_quic_get_connection(c);
>     cg = &qc->congestion;
> 
> -    blocked = (cg->in_flight >= cg->window) ? 1 : 0;
> -
>     cg->in_flight -= f->plen;
> 
>     timer = f->last - cg->recovery_start;
> @@ -358,7 +355,7 @@ ngx_quic_congestion_ack(ngx_connection_t
> 
> done:
> 
> -    if (blocked && cg->in_flight < cg->window) {
> +    if (cg->in_flight < cg->window) {
>         ngx_post_event(&qc->push, &ngx_posted_events);
>     }
> }
> @@ -648,7 +645,6 @@ ngx_quic_resend_frames(ngx_connection_t 
> static void
> ngx_quic_congestion_lost(ngx_connection_t *c, ngx_quic_frame_t *f)
> {
> -    ngx_uint_t              blocked;
>     ngx_msec_t              timer;
>     ngx_quic_congestion_t  *cg;
>     ngx_quic_connection_t  *qc;
> @@ -660,8 +656,6 @@ ngx_quic_congestion_lost(ngx_connection_
>     qc = ngx_quic_get_connection(c);
>     cg = &qc->congestion;
> 
> -    blocked = (cg->in_flight >= cg->window) ? 1 : 0;
> -
>     cg->in_flight -= f->plen;
>     f->plen = 0;
> 
> @@ -690,7 +684,7 @@ ngx_quic_congestion_lost(ngx_connection_
> 
> done:
> 
> -    if (blocked && cg->in_flight < cg->window) {
> +    if (cg->in_flight < cg->window) {
>         ngx_post_event(&qc->push, &ngx_posted_events);
>     }
> }
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

-- 
Sergey Kandaurov