Fix according review

- Codestyle fixes.
- Add comments
- Move `-tfw_h2_hpack_encode_trailer_headers` to http2.c
  and make it static.

Author: EvgeniiMekhanik

fw/cache.c

@@ -203,13 +203,16 @@ __tfw_dbg_dump_ce(const TfwCacheEntry *ce)
 #define TFW_CSTR_SPEC_IDX	0x2
 /*
  * TfwCStr contains header, which should be skipped for http2 responses
- * constructed from cache.
+ * constructed from cache. We skip transfer encoding header, when we
+ * build response from cache for HTTP2 request.
+ * RFC 9113 8.1:
+ * HTTP/2 uses DATA frames to carry message content. The chunked
+ * transfer encoding defined in Section 7.1 of [HTTP/1.1] cannot be
+ * used in HTTP/2; see Section 8.2.2.
  */
 #define TFW_CSTR_SKIP_FOR_H2	0x4
 
-/*
- * TfwCStr contains trailer.
- */
+/* TfwCStr contains trailer. */
 #define TFW_CSTR_TRAILER	0x8
 
 /**
@@ -1065,8 +1068,8 @@ do {									\
 
 		*p += TFW_CSTR_HDRLEN;
 		r = write_actor(db, trec, resp, p, s->len, &dc_iter);
-			if (unlikely(r))
-				break;
+		if (unlikely(r))
+			break;
 
 		dc_iter.skip = skip;
 	}
@@ -2753,7 +2756,8 @@ tfw_cache_purge_method(TfwHttpReq *req)
 
 	/* Deny PURGE requests by default. */
 	if (!(cache_cfg.cache && g_vhost->cache_purge
-	    && g_vhost->cache_purge_acl)) {
+	    && g_vhost->cache_purge_acl))
+	{
 		tfw_http_send_err_resp(req, 403, "purge: not configured");
 		return -EINVAL;
 	}
