From 29a492c3ec53ab5e1ed61708d0362ce0775f774b Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Thu, 31 Oct 2024 10:08:12 +0100 Subject: [PATCH] cache_http1_line: Make the v1l VDP a bit less special V1L is used to send HTTP headers and the body, so it is used directly from delivery/fetch first and then as a VDP. From the times before VDPs, the V1L VDP still had its private pointer in struct wrk. This commit is to move the private pointer to the VDP entry. The caller remains responsible for calling V1L_Close() to keep V1L independent of VDP. --- bin/varnishd/cache/cache.h | 2 - bin/varnishd/cache/cache_varnishd.h | 3 +- bin/varnishd/http1/cache_http1.h | 10 ++-- bin/varnishd/http1/cache_http1_deliver.c | 33 ++++++------- bin/varnishd/http1/cache_http1_fetch.c | 19 +++----- bin/varnishd/http1/cache_http1_fsm.c | 1 - bin/varnishd/http1/cache_http1_line.c | 62 +++++++++++++----------- bin/varnishd/http1/cache_http1_proto.c | 20 ++++---- bin/varnishd/http2/cache_http2_deliver.c | 3 -- 9 files changed, 74 insertions(+), 79 deletions(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index b52cbab01a9..d9cd0094f37 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -252,8 +252,6 @@ struct worker { vtim_real lastused; - struct v1l *v1l; - pthread_cond_t cond; struct ws aws[1]; diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index 31fd57eb6e4..a0e4987faba 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -300,7 +300,8 @@ htc_complete_f HTTP1_Complete; uint16_t HTTP1_DissectRequest(struct http_conn *, struct http *); uint16_t HTTP1_DissectResponse(struct http_conn *, struct http *resp, const struct http *req); -unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*); +struct v1l; +unsigned HTTP1_Write(struct v1l *v1l, const struct http *hp, const int*); /* cache_main.c */ vxid_t VXID_Get(const struct worker *, uint64_t marker); diff --git a/bin/varnishd/http1/cache_http1.h b/bin/varnishd/http1/cache_http1.h index 8c161d5f54d..98ed1e1dfc8 100644 --- a/bin/varnishd/http1/cache_http1.h +++ b/bin/varnishd/http1/cache_http1.h @@ -59,11 +59,11 @@ stream_close_t V1P_Process(const struct req *, int fd, struct v1p_acct *, void V1P_Charge(struct req *, const struct v1p_acct *, struct VSC_vbe *); /* cache_http1_line.c */ -void V1L_Chunked(const struct worker *w); -void V1L_EndChunk(const struct worker *w); +void V1L_Chunked(struct v1l *v1l); +void V1L_EndChunk(struct v1l *v1l); struct v1l * V1L_Open(struct ws *, int *fd, struct vsl_log *, vtim_real deadline, unsigned niov); -stream_close_t V1L_Flush(const struct worker *w); -stream_close_t V1L_Close(struct worker *w, uint64_t *cnt); -size_t V1L_Write(const struct worker *w, const void *ptr, ssize_t len); +stream_close_t V1L_Flush(struct v1l *v1l); +stream_close_t V1L_Close(struct v1l **v1lp, uint64_t *cnt); +size_t V1L_Write(struct v1l *v1l, const void *ptr, ssize_t len); extern const struct vdp * const VDP_v1l; diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c index 6bce2558c22..462a9357f55 100644 --- a/bin/varnishd/http1/cache_http1_deliver.c +++ b/bin/varnishd/http1/cache_http1_deliver.c @@ -40,7 +40,7 @@ /*--------------------------------------------------------------------*/ static void -v1d_error(struct req *req, struct boc *boc, const char *msg) +v1d_error(struct req *req, struct boc *boc, struct v1l **v1lp, const char *msg) { static const char r_500[] = "HTTP/1.1 500 Internal Server Error\r\n" @@ -48,8 +48,9 @@ v1d_error(struct req *req, struct boc *boc, const char *msg) "Connection: close\r\n\r\n"; uint64_t bytes; - if (req->wrk->v1l != NULL) - (void) V1L_Close(req->wrk, &bytes); + AN(v1lp); + if (*v1lp != NULL) + (void) V1L_Close(v1lp, &bytes); VSLbs(req->vsl, SLT_Error, TOSTRAND(msg)); VSLb(req->vsl, SLT_RespProtocol, "HTTP/1.1"); @@ -97,13 +98,10 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody) cache_param->http1_iovs); if (v1l == NULL) { - v1d_error(req, boc, "Failure to init v1d (workspace_thread overflow)"); + v1d_error(req, boc, &v1l, "Failure to init v1d (workspace_thread overflow)"); return; } - AZ(req->wrk->v1l); - req->wrk->v1l = v1l; - if (sendbody) { if (!http_GetHdr(req->resp, H_Content_Length, NULL)) { if (req->http->protover == 11) { @@ -116,41 +114,40 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody) } INIT_OBJ(ctx, VRT_CTX_MAGIC); VCL_Req2Ctx(ctx, req); - if (VDP_Push(ctx, req->vdc, req->ws, VDP_v1l, NULL)) { - v1d_error(req, boc, "Failure to push v1d processor"); + if (VDP_Push(ctx, req->vdc, req->ws, VDP_v1l, v1l)) { + v1d_error(req, boc, &v1l, "Failure to push v1d processor"); return; } } if (WS_Overflowed(req->ws)) { - v1d_error(req, boc, "workspace_client overflow"); + v1d_error(req, boc, &v1l, "workspace_client overflow"); return; } if (WS_Overflowed(req->sp->ws)) { - v1d_error(req, boc, "workspace_session overflow"); + v1d_error(req, boc, &v1l, "workspace_session overflow"); return; } if (WS_Overflowed(req->wrk->aws)) { - v1d_error(req, boc, "workspace_thread overflow"); + v1d_error(req, boc, &v1l, "workspace_thread overflow"); return; } - hdrbytes = HTTP1_Write(req->wrk, req->resp, HTTP1_Resp); + hdrbytes = HTTP1_Write(v1l, req->resp, HTTP1_Resp); if (sendbody) { if (DO_DEBUG(DBG_FLUSH_HEAD)) - (void)V1L_Flush(req->wrk); + (void)V1L_Flush(v1l); if (chunked) - V1L_Chunked(req->wrk); + V1L_Chunked(v1l); err = VDP_DeliverObj(req->vdc, req->objcore); if (!err && chunked) - V1L_EndChunk(req->wrk); + V1L_EndChunk(v1l); } - sc = V1L_Close(req->wrk, &bytes); - AZ(req->wrk->v1l); + sc = V1L_Close(&v1l, &bytes); req->acct.resp_hdrbytes += hdrbytes; req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc); diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c index 817f7a969bd..e0c53b96bad 100644 --- a/bin/varnishd/http1/cache_http1_fetch.c +++ b/bin/varnishd/http1/cache_http1_fetch.c @@ -43,13 +43,13 @@ #include "cache_http1.h" static int -v1f_stackv1l(struct vdp_ctx *vdc, struct busyobj *bo) +v1f_stackv1l(struct vdp_ctx *vdc, struct busyobj *bo, struct v1l *v1l) { struct vrt_ctx ctx[1]; INIT_OBJ(ctx, VRT_CTX_MAGIC); VCL_Bo2Ctx(ctx, bo); - return (VDP_Push(ctx, vdc, ctx->ws, VDP_v1l, NULL)); + return (VDP_Push(ctx, vdc, ctx->ws, VDP_v1l, v1l)); } /*-------------------------------------------------------------------- @@ -108,15 +108,12 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes, */ err = "Failure to open V1L (workspace_thread overflow)"; } - else if (v1f_stackv1l(vdc, bo)) + else if (v1f_stackv1l(vdc, bo, v1l)) err = "Failure to push V1L"; - AZ(wrk->v1l); - wrk->v1l = v1l; - if (err != NULL) { if (v1l != NULL) - (void) V1L_Close(wrk, &bytes); + (void) V1L_Close(&v1l, &bytes); if (VALID_OBJ(vdc, VDP_CTX_MAGIC)) (void) VDP_Close(vdc, NULL, NULL); VSLb(bo->vsl, SLT_FetchError, "%s", err); @@ -131,7 +128,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes, http_PrintfHeader(hp, "Transfer-Encoding: chunked"); VTCP_blocking(*htc->rfd); /* XXX: we should timeout instead */ - hdrbytes = HTTP1_Write(wrk, hp, HTTP1_Req); + hdrbytes = HTTP1_Write(v1l, hp, HTTP1_Req); /* Deal with any message-body the request might (still) have */ i = 0; @@ -144,7 +141,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes, } else if (bo->req != NULL && bo->req->req_body_status != BS_NONE) { if (cl < 0) - V1L_Chunked(wrk); + V1L_Chunked(v1l); i = VRB_Iterate(wrk, bo->vsl, bo->req, VDP_ObjIterate, vdc); if (bo->req->req_body_status != BS_CACHED) @@ -167,10 +164,10 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes, bo->req->doclose = SC_RX_BODY; } if (cl < 0) - V1L_EndChunk(wrk); + V1L_EndChunk(v1l); } - sc = V1L_Close(wrk, &bytes); + sc = V1L_Close(&v1l, &bytes); CHECK_OBJ_NOTNULL(sc, STREAM_CLOSE_MAGIC); /* Bytes accounting */ diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c index cc342036284..93c04dc6cb3 100644 --- a/bin/varnishd/http1/cache_http1_fsm.c +++ b/bin/varnishd/http1/cache_http1_fsm.c @@ -85,7 +85,6 @@ http1_req(struct worker *wrk, void *arg) req->transport = &HTTP1_transport; assert(!WS_IsReserved(wrk->aws)); HTTP1_Session(wrk, req); - AZ(wrk->v1l); WS_Assert(wrk->aws); THR_SetRequest(NULL); } diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c index d18bd6d61f8..771490986d9 100644 --- a/bin/varnishd/http1/cache_http1_line.c +++ b/bin/varnishd/http1/cache_http1_line.c @@ -68,6 +68,7 @@ struct v1l { ssize_t cnt; /* Flushed byte count */ struct ws *ws; uintptr_t ws_snap; + void **vdp_priv; }; /*-------------------------------------------------------------------- @@ -123,17 +124,20 @@ V1L_Open(struct ws *ws, int *fd, struct vsl_log *vsl, } stream_close_t -V1L_Close(struct worker *wrk, uint64_t *cnt) +V1L_Close(struct v1l **v1lp, uint64_t *cnt) { struct v1l *v1l; struct ws *ws; uintptr_t ws_snap; stream_close_t sc; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); AN(cnt); - sc = V1L_Flush(wrk); - TAKE_OBJ_NOTNULL(v1l, &wrk->v1l, V1L_MAGIC); + TAKE_OBJ_NOTNULL(v1l, v1lp, V1L_MAGIC); + if (v1l->vdp_priv != NULL) { + assert(*v1l->vdp_priv == v1l); + *v1l->vdp_priv = NULL; + } + sc = V1L_Flush(v1l); *cnt = v1l->cnt; ws = v1l->ws; ws_snap = v1l->ws_snap; @@ -167,15 +171,12 @@ v1l_prune(struct v1l *v1l, size_t bytes) } stream_close_t -V1L_Flush(const struct worker *wrk) +V1L_Flush(struct v1l *v1l) { ssize_t i; int err; - struct v1l *v1l; char cbuf[32]; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - v1l = wrk->v1l; CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC); CHECK_OBJ_NOTNULL(v1l->werr, STREAM_CLOSE_MAGIC); AN(v1l->wfd); @@ -261,12 +262,9 @@ V1L_Flush(const struct worker *wrk) } size_t -V1L_Write(const struct worker *wrk, const void *ptr, ssize_t len) +V1L_Write(struct v1l *v1l, const void *ptr, ssize_t len) { - struct v1l *v1l; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - v1l = wrk->v1l; CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC); AN(v1l->wfd); if (len == 0 || *v1l->wfd < 0) @@ -280,19 +278,16 @@ V1L_Write(const struct worker *wrk, const void *ptr, ssize_t len) v1l->niov++; v1l->cliov += len; if (v1l->niov >= v1l->siov) { - (void)V1L_Flush(wrk); + (void)V1L_Flush(v1l); VSC_C_main->http1_iovs_flush++; } return (len); } void -V1L_Chunked(const struct worker *wrk) +V1L_Chunked(struct v1l *v1l) { - struct v1l *v1l; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - v1l = wrk->v1l; CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC); assert(v1l->ciov == v1l->siov); @@ -302,7 +297,7 @@ V1L_Chunked(const struct worker *wrk) * a chunk tail, we might as well flush right away. */ if (v1l->niov + 3 >= v1l->siov) { - (void)V1L_Flush(wrk); + (void)V1L_Flush(v1l); VSC_C_main->http1_iovs_flush++; } v1l->siov--; @@ -320,27 +315,39 @@ V1L_Chunked(const struct worker *wrk) */ void -V1L_EndChunk(const struct worker *wrk) +V1L_EndChunk(struct v1l *v1l) { - struct v1l *v1l; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - v1l = wrk->v1l; CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC); assert(v1l->ciov < v1l->siov); - (void)V1L_Flush(wrk); + (void)V1L_Flush(v1l); v1l->siov++; v1l->ciov = v1l->siov; v1l->niov = 0; v1l->cliov = 0; - (void)V1L_Write(wrk, "0\r\n\r\n", -1); + (void)V1L_Write(v1l, "0\r\n\r\n", -1); } /*-------------------------------------------------------------------- * VDP using V1L */ +/* remember priv pointer for V1L_Close() to clear */ +static int v_matchproto_(vdp_init_f) +v1l_init(VRT_CTX, struct vdp_ctx *vdc, void **priv) +{ + struct v1l *v1l; + + (void) ctx; + (void) vdc; + AN(priv); + CAST_OBJ_NOTNULL(v1l, *priv, V1L_MAGIC); + + v1l->vdp_priv = priv; + return (0); +} + static int v_matchproto_(vdp_bytes_f) v1l_bytes(struct vdp_ctx *vdc, enum vdp_action act, void **priv, const void *ptr, ssize_t len) @@ -348,13 +355,13 @@ v1l_bytes(struct vdp_ctx *vdc, enum vdp_action act, void **priv, ssize_t wl = 0; CHECK_OBJ_NOTNULL(vdc, VDP_CTX_MAGIC); - (void)priv; + AN(priv); AZ(vdc->nxt); /* always at the bottom of the pile */ if (len > 0) - wl = V1L_Write(vdc->wrk, ptr, len); - if (act > VDP_NULL && V1L_Flush(vdc->wrk) != SC_NULL) + wl = V1L_Write(*priv, ptr, len); + if (act > VDP_NULL && V1L_Flush(*priv) != SC_NULL) return (-1); if (len != wl) return (-1); @@ -363,5 +370,6 @@ v1l_bytes(struct vdp_ctx *vdc, enum vdp_action act, void **priv, const struct vdp * const VDP_v1l = &(struct vdp){ .name = "V1B", + .init = v1l_init, .bytes = v1l_bytes, }; diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c index f56a29b5271..acb8e11dd54 100644 --- a/bin/varnishd/http1/cache_http1_proto.c +++ b/bin/varnishd/http1/cache_http1_proto.c @@ -465,23 +465,21 @@ HTTP1_DissectResponse(struct http_conn *htc, struct http *hp, /*--------------------------------------------------------------------*/ static unsigned -http1_WrTxt(const struct worker *wrk, const txt *hh, const char *suf) +http1_WrTxt(struct v1l *v1l, const txt *hh, const char *suf) { unsigned u; - CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - AN(wrk); AN(hh); AN(hh->b); AN(hh->e); - u = V1L_Write(wrk, hh->b, hh->e - hh->b); + u = V1L_Write(v1l, hh->b, hh->e - hh->b); if (suf != NULL) - u += V1L_Write(wrk, suf, -1); + u += V1L_Write(v1l, suf, -1); return (u); } unsigned -HTTP1_Write(const struct worker *w, const struct http *hp, const int *hf) +HTTP1_Write(struct v1l *v1l, const struct http *hp, const int *hf) { unsigned u, l; @@ -489,12 +487,12 @@ HTTP1_Write(const struct worker *w, const struct http *hp, const int *hf) AN(hp->hd[hf[0]].b); AN(hp->hd[hf[1]].b); AN(hp->hd[hf[2]].b); - l = http1_WrTxt(w, &hp->hd[hf[0]], " "); - l += http1_WrTxt(w, &hp->hd[hf[1]], " "); - l += http1_WrTxt(w, &hp->hd[hf[2]], "\r\n"); + l = http1_WrTxt(v1l, &hp->hd[hf[0]], " "); + l += http1_WrTxt(v1l, &hp->hd[hf[1]], " "); + l += http1_WrTxt(v1l, &hp->hd[hf[2]], "\r\n"); for (u = HTTP_HDR_FIRST; u < hp->nhd; u++) - l += http1_WrTxt(w, &hp->hd[u], "\r\n"); - l += V1L_Write(w, "\r\n", -1); + l += http1_WrTxt(v1l, &hp->hd[u], "\r\n"); + l += V1L_Write(v1l, "\r\n", -1); return (l); } diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c index 5de2a7fc4ce..4c2f80480b0 100644 --- a/bin/varnishd/http2/cache_http2_deliver.c +++ b/bin/varnishd/http2/cache_http2_deliver.c @@ -331,8 +331,6 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody) sendbody = 0; } - AZ(req->wrk->v1l); - r2->t_send = req->t_prev; H2_Send_Get(req->wrk, r2->h2sess, r2); @@ -351,6 +349,5 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody) (void)VDP_DeliverObj(req->vdc, req->objcore); } - AZ(req->wrk->v1l); req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc); }