Mailing List Archive

r872 - trunk/varnish-cache/bin/varnishd
Author: des
Date: 2006-08-21 19:26:11 +0200 (Mon, 21 Aug 2006)
New Revision: 872

Modified:
trunk/varnish-cache/bin/varnishd/common.h
trunk/varnish-cache/bin/varnishd/mgt.h
trunk/varnish-cache/bin/varnishd/mgt_child.c
trunk/varnish-cache/bin/varnishd/mgt_cli.c
trunk/varnish-cache/bin/varnishd/mgt_vcc.c
trunk/varnish-cache/bin/varnishd/tcp.c
trunk/varnish-cache/bin/varnishd/varnishd.c
Log:
Rename open_tcp() to TCP_open() and modify it to open only one socket of
the appropriate type for the address that was passed in.

Introduce TCP_parse(), which extracts an address and port from a string of
the form "hostname:port", "i.p.v.4:port" or "[i:p:v:6]:port" (where "port"
can be either a decimal number or a service name).

Use TCP_parse() to parse the argument to -a (listen address), -b (backend
address) and -T (telnet address). Eliminate -p (listen port).

While there, rename a bunch of "fooflag" variables which aren't flags to
"foo_arg".

Modified: trunk/varnish-cache/bin/varnishd/common.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/common.h 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/common.h 2006-08-21 17:26:11 UTC (rev 872)
@@ -15,3 +15,5 @@

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);

Modified: trunk/varnish-cache/bin/varnishd/mgt.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt.h 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/mgt.h 2006-08-21 17:26:11 UTC (rev 872)
@@ -10,7 +10,7 @@
extern struct evbase *mgt_evb;

/* mgt_child.c */
-void mgt_run(int dflag, const char *Tflag);
+void mgt_run(int dflag, const char *T_arg);
extern pid_t mgt_pid, child_pid;

/* mgt_cli.c */
@@ -20,16 +20,13 @@
int mgt_cli_askchild(unsigned *status, char **resp, const char *fmt, ...);
void mgt_cli_start_child(int fdi, int fdo);
void mgt_cli_stop_child(void);
-int mgt_cli_telnet(const char *port);
+int mgt_cli_telnet(const char *T_arg);

/* mgt_vcc.c */
void mgt_vcc_init(void);
int mgt_vcc_default(const char *bflag, const char *fflag);
int mgt_push_vcls_and_start(unsigned *status, char **p);

-/* tcp.c */
-int open_tcp(const char *port, int http);
-
#include "stevedore.h"

extern struct stevedore sma_stevedore;

Modified: trunk/varnish-cache/bin/varnishd/mgt_child.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_child.c 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/mgt_child.c 2006-08-21 17:26:11 UTC (rev 872)
@@ -278,7 +278,7 @@
*/

void
-mgt_run(int dflag, const char *Tflag)
+mgt_run(int dflag, const char *T_arg)
{
struct sigaction sac;
struct ev *e;
@@ -292,8 +292,8 @@
if (dflag)
mgt_cli_setup(0, 1, 1);

- if (Tflag)
- mgt_cli_telnet(Tflag);
+ if (T_arg)
+ mgt_cli_telnet(T_arg);

e = ev_new();
assert(e != NULL);

Modified: trunk/varnish-cache/bin/varnishd/mgt_cli.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_cli.c 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/mgt_cli.c 2006-08-21 17:26:11 UTC (rev 872)
@@ -357,10 +357,14 @@
}

