listener: Add configurable accepted connection limits (#153)

Add support for per-listener limits on accepted connections. Signed-off-by: Tony Allen <[email protected]> Signed-off-by: Lizan Zhou <[email protected]>
envoyproxy · Jun 13, 2020 · ad64144 · ad64144
1 parent 0f26e68
commit ad64144
Show file tree

Hide file tree

Showing 27 changed files with 350 additions and 15 deletions.
diff --git a/docs/root/configuration/best_practices/edge.rst b/docs/root/configuration/best_practices/edge.rst
@@ -23,6 +23,8 @@ HTTP proxies should additionally configure:
 * :ref:`HTTP/2 maximum concurrent streams limit <envoy_api_field_core.Http2ProtocolOptions.max_concurrent_streams>` to 100,
 * :ref:`HTTP/2 initial stream window size limit <envoy_api_field_core.Http2ProtocolOptions.initial_stream_window_size>` to 64 KiB,
 * :ref:`HTTP/2 initial connection window size limit <envoy_api_field_core.Http2ProtocolOptions.initial_connection_window_size>` to 1 MiB.
+* :ref:`headers_with_underscores_action setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>` to REJECT_REQUEST, to protect upstream services that treat '_' and '-' as interchangeable.
+* :ref:`Connection limits. <config_listeners_runtime>`
 
 The following is a YAML example of the above recommendation.
 
@@ -109,3 +111,13 @@ The following is a YAML example of the above recommendation.
       http2_protocol_options:
         initial_stream_window_size: 65536 # 64 KiB
         initial_connection_window_size: 1048576 # 1 MiB
+
+  layered_runtime:
+    layers:
+      - name: static_layer_0
+        static_layer:
+          envoy:
+            resource_limits:
+              listener:
+                example_listener_name:
+                  connection_limit: 10000
diff --git a/docs/root/configuration/listeners/listeners.rst b/docs/root/configuration/listeners/listeners.rst
@@ -8,6 +8,7 @@ Listeners
 
   overview
   stats
+  runtime
   listener_filters/listener_filters
   network_filters/network_filters
   udp_filters/udp_filters

diff --git a/docs/root/configuration/listeners/runtime.rst b/docs/root/configuration/listeners/runtime.rst
@@ -0,0 +1,8 @@
+.. _config_listeners_runtime:
+
+Runtime
+-------
+The following runtime settings are supported:
+
+envoy.resource_limits.listener.<name of listener>.connection_limit
+    Sets a limit on the number of active connections to the specified listener.
diff --git a/docs/root/configuration/listeners/stats.rst b/docs/root/configuration/listeners/stats.rst
@@ -16,6 +16,7 @@ Every listener has a statistics tree rooted at *listener.<address>.* with the fo
    downstream_cx_destroy, Counter, Total destroyed connections
    downstream_cx_active, Gauge, Total active connections
    downstream_cx_length_ms, Histogram, Connection length milliseconds
+   downstream_cx_overflow, Counter, Total connections rejected due to enforcement of listener connection limit
    downstream_pre_cx_timeout, Counter, Sockets that timed out during listener filter processing
    downstream_pre_cx_active, Gauge, Sockets currently undergoing listener filter processing
    no_filter_chain_match, Counter, Total connections that didn't match any filter chain

diff --git a/docs/root/faq/configuration/resource_limits.rst b/docs/root/faq/configuration/resource_limits.rst
@@ -0,0 +1,16 @@
+.. _faq_resource_limits:
+
+How does Envoy prevent file descriptor exhaustion?
+==================================================
+
+:ref:`Per-listener connection limits <config_listeners_runtime>` may be configured as an upper bound on
+the number of active connections a particular listener will accept. The listener may accept more
+connections than the configured value on the order of the number of worker threads. On Unix-based
+systems, it is recommended to keep the sum of all connection limits less than half of the system's
+file descriptor limit to account for upstream connections, files, and other usage of file
+descriptors.
+
+.. note::
+
+    This per-listener connection limiting will eventually be handled by the :ref:`overload manager
+    <arch_overview_overload_manager>`.
diff --git a/docs/root/faq/overview.rst b/docs/root/faq/overview.rst
@@ -33,6 +33,7 @@ Configuration
   configuration/zipkin_tracing
   configuration/flow_control
   configuration/timeouts
+  configuration/resource_limits
 
 Load balancing
 --------------

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -3,6 +3,7 @@ Version history
 
 1.13.3 (Pending)
 ================
+* listener: add runtime support for `per-listener limits <config_listeners_runtime>` on active/accepted connections.
 
 1.13.2 (June 8, 2020)
 =====================

diff --git a/examples/front-proxy/front-envoy.yaml b/examples/front-proxy/front-envoy.yaml
@@ -4,6 +4,7 @@ static_resources:
       socket_address:
         address: 0.0.0.0
         port_value: 80
+    name: example_listener_name
     filter_chains:
     - filters:
       - name: envoy.http_connection_manager
@@ -64,3 +65,12 @@ admin:
     socket_address:
       address: 0.0.0.0
       port_value: 8001
+layered_runtime:
+  layers:
+    - name: static_layer_0
+      static_layer:
+        envoy:
+          resource_limits:
+            listener:
+              example_listener_name:
+                connection_limit: 10000
diff --git a/include/envoy/network/listener.h b/include/envoy/network/listener.h
@@ -6,6 +6,7 @@
 
 #include "envoy/api/io_error.h"
 #include "envoy/common/exception.h"
+#include "envoy/common/resource.h"
 #include "envoy/config/core/v3/base.pb.h"
 #include "envoy/network/connection.h"
 #include "envoy/network/connection_balancer.h"
@@ -142,6 +143,11 @@ class ListenerConfig {
    *         though the implementation may be a NOP balancer.
    */
   virtual ConnectionBalancer& connectionBalancer() PURE;
+
+  /**
+   * Open connection resources for this listener.
+   */
+  virtual ResourceLimit& openConnections() PURE;
 };
 
 /**

diff --git a/source/common/upstream/resource_manager_impl.h b/source/common/upstream/resource_manager_impl.h
@@ -61,13 +61,14 @@ class ResourceManagerImpl : public ResourceManager {
       remaining_.set(max);
     }
 
-    // Upstream::Resource
-    bool canCreate() override { return current_ < max(); }
+    ~ManagedResourceImpl() override { ASSERT(count() == 0); }
+
     void inc() override {
       BasicResourceLimitImpl::inc();
       updateRemaining();
       open_gauge_.set(BasicResourceLimitImpl::canCreate() ? 0 : 1);
     }
+
     void decBy(uint64_t amount) override {
       BasicResourceLimitImpl::decBy(amount);
       updateRemaining();

diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc
@@ -292,6 +292,14 @@ void ConnectionHandlerImpl::ActiveTcpSocket::newConnection() {
 }
 
 void ConnectionHandlerImpl::ActiveTcpListener::onAccept(Network::ConnectionSocketPtr&& socket) {
+  if (listenerConnectionLimitReached()) {
+    ENVOY_LOG(trace, "closing connection: listener connection limit reached for {}",
+              config_.name());
+    socket->close();
+    stats_.downstream_cx_overflow_.inc();
+    return;
+  }
+
   onAcceptWorker(std::move(socket), config_.handOffRestoredDestinationConnections(), false);
 }
 

diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h
@@ -28,6 +28,7 @@ namespace Server {
 #define ALL_LISTENER_STATS(COUNTER, GAUGE, HISTOGRAM)                                              \
   COUNTER(downstream_cx_destroy)                                                                   \
   COUNTER(downstream_cx_total)                                                                     \
+  COUNTER(downstream_cx_overflow)                                                                  \
   COUNTER(downstream_pre_cx_timeout)                                                               \
   COUNTER(no_filter_chain_match)                                                                   \
   GAUGE(downstream_cx_active, Accumulate)                                                          \
@@ -108,11 +109,17 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler,
     ActiveTcpListener(ConnectionHandlerImpl& parent, Network::ListenerPtr&& listener,
                       Network::ListenerConfig& config);
     ~ActiveTcpListener() override;
+    bool listenerConnectionLimitReached() const {
+      // TODO(tonya11en): Delegate enforcement of per-listener connection limits to overload
+      // manager.
+      return !config_.openConnections().canCreate();
+    }
     void onAcceptWorker(Network::ConnectionSocketPtr&& socket,
                         bool hand_off_restored_destination_connections, bool rebalanced);
     void decNumConnections() {
       ASSERT(num_listener_connections_ > 0);
       --num_listener_connections_;
+      config_.openConnections().dec();
     }
 
     // Network::ListenerCallbacks
@@ -124,7 +131,10 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler,
 
     // Network::BalancedConnectionHandler
     uint64_t numConnections() const override { return num_listener_connections_; }
-    void incNumConnections() override { ++num_listener_connections_; }
+    void incNumConnections() override {
+      ++num_listener_connections_;
+      config_.openConnections().inc();
+    }
     void post(Network::ConnectionSocketPtr&& socket) override;
 
     /**

diff --git a/source/server/http/BUILD b/source/server/http/BUILD
@@ -32,6 +32,7 @@ envoy_cc_library(
         "//source/common/access_log:access_log_lib",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:assert_lib",
+        "//source/common/common:basic_resource_lib",
         "//source/common/common:empty_string",
         "//source/common/common:enum_to_int",
         "//source/common/common:macros",

diff --git a/source/server/http/admin.h b/source/server/http/admin.h
@@ -23,6 +23,7 @@
 #include "envoy/upstream/outlier_detection.h"
 #include "envoy/upstream/resource_manager.h"
 
+#include "common/common/basic_resource_impl.h"
 #include "common/common/empty_string.h"
 #include "common/common/logger.h"
 #include "common/common/macros.h"
@@ -404,12 +405,14 @@ class AdminImpl : public Admin,
       return envoy::config::core::v3::UNSPECIFIED;
     }
     Network::ConnectionBalancer& connectionBalancer() override { return connection_balancer_; }
+    ResourceLimit& openConnections() override { return open_connections_; }
 
     AdminImpl& parent_;
     const std::string name_;
     Stats::ScopePtr scope_;
     Http::ConnectionManagerListenerStats stats_;
     Network::NopConnectionBalancerImpl connection_balancer_;
+    BasicResourceLimitImpl open_connections_;
   };
   using AdminListenerPtr = std::unique_ptr<AdminListener>;
 

diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc
@@ -147,7 +147,11 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
       config_(config), version_info_(version_info),
       listener_filters_timeout_(
           PROTOBUF_GET_MS_OR_DEFAULT(config, listener_filters_timeout, 15000)),
-      continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()) {
+      continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()),
+      cx_limit_runtime_key_("envoy.resource_limits.listener." + config_.name() +
+                            ".connection_limit"),
+      open_connections_(std::make_shared<BasicResourceLimitImpl>(
+          std::numeric_limits<uint64_t>::max(), parent.server_.runtime(), cx_limit_runtime_key_)) {
   Network::Address::SocketType socket_type =
       Network::Utility::protobufAddressSocketType(config.address());
   if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, transparent, false)) {
@@ -183,6 +187,15 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
     udp_listener_factory_ = config_factory.createActiveUdpListenerFactory(*message);
   }
 
+  const absl::optional<std::string> runtime_val =
+      parent_.server_.runtime().snapshot().get(cx_limit_runtime_key_);
+  if (runtime_val && runtime_val->empty()) {
+    ENVOY_LOG(warn,
+              "Listener connection limit runtime key {} is empty. There are currently no "
+              "limitations on the number of accepted connections for listener {}.",
+              cx_limit_runtime_key_, config_.name());
+  }
+
   if (!config.listener_filters().empty()) {
     switch (socket_type) {
     case Network::Address::SocketType::Datagram:

diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h
@@ -10,6 +10,7 @@
 #include "envoy/server/listener_manager.h"
 #include "envoy/stats/scope.h"
 
+#include "common/common/basic_resource_impl.h"
 #include "common/common/logger.h"
 #include "common/init/manager_impl.h"
 
@@ -150,6 +151,14 @@ class ListenerImpl : public Network::ListenerConfig,
     return udp_listener_factory_.get();
   }
   Network::ConnectionBalancer& connectionBalancer() override { return *connection_balancer_; }
+  ResourceLimit& openConnections() override { return *open_connections_; }
+
+  void ensureSocketOptions() {
+    if (!listen_socket_options_) {
+      listen_socket_options_ =
+          std::make_shared<std::vector<Network::Socket::OptionConstSharedPtr>>();
+    }
+  }
 
   // Server::Configuration::ListenerFactoryContext
   AccessLog::AccessLogManager& accessLogManager() override;
@@ -179,12 +188,6 @@ class ListenerImpl : public Network::ListenerConfig,
   OptProcessContextRef processContext() override;
   Configuration::ServerFactoryContext& getServerFactoryContext() const override;
 
-  void ensureSocketOptions() {
-    if (!listen_socket_options_) {
-      listen_socket_options_ =
-          std::make_shared<std::vector<Network::Socket::OptionConstSharedPtr>>();
-    }
-  }
   // Network::DrainDecision
   bool drainClose() const override;
 
@@ -202,6 +205,7 @@ class ListenerImpl : public Network::ListenerConfig,
     ensureSocketOptions();
     listen_socket_options_->emplace_back(std::move(option));
   }
+
   void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) {
     ensureSocketOptions();
     Network::Socket::appendOptions(listen_socket_options_, options);
@@ -243,6 +247,12 @@ class ListenerImpl : public Network::ListenerConfig,
   Network::ActiveUdpListenerFactoryPtr udp_listener_factory_;
   Network::ConnectionBalancerPtr connection_balancer_;
 
+  // Per-listener connection limits are only specified via runtime.
+  //
+  // TODO (tonya11en): Move this functionality into the overload manager.
+  const std::string cx_limit_runtime_key_;
+  std::shared_ptr<BasicResourceLimitImpl> open_connections_;
+
   // to access ListenerManagerImpl::factory_.
   friend class ListenerFilterChainFactoryBuilder;
 };

diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc
@@ -81,6 +81,7 @@ class ProxyProtocolTest : public testing::TestWithParam<Network::Address::IpVers
   bool continueOnListenerFiltersTimeout() const override { return false; }
   Stats::Scope& listenerScope() override { return stats_store_; }
   uint64_t listenerTag() const override { return 1; }
+  ResourceLimit& openConnections() override { return open_connections_; }
   const std::string& name() const override { return name_; }
   const Network::ActiveUdpListenerFactory* udpListenerFactory() override { return nullptr; }
   envoy::config::core::v3::TrafficDirection direction() const override {
@@ -179,6 +180,7 @@ class ProxyProtocolTest : public testing::TestWithParam<Network::Address::IpVers
   NiceMock<Network::MockConnectionCallbacks> connection_callbacks_;
   Network::Connection* server_connection_;
   Network::MockConnectionCallbacks server_callbacks_;
+  BasicResourceLimitImpl open_connections_;
   std::shared_ptr<Network::MockReadFilter> read_filter_;
   std::string name_;
   const Network::FilterChainSharedPtr filter_chain_;
@@ -939,9 +941,8 @@ class WildcardProxyProtocolTest : public testing::TestWithParam<Network::Address
   bool bindToPort() override { return true; }
   bool handOffRestoredDestinationConnections() const override { return false; }
   uint32_t perConnectionBufferLimitBytes() const override { return 0; }
-  std::chrono::milliseconds listenerFiltersTimeout() const override {
-    return std::chrono::milliseconds();
-  }
+  std::chrono::milliseconds listenerFiltersTimeout() const override { return {}; }
+  ResourceLimit& openConnections() override { return open_connections_; }
   bool continueOnListenerFiltersTimeout() const override { return false; }
   Stats::Scope& listenerScope() override { return stats_store_; }
   uint64_t listenerTag() const override { return 1; }
@@ -1003,6 +1004,7 @@ class WildcardProxyProtocolTest : public testing::TestWithParam<Network::Address
   Stats::IsolatedStoreImpl stats_store_;
   Api::ApiPtr api_;
   Event::DispatcherPtr dispatcher_;
+  BasicResourceLimitImpl open_connections_;
   Network::MockListenSocketFactory socket_factory_;
   std::shared_ptr<Network::TcpListenSocket> socket_;
   Network::Address::InstanceConstSharedPtr local_dst_address_;

diff --git a/test/integration/BUILD b/test/integration/BUILD
@@ -517,6 +517,7 @@ envoy_cc_test_library(
         "//source/common/buffer:buffer_lib",
         "//source/common/buffer:zero_copy_input_stream_lib",
         "//source/common/common:assert_lib",
+        "//source/common/common:basic_resource_lib",
         "//source/common/common:minimal_logger_lib",
         "//source/common/config:api_version_lib",
         "//source/common/config:version_converter_lib",
@@ -1032,3 +1033,18 @@ envoy_cc_test(
         "//test/test_common:utility_lib",
     ],
 )
+
+envoy_cc_test(
+    name = "cx_limit_integration_test",
+    srcs = ["cx_limit_integration_test.cc"],
+    deps = [
+        ":http_integration_lib",
+        "//include/envoy/network:filter_interface",
+        "//include/envoy/registry",
+        "//source/extensions/filters/network/tcp_proxy:config",
+        "//test/config:utility_lib",
+        "//test/test_common:logging_lib",
+        "//test/test_common:simulated_time_system_lib",
+        "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
+    ],
+)