Skip to content

Commit

Permalink
[quic]impose a limit on the number of connection migrations (envoypro…
Browse files Browse the repository at this point in the history
…xy#30109)

Commit Message: Limit the max number of connection migrations to 5.
Additional Description: This prevents too many new sockets being created.
Risk Level: Low. Can be turned off via num_timeouts_to_trigger_port_migration proto
Testing: integration test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Renjie Tang <[email protected]>
  • Loading branch information
RenjieTang authored Oct 12, 2023
1 parent 0537006 commit 73dc561
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 7 deletions.
5 changes: 4 additions & 1 deletion source/common/quic/envoy_quic_client_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions source/common/quic/envoy_quic_client_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion source/common/quic/quic_network_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class QuicNetworkConnection : protected Logger::Loggable<Logger::Id::connection>
// 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<Network::ConnectionSocketPtr> connection_sockets_;
// Points to an instance of EnvoyQuicServerSession or EnvoyQuicClientSession.
Network::Connection* envoy_connection_{nullptr};
Expand Down
50 changes: 45 additions & 5 deletions test/integration/quic_http_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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};
};

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 73dc561

Please sign in to comment.