Reworked HTTP1 -> HTTP2 message transformation #1820

const-t · 2023-02-15T09:54:23Z

This patch introduce another approach to HTTP1 -> HTTP2 message transformation. We had following problems with inplace transformation: Each added header was added to new skb fragment, that caused skb fragmentation. Appending value to existing in message header was limited by size which remains after transformation. We could use mixed approach, encode headers inplace while message has room and then expand skb by new fragments, but maintain both approaches and code complexity not worth it.

Now transformation consist of following steps:

Find skb and fragment contains body or last crlf, if body not exists.
Delete found data from skb. Keep skb.
Hpack encode headers then add to skb from previous step. Headers adds as paged fragments, now we don't use headroom. HTTP2_MAX_OFFSET not used anymore during HTTP1 -> HTTP2 transformation without cache. All paged data in skb allocates from response's TfwPool.
Forward message to client.
Cleanup all data that was deleted from skbs in first step.

TfwPool was patched to statisfy new requirments. Now we use compound pages for order above zero and put_page insted of free_pages, because during free pages might be still used by skbs.

Fixed CONTINUATION frame.

krizhanovsky

I didn't notice any severe issue with the pull request, but there are still some things to do and fix.

Please make all the following changes by new separate patches, do not rewrite the current patches, to simplify next reviews. Later, on the merge time, you can squash senseless patches.

fw/cache.c

fw/http.h

fw/hpack.c

fw/http_msg.c

krizhanovsky · 2023-03-06T11:10:02Z

fw/http_msg.c

+		ss_skb_unlink(&it->skb_head, skb);
+		ss_skb_queue_tail(&cleanup->skb_head, skb);
+
+	} while (it->skb != NULL);


@RomanBelozerov I'm wondering if we have a test, which sends an HTTP response to Tempesta with very small chunks, especially headers, and Tempesta forwards the response to a client via HTTP/2? I believe we have some test for this, but I'm not sure about HTTP/2 client in this scenario...

We have test for small TCP chunks in h2 request. Should I add for responses?

@RomanBelozerov I created tempesta-tech/tempesta-test#431 for this

fw/http.h

fw/http_msg.c

fw/ss_skb.h

EvgeniiMekhanik · 2023-03-06T14:58:12Z

There are several tests which doesn't work on this branch tempesta-tech/tempesta-test@59db00a#diff-46cbe3acf14e83bddf0360bdc9ab952e6673ea0360c67f246da3cbdae49a6eb1R493 (test_big_header_after_setting_header_table_size, test_big_body_after_setting_header_table_size) (very big body or very big header)

EvgeniiMekhanik · 2023-03-06T15:09:58Z

fw/http_msg.c

-		return -ENOENT;
+		T_WARN("Appending to singular header '%.*s'\n",
+		       PR_TFW_STR(TFW_STR_CHUNK(hdr, 0)));
+		return 0;


Why we don't drop response in this case now?

We shouldn't, it is a legit case. Just print a warning.

fw/hpack.c

fw/http.c

fw/http_msg.c

EvgeniiMekhanik · 2023-03-12T02:36:43Z

Please see tempesta-tech/tempesta-test@465eecf i add several tests, which doesn't work. Also test_flow_control_window_with_long_headers_and_body_FOR_1820 crash on this branch

krizhanovsky

LGTM. I created a new issue for a non addressed comment about copying.

Please also update the copyright year in the modified files (e.g. pool.[ch])

fw/ss_skb.h

fw/http_msg.c

s0nx

LGTM

fw/hpack.c

fw/http_msg.c

EvgeniiMekhanik

LGTM, but please squash all fixes in appropriate commits

