lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-24 04:01:07 +03:00
Code Activity

fix numMigrations increment

Browse Source 
Summary: we should only increment numMigrations if the peer is attempting migration from a previously unseen addr

Reviewed By: kvtsoy, sharmafb

Differential Revision: D55884605

fbshipit-source-id: 9204ed785195a321d7f8f7497151b7cfe5f745f7
This commit is contained in:
Hani Damlaj
2024-04-09 10:47:55 -07:00
committed by Facebook GitHub Bot
parent a8543fd9b8
commit 1730ed8a73
2 changed files with 26 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/server/state/ServerStateMachine.cpp
View File

@@ -579,7 +579,6 @@ void onConnectionMigration(
throw QuicTransportException( throw QuicTransportException(
"Too many migrations", TransportErrorCode::INVALID_MIGRATION); "Too many migrations", TransportErrorCode::INVALID_MIGRATION);
} }
++conn.migrationState.numMigrations;
bool hasPendingPathChallenge = conn.pendingEvents.pathChallenge.has_value(); bool hasPendingPathChallenge = conn.pendingEvents.pathChallenge.has_value();
// Clear any pending path challenge frame that is not sent // Clear any pending path challenge frame that is not sent
@@ -591,6 +590,7 @@ void onConnectionMigration(
previousPeerAddresses.end(), previousPeerAddresses.end(),
newPeerAddress); newPeerAddress);
if (it == previousPeerAddresses.end()) { if (it == previousPeerAddresses.end()) {
++conn.migrationState.numMigrations;
// send new path challenge // send new path challenge
conn.pendingEvents.pathChallenge.emplace(folly::Random::secureRand64()); conn.pendingEvents.pathChallenge.emplace(folly::Random::secureRand64());

25
quic/server/test/QuicServerTransportTest.cpp
View File

@@ -1849,6 +1849,7 @@ TEST_P(
EXPECT_EQ( EXPECT_EQ(
event->dropReason, event->dropReason,
PacketDropReason(PacketDropReason::PEER_ADDRESS_CHANGE)._to_string()); PacketDropReason(PacketDropReason::PEER_ADDRESS_CHANGE)._to_string());
EXPECT_EQ(server->getConn().migrationState.numMigrations, 0);
} }
TEST_P( TEST_P(
@@ -1872,6 +1873,8 @@ TEST_P(
0 /* cipherOverhead */, 0 /* cipherOverhead */,
0 /* largestAcked */)); 0 /* largestAcked */));
EXPECT_EQ(server->getConn().migrationState.numMigrations, 0);
// Receive second packet first // Receive second packet first
deliverData(std::move(secondPacket)); deliverData(std::move(secondPacket));
@@ -1884,6 +1887,7 @@ TEST_P(
// No migration for reordered packet // No migration for reordered packet
EXPECT_EQ(server->getConn().peerAddress, peerAddress); EXPECT_EQ(server->getConn().peerAddress, peerAddress);
EXPECT_EQ(server->getConn().migrationState.numMigrations, 0);
} }
TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) { TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) {
@@ -1898,6 +1902,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) {
0 /* largestAcked */)); 0 /* largestAcked */));
EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0);
EXPECT_EQ(server->getConn().migrationState.numMigrations, 0);
auto peerAddress = server->getConn().peerAddress; auto peerAddress = server->getConn().peerAddress;
auto congestionController = server->getConn().congestionController.get(); auto congestionController = server->getConn().congestionController.get();
@@ -1912,6 +1917,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) {
EXPECT_TRUE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().pendingEvents.pathChallenge);
EXPECT_EQ(server->getConn().peerAddress, newPeer); EXPECT_EQ(server->getConn().peerAddress, newPeer);
EXPECT_EQ(server->getConn().migrationState.numMigrations, 1);
EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1);
EXPECT_EQ( EXPECT_EQ(
server->getConn().migrationState.previousPeerAddresses.back(), server->getConn().migrationState.previousPeerAddresses.back(),
@@ -1962,6 +1968,21 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) {
EXPECT_FALSE(server->getConn().outstandingPathValidation); EXPECT_FALSE(server->getConn().outstandingPathValidation);
EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout); EXPECT_FALSE(server->getConn().pendingEvents.schedulePathValidationTimeout);
EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled()); EXPECT_FALSE(server->pathValidationTimeout().isTimerCallbackScheduled());
// receiving data from the original peer address should not increment
// numMigrations counter
EXPECT_CALL(*quicStats_, onPeerAddressChanged).Times(1);
auto nextPacketData = packetToBuf(createStreamPacket(
*clientConnectionId,
*server->getConn().serverConnectionId,
clientNextAppDataPacketNum++,
6,
*data,
0 /* cipherOverhead */,
0 /* largestAcked */));
deliverData(std::move(nextPacketData), false);
EXPECT_EQ(server->getConn().migrationState.numMigrations, 1);
} }
TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) { TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) {
@@ -1975,6 +1996,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) {
0 /* cipherOverhead */, 0 /* cipherOverhead */,
0 /* largestAcked */)); 0 /* largestAcked */));
EXPECT_EQ(server->getConn().migrationState.numMigrations, 0);
EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 0);
auto peerAddress = server->getConn().peerAddress; auto peerAddress = server->getConn().peerAddress;
@@ -1989,6 +2011,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) {
EXPECT_TRUE(server->getConn().pendingEvents.pathChallenge); EXPECT_TRUE(server->getConn().pendingEvents.pathChallenge);
EXPECT_EQ(server->getConn().peerAddress, newPeer); EXPECT_EQ(server->getConn().peerAddress, newPeer);
EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1); EXPECT_EQ(server->getConn().migrationState.previousPeerAddresses.size(), 1);
EXPECT_EQ(server->getConn().migrationState.numMigrations, 1);
EXPECT_EQ( EXPECT_EQ(
server->getConn().migrationState.previousPeerAddresses.back(), server->getConn().migrationState.previousPeerAddresses.back(),
peerAddress); peerAddress);
@@ -2190,6 +2213,8 @@ TEST_F(QuicServerTransportTest, TooManyMigrations) {
EXPECT_CALL(*quicStats_, onPeerAddressChanged).Times(1); EXPECT_CALL(*quicStats_, onPeerAddressChanged).Times(1);
deliverData(packetData->clone(), false, &newPeer); deliverData(packetData->clone(), false, &newPeer);
} }
EXPECT_EQ(
server->getConn().migrationState.numMigrations, maxNumMigrationsAllowed);
folly::SocketAddress newPeer("200.101.102.103", 23456); folly::SocketAddress newPeer("200.101.102.103", 23456);
try { try {