gfe-relnote: In QUIC sent packet manager, replace handshake_confirmed_ and forward_secure_packet_acked_ with handshake_state_. No functional change expected. Not protected.
Also rename QuicConnection::IsHandshakeConfirmed to QuicConnection::IsHandshakeComplete.
PiperOrigin-RevId: 281069350
Change-Id: Id40366a9148add245798b1805ae5f1e3a46d5fc6
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 0d5e1d8..d077196 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2508,7 +2508,8 @@
char* QuicConnection::GetPacketBuffer() {
if (version().CanSendCoalescedPackets() &&
- !sent_packet_manager_.forward_secure_packet_acked()) {
+ sent_packet_manager_.handshake_state() <
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED) {
// Do not use writer's packet buffer for coalesced packets which may contain
// multiple QUIC packets.
return nullptr;
@@ -2658,7 +2659,7 @@
void QuicConnection::OnRetransmissionTimeout() {
DCHECK(!sent_packet_manager_.unacked_packets().empty() ||
(sent_packet_manager_.handshake_mode_disabled() &&
- !sent_packet_manager_.handshake_confirmed()));
+ !IsHandshakeComplete()));
const QuicPacketNumber previous_created_packet_number =
packet_creator_.packet_number();
if (close_connection_after_five_rtos_ &&
@@ -3702,7 +3703,7 @@
DCHECK(fill_up_link_during_probing_);
// Don't send probing retransmissions until the handshake has completed.
- if (!sent_packet_manager_.handshake_confirmed() ||
+ if (!IsHandshakeComplete() ||
sent_packet_manager().HasUnackedCryptoPackets()) {
return;
}
@@ -3898,7 +3899,7 @@
if (perspective_ == Perspective::IS_CLIENT) {
return encryption_level_;
}
- if (sent_packet_manager_.handshake_confirmed()) {
+ if (IsHandshakeComplete()) {
// A forward secure packet has been received.
QUIC_BUG_IF(encryption_level_ != ENCRYPTION_FORWARD_SECURE)
<< ENDPOINT << "Unexpected connection close encryption level "
@@ -4113,7 +4114,8 @@
return SEND_TO_WRITER;
}
if (version().CanSendCoalescedPackets() &&
- !sent_packet_manager_.forward_secure_packet_acked() &&
+ sent_packet_manager_.handshake_state() <
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED &&
!is_mtu_discovery) {
// Before receiving ACK for any 1-RTT packets, always try to coalesce
// packet (except MTU discovery packet).
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 6277f14..90369fc 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -859,9 +859,10 @@
NOT_PADDED_PING, // Set if the packet is not {PING, PADDING}.
};
- // Whether the handshake is confirmed from this connection's perspective.
- bool IsHandshakeConfirmed() const {
- return sent_packet_manager_.handshake_confirmed();
+ // Whether the handshake completes from this connection's perspective.
+ bool IsHandshakeComplete() const {
+ return sent_packet_manager_.handshake_state() >=
+ QuicSentPacketManager::HANDSHAKE_COMPLETE;
}
// Returns the largest received packet number sent by peer.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 5194a9c..9c2a1c8 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -672,7 +672,7 @@
if (!QuicUtils::IsCryptoStreamId(transport_version(), id) &&
this->encryption_level() == ENCRYPTION_INITIAL) {
this->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- if (perspective() == Perspective::IS_CLIENT && !IsHandshakeConfirmed()) {
+ if (perspective() == Perspective::IS_CLIENT && !IsHandshakeComplete()) {
OnHandshakeComplete();
}
if (version().SupportsAntiAmplificationLimit()) {
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 91d9b69..31faade 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -578,7 +578,7 @@
!connection->termination_packets()->empty()) {
action = QuicTimeWaitListManager::SEND_TERMINATION_PACKETS;
} else {
- if (!connection->IsHandshakeConfirmed()) {
+ if (!connection->IsHandshakeComplete()) {
if (!VersionHasIetfInvariantHeader(connection->transport_version())) {
QUIC_CODE_COUNT(gquic_add_to_time_wait_list_with_handshake_failed);
} else {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 408cc2c..ba3c081 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -92,7 +92,7 @@
min_rto_timeout_(
QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs)),
largest_mtu_acked_(0),
- handshake_confirmed_(false),
+ handshake_state_(HANDSHAKE_START),
peer_max_ack_delay_(
QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)),
rtt_updated_(false),
@@ -101,7 +101,6 @@
max_probe_packets_per_pto_(2),
consecutive_pto_count_(0),
handshake_mode_disabled_(false),
- forward_secure_packet_acked_(false),
skip_packet_number_for_pto_(false),
always_include_max_ack_delay_for_pto_timeout_(true),
pto_exponential_backoff_start_point_(0),
@@ -337,11 +336,12 @@
}
void QuicSentPacketManager::SetHandshakeConfirmed() {
- if (!neuter_handshake_packets_once_ || !handshake_confirmed_) {
+ if (!neuter_handshake_packets_once_ ||
+ handshake_state_ < HANDSHAKE_COMPLETE) {
if (neuter_handshake_packets_once_) {
QUIC_RELOADABLE_FLAG_COUNT_N(quic_neuter_handshake_packets_once2, 1, 3);
}
- handshake_confirmed_ = true;
+ handshake_state_ = HANDSHAKE_COMPLETE;
NeuterHandshakePackets();
}
}
@@ -670,7 +670,7 @@
QuicSentPacketManager::RetransmissionTimeoutMode
QuicSentPacketManager::OnRetransmissionTimeout() {
DCHECK(unacked_packets_.HasInFlightPackets() ||
- (handshake_mode_disabled_ && !handshake_confirmed_));
+ (handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE));
DCHECK_EQ(0u, pending_timer_transmission_count_);
// Handshake retransmission, timer based loss detection, TLP, and RTO are
// implemented with a single alarm. The handshake alarm is set when the
@@ -848,8 +848,8 @@
QuicSentPacketManager::RetransmissionTimeoutMode
QuicSentPacketManager::GetRetransmissionMode() const {
DCHECK(unacked_packets_.HasInFlightPackets() ||
- (handshake_mode_disabled_ && !handshake_confirmed_));
- if (!handshake_mode_disabled_ && !handshake_confirmed_ &&
+ (handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE));
+ if (!handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE &&
unacked_packets_.HasPendingCryptoPackets()) {
return HANDSHAKE_MODE;
}
@@ -934,7 +934,7 @@
const QuicTime QuicSentPacketManager::GetRetransmissionTime() const {
if (!unacked_packets_.HasInFlightPackets() &&
- (!handshake_mode_disabled_ || handshake_confirmed_ ||
+ (!handshake_mode_disabled_ || handshake_state_ >= HANDSHAKE_COMPLETE ||
unacked_packets_.perspective() == Perspective::IS_SERVER)) {
// Do not set the timer if there is nothing in flight. However, to avoid
// handshake deadlock due to anti-amplification limit, client needs to set
@@ -1225,7 +1225,7 @@
}
last_ack_frame_.packets.Add(acked_packet.packet_number);
if (info->encryption_level == ENCRYPTION_FORWARD_SECURE) {
- forward_secure_packet_acked_ = true;
+ handshake_state_ = HANDSHAKE_CONFIRMED;
}
largest_packet_peer_knows_is_acked_.UpdateMax(info->largest_acked);
if (supports_multiple_packet_number_spaces()) {
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 4c2b429..5a3f2ca 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -151,8 +151,9 @@
return pacing_sender_.max_pacing_rate();
}
- // Set handshake_confirmed_ to true and neuter packets in HANDSHAKE packet
- // number space.
+ // Called to mark the handshake state complete, and all handshake packets are
+ // neutered.
+ // TODO(fayang): Rename this function to OnHandshakeComplete.
void SetHandshakeConfirmed();
// Requests retransmission of all unacked packets of |retransmission_type|.
@@ -363,7 +364,7 @@
return largest_packet_peer_knows_is_acked_;
}
- bool handshake_confirmed() const { return handshake_confirmed_; }
+ HandshakeState handshake_state() const { return handshake_state_; }
size_t pending_timer_transmission_count() const {
return pending_timer_transmission_count_;
@@ -414,10 +415,6 @@
bool handshake_mode_disabled() const { return handshake_mode_disabled_; }
- bool forward_secure_packet_acked() const {
- return forward_secure_packet_acked_;
- }
-
bool skip_packet_number_for_pto() const {
return skip_packet_number_for_pto_;
}
@@ -598,11 +595,8 @@
// Calls into |send_algorithm_| for the underlying congestion control.
PacingSender pacing_sender_;
- // Set to true after the crypto handshake has successfully completed. After
- // this is true we no longer use HANDSHAKE_MODE, and further frames sent on
- // the crypto stream (i.e. SCUP messages) are treated like normal
- // retransmittable frames.
- bool handshake_confirmed_;
+ // Indicates current handshake state.
+ HandshakeState handshake_state_;
// Records bandwidth from server to client in normal operation, over periods
// of time with no loss events.
@@ -644,10 +638,6 @@
// True if HANDSHAKE mode has been disabled.
bool handshake_mode_disabled_;
- // True if an acknowledgment has been received for a sent
- // ENCRYPTION_FORWARD_SECURE packet.
- bool forward_secure_packet_acked_;
-
// If true, skip packet number before sending the last PTO retransmission.
bool skip_packet_number_for_pto_;
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 3ea50fd..8dd0c30 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -2758,7 +2758,8 @@
}
TEST_F(QuicSentPacketManagerTest, ForwardSecurePacketAcked) {
- EXPECT_FALSE(manager_.forward_secure_packet_acked());
+ EXPECT_LT(manager_.handshake_state(),
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED);
SendDataPacket(1, ENCRYPTION_INITIAL);
// Ack packet 1.
ExpectAck(1);
@@ -2768,7 +2769,8 @@
EXPECT_EQ(PACKETS_NEWLY_ACKED,
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
ENCRYPTION_INITIAL));
- EXPECT_FALSE(manager_.forward_secure_packet_acked());
+ EXPECT_LT(manager_.handshake_state(),
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED);
SendDataPacket(2, ENCRYPTION_ZERO_RTT);
// Ack packet 2.
@@ -2779,7 +2781,8 @@
EXPECT_EQ(PACKETS_NEWLY_ACKED,
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
ENCRYPTION_FORWARD_SECURE));
- EXPECT_FALSE(manager_.forward_secure_packet_acked());
+ EXPECT_LT(manager_.handshake_state(),
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED);
SendDataPacket(3, ENCRYPTION_FORWARD_SECURE);
// Ack packet 3.
@@ -2790,7 +2793,8 @@
EXPECT_EQ(PACKETS_NEWLY_ACKED,
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3),
ENCRYPTION_FORWARD_SECURE));
- EXPECT_TRUE(manager_.forward_secure_packet_acked());
+ EXPECT_EQ(manager_.handshake_state(),
+ QuicSentPacketManager::HANDSHAKE_CONFIRMED);
}
TEST_F(QuicSentPacketManagerTest, PtoTimeoutIncludesMaxAckDelay) {
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index c4f2d98..03090d8 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -73,6 +73,7 @@
};
// CryptoHandshakeEvent enumerates the events generated by a QuicCryptoStream.
+ // TODO(fayang): Replace this enum and with HandshakeState.
enum CryptoHandshakeEvent {
// ENCRYPTION_ESTABLISHED indicates that a client hello has been sent and
// subsequent packets will be encrypted. (Client only.)
diff --git a/quic/core/tls_handshaker_test.cc b/quic/core/tls_handshaker_test.cc
index d3463a8..e9dbd0d 100644
--- a/quic/core/tls_handshaker_test.cc
+++ b/quic/core/tls_handshaker_test.cc
@@ -331,8 +331,8 @@
EXPECT_TRUE(client_stream_->encryption_established());
EXPECT_TRUE(server_stream_->handshake_confirmed());
EXPECT_TRUE(server_stream_->encryption_established());
- EXPECT_TRUE(client_conn_->IsHandshakeConfirmed());
- EXPECT_TRUE(server_conn_->IsHandshakeConfirmed());
+ EXPECT_TRUE(client_conn_->IsHandshakeComplete());
+ EXPECT_TRUE(server_conn_->IsHandshakeComplete());
const auto& client_crypto_params =
client_stream_->crypto_negotiated_params();
@@ -367,8 +367,8 @@
};
TEST_F(TlsHandshakerTest, CryptoHandshake) {
- EXPECT_FALSE(client_conn_->IsHandshakeConfirmed());
- EXPECT_FALSE(server_conn_->IsHandshakeConfirmed());
+ EXPECT_FALSE(client_conn_->IsHandshakeComplete());
+ EXPECT_FALSE(server_conn_->IsHandshakeComplete());
EXPECT_CALL(*client_conn_, CloseConnection(_, _, _)).Times(0);
EXPECT_CALL(*server_conn_, CloseConnection(_, _, _)).Times(0);