This patch introduce another approach to HTTP1 -> HTTP2 message transformation. We had following problems with inplace transformation: Each added header was added to new skb fragment, that caused skb fragmentation. Appending value to existing in message header was limited by size which remains after transformation. We could use mixed approach, encode headers inplace while message has room and then expand skb by new fragments, but maintain both approaches and code complexity not worth it. Now transformation consist of following steps: 1. Find skb and fragment contains body or last crlf, if body not exists. 2. Delete found data from skb. Keep skb. 3. Hpack encode headers then add to skb from previous step. Headers adds as paged fragments, now we don't use headroom. HTTP2_MAX_OFFSET not used anymore during HTTP1 -> HTTP2 transformation without cache. All paged data in skb allocates from response's TfwPool. 4. Forward message to client. 5. Cleanup all data that was deleted from skbs in first step. TfwPool was patched to statisfy new requirments. Now we use compound pages for order above zero and `put_page` insted of `free_pages`, because during free pages might be still used by skbs. Fixed CONTINUATION frame.

tfw_http_msg_resp|req_spec_hid expects header name with colon or even without it, however whole header was passed that cause incorrect "hid" finding

duplicated singular header. Warning message changed to human readable. (SQUASH ME)

Current behaviour: -resp_hdr_add adds header only if it doesn't exists in message. Except special non-singular headers e.g set-cookie, forwarded. These headers can be added anyway. Appending to header NOT IMPLEMENTED for http2. -resp_hdr_set adds headers anyway, even if such header already exists in message. Existed header will be removed.

…coding headers. Now we don't need to have many operations such as "expand" or "substitute". We only need to decide to use or not pool(must not be used for cache and local messages) and what format of header we will encode, http1 raw or http2 ready. added @tfw_hpack_transform that used during HTTP1 -> HTTP2 transformation.

everywhere and avoids unneccessary calls tfw_hpack_hdr_add/expand

There was a case when we exhaust all available room in skb and tso_fragment couldn't correctly split such skb. Now max size of skb data is: SS_SKB_MAX_DATA_LEN (SKB_MAX_HEADER + MAX_SKB_FRAGS * PAGE_SIZE)

const-t force-pushed the kt-1103-h2-msg-trasform branch 8 times, most recently from 3e9ec31 to 1b5cfaa Compare February 17, 2023 15:06

const-t force-pushed the kt-1103-h2-msg-trasform branch 5 times, most recently from 6468634 to 461c529 Compare February 23, 2023 15:12

krizhanovsky requested review from krizhanovsky, s0nx and EvgeniiMekhanik February 23, 2023 15:20

const-t force-pushed the kt-1103-h2-msg-trasform branch 2 times, most recently from 026f948 to e653d84 Compare February 24, 2023 10:18

const-t marked this pull request as ready for review February 27, 2023 09:36

const-t force-pushed the kt-1103-h2-msg-trasform branch 2 times, most recently from 8df8606 to f66aedb Compare March 1, 2023 15:19

krizhanovsky requested changes Mar 6, 2023

View reviewed changes

EvgeniiMekhanik reviewed Mar 6, 2023

View reviewed changes

fw/hpack.c Outdated Show resolved Hide resolved

EvgeniiMekhanik reviewed Mar 6, 2023

View reviewed changes

fw/hpack.c Outdated Show resolved Hide resolved

EvgeniiMekhanik reviewed Mar 6, 2023

View reviewed changes

fw/http.c Outdated Show resolved Hide resolved

krizhanovsky mentioned this pull request Mar 6, 2023

Optimizer for HTTP messages adjustment #1103

Open

s0nx suggested changes Mar 10, 2023

View reviewed changes

fw/http_msg.c Show resolved Hide resolved

fw/http_msg.c Show resolved Hide resolved

fw/http_msg.c Show resolved Hide resolved

This was referenced Apr 4, 2023

Send HTTP response in small chunks and forward it to an h2 client tempesta-tech/tempesta-test#431

Closed

Get rid of copying in tfw_h2_resp_adjust_fwd() #1854

Closed

krizhanovsky approved these changes Apr 4, 2023

View reviewed changes

fw/ss_skb.h Outdated Show resolved Hide resolved

fw/http_msg.c Show resolved Hide resolved

fw/http_msg.c Outdated Show resolved Hide resolved

s0nx approved these changes Apr 4, 2023

View reviewed changes

fw/hpack.c Outdated Show resolved Hide resolved

fw/http_msg.c Outdated Show resolved Hide resolved

EvgeniiMekhanik approved these changes Apr 5, 2023

View reviewed changes

const-t force-pushed the kt-1103-h2-msg-trasform branch 2 times, most recently from df99fad to ca26930 Compare April 5, 2023 11:16

const-t added 9 commits April 5, 2023 14:16

__extend_pgfrags: return code changed to more suitable

6992ea6

fixed improper usage of tfw_http_msg_resp|req_spec_hid

6931832

tfw_http_msg_resp|req_spec_hid expects header name with colon or even without it, however whole header was passed that cause incorrect "hid" finding

tfw_http_msg_hdr_xfrm_str: now doesn't drop response on adding

43e042c

duplicated singular header. Warning message changed to human readable. (SQUASH ME)

introduced tfw_hpack_write_idx which used to write index

c1776e7

everywhere and avoids unneccessary calls tfw_hpack_hdr_add/expand

Deleted unsed functions and structures

44f5aca

Added fix for crashing response with large headers

0913e9e

There was a case when we exhaust all available room in skb and tso_fragment couldn't correctly split such skb. Now max size of skb data is: SS_SKB_MAX_DATA_LEN (SKB_MAX_HEADER + MAX_SKB_FRAGS * PAGE_SIZE)

const-t force-pushed the kt-1103-h2-msg-trasform branch from ca26930 to 0913e9e Compare April 5, 2023 11:16

const-t changed the title ~~Reworked HTTP1 -> HTTP2 message transformation.~~ Reworked HTTP1 -> HTTP2 message transformation Apr 5, 2023

s0nx mentioned this pull request Apr 5, 2023

kernel BUG at include/linux/skbuff.h:970! #1855

Closed

const-t closed this Apr 5, 2023

const-t reopened this Apr 5, 2023

Copyrights updated

75012fb

const-t force-pushed the kt-1103-h2-msg-trasform branch from d63d778 to 75012fb Compare April 5, 2023 16:54

krizhanovsky merged commit 5d64876 into master Apr 6, 2023

krizhanovsky deleted the kt-1103-h2-msg-trasform branch April 6, 2023 10:50

krizhanovsky mentioned this pull request Aug 14, 2024

H2 related corrections and optimizations. #1411

Open

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reworked HTTP1 -> HTTP2 message transformation #1820

Reworked HTTP1 -> HTTP2 message transformation #1820

const-t commented Feb 15, 2023 •

edited

Loading

krizhanovsky left a comment

krizhanovsky Mar 6, 2023

RomanBelozerov Mar 7, 2023

krizhanovsky Apr 4, 2023

EvgeniiMekhanik commented Mar 6, 2023

EvgeniiMekhanik Mar 6, 2023

const-t Mar 6, 2023

EvgeniiMekhanik commented Mar 12, 2023

krizhanovsky left a comment

s0nx left a comment

EvgeniiMekhanik left a comment

Reworked HTTP1 -> HTTP2 message transformation #1820

Reworked HTTP1 -> HTTP2 message transformation #1820

Conversation

const-t commented Feb 15, 2023 • edited Loading

krizhanovsky left a comment

Choose a reason for hiding this comment

krizhanovsky Mar 6, 2023

Choose a reason for hiding this comment

RomanBelozerov Mar 7, 2023

Choose a reason for hiding this comment

krizhanovsky Apr 4, 2023

Choose a reason for hiding this comment

EvgeniiMekhanik commented Mar 6, 2023

EvgeniiMekhanik Mar 6, 2023

Choose a reason for hiding this comment

const-t Mar 6, 2023

Choose a reason for hiding this comment

EvgeniiMekhanik commented Mar 12, 2023

krizhanovsky left a comment

Choose a reason for hiding this comment

s0nx left a comment

Choose a reason for hiding this comment

EvgeniiMekhanik left a comment

Choose a reason for hiding this comment

const-t commented Feb 15, 2023 •

edited

Loading