Address issues raised in code reviews.

Also, rework cache entry freshness calculation.
Related to #525, #507, #513, #523, #493, #416.

tempesta_fw/cache.c

@@ -43,7 +43,6 @@
 
 /* Flags stored in a Cache Entry. */
 #define TFW_CE_MUST_REVAL	0x0001		/* MUST revalidate if stale. */
-#define TFW_CE_NO_CACHE		0x0002		/* ALWAYS revalidate. */
 #define TFW_CE_INVALID		0x0100
 
 /*
@@ -52,6 +51,13 @@
  * @status_len	- length of response satus line;
  * @hdr_num	- numbder of headers;
  * @hdr_len	- length of whole headers data;
+ * @method	- request method, part of the key;
+ * @flags	- various cache entry flags;
+ * @age		- the value of response Age: header field;
+ * @date	- the value of response Date: header field;
+ * @req_time	- the time the request was issued;
+ * @resp_time	- the time the response was received;
+ * @lifetime	- the cache entry's current lifetime;
  * @key		- the cache enty key (URI + Host header);
  * @status	- pointer to status line  (with trailing CRLFs);
  * @hdrs	- pointer to list of HTTP headers (with trailing CRLFs);
@@ -68,7 +74,6 @@ typedef struct {
 	unsigned int	flags: 28;
 	time_t		age;
 	time_t		date;
-	time_t		expires;
 	time_t		req_time;
 	time_t		resp_time;
 	time_t		lifetime;
@@ -264,8 +269,6 @@ tfw_cache_employ_req(TfwHttpReq *req)
 	if (req->cache_ctl.flags & TFW_HTTP_CC_NO_CACHE)
 		return false;
 
-	/* Assume there's no Date: header in the request. */
-	req->cache_ctl.timestamp = tfw_current_timestamp();
 	return true;
 }
 
@@ -290,40 +293,45 @@ tfw_cache_status_bydef(TfwHttpResp *resp)
 static bool
 tfw_cache_employ_resp(TfwHttpReq *req, TfwHttpResp *resp)
 {
-	static const unsigned int __read_mostly cacheit =
-		TFW_HTTP_CC_HDR_EXPIRES | TFW_HTTP_CC_MAX_AGE
-		| TFW_HTTP_CC_S_MAXAGE | TFW_HTTP_CC_PUBLIC;
-	static const unsigned int __read_mostly authcan =
-		TFW_HTTP_CC_S_MAXAGE | TFW_HTTP_CC_PUBLIC
-		| TFW_HTTP_CC_MUST_REVAL | TFW_HTTP_CC_PROXY_REVAL;
-
-	if (req->cache_ctl.flags & TFW_HTTP_CC_CFG_CACHE_BYPASS)
-		return false;
-
-	if ((req->cache_ctl.flags|resp->cache_ctl.flags) & TFW_HTTP_CC_NO_STORE)
-		return false;
+#define CC_REQ_DONTCACHE				\
+	(TFW_HTTP_CC_CFG_CACHE_BYPASS | TFW_HTTP_CC_NO_STORE)
+#define CC_RESP_DONTCACHE				\
+	(TFW_HTTP_CC_NO_STORE | TFW_HTTP_CC_PRIVATE	\
+	 | TFW_HTTP_CC_NO_CACHE)
+#define CC_RESP_CACHEIT					\
+	(TFW_HTTP_CC_HDR_EXPIRES | TFW_HTTP_CC_MAX_AGE	\
+	 | TFW_HTTP_CC_S_MAXAGE | TFW_HTTP_CC_PUBLIC)
+#define CC_RESP_AUTHCAN					\
+	(TFW_HTTP_CC_S_MAXAGE | TFW_HTTP_CC_PUBLIC	\
+	 | TFW_HTTP_CC_MUST_REVAL | TFW_HTTP_CC_PROXY_REVAL)
 	/*
-	 * TODO: no-cache -- should be cached.
+	 * TODO: Response no-cache -- should be cached.
 	 * Should turn on unconditional revalidation.
 	 */
-	if (resp->cache_ctl.flags & TFW_HTTP_CC_NO_CACHE)
+	if (req->cache_ctl.flags & CC_REQ_DONTCACHE)
+		return false;
+	if (resp->cache_ctl.flags & CC_RESP_DONTCACHE)
 		return false;
 	if (!(resp->cache_ctl.flags & TFW_HTTP_CC_IS_PRESENT)
 	    && (resp->cache_ctl.flags & TFW_HTTP_CC_PRAGMA_NO_CACHE))
 		return false;
-	if (resp->cache_ctl.flags & TFW_HTTP_CC_PRIVATE)
-		return false;
 	if ((req->cache_ctl.flags & TFW_HTTP_CC_HDR_AUTHORIZATION)
-	    && !(req->cache_ctl.flags & authcan))
+	    && !(req->cache_ctl.flags & CC_RESP_AUTHCAN))
 		return false;
