[master] ccc1339e6 Collapse body_status and req_body_status into one type.
Poul-Henning Kamp
phk at FreeBSD.org
Fri Feb 21 08:34:07 UTC 2020
commit ccc1339e6628d6e545b8504d0a230a11f261e930
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Fri Feb 21 08:28:54 2020 +0000
Collapse body_status and req_body_status into one type.
A lot of this is s/REQ_BODY_/BS_/g, followed by a reduction
pass to go from testing on specific status values to attributes
of the status value.
For instance the req.body caching code does not need to know if
how the transport sees this, chunked vs. eof for instance, it just
needs to know that there is a body and that we do not know the
length of it yet.
The transports own the "vocabulary", because internally H1 needs
to know the difference between BS_CHUNKED and BS_EOF in order to
setup the VFPs.
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 2b36acc0b..e661804e7 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -71,13 +71,6 @@ typedef const struct body_status *body_status_t;
/*--------------------------------------------------------------------*/
-enum req_body_state_e {
-#define REQ_BODY(U) REQ_BODY_##U,
-#include "tbl/req_body.h"
-};
-
-/*--------------------------------------------------------------------*/
-
enum sess_close {
SC_NULL = 0,
#define SESS_CLOSE(nm, stat, err, desc) SC_##nm,
@@ -464,7 +457,7 @@ struct req {
#define REQ_MAGIC 0x2751aaa1
enum req_step req_step;
- volatile enum req_body_state_e req_body_status;
+ body_status_t req_body_status;
enum sess_close doclose;
unsigned restarts;
unsigned esi_level;
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 301d8f6ed..e8d51637f 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -171,7 +171,7 @@ ved_include(struct req *preq, const char *src, const char *host,
/* Client content already taken care of */
http_Unset(req->http, H_Content_Length);
- req->req_body_status = REQ_BODY_NONE;
+ req->req_body_status = BS_NONE;
AZ(req->vcl);
AN(req->top);
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 0ac2b8338..613327c2d 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -240,18 +240,15 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
bo->ws_bo = WS_Snapshot(bo->ws);
HTTP_Clone(bo->bereq, bo->bereq0);
- if (bo->req->req_body_status == REQ_BODY_NONE) {
+ if (bo->req->req_body_status->avail == 0) {
bo->req = NULL;
ObjSetState(bo->wrk, bo->fetch_objcore, BOS_REQ_DONE);
- } else if (bo->req->req_body_status == REQ_BODY_CACHED) {
+ } else if (bo->req->req_body_status == BS_CACHED) {
AN(bo->req->body_oc);
bo->bereq_body = bo->req->body_oc;
HSH_Ref(bo->bereq_body);
bo->req = NULL;
ObjSetState(bo->wrk, bo->fetch_objcore, BOS_REQ_DONE);
- } else if (bo->req->req_body_status != REQ_BODY_LENGTH &&
- bo->req->req_body_status != REQ_BODY_WITHOUT_LEN) {
- WRONG("Bad req_body_status");
}
return (F_STP_STARTFETCH);
}
diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index 81987f89e..502fcaae0 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -71,19 +71,6 @@ static void pan_req(struct vsb *, const struct req *);
/*--------------------------------------------------------------------*/
-static const char *
-reqbody_status_2str(enum req_body_state_e e)
-{
- switch (e) {
-#define REQ_BODY(U) case REQ_BODY_##U: return("R_BODY_" #U);
-#include "tbl/req_body.h"
- default:
- return ("?");
- }
-}
-
-/*--------------------------------------------------------------------*/
-
static const char *
boc_state_2str(enum boc_state_e e)
{
@@ -524,7 +511,7 @@ pan_req(struct vsb *vsb, const struct req *req)
VSB_printf(vsb, "step = 0x%x,\n", req->req_step);
VSB_printf(vsb, "req_body = %s,\n",
- reqbody_status_2str(req->req_body_status));
+ req->req_body_status ? req->req_body_status->name : "NULL");
if (req->err_code)
VSB_printf(vsb,
diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index 305cea641..122dfdb6d 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -94,7 +94,6 @@ Req_New(const struct worker *wrk, struct sess *sp)
AN(req);
req->magic = REQ_MAGIC;
req->sp = sp;
- req->req_body_status = REQ_BODY_INIT;
e = (char*)req + sz;
p = (char*)(req + 1);
@@ -242,7 +241,7 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
req->t_first = NAN;
req->t_prev = NAN;
req->t_req = NAN;
- req->req_body_status = REQ_BODY_INIT;
+ req->req_body_status = NULL;
req->hash_always_miss = 0;
req->hash_ignore_busy = 0;
diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c
index 08db8fdb6..966526e22 100644
--- a/bin/varnishd/cache/cache_req_body.c
+++ b/bin/varnishd/cache/cache_req_body.c
@@ -77,7 +77,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
req->storage = NULL;
if (STV_NewObject(req->wrk, req->body_oc, stv, 8) == 0) {
- req->req_body_status = REQ_BODY_ERROR;
+ req->req_body_status = BS_ERROR;
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
(void)VFP_Error(vfc, "Object allocation failed:"
@@ -88,7 +88,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
vfc->oc = req->body_oc;
if (VFP_Open(vfc) < 0) {
- req->req_body_status = REQ_BODY_ERROR;
+ req->req_body_status = BS_ERROR;
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
return (-1);
@@ -136,7 +136,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
if (vfps != VFP_END) {
- req->req_body_status = REQ_BODY_ERROR;
+ req->req_body_status = BS_ERROR;
if (r == 0)
r = -1;
}
@@ -148,7 +148,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
HSH_DerefBoc(req->wrk, req->body_oc);
if (vfps != VFP_END) {
- req->req_body_status = REQ_BODY_ERROR;
+ req->req_body_status = BS_ERROR;
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
return (-1);
}
@@ -167,7 +167,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
(uintmax_t)req_bodybytes);
}
- req->req_body_status = REQ_BODY_CACHED;
+ req->req_body_status = BS_CACHED;
return (req_bodybytes);
}
@@ -189,32 +189,27 @@ VRB_Iterate(struct worker *wrk, struct vsl_log *vsl,
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
AN(func);
- switch (req->req_body_status) {
- case REQ_BODY_CACHED:
+ if (req->req_body_status == BS_CACHED) {
AN(req->body_oc);
if (ObjIterate(wrk, req->body_oc, priv, func, 0))
return (-1);
return (0);
- case REQ_BODY_NONE:
+ }
+ if (req->req_body_status == BS_NONE)
return (0);
- case REQ_BODY_LENGTH:
- case REQ_BODY_WITHOUT_LEN:
- break;
- case REQ_BODY_TAKEN:
+ if (req->req_body_status == BS_TAKEN) {
VSLb(vsl, SLT_VCL_Error,
"Uncached req.body can only be consumed once.");
return (-1);
- case REQ_BODY_ERROR:
+ }
+ if (req->req_body_status == BS_ERROR) {
VSLb(vsl, SLT_FetchError,
"Had failed reading req.body before.");
return (-1);
- default:
- WRONG("Wrong req_body_status in VRB_Iterate()");
}
Lck_Lock(&req->sp->mtx);
- if (req->req_body_status == REQ_BODY_LENGTH ||
- req->req_body_status == REQ_BODY_WITHOUT_LEN) {
- req->req_body_status = REQ_BODY_TAKEN;
+ if (req->req_body_status->avail > 0) {
+ req->req_body_status = BS_TAKEN;
i = 0;
} else
i = -1;
@@ -254,9 +249,9 @@ VRB_Ignore(struct req *req)
if (req->doclose)
return (0);
- if (req->req_body_status == REQ_BODY_LENGTH ||
- req->req_body_status == REQ_BODY_WITHOUT_LEN)
- (void)VRB_Iterate(req->wrk, req->vsl, req, httpq_req_body_discard, NULL);
+ if (req->req_body_status->avail > 0)
+ (void)VRB_Iterate(req->wrk, req->vsl, req,
+ httpq_req_body_discard, NULL);
return (0);
}
@@ -292,6 +287,7 @@ VRB_Cache(struct req *req, ssize_t maxsize)
uint64_t u;
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ assert (req->req_step == R_STP_RECV);
assert(maxsize >= 0);
/*
@@ -299,27 +295,22 @@ VRB_Cache(struct req *req, ssize_t maxsize)
* where we know we will have no competition or conflicts for the
* updates to req.http.* etc.
*/
- if (req->restarts > 0 && req->req_body_status != REQ_BODY_CACHED)
+ if (req->restarts > 0 && req->req_body_status != BS_CACHED) {
+ VSLb(req->vsl, SLT_VCL_Error,
+ "req.body must be cached before restarts");
return (-1);
+ }
- assert (req->req_step == R_STP_RECV);
- switch (req->req_body_status) {
- case REQ_BODY_CACHED:
+ if (req->req_body_status == BS_CACHED) {
AZ(ObjGetU64(req->wrk, req->body_oc, OA_LEN, &u));
return (u);
- case REQ_BODY_ERROR:
- return (-1);
- case REQ_BODY_NONE:
- return (0);
- case REQ_BODY_WITHOUT_LEN:
- case REQ_BODY_LENGTH:
- break;
- default:
- WRONG("Wrong req_body_status in VRB_Cache()");
}
+ if (req->req_body_status->avail <= 0)
+ return (req->req_body_status->avail);
+
if (req->htc->content_length > maxsize) {
- req->req_body_status = REQ_BODY_ERROR;
+ req->req_body_status = BS_ERROR;
(void)VFP_Error(req->vfc, "Request body too big to cache");
return (-1);
}
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 1deb2595c..3829b6292 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -66,7 +66,7 @@ cnt_transport(struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(req->http, HTTP_MAGIC);
CHECK_OBJ_NOTNULL(req->transport, TRANSPORT_MAGIC);
- assert(req->req_body_status != REQ_BODY_INIT);
+ AN(req->req_body_status);
if (http_GetHdr(req->http, H_Expect, &p)) {
if (strcasecmp(p, "100-continue")) {
@@ -89,7 +89,7 @@ cnt_transport(struct worker *wrk, struct req *req)
return (REQ_FSM_DONE);
}
- if (req->req_body_status < REQ_BODY_TAKEN) {
+ if (req->req_body_status->avail == 1) {
AN(req->transport->req_body != NULL);
VFP_Setup(req->vfc, wrk);
req->vfc->resp = req->http; // XXX
@@ -741,8 +741,8 @@ cnt_pipe(struct worker *wrk, struct req *req)
bo->req = req;
bo->wrk = wrk;
/* Unless cached, reqbody is not our job */
- if (req->req_body_status != REQ_BODY_CACHED)
- req->req_body_status = REQ_BODY_NONE;
+ if (req->req_body_status != BS_CACHED)
+ req->req_body_status = BS_NONE;
SES_Close(req->sp, VDI_Http1Pipe(req, bo));
nxt = REQ_FSM_DONE;
V1P_Leave();
@@ -877,7 +877,7 @@ cnt_recv(struct worker *wrk, struct req *req)
cnt_recv_prep(req, ci);
- if (req->req_body_status == REQ_BODY_ERROR) {
+ if (req->req_body_status == BS_ERROR) {
req->doclose = SC_OVERLOAD;
return (REQ_FSM_DONE);
}
@@ -899,7 +899,7 @@ cnt_recv(struct worker *wrk, struct req *req)
}
/* Attempts to cache req.body may fail */
- if (req->req_body_status == REQ_BODY_ERROR) {
+ if (req->req_body_status == BS_ERROR) {
req->doclose = SC_RX_BODY;
return (REQ_FSM_DONE);
}
diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c
index 4cecb3148..ad70c6868 100644
--- a/bin/varnishd/http1/cache_http1_fetch.c
+++ b/bin/varnishd/http1/cache_http1_fetch.c
@@ -91,8 +91,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
assert(*htc->rfd > 0);
hp = bo->bereq;
- if (bo->req != NULL &&
- bo->req->req_body_status == REQ_BODY_WITHOUT_LEN) {
+ if (bo->req != NULL && !bo->req->req_body_status->length_known) {
http_PrintfHeader(hp, "Transfer-Encoding: chunked");
do_chunked = 1;
}
@@ -112,15 +111,15 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
(void)ObjIterate(bo->wrk, bo->bereq_body,
bo, vbf_iter_req_body, 0);
} else if (bo->req != NULL &&
- bo->req->req_body_status != REQ_BODY_NONE) {
+ bo->req->req_body_status != BS_NONE) {
if (do_chunked)
V1L_Chunked(wrk);
i = VRB_Iterate(wrk, bo->vsl, bo->req, vbf_iter_req_body, bo);
- if (bo->req->req_body_status != REQ_BODY_CACHED)
+ if (bo->req->req_body_status != BS_CACHED)
bo->no_retry = "req.body not cached";
- if (bo->req->req_body_status == REQ_BODY_ERROR) {
+ if (bo->req->req_body_status == BS_ERROR) {
/*
* XXX: (#2332) We should test to see if the backend
* XXX: sent us some headers explaining why.
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index 6332aca96..67ab7016b 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -151,9 +151,8 @@ http1_req_body(struct req *req)
{
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
- if (req->htc->body_status->avail &&
- V1F_Setup_Fetch(req->vfc, req->htc) != 0)
- req->req_body_status = REQ_BODY_ERROR;
+ if (V1F_Setup_Fetch(req->vfc, req->htc) != 0)
+ req->req_body_status = BS_ERROR;
}
static void
@@ -273,18 +272,8 @@ http1_dissect(struct worker *wrk, struct req *req)
return (-1);
}
- assert (req->req_body_status == REQ_BODY_INIT);
-
- if (req->htc->body_status == BS_CHUNKED)
- req->req_body_status = REQ_BODY_WITHOUT_LEN;
- else if (req->htc->body_status == BS_LENGTH)
- req->req_body_status = REQ_BODY_LENGTH;
- else if (req->htc->body_status == BS_NONE)
- req->req_body_status = REQ_BODY_NONE;
- else if (req->htc->body_status == BS_EOF)
- req->req_body_status = REQ_BODY_WITHOUT_LEN;
- else
- WRONG("Unknown req_body_status situation");
+ AZ(req->req_body_status);
+ req->req_body_status = req->htc->body_status;
return (0);
}
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 183a327cf..317b3287b 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -559,11 +559,6 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
assert(r2->state == H2_S_OPEN);
h2e = h2h_decode_fini(h2);
h2->new_req = NULL;
- if (r2->req->req_body_status == REQ_BODY_NONE) {
- /* REQ_BODY_NONE implies one of the frames in the
- * header block contained END_STREAM */
- r2->state = H2_S_CLOS_REM;
- }
if (h2e != NULL) {
Lck_Lock(&h2->sess->mtx);
VSLb(h2->vsl, SLT_Debug, "HPACK/FINI %s", h2e->name);
@@ -579,13 +574,15 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
// XXX: Have I mentioned H/2 Is hodge-podge ?
http_CollectHdrSep(req->http, H_Cookie, "; "); // rfc7540,l,3114,3120
- if (req->req_body_status == REQ_BODY_INIT) {
+ if (req->req_body_status == NULL) {
if (!http_GetHdr(req->http, H_Content_Length, NULL))
- req->req_body_status = REQ_BODY_WITHOUT_LEN;
+ req->req_body_status = BS_EOF;
else
- req->req_body_status = REQ_BODY_LENGTH;
+ req->req_body_status = BS_LENGTH;
} else {
- assert (req->req_body_status == REQ_BODY_NONE);
+ /* A HEADER frame contained END_STREAM */
+ assert (req->req_body_status == BS_NONE);
+ r2->state = H2_S_CLOS_REM;
if (http_GetContentLength(req->http) > 0)
return (H2CE_PROTOCOL_ERROR); //rfc7540,l,1838,1840
}
@@ -694,7 +691,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
}
if (h2->rxf_flags & H2FF_HEADERS_END_STREAM)
- req->req_body_status = REQ_BODY_NONE;
+ req->req_body_status = BS_NONE;
if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
return (h2_end_headers(wrk, h2, req, r2));
diff --git a/include/Makefile.am b/include/Makefile.am
index d31231ddd..9186df797 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -26,7 +26,6 @@ nobase_pkginclude_HEADERS = \
tbl/oc_exp_flags.h \
tbl/oc_flags.h \
tbl/params.h \
- tbl/req_body.h \
tbl/req_flags.h \
tbl/sess_attr.h \
tbl/sess_close.h \
diff --git a/include/tbl/body_status.h b/include/tbl/body_status.h
index 878248d72..108345c6c 100644
--- a/include/tbl/body_status.h
+++ b/include/tbl/body_status.h
@@ -34,10 +34,12 @@
/* Upper lower nbr, avail len_known */
BODYSTATUS(NONE, none, 0, 0, 1)
-BODYSTATUS(ERROR, error, 1, 0, 0)
+BODYSTATUS(ERROR, error, 1, -1, 0)
BODYSTATUS(CHUNKED, chunked, 2, 1, 0)
BODYSTATUS(LENGTH, length, 3, 1, 1)
BODYSTATUS(EOF, eof, 4, 1, 0)
+BODYSTATUS(TAKEN, taken, 5, 0, 0)
+BODYSTATUS(CACHED, cached, 6, 2, 1)
#undef BODYSTATUS
/*lint -restore */
diff --git a/include/tbl/req_body.h b/include/tbl/req_body.h
deleted file mode 100644
index 6386f4e82..000000000
--- a/include/tbl/req_body.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*-
- * Copyright (c) 2013-2015 Varnish Software AS
- * All rights reserved.
- *
- * Author: Poul-Henning Kamp <phk at phk.freebsd.dk>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- */
-
-/*lint -save -e525 -e539 */
-
-REQ_BODY(INIT)
-REQ_BODY(WITHOUT_LEN)
-REQ_BODY(LENGTH)
-/* states >= TAKEN imply that no body is to be read */
-REQ_BODY(TAKEN)
-REQ_BODY(CACHED)
-REQ_BODY(ERROR)
-REQ_BODY(NONE)
-#undef REQ_BODY
-
-/*lint -restore */
More information about the varnish-commit
mailing list