diff --git a/source/common/quic/envoy_quic_client_connection.cc b/source/common/quic/envoy_quic_client_connection.cc index 8e2ba1aa49b2..09a8745572bd 100644 --- a/source/common/quic/envoy_quic_client_connection.cc +++ b/source/common/quic/envoy_quic_client_connection.cc @@ -106,6 +106,7 @@ void EnvoyQuicClientConnection::switchConnectionSocket( connection_socket->connectionInfoProvider().remoteAddress()->ip()); // The old socket is not closed in this call, because it could still receive useful packets. + num_socket_switches_++; setConnectionSocket(std::move(connection_socket)); setUpConnectionSocket(*connectionSocket(), delegate_); MigratePath(self_address, peer_address, writer.release(), true); @@ -117,7 +118,8 @@ void EnvoyQuicClientConnection::OnPathDegradingDetected() { } void EnvoyQuicClientConnection::maybeMigratePort() { - if (!IsHandshakeConfirmed() || HasPendingPathValidation() || !migrate_port_on_path_degrading_) { + if (!IsHandshakeConfirmed() || HasPendingPathValidation() || !migrate_port_on_path_degrading_ || + num_socket_switches_ >= kMaxNumSocketSwitches) { return; } @@ -172,6 +174,7 @@ void EnvoyQuicClientConnection::onPathValidationSuccess( peer_address() == envoy_context->peer_address()) { // probing_socket will be set as the new default socket. But old sockets are still able to // receive packets. + num_socket_switches_++; setConnectionSocket(std::move(probing_socket)); return; } diff --git a/source/common/quic/envoy_quic_client_connection.h b/source/common/quic/envoy_quic_client_connection.h index 5dc469049fd7..2ddd3e523e76 100644 --- a/source/common/quic/envoy_quic_client_connection.h +++ b/source/common/quic/envoy_quic_client_connection.h @@ -12,6 +12,9 @@ namespace Envoy { namespace Quic { +// Limits the max number of sockets created. +constexpr uint8_t kMaxNumSocketSwitches = 5; + class PacketsToReadDelegate { public: virtual ~PacketsToReadDelegate() = default; @@ -143,6 +146,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection, uint32_t packets_dropped_{0}; Event::Dispatcher& dispatcher_; bool migrate_port_on_path_degrading_{false}; + uint8_t num_socket_switches_{0}; }; } // namespace Quic diff --git a/source/common/quic/quic_network_connection.h b/source/common/quic/quic_network_connection.h index 3a6783d75ae1..828f3eb1f761 100644 --- a/source/common/quic/quic_network_connection.h +++ b/source/common/quic/quic_network_connection.h @@ -61,7 +61,6 @@ class QuicNetworkConnection : protected Logger::Loggable // Hosts a list of active sockets, while only the last one is used for writing data. // Hosts a single default socket upon construction. New sockets can be pushed in later as a result // of QUIC connection migration. - // TODO(renjietang): Impose an upper limit. std::vector connection_sockets_; // Points to an instance of EnvoyQuicServerSession or EnvoyQuicClientSession. Network::Connection* envoy_connection_{nullptr}; diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 528a93d00d12..fae519289a25 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -92,6 +92,8 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection { return AssertionFailure() << "Timed out waiting for path response\n"; } } + waiting_for_path_response_ = false; + saw_path_response_ = false; return AssertionSuccess(); } @@ -133,12 +135,42 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection { return EnvoyQuicClientConnection::OnHandshakeDoneFrame(frame); } + AssertionResult waitForNewCid(std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { + bool timer_fired = false; + if (!saw_new_cid_) { + Event::TimerPtr timer(dispatcher_.createTimer([this, &timer_fired]() -> void { + timer_fired = true; + dispatcher_.exit(); + })); + timer->enableTimer(timeout); + waiting_for_new_cid_ = true; + dispatcher_.run(Event::Dispatcher::RunType::Block); + if (timer_fired) { + return AssertionFailure() << "Timed out waiting for new cid\n"; + } + } + waiting_for_new_cid_ = false; + return AssertionSuccess(); + } + + bool OnNewConnectionIdFrame(const quic::QuicNewConnectionIdFrame& frame) override { + bool ret = EnvoyQuicClientConnection::OnNewConnectionIdFrame(frame); + saw_new_cid_ = true; + if (waiting_for_new_cid_) { + dispatcher_.exit(); + } + saw_new_cid_ = false; + return ret; + } + private: Event::Dispatcher& dispatcher_; bool saw_path_response_{false}; bool saw_handshake_done_{false}; + bool saw_new_cid_{false}; bool waiting_for_path_response_{false}; bool waiting_for_handshake_done_{false}; + bool waiting_for_new_cid_{false}; bool validation_failure_on_path_response_{false}; }; @@ -798,12 +830,20 @@ TEST_P(QuicHttpIntegrationTest, PortMigrationOnPathDegrading) { codec_client_->sendData(*request_encoder_, 1024u, false); ASSERT_TRUE(quic_connection_->waitForHandshakeDone()); - auto old_self_addr = quic_connection_->self_address(); - EXPECT_CALL(*option, setOption(_, _)).Times(3u); + + for (uint8_t i = 0; i < 5; i++) { + auto old_self_addr = quic_connection_->self_address(); + EXPECT_CALL(*option, setOption(_, _)).Times(3u); + quic_connection_->OnPathDegradingDetected(); + ASSERT_TRUE(quic_connection_->waitForPathResponse()); + auto self_addr = quic_connection_->self_address(); + EXPECT_NE(old_self_addr, self_addr); + ASSERT_TRUE(quic_connection_->waitForNewCid()); + } + + // port migration is disabled once socket switch limit is reached. + EXPECT_CALL(*option, setOption(_, _)).Times(0); quic_connection_->OnPathDegradingDetected(); - ASSERT_TRUE(quic_connection_->waitForPathResponse()); - auto self_addr = quic_connection_->self_address(); - EXPECT_NE(old_self_addr, self_addr); // Send the rest data. codec_client_->sendData(*request_encoder_, 1024u, true);