Skip to content

Commit db78e5c

Browse files
EvgeniiMekhanikEvgeniiMekhanik
committed
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.
1 parent af8e456 commit db78e5c

File tree

2 files changed

+88
-59
lines changed

2 files changed

+88
-59
lines changed

‎fw/hpack.c

Lines changed: 46 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3021,19 +3021,17 @@ tfw_hpack_rbuf_commit(TfwHPackETbl *__restrict tbl,
30213021
* headers will be evicted from the index table.
30223022
*/
30233023
static int
3024-
tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
3025-
TfwHPackNodeIter *__restrict place, bool spcolon)
3024+
tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict h_name,
3025+
TfwStr *__restrict h_val, TfwHPackNodeIter *__restrict place)
30263026
{
30273027
char *ptr;
30283028
unsigned long node_size, hdr_len;
30293029
unsigned short new_size, node_len;
30303030
unsigned short cur_size = tbl->size, window = tbl->window;
30313031
TfwHPackNode *del_list[HPACK_MAX_ENC_EVICTION] = {};
3032-
TfwStr s_nm = {}, s_val = {};
30333032
TfwHPackETblIter it = {};
30343033

3035-
hdr_len = tfw_http_hdr_split(hdr, &s_nm, &s_val, spcolon);
3036-
WARN_ON_ONCE(TFW_STR_EMPTY(&s_nm));
3034+
hdr_len = h_name->len + h_val->len;
30373035

30383036
WARN_ON_ONCE(cur_size > window || window > HPACK_ENC_TABLE_MAX_SIZE);
30393037
if ((node_size = hdr_len + HPACK_ENTRY_OVERHEAD) > window) {
@@ -3126,14 +3124,14 @@ tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
31263124
it.size += node_size;
31273125
it.rb_len += node_len;
31283126
it.last->hdr_len = hdr_len;
3129-
it.last->name_len = s_nm.len;
3127+
it.last->name_len = h_name->len;
31303128
it.last->rindex = ++tbl->idx_acc;
31313129

3132-
tfw_hpack_rbuf_commit(tbl, &s_nm, &s_val, del_list, place, &it);
3130+
tfw_hpack_rbuf_commit(tbl, h_name, h_val, del_list, place, &it);
31333131

3134-
ptr = tfw_hpack_write(&s_nm, it.last->hdr);
3135-
if (!TFW_STR_EMPTY(&s_val))
3136-
tfw_hpack_write(&s_val, ptr);
3132+
ptr = tfw_hpack_write(h_name, it.last->hdr);
3133+
if (!TFW_STR_EMPTY(h_val))
3134+
tfw_hpack_write(h_val, ptr);
31373135

31383136
WARN_ON_ONCE(tbl->rb_len > tbl->size);
31393137

@@ -3145,36 +3143,28 @@ tfw_hpack_add_node(TfwHPackETbl *__restrict tbl, TfwStr *__restrict hdr,
31453143
* encoder dynamic table with potentially concurrent access from different
31463144
* threads, so lock is used to protect the find/add/erase operations inside
31473145
* this procedure.
3148-
*
3149-
* TODO #1411: get rid of tfw_http_hdr_split and related safety checks
31503146
*/
31513147
static TfwHPackETblRes
31523148
tfw_hpack_encoder_index(TfwHPackETbl *__restrict tbl,
3153-
TfwStr *__restrict hdr,
3149+
TfwStr *__restrict h_name,
3150+
TfwStr *__restrict h_val,
31543151
unsigned short *__restrict out_index,
3155-
unsigned long *__restrict flags,
3156-
bool spcolon)
3152+
unsigned long *__restrict flags)
31573153
{
31583154
TfwHPackNodeIter place = {};
31593155
const TfwHPackNode *node = NULL;
31603156
TfwHPackETblRes res = HPACK_IDX_ST_NOT_FOUND;
3161-
TfwStr h_name = {}, h_val = {};
31623157

31633158
BUILD_BUG_ON(HPACK_IDX_ST_MASK < _HPACK_IDX_ST_NUM - 1);
3164-
if (WARN_ON_ONCE(!hdr))
3165-
return -EINVAL;
31663159

3167-
tfw_http_hdr_split(hdr, &h_name, &h_val, spcolon);
3168-
if (WARN_ON_ONCE(TFW_STR_EMPTY(&h_name)))
3169-
return -EINVAL;
3170-
res = tfw_hpack_rbtree_find(tbl, &h_name, &h_val, &node, &place);
3160+
res = tfw_hpack_rbtree_find(tbl, h_name, h_val, &node, &place);
31713161

31723162
WARN_ON_ONCE(!node && res != HPACK_IDX_ST_NOT_FOUND);
31733163

31743164
*out_index = HPACK_NODE_GET_INDEX(tbl, node);
31753165

31763166
if(res != HPACK_IDX_ST_FOUND
3177-
&& !tfw_hpack_add_node(tbl, hdr, &place, spcolon))
3167+
&& !tfw_hpack_add_node(tbl, h_name, h_val, &place))
31783168
res |= HPACK_IDX_FLAG_ADD;
31793169

31803170
return res;
@@ -3419,31 +3409,23 @@ tfw_hpack_write_idx(TfwHttpResp *__restrict resp, TfwHPackInt *__restrict idx,
34193409
* response @resp using @resp->pool.
34203410
*/
34213411
static int
3422-
tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
3423-
TfwHPackInt *__restrict idx, bool name_indexed, bool trans)
3412+
tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict h_name,
3413+
TfwStr *__restrict h_val, TfwHPackInt *__restrict idx,
3414+
bool name_indexed, bool trans)
34243415
{
34253416
int r;
34263417
TfwHPackInt vlen;
3427-
TfwStr s_name = {}, s_val = {}, s_vlen = {};
3428-
3429-
if (!hdr)
3430-
return -EINVAL;
3418+
TfwStr s_vlen = {};
34313419

34323420
r = tfw_hpack_write_idx(resp, idx, true);
34333421
if (unlikely(r))
34343422
return r;
34353423

3436-
if (WARN_ON_ONCE(TFW_STR_PLAIN(hdr) || TFW_STR_DUP(hdr)))
3437-
return -EINVAL;
3438-
3439-
if (!tfw_http_hdr_split(hdr, &s_name, &s_val, trans))
3440-
return -EINVAL;
3441-
34423424
if (unlikely(!name_indexed)) {
34433425
TfwHPackInt nlen;
34443426
TfwStr s_nlen = {};
34453427

3446-
write_int(s_name.len, 0x7F, 0, &nlen);
3428+
write_int(h_name->len, 0x7F, 0, &nlen);
34473429
s_nlen.data = nlen.buf;
34483430
s_nlen.len = nlen.sz;
34493431

@@ -3452,23 +3434,23 @@ tfw_hpack_hdr_add(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
34523434
return r;
34533435

34543436
if (trans)
3455-
r = tfw_http_msg_expand_from_pool_lc(resp, &s_name);
3437+
r = tfw_http_msg_expand_from_pool_lc(resp, h_name);
34563438
else
3457-
r = tfw_http_msg_expand_from_pool(resp, &s_name);
3439+
r = tfw_http_msg_expand_from_pool(resp, h_name);
34583440
if (unlikely(r))
34593441
return r;
34603442
}
34613443

3462-
write_int(s_val.len, 0x7F, 0, &vlen);
3444+
write_int(h_val->len, 0x7F, 0, &vlen);
34633445
s_vlen.data = vlen.buf;
34643446
s_vlen.len = vlen.sz;
34653447

34663448
r = tfw_http_msg_expand_from_pool(resp, &s_vlen);
3467-
34683449
if (unlikely(r))
34693450
return r;
3470-
if (!TFW_STR_EMPTY(&s_val))
3471-
r = tfw_http_msg_expand_from_pool(resp, &s_val);
3451+
3452+
if (!TFW_STR_EMPTY(h_val))
3453+
r = tfw_http_msg_expand_from_pool(resp, h_val);
34723454

34733455
return r;
34743456
}
@@ -3560,6 +3542,15 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
35603542
TfwHPackETbl *tbl = &ctx->hpack.enc_tbl;
35613543
int r = HPACK_IDX_ST_NOT_FOUND;
35623544
bool name_indexed = true;
3545+
TfwStr h_name = {}, h_val = {};
3546+
3547+
#define __HDR_SPLIT(hdr) \
3548+
do { \
3549+
if (unlikely(TFW_STR_PLAIN(hdr) || TFW_STR_DUP(hdr) \
3550+
|| !tfw_http_hdr_split(hdr, &h_name, &h_val, trans) \
3551+
|| TFW_STR_EMPTY(&h_name))) \
3552+
return -EINVAL; \
3553+
} while(0)
35633554

35643555
if (WARN_ON_ONCE(!hdr || TFW_STR_EMPTY(hdr)))
35653556
return -EINVAL;
@@ -3572,9 +3563,10 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
35723563
T_DBG_PRINT_HPACK_RBTREE(tbl);
35733564

35743565
if (!st_full_index && dyn_indexing) {
3566+
__HDR_SPLIT(hdr);
35753567
assert_spin_locked(&conn->sk->sk_lock.slock);
3576-
r = tfw_hpack_encoder_index(tbl, hdr, &index, resp->flags,
3577-
trans);
3568+
r = tfw_hpack_encoder_index(tbl, &h_name, &h_val,
3569+
&index, resp->flags);
35783570
if (r < 0)
35793571
return r;
35803572
}
@@ -3626,11 +3618,18 @@ __tfw_hpack_encode(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
36263618
name_indexed = false;
36273619

36283620
encode:
3629-
if (use_pool)
3630-
r = tfw_hpack_hdr_add(resp, hdr, &idx, name_indexed, trans);
3631-
else
3621+
if (use_pool) {
3622+
if (!dyn_indexing)
3623+
__HDR_SPLIT(hdr);
3624+
r = tfw_hpack_hdr_add(resp, &h_name, &h_val, &idx,
3625+
name_indexed, trans);
3626+
} else {
36323627
r = tfw_hpack_hdr_expand(resp, hdr, &idx, name_indexed);
3628+
}
3629+
36333630
return r;
3631+
3632+
#undef __HDR_SPLIT
36343633
}
36353634

36363635
int

‎fw/t/unit/test_hpack.c

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,6 +1495,9 @@ TEST(hpack, enc_table_index)
14951495
TfwHPackNodeIter pl = {};
14961496
const TfwHPackNode *node = NULL;
14971497
unsigned short index = 0;
1498+
TfwStr s1_name = {}, s1_val = {};
1499+
TfwStr s2_name = {}, s2_val = {};
1500+
TfwStr s3_name = {}, s3_val = {};
14981501

14991502
#define HDR_NAME_1 "test-custom-header-name"
15001503
#define HDR_VALUE_1 "foo test example value"
@@ -1535,6 +1538,10 @@ TEST(hpack, enc_table_index)
15351538
collect_compound_str(s3, s3_value, 0);
15361539
collect_compound_str(s3, s3_rws, TFW_STR_OWS);
15371540

1541+
tfw_http_hdr_split(s1, &s1_name, &s1_val, true);
1542+
tfw_http_hdr_split(s2, &s2_name, &s2_val, true);
1543+
tfw_http_hdr_split(s3, &s3_name, &s3_val, true);
1544+
15381545
tbl = &ctx.hpack.enc_tbl;
15391546

15401547
/*
@@ -1544,7 +1551,7 @@ TEST(hpack, enc_table_index)
15441551
res = hpack_rbtree_find(tbl, s1, &node, &pl);
15451552
EXPECT_EQ(res, HPACK_IDX_ST_NOT_FOUND);
15461553
EXPECT_NULL(node);
1547-
EXPECT_OK(tfw_hpack_add_node(tbl, s1, &pl, true));
1554+
EXPECT_OK(tfw_hpack_add_node(tbl, &s1_name, &s1_val, &pl));
15481555

15491556
node = NULL;
15501557
bzero_fast(&pl, sizeof(pl));
@@ -1553,7 +1560,7 @@ TEST(hpack, enc_table_index)
15531560
EXPECT_NULL(node);
15541561
EXPECT_NOT_NULL(pl.parent);
15551562
if (pl.parent)
1556-
EXPECT_OK(tfw_hpack_add_node(tbl, s2, &pl, true));
1563+
EXPECT_OK(tfw_hpack_add_node(tbl, &s2_name, &s2_val, &pl));
15571564

15581565
node = NULL;
15591566
bzero_fast(&pl, sizeof(pl));
@@ -1562,7 +1569,7 @@ TEST(hpack, enc_table_index)
15621569
EXPECT_NULL(node);
15631570
EXPECT_NOT_NULL(pl.parent);
15641571
if (pl.parent)
1565-
EXPECT_OK(tfw_hpack_add_node(tbl, s3, &pl, true));
1572+
EXPECT_OK(tfw_hpack_add_node(tbl, &s3_name, &s3_val, &pl));
15661573

15671574
/*
15681575
* Verify that headers had been correctly added into encoder dynamic
@@ -1623,6 +1630,11 @@ TEST(hpack, enc_table_rbtree)
16231630
const TfwHPackNode *node = NULL;
16241631
const TfwHPackNode *n1 = NULL, *n2 = NULL, *n3 = NULL;
16251632
const TfwHPackNode *n4 = NULL, *n5 = NULL;
1633+
TfwStr s1_name = {}, s1_val = {};
1634+
TfwStr s2_name = {}, s2_val = {};
1635+
TfwStr s3_name = {}, s3_val = {};
1636+
TfwStr s4_name = {}, s4_val = {};
1637+
TfwStr s5_name = {}, s5_val = {};
16261638

16271639
#define HDR_NAME_1 "test-custom-name"
16281640
#define HDR_VALUE_1 "test-custom-value"
@@ -1658,6 +1670,12 @@ TEST(hpack, enc_table_rbtree)
16581670
collect_compound_str(s5, col, 0);
16591671
collect_compound_str(s5, s5_value, 0);
16601672

1673+
tfw_http_hdr_split(s1, &s1_name, &s1_val, true);
1674+
tfw_http_hdr_split(s2, &s2_name, &s2_val, true);
1675+
tfw_http_hdr_split(s3, &s3_name, &s3_val, true);
1676+
tfw_http_hdr_split(s4, &s4_name, &s4_val, true);
1677+
tfw_http_hdr_split(s5, &s5_name, &s5_val, true);
1678+
16611679
tbl = &ctx.hpack.enc_tbl;
16621680

16631681
/*
@@ -1681,7 +1699,7 @@ TEST(hpack, enc_table_rbtree)
16811699
res = hpack_rbtree_find(tbl, s1, &node, &pl);
16821700
EXPECT_EQ(res, HPACK_IDX_ST_NOT_FOUND);
16831701
EXPECT_NULL(node);
1684-
EXPECT_OK(tfw_hpack_add_node(tbl, s1, &pl, true));
1702+
EXPECT_OK(tfw_hpack_add_node(tbl, &s1_name, &s1_val, &pl));
16851703
rbt_validate(tbl);
16861704
bzero_fast(&pl, sizeof(pl));
16871705
res = hpack_rbtree_find(tbl, s1, &n1, &pl);
@@ -1701,7 +1719,7 @@ TEST(hpack, enc_table_rbtree)
17011719
EXPECT_NULL(node);
17021720
EXPECT_NOT_NULL(pl.parent);
17031721
if (pl.parent) {
1704-
EXPECT_OK(tfw_hpack_add_node(tbl, s2, &pl, true));
1722+
EXPECT_OK(tfw_hpack_add_node(tbl, &s2_name, &s2_val, &pl));
17051723
rbt_validate(tbl);
17061724
}
17071725
bzero_fast(&pl, sizeof(pl));
@@ -1717,7 +1735,7 @@ TEST(hpack, enc_table_rbtree)
17171735
EXPECT_NULL(node);
17181736
EXPECT_NOT_NULL(pl.parent);
17191737
if (pl.parent) {
1720-
EXPECT_OK(tfw_hpack_add_node(tbl, s3, &pl, true));
1738+
EXPECT_OK(tfw_hpack_add_node(tbl, &s3_name, &s3_val, &pl));
17211739
rbt_validate(tbl);
17221740
}
17231741
bzero_fast(&pl, sizeof(pl));
@@ -1766,7 +1784,7 @@ TEST(hpack, enc_table_rbtree)
17661784
EXPECT_NULL(node);
17671785
EXPECT_NOT_NULL(pl.parent);
17681786
if (pl.parent) {
1769-
EXPECT_OK(tfw_hpack_add_node(tbl, s4, &pl, true));
1787+
EXPECT_OK(tfw_hpack_add_node(tbl, &s4_name, &s4_val, &pl));
17701788
rbt_validate(tbl);
17711789
}
17721790
bzero_fast(&pl, sizeof(pl));
@@ -1812,7 +1830,7 @@ TEST(hpack, enc_table_rbtree)
18121830
EXPECT_NULL(node);
18131831
EXPECT_NOT_NULL(pl.parent);
18141832
if (pl.parent) {
1815-
EXPECT_OK(tfw_hpack_add_node(tbl, s5, &pl, true));
1833+
EXPECT_OK(tfw_hpack_add_node(tbl, &s5_name, &s5_val, &pl));
18161834
rbt_validate(tbl);
18171835
}
18181836
bzero_fast(&pl, sizeof(pl));
@@ -1956,6 +1974,8 @@ TEST(hpack, enc_table_eviction)
19561974
char long_val[255] = "qwertyuiopqwertyuiopqwertyuiopqwertyuiopqwerty"
19571975
"uiopqwertyuiopqwertyuiop";
19581976
const TfwHPackNode *node = NULL;
1977+
TfwStr f_name = {}, f_val = {};
1978+
TfwStr l_name = {}, l_val = {};
19591979

19601980
TFW_STR(col, ":");
19611981
TFW_STR(f_hdr_val, "first header");
@@ -1969,12 +1989,16 @@ TEST(hpack, enc_table_eviction)
19691989
res = hpack_rbtree_find(tbl, f_hdr, &node, &pl);
19701990
EXPECT_EQ(res, HPACK_IDX_ST_NOT_FOUND);
19711991
EXPECT_NULL(node);
1972-
EXPECT_OK(tfw_hpack_add_node(tbl, f_hdr, &pl, true));
1992+
1993+
tfw_http_hdr_split(f_hdr, &f_name, &f_val, true);
1994+
1995+
EXPECT_OK(tfw_hpack_add_node(tbl, &f_name, &f_val, &pl));
19731996
bzero_fast(&pl, sizeof(pl));
19741997

19751998
for (;;) {
19761999
unsigned long node_size;
19772000
unsigned long new_size;
2001+
TfwStr fil_name = {}, fil_val = {};
19782002

19792003
sprintf(hdr, "Header%i", i);
19802004

@@ -1990,10 +2014,12 @@ TEST(hpack, enc_table_eviction)
19902014
if (new_size > tbl->window)
19912015
break;
19922016

2017+
tfw_http_hdr_split(filler, &fil_name, &fil_val, true);
2018+
19932019
res = hpack_rbtree_find(tbl, filler, &node, &pl);
19942020
EXPECT_GT(res, HPACK_IDX_ST_FOUND);
19952021
EXPECT_NULL(node);
1996-
EXPECT_OK(tfw_hpack_add_node(tbl, filler, &pl, true));
2022+
EXPECT_OK(tfw_hpack_add_node(tbl, &fil_name, &fil_val, &pl));
19972023
bzero_fast(&pl, sizeof(pl));
19982024
bzero_fast(hdr, sizeof(hdr));
19992025
i++;
@@ -2008,10 +2034,12 @@ TEST(hpack, enc_table_eviction)
20082034
collect_compound_str(l_hdr, col, 0);
20092035
collect_compound_str(l_hdr, l_hdr_val, 0);
20102036

2037+
tfw_http_hdr_split(l_hdr, &l_name, &l_val, true);
2038+
20112039
res = hpack_rbtree_find(tbl, l_hdr, &node, &pl);
20122040
EXPECT_EQ(res, HPACK_IDX_ST_NOT_FOUND);
20132041
EXPECT_NULL(node);
2014-
EXPECT_OK(tfw_hpack_add_node(tbl, l_hdr, &pl, true));
2042+
EXPECT_OK(tfw_hpack_add_node(tbl, &l_name, &l_val, &pl));
20152043
bzero_fast(&pl, sizeof(pl));
20162044

20172045
/* first header must not present. */
@@ -2021,10 +2049,12 @@ TEST(hpack, enc_table_eviction)
20212049

20222050
#define ADD_NODE(s, n) \
20232051
do { \
2052+
TfwStr s_name = {}, s_val = {}; \
2053+
tfw_http_hdr_split(s, &s_name, &s_val, true); \
20242054
res = hpack_rbtree_find(tbl, s, &n, &pl); \
20252055
EXPECT_EQ(res, HPACK_IDX_ST_NOT_FOUND); \
20262056
EXPECT_NULL(n); \
2027-
EXPECT_OK(tfw_hpack_add_node(tbl, s, &pl, true)); \
2057+
EXPECT_OK(tfw_hpack_add_node(tbl, &s_name, &s_val, &pl)); \
20282058
bzero_fast(&pl, sizeof(pl)); \
20292059
res = hpack_rbtree_find(tbl, s, &n, &pl); \
20302060
EXPECT_EQ(res, HPACK_IDX_ST_FOUND); \

0 commit comments

Comments
 (0)