Repository: nginx/nginx
File: src/event/quic/ngx_event_quic_output.c (C)

Code before:
    min_payload = ngx_quic_payload_size(&pkt, min);
    max_payload = ngx_quic_payload_size(&pkt, max);

    /* RFC 9001, 5.4.2.  Header Protection Sample */
    pad = 4 - pkt.num_len;
    min_payload = ngx_max(min_payload, pad);

    if (min_payload > max_payload) {
        return 0;
    }

    now = ngx_current_msec;
    nframes = 0;
    p = src;
    len = 0;

    for (q = ngx_queue_head(&ctx->frames);
         q != ngx_queue_sentinel(&ctx->frames);
         q = ngx_queue_next(q))
    {
        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;
        }

        if (len + f->len > max_payload) {
            rc = ngx_quic_split_frame(c, f, max_payload - len);

            if (rc == NGX_ERROR) {
                return NGX_ERROR;
            }

            if (rc == NGX_DECLINED) {
                break;
            }
        }

        if (f->need_ack) {
            pkt.need_ack = 1;
        }

        f->pnum = ctx->pnum;
        f->send_time = now;
        f->plen = 0;

        ngx_quic_log_frame(c->log, f, 1);

Review comment (bug):
`ngx_quic_generate_ack()` always adds ACKs to the head of `ctx->frames`. Then we always* send those ACKs irrespective of congestion window. Then why are we searching for ACKs anywhere beyond the head of the list? Shouldn't it be like this?
```
        if (ack_only && f->type != NGX_QUIC_FT_ACK) {
            break;
        }
```
The following changes become unnecessary in this case (the original patch by Vladimir does not have them BTW).

Overall searching for ACKs till the end of the list introduces linear complexity which we did not have before and it would be nice to avoid it.

*there's one exception though, when we hit anti-amplification limit. It's the very beginning of the connection and this ACK deadlock should not happen at this time.

Code after:
    min_payload = ngx_quic_payload_size(&pkt, min);
    max_payload = ngx_quic_payload_size(&pkt, max);

    /* RFC 9001, 5.4.2.  Header Protection Sample */
    pad = 4 - pkt.num_len;
    min_payload = ngx_max(min_payload, pad);

    if (min_payload > max_payload) {
        return 0;
    }

    now = ngx_current_msec;
    nframes = 0;
    p = src;
    len = 0;

    for (q = ngx_queue_head(&ctx->frames);
         q != ngx_queue_sentinel(&ctx->frames);
         q = ngx_queue_next(q))
    {
        f = ngx_queue_data(q, ngx_quic_frame_t, queue);

        if (ack_only && f->type != NGX_QUIC_FT_ACK) {
            break;
        }

        if (len >= max_payload) {
            break;
        }

        if (len + f->len > max_payload) {
            rc = ngx_quic_split_frame(c, f, max_payload - len);

            if (rc == NGX_ERROR) {
                return NGX_ERROR;
            }

            if (rc == NGX_DECLINED) {
                break;
            }
        }

        if (f->need_ack) {
            pkt.need_ack = 1;
        }

        f->pnum = ctx->pnum;
        f->send_time = now;
        f->plen = 0;

        ngx_quic_log_frame(c->log, f, 1);