bneradt commented on code in PR #11345:
URL: https://github.com/apache/trafficserver/pull/11345#discussion_r1680140767
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -993,6 +1005,18 @@ Http2ConnectionState::rcv_continuation_frame(const
Http2Frame &frame)
"continuation bad client id");
}
+ if (payload_length == 0 && (frame.header().flags &
HTTP2_FLAGS_HEADERS_END_HEADERS) == 0x0) {
+ this->increment_received_empty_frame_count();
+ if (configured_max_empty_frames_per_minute >= 0 &&
+ this->get_received_empty_frame_count() >
static_cast<uint32_t>(configured_max_empty_frames_per_minute)) {
+
Metrics::Counter::increment(http2_rsb.max_empty_frames_per_minute_exceeded);
+ Http2StreamDebug(this->session, stream_id, "Observed too frequent empty
frames: %u within a last minute",
Review Comment:
Being explicit about these being CONTINUATION frames might be helpful to the
reader:
```
Observed too many empty CONTINUATION frames: %u within the last minute
```
##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4564,20 +4568,35 @@ HTTP/2 Configuration
code of ENHANCE_YOUR_CALM. If this is set to 0, the limit logic is disabled.
This limit only will be enforced if
:ts:cv:`proxy.config.http2.stream_priority_enabled`
is set to 1.
+ Any negative value configures no limit to the number of PRIORITY frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_rst_stream_frames_per_minute INT 200
:reloadable:
Specifies how many RST_STREAM frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of RST_STREAM frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_continuation_frames_per_minute INT 120
:reloadable:
Specifies how many CONTINUATION frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of CONTINUATION frames
received.
+
+.. ts:cv:: CONFIG proxy.config.http2.max_empty_frames_per_minute INT 0
+ :reloadable:
+
+ Specifies how many empty frames |TS| receives for a minute at maximum.
Review Comment:
```
Specifies the maximum number of empty frames |TS| will receive per minute
before it will start closing connections.
```
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -176,8 +176,19 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame
&frame)
stream->set_trailing_header_is_possible();
}
- // If payload length is 0 without END_STREAM flag, do nothing
- if (payload_length == 0 && !stream->receive_end_stream) {
+ // If payload length is 0 without END_STREAM flag, just count it
+ const uint32_t unpadded_length = payload_length - pad_length;
+ if (unpadded_length == 0 && !stream->receive_end_stream) {
+ this->increment_received_empty_frame_count();
+ if (configured_max_empty_frames_per_minute >= 0 &&
+ this->get_received_empty_frame_count() >
static_cast<uint32_t>(configured_max_empty_frames_per_minute)) {
+
Metrics::Counter::increment(http2_rsb.max_empty_frames_per_minute_exceeded);
+ Http2StreamDebug(this->session, id, "Observed too frequent empty frames:
%u within a last minute",
Review Comment:
```
Observed too many empty DATA frames: %u within the last minute
```
##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4564,20 +4568,35 @@ HTTP/2 Configuration
code of ENHANCE_YOUR_CALM. If this is set to 0, the limit logic is disabled.
This limit only will be enforced if
:ts:cv:`proxy.config.http2.stream_priority_enabled`
is set to 1.
+ Any negative value configures no limit to the number of PRIORITY frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_rst_stream_frames_per_minute INT 200
:reloadable:
Specifies how many RST_STREAM frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of RST_STREAM frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_continuation_frames_per_minute INT 120
:reloadable:
Specifies how many CONTINUATION frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of CONTINUATION frames
received.
+
+.. ts:cv:: CONFIG proxy.config.http2.max_empty_frames_per_minute INT 0
+ :reloadable:
+
+ Specifies how many empty frames |TS| receives for a minute at maximum.
+ What "empty frames" means here is DATA frame that does not carry payload
+ nor END_STREAM flag, and CONTINUATION frame that does not carry payload
+ nor END_HEADERS flag.
+ Clients exceeded this limit will be immediately disconnected with an error
Review Comment:
`Clients exceeded` -> `Clients exceeding`
##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4564,20 +4568,35 @@ HTTP/2 Configuration
code of ENHANCE_YOUR_CALM. If this is set to 0, the limit logic is disabled.
This limit only will be enforced if
:ts:cv:`proxy.config.http2.stream_priority_enabled`
is set to 1.
+ Any negative value configures no limit to the number of PRIORITY frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_rst_stream_frames_per_minute INT 200
:reloadable:
Specifies how many RST_STREAM frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of RST_STREAM frames
received.
.. ts:cv:: CONFIG proxy.config.http2.max_continuation_frames_per_minute INT 120
:reloadable:
Specifies how many CONTINUATION frames |TS| receives per minute at maximum.
Clients exceeding this limit will be immediately disconnected with an error
code of ENHANCE_YOUR_CALM.
+ Any negative value configures no limit to the number of CONTINUATION frames
received.
+
+.. ts:cv:: CONFIG proxy.config.http2.max_empty_frames_per_minute INT 0
+ :reloadable:
+
+ Specifies how many empty frames |TS| receives for a minute at maximum.
+ What "empty frames" means here is DATA frame that does not carry payload
+ nor END_STREAM flag, and CONTINUATION frame that does not carry payload
+ nor END_HEADERS flag.
Review Comment:
```
In this context, an "empty frame" means either a DATA frame that does not
carry a payload
nor an END_STREAM flag, or a CONTINUATION frame that does not carry
payload
nor an END_HEADERS flag.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
