> On 18 Nov 2021, at 12:52, Roman Arutyunyan <[email protected]> wrote: > > # HG changeset patch > # User Roman Arutyunyan <[email protected]> > # 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 [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
