[6.0] de3ed6b76 New 'h2_rxbuf_storage' param to set rxbuf stevedore
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Wed Oct 18 09:21:09 UTC 2023
commit de3ed6b760bb8fd9a69bbe696cee8ebff81ec709
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date: Wed Aug 11 17:16:13 2021 +0200
New 'h2_rxbuf_storage' param to set rxbuf stevedore
This parameter allows the user to choose which storage backend / stevedore
that the H/2 receive buffers are allocated from. By default it uses
Transient.
Conflicts:
bin/varnishd/mgt/mgt.h
bin/varnishd/mgt/mgt_param.h
bin/varnishd/mgt/mgt_param_tbl.c
include/tbl/params.h
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index eeef75c8b..df3c041aa 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -933,7 +933,8 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
/* Cap the buffer size when we know this is the
* single data frame. */
bufsize = len;
- stvbuf = STV_AllocBuf(wrk, stv_transient,
+ CHECK_OBJ_NOTNULL(stv_h2_rxbuf, STEVEDORE_MAGIC);
+ stvbuf = STV_AllocBuf(wrk, stv_h2_rxbuf,
bufsize + sizeof *rxbuf);
if (stvbuf == NULL) {
VSLb(h2->vsl, SLT_Debug,
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index 7575de2d8..3d10c29df 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -202,6 +202,7 @@ char **MGT_NamedArg(const char *, const char **, const char *);
/* stevedore_mgt.c */
+extern const char *mgt_stv_h2_rxbuf;
void STV_Config(const char *spec);
void STV_Config_Transient(void);
diff --git a/bin/varnishd/mgt/mgt_param.h b/bin/varnishd/mgt/mgt_param.h
index 7188b64e1..bec4b0c6b 100644
--- a/bin/varnishd/mgt/mgt_param.h
+++ b/bin/varnishd/mgt/mgt_param.h
@@ -67,6 +67,7 @@ tweak_t tweak_timeout;
tweak_t tweak_uint;
tweak_t tweak_vsl_buffer;
tweak_t tweak_vsl_reclen;
+tweak_t tweak_h2_rxbuf_storage;
int tweak_generic_uint(struct vsb *vsb, volatile unsigned *dest,
const char *arg, const char *min, const char *max);
diff --git a/bin/varnishd/mgt/mgt_param_tbl.c b/bin/varnishd/mgt/mgt_param_tbl.c
index 2bd7bb92e..f6ed13382 100644
--- a/bin/varnishd/mgt/mgt_param_tbl.c
+++ b/bin/varnishd/mgt/mgt_param_tbl.c
@@ -152,6 +152,12 @@ struct parspec mgt_parspec[] = {
0,
"255b",
"bytes" },
+ { "h2_rxbuf_storage", tweak_h2_rxbuf_storage, &mgt_stv_h2_rxbuf,
+ NULL, NULL,
+ "The name of the storage backend that HTTP/2 receive buffers"
+ " should be allocated from.",
+ MUST_RESTART,
+ "Transient", "" },
{ NULL, NULL, NULL }
};
diff --git a/bin/varnishd/mgt/mgt_param_tweak.c b/bin/varnishd/mgt/mgt_param_tweak.c
index 01cab5f6b..b3b3d8044 100644
--- a/bin/varnishd/mgt/mgt_param_tweak.c
+++ b/bin/varnishd/mgt/mgt_param_tweak.c
@@ -40,6 +40,7 @@
#include "mgt/mgt.h"
#include "mgt/mgt_param.h"
+#include "storage/storage.h"
#include "vav.h"
#include "vnum.h"
@@ -440,3 +441,43 @@ tweak_poolparam(struct vsb *vsb, const struct parspec *par, const char *arg)
}
return (retval);
}
+
+/*--------------------------------------------------------------------
+ * Tweak 'h2_rxbuf_storage'
+ *
+ */
+
+int v_matchproto_(tweak_t)
+tweak_h2_rxbuf_storage(struct vsb *vsb, const struct parspec *par,
+ const char *arg)
+{
+ struct stevedore *stv;
+
+ /* XXX: If we want to remove the MUST_RESTART flag from the
+ * h2_rxbuf_storage parameter, we could have a mechanism here
+ * that when the child is running calls out through CLI to change
+ * the stevedore being used. */
+
+ if (arg == NULL || arg == JSON_FMT)
+ return (tweak_string(vsb, par, arg));
+
+ if (!strcmp(arg, "Transient")) {
+ /* Always allow setting to the special name
+ * "Transient". There will always be a stevedore with this
+ * name, but it may not have been configured at the time
+ * this is called. */
+ } else {
+ /* Only allow setting the value to a known configured
+ * stevedore */
+ STV_Foreach(stv) {
+ CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
+ if (!strcmp(stv->ident, arg))
+ break;
+ }
+ if (stv == NULL) {
+ VSB_printf(vsb, "unknown storage backend '%s'", arg);
+ return (-1);
+ }
+ }
+ return (tweak_string(vsb, par, arg));
+}
diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index c204a3e53..2c95ba0f5 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -52,6 +52,8 @@ static VTAILQ_HEAD(, stevedore) stevedores =
struct stevedore *stv_transient;
+const char *mgt_stv_h2_rxbuf;
+
/*--------------------------------------------------------------------*/
int
@@ -245,7 +247,6 @@ STV_Config(const char *spec)
void
STV_Config_Transient(void)
{
-
ASSERT_MGT();
VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_stv);
diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c
index 72949db42..a6c124fc4 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -41,6 +41,8 @@
#include "storage/storage.h"
#include "vrt_obj.h"
+extern const char *mgt_stv_h2_rxbuf;
+struct stevedore *stv_h2_rxbuf = NULL;
static pthread_mutex_t stv_mtx;
@@ -171,13 +173,23 @@ STV_open(void)
ASSERT_CLI();
AZ(pthread_mutex_init(&stv_mtx, NULL));
+
+ /* This string was prepared for us before the fork, and should
+ * point to a configured stevedore. */
+ AN(mgt_stv_h2_rxbuf);
+
+ stv_h2_rxbuf = NULL;
STV_Foreach(stv) {
+ CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
bprintf(buf, "storage.%s", stv->ident);
stv->vclname = strdup(buf);
AN(stv->vclname);
if (stv->open != NULL)
stv->open(stv);
+ if (!strcmp(stv->ident, mgt_stv_h2_rxbuf))
+ stv_h2_rxbuf = stv;
}
+ AN(stv_h2_rxbuf);
}
void
diff --git a/bin/varnishd/storage/storage.h b/bin/varnishd/storage/storage.h
index 53208236c..7d7345b9b 100644
--- a/bin/varnishd/storage/storage.h
+++ b/bin/varnishd/storage/storage.h
@@ -130,6 +130,7 @@ struct stevedore {
};
extern struct stevedore *stv_transient;
+extern struct stevedore *stv_h2_rxbuf;
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishtest/tests/t02022.vtc b/bin/varnishtest/tests/t02022.vtc
new file mode 100644
index 000000000..6a7ea5cc4
--- /dev/null
+++ b/bin/varnishtest/tests/t02022.vtc
@@ -0,0 +1,86 @@
+varnishtest "Test non-transient rxbuf stevedore with LRU nuking"
+
+barrier b1 sock 2 -cyclic
+
+server s1 {
+ rxreq
+ txresp -body asdf
+ rxreq
+ txresp -bodylen 1048000
+ rxreq
+ txresp -body ASDF
+} -start
+
+varnish v1 -arg "-srxbuf=malloc,1m -smain=malloc,1m" -vcl+backend {
+ import vtc;
+ sub vcl_recv {
+ if (req.url == "/1") {
+ vtc.barrier_sync("${b1_sock}");
+ }
+ }
+ sub vcl_backend_response {
+ if (bereq.url == "/2") {
+ set beresp.storage = storage.rxbuf;
+ } else {
+ set beresp.storage = storage.main;
+ }
+ }
+}
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.reset h2_initial_window_size"
+varnish v1 -cliok "param.reset h2_rx_window_low_water"
+varnish v1 -cliok "param.set h2_rxbuf_storage rxbuf"
+varnish v1 -cliok "param.set debug +syncvsl"
+
+varnish v1 -start
+
+client c1 {
+ stream 1 {
+ txreq -req POST -url /1 -hdr "content-length" "2048" -nostrend
+ txdata -datalen 2048
+ rxresp
+ expect resp.status == 200
+ } -start
+} -start
+
+varnish v1 -expect SMA.rxbuf.g_bytes >= 2048
+varnish v1 -expect SMA.Transient.g_bytes == 0
+varnish v1 -expect MAIN.n_lru_nuked == 0
+
+barrier b1 sync
+client c1 -wait
+
+varnish v1 -expect SMA.rxbuf.g_bytes == 0
+varnish v1 -expect SMA.Transient.g_bytes == 0
+varnish v1 -expect MAIN.n_lru_nuked == 0
+
+client c2 {
+ txreq -url /2
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 1048000
+} -run
+
+varnish v1 -expect SMA.rxbuf.g_bytes >= 1048000
+varnish v1 -expect MAIN.n_lru_nuked == 0
+
+client c3 {
+ stream 1 {
+ txreq -req POST -url /1 -hdr "content-length" "2048" -nostrend
+ txdata -datalen 2048
+ rxresp
+ expect resp.status == 200
+ } -start
+} -start
+
+varnish v1 -expect SMA.rxbuf.g_bytes >= 2048
+varnish v1 -expect SMA.rxbuf.g_bytes < 3000
+varnish v1 -expect SMA.Transient.g_bytes == 0
+varnish v1 -expect MAIN.n_lru_nuked == 1
+
+barrier b1 sync
+client c3 -wait
+
+varnish v1 -expect SMA.rxbuf.g_bytes == 0
+varnish v1 -expect SMA.Transient.g_bytes == 0
diff --git a/include/tbl/params.h b/include/tbl/params.h
index 7a9cf2897..59280f9cc 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -1878,6 +1878,24 @@ PARAM(
/* func */ NULL
)
+#if 0
+/* actual location mgt_param_tbl.c */
+PARAM(
+ /* name */ h2_rxbuf_storage,
+ /* typ */ h2_rxbuf_storage,
+ /* min */ NULL,
+ /* max */ NULL,
+ /* default */ "Transient",
+ /* units */ NULL,
+ /* flags */ MUST_RESTART,
+ /* s-text */
+ "The name of the storage backend that HTTP/2 receive buffers"
+ " should be allocated from.",
+ /* l-text */ "",
+ /* func */ NULL
+)
+#endif
+
#undef PARAM
/*lint -restore */
More information about the varnish-commit
mailing list