int
-mgt_cli_telnet(const char *port)
+mgt_cli_telnet(const char *T_arg)
{
+ char *addr, *port;

- telnet_sock = open_tcp(port, 0);
+ TCP_parse(T_arg, &addr, &port);
+ telnet_sock = TCP_open(addr, port, 0);
+ free(addr);
+ free(port);
if (telnet_sock < 0) {
fprintf(stderr, "Could not open TELNET port\n");
exit (2);

Modified: trunk/varnish-cache/bin/varnishd/mgt_vcc.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_vcc.c 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/mgt_vcc.c 2006-08-21 17:26:11 UTC (rev 872)
@@ -118,8 +118,9 @@
/*--------------------------------------------------------------------*/

int
-mgt_vcc_default(const char *bflag, const char *fflag)
+mgt_vcc_default(const char *b_arg, const char *f_arg)
{
+ char *addr, *port;
char *buf, *vf;
const char *p, *q;
struct vsb *sb;
@@ -127,7 +128,7 @@

sb = vsb_new(NULL, NULL, 0, VSB_AUTOEXTEND);
assert(sb != NULL);
- if (bflag != NULL) {
+ if (b_arg != NULL) {
/*
* XXX: should do a "HEAD /" on the -b argument to see that
* XXX: it even works. On the other hand, we should do that
@@ -136,26 +137,24 @@
* XXX: a bug for a backend to not reply at that time, so then
* XXX: again: we should check it here in the "trivial" case.
*/
- p = strchr(bflag, ' ');
- if (p != NULL) {
- q = p + 1;
- } else {
- p = strchr(bflag, '\0');
- assert(p != NULL);
- q = "http";
+ if (TCP_parse(b_arg, &addr, &port) != 0) {
+ fprintf(stderr, "invalid backend address\n");
+ return (1);
}

buf = NULL;
asprintf(&buf,
"backend default {\n"
- " set backend.host = \"%*.*s\";\n"
+ " set backend.host = \"%s\";\n"
" set backend.port = \"%s\";\n"
- "}\n", (int)(p - bflag), (int)(p - bflag), bflag, q);
+ "}\n", addr, port ? port : "http");
+ free(addr);
+ free(port);
assert(buf != NULL);
vf = VCC_Compile(sb, buf, NULL);
free(buf);
} else {
- vf = VCC_CompileFile(sb, fflag);
+ vf = VCC_CompileFile(sb, f_arg);
}
vsb_finish(sb);
if (vsb_len(sb) > 0) {

Modified: trunk/varnish-cache/bin/varnishd/tcp.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/tcp.c 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/tcp.c 2006-08-21 17:26:11 UTC (rev 872)
@@ -74,38 +74,55 @@
}
#endif

-static int
-try_sock(int family, const char *port, struct addrinfo **resp)
+int
+TCP_parse(const char *str, char **addr, char **port)
{
+ const char *p;
+
+ *addr = *port = NULL;
+
+ if (str[0] == '[') {
+ /* IPv6 address of the form [::1]:80 */
+ if ((p = strchr(str, ']')) == NULL ||
+ p == str + 1 ||
+ (p[1] != '\0' && p[1] != ':'))
+ return (-1);
+ *addr = strndup(str + 1, p - (str + 1));
+ if (p[1] == ':')
+ *port = strdup(p + 2);
+ } else {
+ /* IPv4 address of the form 127.0.0.1:80, or non-numeric */
+ p = strchr(str, ':');
+ if (p == NULL) {
+ *addr = strdup(str);
+ } else {
+ if (p == str)
+ return (-1);
+ *addr = strndup(str, p - str);
+ *port = strdup(p + 1);
+ }
+ }
+ return (0);
+}
+
+int
+TCP_open(const char *addr, const char *port, int http)
+{
struct addrinfo hints, *res;
- int ret, sd;
+ int ret, sd, val;

memset(&hints, 0, sizeof hints);
- hints.ai_family = family;
hints.ai_socktype = SOCK_STREAM;
hints.ai_flags = AI_PASSIVE;
- ret = getaddrinfo(NULL, port, &hints, &res);
- if (ret != 0)
+ 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);
- if (sd < 0)
+ if (sd < 0) {
+ perror("socket()");
freeaddrinfo(res);
- else
- *resp = res;
- return (sd);
-}
-
-int
-open_tcp(const char *port, int http)
-{
- int sd, val;
- struct addrinfo *res;
-
- sd = try_sock(AF_INET6, port, &res);
- if (sd < 0)
- sd = try_sock(AF_INET, port, &res);
- if (sd < 0) {
- fprintf(stderr, "Failed to get listening socket\n");
return (-1);
}
val = 1;
@@ -115,14 +132,6 @@
close(sd);
return (-1);
}
- val = 0;
- if (res->ai_family == AF_INET6 &&
- setsockopt(sd, IPPROTO_IPV6, IPV6_V6ONLY, &val, sizeof val) != 0) {
- perror("setsockopt(IPV6_V6ONLY, 0)");
- freeaddrinfo(res);
- close(sd);
- return (-1);
- }
if (bind(sd, res->ai_addr, res->ai_addrlen) != 0) {
perror("bind()");
freeaddrinfo(res);

Modified: trunk/varnish-cache/bin/varnishd/varnishd.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/varnishd.c 2006-08-21 13:15:40 UTC (rev 871)
+++ trunk/varnish-cache/bin/varnishd/varnishd.c 2006-08-21 17:26:11 UTC (rev 872)
@@ -47,25 +47,25 @@
}

static void
-setup_hash(const char *sflag)
+setup_hash(const char *s_arg)
{
const char *p, *q;
struct hash_slinger *hp;

- p = strchr(sflag, ',');
+ p = strchr(s_arg, ',');
if (p == NULL)
- q = p = strchr(sflag, '\0');
+ q = p = strchr(s_arg, '\0');
else
q = p + 1;
assert(p != NULL);
assert(q != NULL);
- if (!cmp_hash(&hcl_slinger, sflag, p)) {
+ if (!cmp_hash(&hcl_slinger, s_arg, p)) {
hp = &hcl_slinger;
- } else if (!cmp_hash(&hsl_slinger, sflag, p)) {
+ } else if (!cmp_hash(&hsl_slinger, s_arg, p)) {
hp = &hsl_slinger;
} else {
fprintf(stderr, "Unknown hash method \"%.*s\"\n",
- (int)(p - sflag), sflag);
+ (int)(p - s_arg), s_arg);
exit (2);
}
heritage.hash = hp;
@@ -92,25 +92,25 @@
}

static void
-setup_storage(const char *sflag)
+setup_storage(const char *s_arg)
{
const char *p, *q;
struct stevedore *stp;

- p = strchr(sflag, ',');
+ p = strchr(s_arg, ',');
if (p == NULL)
- q = p = strchr(sflag, '\0');
+ q = p = strchr(s_arg, '\0');
else
q = p + 1;
assert(p != NULL);
assert(q != NULL);
- if (!cmp_storage(&sma_stevedore, sflag, p)) {
+ if (!cmp_storage(&sma_stevedore, s_arg, p)) {
stp = &sma_stevedore;
- } else if (!cmp_storage(&smf_stevedore, sflag, p)) {
+ } else if (!cmp_storage(&smf_stevedore, s_arg, p)) {
stp = &smf_stevedore;
} else {
fprintf(stderr, "Unknown storage method \"%.*s\"\n",
- (int)(p - sflag), sflag);
+ (int)(p - s_arg), s_arg);
exit (2);
}
heritage.stevedore = malloc(sizeof *heritage.stevedore);
@@ -125,12 +125,14 @@
usage(void)
{
fprintf(stderr, "usage: varnishd [options]\n");
- fprintf(stderr, " %-28s # %s\n", "-b backend",
- "backend location");
+ fprintf(stderr, " %-28s # %s\n", "-a address:port",
+ "HTTP listen address and port");
+ fprintf(stderr, " %-28s # %s\n", "-b address:port",
+ "backend address and port");
fprintf(stderr, " %-28s # %s\n", "",
" -b <hostname_or_IP>");
fprintf(stderr, " %-28s # %s\n", "",
- " -b '<hostname_or_IP> <port_or_service>'");
+ " -b '<hostname_or_IP>:<port_or_service>'");
fprintf(stderr, " %-28s # %s\n", "-d", "debug");
fprintf(stderr, " %-28s # %s\n", "-f file", "VCL_file");
fprintf(stderr, " %-28s # %s\n",
@@ -143,7 +145,6 @@
" -h classic,<buckets>");
fprintf(stderr, " %-28s # %s\n", "",
" -h classic,<buckets>,<buckets_per_mutex>");
- fprintf(stderr, " %-28s # %s\n", "-p number", "TCP listen port");
fprintf(stderr, " %-28s # %s\n",
"-s kind[,storageoptions]", "Backend storage specification");
fprintf(stderr, " %-28s # %s\n", "",
@@ -155,7 +156,8 @@
fprintf(stderr, " %-28s # %s\n", "",
" -s file,<dir_or_file>,<size>");
fprintf(stderr, " %-28s # %s\n", "-t", "Default TTL");
- fprintf(stderr, " %-28s # %s\n", "-T port", "Telnet port");
+ fprintf(stderr, " %-28s # %s\n", "-T address:port",
+ "Telnet listen address and port");
fprintf(stderr, " %-28s # %s\n", "-V", "version");
fprintf(stderr, " %-28s # %s\n", "-w int[,int[,int]]",
"Number of worker threads");
@@ -317,13 +319,14 @@
main(int argc, char *argv[])
{
int o;
- const char *portnumber = "8080";
- unsigned dflag = 0;
- const char *bflag = NULL;
- const char *fflag = NULL;
- const char *sflag = "file";
- const char *hflag = "classic";
- const char *Tflag = NULL;
+ unsigned d_flag = 0;
+ char *addr, *port;
+ const char *a_arg = "0.0.0.0:http";
+ const char *b_arg = NULL;
+ const char *f_arg = NULL;
+ const char *h_flag = "classic";
+ const char *s_arg = "file";
+ const char *T_arg = NULL;
struct params param;

setbuf(stdout, NULL);
@@ -343,31 +346,31 @@
params->send_timeout = 600;
params->auto_restart = 1;

- while ((o = getopt(argc, argv, "b:df:h:p:s:t:T:Vw:")) != -1)
+ while ((o = getopt(argc, argv, "a:b:df:h:s:t:T:Vw:")) != -1)
switch (o) {
+ case 'a':
+ a_arg = optarg;
+ break;
case 'b':
- bflag = optarg;
+ b_arg = optarg;
break;
case 'd':
- dflag++;
+ d_flag++;
break;
case 'f':
- fflag = optarg;
+ f_arg = optarg;
break;
case 'h':
- hflag = optarg;
+ h_flag = optarg;
break;
- case 'p':
- portnumber = optarg;
- break;
case 's':
- sflag = optarg;
+ s_arg = optarg;
break;
case 't':
params->default_ttl = strtoul(optarg, NULL, 0);
break;
case 'T':
- Tflag = optarg;
+ T_arg = optarg;
break;
case 'V':
varnish_version("varnishd");
@@ -387,20 +390,20 @@
usage();
}

- if (bflag != NULL && fflag != NULL) {
+ if (b_arg != NULL && f_arg != NULL) {
fprintf(stderr, "Only one of -b or -f can be specified\n");
usage();
}
- if (bflag == NULL && fflag == NULL) {
+ if (b_arg == NULL && f_arg == NULL) {
fprintf(stderr, "One of -b or -f must be specified\n");
usage();
}

- if (mgt_vcc_default(bflag, fflag))
+ if (mgt_vcc_default(b_arg, f_arg))
exit (2);

- setup_storage(sflag);
- setup_hash(hflag);
+ setup_storage(s_arg);
+ setup_hash(h_flag);

/*
* XXX: Lacking the suspend/resume facility (due to the socket API
@@ -409,22 +412,26 @@
* but do not answer. That, on the other hand, would eliminate the
* possibility of doing a "no-glitch" restart of the child process.
*/
- heritage.socket = open_tcp(portnumber, 1);
+ if (TCP_parse(a_arg, &addr, &port) != 0)
+ fprintf(stderr, "invalid listen address\n");
+ heritage.socket = TCP_open(addr, port ? port : "http", 1);
+ free(addr);
+ free(port);
if (heritage.socket < 0)
exit (2);

VSL_MgtInit(SHMLOG_FILENAME, 8*1024*1024);

- if (dflag == 1)
+ if (d_flag == 1)
DebugStunt();
- if (dflag < 2)
- daemon(dflag, dflag);
- if (dflag == 1)
+ if (d_flag < 2)
+ daemon(d_flag, d_flag);
+ if (d_flag == 1)
printf("%d\n", getpid());

mgt_cli_init();

- mgt_run(dflag, Tflag);
+ mgt_run(d_flag, T_arg);

exit(0);
}
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
In message <20060821172611.515871EC967 at projects.linpro.no>, des at projects.linpro
.no writes:

>+ return (-1);
>+ *addr = strndup(str + 1, p - (str + 1));

strndup() ??

--
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.
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
In message <20060821172611.515871EC967 at projects.linpro.no>, des at projects.linpro
.no writes:


>Introduce TCP_parse(), which extracts an address and port from a string of
>the form "hostname:port", "i.p.v.4:port" or "[i:p:v:6]:port" (where "port"
>can be either a decimal number or a service name).

So how do I say "ANY_IPV4:80" ?

I think the removal of -p is a step backwards in user-friendlyness.

-a is probably a good idea, but -p is intuitive.

--
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.
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
"Poul-Henning Kamp" <phk at phk.freebsd.dk> writes:
> So how do I say "ANY_IPV4:80" ?

0:80

DES
--
Dag-Erling Sm?rgrav
Senior Software Developer
Linpro AS - www.linpro.no
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
In message <ujru045p86s.fsf at cat.linpro.no>, Dag-Erling =?iso-8859-1?Q?Sm=F8rgra
v?= writes:
>"Poul-Henning Kamp" <phk at phk.freebsd.dk> writes:
>> So how do I say "ANY_IPV4:80" ?
>
>0:80

As far as I know that is not portable. The value of IF_ADDR_ANY is
not a universal zero constant but implementation dependent.

But if you add -p <number> back I can live with it.

--
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.
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
"Poul-Henning Kamp" <phk at phk.freebsd.dk> writes:
> As far as I know that is not portable. The value of IF_ADDR_ANY is
> not a universal zero constant but implementation dependent.

Show me a platform where it isn't...

If you prefer, I can change TCP_parse() to accept :80 instead.

DES
--
Dag-Erling Sm?rgrav
Senior Software Developer
Linpro AS - www.linpro.no
r872 - trunk/varnish-cache/bin/varnishd [ In reply to ]
In message <ujrfyfpp5gv.fsf at cat.linpro.no>, Dag-Erling =?iso-8859-1?Q?Sm=F8rgra
v?= writes:
>"Poul-Henning Kamp" <phk at phk.freebsd.dk> writes:
>> As far as I know that is not portable. The value of IF_ADDR_ANY is
>> not a universal zero constant but implementation dependent.
>
>Show me a platform where it isn't...
>
>If you prefer, I can change TCP_parse() to accept :80 instead.

That would make much more sense to me.

But I still would like the intuitive -p <port> argument as an
alternative.


--
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.