fix #1189: correctly fill header part of skb when writing data into p…

tempesta_fw/cache.c

@@ -1444,19 +1444,33 @@ tfw_cache_build_resp_body(TDB *db, TfwHttpResp *resp, TdbVRec *trec,
 			  TfwMsgIter *it, char *p)
 {
 	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 (WARN_ON_ONCE(!it->skb))
+		return -EINVAL;
+	/*
+	 * If headers perfectly fit allocated skbs, then
+	 * it->skb == it->skb_head, see tfw_msg_iter_next_data_frag().
+	 * Normally all the headers fit single skb, but these two situations
+	 * can't be distingueshed. Start after last fragment of last skb in list.
+	 */
+	if ((it->skb == it->skb_head) || (it->frag == -1)) {
+		it->skb = ss_skb_peek_tail(&it->skb_head);
+		it->frag = skb_shinfo(it->skb)->nr_frags;
+	}
+	else {
+		skb_frag_t *frag = &skb_shinfo(it->skb)->frags[it->frag];
+		if (skb_frag_size(frag))
+			++it->frag;
+	}
+	BUG_ON(it->frag < 0);
+
 	if (it->frag >= MAX_SKB_FRAGS - 1
-	    && (r = tfw_http_msg_setup((TfwHttpMsg *)resp, it, 0)))
+	    && (r = tfw_msg_iter_append_skb(it)))
 		return r;
 
 	while (1) {
 		if (it->frag == MAX_SKB_FRAGS
-		    && (r = tfw_http_msg_setup((TfwHttpMsg *)resp, it, 0)))
+		    && (r = tfw_msg_iter_append_skb(it)))
 			return r;
 
 		/* TDB keeps data by pages and we can reuse the pages. */

tempesta_fw/http_msg.c

@@ -889,80 +889,75 @@ tfw_http_msg_setup(TfwHttpMsg *hm, TfwMsgIter *it, size_t data_len)
 }
 EXPORT_SYMBOL(tfw_http_msg_setup);
 
-static int
-__http_msg_add_data_frag(TfwMsgIter *it, TfwHttpMsg *hm, TfwStr *field,
-			 const TfwStr *data, unsigned int off)
-{
-	char *p;
-	skb_frag_t *frag = &skb_shinfo(it->skb)->frags[it->frag];
-	unsigned int f_size, d_size, f_room, n_copy;
-
-next_frag:
-	if (!frag)
-		return -E2BIG;
-
-	d_size = data->len - off;
-	f_size = skb_frag_size(frag);
-	f_room = PAGE_SIZE - frag->page_offset - f_size;
-	n_copy = min(d_size, f_room);
-	if (!n_copy)
-		return 0;
-
-	p = (char *)skb_frag_address(frag) + f_size;
-	memcpy_fast(p, data->data + off, n_copy);
-	skb_frag_size_add(frag, n_copy);
-	ss_skb_adjust_data_len(it->skb, n_copy);
-
-	if (__tfw_http_msg_add_str_data(hm, field, p, n_copy, it->skb))
-		return -ENOMEM;
-
-	if (d_size > f_room) {
-		frag = ss_skb_frag_next(&it->skb, it->skb_head, &it->frag);
-		off += n_copy;
-		goto next_frag;
-	}
-
-	return 0;
-}
-
 /**
- * Similar to tfw_msg_write(), but properly maintain @hm header
- * fields, so that @hm can be used in regular transformations. However,
- * the header name and the value are not split into different chunks,
- * so advanced headers matching is not available for @hm.
+ * Fill up an HTTP message by iterator @it with data from string @data.
+ * Properly maintain @hm header @field, so that @hm can be used in regular
+ * transformations. However, the header name and the value are not split into
+ * different chunks, so advanced headers matching is not available for @hm.
  */
 int
 tfw_http_msg_add_data(TfwMsgIter *it, TfwHttpMsg *hm, TfwStr *field,
 		      const TfwStr *data)
 {
-	char *p;
-	unsigned int n_copy;
+	const TfwStr *c, *end;
 
-	WARN_ON_ONCE(TFW_STR_DUP(data));
-	WARN_ON_ONCE(!TFW_STR_PLAIN(data));
-
-	n_copy = min(data->len, (unsigned long)skb_tailroom(it->skb));
-	if (!n_copy)
-		return 0;
-
-	if (it->frag >= 0)
-		goto add_frag;
+	BUG_ON(TFW_STR_DUP(data));
+	if (WARN_ON_ONCE(it->frag >= skb_shinfo(it->skb)->nr_frags))
+		return -E2BIG;
 
-	p = skb_put(it->skb, n_copy);
-	memcpy_fast(p, data->data, n_copy);
+	TFW_STR_FOR_EACH_CHUNK(c, data, end) {
+		char *p;
+		unsigned int c_off = 0, c_size, f_room, n_copy;
+this_chunk:
+		c_size = c->len - c_off;
+		if (it->frag >= 0) {
+			unsigned int f_size;
+			skb_frag_t *frag = &skb_shinfo(it->skb)->frags[it->frag];
+
+			f_size = skb_frag_size(frag);
+			f_room = PAGE_SIZE - frag->page_offset - f_size;
+			p = (char *)skb_frag_address(frag) + f_size;
+			n_copy = min(c_size, f_room);
+			skb_frag_size_add(frag, n_copy);
+			ss_skb_adjust_data_len(it->skb, n_copy);
+		} else {
+			f_room = skb_tailroom(it->skb);
+			n_copy = min(c_size, f_room);
+			p = skb_put(it->skb, n_copy);
+		}
 
-	if (__tfw_http_msg_add_str_data(hm, field, p, n_copy, it->skb))
-		return -ENOMEM;
+		memcpy_fast(p, c->data + c_off, n_copy);
+		if (field && n_copy
+		    && __tfw_http_msg_add_str_data(hm, field, p, n_copy,
+						   it->skb))
+		{
+			return -ENOMEM;
+		}
 
-	if (data->len == n_copy) {
-		if (!skb_tailroom(it->skb))
-			it->frag = 0;
-		return 0;
+		/*
+		 * The chunk occupied all the spare space in the SKB fragment,
+		 * switch to the next fragment.
+		 */
+		if (c_size >= f_room) {
+			if (WARN_ON_ONCE(tfw_msg_iter_next_data_frag(it)
+					 && ((c_size != f_room)
+					     || (c + 1 < end))))
+			{
+				return -E2BIG;
+			}
+			/*
+			 * Not all data from the chunk has been copied,
+			 * stay in the current chunk and copy the rest to the
+			 * next fragment.
+			 */
+			if (c_size != f_room) {
+				c_off += n_copy;
+				goto this_chunk;
+			}
+		}
 	}
 
-add_frag:
-	it->frag = 0;
-	return __http_msg_add_data_frag(it, hm, field, data, n_copy);
+	return 0;
 }
 
 void

tempesta_fw/msg.c

@@ -19,6 +19,7 @@
  */
 #include "lib/str.h"
 #include "msg.h"
+#include "http_msg.h"
 
 /**
  * Fill up an HTTP message by iterator @it with data from string @data.
@@ -34,66 +35,15 @@
 int
 tfw_msg_write(TfwMsgIter *it, const TfwStr *data)
 {
-	const TfwStr *c, *end;
-
-	BUG_ON(TFW_STR_DUP(data));
-	if (WARN_ON_ONCE(it->frag >= skb_shinfo(it->skb)->nr_frags))
-		return -E2BIG;
-
-	TFW_STR_FOR_EACH_CHUNK(c, data, end) {
-		char *p;
-		unsigned int c_off = 0, c_size, f_room, n_copy;
-this_chunk:
-
-		c_size = c->len - c_off;
-		if (it->frag >= 0) {
-			unsigned int f_size;
-			skb_frag_t *frag = &skb_shinfo(it->skb)->frags[it->frag];
-
-			f_size = skb_frag_size(frag);
-			f_room = PAGE_SIZE - frag->page_offset - f_size;
-			p = (char *)skb_frag_address(frag) + f_size;
-			n_copy = min(c_size, f_room);
-			skb_frag_size_add(frag, n_copy);
-			ss_skb_adjust_data_len(it->skb, n_copy);
-		} else {
-			f_room = skb_tailroom(it->skb);
-			n_copy = min(c_size, f_room);
-			p = skb_put(it->skb, n_copy);
-		}
-
-		memcpy_fast(p, c->data + c_off, n_copy);
-
-		/*
-		 * The chunk occupied all the spare space in the SKB fragment,
-		 * switch to the next fragment.
-		 */
-		if (c_size >= f_room) {
-			skb_frag_t *frag = ss_skb_frag_next(&it->skb,
-							    it->skb_head,
-							    &it->frag);
-			if (WARN_ON_ONCE(!frag
-					 && ((c_size != f_room)
-					     || (c + 1 < end))))
-			{
-				return -E2BIG;
-			}
-			/*
-			 * Not all data from the chunk has been copied,
-			 * stay in the current chunk and copy the rest to the
-			 * next fragment.
-			 */
-			if (c_size != f_room) {
-				c_off += n_copy;
-				goto this_chunk;
-			}
-		}
-	}
-
-	return 0;
+	return tfw_http_msg_add_data(it, NULL, NULL, data);
 }
 EXPORT_SYMBOL(tfw_msg_write);
 
