[master] 7b21e7e Start introducing "struct suckaddr" which will become our new VCL_IP type.
Poul-Henning Kamp
phk at varnish-cache.org
Sun Oct 27 21:46:54 CET 2013
commit 7b21e7e3a735d472162f7f9e05e89a44bf9855ef
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Sun Oct 27 20:46:25 2013 +0000
Start introducing "struct suckaddr" which will become our new VCL_IP type.
diff --git a/bin/varnishd/flint.lnt b/bin/varnishd/flint.lnt
index e203cb8..bfa3ccb 100644
--- a/bin/varnishd/flint.lnt
+++ b/bin/varnishd/flint.lnt
@@ -1,4 +1,5 @@
-d__flexelint_v9__=1
++fan
-printf(3, VSL)
-printf(2, http_PrintfHeader)
diff --git a/include/vrt.h b/include/vrt.h
index e203168..18b0fba 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -42,6 +42,7 @@ struct cli;
struct director;
struct VCL_conf;
struct sockaddr_storage;
+struct suckaddr;
/***********************************************************************
* This is the central definition of the mapping from VCL types to
@@ -225,6 +226,9 @@ void VRT_synth_page(const struct vrt_ctx *, unsigned flags, const char *, ...);
void VRT_init_dir(struct cli *, struct director **, int idx, const void *priv);
void VRT_fini_dir(struct cli *, struct director *);
+/* Suckaddr related */
+int VRT_VSA_GetPtr(const struct suckaddr *sua, const unsigned char ** dst);
+
/* VMOD/Modules related */
int VRT_Vmod_Init(void **hdl, void *ptr, int len, const char *nm,
const char *path, struct cli *cli);
diff --git a/include/vsa.h b/include/vsa.h
index 3ac8eb2..c1e5509 100644
--- a/include/vsa.h
+++ b/include/vsa.h
@@ -30,6 +30,9 @@
#ifndef VSA_H_INCLUDED
#define VSA_H_INCLUDED
+struct suckaddr;
+extern const int vsa_suckaddr_len;
+
int VSA_Sane(const void *);
socklen_t VSA_Len(const void *);
unsigned VSA_Port(const void *);
diff --git a/lib/libvarnish/vsa.c b/lib/libvarnish/vsa.c
index 1f2a92f..000678d 100644
--- a/lib/libvarnish/vsa.c
+++ b/lib/libvarnish/vsa.c
@@ -40,6 +40,162 @@
#include "vas.h"
#include "vsa.h"
+#include "vrt.h"
+
+/*
+ * Struct sockaddr{|_in|_in6|_storage} is absolutely the worst data
+ * structure I have ever seen gold-plated in international standards.
+ *
+ * Network addresses have multiple different forms, many fewer today
+ * than in last century, but imagine that in addition to IPv4 and IPv6
+ * we had 40 other protocols. Actually, you don't need to imagine that
+ * just count the AF_* macros in /usr/include/sys/socket.h.
+ *
+ * So what do we pass the kernel API for an address to bind(2), connect(2) &
+ * listen(2) etc. etc ?
+ *
+ * We could define a struct which is big enough to hold any and all
+ * of these addresses. That would make it a fixed size argument.
+ * obviously the struct would have to be something like:
+ * struct bla {
+ * int family;
+ * char address[MAX_ADDR_LEN];
+ * }
+ * and MAX_ADDR_LEN would have to be quite large, 128 byte or so.
+ *
+ * Back in last century that was TOTALLY unacceptable waste of space.
+ *
+ * The way which was chosen instead, was to make a "generic" address,
+ * and have per protocol "specific" addresses, and pass the length
+ * argument explicitly to the KPI functions.
+ *
+ * The generic address was called "struct sockaddr", and the specific
+ * were called "struct sockaddr_${whatever}". All of these must have
+ * a "family" field as first element, so the kernel can figure out
+ * which protocol it is.
+ *
+ * The generic struct sockaddr was made big enough for all protocols
+ * supported in the kernel, so it would have different sizes depending
+ * on your machine and kernel configuration.
+ *
+ * However, that allowed you to write protocol-agnostic programs, by
+ * using "struct sockaddr" throughout, and relying on libray APIs for
+ * things like name to address (and vice versa) resolution, and since
+ * nobody were in the business of shipping random UNIX binaries around
+ * the lack of binary portability didn't matter.
+ *
+ * Along the way the BSD people figured out that it was a bother
+ * to carry the length argument separately, and added that to the
+ * format of sockaddr, but other groups found this unclean, as
+ * the length was already an explicit paramter.
+ *
+ * The net result of this is that your "portable" code, must take
+ * care to handle the "sa_len" member on kernels which have it,
+ * while still tracking the separate length argument for all other
+ * kernels.
+ *
+ * Needless to say, there were no neat #define to tell you which
+ * was which, so each programmer found a different heuristic to
+ * decide, often not understanding it fully, which caused the kind
+ * of portability issues which lead to the autocrap tools.
+ *
+ * Then all the other protocols died, we were left with IP and
+ * life were good, the dot-com madness multiplied the IT-business
+ * by a factor 1000, by making any high-school student who had
+ * programmed PERL for 6 weeks a "senior web-programmer".
+ *
+ * Next IPv6 happened, in a rush even, (no seriously, I'm not kidding!),
+ * and since IPv6 addresses were HUGE, like 16 bytes HUGE, the generic
+ * struct sockaddr was not increased in size.
+ *
+ * At least "not yet", because it would break all the shitty code written
+ * by the dot-com generation.
+ *
+ * Nobody used IPv6 anyway so that didn't matter that much.
+ *
+ * Then people actually started using IPv6 and its struct sockaddr_in6,
+ * and realized that all the code which used "struct sockaddr" to allocate
+ * space at compile time were broken.
+ *
+ * Some people took to using sockaddr_in6, since that was known to
+ * be big enough for both IPv4 and IPv6, but "purist" found that
+ * ugly and "prone to future trouble".
+ *
+ * So instead they came up with a "clean solution": The added
+ * "struct sockaddr_storage" which is defined to be "Large enough
+ * to accommodate all supported protocol-specific address structures".
+ *
+ * Since we cannot possibly know what zany protocols will exist in
+ * the future, and since some people think that we will add future
+ * protocols, while retaining ABI compatibility, (totally overlooking
+ * the fact that no code for name-resolution supports that) it is
+ * usually defined so it can cope with 128 byte addresses.
+ *
+ * Does that ring a bell ?
+ *
+ * Only, not quite: Remember that all APIs require you to track
+ * the address and the length separately, so you only get the
+ * size of the specific protocols sockaddr_${whatever} from API
+ * functions, not a full sockaddr_storage, and besides the
+ * prototype for the KPI is still "struct sockaddr *", so you
+ * cannot gain C type-safety back by using sockaddr_storage
+ * as the "generic network address" type.
+ *
+ * So we have come full circle, while causing maximum havoc along
+ * the way and for the forseeable future.
+ *
+ * Do I need to tell you that static code analysis tools have a
+ * really hard time coping with this, and that they give a lot of
+ * false negatives which confuse people ?
+ *
+ * I have decided to try to contain this crap in this single
+ * source-file, with only minimum leakage into the rest of Varnish,
+ * which will only know of pointers to "struct suckaddr", the naming
+ * of which is my of the historical narrative above.
+ *
+ * And you don't need to take my word for this, you can see it all
+ * in various #include files on your own system. If you are on
+ * a Solaris derivative, don't miss the beautiful horror hidden in the
+ * variant definition of IPv6 addresses between kernel and userland.
+ *
+ */
+
+struct suckaddr {
+ union {
+ struct sockaddr sa;
+ struct sockaddr_in sa4;
+ struct sockaddr_in6 sa6;
+ };
+};
+
+const int vsa_suckaddr_len = sizeof(struct suckaddr);
+
+/*
+ * This VRT interface is for the VCC generated ACL code, which needs
+ * to know the address family and a pointer to the actual address.
+ */
+
+int
+VRT_VSA_GetPtr(const struct suckaddr *sua, const unsigned char ** dst)
+{
+ AN(dst);
+ if (sua == NULL)
+ return (-1);
+
+ switch(sua->sa.sa_family) {
+ case PF_INET:
+ assert(sua->sa.sa_family == sua->sa4.sin_family);
+ *dst = (const unsigned char *)&sua->sa4.sin_addr;
+ return (sua->sa4.sin_family);
+ case PF_INET6:
+ assert(sua->sa.sa_family == sua->sa6.sin6_family);
+ *dst = (const unsigned char *)&sua->sa6.sin6_addr;
+ return (sua->sa6.sin6_family);
+ default:
+ *dst = NULL;
+ return (-1);
+ }
+}
int
VSA_Sane(const void *s)
diff --git a/lib/libvcc/vcc_acl.c b/lib/libvcc/vcc_acl.c
index 433d848..182dbec 100644
--- a/lib/libvcc/vcc_acl.c
+++ b/lib/libvcc/vcc_acl.c
@@ -320,34 +320,6 @@ vcc_acl_entry(struct vcc *tl)
* Emit a function to match the ACL we have collected
*/
-/*
- * XXX: this is semi-silly. We try really hard to not depend in the
- * XXX: systems include files while compiling VCL, but we need to know
- * XXX: the size of the sa_familiy member.
- * XXX: FlexeLint complains about these antics, so isolate it in a
- * XXX: separate function.
- */
-
-/*lint -save -e506 -e774 -e550 */
-static void
-c_is_a_silly_language(const struct vcc *tl)
-{
- struct sockaddr sa;
-
- assert(sizeof (unsigned char) == 1);
- assert(sizeof (unsigned short) == 2);
- assert(sizeof (unsigned int) == 4);
- if (sizeof sa.sa_family == 1)
- Fh(tl, 0, "\tunsigned char fam;\n");
- else if (sizeof sa.sa_family == 2)
- Fh(tl, 0, "\tunsigned short fam;\n");
- else if (sizeof sa.sa_family == 4)
- Fh(tl, 0, "\tunsigned int fam;\n");
- else
- assert(0 == __LINE__);
-}
-/*lint -restore */
-
static void
vcc_acl_emit(const struct vcc *tl, const char *acln, int anon)
{
@@ -357,21 +329,14 @@ vcc_acl_emit(const struct vcc *tl, const char *acln, int anon)
const char *oc;
Fh(tl, 0, "\nstatic int\n");
- Fh(tl, 0, "match_acl_%s_%s(const struct vrt_ctx *ctx, const void *p)\n",
+ Fh(tl, 0, "match_acl_%s_%s(const struct vrt_ctx *ctx, const VCL_IP p)\n",
anon ? "anon" : "named", acln);
Fh(tl, 0, "{\n");
Fh(tl, 0, "\tconst unsigned char *a;\n");
- c_is_a_silly_language(tl);
-
+ Fh(tl, 0, "\tint fam;\n");
Fh(tl, 0, "\n");
- Fh(tl, 0, "\ta = p;\n");
- Fh(tl, 0, "\tVRT_memmove(&fam, a + %zd, sizeof fam);\n",
- offsetof(struct sockaddr, sa_family));
- Fh(tl, 0, "\tif (fam == %d)\n", PF_INET);
- Fh(tl, 0, "\t\ta += %zd;\n", offsetof(struct sockaddr_in, sin_addr));
- Fh(tl, 0, "\telse if (fam == %d)\n", PF_INET6);
- Fh(tl, 0, "\t\ta += %zd;\n", offsetof(struct sockaddr_in6, sin6_addr));
- Fh(tl, 0, "\telse {\n");
+ Fh(tl, 0, "\tfam = VRT_VSA_GetPtr(p, &a);\n");
+ Fh(tl, 0, "\tif (fam < 0) {\n");
Fh(tl, 0, "\t\tVRT_acl_log(ctx, \"NO_FAM %s\");\n", acln);
Fh(tl, 0, "\t\treturn(0);\n");
Fh(tl, 0, "\t}\n\n");
More information about the varnish-commit
mailing list