Fix review commits, the rest of #1000(#166) in particular

tempesta_fw/cache.c

@@ -1445,27 +1445,21 @@ static int
 tfw_cache_build_resp_body(TDB *db, TfwHttpResp *resp, TdbVRec *trec,
 			  TfwMsgIter *it, char *p)
 {
-	int off, f_size;
+	int off, f_size, r;
 	skb_frag_t *frag;
 
 	BUG_ON(!it->skb);
 	frag = &skb_shinfo(it->skb)->frags[it->frag];
 	if (skb_frag_size(frag))
 		++it->frag;
-	if (it->frag >= MAX_SKB_FRAGS - 1) {
-		if (!(it->skb = ss_skb_alloc(0)))
-			return -ENOMEM;
-		ss_skb_queue_tail(&resp->msg.skb_head, it->skb);
-		it->frag = 0;
-	}
+	if (it->frag >= MAX_SKB_FRAGS - 1
+	    && (r = tfw_http_msg_setup((TfwHttpMsg *)resp, it, 0)))
+		return r;
 
 	while (1) {
-		if (it->frag == MAX_SKB_FRAGS) {
-			if (!(it->skb = ss_skb_alloc(0)))
-				return -ENOMEM;
-			ss_skb_queue_tail(&resp->msg.skb_head, it->skb);
-			it->frag = 0;
-		}
+		if (it->frag == MAX_SKB_FRAGS
+		    && (r = tfw_http_msg_setup((TfwHttpMsg *)resp, it, 0)))
+			return r;
 
 		/* TDB keeps data by pages and we can reuse the pages. */
 		off = (unsigned long)p & ~PAGE_MASK;

tempesta_fw/http.c

@@ -466,11 +466,10 @@ tfw_http_prep_redirect(TfwHttpMsg *resp, unsigned short status, TfwStr *rmark,
  * HTTP 304 response: Not Modified.
  */
 int
-tfw_http_prep_304(TfwHttpMsg *resp, TfwHttpReq *req, void *msg_it,
+tfw_http_prep_304(TfwHttpMsg *resp, TfwHttpReq *req, TfwMsgIter *it,
 		  size_t hdrs_size)
 {
 	size_t data_len = SLEN(S_304_PART_01);
-	TfwMsgIter *it = (TfwMsgIter *)msg_it;
 	int conn_flag = req->flags & __TFW_HTTP_MSG_M_CONN_MASK, ret = 0;
 	static TfwStr rh = {
 		.ptr = S_304_PART_01, .len = SLEN(S_304_PART_01) };

tempesta_fw/http_msg.c

@@ -799,7 +799,7 @@ tfw_http_msg_setup(TfwHttpMsg *hm, TfwMsgIter *it, size_t data_len)
 
 	if ((r = tfw_msg_iter_setup(it, &hm->msg.skb_head, data_len)))
 		return r;
-	TFW_DBG2("Set up new HTTP message %p: len=%lu\n", hm, data_len);
+	TFW_DBG2("Set up HTTP message %pK with %lu bytes data\n", hm, data_len);
 
 	return 0;
 }
@@ -833,7 +833,7 @@ __http_msg_add_data_frag(TfwMsgIter *it, TfwHttpMsg *hm, TfwStr *field,
 		return -ENOMEM;
 
 	if (d_size > f_room) {
-		frag = ss_skb_frag_next(&it->skb, &it->frag);
+		frag = ss_skb_frag_next(&it->skb, it->skb_head, &it->frag);
 		off += n_copy;
 		goto next_frag;
 	}

tempesta_fw/log.h

@@ -32,7 +32,6 @@
 #define TFW_WARN_NL(...)	T_WARN_NL(__VA_ARGS__)
 #define TFW_LOG(...)		T_LOG(__VA_ARGS__)
 #define TFW_LOG_NL(...)		T_LOG_NL(__VA_ARGS__)
-#define TFW_ERR(...)		T_ERR(__VA_ARGS__)
 #define TFW_DBG(...)		T_DBG(__VA_ARGS__)
 #define TFW_DBG2(...)		T_DBG2(__VA_ARGS__)
 #define TFW_DBG3(...)		T_DBG3(__VA_ARGS__)

tempesta_fw/msg.c

@@ -70,7 +70,8 @@ tfw_msg_write(TfwMsgIter *it, const TfwStr *data)
 			 */
 			c_off = 0;
 		} else {
-			frag = ss_skb_frag_next(&it->skb, &it->frag);
+			frag = ss_skb_frag_next(&it->skb, it->skb_head,
+						&it->frag);
 			/*
 			 * If all data from the chunk has been copied,
 			 * then switch to the next chunk. Otherwise,
@@ -96,8 +97,8 @@ tfw_msg_iter_setup(TfwMsgIter *it, struct sk_buff **skb_head, size_t data_len)
 
 	if ((r = ss_skb_alloc_data(skb_head, data_len)))
 		return r;
-	it->skb = *skb_head;
-	it->frag = -1;
+	it->skb = it->skb_head = *skb_head;
+	it->frag = data_len ? -1 /* first 'frag' is the skb head */ : 0;
 
 	BUG_ON(!it->skb);
 

tempesta_fw/msg.h

@@ -51,11 +51,14 @@ typedef struct {
 /**
  * Iterator for @skb fragments.
  *
- * @frag	- current fragment index or @skb->head if -1.
+ * @frag	- current fragment index or @skb->head if -1;
+ * @skb		- current skb fragment;
+ * @skb_head	- begin of the skb fragments list.
  */
 typedef struct {
 	int		frag;
 	struct sk_buff	*skb;
+	struct sk_buff	*skb_head;
 } TfwMsgIter;
 
 int tfw_msg_write(TfwMsgIter *it, const TfwStr *data);

tempesta_fw/ss_skb.c

@@ -107,11 +107,13 @@ ss_skb_alloc_pages(size_t len)
 int
 ss_skb_alloc_data(struct sk_buff **skb_head, size_t len)
 {
-	int i_skb, nr_skbs = DIV_ROUND_UP(len, SS_SKB_MAX_DATA_LEN);
+	int i_skb, nr_skbs = len ? DIV_ROUND_UP(len, SS_SKB_MAX_DATA_LEN) : 1;
+	size_t n = 0;
 	struct sk_buff *skb;
 
-	for (i_skb = 0; i_skb < nr_skbs; ++i_skb) {
-		skb = ss_skb_alloc_pages(min(len, SS_SKB_MAX_DATA_LEN));
+	for (i_skb = 0; i_skb < nr_skbs; ++i_skb, len -= n) {
+		n = min(len, SS_SKB_MAX_DATA_LEN);
+		skb = ss_skb_alloc_pages(n);
 		if (!skb)
 			return -ENOMEM;
 		ss_skb_queue_tail(skb_head, skb);
@@ -290,7 +292,7 @@ __extend_pgfrags(struct sk_buff *skb_head, struct sk_buff *skb, int from, int n)
 		struct sk_buff *nskb;
 		unsigned int e_size = 0;
 
-		/* Going out of the @skb is prohibied by the caller. */
+		/* Going out if the @skb is prohibied by the caller. */
 		if (!skb_head)
 			return -ENOMEM;
 

tempesta_fw/ss_skb.h

@@ -90,6 +90,11 @@ ss_skb_peek_tail(struct sk_buff **skb_head)
 	return *skb_head ? (*skb_head)->prev : NULL;
 }
 
+/**
+ * Almost a copy of standard skb_dequeue() except it works with skb list
+ * instead of sk_buff_head. Several crucial data include skb list and we don't
+ * want to spend extra memory for unused members of skb_buff_head.
+ */
 static inline struct sk_buff *
 ss_skb_dequeue(struct sk_buff **skb_head)
 {
@@ -116,15 +121,15 @@ ss_skb_adjust_data_len(struct sk_buff *skb, int delta)
 }
 
 static inline skb_frag_t *
-ss_skb_frag_next(struct sk_buff **skb, int *f)
+ss_skb_frag_next(struct sk_buff **skb, const struct sk_buff *skb_head, int *f)
 {
 	if (skb_shinfo(*skb)->nr_frags > *f + 1) {
 		++*f;
 		return &skb_shinfo(*skb)->frags[*f];
 	}
 
 	*skb = (*skb)->next;
-	if (!*skb || !skb_shinfo(*skb)->nr_frags)
+	if (*skb == skb_head || !skb_shinfo(*skb)->nr_frags)
 		return NULL;
 	*f = 0;
 	return &skb_shinfo(*skb)->frags[0];

tempesta_fw/tls.c

@@ -284,7 +284,7 @@ tfw_tls_send(TlsCtx *tls, struct sg_table *sgt)
 
 		for_each_sg(sgt->sgl, sg, sgt->nents, f) {
 			if (i >= MAX_SKB_FRAGS) {
-				if (!(it.skb = ss_skb_alloc(0)))
+				if (!(skb = ss_skb_alloc(0)))
 					return -ENOMEM;
 				tfw_tls_skb_set_enc(tls, skb);
 				ss_skb_queue_tail(&io->skb_list, skb);