On Mon, Nov 15, 2021 at 03:33:25PM +0300, Roman Arutyunyan wrote:
> # HG changeset patch
> # User Roman Arutyunyan <arut@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


> 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.

> +
> + 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@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel