Mailing List Archive

r1423 - trunk/varnish-cache/bin/varnishd
Author: des
Date: 2007-05-15 13:35:44 +0200 (Tue, 15 May 2007)
New Revision: 1423

Modified:
trunk/varnish-cache/bin/varnishd/common.h
trunk/varnish-cache/bin/varnishd/heritage.h
trunk/varnish-cache/bin/varnishd/mgt_child.c
trunk/varnish-cache/bin/varnishd/mgt_cli.c
trunk/varnish-cache/bin/varnishd/mgt_param.c
trunk/varnish-cache/bin/varnishd/tcp.c
Log:
Attempt to fix the bind-to-any problem:

- Introduce a "struct tcp_addr" which is a lightweight form of struct
addrinfo for our own internal use.

- Add a TCP_resolve() function which takes the output from TCP_parse()
and fills in a list of pointers to struct tcp_addr, one for each
address returned by getaddrinfo().

- Modify all TCP_open() callers to use TCP_resolve() and call TCP_open()
once for every address returned.

- Add some explanatory comments to tcp.c.


Modified: trunk/varnish-cache/bin/varnishd/common.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/common.h 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/common.h 2007-05-15 11:35:44 UTC (rev 1423)
@@ -41,8 +41,10 @@
#define TCP_ADDRBUFSIZE 64
#define TCP_PORTBUFSIZE 16

+struct tcp_addr;
+
void TCP_name(struct sockaddr *addr, unsigned l, char *abuf, unsigned alen, char *pbuf, unsigned plen);
void TCP_myname(int sock, char *abuf, unsigned alen, char *pbuf, unsigned plen);
int TCP_parse(const char *str, char **addr, char **port);
-int TCP_open(const char *addr, const char *port, int http);
-void TCP_check(struct cli *cli, const char *addr, const char *port);
+int TCP_resolve(const char *addr, const char *port, struct tcp_addr ***ta);
+int TCP_open(const struct tcp_addr *addr, int http);

Modified: trunk/varnish-cache/bin/varnishd/heritage.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/heritage.h 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/heritage.h 2007-05-15 11:35:44 UTC (rev 1423)
@@ -36,8 +36,7 @@
struct listen_sock {
TAILQ_ENTRY(listen_sock) list;
int sock;
- char *host;
- char *port;
+ struct tcp_addr *addr;
};

TAILQ_HEAD(listen_sock_head, listen_sock);

Modified: trunk/varnish-cache/bin/varnishd/mgt_child.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_child.c 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/mgt_child.c 2007-05-15 11:35:44 UTC (rev 1423)
@@ -130,7 +130,7 @@
TAILQ_FOREACH(ls, &heritage.socks, list) {
if (ls->sock >= 0)
continue;
- ls->sock = TCP_open(ls->host, ls->port, 1);
+ ls->sock = TCP_open(ls->addr, 1);
if (ls->sock < 0)
return (1);
}

Modified: trunk/varnish-cache/bin/varnishd/mgt_cli.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_cli.c 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/mgt_cli.c 2007-05-15 11:35:44 UTC (rev 1423)
@@ -55,8 +55,6 @@
#include "shmlog.h"

static int cli_i = -1, cli_o = -1;
-static int telnet_sock;
-static struct ev *telnet_ev;

/*--------------------------------------------------------------------*/

