From f5f8f271a32fb4ded20631e3b9761bc58fc0c813 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Tue, 17 Oct 2023 13:47:39 +0200 Subject: [PATCH] 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 a006d708f9d..dc3d4a14476 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -328,20 +328,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 29af53093ee..cdd99d3dfb4 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 d5e080758f3..679d29894c6 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1243,6 +1243,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, @@ -1254,8 +1256,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( @@ -1269,8 +1271,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( @@ -1281,8 +1283,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, ) /*--------------------------------------------------------------------