Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 1 of 2] QUIC: update stream flow control credit on STREAM_DATA_BLOCKED

Roman Arutyunyan
November 22, 2021 06:52AM
On Mon, Nov 22, 2021 at 11:21:37AM +0300, Vladimir Homutov wrote:
> On Wed, Nov 17, 2021 at 11:17:27AM +0300, Roman Arutyunyan wrote:
> > On Wed, Nov 17, 2021 at 10:31:01AM +0300, Roman Arutyunyan wrote:
> > > # HG changeset patch
> > > # User Roman Arutyunyan <arut@nginx.com>
> > > # Date 1637133234 -10800
> > > # Wed Nov 17 10:13:54 2021 +0300
> > > # Branch quic
> > > # Node ID 4e3a7fc0533192f51a01042a1e9dd2b595881420
> > > # Parent 4ad8fc79cb33257c928a9098a87324b350576551
> > > QUIC: update stream flow control credit on STREAM_DATA_BLOCKED.
> > >
> > > Previously, after receiving STREAM_DATA_BLOCKED, current flow control limit
> > > was sent to client. Now, if the limit can be updated to the full window size,
> > > it is updated and the new value is sent to client, otherwise nothing is sent.
> > >
> > > The change lets client update flow control credit on demand. Also, it saves
> > > traffic by not sending MAX_STREAM_DATA with the same value twice.
> > >
> > > diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
> > > --- a/src/event/quic/ngx_event_quic_streams.c
> > > +++ b/src/event/quic/ngx_event_quic_streams.c
> > > @@ -31,6 +31,7 @@ static size_t ngx_quic_max_stream_flow(n
> > > static void ngx_quic_stream_cleanup_handler(void *data);
> > > static ngx_int_t ngx_quic_control_flow(ngx_connection_t *c, uint64_t last);
> > > static ngx_int_t ngx_quic_update_flow(ngx_connection_t *c, uint64_t last);
> > > +static ngx_int_t ngx_quic_update_max_stream_data(ngx_connection_t *c);
> > >
> > >
> > > ngx_connection_t *
> > > @@ -1190,8 +1191,6 @@ ngx_int_t
> > > ngx_quic_handle_stream_data_blocked_frame(ngx_connection_t *c,
> > > ngx_quic_header_t *pkt, ngx_quic_stream_data_blocked_frame_t *f)
> > > {
> > > - uint64_t limit;
> > > - ngx_quic_frame_t *frame;
> > > ngx_quic_stream_t *qs;
> > > ngx_quic_connection_t *qc;
> > >
> > > @@ -1217,29 +1216,10 @@ ngx_quic_handle_stream_data_blocked_fram
> > > return NGX_OK;
> > > }
> > >
> > > - limit = qs->recv_max_data;
> > > -
> > > - if (ngx_quic_init_stream(qs) != NGX_OK) {
> > > - return NGX_ERROR;
> > > - }
> > > -
> > > - } else {
> > > - limit = qs->recv_max_data;
> > > + return ngx_quic_init_stream(qs);
> > > }
> > >
> > > - frame = ngx_quic_alloc_frame(c);
> > > - if (frame == NULL) {
> > > - return NGX_ERROR;
> > > - }
> > > -
> > > - frame->level = pkt->level;
> > > - frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
> > > - frame->u.max_stream_data.id = f->id;
> > > - frame->u.max_stream_data.limit = limit;
> > > -
> > > - ngx_quic_queue_frame(qc, frame);
> > > -
> > > - return NGX_OK;
> > > + return ngx_quic_update_max_stream_data(qs->connection);
> > > }
> > >
> > >
> > > @@ -1587,22 +1567,9 @@ ngx_quic_update_flow(ngx_connection_t *c
> > > if (!rev->pending_eof && !rev->error
> > > && qs->recv_max_data <= qs->recv_offset + qs->recv_window / 2)
> > > {
> > > - qs->recv_max_data = qs->recv_offset + qs->recv_window;
> > > -
> > > - ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
> > > - "quic flow update msd:%uL", qs->recv_max_data);
> > > -
> > > - frame = ngx_quic_alloc_frame(pc);
> > > - if (frame == NULL) {
> > > + if (ngx_quic_update_max_stream_data(c) != NGX_OK) {
> > > return NGX_ERROR;
> > > }
> > > -
> > > - frame->level = ssl_encryption_application;
> > > - frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
> > > - frame->u.max_stream_data.id = qs->id;
> > > - frame->u.max_stream_data.limit = qs->recv_max_data;
> > > -
> > > - ngx_quic_queue_frame(qc, frame);
> > > }
> > >
> > > qc->streams.recv_offset += len;
> > > @@ -1632,6 +1599,44 @@ ngx_quic_update_flow(ngx_connection_t *c
> > > }
> > >
> > >
> > > +static ngx_int_t
> > > +ngx_quic_update_max_stream_data(ngx_connection_t *c)
> > > +{
> > > + uint64_t recv_max_data;
> > > + ngx_quic_frame_t *frame;
> > > + ngx_quic_stream_t *qs;
> > > + ngx_quic_connection_t *qc;
> > > +
> > > + qs = c->quic;
> > > + qc = ngx_quic_get_connection(qs->parent);
> > > +
> > > + recv_max_data = qs->recv_offset + qs->recv_window;
> > > +
> > > + if (qs->recv_max_data == recv_max_data) {
>
> shouldn't it be >= ? (i.e. we want to avoid sending frame if current
> window doesn't extend recv_max_data; could qs->recv_window change ?)

It can't be larger than that. Initially qs->recv_max_data equals
qs->recv_window and then qs->recv_offset grows. And no, qs->recv_window
never changes.

> > > + return NGX_OK;
> > > + }
> > > +
> > > + qs->recv_max_data = recv_max_data;
> > > +
> > > + ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
> > > + "quic flow update msd:%uL", qs->recv_max_data);
> > > +
> > > + frame = ngx_quic_alloc_frame(c);
> >
> > The argument should be "pc":
> >
> > frame = ngx_quic_alloc_frame(pc);
>
> also, it need to be declared/initialized, similar to other places

Sure, my bad.

> > > + if (frame == NULL) {
> > > + return NGX_ERROR;
> > > + }
> > > +
> > > + frame->level = ssl_encryption_application;
> > > + frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
> > > + frame->u.max_stream_data.id = qs->id;
> > > + frame->u.max_stream_data.limit = qs->recv_max_data;
> > > +
> > > + ngx_quic_queue_frame(qc, frame);
> > > +
> > > + return NGX_OK;
> > > +}
> > > +
> > > +
> > > ngx_int_t
> > > ngx_quic_handle_read_event(ngx_event_t *rev, ngx_uint_t flags)
> > > {
> > > _______________________________________________
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Roman Arutyunyan
# HG changeset patch
# User Roman Arutyunyan <arut@nginx.com>
# Date 1637179658 -10800
# Wed Nov 17 23:07:38 2021 +0300
# Branch quic
# Node ID a316d316a3356de408f77e8c63b42fdc315e926e
# Parent 7a14f995dcf71ade5b896d19a9db3098fb5bceac
QUIC: update stream flow control credit on STREAM_DATA_BLOCKED.

Previously, after receiving STREAM_DATA_BLOCKED, current flow control limit
was sent to client. Now, if the limit can be updated to the full window size,
it is updated and the new value is sent to client, otherwise nothing is sent.

The change lets client update flow control credit on demand. Also, it saves
traffic by not sending MAX_STREAM_DATA with the same value twice.

diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
--- a/src/event/quic/ngx_event_quic_streams.c
+++ b/src/event/quic/ngx_event_quic_streams.c
@@ -31,6 +31,7 @@ static size_t ngx_quic_max_stream_flow(n
static void ngx_quic_stream_cleanup_handler(void *data);
static ngx_int_t ngx_quic_control_flow(ngx_connection_t *c, uint64_t last);
static ngx_int_t ngx_quic_update_flow(ngx_connection_t *c, uint64_t last);
+static ngx_int_t ngx_quic_update_max_stream_data(ngx_connection_t *c);


ngx_connection_t *
@@ -1189,8 +1190,6 @@ ngx_int_t
ngx_quic_handle_stream_data_blocked_frame(ngx_connection_t *c,
ngx_quic_header_t *pkt, ngx_quic_stream_data_blocked_frame_t *f)
{
- uint64_t limit;
- ngx_quic_frame_t *frame;
ngx_quic_stream_t *qs;
ngx_quic_connection_t *qc;

@@ -1216,29 +1215,10 @@ ngx_quic_handle_stream_data_blocked_fram
return NGX_OK;
}

- limit = qs->recv_max_data;
-
- if (ngx_quic_init_stream(qs) != NGX_OK) {
- return NGX_ERROR;
- }
-
- } else {
- limit = qs->recv_max_data;
+ return ngx_quic_init_stream(qs);
}

- frame = ngx_quic_alloc_frame(c);
- if (frame == NULL) {
- return NGX_ERROR;
- }
-
- frame->level = pkt->level;
- frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
- frame->u.max_stream_data.id = f->id;
- frame->u.max_stream_data.limit = limit;
-
- ngx_quic_queue_frame(qc, frame);
-
- return NGX_OK;
+ return ngx_quic_update_max_stream_data(qs->connection);
}


