Renumber IETF_ACK_RECEIVE_TIMESTAMPS and implement IETF_ACK_RECEIVE_TIMESTAMPS_ECN. Also fix a bug where we allowed IETF_ACK_RECEIVE_TIMESTAMPS in the handshake (the draft only allows it in 1-RTT). This is a part of update to support draft-ietf-quic-receive-ts-02. PiperOrigin-RevId: 933481951
diff --git a/quiche/quic/core/quic_framer.cc b/quiche/quic/core/quic_framer.cc index 79931e8..df8e07f 100644 --- a/quiche/quic/core/quic_framer.cc +++ b/quiche/quic/core/quic_framer.cc
@@ -397,6 +397,34 @@ } } +bool AckHasEcn(uint64_t type) { + switch (type) { + case IETF_ACK_ECN: + case IETF_ACK_RECEIVE_TIMESTAMPS_ECN: + return true; + default: + return false; + } +} + +bool AckHasTimestamps(uint64_t type) { + switch (type) { + case IETF_ACK_RECEIVE_TIMESTAMPS: + case IETF_ACK_RECEIVE_TIMESTAMPS_ECN: + return true; + default: + return false; + } +} + +uint64_t SelectAckFrameType(bool has_ecn, bool has_timestamps) { + if (has_timestamps) { + return has_ecn ? IETF_ACK_RECEIVE_TIMESTAMPS_ECN + : IETF_ACK_RECEIVE_TIMESTAMPS; + } + return has_ecn ? IETF_ACK_ECN : IETF_ACK; +} + } // namespace QuicFramer::QuicFramer(const ParsedQuicVersionVector& supported_versions, @@ -482,8 +510,10 @@ // Acknowledged, ACK Delay, 0 ACK Block Count, First ACK Block and either 0 // Timestamp Range Count or ECN counts. // Type byte + largest acked. - size_t min_size = - kQuicFrameTypeSize + + size_t min_size = QuicDataWriter::GetVarInt62Len(SelectAckFrameType( + /*has_ecn=*/ack_frame.ecn_counters.has_value(), + /*has_timestamps=*/use_ietf_ack_with_receive_timestamp)); + min_size += QuicDataWriter::GetVarInt62Len(LargestAcked(ack_frame).ToUint64()); // Ack delay. min_size += QuicDataWriter::GetVarInt62Len( @@ -498,9 +528,8 @@ if (use_ietf_ack_with_receive_timestamp) { // 0 Timestamp Range Count. min_size += QuicDataWriter::GetVarInt62Len(0); - } else { - min_size += AckEcnCountSize(ack_frame); } + min_size += AckEcnCountSize(ack_frame); return min_size; } return kQuicFrameTypeSize + @@ -2825,17 +2854,13 @@ case ENCRYPTION_INITIAL: case ENCRYPTION_HANDSHAKE: return frame_type == IETF_CRYPTO || frame_type == IETF_ACK || - frame_type == IETF_ACK_ECN || - frame_type == IETF_ACK_RECEIVE_TIMESTAMPS || - frame_type == IETF_PING || frame_type == IETF_PADDING || - frame_type == IETF_CONNECTION_CLOSE; + frame_type == IETF_ACK_ECN || frame_type == IETF_PING || + frame_type == IETF_PADDING || frame_type == IETF_CONNECTION_CLOSE; case ENCRYPTION_ZERO_RTT: - return !(frame_type == IETF_ACK || frame_type == IETF_ACK_ECN || - frame_type == IETF_ACK_RECEIVE_TIMESTAMPS || - frame_type == IETF_HANDSHAKE_DONE || - frame_type == IETF_NEW_TOKEN || - frame_type == IETF_PATH_RESPONSE || - frame_type == IETF_RETIRE_CONNECTION_ID); + return !( + IsIetfAckFrame(frame_type) || frame_type == IETF_HANDSHAKE_DONE || + frame_type == IETF_NEW_TOKEN || frame_type == IETF_PATH_RESPONSE || + frame_type == IETF_RETIRE_CONNECTION_ID); case ENCRYPTION_FORWARD_SECURE: return true; default: @@ -3105,6 +3130,7 @@ break; } case IETF_ACK_RECEIVE_TIMESTAMPS: + case IETF_ACK_RECEIVE_TIMESTAMPS_ECN: if (!process_timestamps_) { set_detailed_error("Unsupported frame type."); QUIC_DLOG(WARNING) @@ -3826,12 +3852,7 @@ } QUICHE_DCHECK(!ack_frame->ecn_counters.has_value()); - if (frame_type == IETF_ACK_RECEIVE_TIMESTAMPS) { - QUICHE_DCHECK(process_timestamps_); - if (!ProcessIetfTimestampsInAckFrame(ack_frame->largest_acked, reader)) { - return false; - } - } else if (frame_type == IETF_ACK_ECN) { + if (AckHasEcn(frame_type)) { ack_frame->ecn_counters = QuicEcnCounts(); if (!reader->ReadVarInt62(&ack_frame->ecn_counters->ect0)) { set_detailed_error("Unable to read ack ect_0_count."); @@ -3847,6 +3868,13 @@ } } + if (AckHasTimestamps(frame_type)) { + QUICHE_DCHECK(process_timestamps_); + if (!ProcessIetfTimestampsInAckFrame(ack_frame->largest_acked, reader)) { + return false; + } + } + if (!visitor_->OnAckFrameEnd(QuicPacketNumber(block_low), ack_frame->ecn_counters)) { set_detailed_error( @@ -4754,7 +4782,10 @@ size_t QuicFramer::GetIetfAckFrameSize(const QuicAckFrame& frame) { // Type byte, largest_acked, and delay_time are straight-forward. - size_t ack_frame_size = kQuicFrameTypeSize; + const bool send_timestamps = UseIetfAckWithReceiveTimestamp(frame); + const bool send_ecn = frame.ecn_counters.has_value(); + size_t ack_frame_size = QuicDataWriter::GetVarInt62Len( + SelectAckFrameType(send_ecn, send_timestamps)); QuicPacketNumber largest_acked = LargestAcked(frame); ack_frame_size += QuicDataWriter::GetVarInt62Len(largest_acked.ToUint64()); uint64_t ack_delay_time_us; @@ -4789,9 +4820,8 @@ if (UseIetfAckWithReceiveTimestamp(frame)) { ack_frame_size += GetIetfAckFrameTimestampSize(frame); - } else { - ack_frame_size += AckEcnCountSize(frame); } + ack_frame_size += AckEcnCountSize(frame); return ack_frame_size; } @@ -5714,13 +5744,11 @@ bool QuicFramer::AppendIetfAckFrameAndTypeByte(const QuicAckFrame& frame, QuicDataWriter* writer) { - uint8_t type = IETF_ACK; + const bool send_timestamps = UseIetfAckWithReceiveTimestamp(frame); + const bool send_ecn = frame.ecn_counters.has_value(); + const uint64_t type = SelectAckFrameType(send_ecn, send_timestamps); uint64_t ecn_size = 0; - if (UseIetfAckWithReceiveTimestamp(frame)) { - type = IETF_ACK_RECEIVE_TIMESTAMPS; - } else if (frame.ecn_counters.has_value()) { - // Change frame type to ACK_ECN if any ECN count is available. - type = IETF_ACK_ECN; + if (send_ecn) { ecn_size = AckEcnCountSize(frame); } @@ -5770,23 +5798,16 @@ ++iter; // Append remaining ACK blocks. uint64_t appended_ack_blocks = 0; + const QuicByteCount min_tail_size = + ecn_size + + (AckHasTimestamps(type) ? QuicDataWriter::GetVarInt62Len(0) : 0); for (; iter != frame.packets.rend(); ++iter) { const uint64_t gap = previous_smallest - iter->max() - 1; const uint64_t ack_range = iter->Length() - 1; + const size_t block_size = QuicDataWriter::GetVarInt62Len(gap) + + QuicDataWriter::GetVarInt62Len(ack_range); - if (type == IETF_ACK_RECEIVE_TIMESTAMPS && - writer->remaining() < - static_cast<size_t>(QuicDataWriter::GetVarInt62Len(gap) + - QuicDataWriter::GetVarInt62Len(ack_range) + - QuicDataWriter::GetVarInt62Len(0))) { - // If we write this ACK range we won't have space for a timestamp range - // count of 0. - break; - } else if (writer->remaining() < ecn_size || - writer->remaining() - ecn_size < - static_cast<size_t>( - QuicDataWriter::GetVarInt62Len(gap) + - QuicDataWriter::GetVarInt62Len(ack_range))) { + if (writer->remaining() < block_size + min_tail_size) { // ACK range does not fit, truncate it. break; } @@ -5814,7 +5835,7 @@ << ack_block_count << " to " << appended_ack_blocks; } - if (type == IETF_ACK_ECN) { + if (AckHasEcn(type)) { // Encode the ECN counts. if (!writer->WriteVarInt62(frame.ecn_counters->ect0)) { set_detailed_error("No room for ect_0_count in ack frame"); @@ -5830,7 +5851,7 @@ } } - if (type == IETF_ACK_RECEIVE_TIMESTAMPS) { + if (AckHasTimestamps(type)) { if (!AppendIetfTimestampsToAckFrame(frame, writer)) { return false; }
diff --git a/quiche/quic/core/quic_framer_test.cc b/quiche/quic/core/quic_framer_test.cc index 004092a..306995f 100644 --- a/quiche/quic/core/quic_framer_test.cc +++ b/quiche/quic/core/quic_framer_test.cc
@@ -21,11 +21,16 @@ #include "quiche/quic/core/crypto/null_decrypter.h" #include "quiche/quic/core/crypto/quic_decrypter.h" #include "quiche/quic/core/crypto/quic_encrypter.h" +#include "quiche/quic/core/frames/quic_ack_frame.h" +#include "quiche/quic/core/frames/quic_frame.h" #include "quiche/quic/core/frames/quic_reset_stream_at_frame.h" #include "quiche/quic/core/quic_connection_id.h" #include "quiche/quic/core/quic_constants.h" #include "quiche/quic/core/quic_error_codes.h" +#include "quiche/quic/core/quic_interval.h" +#include "quiche/quic/core/quic_packet_number.h" #include "quiche/quic/core/quic_packets.h" +#include "quiche/quic/core/quic_time.h" #include "quiche/quic/core/quic_types.h" #include "quiche/quic/core/quic_utils.h" #include "quiche/quic/core/quic_versions.h" @@ -38,11 +43,13 @@ #include "quiche/quic/test_tools/quic_test_utils.h" #include "quiche/quic/test_tools/simple_data_producer.h" #include "quiche/common/platform/api/quiche_flags.h" +#include "quiche/common/platform/api/quiche_logging.h" #include "quiche/common/quiche_endian.h" #include "quiche/common/test_tools/quiche_test_utils.h" using testing::_; using testing::ContainerEq; +using testing::ElementsAre; using testing::Optional; using testing::Return; @@ -356,10 +363,7 @@ ack_frame.ack_delay_time = ack_delay_time; ack_frames_.push_back(std::make_unique<QuicAckFrame>(ack_frame)); if (VersionIsIetfQuic(transport_version_)) { - EXPECT_TRUE(IETF_ACK == framer_->current_received_frame_type() || - IETF_ACK_ECN == framer_->current_received_frame_type() || - IETF_ACK_RECEIVE_TIMESTAMPS == - framer_->current_received_frame_type()); + EXPECT_TRUE(IsIetfAckFrame(framer_->current_received_frame_type())); } else { EXPECT_EQ(0u, framer_->current_received_frame_type()); } @@ -370,10 +374,7 @@ QUICHE_DCHECK(!ack_frames_.empty()); ack_frames_[ack_frames_.size() - 1]->packets.AddRange(start, end); if (VersionIsIetfQuic(transport_version_)) { - EXPECT_TRUE(IETF_ACK == framer_->current_received_frame_type() || - IETF_ACK_ECN == framer_->current_received_frame_type() || - IETF_ACK_RECEIVE_TIMESTAMPS == - framer_->current_received_frame_type()); + EXPECT_TRUE(IsIetfAckFrame(framer_->current_received_frame_type())); } else { EXPECT_EQ(0u, framer_->current_received_frame_type()); } @@ -385,19 +386,17 @@ ack_frames_[ack_frames_.size() - 1]->received_packet_times.push_back( std::make_pair(packet_number, timestamp)); if (VersionIsIetfQuic(transport_version_)) { - EXPECT_TRUE(IETF_ACK == framer_->current_received_frame_type() || - IETF_ACK_ECN == framer_->current_received_frame_type() || - IETF_ACK_RECEIVE_TIMESTAMPS == - framer_->current_received_frame_type()); + EXPECT_TRUE(IsIetfAckFrame(framer_->current_received_frame_type())); } else { EXPECT_EQ(0u, framer_->current_received_frame_type()); } return true; } - bool OnAckFrameEnd( - QuicPacketNumber /*start*/, - const std::optional<QuicEcnCounts>& /*ecn_counts*/) override { + bool OnAckFrameEnd(QuicPacketNumber /*start*/, + const std::optional<QuicEcnCounts>& ecn_counts) override { + QUICHE_DCHECK(!ack_frames_.empty()); + ack_frames_[ack_frames_.size() - 1]->ecn_counters = ecn_counts; return true; } @@ -939,6 +938,60 @@ QuicTime::Delta::FromMicroseconds(offset_us); } + // Returns the size of the provided frame list when serialized, excluding the + // overhead of the packet header. + std::optional<QuicByteCount> GetFramesSize(const QuicFrames& frames, + QuicByteCount available_size) { + QuicPacketHeader header; + header.destination_connection_id = FramerTestConnectionId(); + header.reset_flag = false; + header.version_flag = false; + header.packet_number = kPacketNumber; + const QuicByteCount header_size = + GetPacketHeaderSize(framer_.transport_version(), header); + std::unique_ptr<QuicPacket> packet = + BuildDataPacket(header, frames, header_size + available_size); + if (packet == nullptr) { + return std::nullopt; + } + return packet->length() - header_size; + } + + // For a given ACK frame, returns the list of possible distinct sizes it can + // have. + std::vector<QuicByteCount> GetAckFrameSizes(QuicAckFrame ack_frame) { + constexpr QuicByteCount kMaxAvailableSize = 1200; + + const QuicByteCount max_serialized_size = + *GetFramesSize({QuicFrame(&ack_frame)}, kMaxAvailableSize); + + // Find minimum ACK size via linear search. This is necessary since + // attempting to serialize anything below that will result in a QUIC_BUG. + QuicByteCount min_size = 1; + while (framer_.GetSerializedFrameLength( + QuicFrame(&ack_frame), min_size, /*first_frame=*/true, + /*last_frame=*/true, PACKET_1BYTE_PACKET_NUMBER) == 0) { + ++min_size; + } + + std::vector<QuicByteCount> sizes; + std::optional<QuicByteCount> last_size; + for (QuicByteCount available_space = min_size; + available_space < kMaxAvailableSize; ++available_space) { + const std::optional<QuicByteCount> actual_size = + GetFramesSize({QuicFrame(&ack_frame)}, available_space); + QUICHE_CHECK(actual_size.has_value()); + if (actual_size != last_size) { + sizes.push_back(*actual_size); + } + if (actual_size >= max_serialized_size) { + break; + } + last_size = actual_size; + } + return sizes; + } + test::TestEncrypter* encrypter_; test::TestDecrypter* decrypter_; ParsedQuicVersion version_; @@ -3568,7 +3621,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -3667,7 +3720,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -3740,6 +3793,85 @@ })); } +TEST_P(QuicFramerTest, AckFrameReceiveTimestampsAndEcn) { + if (!VersionIsIetfQuic(framer_.transport_version())) { + return; + } + SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE); + // clang-format off + PacketFragments packet_ietf = { + // type (short header, 4 byte packet number) + {"", + { 0x43 }}, + // connection_id + {"", + { 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10 }}, + // packet number + {"", + { 0x12, 0x34, 0x56, 0x78 }}, + + // frame type (IETF_ACK_RECEIVE_TIMESTAMPS_ECN frame) + {"", + { 0x83, 0x17, 0x83, 0x08 }}, + // largest acked + {"Unable to read largest acked.", + { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 + // Zero delta time. + {"Unable to read ack delay time.", + { kVarInt62OneByte + 0x00 }}, + // number of additional ack blocks + {"Unable to read ack block count.", + { kVarInt62OneByte + 0x00 }}, + // first ack block length. + {"Unable to read first ack block length.", + { kVarInt62OneByte + 0x00 }}, // 1st block length = 1 + + // ECN counts + { "Unable to read ack ect_0_count.", + { kVarInt62OneByte + 10 }}, + { "Unable to read ack ect_1_count.", + { kVarInt62OneByte + 20 }}, + { "Unable to read ack ecn_ce_count.", + { kVarInt62OneByte + 30 }}, + + // Receive Timestamps. + { "Unable to read receive timestamp range count.", + { kVarInt62OneByte + 0x01 }}, + { "Unable to read receive timestamp delta largest acked.", + { kVarInt62OneByte + 0x02 }}, + { "Unable to read receive timestamp count.", + { kVarInt62OneByte + 0x03 }}, + { "Unable to read receive timestamp delta.", + { kVarInt62FourBytes + 0x29, 0xff, 0xff, 0xff}}, + { "Unable to read receive timestamp delta.", + { kVarInt62TwoBytes + 0x11, 0x11 }}, + { "Unable to read receive timestamp delta.", + { kVarInt62OneByte + 0x01}}, + }; + // clang-format on + + std::unique_ptr<QuicEncryptedPacket> encrypted( + AssemblePacketFromFragments(packet_ietf)); + + framer_.set_process_timestamps(true); + EXPECT_TRUE(framer_.ProcessPacket(*encrypted)); + EXPECT_THAT(framer_.error(), IsQuicNoError()); + ASSERT_TRUE(visitor_.header_.get()); + const QuicAckFrame& frame = *visitor_.ack_frames_[0]; + + ASSERT_TRUE(frame.ecn_counters.has_value()); + EXPECT_EQ(10u, frame.ecn_counters->ect0); + EXPECT_EQ(20u, frame.ecn_counters->ect1); + EXPECT_EQ(30u, frame.ecn_counters->ce); + + EXPECT_THAT(frame.received_packet_times, + ContainerEq(PacketTimeVector{ + {LargestAcked(frame) - 2, CreationTimePlus(0x29ffffff)}, + {LargestAcked(frame) - 3, CreationTimePlus(0x29ffeeee)}, + {LargestAcked(frame) - 4, CreationTimePlus(0x29ffeeed)}, + })); +} + TEST_P(QuicFramerTest, AckFrameReceiveTimestampWithExponent) { if (!VersionIsIetfQuic(framer_.transport_version())) { return; @@ -3759,7 +3891,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -3827,7 +3959,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -3881,7 +4013,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -3937,7 +4069,7 @@ // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) {"", - { 0x22 }}, + { 0x83, 0x17, 0x83, 0x07 }}, // largest acked {"Unable to read largest acked.", { kVarInt62TwoBytes + 0x12, 0x34 }}, // = 4660 @@ -6447,7 +6579,10 @@ 0x78, // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) - 0x22, + 0x83, + 0x17, + 0x83, + 0x07, // largest acked kVarInt62TwoBytes + 0x12, 0x34, // = 4660 @@ -6511,6 +6646,100 @@ ABSL_ARRAYSIZE(packet_ietf)); } +TEST_P(QuicFramerTest, BuildAckReceiveTimestampsAndEcnFrame) { + if (!VersionIsIetfQuic(framer_.transport_version())) { + return; + } + + 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; + + QuicAckFrame ack_frame = InitAckFrame(kSmallLargestObserved); + ack_frame.ecn_counters = QuicEcnCounts(10, 20, 30); + ack_frame.received_packet_times = PacketTimeVector{ + // Timestamp Range 1. + {kSmallLargestObserved - 4, CreationTimePlus(0x29ffeeed)}, + {kSmallLargestObserved - 3, CreationTimePlus(0x29ffeeee)}, + {kSmallLargestObserved - 2, CreationTimePlus(0x29ffffff)}, + }; + ack_frame.ack_delay_time = QuicTime::Delta::Zero(); + QuicFrames frames = {QuicFrame(&ack_frame)}; + + unsigned char packet_ietf[] = { + // type (short header, 4 byte packet number) + 0x43, + // connection_id + 0xFE, + 0xDC, + 0xBA, + 0x98, + 0x76, + 0x54, + 0x32, + 0x10, + // packet number + 0x12, + 0x34, + 0x56, + 0x78, + + // frame type (IETF_ACK_RECEIVE_TIMESTAMPS_ECN frame) + 0x83, + 0x17, + 0x83, + 0x08, + // largest acked + kVarInt62TwoBytes + 0x12, + 0x34, // = 4660 + // Zero delta time. + kVarInt62OneByte + 0x00, + // number of additional ack blocks + kVarInt62OneByte + 0x00, + // first ack block length. + kVarInt62TwoBytes + 0x12, + 0x33, + + // ECN counts + kVarInt62OneByte + 10, + kVarInt62OneByte + 20, + kVarInt62OneByte + 30, + + // Receive Timestamps. + + // Timestamp Range Count + kVarInt62OneByte + 0x01, + + // Timestamp range 1 (three packets). + // Delta Largest Acknowledged + kVarInt62OneByte + 0x02, + // Timestamp Range Count + kVarInt62OneByte + 0x03, + // Timestamp Delta + kVarInt62FourBytes + 0x29, + 0xff, + 0xff, + 0xff, + // Timestamp Delta + kVarInt62TwoBytes + 0x11, + 0x11, + // Timestamp Delta + kVarInt62OneByte + 0x01, + }; + // clang-format on + + framer_.set_process_timestamps(true); + framer_.set_max_receive_timestamps_per_ack(8); + std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames)); + ASSERT_TRUE(data != nullptr); + quiche::test::CompareCharArraysWithHexError( + "constructed packet", data->data(), data->length(), AsChars(packet_ietf), + ABSL_ARRAYSIZE(packet_ietf)); +} + TEST_P(QuicFramerTest, BuildAckReceiveTimestampsFrameExceedsMaxTimestamps) { if (!VersionIsIetfQuic(framer_.transport_version())) { return; @@ -6557,7 +6786,10 @@ 0x78, // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) - 0x22, + 0x83, + 0x17, + 0x83, + 0x07, // largest acked kVarInt62TwoBytes + 0x12, 0x34, // = 4660 @@ -6656,7 +6888,10 @@ 0x78, // frame type (IETF_ACK_RECEIVE_TIMESTAMPS frame) - 0x22, + 0x83, + 0x17, + 0x83, + 0x07, // largest acked kVarInt62TwoBytes + 0x12, 0x34, // = 4660 @@ -7097,8 +7332,8 @@ ack_frame.packets.NumIntervals()); EXPECT_EQ(QuicPacketNumber(kMaxIetfVarInt), processed_ack_frame.packets.Max()); - // But the receive timestamps are not truncated because they are small. - EXPECT_FALSE(processed_ack_frame.received_packet_times.empty()); + // The receive timestamps are also truncated. + EXPECT_TRUE(processed_ack_frame.received_packet_times.empty()); } TEST_P(QuicFramerTest, BuildAckFramePacketOneAckBlockMaxLength) { @@ -15304,6 +15539,92 @@ EXPECT_TRUE(data->data()[0] & FLAGS_SPIN_BIT); } +TEST_P(QuicFramerTest, CheckAckPacketSizes) { + if (!framer_.version().IsIetfQuic()) { + return; + } + + // The baseline for the length of a serialized ACK frame is 5 bytes: + // - 1 byte for type + // - 1 byte for Largest Acknowledged + // - 1 byte for ACK Delay + // - 1 byte for ACK Range Count + // - 1 byte for First ACK Range + QuicAckFrame ack_frame = InitAckFrame(12); + ack_frame.ack_delay_time = QuicTimeDelta::Zero(); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(5)); + + // ECN counts add extra three bytes. + ack_frame.ecn_counters = QuicEcnCounts(1, 2, 3); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(8)); + + // Timestamps are not enabled yet, so the results here should be the same as + // above. + ack_frame.ecn_counters = std::nullopt; + ack_frame.received_packet_times.push_back( + {QuicPacketNumber(11), CreationTimePlus(11)}); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(5)); + ack_frame.ecn_counters = QuicEcnCounts(1, 2, 3); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(8)); + + // Timestamps add extra bytes: + // - The frame type is 4 bytes, meaning 3 extra bytes. + // - The timestamp count always has to be included, meaning 1 extra byte. + // - The timestamps themselves are truncatable, but the range specified above + // is 3 bytes. + framer_.set_process_timestamps(true); + ack_frame.ecn_counters = std::nullopt; + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(9, 12)); + ack_frame.ecn_counters = QuicEcnCounts(1, 2, 3); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(12, 15)); + + // The timestamp truncation is all-or-nothing, meaning that every new + // timestamp will result in the upper bound on the ACK size going up. + ack_frame.received_packet_times.push_back( + {QuicPacketNumber(12), CreationTimePlus(12)}); + ack_frame.ecn_counters = std::nullopt; + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(9, 13)); + ack_frame.received_packet_times.insert( + ack_frame.received_packet_times.begin(), + {QuicPacketNumber(6), CreationTimePlus(6)}); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(9, 16)); +} + +TEST_P(QuicFramerTest, CheckAckPacketSizesWithTwoRanges) { + if (!framer_.version().IsIetfQuic()) { + return; + } + + // The baseline for the length of a serialized ACK frame is 5 bytes: + // - 1 byte for type + // - 1 byte for Largest Acknowledged + // - 1 byte for ACK Delay + // - 1 byte for ACK Range Count + // - 1 byte for First ACK Range + // The additional ACK range is 2 bytes and is truncatable. + QuicAckFrame ack_frame = InitAckFrame({ + {QuicPacketNumber(1), QuicPacketNumber(2)}, + {QuicPacketNumber(3), QuicPacketNumber(4)}, + }); + ack_frame.ack_delay_time = QuicTimeDelta::Zero(); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(5, 7)); + + // ECN counts add extra three bytes. + ack_frame.ecn_counters = QuicEcnCounts(1, 2, 3); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(8, 10)); + + // Timestamps add extra bytes: + // - The frame type is 4 bytes, meaning 3 extra bytes. + // - The timestamp count always has to be included, meaning 1 extra byte. + // - The timestamps themselves are truncatable 3 extra bytes. + // This means that the minimum size is now 12 bytes; the extra 3 bytes of + // timestamps will be truncated before the extra 2 bytes of ACK ranges. + framer_.set_process_timestamps(true); + ack_frame.received_packet_times.push_back( + {QuicPacketNumber(3), CreationTimePlus(11)}); + EXPECT_THAT(GetAckFrameSizes(ack_frame), ElementsAre(12, 14, 17)); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_types.h b/quiche/quic/core/quic_types.h index 7f5c2ef..7feba26 100644 --- a/quiche/quic/core/quic_types.h +++ b/quiche/quic/core/quic_types.h
@@ -355,9 +355,10 @@ IETF_ACK_FREQUENCY = 0xaf, // A QUIC extension frame which augments the IETF_ACK frame definition with - // packet receive timestamps. - // TODO(ianswett): Determine a proper value to replace this temporary value. - IETF_ACK_RECEIVE_TIMESTAMPS = 0x22, + // packet receive timestamps. The draft codepoints are from + // <https://github.com/quicwg/receive-ts/commit/108fc85425fd6a698fa6d5784566825cc8c47aa6>. + IETF_ACK_RECEIVE_TIMESTAMPS = 0x03178307, + IETF_ACK_RECEIVE_TIMESTAMPS_ECN = 0x03178308, // https://datatracker.ietf.org/doc/html/draft-ietf-quic-reliable-stream-reset IETF_RESET_STREAM_AT = 0x24, @@ -366,6 +367,19 @@ const QuicIetfFrameType& c); QUICHE_EXPORT std::string QuicIetfFrameTypeString(QuicIetfFrameType t); +// Returns true if the specified IETF frame type is an ACK frame. +QUICHE_EXPORT constexpr bool IsIetfAckFrame(uint64_t type) { + switch (type) { + case IETF_ACK: + case IETF_ACK_ECN: + case IETF_ACK_RECEIVE_TIMESTAMPS: + case IETF_ACK_RECEIVE_TIMESTAMPS_ECN: + return true; + default: + return false; + } +} + // Masks for the bits that indicate the frame is a Stream frame vs the // bits used as flags. #define IETF_STREAM_FRAME_TYPE_MASK 0xfffffffffffffff8