From e960ab79e0586b95d4bd7b7d765c43275bffebee Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Sat, 10 Sep 2022 23:28:27 +0000 Subject: [PATCH 1/4] Add HTTP/2 flow control configurability Issue #8199 outlines the issue. This PR adds the ability to specify different flow control mechanisms. Closes #8199 --- doc/admin-guide/files/records.config.en.rst | 35 +++- mgmt/RecordsConfig.cc | 2 + proxy/http2/HTTP2.cc | 72 ++++--- proxy/http2/HTTP2.h | 21 +- proxy/http2/Http2ConnectionState.cc | 187 ++++++++++++++---- proxy/http2/Http2ConnectionState.h | 52 ++++- proxy/http2/Http2Frame.h | 2 +- proxy/http2/Http2Stream.cc | 7 +- proxy/http2/Http2Stream.h | 13 +- .../gold_tests/h2/http2_flow_control.test.py | 131 +++++++++--- 10 files changed, 412 insertions(+), 110 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 85630183ca3..241a32648c8 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -4152,7 +4152,40 @@ HTTP/2 Configuration :reloadable: :units: bytes - The initial window size for inbound connections. + The initial HTTP/2 stream window size for inbound connections that |TS| as a + receiver advertises to the peer. See IETF RFC 9113 section 5.2 for details + concerning HTTP/2 flow control. See + :ts:cv:`proxy.config.http2.flow_control.in.policy` for how HTTP/2 stream and + session windows are maintained over the lifetime of HTTP/2 connections. + +.. ts:cv:: CONFIG proxy.config.http2.flow_control.in.policy INT 0 + :reloadable: + + Specifies the mechanism |TS| uses to maintian flow control via the HTTP/2 + stream and session windows for inbound connections. See IETF RFC 9113 + section 5.2 for details concerning HTTP/2 flow control. + + ===== =========================================================================================== + Value Description + ===== =========================================================================================== + ``0`` Session and stream receive windows are initialized and maintained at the value as specified + in :ts:cv:`proxy.config.http2.initial_window_size_in` over the lifetime of HTTP/2 + connections. + ``1`` Session receive windows are initialized to the value of the product of + :ts:cv:`proxy.config.http2.initial_window_size_in` and + :ts:cv:`proxy.config.http2.max_concurrent_streams_in` and are maintained as such over the + lifetime of HTTP/2 connections. Stream windows are initialized to the value of + :ts:cv:`proxy.config.http2.initial_window_size_in` and are maintained as such over the + lifetime of each HTTP/2 stream. + ``2`` Session receive windows are initialized to the value of the product of + :ts:cv:`proxy.config.http2.initial_window_size_in` and + :ts:cv:`proxy.config.http2.max_concurrent_streams_in` and are maintained as such over the + lifetime of HTTP/2 connections. Stream windows are initialized to the value of + :ts:cv:`proxy.config.http2.initial_window_size_in` but are dynamically adjusted to the + session window size divided by the number of concurrent streams over the lifetime of HTTP/2 + connections. That is, stream window sizes dynamically adjust to fill the session window in + a way that shares the window equally among all concurrent streams. + ===== =========================================================================================== .. ts:cv:: CONFIG proxy.config.http2.max_frame_size INT 16384 :reloadable: diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 14d51e2766c..2c46c809e77 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1301,6 +1301,8 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http2.initial_window_size_in", RECD_INT, "65535", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , + {RECT_CONFIG, "proxy.config.http2.flow_control.in.policy", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL} + , {RECT_CONFIG, "proxy.config.http2.max_frame_size", RECD_INT, "16384", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , {RECT_CONFIG, "proxy.config.http2.header_table_size", RECD_INT, "4096", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc index 44860e14c02..e29f6ab4a6e 100644 --- a/proxy/http2/HTTP2.cc +++ b/proxy/http2/HTTP2.cc @@ -567,35 +567,36 @@ http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const uint32_ } // Initialize this subsystem with librecords configs (for now) -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::stream_priority_enabled = 0; -uint32_t Http2::initial_window_size = 65535; -uint32_t Http2::max_frame_size = 16384; -uint32_t Http2::header_table_size = 4096; -uint32_t Http2::max_header_list_size = 4294967295; -uint32_t Http2::accept_no_activity_timeout = 120; -uint32_t Http2::no_activity_timeout_in = 120; -uint32_t Http2::active_timeout_in = 0; -uint32_t Http2::push_diary_size = 256; -uint32_t Http2::zombie_timeout_in = 0; -float Http2::stream_error_rate_threshold = 0.1; -uint32_t Http2::stream_error_sampling_threshold = 10; -uint32_t Http2::max_settings_per_frame = 7; -uint32_t Http2::max_settings_per_minute = 14; -uint32_t Http2::max_settings_frames_per_minute = 14; -uint32_t Http2::max_ping_frames_per_minute = 60; -uint32_t Http2::max_priority_frames_per_minute = 120; -float Http2::min_avg_window_update = 2560.0; -uint32_t Http2::con_slow_log_threshold = 0; -uint32_t Http2::stream_slow_log_threshold = 0; -uint32_t Http2::header_table_size_limit = 65536; -uint32_t Http2::write_buffer_block_size = 262144; -float Http2::write_size_threshold = 0.5; -uint32_t Http2::write_time_threshold = 100; -uint32_t Http2::buffer_water_mark = 0; +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::stream_priority_enabled = 0; +uint32_t Http2::initial_window_size_in = 65535; +Http2FlowControlPolicy Http2::flow_control_policy_in = Http2FlowControlPolicy::STATIC_SESSION_AND_STATIC_STREAM; +uint32_t Http2::max_frame_size = 16384; +uint32_t Http2::header_table_size = 4096; +uint32_t Http2::max_header_list_size = 4294967295; +uint32_t Http2::accept_no_activity_timeout = 120; +uint32_t Http2::no_activity_timeout_in = 120; +uint32_t Http2::active_timeout_in = 0; +uint32_t Http2::push_diary_size = 256; +uint32_t Http2::zombie_timeout_in = 0; +float Http2::stream_error_rate_threshold = 0.1; +uint32_t Http2::stream_error_sampling_threshold = 10; +uint32_t Http2::max_settings_per_frame = 7; +uint32_t Http2::max_settings_per_minute = 14; +uint32_t Http2::max_settings_frames_per_minute = 14; +uint32_t Http2::max_ping_frames_per_minute = 60; +uint32_t Http2::max_priority_frames_per_minute = 120; +float Http2::min_avg_window_update = 2560.0; +uint32_t Http2::con_slow_log_threshold = 0; +uint32_t Http2::stream_slow_log_threshold = 0; +uint32_t Http2::header_table_size_limit = 65536; +uint32_t Http2::write_buffer_block_size = 262144; +float Http2::write_size_threshold = 0.5; +uint32_t Http2::write_time_threshold = 100; +uint32_t Http2::buffer_water_mark = 0; void Http2::init() @@ -604,7 +605,16 @@ Http2::init() 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(stream_priority_enabled, "proxy.config.http2.stream_priority_enabled"); - REC_EstablishStaticConfigInt32U(initial_window_size, "proxy.config.http2.initial_window_size_in"); + REC_EstablishStaticConfigInt32U(initial_window_size_in, "proxy.config.http2.initial_window_size_in"); + + uint32_t flow_control_policy_in_int = 0; + REC_EstablishStaticConfigInt32U(flow_control_policy_in_int, "proxy.config.http2.flow_control.in.policy"); + if (flow_control_policy_in_int > 2) { + Error("Invalid value for proxy.config.http2.flow_control.in.policy: %d", flow_control_policy_in_int); + flow_control_policy_in_int = 0; + } + flow_control_policy_in = static_cast(flow_control_policy_in_int); + REC_EstablishStaticConfigInt32U(max_frame_size, "proxy.config.http2.max_frame_size"); REC_EstablishStaticConfigInt32U(header_table_size, "proxy.config.http2.header_table_size"); REC_EstablishStaticConfigInt32U(max_header_list_size, "proxy.config.http2.max_header_list_size"); @@ -632,7 +642,7 @@ Http2::init() // 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_in})); ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, min_concurrent_streams_in})); - ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, initial_window_size})); + ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, initial_window_size_in})); ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_FRAME_SIZE, max_frame_size})); ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_HEADER_TABLE_SIZE, header_table_size})); ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, max_header_list_size})); diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h index 4418d6856a5..c5b9a62ddda 100644 --- a/proxy/http2/HTTP2.h +++ b/proxy/http2/HTTP2.h @@ -31,14 +31,15 @@ class HTTPHdr; -typedef unsigned Http2StreamId; +// [RFC 9113] 5.1.1 Stream identifiers. +using Http2StreamId = uint32_t; -constexpr Http2StreamId HTTP2_CONNECTION_CONTROL_STRTEAM = 0; -constexpr uint8_t HTTP2_FRAME_NO_FLAG = 0; +constexpr Http2StreamId HTTP2_CONNECTION_CONTROL_STREAM = 0; +constexpr uint8_t HTTP2_FRAME_NO_FLAG = 0; // [RFC 7540] 6.9.2. Initial Flow Control Window Size // the flow control window can be come negative so we need to track it with a signed type. -typedef int32_t Http2WindowSize; +using Http2WindowSize = int32_t; extern const char *const HTTP2_CONNECTION_PREFACE; const size_t HTTP2_CONNECTION_PREFACE_LEN = 24; @@ -360,6 +361,15 @@ ParseResult http2_convert_header_from_2_to_1_1(HTTPHdr *); ParseResult http2_convert_header_from_1_1_to_2(HTTPHdr *); void http2_init(); +/** Each of these values correspond to the flow control policy described in or + * records.config documentation for proxy.config.http2.flow_control.in.policy. + */ +enum class Http2FlowControlPolicy { + STATIC_SESSION_AND_STATIC_STREAM, + LARGE_SESSION_AND_STATIC_STREAM, + LARGE_SESSION_AND_DYNAMIC_STREAM, +}; + // Not sure where else to put this, but figure this is as good of a start as // anything else. // Right now, only the static init() is available, which sets up some basic @@ -373,7 +383,8 @@ class Http2 static uint32_t max_active_streams_in; static bool throttling; static uint32_t stream_priority_enabled; - static uint32_t initial_window_size; + static uint32_t initial_window_size_in; + static Http2FlowControlPolicy flow_control_policy_in; static uint32_t max_frame_size; static uint32_t header_table_size; static uint32_t max_header_list_size; diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 4a396d543d5..f0b564eddba 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -22,6 +22,7 @@ */ #include "P_Net.h" +#include "HTTP2.h" #include "Http2ConnectionState.h" #include "Http2ClientSession.h" #include "Http2Stream.h" @@ -29,6 +30,7 @@ #include "Http2DebugNames.h" #include "HttpDebugNames.h" +#include "tscore/ink_assert.h" #include "tscpp/util/PostScript.h" #include "tscpp/util/LocalBuffer.h" @@ -170,9 +172,10 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame) stream->decrement_server_rwnd(payload_length); if (is_debug_tag_set("http2_con")) { - uint32_t rwnd = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + uint32_t const stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + uint32_t const session_window = this->_get_configured_receive_session_window_size_in(); Http2StreamDebug(this->session, id, "Received DATA frame: rwnd con=%zd/%" PRId32 " stream=%zd/%" PRId32, this->server_rwnd(), - rwnd, stream->server_rwnd(), rwnd); + session_window, stream->server_rwnd(), stream_window); } const uint32_t unpadded_length = payload_length - pad_length; @@ -429,7 +432,7 @@ Http2ConnectionState::rcv_priority_frame(const Http2Frame &frame) // If a PRIORITY frame is received with a stream identifier of 0x0, the // recipient MUST respond with a connection error of type PROTOCOL_ERROR. - if (stream_id == 0) { + if (stream_id == HTTP2_CONNECTION_CONTROL_STREAM) { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, "priority 0 stream_id"); } @@ -513,7 +516,7 @@ Http2ConnectionState::rcv_rst_stream_frame(const Http2Frame &frame) // frame is received with a stream identifier of 0x0, the recipient MUST // treat this as a connection error (Section 5.4.1) of type // PROTOCOL_ERROR. - if (stream_id == 0) { + if (stream_id == HTTP2_CONNECTION_CONTROL_STREAM) { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, "reset access stream with invalid id"); } @@ -646,7 +649,7 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) // windows that it maintains by the difference between the new value and // the old value. if (param.id == HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) { - this->update_initial_rwnd(param.value); + this->update_initial_client_rwnd(param.value); } this->peer_settings.set(static_cast(param.id), param.value); @@ -666,7 +669,7 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) // [RFC 7540] 6.5. Once all values have been applied, the recipient MUST // immediately emit a SETTINGS frame with the ACK flag set. - Http2SettingsFrame ack_frame(0, HTTP2_FLAGS_SETTINGS_ACK); + Http2SettingsFrame ack_frame(HTTP2_CONNECTION_CONTROL_STREAM, HTTP2_FLAGS_SETTINGS_ACK); this->session->xmit(ack_frame); return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE); @@ -787,7 +790,7 @@ Http2ConnectionState::rcv_window_update_frame(const Http2Frame &frame) // A receiver MUST treat the receipt of a WINDOW_UPDATE frame with a flow // control window increment of 0 as a connection error of type PROTOCOL_ERROR; if (size == 0) { - if (stream_id == 0) { + if (stream_id == HTTP2_CONNECTION_CONTROL_STREAM) { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, "window update length=0 and id=0"); } else { @@ -796,7 +799,7 @@ Http2ConnectionState::rcv_window_update_frame(const Http2Frame &frame) } } - if (stream_id == 0) { + if (stream_id == HTTP2_CONNECTION_CONTROL_STREAM) { // Connection level window update Http2StreamDebug(this->session, stream_id, "Received WINDOW_UPDATE frame - updated to: %zd delta: %u", (this->client_rwnd() + size), size); @@ -990,7 +993,7 @@ void Http2ConnectionSettings::settings_from_configs() { 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_INITIAL_WINDOW_SIZE)] = Http2::initial_window_size_in; settings[indexof(HTTP2_SETTINGS_MAX_FRAME_SIZE)] = Http2::max_frame_size; settings[indexof(HTTP2_SETTINGS_HEADER_TABLE_SIZE)] = Http2::header_table_size; settings[indexof(HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE)] = Http2::max_header_list_size; @@ -1039,16 +1042,17 @@ Http2ConnectionState::Http2ConnectionState() : stream_list() void Http2ConnectionState::init(Http2CommonSession *ssn) { - session = ssn; + session = ssn; + uint32_t const configured_session_window = this->_get_configured_receive_session_window_size_in(); - if (Http2::initial_window_size < HTTP2_INITIAL_WINDOW_SIZE) { + if (configured_session_window < HTTP2_INITIAL_WINDOW_SIZE) { // There is no HTTP/2 specified way to shrink the connection window size // other than to receive data and not send WINDOW_UPDATE frames for a // while. this->_server_rwnd = HTTP2_INITIAL_WINDOW_SIZE; this->_server_rwnd_is_shrinking = true; } else { - this->_server_rwnd = Http2::initial_window_size; + this->_server_rwnd = configured_session_window; this->_server_rwnd_is_shrinking = false; } @@ -1081,10 +1085,23 @@ Http2ConnectionState::send_connection_preface() configured_settings.settings_from_configs(); configured_settings.set(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, _adjust_concurrent_stream()); + if (this->_has_dynamic_stream_window()) { + // Since this is the beginning of the connection and there are no streams + // yet, we can just set the stream window size to fill the entire session + // window size. + uint32_t const stream_window = this->_get_configured_receive_session_window_size_in(); + configured_settings.set(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, stream_window); + } + send_settings_frame(configured_settings); - if (local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) > HTTP2_INITIAL_WINDOW_SIZE) { - send_window_update_frame(0, local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - HTTP2_INITIAL_WINDOW_SIZE); + // If the session window size is non-default, send a WINDOW_UPDATE right + // away. Note that there is no session window size setting in HTTP/2. The + // session window size is controlled entirely by WINDOW_UPDATE frames. + if (this->_get_configured_receive_session_window_size_in() > HTTP2_INITIAL_WINDOW_SIZE) { + auto const diff = this->_get_configured_receive_session_window_size_in() - HTTP2_INITIAL_WINDOW_SIZE; + Http2ConDebug(session, "Updating the session window with a WINDOW_UPDATE frame: %u", diff); + send_window_update_frame(HTTP2_CONNECTION_CONTROL_STREAM, diff); } } @@ -1343,17 +1360,34 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error) } } + uint32_t stream_window = 0; + if (client_streamid && this->_has_dynamic_stream_window()) { + stream_window = this->_get_configured_receive_session_window_size_in() / (client_streams_in_count.load() + 1); + } else { + stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + } Http2Stream *new_stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread(), session->get_proxy_session(), new_id, - peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)); + peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), stream_window); ink_assert(nullptr != new_stream); ink_assert(!stream_list.in(new_stream)); + new_stream->mutex = new_ProxyMutex(); + new_stream->is_first_transaction_flag = get_stream_requests() == 0; + stream_list.enqueue(new_stream); if (client_streamid) { latest_streamid_in = new_id; ink_assert(client_streams_in_count < UINT32_MAX); ++client_streams_in_count; + if (this->_has_dynamic_stream_window()) { + Http2ConnectionSettings new_settings = local_settings; + new_settings.set(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, stream_window); + send_settings_frame(new_settings); + + // Update all the streams for the new window. + this->update_initial_server_rwnd(stream_window); + } } else { latest_streamid_out = new_id; ink_assert(client_streams_out_count < UINT32_MAX); @@ -1365,9 +1399,6 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error) zombie_event->cancel(); zombie_event = nullptr; } - - new_stream->mutex = new_ProxyMutex(); - new_stream->is_first_transaction_flag = get_stream_requests() == 0; increment_stream_requests(); return new_stream; @@ -1427,31 +1458,45 @@ Http2ConnectionState::restart_streams() void Http2ConnectionState::restart_receiving(Http2Stream *stream) { - uint32_t initial_rwnd = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); - uint32_t min_rwnd = std::min(initial_rwnd, this->local_settings.get(HTTP2_SETTINGS_MAX_FRAME_SIZE)); - // Connection level WINDOW UPDATE - if (this->server_rwnd() < min_rwnd) { - Http2WindowSize diff_size = initial_rwnd - this->server_rwnd(); - this->increment_server_rwnd(diff_size); - this->_server_rwnd_is_shrinking = false; - this->send_window_update_frame(0, diff_size); + uint32_t const configured_session_window = this->_get_configured_receive_session_window_size_in(); + uint32_t const min_session_window = std::min(configured_session_window, this->local_settings.get(HTTP2_SETTINGS_MAX_FRAME_SIZE)); + if (this->server_rwnd() < min_session_window) { + Http2WindowSize diff_size = configured_session_window - this->server_rwnd(); + if (diff_size > 0) { + this->increment_server_rwnd(diff_size); + this->_server_rwnd_is_shrinking = false; + this->send_window_update_frame(HTTP2_CONNECTION_CONTROL_STREAM, diff_size); + } } // Stream level WINDOW UPDATE - if (stream == nullptr || stream->server_rwnd() >= min_rwnd) { + if (stream == nullptr || stream->server_rwnd() >= min_session_window) { + // There's no need to increase the stream window size if it is already big + // enough to hold what the stream/max frame size can receive. return; } // If read_vio is buffering data, do not fully update window - int64_t data_size = stream->read_vio_read_avail(); - if (data_size >= initial_rwnd) { + uint32_t const stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + int64_t data_size = stream->read_vio_read_avail(); + if (data_size >= stream_window) { return; } - Http2WindowSize diff_size = initial_rwnd - std::max(static_cast(stream->server_rwnd()), data_size); - stream->increment_server_rwnd(diff_size); - this->send_window_update_frame(stream->get_id(), diff_size); + // Update the window size for the stream + Http2WindowSize diff_size = 0; + if (stream->server_rwnd() < 0 && data_size == 0) { + // Receive windows can be negative if we sent a SETTINGS frame that + // decreased the stream window size mid-stream. + diff_size = stream_window - stream->server_rwnd(); + } else { + diff_size = stream_window - std::max(static_cast(stream->server_rwnd()), data_size); + } + if (diff_size > 0) { + stream->increment_server_rwnd(diff_size); + this->send_window_update_frame(stream->get_id(), diff_size); + } } void @@ -1572,12 +1617,44 @@ Http2ConnectionState::release_stream() } void -Http2ConnectionState::update_initial_rwnd(Http2WindowSize new_size) +Http2ConnectionState::update_initial_client_rwnd(Http2WindowSize new_size) +{ + // Update stream level window sizes + for (Http2Stream *s = stream_list.head; s; s = static_cast(s->link.next)) { + SCOPED_MUTEX_LOCK(lock, s->mutex, this_ethread()); + // Set the new window size, but take into account the already adjusted + // window based on previously sent bytes. + // + // For example: + // 1. Client initializes the stream window to 10K bytes. + // 2. ATS sends 3K bytes to the client. The stream window is now 7K bytes. + // 3. The client sends a SETTINGS frame to update the initial window size to 20K bytes. + // 4. ATS should update the stream window to 17K bytes: 20K - (10K - 7K). + // + // Note that if the client reduces the stream window, this may result in + // negative receive window values. + s->set_client_rwnd(new_size - (peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - s->client_rwnd())); + } +} + +void +Http2ConnectionState::update_initial_server_rwnd(Http2WindowSize new_size) { // Update stream level window sizes for (Http2Stream *s = stream_list.head; s; s = static_cast(s->link.next)) { SCOPED_MUTEX_LOCK(lock, s->mutex, this_ethread()); - s->update_initial_rwnd(new_size - (peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - s->client_rwnd())); + // Set the new window size, but take into account the already adjusted + // window based on previously sent bytes. + // + // For example: + // 1. ATS initializes the stream window to 10K bytes. + // 2. ATS receives 3K bytes from the client. The stream window is now 7K bytes. + // 3. ATS sends a SETTINGS frame to the client to update the initial window size to 20K bytes. + // 4. The stream window should be updated to 17K bytes: 20K - (10K - 7K). + // + // Note that if ATS reduces the stream window, this may result in negative + // receive window values. + s->set_server_rwnd(new_size - (local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - s->server_rwnd())); } } @@ -1971,13 +2048,14 @@ Http2ConnectionState::send_settings_frame(const Http2ConnectionSettings &new_set for (int i = HTTP2_SETTINGS_HEADER_TABLE_SIZE; i < HTTP2_SETTINGS_MAX; ++i) { Http2SettingsIdentifier id = static_cast(i); - unsigned settings_value = new_settings.get(id); + unsigned const old_value = local_settings.get(id); + unsigned const new_value = new_settings.get(id); // Send only difference - if (settings_value != local_settings.get(id)) { - Http2StreamDebug(session, stream_id, " %s : %u", Http2DebugNames::get_settings_param_name(id), settings_value); + if (new_value != old_value) { + Http2StreamDebug(session, stream_id, " %s : %u -> %u", Http2DebugNames::get_settings_param_name(id), old_value, new_value); - params[params_size++] = {static_cast(id), settings_value}; + params[params_size++] = {static_cast(id), new_value}; // Update current settings local_settings.set(id, new_settings.get(id)); @@ -2025,6 +2103,9 @@ Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t size) { Http2StreamDebug(session, id, "Send WINDOW_UPDATE frame: size=%" PRIu32, size); + // By specification, the window update increment must be greater than 0. + ink_release_assert(size > 0); + // Create WINDOW_UPDATE frame Http2WindowUpdateFrame window_update(id, size); this->session->xmit(window_update); @@ -2111,6 +2192,36 @@ Http2ConnectionState::_adjust_concurrent_stream() return Http2::max_concurrent_streams_in; } +uint32_t +Http2ConnectionState::_get_configured_receive_session_window_size_in() const +{ + switch (Http2::flow_control_policy_in) { + case Http2FlowControlPolicy::STATIC_SESSION_AND_STATIC_STREAM: + return Http2::initial_window_size_in; + case Http2FlowControlPolicy::LARGE_SESSION_AND_STATIC_STREAM: + case Http2FlowControlPolicy::LARGE_SESSION_AND_DYNAMIC_STREAM: + return Http2::initial_window_size_in * Http2::max_concurrent_streams_in; + } + + // This is unreachable, but adding a return here quiets a compiler warning. + return Http2::initial_window_size_in; +} + +bool +Http2ConnectionState::_has_dynamic_stream_window() const +{ + switch (Http2::flow_control_policy_in) { + case Http2FlowControlPolicy::STATIC_SESSION_AND_STATIC_STREAM: + case Http2FlowControlPolicy::LARGE_SESSION_AND_STATIC_STREAM: + return false; + case Http2FlowControlPolicy::LARGE_SESSION_AND_DYNAMIC_STREAM: + return true; + } + + // This is unreachable, but adding a return here quiets a compiler warning. + return false; +} + ssize_t Http2ConnectionState::client_rwnd() const { diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h index a9ba84d9350..80e63425025 100644 --- a/proxy/http2/Http2ConnectionState.h +++ b/proxy/http2/Http2ConnectionState.h @@ -86,10 +86,12 @@ class Http2ConnectionState : public Continuation DependencyTree *dependency_tree = nullptr; ActivityCop _cop; - /// The HTTP/2 settings configured by ATS. + /** The HTTP/2 settings configured by ATS and dictated to the peer via + * SETTINGS frames. */ Http2ConnectionSettings local_settings; - /// The HTTP/2 settings configured by the peer. + /** The HTTP/2 settings configured by the peer and dictated to ATS via + * SETTINGS frames. */ Http2ConnectionSettings peer_settings; void init(Http2CommonSession *ssn); @@ -109,7 +111,12 @@ class Http2ConnectionState : public Continuation void release_stream(); void cleanup_streams(); void restart_receiving(Http2Stream *stream); - void update_initial_rwnd(Http2WindowSize new_size); + + /** Update all streams for the peer's newly dictated stream window size. */ + void update_initial_client_rwnd(Http2WindowSize new_size); + + /** Update all streams for our newly dictated stream window size. */ + void update_initial_server_rwnd(Http2WindowSize new_size); Http2StreamId get_latest_stream_id_in() const; Http2StreamId get_latest_stream_id_out() const; @@ -134,6 +141,14 @@ class Http2ConnectionState : public Continuation void send_headers_frame(Http2Stream *stream); bool send_push_promise_frame(Http2Stream *stream, URL &url, const MIMEField *accept_encoding); void send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec); + + /** Send a SETTINGS frame to the peer. + * + * local_settings is updated to the value of @a new_settings as a byproduct + * of this call. + * + * @param[in] new_settings The settings to send to the peer. + */ void send_settings_frame(const Http2ConnectionSettings &new_settings); void send_ping_frame(Http2StreamId id, uint8_t flag, const uint8_t *opaque_data); void send_goaway_frame(Http2StreamId id, Http2ErrorCode ec); @@ -193,6 +208,18 @@ class Http2ConnectionState : public Continuation unsigned _adjust_concurrent_stream(); + /** Calculate the initial session window size that we communicate to peers. + * + * @return The initial receive window size. + */ + uint32_t _get_configured_receive_session_window_size_in() const; + + /** Whether our stream window can change over the lifetime of a session. + * + * @return @c true if the stream window can change, @c false otherwise. + */ + bool _has_dynamic_stream_window() const; + // 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_in, the state of Stream @@ -217,7 +244,26 @@ class Http2ConnectionState : public Continuation uint32_t stream_error_count = 0; // Connection level window size + + /** The session level window that we have to respect when we send data to the + * peer. + * + * This is the session window configured by the peer via WINDOW_UPDATE + * frames. Per specification, this defaults to HTTP2_INITIAL_WINDOW_SIZE (see + * RFC 9113, section 6.9.2). As we send data, we decrement this value. If it + * reaches zero, we stop sending data to respect the peer's flow control + * specification. When we receive WINDOW_UPDATE frames, we increment this + * value. + */ ssize_t _client_rwnd = HTTP2_INITIAL_WINDOW_SIZE; + + /** The session window we maintain with the peer via WINDOW_UPDATE frames. + * + * We maintain the window we expect the peer to respect by sending + * WINDOW_UPDATE frames to the peer. As we receive data, we decrement this + * value, as we send WINDOW_UPDATE frames, we increment it. If it reaches + * zero, we generate a connection-level error. + */ ssize_t _server_rwnd = 0; /** Whether the session window is in a shrinking state before we send the diff --git a/proxy/http2/Http2Frame.h b/proxy/http2/Http2Frame.h index f893d118c41..927f5b3d218 100644 --- a/proxy/http2/Http2Frame.h +++ b/proxy/http2/Http2Frame.h @@ -210,7 +210,7 @@ class Http2GoawayFrame : public Http2TxFrame { public: Http2GoawayFrame(Http2Goaway p) - : Http2TxFrame({HTTP2_GOAWAY_LEN, HTTP2_FRAME_TYPE_GOAWAY, HTTP2_FRAME_NO_FLAG, HTTP2_CONNECTION_CONTROL_STRTEAM}), _params(p) + : Http2TxFrame({HTTP2_GOAWAY_LEN, HTTP2_FRAME_TYPE_GOAWAY, HTTP2_FRAME_NO_FLAG, HTTP2_CONNECTION_CONTROL_STREAM}), _params(p) { } diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index 8da319fb6a1..a44232c856a 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -40,18 +40,15 @@ ClassAllocator http2StreamAllocator("http2StreamAllocator"); -Http2Stream::Http2Stream(ProxySession *session, Http2StreamId sid, ssize_t initial_rwnd) - : super(session), _id(sid), _client_rwnd(initial_rwnd) +Http2Stream::Http2Stream(ProxySession *session, Http2StreamId sid, ssize_t initial_client_rwnd, ssize_t initial_server_rwnd) + : super(session), _id(sid), _client_rwnd(initial_client_rwnd), _server_rwnd(initial_server_rwnd) { SET_HANDLER(&Http2Stream::main_event_handler); this->mark_milestone(Http2StreamMilestone::OPEN); this->_sm = nullptr; - this->_id = sid; this->_thread = this_ethread(); - this->_client_rwnd = initial_rwnd; - this->_server_rwnd = Http2::initial_window_size; this->upstream_outbound_options = *(session->accept_options); this->_reader = this->_receive_buffer.alloc_reader(); diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index ec0c53fcdee..2ce8454390a 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -55,7 +55,7 @@ class Http2Stream : public ProxyTransaction using super = ProxyTransaction; ///< Parent type. Http2Stream() {} // Just to satisfy ClassAllocator - Http2Stream(ProxySession *session, Http2StreamId sid, ssize_t initial_rwnd); + Http2Stream(ProxySession *session, Http2StreamId sid, ssize_t initial_client_rwnd, ssize_t initial_server_rwnd); ~Http2Stream(); int main_event_handler(int event, void *edata); @@ -125,7 +125,8 @@ class Http2Stream : public ProxyTransaction Http2StreamId get_id() const; Http2StreamState get_state() const; bool change_state(uint8_t type, uint8_t flags); - void update_initial_rwnd(Http2WindowSize new_size); + void set_client_rwnd(Http2WindowSize new_size); + void set_server_rwnd(Http2WindowSize new_size); bool has_trailing_header() const; void set_receive_headers(HTTPHdr &h2_headers); MIOBuffer *read_vio_writer() const; @@ -259,11 +260,17 @@ Http2Stream::get_state() const } inline void -Http2Stream::update_initial_rwnd(Http2WindowSize new_size) +Http2Stream::set_client_rwnd(Http2WindowSize new_size) { this->_client_rwnd = new_size; } +inline void +Http2Stream::set_server_rwnd(Http2WindowSize new_size) +{ + this->_server_rwnd = new_size; +} + inline bool Http2Stream::has_trailing_header() const { diff --git a/tests/gold_tests/h2/http2_flow_control.test.py b/tests/gold_tests/h2/http2_flow_control.test.py index 78b087fe2ca..e29220cb7a6 100644 --- a/tests/gold_tests/h2/http2_flow_control.test.py +++ b/tests/gold_tests/h2/http2_flow_control.test.py @@ -28,10 +28,13 @@ class Http2FlowControlTest: """Define an object to test HTTP/2 flow control behavior.""" _replay_file: str = 'http2_flow_control.replay.yaml' - _valid_policy_values: List[int] = list(range(0, 2)) + _valid_policy_values: List[int] = list(range(0, 3)) + _flow_control_policy: Optional[int] = None + _flow_control_policy_is_malformed: bool = False _default_initial_window_size: int = 65535 _default_max_concurrent_streams_in: int = 100 + _default_flow_control_policy: int = 0 _dns_counter: int = 0 _server_counter: int = 0 @@ -43,7 +46,7 @@ def __init__( description: str, initial_window_size: Optional[int] = None, max_concurrent_streams_in: Optional[int] = None, - verify_window_update_frames: bool = False): + flow_control_policy: Optional[int] = None): """Declare the various test Processes. :param description: A description of the test. @@ -58,13 +61,16 @@ def __init__( records.config file. If the paramenter is None, then no window size will be explicitly set and ATS will use the default value. - :param verify_window_update_frames: If True, then the test will verify - that it sees HTTP/2 WINDOW_UPDATE frames as data is sent. + :param flow_control_policy: The value with which to configure the + proxy.config.http2.flow_control.in.policy ATS parameter the + records.config file. If the paramenter is None, then no policy + configuration will be explicitly set and ATS will use the default + value. """ self._description = description self._initial_window_size = initial_window_size - self._expected_initial_window_size = ( + self._expected_initial_stream_window_size = ( initial_window_size if initial_window_size is not None else self._default_initial_window_size) @@ -73,7 +79,14 @@ def __init__( max_concurrent_streams_in if max_concurrent_streams_in is not None else self._default_max_concurrent_streams_in) - self._verify_window_update_frames = verify_window_update_frames + self._flow_control_policy = flow_control_policy + self._expected_flow_control_policy = ( + flow_control_policy if flow_control_policy is not None + else self._default_flow_control_policy) + + self._flow_control_policy_is_malformed = ( + self._flow_control_policy is not None and + self._flow_control_policy not in self._valid_policy_values) self._dns = self._configure_dns() self._server = self._configure_server() @@ -118,6 +131,11 @@ def _configure_trafficserver(self): 'proxy.config.http2.initial_window_size_in': self._initial_window_size, }) + if self._flow_control_policy is not None: + ts.Disk.records_config.update({ + 'proxy.config.http2.flow_control.in.policy': self._flow_control_policy, + }) + if self._max_concurrent_streams_in is not None: ts.Disk.records_config.update({ 'proxy.config.http2.max_concurrent_streams_in': self._max_concurrent_streams_in, @@ -131,6 +149,11 @@ def _configure_trafficserver(self): f'map / http://127.0.0.1:{self._server.Variables.http_port}' ) + if self._flow_control_policy_is_malformed: + ts.Disk.diags_log.Content = Testers.ContainsExpression( + "ERROR.*proxy.config.http2.flow_control.in.policy", + "There should be an about an invalid flow control policy.") + return ts def _configure_client(self, tr): @@ -144,39 +167,84 @@ def _configure_client(self, tr): https_ports=[self._ts.Variables.ssl_port]) Http2FlowControlTest._client_counter += 1 + if self._flow_control_policy_is_malformed: + # Since we're just testing ATS configuration errors, there's no + # need to set up client expectations. + return + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( f'MAX_CONCURRENT_STREAMS:{self._expected_max_concurrent_streams_in}', "Client should receive a MAX_CONCURRENT_STREAMS setting.") if self._initial_window_size is not None: tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'INITIAL_WINDOW_SIZE:{self._expected_initial_window_size}', + f'INITIAL_WINDOW_SIZE:{self._expected_initial_stream_window_size}', "Client should receive an INITIAL_WINDOW_SIZE setting.") - if self._verify_window_update_frames: - tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'WINDOW_UPDATE.*id 0: {self._expected_initial_window_size}', - "Client should receive a session WINDOW_UPDATE.") - + if self._expected_flow_control_policy == 0: + update_window_size = ( + self._expected_initial_stream_window_size - + self._default_initial_window_size) + if update_window_size > 0: + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id 0: {update_window_size}', + "Client should receive a session WINDOW_UPDATE.") + + if self._expected_flow_control_policy in (1, 2): + # Verify the larger window size. + + session_window_size = ( + self._expected_initial_stream_window_size * + self._expected_max_concurrent_streams_in) + # ATS will send a WINDOW_UPDATE frame to the client to increase + # the session window size to the configured value from the default + # value. + update_window_size = ( + session_window_size - self._expected_initial_stream_window_size) + + # A WINDOW_UPDATE can only increase the window size. So make sure that + # the new window size is greater than the default window size. + if update_window_size > Http2FlowControlTest._default_initial_window_size: + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id 0: {update_window_size}', + "Client should receive a session WINDOW_UPDATE.") + + if self._expected_flow_control_policy == 2: + # Verify the streams window sizes get updated. + stream_window_1 = session_window_size + stream_window_2 = int(session_window_size / 2) + stream_window_3 = int(session_window_size / 3) + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + (f'INITIAL_WINDOW_SIZE:{stream_window_1}.*' + f'INITIAL_WINDOW_SIZE:{stream_window_2}.*' + f'INITIAL_WINDOW_SIZE:{stream_window_3}'), + "Client should stream receive window updates", + reflags=re.DOTALL | re.MULTILINE) + + if self._expected_initial_stream_window_size < 1000: + # For the smaller session window sizes, we expect WINDOW_UPDATE frames. tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'WINDOW_UPDATE.*id 3: {self._expected_initial_window_size}', + f'WINDOW_UPDATE.*id 3: {self._expected_initial_stream_window_size}', "Client should receive a stream WINDOW_UPDATE.") tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'WINDOW_UPDATE.*id 5: {self._expected_initial_window_size}', + f'WINDOW_UPDATE.*id 5: {self._expected_initial_stream_window_size}', "Client should receive a stream WINDOW_UPDATE.") tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'WINDOW_UPDATE.*id 7: {self._expected_initial_window_size}', + f'WINDOW_UPDATE.*id 7: {self._expected_initial_stream_window_size}', "Client should receive a stream WINDOW_UPDATE.") def run(self): """Configure the TestRun.""" tr = Test.AddTestRun(self._description) - self._configure_client(tr) - tr.Processes.Default.StartBefore(self._dns) - tr.Processes.Default.StartBefore(self._server) - tr.StillRunningAfter = self._ts + if not self._flow_control_policy_is_malformed: + self._configure_client(tr) + tr.Processes.Default.StartBefore(self._dns) + tr.Processes.Default.StartBefore(self._server) + tr.StillRunningAfter = self._ts + else: + tr.Processes.Default.Command = "true" tr.Processes.Default.StartBefore(self._ts) @@ -198,14 +266,31 @@ def run(self): # Configuring initial_window_size. # test = Http2FlowControlTest( - description="Configure a large initial_window_size_in", + description="Configure a larger initial_window_size_in", initial_window_size=100123) test.run() - +# +# Configuring flow_control_policy. +# +test = Http2FlowControlTest( + description="Configure an unrecognized flow_control.in.policy", + flow_control_policy=23) +test.run() +test = Http2FlowControlTest( + description="Static flow control with a small initial_window_size_in", + initial_window_size=500, + flow_control_policy=0) +test.run() +test = Http2FlowControlTest( + description="Static flow control with a large initial_window_size_in", + max_concurrent_streams_in=10, + initial_window_size=10, + flow_control_policy=1) +test.run() test = Http2FlowControlTest( - description="Configure a small initial_window_size_in", + description="Dynamic flow control with a small initial_window_size_in", max_concurrent_streams_in=10, initial_window_size=10, - verify_window_update_frames=True) + flow_control_policy=2) test.run() From b042ad21e22e2775aafa0ddfaab69c6730334666 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 14 Oct 2022 23:08:07 +0000 Subject: [PATCH 2/4] Handle shrinking stream window sizes. --- doc/admin-guide/files/records.config.en.rst | 10 +- proxy/http2/Http2ConnectionState.cc | 101 +++++++++++++----- proxy/http2/Http2ConnectionState.h | 73 +++++++++++++ .../gold_tests/h2/http2_flow_control.test.py | 36 ++++--- 4 files changed, 177 insertions(+), 43 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 241a32648c8..0ebce8ec14a 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -4156,7 +4156,7 @@ HTTP/2 Configuration receiver advertises to the peer. See IETF RFC 9113 section 5.2 for details concerning HTTP/2 flow control. See :ts:cv:`proxy.config.http2.flow_control.in.policy` for how HTTP/2 stream and - session windows are maintained over the lifetime of HTTP/2 connections. + session windows are maintained over the lifetime of HTTP/2 sessions. .. ts:cv:: CONFIG proxy.config.http2.flow_control.in.policy INT 0 :reloadable: @@ -4170,20 +4170,20 @@ HTTP/2 Configuration ===== =========================================================================================== ``0`` Session and stream receive windows are initialized and maintained at the value as specified in :ts:cv:`proxy.config.http2.initial_window_size_in` over the lifetime of HTTP/2 - connections. + sessions. ``1`` Session receive windows are initialized to the value of the product of :ts:cv:`proxy.config.http2.initial_window_size_in` and :ts:cv:`proxy.config.http2.max_concurrent_streams_in` and are maintained as such over the - lifetime of HTTP/2 connections. Stream windows are initialized to the value of + lifetime of HTTP/2 sessions. Stream windows are initialized to the value of :ts:cv:`proxy.config.http2.initial_window_size_in` and are maintained as such over the lifetime of each HTTP/2 stream. ``2`` Session receive windows are initialized to the value of the product of :ts:cv:`proxy.config.http2.initial_window_size_in` and :ts:cv:`proxy.config.http2.max_concurrent_streams_in` and are maintained as such over the - lifetime of HTTP/2 connections. Stream windows are initialized to the value of + lifetime of HTTP/2 sessions. Stream windows are initialized to the value of :ts:cv:`proxy.config.http2.initial_window_size_in` but are dynamically adjusted to the session window size divided by the number of concurrent streams over the lifetime of HTTP/2 - connections. That is, stream window sizes dynamically adjust to fill the session window in + sessions. That is, stream window sizes dynamically adjust to fill the session window in a way that shares the window equally among all concurrent streams. ===== =========================================================================================== diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index f0b564eddba..cf6355b2fe9 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -172,7 +172,7 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame) stream->decrement_server_rwnd(payload_length); if (is_debug_tag_set("http2_con")) { - uint32_t const stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + uint32_t const stream_window = this->acknowledged_local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); uint32_t const session_window = this->_get_configured_receive_session_window_size_in(); Http2StreamDebug(this->session, id, "Received DATA frame: rwnd con=%zd/%" PRId32 " stream=%zd/%" PRId32, this->server_rwnd(), session_window, stream->server_rwnd(), stream_window); @@ -371,8 +371,8 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) } stream->mark_milestone(Http2StreamMilestone::START_DECODE_HEADERS); - Http2ErrorCode result = - stream->decode_header_blocks(*this->local_hpack_handle, this->local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); + Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, + this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); if (result != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) { if (result == Http2ErrorCode::HTTP2_ERROR_COMPRESSION_ERROR) { @@ -601,6 +601,7 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) // error of type FRAME_SIZE_ERROR. if (frame.header().flags & HTTP2_FLAGS_SETTINGS_ACK) { if (frame.header().length == 0) { + this->_process_incoming_settings_ack_frame(); return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE); } else { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_FRAME_SIZE_ERROR, @@ -935,8 +936,8 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) "continuation no state change"); } - Http2ErrorCode result = - stream->decode_header_blocks(*this->local_hpack_handle, this->local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); + Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, + this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); if (result != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) { if (result == Http2ErrorCode::HTTP2_ERROR_COMPRESSION_ERROR) { @@ -1345,7 +1346,7 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error) // that receives a HEADERS frame that causes their advertised concurrent // stream limit to be exceeded MUST treat this as a stream error. if (client_streamid) { - if (client_streams_in_count >= local_settings.get(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)) { + if (client_streams_in_count >= acknowledged_local_settings.get(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)) { HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_MAX_CONCURRENT_STREAMS_EXCEEDED_IN, this_ethread()); error = Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM, "recv headers creating inbound stream beyond max_concurrent limit"); @@ -1360,14 +1361,24 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error) } } - uint32_t stream_window = 0; + uint32_t initial_stream_window = this->acknowledged_local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + uint32_t initial_stream_window_target = initial_stream_window; if (client_streamid && this->_has_dynamic_stream_window()) { - stream_window = this->_get_configured_receive_session_window_size_in() / (client_streams_in_count.load() + 1); - } else { - stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + // For dynamic stream windows, the peer's idea of what the window size is + // may be different than what we are configuring. Our calulated server + // receive window is always maintained at what the peer has acknowledged so + // far. This prevents us from enforcing window sizes that have been + // adjusted by SETTINGS frames which the peer has not received yet. So we + // initialize the receive window to what the peer has acknowledged while in + // the meantime calculating initial_stream_window_target for the SETTINGS + // frame which will shrink or enlarge it to our new desired size. + // + // The situation of dynamic stream window sizes is described in [RFC 9113] + // 6.9.3. + initial_stream_window_target = this->_get_configured_receive_session_window_size_in() / (client_streams_in_count.load() + 1); } Http2Stream *new_stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread(), session->get_proxy_session(), new_id, - peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), stream_window); + peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), initial_stream_window); ink_assert(nullptr != new_stream); ink_assert(!stream_list.in(new_stream)); @@ -1382,11 +1393,8 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error) ++client_streams_in_count; if (this->_has_dynamic_stream_window()) { Http2ConnectionSettings new_settings = local_settings; - new_settings.set(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, stream_window); + new_settings.set(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, initial_stream_window_target); send_settings_frame(new_settings); - - // Update all the streams for the new window. - this->update_initial_server_rwnd(stream_window); } } else { latest_streamid_out = new_id; @@ -1460,7 +1468,8 @@ Http2ConnectionState::restart_receiving(Http2Stream *stream) { // Connection level WINDOW UPDATE uint32_t const configured_session_window = this->_get_configured_receive_session_window_size_in(); - uint32_t const min_session_window = std::min(configured_session_window, this->local_settings.get(HTTP2_SETTINGS_MAX_FRAME_SIZE)); + uint32_t const min_session_window = + std::min(configured_session_window, this->acknowledged_local_settings.get(HTTP2_SETTINGS_MAX_FRAME_SIZE)); if (this->server_rwnd() < min_session_window) { Http2WindowSize diff_size = configured_session_window - this->server_rwnd(); if (diff_size > 0) { @@ -1478,21 +1487,26 @@ Http2ConnectionState::restart_receiving(Http2Stream *stream) } // If read_vio is buffering data, do not fully update window - uint32_t const stream_window = this->local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); - int64_t data_size = stream->read_vio_read_avail(); - if (data_size >= stream_window) { + uint32_t const initial_stream_window = this->acknowledged_local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); + int64_t data_size = stream->read_vio_read_avail(); + if (data_size >= initial_stream_window) { return; } - // Update the window size for the stream Http2WindowSize diff_size = 0; - if (stream->server_rwnd() < 0 && data_size == 0) { + if (stream->server_rwnd() < 0) { // Receive windows can be negative if we sent a SETTINGS frame that - // decreased the stream window size mid-stream. - diff_size = stream_window - stream->server_rwnd(); + // decreased the stream window size mid-stream. This is not a problem: we + // simply compute a WINDOW_UPDATE value to bring the window up to the + // target initial_stream_window size. + diff_size = initial_stream_window - stream->server_rwnd(); } else { - diff_size = stream_window - std::max(static_cast(stream->server_rwnd()), data_size); + diff_size = initial_stream_window - std::max(static_cast(stream->server_rwnd()), data_size); } + + // Dynamic stream window sizes may result in negative values. In this case, + // we'll just be waiting for the peer to send more data until the receive + // window decreases to be under the initial window size. if (diff_size > 0) { stream->increment_server_rwnd(diff_size); this->send_window_update_frame(stream->get_id(), diff_size); @@ -1654,7 +1668,7 @@ Http2ConnectionState::update_initial_server_rwnd(Http2WindowSize new_size) // // Note that if ATS reduces the stream window, this may result in negative // receive window values. - s->set_server_rwnd(new_size - (local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - s->server_rwnd())); + s->set_server_rwnd(new_size - (acknowledged_local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) - s->server_rwnd())); } } @@ -2039,7 +2053,7 @@ Http2ConnectionState::send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec) void Http2ConnectionState::send_settings_frame(const Http2ConnectionSettings &new_settings) { - const Http2StreamId stream_id = 0; + constexpr Http2StreamId stream_id = HTTP2_CONNECTION_CONTROL_STREAM; Http2StreamDebug(session, stream_id, "Send SETTINGS frame"); @@ -2063,9 +2077,44 @@ Http2ConnectionState::send_settings_frame(const Http2ConnectionSettings &new_set } Http2SettingsFrame settings(stream_id, HTTP2_FRAME_NO_FLAG, params, params_size); + + this->_outstanding_settings_frames.emplace(new_settings); this->session->xmit(settings); } +void +Http2ConnectionState::_process_incoming_settings_ack_frame() +{ + constexpr Http2StreamId stream_id = HTTP2_CONNECTION_CONTROL_STREAM; + Http2StreamDebug(session, stream_id, "Processing SETTINGS ACK frame with a queue size of %zu", + this->_outstanding_settings_frames.size()); + + // Do not update this->acknowledged_local_settings yet as + // update_initial_server_rwnd relies upon it still pointing to the old value. + Http2ConnectionSettings const &old_settings = this->acknowledged_local_settings; + Http2ConnectionSettings const &new_settings = this->_outstanding_settings_frames.front().get_outstanding_settings(); + + for (int i = HTTP2_SETTINGS_HEADER_TABLE_SIZE; i < HTTP2_SETTINGS_MAX; ++i) { + Http2SettingsIdentifier id = static_cast(i); + unsigned const old_value = old_settings.get(id); + unsigned const new_value = new_settings.get(id); + + if (new_value == old_value) { + continue; + } + + Http2StreamDebug(session, stream_id, "SETTINGS ACK %s : %u -> %u", Http2DebugNames::get_settings_param_name(id), old_value, + new_value); + + if (id == HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) { + // Update all the streams for the newly acknowledged window size. + this->update_initial_server_rwnd(new_value); + } + } + this->acknowledged_local_settings = new_settings; + this->_outstanding_settings_frames.pop(); +} + void Http2ConnectionState::send_ping_frame(Http2StreamId id, uint8_t flag, const uint8_t *opaque_data) { diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h index 80e63425025..24315b4e77e 100644 --- a/proxy/http2/Http2ConnectionState.h +++ b/proxy/http2/Http2ConnectionState.h @@ -24,6 +24,7 @@ #pragma once #include +#include #include "NetTimeout.h" @@ -90,6 +91,22 @@ class Http2ConnectionState : public Continuation * SETTINGS frames. */ Http2ConnectionSettings local_settings; + /** The latest set of settings that have been acknowledged by the peer. + * + * The default constructed value of this via the Http2ConnectionSettings + * constructor (i.e., the value before any SETTINGS ACK frames have been + * received) will instantiate these settings to the default HTTP/2 settings + * values. + * + * @note that @a local_settings are our latest configured settings which have + * been sent to the peer but may not be acknowledged yet. @a + * last_acknowledged_settings are the latest settings of which the peer has + * acknowledged receipt. For this reason, window enforcement behavior and + * WINDOW_UPDATE calculations should be based upon @a + * last_acknowledged_settings. + */ + Http2ConnectionSettings acknowledged_local_settings; + /** The HTTP/2 settings configured by the peer and dictated to ATS via * SETTINGS frames. */ Http2ConnectionSettings peer_settings; @@ -150,6 +167,7 @@ class Http2ConnectionState : public Continuation * @param[in] new_settings The settings to send to the peer. */ void send_settings_frame(const Http2ConnectionSettings &new_settings); + void send_ping_frame(Http2StreamId id, uint8_t flag, const uint8_t *opaque_data); void send_goaway_frame(Http2StreamId id, Http2ErrorCode ec); void send_window_update_frame(Http2StreamId id, uint32_t size); @@ -208,6 +226,13 @@ class Http2ConnectionState : public Continuation unsigned _adjust_concurrent_stream(); + /** Receive and process a SETTINGS frame with the ACK flag set. + * + * This function will process any settings updates that have now been + * acknowleged by the peer. + */ + void _process_incoming_settings_ack_frame(); + /** Calculate the initial session window size that we communicate to peers. * * @return The initial receive window size. @@ -286,6 +311,54 @@ class Http2ConnectionState : public Continuation Http2FrequencyCounter _received_ping_frame_counter; Http2FrequencyCounter _received_priority_frame_counter; + /** Records the various settings for each SETTINGS frame that we've sent. + * + * There are certain SETTINGS values that we send but cannot act upon until the + * peer acknowledges them. For instance, we cannot enforce reduced stream + * window sizes until the peer acknowledges the SETTINGS frame that shrinks the + * size. + * + * There is an OutstandingSettingsFrame instance for each SETTINGS frame that + * we send. We store these in a queue and associate each SETTINGS frame with + * an ACK flag from the peer with a corresponding OutstandingSettingsFrame + * instance. + * + * For details about SETTINGS synchronization via the ACK flag, see: + * + * [RFC 9113] 6.5.3 Settings Synchronization + */ + class OutstandingSettingsFrame + { + public: + OutstandingSettingsFrame(const Http2ConnectionSettings &settings) : _settings(settings) {} + + /** Returns the settings parameters that were configured via the SETTINGS frame + * associated with this instance. + * + * @return The settigns parameters that were configured at the time the + * associated SETTINGS frame was sent. @note that this is not just the + * values in the SETTINGS frame, but those values along with all the local + * settings that were in place but not explicitly configured via the frame. + * Thus this returns the snapshot of the entire set of settings configured + * when the SETTINGS frame was sent. + */ + Http2ConnectionSettings const & + get_outstanding_settings() const + { + return _settings; + } + + private: + /** The SETTINGS parameters that were set at the time of the associated + * SETTINGS frame being sent. + */ + Http2ConnectionSettings const _settings; + }; + + /** The queue of SETTINGS frames that we have sent but have not yet been + * acknowledged by the peer. */ + std::queue _outstanding_settings_frames; + // NOTE: Id of stream which MUST receive CONTINUATION frame. // - [RFC 7540] 6.2 HEADERS // "A HEADERS frame without the END_HEADERS flag set MUST be followed by a diff --git a/tests/gold_tests/h2/http2_flow_control.test.py b/tests/gold_tests/h2/http2_flow_control.test.py index e29220cb7a6..927c90b60ac 100644 --- a/tests/gold_tests/h2/http2_flow_control.test.py +++ b/tests/gold_tests/h2/http2_flow_control.test.py @@ -172,6 +172,7 @@ def _configure_client(self, tr): # need to set up client expectations. return + # ATS currently always sends a MAX_CONCURRENT_STREAMS setting. tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( f'MAX_CONCURRENT_STREAMS:{self._expected_max_concurrent_streams_in}', "Client should receive a MAX_CONCURRENT_STREAMS setting.") @@ -181,14 +182,14 @@ def _configure_client(self, tr): f'INITIAL_WINDOW_SIZE:{self._expected_initial_stream_window_size}', "Client should receive an INITIAL_WINDOW_SIZE setting.") - if self._expected_flow_control_policy == 0: - update_window_size = ( - self._expected_initial_stream_window_size - - self._default_initial_window_size) - if update_window_size > 0: - tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - f'WINDOW_UPDATE.*id 0: {update_window_size}', - "Client should receive a session WINDOW_UPDATE.") + if self._expected_flow_control_policy == 0: + update_window_size = ( + self._expected_initial_stream_window_size - + self._default_initial_window_size) + if update_window_size > 0: + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id 0: {update_window_size}', + "Client should receive a session WINDOW_UPDATE.") if self._expected_flow_control_policy in (1, 2): # Verify the larger window size. @@ -196,6 +197,7 @@ def _configure_client(self, tr): session_window_size = ( self._expected_initial_stream_window_size * self._expected_max_concurrent_streams_in) + # ATS will send a WINDOW_UPDATE frame to the client to increase # the session window size to the configured value from the default # value. @@ -207,6 +209,16 @@ def _configure_client(self, tr): if update_window_size > Http2FlowControlTest._default_initial_window_size: tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( f'WINDOW_UPDATE.*id 0: {update_window_size}', + "Client should receive an initial session WINDOW_UPDATE.") + else: + # Our test traffic is large enough that eventually we should + # send a session WINDOW_UPDATE frame for the smaller window. + # It's not clear what it will be in advance though. A 100 byte + # session window may not receive a 100 byte WINDOW_UPDATE frame + # if the client is sending DATA frames in 10 byte chunks due to + # a smaller stream window. + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'WINDOW_UPDATE.*id 0: ', "Client should receive a session WINDOW_UPDATE.") if self._expected_flow_control_policy == 2: @@ -278,18 +290,18 @@ def run(self): flow_control_policy=23) test.run() test = Http2FlowControlTest( - description="Static flow control with a small initial_window_size_in", - initial_window_size=500, + description="Flow control policy 0 (default): small initial_window_size_in", + initial_window_size=500, # The default is 65 KB. flow_control_policy=0) test.run() test = Http2FlowControlTest( - description="Static flow control with a large initial_window_size_in", + description="Flow control policy 1: 100 byte session, 10 byte stream windows", max_concurrent_streams_in=10, initial_window_size=10, flow_control_policy=1) test.run() test = Http2FlowControlTest( - description="Dynamic flow control with a small initial_window_size_in", + description="Flow control policy 2: 100 byte session, dynamic stream windows", max_concurrent_streams_in=10, initial_window_size=10, flow_control_policy=2) From a0630163f92bcaa705362586e87882db281a1e38 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 19 Oct 2022 03:27:21 +0000 Subject: [PATCH 3/4] http2.flow_control.in.policy -> http2.flow_control.policy.in --- doc/admin-guide/files/records.config.en.rst | 4 ++-- mgmt/RecordsConfig.cc | 2 +- proxy/http2/HTTP2.cc | 4 ++-- proxy/http2/HTTP2.h | 2 +- tests/gold_tests/h2/http2_flow_control.test.py | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 0ebce8ec14a..aca633f3ba9 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -4155,10 +4155,10 @@ HTTP/2 Configuration The initial HTTP/2 stream window size for inbound connections that |TS| as a receiver advertises to the peer. See IETF RFC 9113 section 5.2 for details concerning HTTP/2 flow control. See - :ts:cv:`proxy.config.http2.flow_control.in.policy` for how HTTP/2 stream and + :ts:cv:`proxy.config.http2.flow_control.policy.in` for how HTTP/2 stream and session windows are maintained over the lifetime of HTTP/2 sessions. -.. ts:cv:: CONFIG proxy.config.http2.flow_control.in.policy INT 0 +.. ts:cv:: CONFIG proxy.config.http2.flow_control.policy.in INT 0 :reloadable: Specifies the mechanism |TS| uses to maintian flow control via the HTTP/2 diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 2c46c809e77..22284953291 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1301,7 +1301,7 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http2.initial_window_size_in", RECD_INT, "65535", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , - {RECT_CONFIG, "proxy.config.http2.flow_control.in.policy", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL} + {RECT_CONFIG, "proxy.config.http2.flow_control.policy.in", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL} , {RECT_CONFIG, "proxy.config.http2.max_frame_size", RECD_INT, "16384", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc index e29f6ab4a6e..c8dcb86f29c 100644 --- a/proxy/http2/HTTP2.cc +++ b/proxy/http2/HTTP2.cc @@ -608,9 +608,9 @@ Http2::init() REC_EstablishStaticConfigInt32U(initial_window_size_in, "proxy.config.http2.initial_window_size_in"); uint32_t flow_control_policy_in_int = 0; - REC_EstablishStaticConfigInt32U(flow_control_policy_in_int, "proxy.config.http2.flow_control.in.policy"); + REC_EstablishStaticConfigInt32U(flow_control_policy_in_int, "proxy.config.http2.flow_control.policy.in"); if (flow_control_policy_in_int > 2) { - Error("Invalid value for proxy.config.http2.flow_control.in.policy: %d", flow_control_policy_in_int); + Error("Invalid value for proxy.config.http2.flow_control.policy.in: %d", flow_control_policy_in_int); flow_control_policy_in_int = 0; } flow_control_policy_in = static_cast(flow_control_policy_in_int); diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h index c5b9a62ddda..23108648339 100644 --- a/proxy/http2/HTTP2.h +++ b/proxy/http2/HTTP2.h @@ -362,7 +362,7 @@ ParseResult http2_convert_header_from_1_1_to_2(HTTPHdr *); void http2_init(); /** Each of these values correspond to the flow control policy described in or - * records.config documentation for proxy.config.http2.flow_control.in.policy. + * records.config documentation for proxy.config.http2.flow_control.policy.in. */ enum class Http2FlowControlPolicy { STATIC_SESSION_AND_STATIC_STREAM, diff --git a/tests/gold_tests/h2/http2_flow_control.test.py b/tests/gold_tests/h2/http2_flow_control.test.py index 927c90b60ac..7c0fe5d4d81 100644 --- a/tests/gold_tests/h2/http2_flow_control.test.py +++ b/tests/gold_tests/h2/http2_flow_control.test.py @@ -62,7 +62,7 @@ def __init__( will be explicitly set and ATS will use the default value. :param flow_control_policy: The value with which to configure the - proxy.config.http2.flow_control.in.policy ATS parameter the + proxy.config.http2.flow_control.policy.in ATS parameter the records.config file. If the paramenter is None, then no policy configuration will be explicitly set and ATS will use the default value. @@ -133,7 +133,7 @@ def _configure_trafficserver(self): if self._flow_control_policy is not None: ts.Disk.records_config.update({ - 'proxy.config.http2.flow_control.in.policy': self._flow_control_policy, + 'proxy.config.http2.flow_control.policy.in': self._flow_control_policy, }) if self._max_concurrent_streams_in is not None: @@ -151,7 +151,7 @@ def _configure_trafficserver(self): if self._flow_control_policy_is_malformed: ts.Disk.diags_log.Content = Testers.ContainsExpression( - "ERROR.*proxy.config.http2.flow_control.in.policy", + "ERROR.*proxy.config.http2.flow_control.policy.in", "There should be an about an invalid flow control policy.") return ts From 39dcd61cdd64adb7a222689aba913b8c53e187ed Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 19 Oct 2022 15:05:47 +0000 Subject: [PATCH 4/4] http2.flow_control.policy.in -> http2.flow_control.policy_in --- doc/admin-guide/files/records.config.en.rst | 4 ++-- mgmt/RecordsConfig.cc | 2 +- proxy/http2/HTTP2.cc | 4 ++-- proxy/http2/HTTP2.h | 2 +- tests/gold_tests/h2/http2_flow_control.test.py | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index aca633f3ba9..b0dc36c329e 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -4155,10 +4155,10 @@ HTTP/2 Configuration The initial HTTP/2 stream window size for inbound connections that |TS| as a receiver advertises to the peer. See IETF RFC 9113 section 5.2 for details concerning HTTP/2 flow control. See - :ts:cv:`proxy.config.http2.flow_control.policy.in` for how HTTP/2 stream and + :ts:cv:`proxy.config.http2.flow_control.policy_in` for how HTTP/2 stream and session windows are maintained over the lifetime of HTTP/2 sessions. -.. ts:cv:: CONFIG proxy.config.http2.flow_control.policy.in INT 0 +.. ts:cv:: CONFIG proxy.config.http2.flow_control.policy_in INT 0 :reloadable: Specifies the mechanism |TS| uses to maintian flow control via the HTTP/2 diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 22284953291..f209603ec6d 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1301,7 +1301,7 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http2.initial_window_size_in", RECD_INT, "65535", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , - {RECT_CONFIG, "proxy.config.http2.flow_control.policy.in", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL} + {RECT_CONFIG, "proxy.config.http2.flow_control.policy_in", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL} , {RECT_CONFIG, "proxy.config.http2.max_frame_size", RECD_INT, "16384", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc index c8dcb86f29c..65622a8189d 100644 --- a/proxy/http2/HTTP2.cc +++ b/proxy/http2/HTTP2.cc @@ -608,9 +608,9 @@ Http2::init() REC_EstablishStaticConfigInt32U(initial_window_size_in, "proxy.config.http2.initial_window_size_in"); uint32_t flow_control_policy_in_int = 0; - REC_EstablishStaticConfigInt32U(flow_control_policy_in_int, "proxy.config.http2.flow_control.policy.in"); + REC_EstablishStaticConfigInt32U(flow_control_policy_in_int, "proxy.config.http2.flow_control.policy_in"); if (flow_control_policy_in_int > 2) { - Error("Invalid value for proxy.config.http2.flow_control.policy.in: %d", flow_control_policy_in_int); + Error("Invalid value for proxy.config.http2.flow_control.policy_in: %d", flow_control_policy_in_int); flow_control_policy_in_int = 0; } flow_control_policy_in = static_cast(flow_control_policy_in_int); diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h index 23108648339..d55e3804c7b 100644 --- a/proxy/http2/HTTP2.h +++ b/proxy/http2/HTTP2.h @@ -362,7 +362,7 @@ ParseResult http2_convert_header_from_1_1_to_2(HTTPHdr *); void http2_init(); /** Each of these values correspond to the flow control policy described in or - * records.config documentation for proxy.config.http2.flow_control.policy.in. + * records.config documentation for proxy.config.http2.flow_control.policy_in. */ enum class Http2FlowControlPolicy { STATIC_SESSION_AND_STATIC_STREAM, diff --git a/tests/gold_tests/h2/http2_flow_control.test.py b/tests/gold_tests/h2/http2_flow_control.test.py index 7c0fe5d4d81..39ed1b03698 100644 --- a/tests/gold_tests/h2/http2_flow_control.test.py +++ b/tests/gold_tests/h2/http2_flow_control.test.py @@ -62,7 +62,7 @@ def __init__( will be explicitly set and ATS will use the default value. :param flow_control_policy: The value with which to configure the - proxy.config.http2.flow_control.policy.in ATS parameter the + proxy.config.http2.flow_control.policy_in ATS parameter the records.config file. If the paramenter is None, then no policy configuration will be explicitly set and ATS will use the default value. @@ -133,7 +133,7 @@ def _configure_trafficserver(self): if self._flow_control_policy is not None: ts.Disk.records_config.update({ - 'proxy.config.http2.flow_control.policy.in': self._flow_control_policy, + 'proxy.config.http2.flow_control.policy_in': self._flow_control_policy, }) if self._max_concurrent_streams_in is not None: @@ -151,7 +151,7 @@ def _configure_trafficserver(self): if self._flow_control_policy_is_malformed: ts.Disk.diags_log.Content = Testers.ContainsExpression( - "ERROR.*proxy.config.http2.flow_control.policy.in", + "ERROR.*proxy.config.http2.flow_control.policy_in", "There should be an about an invalid flow control policy.") return ts