[master] cc885760c Add "+table" flag to ACL's

Poul-Henning Kamp phk at FreeBSD.org
Mon Apr 12 12:09:05 UTC 2021


commit cc885760c1b2fbd6467084a3437a2c5327f6407d
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Apr 12 11:58:00 2021 +0000

    Add "+table" flag to ACL's
    
    The +table flag causes all overlapped ACL entries to be compiled
    as usual, and non-overlapped entries to be emitted as a table of
    bytes.
    
    When testing the ACL the compiled code is run before VPI_acl_table()
    is used to do a binary search on the table.
    
    Even with a binary search, the table is approx 3 times slower than
    the regular compiled ACLs (ie: only "blindingly fast" as oppposed
    to "lighting fast").
    
    The advantage of +table is that C-compilers literally take no time,
    no matter the size of the ACL, where they will take seconds
    or even minutes compiling large ACLs as code.

diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 5cdaf2b3f..76b427418 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -138,6 +138,74 @@ VRT_acl_match(VRT_CTX, VCL_ACL acl, VCL_IP ip)
 	return (acl->match(ctx, ip));
 }
 
+static int
+acl_tbl_cmp(int fam, const uint8_t *key, const uint8_t *b)
+{
+	int rv;
+
+	rv = fam - (int)b[3];
+	if (rv == 0 && b[1] > 0)
+		rv = memcmp(key, b + 4, b[1]);
+	if (rv == 0 && b[2])
+		rv = (int)(key[b[1]] & b[2]) - (int)b[4 + b[1]];
+	return (rv);
+}
+
+static const uint8_t *
+bbsearch(int fam, const uint8_t *key, const uint8_t *base0,
+    size_t nmemb, size_t size)
+{
+        const uint8_t *base = base0;
+        size_t lim;
+        int cmp;
+        const uint8_t *p;
+
+        for (lim = nmemb; lim != 0; lim >>= 1) {
+                p = base + (lim >> 1) * size;
+                cmp = acl_tbl_cmp(fam, key, p);
+                if (cmp == 0)
+                        return (p);
+                if (cmp > 0) {
+			/* key > p: move right */
+                        base = p + size;
+                        lim--;
+                } /* else move left */
+        }
+        return (NULL);
+}
+
+int
+VPI_acl_table(VRT_CTX, VCL_IP p, unsigned n, unsigned m, const uint8_t *tbl,
+    const char * const *str, const char *fin)
+{
+	int fam;
+	const uint8_t *key;
+	const uint8_t *ptr;
+	size_t sz;
+
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	AN(p);
+	AN(n);
+	assert(m == 20);
+	AN(tbl);
+	AN(fin);
+
+	fam = VRT_VSA_GetPtr(ctx, p, &key);
+	ptr = bbsearch(fam, key, tbl, n, m);
+
+	if (ptr != NULL) {
+		sz = ptr - tbl;
+		AZ(sz % m);
+		sz /= m;
+		if (str != NULL)
+			VPI_acl_log(ctx, str[sz]);
+		return *ptr;
+	}
+	if (str != NULL)
+		VPI_acl_log(ctx, fin);
+	return (0);
+}
+
 /*--------------------------------------------------------------------*/
 
 VCL_VOID
diff --git a/bin/varnishtest/tests/r01312.vtc b/bin/varnishtest/tests/r01312.vtc
index 583871d84..ded2787e2 100644
--- a/bin/varnishtest/tests/r01312.vtc
+++ b/bin/varnishtest/tests/r01312.vtc
@@ -1,24 +1,6 @@
 varnishtest "acl functional (& historic miscompile)"
 
