[nginx] QUIC: do not increase underutilized congestion window.

[Vladimir Homutov]

On Fri, Apr 25, 2025 at 10:51:12PM +0400, Sergey Kandaurov wrote:
> On Fri, Apr 25, 2025 at 01:55:11PM +0300, Vladimir Homutov via nginx-devel wrote:
> > missing attachments
> >
>
> Thanks for the provided debug, it is really helpful.
>
> Looking into debug, it becomes clear that once we sent something
> (be it a large response or MTU probe) that temporarily exceeds
> the congestion window (which is permitted for MTU probes),
> we can no longer send acknowledgments.
>
> This is especially visible if we don't receive ACKs for some reason,
> such as in your case,
> to decrease our inflight counter, so the connection becomes stalled.
>
> Below is a fix I have made without looking into your version
> (no offense, it is purely for aesthetic reasons).
>
> Although it appears to be quite similar to yours, it's somewhat less
> intrusive.  I tried not to break sending segments for exceeded
> congestion, as well as to send correct frames in the ack_only mode.

indeed, my patch misses the part about queue management, this is
required part.

I'm agree regarding segmentation - sending pure ACK is definitely not
the case for optimizations;

otherwise, looks good for me.

>
>
> diff --git a/src/event/quic/ngx_event_quic_output.c b/src/event/quic/ngx_event_quic_output.c
> index a92a539f3..3fc2091e0 100644
> --- a/src/event/quic/ngx_event_quic_output.c
> +++ b/src/event/quic/ngx_event_quic_output.c
> @@ -55,7 +55,8 @@ static ssize_t ngx_quic_send_segments(ngx_connection_t *c, u_char *buf,
>      size_t len, struct sockaddr *sockaddr, socklen_t socklen, size_t segment);
>  #endif
>  static ssize_t ngx_quic_output_packet(ngx_connection_t *c,
> -    ngx_quic_send_ctx_t *ctx, u_char *data, size_t max, size_t min);
> +    ngx_quic_send_ctx_t *ctx, u_char *data, size_t max, size_t min,
> +    ngx_int_t ack_only);
>  static void ngx_quic_init_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
>      ngx_quic_header_t *pkt, ngx_quic_path_t *path);
>  static ngx_uint_t ngx_quic_get_padding_level(ngx_connection_t *c);
> @@ -131,8 +132,7 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
>      ngx_memzero(preserved_pnum, sizeof(preserved_pnum));
>  #endif
>
> -    while (cg->in_flight < cg->window) {
> -
> +    do {
>          p = dst;
>
>          len = ngx_quic_path_limit(c, path, path->mtu);
> @@ -158,7 +158,8 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
>                  return NGX_OK;
>              }
>
> -            n = ngx_quic_output_packet(c, ctx, p, len, min);
> +            n = ngx_quic_output_packet(c, ctx, p, len, min,
> +                                       cg->in_flight >= cg->window);
>              if (n == NGX_ERROR) {
>                  return NGX_ERROR;
>              }
> @@ -187,7 +188,8 @@ ngx_quic_create_datagrams(ngx_connection_t *c)
>          ngx_quic_commit_send(c);
>
>          path->sent += len;
> -    }
> +
> +    } while (cg->in_flight < cg->window);
>
>      return NGX_OK;
>  }
> @@ -315,6 +317,10 @@ ngx_quic_allow_segmentation(ngx_connection_t *c)
>
>          bytes += f->len;
>
> +        if (qc->congestion->in_flight + bytes >= qc->congestion->window)) {
> +            return 0;
> +        }
> +
>          if (bytes > len * 3) {
>              /* require at least ~3 full packets to batch */
>              return 1;
> @@ -364,7 +370,7 @@ ngx_quic_create_segments(ngx_connection_t *c)
>
>          if (len && cg->in_flight + (p - dst) < cg->window) {
>
> -            n = ngx_quic_output_packet(c, ctx, p, len, len);
> +            n = ngx_quic_output_packet(c, ctx, p, len, len, 0);
>              if (n == NGX_ERROR) {
>                  return NGX_ERROR;
>              }
> @@ -521,7 +527,7 @@ ngx_quic_get_padding_level(ngx_connection_t *c)
>
>  static ssize_t
>  ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
> -    u_char *data, size_t max, size_t min)
> +    u_char *data, size_t max, size_t min, ngx_int_t ack_only)
>  {
>      size_t                  len, pad, min_payload, max_payload;
>      u_char                 *p;
> @@ -585,6 +591,10 @@ ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
>      {
>          f = ngx_queue_data(q, ngx_quic_frame_t, queue);
>
> +        if (ack_only && f->type != NGX_QUIC_FT_ACK) {
> +            continue;
> +        }
> +
>          if (len >= max_payload) {
>              break;
>          }
> @@ -651,14 +661,20 @@ ngx_quic_output_packet(ngx_connection_t *c, ngx_quic_send_ctx_t *ctx,
>          f->plen = res.len;
>      }
>
> -    while (nframes--) {
> +    while (nframes) {
>          q = ngx_queue_head(&ctx->frames);
>          f = ngx_queue_data(q, ngx_quic_frame_t, queue);
>
> +        if (ack_only && f->type != NGX_QUIC_FT_ACK) {
> +            continue;
> +        }
> +
>          f->pkt_need_ack = pkt.need_ack;
>
>          ngx_queue_remove(q);
>          ngx_queue_insert_tail(&ctx->sending, q);
> +
> +        nframes--;
>      }
>
>      return res.len;