Call tfw_http_hdr_split only during hpack encoding

Previously we call `tfw_http_hdr_split` twice -
in `tfw_hpack_encoder_index` and in `tfw_hpack_hdr_add`,
this patch fixes this behaviour, we split header at
the beginning of `__tfw_hpack_encode` if it is necessary
and use header name and header value later.

Author: EvgeniiMekhanik

fw/hpack.c

@@ -3021,19 +3021,17 @@ tfw_hpack_rbuf_commit(TfwHPackETbl *__restrict tbl,
  * headers will be evicted from the index table.
  */
 static int
-tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
-		   TfwHPackNodeIter *__restrict place, bool spcolon)
+tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict h_name,
+		   TfwStr *__restrict h_val, TfwHPackNodeIter *__restrict place)
 {
 	char *ptr;
 	unsigned long node_size, hdr_len;
 	unsigned short new_size, node_len;
 	unsigned short cur_size = tbl->size, window = tbl->window;
 	TfwHPackNode *del_list[HPACK_MAX_ENC_EVICTION] = {};
-	TfwStr s_nm = {}, s_val = {};
 	TfwHPackETblIter it = {};
 
-	hdr_len = tfw_http_hdr_split(hdr, &s_nm, &s_val, spcolon);
-	WARN_ON_ONCE(TFW_STR_EMPTY(&s_nm));
+	hdr_len = h_name->len + h_val->len;
 
 	WARN_ON_ONCE(cur_size > window || window > HPACK_ENC_TABLE_MAX_SIZE);
 	if ((node_size = hdr_len + HPACK_ENTRY_OVERHEAD) > window) {
@@ -3126,14 +3124,14 @@ tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
 	it.size += node_size;
 	it.rb_len += node_len;
 	it.last->hdr_len = hdr_len;
-	it.last->name_len = s_nm.len;
+	it.last->name_len = h_name->len;
 	it.last->rindex = ++tbl->idx_acc;
 
-	tfw_hpack_rbuf_commit(tbl, &s_nm, &s_val, del_list, place, &it);
+	tfw_hpack_rbuf_commit(tbl, h_name, h_val, del_list, place, &it);
 
-	ptr = tfw_hpack_write(&s_nm, it.last->hdr);
-	if (!TFW_STR_EMPTY(&s_val))
-		tfw_hpack_write(&s_val, ptr);
+	ptr = tfw_hpack_write(h_name, it.last->hdr);
+	if (!TFW_STR_EMPTY(h_val))
+		tfw_hpack_write(h_val, ptr);
 
 	WARN_ON_ONCE(tbl->rb_len > tbl->size);
 
@@ -3145,36 +3143,28 @@ tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
  * encoder dynamic table with potentially concurrent access from different
  * threads, so lock is used to protect the find/add/erase operations inside
  * this procedure.
- * 
- * TODO #1411: get rid of tfw_http_hdr_split and related safety checks
  */
 static TfwHPackETblRes
 tfw_hpack_encoder_index(TfwHPackETbl *__restrict tbl,
-			TfwStr *__restrict hdr,
+			TfwStr *__restrict h_name,
+			TfwStr *__restrict h_val,
 			unsigned short *__restrict out_index,
-			unsigned long *__restrict flags,
-			bool spcolon)
+			unsigned long *__restrict flags)
 {
 	TfwHPackNodeIter place = {};
 	const TfwHPackNode *node = NULL;
 	TfwHPackETblRes res = HPACK_IDX_ST_NOT_FOUND;
-	TfwStr h_name = {}, h_val = {};
 
 	BUILD_BUG_ON(HPACK_IDX_ST_MASK < _HPACK_IDX_ST_NUM - 1);
-	if (WARN_ON_ONCE(!hdr))
-		return -EINVAL;
 
-	tfw_http_hdr_split(hdr, &h_name, &h_val, spcolon);
-	if (WARN_ON_ONCE(TFW_STR_EMPTY(&h_name)))
-		return -EINVAL;
-	res = tfw_hpack_rbtree_find(tbl, &h_name, &h_val, &node, &place);
+	res = tfw_hpack_rbtree_find(tbl, h_name, h_val, &node, &place);
 
 	WARN_ON_ONCE(!node && res != HPACK_IDX_ST_NOT_FOUND);
 
 	*out_index = HPACK_NODE_GET_INDEX(tbl, node);
 
 	if(res != HPACK_IDX_ST_FOUND
-	   && !tfw_hpack_add_node(tbl, hdr, &place, spcolon))
+	   && !tfw_hpack_add_node(tbl, h_name, h_val, &place))
 		res |= HPACK_IDX_FLAG_ADD;
 
 	return res;
@@ -3419,31 +3409,23 @@ tfw_hpack_write_idx(TfwHttpResp *__restrict resp, TfwHPackInt *__restrict idx,
  * response @resp using @resp->pool.
  */
 static int
-tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
-		  TfwHPackInt *__restrict idx, bool name_indexed, bool trans)
+tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict h_name,
+		  TfwStr *__restrict h_val, TfwHPackInt *__restrict idx,
+		  bool name_indexed, bool trans)
 {
 	int r;
 	TfwHPackInt vlen;
-	TfwStr s_name = {}, s_val = {}, s_vlen = {};
-
-	if (!hdr)
-		return -EINVAL;
+	TfwStr s_vlen = {};
 
 	r = tfw_hpack_write_idx(resp, idx, true);
 	if (unlikely(r))
 		return r;
 
-	if (WARN_ON_ONCE(TFW_STR_PLAIN(hdr) || TFW_STR_DUP(hdr)))
-		return -EINVAL;
-
-	if (!tfw_http_hdr_split(hdr, &s_name, &s_val, trans))
-		return -EINVAL;
-
 	if (unlikely(!name_indexed)) {
 		TfwHPackInt nlen;
 		TfwStr s_nlen = {};
 
-		write_int(s_name.len, 0x7F, 0, &nlen);
+		write_int(h_name->len, 0x7F, 0, &nlen);
 		s_nlen.data = nlen.buf;
 		s_nlen.len = nlen.sz;
 
@@ -3452,23 +3434,23 @@ tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
 			return r;
 
 		if (trans)
-			r = tfw_http_msg_expand_from_pool_lc(resp, &s_name);
+			r = tfw_http_msg_expand_from_pool_lc(resp, h_name);
 		else
-			r = tfw_http_msg_expand_from_pool(resp, &s_name);
+			r = tfw_http_msg_expand_from_pool(resp, h_name);
 		if (unlikely(r))
 			return r;
 	}
 
-	write_int(s_val.len, 0x7F, 0, &vlen);
+	write_int(h_val->len, 0x7F, 0, &vlen);
 	s_vlen.data = vlen.buf;
 	s_vlen.len = vlen.sz;
 
 	r = tfw_http_msg_expand_from_pool(resp, &s_vlen);
-
 	if (unlikely(r))
 		return r;
-	if (!TFW_STR_EMPTY(&s_val))
-		r = tfw_http_msg_expand_from_pool(resp, &s_val);
+
+	if (!TFW_STR_EMPTY(h_val))
+		r = tfw_http_msg_expand_from_pool(resp, h_val);
 
 	return r;
 }
