[master] 952115fdb Simplify memory handling around struct h2h_decode
Dag Haavi Finstad
daghf at varnish-software.com
Fri Jun 29 14:22:07 UTC 2018
commit 952115fdb02ef34f17e877686f818e24c385687f
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date: Fri Jun 29 15:04:11 2018 +0200
Simplify memory handling around struct h2h_decode
Since we verify that header blocks are not interleaved, and we zero the
struct h2h_decode on every new header block, there is no need to malloc
a separate struct h2h_decode per stream.
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index fbc190535..792fd985a 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -127,7 +127,6 @@ struct h2_req {
VTAILQ_ENTRY(h2_req) list;
int64_t t_window;
int64_t r_window;
- struct h2h_decode *decode;
/* Where to wake this stream up */
struct worker *wrk;
@@ -161,6 +160,7 @@ struct h2_sess {
struct ws *ws;
struct http_conn *htc;
struct vsl_log *vsl;
+ struct h2h_decode *decode;
struct vht_table dectbl[1];
unsigned rxf_len;
@@ -208,10 +208,10 @@ struct h2h_decode {
struct vhd_decode vhd[1];
};
-void h2h_decode_init(const struct h2_sess *h2, struct h2h_decode *d);
-h2_error h2h_decode_fini(const struct h2_sess *h2, struct h2h_decode *d);
-h2_error h2h_decode_bytes(struct h2_sess *h2, struct h2h_decode *d,
- const uint8_t *ptr, size_t len);
+void h2h_decode_init(const struct h2_sess *h2);
+h2_error h2h_decode_fini(const struct h2_sess *h2);
+h2_error h2h_decode_bytes(struct h2_sess *h2, const uint8_t *ptr,
+ size_t len);
/* cache_http2_send.c */
void H2_Send_Get(struct worker *, struct h2_sess *, struct h2_req *);
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index 3d7c2d00b..0524f3e32 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -164,13 +164,15 @@ h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len)
}
void
-h2h_decode_init(const struct h2_sess *h2, struct h2h_decode *d)
+h2h_decode_init(const struct h2_sess *h2)
{
+ struct h2h_decode *d;
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
CHECK_OBJ_NOTNULL(h2->new_req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(h2->new_req->http, HTTP_MAGIC);
- AN(d);
+ AN(h2->decode);
+ d = h2->decode;
INIT_OBJ(d, H2H_DECODE_MAGIC);
VHD_Init(d->vhd);
d->out_l = WS_Reserve(h2->new_req->http->ws, 0);
@@ -189,11 +191,13 @@ h2h_decode_init(const struct h2_sess *h2, struct h2h_decode *d)
* is a stream level error.
*/
h2_error
-h2h_decode_fini(const struct h2_sess *h2, struct h2h_decode *d)
+h2h_decode_fini(const struct h2_sess *h2)
{
h2_error ret;
+ struct h2h_decode *d;
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
+ d = h2->decode;
CHECK_OBJ_NOTNULL(h2->new_req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(d, H2H_DECODE_MAGIC);
WS_ReleaseP(h2->new_req->http->ws, d->out);
@@ -217,10 +221,10 @@ h2h_decode_fini(const struct h2_sess *h2, struct h2h_decode *d)
* H2E_PROTOCOL_ERROR: Malformed header or duplicate pseudo-header.
*/
h2_error
-h2h_decode_bytes(struct h2_sess *h2, struct h2h_decode *d,
- const uint8_t *in, size_t in_l)
+h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
{
struct http *hp;
+ struct h2h_decode *d;
size_t in_u = 0;
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
@@ -229,6 +233,7 @@ h2h_decode_bytes(struct h2_sess *h2, struct h2h_decode *d,
CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
CHECK_OBJ_NOTNULL(hp->ws, WS_MAGIC);
AN(hp->ws->r);
+ d = h2->decode;
CHECK_OBJ_NOTNULL(d, H2H_DECODE_MAGIC);
/* Only H2E_ENHANCE_YOUR_CALM indicates that we should continue
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index e5106e8a4..f4cc294e5 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -225,10 +225,8 @@ h2_kill_req(struct worker *wrk, const struct h2_sess *h2,
AZ(pthread_cond_signal(r2->cond));
r2 = NULL;
} else {
- if (r2->state == H2_S_OPEN && r2->decode != NULL) {
- (void)h2h_decode_fini(h2, r2->decode);
- FREE_OBJ(r2->decode);
- }
+ if (r2->state == H2_S_OPEN && h2->new_req == r2->req)
+ (void)h2h_decode_fini(h2);
}
Lck_Unlock(&h2->sess->mtx);
if (r2 != NULL)
@@ -557,8 +555,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
ASSERT_RXTHR(h2);
assert(r2->state == H2_S_OPEN);
- h2e = h2h_decode_fini(h2, r2->decode);
- FREE_OBJ(r2->decode);
+ 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
@@ -650,9 +647,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);
http_SetH(req->http, HTTP_HDR_PROTO, "HTTP/2.0");
- ALLOC_OBJ(r2->decode, H2H_DECODE_MAGIC);
- AN(r2->decode);
- h2h_decode_init(h2, r2->decode);
+ h2h_decode_init(h2);
p = h2->rxf_data;
l = h2->rxf_len;
@@ -668,13 +663,12 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
l -= 5;
p += 5;
}
- h2e = h2h_decode_bytes(h2, r2->decode, p, l);
+ h2e = h2h_decode_bytes(h2, p, l);
if (h2e != NULL) {
Lck_Lock(&h2->sess->mtx);
VSLb(h2->vsl, SLT_Debug, "HPACK(hdr) %s", h2e->name);
Lck_Unlock(&h2->sess->mtx);
- (void)h2h_decode_fini(h2, r2->decode);
- FREE_OBJ(r2->decode);
+ (void)h2h_decode_fini(h2);
AZ(r2->req->ws->r);
h2_del_req(wrk, r2);
return (h2e);
@@ -700,14 +694,13 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
if (r2 == NULL || r2->state != H2_S_OPEN || r2->req != h2->new_req)
return (H2CE_PROTOCOL_ERROR); // XXX spec ?
req = r2->req;
- h2e = h2h_decode_bytes(h2, r2->decode, h2->rxf_data, h2->rxf_len);
+ h2e = h2h_decode_bytes(h2, h2->rxf_data, h2->rxf_len);
r2->req->acct.req_hdrbytes += h2->rxf_len;
if (h2e != NULL) {
Lck_Lock(&h2->sess->mtx);
VSLb(h2->vsl, SLT_Debug, "HPACK(cont) %s", h2e->name);
Lck_Unlock(&h2->sess->mtx);
- (void)h2h_decode_fini(h2, r2->decode);
- FREE_OBJ(r2->decode);
+ (void)h2h_decode_fini(h2);
AZ(r2->req->ws->r);
h2_del_req(wrk, r2);
return (h2e);
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index c0545265c..40a111754 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -97,7 +97,7 @@ h2_local_settings(struct h2_settings *h2s)
static struct h2_sess *
h2_init_sess(const struct worker *wrk, struct sess *sp,
- struct h2_sess *h2s, struct req *srq)
+ struct h2_sess *h2s, struct req *srq, struct h2h_decode *decode)
{
uintptr_t *up;
struct h2_sess *h2;
@@ -128,6 +128,7 @@ h2_init_sess(const struct worker *wrk, struct sess *sp,
VTAILQ_INIT(&h2->txqueue);
h2_local_settings(&h2->local_settings);
h2->remote_settings = H2_proto_settings;
+ h2->decode = decode;
AZ(VHT_Init(h2->dectbl,
h2->local_settings.header_table_size));
@@ -338,6 +339,7 @@ h2_new_session(struct worker *wrk, void *arg)
struct h2_req *r2, *r22;
int again;
uint8_t settings[48];
+ struct h2h_decode decode;
size_t l;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -350,7 +352,7 @@ h2_new_session(struct worker *wrk, void *arg)
assert (req->err_code == H2_PU_MARKER || req->err_code == H2_OU_MARKER);
h2 = h2_init_sess(wrk, sp, &h2s,
- req->err_code == H2_PU_MARKER ? req : NULL);
+ req->err_code == H2_PU_MARKER ? req : NULL, &decode);
h2->req0 = h2_new_req(wrk, h2, 0, NULL);
AZ(h2->htc->priv);
h2->htc->priv = h2;
More information about the varnish-commit
mailing list