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 ?)

> > + 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

>
> > + 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