@@ -3560,6 +3542,15 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
 	TfwHPackETbl *tbl = &ctx->hpack.enc_tbl;
 	int r = HPACK_IDX_ST_NOT_FOUND;
 	bool name_indexed = true;
+	TfwStr h_name = {}, h_val = {};
+
+#define __HDR_SPLIT(hdr)						\
+do {									\
+	if (unlikely(TFW_STR_PLAIN(hdr) || TFW_STR_DUP(hdr)		\
+		     || !tfw_http_hdr_split(hdr, &h_name, &h_val, trans) \
+		     || TFW_STR_EMPTY(&h_name)))			\
+			return -EINVAL;					\
+} while(0)
 
 	if (WARN_ON_ONCE(!hdr || TFW_STR_EMPTY(hdr)))
 		return -EINVAL;
@@ -3572,9 +3563,10 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
 	T_DBG_PRINT_HPACK_RBTREE(tbl);
 
 	if (!st_full_index && dyn_indexing) {
+		__HDR_SPLIT(hdr);
 		assert_spin_locked(&conn->sk->sk_lock.slock);
-		r = tfw_hpack_encoder_index(tbl, hdr, &index, resp->flags,
-					    trans);
+		r = tfw_hpack_encoder_index(tbl, &h_name, &h_val,
+					    &index, resp->flags);
 		if (r < 0)
 			return r;
 	}
@@ -3626,11 +3618,18 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
 	name_indexed = false;
 
 encode:
-	if (use_pool)
-		r = tfw_hpack_hdr_add(resp, hdr, &idx, name_indexed, trans);
-	else
+	if (use_pool) {
+		if (!dyn_indexing)
+			__HDR_SPLIT(hdr);
+		r = tfw_hpack_hdr_add(resp, &h_name, &h_val, &idx,
+				      name_indexed, trans);
+	} else {
 		r = tfw_hpack_hdr_expand(resp, hdr, &idx, name_indexed);
+	}
+
 	return r;
+
+#undef __HDR_SPLIT
 }
 
 int

fw/str.c

@@ -1367,24 +1367,24 @@ tfw_str_crc32_calc(const TfwStr *str)
 	return crc;
 }
 
-#ifdef DEBUG
 void
 tfw_str_dprint(const TfwStr *str, const char *msg)
 {
 	const TfwStr *dup, *dup_end, *c, *chunk_end;
 
-	T_DBG("%s: addr=%p skb=%p len=%lu flags=%x eolen=%u:\n", msg,
+	printk(KERN_ALERT "%s: addr=%p skb=%p len=%lu flags=%x eolen=%u:\n", msg,
 	      str, str->skb, str->len, str->flags, str->eolen);
 	TFW_STR_FOR_EACH_DUP(dup, str, dup_end) {
-		T_DBG("  duplicate %p, len=%lu, flags=%x eolen=%u:\n",
+		printk(KERN_ALERT "  duplicate %p, len=%lu, flags=%x eolen=%u:\n",
 		      dup, dup->len, dup->flags, dup->eolen);
 		TFW_STR_FOR_EACH_CHUNK(c, dup, chunk_end)
-			T_DBG("   len=%lu, eolen=%u ptr=%p flags=%x '%.*s'\n",
+			printk(KERN_ALERT "   len=%lu, eolen=%u ptr=%p flags=%x '%.*s'\n",
 			      c->len, c->eolen, c->data, c->flags,
 			      (int)c->len, c->data);
 	}
 }
 
+#ifdef DEBUG
 #define D(n)	(unsigned int)v[n]
 
 void

fw/str.h

@@ -478,8 +478,8 @@ size_t tfw_str_to_cstr(const TfwStr *str, char *out_buf, int buf_size);
 TfwStr tfw_str_next_str_val(const TfwStr *str);
 u32 tfw_str_crc32_calc(const TfwStr *str);
 
-#ifdef DEBUG
 void tfw_str_dprint(const TfwStr *str, const char *msg);
+#ifdef DEBUG
 void tfw_dbg_vprint32(const char *prefix, const unsigned char *v);
 #endif