@@ -1586,22 +1566,9 @@ ngx_quic_update_flow(ngx_connection_t *c
if (!rev->pending_eof && !rev->error
&& qs->recv_max_data <= qs->recv_offset + qs->recv_window / 2)
{
- qs->recv_max_data = qs->recv_offset + qs->recv_window;
-
- ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
- "quic flow update msd:%uL", qs->recv_max_data);
-
- frame = ngx_quic_alloc_frame(pc);
- if (frame == NULL) {
+ if (ngx_quic_update_max_stream_data(c) != NGX_OK) {
return NGX_ERROR;
}
-
- frame->level = ssl_encryption_application;
- frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
- frame->u.max_stream_data.id = qs->id;
- frame->u.max_stream_data.limit = qs->recv_max_data;
-
- ngx_quic_queue_frame(qc, frame);
}

qc->streams.recv_offset += len;
@@ -1631,6 +1598,46 @@ ngx_quic_update_flow(ngx_connection_t *c
}


+static ngx_int_t
+ngx_quic_update_max_stream_data(ngx_connection_t *c)
+{
+ uint64_t recv_max_data;
+ ngx_connection_t *pc;
+ ngx_quic_frame_t *frame;
+ ngx_quic_stream_t *qs;
+ ngx_quic_connection_t *qc;
+
+ qs = c->quic;
+ pc = qs->parent;
+ qc = ngx_quic_get_connection(pc);
+
+ recv_max_data = qs->recv_offset + qs->recv_window;
+
+ if (qs->recv_max_data == recv_max_data) {
+ return NGX_OK;
+ }
+
+ qs->recv_max_data = recv_max_data;
+
+ ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
+ "quic flow update msd:%uL", qs->recv_max_data);
+
+ frame = ngx_quic_alloc_frame(pc);
+ if (frame == NULL) {
+ return NGX_ERROR;
+ }
+
+ frame->level = ssl_encryption_application;
+ frame->type = NGX_QUIC_FT_MAX_STREAM_DATA;
+ frame->u.max_stream_data.id = qs->id;
+ frame->u.max_stream_data.limit = qs->recv_max_data;
+
+ ngx_quic_queue_frame(qc, frame);
+
+ return NGX_OK;
+}
+
+
ngx_int_t
ngx_quic_handle_read_event(ngx_event_t *rev, ngx_uint_t flags)
{
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 2] QUIC flow control update

