Refactor QuicConnection::UpdatePacketContent() to take a different enum to reflect frame type. No behavior change, not protected. Currently it takes PacketContent which is also used as current_packet_content_ connection state. This state is only needed in gQUIC. However UpdatePacketContent() can be used to update iQUIC connection state to indicate whether current packet is a probing packet or a non-probing packet depending on frames in it. Adding calls to UpdatePacketContent() in all IETF frame callbacks as well. These calls will be no-op in UpdatePacketContent(). PiperOrigin-RevId: 324700454 Change-Id: I96d35d990ee1727e2f781e9ae11e94113a332ec2
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 9e2f885..f3d5431 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -1126,7 +1126,7 @@ // Since a stream frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(STREAM_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnStreamFrame(frame); @@ -1162,7 +1162,7 @@ // Since a CRYPTO frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(CRYPTO_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnCryptoFrame(frame); @@ -1185,7 +1185,7 @@ // Since an ack frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(ACK_FRAME); QUIC_DVLOG(1) << ENDPOINT << "OnAckFrameStart, largest_acked: " << largest_acked; @@ -1305,7 +1305,7 @@ // Since a stop waiting frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(STOP_WAITING_FRAME); if (no_stop_waiting_frames_) { return true; @@ -1336,7 +1336,7 @@ bool QuicConnection::OnPaddingFrame(const QuicPaddingFrame& frame) { DCHECK(connected_); - UpdatePacketContent(SECOND_FRAME_IS_PADDING); + UpdatePacketContent(PADDING_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnPaddingFrame(frame); @@ -1346,7 +1346,7 @@ bool QuicConnection::OnPingFrame(const QuicPingFrame& frame) { DCHECK(connected_); - UpdatePacketContent(FIRST_FRAME_IS_PING); + UpdatePacketContent(PING_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnPingFrame(frame); @@ -1384,7 +1384,7 @@ // Since a reset stream frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(RST_STREAM_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnRstStreamFrame(frame); @@ -1403,7 +1403,7 @@ // Since a reset stream frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(STOP_SENDING_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnStopSendingFrame(frame); @@ -1418,6 +1418,7 @@ } bool QuicConnection::OnPathChallengeFrame(const QuicPathChallengeFrame& frame) { + UpdatePacketContent(PATH_CHALLENGE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnPathChallengeFrame(frame); } @@ -1430,6 +1431,7 @@ } bool QuicConnection::OnPathResponseFrame(const QuicPathResponseFrame& frame) { + UpdatePacketContent(PATH_RESPONSE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnPathResponseFrame(frame); } @@ -1450,7 +1452,7 @@ // Since a connection close frame was received, this is not a connectivity // probe. A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(CONNECTION_CLOSE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnConnectionCloseFrame(frame); @@ -1492,6 +1494,7 @@ } bool QuicConnection::OnMaxStreamsFrame(const QuicMaxStreamsFrame& frame) { + UpdatePacketContent(MAX_STREAMS_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnMaxStreamsFrame(frame); } @@ -1500,6 +1503,7 @@ bool QuicConnection::OnStreamsBlockedFrame( const QuicStreamsBlockedFrame& frame) { + UpdatePacketContent(STREAMS_BLOCKED_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnStreamsBlockedFrame(frame); } @@ -1511,7 +1515,7 @@ // Since a go away frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(GOAWAY_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnGoAwayFrame(frame); @@ -1530,7 +1534,7 @@ // Since a window update frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(WINDOW_UPDATE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnWindowUpdateFrame( @@ -1544,6 +1548,7 @@ bool QuicConnection::OnNewConnectionIdFrame( const QuicNewConnectionIdFrame& frame) { + UpdatePacketContent(NEW_CONNECTION_ID_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnNewConnectionIdFrame(frame); } @@ -1552,6 +1557,7 @@ bool QuicConnection::OnRetireConnectionIdFrame( const QuicRetireConnectionIdFrame& frame) { + UpdatePacketContent(RETIRE_CONNECTION_ID_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnRetireConnectionIdFrame(frame); } @@ -1559,6 +1565,7 @@ } bool QuicConnection::OnNewTokenFrame(const QuicNewTokenFrame& frame) { + UpdatePacketContent(NEW_TOKEN_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnNewTokenFrame(frame); } @@ -1570,7 +1577,7 @@ // Since a message frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(MESSAGE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnMessageFrame(frame); @@ -1599,7 +1606,7 @@ // Since a handshake done frame was received, this is not a connectivity // probe. A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(HANDSHAKE_DONE_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnHandshakeDoneFrame(frame); @@ -1611,6 +1618,7 @@ bool QuicConnection::OnAckFrequencyFrame( const QuicAckFrequencyFrame& /*frame*/) { + UpdatePacketContent(ACK_FREQUENCY_FRAME); // TODO(b/148614353): implement this fully. QUIC_LOG_EVERY_N_SEC(ERROR, 120) << "Get unexpected AckFrequencyFrame."; return false; @@ -1620,7 +1628,7 @@ // Since a blocked frame was received, this is not a connectivity probe. // A probe only contains a PING and full padding. - UpdatePacketContent(NOT_PADDED_PING); + UpdatePacketContent(BLOCKED_FRAME); if (debug_visitor_ != nullptr) { debug_visitor_->OnBlockedFrame(frame); @@ -4047,12 +4055,13 @@ sent_packet_manager_.OnApplicationLimited(); } -void QuicConnection::UpdatePacketContent(PacketContent type) { +void QuicConnection::UpdatePacketContent(QuicFrameType type) { // Packet content is tracked to identify connectivity probe in non-IETF // version, where a connectivity probe is defined as // - a padded PING packet with peer address change received by server, // - a padded PING packet on new path received by client. if (version().HasIetfQuicFrames()) { + // TODO(b/149315151) detect IETF non-probing packet. return; } @@ -4063,11 +4072,7 @@ return; } - if (type == NO_FRAMES_RECEIVED) { - return; - } - - if (type == FIRST_FRAME_IS_PING) { + if (type == PING_FRAME) { if (current_packet_content_ == NO_FRAMES_RECEIVED) { current_packet_content_ = FIRST_FRAME_IS_PING; return; @@ -4077,8 +4082,7 @@ // In Google QUIC, we look for a packet with just a PING and PADDING. // If the condition is met, mark things as connectivity-probing, causing // later processing to generate the correct response. - if (type == SECOND_FRAME_IS_PADDING && - current_packet_content_ == FIRST_FRAME_IS_PING) { + if (type == PADDING_FRAME && current_packet_content_ == FIRST_FRAME_IS_PING) { current_packet_content_ = SECOND_FRAME_IS_PADDING; if (perspective_ == Perspective::IS_SERVER) { is_current_packet_connectivity_probing_ =
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index afddeeb..c1bf5bb 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1218,7 +1218,7 @@ // starts effective peer migration if current packet is confirmed not a // connectivity probe and |current_effective_peer_migration_type_| indicates // effective peer address change. - void UpdatePacketContent(PacketContent type); + void UpdatePacketContent(QuicFrameType type); // Called when last received ack frame has been processed. // |send_stop_waiting| indicates whether a stop waiting needs to be sent.