Flow contol and pull-model for message forwarding #1394

vankoven · 2020-03-25T10:00:02Z

Scope

Need to cover HTTP/2 flow control requirements from RFC 9113 Section 5.2.

When connecting, client sends initial window size and can control receive window size using WINDOW_UPDATE frame. This shouldn't be confused for TCP flow control, since HTTP/2 implements it's own one. The flow control in HTTP/2 can be applied at connection and stream level. Only for DATA frames are subject for flow control.

Great discussion about implementation details took place in #1378 issue. But HLD presented there is not a reasonable solution. The proposed solution was to extend current "push" model for sending messages which lead to TCP head-of-line blocking, reverse prioritisation issues and bufferbloats. The streams ready to send will stored inside TfwConnection and theirs skb will be pushed into TCP send queue. Currently we aggressively push messages into TCP send queues and can stress connections even harder than userspace applications has, thanks to direct socket access.

Instead we should maintain a set of flows which has data ready to send, and chose and frame data from that set according to current congestion window, and http/2 flow control window. So we avoid to use send queue and directly pull data from TfwConnection in tcp_write_xmit(). This is called pull-model.

During pull we can prioritise streams with O(1) complexity, so this task is required for #1196

Useful resources: #391 https://www.slideshare.net/kazuho/reorganizing-website-architecture-for-http2-and-beyond

Testing

Check that Tempesta:

Doesn't send more than allowed by per-connection flow control (https://httpwg.org/specs/rfc9113.html#n-the-flow-control-window)
~~Doesn't send more than allowed by per-stream flow control~~ Test to reproduce or h2spec: http2/6.9.1
Doesn't block connection and continue to forward Headers frames while flow control limit is exhausted
Doesn't truncate and suspend any frames except DATA ones on reaching close to flow control limit.
Follows WINDOW_UPDATE frames.
correctly process the case of negative flow-control window https://httpwg.org/specs/rfc9113.html#InitialWindowSize
curl with CURLPIPE_MULTIPLEX should correctly fetch many resources

The text was updated successfully, but these errors were encountered:

krizhanovsky · 2020-03-28T22:36:51Z

References #488 in HTTP/2 flow control. #498 also discusses HTTP/2 flow control.

Also read (from https://h3.edm.uhasselt.be/):

At some point we'll have QUIC and HTTP/3, which share some mechanisms with HTTP/2, so it's good to have #724 in mind during the implementation to probably reuse some code for HTTP/3

krizhanovsky · 2022-06-23T19:04:42Z

from #1641 (probably we should use this as a test case) . See https://httpwg.org/specs/rfc9113.html#n-the-flow-control-window

FLOW_CONTROL_ERROR appears when client sends multiple requests in one connection.

It can be reproduced in Chromium and nghttp2.
nghttp2: To reproduce this issue we need to make html file with links to something in it and run the nghttp2 with -a argument.
Run: nghttp -anvy https://debian/file.html

file.html example:

<!DOCTYPE html>
<html>
<body>
	<h1>TEST PAGE</h1>
	<img src="1.png"/>
	<img src="2.png"/>
	<img src="3.png"/>
	<img src="4.png"/>
</body>
</html>

All images must present. Image 1.png must be largest(above 1MB) image, other images can be smaller(100 KB or less). If all files have same size images loads correctly.

Also same error appear if we use large amount of images in one page. For example we can place into file.html 30 images and nghttp will sent frame GO_AWAY error_code=FLOW_CONTROL_ERROR even if all files are same.

File with large amount of images:

<!DOCTYPE html>
<html>
<body>
	<h1>TEST PAGE</h1>
	<img src="1.png"/>
	<img src="2.png"/>
	<img src="3.png"/>
	<img src="4.png"/>
	<img src="4.png?rnd=0"/>
	<img src="4.png?rnd=1"/>
	<img src="4.png?rnd=2"/>
	<img src="4.png?rnd=3"/>
	<img src="4.png?rnd=4"/>
	<! -- And so on. Better to use large amount of img -->
</body>
</html>

Chromium: In this case we can use mixed approach: file.html must contain many(30+) images with first large image. Request to largest image will be failed.

b3b · 2022-11-02T12:51:45Z

Test to reproduce: t_sites.test_wordpress.TestWordpressSiteH2.test_get_resource_with_assets

krizhanovsky · 2023-02-05T23:00:57Z

This task must also solve #391 point 1 and somewhat #687 in that we must switch to pulling mode. See also slide 18 on https://www.slideshare.net/kazuho/reorganizing-website-architecture-for-http2-and-beyond .

All data should be just sent to a TCP socket with ss_do_send() as now for TLS data, but tfw_tls_encrypt() must upcall the HTTP/2 framing layer that it can make optimal number of frames of optimal size. With this we must get rid of the double TfwStr chunks processing in tfw_h2_make_frames() (discussed in https://github.com/tempesta-tech/tempesta/pull/1418/files#r993920148). Probably the original #391 p1 will still exist for plain HTTP, but it's hard to find plain HTTP nowadays, so we can ignore it for now. Please rework tfw_tls_encrypt() if possible to split it into several functions and make the code simpler (now the function is overcomplicated).

When a client's WINDOW_UPDATE arrives it seems we just can call tcp_push() on the client socket to kick the TCP -> TLS -> HTTP framing calls chain. Probably we should and can make an optimization on the call path to return early if there is no HTTP/2 window to send data (I'd imagine that it could be a flag in sk_user_data to return earlier and not to make many indirect calls... Or maybe there won't be too much calls. I.e. TBD).

Since at the moment Tempesta FW works in buffering mode only, we'll send response data in chunks and we do need to track where is the current pointer for transmission. With #498 we'll need to deal with partially build HTTP responses, so I think we should keep the transmission pointer in TfwHttpResp, ideally if we can reuse of the the existing data in it with union.

It seems TfwStream should be extended by the current flow-control value window (also required for #1810).

Please, during the development keep in mind (maybe write necessary zero-logic functions and/or TODOs)

HTTP message buffering and streaming #498 - we'll need to update server TCP receive window on WINDOW_UPDATE frame updates in proxying mode and increase the window when we send some data to client and TCP acknowledges reception of the data.
execute streams scheduler from HTTP/2 streams prioritization #1196

We should also not to fetch a full response body from the cache can call the TDB routines for the next data chunk also by an upcall from TCP -> TLS -> HTTP framing. It's TBD and maybe subject for #498 or just a point for another pull request.

RomanBelozerov · 2023-02-08T12:40:56Z

Please enable http2_general.test_flow_control_window tests after fixes. And also h2spec tests.

Since we decide to move frame making into `xmit` callback, we need to save stream id in skb private area. In `xmit` callback we use this id to find appropriate stream. Part of #1394

Since we decide to move frame making into `xmit` callback, we need to save stream id in skb private area and mark skb as skb, which contains headers or data frame. In `xmit` callback we use this id to find appropriate stream and use information of skb type to add appropriate frame header (HEADERS/DATA). Part of #1394

Adding of frame header can leades to allocation of a new skb, which header should be filled correctly. For this purposes we copy some static inline functions from `tcp_output.c` to our code. Part of #1394

Previously, when we forward response, we put appropriate stream into `hclosed_streams` queue and if count of such streams become greater then TFW_MAX_CLOSED_STREAMS we delete streams from this queue. But now we should not delete such streams, because they can used in `xmit` callback, that's why we implement additional queue for such streams. Now when we forward response, we put appropriate stream into `pending_hcl_streams` queue and then when all response data will be sent, we move this stream to `hclosed_streams` queue for further deleting. Part of #1394