tcp connection: deprecate tcp detect and raise RST runtime guard (env…

…oyproxy#33633)

Signed-off-by: Boteng Yao <boteng@google.com>

changelogs/current.yaml

@@ -21,6 +21,9 @@ bug_fixes:
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
+- area: tcp
+  change: |
+    Removed ``envoy.reloadable_features.detect_and_raise_rst_tcp_connection`` runtime flag and legacy code paths.
 
 new_features:
 

source/common/network/connection_impl.cc

@@ -85,9 +85,6 @@ ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPt
       write_end_stream_(false), current_write_end_stream_(false), dispatch_buffered_data_(false),
       transport_wants_read_(false) {
 
-  // Keep it as a bool flag to reduce the times calling runtime method..
-  enable_rst_detect_send_ = Runtime::runtimeFeatureEnabled(
-      "envoy.reloadable_features.detect_and_raise_rst_tcp_connection");
   if (!socket_->isOpen()) {
     IS_ENVOY_BUG("Client socket failure");
     return;
@@ -148,12 +145,6 @@ void ConnectionImpl::close(ConnectionCloseType type) {
   ENVOY_CONN_LOG_EVENT(debug, "connection_closing", "closing data_to_write={} type={}", *this,
                        data_to_write, enumToInt(type));
 
-  // RST will be sent only if enable_rst_detect_send_ is true, otherwise it is converted to normal
-  // ConnectionCloseType::Abort.
-  if (!enable_rst_detect_send_ && type == ConnectionCloseType::AbortReset) {
-    type = ConnectionCloseType::Abort;
-  }
-
   // The connection is closed by Envoy by sending RST, and the connection is closed immediately.
   if (type == ConnectionCloseType::AbortReset) {
     ENVOY_CONN_LOG(
@@ -293,8 +284,8 @@ void ConnectionImpl::closeSocket(ConnectionEvent close_type) {
 
   connection_stats_.reset();
 
-  if (enable_rst_detect_send_ && (detected_close_type_ == DetectedCloseType::RemoteReset ||
-                                  detected_close_type_ == DetectedCloseType::LocalReset)) {
+  if (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_,
@@ -687,7 +678,7 @@ void ConnectionImpl::onReadReady() {
   updateReadBufferStats(result.bytes_processed_, new_buffer_size);
 
   // The socket is closed immediately when receiving RST.
-  if (enable_rst_detect_send_ && result.err_code_.has_value() &&
+  if (result.err_code_.has_value() &&
       result.err_code_ == Api::IoError::IoErrorCode::ConnectionReset) {
     ENVOY_CONN_LOG(trace, "read: rst close from peer", *this);
     if (result.bytes_processed_ != 0) {
@@ -771,7 +762,7 @@ void ConnectionImpl::onWriteReady() {
   updateWriteBufferStats(result.bytes_processed_, new_buffer_size);
 
   // The socket is closed immediately when receiving RST.
-  if (enable_rst_detect_send_ && result.err_code_.has_value() &&
+  if (result.err_code_.has_value() &&
       result.err_code_ == Api::IoError::IoErrorCode::ConnectionReset) {
     // Discard anything in the buffer.
     ENVOY_CONN_LOG(debug, "write: rst close from peer.", *this);

source/common/network/connection_impl.h

@@ -244,7 +244,6 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback
   // read_disable_count_ == 0 to ensure that read resumption happens when remaining bytes are held
   // in transport socket internal buffers.
   bool transport_wants_read_ : 1;
-  bool enable_rst_detect_send_ : 1;
 };
 
 class ServerConnectionImpl : public ConnectionImpl, virtual public ServerConnection {

source/common/runtime/runtime_features.cc

@@ -36,7 +36,6 @@ RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle);
 RUNTIME_GUARD(envoy_reloadable_features_convert_legacy_lb_config);
 RUNTIME_GUARD(envoy_reloadable_features_copy_response_code_to_downstream_stream_info);
 RUNTIME_GUARD(envoy_reloadable_features_defer_processing_backedup_streams);
-RUNTIME_GUARD(envoy_reloadable_features_detect_and_raise_rst_tcp_connection);
 RUNTIME_GUARD(envoy_reloadable_features_dfp_mixed_scheme);
 RUNTIME_GUARD(envoy_reloadable_features_disallow_quic_client_udp_mmsg);
 RUNTIME_GUARD(envoy_reloadable_features_dns_cache_set_first_resolve_complete);

test/common/network/connection_impl_test.cc

@@ -796,29 +796,6 @@ TEST_P(ConnectionImplTest, ServerResetClose) {
   server_connection_->close(ConnectionCloseType::AbortReset);
   dispatcher_->run(Event::Dispatcher::RunType::Block);
 }
-
-// Test the RST close and detection feature is disabled by runtime_guard.
-TEST_P(ConnectionImplTest, ServerResetCloseRuntimeDisabled) {
-  TestScopedRuntime scoped_runtime;
-  scoped_runtime.mergeValues(
-      {{"envoy.reloadable_features.detect_and_raise_rst_tcp_connection", "false"}});
-  setUpBasicConnection();
-  connect();
-
-  EXPECT_CALL(client_callbacks_, onEvent(ConnectionEvent::RemoteClose))
-      .WillOnce(InvokeWithoutArgs([&]() -> void {
-        EXPECT_EQ(client_connection_->detectedCloseType(), DetectedCloseType::Normal);
-        dispatcher_->exit();
-      }));
-  EXPECT_CALL(server_callbacks_, onEvent(ConnectionEvent::LocalClose))
-      .WillOnce(InvokeWithoutArgs([&]() -> void {
-        EXPECT_EQ(server_connection_->detectedCloseType(), DetectedCloseType::Normal);
-      }));
-
-  // Originally it should close the connection by RST.
-  server_connection_->close(ConnectionCloseType::AbortReset);
-  dispatcher_->run(Event::Dispatcher::RunType::Block);
-}
 #endif
 
 struct MockConnectionStats {