-	if (!(resp->cache_ctl.flags & cacheit) && !tfw_cache_status_bydef(resp))
+	if (!(resp->cache_ctl.flags & CC_RESP_CACHEIT)
+	    && !tfw_cache_status_bydef(resp))
 		return false;
+#undef CC_RESP_AUTHCAN
+#undef CC_RESP_CACHEIT
+#undef CC_RESP_DONTCACHE
+#undef CC_REQ_DONTCACHE
 
-	if (!resp->cache_ctl.timestamp)
-		resp->cache_ctl.timestamp = tfw_current_timestamp();
 	return true;
 }
 
+/*
+ * Calculate freshness lifetime according to RFC 7234 4.2.1.
+ */
 static time_t
 tfw_cache_calc_lifetime(TfwHttpResp *resp)
 {
@@ -334,13 +342,16 @@ tfw_cache_calc_lifetime(TfwHttpResp *resp)
 	else if (resp->cache_ctl.flags & TFW_HTTP_CC_MAX_AGE)
 		lifetime = resp->cache_ctl.max_age;
 	else if (resp->cache_ctl.flags & TFW_HTTP_CC_HDR_EXPIRES)
-		lifetime = resp->cache_ctl.expires;
+		lifetime = resp->cache_ctl.expires - resp->date;
 	else
 		lifetime = 0;	/* TODO: Heuristic lifetime. */
 
 	return lifetime;
 }
 
+/*
+ * Calculate the current entry age according to RFC 7234 4.2.3.
+ */
 static time_t
 tfw_cache_entry_age(TfwCacheEntry *ce)
 {
@@ -350,29 +361,45 @@ tfw_cache_entry_age(TfwCacheEntry *ce)
 	return (initial_age + tfw_current_timestamp() - ce->resp_time);
 }
 
-static bool
-tfw_cache_entry_is_fresh(TfwHttpReq *req, TfwCacheEntry *ce)
+/*
+ * Given Cache Control arguments in the request and the response,
+ * as well as the stored cache entry parameters, determine if the
+ * cache entry is live and may be served to a client. For that,
+ * the cache entry freshness is calculated according to RFC 7234
+ * 4.2, 5.2.1.1, 5.2.1.2, and 5.2.1.3.
+ *
+ * Returns the value of calculated cache entry lifetime if the entry
+ * is live and may be served to a client. Returns zero if the entry
+ * may not be served.
+ *
+ * Note that if the returned value of lifetime is greater than
+ * ce->lifetime, then the entry is stale but still may be served
+ * to a client, provided that the cache policy allows that.
+ */
+static time_t
+tfw_cache_entry_is_live(TfwHttpReq *req, TfwCacheEntry *ce)
 {
 	time_t ce_age = tfw_cache_entry_age(ce);
-	time_t ce_lifetime = ce->lifetime;
+	time_t ce_lifetime, lt_fresh = UINT_MAX;
 
-	if (req->cache_ctl.flags & (TFW_HTTP_CC_MAX_AGE
-				    | TFW_HTTP_CC_MAX_STALE
-				    | TFW_HTTP_CC_MIN_FRESH))
-	{
-		time_t lt_max_age, lt_max_stale, lt_min_fresh;
-
-		lt_max_age = lt_max_stale = lt_min_fresh = UINT_MAX;
+#define CC_LIFETIME_FRESH	(TFW_HTTP_CC_MAX_AGE | TFW_HTTP_CC_MIN_FRESH)
+	if (req->cache_ctl.flags & CC_LIFETIME_FRESH) {
+		time_t lt_max_age = UINT_MAX, lt_min_fresh = UINT_MAX;
 		if (req->cache_ctl.flags & TFW_HTTP_CC_MAX_AGE)
 			lt_max_age = req->cache_ctl.max_age;
-		if (req->cache_ctl.flags & TFW_HTTP_CC_MAX_STALE)
-			lt_max_stale += req->cache_ctl.max_stale;
 		if (req->cache_ctl.flags & TFW_HTTP_CC_MIN_FRESH)
-			lt_min_fresh -= req->cache_ctl.min_fresh;
-		ce_lifetime = min(min(lt_max_age, lt_max_stale), lt_min_fresh);
+			lt_min_fresh = ce->lifetime - req->cache_ctl.min_fresh;
+		lt_fresh = min(lt_max_age, lt_min_fresh);
+	}
+	if (!(req->cache_ctl.flags & TFW_HTTP_CC_MAX_STALE)) {
+		ce_lifetime = min(lt_fresh, ce->lifetime);
+	} else {
+		time_t lt_max_stale = ce->lifetime + req->cache_ctl.max_stale;
+		ce_lifetime = min(lt_fresh, lt_max_stale);
 	}
+#undef CC_LIFETIME_FRESH
 
-	return ce_lifetime > ce_age;
+	return ce_lifetime > ce_age ? ce_lifetime : 0;
 }
 
 static bool
@@ -671,11 +698,10 @@ tfw_cache_copy_resp(TfwCacheEntry *ce, TfwHttpResp *resp, TfwHttpReq *req,
 	BUG_ON(tot_len != 0);
 
 	if (resp->cache_ctl.flags
-	    & (TFW_HTTP_CC_MUST_REVAL | TFW_HTTP_CC_MUST_REVAL))
+	    & (TFW_HTTP_CC_MUST_REVAL | TFW_HTTP_CC_PROXY_REVAL))
 		ce->flags |= TFW_CE_MUST_REVAL;
 	ce->date = resp->date;
 	ce->age = resp->cache_ctl.age;
-	ce->expires = resp->cache_ctl.expires;
 	ce->req_time = req->cache_ctl.timestamp;
 	ce->resp_time = resp->cache_ctl.timestamp;
 	ce->lifetime = tfw_cache_calc_lifetime(resp);
@@ -1064,7 +1090,7 @@ cache_req_process_node(TfwHttpReq *req, unsigned long key,
 		}
 	}
 
