On Thu, Nov 11, 2021 at 06:17:43PM +0300, Sergey Kandaurov wrote:
>
> > On 21 Oct 2021, at 16:40, Roman Arutyunyan <arut@nginx.com> wrote:
> >
> > # HG changeset patch
> > # User Roman Arutyunyan <arut@nginx.com>
> > # Date 1634219818 -10800
> > # Thu Oct 14 16:56:58 2021 +0300
> > # Branch quic
> > # Node ID 8b049432ef2dcdb8d1a8ec1a5e41c0a340285b65
> > # Parent 404de224517e33f685613d6425dcdb3c8ef5b97e
> > QUIC: allowed main QUIC connection for some operations.
> >
> > Operations like ngx_quic_open_stream(), ngx_http_quic_get_connection(),
> > ngx_http_v3_finalize_connection(), ngx_http_v3_shutdown_connection() used to
> > receive a QUIC stream connection. Now they can receive the main QUIC
> > connection as well. This is useful when calling them out of a stream context.
> >
> > 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
> > @@ -35,11 +35,12 @@ ngx_connection_t *
> > ngx_quic_open_stream(ngx_connection_t *c, ngx_uint_t bidi)
> > {
> > uint64_t id;
> > - ngx_quic_stream_t *qs, *nqs;
> > + ngx_connection_t *pc;
> > + ngx_quic_stream_t *nqs;
> > ngx_quic_connection_t *qc;
> >
> > - qs = c->quic;
> > - qc = ngx_quic_get_connection(qs->parent);
> > + pc = c->quic ? c->quic->parent : c;
> > + qc = ngx_quic_get_connection(pc);
>
> is it really needed?

ngx_http_v3_inc_insert_count_handler() calls
ngx_http_v3_send_inc_insert_count(), which calls ngx_http_v3_get_uni_stream(),
which may call ngx_quic_open_stream(). All these calls are made in the context
of parent QUIC connection. So we need ngx_quic_open_stream() to be able to
operate on it.

Maybe it's even better to support only parent QUIC connection in
ngx_quic_open_stream() and drop stream support.

> > if (bidi) {
> > if (qc->streams.server_streams_bidi
> > @@ -85,7 +86,7 @@ ngx_quic_open_stream(ngx_connection_t *c
> > qc->streams.server_streams_uni++;
> > }
> >
> > - nqs = ngx_quic_create_stream(qs->parent, id);
> > + nqs = ngx_quic_create_stream(pc, id);
> > if (nqs == NULL) {
> > return NULL;
> > }
> > diff --git a/src/http/modules/ngx_http_quic_module.h b/src/http/modules/ngx_http_quic_module.h
> > --- a/src/http/modules/ngx_http_quic_module.h
> > +++ b/src/http/modules/ngx_http_quic_module.h
> > @@ -19,7 +19,8 @@
> >
> >
> > #define ngx_http_quic_get_connection(c) \
> > - ((ngx_http_connection_t *) (c)->quic->parent->data)
> > + ((ngx_http_connection_t *) ((c)->quic ? (c)->quic->parent->data \
> > + : (c)->data))
> >
>
> Looks like this is the only change useful in the 2nd patch.
> To avoid it, ngx_http_v3_inc_insert_count_handler() could get h3c from
> the decoder stream, but ngx_http_v3_get_uni_stream() isn't external.

ngx_http_v3_get_uni_stream() calls ngx_http_v3_get_session(), which
calls ngx_http_quic_get_connection() anyway.

> > ngx_int_t ngx_http_quic_init(ngx_connection_t *c);
> > diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c
> > --- a/src/http/v3/ngx_http_v3.c
> > +++ b/src/http/v3/ngx_http_v3.c
> > @@ -70,8 +70,8 @@ ngx_http_v3_keepalive_handler(ngx_event_
> >
> > ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 keepalive handler");
> >
> > - ngx_quic_finalize_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> > - "keepalive timeout");
> > + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> > + "keepalive timeout");
> > }
> >
> >
> > diff --git a/src/http/v3/ngx_http_v3.h b/src/http/v3/ngx_http_v3.h
> > --- a/src/http/v3/ngx_http_v3.h
> > +++ b/src/http/v3/ngx_http_v3.h
> > @@ -85,10 +85,12 @@
> > module)
> >
> > #define ngx_http_v3_finalize_connection(c, code, reason) \
> > - ngx_quic_finalize_connection(c->quic->parent, code, reason)
> > + ngx_quic_finalize_connection((c)->quic ? (c)->quic->parent : (c), \
> > + code, reason)
> >
> > #define ngx_http_v3_shutdown_connection(c, code, reason) \
> > - ngx_quic_shutdown_connection(c->quic->parent, code, reason)
> > + ngx_quic_shutdown_connection((c)->quic ? (c)->quic->parent : (c), \
> > + code, reason)

Maybe we need to eliminate these at all.

> --
> Sergey Kandaurov
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel