Fix request h2 -> http1.1 conversion

[vankoven]

Main changes:

1. Keep h_tbl always valid since we need to read the table during
   response caching. Keep the unsent skbs within request to avoid huge
   updates of h_tbl
2. Optimize header alteration for h2, use compatibility mode for http1.1
3. Use tfw_msg_write functions to take care of all possible caveats at
   message creation stage. (E.g. headers may be longer than one page)

Known issues:

1. Body and trailer conversion is not tested due to parsing issues:
   body is not appended to the request and WARN_ON is triggered.

[aleksostapenko]

Looks good to me, with minor cleanups.

[aleksostapenko]

Looks like excessive SLEN(S_DLM) counted here for Host header; it seems, that this size has already counted above, in the following code:

h1_hdrs_sz = pit->hdrs_len
		+ (pit->hdrs_cnt - pseudo_num) * (SLEN(S_DLM) + SLEN(S_CRLF))
		- req->mark.len;

[vankoven]

Fixed, please check once more time.

[aleksostapenko]

I mean - from the previous code:

if (host) {
                        h1_hdrs_sz -= ht->tbl[TFW_HTTP_HDR_HOST].len;
                        h1_hdrs_sz -= SLEN(S_H2_AUTH);
                        h1_hdrs_sz += SLEN(S_F_HOST);
                }
                else {
                        h1_hdrs_sz -= SLEN(S_H2_AUTH);
                        h1_hdrs_sz += SLEN(S_F_HOST) + SLEN(S_CRLF);
                }

just to make

if (host) {
                        h1_hdrs_sz -= ht->tbl[TFW_HTTP_HDR_HOST].len;
                        h1_hdrs_sz -= SLEN(S_H2_AUTH);
                        h1_hdrs_sz += SLEN(S_F_HOST) - SLEN(S_DLM);
                }
                else {
                        h1_hdrs_sz -= SLEN(S_H2_AUTH);
                        h1_hdrs_sz += SLEN(S_F_HOST) + SLEN(S_CRLF);
                }

to evict SLEN(S_DLM) length from SLEN(S_F_HOST), since SLEN(S_DLM) for Host header (as well as SLEN(S_CRLF)) is already contained in h1_hdrs_sz in case of Host header existence in source HTTP/2-request.

[vankoven]

Let me clear up the calculations:

• :authority psedo-header is counted in h1_hdrs_sz,
• if host header is present it is counted in h1_hdrs_sz and SLEN(S_DLM) + SLEN(S_CRLF) was added.

So here we need to make some alterations (assuming that the :authority pseudo header exists):

If the host header exists, we replace its value with the :authority ones. So we need to:

• subtract original length of host header and length of S_DLM and S_CRLF added to it;
• we need subtract difference between :authority and host length:

h1_hdrs_sz -= SLEN(S_H2_AUTH);
/* S_F_HOST already contains S_DLM, but we already calculated `S_DLM` for authority header. */
h1_hdrs_sz += SLEN(S_F_HOST) - SLEN(S_DLM);

If host header doesn't exist, the we only need to rename :authority into host.

That's why che calculations are done in current way.

[aleksostapenko]

:authority psedo-header is counted in h1_hdrs_sz

Yes, but in previous version of code the :authority pseudo-header had not been counted in pit->hdrs_cnt (i.e. subtracted from it), so the SLEN(S_DLM) + SLEN(S_CRLF) was not added for it and - that's why my previous comment appeared. But with last correction in 'fix header calculations' commit - it seems the problem is gone.

By the way: the pseudo_num local variable is constant now and used only once, thus maybe it should be better to move its definition to the beginning of the function, or just evict it (and replace with 3 in the place of usage with adding appropriate comment there).

[aleksostapenko]

Looks good to me, with minor comments.

[aleksostapenko]

The verification of HEADERS/CONTINUATION frame sequence is done in tfw_h2_stream_fsm(). The different streams intersection can be verified here (in tfw_h2_frame_type_process()) - e.g. via implementing the active_stream number in TfwH2Ctx structure, and setting/zeroing it for appropriate stream states in tfw_h2_stream_fsm().

[vankoven]

The verification of HEADERS/CONTINUATION frame sequence is done in tfw_h2_stream_fsm()

Yes, no problem there.

The different streams intersection

Yes, that is what I meant. If peer has sent a HEADER frame without HEADERS_END flag, then he can't sent messages on other streams until the headers are closed with CONTINUATION frames followed by HEADERS_END flag.

e.g. via implementing the active_stream number in TfwH2Ctx structure, and setting/zeroing it for appropriate stream states in tfw_h2_stream_fsm().

Seems like it's enough to set some ON_CONTINUATION flag for the whole connection and check that the current stream ID is the same as the lstream_id. Many variants are possible here. I didn't implemented it, but the tfw_h2_frame_type_process() looks like a good candidate to place a TODO note, since all required information is available on this state.

[aleksostapenko]

The problem with lstream_id - is that it will not allow to handle this case for requests (i.e. streams) with HEADERS/CONTINUATION frames after DATA frames.