From f820f8016193d0aadbd8b2a52ff241a3145686f4 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Tue, 17 Oct 2023 13:47:39 +0200 Subject: [PATCH 1/2] Copy rapid reset parameters to the h2 session This will allow per-session adjustments and also significantly lower the risk of inconsistent calculations in the rate limit code during parameter changes. Ref #3996 --- bin/varnishd/http2/cache_http2.h | 7 +++++++ bin/varnishd/http2/cache_http2_proto.c | 10 +++++----- bin/varnishd/http2/cache_http2_session.c | 7 ++++++- include/tbl/params.h | 14 ++++++++------ 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index 65ea1d2df8d..ca2e6599301 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -194,6 +194,13 @@ struct h2_sess { VTAILQ_HEAD(,h2_req) txqueue; h2_error error; + + // rst rate limit parameters, copied from h2_* parameters + vtim_dur rapid_reset; + int64_t rapid_reset_limit; + vtim_dur rapid_reset_period; + + // rst rate limit state double rst_budget; vtim_real last_rst; }; diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index 50504c02138..edcc709d920 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -331,20 +331,20 @@ h2_rapid_reset(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) ASSERT_RXTHR(h2); CHECK_OBJ_NOTNULL(r2, H2_REQ_MAGIC); - if (cache_param->h2_rapid_reset_limit == 0) + if (h2->rapid_reset_limit == 0) return (0); now = VTIM_real(); CHECK_OBJ_NOTNULL(r2->req, REQ_MAGIC); AN(r2->req->t_first); - if (now - r2->req->t_first > cache_param->h2_rapid_reset) + if (now - r2->req->t_first > h2->rapid_reset) return (0); d = now - h2->last_rst; - h2->rst_budget += cache_param->h2_rapid_reset_limit * d / - cache_param->h2_rapid_reset_period; + h2->rst_budget += h2->rapid_reset_limit * d / + h2->rapid_reset_period; h2->rst_budget = vmin_t(double, h2->rst_budget, - cache_param->h2_rapid_reset_limit); + h2->rapid_reset_limit); h2->last_rst = now; if (h2->rst_budget < 1.0) { diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c index ef0d88ede7e..acbfdbb5192 100644 --- a/bin/varnishd/http2/cache_http2_session.c +++ b/bin/varnishd/http2/cache_http2_session.c @@ -127,7 +127,12 @@ h2_init_sess(struct sess *sp, h2_local_settings(&h2->local_settings); h2->remote_settings = H2_proto_settings; h2->decode = decode; - h2->rst_budget = cache_param->h2_rapid_reset_limit; + + h2->rapid_reset = cache_param->h2_rapid_reset; + h2->rapid_reset_limit = cache_param->h2_rapid_reset_limit; + h2->rapid_reset_period = cache_param->h2_rapid_reset_period; + + h2->rst_budget = h2->rapid_reset_limit; h2->last_rst = sp->t_open; AZ(isnan(h2->last_rst)); diff --git a/include/tbl/params.h b/include/tbl/params.h index ae622e53d17..003f08d2a30 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1257,6 +1257,8 @@ PARAM_SIMPLE( "HTTP2 maximum size of an uncompressed header list." ) +#define H2_RR_INFO \ + "Changes to this parameter affect the default for new HTTP2 sessions" PARAM_SIMPLE( /* name */ h2_rapid_reset, /* typ */ timeout, @@ -1268,8 +1270,8 @@ PARAM_SIMPLE( "The upper threshold for how soon an http/2 RST_STREAM frame has " "to be parsed after a HEADERS frame for it to be treated as " "suspect and subjected to the rate limits specified by " - "h2_rapid_reset_limit and h2_rapid_reset_period.", - /* flags */ EXPERIMENTAL, + "h2_rapid_reset_limit and h2_rapid_reset_period.\n" H2_RR_INFO, + /* flags */ EXPERIMENTAL|DELAYED_EFFECT, ) PARAM_SIMPLE( @@ -1283,8 +1285,8 @@ PARAM_SIMPLE( "HTTP2 RST Allowance.\n" "Specifies the maximum number of allowed stream resets issued by\n" "a client over a time period before the connection is closed.\n" - "Setting this parameter to 0 disables the limit.", - /* flags */ EXPERIMENTAL, + "Setting this parameter to 0 disables the limit.\n" H2_RR_INFO, + /* flags */ EXPERIMENTAL|DELAYED_EFFECT, ) PARAM_SIMPLE( @@ -1295,8 +1297,8 @@ PARAM_SIMPLE( /* def */ "60.000", /* units */ "seconds", /* descr */ - "HTTP2 sliding window duration for h2_rapid_reset_limit.", - /* flags */ EXPERIMENTAL|WIZARD, + "HTTP2 sliding window duration for h2_rapid_reset_limit.\n" H2_RR_INFO, + /* flags */ EXPERIMENTAL|DELAYED_EFFECT|WIZARD, ) /*-------------------------------------------------------------------- From 2cfb561ed46a2db1e6c2ef9f88cbb8eebb792f25 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Mon, 16 Oct 2023 16:07:05 +0200 Subject: [PATCH 2/2] Add vmod_h2 to control rapid_reset parameters per session --- include/tbl/params.h | 4 +- vmod/Makefile.am | 1 + vmod/automake_boilerplate_h2.am | 42 ++++++++++++++ vmod/tests/h2_b00000.vtc | 90 ++++++++++++++++++++++++++++ vmod/vmod_h2.c | 100 ++++++++++++++++++++++++++++++++ vmod/vmod_h2.vcc | 93 +++++++++++++++++++++++++++++ 6 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 vmod/automake_boilerplate_h2.am create mode 100644 vmod/tests/h2_b00000.vtc create mode 100644 vmod/vmod_h2.c create mode 100644 vmod/vmod_h2.vcc diff --git a/include/tbl/params.h b/include/tbl/params.h index 003f08d2a30..c369bc9e7a1 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1258,7 +1258,9 @@ PARAM_SIMPLE( ) #define H2_RR_INFO \ - "Changes to this parameter affect the default for new HTTP2 sessions" + "Changes to this parameter affect the default for new HTTP2 " \ + "sessions. vmod_h2(3) can be used to adjust it from VCL." + PARAM_SIMPLE( /* name */ h2_rapid_reset, /* typ */ timeout, diff --git a/vmod/Makefile.am b/vmod/Makefile.am index 5f3fa9250f6..ad3d4b95a4d 100644 --- a/vmod/Makefile.am +++ b/vmod/Makefile.am @@ -28,6 +28,7 @@ vmod_vcc_files = vmod_debug_vcc = include $(srcdir)/automake_boilerplate_blob.am +include $(srcdir)/automake_boilerplate_h2.am include $(srcdir)/automake_boilerplate_cookie.am include $(srcdir)/automake_boilerplate_debug.am include $(srcdir)/automake_boilerplate_directors.am diff --git a/vmod/automake_boilerplate_h2.am b/vmod/automake_boilerplate_h2.am new file mode 100644 index 00000000000..fac5fd51710 --- /dev/null +++ b/vmod/automake_boilerplate_h2.am @@ -0,0 +1,42 @@ +# Generated by vmodtool.py --boilerplate. + +vmod_h2_vcc ?= $(srcdir)/vmod_h2.vcc + +vmod_vcc_files += $(vmod_h2_vcc) + +vmod_LTLIBRARIES += libvmod_h2.la + +libvmod_h2_la_SOURCES = \ + vmod_h2.c + +libvmod_h2_la_CFLAGS = + +vmodtoolargs_h2 ?= --strict --boilerplate -o vcc_h2_if +vmod_h2_symbols_regex ?= Vmod_h2_Data + +libvmod_h2_la_LDFLAGS = \ + -export-symbols-regex $(vmod_h2_symbols_regex) \ + $(AM_LDFLAGS) \ + $(VMOD_LDFLAGS) + +nodist_libvmod_h2_la_SOURCES = vcc_h2_if.c vcc_h2_if.h + +EXTRA_libvmod_h2_la_DEPENDENCIES = $(nodist_libvmod_h2_la_SOURCES) + +EXTRA_DIST += automake_boilerplate_h2.am + +$(libvmod_h2_la_OBJECTS): vcc_h2_if.h + +vcc_h2_if.h vmod_h2.rst vmod_h2.man.rst: vcc_h2_if.c + +# A doc-change will not update mtime on the .h and .c files, so a +# touch(1) is necessary to signal that vmodtool was in fact run. +vcc_h2_if.c: $(VMODTOOL) $(srcdir)/vmod_h2.vcc + @PYTHON@ $(VMODTOOL) $(vmodtoolargs_h2) $(srcdir)/vmod_h2.vcc + touch vcc_h2_if.c + +clean-local: clean-vmod-h2 + +clean-vmod-h2: + rm -f $(nodist_libvmod_h2_la_SOURCES) + rm -f vmod_h2.rst vmod_h2.man.rst diff --git a/vmod/tests/h2_b00000.vtc b/vmod/tests/h2_b00000.vtc new file mode 100644 index 00000000000..e6754fc84dd --- /dev/null +++ b/vmod/tests/h2_b00000.vtc @@ -0,0 +1,90 @@ +varnishtest "VMOD h2 basics" + +varnish v1 -arg "-p feature=+http2" -vcl { + import h2; + + backend proforma none; + + sub vcl_recv { + return(synth(200)); + } + + sub vcl_synth { + set resp.http.http2-is = h2.is(); + set resp.body = ""; + return (deliver); + } +} -start + +client c1 { + txreq + rxresp + expect resp.status == 200 + expect resp.http.http2-is == false +} -start + +client c2 { + stream 7 { + txreq + rxresp + expect resp.status == 200 + expect resp.http.http2-is == true + } -run +} -start + +client c1 -wait +client c2 -wait + +# coverage +varnish v1 -vcl { + import h2; + + backend proforma none; + + sub vcl_recv { + return(synth(200)); + } + + sub vcl_synth { + set resp.http.rapid-reset-o = h2.rapid_reset(10ms); + set resp.http.rapid-reset-n = h2.rapid_reset(); + set resp.http.rapid-reset-limit-o = h2.rapid_reset_limit(100); + set resp.http.rapid-reset-limit-n = h2.rapid_reset_limit(); + set resp.http.rapid-reset-period-o = h2.rapid_reset_period(10s); + set resp.http.rapid-reset-period-n = h2.rapid_reset_period(); + set resp.http.rapid-reset-budget = h2.rapid_reset_budget(); + set resp.body = ""; + return (deliver); + } +} + +client c1 { + txreq + rxresp + expect resp.status == 200 + expect resp.http.rapid-reset-o == -1.000 + expect resp.http.rapid-reset-n == -1.000 + expect resp.http.rapid-reset-limit-o == -1 + expect resp.http.rapid-reset-limit-n == -1 + expect resp.http.rapid-reset-period-o == -1.000 + expect resp.http.rapid-reset-period-n == -1.000 + expect resp.http.rapid-reset-budget == -1.000 +} -start + +client c2 { + stream 7 { + txreq + rxresp + expect resp.status == 200 + expect resp.http.rapid-reset-o == 1.000 + expect resp.http.rapid-reset-n == 0.010 + expect resp.http.rapid-reset-limit-o == 0 + expect resp.http.rapid-reset-limit-n == 100 + expect resp.http.rapid-reset-period-o == 60.000 + expect resp.http.rapid-reset-period-n == 10.000 + expect resp.http.rapid-reset-budget == 100.000 + } -run +} -start + +client c1 -wait +client c2 -wait diff --git a/vmod/vmod_h2.c b/vmod/vmod_h2.c new file mode 100644 index 00000000000..bded788f45d --- /dev/null +++ b/vmod/vmod_h2.c @@ -0,0 +1,100 @@ +/*- + * Copyright 2023 UPLEX - Nils Goroll Systemoptimierung + * All rights reserved. + * + * Author: Nils Goroll + * + * SPDX-License-Identifier: BSD-2-Clause + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "config.h" + +#include "cache/cache_varnishd.h" + +#include "vcc_h2_if.h" + +#include "cache/cache_transport.h" +#include "http2/cache_http2.h" + +static struct h2_sess * +h2get(VRT_CTX) +{ + struct h2_sess *h2; + uintptr_t *up; + + CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); + CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); // $Restrict client + if (ctx->req->transport != &HTTP2_transport) + return (NULL); + AZ(SES_Get_proto_priv(ctx->req->sp, &up)); + CAST_OBJ_NOTNULL(h2, (void *)*up, H2_SESS_MAGIC); + return (h2); +} +VCL_BOOL +vmod_is(VRT_CTX) +{ + struct h2_sess *h2 = h2get(ctx); + + return (h2 != NULL); +} + +#define GETSET(type, name, argname) \ +type \ +vmod_ ## name(VRT_CTX, struct VARGS(name) *args) \ +{ \ + struct h2_sess *h2 = h2get(ctx); \ + type r; \ + \ + if (h2 == NULL) \ + return (-1); \ + \ + if (! args->valid_ ## argname) \ + return (h2->name); \ + if (h2->name == args->argname) \ + return (h2->name); \ + \ + Lck_Lock(&h2->sess->mtx); \ + r = h2->name; \ + if (h2->name != args->argname) { \ + h2->name = args->argname; \ + h2->rst_budget = h2->rapid_reset_limit; \ + h2->last_rst = ctx->now; \ + } \ + Lck_Unlock(&h2->sess->mtx); \ + return (r); \ +} + +GETSET(VCL_DURATION, rapid_reset, threshold) +GETSET(VCL_INT, rapid_reset_limit, number) +GETSET(VCL_DURATION, rapid_reset_period, duration) + +VCL_REAL +vmod_rapid_reset_budget(VRT_CTX) +{ + struct h2_sess *h2 = h2get(ctx); + + if (h2 == NULL) + return (-1); + + return (h2->rst_budget); +} diff --git a/vmod/vmod_h2.vcc b/vmod/vmod_h2.vcc new file mode 100644 index 00000000000..28f821fe7f8 --- /dev/null +++ b/vmod/vmod_h2.vcc @@ -0,0 +1,93 @@ +#- +# Copyright 2023 UPLEX - Nils Goroll Systemoptimierung +# All rights reserved. +# +# Author: Nils Goroll +# +# SPDX-License-Identifier: BSD-2-Clause +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +$ABI strict +$Module h2 3 "Module to control the built-in HTTP2 transport" + +DESCRIPTION +=========== + +This VMOD contains functions to control the HTTP2 transport built into +Varnish-Cache. + +$Function BOOL is() +$Restrict client + +Returns true when called on a session handled by the built-in HTTP2 transport. + +$Function DURATION rapid_reset([DURATION threshold]) +$Restrict client + +Get and optionally set the ``h2_rapid_reset`` parameter (See +:ref:`varnishd(1)`) for this HTTP2 session only. + +Returns -1 when used outside the HTTP2 transport. Otherwise returns +the previous value. + +If the call leads to a change in the rate limit parameters, the +current budget as retuned by +`h2.rapid_reset_budget()`_ is reset. + +$Function INT rapid_reset_limit([INT number]) +$Restrict client + +Get and optionally set the ``h2_rapid_reset_limit`` parameter (See +:ref:`varnishd(1)`) for this HTTP2 session only. + +Returns -1 when used outside the HTTP2 transport. Otherwise returns +the previous value. + +If the call leads to a change in the rate limit parameters, the +current budget as retuned by +`h2.rapid_reset_budget()`_ is reset. + +$Function DURATION rapid_reset_period([DURATION duration]) +$Restrict client + +Get and optionally set the ``h2_rapid_reset_period`` parameter (See +:ref:`varnishd(1)`) for this HTTP2 session only. + +Returns -1 when used outside the HTTP2 transport. Otherwise returns +the previous value. + +If the call leads to a change in the rate limit parameters, the +current budget as retuned by +`h2.rapid_reset_budget()`_ is reset. + +$Function REAL rapid_reset_budget() +$Restrict client + +Return how many RST frames classified as "rapid" the client is still +allowed to send before the session is going to be closed. + +SEE ALSO +======== + +* :ref:`varnishd(1)` +* :ref:`vsl(7)`