@@ -374,14 +372,13 @@
static int
telnet_accept(struct ev *ev, int what)
{
- socklen_t l;
- struct sockaddr addr[2]; /* XXX IPv6 hack */
+ struct sockaddr_storage addr;
+ socklen_t addrlen;
int i;

- (void)ev;
(void)what;
- l = sizeof addr;
- i = accept(telnet_sock, addr, &l);
+ addrlen = sizeof addr;
+ i = accept(ev->fd, (struct sockaddr *)&addr, &addrlen);
if (i < 0)
return (0);

@@ -392,21 +389,29 @@
int
mgt_cli_telnet(const char *T_arg)
{
+ struct tcp_addr **ta;
char *addr, *port;
+ int i, n;

- TCP_parse(T_arg, &addr, &port);
- telnet_sock = TCP_open(addr, port, 0);
+ XXXAZ(TCP_parse(T_arg, &addr, &port));
+ XXXAN(n = TCP_resolve(addr, port, &ta));
free(addr);
free(port);
- if (telnet_sock < 0) {
+ if (n == 0) {
fprintf(stderr, "Could not open TELNET port\n");
- exit (2);
+ exit(2);
}
- telnet_ev = ev_new();
- XXXAN(telnet_ev);
- telnet_ev->fd = telnet_sock;
- telnet_ev->fd_flags = POLLIN;
- telnet_ev->callback = telnet_accept;
- ev_add(mgt_evb, telnet_ev);
+ for (i = 0; i < n; ++i) {
+ int sock = TCP_open(ta[i], 0);
+ struct ev *ev = ev_new();
+ XXXAN(ev);
+ ev->fd = sock;
+ ev->fd_flags = POLLIN;
+ ev->callback = telnet_accept;
+ ev_add(mgt_evb, ev);
+ free(ta[i]);
+ ta[i] = NULL;
+ }
+ free(ta);
return (0);
}

Modified: trunk/varnish-cache/bin/varnishd/mgt_param.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_param.c 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/mgt_param.c 2007-05-15 11:35:44 UTC (rev 1423)
@@ -29,11 +29,11 @@
* $Id$
*/

+#include <limits.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
-#include <stdlib.h>
#include <unistd.h>
-#include <limits.h>

#include "cli.h"
#include "cli_priv.h"
@@ -284,8 +284,7 @@

TAILQ_FOREACH_SAFE(ls, lsh, list, ls2) {
TAILQ_REMOVE(lsh, ls, list);
- free(ls->host);
- free(ls->port);
+ free(ls->addr);
free(ls);
}
}
@@ -323,21 +322,31 @@
}
TAILQ_INIT(&lsh);
for (i = 1; av[i] != NULL; i++) {
- ls = calloc(sizeof *ls, 1);
- AN(ls);
- ls->sock = -1;
- TAILQ_INSERT_TAIL(&lsh, ls, list);
- if (TCP_parse(av[i], &ls->host, &ls->port) != 0) {
+ struct tcp_addr **ta;
+ char *host, *port;
+ int j, n;
+
+ if (TCP_parse(av[i], &host, &port) != 0) {
cli_out(cli, "Invalid listen address \"%s\"", av[i]);
cli_result(cli, CLIS_PARAM);
break;
}
- if (ls->port == NULL)
- ls->port = strdup("http");
- AN(ls->port);
- TCP_check(cli, ls->host, ls->port);
- if (cli->result != CLIS_OK)
+ n = TCP_resolve(host, port ? port : "http", &ta);
+ free(host);
+ free(port);
+ if (n == 0) {
+ cli_out(cli, "Invalid listen address \"%s\"", av[i]);
+ cli_result(cli, CLIS_PARAM);
break;
+ }
+ for (j = 0; j < n; ++j) {
+ ls = calloc(sizeof *ls, 1);
+ AN(ls);
+ ls->sock = -1;
+ ls->addr = ta[j];
+ TAILQ_INSERT_TAIL(&lsh, ls, list);
+ }
+ free(ta);
}
FreeArgv(av);
if (cli->result != CLIS_OK) {

Modified: trunk/varnish-cache/bin/varnishd/tcp.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/tcp.c 2007-05-15 11:19:29 UTC (rev 1422)
+++ trunk/varnish-cache/bin/varnishd/tcp.c 2007-05-15 11:35:44 UTC (rev 1423)
@@ -53,6 +53,15 @@
#include "cli.h"
#include "cli_priv.h"

+/* lightweight addrinfo */
+struct tcp_addr {
+ int ta_family;
+ int ta_socktype;
+ int ta_protocol;
+ socklen_t ta_addrlen;
+ struct sockaddr_storage ta_addr;
+};
+
/*--------------------------------------------------------------------*/

void
@@ -109,6 +118,16 @@
}
#endif

+/*
+ * Take a string provided by the user and break it up into address and
+ * port parts. Examples of acceptable input include:
+ *
+ * "localhost" - "localhost:80"
+ * "127.0.0.1" - "127.0.0.1:80"
+ * "0.0.0.0" - "0.0.0.0:80"
+ * "[::1]" - "[::1]:80"
+ * "[::]" - "[::]:80"
+ */
int
TCP_parse(const char *str, char **addr, char **port)
{
@@ -123,88 +142,124 @@
(p[1] != '\0' && p[1] != ':'))
return (-1);
*addr = strndup(str + 1, p - (str + 1));
- if (p[1] == ':')
+ XXXAN(*addr);
+ if (p[1] == ':') {
*port = strdup(p + 2);
+ XXXAN(*port);
+ }
} else {
/* IPv4 address of the form 127.0.0.1:80, or non-numeric */
p = strchr(str, ':');
if (p == NULL) {
*addr = strdup(str);
+ XXXAN(*addr);
} else {
- if (p > str)
+ if (p > str) {
*addr = strndup(str, p - str);
+ XXXAN(*addr);
+ }
*port = strdup(p + 1);
+ XXXAN(*port);
}
}
return (0);
}