@@ -2851,14 +2855,6 @@ tfw_cache_build_resp_body(TDB *db, TdbVRec *trec, TfwMsgIter *it, char *p,
 	 * Finish chunked body encoding. Add 0\r\n
 	 * after chunked body.
 	 */
-	TfwStr b_len = {
-		.chunks = (TfwStr []){
-			{.data = S_ZERO, .len = SLEN(S_ZERO)},
-			{.data = S_CRLF, .len = SLEN(S_CRLF)}
-		},
-		.len = SLEN(S_ZERO S_CRLF),
-		.nchunks = 2
-	};
 	bool sh_frag = h2 ? false : true;
 	int r;
 
@@ -2941,6 +2937,11 @@ tfw_cache_build_resp_body(TDB *db, TdbVRec *trec, TfwMsgIter *it, char *p,
 	}
 
 	if (chunked_body) {
+		static TfwStr b_len = {
+			.data = S_ZERO S_CRLF,
+			.len = SLEN(S_ZERO S_CRLF)
+		};
+
 		r = tfw_http_msg_expand_data(it, &it->skb_head,
 					     &g_crlf, NULL);
 		if (unlikely(r))
@@ -3057,7 +3058,6 @@ tfw_cache_build_resp(TfwHttpReq *req, TfwCacheEntry *ce, long age)
 	if (!(resp = tfw_http_msg_alloc_resp(req)))
 		goto out;
 
-
 	/* Copy version information and flags */
 	resp->version = ce->version;
 	tfw_http_copy_flags(resp->flags, ce->hmflags);
@@ -3171,15 +3171,17 @@ tfw_cache_build_resp(TfwHttpReq *req, TfwCacheEntry *ce, long age)
 	/* Fill skb with body from cache for HTTP/2 or HTTP/1.1 response. */
 	BUG_ON(p != TDB_PTR(db->hdr, ce->body));
 	if ((ce->body_len || chunked_body)
-	    && req->method != TFW_HTTP_METH_HEAD) {
+	    && req->method != TFW_HTTP_METH_HEAD)
+	{
 		if (tfw_cache_build_resp_body(db, trec, it, p, ce->body_len,
 					      h2_mode, chunked_body))
 			goto free;
 	}
 	resp->content_length = ce->body_len;
 
 	if (unlikely(ce->trailer_off < ce->hdr_num)
-	    && req->method != TFW_HTTP_METH_HEAD) {
+	    && req->method != TFW_HTTP_METH_HEAD)
+	{
 		unsigned long t_len = 0;
 
 		if (h2_mode)

fw/http.c

@@ -5080,47 +5080,6 @@ tfw_h2_hpack_encode_headers(TfwHttpResp *resp, const TfwHdrMods *h_mods)
 	return 0;
 }
 
-int
-tfw_h2_hpack_encode_trailer_headers(TfwHttpResp *resp)
-{
-	TfwHttpHdrMap *map = resp->mit.map;
-	TfwHttpHdrTbl *ht = resp->h_tbl;
-	unsigned int i;
-	int r;
-
-	for (i = map->trailer_idx; i < map->count; ++i) {
-		unsigned short hid = map->index[i].idx;
-		unsigned short d_num = map->index[i].d_idx;
-		TfwStr *tgt = &ht->tbl[hid];
-
-		if (TFW_STR_DUP(tgt))
-			tgt = TFW_STR_CHUNK(tgt, d_num);
-
-		if (WARN_ON_ONCE(!tgt
-				 || TFW_STR_EMPTY(tgt)
-				 || TFW_STR_DUP(tgt)))
-			return -EINVAL;
-
-		T_DBG3("%s: hid=%hu, d_num=%hu, nchunks=%u\n",
-		       __func__, hid, d_num, ht->tbl[hid].nchunks);
-
-		/*
-		 * 'Server' header must be replaced; thus, remove the original
-		 * header (and all its duplicates) skipping it here; the new
-		 * header will be written later, during new headers' addition
-		 * stage.
-		 */
-		if (hid == TFW_HTTP_HDR_SERVER)
-			continue;
-
-		r = tfw_hpack_transform(resp, tgt);
-		if (unlikely(r))
-			return r;
-	}
-
-	return 0;
-}
-
 /**
  * Split response body stored locally. Allocate a new skb and put body there
  * by fragments. Every skb fragment has size of single page and has frame

fw/http.h

@@ -766,7 +766,6 @@ int tfw_h2_resp_add_loc_hdrs(TfwHttpResp *resp, const TfwHdrMods *h_mods,
 int tfw_h2_resp_status_write(TfwHttpResp *resp, unsigned short status,
 			     bool use_pool, bool cache);
 int tfw_h2_resp_encode_headers(TfwHttpResp *resp);
-int tfw_h2_hpack_encode_trailer_headers(TfwHttpResp *resp);
 /*
  * Functions to send an HTTP error response to a client.
  */

fw/http2.c

@@ -480,6 +480,47 @@ tfw_h2_req_unlink_and_close_stream(TfwHttpReq *req)
 	spin_unlock(&ctx->lock);
 }
 
+static int
+tfw_h2_hpack_encode_trailer_headers(TfwHttpResp *resp)
+{
+	TfwHttpHdrMap *map = resp->mit.map;
+	TfwHttpHdrTbl *ht = resp->h_tbl;
+	unsigned int i;
+	int r;
+
+	for (i = map->trailer_idx; i < map->count; ++i) {
+		unsigned short hid = map->index[i].idx;
+		unsigned short d_num = map->index[i].d_idx;
+		TfwStr *tgt = &ht->tbl[hid];
+
+		if (TFW_STR_DUP(tgt))
+			tgt = TFW_STR_CHUNK(tgt, d_num);
+
+		if (WARN_ON_ONCE(!tgt
+				 || TFW_STR_EMPTY(tgt)
+				 || TFW_STR_DUP(tgt)))
+			return -EINVAL;
+
+		T_DBG3("%s: hid=%hu, d_num=%hu, nchunks=%u\n",
+		       __func__, hid, d_num, ht->tbl[hid].nchunks);
+
+		/*
+		 * 'Server' header must be replaced; thus, remove the original
+		 * header (and all its duplicates) skipping it here; the new
+		 * header will be written later, during new headers' addition
+		 * stage.
+		 */
+		if (hid == TFW_HTTP_HDR_SERVER)
+			continue;
+
+		r = tfw_hpack_transform(resp, tgt);
+		if (unlikely(r))
+			return r;
+	}
+
+	return 0;
+}
+
 int
 tfw_h2_stream_xmit_prepare_resp(TfwStream *stream)
 {
@@ -536,7 +577,8 @@ tfw_h2_stream_xmit_prepare_resp(TfwStream *stream)
 		 * on HEAD request.
 		 */
 		if (test_bit(TFW_HTTP_B_CHUNKED, resp->flags)
-		    && !test_bit(TFW_HTTP_B_VOID_BODY, resp->flags)) {
+		    && !test_bit(TFW_HTTP_B_VOID_BODY, resp->flags))
+		{
 			r = tfw_http_msg_cutoff_body_chunks(resp);
 			if (unlikely(r)) {
 				T_WARN("Failed to encode body");

fw/http_frame.c

@@ -1897,7 +1897,6 @@ tfw_h2_calc_frame_flags(TfwStream *stream, TfwFrameType type,
 	    && type == HTTP2_HEADERS && trailers)
 		flags |= HTTP2_F_END_STREAM;
 
-
 	if (!stream->xmit.h_len && type != HTTP2_DATA && !trailers)
 		flags |= HTTP2_F_END_HEADERS;
 

fw/http_parser.c

@@ -467,7 +467,6 @@ __FSM_STATE(st, cold) {							\
 	__FSM_JMP(RGen_HdrOtherN);					\
 }
 
-
 #define __FSM_TX_AF_OWS_LAMBDA(st, st_next, lambda)			\
 __FSM_STATE(st, cold) {							\
 	if (likely(c == ':')) {						\
@@ -5331,9 +5330,9 @@ tfw_http_parse_req(void *req_data, unsigned char *data, unsigned int len,
 				   && C8_INT_LCM(p, 't', 'r', 'a', 'i',
 						    'l', 'e', 'r', ':'))
 			{
-				__msg_hdr_chunk_fixup(data, __data_off(p + 8));
+				__msg_hdr_chunk_fixup(data, __data_off(p + 7));
 				parser->_i_st = &&Req_HdrTrailerV;
-				p += 8;
+				p += 7;
 				__FSM_MOVE_hdr_fixup(RGen_LWS, 1);
 			}
 			__FSM_MOVE(Req_HdrT);
@@ -11245,17 +11244,17 @@ __resp_parse_cache_control(TfwHttpResp *resp, unsigned char *data, size_t len)
 
 	__FSM_START(parser->_i_st);
 
-	/*
-	 * According RFC 7230 4.1.2:
-	 * A sender MUST NOT generate a trailer that contains response
-	 * control data (e.g., see Section 7.1 of [RFC7231]).
-	 */
-	if (unlikely(test_bit(TFW_HTTP_B_HEADERS_PARSED, resp->flags)))
-		return r;
-
 	parser->cc_dir_flag = 0;
 
 	__FSM_STATE(Resp_I_CC_start) {
+		/*
+		 * According RFC 7230 4.1.2:
+		 * A sender MUST NOT generate a trailer that contains response
+		 * control data (e.g., see Section 7.1 of [RFC7231]).
+		 */
+		if (unlikely(test_bit(TFW_HTTP_B_HEADERS_PARSED, resp->flags)))
+			return r;
+
 		/* Spaces already skipped by RGen_LWS */
 		/* Leading comma allowed per RFC 7230 Section 7 */
 		if (c == ',')

fw/ss_skb.h

@@ -429,7 +429,7 @@ ss_skb_data_ptr_by_offset(struct sk_buff *skb, unsigned int off)
 		if (ss_skb_is_within_fragment(begin, begin + off, end))
 			return begin + off;
 
-		off -= (end - begin);
+		off -= end - begin;
 	}
 
 	return NULL;