Adjust documentation and add h2_rapid_reset parameter

Ref varnishcache#3996
nigoroll · Oct 16, 2023 · 19e8c44 · 19e8c44
1 parent bdf391b
commit 19e8c44
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 7 deletions.
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
@@ -336,7 +336,8 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 		return (0);
 	h2_kill_req(wrk, h2, r2, h2_streamerror(vbe32dec(h2->rxf_data)));
 
-	if (cache_param->h2_rapid_reset_limit == 0)
+	if (cache_param->h2_rapid_reset == 0 ||
+	    cache_param->h2_rapid_reset_limit == 0)
 		return (0);
 
 	now = VTIM_real();
@@ -353,7 +354,8 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 		Lck_Unlock(&h2->sess->mtx);
 		return (H2CE_ENHANCE_YOUR_CALM);
 	}
-	h2->rst_allowance -= 1.0;
+	if (d < cache_param->h2_rapid_reset)
+		h2->rst_allowance -= 1.0;
 
 	return (0);
 }

diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc
@@ -7,6 +7,7 @@ server s1 {
 
 varnish v1 -cliok "param.set feature +http2"
 varnish v1 -cliok "param.set h2_rapid_reset_limit 3"
+varnish v1 -cliok "param.set h2_rapid_reset 0.1"
 
 varnish v1 -vcl+backend {} -start
 

diff --git a/include/tbl/params.h b/include/tbl/params.h
@@ -1257,17 +1257,34 @@ PARAM_SIMPLE(
 	"HTTP2 maximum size of an uncompressed header list."
 )
 
+PARAM_SIMPLE(
+	/* name */	h2_rapid_reset,
+	/* typ */	timeout,
+	/* min */	"0.000",
+	/* max */	NULL,
+	/* def */	"0.001",
+	/* units */	NULL,
+	/* descr */
+	"Interval for classification as HTTP2 rapid reset.\n"
+	"Specifies the interval between successive HTTP2 RST_STREAM frames "
+	"to qualify for rate limiting by rapid_reset_limit over "
+	"h2_rapid_reset_period."
+	"Setting this parameter to 0 disables the limit.",
+	/* flags */	EXPERIMENTAL,
+)
+
 PARAM_SIMPLE(
 	/* name */	h2_rapid_reset_limit,
 	/* typ */	uint,
 	/* min */	"0",
 	/* max */	NULL,
-	/* def */	"0",
+	/* def */	"0",	// XXX SHOULD WE HAVE A SAFE DEFAULT ?
 	/* units */	NULL,
 	/* descr */
-	"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"
+	"HTTP2 rapid reset limit.\n"
+	"Specifies the maximum number of allowed stream resets classified as "
+	"\"rapid\" per the h2_rapid_reset prameter issued by\n"
+	"a client over h2_rapid_reset_period before the connection is closed.\n"
 	"Setting this parameter to 0 disables the limit.",
 	/* flags */	EXPERIMENTAL,
 )
@@ -1280,7 +1297,7 @@ PARAM_SIMPLE(
 	/* def */	"60.000",
 	/* units */	"seconds",
 	/* descr */
-	"HTTP2 RST allowance time period.",
+	"Period to which h2_rapid_reset_limit applies.",
 	/* flags */	EXPERIMENTAL|WIZARD,
 )