-/*--------------------------------------------------------------------*/
-
-void
-TCP_check(struct cli *cli, const char *addr, const char *port)
+/*
+ * For a given host and port, return a list of struct tcp_addr, which
+ * contains all the information necessary to open and bind a socket. One
+ * tcp_addr is returned for each distinct address returned by
+ * getaddrinfo().
+ *
+ * The value pointed to by the tap parameter receives a pointer to an
+ * array of pointers to struct tcp_addr. The caller is responsible for
+ * freeing each individual struct tcp_addr as well as the array.
+ *
+ * The return value is the number of addresses resoved, or zero.
+ */
+int
+TCP_resolve(const char *addr, const char *port, struct tcp_addr ***tap)
{
- struct addrinfo hints, *res;
- int ret;
+ struct addrinfo hints, *res0, *res;
+ struct tcp_addr **ta;
+ int i, ret;

- memset(&hints, 0, sizeof hints);
- hints.ai_socktype = SOCK_STREAM;
- hints.ai_flags = AI_PASSIVE;
- ret = getaddrinfo(addr, port, &hints, &res);
- if (ret == 0) {
- freeaddrinfo(res);
- return;
+ memset(&hints, 0, sizeof hints);
+ hints.ai_socktype = SOCK_STREAM;
+ hints.ai_flags = AI_PASSIVE;
+ ret = getaddrinfo(addr, port, &hints, &res0);
+ if (ret != 0) {
+ fprintf(stderr, "getaddrinfo(): %s\n", gai_strerror(ret));
+ return (0);
+ }
+ for (res = res0, i = 0; res != NULL; res = res->ai_next)
+ ++i;
+ ta = calloc(i, sizeof *ta);
+ XXXAN(ta);
+ *tap = ta;
+ for (res = res0, i = 0; res != NULL; res = res->ai_next, ++i) {
+ ta[i] = calloc(1, sizeof *ta[i]);
+ XXXAN(ta[i]);
+ ta[i]->ta_family = res->ai_family;
+ ta[i]->ta_socktype = res->ai_socktype;
+ ta[i]->ta_protocol = res->ai_protocol;
+ ta[i]->ta_addrlen = res->ai_addrlen;
+ xxxassert(ta[i]->ta_addrlen <= sizeof ta[i]->ta_addr);
+ memcpy(&ta[i]->ta_addr, res->ai_addr, ta[i]->ta_addrlen);
}
- cli_out(cli, "getaddrinfo(%s, %s): %s\n",
- addr, port, gai_strerror(ret));
- cli_result(cli, CLIS_PARAM);
+ freeaddrinfo(res0);
+ return (i);
}

+/*
+ * Given a struct tcp_addr, open a socket of the appropriate type, bind it
+ * to the requested address, and start listening.
+ *
+ * If the address is an IPv6 address, the IPV6_V6ONLY option is set to
+ * avoid conflicts between INADDR_ANY and IN6ADDR_ANY.
+ *
+ * If the http parameter is non-zero and accept filters are available,
+ * install an HTTP accept filter on the socket.
+ */
int
-TCP_open(const char *addr, const char *port, int http)
+TCP_open(const struct tcp_addr *ta, int http)
{
- struct addrinfo hints, *res;
- int ret, sd, val;
+ int sd, val;

- memset(&hints, 0, sizeof hints);
- hints.ai_socktype = SOCK_STREAM;
- hints.ai_flags = AI_PASSIVE;
- ret = getaddrinfo(addr, port, &hints, &res);
- if (ret != 0) {
- fprintf(stderr, "getaddrinfo(): %s\n", gai_strerror(ret));
- return (-1);
- }
- sd = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
+ sd = socket(ta->ta_family, ta->ta_socktype, ta->ta_protocol);
if (sd < 0) {
perror("socket()");
- freeaddrinfo(res);
return (-1);
}
val = 1;
if (setsockopt(sd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof val) != 0) {
perror("setsockopt(SO_REUSEADDR, 1)");
- freeaddrinfo(res);
close(sd);
return (-1);
}
- if (bind(sd, res->ai_addr, res->ai_addrlen) != 0) {
+#ifdef IPV6_V6ONLY
+ /* forcibly use separate sockets for IPv4 and IPv6 */
+ val = 1;
+ if (ta->ta_family == AF_INET6 &&
+ setsockopt(sd, IPPROTO_IPV6, IPV6_V6ONLY, &val, sizeof val) != 0) {
+ perror("setsockopt(IPV6_V6ONLY, 1)");
+ close(sd);
+ return (-1);
+ }
+#endif
+ if (bind(sd, (const struct sockaddr *)&ta->ta_addr, ta->ta_addrlen) != 0) {
perror("bind()");
- freeaddrinfo(res);
close(sd);
return (-1);
}
if (listen(sd, http ? params->listen_depth : 16) != 0) {
perror("listen()");
- freeaddrinfo(res);
close(sd);
return (-1);
}
#ifdef HAVE_ACCEPT_FILTERS
if (http)
accept_filter(sd);
-#else
- (void)http;
#endif
- freeaddrinfo(res);
return (sd);
}
r1423 - trunk/varnish-cache/bin/varnishd [ In reply to ]
In message <20070515113544.4788B1EC439 at projects.linpro.no>, des at projects.linpro
.no writes:

> struct listen_sock {
> TAILQ_ENTRY(listen_sock) list;
> int sock;
>- char *host;
>- char *port;
>+ struct tcp_addr *addr;
> };

I'm not happy with this one.

I kept the string forms given in argv around so that we could
do periodic DNS lookups on it, to see if things moved.

--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
r1423 - trunk/varnish-cache/bin/varnishd [ In reply to ]
"Poul-Henning Kamp" <phk at phk.freebsd.dk> writes:
> I'm not happy with this one.
>
> I kept the string forms given in argv around so that we could
> do periodic DNS lookups on it, to see if things moved.

Well, no such periodic lookups were implemented, so I had no way of
knowing they were intended.

We can't keep the string forms in struct listen_sock anyway, because a
single string can resolve to multiple sockets.

We could probably store the address spec in a run-time parameter,
though.

DES
--
Dag-Erling Sm?rgrav
Senior Software Developer
Linpro AS - www.linpro.no