Welcome! Log In Create A New Profile

Advanced

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

Roman Arutyunyan
September 13, 2023 11:56AM
On Tue, Sep 12, 2023 at 05:04:59PM +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 1694415935 -14400
> > # Mon Sep 11 11:05:35 2023 +0400
> > # Node ID 65574b876c4047495ac8622c6161fc87c1b75913
> > # Parent daf8f5ba23d8e9955b22782d945f9c065f4b6baa
> > QUIC: "handshake_timeout" configuration parameter.
> >
> > Previously QUIC did not have such parameter and handshake duration was
> > controlled by HTTP/3. However that required creating and storing HTTP/3
> > session while processing the first client datagram. Apparently there's
>
> s/while/before ?

I'd still keep the "while". However, since apparently it sounds confusing,
I have rephrased it a little.

> > no convenient way to store the session object before QUIC handshake. In
> > the following patch session creation will be postponed until first stream
> > creation.
> >
> > diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> > --- a/src/event/quic/ngx_event_quic.c
> > +++ b/src/event/quic/ngx_event_quic.c
> > @@ -213,6 +213,8 @@ ngx_quic_run(ngx_connection_t *c, ngx_qu
> > ngx_add_timer(c->read, qc->tp.max_idle_timeout);
> > ngx_quic_connstate_dbg(c);
> >
> > + ngx_add_timer(&qc->close, qc->conf->handshake_timeout);
> > +
>
> It makes sense to cap handshake_timeout to idle timeout.
> I.e. ngx_min(qc->conf->handshake_timeout, qc->tp.max_idle_timeout);
>
> Or additionally handle this in ngx_quic_close_connection(),
> by removing the timer if set for the NGX_DONE case.

I like this option better. In fact, we can remove it once for both cases.

> Otherwise the connection would prolong unnecessarily on idle timeout
> for no purpose, because close timer set prevents closing a connection.
> As a (somewhat degenerate) ready example, see h3_keepalive.t TODO
> with too long connection close after the patch applied.
> The test uses "keepalive_timeout 0;" in the configuration.
>
> Also I would move setting close timer before calling
> ngx_quic_connstate_dbg() to reflect it's set in the debug log.

OK

> > c->read->handler = ngx_quic_input_handler;
> >
> > return;
> > @@ -521,6 +523,10 @@ ngx_quic_close_connection(ngx_connection
> > qc->error_app ? "app " : "", qc->error,
> > qc->error_reason ? qc->error_reason : "");
> >
> > + if (qc->close.timer_set) {
> > + ngx_del_timer(&qc->close);
> > + }
> > +
>
> Removing timer there makes the check below for the timer set unnecessary:
>
> : if (rc == NGX_OK && !qc->close.timer_set) {

Now it seems to be we don't need this condition at all and it has nothing to
do with the patch. Normally, we'd like to use a timeout for the lower level.
However, the PTO() value is only different for the application level due to
max_ack_delay and only after handshake completion. However, as discussed
before, after handshake completion other levels are cleared. Also, resetting
the timer for the same value has no performance implications.

So we can remove the condition in a separate patch.

> > for (i = 0; i < NGX_QUIC_SEND_CTX_LAST; i++) {
> > ctx = &qc->send_ctx[i];
> >
> > 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
> > @@ -67,7 +67,8 @@ typedef struct {
> > ngx_flag_t retry;
> > ngx_flag_t gso_enabled;
> > ngx_flag_t disable_active_migration;
> > - ngx_msec_t timeout;
> > + ngx_msec_t handshake_timeout;
> > + ngx_msec_t idle_timeout;
> > ngx_str_t host_key;
> > size_t stream_buffer_size;
> > ngx_uint_t max_concurrent_streams_bidi;
> > 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
> > @@ -630,6 +630,10 @@ ngx_quic_do_init_streams(ngx_connection_
> >
> > qc->streams.initialized = 1;
> >
> > + if (!qc->closing && qc->close.timer_set) {
> > + ngx_del_timer(&qc->close);
> > + }
> > +
> > return NGX_OK;
> > }
> >
> > diff --git a/src/event/quic/ngx_event_quic_transport.c b/src/event/quic/ngx_event_quic_transport.c
> > --- a/src/event/quic/ngx_event_quic_transport.c
> > +++ b/src/event/quic/ngx_event_quic_transport.c
> > @@ -1985,7 +1985,7 @@ ngx_quic_init_transport_params(ngx_quic_
> > * tp->preferred_address = NULL
> > */
> >
> > - tp->max_idle_timeout = qcf->timeout;
> > + tp->max_idle_timeout = qcf->idle_timeout;
> >
> > tp->max_udp_payload_size = NGX_QUIC_MAX_UDP_PAYLOAD_SIZE;
> >
> > diff --git a/src/http/v3/ngx_http_v3_module.c b/src/http/v3/ngx_http_v3_module.c
> > --- a/src/http/v3/ngx_http_v3_module.c
> > +++ b/src/http/v3/ngx_http_v3_module.c
> > @@ -192,7 +192,7 @@ ngx_http_v3_create_srv_conf(ngx_conf_t *
> > * h3scf->quic.host_key = { 0, NULL }
> > * h3scf->quic.stream_reject_code_uni = 0;
> > * h3scf->quic.disable_active_migration = 0;
> > - * h3scf->quic.timeout = 0;
> > + * h3scf->quic.idle_timeout = 0;
> > * h3scf->max_blocked_streams = 0;
> > */
> >
> > @@ -223,7 +223,8 @@ ngx_http_v3_merge_srv_conf(ngx_conf_t *c
> > ngx_http_v3_srv_conf_t *prev = parent;
> > ngx_http_v3_srv_conf_t *conf = child;
> >
> > - ngx_http_ssl_srv_conf_t *sscf;
> > + ngx_http_ssl_srv_conf_t *sscf;
> > + ngx_http_core_srv_conf_t *cscf;
> >
> > ngx_conf_merge_value(conf->enable, prev->enable, 1);
> >
> > @@ -281,6 +282,9 @@ ngx_http_v3_merge_srv_conf(ngx_conf_t *c
> > return NGX_CONF_ERROR;
> > }
> >
> > + cscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_core_module);
> > + conf->quic.handshake_timeout = cscf->client_header_timeout;
> > +
> > sscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_ssl_module);
> > conf->quic.ssl = &sscf->ssl;
> >
> > 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
> > @@ -58,18 +58,15 @@ static const struct {
> > void
> > ngx_http_v3_init_stream(ngx_connection_t *c)
> > {
> > - ngx_http_v3_session_t *h3c;
> > ngx_http_connection_t *hc, *phc;
> > ngx_http_v3_srv_conf_t *h3scf;
> > ngx_http_core_loc_conf_t *clcf;
> > - ngx_http_core_srv_conf_t *cscf;
> >
> > hc = c->data;
> >
> > hc->ssl = 1;
> >
> > clcf = ngx_http_get_module_loc_conf(hc->conf_ctx, ngx_http_core_module);
> > - cscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_core_module);
> > h3scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v3_module);
>
> Not directly related, h3scf initialization can be moved
> under the below condition where it is only used.
>
> >
> > if (c->quic == NULL) {
> > @@ -78,10 +75,7 @@ ngx_http_v3_init_stream(ngx_connection_t
> > return;
> > }
> >
> > - h3c = hc->v3_session;
> > - ngx_add_timer(&h3c->keepalive, cscf->client_header_timeout);
> > -
> > - h3scf->quic.timeout = clcf->keepalive_timeout;
> > + h3scf->quic.idle_timeout = clcf->keepalive_timeout;
> > ngx_quic_run(c, &h3scf->quic);
> > return;
> > }
>
> --
> 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 344 September 11, 2023 07:32AM

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

Roman Arutyunyan 71 September 11, 2023 07:32AM

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

Sergey Kandaurov 69 September 12, 2023 09:06AM

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

Roman Arutyunyan 74 September 13, 2023 11:56AM

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

Roman Arutyunyan 77 September 11, 2023 07:32AM

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

Sergey Kandaurov 71 September 12, 2023 09:06AM

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

Roman Arutyunyan 68 September 13, 2023 12:04PM

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

Roman Arutyunyan 72 September 11, 2023 07:32AM

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

Sergey Kandaurov 77 September 12, 2023 09:06AM

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

Roman Arutyunyan 70 September 13, 2023 12:08PM

[PATCH 0 of 6] QUIC module compatibility issues

Roman Arutyunyan 75 September 14, 2023 06:18AM

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

Roman Arutyunyan 94 September 14, 2023 06:18AM

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

Roman Arutyunyan 82 September 14, 2023 06:18AM

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

Roman Arutyunyan 75 September 14, 2023 06:18AM

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

Sergey Kandaurov 75 September 19, 2023 06:00AM

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

Sergey Kandaurov 76 September 19, 2023 06:34AM

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

Roman Arutyunyan 80 September 21, 2023 11:42AM

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

Sergey Kandaurov 82 September 22, 2023 07:24AM

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

Roman Arutyunyan 74 September 14, 2023 06:18AM

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

Roman Arutyunyan 77 September 14, 2023 06:18AM

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

Roman Arutyunyan 66 September 14, 2023 06:18AM



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

Online Users

Guests: 326
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready