Enhancement for cache_use_stale

Now in cache we don't lock cache entry, we just
reference count it. Relying on that fact this patch
introduce new approach for storing stale response,
without building it each time when we have stale record
in the cache.

Simply find the record in the cache, save it to
`TfwHttpReq` as `stale_ce` and keep the reference.
If `stale_ce` was not used, just put the the reference,
if it needed just build the response then put the
reference and forward response to client.

Author: const-t

fw/cache.c

@@ -3018,6 +3018,38 @@ tfw_cache_build_resp(TfwHttpReq *req, TfwCacheEntry *ce, long age)
 	return NULL;
 }
 
+TfwHttpResp *
+tfw_cache_build_resp_stale(TfwHttpReq *req)
+{
+	TDB *db = node_db();
+	TfwCacheEntry *ce = req->stale_ce;
+	TfwHttpResp *resp = tfw_cache_build_resp(req, ce, req->stale_ce_age);
+
+#if defined(DUBEG)
+	if (resp)
+		T_DBG("Cache: Stale response assigned to req [%p] w/ key=%lx, \
+		      ce=%p", req, ce->trec.key, ce);
+	else
+		T_DBG("Cache: Cannot assigne stale response to req [%p] w/ \
+		      key=%lx, ce=%p", req, ce->trec.key, ce);
+#endif
+
+	tdb_rec_put(db, ce);
+	/* Set to NULL to prevent double free in req destructor. */
+	req->stale_ce = NULL;
+
+	return resp;
+}
+
+/**
+ * Release cache entry reference.
+ */
+void
+tfw_cache_put_entry(void *ce)
+{
+	tdb_rec_put(node_db(), ce);
+}
+
 static bool
 tfw_cache_can_use_stale(TfwHttpReq *req, TfwCacheEntry *ce, long age)
 {
@@ -3093,10 +3125,12 @@ cache_req_process_node(TfwHttpReq *req, tfw_http_cache_cb_t action)
 			TFW_INC_STAT_BH(cache.misses);
 			goto out;
 		}
+		req->stale_ce = ce;
+		req->stale_ce_age = age;
 	}
 
-	T_DBG("Cache: service request [%p] w/ key=%lx, ce=%p",
-	      req, ce->trec.key, ce);
+	T_DBG("Cache: service request [%p] w/ key=%lx, ce=%p", req,
+	      ce->trec.key, ce);
 
 	TFW_INC_STAT_BH(cache.hits);
 
@@ -3116,20 +3150,8 @@ cache_req_process_node(TfwHttpReq *req, tfw_http_cache_cb_t action)
 		}
 	}
 
-	resp = tfw_cache_build_resp(req, ce, age);
-
-	if (resp && stale) {
-		req->resp = NULL;
-		req->stale_resp = resp;
-
-		if (ce->flags & TFW_CE_STALE_IF_ERROR) {
-			resp->cache_ctl.stale_if_error = ce->stale_if_error;
-			resp->cache_ctl.flags |= TFW_HTTP_CC_STALE_IF_ERROR;
-		}
-
-		T_DBG("Cache: Stale response assigned to req [%p] w/ key=%lx, \
-		      ce=%p", req, ce->trec.key, ce);
-	}
+	if (!stale)
+		resp = tfw_cache_build_resp(req, ce, age);
 
 	/*
 	 * The stream of HTTP/2-request should be closed here since we have
@@ -3141,7 +3163,7 @@ cache_req_process_node(TfwHttpReq *req, tfw_http_cache_cb_t action)
 	 * is stale, request will be forwarded to server, some forwardning
 	 * functions requires alive stream. E.g: @tfw_http_req_evict_dropped().
 	 */
-	if (resp && TFW_MSG_H2(req) && !stale) {
+	if (resp && TFW_MSG_H2(req)) {
 		id = tfw_h2_req_stream_id(req);
 		if (unlikely(!id)) {
 			tfw_http_msg_free((TfwHttpMsg *)resp);
@@ -3151,7 +3173,7 @@ cache_req_process_node(TfwHttpReq *req, tfw_http_cache_cb_t action)
 		tfw_h2_req_unlink_stream(req);
 	}
 out:
-	if (!resp && (req->cache_ctl.flags & TFW_HTTP_CC_OIFCACHED)) {
+	if (!stale && !resp && (req->cache_ctl.flags & TFW_HTTP_CC_OIFCACHED)) {
 		tfw_http_send_err_resp(req, 504, "resource not cached");
 	} else {
 		/*
@@ -3163,8 +3185,9 @@ cache_req_process_node(TfwHttpReq *req, tfw_http_cache_cb_t action)
 			set_bit(TFW_HTTP_B_REQ_HEAD_TO_GET, req->flags);
 		action((TfwHttpMsg *)req);
 	}
+	/* For stale we put entry after building response, during forwarding. */
+	if (ce && !stale)
 put:
-	if (ce)
 		tdb_rec_put(db, ce);
 }
 

fw/cache.h

@@ -25,6 +25,8 @@
 
 int tfw_cache_process(TfwHttpMsg *msg, tfw_http_cache_cb_t action);
 bool tfw_cache_is_enabled_or_not_configured(void);
+TfwHttpResp *tfw_cache_build_resp_stale(TfwHttpReq *req);
+void tfw_cache_put_entry(void *ce);
 
 extern unsigned int cache_default_ttl;
 

fw/http.c

@@ -1483,13 +1483,80 @@ tfw_http_nip_req_resched_err(TfwSrvConn *srv_conn, TfwHttpReq *req,
 			  " re-forwarded or re-scheduled");
 }
 
-static inline void
-tfw_http_send_err_resp_nolog(TfwHttpReq *req, int status)
+/**
+ * The request is serviced from cache.
+ * Send the response as is and unrefer its data.
+ */
+static void
+tfw_http_req_cache_service(TfwHttpResp *resp)
 {
+	TfwHttpReq *req = resp->req;
+
+	WARN_ON_ONCE(!list_empty(&req->fwd_list));
+	WARN_ON_ONCE(!list_empty(&req->nip_list));
+
 	if (TFW_MSG_H2(req))
-		tfw_h2_send_err_resp(req, status, false);
+		tfw_h2_resp_fwd(resp);
 	else
-		tfw_h1_send_err_resp(req, status);
+		tfw_http_resp_fwd(resp);
+
+	TFW_INC_STAT_BH(clnt.msgs_fromcache);
+}
+
+/**
+ * Build stale response from the @req->stale_ce and link request with response.
+ * Forward response to client, free previous unsuccessful response from upstream
+ * @hmresp.
+ *
+ * @return true if response successfully forwarded otherwise false.
+ */
+static bool
+__tfw_http_resp_fwd_stale(TfwHttpMsg *hmresp)
+{
+	TfwHttpReq *req = hmresp->req;
+	TfwHttpResp *stale_resp;
+	bool sent = false;
+
+	tfw_stream_unlink_msg(hmresp->stream);
+	/* Unlink response. */
+	req->resp = NULL;
+
+	stale_resp = tfw_cache_build_resp_stale(req);
+	/* For HTTP2 response will not be built if stream already closed. */
+	if (!stale_resp)
+		goto free;
+
+	req->resp->conn = hmresp->conn;
+	hmresp->pair = NULL;
+
+	if (TFW_MSG_H2(req))
+		tfw_h2_req_unlink_stream(req);
+
+	tfw_http_req_cache_service(req->resp);
+	sent = true;
+
+free:
+	tfw_http_msg_free(hmresp);
+
+	return sent;
+}
+
+/**
+ * The same as @__tfw_http_resp_fwd_stale(), but used in case when we don't
+ * have response from upstream.
+ */
+static bool
+__tfw_http_resp_fwd_stale_noresp(TfwHttpReq *req)
+{
+	if (!tfw_cache_build_resp_stale(req))
+		return false;
+
+	if (TFW_MSG_H2(req))
+		tfw_h2_req_unlink_stream(req);
+
+	tfw_http_req_cache_service(req->resp);
+
+	return true;
 }
 
 static bool
@@ -1511,9 +1578,8 @@ static bool
 tfw_http_resp_should_fwd_stale(TfwHttpReq *req, unsigned short status)
 {
 	TfwCacheUseStale *stale_opt;
-	TfwHttpResp *resp = req->stale_resp;
 
-	if (!resp)
+	if (!req->stale_ce)
 		return false;
 
 	stale_opt = tfw_vhost_get_cache_use_stale(req->location, req->vhost);
@@ -1536,29 +1602,21 @@ tfw_http_resp_should_fwd_stale(TfwHttpReq *req, unsigned short status)
 	 * from cache. Therefore we response with inaccurate age or even
 	 * with violation of max-stale param.
 	 */
-	return (resp->cache_ctl.flags & TFW_HTTP_CC_STALE_IF_ERROR ||
-		req->cache_ctl.flags & TFW_HTTP_CC_STALE_IF_ERROR) &&
-		tfw_http_use_stale_if_error(status);
+	return tfw_http_use_stale_if_error(status);
 }
 
-/**
- * The request is serviced from cache.
- * Send the response as is and unrefer its data.
- */
-static void
-tfw_http_req_cache_service(TfwHttpResp *resp)
+static inline void
+tfw_http_send_err_resp_nolog(TfwHttpReq *req, int status)
 {
-	TfwHttpReq *req = resp->req;
-
-	WARN_ON_ONCE(!list_empty(&req->fwd_list));
-	WARN_ON_ONCE(!list_empty(&req->nip_list));
-
-	if (TFW_MSG_H2(req))
-		tfw_h2_resp_fwd(resp);
-	else
-		tfw_http_resp_fwd(resp);
-
-	TFW_INC_STAT_BH(clnt.msgs_fromcache);
+	/* Response must be freed before calling tfw_http_send_err_resp_nolog(). */
+	if (tfw_http_resp_should_fwd_stale(req, status)) {
+		__tfw_http_resp_fwd_stale_noresp(req);
+	} else {
+		if (TFW_MSG_H2(req))
+			tfw_h2_send_err_resp(req, status, false);
+		else
+			tfw_h1_send_err_resp(req, status);
+	}
 }
 
 /* Common interface for sending error responses. */
@@ -1569,16 +1627,7 @@ tfw_http_send_err_resp(TfwHttpReq *req, int status, const char *reason)
 		T_WARN_ADDR_STATUS(reason, &req->conn->peer->addr,
 				   TFW_NO_PORT, status);
 
-	/* Response must be freed before calling tfw_http_send_err_resp(). */
-	if (tfw_http_resp_should_fwd_stale(req, status)) {
-		req->resp = req->stale_resp;
-		req->stale_resp = NULL;
-		if (TFW_MSG_H2(req))
-			tfw_h2_req_unlink_stream(req);
-		tfw_http_req_cache_service(req->resp);
-	} else {
-		tfw_http_send_err_resp_nolog(req, status);
-	}
+	tfw_http_send_err_resp_nolog(req, status);
 }
 
 static void
@@ -2647,8 +2696,8 @@ tfw_http_req_destruct(void *msg)
 	if (req->old_head)
 		ss_skb_queue_purge(&req->old_head);
 
-	if (req->stale_resp)
-		tfw_http_msg_free((TfwHttpMsg *)req->stale_resp);
+	if (req->stale_ce)
+		tfw_cache_put_entry(req->stale_ce);
 }
 
 /**
@@ -6658,25 +6707,6 @@ tfw_http_resp_terminate(TfwHttpMsg *hm)
 	tfw_http_resp_cache(hm);
 }
 
-static void
-__tfw_http_resp_fwd_stale(TfwHttpMsg *hmresp)
-{
-	TfwHttpReq *req = hmresp->req;
-
-	tfw_stream_unlink_msg(hmresp->stream);
-	req->resp = req->stale_resp;
-	req->stale_resp = NULL;
-	req->resp->conn = hmresp->conn;
-	hmresp->pair = NULL;
-
-	if (TFW_MSG_H2(req))
-		tfw_h2_req_unlink_stream(req);
-
-	tfw_http_req_cache_service(req->resp);
-
-	tfw_http_msg_free(hmresp);
-}
-
 static int
 tfw_http_resp_fwd_stale(TfwHttpMsg *hmresp)
 {
@@ -6711,7 +6741,8 @@ tfw_http_resp_fwd_stale(TfwHttpMsg *hmresp)
 		return T_BLOCK;
 	}
 
-	__tfw_http_resp_fwd_stale(hmresp);
+	if (!__tfw_http_resp_fwd_stale(hmresp))
+		return T_BAD;
 
 	return T_OK;
 }
@@ -6949,6 +6980,7 @@ tfw_http_resp_process(TfwConn *conn, TfwStream *stream, struct sk_buff *skb,
 	if (unlikely(r != T_OK)) {
 		if (hmsib)
 			tfw_http_conn_msg_free(hmsib);
+		return r;
 	}
 
 	*split = NULL;
@@ -6998,7 +7030,7 @@ tfw_http_resp_process(TfwConn *conn, TfwStream *stream, struct sk_buff *skb,
 		__tfw_http_resp_fwd_stale(hmresp);
 		/*
 		 * Close connection with backend immediately
-		 * and try to reastablish it later.
+		 * and try to re-establish it later.
 		 */
 		r = T_BAD;
 	} else {
@@ -7013,7 +7045,7 @@ tfw_http_resp_process(TfwConn *conn, TfwStream *stream, struct sk_buff *skb,
 					  HTTP2_ECODE_PROTO);
 			/*
 			 * Close connection with backend immediately
-			 * and try to reastablish it later.
+			 * and try to re-establish it later.
 			 */
 			r = T_BAD;
 		}

