commit bb44b34d5e9078ede3769ef519badb65d340351a Author: Tomas Korbar Date: Wed Oct 18 12:32:24 2023 +0200 vcl_vrt: Skip VCL execution if the client is gone Upstream PR #3998 and 4991d9f6e40f381d058a83fc21ceed90e34a822e for r03996.vtc diff --git a/bin/varnishd/VSC_main.vsc b/bin/varnishd/VSC_main.vsc index d55b9df..0978c2f 100644 --- a/bin/varnishd/VSC_main.vsc +++ b/bin/varnishd/VSC_main.vsc @@ -342,6 +342,15 @@ Number of times an HTTP/2 stream was refused because the queue was too long already. See also parameter thread_queue_limit. +.. varnish_vsc:: req_reset + :group: wrk + :oneliner: Requests reset + + Number of times a client left before the VCL processing of its + requests completed. For HTTP/2 sessions, either the stream was + reset by an RST_STREAM frame from the client, or a stream or + connection error occurred. + .. varnish_vsc:: n_object :type: gauge :group: wrk diff --git a/bin/varnishd/cache/cache_transport.h b/bin/varnishd/cache/cache_transport.h index 3650291..be396b9 100644 --- a/bin/varnishd/cache/cache_transport.h +++ b/bin/varnishd/cache/cache_transport.h @@ -44,6 +44,7 @@ typedef void vtr_sess_panic_f (struct vsb *, const struct sess *); typedef void vtr_req_panic_f (struct vsb *, const struct req *); typedef void vtr_req_fail_f (struct req *, enum sess_close); typedef void vtr_reembark_f (struct worker *, struct req *); +typedef int vtr_poll_f (struct req *); typedef int vtr_minimal_response_f (struct req *, uint16_t status); struct transport { @@ -64,6 +65,7 @@ struct transport { vtr_sess_panic_f *sess_panic; vtr_req_panic_f *req_panic; vtr_reembark_f *reembark; + vtr_poll_f *poll; vtr_minimal_response_f *minimal_response; VTAILQ_ENTRY(transport) list; diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c index 023ba00..2fbaff6 100644 --- a/bin/varnishd/cache/cache_vrt_vcl.c +++ b/bin/varnishd/cache/cache_vrt_vcl.c @@ -42,6 +42,7 @@ #include "vbm.h" #include "cache_director.h" +#include "cache_transport.h" #include "cache_vcl.h" #include "vcc_interface.h" @@ -437,6 +438,40 @@ VRT_VCL_Allow_Discard(struct vclref **refp) FREE_OBJ(ref); } +/*-------------------------------------------------------------------- + */ + +static int +req_poll(struct worker *wrk, struct req *req) +{ + struct req *top; + + /* NB: Since a fail transition leads to vcl_synth, the request may be + * short-circuited twice. + */ + if (req->req_reset) { + wrk->handling = VCL_RET_FAIL; + return (-1); + } + + top = req->top->topreq; + CHECK_OBJ_NOTNULL(top, REQ_MAGIC); + CHECK_OBJ_NOTNULL(top->transport, TRANSPORT_MAGIC); + + if (!FEATURE(FEATURE_VCL_REQ_RESET)) + return (0); + if (top->transport->poll == NULL) + return (0); + if (top->transport->poll(top) >= 0) + return (0); + + VSLb_ts_req(req, "Reset", W_TIM_real(wrk)); + wrk->stats->req_reset++; + wrk->handling = VCL_RET_FAIL; + req->req_reset = 1; + return (-1); +} + /*-------------------------------------------------------------------- * Method functions to call into VCL programs. * @@ -468,6 +503,8 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo, CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC); CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC); CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC); + if (req_poll(wrk, req)) + return; VCL_Req2Ctx(&ctx, req); } assert(ctx.now != 0); diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c index f81c94a..f978763 100644 --- a/bin/varnishd/http2/cache_http2_session.c +++ b/bin/varnishd/http2/cache_http2_session.c @@ -439,6 +439,16 @@ h2_new_session(struct worker *wrk, void *arg) h2_del_sess(wrk, h2, h2->error->reason); } +static int v_matchproto_(vtr_poll_f) +h2_poll(struct req *req) +{ + struct h2_req *r2; + + CHECK_OBJ_NOTNULL(req, REQ_MAGIC); + CAST_OBJ_NOTNULL(r2, req->transport_priv, H2_REQ_MAGIC); + return (r2->error ? -1 : 1); +} + struct transport H2_transport = { .name = "H2", .magic = TRANSPORT_MAGIC, @@ -448,4 +458,5 @@ struct transport H2_transport = { .req_body = h2_req_body, .req_fail = h2_req_fail, .sess_panic = h2_sess_panic, + .poll = h2_poll, }; diff --git a/bin/varnishd/mgt/mgt_param_bits.c b/bin/varnishd/mgt/mgt_param_bits.c index d6a9c3f..6d9b32a 100644 --- a/bin/varnishd/mgt/mgt_param_bits.c +++ b/bin/varnishd/mgt/mgt_param_bits.c @@ -276,7 +276,7 @@ struct parspec VSL_parspec[] = { #undef DEBUG_BIT }, { "feature", tweak_feature, NULL, - NULL, NULL, "default", + NULL, NULL, "+validate_headers +vcl_req_reset", NULL, "Enable/Disable various minor features.\n" "\tdefault\tSet default value\n" diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc index 3fee370..7faf783 100644 --- a/bin/varnishtest/tests/r03996.vtc +++ b/bin/varnishtest/tests/r03996.vtc @@ -1,6 +1,7 @@ varnishtest "h2 rapid reset" -barrier b1 sock 5 +barrier b1 sock 2 -cyclic +barrier b2 sock 5 -cyclic server s1 { rxreq @@ -16,7 +17,10 @@ varnish v1 -vcl+backend { import vtc; sub vcl_recv { - vtc.barrier_sync("${b1_sock}"); + if (req.http.barrier) { + vtc.barrier_sync(req.http.barrier); + } + vtc.barrier_sync("${b2_sock}"); } } -start @@ -27,6 +31,41 @@ client c1 { 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 + +varnish v1 -expect sc_rapid_reset == 1 + +varnish v1 -cliok "param.set feature -vcl_req_reset" + +client c2 { + stream 0 { + rxgoaway + expect goaway.err == ENHANCE_YOUR_CALM + } -start + stream 1 { txreq txrst @@ -44,8 +83,8 @@ client c1 { txrst } -run - barrier b1 sync + barrier b2 sync stream 0 -wait } -run -varnish v1 -expect sc_rapid_reset == 1 +varnish v1 -expect sc_rapid_reset == 2 diff --git a/bin/varnishtest/tests/t02025.vtc b/bin/varnishtest/tests/t02025.vtc new file mode 100644 index 0000000..3b7e90e --- /dev/null +++ b/bin/varnishtest/tests/t02025.vtc @@ -0,0 +1,49 @@ +varnishtest "h2 reset interrupt" + +barrier b1 sock 2 +barrier b2 sock 2 + +varnish v1 -cliok "param.set feature +http2" +varnish v1 -cliok "param.set debug +syncvsl" +varnish v1 -vcl { + import vtc; + + backend be none; + + sub vcl_recv { + vtc.barrier_sync("${b1_sock}"); + vtc.barrier_sync("${b2_sock}"); + } + + sub vcl_miss { + vtc.panic("unreachable"); + } +} -start + +logexpect l1 -v v1 -g raw -i Debug { + expect * * Debug "^H2RXF RST_STREAM" +} -start + +client c1 { + stream 1 { + txreq + barrier b1 sync + txrst + } -run +} -start + +logexpect l1 -wait +barrier b2 sync + +varnish v1 -vsl_catchup +varnish v1 -expect req_reset == 1 + +# NB: The varnishncsa command below shows a minimal pattern to collect +# "rapid reset" suspects per session, with the IP address. Here rapid +# is interpreted as before a second elapsed. Session VXIDs showing up +# numerous times become increasingly more suspicious. The format can of +# course be extended to add anything else useful for data mining. +shell -expect "1000 ${localhost}" { + varnishncsa -n ${v1_name} -d \ + -q 'Timestamp:Reset[2] < 1.0' -F '%{VSL:Begin[2]}x %h' +} diff --git a/doc/sphinx/reference/vsl.rst b/doc/sphinx/reference/vsl.rst index cf63089..f1ed987 100644 --- a/doc/sphinx/reference/vsl.rst +++ b/doc/sphinx/reference/vsl.rst @@ -76,6 +76,11 @@ Resp Restart Client request is being restarted. +Reset + The client closed its connection, reset its stream or caused + a stream error that forced Varnish to reset the stream. Request + processing is interrupted and considered failed. + Pipe handling timestamps ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/include/tbl/feature_bits.h b/include/tbl/feature_bits.h index d51b22c..3d6ac35 100644 --- a/include/tbl/feature_bits.h +++ b/include/tbl/feature_bits.h @@ -82,6 +82,11 @@ FEATURE_BIT(BUSY_STATS_RATE, busy_stats_rate, "Make busy workers comply with thread_stats_rate." ) +FEATURE_BIT(VCL_REQ_RESET, vcl_req_reset, + "Stop processing client VCL once the client is gone. " + "When this happens MAIN.req_reset is incremented." +) + #undef FEATURE_BIT /*lint -restore */ diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h index 2e82660..9e72312 100644 --- a/include/tbl/req_flags.h +++ b/include/tbl/req_flags.h @@ -41,6 +41,7 @@ REQ_FLAG(is_hitpass, 1, 0, "") REQ_FLAG(waitinglist, 0, 0, "") REQ_FLAG(want100cont, 0, 0, "") REQ_FLAG(late100cont, 0, 0, "") +REQ_FLAG(req_reset, 0, 0, "") #undef REQ_FLAG /*lint -restore */