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

[Vladimir Homutov]

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 at 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)
> >  {
> > _______________________________________________