Skip to content

Commit cacbf78

Browse files
Do not move body during encode trailers
Trailers should be placed after body, so we should not move body to the next skb, just allocate new one and use it. During work on #2136 we should try to remove/rework this part of code, because it is better to not move body at all. Closes #2391
1 parent 4815fa3 commit cacbf78

File tree

4 files changed

+92
-5
lines changed

4 files changed

+92
-5
lines changed

fw/http2.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,13 @@ tfw_h2_hpack_encode_trailer_headers(TfwHttpResp *resp)
486486
TfwHttpHdrMap *map = resp->mit.map;
487487
TfwHttpHdrTbl *ht = resp->h_tbl;
488488
unsigned int i;
489-
int r;
489+
int r = 0;
490+
491+
/*
492+
* TODO #2136: Remove this flag during reworking
493+
* `tfw_http_msg_expand_from_pool` function.
494+
*/
495+
__set_bit(TFW_HTTP_B_RESP_ENCODE_TRAILERS, resp->flags);
490496

491497
for (i = map->trailer_idx; i < map->count; ++i) {
492498
unsigned short hid = map->index[i].idx;
@@ -499,17 +505,22 @@ tfw_h2_hpack_encode_trailer_headers(TfwHttpResp *resp)
499505
if (WARN_ON_ONCE(!tgt
500506
|| TFW_STR_EMPTY(tgt)
501507
|| TFW_STR_DUP(tgt)))
502-
return -EINVAL;
508+
{
509+
r = -EINVAL;
510+
goto finish;
511+
}
503512

504513
T_DBG3("%s: hid=%hu, d_num=%hu, nchunks=%u\n",
505514
__func__, hid, d_num, ht->tbl[hid].nchunks);
506515

507516
r = tfw_hpack_transform(resp, tgt);
508517
if (unlikely(r))
509-
return r;
518+
goto finish;
510519
}
511520

512-
return 0;
521+
finish:
522+
clear_bit(TFW_HTTP_B_RESP_ENCODE_TRAILERS, resp->flags);
523+
return r;
513524
}
514525

515526
int

fw/http_msg.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1254,7 +1254,15 @@ __tfw_http_msg_expand_from_pool(TfwHttpMsg *hm, const TfwStr *str,
12541254
if (!nskb)
12551255
return -ENOMEM;
12561256

1257-
if (hm->body.len > 0) {
1257+
/*
1258+
* TODO #2136: Remove this flag during reworking
1259+
* this function. Try to process headers and
1260+
* trailers without moving body.
1261+
*/
1262+
if (hm->body.len > 0
1263+
&& !test_bit(TFW_HTTP_B_RESP_ENCODE_TRAILERS,
1264+
resp->flags))
1265+
{
12581266
r = __tfw_http_msg_move_body(resp, nskb,
12591267
&frag);
12601268
if (unlikely(r < 0)) {

fw/http_types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ enum {
151151
/* This is 100-continue response. */
152152
TFW_HTTP_B_CONTINUE_RESP,
153153

154+
/* This response is during trailers encoding. */
155+
TFW_HTTP_B_RESP_ENCODE_TRAILERS,
156+
154157
_TFW_HTTP_FLAGS_NUM
155158
};
156159

fw/t/unit/test_http_msg.c

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,75 @@ TEST(http_msg, expand_from_pool_for_headers)
271271
tfw_http_msg_free((TfwHttpMsg *)resp);
272272
}
273273

274+
TEST(http_msg, expand_from_pool_for_trailers)
275+
{
276+
static TfwStr frags[] = {
277+
TFW_STR_STRING("trailers"),
278+
TFW_STR_STRING("headers"),
279+
TFW_STR_STRING("paged_body")
280+
};
281+
TfwStr *trailer = &frags[0], *head = &frags[1], *pgd = &frags[2];
282+
TfwHttpResp *resp = __test_resp_alloc(head, pgd, MAX_SKB_FRAGS - 1);
283+
TfwHttpMsg *msg = (TfwHttpMsg *)resp;
284+
TfwHttpMsgCleanup cleanup = {};
285+
TfwMsgIter *it;
286+
int i;
287+
288+
EXPECT_NOT_NULL(resp);
289+
if (!resp)
290+
return;
291+
292+
it = &resp->iter;
293+
set_bit(TFW_HTTP_B_CHUNKED, resp->flags);
294+
resp->body.data = skb_frag_address(&skb_shinfo(it->skb)->frags[0]);
295+
resp->body_start_data = skb_frag_address(&skb_shinfo(it->skb)->frags[0]);
296+
resp->body_start_skb = it->skb;
297+
resp->body.len = (MAX_SKB_FRAGS - 1) * SLEN("paged_body");
298+
299+
EXPECT_EQ(tfw_http_msg_cutoff_headers(msg, &cleanup), 0);
300+
301+
/* Linear part MUST be moved to paged fragments */
302+
EXPECT_TRUE(!skb_headlen(it->skb));
303+
EXPECT_NULL(cleanup.skb_head);
304+
305+
it->frag = skb_shinfo(it->skb)->nr_frags - 1;
306+
tfw_http_msg_setup_transform_pool(&resp->mit, it, resp->pool);
307+
308+
__set_bit(TFW_HTTP_B_RESP_ENCODE_TRAILERS, resp->flags);
309+
310+
EXPECT_EQ(tfw_http_msg_expand_from_pool(msg, trailer), 0);
311+
EXPECT_EQ(tfw_http_msg_expand_from_pool(msg, trailer), 0);
312+
313+
clear_bit(TFW_HTTP_B_RESP_ENCODE_TRAILERS, resp->flags);
314+
315+
for (i = 0; i < MAX_SKB_FRAGS - 1; i++) {
316+
skb_frag_t *frag = &skb_shinfo(resp->msg.skb_head)->frags[i];
317+
char* addr = skb_frag_address(frag);
318+
unsigned int fragsz = skb_frag_size(frag);
319+
320+
EXPECT_ZERO(memcmp(addr, pgd->data, fragsz));
321+
}
322+
323+
EXPECT_TRUE(resp->msg.skb_head != resp->msg.skb_head->next);
324+
EXPECT_TRUE(resp->msg.skb_head->next->next == resp->msg.skb_head);
325+
EXPECT_EQ(skb_shinfo(resp->msg.skb_head->next)->nr_frags, 1);
326+
327+
{
328+
skb_frag_t *frag = &skb_shinfo(resp->msg.skb_head->next)->frags[0];
329+
char* addr = skb_frag_address(frag);
330+
unsigned int fragsz = skb_frag_size(frag);
331+
332+
EXPECT_ZERO(memcmp(addr, "trailerstrailers", fragsz));
333+
}
334+
335+
tfw_http_msg_free((TfwHttpMsg *)resp);
336+
}
337+
274338
TEST_SUITE(http_msg)
275339
{
276340
TEST_RUN(http_msg, hdr_in_array);
277341
TEST_RUN(http_msg, cutoff_linear_headers_paged_body);
278342
TEST_RUN(http_msg, cutoff_linear_headers_and_linear_body);
279343
TEST_RUN(http_msg, expand_from_pool_for_headers);
344+
TEST_RUN(http_msg, expand_from_pool_for_trailers);
280345
}

0 commit comments

Comments
 (0)