fw/http.h

@@ -347,8 +347,8 @@ typedef struct {
  *		  the same;
  * @old_head	- Original request head. Required for keep request data until
  * 		  the response is sent to the client;
- * @stale_resp	- Stale response retrieved from the cache. Must be assigned only
- * 		  when "cache_use_stale" is configured;
+ * @stale_ce	- Stale cache entry retrieved from the cache. Must be assigned
+ *		  only when "cache_use_stale" is configured;
  * @pit		- iterator for tracking transformed data allocation (applicable
  *		  for HTTP/2 mode only);
  * @userinfo	- userinfo in URI, not mandatory;
@@ -361,7 +361,9 @@ typedef struct {
  * @nip_list	- member in the queue of non-idempotent requests;
  * @jtxtstamp	- time the request is forwarded to a server, in jiffies;
  * @jrxtstamp	- time the request is received from a client, in jiffies;
- * @tm_header	- time HTTP header started coming;
+ * @tm_header	- time HTTP header started coming. Only rx path;
+ * @stale_ce_age - calculated age of stale response. Must be assigned only when
+ *		  "cache_use_stale" is configured on tx path with cache;
  * @tm_bchunk	- time previous chunk of HTTP body had come at;
  * @hash	- hash value for caching calculated for the request;
  * @frang_st	- current state of FRANG classifier;
@@ -382,7 +384,7 @@ struct tfw_http_req_t {
 	TfwHttpSess		*sess;
 	TfwClient		*peer;
 	struct sk_buff		*old_head;
-	TfwHttpResp		*stale_resp;
+	void			*stale_ce;
 	TfwHttpCond		cond;
 	TfwMsgParseIter		pit;
 	TfwStr			userinfo;
@@ -394,7 +396,10 @@ struct tfw_http_req_t {
 	struct list_head	nip_list;
 	unsigned long		jtxtstamp;
 	unsigned long		jrxtstamp;
-	unsigned long		tm_header;
+	union {
+		unsigned long		tm_header;
+		long			stale_ce_age;
+	};
 	unsigned long		tm_bchunk;
 	unsigned long		hash;
 	unsigned int		frang_st;