From 4f3707c5c775ac83a0cf005617e1fdffd027b8a4 Mon Sep 17 00:00:00 2001 From: Roman Arutyunyan Date: Mon, 14 Aug 2023 08:28:30 +0400 Subject: [PATCH] QUIC: fixed probe-congestion deadlock. When probe timeout expired while congestion window was exhausted, probe PINGs could not be sent. As a result, lost packets could not be declared lost and congestion window could not be freed for new packets. This deadlock continued until connection idle timeout expiration. Now PINGs are sent separately from the frame queue without congestion control, as specified by RFC 9002, Section 7: An endpoint MUST NOT send a packet if it would cause bytes_in_flight (see Appendix B.2) to be larger than the congestion window, unless the packet is sent on a PTO timer expiration (see Section 6.2) or when entering recovery (see Section 7.3.2). --- src/event/quic/ngx_event_quic_ack.c | 66 +++++------------------ src/event/quic/ngx_event_quic_output.c | 4 -- src/event/quic/ngx_event_quic_transport.h | 1 - 3 files changed, 13 insertions(+), 58 deletions(-) diff --git a/src/event/quic/ngx_event_quic_ack.c b/src/event/quic/ngx_event_quic_ack.c index 04fc66760..182246568 100644 --- a/src/event/quic/ngx_event_quic_ack.c +++ b/src/event/quic/ngx_event_quic_ack.c @@ -820,9 +820,9 @@ ngx_quic_pto_handler(ngx_event_t *ev) { ngx_uint_t i; ngx_msec_t now; - ngx_queue_t *q, *next; + ngx_queue_t *q; ngx_connection_t *c; - ngx_quic_frame_t *f; + ngx_quic_frame_t *f, frame; ngx_quic_send_ctx_t *ctx; ngx_quic_connection_t *qc; @@ -859,63 +859,23 @@ ngx_quic_pto_handler(ngx_event_t *ev) "quic pto %s pto_count:%ui", ngx_quic_level_name(ctx->level), qc->pto_count); - for (q = ngx_queue_head(&ctx->frames); - q != ngx_queue_sentinel(&ctx->frames); - /* void */) + ngx_memzero(&frame, sizeof(ngx_quic_frame_t)); + + frame.level = ctx->level; + frame.type = NGX_QUIC_FT_PING; + + if (ngx_quic_frame_sendto(c, &frame, 0, qc->path) != NGX_OK + || ngx_quic_frame_sendto(c, &frame, 0, qc->path) != NGX_OK) { - next = ngx_queue_next(q); - f = ngx_queue_data(q, ngx_quic_frame_t, queue); - - if (f->type == NGX_QUIC_FT_PING) { - ngx_queue_remove(q); - ngx_quic_free_frame(c, f); - } - - q = next; + ngx_quic_close_connection(c, NGX_ERROR); + return; } - - for (q = ngx_queue_head(&ctx->sent); - q != ngx_queue_sentinel(&ctx->sent); - /* void */) - { - next = ngx_queue_next(q); - f = ngx_queue_data(q, ngx_quic_frame_t, queue); - - if (f->type == NGX_QUIC_FT_PING) { - ngx_quic_congestion_lost(c, f); - ngx_queue_remove(q); - ngx_quic_free_frame(c, f); - } - - q = next; - } - - /* enforce 2 udp datagrams */ - - f = ngx_quic_alloc_frame(c); - if (f == NULL) { - break; - } - - f->level = ctx->level; - f->type = NGX_QUIC_FT_PING; - f->flush = 1; - - ngx_quic_queue_frame(qc, f); - - f = ngx_quic_alloc_frame(c); - if (f == NULL) { - break; - } - - f->level = ctx->level; - f->type = NGX_QUIC_FT_PING; - - ngx_quic_queue_frame(qc, f); } qc->pto_count++; + ngx_quic_set_lost_timer(c); + ngx_quic_connstate_dbg(c); } diff --git a/src/event/quic/ngx_event_quic_output.c b/src/event/quic/ngx_event_quic_output.c index 37de44c74..ecd5c2cef 100644 --- a/src/event/quic/ngx_event_quic_output.c +++ b/src/event/quic/ngx_event_quic_output.c @@ -631,10 +631,6 @@ ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx, f->plen = 0; nframes++; - - if (f->flush) { - break; - } } if (nframes == 0) { diff --git a/src/event/quic/ngx_event_quic_transport.h b/src/event/quic/ngx_event_quic_transport.h index 16d9095ef..02e44650b 100644 --- a/src/event/quic/ngx_event_quic_transport.h +++ b/src/event/quic/ngx_event_quic_transport.h @@ -271,7 +271,6 @@ struct ngx_quic_frame_s { ssize_t len; unsigned need_ack:1; unsigned pkt_need_ack:1; - unsigned flush:1; ngx_chain_t *data; union {