Skip to content

TS-4087: Reduce SETTINGS_MAX_CONCURRENT_STREAMS when too many streams #485

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

Closed
masaori335 wants to merge 1 commit into from
Closed

TS-4087: Reduce SETTINGS_MAX_CONCURRENT_STREAMS when too many streams #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 mgmt/RecordsConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1983,6 +1983,10 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http2.max_concurrent_streams_in", RECD_INT, "100", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http2.min_concurrent_streams_in", RECD_INT, "10", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http2.max_active_streams_in", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http2.initial_window_size_in", RECD_INT, "1048576", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http2.max_frame_size", RECD_INT, "16384", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
Expand Down
12 changes: 9 additions & 3 deletions proxy/http2/HTTP2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,10 @@ http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const uint8_t
}

// Initialize this subsystem with librecords configs (for now)
uint32_t Http2::max_concurrent_streams = 100;
uint32_t Http2::max_concurrent_streams_in = 100;
uint32_t Http2::min_concurrent_streams_in = 10;
uint32_t Http2::max_active_streams_in = 0;
bool Http2::throttling = false;
uint32_t Http2::initial_window_size = 1048576;
uint32_t Http2::max_frame_size = 16384;
uint32_t Http2::header_table_size = 4096;
Expand All @@ -740,7 +743,9 @@ uint32_t Http2::active_timeout_in = 0;
void
Http2::init()
{
REC_EstablishStaticConfigInt32U(max_concurrent_streams, "proxy.config.http2.max_concurrent_streams_in");
REC_EstablishStaticConfigInt32U(max_concurrent_streams_in, "proxy.config.http2.max_concurrent_streams_in");
REC_EstablishStaticConfigInt32U(min_concurrent_streams_in, "proxy.config.http2.min_concurrent_streams_in");
REC_EstablishStaticConfigInt32U(max_active_streams_in, "proxy.config.http2.max_active_streams_in");
REC_EstablishStaticConfigInt32U(initial_window_size, "proxy.config.http2.initial_window_size_in");
REC_EstablishStaticConfigInt32U(max_frame_size, "proxy.config.http2.max_frame_size");
REC_EstablishStaticConfigInt32U(header_table_size, "proxy.config.http2.header_table_size");
Expand All @@ -751,7 +756,8 @@ Http2::init()
REC_EstablishStaticConfigInt32U(active_timeout_in, "proxy.config.http2.active_timeout_in");

// If any settings is broken, ATS should not start
ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, max_concurrent_streams}) &&
ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, max_concurrent_streams_in}) &&
http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, min_concurrent_streams_in}) &&
http2_settings_parameter_is_valid({HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, initial_window_size}) &&
http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_FRAME_SIZE, max_frame_size}) &&
http2_settings_parameter_is_valid({HTTP2_SETTINGS_HEADER_TABLE_SIZE, header_table_size}) &&
Expand Down
5 changes: 4 additions & 1 deletion proxy/http2/HTTP2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,10 @@ int64_t http2_write_header_fragment(HTTPHdr *, MIMEFieldIter &, uint8_t *, uint6
class Http2
{
public:
static uint32_t max_concurrent_streams;
static uint32_t max_concurrent_streams_in;
static uint32_t min_concurrent_streams_in;
static uint32_t max_active_streams_in;
static bool throttling;
static uint32_t initial_window_size;
static uint32_t max_frame_size;
static uint32_t header_table_size;
Expand Down
35 changes: 35 additions & 0 deletions proxy/http2/Http2ConnectionState.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,8 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
// settings.
Http2ConnectionSettings configured_settings;
configured_settings.settings_from_configs();
configured_settings.set(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, _adjust_concurrent_stream());

send_settings_frame(configured_settings);

if (server_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) > HTTP2_INITIAL_WINDOW_SIZE) {
Expand Down Expand Up @@ -1160,3 +1162,36 @@ Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t size)
SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &window_update);
}

// Return min_concurrent_streams_in when current client streams number is larger than max_active_streams_in.
// Main purpose of this is preventing DDoS Attacks.
unsigned
Http2ConnectionState::_adjust_concurrent_stream()
{
if (Http2::max_active_streams_in == 0) {
// Throttling down is disabled.
return Http2::max_concurrent_streams_in;
}

int64_t current_client_streams = 0;
RecGetRawStatSum(http2_rsb, HTTP2_STAT_CURRENT_CLIENT_STREAM_COUNT, &current_client_streams);
Copy link
Contributor

@bryancall bryancall Feb 24, 2016

Choose a reason for hiding this comment

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

This is getting the total stream value across all the threads and then you are comparing it to the per thread value below.

Copy link
Contributor Author

@masaori335 masaori335 Feb 25, 2016

Choose a reason for hiding this comment

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

Ah, RecGetRawStatSum gets global + thread local stats. I'll fix this. Thanks.
I thought RecGetGlobalRawStatSum is global and RecGetRawStatSum is thread local one;)


DebugHttp2Con(ua_session, "current client streams: %" PRId64, current_client_streams);

if (current_client_streams >= Http2::max_active_streams_in) {
if (!Http2::throttling) {
Warning("too many streams: %" PRId64 ", reduce SETTINGS_MAX_CONCURRENT_STREAMS to %d", current_client_streams,
Http2::min_concurrent_streams_in);
Http2::throttling = true;
}

return Http2::min_concurrent_streams_in;
} else {
if (Http2::throttling) {
Note("revert SETTINGS_MAX_CONCURRENT_STREAMS to %d", Http2::max_concurrent_streams_in);
Http2::throttling = false;
}
}

return Http2::max_concurrent_streams_in;
}
4 changes: 3 additions & 1 deletion proxy/http2/Http2ConnectionState.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Http2ConnectionSettings
void
settings_from_configs()
{
settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] = Http2::max_concurrent_streams;
settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] = Http2::max_concurrent_streams_in;
settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] = Http2::initial_window_size;
settings[indexof(HTTP2_SETTINGS_MAX_FRAME_SIZE)] = Http2::max_frame_size;
settings[indexof(HTTP2_SETTINGS_HEADER_TABLE_SIZE)] = Http2::header_table_size;
Expand Down Expand Up @@ -199,6 +199,8 @@ class Http2ConnectionState : public Continuation
Http2ConnectionState(const Http2ConnectionState &); // noncopyable
Http2ConnectionState &operator=(const Http2ConnectionState &); // noncopyable

unsigned _adjust_concurrent_stream();

// NOTE: 'stream_list' has only active streams.
// If given Stream Identifier is not found in stream_list and it is less
// than or equal to latest_streamid, the state of Stream
Expand Down