Turn the guts of http1_wait() into a more general and protocol

independent SES_RxReq() function.

Eliminate timeout_req and rely on timeout_idle to do the job.
(Length of this timeout still under debate on -dev)

bin/varnishd/cache/cache.h

@@ -677,6 +677,10 @@ struct sess {
 
 /* Prototypes etc ----------------------------------------------------*/
 
+/* Cross file typedefs */
+
+typedef enum htc_status_e htc_complete_f(struct http_conn *);
+
 /* cache_acceptor.c */
 void VCA_Init(void);
 void VCA_Shutdown(void);
@@ -853,7 +857,7 @@ enum sess_close http_DoConnection(struct http *hp);
 
 /* cache_http1_proto.c */
 
-enum htc_status_e HTTP1_Complete(struct http_conn *htc);
+htc_complete_f HTTP1_Complete;
 uint16_t HTTP1_DissectRequest(struct http_conn *htc, struct http *hp);
 uint16_t HTTP1_DissectResponse(struct http *sp, struct http_conn *htc);
 unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*);
@@ -985,18 +989,22 @@ task_func_t SES_Proto_Sess;
 task_func_t SES_Proto_Req;
 
 enum htc_status_e {
-	HTC_S_EMPTY =		-4,
+	HTC_S_CLOSE =		-4,
 	HTC_S_TIMEOUT =		-3,
 	HTC_S_OVERFLOW =	-2,
 	HTC_S_EOF =		-1,
-	HTC_S_OK =		0,
-	HTC_S_COMPLETE =	 1
+	HTC_S_EMPTY =		 0,
+	HTC_S_MORE =		 1,
+	HTC_S_COMPLETE =	 2,
+	HTC_S_IDLE =		 3,
 };
 
 void SES_RxInit(struct http_conn *htc, struct ws *ws,
     unsigned maxbytes, unsigned maxhdr);
 void SES_RxReInit(struct http_conn *htc);
 enum htc_status_e SES_Rx(struct http_conn *htc, double tmo);
+enum htc_status_e SES_RxReq(const struct worker *, struct req *,
+    htc_complete_f *func);
 
 #define SESS_ATTR(UP, low, typ, len)				\
 	int SES_Get_##low(const struct sess *sp, typ *dst);	\

bin/varnishd/cache/cache_session.c

@@ -234,7 +234,65 @@ SES_Rx(struct http_conn *htc, double tmo)
 		return (HTC_S_EOF);
 	htc->rxbuf_e += i;
 	*htc->rxbuf_e = '\0';
-	return (HTC_S_OK);
+	return (HTC_S_MORE);
+}
+
+/*----------------------------------------------------------------------
+ * Receive a request/packet/whatever, with timeouts
+ */
+
+enum htc_status_e
+SES_RxReq(const struct worker *wrk, struct req *req, htc_complete_f *func)
+{
+	double tmo;
+	double now, when;
+	struct sess *sp;
+	enum htc_status_e hs;
+
+	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	sp = req->sp;
+	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
+
+	AZ(isnan(sp->t_idle));
+	assert(isnan(req->t_first));
+
+	when = sp->t_idle + cache_param->timeout_idle;
+	tmo = cache_param->timeout_linger;
+	while (1) {
+		hs = SES_Rx(req->htc, tmo);
+		now = VTIM_real();
+		if (hs == HTC_S_EOF) {
+			WS_ReleaseP(req->htc->ws, req->htc->rxbuf_b);
+			return (HTC_S_CLOSE);
+		}
+		if (hs == HTC_S_OVERFLOW) {
+			WS_ReleaseP(req->htc->ws, req->htc->rxbuf_b);
+			return (HTC_S_OVERFLOW);
+		}
+		hs = func(req->htc);
+		if (hs == HTC_S_COMPLETE) {
+			/* Got it, run with it */
+			if (isnan(req->t_first))
+				req->t_first = now;
+			req->t_req = now;
+			return (HTC_S_COMPLETE);
+		}
+		if (when < now)
+			return (HTC_S_TIMEOUT);
+		if (hs == HTC_S_MORE) {
+			/* Working on it */
+			if (isnan(req->t_first))
+				req->t_first = now;
+			tmo = when - now;
+			continue;
+		}
+		assert(hs == HTC_S_EMPTY);
+		/* Nothing but whitespace */
+		tmo = sp->t_idle + cache_param->timeout_linger - now;
+		if (tmo < 0)
+			return (HTC_S_IDLE);
+	}
 }
 
 /*--------------------------------------------------------------------
@@ -422,13 +480,16 @@ SES_Wait(struct sess *sp)
 	 * XXX: waiter_epoll prevents us from zeroing the struct because
 	 * XXX: it keeps state across calls.
 	 */
