Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Roman Arutyunyan
September 13, 2023 12:04PM
Hi,

On Tue, Sep 12, 2023 at 05:05:02PM +0400, Sergey Kandaurov wrote:
>
> > On 11 Sep 2023, at 15:30, Roman Arutyunyan <arut@nginx.com> wrote:
> >
> > # HG changeset patch
> > # User Roman Arutyunyan <arut@nginx.com>
> > # Date 1694431436 -14400
> > # Mon Sep 11 15:23:56 2023 +0400
> > # Node ID 9ee4158b9d3fa41e647b772e707c29b3e4cb77b5
> > # Parent 65574b876c4047495ac8622c6161fc87c1b75913
> > HTTP/3: eliminated v3_session field from ngx_http_connection_t.
> >
> > The field was under NGX_HTTP_V3 macro, which was a source of binary
> > compatibility problems when nginx/module is build with/without HTTP/3 support.
> >
> > Now ngx_http_v3_session_t is assigned to c->data on streams initialization,
> > while ngx_http_connection_t object is referenced by http_connection field of
> > the session object, similar to ngx_http_v2_connection_t and ngx_http_request_t.
>
> I'd write explicitly (taking from the 1st patch commit log) that session
> creation is now postponed, to make the description self contained, YMMV.
>
> Note that this change will trigger segfaults on graceful shutdown
> if there are connections with handshake not yet complete,
> due to now postponed session creation.
> In this case, http3 shutdown() callback is called, used to send
> GOAWAY and proceed with connection close, but h3c is not yet.
> Probably calling the callback should depend on c->ssl->handshaked,
> because it has little sense if handshake isn't complete.

Thanks for noticing this. Indeed, hc is treated as h3c, which may lead to a
crash. The solution is to postpone calling shutdown() until after init().
I will add a separate patch for this. An alternative solution could be
to terminate the handshake since the worker is exiting anyway. However, as we
try to be as graceful as possible while shutting down the old worker, it's
better to finish the handshake and send GOAWAY after that.

> > diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
> > --- a/src/http/ngx_http_request.h
> > +++ b/src/http/ngx_http_request.h
> > @@ -324,10 +324,6 @@ typedef struct {
> > #endif
> > #endif
> >
> > -#if (NGX_HTTP_V3 || NGX_COMPAT)
> > - ngx_http_v3_session_t *v3_session;
> > -#endif
> > -
> > ngx_chain_t *busy;
> > ngx_int_t nbusy;
> >
> > 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
> > @@ -30,6 +30,8 @@ ngx_http_v3_init_session(ngx_connection_
> > goto failed;
> > }
> >
> > + h3c->http_connection = hc;
> > +
> > ngx_queue_init(&h3c->blocked);
> >
> > h3c->keepalive.log = c->log;
> > @@ -48,7 +50,7 @@ ngx_http_v3_init_session(ngx_connection_
> > cln->handler = ngx_http_v3_cleanup_session;
> > cln->data = h3c;
> >
> > - hc->v3_session = h3c;
> > + c->data = h3c;
> >
> > return NGX_OK;
> >
> > 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
> > @@ -78,11 +78,12 @@
> > #define NGX_HTTP_V3_ERR_DECODER_STREAM_ERROR 0x202
> >
> >
> > -#define ngx_http_quic_get_connection(c) \
> > - ((ngx_http_connection_t *) ((c)->quic ? (c)->quic->parent->data \
> > +#define ngx_http_v3_get_session(c) \
> > + ((ngx_http_v3_session_t *) ((c)->quic ? (c)->quic->parent->data \
> > : (c)->data))
> >
> > -#define ngx_http_v3_get_session(c) ngx_http_quic_get_connection(c)->v3_session
> > +#define ngx_http_quic_get_connection(c) \
> > + (ngx_http_v3_get_session(c)->http_connection)
> >
> > #define ngx_http_v3_get_module_loc_conf(c, module) \
> > ngx_http_get_module_loc_conf(ngx_http_quic_get_connection(c)->conf_ctx, \
> > @@ -120,6 +121,8 @@ struct ngx_http_v3_parse_s {
> >
> >
> > struct ngx_http_v3_session_s {
> > + ngx_http_connection_t *http_connection;
> > +
> > ngx_http_v3_dynamic_table_t table;
> >
> > ngx_event_t keepalive;
> > diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> > --- a/src/http/v3/ngx_http_v3_request.c
> > +++ b/src/http/v3/ngx_http_v3_request.c
> > @@ -70,11 +70,6 @@ ngx_http_v3_init_stream(ngx_connection_t
> > h3scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v3_module);
> >
> > if (c->quic == NULL) {
> > - if (ngx_http_v3_init_session(c) != NGX_OK) {
> > - ngx_http_close_connection(c);
> > - return;
> > - }
> > -
> > h3scf->quic.idle_timeout = clcf->keepalive_timeout;
> > ngx_quic_run(c, &h3scf->quic);
> > return;
> > @@ -112,6 +107,10 @@ ngx_http_v3_init(ngx_connection_t *c)
> >
> > ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 init");
> >
> > + if (ngx_http_v3_init_session(c) != NGX_OK) {
> > + return NGX_ERROR;
> > + }
> > +
> > h3c = ngx_http_v3_get_session(c);
> > clcf = ngx_http_v3_get_module_loc_conf(c, ngx_http_core_module);
> > ngx_add_timer(&h3c->keepalive, clcf->keepalive_timeout);
>
> --
> Sergey Kandaurov
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 3] QUIC module compatibility issues

Roman Arutyunyan 370 September 11, 2023 07:32AM

[PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 83 September 11, 2023 07:32AM

Re: [PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Sergey Kandaurov 85 September 12, 2023 09:06AM

Re: [PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 86 September 13, 2023 11:56AM

[PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Roman Arutyunyan 87 September 11, 2023 07:32AM

Re: [PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Sergey Kandaurov 83 September 12, 2023 09:06AM

Re: [PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Roman Arutyunyan 80 September 13, 2023 12:04PM

[PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 85 September 11, 2023 07:32AM

Re: [PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Sergey Kandaurov 89 September 12, 2023 09:06AM

Re: [PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 82 September 13, 2023 12:08PM

[PATCH 0 of 6] QUIC module compatibility issues

Roman Arutyunyan 96 September 14, 2023 06:18AM

[PATCH 1 of 6] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 110 September 14, 2023 06:18AM

[PATCH 2 of 6] HTTP/3: moved variable initialization

Roman Arutyunyan 94 September 14, 2023 06:18AM

[PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Roman Arutyunyan 88 September 14, 2023 06:18AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 87 September 19, 2023 06:00AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 93 September 19, 2023 06:34AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Roman Arutyunyan 90 September 21, 2023 11:42AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 93 September 22, 2023 07:24AM

[PATCH 4 of 6] HTTP/3: postponed session creation to init() callback

Roman Arutyunyan 86 September 14, 2023 06:18AM

[PATCH 5 of 6] QUIC: simplified setting close timer when closing connection

Roman Arutyunyan 89 September 14, 2023 06:18AM

[PATCH 6 of 6] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 77 September 14, 2023 06:18AM



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

Online Users

Guests: 336
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