From 039b440b87214b74c8be11d552f6124d638766e6 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Thu, 30 Dec 2021 08:01:08 +0000 Subject: [PATCH 01/19] Enable to create ActiveTcpListener through the socket Signed-off-by: He Jie Xu --- source/server/active_tcp_listener.cc | 13 ++++++------- source/server/active_tcp_listener.h | 2 +- source/server/connection_handler_impl.cc | 6 ++++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/source/server/active_tcp_listener.cc b/source/server/active_tcp_listener.cc index 78b435c0aad62..3581e72ad58ad 100644 --- a/source/server/active_tcp_listener.cc +++ b/source/server/active_tcp_listener.cc @@ -14,13 +14,12 @@ namespace Server { ActiveTcpListener::ActiveTcpListener(Network::TcpConnectionHandler& parent, Network::ListenerConfig& config, Runtime::Loader& runtime, - uint32_t worker_index) - : OwnedActiveStreamListenerBase(parent, parent.dispatcher(), - parent.dispatcher().createListener( - config.listenSocketFactory().getListenSocket(worker_index), - *this, runtime, config.bindToPort(), - config.ignoreGlobalConnLimit()), - config), + Network::SocketSharedPtr&& socket) + : OwnedActiveStreamListenerBase( + parent, parent.dispatcher(), + parent.dispatcher().createListener(std::move(socket), *this, runtime, config.bindToPort(), + config.ignoreGlobalConnLimit()), + config), tcp_conn_handler_(parent) { config.connectionBalancer().registerHandler(*this); } diff --git a/source/server/active_tcp_listener.h b/source/server/active_tcp_listener.h index ba3fc15fbdb63..6e9fd2c2a541b 100644 --- a/source/server/active_tcp_listener.h +++ b/source/server/active_tcp_listener.h @@ -27,7 +27,7 @@ class ActiveTcpListener final : public Network::TcpListenerCallbacks, public Network::BalancedConnectionHandler { public: ActiveTcpListener(Network::TcpConnectionHandler& parent, Network::ListenerConfig& config, - Runtime::Loader& runtime, uint32_t worker_index); + Runtime::Loader& runtime, Network::SocketSharedPtr&& socket); ActiveTcpListener(Network::TcpConnectionHandler& parent, Network::ListenerPtr&& listener, Network::ListenerConfig& config, Runtime::Loader& runtime); ~ActiveTcpListener() override; diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 94699da0ed53a..06505cecb10f3 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -71,8 +71,10 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list IS_ENVOY_BUG("unexpected"); } // worker_index_ doesn't have a value on the main thread for the admin server. - auto tcp_listener = std::make_unique( - *this, config, runtime, worker_index_.has_value() ? *worker_index_ : 0); + auto tcp_listener = + std::make_unique(*this, config, runtime, + config.listenSocketFactory().getListenSocket( + worker_index_.has_value() ? *worker_index_ : 0)); details->typed_listener_ = *tcp_listener; details->listener_ = std::move(tcp_listener); } else { From 52b6994ca27bb59d1b050c9d73660c5c141d2813 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Thu, 30 Dec 2021 08:17:02 +0000 Subject: [PATCH 02/19] Create UDP listener through the socket Signed-off-by: He Jie Xu quic: remove useless constructor Signed-off-by: He Jie Xu udp: remove useless constructor Signed-off-by: He Jie Xu --- envoy/network/connection_handler.h | 6 +++-- source/common/quic/active_quic_listener.cc | 23 ++++--------------- source/common/quic/active_quic_listener.h | 17 ++++---------- .../server/active_raw_udp_listener_config.cc | 5 ++-- .../server/active_raw_udp_listener_config.h | 5 ++-- source/server/active_udp_listener.cc | 8 ------- source/server/active_udp_listener.h | 3 --- source/server/connection_handler_impl.cc | 3 ++- test/common/quic/active_quic_listener_test.cc | 4 +++- test/server/active_udp_listener_test.cc | 4 ++-- 10 files changed, 26 insertions(+), 52 deletions(-) diff --git a/envoy/network/connection_handler.h b/envoy/network/connection_handler.h index 1b5ff88200158..e2b0cd78e2911 100644 --- a/envoy/network/connection_handler.h +++ b/envoy/network/connection_handler.h @@ -219,6 +219,7 @@ class ActiveUdpListenerFactory { * @param runtime the runtime for this server. * @param worker_index The index of the worker this listener is being created on. * @param parent is the owner of the created ActiveListener objects. + * @param listen_socket_ptr is the UDP socket. * @param dispatcher is used to create actual UDP listener. * @param config provides information needed to create ActiveUdpListener and * UdpListener objects. @@ -226,8 +227,9 @@ class ActiveUdpListenerFactory { */ virtual ConnectionHandler::ActiveUdpListenerPtr createActiveUdpListener(Runtime::Loader& runtime, uint32_t worker_index, - UdpConnectionHandler& parent, Event::Dispatcher& dispatcher, - Network::ListenerConfig& config) PURE; + UdpConnectionHandler& parent, + Network::SocketSharedPtr&& listen_socket_ptr, + Event::Dispatcher& dispatcher, Network::ListenerConfig& config) PURE; /** * @return true if the UDP passing through listener doesn't form stateful connections. diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc index 17775ef19f663..8cf1db5699bdf 100644 --- a/source/common/quic/active_quic_listener.cc +++ b/source/common/quic/active_quic_listener.cc @@ -26,21 +26,7 @@ bool ActiveQuicListenerFactory::disable_kernel_bpf_packet_routing_for_test_ = fa ActiveQuicListener::ActiveQuicListener( Runtime::Loader& runtime, uint32_t worker_index, uint32_t concurrency, Event::Dispatcher& dispatcher, Network::UdpConnectionHandler& parent, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - bool kernel_worker_routing, const envoy::config::core::v3::RuntimeFeatureFlag& enabled, - QuicStatNames& quic_stat_names, uint32_t packets_received_to_connection_count_ratio, - EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory, - EnvoyQuicProofSourceFactoryInterface& proof_source_factory) - : ActiveQuicListener(runtime, worker_index, concurrency, dispatcher, parent, - listener_config.listenSocketFactory().getListenSocket(worker_index), - listener_config, quic_config, kernel_worker_routing, enabled, - quic_stat_names, packets_received_to_connection_count_ratio, - crypto_server_stream_factory, proof_source_factory) {} - -ActiveQuicListener::ActiveQuicListener( - Runtime::Loader& runtime, uint32_t worker_index, uint32_t concurrency, - Event::Dispatcher& dispatcher, Network::UdpConnectionHandler& parent, - Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, + Network::SocketSharedPtr&& listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, bool kernel_worker_routing, const envoy::config::core::v3::RuntimeFeatureFlag& enabled, QuicStatNames& quic_stat_names, uint32_t packets_to_read_to_connection_count_ratio, @@ -338,11 +324,12 @@ ActiveQuicListenerFactory::ActiveQuicListenerFactory( Network::ConnectionHandler::ActiveUdpListenerPtr ActiveQuicListenerFactory::createActiveUdpListener( Runtime::Loader& runtime, uint32_t worker_index, Network::UdpConnectionHandler& parent, - Event::Dispatcher& disptacher, Network::ListenerConfig& config) { + Network::SocketSharedPtr&& listen_socket_ptr, Event::Dispatcher& disptacher, + Network::ListenerConfig& config) { ASSERT(crypto_server_stream_factory_.has_value()); return std::make_unique( - runtime, worker_index, concurrency_, disptacher, parent, config, quic_config_, - kernel_worker_routing_, enabled_, quic_stat_names_, + runtime, worker_index, concurrency_, disptacher, parent, std::move(listen_socket_ptr), config, + quic_config_, kernel_worker_routing_, enabled_, quic_stat_names_, packets_to_read_to_connection_count_ratio_, crypto_server_stream_factory_.value(), proof_source_factory_.value()); } diff --git a/source/common/quic/active_quic_listener.h b/source/common/quic/active_quic_listener.h index 6feda1abc7c22..b8a1b3a58d118 100644 --- a/source/common/quic/active_quic_listener.h +++ b/source/common/quic/active_quic_listener.h @@ -30,17 +30,7 @@ class ActiveQuicListener : public Envoy::Server::ActiveUdpListenerBase, ActiveQuicListener(Runtime::Loader& runtime, uint32_t worker_index, uint32_t concurrency, Event::Dispatcher& dispatcher, Network::UdpConnectionHandler& parent, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - bool kernel_worker_routing, - const envoy::config::core::v3::RuntimeFeatureFlag& enabled, - QuicStatNames& quic_stat_names, - uint32_t packets_to_read_to_connection_count_ratio, - EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory, - EnvoyQuicProofSourceFactoryInterface& proof_source_factory); - - ActiveQuicListener(Runtime::Loader& runtime, uint32_t worker_index, uint32_t concurrency, - Event::Dispatcher& dispatcher, Network::UdpConnectionHandler& parent, - Network::SocketSharedPtr listen_socket, + Network::SocketSharedPtr&& listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, bool kernel_worker_routing, const envoy::config::core::v3::RuntimeFeatureFlag& enabled, @@ -108,8 +98,9 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory, // Network::ActiveUdpListenerFactory. Network::ConnectionHandler::ActiveUdpListenerPtr createActiveUdpListener(Runtime::Loader& runtime, uint32_t worker_index, - Network::UdpConnectionHandler& parent, Event::Dispatcher& disptacher, - Network::ListenerConfig& config) override; + Network::UdpConnectionHandler& parent, + Network::SocketSharedPtr&& listen_socket_ptr, + Event::Dispatcher& disptacher, Network::ListenerConfig& config) override; bool isTransportConnectionless() const override { return false; } const Network::Socket::OptionsSharedPtr& socketOptions() const override { return options_; } diff --git a/source/server/active_raw_udp_listener_config.cc b/source/server/active_raw_udp_listener_config.cc index e149dadf00716..dca62508ed01e 100644 --- a/source/server/active_raw_udp_listener_config.cc +++ b/source/server/active_raw_udp_listener_config.cc @@ -15,10 +15,11 @@ ActiveRawUdpListenerFactory::ActiveRawUdpListenerFactory(uint32_t concurrency) Network::ConnectionHandler::ActiveUdpListenerPtr ActiveRawUdpListenerFactory::createActiveUdpListener(Runtime::Loader&, uint32_t worker_index, Network::UdpConnectionHandler& parent, + Network::SocketSharedPtr&& listen_socket_ptr, Event::Dispatcher& dispatcher, Network::ListenerConfig& config) { - return std::make_unique(worker_index, concurrency_, parent, dispatcher, - config); + return std::make_unique(worker_index, concurrency_, parent, + std::move(listen_socket_ptr), dispatcher, config); } } // namespace Server diff --git a/source/server/active_raw_udp_listener_config.h b/source/server/active_raw_udp_listener_config.h index 4b79d9ed6d4f5..a94e31e0c1fe3 100644 --- a/source/server/active_raw_udp_listener_config.h +++ b/source/server/active_raw_udp_listener_config.h @@ -11,8 +11,9 @@ class ActiveRawUdpListenerFactory : public Network::ActiveUdpListenerFactory { Network::ConnectionHandler::ActiveUdpListenerPtr createActiveUdpListener(Runtime::Loader&, uint32_t worker_index, - Network::UdpConnectionHandler& parent, Event::Dispatcher& disptacher, - Network::ListenerConfig& config) override; + Network::UdpConnectionHandler& parent, + Network::SocketSharedPtr&& listen_socket_ptr, + Event::Dispatcher& disptacher, Network::ListenerConfig& config) override; bool isTransportConnectionless() const override { return true; } const Network::Socket::OptionsSharedPtr& socketOptions() const override { return options_; } diff --git a/source/server/active_udp_listener.cc b/source/server/active_udp_listener.cc index e0d4c64f73f89..64a46a0c68ef5 100644 --- a/source/server/active_udp_listener.cc +++ b/source/server/active_udp_listener.cc @@ -63,14 +63,6 @@ void ActiveUdpListenerBase::onData(Network::UdpRecvData&& data) { } } -ActiveRawUdpListener::ActiveRawUdpListener(uint32_t worker_index, uint32_t concurrency, - Network::UdpConnectionHandler& parent, - Event::Dispatcher& dispatcher, - Network::ListenerConfig& config) - : ActiveRawUdpListener(worker_index, concurrency, parent, - config.listenSocketFactory().getListenSocket(worker_index), dispatcher, - config) {} - ActiveRawUdpListener::ActiveRawUdpListener(uint32_t worker_index, uint32_t concurrency, Network::UdpConnectionHandler& parent, Network::SocketSharedPtr listen_socket_ptr, diff --git a/source/server/active_udp_listener.h b/source/server/active_udp_listener.h index dff9406c44366..37e559828cf5d 100644 --- a/source/server/active_udp_listener.h +++ b/source/server/active_udp_listener.h @@ -65,9 +65,6 @@ class ActiveRawUdpListener : public ActiveUdpListenerBase, public Network::UdpReadFilterCallbacks, Logger::Loggable { public: - ActiveRawUdpListener(uint32_t worker_index, uint32_t concurrency, - Network::UdpConnectionHandler& parent, Event::Dispatcher& dispatcher, - Network::ListenerConfig& config); ActiveRawUdpListener(uint32_t worker_index, uint32_t concurrency, Network::UdpConnectionHandler& parent, Network::SocketSharedPtr listen_socket_ptr, Event::Dispatcher& dispatcher, diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 06505cecb10f3..0427dc90d7845 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -82,7 +82,8 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list ASSERT(worker_index_.has_value()); ConnectionHandler::ActiveUdpListenerPtr udp_listener = config.udpListenerConfig()->listenerFactory().createActiveUdpListener( - runtime, *worker_index_, *this, dispatcher_, config); + runtime, *worker_index_, *this, + config.listenSocketFactory().getListenSocket(*worker_index_), dispatcher_, config); details->typed_listener_ = *udp_listener; details->listener_ = std::move(udp_listener); } diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index 85a9955560580..e54bf114f5697 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -120,7 +120,9 @@ class ActiveQuicListenerTest : public testing::TestWithParam(listener_factory_->createActiveUdpListener( - scoped_runtime_.loader(), 0, connection_handler_, *dispatcher_, listener_config_)); + scoped_runtime_.loader(), 0, connection_handler_, + listener_config_.listenSocketFactory().getListenSocket(0), *dispatcher_, + listener_config_)); quic_dispatcher_ = ActiveQuicListenerPeer::quicDispatcher(*quic_listener_); quic::QuicCryptoServerConfig& crypto_config = ActiveQuicListenerPeer::cryptoConfig(*quic_listener_); diff --git a/test/server/active_udp_listener_test.cc b/test/server/active_udp_listener_test.cc index 7337c0d2e5550..62eea4f252170 100644 --- a/test/server/active_udp_listener_test.cc +++ b/test/server/active_udp_listener_test.cc @@ -77,8 +77,8 @@ class ActiveUdpListenerTest : public testing::TestWithParam(0, 1, conn_handler_, dispatcher_, listener_config_); + active_listener_ = std::make_unique(0, 1, conn_handler_, listen_socket_, + dispatcher_, listener_config_); } std::string listener_stat_prefix_{"listener_stat_prefix"}; From 16774552c781b9d33c4ceb78b22bcb78662188dd Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 01:40:34 +0000 Subject: [PATCH 03/19] ListenerImpl: add listenSocketFactories interface Signed-off-by: He Jie Xu --- envoy/network/listener.h | 5 +++++ source/server/admin/admin.cc | 2 +- source/server/admin/admin.h | 7 +++++-- source/server/listener_impl.cc | 4 ++-- source/server/listener_impl.h | 9 ++++++--- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/envoy/network/listener.h b/envoy/network/listener.h index 2a4c3f97908ce..c37bac1b8aead 100644 --- a/envoy/network/listener.h +++ b/envoy/network/listener.h @@ -150,6 +150,11 @@ class ListenerConfig { */ virtual ListenSocketFactory& listenSocketFactory() PURE; + /** + * @return std::vector& the factories to create listen sockets. + */ + virtual std::vector& listenSocketFactories() PURE; + /** * @return bool specifies whether the listener should actually listen on the port. * A listener that doesn't listen on a port can only receive connections diff --git a/source/server/admin/admin.cc b/source/server/admin/admin.cc index 629b8fcda0171..9efe33c00a191 100644 --- a/source/server/admin/admin.cc +++ b/source/server/admin/admin.cc @@ -128,7 +128,7 @@ void AdminImpl::startHttpListener(const std::list& socket_ = std::make_shared(address, socket_options, true); RELEASE_ASSERT(0 == socket_->ioHandle().listen(ENVOY_TCP_BACKLOG_SIZE).return_value_, "listen() failed on admin listener"); - socket_factory_ = std::make_unique(socket_); + socket_factories_.emplace_back(std::make_unique(socket_)); listener_ = std::make_unique(*this, std::move(listener_scope)); ENVOY_LOG(info, "admin address: {}", socket().connectionInfoProvider().localAddress()->asString()); diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 151d10ba6eee9..60cfce495d87a 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -390,7 +390,10 @@ class AdminImpl : public Admin, Network::FilterChainManager& filterChainManager() override { return parent_; } Network::FilterChainFactory& filterChainFactory() override { return parent_; } Network::ListenSocketFactory& listenSocketFactory() override { - return *parent_.socket_factory_; + return *parent_.socket_factories_[0]; + } + std::vector& listenSocketFactories() override { + return parent_.socket_factories_; } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } @@ -495,7 +498,7 @@ class AdminImpl : public Admin, ConfigTrackerImpl config_tracker_; const Network::FilterChainSharedPtr admin_filter_chain_; Network::SocketSharedPtr socket_; - Network::ListenSocketFactoryPtr socket_factory_; + std::vector socket_factories_; AdminListenerPtr listener_; const AdminInternalAddressConfig internal_address_config_; const LocalReply::LocalReplyPtr local_reply_; diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index cdcbceab6f13c..bb23e7f946a68 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -863,8 +863,8 @@ ListenerImpl::~ListenerImpl() { Init::Manager& ListenerImpl::initManager() { return *dynamic_init_manager_; } void ListenerImpl::setSocketFactory(Network::ListenSocketFactoryPtr&& socket_factory) { - ASSERT(!socket_factory_); - socket_factory_ = std::move(socket_factory); + ASSERT(socket_factories_.size() == 0); + socket_factories_.emplace_back(std::move(socket_factory)); } bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::Listener& config, diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index 8e2eee26d7bfe..11f97e1505601 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -284,7 +284,7 @@ class ListenerImpl final : public Network::ListenerConfig, Network::Address::InstanceConstSharedPtr address() const { return address_; } const envoy::config::listener::v3::Listener& config() const { return config_; } - const Network::ListenSocketFactory& getSocketFactory() const { return *socket_factory_; } + const Network::ListenSocketFactory& getSocketFactory() const { return *socket_factories_[0]; } void debugLog(const std::string& message); void initialize(); DrainManager& localDrainManager() const { @@ -304,7 +304,10 @@ class ListenerImpl final : public Network::ListenerConfig, // Network::ListenerConfig Network::FilterChainManager& filterChainManager() override { return filter_chain_manager_; } Network::FilterChainFactory& filterChainFactory() override { return *this; } - Network::ListenSocketFactory& listenSocketFactory() override { return *socket_factory_; } + Network::ListenSocketFactory& listenSocketFactory() override { return *socket_factories_[0]; } + std::vector& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return bind_to_port_; } bool mptcpEnabled() { return mptcp_enabled_; } bool handOffRestoredDestinationConnections() const override { @@ -418,7 +421,7 @@ class ListenerImpl final : public Network::ListenerConfig, ListenerManagerImpl& parent_; Network::Address::InstanceConstSharedPtr address_; - Network::ListenSocketFactoryPtr socket_factory_; + std::vector socket_factories_; const bool bind_to_port_; const bool mptcp_enabled_; const bool hand_off_restored_destination_connections_; From f6b64a968a0a98e93eab49b0facdceea29279a95 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Sat, 14 May 2022 08:02:38 +0000 Subject: [PATCH 04/19] ConnectionHandlerImpl: enable create multiple active listener Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 271 +++++++++++++---------- source/server/connection_handler_impl.h | 41 +++- test/server/connection_handler_test.cc | 151 +++++++------ 3 files changed, 264 insertions(+), 199 deletions(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 0427dc90d7845..8ed5b10b9c823 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -36,12 +36,16 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list ActiveListenerDetailsOptRef listener_detail = findActiveListenerByTag(overridden_listener.value()); ASSERT(listener_detail.has_value()); - listener_detail->get().listener_->updateListenerConfig(config); + listener_detail->get().invokeListenerMethod( + [&config](Network::ConnectionHandler::ActiveListener& listener) { + listener.updateListenerConfig(config); + }); return; } - auto details = std::make_shared(); + auto details = std::make_unique(); if (config.internalListenerConfig().has_value()) { + auto pre_address_details = std::make_shared(); // Ensure the this ConnectionHandlerImpl link to the thread local registry. Ideally this step // should be done only once. However, an extra phase and interface is overkill. Network::InternalListenerRegistry& internal_listener_registry = @@ -53,93 +57,97 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list if (overridden_listener.has_value()) { if (auto iter = listener_map_by_tag_.find(overridden_listener.value()); iter != listener_map_by_tag_.end()) { - iter->second->internalListener()->get().updateListenerConfig(config); + iter->second->invokeListenerMethod( + [&config](Network::ConnectionHandler::ActiveListener& listener) { + listener.updateListenerConfig(config); + }); return; } IS_ENVOY_BUG("unexpected"); } auto internal_listener = std::make_unique(*this, dispatcher(), config); - details->typed_listener_ = *internal_listener; - details->listener_ = std::move(internal_listener); - } else if (config.listenSocketFactory().socketType() == Network::Socket::Type::Stream) { + // The internal address doesn't support multiple addresses. + ASSERT(config.listenSocketFactories().size() == 1); + details->addActiveListener(config, config.listenSocketFactories()[0], listener_reject_fraction_, + disable_listeners_, std::move(internal_listener)); + } else if (config.listenSocketFactories()[0]->socketType() == Network::Socket::Type::Stream) { if (!support_udp_in_place_filter_chain_update && overridden_listener.has_value()) { if (auto iter = listener_map_by_tag_.find(overridden_listener.value()); iter != listener_map_by_tag_.end()) { - iter->second->tcpListener()->get().updateListenerConfig(config); + iter->second->invokeListenerMethod( + [&config](Network::ConnectionHandler::ActiveListener& listener) { + listener.updateListenerConfig(config); + }); return; } IS_ENVOY_BUG("unexpected"); } - // worker_index_ doesn't have a value on the main thread for the admin server. - auto tcp_listener = - std::make_unique(*this, config, runtime, - config.listenSocketFactory().getListenSocket( - worker_index_.has_value() ? *worker_index_ : 0)); - details->typed_listener_ = *tcp_listener; - details->listener_ = std::move(tcp_listener); + for (auto& socket_factory : config.listenSocketFactories()) { + // worker_index_ doesn't have a value on the main thread for the admin server. + details->addActiveListener( + config, socket_factory, listener_reject_fraction_, disable_listeners_, + std::make_unique( + *this, config, runtime, + socket_factory->getListenSocket(worker_index_.has_value() ? *worker_index_ : 0))); + } + } else { ASSERT(config.udpListenerConfig().has_value(), "UDP listener factory is not initialized."); ASSERT(worker_index_.has_value()); - ConnectionHandler::ActiveUdpListenerPtr udp_listener = - config.udpListenerConfig()->listenerFactory().createActiveUdpListener( - runtime, *worker_index_, *this, - config.listenSocketFactory().getListenSocket(*worker_index_), dispatcher_, config); - details->typed_listener_ = *udp_listener; - details->listener_ = std::move(udp_listener); - } - - if (disable_listeners_) { - details->listener_->pauseListening(); - } - if (auto* listener = details->listener_->listener(); listener != nullptr) { - listener->setRejectFraction(listener_reject_fraction_); + for (auto& socket_factory : config.listenSocketFactories()) { + details->addActiveListener( + config, socket_factory, listener_reject_fraction_, disable_listeners_, + config.udpListenerConfig()->listenerFactory().createActiveUdpListener( + runtime, *worker_index_, *this, + config.listenSocketFactories()[0]->getListenSocket(*worker_index_), dispatcher_, + config)); + } } - details->listener_tag_ = config.listenerTag(); - details->address_ = config.listenSocketFactory().localAddress(); - ASSERT(!listener_map_by_tag_.contains(config.listenerTag())); - listener_map_by_tag_.emplace(config.listenerTag(), details); - // This map only store the new listener. - if (absl::holds_alternative>( - details->typed_listener_)) { - tcp_listener_map_by_address_.insert_or_assign( - config.listenSocketFactory().localAddress()->asStringView(), details); - - auto& address = details->address_; - // If the address is Ipv6 and isn't v6only, parse out the ipv4 compatible address from the Ipv6 - // address and put an item to the map. Then this allows the `getBalancedHandlerByAddress` - // can match the Ipv4 request to Ipv4-mapped address also. - if (address->type() == Network::Address::Type::Ip && - address->ip()->version() == Network::Address::IpVersion::v6 && - !address->ip()->ipv6()->v6only()) { - if (address->ip()->isAnyAddress()) { - // Since both "::" with ipv4_compat and "0.0.0.0" can be supported. - // If there already one and it isn't shutdown for compatible addr, - // then won't insert a new one. - auto ipv4_any_address = Network::Address::Ipv4Instance(address->ip()->port()).asString(); - auto ipv4_any_listener = tcp_listener_map_by_address_.find(ipv4_any_address); - if (ipv4_any_listener == tcp_listener_map_by_address_.end() || - ipv4_any_listener->second->listener_->listener() == nullptr) { - tcp_listener_map_by_address_.insert_or_assign(ipv4_any_address, details); - } - } else { - auto v4_compatible_addr = address->ip()->ipv6()->v4CompatibleAddress(); - // Remove this check when runtime flag - // `envoy.reloadable_features.strict_check_on_ipv4_compat` deprecated. - // If this isn't a valid Ipv4-mapped address, then do nothing. - if (v4_compatible_addr != nullptr) { - tcp_listener_map_by_address_.insert_or_assign(v4_compatible_addr->asStringView(), - details); + for (auto& per_address_details : details->per_address_details_) { + // This map only store the new listener. + if (absl::holds_alternative>( + per_address_details->typed_listener_)) { + tcp_listener_map_by_address_.insert_or_assign(per_address_details->address_->asStringView(), + per_address_details); + + auto& address = per_address_details->address_; + // If the address is Ipv6 and isn't v6only, parse out the ipv4 compatible address from the + // Ipv6 address and put an item to the map. Then this allows the `getBalancedHandlerByAddress` + // can match the Ipv4 request to Ipv4-mapped address also. + if (address->type() == Network::Address::Type::Ip && + address->ip()->version() == Network::Address::IpVersion::v6 && + !address->ip()->ipv6()->v6only()) { + if (address->ip()->isAnyAddress()) { + // Since both "::" with ipv4_compat and "0.0.0.0" can be supported. + // If there already one and it isn't shutdown for compatible addr, + // then won't insert a new one. + auto ipv4_any_address = Network::Address::Ipv4Instance(address->ip()->port()).asString(); + auto ipv4_any_listener = tcp_listener_map_by_address_.find(ipv4_any_address); + if (ipv4_any_listener == tcp_listener_map_by_address_.end() || + ipv4_any_listener->second->listener_->listener() == nullptr) { + tcp_listener_map_by_address_.insert_or_assign(ipv4_any_address, per_address_details); + } + } else { + auto v4_compatible_addr = address->ip()->ipv6()->v4CompatibleAddress(); + // Remove this check when runtime flag + // `envoy.reloadable_features.strict_check_on_ipv4_compat` deprecated. + // If this isn't a valid Ipv4-mapped address, then do nothing. + if (v4_compatible_addr != nullptr) { + tcp_listener_map_by_address_.insert_or_assign(v4_compatible_addr->asStringView(), + per_address_details); + } } } + } else if (absl::holds_alternative>( + per_address_details->typed_listener_)) { + internal_listener_map_by_address_.insert_or_assign( + per_address_details->address_->asStringView(), per_address_details); } - } else if (absl::holds_alternative>( - details->typed_listener_)) { - internal_listener_map_by_address_.insert_or_assign( - config.listenSocketFactory().localAddress()->asStringView(), details); } + listener_map_by_tag_.emplace(config.listenerTag(), std::move(details)); } void ConnectionHandlerImpl::removeListeners(uint64_t listener_tag) { @@ -147,41 +155,43 @@ void ConnectionHandlerImpl::removeListeners(uint64_t listener_tag) { listener_iter != listener_map_by_tag_.end()) { // listener_map_by_address_ may already update to the new listener. Compare it with the one // which find from listener_map_by_tag_, only delete it when it is same listener. - auto& address = listener_iter->second->address_; - auto address_view = address->asStringView(); - if (tcp_listener_map_by_address_.contains(address_view) && - tcp_listener_map_by_address_[address_view]->listener_tag_ == - listener_iter->second->listener_tag_) { - tcp_listener_map_by_address_.erase(address_view); - - // If the address is Ipv6 and isn't v6only, delete the corresponding Ipv4 item from the map. - if (address->type() == Network::Address::Type::Ip && - address->ip()->version() == Network::Address::IpVersion::v6 && - !address->ip()->ipv6()->v6only()) { - if (address->ip()->isAnyAddress()) { - auto ipv4_any_addr_iter = tcp_listener_map_by_address_.find( - Network::Address::Ipv4Instance(address->ip()->port()).asStringView()); - // Since both "::" with ipv4_compat and "0.0.0.0" can be supported, ensure they are same - // listener by tag. - if (ipv4_any_addr_iter != tcp_listener_map_by_address_.end() && - ipv4_any_addr_iter->second->listener_tag_ == listener_iter->second->listener_tag_) { - tcp_listener_map_by_address_.erase(ipv4_any_addr_iter); - } - } else { - auto v4_compatible_addr = address->ip()->ipv6()->v4CompatibleAddress(); - // Remove this check when runtime flag - // `envoy.reloadable_features.strict_check_on_ipv4_compat` deprecated. - if (v4_compatible_addr != nullptr) { - // both "::FFFF:" with ipv4_compat and "" isn't valid case, - // remove the v4 compatible addr item directly. - tcp_listener_map_by_address_.erase(v4_compatible_addr->asStringView()); + for (auto& per_address_details : listener_iter->second->per_address_details_) { + auto& address = per_address_details->address_; + auto address_view = address->asStringView(); + if (tcp_listener_map_by_address_.contains(address_view) && + tcp_listener_map_by_address_[address_view]->listener_tag_ == + per_address_details->listener_tag_) { + tcp_listener_map_by_address_.erase(address_view); + + // If the address is Ipv6 and isn't v6only, delete the corresponding Ipv4 item from the map. + if (address->type() == Network::Address::Type::Ip && + address->ip()->version() == Network::Address::IpVersion::v6 && + !address->ip()->ipv6()->v6only()) { + if (address->ip()->isAnyAddress()) { + auto ipv4_any_addr_iter = tcp_listener_map_by_address_.find( + Network::Address::Ipv4Instance(address->ip()->port()).asStringView()); + // Since both "::" with ipv4_compat and "0.0.0.0" can be supported, ensure they are same + // listener by tag. + if (ipv4_any_addr_iter != tcp_listener_map_by_address_.end() && + ipv4_any_addr_iter->second->listener_tag_ == per_address_details->listener_tag_) { + tcp_listener_map_by_address_.erase(ipv4_any_addr_iter); + } + } else { + auto v4_compatible_addr = address->ip()->ipv6()->v4CompatibleAddress(); + // Remove this check when runtime flag + // `envoy.reloadable_features.strict_check_on_ipv4_compat` deprecated. + if (v4_compatible_addr != nullptr) { + // both "::FFFF:" with ipv4_compat and "" isn't valid case, + // remove the v4 compatible addr item directly. + tcp_listener_map_by_address_.erase(v4_compatible_addr->asStringView()); + } } } + } else if (internal_listener_map_by_address_.contains(address_view) && + internal_listener_map_by_address_[address_view]->listener_tag_ == + per_address_details->listener_tag_) { + internal_listener_map_by_address_.erase(address_view); } - } else if (internal_listener_map_by_address_.contains(address_view) && - internal_listener_map_by_address_[address_view]->listener_tag_ == - listener_iter->second->listener_tag_) { - internal_listener_map_by_address_.erase(address_view); } listener_map_by_tag_.erase(listener_iter); } @@ -192,7 +202,8 @@ ConnectionHandlerImpl::getUdpListenerCallbacks(uint64_t listener_tag) { auto listener = findActiveListenerByTag(listener_tag); if (listener.has_value()) { // If the tag matches this must be a UDP listener. - auto udp_listener = listener->get().udpListener(); + // TODO(soulxu): find listener by address. + auto udp_listener = listener->get().per_address_details_[0]->udpListener(); ASSERT(udp_listener.has_value()); return udp_listener; } @@ -205,7 +216,10 @@ void ConnectionHandlerImpl::removeFilterChains( std::function completion) { if (auto listener_it = listener_map_by_tag_.find(listener_tag); listener_it != listener_map_by_tag_.end()) { - listener_it->second->listener_->onFilterChainDraining(filter_chains); + listener_it->second->invokeListenerMethod( + [&filter_chains](Network::ConnectionHandler::ActiveListener& listener) { + listener.onFilterChainDraining(filter_chains); + }); } // Reach here if the target listener is found or the target listener was removed by a full @@ -216,44 +230,55 @@ void ConnectionHandlerImpl::removeFilterChains( void ConnectionHandlerImpl::stopListeners(uint64_t listener_tag) { if (auto iter = listener_map_by_tag_.find(listener_tag); iter != listener_map_by_tag_.end()) { - if (iter->second->listener_->listener() != nullptr) { - iter->second->listener_->shutdownListener(); - } + iter->second->invokeListenerMethod([](Network::ConnectionHandler::ActiveListener& listener) { + if (listener.listener() != nullptr) { + listener.shutdownListener(); + } + }); } } void ConnectionHandlerImpl::stopListeners() { for (auto& iter : listener_map_by_tag_) { - if (iter.second->listener_->listener() != nullptr) { - iter.second->listener_->shutdownListener(); - } + iter.second->invokeListenerMethod([](Network::ConnectionHandler::ActiveListener& listener) { + if (listener.listener() != nullptr) { + listener.shutdownListener(); + } + }); } } void ConnectionHandlerImpl::disableListeners() { disable_listeners_ = true; for (auto& iter : listener_map_by_tag_) { - if (iter.second->listener_->listener() != nullptr) { - iter.second->listener_->pauseListening(); - } + iter.second->invokeListenerMethod([](Network::ConnectionHandler::ActiveListener& listener) { + if (listener.listener() != nullptr) { + listener.pauseListening(); + } + }); } } void ConnectionHandlerImpl::enableListeners() { disable_listeners_ = false; for (auto& iter : listener_map_by_tag_) { - if (iter.second->listener_->listener() != nullptr) { - iter.second->listener_->resumeListening(); - } + iter.second->invokeListenerMethod([](Network::ConnectionHandler::ActiveListener& listener) { + if (listener.listener() != nullptr) { + listener.resumeListening(); + } + }); } } void ConnectionHandlerImpl::setListenerRejectFraction(UnitFloat reject_fraction) { listener_reject_fraction_ = reject_fraction; for (auto& iter : listener_map_by_tag_) { - if (iter.second->listener_->listener() != nullptr) { - iter.second->listener_->listener()->setRejectFraction(reject_fraction); - } + iter.second->invokeListenerMethod( + [&reject_fraction](Network::ConnectionHandler::ActiveListener& listener) { + if (listener.listener() != nullptr) { + listener.listener()->setRejectFraction(reject_fraction); + } + }); } } @@ -268,19 +293,19 @@ ConnectionHandlerImpl::findByAddress(const Network::Address::InstanceConstShared } ConnectionHandlerImpl::ActiveTcpListenerOptRef -ConnectionHandlerImpl::ActiveListenerDetails::tcpListener() { +ConnectionHandlerImpl::PerAddressActiveListenerDetails::tcpListener() { auto* val = absl::get_if>(&typed_listener_); return (val != nullptr) ? absl::make_optional(*val) : absl::nullopt; } ConnectionHandlerImpl::UdpListenerCallbacksOptRef -ConnectionHandlerImpl::ActiveListenerDetails::udpListener() { +ConnectionHandlerImpl::PerAddressActiveListenerDetails::udpListener() { auto* val = absl::get_if>(&typed_listener_); return (val != nullptr) ? absl::make_optional(*val) : absl::nullopt; } ConnectionHandlerImpl::ActiveInternalListenerOptRef -ConnectionHandlerImpl::ActiveListenerDetails::internalListener() { +ConnectionHandlerImpl::PerAddressActiveListenerDetails::internalListener() { auto* val = absl::get_if>(&typed_listener_); return (val != nullptr) ? absl::make_optional(*val) : absl::nullopt; } @@ -297,11 +322,11 @@ Network::BalancedConnectionHandlerOptRef ConnectionHandlerImpl::getBalancedHandlerByTag(uint64_t listener_tag) { auto active_listener = findActiveListenerByTag(listener_tag); if (active_listener.has_value()) { + // TODO(soulxu): find by address. ASSERT(absl::holds_alternative>( - active_listener->get().typed_listener_) && - active_listener->get().listener_->listener() != nullptr); - return Network::BalancedConnectionHandlerOptRef( - active_listener->get().tcpListener().value().get()); + active_listener->get().per_address_details_[0]->typed_listener_) && + active_listener->get().per_address_details_[0]->listener_->listener() != nullptr); + return active_listener->get().per_address_details_[0]->tcpListener().value().get(); } return absl::nullopt; } @@ -320,7 +345,7 @@ ConnectionHandlerImpl::getBalancedHandlerByAddress(const Network::Address::Insta listener_it->second->tcpListener().value().get()); } - OptRef details; + OptRef details; // Otherwise, we need to look for the wild card match, i.e., 0.0.0.0:[address_port]. // We do not return stopped listeners. // TODO(wattli): consolidate with previous search for more efficiency. diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index 52a365ce7c689..324d278bc1b53 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -72,7 +72,7 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, findByAddress(const Network::Address::InstanceConstSharedPtr& listen_address) override; private: - struct ActiveListenerDetails { + struct PerAddressActiveListenerDetails { // Strong pointer to the listener, whether TCP, UDP, QUIC, etc. Network::ConnectionHandler::ActiveListenerPtr listener_; Network::Address::InstanceConstSharedPtr address_; @@ -88,6 +88,39 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, UdpListenerCallbacksOptRef udpListener(); ActiveInternalListenerOptRef internalListener(); }; + + struct ActiveListenerDetails { + std::vector> per_address_details_; + + using ListenerMethodFn = std::function; + + void invokeListenerMethod(ListenerMethodFn fn) { + std::for_each(per_address_details_.begin(), per_address_details_.end(), + [&fn](std::shared_ptr& details) { + fn(*details->listener_); + }); + } + + template + void addActiveListener(Network::ListenerConfig& config, + const Network::ListenSocketFactoryPtr& socket_factory, + UnitFloat& listener_reject_fraction, bool disable_listeners, + ActiveListener&& listener) { + auto pre_address_details = std::make_shared(); + pre_address_details->typed_listener_ = *listener; + pre_address_details->listener_ = std::move(listener); + pre_address_details->address_ = socket_factory->localAddress(); + if (disable_listeners) { + pre_address_details->listener_->pauseListening(); + } + if (auto* listener = pre_address_details->listener_->listener(); listener != nullptr) { + listener->setRejectFraction(listener_reject_fraction); + } + pre_address_details->listener_tag_ = config.listenerTag(); + per_address_details_.emplace_back(pre_address_details); + } + }; + using ActiveListenerDetailsOptRef = absl::optional>; ActiveListenerDetailsOptRef findActiveListenerByTag(uint64_t listener_tag); @@ -95,10 +128,10 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, const absl::optional worker_index_; Event::Dispatcher& dispatcher_; const std::string per_handler_stat_prefix_; - absl::flat_hash_map> listener_map_by_tag_; - absl::flat_hash_map> + absl::flat_hash_map> listener_map_by_tag_; + absl::flat_hash_map> tcp_listener_map_by_address_; - absl::flat_hash_map> + absl::flat_hash_map> internal_listener_map_by_address_; std::atomic num_handler_connections_{}; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 9fba04c607ca2..b8114d22bbab9 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -85,6 +85,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable()); envoy::config::listener::v3::UdpListenerConfig udp_config; udp_listener_config_ = std::make_unique(udp_config); udp_listener_config_->listener_factory_ = @@ -144,13 +145,19 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable(socket_factories_[0].get()); + } + // Network::ListenerConfig Network::FilterChainManager& filterChainManager() override { return inline_filter_chain_manager_ == nullptr ? parent_.manager_ : *inline_filter_chain_manager_; } Network::FilterChainFactory& filterChainFactory() override { return parent_.factory_; } - Network::ListenSocketFactory& listenSocketFactory() override { return socket_factory_; } + Network::ListenSocketFactory& listenSocketFactory() override { return *socket_factories_[0]; } + std::vector& listenSocketFactories() override { return socket_factories_; } bool bindToPort() override { return bind_to_port_; } bool handOffRestoredDestinationConnections() const override { return hand_off_restored_destination_connections_; @@ -192,7 +199,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> socket_; - Network::MockListenSocketFactory socket_factory_; + std::vector socket_factories_; uint64_t tag_; bool bind_to_port_; const uint32_t tcp_backlog_size_; @@ -273,8 +280,8 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggablesocket_factory_, socketType()).WillOnce(Return(socket_type)); - EXPECT_CALL(listeners_.back()->socket_factory_, getListenSocket(_)) + EXPECT_CALL(listeners_.back()->socketFactory(), socketType()).WillOnce(Return(socket_type)); + EXPECT_CALL(listeners_.back()->socketFactory(), getListenSocket(_)) .WillOnce(Return(listeners_.back()->socket_)); if (socket_type == Network::Socket::Type::Stream) { EXPECT_CALL(dispatcher_, createListener_(_, _, _, _, _)) @@ -332,7 +339,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggableip()->port()); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -398,7 +405,7 @@ TEST_F(ConnectionHandlerTest, RemoveListenerDuringRebalance) { TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks, connection_balancer, ¤t_handler); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -439,7 +446,7 @@ TEST_F(ConnectionHandlerTest, ListenerConnectionLimitEnforced) { addListener(1, false, false, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); // Only allow a single connection on this listener. test_listener1->setMaxConnections(1); @@ -451,7 +458,7 @@ TEST_F(ConnectionHandlerTest, ListenerConnectionLimitEnforced) { addListener(2, false, false, "test_listener2", listener2, &listener_callbacks2); Network::Address::InstanceConstSharedPtr alt_address( new Network::Address::Ipv4Instance("127.0.0.2", 20002)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(alt_address)); // Do not allow any connections on this listener. test_listener2->setMaxConnections(0); @@ -524,7 +531,7 @@ TEST_F(ConnectionHandlerTest, RemoveListener) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -555,7 +562,7 @@ TEST_F(ConnectionHandlerTest, DisableListener) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -573,7 +580,7 @@ TEST_F(ConnectionHandlerTest, StopAndDisableStoppedListener) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -594,9 +601,9 @@ TEST_F(ConnectionHandlerTest, AddDisabledListener) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(*listener, disable()); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); + EXPECT_CALL(*listener, disable()); EXPECT_CALL(*listener, onDestroy()); handler_->disableListeners(); @@ -610,7 +617,7 @@ TEST_F(ConnectionHandlerTest, SetListenerRejectFraction) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -627,9 +634,9 @@ TEST_F(ConnectionHandlerTest, AddListenerSetRejectFraction) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(*listener, setRejectFraction(UnitFloat(0.12345f))); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); + EXPECT_CALL(*listener, setRejectFraction(UnitFloat(0.12345f))); EXPECT_CALL(*listener, onDestroy()); handler_->setListenerRejectFraction(UnitFloat(0.12345f)); @@ -644,7 +651,7 @@ TEST_F(ConnectionHandlerTest, SetsTransportSocketConnectTimeout) { TestListener* test_listener = addListener(1, false, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -670,7 +677,7 @@ TEST_F(ConnectionHandlerTest, DestroyCloseConnections) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -691,7 +698,7 @@ TEST_F(ConnectionHandlerTest, CloseDuringFilterChainCreate) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -716,7 +723,7 @@ TEST_F(ConnectionHandlerTest, CloseConnectionOnEmptyFilterChain) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -741,7 +748,7 @@ TEST_F(ConnectionHandlerTest, NormalRedirect) { addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -751,7 +758,7 @@ TEST_F(ConnectionHandlerTest, NormalRedirect) { addListener(2, false, false, "test_listener2", listener2, &listener_callbacks2); Network::Address::InstanceConstSharedPtr alt_address( new Network::Address::Ipv4Instance("127.0.0.2", 20002)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(alt_address)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -811,7 +818,7 @@ TEST_F(ConnectionHandlerTest, MatchLatestListener) { auto listener1 = new NiceMock(); TestListener* test_listener1 = addListener(1, true, true, "test_listener1", listener1, &listener_callbacks); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -825,7 +832,7 @@ TEST_F(ConnectionHandlerTest, MatchLatestListener) { listener2_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr listener2_address( new Network::Address::Ipv4Instance("127.0.0.1", 10002)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(listener2_address)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -839,7 +846,7 @@ TEST_F(ConnectionHandlerTest, MatchLatestListener) { listener3_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr listener3_address( new Network::Address::Ipv4Instance("127.0.0.1", 10002)); - EXPECT_CALL(test_listener3->socket_factory_, localAddress()) + EXPECT_CALL(test_listener3->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(listener3_address)); // This emulated the case of update listener in-place. Stop the old listener and @@ -894,7 +901,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedListener) { auto listener1 = new NiceMock(); TestListener* test_listener1 = addListener(1, true, true, "test_listener1", listener1, &listener_callbacks); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -907,7 +914,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedListener) { listener2_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr listener2_address( new Network::Address::Ipv4Instance("127.0.0.1", 10002)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(listener2_address)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -958,7 +965,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) { auto listener1 = new NiceMock(); TestListener* test_listener1 = addListener(1, true, true, "test_listener1", listener1, &listener_callbacks); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -971,7 +978,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) { listener2_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr listener2_address( new Network::Address::Ipv4Instance("0.0.0.0", 10002)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(listener2_address)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -1023,7 +1030,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1032,7 +1039,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { TestListener* test_listener2 = addListener(2, false, false, "test_listener2", listener2, &listener_callbacks2); Network::Address::InstanceConstSharedPtr any_address = Network::Utility::getIpv4AnyAddress(); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -1077,7 +1084,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListener) { addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1092,7 +1099,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListener) { Network::Address::InstanceConstSharedPtr any_address( new Network::Address::Ipv4Instance("0.0.0.0", 80)); - EXPECT_CALL(ipv4_any_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv4_any_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address)); handler_->addListener(absl::nullopt, *ipv4_any_listener, runtime_); @@ -1106,7 +1113,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListener) { std::chrono::milliseconds(15000), false, ipv6_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr any_address_ipv6( new Network::Address::Ipv6Instance("::", 80)); - EXPECT_CALL(ipv6_any_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_any_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address_ipv6)); handler_->addListener(absl::nullopt, *ipv6_any_listener, runtime_); @@ -1160,7 +1167,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListenerWithAnyAddressAndIpv4Comp addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1175,7 +1182,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListenerWithAnyAddressAndIpv4Comp // Set the ipv6only as false. Network::Address::InstanceConstSharedPtr any_address_ipv6( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(ipv6_any_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_any_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address_ipv6)); handler_->addListener(absl::nullopt, *ipv6_any_listener, runtime_); @@ -1229,7 +1236,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4CompatiableIPv6ListenerWithIpv4CompatFla addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1244,7 +1251,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4CompatiableIPv6ListenerWithIpv4CompatFla // Set the ipv6only as false. Network::Address::InstanceConstSharedPtr ipv4_mapped_ipv6_address( new Network::Address::Ipv6Instance("::FFFF:192.168.0.1", 80, nullptr, false)); - EXPECT_CALL(ipv6_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_mapped_ipv6_address)); handler_->addListener(absl::nullopt, *ipv6_listener, runtime_); @@ -1298,7 +1305,7 @@ TEST_F(ConnectionHandlerTest, NotMatchIPv6WildcardListenerWithoutIpv4CompatFlag) addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1313,7 +1320,7 @@ TEST_F(ConnectionHandlerTest, NotMatchIPv6WildcardListenerWithoutIpv4CompatFlag) // not set the ipv6only flag, the default value is true. Network::Address::InstanceConstSharedPtr any_address_ipv6( new Network::Address::Ipv6Instance("::", 80)); - EXPECT_CALL(ipv6_any_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_any_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address_ipv6)); handler_->addListener(absl::nullopt, *ipv6_any_listener, runtime_); @@ -1364,7 +1371,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag) { addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1380,7 +1387,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag) { // Set the ipv6only as false. Network::Address::InstanceConstSharedPtr ipv6_any_address( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(ipv6_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv6_any_address)); handler_->addListener(absl::nullopt, *ipv6_listener, runtime_); @@ -1395,7 +1402,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag) { false, ipv4_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr ipv4_address( new Network::Address::Ipv4Instance("0.0.0.0", 80, nullptr)); - EXPECT_CALL(ipv4_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv4_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_address)); handler_->addListener(absl::nullopt, *ipv4_listener, runtime_); @@ -1450,7 +1457,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag2) addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1465,7 +1472,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag2) false, ipv4_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr ipv4_address( new Network::Address::Ipv4Instance("0.0.0.0", 80, nullptr)); - EXPECT_CALL(ipv4_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv4_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_address)); handler_->addListener(absl::nullopt, *ipv4_listener, runtime_); @@ -1481,7 +1488,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag2) // Set the ipv6only as false. Network::Address::InstanceConstSharedPtr ipv4_mapped_ipv6_address( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(ipv6_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_mapped_ipv6_address)); handler_->addListener(absl::nullopt, *ipv6_listener, runtime_); @@ -1537,7 +1544,7 @@ TEST_F(ConnectionHandlerTest, AddIpv4MappedListenerAfterIpv4ListenerStopped) { addListener(1, true, true, "test_listener1", listener1, &listener_callbacks1); Network::Address::InstanceConstSharedPtr normal_address( new Network::Address::Ipv4Instance("127.0.0.1", 10001)); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(normal_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1552,7 +1559,7 @@ TEST_F(ConnectionHandlerTest, AddIpv4MappedListenerAfterIpv4ListenerStopped) { false, ipv4_overridden_filter_chain_manager); Network::Address::InstanceConstSharedPtr ipv4_address( new Network::Address::Ipv4Instance("0.0.0.0", 80, nullptr)); - EXPECT_CALL(ipv4_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv4_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_address)); handler_->addListener(absl::nullopt, *ipv4_listener, runtime_); @@ -1572,7 +1579,7 @@ TEST_F(ConnectionHandlerTest, AddIpv4MappedListenerAfterIpv4ListenerStopped) { // Set the ipv6only as false. Network::Address::InstanceConstSharedPtr ipv4_mapped_ipv6_address( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(ipv6_listener->socket_factory_, localAddress()) + EXPECT_CALL(ipv6_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(ipv4_mapped_ipv6_address)); handler_->addListener(absl::nullopt, *ipv6_listener, runtime_); @@ -1646,7 +1653,7 @@ TEST_F(ConnectionHandlerTest, WildcardListenerWithNoOriginalDst) { new Network::Address::Ipv4Instance("127.0.0.1", 80)); Network::Address::InstanceConstSharedPtr any_address = Network::Utility::getAddressWithPort( *Network::Utility::getIpv4AnyAddress(), normal_address->ip()->port()); - EXPECT_CALL(test_listener1->socket_factory_, localAddress()) + EXPECT_CALL(test_listener1->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address)); handler_->addListener(absl::nullopt, *test_listener1, runtime_); @@ -1676,7 +1683,7 @@ TEST_F(ConnectionHandlerTest, TransportProtocolDefault) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1696,7 +1703,7 @@ TEST_F(ConnectionHandlerTest, TransportProtocolCustom) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1731,7 +1738,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeout) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1784,7 +1791,7 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks, nullptr, nullptr, Network::Socket::Type::Stream, std::chrono::milliseconds(15000), true); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1846,7 +1853,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeoutResetOnSuccess) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1900,7 +1907,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterDisabledTimeout) { TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks, nullptr, nullptr, Network::Socket::Type::Stream, std::chrono::milliseconds()); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1937,7 +1944,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterReportError) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -1983,7 +1990,7 @@ TEST_F(ConnectionHandlerTest, UdpListenerNoFilter) { EXPECT_CALL(factory_, createUdpListenerFilterChain(_, _)) .WillOnce(Invoke([&](Network::UdpListenerFilterManager&, Network::UdpReadFilterCallbacks&) -> bool { return true; })); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2009,7 +2016,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdate) { TestListener* old_test_listener = addListener(old_listener_tag, true, false, "test_listener", old_listener, &old_listener_callbacks, mock_connection_balancer, ¤t_handler); - EXPECT_CALL(old_test_listener->socket_factory_, localAddress()) + EXPECT_CALL(old_test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *old_test_listener, runtime_); ASSERT_NE(old_test_listener, nullptr); @@ -2047,7 +2054,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveFilterChain) { auto listener = new NiceMock(); TestListener* test_listener = addListener(listener_tag, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2096,7 +2103,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveFilterChainCalledAfterListenerIsR auto listener = new NiceMock(); TestListener* test_listener = addListener(listener_tag, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2159,7 +2166,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveListener) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2193,7 +2200,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveIpv6AnyAddressWithIpv4CompatListe addListener(1, true, false, "test_listener", listener, &listener_callbacks); Network::Address::InstanceConstSharedPtr any_address_ipv6( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(any_address_ipv6)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2224,7 +2231,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveIpv4CompatAddressListener) { addListener(1, true, false, "test_listener", listener, &listener_callbacks); Network::Address::InstanceConstSharedPtr address_ipv6( new Network::Address::Ipv6Instance("::FFFF:192.168.0.1", 80, nullptr, false)); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(address_ipv6)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2255,7 +2262,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveWithBothIpv4AnyAndIpv6Any) { addListener(1, true, false, "test_listener", listener, &listener_callbacks); Network::Address::InstanceConstSharedPtr address_ipv6( new Network::Address::Ipv6Instance("::", 80, nullptr, false)); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(address_ipv6)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2265,7 +2272,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveWithBothIpv4AnyAndIpv6Any) { addListener(2, true, false, "test_listener2", listener2, &listener_callbacks2); Network::Address::InstanceConstSharedPtr address_ipv4( new Network::Address::Ipv4Instance("0.0.0.0", 80, nullptr)); - EXPECT_CALL(test_listener2->socket_factory_, localAddress()) + EXPECT_CALL(test_listener2->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(address_ipv4)); handler_->addListener(absl::nullopt, *test_listener2, runtime_); @@ -2305,7 +2312,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerGlobalCxLimitReject) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2321,7 +2328,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerOverloadActionReject) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2338,7 +2345,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterWorks) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener, runtime_); @@ -2384,7 +2391,7 @@ TEST_F(ConnectionHandlerTest, ShutdownUdpListener) { udp_listener.addReadFilter(std::move(filter)); return true; })); - EXPECT_CALL(test_listener->socket_factory_, localAddress()) + EXPECT_CALL(test_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address_)); EXPECT_CALL(dummy_callbacks.udp_listener_, onDestroy()); @@ -2402,7 +2409,7 @@ TEST_F(ConnectionHandlerTest, DisableInternalListener) { TestListener* internal_listener = addInternalListener(1, "test_internal_listener", std::chrono::milliseconds(), false, nullptr); - EXPECT_CALL(internal_listener->socket_factory_, localAddress()) + EXPECT_CALL(internal_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address)); handler_->addListener(absl::nullopt, *internal_listener, runtime_); auto internal_listener_cb = handler_->findByAddress(local_address); @@ -2428,7 +2435,7 @@ TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdate) { TestListener* internal_listener = addInternalListener( old_listener_tag, "test_internal_listener", std::chrono::milliseconds(), false, nullptr); - EXPECT_CALL(internal_listener->socket_factory_, localAddress()) + EXPECT_CALL(internal_listener->socketFactory(), localAddress()) .WillRepeatedly(ReturnRef(local_address)); handler_->addListener(absl::nullopt, *internal_listener, runtime_); From 7c9c661a5d2b1a31bf762d44e7f955d0c441dd45 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 04:14:25 +0000 Subject: [PATCH 05/19] fix test Signed-off-by: He Jie Xu --- .../proxy_protocol_regression_test.cc | 19 +++++++--- .../common/fuzz/listener_filter_fuzzer.cc | 12 ++++-- .../common/fuzz/listener_filter_fuzzer.h | 7 +++- .../proxy_protocol/proxy_protocol_test.cc | 38 ++++++++++++++----- test/integration/fake_upstream.cc | 4 +- test/integration/fake_upstream.h | 7 +++- test/mocks/network/mocks.h | 1 + test/server/connection_handler_test.cc | 4 +- 8 files changed, 67 insertions(+), 25 deletions(-) diff --git a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc index fe07517d8c84d..090f76a0fa3f1 100644 --- a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc +++ b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc @@ -46,10 +46,16 @@ class ProxyProtocolRegressionTest : public testing::TestWithParam()); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(socket_->connectionInfoProvider().localAddress())); - EXPECT_CALL(socket_factory_, getListenSocket(_)).WillOnce(Return(socket_)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + getListenSocket(_)) + .WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -60,7 +66,10 @@ class ProxyProtocolRegressionTest : public testing::TestWithParam& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } uint32_t perConnectionBufferLimitBytes() const override { return 0; } @@ -166,7 +175,7 @@ class ProxyProtocolRegressionTest : public testing::TestWithParam socket_; - Network::MockListenSocketFactory socket_factory_; + std::vector socket_factories_; Network::NopConnectionBalancerImpl connection_balancer_; Network::ConnectionHandlerPtr connection_handler_; Network::MockFilterChainFactory factory_; diff --git a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc index 88241d66d935d..06edc6a57b87c 100644 --- a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc +++ b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc @@ -35,10 +35,16 @@ ListenerFilterWithDataFuzzer::ListenerFilterWithDataFuzzer() connection_handler_(new Server::ConnectionHandlerImpl(*dispatcher_, absl::nullopt)), name_("proxy"), filter_chain_(Network::Test::createEmptyFilterChainWithRawBufferSockets()), init_manager_(nullptr) { - EXPECT_CALL(socket_factory_, socketType()).WillOnce(Return(Network::Socket::Type::Stream)); - EXPECT_CALL(socket_factory_, localAddress()) + socket_factories_.emplace_back(std::make_unique()); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(socket_->connectionInfoProvider().localAddress())); - EXPECT_CALL(socket_factory_, getListenSocket(_)).WillOnce(Return(socket_)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + getListenSocket(_)) + .WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), diff --git a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.h b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.h index 9a52da99823c0..71d182145c602 100644 --- a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.h +++ b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.h @@ -48,7 +48,10 @@ class ListenerFilterWithDataFuzzer : public Network::ListenerConfig, // Network::ListenerConfig Network::FilterChainManager& filterChainManager() override { return *this; } Network::FilterChainFactory& filterChainFactory() override { return factory_; } - Network::ListenSocketFactory& listenSocketFactory() override { return socket_factory_; } + Network::ListenSocketFactory& listenSocketFactory() override { return *socket_factories_[0]; } + std::vector& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } uint32_t perConnectionBufferLimitBytes() const override { return 0; } @@ -98,7 +101,7 @@ class ListenerFilterWithDataFuzzer : public Network::ListenerConfig, BasicResourceLimitImpl open_connections_; Event::DispatcherPtr dispatcher_; std::shared_ptr socket_; - Network::MockListenSocketFactory socket_factory_; + std::vector socket_factories_; Network::NopConnectionBalancerImpl connection_balancer_; Network::ConnectionHandlerPtr connection_handler_; Network::MockFilterChainFactory factory_; diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index afeba9505b0ac..033e44c865878 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -63,10 +63,16 @@ class ProxyProtocolTest : public testing::TestWithParam()); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(socket_->connectionInfoProvider().localAddress())); - EXPECT_CALL(socket_factory_, getListenSocket(_)).WillOnce(Return(socket_)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + getListenSocket(_)) + .WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -77,7 +83,10 @@ class ProxyProtocolTest : public testing::TestWithParam& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } uint32_t perConnectionBufferLimitBytes() const override { return 0; } @@ -198,7 +207,7 @@ class ProxyProtocolTest : public testing::TestWithParam socket_; - Network::MockListenSocketFactory socket_factory_; + std::vector socket_factories_; Network::NopConnectionBalancerImpl connection_balancer_; Network::ConnectionHandlerPtr connection_handler_; Network::MockFilterChainFactory factory_; @@ -1745,10 +1754,16 @@ class WildcardProxyProtocolTest : public testing::TestWithParam()); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(socket_->connectionInfoProvider().localAddress())); - EXPECT_CALL(socket_factory_, getListenSocket(_)).WillOnce(Return(socket_)); + EXPECT_CALL(*static_cast(socket_factories_[0].get()), + getListenSocket(_)) + .WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(local_dst_address_, Network::Address::InstanceConstSharedPtr(), @@ -1769,7 +1784,10 @@ class WildcardProxyProtocolTest : public testing::TestWithParam& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } uint32_t perConnectionBufferLimitBytes() const override { return 0; } @@ -1849,7 +1867,7 @@ class WildcardProxyProtocolTest : public testing::TestWithParam socket_factories_; std::shared_ptr socket_; Network::Address::InstanceConstSharedPtr local_dst_address_; Network::NopConnectionBalancerImpl connection_balancer_; diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 5312bdd4c7141..550c3d36ef21d 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -561,7 +561,6 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket : http_type_(config.upstream_protocol_), http2_options_(config.http2_options_), http3_options_(config.http3_options_), quic_options_(config.quic_options_), socket_(Network::SocketSharedPtr(listen_socket.release())), - socket_factory_(std::make_unique(socket_)), api_(Api::createApiForTest(stats_store_)), time_system_(config.time_system_), dispatcher_(api_->allocateDispatcher("fake_upstream")), handler_(new Server::ConnectionHandlerImpl(*dispatcher_, 0)), config_(config), @@ -569,6 +568,7 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket listener_(*this, http_type_ == Http::CodecType::HTTP3), filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))), stats_scope_(stats_store_.createScope("test_server_scope")) { + socket_factories_.emplace_back(std::make_unique(socket_)); ENVOY_LOG(info, "starting fake server at {}. UDP={} codec={}", localAddress()->asString(), config.udp_fake_upstream_.has_value(), FakeHttpConnection::typeToString(http_type_)); if (config.udp_fake_upstream_.has_value() && @@ -624,7 +624,7 @@ void FakeUpstream::createUdpListenerFilterChain(Network::UdpListenerFilterManage } void FakeUpstream::threadRoutine() { - socket_factory_->doFinalPreWorkerInit(); + socket_factories_[0]->doFinalPreWorkerInit(); handler_->addListener(absl::nullopt, listener_, runtime_); server_initialized_.setReady(); dispatcher_->run(Event::Dispatcher::RunType::Block); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 1f49cfacea48d..c5469f7338a41 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -795,7 +795,10 @@ class FakeUpstream : Logger::Loggable, Network::FilterChainManager& filterChainManager() override { return parent_; } Network::FilterChainFactory& filterChainFactory() override { return parent_; } Network::ListenSocketFactory& listenSocketFactory() override { - return *parent_.socket_factory_; + return *parent_.socket_factories_[0]; + } + std::vector& listenSocketFactories() override { + return parent_.socket_factories_; } bool bindToPort() override { return true; } bool handOffRestoredDestinationConnections() const override { return false; } @@ -845,7 +848,7 @@ class FakeUpstream : Logger::Loggable, const envoy::config::core::v3::Http3ProtocolOptions http3_options_; envoy::config::listener::v3::QuicProtocolOptions quic_options_; Network::SocketSharedPtr socket_; - Network::ListenSocketFactoryPtr socket_factory_; + std::vector socket_factories_; ConditionalInitializer server_initialized_; // Guards any objects which can be altered both in the upstream thread and the // main test thread. diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 27c5799b00f9a..c6741474bd5ab 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -427,6 +427,7 @@ class MockListenerConfig : public ListenerConfig { MOCK_METHOD(FilterChainManager&, filterChainManager, ()); MOCK_METHOD(FilterChainFactory&, filterChainFactory, ()); MOCK_METHOD(ListenSocketFactory&, listenSocketFactory, ()); + MOCK_METHOD(std::vector&, listenSocketFactories, ()); MOCK_METHOD(bool, bindToPort, ()); MOCK_METHOD(bool, handOffRestoredDestinationConnections, (), (const)); MOCK_METHOD(uint32_t, perConnectionBufferLimitBytes, (), (const)); diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index b8114d22bbab9..336db9fa90d88 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -157,7 +157,9 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable& listenSocketFactories() override { return socket_factories_; } + std::vector& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return bind_to_port_; } bool handOffRestoredDestinationConnections() const override { return hand_off_restored_destination_connections_; From 38bf82f9c3a07b82362d81a07dace0ce377db082 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 04:23:44 +0000 Subject: [PATCH 06/19] correct comments Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 8ed5b10b9c823..19a47a1565d0d 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -202,7 +202,8 @@ ConnectionHandlerImpl::getUdpListenerCallbacks(uint64_t listener_tag) { auto listener = findActiveListenerByTag(listener_tag); if (listener.has_value()) { // If the tag matches this must be a UDP listener. - // TODO(soulxu): find listener by address. + // TODO(soulxu): return first listener here, this will be changed + // when UdpWorkerRouter supports the multiple addresses. auto udp_listener = listener->get().per_address_details_[0]->udpListener(); ASSERT(udp_listener.has_value()); return udp_listener; @@ -322,7 +323,8 @@ Network::BalancedConnectionHandlerOptRef ConnectionHandlerImpl::getBalancedHandlerByTag(uint64_t listener_tag) { auto active_listener = findActiveListenerByTag(listener_tag); if (active_listener.has_value()) { - // TODO(soulxu): find by address. + // TODO(soulxu): return first listener here, this will be changed + // when ConnectionBalancer supports the multiple addresses. ASSERT(absl::holds_alternative>( active_listener->get().per_address_details_[0]->typed_listener_) && active_listener->get().per_address_details_[0]->listener_->listener() != nullptr); From 5cc2bad249456b7ee4b8d4230909d7ad279a3d4d Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 04:30:46 +0000 Subject: [PATCH 07/19] correct the variable name Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 12 ++++++------ source/server/connection_handler_impl.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 19a47a1565d0d..21d1b91941481 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -106,7 +106,7 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list ASSERT(!listener_map_by_tag_.contains(config.listenerTag())); - for (auto& per_address_details : details->per_address_details_) { + for (auto& per_address_details : details->per_address_details_list_) { // This map only store the new listener. if (absl::holds_alternative>( per_address_details->typed_listener_)) { @@ -155,7 +155,7 @@ void ConnectionHandlerImpl::removeListeners(uint64_t listener_tag) { listener_iter != listener_map_by_tag_.end()) { // listener_map_by_address_ may already update to the new listener. Compare it with the one // which find from listener_map_by_tag_, only delete it when it is same listener. - for (auto& per_address_details : listener_iter->second->per_address_details_) { + for (auto& per_address_details : listener_iter->second->per_address_details_list_) { auto& address = per_address_details->address_; auto address_view = address->asStringView(); if (tcp_listener_map_by_address_.contains(address_view) && @@ -204,7 +204,7 @@ ConnectionHandlerImpl::getUdpListenerCallbacks(uint64_t listener_tag) { // If the tag matches this must be a UDP listener. // TODO(soulxu): return first listener here, this will be changed // when UdpWorkerRouter supports the multiple addresses. - auto udp_listener = listener->get().per_address_details_[0]->udpListener(); + auto udp_listener = listener->get().per_address_details_list_[0]->udpListener(); ASSERT(udp_listener.has_value()); return udp_listener; } @@ -326,9 +326,9 @@ ConnectionHandlerImpl::getBalancedHandlerByTag(uint64_t listener_tag) { // TODO(soulxu): return first listener here, this will be changed // when ConnectionBalancer supports the multiple addresses. ASSERT(absl::holds_alternative>( - active_listener->get().per_address_details_[0]->typed_listener_) && - active_listener->get().per_address_details_[0]->listener_->listener() != nullptr); - return active_listener->get().per_address_details_[0]->tcpListener().value().get(); + active_listener->get().per_address_details_list_[0]->typed_listener_) && + active_listener->get().per_address_details_list_[0]->listener_->listener() != nullptr); + return active_listener->get().per_address_details_list_[0]->tcpListener().value().get(); } return absl::nullopt; } diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index 324d278bc1b53..1ba8e7d951c31 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -90,12 +90,12 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, }; struct ActiveListenerDetails { - std::vector> per_address_details_; + std::vector> per_address_details_list_; using ListenerMethodFn = std::function; void invokeListenerMethod(ListenerMethodFn fn) { - std::for_each(per_address_details_.begin(), per_address_details_.end(), + std::for_each(per_address_details_list_.begin(), per_address_details_list_.end(), [&fn](std::shared_ptr& details) { fn(*details->listener_); }); @@ -117,7 +117,7 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, listener->setRejectFraction(listener_reject_fraction); } pre_address_details->listener_tag_ = config.listenerTag(); - per_address_details_.emplace_back(pre_address_details); + per_address_details_list_.emplace_back(pre_address_details); } }; From 375e492acea416bd0f35a694bb9712944709aa4b Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 04:33:28 +0000 Subject: [PATCH 08/19] fix comments Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 21d1b91941481..73cf48947eb70 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -66,7 +66,7 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list IS_ENVOY_BUG("unexpected"); } auto internal_listener = std::make_unique(*this, dispatcher(), config); - // The internal address doesn't support multiple addresses. + // TODO(soulxu): support multiple internal addresses in listener in the future. ASSERT(config.listenSocketFactories().size() == 1); details->addActiveListener(config, config.listenSocketFactories()[0], listener_reject_fraction_, disable_listeners_, std::move(internal_listener)); From f9773514265068e1b49bed223cc1303a63e7197a Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 1 Jun 2022 06:45:24 +0000 Subject: [PATCH 09/19] fix clang-tidy Signed-off-by: He Jie Xu --- source/server/listener_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index bb23e7f946a68..b6a6999a04ac4 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -863,7 +863,7 @@ ListenerImpl::~ListenerImpl() { Init::Manager& ListenerImpl::initManager() { return *dynamic_init_manager_; } void ListenerImpl::setSocketFactory(Network::ListenSocketFactoryPtr&& socket_factory) { - ASSERT(socket_factories_.size() == 0); + ASSERT(socket_factories_.empty()); socket_factories_.emplace_back(std::move(socket_factory)); } From c3feb8071fb6af8041f40813c487700ef850b34d Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Thu, 2 Jun 2022 06:42:08 +0000 Subject: [PATCH 10/19] update coverage Signed-off-by: He Jie Xu --- test/per_file_coverage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 0aa174b64e66c..0187e7b9aea56 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -88,7 +88,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/watchdog:83.3" # Death tests within extensions "source/extensions/watchdog/profile_action:83.3" "source/server:93.3" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239 -"source/server/admin:97.6" +"source/server/admin:97.5" "source/server/config_validation:74.8" ) From d95aefb14c00cae481bfc5e131723e811552bccb Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Mon, 6 Jun 2022 01:17:37 +0000 Subject: [PATCH 11/19] fix format Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index f8f267a896db5..75b52b181d535 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -64,7 +64,8 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list } IS_ENVOY_BUG("unexpected"); } - auto internal_listener = local_registry->createActiveInternalListener(*this, config, dispatcher()); + auto internal_listener = + local_registry->createActiveInternalListener(*this, config, dispatcher()); // TODO(soulxu): support multiple internal addresses in listener in the future. ASSERT(config.listenSocketFactories().size() == 1); details->addActiveListener(config, config.listenSocketFactories()[0], listener_reject_fraction_, @@ -304,7 +305,8 @@ ConnectionHandlerImpl::PerAddressActiveListenerDetails::udpListener() { return (val != nullptr) ? absl::make_optional(*val) : absl::nullopt; } -Network::InternalListenerOptRef ConnectionHandlerImpl::PerAddressActiveListenerDetails::internalListener() { +Network::InternalListenerOptRef +ConnectionHandlerImpl::PerAddressActiveListenerDetails::internalListener() { auto* val = absl::get_if>(&typed_listener_); return (val != nullptr) ? makeOptRef(val->get()) : absl::nullopt; } From 5c96078f3c09f2dfe3f6defc345079891b574ca3 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Mon, 6 Jun 2022 06:15:24 +0000 Subject: [PATCH 12/19] fix active internal listener test Signed-off-by: He Jie Xu --- .../active_internal_listener_test.cc | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc index 223f6acc6d797..695dd9d68769c 100644 --- a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc +++ b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc @@ -267,6 +267,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable()); ON_CALL(*socket_, socketType()).WillByDefault(Return(socket_type)); } @@ -292,7 +293,10 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable& listenSocketFactories() override { + return socket_factories_; + } bool bindToPort() override { return bind_to_port_; } bool handOffRestoredDestinationConnections() const override { return hand_off_restored_destination_connections_; @@ -334,7 +338,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> socket_; - Network::MockListenSocketFactory socket_factory_; + std::vector socket_factories_; uint64_t tag_; bool bind_to_port_; const uint32_t tcp_backlog_size_; @@ -393,7 +397,9 @@ TEST_F(ConnectionHandlerTest, DisableInternalListener) { TestListener* internal_listener = addInternalListener(1, "test_internal_listener", std::chrono::milliseconds(), false, nullptr); - EXPECT_CALL(internal_listener->socket_factory_, localAddress()) + EXPECT_CALL(*static_cast( + internal_listener->socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(local_address)); handler_->addListener(absl::nullopt, *internal_listener, runtime_); auto internal_listener_cb = handler_->findByAddress(local_address); @@ -419,7 +425,9 @@ TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdate) { TestListener* internal_listener = addInternalListener( old_listener_tag, "test_internal_listener", std::chrono::milliseconds(), false, nullptr); - EXPECT_CALL(internal_listener->socket_factory_, localAddress()) + EXPECT_CALL(*static_cast( + internal_listener->socket_factories_[0].get()), + localAddress()) .WillRepeatedly(ReturnRef(local_address)); handler_->addListener(absl::nullopt, *internal_listener, runtime_); From 31f2ffedc5aae3814470c14ad82ef7b42a14f7b2 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Wed, 15 Jun 2022 02:31:39 +0000 Subject: [PATCH 13/19] add comments Signed-off-by: He Jie Xu --- envoy/network/listener.h | 1 + test/per_file_coverage.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/envoy/network/listener.h b/envoy/network/listener.h index f805893048327..62dbfd4759cc9 100644 --- a/envoy/network/listener.h +++ b/envoy/network/listener.h @@ -146,6 +146,7 @@ class ListenerConfig { virtual FilterChainFactory& filterChainFactory() PURE; /** + * TODO(soulxu): This will be removed when multiple addresses listener implemented. * @return ListenSocketFactory& the factory to create listen socket. */ virtual ListenSocketFactory& listenSocketFactory() PURE; diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 0187e7b9aea56..6688511a4dca3 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -88,7 +88,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/watchdog:83.3" # Death tests within extensions "source/extensions/watchdog/profile_action:83.3" "source/server:93.3" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239 -"source/server/admin:97.5" +"source/server/admin:97.5" # TODO(soulxu) try to raise this back to `97.6` when multiple addresses listener implemented and the old interface of Network::ListenerConfig is removed. "source/server/config_validation:74.8" ) From 1b07b79c9c8caf6a6db07b34c2b662ecf40a4cb9 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Fri, 17 Jun 2022 05:58:48 +0000 Subject: [PATCH 14/19] Address comments Signed-off-by: He Jie Xu --- source/server/admin/admin.h | 1 + source/server/connection_handler_impl.cc | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 60cfce495d87a..ef314d9ca4425 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -390,6 +390,7 @@ class AdminImpl : public Admin, Network::FilterChainManager& filterChainManager() override { return parent_; } Network::FilterChainFactory& filterChainFactory() override { return parent_; } Network::ListenSocketFactory& listenSocketFactory() override { + ASSERT(parent_.socket_factories_.size() == 1); return *parent_.socket_factories_[0]; } std::vector& listenSocketFactories() override { diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 75b52b181d535..dd3b38a2d85cc 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -44,7 +44,6 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list auto details = std::make_unique(); if (config.internalListenerConfig().has_value()) { - auto pre_address_details = std::make_shared(); // Ensure the this ConnectionHandlerImpl link to the thread local registry. Ideally this step // should be done only once. However, an extra phase and interface is overkill. Network::InternalListenerRegistry& internal_listener_registry = From b005722560a0523f2d4b4563493c0f9fd4f86b39 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Sun, 15 May 2022 03:35:26 +0000 Subject: [PATCH 15/19] ConnectionHandlerImpl: Add test case for multiple addreses Signed-off-by: He Jie Xu --- test/server/connection_handler_test.cc | 201 ++++++++++++++++++++++++- 1 file changed, 197 insertions(+), 4 deletions(-) diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index be0f06cc30fb9..e19eda0454f36 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -74,7 +74,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> filter_chain_manager = nullptr, uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE, Network::ConnectionBalancerSharedPtr connection_balancer = nullptr, - bool ignore_global_conn_limit = false) + bool ignore_global_conn_limit = false, int num_of_socket_factories = 1) : parent_(parent), socket_(std::make_shared>()), tag_(tag), bind_to_port_(bind_to_port), tcp_backlog_size_(tcp_backlog_size), hand_off_restored_destination_connections_(hand_off_restored_destination_connections), @@ -85,7 +85,9 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable()); + for (int i = 0; i < num_of_socket_factories; i++) { + socket_factories_.emplace_back(std::make_unique()); + } envoy::config::listener::v3::UdpListenerConfig udp_config; udp_listener_config_ = std::make_unique(udp_config); udp_listener_config_->listener_factory_ = @@ -113,8 +115,8 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable(socket_factories_[0].get()); + Network::MockListenSocketFactory& socketFactory(int index = 0) { + return *static_cast(socket_factories_[index].get()); } // Network::ListenerConfig @@ -276,6 +278,43 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable& mock_listeners, + std::vector& addresses, + Network::TcpListenerCallbacks** listener_callbacks = nullptr, bool disable_listener = false) { + listeners_.emplace_back(std::make_unique( + *this, tag, bind_to_port, hand_off_restored_destination_connections, name, + Network::Socket::Type::Stream, std::chrono::milliseconds(15000), false, access_log_, + nullptr, ENVOY_TCP_BACKLOG_SIZE, nullptr, false, mock_listeners.size())); + + EXPECT_CALL(listeners_.back()->socketFactory(0), socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); + for (std::vector::size_type i = 0; i < mock_listeners.size(); i++) { + EXPECT_CALL(listeners_.back()->socketFactory(i), getListenSocket(_)) + .WillOnce(Return(listeners_.back()->socket_)); + + EXPECT_CALL(dispatcher_, createListener_(_, _, _, _, _)) + .WillOnce(Invoke([i, &mock_listeners, listener_callbacks]( + Network::SocketSharedPtr&&, Network::TcpListenerCallbacks& cb, + Runtime::Loader&, bool, bool) -> Network::Listener* { + if (listener_callbacks != nullptr) { + *listener_callbacks = &cb; + } + return mock_listeners[i]; + })) + .RetiresOnSaturation(); + + EXPECT_CALL(listeners_.back()->socketFactory(i), localAddress()) + .WillRepeatedly(ReturnRef(addresses[i])); + if (disable_listener) { + EXPECT_CALL(*static_cast(mock_listeners[i]), disable()); + } + } + + return listeners_.back().get(); + } + void validateOriginalDst(Network::TcpListenerCallbacks** listener_callbacks, TestListener* test_listener, Network::MockListener* listener) { Network::Address::InstanceConstSharedPtr normal_address( @@ -500,6 +539,47 @@ TEST_F(ConnectionHandlerTest, RemoveListener) { handler_->removeListeners(0); } +TEST_F(ConnectionHandlerTest, RemoveListenerWithMultiAddrs) { + InSequence s; + + Network::TcpListenerCallbacks* listener_callbacks; + auto listener1 = new NiceMock(); + auto listener2 = new NiceMock(); + std::vector mock_listeners; + mock_listeners.emplace_back(listener1); + mock_listeners.emplace_back(listener2); + Network::Address::InstanceConstSharedPtr address1( + new Network::Address::Ipv4Instance("127.0.0.1", 80, nullptr)); + Network::Address::InstanceConstSharedPtr address2( + new Network::Address::Ipv4Instance("127.0.0.2", 80, nullptr)); + std::vector addresses; + addresses.emplace_back(address1); + addresses.emplace_back(address2); + TestListener* test_listener = addMultiAddrsListener( + 1, true, false, "test_listener", mock_listeners, addresses, &listener_callbacks); + handler_->addListener(absl::nullopt, *test_listener, runtime_); + + Network::MockConnectionSocket* connection = new NiceMock(); + EXPECT_CALL(*access_log_, log(_, _, _, _)); + listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); + EXPECT_EQ(0UL, handler_->numConnections()); + + // Test stop/remove of not existent listener. + handler_->stopListeners(0); + handler_->removeListeners(0); + + EXPECT_CALL(*listener1, onDestroy()); + EXPECT_CALL(*listener2, onDestroy()); + handler_->stopListeners(1); + EXPECT_CALL(dispatcher_, clearDeferredDeleteList()).Times(2); + handler_->removeListeners(1); + EXPECT_EQ(0UL, handler_->numConnections()); + + // Test stop/remove of not existent listener. + handler_->stopListeners(0); + handler_->removeListeners(0); +} + TEST_F(ConnectionHandlerTest, DisableListener) { InSequence s; @@ -517,6 +597,32 @@ TEST_F(ConnectionHandlerTest, DisableListener) { handler_->disableListeners(); } +TEST_F(ConnectionHandlerTest, DisableListenerWithMultiAddrs) { + Network::TcpListenerCallbacks* listener_callbacks; + auto listener1 = new NiceMock(); + auto listener2 = new NiceMock(); + std::vector mock_listeners; + mock_listeners.emplace_back(listener1); + mock_listeners.emplace_back(listener2); + Network::Address::InstanceConstSharedPtr address1( + new Network::Address::Ipv4Instance("127.0.0.1", 80, nullptr)); + Network::Address::InstanceConstSharedPtr address2( + new Network::Address::Ipv4Instance("127.0.0.2", 80, nullptr)); + std::vector addresses; + addresses.emplace_back(address1); + addresses.emplace_back(address2); + TestListener* test_listener = addMultiAddrsListener( + 1, false, false, "test_listener", mock_listeners, addresses, &listener_callbacks); + handler_->addListener(absl::nullopt, *test_listener, runtime_); + + EXPECT_CALL(*listener1, disable()); + EXPECT_CALL(*listener2, disable()); + EXPECT_CALL(*listener1, onDestroy()); + EXPECT_CALL(*listener2, onDestroy()); + + handler_->disableListeners(); +} + // Envoy doesn't have such case yet, just ensure the code won't break with it. TEST_F(ConnectionHandlerTest, StopAndDisableStoppedListener) { InSequence s; @@ -555,6 +661,29 @@ TEST_F(ConnectionHandlerTest, AddDisabledListener) { handler_->addListener(absl::nullopt, *test_listener, runtime_); } +TEST_F(ConnectionHandlerTest, AddDisabledListenerWithMultiAddrs) { + Network::TcpListenerCallbacks* listener_callbacks; + auto listener1 = new NiceMock(); + auto listener2 = new NiceMock(); + std::vector mock_listeners; + mock_listeners.emplace_back(listener1); + mock_listeners.emplace_back(listener2); + Network::Address::InstanceConstSharedPtr address1( + new Network::Address::Ipv4Instance("127.0.0.1", 80, nullptr)); + Network::Address::InstanceConstSharedPtr address2( + new Network::Address::Ipv4Instance("127.0.0.2", 80, nullptr)); + std::vector addresses; + addresses.emplace_back(address1); + addresses.emplace_back(address2); + TestListener* test_listener = addMultiAddrsListener( + 1, false, false, "test_listener", mock_listeners, addresses, &listener_callbacks, true); + EXPECT_CALL(*listener1, onDestroy()); + EXPECT_CALL(*listener2, onDestroy()); + + handler_->disableListeners(); + handler_->addListener(absl::nullopt, *test_listener, runtime_); +} + TEST_F(ConnectionHandlerTest, SetListenerRejectFraction) { InSequence s; @@ -753,6 +882,70 @@ TEST_F(ConnectionHandlerTest, NormalRedirect) { EXPECT_CALL(*listener1, onDestroy()); } +TEST_F(ConnectionHandlerTest, NormalRedirectWithMultiAddrs) { + Network::TcpListenerCallbacks* listener_callbacks1; + auto listener1 = new NiceMock(); + auto listener2 = new NiceMock(); + std::vector mock_listeners; + mock_listeners.emplace_back(listener1); + mock_listeners.emplace_back(listener2); + Network::Address::InstanceConstSharedPtr normal_address( + new Network::Address::Ipv4Instance("127.0.0.1", 10001, nullptr)); + Network::Address::InstanceConstSharedPtr alt_address( + new Network::Address::Ipv4Instance("127.0.0.2", 20002, nullptr)); + std::vector addresses; + addresses.emplace_back(normal_address); + addresses.emplace_back(alt_address); + TestListener* test_listener1 = addMultiAddrsListener( + 1, true, true, "test_listener1", mock_listeners, addresses, &listener_callbacks1); + handler_->addListener(absl::nullopt, *test_listener1, runtime_); + + auto* test_filter = new NiceMock(); + EXPECT_CALL(*test_filter, destroy_()); + Network::MockConnectionSocket* accepted_socket = new NiceMock(); + bool redirected = false; + EXPECT_CALL(factory_, createListenerFilterChain(_)) + .WillRepeatedly(Invoke([&](Network::ListenerFilterManager& manager) -> bool { + // Insert the Mock filter. + if (!redirected) { + manager.addAcceptFilter(nullptr, Network::ListenerFilterPtr{test_filter}); + redirected = true; + } + return true; + })); + EXPECT_CALL(*test_filter, onAccept(_)) + .WillOnce(Invoke([&](Network::ListenerFilterCallbacks& cb) -> Network::FilterStatus { + cb.socket().connectionInfoProvider().restoreLocalAddress(alt_address); + return Network::FilterStatus::Continue; + })); + EXPECT_CALL(manager_, findFilterChain(_)).WillOnce(Return(filter_chain_.get())); + auto* connection = new NiceMock(); + EXPECT_CALL(dispatcher_, createServerConnection_()).WillOnce(Return(connection)); + EXPECT_CALL(factory_, createNetworkFilterChain(_, _)).WillOnce(Return(true)); + listener_callbacks1->onAccept(Network::ConnectionSocketPtr{accepted_socket}); + + // Verify per-listener connection stats. + EXPECT_EQ(1UL, handler_->numConnections()); + EXPECT_EQ(1UL, TestUtility::findCounter(stats_store_, "downstream_cx_total")->value()); + EXPECT_EQ(1UL, TestUtility::findGauge(stats_store_, "downstream_cx_active")->value()); + EXPECT_EQ(1UL, TestUtility::findCounter(stats_store_, "test.downstream_cx_total")->value()); + EXPECT_EQ(1UL, TestUtility::findGauge(stats_store_, "test.downstream_cx_active")->value()); + + EXPECT_CALL(*access_log_, log(_, _, _, _)) + .WillOnce( + Invoke([&](const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, + const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(alt_address, stream_info.downstreamAddressProvider().localAddress()); + })); + connection->close(Network::ConnectionCloseType::NoFlush); + dispatcher_.clearDeferredDeleteList(); + EXPECT_EQ(0UL, TestUtility::findGauge(stats_store_, "downstream_cx_active")->value()); + EXPECT_EQ(0UL, TestUtility::findGauge(stats_store_, "test.downstream_cx_active")->value()); + + EXPECT_CALL(*listener2, onDestroy()); + EXPECT_CALL(*listener1, onDestroy()); +} + // When update a listener, the old listener will be stopped and the new listener will // be added into ConnectionHandler before remove the old listener from ConnectionHandler. // This test ensure ConnectionHandler can query the correct Listener when redirect the connection From 52d468178b02e253aa5f1b2e98ed96bf561d5903 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Fri, 17 Jun 2022 07:03:01 +0000 Subject: [PATCH 16/19] Address comments Signed-off-by: He Jie Xu --- source/server/connection_handler_impl.cc | 6 +++--- source/server/connection_handler_impl.h | 22 ++++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index dd3b38a2d85cc..93c5ef1357015 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -105,8 +105,8 @@ void ConnectionHandlerImpl::addListener(absl::optional overridden_list ASSERT(!listener_map_by_tag_.contains(config.listenerTag())); - for (auto& per_address_details : details->per_address_details_list_) { - // This map only store the new listener. + for (const auto& per_address_details : details->per_address_details_list_) { + // This map only stores the new listener. if (absl::holds_alternative>( per_address_details->typed_listener_)) { tcp_listener_map_by_address_.insert_or_assign(per_address_details->address_->asStringView(), @@ -154,7 +154,7 @@ void ConnectionHandlerImpl::removeListeners(uint64_t listener_tag) { listener_iter != listener_map_by_tag_.end()) { // listener_map_by_address_ may already update to the new listener. Compare it with the one // which find from listener_map_by_tag_, only delete it when it is same listener. - for (auto& per_address_details : listener_iter->second->per_address_details_list_) { + for (const auto& per_address_details : listener_iter->second->per_address_details_list_) { auto& address = per_address_details->address_; auto address_view = address->asStringView(); if (tcp_listener_map_by_address_.contains(address_view) && diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index 3b97c5deedb5f..95ed03f04e928 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -92,6 +92,9 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, using ListenerMethodFn = std::function; + /** + * A helper to execute specific method on each PerAddressActiveListenerDetails item. + */ void invokeListenerMethod(ListenerMethodFn fn) { std::for_each(per_address_details_list_.begin(), per_address_details_list_.end(), [&fn](std::shared_ptr& details) { @@ -99,23 +102,26 @@ class ConnectionHandlerImpl : public Network::TcpConnectionHandler, }); } + /** + * Add an ActiveListener into the list. + */ template void addActiveListener(Network::ListenerConfig& config, const Network::ListenSocketFactoryPtr& socket_factory, UnitFloat& listener_reject_fraction, bool disable_listeners, ActiveListener&& listener) { - auto pre_address_details = std::make_shared(); - pre_address_details->typed_listener_ = *listener; - pre_address_details->listener_ = std::move(listener); - pre_address_details->address_ = socket_factory->localAddress(); + auto per_address_details = std::make_shared(); + per_address_details->typed_listener_ = *listener; + per_address_details->listener_ = std::move(listener); + per_address_details->address_ = socket_factory->localAddress(); if (disable_listeners) { - pre_address_details->listener_->pauseListening(); + per_address_details->listener_->pauseListening(); } - if (auto* listener = pre_address_details->listener_->listener(); listener != nullptr) { + if (auto* listener = per_address_details->listener_->listener(); listener != nullptr) { listener->setRejectFraction(listener_reject_fraction); } - pre_address_details->listener_tag_ = config.listenerTag(); - per_address_details_list_.emplace_back(pre_address_details); + per_address_details->listener_tag_ = config.listenerTag(); + per_address_details_list_.emplace_back(per_address_details); } }; From 289179d326d57e280e364f6138af20415de5b38d Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Fri, 17 Jun 2022 23:27:52 +0000 Subject: [PATCH 17/19] fix coverage Signed-off-by: He Jie Xu --- test/per_file_coverage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 6688511a4dca3..c6d5af030b5ac 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -88,7 +88,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/watchdog:83.3" # Death tests within extensions "source/extensions/watchdog/profile_action:83.3" "source/server:93.3" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239 -"source/server/admin:97.5" # TODO(soulxu) try to raise this back to `97.6` when multiple addresses listener implemented and the old interface of Network::ListenerConfig is removed. +"source/server/admin:97.4" # TODO(soulxu) try to raise this back to `97.6` when multiple addresses listener implemented and the old interface of Network::ListenerConfig is removed. "source/server/config_validation:74.8" ) From 2bb45fe6b338d0e1ba6e5151bfaaed346dd499d6 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Fri, 17 Jun 2022 23:57:04 +0000 Subject: [PATCH 18/19] Add more test for inplace update Signed-off-by: He Jie Xu --- .../active_internal_listener_test.cc | 41 +++++++++++++++++ test/server/connection_handler_test.cc | 46 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc index 695dd9d68769c..fda639b60ca14 100644 --- a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc +++ b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc @@ -457,6 +457,47 @@ TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdate) { dispatcher_.clearDeferredDeleteList(); } +TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdateWithoutUdpInplaceUpdateSupport) { + runtime_.mergeValues({{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); + InSequence s; + uint64_t old_listener_tag = 1; + uint64_t new_listener_tag = 2; + Network::Address::InstanceConstSharedPtr local_address{ + new Network::Address::EnvoyInternalInstance("server_internal_address")}; + + TestListener* internal_listener = addInternalListener( + old_listener_tag, "test_internal_listener", std::chrono::milliseconds(), false, nullptr); + EXPECT_CALL(*static_cast( + internal_listener->socket_factories_[0].get()), + localAddress()) + .WillRepeatedly(ReturnRef(local_address)); + handler_->addListener(absl::nullopt, *internal_listener, runtime_); + + ASSERT_NE(internal_listener, nullptr); + + auto overridden_filter_chain_manager = + std::make_shared>(); + TestListener* new_test_listener = + addInternalListener(new_listener_tag, "test_internal_listener", std::chrono::milliseconds(), + false, overridden_filter_chain_manager); + handler_->addListener(old_listener_tag, *new_test_listener, runtime_); + + Network::MockConnectionSocket* connection = new NiceMock(); + + auto internal_listener_cb = handler_->findByAddress(local_address); + + EXPECT_CALL(manager_, findFilterChain(_)).Times(0); + EXPECT_CALL(*overridden_filter_chain_manager, findFilterChain(_)).WillOnce(Return(nullptr)); + EXPECT_CALL(*access_log_, log(_, _, _, _)); + internal_listener_cb.value().get().onAccept(Network::ConnectionSocketPtr{connection}); + EXPECT_EQ(0UL, handler_->numConnections()); + + testing::MockFunction completion; + handler_->removeFilterChains(old_listener_tag, {}, completion.AsStdFunction()); + EXPECT_CALL(completion, Call()); + dispatcher_.clearDeferredDeleteList(); +} + } // namespace } // namespace InternalListener } // namespace Bootstrap diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index e19eda0454f36..3c5de643ad803 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -2185,6 +2185,52 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdate) { EXPECT_CALL(*old_listener, onDestroy()); } +TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdateWithoutUdpInplaceSupport) { + runtime_.mergeValues({{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); + InSequence s; + uint64_t old_listener_tag = 1; + uint64_t new_listener_tag = 2; + Network::TcpListenerCallbacks* old_listener_callbacks; + Network::BalancedConnectionHandler* current_handler; + + auto old_listener = new NiceMock(); + auto mock_connection_balancer = std::make_shared(); + + TestListener* old_test_listener = + addListener(old_listener_tag, true, false, "test_listener", old_listener, + &old_listener_callbacks, mock_connection_balancer, ¤t_handler); + EXPECT_CALL(old_test_listener->socketFactory(), localAddress()) + .WillRepeatedly(ReturnRef(local_address_)); + handler_->addListener(absl::nullopt, *old_test_listener, runtime_); + ASSERT_NE(old_test_listener, nullptr); + + Network::TcpListenerCallbacks* new_listener_callbacks = nullptr; + + auto overridden_filter_chain_manager = + std::make_shared>(); + TestListener* new_test_listener = addListener( + new_listener_tag, true, false, "test_listener", /* Network::Listener */ nullptr, + &new_listener_callbacks, mock_connection_balancer, nullptr, Network::Socket::Type::Stream, + std::chrono::milliseconds(15000), false, overridden_filter_chain_manager); + EXPECT_CALL(new_test_listener->socketFactory(), socketType()).WillOnce(Return(Network::Socket::Type::Stream)); + handler_->addListener(old_listener_tag, *new_test_listener, runtime_); + ASSERT_EQ(new_listener_callbacks, nullptr) + << "new listener should be inplace added and callback should not change"; + + Network::MockConnectionSocket* connection = new NiceMock(); + current_handler->incNumConnections(); + + EXPECT_CALL(*mock_connection_balancer, pickTargetHandler(_)) + .WillOnce(ReturnRef(*current_handler)); + EXPECT_CALL(manager_, findFilterChain(_)).Times(0); + EXPECT_CALL(*overridden_filter_chain_manager, findFilterChain(_)).WillOnce(Return(nullptr)); + EXPECT_CALL(*access_log_, log(_, _, _, _)); + EXPECT_CALL(*mock_connection_balancer, unregisterHandler(_)); + old_listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); + EXPECT_EQ(0UL, handler_->numConnections()); + EXPECT_CALL(*old_listener, onDestroy()); +} + TEST_F(ConnectionHandlerTest, TcpListenerRemoveFilterChain) { InSequence s; uint64_t listener_tag = 1; From 5ceed88ef1cae1b700cd42543509b3d52bf60149 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Fri, 17 Jun 2022 23:57:48 +0000 Subject: [PATCH 19/19] fix format Signed-off-by: He Jie Xu --- .../internal_listener/active_internal_listener_test.cc | 3 ++- test/server/connection_handler_test.cc | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc index fda639b60ca14..5cb818a9ecf14 100644 --- a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc +++ b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc @@ -458,7 +458,8 @@ TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdate) { } TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdateWithoutUdpInplaceUpdateSupport) { - runtime_.mergeValues({{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); + runtime_.mergeValues( + {{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); InSequence s; uint64_t old_listener_tag = 1; uint64_t new_listener_tag = 2; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 3c5de643ad803..225508b64b308 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -2186,7 +2186,8 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdate) { } TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdateWithoutUdpInplaceSupport) { - runtime_.mergeValues({{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); + runtime_.mergeValues( + {{"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "false"}}); InSequence s; uint64_t old_listener_tag = 1; uint64_t new_listener_tag = 2; @@ -2212,7 +2213,8 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdateWithoutUdpInplaceSupport) new_listener_tag, true, false, "test_listener", /* Network::Listener */ nullptr, &new_listener_callbacks, mock_connection_balancer, nullptr, Network::Socket::Type::Stream, std::chrono::milliseconds(15000), false, overridden_filter_chain_manager); - EXPECT_CALL(new_test_listener->socketFactory(), socketType()).WillOnce(Return(Network::Socket::Type::Stream)); + EXPECT_CALL(new_test_listener->socketFactory(), socketType()) + .WillOnce(Return(Network::Socket::Type::Stream)); handler_->addListener(old_listener_tag, *new_test_listener, runtime_); ASSERT_EQ(new_listener_callbacks, nullptr) << "new listener should be inplace added and callback should not change";