+	if (VTCP_nonblocking(sp->fd)) {
+		SES_Delete(sp, SC_REM_CLOSE, NAN);
+		return;
+	}
 	sp->waited.magic = WAITED_MAGIC;
 	sp->waited.fd = sp->fd;
 	sp->waited.ptr = sp;
 	sp->waited.idle = sp->t_idle;
-	if (Wait_Enter(pp->http1_waiter, &sp->waited)) {
+	if (Wait_Enter(pp->http1_waiter, &sp->waited))
 		SES_Delete(sp, SC_PIPE_OVERFLOW, NAN);
-	}
 }
 
 /*--------------------------------------------------------------------

bin/varnishd/common/params.h

@@ -111,7 +111,6 @@ struct params {
 
 	double			timeout_linger;
 	double			timeout_idle;
-	double			timeout_req;
 	double			pipe_timeout;
 	double			send_timeout;
 	double			idle_send_timeout;

bin/varnishd/http1/cache_http1_fetch.c

@@ -154,7 +154,7 @@ V1F_fetch_hdr(struct worker *wrk, struct busyobj *bo, const char *def_host)
 	first = 1;
 	do {
 		hs = SES_Rx(htc, 0);
-		if (hs == HTC_S_OK)
+		if (hs == HTC_S_MORE)
 			hs = HTTP1_Complete(htc);
 		if (hs == HTC_S_OVERFLOW) {
 			WS_ReleaseP(htc->ws, htc->rxbuf_b);

bin/varnishd/http1/cache_http1_fsm.c

@@ -51,89 +51,39 @@
 static enum req_fsm_nxt
 http1_wait(struct sess *sp, struct worker *wrk, struct req *req)
 {
-	double tmo;
-	double now, when;
-	enum sess_close why = SC_NULL;
 	enum htc_status_e hs;
 
-	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
-	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-
-	assert(req->sp == sp);
-
-	AZ(req->vcl);
-	AZ(req->esi_level);
-	AZ(isnan(sp->t_idle));
-	assert(isnan(req->t_first));
 	assert(isnan(req->t_prev));
 	assert(isnan(req->t_req));
+	AZ(req->vcl);
+	AZ(req->esi_level);
 
-	tmo = cache_param->timeout_linger;
-	while (1) {
-		hs = SES_Rx(req->htc, tmo);
-		now = VTIM_real();
-		if (hs == HTC_S_EOF) {
-			WS_ReleaseP(req->htc->ws, req->htc->rxbuf_b);
-			why = SC_REM_CLOSE;
-			break;
-		}
-		if (hs == HTC_S_OVERFLOW) {
-			WS_ReleaseP(req->htc->ws, req->htc->rxbuf_b);
-			why = SC_RX_OVERFLOW;
-			break;
-		}
-		hs = HTTP1_Complete(req->htc);
-		if (hs == HTC_S_OK) {
-			/* Working on it */
-			if (isnan(req->t_first))
-				/* Record first byte received time stamp */
-				req->t_first = now;
-			when = req->t_first + cache_param->timeout_req;
-			tmo = when - now;
-			if (tmo <= 0) {
-				why = SC_RX_TIMEOUT;
-				break;
-			}
-			continue;
-		}
-		if (hs == HTC_S_COMPLETE) {
-			/* Got it, run with it */
-			if (isnan(req->t_first))
-				req->t_first = now;
-			if (isnan(req->t_req))
-				req->t_req = now;
-			req->acct.req_hdrbytes +=
-			    req->htc->rxbuf_e - req->htc->rxbuf_b;
-			return (REQ_FSM_MORE);
-		}
-		assert(hs == HTC_S_EMPTY);
-		/* Nothing but whitespace */
-		when = sp->t_idle + cache_param->timeout_idle;
-		if (when < now) {
-			why = SC_RX_TIMEOUT;
-			break;
+	hs = SES_RxReq(wrk, req, HTTP1_Complete);
+	if (hs < HTC_S_EMPTY) {
+		req->acct.req_hdrbytes += req->htc->rxbuf_e - req->htc->rxbuf_b;
+		CNT_AcctLogCharge(wrk->stats, req);
+		SES_ReleaseReq(req);
+		switch(hs) {
+		case HTC_S_CLOSE: SES_Delete(sp, SC_REM_CLOSE, 0.0); break;
+		case HTC_S_TIMEOUT: SES_Delete(sp, SC_RX_TIMEOUT, 0.0); break;
+		case HTC_S_OVERFLOW: SES_Delete(sp, SC_RX_OVERFLOW, 0.0); break;
+		case HTC_S_EOF: SES_Delete(sp, SC_REM_CLOSE, 0.0); break;
+		default: WRONG("htc_status (bad)");
 		}
-		when = sp->t_idle + cache_param->timeout_linger;
-		tmo = when - now;
-		if (tmo > 0)
-			continue;
-
+		return (REQ_FSM_DONE);
+	}
+	if (hs == HTC_S_COMPLETE) {
+		req->acct.req_hdrbytes +=
+		    req->htc->rxbuf_e - req->htc->rxbuf_b;
+		return (REQ_FSM_MORE);
+	}
+	if (hs == HTC_S_IDLE) {
 		wrk->stats->sess_herd++;
 		SES_ReleaseReq(req);
-		if (VTCP_nonblocking(sp->fd)) {
-			why = SC_REM_CLOSE;
-			break;
-		}
 		SES_Wait(sp);
 		return (REQ_FSM_DONE);
 	}
-	req->acct.req_hdrbytes += req->htc->rxbuf_e - req->htc->rxbuf_b;
-	CNT_AcctLogCharge(wrk->stats, req);
-	SES_ReleaseReq(req);
-	assert(why != SC_NULL);
-	SES_Delete(sp, why, now);
-	return (REQ_FSM_DONE);
+	WRONG("htc_status (nonbad)");
 }
 
 /*----------------------------------------------------------------------

bin/varnishd/http1/cache_http1_proto.c

@@ -61,7 +61,7 @@ const int HTTP1_Resp[3] = {
  * Check if we have a complete HTTP request or response yet
  */
 