-server s1 {
-	rxreq
-	txresp
-} -start
-
-varnish v1 -vcl+backend {
-	import std;
-	import debug;
-
-	acl foo {
-		"127.0.0.2";
-		"127.0.1"/19;
-	}
-	acl bar {
-		"127.0.1.2";
-		"127.0.1"/19;
-	}
-
-	acl block {
+shell {echo '
 		# Tests all boundary conditions
 		  "192.168.8.0" / 21;
 		! "192.168.16" / 21;
@@ -46,6 +28,29 @@ varnish v1 -vcl+backend {
 		! "::0100" / 124;
 		  "::0130" / 124;
 		  "::0170" / 124;
+' > ${tmpdir}/_acl.vcl 
+}
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {
+	import std;
+	import debug;
+
+	acl foo {
+		"127.0.0.2";
+		"127.0.1"/19;
+	}
+	acl bar {
+		"127.0.1.2";
+		"127.0.1"/19;
+	}
+
+	acl block {
+		include "${tmpdir}/_acl.vcl";
 	}
 
 	sub vcl_deliver {
@@ -100,3 +105,67 @@ client c1 {
 } -run
 
 logexpect l1 -wait
+
+varnish v1 -vcl+backend {
+	import std;
+	import debug;
+
+	acl block +table {
+		include "${tmpdir}/_acl.vcl";
+	}
+
+	sub vcl_deliver {
+		set resp.http.acl4 = debug.sweep_acl(
+		    block,
+		    std.ip("192.168.0.0"),
+		    std.ip("192.168.32.255"),
+		    256
+		);
+		set resp.http.acl6 = debug.sweep_acl(
+		    block,
+		    std.ip("::"),
+		    std.ip("::0200"),
+		    16
+		);
+	}
+}
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.acl4 == ":4thASR0O18ZxnoKtc4zd8KuO25rPvwvMQyAvRfilz6o=:"
+	expect resp.http.acl6 == ":NSi+7wpvQe7XJj8DPbESjpYPGnIzvjOsA5QCyCnW3kc=:"
+} -run
+
+varnish v1 -vcl+backend {
+	import std;
+	import debug;
+
+	acl block +log +table {
+		include "${tmpdir}/_acl.vcl";
+	}
+
+	sub vcl_deliver {
+		set resp.http.acl4 = debug.sweep_acl(
+		    block,
+		    std.ip("192.168.0.0"),
+		    std.ip("192.168.32.255"),
+		    256
+		);
+		set resp.http.acl6 = debug.sweep_acl(
+		    block,
+		    std.ip("::"),
+		    std.ip("::0200"),
+		    16
+		);
+	}
+}
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.acl4 == ":4thASR0O18ZxnoKtc4zd8KuO25rPvwvMQyAvRfilz6o=:"
+	expect resp.http.acl6 == ":NSi+7wpvQe7XJj8DPbESjpYPGnIzvjOsA5QCyCnW3kc=:"
+} -run
+
+shell {rm -f ${tmpdir}/_acl.vcl}
diff --git a/include/vcc_interface.h b/include/vcc_interface.h
index 7ad670ea3..8c81bddc0 100644
--- a/include/vcc_interface.h
+++ b/include/vcc_interface.h
@@ -61,6 +61,8 @@ int VPI_Vmod_Init(VRT_CTX, struct vmod **hdl, unsigned nbr, void *ptr, int len,
 void VPI_Vmod_Unload(VRT_CTX, struct vmod **hdl);
 
 typedef int acl_match_f(VRT_CTX, const VCL_IP);
+int VPI_acl_table(VRT_CTX, VCL_IP, unsigned n, unsigned m, const uint8_t *tbl,
+    const char * const *str, const char *fin);
 
 struct vrt_acl {
 	unsigned        magic;
diff --git a/lib/libvcc/vcc_acl.c b/lib/libvcc/vcc_acl.c
index 6f2c2c923..226215fda 100644
--- a/lib/libvcc/vcc_acl.c
+++ b/lib/libvcc/vcc_acl.c
@@ -53,6 +53,7 @@ struct acl {
 #define VCC_ACL_MAGIC		0xb9fb3cd0
 
 	int			flag_log;
+	int			flag_table;
 
 	struct acl_tree		acl_tree;
 };
@@ -65,6 +66,7 @@ struct acl_e {
 	unsigned		mask;
 	unsigned		not;
 	unsigned		para;
+	unsigned		overlapped;
 	char			*addr;
 	const char		*fixed;
 	struct token		*t_addr;
@@ -121,10 +123,38 @@ vcl_acl_cmp(const struct acl_e *ae1, const struct acl_e *ae2)
 	return (0);
 }
 
+static int
+vcl_acl_disjoint(const struct acl_e *ae1, const struct acl_e *ae2)
+{
+	const unsigned char *p1, *p2;
+	unsigned m;
+
+	CHECK_OBJ_NOTNULL(ae1, VCC_ACL_E_MAGIC);
+	CHECK_OBJ_NOTNULL(ae2, VCC_ACL_E_MAGIC);
+
+	p1 = ae1->data;
+	p2 = ae2->data;
+	m = ae1->mask;
+	if (ae2->mask < m)
+		m = ae2->mask;
+	for (; m >= 8; m -= 8) {
+		CMP(*p1, *p2);
+		p1++;
+		p2++;
+	}
+	if (m) {
+		m = 0xff00 >> m;
+		m &= 0xff;
+		CMP(*p1 & m, *p2 & m);
+	}
+	return (0);
+}
+
 VRBT_GENERATE_INSERT_COLOR(acl_tree, acl_e, branch, static)
 VRBT_GENERATE_INSERT(acl_tree, acl_e, branch, vcl_acl_cmp, static)
 VRBT_GENERATE_MINMAX(acl_tree, acl_e, branch, static)
 VRBT_GENERATE_NEXT(acl_tree, acl_e, branch, static)
+VRBT_GENERATE_PREV(acl_tree, acl_e, branch, static)
 
 static char *
 vcc_acl_chk(struct vcc *tl, const struct acl_e *ae, const int l,
@@ -454,6 +484,52 @@ vcc_acl_emit_tokens(const struct vcc *tl, const struct acl_e *ae)
 		Fh(tl, 0, "\" fixed: %s\"", ae->fixed);
 }
 
+/*********************************************************************
+ * Emit ACL on table format
+ */
+
+static unsigned
+vcc_acl_emit_tables(const struct vcc *tl, unsigned n, const char *name)
+{
+	struct acl_e *ae;
+	unsigned rv = sizeof(ae->data) + 3;
+	unsigned nn = 0;
+	size_t sz;
+
+	Fh(tl, 0, "\nstatic unsigned char acl_tbl_%s[%u*%u] = {\n",
+	    name, n, rv);
+	VRBT_FOREACH(ae, acl_tree, &tl->acl->acl_tree) {
+		if (ae->overlapped)
+			continue;
+		Fh(tl, 0, "\t0x%02x,", ae->not ? 0 : 1);
+		Fh(tl, 0, "0x%02x,", (ae->mask >> 3) - 1);
+		Fh(tl, 0, "0x%02x,", (0xff00 >> (ae->mask & 7)) & 0xff);
+		for (sz = 0; sz < sizeof(ae->data); sz++)
+			Fh(tl, 0, "0x%02x,", ae->data[sz]);
+		for (; sz < rv - 3; sz++)
+			Fh(tl, 0, "0,");
+		Fh(tl, 0, "\n");
+		nn++;
+	}
+	assert(n == nn);
+	Fh(tl, 0, "};\n");
+	if (tl->acl->flag_log) {
+		Fh(tl, 0, "\nstatic const char *acl_str_%s[%d] = {\n",
+		    name, n);
+		VRBT_FOREACH(ae, acl_tree, &tl->acl->acl_tree) {
+			if (ae->overlapped)
+				continue;
+			Fh(tl, 0, "\t");
+			Fh(tl, 0, "\"%sMATCH %s \" ",
+			    ae->not ? "NEG_" : "", name);
+			vcc_acl_emit_tokens(tl, ae);
+			Fh(tl, 0, ",\n");
+		}
+		Fh(tl, 0, "};\n");
+	}
+	return  (rv);
+}
+
 /*********************************************************************
  * Emit a function to match the ACL we have collected
  */
@@ -461,11 +537,12 @@ vcc_acl_emit_tokens(const struct vcc *tl, const struct acl_e *ae)
 static void
 vcc_acl_emit(struct vcc *tl, const struct symbol *sym)
 {
-	struct acl_e *ae;
+	struct acl_e *ae, *ae2;
 	int depth, l, m, i;
 	unsigned at[ACL_MAXADDR];
 	struct inifin *ifp = NULL;
 	struct vsb *func;
+	unsigned n, no, nw = 0;
 
 	func = VSB_new_auto();
 	AN(func);
@@ -473,6 +550,32 @@ vcc_acl_emit(struct vcc *tl, const struct symbol *sym)
 	VCC_PrintCName(func, sym->name, NULL);
 	AZ(VSB_finish(func));
 
+	depth = -1;
+	at[0] = 256;
+	ae2 = NULL;
+	n = no = 0;
+	VRBT_FOREACH_REVERSE(ae, acl_tree, &tl->acl->acl_tree) {
+		n++;
+		if (ae2 == NULL) {
+			ae2 = ae;
+		} else if (vcl_acl_disjoint(ae, ae2)) {
+			ae2 = ae;
+		} else {
+			no++;
+			ae->overlapped = 1;
+		}
+	}
+
+	Fh(tl, 0, "/* acl_n_%s n %u no %u */\n", sym->name, n, no);
+	if (n - no < (1<<1))
+		no = n;
+	else if (!tl->acl->flag_table)
+		no = n;
+
+	if (no < n)
+		nw = vcc_acl_emit_tables(tl, n - no, sym->name);
+
+
 	Fh(tl, 0, "\nstatic int v_matchproto_(acl_match_f)\n");
 	Fh(tl, 0, "%s(VRT_CTX, const VCL_IP p)\n", VSB_data(func));
 	Fh(tl, 0, "{\n");
@@ -490,10 +593,12 @@ vcc_acl_emit(struct vcc *tl, const struct symbol *sym)
 		VSB_printf(ifp->ini,
 			"\tif (0) %s(0, 0);\n", VSB_data(func));
 	}
-	depth = -1;
-	at[0] = 256;
+
 	VRBT_FOREACH(ae, acl_tree, &tl->acl->acl_tree) {
 
+		if (no < n && !ae->overlapped)
+			continue;
+
 		/* Find how much common prefix we have */
 		for (l = 0; l <= depth && l * 8 < (int)ae->mask - 7; l++) {
 			assert(l >= 0);
@@ -549,10 +654,24 @@ vcc_acl_emit(struct vcc *tl, const struct symbol *sym)
 	for (; 0 <= depth; depth--)
 		Fh(tl, 0, "\t%*.*s}\n", depth, depth, "");
 
-	/* Deny by default */
-	if (tl->acl->flag_log)
-		Fh(tl, 0, "\tVPI_acl_log(ctx, \"NO_MATCH %s\");\n", sym->name);
-	Fh(tl, 0, "\treturn (0);\n}\n");
+	if (no < n) {
+		Fh(tl, 0, "\treturn(\n\t    VPI_acl_table(ctx,\n");
+		Fh(tl, 0, "\t\tp,\n");
+		Fh(tl, 0, "\t\t%u, %u,\n", n - no, nw);
+		Fh(tl, 0, "\t\tacl_tbl_%s,\n", sym->name);
+		if (tl->acl->flag_log)
+			Fh(tl, 0, "\t\tacl_str_%s,\n", sym->name);
+		else
+			Fh(tl, 0, "\t\tNULL,\n");
+		Fh(tl, 0, "\t\t\"NO MATCH %s\"\n\t    )\n\t);\n", sym->name);
+	} else {
+		/* Deny by default */
+		if (tl->acl->flag_log)
+			Fh(tl, 0, "\tVPI_acl_log(ctx, \"NO_MATCH %s\");\n",
+			    sym->name);
+		Fh(tl, 0, "\treturn(0);\n");
+	}
+	Fh(tl, 0, "}\n");
 
 	/* Emit the struct that will be referenced */
 	Fh(tl, 0, "\nstatic const struct vrt_acl %s[] = {{\n", sym->rname);
@@ -596,6 +715,9 @@ vcc_ParseAcl(struct vcc *tl)
 		if (vcc_IdIs(tl->t, "log")) {
 			acl->flag_log = sign->tok == '+';
 			vcc_NextToken(tl);
+		} else if (vcc_IdIs(tl->t, "table")) {
+			acl->flag_table = sign->tok == '+';
+			vcc_NextToken(tl);
 		} else {
 			VSB_cat(tl->sb, "Unknown ACL flag:\n");
 			vcc_ErrWhere(tl, tl->t);


More information about the varnish-commit mailing list