[PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse
Vladimir Homutov
vl at nginx.com
Wed Nov 17 08:23:19 UTC 2021
17.11.2021 10:12, Roman Arutyunyan пишет:
> On Tue, Nov 16, 2021 at 12:18:47PM +0300, Vladimir Homutov wrote:
>> On Mon, Nov 15, 2021 at 03:33:25PM +0300, Roman Arutyunyan wrote:
>>> # HG changeset patch
>>> # User Roman Arutyunyan <arut at nginx.com>
>>> # Date 1636646820 -10800
>>> # Thu Nov 11 19:07:00 2021 +0300
>>> # Branch quic
>>> # Node ID 801103b7645d93d0d06f63019e54d9e76f1baa6c
>>> # Parent d2c193aa84800da00314f1af72ae722d964445a4
>>> QUIC: reject streams which we could not create.
>>>
>>> The reasons why a stream may not be created by server currently include hitting
>>> worker_connections limit and memory allocation error. Previously in these
>>> cases the entire QUIC connection was closed and all its streams were shut down.
>>> Now the new stream is rejected and existing streams continue working.
>>>
>>> To reject an HTTP/3 request stream, RESET_STREAM and STOP_SENDING with
>>> H3_REQUEST_REJECTED error code are sent to client. HTTP/3 uni streams and
>>> Stream streams are not rejected.
>>>
>>> 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
>>> @@ -61,6 +61,9 @@ typedef struct {
>>> ngx_flag_t retry;
>>> ngx_flag_t gso_enabled;
>>> ngx_str_t host_key;
>>> + ngx_int_t close_stream_code;
>>> + ngx_int_t reject_uni_stream_code;
>>> + ngx_int_t reject_bidi_stream_code;
>>
>> i would prefer stream_close_code and stream_reject_code_uni|bidi,
>> a bit similar to transport parameter naming like
>> 'initial_max_stream_data_bidi_local', YMMV
>
> OK, let's do this.
>
>>> u_char av_token_key[NGX_QUIC_AV_KEY_LEN];
>>> u_char sr_token_key[NGX_QUIC_SR_KEY_LEN];
>>> } ngx_quic_conf_t;
>>> 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
>>> @@ -15,6 +15,7 @@
>>>
>>> static ngx_quic_stream_t *ngx_quic_create_client_stream(ngx_connection_t *c,
>>> uint64_t id);
>>> +static ngx_int_t ngx_quic_reject_stream(ngx_connection_t *c, uint64_t id);
>>> static ngx_int_t ngx_quic_init_stream(ngx_quic_stream_t *qs);
>>> static void ngx_quic_init_streams_handler(ngx_connection_t *c);
>>> static ngx_quic_stream_t *ngx_quic_create_stream(ngx_connection_t *c,
>>> @@ -377,8 +378,13 @@ ngx_quic_create_client_stream(ngx_connec
>>> for ( /* void */ ; min_id < id; min_id += 0x04) {
>>>
>>> qs = ngx_quic_create_stream(c, min_id);
>>> +
>>> if (qs == NULL) {
>>> - return NULL;
>>> + if (ngx_quic_reject_stream(c, min_id) != NGX_OK) {
>>> + return NULL;
>>> + }
>>> +
>>> + continue;
>>> }
>>>
>>> if (ngx_quic_init_stream(qs) != NGX_OK) {
>>> @@ -390,7 +396,66 @@ ngx_quic_create_client_stream(ngx_connec
>>> }
>>> }
>>>
>>> - return ngx_quic_create_stream(c, id);
>>> + qs = ngx_quic_create_stream(c, id);
>>> +
>>> + if (qs == NULL) {
>>> + if (ngx_quic_reject_stream(c, id) != NGX_OK) {
>>> + return NULL;
>>> + }
>>> +
>>> + return NGX_QUIC_STREAM_GONE;
>>> + }
>>> +
>>> + return qs;
>>> +}
>>> +
>>> +
>>> +static ngx_int_t
>>> +ngx_quic_reject_stream(ngx_connection_t *c, uint64_t id)
>>> +{
>>> + uint64_t code;
>>> + ngx_quic_frame_t *frame;
>>> + ngx_quic_connection_t *qc;
>>> +
>>> + qc = ngx_quic_get_connection(c);
>>> +
>>> + code = (id & NGX_QUIC_STREAM_UNIDIRECTIONAL)
>>> + ? qc->conf->reject_uni_stream_code
>>> + : qc->conf->reject_bidi_stream_code;
>>> +
>>> + ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
>>> + "quic stream id:0x%xL reject err:0x%xL", id, code);
>>
>> Here we may decline stream rejection, but have already logged it.
>> I suggest putting debug below 'code == 0' test.
>
> Zero code still carries some information. If it looks misleading, then yes,
> let's move it below.
>
not really necessary, let it stay as is; we have many places where we
debug just fact that function was entered.
>>> + if (code == 0) {
>>> + return NGX_DECLINED;
>>> + }
>>> +
>>> + frame = ngx_quic_alloc_frame(c);
>>> + if (frame == NULL) {
>>> + return NGX_ERROR;
>>> + }
>>> +
>>> + frame->level = ssl_encryption_application;
>>> + frame->type = NGX_QUIC_FT_RESET_STREAM;
>>> + frame->u.reset_stream.id = id;
>>> + frame->u.reset_stream.error_code = code;
>>> + frame->u.reset_stream.final_size = 0;
>>> +
>>> + ngx_quic_queue_frame(qc, frame);
>>> +
>>> + frame = ngx_quic_alloc_frame(c);
>>> + if (frame == NULL) {
>>> + return NGX_ERROR;
>>> + }
>>> +
>>> + frame->level = ssl_encryption_application;
>>> + frame->type = NGX_QUIC_FT_STOP_SENDING;
>>> + frame->u.stop_sending.id = id;
>>> + frame->u.stop_sending.error_code = code;
>>> +
>>> + ngx_quic_queue_frame(qc, frame);
>>> +
>>> + return NGX_OK;
>>> }
>>>
>>>
>>> @@ -866,7 +931,9 @@ ngx_quic_stream_cleanup_handler(void *da
>>> if ((qs->id & NGX_QUIC_STREAM_SERVER_INITIATED) == 0
>>> || (qs->id & NGX_QUIC_STREAM_UNIDIRECTIONAL) == 0)
>>> {
>>> - if (!c->read->pending_eof && !c->read->error) {
>>> + if (!c->read->pending_eof && !c->read->error
>>> + && qc->conf->close_stream_code)
>>> + {
>>> frame = ngx_quic_alloc_frame(pc);
>>> if (frame == NULL) {
>>> goto done;
>>> @@ -875,7 +942,7 @@ ngx_quic_stream_cleanup_handler(void *da
>>> frame->level = ssl_encryption_application;
>>> frame->type = NGX_QUIC_FT_STOP_SENDING;
>>> frame->u.stop_sending.id = qs->id;
>>> - frame->u.stop_sending.error_code = 0x100; /* HTTP/3 no error */
>>> + frame->u.stop_sending.error_code = qc->conf->close_stream_code;
>>>
>>> ngx_quic_queue_frame(qc, frame);
>>> }
>>> diff --git a/src/http/modules/ngx_http_quic_module.c b/src/http/modules/ngx_http_quic_module.c
>>> --- a/src/http/modules/ngx_http_quic_module.c
>>> +++ b/src/http/modules/ngx_http_quic_module.c
>>> @@ -314,6 +314,7 @@ ngx_http_quic_create_srv_conf(ngx_conf_t
>>> * conf->tp.sr_enabled = 0
>>> * conf->tp.preferred_address = NULL
>>> * conf->host_key = { 0, NULL }
>>> + * cong->reject_uni_stream_code = 0;
>>> */
>>>
>>> conf->tp.max_idle_timeout = NGX_CONF_UNSET_MSEC;
>>> @@ -331,6 +332,8 @@ ngx_http_quic_create_srv_conf(ngx_conf_t
>>>
>>> conf->retry = NGX_CONF_UNSET;
>>> conf->gso_enabled = NGX_CONF_UNSET;
>>> + conf->close_stream_code = NGX_HTTP_V3_ERR_NO_ERROR;
>>> + conf->reject_bidi_stream_code = NGX_HTTP_V3_ERR_REQUEST_REJECTED;
>>>
>>> return conf;
>>> }
>>> diff --git a/src/stream/ngx_stream_quic_module.c b/src/stream/ngx_stream_quic_module.c
>>> --- a/src/stream/ngx_stream_quic_module.c
>>> +++ b/src/stream/ngx_stream_quic_module.c
>>> @@ -241,6 +241,9 @@ ngx_stream_quic_create_srv_conf(ngx_conf
>>> * conf->tp.retry_scid = { 0, NULL };
>>> * conf->tp.preferred_address = NULL
>>> * conf->host_key = { 0, NULL }
>>> + * conf->close_stream_code = 0;
>>> + * conf->reject_uni_stream_code = 0;
>>> + * conf->reject_bidi_stream_code = 0;
>>> */
>>>
>>> conf->tp.max_idle_timeout = NGX_CONF_UNSET_MSEC;
>>
>>
>> Overal patch looks good to me
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
More information about the nginx-devel
mailing list