[master] 2afc943 Don't rely on continued access to the vrt_backend struct used to create a backend, that may be true of VCC compiled backends, but won't be for VMOD dynamic backends.

Poul-Henning Kamp phk at FreeBSD.org
Mon Jun 22 23:31:30 CEST 2015


commit 2afc943ab10511b5dab5d2947fd5a5db6545b067
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Jun 22 21:29:55 2015 +0000

    Don't rely on continued access to the vrt_backend struct used to
    create a backend, that may be true of VCC compiled backends, but
    won't be for VMOD dynamic backends.
    
    Eliminate some excessive copying at the same time.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index da22bf4..706c92b 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -119,7 +119,6 @@ struct req;
 struct sess;
 struct suckaddr;
 struct vbc;
-struct vrt_backend;
 struct vrt_priv;
 struct vsb;
 struct waitinglist;
diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index 789655f..5824504 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -83,7 +83,6 @@ vbe_dir_getfd(struct worker *wrk, const struct director *d, struct busyobj *bo)
 	struct vbc *vc;
 	struct backend *bp;
 	double tmod;
-	const struct vrt_backend *vrt;
 	char abuf1[VTCP_ADDRBUFSIZE], abuf2[VTCP_ADDRBUFSIZE];
 	char pbuf1[VTCP_PORTBUFSIZE], pbuf2[VTCP_PORTBUFSIZE];
 
@@ -92,7 +91,6 @@ vbe_dir_getfd(struct worker *wrk, const struct director *d, struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
 	CAST_OBJ_NOTNULL(bp, d->priv, BACKEND_MAGIC);
 	AN(bp->vsc);
-	CAST_OBJ_NOTNULL(vrt, d->priv2, VRT_BACKEND_MAGIC);
 
 	if (!VBE_Healthy(bp, NULL)) {
 		// XXX: per backend stats ?
@@ -100,8 +98,7 @@ vbe_dir_getfd(struct worker *wrk, const struct director *d, struct busyobj *bo)
 		return (-1);
 	}
 
-	if (vrt->max_connections > 0 &&
-	    bp->n_conn >= vrt->max_connections) {
+	if (bp->max_connections > 0 && bp->n_conn >= bp->max_connections) {
 		// XXX: per backend stats ?
 		VSC_C_main->backend_busy++;
 		return (-1);
@@ -113,7 +110,7 @@ vbe_dir_getfd(struct worker *wrk, const struct director *d, struct busyobj *bo)
 		/* XXX: counter ? */
 		return (-1);
 
-	FIND_TMO(connect_timeout, tmod, bo, vrt);
+	FIND_TMO(connect_timeout, tmod, bo, bp);
 	vc = VBT_Get(bp->tcp_pool, tmod, bp, wrk);
 	if (vc == NULL) {
 		// XXX: Per backend stats ?
@@ -142,9 +139,9 @@ vbe_dir_getfd(struct worker *wrk, const struct director *d, struct busyobj *bo)
 	bo->htc->vbc = vc;
 	bo->htc->fd = vc->fd;
 	FIND_TMO(first_byte_timeout,
-	    bo->htc->first_byte_timeout, bo, vrt);
+	    bo->htc->first_byte_timeout, bo, bp);
 	FIND_TMO(between_bytes_timeout,
-	    bo->htc->between_bytes_timeout, bo, vrt);
+	    bo->htc->between_bytes_timeout, bo, bp);
 	return (vc->fd);
 }
 
@@ -200,12 +197,12 @@ vbe_dir_gethdrs(const struct director *d, struct worker *wrk,
     struct busyobj *bo)
 {
 	int i, extrachance = 0;
-	const struct vrt_backend *vrt;
+	struct backend *bp;
 
 	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
-	CAST_OBJ_NOTNULL(vrt, d->priv2, VRT_BACKEND_MAGIC);
+	CAST_OBJ_NOTNULL(bp, d->priv, BACKEND_MAGIC);
 
 	i = vbe_dir_getfd(wrk, d, bo);
 	if (i < 0) {
@@ -216,7 +213,7 @@ vbe_dir_gethdrs(const struct director *d, struct worker *wrk,
 	if (bo->htc->vbc->state == VBC_STATE_STOLEN)
 		extrachance = 1;
 
-	i = V1F_fetch_hdr(wrk, bo, vrt->hosthdr);
+	i = V1F_fetch_hdr(wrk, bo, bp->hosthdr);
 	/*
 	 * If we recycle a backend connection, there is a finite chance
 	 * that the backend closed it before we get a request to it.
@@ -234,7 +231,7 @@ vbe_dir_gethdrs(const struct director *d, struct worker *wrk,
 			return (-1);
 		}
 		AN(bo->htc);
-		i = V1F_fetch_hdr(wrk, bo, vrt->hosthdr);
+		i = V1F_fetch_hdr(wrk, bo, bp->hosthdr);
 	}
 	if (i != 0) {
 		vbe_dir_finish(d, wrk, bo);
@@ -299,19 +296,17 @@ vbe_dir_http1pipe(const struct director *d, struct req *req, struct busyobj *bo)
 /*--------------------------------------------------------------------*/
 
 void
-VBE_fill_director(struct backend *be, const struct vrt_backend *vrt)
+VBE_fill_director(struct backend *be)
 {
 	struct director *d;
 
 	CHECK_OBJ_NOTNULL(be, BACKEND_MAGIC);
-	CHECK_OBJ_NOTNULL(vrt, VRT_BACKEND_MAGIC);
 
 	INIT_OBJ(be->director, DIRECTOR_MAGIC);
 	d = be->director;
 	d->priv = be;
-	d->priv2 = vrt;
 	d->name = "backend";
-	REPLACE(d->vcl_name, vrt->vcl_name);
+	d->vcl_name = be->vcl_name;
 	d->http1pipe = vbe_dir_http1pipe;
 	d->healthy = vbe_dir_healthy;
 	d->gethdrs = vbe_dir_gethdrs;
diff --git a/bin/varnishd/cache/cache_backend.h b/bin/varnishd/cache/cache_backend.h
index b6b96e4..22655f9 100644
--- a/bin/varnishd/cache/cache_backend.h
+++ b/bin/varnishd/cache/cache_backend.h
@@ -63,15 +63,10 @@ struct backend {
 	int			refcount;
 	struct lock		mtx;
 
+	VRT_BACKEND_FIELDS()
+
 	const struct vcl	*vcl;
-	const char		*vcl_name;
 	char			*display_name;
-	const char		*ipv4_addr;
-	const char		*ipv6_addr;
-	const char		*port;
-
-	struct suckaddr		*ipv4;
-	struct suckaddr		*ipv6;
 
 	unsigned		n_conn;
 
@@ -109,7 +104,7 @@ struct vbc {
 };
 
 /* cache_backend.c */
-void VBE_fill_director(struct backend *be, const struct vrt_backend *vrt);
+void VBE_fill_director(struct backend *be);
 
 /* cache_backend_cfg.c */
 unsigned VBE_Healthy(const struct backend *b, double *changed);
diff --git a/bin/varnishd/cache/cache_backend_cfg.c b/bin/varnishd/cache/cache_backend_cfg.c
index b3766cd..d9cf6fe 100644
--- a/bin/varnishd/cache/cache_backend_cfg.c
+++ b/bin/varnishd/cache/cache_backend_cfg.c
@@ -42,7 +42,6 @@
 #include "vcli.h"
 #include "vcli_priv.h"
 #include "vrt.h"
-#include "vsa.h"
 #include "vtim.h"
 
 #include "cache_director.h"
@@ -64,6 +63,8 @@ VRT_new_backend(VRT_CTX, const struct vrt_backend *vrt)
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(vrt, VRT_BACKEND_MAGIC);
 
+	assert(vrt->ipv4_suckaddr != NULL || vrt->ipv6_suckaddr != NULL);
+
 	vcl = ctx->vcl;
 	AN(vcl);
 	AN(vrt->vcl_name);
@@ -74,24 +75,19 @@ VRT_new_backend(VRT_CTX, const struct vrt_backend *vrt)
 	XXXAN(b);
 	Lck_New(&b->mtx, lck_backend);
 
+#define DA(x)	do { if (vrt->x != NULL) REPLACE((b->x), (vrt->x)); } while (0)
+#define DN(x)	do { b->x = vrt->x; } while (0)
+	VRT_BACKEND_HANDLE();
+#undef DA
+#undef DN
+
 	bprintf(buf, "%s.%s", VCL_Name(vcl), vrt->vcl_name);
 	REPLACE(b->display_name, buf);
 
 	b->vcl = vcl;
-	b->vcl_name =  vrt->vcl_name;
-	b->ipv4_addr = vrt->ipv4_addr;
-	b->ipv6_addr = vrt->ipv6_addr;
-	b->port = vrt->port;
 
 	b->tcp_pool = VBT_Ref(vrt->ipv4_suckaddr, vrt->ipv6_suckaddr);
 
-	if (vrt->ipv4_suckaddr != NULL)
-		b->ipv4 = VSA_Clone(vrt->ipv4_suckaddr);
-	if (vrt->ipv6_suckaddr != NULL)
-		b->ipv6 = VSA_Clone(vrt->ipv6_suckaddr);
-
-	assert(b->ipv4 != NULL || b->ipv6 != NULL);
-
 	b->healthy = 1;
 	b->health_changed = VTIM_real();
 	b->admin_health = ah_probe;
@@ -101,7 +97,7 @@ VRT_new_backend(VRT_CTX, const struct vrt_backend *vrt)
 	VSC_C_main->n_backend++;
 	Lck_Unlock(&backends_mtx);
 
-	VBE_fill_director(b, vrt);
+	VBE_fill_director(b);
 
 	if (vrt->probe != NULL)
 		VBP_Insert(b, vrt->probe, vrt->hosthdr);
@@ -170,8 +166,12 @@ VBE_Delete(struct backend *be)
 	VSC_C_main->n_backend--;
 	Lck_Unlock(&backends_mtx);
 
-	free(be->ipv4);
-	free(be->ipv6);
+#define DA(x)	do { if (be->x != NULL) free(be->x); } while (0)
+#define DN(x)	/**/
+	VRT_BACKEND_HANDLE();
+#undef DA
+#undef DN
+
 	free(be->display_name);
 	AZ(be->vsc);
 	VBT_Rel(&be->tcp_pool);
diff --git a/bin/varnishd/cache/cache_backend_poll.c b/bin/varnishd/cache/cache_backend_poll.c
index d88f6e8..d196eda 100644
--- a/bin/varnishd/cache/cache_backend_poll.c
+++ b/bin/varnishd/cache/cache_backend_poll.c
@@ -62,8 +62,6 @@ struct vbp_target {
 
 	struct backend			*backend;
 
-	struct tcp_pool			*tcp_pool;
-
 	struct vrt_backend_probe	probe;
 
 	char				*req;
@@ -217,7 +215,7 @@ vbp_poke(struct vbp_target *vt)
 	t_start = t_now = VTIM_real();
 	t_end = t_start + vt->probe.timeout;
 
-	s = VBT_Open(vt->tcp_pool, t_end - t_now, &sa);
+	s = VBT_Open(vt->backend->tcp_pool, t_end - t_now, &sa);
 	if (s < 0) {
 		/* Got no connection: failed */
 		return;
@@ -320,7 +318,6 @@ vbp_task(struct worker *wrk, void *priv)
 
 	Lck_Lock(&vbp_mtx);
 	if (vt->running < 0) {
-		VBT_Rel(&vt->tcp_pool);
 		free(vt->req);
 		FREE_OBJ(vt);
 	} else {
@@ -537,9 +534,6 @@ VBP_Insert(struct backend *b, const struct vrt_backend_probe *p,
 	ALLOC_OBJ(vt, VBP_TARGET_MAGIC);
 	XXXAN(vt);
 
-	vt->tcp_pool = VBT_Ref(b->ipv4, b->ipv6);
-	AN(vt->tcp_pool);
-
 	vt->probe = *p;
 	vbp_set_defaults(vt);
 
@@ -572,7 +566,6 @@ VBP_Remove(struct backend *be)
 	}
 	Lck_Unlock(&vbp_mtx);
 	if (vt != NULL) {
-		VBT_Rel(&vt->tcp_pool);
 		free(vt->req);
 		FREE_OBJ(vt);
 	}
diff --git a/include/vrt.h b/include/vrt.h
index 1dd5d0c..2f0887d 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -166,29 +166,51 @@ struct vrt_backend_probe {
 	unsigned			initial;
 };
 
-/*
- * A backend is a host+port somewhere on the network
+/***********************************************************************
+ * We want the VCC to spit this structs out as const, but when VMODs
+ * come up with them we want to clone them into malloc'ed space which
+ * we can free again.
+ * We collect all the knowledge here by macroizing the fields and make
+ * a macro for handling them all.
+ * See also:  cache_backend.h & cache_backend_cfg.c
+ * One of those things...
  */
+
+#define VRT_BACKEND_FIELDS(rigid)				\
+	rigid char			*vcl_name;		\
+	rigid char			*ipv4_addr;		\
+	rigid char			*ipv6_addr;		\
+	rigid char			*port;			\
+	rigid char			*hosthdr;		\
+	double				connect_timeout;	\
+	double				first_byte_timeout;	\
+	double				between_bytes_timeout;	\
+	unsigned			max_connections;
+
+#define VRT_BACKEND_HANDLE()			\
+	do {					\
+		DA(vcl_name);			\
+		DA(ipv4_addr);			\
+		DA(ipv6_addr);			\
+		DA(port);			\
+		DA(hosthdr);			\
+		DN(connect_timeout);		\
+		DN(first_byte_timeout);		\
+		DN(between_bytes_timeout);	\
+		DN(max_connections);		\
+	} while(0)
+
 struct vrt_backend {
 	unsigned			magic;
 #define VRT_BACKEND_MAGIC		0x4799ce6b
-	const char			*vcl_name;
-	const char			*ipv4_addr;
-	const char			*ipv6_addr;
-	const char			*port;
-
+	VRT_BACKEND_FIELDS(const)
 	const struct suckaddr		*ipv4_suckaddr;
 	const struct suckaddr		*ipv6_suckaddr;
-
-	const char			*hosthdr;
-
-	double				connect_timeout;
-	double				first_byte_timeout;
-	double				between_bytes_timeout;
-	unsigned			max_connections;
 	const struct vrt_backend_probe	*probe;
 };
 
+/***********************************************************************/
+
 /*
  * other stuff.
  * XXX: document when bored



More information about the varnish-commit mailing list