-enum htc_status_e
+enum htc_status_e __match_proto__(htc_complete_f)
 HTTP1_Complete(struct http_conn *htc)
 {
 	char *p;
@@ -85,7 +85,7 @@ HTTP1_Complete(struct http_conn *htc)
 	while (1) {
 		p = strchr(p, '\n');
 		if (p == NULL)
-			return (HTC_S_OK);
+			return (HTC_S_MORE);
 		p++;
 		if (*p == '\r')
 			p++;

bin/varnishd/mgt/mgt_param_tbl.c

@@ -172,16 +172,10 @@ struct parspec mgt_parspec[] = {
 	{ "timeout_idle", tweak_timeout, &mgt_param.timeout_idle,
 		"0", NULL,
 		"Idle timeout for client connections.\n"
-		"A connection is considered idle, until we receive"
-		" a non-white-space character on it.",
+		"A connection is considered idle, until we have"
+		"received the full request headers.",
 		0,
 		"5", "seconds" },
-	{ "timeout_req", tweak_timeout, &mgt_param.timeout_req,
-		"0", NULL,
-		"Max time to receive clients request headers, measured"
-		" from first non-white-space character to double CRNL.",
-		0,
-		"2", "seconds" },
 	{ "pipe_timeout", tweak_timeout, &mgt_param.pipe_timeout,
 		"0", NULL,
 		"Idle timeout for PIPE sessions. "