[experimental-ims] a9852b4 Rework the code that receives the client request.
Geoff Simmons
geoff at varnish-cache.org
Tue Jan 10 00:03:30 CET 2012
commit a9852b4c420afea2bf7191d1b5cb47dc64b92399
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Thu Dec 22 23:51:16 2011 +0000
Rework the code that receives the client request.
Collect all the code in cnt_wait()
Add a new -3 return code from HTC which says that all we've seen so
far is white space. Use the RFC2616 "LWS" definition of white-space.
Rename "sess_timeout" parameter to "timeout_idle" and describe it more
precisely. Make it a double, so more precise values can be specified.
The sessions is considered idle even if we receive white-space.
Rename "session_linger" parameter to "timeout_linger", make it a double
and give it units of seconds like other timeouts.
Add new parameter "timeout_req" which controls how long time we give
the client to send the request headers, from the first non-white char.
Set it to two seconds. Experimentation/Experience requested.
"timeout_req" also gives us positive control of any kind of
"slowlaris" attack scenarios.
I'm trying to group parameters more sensibly, since we sort them
by name, calling them all timeout_*, pool_* etc, makes some sense I hope.
diff --git a/bin/varnishd/cache/cache_acceptor.c b/bin/varnishd/cache/cache_acceptor.c
index 1c6d6fb..1f9616a 100644
--- a/bin/varnishd/cache/cache_acceptor.c
+++ b/bin/varnishd/cache/cache_acceptor.c
@@ -276,6 +276,7 @@ VCA_SetupSess(struct worker *w)
sp->vsl_id = wa->acceptsock | VSL_CLIENTMARKER ;
wa->acceptsock = -1;
sp->t_open = VTIM_real();
+ sp->t_req = sp->t_open;
sp->t_idle = sp->t_open;
sp->mylsock = wa->acceptlsock;
CHECK_OBJ_NOTNULL(sp->mylsock, LISTEN_SOCK_MAGIC);
@@ -293,7 +294,7 @@ static void *
vca_acct(void *arg)
{
#ifdef SO_RCVTIMEO_WORKS
- double sess_timeout = 0;
+ double timeout_idle = 0;
#endif
#ifdef SO_SNDTIMEO_WORKS
double send_timeout = 0;
@@ -333,10 +334,10 @@ vca_acct(void *arg)
}
#endif
#ifdef SO_RCVTIMEO_WORKS
- if (cache_param->sess_timeout != sess_timeout) {
+ if (cache_param->timeout_idle != timeout_idle) {
need_test = 1;
- sess_timeout = cache_param->sess_timeout;
- tv_rcvtimeo = VTIM_timeval(sess_timeout);
+ timeout_idle = cache_param->timeout_idle;
+ tv_rcvtimeo = VTIM_timeval(timeout_idle);
VTAILQ_FOREACH(ls, &heritage.socks, list) {
if (ls->sock < 0)
continue;
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index f926932..b14ec24 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -98,9 +98,10 @@ DOT }
static int
cnt_wait(struct sess *sp)
{
- int i;
+ int i, j, tmo;
struct pollfd pfd[1];
struct worker *wrk;
+ double now, when;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
wrk = sp->wrk;
@@ -110,33 +111,59 @@ cnt_wait(struct sess *sp)
AZ(sp->esi_level);
assert(sp->xid == 0);
- i = HTC_Complete(sp->htc);
- if (i == 0 && cache_param->session_linger > 0) {
+ assert(!isnan(sp->t_req));
+ tmo = (int)(1e3 * cache_param->timeout_linger);
+ while (1) {
pfd[0].fd = sp->fd;
pfd[0].events = POLLIN;
pfd[0].revents = 0;
- i = poll(pfd, 1, cache_param->session_linger);
- if (i)
+ j = poll(pfd, 1, tmo);
+ assert(j >= 0);
+ if (j != 0)
i = HTC_Rx(sp->htc);
+ else
+ i = HTC_Complete(sp->htc);
+ if (i == -1) {
+ SES_Close(sp, "EOF");
+ break;
+ }
+ if (i == -2) {
+ SES_Close(sp, "overflow");
+ break;
+ }
+ now = VTIM_real();
+ if (i == 1) {
+ /* Got it, run with it */
+ sp->t_req = now;
+ sp->step = STP_START;
+ return (0);
+ }
+ if (i == -3) {
+ /* Nothing but whitespace */
+ when = sp->t_idle + cache_param->timeout_idle;
+ if (when < now) {
+ SES_Close(sp, "timeout");
+ break;
+ }
+ when = sp->t_idle + cache_param->timeout_linger;
+ tmo = (int)(1e3 * (when - now));
+ if (when < now || tmo == 0) {
+ sp->t_req = NAN;
+ wrk->stats.sess_herd++;
+ SES_Charge(sp);
+ WAIT_Enter(sp);
+ return (1);
+ }
+ } else {
+ /* Working on it */
+ when = sp->t_req + cache_param->timeout_req;
+ tmo = (int)(1e3 * (when - now));
+ if (when < now || tmo == 0) {
+ SES_Close(sp, "req timeout");
+ break;
+ }
+ }
}
- if (i == 0) {
- WSP(sp, SLT_Debug, "herding");
- wrk->stats.sess_herd++;
- SES_Charge(sp);
- WAIT_Enter(sp);
- return (1);
- }
- if (i == 1) {
- sp->step = STP_START;
- return (0);
- }
- if (i == -2) {
- SES_Close(sp, "overflow");
- } else if (i == -1 && Tlen(sp->htc->rxbuf) == 0 &&
- (errno == 0 || errno == ECONNRESET))
- SES_Close(sp, "EOF");
- else
- SES_Close(sp, "error");
sp->step = STP_DONE;
return (0);
}
@@ -366,8 +393,6 @@ cnt_done(struct sess *sp)
sp->t_idle = W_TIM_real(wrk);
-WSP(sp, SLT_Debug, "PHK req %.9f resp %.9f end %.9f open %.9f",
- sp->t_req, sp->t_resp, sp->t_idle, sp->t_open);
if (sp->xid == 0) {
sp->t_resp = sp->t_idle;
} else {
@@ -418,17 +443,20 @@ WSP(sp, SLT_Debug, "PHK req %.9f resp %.9f end %.9f open %.9f",
i = HTC_Reinit(sp->htc);
if (i == 1) {
wrk->stats.sess_pipeline++;
+ sp->t_req = sp->t_idle;
sp->step = STP_START;
return (0);
}
if (Tlen(sp->htc->rxbuf)) {
wrk->stats.sess_readahead++;
sp->step = STP_WAIT;
+ sp->t_req = sp->t_idle;
return (0);
}
- if (cache_param->session_linger > 0) {
+ if (cache_param->timeout_linger > 0.) {
wrk->stats.sess_linger++;
sp->step = STP_WAIT;
+ sp->t_req = sp->t_idle; // XXX: not quite correct
return (0);
}
wrk->stats.sess_herd++;
@@ -1574,7 +1602,7 @@ cnt_start(struct sess *sp)
/* Update stats of various sorts */
wrk->stats.client_req++;
- sp->t_req = W_TIM_real(wrk);
+ assert(!isnan(sp->t_req));
wrk->acct_tmp.req++;
/* Assign XID and log */
diff --git a/bin/varnishd/cache/cache_httpconn.c b/bin/varnishd/cache/cache_httpconn.c
index 9e0a052..1abd2a4 100644
--- a/bin/varnishd/cache/cache_httpconn.c
+++ b/bin/varnishd/cache/cache_httpconn.c
@@ -50,6 +50,7 @@
* Check if we have a complete HTTP request or response yet
*
* Return values:
+ * -3 All whitespace so far
* 0 No, keep trying
* >0 Yes, it is this many bytes long.
*/
@@ -62,13 +63,13 @@ htc_header_complete(txt *t)
Tcheck(*t);
assert(*t->e == '\0');
/* Skip any leading white space */
- for (p = t->b ; vct_issp(*p); p++)
+ for (p = t->b ; vct_islws(*p); p++)
continue;
if (p == t->e) {
/* All white space */
t->e = t->b;
*t->e = '\0';
- return (0);
+ return (-3);
}
while (1) {
p = strchr(p, '\n');
@@ -133,6 +134,8 @@ HTC_Reinit(struct http_conn *htc)
}
/*--------------------------------------------------------------------
+ * Return -3 if it's all whitespace so far
+ * Return 0 if we need more text
* Return 1 if we have a complete HTTP procol header
*/
@@ -143,9 +146,8 @@ HTC_Complete(struct http_conn *htc)
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
i = htc_header_complete(&htc->rxbuf);
- assert(i >= 0);
- if (i == 0)
- return (0);
+ if (i <= 0)
+ return (i);
WS_ReleaseP(htc->ws, htc->rxbuf.e);
AZ(htc->pipeline.b);
AZ(htc->pipeline.e);
@@ -160,8 +162,9 @@ HTC_Complete(struct http_conn *htc)
/*--------------------------------------------------------------------
* Receive more HTTP protocol bytes
* Returns:
+ * -3 all whitespace so far
* -2 overflow
- * -1 error
+ * -1 error/EOF
* 0 more needed
* 1 got complete HTTP header
*/
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index a024452..0a476d4 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -85,8 +85,9 @@ struct params {
unsigned shm_reclen;
- /* Acceptor hints */
- unsigned sess_timeout;
+ double timeout_linger;
+ double timeout_idle;
+ double timeout_req;
unsigned pipe_timeout;
unsigned send_timeout;
unsigned idle_send_timeout;
@@ -140,9 +141,6 @@ struct params {
double first_byte_timeout;
double between_bytes_timeout;
- /* How long to linger on sessions */
- unsigned session_linger;
-
/* CLI buffer size */
unsigned cli_buffer;
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index c236323..2f7bd53 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -773,12 +773,19 @@ static const struct parspec input_parspec[] = {
"cache at the end of ttl+grace+keep.",
DELAYED_EFFECT,
"0", "seconds" },
- { "sess_timeout", tweak_timeout, &mgt_param.sess_timeout, 0, 0,
- "Idle timeout for persistent sessions. "
- "If a HTTP request has not been received in this many "
- "seconds, the session is closed.",
+ { "timeout_idle", tweak_timeout_double, &mgt_param.timeout_idle,
+ 0, UINT_MAX,
+ "Idle timeout for client connections.\n"
+ "A connection is considered idle, until we receive"
+ " a non-white-space character on it.",
0,
"5", "seconds" },
+ { "timeout_req", tweak_timeout_double, &mgt_param.timeout_req,
+ 0, UINT_MAX,
+ "Max time to receive clients request header, measured"
+ " from first non-white-space character to double CRNL.",
+ 0,
+ "2", "seconds" },
{ "expiry_sleep", tweak_timeout_double, &mgt_param.expiry_sleep, 0, 60,
"How long the expiry thread sleeps when there is nothing "
"for it to do.\n",
@@ -993,18 +1000,18 @@ static const struct parspec input_parspec[] = {
"it.\n",
0,
"100000", "sessions" },
- { "session_linger", tweak_uint,
- &mgt_param.session_linger,0, UINT_MAX,
- "How long time the workerthread lingers on the session "
- "to see if a new request appears right away.\n"
- "If sessions are reused, as much as half of all reuses "
+ { "timeout_linger", tweak_timeout_double, &mgt_param.timeout_linger,
+ 0, UINT_MAX,
+ "How long time the workerthread lingers on an idle session "
+ "before handing it over to the waiter.\n"
+ "When sessions are reused, as much as half of all reuses "
"happen within the first 100 msec of the previous request "
"completing.\n"
"Setting this too high results in worker threads not doing "
"anything for their keep, setting it too low just means that "
"more sessions take a detour around the waiter.",
EXPERIMENTAL,
- "50", "ms" },
+ "0.050", "seconds" },
{ "log_hashstring", tweak_bool, &mgt_param.log_hash, 0, 0,
"Log the hash string components to shared memory log.\n",
0,
diff --git a/bin/varnishd/waiter/cache_waiter_epoll.c b/bin/varnishd/waiter/cache_waiter_epoll.c
index e8f5e2a..6ad5be5 100644
--- a/bin/varnishd/waiter/cache_waiter_epoll.c
+++ b/bin/varnishd/waiter/cache_waiter_epoll.c
@@ -183,7 +183,7 @@ vwe_thread(void *priv)
continue;
/* check for timeouts */
- deadline = now - cache_param->sess_timeout;
+ deadline = now - cache_param->timeout_idle;
for (;;) {
sp = VTAILQ_FIRST(&vwe->sesshead);
if (sp == NULL)
@@ -201,13 +201,13 @@ vwe_thread(void *priv)
/*--------------------------------------------------------------------*/
static void *
-vwe_sess_timeout_ticker(void *priv)
+vwe_timeout_idle_ticker(void *priv)
{
char ticker = 'R';
struct vwe *vwe;
CAST_OBJ_NOTNULL(vwe, priv, VWE_MAGIC);
- THR_SetName("cache-epoll-sess_timeout_ticker");
+ THR_SetName("cache-epoll-timeout_idle_ticker");
while (1) {
/* ticking */
@@ -255,7 +255,7 @@ vwe_init(void)
assert(i != -1);
AZ(pthread_create(&vwe->timer_thread,
- NULL, vwe_sess_timeout_ticker, vwe));
+ NULL, vwe_timeout_idle_ticker, vwe));
AZ(pthread_create(&vwe->epoll_thread, NULL, vwe_thread, vwe));
return(vwe);
}
diff --git a/bin/varnishd/waiter/cache_waiter_kqueue.c b/bin/varnishd/waiter/cache_waiter_kqueue.c
index 707c902..423bf26 100644
--- a/bin/varnishd/waiter/cache_waiter_kqueue.c
+++ b/bin/varnishd/waiter/cache_waiter_kqueue.c
@@ -195,7 +195,7 @@ vwk_thread(void *priv)
* would not know we meant "the old fd of this number".
*/
vwk_kq_flush(vwk);
- deadline = now - cache_param->sess_timeout;
+ deadline = now - cache_param->timeout_idle;
for (;;) {
sp = VTAILQ_FIRST(&vwk->sesshead);
if (sp == NULL)
diff --git a/bin/varnishd/waiter/cache_waiter_poll.c b/bin/varnishd/waiter/cache_waiter_poll.c
index 3a39c6b..a31bec1 100644
--- a/bin/varnishd/waiter/cache_waiter_poll.c
+++ b/bin/varnishd/waiter/cache_waiter_poll.c
@@ -141,7 +141,7 @@ vwp_main(void *priv)
v = poll(vwp->pollfd, vwp->hpoll + 1, 100);
assert(v >= 0);
now = VTIM_real();
- deadline = now - cache_param->sess_timeout;
+ deadline = now - cache_param->timeout_idle;
v2 = v;
VTAILQ_FOREACH_SAFE(sp, &vwp->sesshead, list, sp2) {
if (v != 0 && v2 == 0)
diff --git a/bin/varnishd/waiter/cache_waiter_ports.c b/bin/varnishd/waiter/cache_waiter_ports.c
index 0d05775..4f39ca4 100644
--- a/bin/varnishd/waiter/cache_waiter_ports.c
+++ b/bin/varnishd/waiter/cache_waiter_ports.c
@@ -191,7 +191,7 @@ vws_thread(void *priv)
vws_port_ev(vws, ev + ei, now);
/* check for timeouts */
- deadline = now - cache_param->sess_timeout;
+ deadline = now - cache_param->timeout_idle;
/*
* This loop assumes that the oldest sessions are always at the
@@ -220,7 +220,7 @@ vws_thread(void *priv)
if (sp) {
double tmo =
- (sp->t_open + cache_param->sess_timeout) - now;
+ (sp->t_open + cache_param->timeout_idle) - now;
/*
* we should have removed all sps whose timeout
diff --git a/bin/varnishtest/tests/r00262.vtc b/bin/varnishtest/tests/r00262.vtc
index 1ef7255..b3c1cf9 100644
--- a/bin/varnishtest/tests/r00262.vtc
+++ b/bin/varnishtest/tests/r00262.vtc
@@ -7,7 +7,7 @@ server s1 {
-body "012345\n"
} -start
-varnish v1 -arg "-p session_linger=20" -vcl+backend { } -start
+varnish v1 -arg "-p timeout_linger=20" -vcl+backend { } -start
client c1 {
send "GET / HTTP/1.1\r\n\r\n\r\n"
diff --git a/bin/varnishtest/tests/r00524.vtc b/bin/varnishtest/tests/r00524.vtc
index 157eebe..cd43e97 100644
--- a/bin/varnishtest/tests/r00524.vtc
+++ b/bin/varnishtest/tests/r00524.vtc
@@ -21,7 +21,7 @@ varnish v1 -vcl+backend {
sub vcl_fetch {
set beresp.do_esi = true;
}
-} -cliok "param.set sess_timeout 60" -start
+} -cliok "param.set timeout_idle 60" -start
client c1 {
txreq -proto HTTP/1.0 -hdr "Connection: kEep-alive"
diff --git a/bin/varnishtest/tests/r00641.vtc b/bin/varnishtest/tests/r00641.vtc
index ef825d7..da54121 100644
--- a/bin/varnishtest/tests/r00641.vtc
+++ b/bin/varnishtest/tests/r00641.vtc
@@ -18,7 +18,7 @@ varnish v1 -vcl+backend {
sub vcl_fetch {
set beresp.do_esi = true;
}
-} -cliok "param.set sess_timeout 60" -start
+} -cliok "param.set timeout_idle 60" -start
client c1 {
txreq -proto HTTP/1.1
More information about the varnish-commit
mailing list