Skip to content

Commit

Permalink
QUIC: fixed probe-congestion deadlock.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
arut committed Aug 14, 2023
1 parent 842a930 commit 4f3707c
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 56 deletions.
62 changes: 11 additions & 51 deletions src/event/quic/ngx_event_quic_ack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 */)
{
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);
}
ngx_memzero(&frame, sizeof(ngx_quic_frame_t));

q = next;
}
frame.level = ctx->level;
frame.type = NGX_QUIC_FT_PING;

for (q = ngx_queue_head(&ctx->sent);
q != ngx_queue_sentinel(&ctx->sent);
/* void */)
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_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;
ngx_quic_close_connection(c, NGX_ERROR);
return;
}

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);
}

Expand Down
4 changes: 0 additions & 4 deletions src/event/quic/ngx_event_quic_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion src/event/quic/ngx_event_quic_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 4f3707c

Please sign in to comment.