In quic client connection, reset rtt and cwnd after successfully migrate to a new IP address if using IETF version. To code merger: in the two call sites of QuicConnection::OnSuccessfulMigration() OnConnectionMigrationProbeSucceeded() and OnPortMigrationProbeSucceeded(), pass false in the former caller and true in the latter caller. client side change only. PiperOrigin-RevId: 358208758 Change-Id: I63dd3dc8b06dec3c3d1922bbe9d4d064de31f00a
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 5b35138..81a15ad 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -1088,7 +1088,7 @@ } } -void QuicConnection::OnSuccessfulMigration() { +void QuicConnection::OnSuccessfulMigration(bool is_port_change) { QUICHE_DCHECK_EQ(perspective_, Perspective::IS_CLIENT); if (IsPathDegrading()) { // If path was previously degrading, and migration is successful after @@ -1100,8 +1100,10 @@ // Reset alternative path state even if it is still under validation. alternative_path_.Clear(); } - // TODO(b/159074035): notify SentPacketManger with RTT sample from probing and - // reset cwnd if this is a successful network migration. + // TODO(b/159074035): notify SentPacketManger with RTT sample from probing. + if (version().HasIetfQuicFrames() && !is_port_change) { + sent_packet_manager_.OnConnectionMigration(/*reset_send_algorithm=*/true); + } } void QuicConnection::OnTransportParametersSent( @@ -5780,10 +5782,15 @@ if (!connected_) { return; } + const bool is_port_change = + QuicUtils::DetermineAddressChangeType(default_path_.self_address, + self_address) == PORT_CHANGE && + QuicUtils::DetermineAddressChangeType(default_path_.peer_address, + peer_address) == PORT_CHANGE; SetSelfAddress(self_address); UpdatePeerAddress(peer_address); SetQuicPacketWriter(writer, owns_writer); - OnSuccessfulMigration(); + OnSuccessfulMigration(is_port_change); } std::vector<QuicConnectionId> QuicConnection::GetActiveServerConnectionIds()
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 47a295a..ab111e5 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1095,7 +1095,7 @@ void OnSuccessfulVersionNegotiation(); // Called when self migration succeeds after probing. - void OnSuccessfulMigration(); + void OnSuccessfulMigration(bool is_port_change); // Called for QUIC+TLS versions when we send transport parameters. void OnTransportParametersSent(
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 74534df..910af57 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -699,6 +699,8 @@ .Times(AnyNumber()); EXPECT_CALL(*send_algorithm_, InSlowStart()).Times(AnyNumber()); EXPECT_CALL(*send_algorithm_, InRecovery()).Times(AnyNumber()); + EXPECT_CALL(*send_algorithm_, GetCongestionControlType()) + .Times(AnyNumber()); EXPECT_CALL(*send_algorithm_, OnApplicationLimited(_)).Times(AnyNumber()); EXPECT_CALL(visitor_, WillingAndAbleToWrite()).Times(AnyNumber()); EXPECT_CALL(visitor_, OnPacketDecrypted(_)).Times(AnyNumber()); @@ -8594,12 +8596,6 @@ TEST_P(QuicConnectionTest, RestartPathDegradingDetectionAfterMigrationWithProbe) { - // TODO(b/150095484): add test coverage for IETF to verify that client takes - // PATH RESPONSE with peer address change as correct validation on the new - // path. - if (GetParam().version.HasIetfQuicFrames()) { - return; - } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); PathProbeTestInit(Perspective::IS_CLIENT); @@ -8622,24 +8618,21 @@ EXPECT_TRUE(connection_.IsPathDegrading()); EXPECT_FALSE(connection_.PathDegradingDetectionInProgress()); - // Simulate path degrading handling by sending a probe on an alternet path. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); - TestPacketWriter probing_writer(version(), &clock_, Perspective::IS_CLIENT); - connection_.SendConnectivityProbingPacket(&probing_writer, - connection_.peer_address()); - // Verify that path degrading detection is not reset. - EXPECT_FALSE(connection_.PathDegradingDetectionInProgress()); - - // Simulate successful path degrading handling by receiving probe response. - clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(20)); - if (!GetParam().version.HasIetfQuicFrames()) { + // Simulate path degrading handling by sending a probe on an alternet path. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); + TestPacketWriter probing_writer(version(), &clock_, Perspective::IS_CLIENT); + connection_.SendConnectivityProbingPacket(&probing_writer, + connection_.peer_address()); + // Verify that path degrading detection is not reset. + EXPECT_FALSE(connection_.PathDegradingDetectionInProgress()); + + // Simulate successful path degrading handling by receiving probe response. + clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(20)); + EXPECT_CALL(visitor_, OnPacketReceived(_, _, /*is_connectivity_probe=*/true)) .Times(1); - } else { - EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0); - } const QuicSocketAddress kNewSelfAddress = QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456); @@ -8657,14 +8650,46 @@ EXPECT_EQ(kPeerAddress, connection_.peer_address()); EXPECT_EQ(kPeerAddress, connection_.effective_peer_address()); EXPECT_TRUE(connection_.IsPathDegrading()); + } // Verify new path degrading detection is activated. EXPECT_CALL(visitor_, OnForwardProgressMadeAfterPathDegrading()).Times(1); - connection_.OnSuccessfulMigration(); + connection_.OnSuccessfulMigration(/*is_port_change*/ true); EXPECT_FALSE(connection_.IsPathDegrading()); EXPECT_TRUE(connection_.PathDegradingDetectionInProgress()); } +TEST_P(QuicConnectionTest, ClientsResetCwndAfterConnectionMigration) { + if (!GetParam().version.HasIetfQuicFrames()) { + return; + } + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + PathProbeTestInit(Perspective::IS_CLIENT); + EXPECT_EQ(kSelfAddress, connection_.self_address()); + + RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats()); + QuicTime::Delta default_init_rtt = rtt_stats->initial_rtt(); + rtt_stats->set_initial_rtt(default_init_rtt * 2); + EXPECT_EQ(2 * default_init_rtt, rtt_stats->initial_rtt()); + + QuicSentPacketManagerPeer::SetConsecutiveRtoCount(manager_, 1); + EXPECT_EQ(1u, manager_->GetConsecutiveRtoCount()); + QuicSentPacketManagerPeer::SetConsecutiveTlpCount(manager_, 2); + EXPECT_EQ(2u, manager_->GetConsecutiveTlpCount()); + const SendAlgorithmInterface* send_algorithm = manager_->GetSendAlgorithm(); + + // Migrate to a new address with different IP. + const QuicSocketAddress kNewSelfAddress = + QuicSocketAddress(QuicIpAddress::Loopback4(), /*port=*/23456); + TestPacketWriter new_writer(version(), &clock_, Perspective::IS_CLIENT); + connection_.MigratePath(kNewSelfAddress, connection_.peer_address(), + &new_writer, false); + EXPECT_EQ(default_init_rtt, manager_->GetRttStats()->initial_rtt()); + EXPECT_EQ(0u, manager_->GetConsecutiveRtoCount()); + EXPECT_EQ(0u, manager_->GetConsecutiveTlpCount()); + EXPECT_NE(send_algorithm, manager_->GetSendAlgorithm()); +} + // Regression test for b/110259444 TEST_P(QuicConnectionTest, DoNotScheduleSpuriousAckAlarm) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));