On Wed, Feb 02, 2022 at 03:05:07PM +0300, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1643803485 -10800
> # Wed Feb 02 15:04:45 2022 +0300
> # Branch quic
> # Node ID 768445d1ba6e2bce9001704c52b516ad421ae776
> # Parent cd8018bc81a52ca7de2eb4e779dfd574c8a661a2
> QUIC: do not arm loss detection timer if nothing was sent.
>
> Notably, this became quite practicable after the recent fix in cd8018bc81a5.
>
> 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
> @@ -109,7 +109,9 @@ ngx_quic_output(ngx_connection_t *c)
> ngx_add_timer(c->read, qc->tp.max_idle_timeout);
> }
>
> - ngx_quic_set_lost_timer(c);
> + if (in_flight != cg->in_flight) {
> + ngx_quic_set_lost_timer(c);
> + }
>
> return NGX_OK;
> }
>

Instead of adding one more check, I would invert condition and test if
we need to set any timers first, and then arm whatever needed;
This would simplify conditions and make logic simpler;
i.e. something like:


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
@@ -104,7 +104,12 @@ ngx_quic_output(ngx_connection_t *c)
return NGX_ERROR;
}

- if (in_flight != cg->in_flight && !qc->send_timer_set && !qc->closing) {
+ if (in_flight == cg->in_flight || qc->closing) {
+ /* no ack-eliciting data was sent or we are done */
+ return NGX_OK;
+ }
+
+ if (!qc->send_timer_set) {
qc->send_timer_set = 1;
ngx_add_timer(c->read, qc->tp.max_idle_timeout);
}

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org