Roman Arutyunyan 486 November 17, 2021 02:32AM

[PATCH 1 of 2] QUIC: update stream flow control credit on STREAM_DATA_BLOCKED

Roman Arutyunyan 202 November 17, 2021 02:32AM

Re: [PATCH 1 of 2] QUIC: update stream flow control credit on STREAM_DATA_BLOCKED

Roman Arutyunyan 308 November 17, 2021 03:22AM

Re: [PATCH 1 of 2] QUIC: update stream flow control credit on STREAM_DATA_BLOCKED

Vladimir Homutov 196 November 22, 2021 03:22AM

Re: [PATCH 1 of 2] QUIC: update stream flow control credit on STREAM_DATA_BLOCKED

Roman Arutyunyan 214 November 22, 2021 06:52AM

[PATCH 2 of 2] QUIC: handle DATA_BLOCKED frame from client

Roman Arutyunyan 331 November 17, 2021 02:32AM

Re: [PATCH 2 of 2] QUIC: handle DATA_BLOCKED frame from client

Vladimir Homutov 192 November 22, 2021 03:28AM

Re: [PATCH 2 of 2] QUIC: handle DATA_BLOCKED frame from client

Roman Arutyunyan 180 November 22, 2021 06:56AM

Re: [PATCH 2 of 2] QUIC: handle DATA_BLOCKED frame from client

Vladimir Homutov 250 November 22, 2021 07:02AM



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

Online Users

Guests: 177
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready