From 2ce17c15281f251e0bc4610a9f679b88cd6e7d23 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune Date: Wed, 17 May 2017 10:18:27 +0200 Subject: [PATCH] Introduce `$ABI [strict|vrt]` for VMOD descriptors When versioning appeared in the VRT API, the goal was to allow loose ABI compliance on loaded VMODs based on the major/minor revision against which it was built. Strict checking was performed if Varnish was built from the master branch in the VCC code, but omitted by the child. This however has two flaws: 1) Release management might go wrong like it happened in 5.1.2 that got released from the master branch. 2) This doesn't solve the original problem that some VMODs might rely on supported symbols encompassed by the VRT major/minor while others may choose to integrate deeper with Varnish and lose guarantees. This patch retires the `VCS_Branch` macro that is no longer needed and provides a new `$ABI` stanza that defaults to strict when omitted. To help discovery, in-tree modules advertise a strict match. To indicate that a VMOD needs the exact Varnish build to be loaded, the VMOD's metadata contains 0.0 for the VRT major/minor revision. In addition, both the VCC and child now perform the full ABI compliance check, picking the strict or vrt option depending on the VMOD's metadata. Closes #2330 --- bin/varnishd/cache/cache_vrt_vmod.c | 17 +++++++++++++---- config.phk | 28 +++++++++++++--------------- lib/libvcc/generate.py | 1 - lib/libvcc/vcc_vmod.c | 6 +++--- lib/libvcc/vmodtool.py | 18 ++++++++++++++++-- lib/libvmod_debug/vmod.vcc | 1 + lib/libvmod_directors/vmod.vcc | 1 + lib/libvmod_std/vmod.vcc | 1 + 8 files changed, 48 insertions(+), 25 deletions(-) diff --git a/bin/varnishd/cache/cache_vrt_vmod.c b/bin/varnishd/cache/cache_vrt_vmod.c index 65e707480cf..719df9f5064 100644 --- a/bin/varnishd/cache/cache_vrt_vmod.c +++ b/bin/varnishd/cache/cache_vrt_vmod.c @@ -38,6 +38,7 @@ #include #include "vcli_serve.h" +#include "vmod_abi.h" #include "vrt.h" /*-------------------------------------------------------------------- @@ -65,6 +66,16 @@ struct vmod { static VTAILQ_HEAD(,vmod) vmods = VTAILQ_HEAD_INITIALIZER(vmods); +static unsigned +vmod_abi_mismatch(const struct vmod_data *d) +{ + + if (d->vrt_major == 0 && d->vrt_minor == 0) + return (d->abi == NULL || strcmp(d->abi, VMOD_ABI_Version)); + + return (d->vrt_major != VRT_MAJOR_VERSION || + d->vrt_minor > VRT_MINOR_VERSION); +} int VRT_Vmod_Init(VRT_CTX, struct vmod **hdl, void *ptr, int len, const char *nm, @@ -112,15 +123,13 @@ VRT_Vmod_Init(VRT_CTX, struct vmod **hdl, void *ptr, int len, const char *nm, FREE_OBJ(v); return (1); } - if (d->vrt_major != VRT_MAJOR_VERSION || - d->vrt_minor > VRT_MINOR_VERSION || + if (vmod_abi_mismatch(d) || d->name == NULL || strcmp(d->name, nm) || d->func == NULL || d->func_len <= 0 || d->proto == NULL || - d->spec == NULL || - d->abi == NULL) { + d->spec == NULL) { VSB_printf(ctx->msg, "Loading VMOD %s from %s:\n", nm, path); VSB_printf(ctx->msg, "VMOD data is mangled.\n"); diff --git a/config.phk b/config.phk index 7356757453c..b54257a8fec 100644 --- a/config.phk +++ b/config.phk @@ -107,25 +107,23 @@ fi VCSF=include/vcs_version.h VMAV=include/vmod_abi.h +V=NOGIT if [ -d ./.git ] ; then V=`git show -s --pretty=format:%h` - B=`git rev-parse --abbrev-ref HEAD` -else - V="NOGIT" - B="NOGIT" fi -( -echo "/* $V */" -echo "/*" -echo " * NB: This file is machine generated, DO NOT EDIT!" -echo " *" -echo " * make(1) updates this when necessary" -echo " *" -echo " */" -echo "#define VCS_Version \"$V\"" -echo "#define VCS_Branch \"$B\"" -) > ${VCSF}_ + +cat > ${VCSF}_ <vrt_major == 0 && vmd->vrt_minor == 0 && strcmp(vmd->abi, VMOD_ABI_Version) != 0) { VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod)); VSB_printf(tl->sb, "\tFile name: %s\n", fnp); @@ -164,8 +164,8 @@ vcc_ParseImport(struct vcc *tl) vcc_ErrWhere(tl, mod); return; } - if (vmd->vrt_major != VRT_MAJOR_VERSION || - vmd->vrt_minor > VRT_MINOR_VERSION) { + if (vmd->vrt_major != 0 && (vmd->vrt_major != VRT_MAJOR_VERSION || + vmd->vrt_minor > VRT_MINOR_VERSION)) { VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod)); VSB_printf(tl->sb, "\tFile name: %s\n", fnp); VSB_printf(tl->sb, "\tVMOD version %u.%u\n", diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py index 90b888eb5b0..ab78cff023d 100755 --- a/lib/libvcc/vmodtool.py +++ b/lib/libvcc/vmodtool.py @@ -44,6 +44,7 @@ import random rstfmt = False +strict_abi = True ctypes = { 'ACL': "VCL_ACL", @@ -434,6 +435,13 @@ def rsttail(self, fo, man): fo.write("* :ref:`%s`\n" % i[1]) fo.write("\n") +class s_abi(stanza): + def parse(self): + if self.line[1] not in ('strict', 'vrt'): + err("Valid ABI types are 'strict' or 'vrt', got '%s'\n" % + self.line[1]) + strict_abi = self.line[1] == 'strict' + class s_event(stanza): def parse(self): self.event_func = self.line[1] @@ -651,6 +659,8 @@ def parse(self): err("$Module must be first stanze") if c[0] == "Module": s_module(c, b[1:], self) + elif c[0] == "ABI": + s_abi(c, b[1:], self) elif c[0] == "Event": s_event(c, b[1:], self) elif c[0] == "Function": @@ -734,8 +744,12 @@ def api(self, fo): fo.write("\n/*lint -esym(%d, Vmod_%s_Data) */\n" % (i, self.modname)) fo.write("const struct vmod_data Vmod_%s_Data = {\n" % self.modname) - fo.write("\t.vrt_major =\tVRT_MAJOR_VERSION,\n") - fo.write("\t.vrt_minor =\tVRT_MINOR_VERSION,\n") + if strict_abi: + fo.write("\t.vrt_major =\t0,\n") + fo.write("\t.vrt_minor =\t0,\n") + else: + fo.write("\t.vrt_major =\tVRT_MAJOR_VERSION,\n") + fo.write("\t.vrt_minor =\tVRT_MINOR_VERSION,\n") fo.write('\t.name =\t\t"%s",\n' % self.modname) fo.write('\t.func =\t\t&Vmod_Func,\n') fo.write('\t.func_len =\tsizeof(Vmod_Func),\n') diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc index 93234886256..1b7889d9bdb 100644 --- a/lib/libvmod_debug/vmod.vcc +++ b/lib/libvmod_debug/vmod.vcc @@ -26,6 +26,7 @@ # SUCH DAMAGE. $Module debug 3 Development, test and debug +$ABI strict DESCRIPTION =========== diff --git a/lib/libvmod_directors/vmod.vcc b/lib/libvmod_directors/vmod.vcc index cce8acb25ab..9282ba35061 100644 --- a/lib/libvmod_directors/vmod.vcc +++ b/lib/libvmod_directors/vmod.vcc @@ -33,6 +33,7 @@ # SUCH DAMAGE. $Module directors 3 Varnish Directors Module +$ABI strict DESCRIPTION =========== diff --git a/lib/libvmod_std/vmod.vcc b/lib/libvmod_std/vmod.vcc index 0923fb7203a..c98eb262d76 100644 --- a/lib/libvmod_std/vmod.vcc +++ b/lib/libvmod_std/vmod.vcc @@ -26,6 +26,7 @@ # SUCH DAMAGE. $Module std 3 Varnish Standard Module +$ABI strict DESCRIPTION ===========