-	if (!tfw_cache_entry_is_fresh(req, ce))
+	if (!tfw_cache_entry_is_live(req, ce))
 		goto out;
 
 	TFW_DBG("Cache: service request w/ key=%lx, ce=%p (len=%u key_len=%u"
@@ -1078,7 +1104,7 @@ cache_req_process_node(TfwHttpReq *req, unsigned long key,
 	resp = tfw_cache_build_resp(ce);
 out:
 	if (!resp && (req->cache_ctl.flags & TFW_HTTP_CC_OIFCACHED))
-		tfw_http_send_502((TfwHttpMsg *)req); /* XXX: Change to 504. */
+		tfw_http_send_504((TfwHttpMsg *)req);
 	else
 		action(req, resp);
 

tempesta_fw/http.c

@@ -44,6 +44,7 @@ int ghprio; /* GFSM hook priority. */
 #define S_404			"HTTP/1.1 404 Not Found"
 #define S_500			"HTTP/1.1 500 Internal Server Error"
 #define S_502			"HTTP/1.1 502 Bad Gateway"
+#define S_504			"HTTP/1.1 504 Gateway Timeout"
 
 #define S_F_HOST		"Host: "
 #define S_F_DATE		"Date: "
@@ -316,6 +317,31 @@ tfw_http_send_502(TfwHttpMsg *hmreq)
 	return tfw_http_send_resp(hmreq, &rh, __TFW_STR_CH(&rh, 1));
 }
 
+#define S_504_PART_01	S_504 S_CRLF S_F_DATE
+#define S_504_PART_02	S_CRLF S_F_CONTENT_LENGTH "0" S_CRLF
+/*
+ * HTTP 504 response: did not receive a timely response from
+ * the designated server.
+ */
+int
+tfw_http_send_504(TfwHttpMsg *hmreq)
+{
+	TfwStr rh = {
+		.ptr = (TfwStr []){
+			{ .ptr = S_504_PART_01, .len = SLEN(S_504_PART_01) },
+			{ .ptr = *this_cpu_ptr(&g_buf), .len = SLEN(S_V_DATE) },
+			{ .ptr = S_504_PART_02, .len = SLEN(S_504_PART_02) },
+			{ .ptr = S_CRLF, .len = SLEN(S_CRLF) },
+		},
+		.len = SLEN(S_504_PART_01 S_V_DATE S_504_PART_02 S_CRLF),
+		.flags = 4 << TFW_STR_CN_SHIFT
+	};
+
+	TFW_DBG("Send HTTP 504 response to the client\n");
+
+	return tfw_http_send_resp(hmreq, &rh, __TFW_STR_CH(&rh, 1));
+}
+
 /*
  * Allocate a new HTTP message structure, and link it with
  * the connection structure. Increment the number of users
@@ -848,6 +874,12 @@ tfw_http_req_process(TfwConnection *conn, struct sk_buff *skb, unsigned int off)
 			return TFW_BLOCK;
 		}
 
+		/*
+		 * The time the request was received is used in cache
+		 * for age calculations, and for APM and Load Balancing.
+		 */
+		hmreq->cache_ctl.timestamp = tfw_current_timestamp();
+
 		/* Assign the right Vhost for this request. */
 		if (tfw_http_req_set_context((TfwHttpReq *)hmreq))
 			return TFW_BLOCK;
@@ -1075,17 +1107,19 @@ static int
 tfw_http_resp_cache(TfwHttpMsg *hmresp)
 {
 	TfwHttpMsg *hmreq;
+	time_t timestamp = tfw_current_timestamp();
 
 	/*
-	 * If 'Date:' header is missing in the response, then set
-	 * the date to the time the response was received. That's
-	 * the same timestamp that is needed if cache is enabled.
+	 * The time the response was received is used in cache
+	 * for age calculations, and for APM and Load Balancing.
 	 */
-	if (!(hmresp->flags & TFW_HTTP_HAS_HDR_DATE)) {
-		time_t timestamp = tfw_current_timestamp();
+	hmresp->cache_ctl.timestamp = timestamp;
+	/*
+	 * If 'Date:' header is missing in the response, then
+	 * set the date to the time the response was received.
+	 */
+	if (!(hmresp->flags & TFW_HTTP_HAS_HDR_DATE))
 		((TfwHttpResp *)hmresp)->date = timestamp;
-		hmresp->cache_ctl.timestamp = timestamp;
-	}
 	/*
 	 * Cache adjusted and filtered responses only. Responses
 	 * are received in the same order as requests, so we can

tempesta_fw/http.h

@@ -360,6 +360,7 @@ void tfw_http_prep_hexstring(char *buf, u_char *value, size_t len);
  */
 int tfw_http_prep_302(TfwHttpMsg *resp, TfwHttpMsg *hm, TfwStr *cookie);
 int tfw_http_send_502(TfwHttpMsg *hm);
+int tfw_http_send_504(TfwHttpMsg *hm);
 
 /*
  * Functions to create SKBs with data stream.

tempesta_fw/http_parser.c

@@ -1353,10 +1353,7 @@ __req_parse_cache_control(TfwHttpReq *req, unsigned char *data, size_t len)
 	__FSM_STATE(Req_I_CC_m) {
 		TRY_STR("max-age=", Req_I_CC_MaxAgeV);
 		TRY_STR("min-fresh=", Req_I_CC_MinFreshV);
-		TRY_STR("max_stale", Req_I_CC_MaxStale);
-		TRY_STR_LAMBDA("max-stale", {
-			req->cache_ctl.flags |= TFW_HTTP_CC_MAX_STALE;
-		}, Req_I_CC_EoT);
+		TRY_STR("max-stale", Req_I_CC_MaxStale);
 		TRY_STR_INIT();
 		__FSM_I_MOVE_n(Req_I_CC_Ext, 0);
 	}
@@ -3008,12 +3005,6 @@ __resp_parse_age(TfwHttpResp *resp, unsigned char *data, size_t len)
 	__FSM_START(parser->_i_st) {
 
 	__FSM_STATE(Resp_I_Age) {
-		/* Eat OWS before the node ID. */
-		if (unlikely(IS_WS(c)))
-			__FSM_I_MOVE(Resp_I_Age);
-		__FSM_I_MOVE(Resp_I_AgeVal);
-	}
-	__FSM_STATE(Resp_I_AgeVal) {
 		__fsm_sz = __data_remain(p);
 		__fsm_n = parse_int_ws(p, __fsm_sz, &parser->_acc);
 		if (__fsm_n < 0)