[PATCH 1 of 3] QUIC: introduced explicit stream states

[Roman Arutyunyan]

On Mon, Jan 31, 2022 at 01:18:32PM +0300, Vladimir Homutov wrote:
> On Mon, Jan 31, 2022 at 10:34:06AM +0300, Roman Arutyunyan wrote:
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at nginx.com>
> > # Date 1643611562 -10800
> > #      Mon Jan 31 09:46:02 2022 +0300
> > # Branch quic
> > # Node ID 8dcb9908989401d750b14fe5dccf444a5485c23d
> > # Parent  81a3429db8b00ec9fc476d3687d1cd18088f3365
> > QUIC: introduced explicit stream states.
> >
> > This allows to eliminate the usage of stream connection event flags for tracking
> > stream state.
> >
> > diff --git a/src/event/quic/ngx_event_quic.h b/src/event/quic/ngx_event_quic.h
> > --- a/src/event/quic/ngx_event_quic.h
> > +++ b/src/event/quic/ngx_event_quic.h
> > @@ -28,6 +28,26 @@
> >  #define NGX_QUIC_STREAM_UNIDIRECTIONAL       0x02
> >
> >
> > +typedef enum {
> > +    NGX_QUIC_STREAM_SEND_READY = 0,
> > +    NGX_QUIC_STREAM_SEND_SEND,
> > +    NGX_QUIC_STREAM_SEND_DATA_SENT,
> > +    NGX_QUIC_STREAM_SEND_DATA_RECVD,
> > +    NGX_QUIC_STREAM_SEND_RESET_SENT,
> > +    NGX_QUIC_STREAM_SEND_RESET_RECVD
> > +} ngx_quic_stream_send_state_e;
> > +
> > +
> > +typedef enum {
> > +    NGX_QUIC_STREAM_RECV_RECV = 0,
> > +    NGX_QUIC_STREAM_RECV_SIZE_KNOWN,
> > +    NGX_QUIC_STREAM_RECV_DATA_RECVD,
> > +    NGX_QUIC_STREAM_RECV_DATA_READ,
> > +    NGX_QUIC_STREAM_RECV_RESET_RECVD,
> > +    NGX_QUIC_STREAM_RECV_RESET_READ
> > +} ngx_quic_stream_recv_state_e;
> > +
> > +
> >  typedef struct {
> >      ngx_ssl_t                 *ssl;
> >
> > @@ -66,6 +86,8 @@ struct ngx_quic_stream_s {
> >      ngx_chain_t               *in;
> >      ngx_chain_t               *out;
> >      ngx_uint_t                 cancelable;  /* unsigned  cancelable:1; */
> > +    ngx_quic_stream_send_state_e  send_state;
> > +    ngx_quic_stream_recv_state_e  recv_state;
> >  };
> 
> let's fix this little style incosistency in a separate patch by moving
> all struct stuff to the right.

OK, added a patch for this.

> [..]
> 
> > @@ -780,8 +764,23 @@ ngx_quic_stream_recv(ngx_connection_t *c
> >
> >      ngx_quic_free_chain(pc, in);
> >
> > -    if (qs->in == NULL) {
> > -        rev->ready = rev->pending_eof;
> > +    if (len == 0) {
> 
> this also covers the case when ngx_quic_stream_recv() is called
> with zero-length buffer. Not sure what semantic should be implemented.
> 
> man 2 read says:
> 
>     If count is zero, read() may detect the errors described below.  In
>     the absence of any errors, or if read() does not check for errors, a
>     read()  with a count of 0 returns zero and has no other effects.
> 
> i.e. if we have data in buffer, but we are called with zero, we should
> not change state probably and handle this case separately.

You're right, now handling this case separately.

> > +        rev->ready = 0;
> > +
> > +        if (qs->recv_state == NGX_QUIC_STREAM_RECV_SIZE_KNOWN
> > +            && qs->recv_offset == qs->final_size)
> > +        {
> > +            qs->recv_state = NGX_QUIC_STREAM_RECV_DATA_READ;
> > +        }
> > +
> > +        if (qs->recv_state == NGX_QUIC_STREAM_RECV_DATA_READ) {
> > +            rev->eof = 1;
> > +            return 0;
> > +        }
> > +
> > +        ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
> > +                       "quic stream id:0x%xL recv() not ready", qs->id);
> > +        return NGX_AGAIN;
> >      }
> >
> >      ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> 
> 
> side note:  while looking at state transitions, i've realized we never send
> STREAM_DATA_BLOCKED and DATA_BLOCKED.
> 
> _______________________________________________
> nginx-devel mailing list -- nginx-devel at nginx.org
> To unsubscribe send an email to nginx-devel-leave at nginx.org

-- 
Roman Arutyunyan