Split ACK timestamp parameters into local and remote ones. Those can be different since each peer specifies its own parameters. PiperOrigin-RevId: 936110023
diff --git a/quiche/quic/core/quic_framer.cc b/quiche/quic/core/quic_framer.cc index a002fb8..60d4eed 100644 --- a/quiche/quic/core/quic_framer.cc +++ b/quiche/quic/core/quic_framer.cc
@@ -440,8 +440,12 @@ perspective_(perspective), validate_flags_(true), process_timestamps_(false), - max_receive_timestamps_per_ack_(std::numeric_limits<uint32_t>::max()), - receive_timestamps_exponent_(0), + peer_max_receive_timestamps_per_ack_( + std::numeric_limits<uint32_t>::max()), + local_max_receive_timestamps_per_ack_( + std::numeric_limits<uint32_t>::max()), + peer_receive_timestamps_exponent_(0), + local_receive_timestamps_exponent_(0), process_reset_stream_at_(false), creation_time_(creation_time), last_timestamp_(QuicTime::Delta::Zero()), @@ -3929,7 +3933,7 @@ return false; } total_timestamp_count += timestamp_count; - if (total_timestamp_count > max_receive_timestamps_per_ack_) { + if (total_timestamp_count > local_max_receive_timestamps_per_ack_) { set_detailed_error("Too many receive timestamps in ACK frame."); return false; } @@ -3942,7 +3946,7 @@ // The first timestamp delta is relative to framer creation time; whereas // subsequent deltas are relative to the previous delta in decreasing // packet order. - timestamp_delta = timestamp_delta << receive_timestamps_exponent_; + timestamp_delta = timestamp_delta << local_receive_timestamps_exponent_; if (i == 0 && j == 0) { last_timestamp_ = QuicTime::Delta::FromMicroseconds(timestamp_delta); } else { @@ -5590,7 +5594,7 @@ absl::InlinedVector<AckTimestampRange, 2> timestamp_ranges; - for (size_t r = 0; r < std::min<size_t>(max_receive_timestamps_per_ack_, + for (size_t r = 0; r < std::min<size_t>(peer_max_receive_timestamps_per_ack_, frame.received_packet_times.size()); ++r) { const size_t i = frame.received_packet_times.size() - 1 - r; @@ -5689,28 +5693,31 @@ time_delta = (*effective_prev_time - receive_timestamp).ToMicroseconds(); QUIC_DVLOG(3) << "time_delta:" << time_delta - << ", exponent:" << receive_timestamps_exponent_ + << ", exponent:" << peer_receive_timestamps_exponent_ << ", effective_prev_time:" << *effective_prev_time << ", recv_time:" << receive_timestamp; - time_delta = time_delta >> receive_timestamps_exponent_; - effective_prev_time = *effective_prev_time - - QuicTime::Delta::FromMicroseconds( - time_delta << receive_timestamps_exponent_); + time_delta = time_delta >> peer_receive_timestamps_exponent_; + effective_prev_time = + *effective_prev_time - + QuicTime::Delta::FromMicroseconds( + time_delta << peer_receive_timestamps_exponent_); } else { // The first delta is from framer creation to the current receive // timestamp (forward in time), whereas in the common case subsequent // deltas move backwards in time. time_delta = (receive_timestamp - creation_time_).ToMicroseconds(); QUIC_DVLOG(3) << "First time_delta:" << time_delta - << ", exponent:" << receive_timestamps_exponent_ + << ", exponent:" << peer_receive_timestamps_exponent_ << ", recv_time:" << receive_timestamp << ", creation_time:" << creation_time_; // Round up the first exponent-encoded time delta so that the next // receive timestamp is guaranteed to be decreasing. - time_delta = ((time_delta - 1) >> receive_timestamps_exponent_) + 1; + time_delta = + ((time_delta - 1) >> peer_receive_timestamps_exponent_) + 1; effective_prev_time = - creation_time_ + QuicTime::Delta::FromMicroseconds( - time_delta << receive_timestamps_exponent_); + creation_time_ + + QuicTime::Delta::FromMicroseconds( + time_delta << peer_receive_timestamps_exponent_); } if (!maybe_write_var_int62(time_delta)) {
diff --git a/quiche/quic/core/quic_framer.h b/quiche/quic/core/quic_framer.h index d614bae..0020db4 100644 --- a/quiche/quic/core/quic_framer.h +++ b/quiche/quic/core/quic_framer.h
@@ -322,23 +322,30 @@ QuicErrorCode error() const { return error_; } + // TODO(b/132632465): Remove the const in timestamp-related setters once + // timestamps are negotiated via transport params. + // Allows enabling or disabling of timestamp processing and serialization. - // TODO(ianswett): Remove the const once timestamps are negotiated via - // transport params. void set_process_timestamps(bool process_timestamps) const { process_timestamps_ = process_timestamps; } + // Sets the max number of receive timestamps to parse per ACK frame. + void set_local_max_receive_timestamps_per_ack(uint32_t max_timestamps) const { + local_max_receive_timestamps_per_ack_ = max_timestamps; + } // Sets the max number of receive timestamps to send per ACK frame. - // TODO(wub): Remove the const once timestamps are negotiated via - // transport params. - void set_max_receive_timestamps_per_ack(uint32_t max_timestamps) const { - max_receive_timestamps_per_ack_ = max_timestamps; + void set_peer_max_receive_timestamps_per_ack(uint32_t max_timestamps) const { + peer_max_receive_timestamps_per_ack_ = max_timestamps; } - // Sets the exponent to use when writing/reading ACK receive timestamps. - void set_receive_timestamps_exponent(uint32_t exponent) const { - receive_timestamps_exponent_ = exponent; + // Sets the exponent to use when reading ACK receive timestamps. + void set_local_receive_timestamps_exponent(uint32_t exponent) const { + local_receive_timestamps_exponent_ = exponent; + } + // Sets the exponent to use when writing ACK receive timestamps. + void set_peer_receive_timestamps_exponent(uint32_t exponent) const { + peer_receive_timestamps_exponent_ = exponent; } bool process_reset_stream_at() const { return process_reset_stream_at_; } @@ -1127,7 +1134,7 @@ bool UseIetfAckWithReceiveTimestamp(const QuicAckFrame& frame) const { return VersionIsIetfQuic(version_.transport_version) && process_timestamps_ && - std::min<uint64_t>(max_receive_timestamps_per_ack_, + std::min<uint64_t>(peer_max_receive_timestamps_per_ack_, frame.received_packet_times.size()) > 0; } @@ -1172,9 +1179,13 @@ // set_receive_timestamp_exponent_ aren't const. mutable bool process_timestamps_; // The max number of receive timestamps to send per ACK frame. - mutable uint32_t max_receive_timestamps_per_ack_; - // The exponent to use when writing/reading ACK receive timestamps. - mutable uint32_t receive_timestamps_exponent_; + mutable uint32_t peer_max_receive_timestamps_per_ack_; + // The max number of receive timestamps to allow in an incoming ACK frame. + mutable uint32_t local_max_receive_timestamps_per_ack_; + // The exponent to use when writing ACK receive timestamps. + mutable uint32_t peer_receive_timestamps_exponent_; + // The exponent to use when reading ACK receive timestamps. + mutable uint32_t local_receive_timestamps_exponent_; // If true, process RESET_STREAM_AT frames. bool process_reset_stream_at_; // The creation time of the connection, used to calculate timestamps.
diff --git a/quiche/quic/core/quic_framer_test.cc b/quiche/quic/core/quic_framer_test.cc index 037e86e..4b86ff2 100644 --- a/quiche/quic/core/quic_framer_test.cc +++ b/quiche/quic/core/quic_framer_test.cc
@@ -3924,7 +3924,7 @@ std::unique_ptr<QuicEncryptedPacket> encrypted( AssemblePacketFromFragments(packet_ietf)); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_receive_timestamps_exponent(3); framer_.set_process_timestamps(true); EXPECT_TRUE(framer_.ProcessPacket(*encrypted)); EXPECT_THAT(framer_.error(), IsQuicNoError()); @@ -6638,7 +6638,7 @@ // clang-format on framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); ASSERT_TRUE(data != nullptr); quiche::test::CompareCharArraysWithHexError( @@ -6740,7 +6740,7 @@ // clang-format on framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); ASSERT_TRUE(data != nullptr); quiche::test::CompareCharArraysWithHexError( @@ -6834,7 +6834,7 @@ // clang-format on framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); ASSERT_TRUE(data != nullptr); quiche::test::CompareCharArraysWithHexError( @@ -6936,7 +6936,7 @@ // clang-format on framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(4); + framer_.set_peer_max_receive_timestamps_per_ack(4); std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); ASSERT_TRUE(data != nullptr); quiche::test::CompareCharArraysWithHexError( @@ -7044,8 +7044,8 @@ // clang-format on framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_peer_receive_timestamps_exponent(3); std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); ASSERT_TRUE(data != nullptr); quiche::test::CompareCharArraysWithHexError( @@ -7061,7 +7061,8 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7114,7 +7115,8 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(2); + framer_.set_local_max_receive_timestamps_per_ack(2); + framer_.set_peer_max_receive_timestamps_per_ack(2); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7162,8 +7164,10 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7210,8 +7214,10 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7263,8 +7269,10 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7303,6 +7311,60 @@ })); } +TEST_P(QuicFramerTest, AckReceiveTimestampsDifferentExponents) { + if (!VersionIsIetfQuic(framer_.transport_version())) { + return; + } + framer_.InstallDecrypter(ENCRYPTION_FORWARD_SECURE, + std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); + framer_.SetKeyUpdateSupportForConnection(true); + framer_.set_process_timestamps(true); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + + // Set the local exponent to 2; it will be used to parse incoming ACKs. + framer_.set_local_receive_timestamps_exponent(2); + // Set the peer exponent to 5; it will be used to serialize outgoing ACKs. + framer_.set_peer_receive_timestamps_exponent(5); + + QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); + QuicPacketHeader header; + header.destination_connection_id = FramerTestConnectionId(); + header.reset_flag = false; + header.version_flag = false; + header.packet_number = kPacketNumber; + + // Outgoing ACKs are encoded relative to the exponent of 5. + QuicAckFrame ack_frame = InitAckFrame(kSmallLargestObserved); + ack_frame.received_packet_times = PacketTimeVector{ + {kSmallLargestObserved - 5, CreationTimePlus((0x29ff << 5))}, + {kSmallLargestObserved - 4, CreationTimePlus((0x29ff << 5))}, + {kSmallLargestObserved - 3, CreationTimePlus((0x29ff << 5))}, + {kSmallLargestObserved - 2, CreationTimePlus((0x29ff << 5))}, + }; + ack_frame.ack_delay_time = QuicTime::Delta::Zero(); + QuicFrames frames = {QuicFrame(&ack_frame)}; + + std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); + ASSERT_TRUE(data != nullptr); + std::unique_ptr<QuicEncryptedPacket> encrypted( + EncryptPacketWithTagAndPhase(*data, 0, false)); + ASSERT_TRUE(encrypted); + QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_SERVER); + EXPECT_TRUE(framer_.ProcessPacket(*encrypted)); + EXPECT_THAT(framer_.error(), IsQuicNoError()); + + // When the same packet is processed as incoming, the exponent becomes 2. + const QuicAckFrame& frame = *visitor_.ack_frames_[0]; + EXPECT_THAT(frame.received_packet_times, + ContainerEq(PacketTimeVector{ + {kSmallLargestObserved - 2, CreationTimePlus(0x29ff << 2)}, + {kSmallLargestObserved - 3, CreationTimePlus(0x29ff << 2)}, + {kSmallLargestObserved - 4, CreationTimePlus(0x29ff << 2)}, + {kSmallLargestObserved - 5, CreationTimePlus(0x29ff << 2)}, + })); +} + TEST_P(QuicFramerTest, BuildAndProcessAckReceiveTimestampsPacketOutOfOrder) { if (!VersionIsIetfQuic(framer_.transport_version())) { return; @@ -7311,7 +7373,8 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7357,8 +7420,8 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7388,8 +7451,10 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7415,7 +7480,7 @@ ASSERT_TRUE(encrypted); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_SERVER); - framer_.set_max_receive_timestamps_per_ack(2); + framer_.set_local_max_receive_timestamps_per_ack(2); EXPECT_FALSE(framer_.ProcessPacket(*encrypted)); EXPECT_THAT(framer_.error(), IsError(QUIC_INVALID_ACK_DATA)); EXPECT_EQ("Too many receive timestamps in ACK frame.", @@ -7432,8 +7497,10 @@ std::make_unique<StrictTaggingDecrypter>(/*key=*/0)); framer_.SetKeyUpdateSupportForConnection(true); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8192); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8192); + framer_.set_peer_max_receive_timestamps_per_ack(8192); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); QuicPacketHeader header; @@ -7473,8 +7540,10 @@ } SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE); framer_.set_process_timestamps(true); - framer_.set_max_receive_timestamps_per_ack(8); - framer_.set_receive_timestamps_exponent(3); + framer_.set_local_max_receive_timestamps_per_ack(8); + framer_.set_peer_max_receive_timestamps_per_ack(8); + framer_.set_local_receive_timestamps_exponent(3); + framer_.set_peer_receive_timestamps_exponent(3); QuicPacketHeader header; header.destination_connection_id = FramerTestConnectionId();