[master] 0c8448e0e hpack: Track the workspace where headers are decoded

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Jul 8 17:12:06 UTC 2024


commit 0c8448e0ef2f9a1bb2e23eb1f6440b75e6c4d5a7
Author: Walid Boudebouda <walid.boudebouda at gmail.com>
Date:   Fri Jun 14 20:35:22 2024 +0200

    hpack: Track the workspace where headers are decoded
    
    The reset pointer is effectively the beginning of the reservation, so in
    order to better generalize HEADERS frames processing between headers and
    trailers, we reference the workspace directly.

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 89e32309c..db0afcb43 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -234,8 +234,8 @@ struct h2h_decode {
 	unsigned			has_scheme:1;
 	h2_error			error;
 	enum vhd_ret_e			vhd_ret;
+	struct ws			*ws;
 	char				*out;
-	char				*reset;
 	int64_t				limit;
 	size_t				out_l;
 	size_t				out_u;
@@ -243,8 +243,8 @@ struct h2h_decode {
 	struct vhd_decode		vhd[1];
 };
 
-void h2h_decode_init(const struct h2_sess *h2);
-h2_error h2h_decode_fini(const struct h2_sess *h2);
+void h2h_decode_hdr_init(const struct h2_sess *h2);
+h2_error h2h_decode_hdr_fini(const struct h2_sess *h2);
 h2_error h2h_decode_bytes(struct h2_sess *h2, const uint8_t *ptr,
     size_t len);
 
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index c49f9d2b9..0bbedb27b 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -257,27 +257,25 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 	return (0);
 }
 
-void
-h2h_decode_init(const struct h2_sess *h2)
+static void
+h2h_decode_init(const struct h2_sess *h2, struct ws *ws)
 {
 	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);
+	CHECK_OBJ_NOTNULL(ws, WS_MAGIC);
+
 	AN(h2->decode);
 	d = h2->decode;
 	INIT_OBJ(d, H2H_DECODE_MAGIC);
 	VHD_Init(d->vhd);
-	d->out_l = WS_ReserveSize(h2->new_req->http->ws,
-	    cache_param->http_req_size);
+	d->out_l = WS_ReserveSize(ws, cache_param->http_req_size);
 	/*
 	 * Can't do any work without any buffer
 	 * space. Require non-zero size.
 	 */
 	XXXAN(d->out_l);
-	d->out = WS_Reservation(h2->new_req->http->ws);
-	d->reset = d->out;
+	d->out = WS_Reservation(ws);
 
 	if (cache_param->h2_max_header_list_size == 0)
 		d->limit =
@@ -287,6 +285,18 @@ h2h_decode_init(const struct h2_sess *h2)
 
 	if (d->limit < h2->local_settings.max_header_list_size)
 		d->limit = INT64_MAX;
+
+	d->ws = ws;
+}
+
+void
+h2h_decode_hdr_init(const struct h2_sess *h2)
+{
+
+	CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
+	CHECK_OBJ_NOTNULL(h2->new_req, REQ_MAGIC);
+	CHECK_OBJ_NOTNULL(h2->new_req->http, HTTP_MAGIC);
+	h2h_decode_init(h2, h2->new_req->ws);
 }
 
 /* Possible error returns:
@@ -298,7 +308,7 @@ h2h_decode_init(const struct h2_sess *h2)
  * is a stream level error.
  */
 h2_error
-h2h_decode_fini(const struct h2_sess *h2)
+h2h_decode_hdr_fini(const struct h2_sess *h2)
 {
 	h2_error ret;
 	struct h2h_decode *d;
@@ -307,7 +317,7 @@ h2h_decode_fini(const struct h2_sess *h2)
 	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);
+	WS_ReleaseP(d->ws, d->out);
 	if (d->vhd_ret != VHD_OK) {
 		/* HPACK header block didn't finish at an instruction
 		   boundary */
@@ -347,12 +357,12 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 	CHECK_OBJ_NOTNULL(h2->new_req, REQ_MAGIC);
 	hp = h2->new_req->http;
 	CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
-	CHECK_OBJ_NOTNULL(hp->ws, WS_MAGIC);
-	r = WS_Reservation(hp->ws);
-	AN(r);
-	e = r + WS_ReservationSize(hp->ws);
 	d = h2->decode;
 	CHECK_OBJ_NOTNULL(d, H2H_DECODE_MAGIC);
+	CHECK_OBJ_NOTNULL(d->ws, WS_MAGIC);
+	r = WS_Reservation(d->ws);
+	AN(r);
+	e = r + WS_ReservationSize(d->ws);
 
 	/* Only H2E_ENHANCE_YOUR_CALM indicates that we should continue
 	   processing. Other errors should have been returned and handled
@@ -427,7 +437,7 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 		}
 
 		if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM)) {
-			d->out = d->reset;
+			d->out = WS_Reservation(d->ws);
 			d->out_l = e - d->out;
 			d->limit -= d->out_u;
 			d->out_u = 0;
@@ -444,7 +454,7 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 	}
 
 	if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM)) {
-		/* Stream error, delay reporting until h2h_decode_fini so
+		/* Stream error, delay reporting until h2h_decode_hdr_fini so
 		 * that we can process the complete header block. */
 		return (NULL);
 	}
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 5d0321160..25937cebe 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -234,7 +234,7 @@ h2_kill_req(struct worker *wrk, struct h2_sess *h2,
 		r2 = NULL;
 	} else {
 		if (r2->state == H2_S_OPEN && h2->new_req == r2->req)
-			(void)h2h_decode_fini(h2);
+			(void)h2h_decode_hdr_fini(h2);
 	}
 	Lck_Unlock(&h2->sess->mtx);
 	if (r2 != NULL)
@@ -634,7 +634,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);
+	h2e = h2h_decode_hdr_fini(h2);
 	h2->new_req = NULL;
 	if (h2e != NULL) {
 		Lck_Lock(&h2->sess->mtx);
@@ -767,7 +767,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");
 
-	h2h_decode_init(h2);
+	h2h_decode_hdr_init(h2);
 
 	p = h2->rxf_data;
 	l = h2->rxf_len;
@@ -788,7 +788,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 		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);
+		(void)h2h_decode_hdr_fini(h2);
 		assert(!WS_IsReserved(r2->req->ws));
 		h2_del_req(wrk, r2);
 		return (h2e);
@@ -823,7 +823,7 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 		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);
+		(void)h2h_decode_hdr_fini(h2);
 		assert(!WS_IsReserved(r2->req->ws));
 		h2_del_req(wrk, r2);
 		return (h2e);


More information about the varnish-commit mailing list