Skip to content

Commit

Permalink
tcp: add platform check to avoid RST setting spam (envoyproxy#31919)
Browse files Browse the repository at this point in the history
---------

Signed-off-by: Boteng Yao <[email protected]>
  • Loading branch information
botengyao authored Jan 23, 2024
1 parent 8879434 commit ef9c1bf
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 2 deletions.
6 changes: 6 additions & 0 deletions envoy/common/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,9 @@ struct mmsghdr {
// On non-Linux platforms use 128 which is libevent listener default
#define ENVOY_TCP_BACKLOG_SIZE 128
#endif

#if defined(__linux__)
#define ENVOY_PLATFORM_ENABLE_SEND_RST 1
#else
#define ENVOY_PLATFORM_ENABLE_SEND_RST 0
#endif
2 changes: 2 additions & 0 deletions source/common/network/connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,14 @@ void ConnectionImpl::closeSocket(ConnectionEvent close_type) {

if (enable_rst_detect_send_ && (detected_close_type_ == DetectedCloseType::RemoteReset ||
detected_close_type_ == DetectedCloseType::LocalReset)) {
#if ENVOY_PLATFORM_ENABLE_SEND_RST
const bool ok = Network::Socket::applyOptions(
Network::SocketOptionFactory::buildZeroSoLingerOptions(), *socket_,
envoy::config::core::v3::SocketOption::STATE_LISTENING);
if (!ok) {
ENVOY_LOG_EVERY_POW_2(error, "rst setting so_linger=0 failed on connection {}", id());
}
#endif
}

// It is safe to call close() since there is an IO handle check.
Expand Down
6 changes: 5 additions & 1 deletion test/common/network/connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ TEST_P(ConnectionImplTest, SocketOptionsFailureTest) {
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

#if ENVOY_PLATFORM_ENABLE_SEND_RST
// Test that connection is AbortReset closed during callback.
TEST_P(ConnectionImplTest, ClientAbortResetDuringCallback) {
Network::ClientConnectionPtr upstream_connection_;
Expand Down Expand Up @@ -786,6 +787,7 @@ TEST_P(ConnectionImplTest, ServerResetCloseRuntimeDisabled) {
server_connection_->close(ConnectionCloseType::AbortReset);
dispatcher_->run(Event::Dispatcher::RunType::Block);
}
#endif

struct MockConnectionStats {
Connection::ConnectionStats toBufferStats() {
Expand Down Expand Up @@ -1111,6 +1113,7 @@ TEST_P(ConnectionImplTest, CloseOnReadDisableWithoutCloseDetection) {
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

#if ENVOY_PLATFORM_ENABLE_SEND_RST
// Test normal RST close without readDisable.
TEST_P(ConnectionImplTest, RstCloseOnNotReadDisabledConnection) {
setUpBasicConnection();
Expand Down Expand Up @@ -1188,6 +1191,7 @@ TEST_P(ConnectionImplTest, RstCloseOnReadEarlyCloseDisabledThenWrite) {
client_connection_->write(buffer, false);
dispatcher_->run(Event::Dispatcher::RunType::Block);
}
#endif

// Test that connection half-close is sent and received properly.
TEST_P(ConnectionImplTest, HalfClose) {
Expand Down Expand Up @@ -1229,6 +1233,7 @@ TEST_P(ConnectionImplTest, HalfClose) {
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

#if ENVOY_PLATFORM_ENABLE_SEND_RST
// Test that connection is immediately closed when RST is detected even
// half-close is enabled
TEST_P(ConnectionImplTest, HalfCloseResetClose) {
Expand Down Expand Up @@ -1346,7 +1351,6 @@ TEST_P(ConnectionImplTest, HalfCloseThenResetClose) {
dispatcher_->run(Event::Dispatcher::RunType::Block);
}

#if !defined(WIN32)
// Test that no remote close event will be propagated back to peer, when a connection is
// half-closed and then the connection is RST closed. Writing data to the half closed and then
// reset connection will lead to broken pipe error rather than reset error.
Expand Down
2 changes: 1 addition & 1 deletion test/integration/tcp_async_client_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ TEST_P(TcpAsyncClientIntegrationTest, MultipleResponseFrames) {
tcp_client->close();
}

#if ENVOY_PLATFORM_ENABLE_SEND_RST
// Test if RST close can be detected from downstream and upstream is closed by RST.
TEST_P(TcpAsyncClientIntegrationTest, TestClientCloseRST) {
enableHalfClose(true);
Expand Down Expand Up @@ -175,7 +176,6 @@ TEST_P(TcpAsyncClientIntegrationTest, TestUpstreamCloseRST) {
tcp_client->waitForDisconnect();
}

#if !defined(WIN32)
// Test the behaviour when the connection is half closed and then the connection is reset by
// the client. The behavior is different for windows, since RST support is literally supported for
// unix like system, disabled the test for windows.
Expand Down

0 comments on commit ef9c1bf

Please sign in to comment.