[
https://issues.apache.org/jira/browse/TS-3475?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14396260#comment-14396260
]
Leif Hedstrom edited comment on TS-3475 at 4/5/15 2:45 PM:
-----------------------------------------------------------
[~maskit] I haven't tested this yet, but what do you think of something like
this (patch over the -3 diff above):
{code}
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
loki (08:41) 272/1 $ git diff | cat
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index b852186..a51fd35 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -50,9 +50,6 @@ const size_t HTTP2_GOAWAY_LEN = 8;
const size_t HTTP2_WINDOW_UPDATE_LEN = 4;
const size_t HTTP2_SETTINGS_PARAMETER_LEN = 6;
-// SETTINGS initial values
-const uint32_t HTTP2_ENABLE_PUSH = 0; // Server Push is NOT supported
-
// 6.9.1 The Flow Control Window
static const Http2WindowSize HTTP2_MAX_WINDOW_SIZE = 0x7FFFFFFF;
diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h
index 137bcc9..21f8476 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -43,7 +43,7 @@ static size_t const HTTP2_HEADER_BUFFER_SIZE_INDEX =
CLIENT_CONNECTION_FIRST_REA
// To support Upgrade: h2c
struct Http2UpgradeContext {
- Http2UpgradeContext() { client_settings.init(); }
+ Http2UpgradeContext() { }
~Http2UpgradeContext()
{
diff --git a/proxy/http2/Http2ConnectionState.h
b/proxy/http2/Http2ConnectionState.h
index 791f5e4..d829d45 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -36,13 +36,20 @@ public:
Http2ConnectionSettings()
{
// 6.5.2. Defined SETTINGS Parameters
- // TODO these values should be configurable.
- settings[indexof(HTTP2_SETTINGS_ENABLE_PUSH)] = HTTP2_ENABLE_PUSH;
- init();
+
+ // IMPORTANT: These are the default settings as per the HTTP/2 protocol.
+ // They should *not* be modified, and I make these hardcoded here to avoid
+ // tempting people to later change the defines or constants.
+ settings[indexof(HTTP2_SETTINGS_ENABLE_PUSH)] = 0; // Disabled for now
+ settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] = 100;
+ settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] = 65535;
+ settings[indexof(HTTP2_SETTINGS_MAX_FRAME_SIZE)] = 16384;
+ settings[indexof(HTTP2_SETTINGS_HEADER_TABLE_SIZE)] = 4096;
+ settings[indexof(HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE)] = UINT_MAX;
}
void
- init()
+ settings_from_configs()
{
settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] =
Http2::max_concurrent_streams;
settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] =
Http2::initial_window_size;
@@ -206,7 +213,7 @@ public:
continued_buffer.iov_base = NULL;
continued_buffer.iov_len = 0;
- server_settings.init();
+ server_settings.settings_from_configs();
}
void
{code}
As you can see, I opted for not relying on the allocator template object to
retain the configs. It feels fragile, in that someone might change the (server)
defaults in RecordsConfig.cc.
Thanks for all the valuable input, I'm finally starting to actually understand
some of this code now :).
was (Author: zwoop):
[~maskit] I haven't tested this yet, but what do you think of something like
this (patch over the -3 diff above):
{code}
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
loki (08:41) 272/1 $ git diff | cat
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index b852186..a51fd35 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -50,9 +50,6 @@ const size_t HTTP2_GOAWAY_LEN = 8;
const size_t HTTP2_WINDOW_UPDATE_LEN = 4;
const size_t HTTP2_SETTINGS_PARAMETER_LEN = 6;
-// SETTINGS initial values
-const uint32_t HTTP2_ENABLE_PUSH = 0; // Server Push is NOT supported
-
// 6.9.1 The Flow Control Window
static const Http2WindowSize HTTP2_MAX_WINDOW_SIZE = 0x7FFFFFFF;
diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h
index 137bcc9..21f8476 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -43,7 +43,7 @@ static size_t const HTTP2_HEADER_BUFFER_SIZE_INDEX =
CLIENT_CONNECTION_FIRST_REA
// To support Upgrade: h2c
struct Http2UpgradeContext {
- Http2UpgradeContext() { client_settings.init(); }
+ Http2UpgradeContext() { }
~Http2UpgradeContext()
{
diff --git a/proxy/http2/Http2ConnectionState.h
b/proxy/http2/Http2ConnectionState.h
index 791f5e4..d829d45 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -36,13 +36,20 @@ public:
Http2ConnectionSettings()
{
// 6.5.2. Defined SETTINGS Parameters
- // TODO these values should be configurable.
- settings[indexof(HTTP2_SETTINGS_ENABLE_PUSH)] = HTTP2_ENABLE_PUSH;
- init();
+
+ // IMPORTANT: These are the default settings as per the HTTP/2 protocol.
+ // They should *not* be modified, and I make these hardcoded here to avoid
+ // tempting people to later change the defines or constants.
+ settings[indexof(HTTP2_SETTINGS_ENABLE_PUSH)] = 0; // Disabled for now
+ settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] = 100;
+ settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] = 65535;
+ settings[indexof(HTTP2_SETTINGS_MAX_FRAME_SIZE)] = 16384;
+ settings[indexof(HTTP2_SETTINGS_HEADER_TABLE_SIZE)] = 4096;
+ settings[indexof(HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE)] = UINT_MAX;
}
void
- init()
+ settings_from_configs()
{
settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] =
Http2::max_concurrent_streams;
settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] =
Http2::initial_window_size;
@@ -206,7 +213,7 @@ public:
continued_buffer.iov_base = NULL;
continued_buffer.iov_len = 0;
- server_settings.init();
+ server_settings.settings_from_configs();
}
void
{code}
> Add configuration knobs for HTTP/2
> ----------------------------------
>
> Key: TS-3475
> URL: https://issues.apache.org/jira/browse/TS-3475
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
> Reporter: Leif Hedstrom
> Assignee: Leif Hedstrom
> Fix For: 6.0.0
>
> Attachments: TS-3475-3.diff
>
>
> We currently have the following SPDY configs, we should have at least the
> same ones for HTTP/2:
> {code}
> {RECT_CONFIG, "proxy.config.spdy.max_concurrent_streams_in", RECD_INT,
> "100", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> {RECT_CONFIG, "proxy.config.spdy.no_activity_timeout_in", RECD_INT, "115",
> RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> {RECT_CONFIG, "proxy.config.spdy.initial_window_size_in", RECD_INT,
> "1048576", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> {RECT_CONFIG, "proxy.config.spdy.accept_no_activity_timeout", RECD_INT,
> "120", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
