Welcome! Log In Create A New Profile

Advanced

Re: [QUIC] padding of Initial packets

Vladimir Homutov
February 09, 2022 08:38AM
On Tue, Feb 08, 2022 at 03:42:54PM +0300, Vladimir Homutov wrote:
> On Tue, Feb 08, 2022 at 02:10:04PM +0300, Andrey Kolyshkin wrote:
> > Hello.
> >
> > This patch is strange.
> > 1. ngx_quic_revert_send can set to ctx an uninitialized value from
> > preserved_pnum. (example if min > len and i = 0, only 0 element is filled
> > in preserved_pnum but restored all)
> > 2. ngx_quic_revert_send will restored pnum for ctx that have already called
> > ngx_quic_output_packet and the packet with this pnum will be queued.
> > (example if min > len and i = 1)
>
> thank you for noticing.
> indeed, this needs to be fixed. we don't want to restore contexts we
> didn't yet touch.


The suggested fix is below. Also, while investigating the issue
thoroughly, we found that it is also possible to run into negative ctx->inflight
when discarding context. This is addressed by a second patch.

# HG changeset patch
# User Vladimir Homutov <vl@nginx.com>
# Date 1644411201 -10800
# Wed Feb 09 15:53:21 2022 +0300
# Branch quic
# Node ID a4fb28741e19af426228e64b8d2c02ed3950b538
# Parent dde5cb0205ef8c2a2a3255e7bd369a9c644f2049
QUIC: fixed output context restoring.

The cd8018bc81a5 fixed unintended send of non-padded initial packets,
but failed to restore context properly: only processed contexts need
to be restored. As a consequence, a packet number could be restored
from uninitialized value.

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
@@ -165,7 +165,7 @@ ngx_quic_create_datagrams(ngx_connection
if (min > len) {
/* padding can't be applied - avoid sending the packet */

- for (i = 0; i < NGX_QUIC_SEND_CTX_LAST; i++) {
+ while (i-- > 0) {
ctx = &qc->send_ctx[i];
ngx_quic_revert_send(c, ctx, preserved_pnum[i]);
}


# HG changeset patch
# User Vladimir Homutov <vl@nginx.com>
# Date 1644411102 -10800
# Wed Feb 09 15:51:42 2022 +0300
# Branch quic
# Node ID 2e27c45e2edb2c9540b211040d314b1748865820
# Parent a4fb28741e19af426228e64b8d2c02ed3950b538
QUIC: fixed in-flight bytes accounting.

Initially, frames are genereated and stored in ctx->frames.
Next, ngx_quic_output() collects frames to be sent in in ctx->sending.
On failure, ngx_quic_revert_sned() returns frames into ctx->frames.

On success, the ngx_quic_commit_send() moves ack-eliciting frames into
ctx->sent and frees non-ack-eliciting frames.
This function also updates in-flight bytes counter, so only actually sent
frames are accounted.

The counter is decremented in the following cases:
- acknowledgment is received
- packet was declared lost
- we are discarding context completely

In each of this cases frame is removed from ctx->sent queue and in-flight
counter is accordingly decremented.

The patch fixes the case of discarding context - only removing frames
from ctx->sent must be followed by in-flight bytes counter decrement,
otherwise cg->in_flight could experience type underflow.

The issue appeared in b1676cd64dc9.

diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
--- a/src/event/quic/ngx_event_quic.c
+++ b/src/event/quic/ngx_event_quic.c
@@ -1092,7 +1092,6 @@ ngx_quic_discard_ctx(ngx_connection_t *c
ngx_queue_remove(q);

f = ngx_queue_data(q, ngx_quic_frame_t, queue);
- ngx_quic_congestion_ack(c, f);
ngx_quic_free_frame(c, f);
}

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

[QUIC] padding of Initial packets

Vladimir Homutov 281 February 02, 2022 05:58AM

Re: [QUIC] padding of Initial packets

Sergey Kandaurov 84 February 02, 2022 06:10AM

Re: [QUIC] padding of Initial packets

Andrey Kolyshkin 47 February 08, 2022 06:12AM

Re: [QUIC] padding of Initial packets

Vladimir Homutov 42 February 08, 2022 07:46AM

Re: [QUIC] padding of Initial packets

Vladimir Homutov 104 February 09, 2022 08:38AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 125
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready