Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP/2 Framing layer implementation (#309). #1233

Merged
merged 17 commits into from
May 16, 2019
Merged

HTTP/2 Framing layer implementation (#309). #1233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3d77573
HTTP/2 Framing layer implementation (#309).
aleksostapenko Mar 28, 2019
10be35a
Minor layout correction (#309).
aleksostapenko Mar 28, 2019
7139f53
Correction of HTTP/2 switching (#309).
aleksostapenko Mar 29, 2019
26cc780
Minor correction in unit tests (#309).
aleksostapenko Mar 29, 2019
fe57412
HTTP/2 Starting/Switching implementation (#309).
aleksostapenko Apr 10, 2019
ed79830
Remove forgotten debug code (#309).
aleksostapenko Apr 10, 2019
f6e8edc
Implementation of base functionality for HTTP/2 Stream Scheduler (#309).
aleksostapenko Apr 25, 2019
79fcd17
Corrections according review comments (#309).
aleksostapenko Apr 28, 2019
05bd194
Add processing for zero length of application frames payload (#309).
aleksostapenko Apr 29, 2019
23d53db
Corrections according review comments (#309).
aleksostapenko Apr 29, 2019
5b93ee8
Implementation of HTTP/2 Stream FSM (#309).
aleksostapenko May 7, 2019
1e8be28
Split header and payload processing for service frames (#309).
aleksostapenko May 7, 2019
1d962dd
Add support for maximum window of Flow Control (#309).
aleksostapenko May 13, 2019
b22c1f7
Corrections according review comments (#309).
aleksostapenko May 15, 2019
5515e82
Corrections according review comments (#309).
aleksostapenko May 16, 2019
ad183a6
Add description in Tempesta FW configuration for HTTP/2 (#309).
aleksostapenko May 16, 2019
dc1f523
Make HTTP/1.1 protocol default (#309).
aleksostapenko May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ DBG_HTTP_PARSER ?= 0
DBG_SS ?= 0
DBG_TLS ?= 0
DBG_APM ?= 0
DBG_HTTP_FRAME ?= 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet there will be families of debug functions for HTTP2: for frames, HPACK, and what ever else. The whole group will have the same meaning to HTTP2 as DBG_HTTP_PARSER for HTTP1.1. Should we rename it as DBG_HTTP2_FRAME?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I cannot do such separation: these sets do not intersect yet; also both HTTP/1.1 and HTTP/2 protocols belong to the same HTTP domain, so there is no contradiction here. But in the nearest future (e.g. during HTTP parser implementation changing in context of HTTP/2 processing) mentioned intersections may appear, and in this case the separation for HTTP1_* and HTTP2_* may be really needed and will be made for preprocessor variables (and possibly for filenames too, e.g. http_frame.c/h, http_stream.c/h).

DBG_HTTP_STREAM ?= 0
TFW_CFLAGS += -DDBG_CFG=$(DBG_CFG) -DDBG_HTTP_PARSER=$(DBG_HTTP_PARSER)
TFW_CFLAGS += -DDBG_SS=$(DBG_SS) -DDBG_TLS=$(DBG_TLS) -DDBG_APM=$(DBG_APM)
TFW_CFLAGS += -DDBG_HTTP_FRAME=$(DBG_HTTP_FRAME)
TFW_CFLAGS += -DDBG_HTTP_STREAM=$(DBG_HTTP_STREAM)

PROC = $(shell cat /proc/cpuinfo)
ARCH = $(shell uname -m)
Expand Down
15 changes: 11 additions & 4 deletions etc/tempesta_fw.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@
# Tempesta FW listening address.
#
# Syntax:
# listen PORT | IPADDR[:PORT] [proto=http|https]
# listen PORT | IPADDR[:PORT] [proto=http|https[:h2|http/1.1]]
#
# IPADDR may be either an IPv4 or IPv6 address, no host names allowed.
# IPv6 address must be enclosed in square brackets (e.g. "[::0]" but not "::0").
Expand All @@ -427,13 +427,20 @@
# If only IPADDR is given, then the default HTTP port 80 is used.
#
# It is allowed to specify the type of listening socket via the 'proto'. At
# the moment HTTP and HTTPS protos are supported. If no 'proto' option was
# given, then HTTP is supposed by the default.
# the moment HTTP and HTTPS protos are supported. For HTTPS proto (TLS actually)
# it is allowed the application level protocol (in context of ALPN extension of
# TLS) to be specified: HTTP/2 or HTTP/1.1 with RFC compliant names 'h2' and
# 'http/1.1' respectively. It is also possible to specify both protocols
# separated by comma; in this case, the protocols' order is significant for
# TLS ALPN: first specified protocol takes precedence over the last one in
# negotiation procedure. If no application level protocol is given, then
# HTTP/1.1 is default. If the entire 'proto' option is absent, then plain
# HTTP protocol (over TCP) is supposed by the default.
#
# Tempesta FW opens one socket for each 'listen' entry, so it may be repeated
# to listen on multiple addresses/ports. For example:
# listen 80;
# listen 443 proto=https;
# listen 443 proto=https:h2,http/1.1;
# listen [::0]:80;
# listen 127.0.0.1:8001;
# listen [::1]:8001;
Expand Down
14 changes: 13 additions & 1 deletion tempesta_fw/connection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "http_parser.h"

#include "sync_socket.h"
#include "http_frame.h"
#include "tls.h"

/*
Expand Down Expand Up @@ -110,6 +111,7 @@ typedef struct {

#define TFW_CONN_TYPE(c) ((c)->proto.type)
#define TFW_CONN_PROTO(c) TFW_CONN_TYPE2IDX(TFW_CONN_TYPE(c))
#define TFW_CONN_TLS(c) (TFW_CONN_TYPE(c) & TFW_FSM_HTTPS)

/*
* Queues in client and server connections provide support for correct
Expand Down Expand Up @@ -211,7 +213,17 @@ typedef struct {
TlsCtx tls;
} TfwTlsConn;

#define tfw_tls_context(conn) (TlsCtx *)(&((TfwTlsConn *)conn)->tls)
#define tfw_tls_context(conn) ((TlsCtx *)(&((TfwTlsConn *)conn)->tls))

/**
* HTTP/2 connection.
*/
typedef struct {
TfwTlsConn tls_conn;
TfwH2Ctx h2;
} TfwH2Conn;

#define tfw_h2_context(conn) ((TfwH2Ctx *)(&((TfwH2Conn *)conn)->h2))

/* Callbacks used by l5-l7 protocols to operate on connection level. */
typedef struct {
Expand Down
9 changes: 6 additions & 3 deletions tempesta_fw/http.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "http_limits.h"
#include "http_tbl.h"
#include "http_parser.h"
#include "http_frame.h"
#include "client.h"
#include "http_msg.h"
#include "http_sess.h"
Expand Down Expand Up @@ -3884,8 +3885,8 @@ tfw_http_resp_process(TfwConn *conn, const TfwFsmData *data)
/**
* @return status (application logic decision) of the message processing.
*/
static int
__tfw_http_msg_process(void *conn, TfwFsmData *data)
int
tfw_http_msg_process_generic(void *conn, TfwFsmData *data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad naming. It's not generic. It's HTTP1 processing.

Copy link
Contributor Author

@aleksostapenko aleksostapenko Apr 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the nearest future PRs (in context of #309) this function is planned for usage for HTTP/2 processing too, so I have run a little forward here. In my opinion, most part of tfw_http_msg_process_generic() is general processing, whatever HTTP/1.1 or HTTP/2 protocol (except the parsing stage only), so it is worth to try adapt it for both protocols. In any case if this idea will fail, and the separate procedures will be needed - the name will be reverted accordingly.

{
TfwConn *c = (TfwConn *)conn;

Expand Down Expand Up @@ -3930,7 +3931,9 @@ tfw_http_msg_process(void *conn, TfwFsmData *data)
if (likely(r == T_OK || r == T_POSTPONE)) {
data->skb->next = data->skb->prev = NULL;
data->trail = !next ? trail : 0;
r = __tfw_http_msg_process(conn, data);
r = TFW_CONN_TLS((TfwConn *)conn) && TFW_CONN_H2(conn)
? tfw_h2_frame_process(conn, data)
: tfw_http_msg_process_generic(conn, data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #309 we discussed that we should split HTTP parser in at least 2 pieces: headers and body and both the parsers will be called separately from H2 layer. How do you see the split in current code? Check H2 state in tfw_http_req_process()?

Frankly, now we have single-jump cost in our parser to enter any state and I believe we're able to make GCC, probably with some assembly help, to generate proper FSM code layout to make the large parser code faster (after all, even after the parser split, we'll have complex FSM due to many checks), so probably we'll be fine with current code if it simpler than possible split parser.

Copy link
Contributor Author

@aleksostapenko aleksostapenko May 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check H2 state in tfw_http_req_process()?

Yeah - this must be a frame type and, maybe, a stream state (since we have to track request's start/end, and this exactly matches with stream's states, as mentioned in penultimate paragraph in https://tools.ietf.org/html/rfc7540#section-8.1).

Сurrently I'm not very optimistic about parser division into separate parts: indeed, we have rather effective implementation with single-jump cost to enter any state; and with divided parser we will get costs of additional function calls (if not inlined) and necessity for additional branching to call separate parser parts (with associated costs). But in general, I am still thinking about this point.

} else {
kfree(data->skb);
}
Expand Down
1 change: 1 addition & 0 deletions tempesta_fw/http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ typedef void (*tfw_http_cache_cb_t)(TfwHttpMsg *);

/* External HTTP functions. */
int tfw_http_msg_process(void *conn, TfwFsmData *data);
int tfw_http_msg_process_generic(void *conn, TfwFsmData *data);
unsigned long tfw_http_req_key_calc(TfwHttpReq *req);
void tfw_http_req_destruct(void *msg);
void tfw_http_resp_fwd(TfwHttpResp *resp);
Expand Down
Loading