Hi Here's a suggestion (and a set of patches) for what PRIV_REQ/SESS could look like in vcl_backend_*, mostly based around IRC discussion.
The idea is to make PRIV_REQ available also in vcl_backend_*, and then give a separate priv object from the one used on the client side. Realizing this could lead to confusion, it was suggested to rename it PRIV_XID. The priv object will however survive a VCL restart/retry, so the name is not completely honest in that it is not restricted to a single XID. I think this is the behavior that makes the most sense, but I'm not super excited about the name. A second point is regarding PRIV_SESS. A concern with PRIV_SESS is that the remote Varnish is talking to very often tends to be not the client itself, but rather a load balancer dispatching requests from a bunch of clients over the same persistent connections. So if you were to introduce a priv_sess vmod that handles some sort of client state in such a scenario, you will very quickly shoot yourself in the foot and leak state across clients unless you know what you are doing. I think the problems solved by PRIV_SESS are typically solved much safer via PRIV_REQ. A point brought up by Tollef on IRC is that PRIV_SESS lets you share state between a request and its ESI subrequests, something that has legitimate use cases. Patch #5 (attached) drops PRIV_SESS and introduces PRIV_ESI, which restricts the scope to a request and its client-side subrequests. Feedback/comments/complaints very welcome. :-) regards, Dag -- Dag Haavi Finstad Software Developer | Varnish Software Mobile: +47 476 64 134 We Make Websites Fly!
From 4e511a3cca4c3f43d777535f0e82d867eb41e0bd Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad <[email protected]> Date: Tue, 18 Nov 2014 20:55:31 +0100 Subject: [PATCH 1/5] Add a test case for using PRIV_* in an ESI context, and fix a bug where we leak priv objects in ESI subrequests. --- bin/varnishd/cache/cache_esi_deliver.c | 1 + bin/varnishtest/tests/v00042.vtc | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 bin/varnishtest/tests/v00042.vtc diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c index 3e6e9d3..3c3c36d 100644 --- a/bin/varnishd/cache/cache_esi_deliver.c +++ b/bin/varnishd/cache/cache_esi_deliver.c @@ -147,6 +147,7 @@ ved_include(struct req *preq, const char *src, const char *host) (void)usleep(10000); } + VRTPRIV_dynamic_kill(req->sp, (uintptr_t)req); CNT_AcctLogCharge(wrk->stats, req); VSL_End(req->vsl); diff --git a/bin/varnishtest/tests/v00042.vtc b/bin/varnishtest/tests/v00042.vtc new file mode 100644 index 0000000..cf5ac86 --- /dev/null +++ b/bin/varnishtest/tests/v00042.vtc @@ -0,0 +1,58 @@ +varnishtest "Test PRIV_{SESS,REQ} in an ESI context." + +server s1 { + rxreq + expect req.url == "/a" + txresp -body { + <html> + <esi:include src="/foo"/> + } + + rxreq + expect req.url == "/foobar" + txresp + + rxreq + expect req.url == "/bar" + txresp +} -start + +varnish v1 -vcl+backend { + import ${vmod_debug}; + + sub vcl_recv { + if (req.url == "/a") { + set req.http.y0 = debug.test_priv_sess("bar"); + set req.http.x0 = debug.test_priv_req("baz"); + } else { + set req.url = req.url + debug.test_priv_sess("") + debug.test_priv_req(""); + } + } + + sub vcl_backend_response { + if (bereq.url == "/a") { + set beresp.do_esi = true; + } + } + + sub vcl_deliver { + set resp.http.y1 = debug.test_priv_sess(""); + set resp.http.x1 = debug.test_priv_req(""); + } +} -start + + +client c1 { + txreq -url /a + rxresp + expect resp.http.y1 == "bar" + expect resp.http.x1 == "baz" + + txreq -url / + rxresp + expect resp.http.y1 == "bar" + expect resp.http.x1 == "" + +} -run + +varnish v1 -expect s_req == 2 -- 2.1.3
From 2eb45889aa1e11ac63c05ea5ce0070f4a6419b39 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad <[email protected]> Date: Tue, 18 Nov 2014 21:11:38 +0100 Subject: [PATCH 2/5] PRIV_REQ -> PRIV_XID. Renamed to make the situation less confusing with regard to the fact that we will get a different PRIV context in vcl_backend_*. --- bin/varnishd/cache/cache_vrt_priv.c | 2 +- bin/varnishtest/tests/v00041.vtc | 6 +++--- bin/varnishtest/tests/v00042.vtc | 8 ++++---- include/vrt.h | 2 +- lib/libvcc/vcc_expr.c | 4 ++-- lib/libvcc/vmodtool.py | 2 +- lib/libvmod_debug/vmod.vcc | 2 +- lib/libvmod_debug/vmod_debug.c | 4 ++-- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c index 4998e74..4fec29f 100644 --- a/bin/varnishd/cache/cache_vrt_priv.c +++ b/bin/varnishd/cache/cache_vrt_priv.c @@ -94,7 +94,7 @@ VRTPRIV_dynamic_kill(struct sess *sp, uintptr_t id) } struct vmod_priv * -VRT_priv_req(VRT_CTX, void *vmod_id) +VRT_priv_xid(VRT_CTX, void *vmod_id) { return (VRT_priv_dynamic(ctx, (uintptr_t)ctx->req, (uintptr_t)vmod_id)); } diff --git a/bin/varnishtest/tests/v00041.vtc b/bin/varnishtest/tests/v00041.vtc index af951f9..b344e49 100644 --- a/bin/varnishtest/tests/v00041.vtc +++ b/bin/varnishtest/tests/v00041.vtc @@ -1,4 +1,4 @@ -varnishtest "Test priv_sess and priv_req" +varnishtest "Test priv_sess and priv_xid" server s1 { rxreq @@ -11,13 +11,13 @@ varnish v1 -vcl+backend { import ${vmod_debug}; sub vcl_recv { - set req.http.x0 = debug.test_priv_req(req.url); + set req.http.x0 = debug.test_priv_xid(req.url); set req.http.y0 = debug.test_priv_sess(req.url); } sub vcl_deliver { set resp.http.x0 = req.http.x0; - set resp.http.x1 = debug.test_priv_req(""); + set resp.http.x1 = debug.test_priv_xid(""); set resp.http.y0 = req.http.y0; set resp.http.y1 = debug.test_priv_sess(""); } diff --git a/bin/varnishtest/tests/v00042.vtc b/bin/varnishtest/tests/v00042.vtc index cf5ac86..ed77819 100644 --- a/bin/varnishtest/tests/v00042.vtc +++ b/bin/varnishtest/tests/v00042.vtc @@ -1,4 +1,4 @@ -varnishtest "Test PRIV_{SESS,REQ} in an ESI context." +varnishtest "Test PRIV_{SESS,XID} in an ESI context." server s1 { rxreq @@ -23,9 +23,9 @@ varnish v1 -vcl+backend { sub vcl_recv { if (req.url == "/a") { set req.http.y0 = debug.test_priv_sess("bar"); - set req.http.x0 = debug.test_priv_req("baz"); + set req.http.x0 = debug.test_priv_xid("baz"); } else { - set req.url = req.url + debug.test_priv_sess("") + debug.test_priv_req(""); + set req.url = req.url + debug.test_priv_sess("") + debug.test_priv_xid(""); } } @@ -37,7 +37,7 @@ varnish v1 -vcl+backend { sub vcl_deliver { set resp.http.y1 = debug.test_priv_sess(""); - set resp.http.x1 = debug.test_priv_req(""); + set resp.http.x1 = debug.test_priv_xid(""); } } -start diff --git a/include/vrt.h b/include/vrt.h index f9624a6..e5fcbe4 100644 --- a/include/vrt.h +++ b/include/vrt.h @@ -247,7 +247,7 @@ typedef int vmod_init_f(struct vmod_priv *, const struct VCL_conf *); void VRT_priv_fini(const struct vmod_priv *p); struct vmod_priv *VRT_priv_sess(VRT_CTX, void *vmod_id); -struct vmod_priv *VRT_priv_req(VRT_CTX, void *vmod_id); +struct vmod_priv *VRT_priv_xid(VRT_CTX, void *vmod_id); /* Stevedore related functions */ int VRT_Stv(const char *nm); diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c index 32e544c..8fa77b7 100644 --- a/lib/libvcc/vcc_expr.c +++ b/lib/libvcc/vcc_expr.c @@ -566,11 +566,11 @@ vcc_func(struct vcc *tl, struct expr **e, const char *cfunc, VSB_printf(ifp->fin, "\tVRT_priv_fini(&%s);", buf); e2 = vcc_mk_expr(VOID, "&%s", buf); p += strlen(p) + 1; - } else if (fmt == VOID && !strcmp(p, "PRIV_REQ")) { + } else if (fmt == VOID && !strcmp(p, "PRIV_XID")) { r = strchr(name, '.'); AN(r); e2 = vcc_mk_expr(VOID, - "VRT_priv_req(ctx, &VGC_vmod_%.*s)", + "VRT_priv_xid(ctx, &VGC_vmod_%.*s)", (int) (r - name), name); p += strlen(p) + 1; } else if (fmt == VOID && !strcmp(p, "PRIV_SESS")) { diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py index bff415a..1ce2fde 100755 --- a/lib/libvcc/vmodtool.py +++ b/lib/libvcc/vmodtool.py @@ -59,7 +59,7 @@ ctypes = { 'PRIV_CALL': "struct vmod_priv *", 'PRIV_VCL': "struct vmod_priv *", 'PRIV_SESS': "struct vmod_priv *", - 'PRIV_REQ': "struct vmod_priv *", + 'PRIV_XID': "struct vmod_priv *", 'REAL': "VCL_REAL", 'STRING': "VCL_STRING", 'STRING_LIST': "const char *, ...", diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc index 468fc51..f2d4284 100644 --- a/lib/libvmod_debug/vmod.vcc +++ b/lib/libvmod_debug/vmod.vcc @@ -51,7 +51,7 @@ $Function VOID test_priv_vcl(PRIV_VCL) Test function for VCL private pointers -$Function STRING test_priv_req(PRIV_REQ, STRING) +$Function STRING test_priv_xid(PRIV_XID, STRING) Test function for REQ private pointers diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c index 08b5aa8..cc182fd 100644 --- a/lib/libvmod_debug/vmod_debug.c +++ b/lib/libvmod_debug/vmod_debug.c @@ -100,8 +100,8 @@ vmod_test_priv_sess(VRT_CTX, struct vmod_priv *priv, VCL_STRING s) return (priv->priv); } -VCL_STRING __match_proto__(td_debug_test_priv_req) -vmod_test_priv_req(VRT_CTX, struct vmod_priv *priv, VCL_STRING s) +VCL_STRING __match_proto__(td_debug_test_priv_xid) +vmod_test_priv_xid(VRT_CTX, struct vmod_priv *priv, VCL_STRING s) { CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); -- 2.1.3
From 969574eab037d76d09c754f925bc8c2ea4a7f8f0 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad <[email protected]> Date: Tue, 18 Nov 2014 21:17:54 +0100 Subject: [PATCH 3/5] Make the VRTPRIV_* interface slightly more generic to let us use it without needing a req/sess pointer. --- bin/varnishd/cache/cache.h | 15 ++++++-- bin/varnishd/cache/cache_esi_deliver.c | 2 +- bin/varnishd/cache/cache_session.c | 4 +-- bin/varnishd/cache/cache_vrt_priv.c | 64 ++++++++++++++++++++-------------- bin/varnishd/http1/cache_http1_fsm.c | 2 +- 5 files changed, 53 insertions(+), 34 deletions(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index d8007c8..d69b4bd 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -119,7 +119,7 @@ struct sess; struct sesspool; struct vbc; struct vrt_backend; -struct vrt_privs; +struct vrt_priv; struct vsb; struct waitinglist; struct worker; @@ -299,6 +299,14 @@ struct vxid_pool { /*--------------------------------------------------------------------*/ +struct vrt_privs { + unsigned magic; +#define VRT_PRIVS_MAGIC 0x03ba7501 + VTAILQ_HEAD(,vrt_priv) privs; +}; + +/*--------------------------------------------------------------------*/ + struct wrk_accept { unsigned magic; #define WRK_ACCEPT_MAGIC 0x8c4b4d59 @@ -663,7 +671,7 @@ struct sess { double t_open; /* fd accepted */ double t_idle; /* fd accepted or resp sent */ - VTAILQ_HEAD(,vrt_privs) privs; + struct vrt_privs privs[1]; #if defined(HAVE_EPOLL_CTL) struct epoll_event ev; @@ -1046,7 +1054,8 @@ const char *VCL_Method_Name(unsigned); */ const char *VRT_String(struct ws *ws, const char *h, const char *p, va_list ap); char *VRT_StringList(char *d, unsigned dl, const char *p, va_list ap); -void VRTPRIV_dynamic_kill(struct sess *sp, uintptr_t id); +void VRTPRIV_init(struct vrt_privs *privs); +void VRTPRIV_dynamic_kill(struct vrt_privs *privs, uintptr_t id); void ESI_Deliver(struct req *); void ESI_DeliverChild(struct req *, struct busyobj *); diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c index 3c3c36d..39df537 100644 --- a/bin/varnishd/cache/cache_esi_deliver.c +++ b/bin/varnishd/cache/cache_esi_deliver.c @@ -147,7 +147,7 @@ ved_include(struct req *preq, const char *src, const char *host) (void)usleep(10000); } - VRTPRIV_dynamic_kill(req->sp, (uintptr_t)req); + VRTPRIV_dynamic_kill(req->sp->privs, (uintptr_t)req); CNT_AcctLogCharge(wrk->stats, req); VSL_End(req->vsl); diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c index 7749216..0308141 100644 --- a/bin/varnishd/cache/cache_session.c +++ b/bin/varnishd/cache/cache_session.c @@ -91,7 +91,7 @@ ses_new(struct sesspool *pp) sp->t_open = NAN; sp->t_idle = NAN; - VTAILQ_INIT(&sp->privs); + VRTPRIV_init(sp->privs); Lck_New(&sp->mtx, lck_sess); CHECK_OBJ_NOTNULL(sp, SESS_MAGIC); return (sp); @@ -300,7 +300,7 @@ SES_Delete(struct sess *sp, enum sess_close reason, double now) now = VTIM_real(); AZ(isnan(sp->t_open)); - VRTPRIV_dynamic_kill(sp, 0); + VRTPRIV_dynamic_kill(sp->privs, 0); VSL(SLT_SessClose, sp->vxid, "%s %.3f", sess_close_2str(sp->reason, 0), now - sp->t_open); VSL(SLT_End, sp->vxid, "%s", ""); diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c index 4fec29f..25267c2 100644 --- a/bin/varnishd/cache/cache_vrt_priv.c +++ b/bin/varnishd/cache/cache_vrt_priv.c @@ -40,10 +40,10 @@ #include "vcl.h" #include "vrt.h" -struct vrt_privs { +struct vrt_priv { unsigned magic; -#define VRT_PRIVS_MAGIC 0x24157a52 - VTAILQ_ENTRY(vrt_privs) list; +#define VRT_PRIV_MAGIC 0x24157a52 + VTAILQ_ENTRY(vrt_priv) list; struct vmod_priv priv[1]; const struct VCL_conf *vcl; uintptr_t id; @@ -53,44 +53,55 @@ struct vrt_privs { /*-------------------------------------------------------------------- */ +void +VRTPRIV_init(struct vrt_privs *privs) +{ + privs->magic = VRT_PRIVS_MAGIC; + VTAILQ_INIT(&privs->privs); +} + static struct vmod_priv * VRT_priv_dynamic(VRT_CTX, uintptr_t id, uintptr_t vmod_id) { - struct vrt_privs *vps; + struct vrt_priv *vp; CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); - VTAILQ_FOREACH(vps, &ctx->req->sp->privs, list) { - CHECK_OBJ_NOTNULL(vps, VRT_PRIVS_MAGIC); - if (vps->vcl == ctx->vcl && vps->id == id - && vps->vmod_id == vmod_id) - return (vps->priv); + CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); + CHECK_OBJ_NOTNULL(ctx->req->sp, SESS_MAGIC); + CHECK_OBJ_NOTNULL(ctx->req->sp->privs, VRT_PRIVS_MAGIC); + + VTAILQ_FOREACH(vp, &ctx->req->sp->privs->privs, list) { + CHECK_OBJ_NOTNULL(vp, VRT_PRIV_MAGIC); + if (vp->vcl == ctx->vcl && vp->id == id + && vp->vmod_id == vmod_id) + return (vp->priv); } - ALLOC_OBJ(vps, VRT_PRIVS_MAGIC); - AN(vps); - vps->vcl = ctx->vcl; - vps->id = id; - vps->vmod_id = vmod_id; - VTAILQ_INSERT_TAIL(&ctx->req->sp->privs, vps, list); - return (vps->priv); + ALLOC_OBJ(vp, VRT_PRIV_MAGIC); + AN(vp); + vp->vcl = ctx->vcl; + vp->id = id; + vp->vmod_id = vmod_id; + VTAILQ_INSERT_TAIL(&ctx->req->sp->privs->privs, vp, list); + return (vp->priv); } void -VRTPRIV_dynamic_kill(struct sess *sp, uintptr_t id) +VRTPRIV_dynamic_kill(struct vrt_privs *privs, uintptr_t id) { - struct vrt_privs *vps, *vps1; + struct vrt_priv *vp, *vp1; - CHECK_OBJ_NOTNULL(sp, SESS_MAGIC); + CHECK_OBJ_NOTNULL(privs, VRT_PRIVS_MAGIC); - VTAILQ_FOREACH_SAFE(vps, &sp->privs, list, vps1) { - CHECK_OBJ_NOTNULL(vps, VRT_PRIVS_MAGIC); - if (id == vps->id) { - VTAILQ_REMOVE(&sp->privs, vps, list); - VRT_priv_fini(vps->priv); - FREE_OBJ(vps); + VTAILQ_FOREACH_SAFE(vp, &privs->privs, list, vp1) { + CHECK_OBJ_NOTNULL(vp, VRT_PRIV_MAGIC); + if (id == vp->id) { + VTAILQ_REMOVE(&privs->privs, vp, list); + VRT_priv_fini(vp->priv); + FREE_OBJ(vp); } } if (id == 0) - assert(VTAILQ_EMPTY(&sp->privs)); + assert(VTAILQ_EMPTY(&privs->privs)); } struct vmod_priv * @@ -115,4 +126,3 @@ VRT_priv_fini(const struct vmod_priv *p) if (p->priv != (void*)0 && p->free != (void*)0) p->free(p->priv); } - diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c index a7ca699..29bd789 100644 --- a/bin/varnishd/http1/cache_http1_fsm.c +++ b/bin/varnishd/http1/cache_http1_fsm.c @@ -166,7 +166,7 @@ http1_cleanup(struct sess *sp, struct worker *wrk, struct req *req) req->vcl = NULL; } - VRTPRIV_dynamic_kill(sp, (uintptr_t)req); + VRTPRIV_dynamic_kill(sp->privs, (uintptr_t)req); /* Charge and log byte counters */ AN(req->vsl->wid); CNT_AcctLogCharge(wrk->stats, req); -- 2.1.3
From 6cff9a01f7528fc9269f26b44add9f9d7a6fdbd4 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad <[email protected]> Date: Tue, 18 Nov 2014 21:23:58 +0100 Subject: [PATCH 4/5] Introduce PRIV_{XID,SESS} in vcl_backend_*. --- bin/varnishd/cache/cache.h | 1 + bin/varnishd/cache/cache_busyobj.c | 6 ++++++ bin/varnishd/cache/cache_vrt_priv.c | 19 +++++++++++++------ bin/varnishtest/tests/r01038.vtc | 2 +- bin/varnishtest/tests/v00041.vtc | 22 ++++++++++++++++++++++ 5 files changed, 43 insertions(+), 7 deletions(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index d69b4bd..5ad4519 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -525,6 +525,7 @@ struct busyobj { struct vsb *synth_body; uint8_t digest[DIGEST_LEN]; + struct vrt_privs privs[1]; }; diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c index ad31e11..743bd71 100644 --- a/bin/varnishd/cache/cache_busyobj.c +++ b/bin/varnishd/cache/cache_busyobj.c @@ -154,6 +154,8 @@ VBO_GetBusyObj(struct worker *wrk, const struct req *req) memcpy(bo->digest, req->digest, sizeof bo->digest); + VRTPRIV_init(bo->privs); + return (bo); } @@ -190,6 +192,10 @@ VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo) AZ(bo->htc); + VRTPRIV_dynamic_kill(bo->privs, (uintptr_t)bo); + VRTPRIV_dynamic_kill(bo->privs, 0); + assert(VTAILQ_EMPTY(&bo->privs->privs)); + VSLb(bo->vsl, SLT_BereqAcct, "%ju %ju %ju %ju %ju %ju", (uintmax_t)bo->acct.bereq_hdrbytes, (uintmax_t)bo->acct.bereq_bodybytes, diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c index 25267c2..f103ae0 100644 --- a/bin/varnishd/cache/cache_vrt_priv.c +++ b/bin/varnishd/cache/cache_vrt_priv.c @@ -63,14 +63,20 @@ VRTPRIV_init(struct vrt_privs *privs) static struct vmod_priv * VRT_priv_dynamic(VRT_CTX, uintptr_t id, uintptr_t vmod_id) { + struct vrt_privs *vps; struct vrt_priv *vp; CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); - CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); - CHECK_OBJ_NOTNULL(ctx->req->sp, SESS_MAGIC); - CHECK_OBJ_NOTNULL(ctx->req->sp->privs, VRT_PRIVS_MAGIC); + if (ctx->req) { + CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); + CHECK_OBJ_NOTNULL(ctx->req->sp, SESS_MAGIC); + CAST_OBJ_NOTNULL(vps, ctx->req->sp->privs, VRT_PRIVS_MAGIC); + } else { + CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC); + CAST_OBJ_NOTNULL(vps, ctx->bo->privs, VRT_PRIVS_MAGIC); + } - VTAILQ_FOREACH(vp, &ctx->req->sp->privs->privs, list) { + VTAILQ_FOREACH(vp, &vps->privs, list) { CHECK_OBJ_NOTNULL(vp, VRT_PRIV_MAGIC); if (vp->vcl == ctx->vcl && vp->id == id && vp->vmod_id == vmod_id) @@ -81,7 +87,7 @@ VRT_priv_dynamic(VRT_CTX, uintptr_t id, uintptr_t vmod_id) vp->vcl = ctx->vcl; vp->id = id; vp->vmod_id = vmod_id; - VTAILQ_INSERT_TAIL(&ctx->req->sp->privs->privs, vp, list); + VTAILQ_INSERT_TAIL(&vps->privs, vp, list); return (vp->priv); } @@ -107,7 +113,8 @@ VRTPRIV_dynamic_kill(struct vrt_privs *privs, uintptr_t id) struct vmod_priv * VRT_priv_xid(VRT_CTX, void *vmod_id) { - return (VRT_priv_dynamic(ctx, (uintptr_t)ctx->req, (uintptr_t)vmod_id)); + uintptr_t p = (ctx->req ? (uintptr_t)ctx->req : (uintptr_t)ctx->bo); + return (VRT_priv_dynamic(ctx, p, (uintptr_t)vmod_id)); } struct vmod_priv * diff --git a/bin/varnishtest/tests/r01038.vtc b/bin/varnishtest/tests/r01038.vtc index bb57072..705e11c 100644 --- a/bin/varnishtest/tests/r01038.vtc +++ b/bin/varnishtest/tests/r01038.vtc @@ -45,7 +45,7 @@ server s1 { txresp -body "foo8" } -start -varnish v1 -arg "-p workspace_backend=9k" -vcl+backend { +varnish v1 -arg "-p workspace_backend=10k" -vcl+backend { sub vcl_backend_response { set beresp.do_esi = true; } diff --git a/bin/varnishtest/tests/v00041.vtc b/bin/varnishtest/tests/v00041.vtc index b344e49..2872565 100644 --- a/bin/varnishtest/tests/v00041.vtc +++ b/bin/varnishtest/tests/v00041.vtc @@ -21,6 +21,18 @@ varnish v1 -vcl+backend { set resp.http.y0 = req.http.y0; set resp.http.y1 = debug.test_priv_sess(""); } + + sub vcl_backend_fetch { + set bereq.http.bx0 = debug.test_priv_xid(bereq.url); + set bereq.http.by0 = debug.test_priv_sess(bereq.url); + } + + sub vcl_backend_response { + set beresp.http.bx0 = bereq.http.bx0; + set beresp.http.bx1 = debug.test_priv_xid(""); + set beresp.http.by0 = bereq.http.by0; + set beresp.http.by1 = debug.test_priv_sess(""); + } } -start @@ -31,6 +43,11 @@ client c1 { expect resp.http.x1 == /foobar expect resp.http.y0 == /foobar expect resp.http.y1 == /foobar + expect resp.http.bx0 == /foobar + expect resp.http.bx1 == /foobar + expect resp.http.by0 == /foobar + expect resp.http.by1 == /foobar + txreq -url /snafu rxresp @@ -38,4 +55,9 @@ client c1 { expect resp.http.x1 == /snafu expect resp.http.y0 == /foobar expect resp.http.y1 == /foobar + expect resp.http.bx0 == /snafu + expect resp.http.bx1 == /snafu + expect resp.http.by0 == /snafu + expect resp.http.by1 == /snafu + } -run -- 2.1.3
From 9a5fe8bfea93eaca910507202e59e89c2bcd5587 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad <[email protected]> Date: Tue, 18 Nov 2014 23:03:07 +0100 Subject: [PATCH 5/5] Drop PRIV_SESS and welcome PRIV_ESI. Unlike PRIV_SESS, PRIV_ESI will not make the priv object persist across requests, but it will persist for a request and its ESI subrequests. --- bin/varnishd/cache/cache_session.c | 2 +- bin/varnishd/cache/cache_vrt_priv.c | 3 +-- bin/varnishd/http1/cache_http1_fsm.c | 1 + bin/varnishtest/tests/v00041.vtc | 15 +++++++-------- bin/varnishtest/tests/v00042.vtc | 12 ++++++------ include/vrt.h | 2 +- lib/libvcc/vcc_expr.c | 4 ++-- lib/libvcc/vmodtool.py | 2 +- lib/libvmod_debug/vmod.vcc | 4 ++-- lib/libvmod_debug/vmod_debug.c | 4 ++-- 10 files changed, 24 insertions(+), 25 deletions(-) diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c index 0308141..60c893e 100644 --- a/bin/varnishd/cache/cache_session.c +++ b/bin/varnishd/cache/cache_session.c @@ -300,7 +300,7 @@ SES_Delete(struct sess *sp, enum sess_close reason, double now) now = VTIM_real(); AZ(isnan(sp->t_open)); - VRTPRIV_dynamic_kill(sp->privs, 0); + assert(VTAILQ_EMPTY(&sp->privs->privs)); VSL(SLT_SessClose, sp->vxid, "%s %.3f", sess_close_2str(sp->reason, 0), now - sp->t_open); VSL(SLT_End, sp->vxid, "%s", ""); diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c index f103ae0..5d5e131 100644 --- a/bin/varnishd/cache/cache_vrt_priv.c +++ b/bin/varnishd/cache/cache_vrt_priv.c @@ -97,7 +97,6 @@ VRTPRIV_dynamic_kill(struct vrt_privs *privs, uintptr_t id) struct vrt_priv *vp, *vp1; CHECK_OBJ_NOTNULL(privs, VRT_PRIVS_MAGIC); - VTAILQ_FOREACH_SAFE(vp, &privs->privs, list, vp1) { CHECK_OBJ_NOTNULL(vp, VRT_PRIV_MAGIC); if (id == vp->id) { @@ -118,7 +117,7 @@ VRT_priv_xid(VRT_CTX, void *vmod_id) } struct vmod_priv * -VRT_priv_sess(VRT_CTX, void *vmod_id) +VRT_priv_esi(VRT_CTX, void *vmod_id) { return (VRT_priv_dynamic(ctx, (uintptr_t)NULL, (uintptr_t)vmod_id)); } diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c index 29bd789..5325481 100644 --- a/bin/varnishd/http1/cache_http1_fsm.c +++ b/bin/varnishd/http1/cache_http1_fsm.c @@ -167,6 +167,7 @@ http1_cleanup(struct sess *sp, struct worker *wrk, struct req *req) } VRTPRIV_dynamic_kill(sp->privs, (uintptr_t)req); + VRTPRIV_dynamic_kill(sp->privs, 0); /* Charge and log byte counters */ AN(req->vsl->wid); CNT_AcctLogCharge(wrk->stats, req); diff --git a/bin/varnishtest/tests/v00041.vtc b/bin/varnishtest/tests/v00041.vtc index 2872565..8574534 100644 --- a/bin/varnishtest/tests/v00041.vtc +++ b/bin/varnishtest/tests/v00041.vtc @@ -1,4 +1,4 @@ -varnishtest "Test priv_sess and priv_xid" +varnishtest "Test priv_esi and priv_xid" server s1 { rxreq @@ -12,26 +12,26 @@ varnish v1 -vcl+backend { sub vcl_recv { set req.http.x0 = debug.test_priv_xid(req.url); - set req.http.y0 = debug.test_priv_sess(req.url); + set req.http.y0 = debug.test_priv_esi(req.url); } sub vcl_deliver { set resp.http.x0 = req.http.x0; set resp.http.x1 = debug.test_priv_xid(""); set resp.http.y0 = req.http.y0; - set resp.http.y1 = debug.test_priv_sess(""); + set resp.http.y1 = debug.test_priv_esi(""); } sub vcl_backend_fetch { set bereq.http.bx0 = debug.test_priv_xid(bereq.url); - set bereq.http.by0 = debug.test_priv_sess(bereq.url); + set bereq.http.by0 = debug.test_priv_esi(bereq.url); } sub vcl_backend_response { set beresp.http.bx0 = bereq.http.bx0; set beresp.http.bx1 = debug.test_priv_xid(""); set beresp.http.by0 = bereq.http.by0; - set beresp.http.by1 = debug.test_priv_sess(""); + set beresp.http.by1 = debug.test_priv_esi(""); } } -start @@ -48,13 +48,12 @@ client c1 { expect resp.http.by0 == /foobar expect resp.http.by1 == /foobar - txreq -url /snafu rxresp expect resp.http.x0 == /snafu expect resp.http.x1 == /snafu - expect resp.http.y0 == /foobar - expect resp.http.y1 == /foobar + expect resp.http.y0 == /snafu + expect resp.http.y1 == /snafu expect resp.http.bx0 == /snafu expect resp.http.bx1 == /snafu expect resp.http.by0 == /snafu diff --git a/bin/varnishtest/tests/v00042.vtc b/bin/varnishtest/tests/v00042.vtc index ed77819..4d221bb 100644 --- a/bin/varnishtest/tests/v00042.vtc +++ b/bin/varnishtest/tests/v00042.vtc @@ -1,4 +1,4 @@ -varnishtest "Test PRIV_{SESS,XID} in an ESI context." +varnishtest "Test PRIV_{ESI,XID} in an ESI context." server s1 { rxreq @@ -13,7 +13,7 @@ server s1 { txresp rxreq - expect req.url == "/bar" + expect req.url == "/" txresp } -start @@ -22,10 +22,10 @@ varnish v1 -vcl+backend { sub vcl_recv { if (req.url == "/a") { - set req.http.y0 = debug.test_priv_sess("bar"); + set req.http.y0 = debug.test_priv_esi("bar"); set req.http.x0 = debug.test_priv_xid("baz"); } else { - set req.url = req.url + debug.test_priv_sess("") + debug.test_priv_xid(""); + set req.url = req.url + debug.test_priv_esi("") + debug.test_priv_xid(""); } } @@ -36,7 +36,7 @@ varnish v1 -vcl+backend { } sub vcl_deliver { - set resp.http.y1 = debug.test_priv_sess(""); + set resp.http.y1 = debug.test_priv_esi(""); set resp.http.x1 = debug.test_priv_xid(""); } } -start @@ -50,7 +50,7 @@ client c1 { txreq -url / rxresp - expect resp.http.y1 == "bar" + expect resp.http.y1 == "" expect resp.http.x1 == "" } -run diff --git a/include/vrt.h b/include/vrt.h index e5fcbe4..5c68b7e 100644 --- a/include/vrt.h +++ b/include/vrt.h @@ -246,7 +246,7 @@ struct vmod_priv { typedef int vmod_init_f(struct vmod_priv *, const struct VCL_conf *); void VRT_priv_fini(const struct vmod_priv *p); -struct vmod_priv *VRT_priv_sess(VRT_CTX, void *vmod_id); +struct vmod_priv *VRT_priv_esi(VRT_CTX, void *vmod_id); struct vmod_priv *VRT_priv_xid(VRT_CTX, void *vmod_id); /* Stevedore related functions */ diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c index 8fa77b7..117cfbe 100644 --- a/lib/libvcc/vcc_expr.c +++ b/lib/libvcc/vcc_expr.c @@ -573,11 +573,11 @@ vcc_func(struct vcc *tl, struct expr **e, const char *cfunc, "VRT_priv_xid(ctx, &VGC_vmod_%.*s)", (int) (r - name), name); p += strlen(p) + 1; - } else if (fmt == VOID && !strcmp(p, "PRIV_SESS")) { + } else if (fmt == VOID && !strcmp(p, "PRIV_ESI")) { r = strchr(name, '.'); AN(r); e2 = vcc_mk_expr(VOID, - "VRT_priv_sess(ctx, &VGC_vmod_%.*s)", + "VRT_priv_esi(ctx, &VGC_vmod_%.*s)", (int) (r - name), name); p += strlen(p) + 1; } else if (fmt == ENUM) { diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py index 1ce2fde..9ba0f2c 100755 --- a/lib/libvcc/vmodtool.py +++ b/lib/libvcc/vmodtool.py @@ -58,7 +58,7 @@ ctypes = { 'IP': "VCL_IP", 'PRIV_CALL': "struct vmod_priv *", 'PRIV_VCL': "struct vmod_priv *", - 'PRIV_SESS': "struct vmod_priv *", + 'PRIV_ESI': "struct vmod_priv *", 'PRIV_XID': "struct vmod_priv *", 'REAL': "VCL_REAL", 'STRING': "VCL_STRING", diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc index f2d4284..6eee1eb 100644 --- a/lib/libvmod_debug/vmod.vcc +++ b/lib/libvmod_debug/vmod.vcc @@ -55,9 +55,9 @@ $Function STRING test_priv_xid(PRIV_XID, STRING) Test function for REQ private pointers -$Function STRING test_priv_sess(PRIV_SESS, STRING) +$Function STRING test_priv_esi(PRIV_ESI, STRING) -Test function for SESS private pointers +Test function for ESI private pointers $Function BLOB str2blob(STRING src) diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c index cc182fd..356163d 100644 --- a/lib/libvmod_debug/vmod_debug.c +++ b/lib/libvmod_debug/vmod_debug.c @@ -88,8 +88,8 @@ vmod_test_priv_call(VRT_CTX, struct vmod_priv *priv) } } -VCL_STRING __match_proto__(td_debug_test_priv_sess) -vmod_test_priv_sess(VRT_CTX, struct vmod_priv *priv, VCL_STRING s) +VCL_STRING __match_proto__(td_debug_test_priv_esi) +vmod_test_priv_esi(VRT_CTX, struct vmod_priv *priv, VCL_STRING s) { CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); -- 2.1.3
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
