From 078a4a42d91a9afccd4204814df7625552137337 Mon Sep 17 00:00:00 2001 From: Asad Sajjad Ahmed Date: Tue, 28 Sep 2021 14:18:50 +0200 Subject: [PATCH 01/10] vdef: implement vmin[_t], vmax[_t] and vlimit[_t] This should make the code easier to read and stop us from being inconsistent. Signed-off-by: Asad Sajjad Ahmed --- include/vdef.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/include/vdef.h b/include/vdef.h index 60d833c3303..128f44148d5 100644 --- a/include/vdef.h +++ b/include/vdef.h @@ -120,6 +120,55 @@ #define RDN2(x, y) ((x)&(~((uintptr_t)(y)-1UL))) /* PWR2(y) true */ #define RUP2(x, y) (((x)+((y)-1))&(~((uintptr_t)(y)-1UL))) /* PWR2(y) true */ +/********************************************************************** + * Find the minimum or maximum values. + * Only evaluate the expression once and perform type checking. + */ + +/* ref: https://stackoverflow.com/a/17624752 */ + +#define VINDIRECT(a, b, c) a ## b ## c +#define VCOMBINE(a, b, c) VINDIRECT(a, b, c) + +#if defined(__COUNTER__) +# define VUNIQ_NAME(base) VCOMBINE(base, __LINE__, __COUNTER__) +#else +# define VUNIQ_NAME(base) VCOMBINE(base, __LINE__, 0) +#endif + +/* ref: https://gcc.gnu.org/onlinedocs/gcc/Typeof.html */ + +#define _vmin(a, b, _va, _vb) \ +({ \ + typeof (a) _va = (a); \ + typeof (b) _vb = (b); \ + (void)(&_va == &_vb); \ + _va < _vb ? _va : _vb; \ +}) + +#define _vmax(a, b, _va, _vb) \ +({ \ + typeof (a) _va = (a); \ + typeof (b) _vb = (b); \ + (void)(&_va == &_vb); \ + _va > _vb ? _va : _vb; \ +}) + +#define vmin(a, b) _vmin((a), (b), VUNIQ_NAME(_vmina), \ + VUNIQ_NAME(_vminb)) +#define vmax(a, b) _vmax((a), (b), VUNIQ_NAME(_vmaxa), \ + VUNIQ_NAME(_vmaxb)) + +#define vmin_t(type, a, b) vmin((type)(a), (type)(b)) +#define vmax_t(type, a, b) vmax((type)(a), (type)(b)) + +/********************************************************************** + * Clamp the value between two limits. + */ + +#define vlimit(a, l, u) vmax((l), vmin((a), (u))) +#define vlimit_t(type, a, l, u) vmax_t(type, (l), vmin_t(type, (a), (u))) + /********************************************************************** * FlexeLint and compiler shutuppery */ From 109f5db809b3cc9c47b78d541311406728071542 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 11 Oct 2023 15:16:36 +0200 Subject: [PATCH 02/10] h2: Add a rate limit facility for h/2 RST handling This adds parameters h2_rapid_reset_limit and h2_rapid_reset_period, which govern the rate of which we permit clients to reset streams. If the limit is exceeded the connection is closed. Related to: #1851 Conflicts: include/tbl/params.h --- bin/varnishd/http2/cache_http2.h | 2 + bin/varnishd/http2/cache_http2_proto.c | 35 ++++++++++++++- bin/varnishd/http2/cache_http2_session.c | 3 ++ bin/varnishtest/tests/r03996.vtc | 56 ++++++++++++++++++++++++ include/tbl/params.h | 32 ++++++++++++++ 5 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 bin/varnishtest/tests/r03996.vtc diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index 533e00fa87d..80e8a165d3d 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -193,6 +193,8 @@ struct h2_sess { h2_error error; int open_streams; + double rst_budget; + vtim_real last_rst; }; #define ASSERT_RXTHR(h2) do {assert(h2->rxthr == pthread_self());} while(0) diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index df3c041aa94..51428521398 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -314,9 +314,41 @@ h2_rx_push_promise(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) /********************************************************************** */ +static h2_error +h2_rapid_reset(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) +{ + vtim_real now; + vtim_dur d; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + ASSERT_RXTHR(h2); + CHECK_OBJ_NOTNULL(r2, H2_REQ_MAGIC); + + if (cache_param->h2_rapid_reset_limit == 0) + return (0); + + now = VTIM_real(); + d = now - h2->last_rst; + h2->rst_budget += cache_param->h2_rapid_reset_limit * d / + cache_param->h2_rapid_reset_period; + h2->rst_budget = vmin_t(double, h2->rst_budget, + cache_param->h2_rapid_reset_limit); + h2->last_rst = now; + + if (h2->rst_budget < 1.0) { + Lck_Lock(&h2->sess->mtx); + VSLb(h2->vsl, SLT_Error, "H2: Hit RST limit. Closing session."); + Lck_Unlock(&h2->sess->mtx); + return (H2CE_ENHANCE_YOUR_CALM); + } + h2->rst_budget -= 1.0; + return (0); +} + static h2_error v_matchproto_(h2_rxframe_f) h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) { + h2_error h2e; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); ASSERT_RXTHR(h2); @@ -326,8 +358,9 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) return (H2CE_FRAME_SIZE_ERROR); if (r2 == NULL) return (0); + h2e = h2_rapid_reset(wrk, h2, r2); h2_kill_req(wrk, h2, r2, h2_streamerror(vbe32dec(h2->rxf_data))); - return (0); + return (h2e); } /********************************************************************** diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c index add9b7d6c65..b45b6e3ac15 100644 --- a/bin/varnishd/http2/cache_http2_session.c +++ b/bin/varnishd/http2/cache_http2_session.c @@ -127,6 +127,9 @@ h2_init_sess(const struct worker *wrk, 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->last_rst = sp->t_open; + AZ(isnan(h2->last_rst)); AZ(VHT_Init(h2->dectbl, h2->local_settings.header_table_size)); diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc new file mode 100644 index 00000000000..4d7ff8c5155 --- /dev/null +++ b/bin/varnishtest/tests/r03996.vtc @@ -0,0 +1,56 @@ +varnishtest "h2 rapid reset" + +barrier b1 sock 2 -cyclic +barrier b2 sock 5 -cyclic + +server s1 { + rxreq + txresp +} -start + +varnish v1 -cliok "param.set feature +http2" +varnish v1 -cliok "param.set debug +syncvsl" +varnish v1 -cliok "param.set h2_rapid_reset_limit 3" + +varnish v1 -vcl+backend { + import vtc; + + sub vcl_recv { + if (req.http.barrier) { + vtc.barrier_sync(req.http.barrier); + } + vtc.barrier_sync("${b2_sock}"); + } + +} -start + +client c1 { + stream 0 { + rxgoaway + expect goaway.err == ENHANCE_YOUR_CALM + } -start + + stream 1 { + txreq -hdr barrier ${b1_sock} + barrier b1 sync + txrst + } -run + stream 3 { + txreq -hdr barrier ${b1_sock} + barrier b1 sync + txrst + } -run + stream 5 { + txreq -hdr barrier ${b1_sock} + barrier b1 sync + txrst + } -run + stream 7 { + txreq -hdr barrier ${b1_sock} + barrier b1 sync + txrst + } -run + + barrier b2 sync + stream 0 -wait +} -run diff --git a/include/tbl/params.h b/include/tbl/params.h index 59280f9cc7c..9143843576e 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1896,6 +1896,38 @@ PARAM( ) #endif +PARAM( + /* name */ h2_rapid_reset_limit, + /* typ */ uint, + /* min */ "0", + /* max */ NULL, + /* default */ "0", + /* units */ NULL, + /* flags */ EXPERIMENTAL, + /* s-text */ + "HTTP2 RST Allowance.\n\n" + "Specifies the maximum number of allowed stream resets issued by " + "a client over a time period before the connection is closed. Setting " + "this parameter to 0 disables the limit.", + /* l-text */ "", + /* func */ NULL +) + + +PARAM( + /* name */ h2_rapid_reset_period, + /* typ */ timeout, + /* min */ "1.000", + /* max */ NULL, + /* default */ "60.000", + /* units */ "seconds", + /* flags */ EXPERIMENTAL|WIZARD, + /* s-text */ + "HTTP2 sliding window duration for h2_rapid_reset_limit.", + /* l-text */ "", + /* func */ NULL +) + #undef PARAM /*lint -restore */ From 2f12eb55a6b5c418276479c433256b9f49d7ef5c Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Tue, 17 Oct 2023 11:20:11 +0200 Subject: [PATCH 03/10] Introduce RAPID_RESET as a sess_close reason Conflicts: bin/varnishd/VSC_main.vsc include/tbl/sess_close.h --- bin/varnishd/VSC_main.vsc | 8 ++++++++ bin/varnishd/http2/cache_http2_proto.c | 3 ++- bin/varnishtest/tests/r03996.vtc | 2 ++ include/tbl/h2_error.h | 12 ++++++++++++ include/tbl/sess_close.h | 1 + 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/bin/varnishd/VSC_main.vsc b/bin/varnishd/VSC_main.vsc index 8d6277ed7d7..4bb4697bf0b 100644 --- a/bin/varnishd/VSC_main.vsc +++ b/bin/varnishd/VSC_main.vsc @@ -601,6 +601,14 @@ Number of session closes with Error VCL_FAILURE (VCL failure) +.. varnish_vsc:: sc_rapid_reset + :level: diag + :oneliner: Session Err RAPID_RESET + + Number of times we failed an http/2 session because it hit its + configured limits for the number of permitted rapid stream + resets. + .. varnish_vsc:: client_resp_500 :level: diag :group: wrk diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index 51428521398..fc93d2b33b5 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -45,6 +45,7 @@ #include "vtcp.h" #include "vtim.h" +#define H2_CUSTOM_ERRORS #define H2EC1(U,v,r,d) const struct h2_error_s H2CE_##U[1] = {{#U,d,v,0,1,r}}; #define H2EC2(U,v,r,d) const struct h2_error_s H2SE_##U[1] = {{#U,d,v,1,0,r}}; #define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d) @@ -339,7 +340,7 @@ h2_rapid_reset(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) Lck_Lock(&h2->sess->mtx); VSLb(h2->vsl, SLT_Error, "H2: Hit RST limit. Closing session."); Lck_Unlock(&h2->sess->mtx); - return (H2CE_ENHANCE_YOUR_CALM); + return (H2CE_RAPID_RESET); } h2->rst_budget -= 1.0; return (0); diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc index 4d7ff8c5155..c9b1a4c378c 100644 --- a/bin/varnishtest/tests/r03996.vtc +++ b/bin/varnishtest/tests/r03996.vtc @@ -54,3 +54,5 @@ client c1 { barrier b2 sync stream 0 -wait } -run + +varnish v1 -expect sc_rapid_reset == 1 diff --git a/include/tbl/h2_error.h b/include/tbl/h2_error.h index 7481b9836f1..8f3485c244f 100644 --- a/include/tbl/h2_error.h +++ b/include/tbl/h2_error.h @@ -145,5 +145,17 @@ H2_ERROR( /* descr */ "Use HTTP/1.1 for the request" ) +#ifdef H2_CUSTOM_ERRORS +H2_ERROR( + /* name */ RAPID_RESET, + /* val */ 11, /* ENHANCE_YOUR_CALM */ + /* types */ 1, + /* reason */ SC_RAPID_RESET, + /* descr */ "http/2 rapid reset detected" +) + +# undef H2_CUSTOM_ERRORS +#endif + #undef H2_ERROR /*lint -restore */ diff --git a/include/tbl/sess_close.h b/include/tbl/sess_close.h index c20e71ca6d8..de130aa87c2 100644 --- a/include/tbl/sess_close.h +++ b/include/tbl/sess_close.h @@ -47,6 +47,7 @@ SESS_CLOSE(PIPE_OVERFLOW, pipe_overflow,1, "Session pipe overflow") SESS_CLOSE(RANGE_SHORT, range_short, 1, "Insufficient data for range") SESS_CLOSE(REQ_HTTP20, req_http20, 1, "HTTP2 not accepted") SESS_CLOSE(VCL_FAILURE, vcl_failure, 1, "VCL failure") +SESS_CLOSE(RAPID_RESET, rapid_reset, 1, "HTTP2 rapid reset") #undef SESS_CLOSE /*lint -restore */ From 0aa255168755d0b9068ce84f7db1df5d6af55b55 Mon Sep 17 00:00:00 2001 From: Poul-Henning Kamp Date: Tue, 17 Oct 2023 14:35:22 +0000 Subject: [PATCH 04/10] Flexelinting --- bin/varnishd/http2/cache_http2.h | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index 80e8a165d3d..8fa1da6109c 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -47,6 +47,7 @@ struct h2_error_s { typedef const struct h2_error_s *h2_error; +#define H2_CUSTOM_ERRORS #define H2EC1(U,v,r,d) extern const struct h2_error_s H2CE_##U[1]; #define H2EC2(U,v,r,d) extern const struct h2_error_s H2SE_##U[1]; #define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d) From c4f4416236b264aba392352a4641905ef576fba1 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Mon, 16 Oct 2023 15:56:37 +0200 Subject: [PATCH 05/10] Add param h2_rapid_reset Only RST frames received earlier than this duration will be considered rapid. Includes backports of the following upstream doc-touchups: - 151f4d50f98ed43ad04b60208e65697ddb71dbf7 - 46d4f82386ab10ea4798637a70dc7122a9b81bba Conflicts: include/tbl/params.h --- bin/varnishd/http2/cache_http2_proto.c | 5 +++++ bin/varnishtest/tests/r03996.vtc | 1 + include/tbl/params.h | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index fc93d2b33b5..e6ac6a6c473 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -329,6 +329,11 @@ h2_rapid_reset(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) 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) + return (0); + d = now - h2->last_rst; h2->rst_budget += cache_param->h2_rapid_reset_limit * d / cache_param->h2_rapid_reset_period; diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc index c9b1a4c378c..d9493d1f93c 100644 --- a/bin/varnishtest/tests/r03996.vtc +++ b/bin/varnishtest/tests/r03996.vtc @@ -11,6 +11,7 @@ server s1 { varnish v1 -cliok "param.set feature +http2" varnish v1 -cliok "param.set debug +syncvsl" varnish v1 -cliok "param.set h2_rapid_reset_limit 3" +varnish v1 -cliok "param.set h2_rapid_reset 5" varnish v1 -vcl+backend { import vtc; diff --git a/include/tbl/params.h b/include/tbl/params.h index 9143843576e..78a793470e8 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1896,6 +1896,24 @@ PARAM( ) #endif +PARAM( + /* name */ h2_rapid_reset, + /* typ */ timeout, + /* min */ "0", + /* max */ NULL, + /* default */ "1.0", + /* units */ "seconds", + /* flags */ EXPERIMENTAL, + /* s-text */ + "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.", + /* l-text */ "", + /* func */ NULL +) + + PARAM( /* name */ h2_rapid_reset_limit, /* typ */ uint, From 24470e293a77dda1904c77c588dc3ed8fec7ec4c Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 18 Oct 2023 14:14:52 +0200 Subject: [PATCH 06/10] h2: Set a default h2_rapid_reset_limit --- include/tbl/params.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/tbl/params.h b/include/tbl/params.h index 78a793470e8..7b778820d16 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1919,7 +1919,7 @@ PARAM( /* typ */ uint, /* min */ "0", /* max */ NULL, - /* default */ "0", + /* default */ "100", /* units */ NULL, /* flags */ EXPERIMENTAL, /* s-text */ From 2ed3c8e075aba61a047158b86b7e6b29ee3da607 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 18 Oct 2023 14:32:32 +0200 Subject: [PATCH 07/10] Copy rapid reset parameters to the h2 session backport of f820f8016193d0aadbd8b2a52ff241a3145686f4 Conflicts: include/tbl/params.h --- bin/varnishd/http2/cache_http2.h | 8 +++++++- bin/varnishd/http2/cache_http2_proto.c | 10 +++++----- bin/varnishd/http2/cache_http2_session.c | 7 ++++++- include/tbl/params.h | 16 ++++++++++------ 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index 8fa1da6109c..4020b288729 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -192,8 +192,14 @@ struct h2_sess { VTAILQ_HEAD(,h2_req) txqueue; h2_error error; - int open_streams; + + // 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 e6ac6a6c473..afa9f03ef30 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -325,20 +325,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 b45b6e3ac15..06e625f33e2 100644 --- a/bin/varnishd/http2/cache_http2_session.c +++ b/bin/varnishd/http2/cache_http2_session.c @@ -127,7 +127,12 @@ h2_init_sess(const struct worker *wrk, 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 7b778820d16..665303db444 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1896,6 +1896,10 @@ PARAM( ) #endif +#define H2_RR_INFO \ + "Changes to this parameter affect the default for new HTTP2 " \ + "sessions." + PARAM( /* name */ h2_rapid_reset, /* typ */ timeout, @@ -1903,12 +1907,12 @@ PARAM( /* max */ NULL, /* default */ "1.0", /* units */ "seconds", - /* flags */ EXPERIMENTAL, + /* flags */ EXPERIMENTAL | DELAYED_EFFECT, /* s-text */ "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.", + "h2_rapid_reset_limit and h2_rapid_reset_period." H2_RR_INFO, /* l-text */ "", /* func */ NULL ) @@ -1921,12 +1925,12 @@ PARAM( /* max */ NULL, /* default */ "100", /* units */ NULL, - /* flags */ EXPERIMENTAL, + /* flags */ EXPERIMENTAL | DELAYED_EFFECT, /* s-text */ "HTTP2 RST Allowance.\n\n" "Specifies the maximum number of allowed stream resets issued by " "a client over a time period before the connection is closed. Setting " - "this parameter to 0 disables the limit.", + "this parameter to 0 disables the limit." H2_RR_INFO, /* l-text */ "", /* func */ NULL ) @@ -1939,9 +1943,9 @@ PARAM( /* max */ NULL, /* default */ "60.000", /* units */ "seconds", - /* flags */ EXPERIMENTAL|WIZARD, + /* flags */ EXPERIMENTAL | DELAYED_EFFECT | WIZARD, /* s-text */ - "HTTP2 sliding window duration for h2_rapid_reset_limit.", + "HTTP2 sliding window duration for h2_rapid_reset_limit." H2_RR_INFO, /* l-text */ "", /* func */ NULL ) From 3902850d15ade9a3fb7f28e74236401e254bb93d Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 18 Oct 2023 15:17:27 +0200 Subject: [PATCH 08/10] Add vmod_h2 to control rapid_reset parameters per session Backport of 2cfb561ed46a2db1e6c2ef9f88cbb8eebb792f25 and 1a406e3f9a085f9d9321242c94a99080eb2be525 Conflicts: bin/varnishtest/vmods.h configure.ac lib/Makefile.am --- bin/varnishtest/vmods.h | 1 + bin/varnishtest/vmodtests/h2/b00000.vtc | 90 +++++++++++++++++++++ configure.ac | 1 + lib/Makefile.am | 3 +- lib/libvmod_h2/Makefile.am | 3 + lib/libvmod_h2/automake_boilerplate.am | 37 +++++++++ lib/libvmod_h2/vmod.vcc | 88 +++++++++++++++++++++ lib/libvmod_h2/vmod_h2.c | 100 ++++++++++++++++++++++++ man/Makefile.am | 6 +- 9 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 bin/varnishtest/vmodtests/h2/b00000.vtc create mode 100644 lib/libvmod_h2/Makefile.am create mode 100644 lib/libvmod_h2/automake_boilerplate.am create mode 100644 lib/libvmod_h2/vmod.vcc create mode 100644 lib/libvmod_h2/vmod_h2.c diff --git a/bin/varnishtest/vmods.h b/bin/varnishtest/vmods.h index 06ab9b80c1e..81c12cbdb22 100644 --- a/bin/varnishtest/vmods.h +++ b/bin/varnishtest/vmods.h @@ -35,3 +35,4 @@ VTC_VMOD(vtc) VTC_VMOD(blob) VTC_VMOD(unix) VTC_VMOD(proxy) +VTC_VMOD(h2) diff --git a/bin/varnishtest/vmodtests/h2/b00000.vtc b/bin/varnishtest/vmodtests/h2/b00000.vtc new file mode 100644 index 00000000000..aedb6aad8c5 --- /dev/null +++ b/bin/varnishtest/vmodtests/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(10); + 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 == 100 + expect resp.http.rapid-reset-limit-n == 10 + expect resp.http.rapid-reset-period-o == 60.000 + expect resp.http.rapid-reset-period-n == 10.000 + expect resp.http.rapid-reset-budget == 10.000 + } -run +} -start + +client c1 -wait +client c2 -wait diff --git a/configure.ac b/configure.ac index b5e8b45ab69..d7c7c33aef4 100644 --- a/configure.ac +++ b/configure.ac @@ -784,6 +784,7 @@ AC_CONFIG_FILES([ lib/libvmod_blob/Makefile lib/libvmod_unix/Makefile lib/libvmod_proxy/Makefile + lib/libvmod_h2/Makefile man/Makefile varnishapi.pc varnishapi-uninstalled.pc diff --git a/lib/Makefile.am b/lib/Makefile.am index 907ee4a5e64..986abf89006 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -12,4 +12,5 @@ SUBDIRS = \ libvmod_vtc \ libvmod_blob \ libvmod_unix \ - libvmod_proxy + libvmod_proxy \ + libvmod_h2 diff --git a/lib/libvmod_h2/Makefile.am b/lib/libvmod_h2/Makefile.am new file mode 100644 index 00000000000..5013d1de1a4 --- /dev/null +++ b/lib/libvmod_h2/Makefile.am @@ -0,0 +1,3 @@ + +libvmod_h2_la_SOURCES = vmod_h2.c +include $(srcdir)/automake_boilerplate.am diff --git a/lib/libvmod_h2/automake_boilerplate.am b/lib/libvmod_h2/automake_boilerplate.am new file mode 100644 index 00000000000..cf7dc7b1b8b --- /dev/null +++ b/lib/libvmod_h2/automake_boilerplate.am @@ -0,0 +1,37 @@ + +# Boilerplate generated by vmodtool.py - changes will be overwritten + +AM_LDFLAGS = $(AM_LT_LDFLAGS) + +AM_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/bin/varnishd \ + -I$(top_builddir)/include + +vmoddir = $(pkglibdir)/vmods +vmodtool = $(top_srcdir)/lib/libvcc/vmodtool.py +vmodtoolargs = --strict --boilerplate + +vmod_LTLIBRARIES = libvmod_h2.la + +libvmod_h2_la_CFLAGS = + +libvmod_h2_la_LDFLAGS = \ + $(AM_LDFLAGS) \ + $(VMOD_LDFLAGS) + +nodist_libvmod_h2_la_SOURCES = vcc_if.c vcc_if.h + +$(libvmod_h2_la_OBJECTS): vcc_if.h + +vcc_if.h vmod_h2.rst vmod_h2.man.rst: vcc_if.c + +vcc_if.c: $(vmodtool) $(srcdir)/vmod.vcc + @PYTHON@ $(vmodtool) $(vmodtoolargs) $(srcdir)/vmod.vcc + +EXTRA_DIST = vmod.vcc automake_boilerplate.am + +CLEANFILES = $(builddir)/vcc_if.c $(builddir)/vcc_if.h \ + $(builddir)/vmod_h2.rst \ + $(builddir)/vmod_h2.man.rst + diff --git a/lib/libvmod_h2/vmod.vcc b/lib/libvmod_h2/vmod.vcc new file mode 100644 index 00000000000..65702aaf6d7 --- /dev/null +++ b/lib/libvmod_h2/vmod.vcc @@ -0,0 +1,88 @@ +#- +# 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() + +Returns true when called on a session handled by the built-in HTTP2 transport. + +$Function DURATION rapid_reset([DURATION threshold]) + +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]) + +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]) + +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() + +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)` diff --git a/lib/libvmod_h2/vmod_h2.c b/lib/libvmod_h2/vmod_h2.c new file mode 100644 index 00000000000..2153e979e20 --- /dev/null +++ b/lib/libvmod_h2/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_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 != &H2_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 vmod_##name##_arg *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/man/Makefile.am b/man/Makefile.am index 0e35dc5bf5f..1c2714872a6 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -21,7 +21,8 @@ dist_man_MANS = \ vmod_vtc.3 \ vmod_blob.3 \ vmod_unix.3 \ - vmod_proxy.3 + vmod_proxy.3 \ + vmod_h2.3 CLEANFILES = $(dist_man_MANS) @@ -109,4 +110,7 @@ vmod_unix.3: $(top_builddir)/lib/libvmod_unix/vmod_unix.man.rst vmod_proxy.3: $(top_builddir)/lib/libvmod_proxy/vmod_proxy.man.rst ${RST2MAN} $(RST2ANY_FLAGS) $? $@ +vmod_h2.3: $(top_builddir)/lib/libvmod_h2/vmod_h2.man.rst + ${RST2MAN} $(RST2ANY_FLAGS) $? $@ + .NOPATH: $(dist_man_MANS) From c2bc7d144de7f2cdd24f9851dfb499841266b5df Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Wed, 18 Oct 2023 15:32:32 +0200 Subject: [PATCH 09/10] vmod_h2: VRT_fail if called outside of client context --- lib/libvmod_h2/vmod_h2.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/libvmod_h2/vmod_h2.c b/lib/libvmod_h2/vmod_h2.c index 2153e979e20..009aafc6f47 100644 --- a/lib/libvmod_h2/vmod_h2.c +++ b/lib/libvmod_h2/vmod_h2.c @@ -43,7 +43,12 @@ h2get(VRT_CTX) uintptr_t *up; CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); - CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); // $Restrict client + if (ctx->req == NULL) { + VRT_fail(ctx, + "vmod_h2 can only be called from client-side VCL."); + return (NULL); + } + CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); if (ctx->req->transport != &H2_transport) return (NULL); AZ(SES_Get_proto_priv(ctx->req->sp, &up)); From 822995fc704aac8d8cc6d6463db1e6856de4284c Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Wed, 18 Oct 2023 15:17:11 +0200 Subject: [PATCH 10/10] Flexelinting we can not make the parameter const because API. --- lib/libvmod_h2/vmod_h2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/libvmod_h2/vmod_h2.c b/lib/libvmod_h2/vmod_h2.c index 009aafc6f47..cbab4d22e45 100644 --- a/lib/libvmod_h2/vmod_h2.c +++ b/lib/libvmod_h2/vmod_h2.c @@ -70,6 +70,8 @@ vmod_ ## name(VRT_CTX, struct vmod_##name##_arg *args) \ struct h2_sess *h2 = h2get(ctx); \ type r; \ \ + (void)args; \ + \ if (h2 == NULL) \ return (-1); \ \