> On 18 Nov 2021, at 12:52, Roman Arutyunyan <arut@nginx.com> wrote:
>
> # HG changeset patch
> # User Roman Arutyunyan <arut@nginx.com>
> # Date 1637160358 -10800
> # Wed Nov 17 17:45:58 2021 +0300
> # Branch quic
> # Node ID b844c77ff22218a4863d1d926bcaaa0b043c8af5
> # Parent 41caf541011045612975b7bb8423a18fd424df77
> HTTP/3: use parent QUIC connection as argument when possible.
>
> Functions in ngx_http_v3.c, ngx_http_v3_streams.c and ngx_http_v3_tables.c
> now receive parent QUIC connection as the first argument instead of QUIC stream
> connection. It makes sense since they are not related to a QUIC stream and
> operate connection-wise.
>
> Also, ngx_quic_open_stream() now receives parent QUIC connection instead of
> QUIC stream connection for the same reason.
>
> Also, ngx_http_v3_finalize_connection() and ngx_http_v3_shutdown_connection()
> macros are eliminated. Instead, ngx_quic_finalize_connection() and
> ngx_quic_shutdown_connection() are called directly with the parent QUIC
> connection.
>
> 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
> @@ -37,11 +37,10 @@ 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_quic_stream_t *qs;
> ngx_quic_connection_t *qc;
>
> - qs = c->quic;
> - qc = ngx_quic_get_connection(qs->parent);
> + qc = ngx_quic_get_connection(c);
>
> if (bidi) {
> if (qc->streams.server_streams_bidi
> @@ -87,12 +86,12 @@ ngx_quic_open_stream(ngx_connection_t *c
> qc->streams.server_streams_uni++;
> }
>
> - nqs = ngx_quic_create_stream(qs->parent, id);
> - if (nqs == NULL) {
> + qs = ngx_quic_create_stream(c, id);
> + if (qs == NULL) {
> return NULL;
> }
>
> - return nqs->connection;
> + return qs->connection;
> }
>
>
> 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))
>
>
> 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
> @@ -17,13 +17,11 @@ static void ngx_http_v3_cleanup_session(
> ngx_int_t
> ngx_http_v3_init_session(ngx_connection_t *c)
> {
> - ngx_connection_t *pc;
> ngx_pool_cleanup_t *cln;
> ngx_http_connection_t *hc;
> ngx_http_v3_session_t *h3c;
>
> - pc = c->quic->parent;
> - hc = pc->data;
> + hc = c->data;
>
> if (hc->v3_session) {
> return NGX_OK;
> @@ -31,7 +29,7 @@ ngx_http_v3_init_session(ngx_connection_
>
> ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 init session");
>
> - h3c = ngx_pcalloc(pc->pool, sizeof(ngx_http_v3_session_t));
> + h3c = ngx_pcalloc(c->pool, sizeof(ngx_http_v3_session_t));
> if (h3c == NULL) {
> goto failed;
> }
> @@ -42,12 +40,12 @@ ngx_http_v3_init_session(ngx_connection_
> ngx_queue_init(&h3c->blocked);
> ngx_queue_init(&h3c->pushing);
>
> - h3c->keepalive.log = pc->log;
> - h3c->keepalive.data = pc;
> + h3c->keepalive.log = c->log;
> + h3c->keepalive.data = c;
> h3c->keepalive.handler = ngx_http_v3_keepalive_handler;
> h3c->keepalive.cancelable = 1;
>
> - cln = ngx_pool_cleanup_add(pc->pool, 0);
> + cln = ngx_pool_cleanup_add(c->pool, 0);
> if (cln == NULL) {
> goto failed;
> }
> @@ -63,8 +61,8 @@ failed:
>
> ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to create http3 session");
>
> - ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> - "failed to create http3 session");
> + ngx_quic_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> + "failed to create http3 session");
> return NGX_ERROR;
> }
>
> @@ -106,8 +104,8 @@ ngx_http_v3_check_flood(ngx_connection_t
> if (h3c->total_bytes / 8 > h3c->payload_bytes + 1048576) {
> ngx_log_error(NGX_LOG_INFO, c->log, 0, "http3 flood detected");
>
> - ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> - "HTTP/3 flood detected");
> + ngx_quic_finalize_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> + "HTTP/3 flood detected");
> return NGX_ERROR;
> }
>
> 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
> @@ -84,12 +84,6 @@
> ngx_http_get_module_srv_conf(ngx_http_quic_get_connection(c)->conf_ctx, \
> module)
>
> -#define ngx_http_v3_finalize_connection(c, code, reason) \
> - ngx_quic_finalize_connection(c->quic->parent, code, reason)
> -
> -#define ngx_http_v3_shutdown_connection(c, code, reason) \
> - ngx_quic_shutdown_connection(c->quic->parent, code, reason)
> -
> #define ngx_http_v3_connection(c) \
> ((c)->quic ? ngx_http_quic_get_connection(c)->addr_conf->http3 : 0)
>
> diff --git a/src/http/v3/ngx_http_v3_filter_module.c b/src/http/v3/ngx_http_v3_filter_module.c
> --- a/src/http/v3/ngx_http_v3_filter_module.c
> +++ b/src/http/v3/ngx_http_v3_filter_module.c
> @@ -907,7 +907,7 @@ ngx_http_v3_create_push_request(ngx_http
> ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0,
> "http3 create push request id:%uL", push_id);
>
> - c = ngx_http_v3_create_push_stream(pc, push_id);
> + c = ngx_http_v3_create_push_stream(pc->quic->parent, push_id);
> if (c == NULL) {
> return NGX_ABORT;
> }
> diff --git a/src/http/v3/ngx_http_v3_parse.c b/src/http/v3/ngx_http_v3_parse.c
> --- a/src/http/v3/ngx_http_v3_parse.c
> +++ b/src/http/v3/ngx_http_v3_parse.c
> @@ -353,7 +353,8 @@ ngx_http_v3_parse_headers(ngx_connection
>
> case sw_verify:
>
> - rc = ngx_http_v3_check_insert_count(c, st->prefix.insert_count);
> + rc = ngx_http_v3_check_insert_count(c->quic->parent,
> + st->prefix.insert_count);

For the record, as previously reported in
http://mailman.nginx.org/pipermail/nginx-devel/2021-December/014607.html
and discussed privately, this needs to be a stream connection, as otherwise,
when inserting a new dynamic entry that unblocks a stream, an event will be
mis-posted on the main connection, instead. Reverting this part helps.

> if (rc != NGX_OK) {
> return rc;
> }
> @@ -392,7 +393,9 @@ done:
> ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 parse headers done");
>
> if (st->prefix.insert_count > 0) {
> - if (ngx_http_v3_send_ack_section(c, c->quic->id) != NGX_OK) {
> + if (ngx_http_v3_send_ack_section(c->quic->parent, c->quic->id)
> + != NGX_OK)
> + {

Similarly, I doubt if this kind of changes is really necessary
throughout ngx_http_v3_parse.c. In general, c->quic->parent is
used to finalize a connection, so I'd consider pushing this down
to ngx_quic_finalize_connection().

> return NGX_ERROR;
> }
> }
> @@ -466,7 +469,7 @@ ngx_http_v3_parse_field_section_prefix(n
>
> done:
>
> - rc = ngx_http_v3_decode_insert_count(c, &st->insert_count);
> + rc = ngx_http_v3_decode_insert_count(c->quic->parent, &st->insert_count);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1102,14 +1105,16 @@ ngx_http_v3_parse_lookup(ngx_connection_
> u_char *p;
>
> if (!dynamic) {
> - if (ngx_http_v3_lookup_static(c, index, name, value) != NGX_OK) {
> + if (ngx_http_v3_lookup_static(c->quic->parent, index, name, value)
> + != NGX_OK)
> + {
> return NGX_HTTP_V3_ERR_DECOMPRESSION_FAILED;
> }
>
> return NGX_OK;
> }
>
> - if (ngx_http_v3_lookup(c, index, name, value) != NGX_OK) {
> + if (ngx_http_v3_lookup(c->quic->parent, index, name, value) != NGX_OK) {
> return NGX_HTTP_V3_ERR_DECOMPRESSION_FAILED;
> }
>
> @@ -1264,7 +1269,7 @@ ngx_http_v3_parse_control(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_cancel_push(c, st->vlint.value);
> + rc = ngx_http_v3_cancel_push(c->quic->parent, st->vlint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1310,7 +1315,7 @@ ngx_http_v3_parse_control(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_set_max_push_id(c, st->vlint.value);
> + rc = ngx_http_v3_set_max_push_id(c->quic->parent, st->vlint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1334,7 +1339,7 @@ ngx_http_v3_parse_control(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_goaway(c, st->vlint.value);
> + rc = ngx_http_v3_goaway(c->quic->parent, st->vlint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1398,7 +1403,9 @@ ngx_http_v3_parse_settings(ngx_connectio
> return rc;
> }
>
> - if (ngx_http_v3_set_param(c, st->id, st->vlint.value) != NGX_OK) {
> + if (ngx_http_v3_set_param(c->quic->parent, st->id, st->vlint.value)
> + != NGX_OK)
> + {
> return NGX_HTTP_V3_ERR_SETTINGS_ERROR;
> }
>
> @@ -1493,7 +1500,7 @@ ngx_http_v3_parse_encoder(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_set_capacity(c, st->pint.value);
> + rc = ngx_http_v3_set_capacity(c->quic->parent, st->pint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1508,7 +1515,7 @@ ngx_http_v3_parse_encoder(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_duplicate(c, st->pint.value);
> + rc = ngx_http_v3_duplicate(c->quic->parent, st->pint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1613,7 +1620,8 @@ done:
> st->dynamic ? "dynamic" : "static",
> st->index, &st->value);
>
> - rc = ngx_http_v3_ref_insert(c, st->dynamic, st->index, &st->value);
> + rc = ngx_http_v3_ref_insert(c->quic->parent, st->dynamic, st->index,
> + &st->value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1731,7 +1739,7 @@ done:
> "http3 parse field iln done \"%V\":\"%V\"",
> &st->name, &st->value);
>
> - rc = ngx_http_v3_insert(c, &st->name, &st->value);
> + rc = ngx_http_v3_insert(c->quic->parent, &st->name, &st->value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1793,7 +1801,7 @@ ngx_http_v3_parse_decoder(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_ack_section(c, st->pint.value);
> + rc = ngx_http_v3_ack_section(c->quic->parent, st->pint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1808,7 +1816,7 @@ ngx_http_v3_parse_decoder(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_cancel_stream(c, st->pint.value);
> + rc = ngx_http_v3_cancel_stream(c->quic->parent, st->pint.value);
> if (rc != NGX_OK) {
> return rc;
> }
> @@ -1823,7 +1831,7 @@ ngx_http_v3_parse_decoder(ngx_connection
> return rc;
> }
>
> - rc = ngx_http_v3_inc_insert_count(c, st->pint.value);
> + rc = ngx_http_v3_inc_insert_count(c->quic->parent, st->pint.value);
> if (rc != NGX_OK) {
> return rc;
> }

[..]

--
Sergey Kandaurov

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