+/**
+ * Allocate list of skbs to store data with given length @data_len and
+ * initialise the iterator it. Shouldn't be called against previously used
+ * iterator, since its current state is to be rewritten.
+ */
 int
 tfw_msg_iter_setup(TfwMsgIter *it, struct sk_buff **skb_head, size_t data_len)
 {
@@ -108,3 +58,21 @@ tfw_msg_iter_setup(TfwMsgIter *it, struct sk_buff **skb_head, size_t data_len)
 
 	return 0;
 }
+
+/**
+ * Allocate and add a single empty skb (with a place for TCP headers though)
+ * to the iterator. The allocated skb has no space for the data, user is
+ * expected to add new paged fragments.
+ */
+int
+tfw_msg_iter_append_skb(TfwMsgIter *it)
+{
+	int r;
+
+	if ((r = ss_skb_alloc_data(&it->skb_head, 0)))
+		return r;
+	it->skb = ss_skb_peek_tail(&it->skb_head);
+	it->frag = 0;
+
+	return 0;
+}

tempesta_fw/msg.h

@@ -64,5 +64,24 @@ typedef struct {
 int tfw_msg_write(TfwMsgIter *it, const TfwStr *data);
 int tfw_msg_iter_setup(TfwMsgIter *it, struct sk_buff **skb_head,
 		       size_t data_len);
+int tfw_msg_iter_append_skb(TfwMsgIter *it);
+
+static inline int
+tfw_msg_iter_next_data_frag(TfwMsgIter *it)
+{
+	if (skb_shinfo(it->skb)->nr_frags > it->frag + 1) {
+		++it->frag;
+		return 0;
+	}
+
+	it->skb = it->skb->next;
+	if (it->skb == it->skb_head || !skb_shinfo(it->skb)->nr_frags) {
+		it->frag = MAX_SKB_FRAGS;
+		return -EINVAL;
+	}
+	it->frag = -1;
+
+	return 0;
+}
 
 #endif /* __TFW_MSG_H__ */

tempesta_fw/ss_skb.h

@@ -120,21 +120,6 @@ ss_skb_adjust_data_len(struct sk_buff *skb, int delta)
 	skb->truesize += delta;
 }
 
-static inline skb_frag_t *
-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;
-	*f = 0;
-	if (*skb == skb_head || !skb_shinfo(*skb)->nr_frags)
-		return NULL;
-	return &skb_shinfo(*skb)->frags[0];
-}
-
 /*
  * skb_tailroom - number of bytes at buffer end
  *