Update ACK timestamp encoding to use Delta Largest Acknowledged rather than Gap. I am not sure this will actually cause us to send the reordered timestamps, but it at least will now be the same wire encoding as the draft. PiperOrigin-RevId: 916791321
diff --git a/quiche/quic/core/quic_framer.cc b/quiche/quic/core/quic_framer.cc index 2eaa480..e2c9f6e 100644 --- a/quiche/quic/core/quic_framer.cc +++ b/quiche/quic/core/quic_framer.cc
@@ -20,6 +20,7 @@ #include "absl/base/attributes.h" #include "absl/base/macros.h" #include "absl/base/optimization.h" +#include "absl/container/inlined_vector.h" #include "absl/status/status.h" #include "absl/strings/escaping.h" #include "absl/strings/numbers.h" @@ -35,6 +36,7 @@ #include "quiche/quic/core/crypto/quic_decrypter.h" #include "quiche/quic/core/crypto/quic_encrypter.h" #include "quiche/quic/core/crypto/quic_random.h" +#include "quiche/quic/core/frames/quic_ack_frame.h" #include "quiche/quic/core/frames/quic_ack_frequency_frame.h" #include "quiche/quic/core/frames/quic_datagram_frame.h" #include "quiche/quic/core/frames/quic_immediate_ack_frame.h" @@ -44,6 +46,7 @@ #include "quiche/quic/core/quic_data_reader.h" #include "quiche/quic/core/quic_data_writer.h" #include "quiche/quic/core/quic_error_codes.h" +#include "quiche/quic/core/quic_packet_number.h" #include "quiche/quic/core/quic_packets.h" #include "quiche/quic/core/quic_socket_address_coder.h" #include "quiche/quic/core/quic_stream_frame_data_producer.h" @@ -3860,8 +3863,8 @@ return true; } -bool QuicFramer::ProcessIetfTimestampsInAckFrame(QuicPacketNumber largest_acked, - QuicDataReader* reader) { +bool QuicFramer::ProcessIetfTimestampsInAckFrame( + const QuicPacketNumber largest_acked, QuicDataReader* reader) { uint64_t timestamp_range_count; if (!reader->ReadVarInt62(×tamp_range_count)) { set_detailed_error("Unable to read receive timestamp range count."); @@ -3871,28 +3874,27 @@ return true; } - QuicPacketNumber packet_number = largest_acked; - // Iterate through all timestamp ranges, each of which represents a block of // contiguous packets for which receive timestamps are being reported. Each // range is of the form: // // Timestamp Range { - // Gap (i), + // Delta Largest Acknowledged (i), // Timestamp Delta Count (i), // Timestamp Delta (i) ..., // } for (uint64_t i = 0; i < timestamp_range_count; i++) { - uint64_t gap; - if (!reader->ReadVarInt62(&gap)) { - set_detailed_error("Unable to read receive timestamp gap."); + uint64_t delta; + if (!reader->ReadVarInt62(&delta)) { + set_detailed_error( + "Unable to read receive timestamp packet number delta."); return false; } - if (packet_number.ToUint64() < gap) { - set_detailed_error("Receive timestamp gap too high."); + if (largest_acked.ToUint64() < delta) { + set_detailed_error("Receive delta largest acked too high."); return false; } - packet_number = packet_number - gap; + QuicPacketNumber packet_number = largest_acked - delta; uint64_t timestamp_count; if (!reader->ReadVarInt62(×tamp_count)) { set_detailed_error("Unable to read receive timestamp count."); @@ -3925,7 +3927,6 @@ visitor_->OnAckTimestamp(packet_number, creation_time_ + last_timestamp_); packet_number--; } - packet_number--; } return true; } @@ -5579,7 +5580,8 @@ return {}; } timestamp_ranges.push_back(AckTimestampRange()); - timestamp_ranges.back().gap = LargestAcked(frame) - packet_number; + timestamp_ranges.back().delta_from_largest_acked = + LargestAcked(frame) - packet_number; timestamp_ranges.back().range_begin = i; timestamp_ranges.back().range_end = i; continue; @@ -5608,7 +5610,8 @@ timestamp_ranges.back().range_end = i; } else { timestamp_ranges.push_back(AckTimestampRange()); - timestamp_ranges.back().gap = prev_packet_number - 2 - packet_number; + timestamp_ranges.back().delta_from_largest_acked = + LargestAcked(frame) - packet_number; timestamp_ranges.back().range_begin = i; timestamp_ranges.back().range_end = i; } @@ -5638,9 +5641,10 @@ // packet. std::optional<QuicTime> effective_prev_time; for (const AckTimestampRange& range : timestamp_ranges) { - QUIC_DVLOG(3) << "Range: gap:" << range.gap << ", beg:" << range.range_begin + QUIC_DVLOG(3) << "Range: delta:" << range.delta_from_largest_acked + << ", beg:" << range.range_begin << ", end:" << range.range_end; - if (!maybe_write_var_int62(range.gap)) { + if (!maybe_write_var_int62(range.delta_from_largest_acked)) { return -1; }
diff --git a/quiche/quic/core/quic_framer.h b/quiche/quic/core/quic_framer.h index 40719b4..0726d7f 100644 --- a/quiche/quic/core/quic_framer.h +++ b/quiche/quic/core/quic_framer.h
@@ -791,7 +791,7 @@ // AckTimestampRange is a data structure derived from a QuicAckFrame. It is // used to serialize timestamps in a IETF_ACK_RECEIVE_TIMESTAMPS frame. struct QUICHE_EXPORT AckTimestampRange { - QuicPacketCount gap; + QuicPacketCount delta_from_largest_acked; // |range_begin| and |range_end| are index(es) in // QuicAckFrame.received_packet_times, representing a continuous range of // packet numbers in descending order. |range_begin| >= |range_end|.
diff --git a/quiche/quic/core/quic_framer_test.cc b/quiche/quic/core/quic_framer_test.cc index 320421b..96530d5 100644 --- a/quiche/quic/core/quic_framer_test.cc +++ b/quiche/quic/core/quic_framer_test.cc
@@ -3617,7 +3617,7 @@ // Receive Timestamps. { "Unable to read receive timestamp range count.", { kVarInt62OneByte + 0x01 }}, - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62OneByte + 0x01 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x02 }}, @@ -3690,7 +3690,7 @@ { kVarInt62OneByte + 0x03 }}, // Timestamp range 1 (three packets). - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62OneByte + 0x02 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x03 }}, @@ -3702,16 +3702,16 @@ { kVarInt62OneByte + 0x01}}, // Timestamp range 2 (one packet). - { "Unable to read receive timestamp gap.", - { kVarInt62OneByte + 0x05 }}, + { "Unable to read receive timestamp delta largest acked.", + { kVarInt62OneByte + 0x0b }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x01 }}, { "Unable to read receive timestamp delta.", { kVarInt62TwoBytes + 0x10, 0x00 }}, // Timestamp range 3 (two packets). - { "Unable to read receive timestamp gap.", - { kVarInt62OneByte + 0x08 }}, + { "Unable to read receive timestamp delta largest acked.", + { kVarInt62OneByte + 0x15 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x02 }}, { "Unable to read receive timestamp delta.", @@ -3780,7 +3780,7 @@ // Receive Timestamps. { "Unable to read receive timestamp range count.", { kVarInt62OneByte + 0x01 }}, - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62OneByte + 0x00 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x03 }}, @@ -3848,7 +3848,7 @@ // Receive Timestamps. { "Unable to read receive timestamp range count.", { kVarInt62OneByte + 0x01 }}, - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62FourBytes + 0x12, 0x34, 0x56, 0x79 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x01 }}, @@ -3863,7 +3863,7 @@ framer_.set_process_timestamps(true); EXPECT_FALSE(framer_.ProcessPacket(*encrypted)); EXPECT_TRUE(absl::StartsWith(framer_.detailed_error(), - "Receive timestamp gap too high.")); + "Receive delta largest acked too high.")); } TEST_P(QuicFramerTest, AckFrameReceiveTimestampCountTooHigh) { @@ -3902,7 +3902,7 @@ // Receive Timestamps. { "Unable to read receive timestamp range count.", { kVarInt62OneByte + 0x01 }}, - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62OneByte + 0x02 }}, { "Unable to read receive timestamp count.", { kVarInt62OneByte + 0x02 }}, @@ -3958,7 +3958,7 @@ // Receive Timestamps. { "Unable to read receive timestamp range count.", { kVarInt62OneByte + 0x01 }}, - { "Unable to read receive timestamp gap.", + { "Unable to read receive timestamp delta largest acked.", { kVarInt62OneByte + 0x02 }}, { "Unable to read receive timestamp count.", { kVarInt62FourBytes + 0x12, 0x34, 0x56, 0x77 }}, @@ -6473,7 +6473,7 @@ kVarInt62OneByte + 0x03, // Timestamp range 1 (three packets). - // Gap + // Delta Largest Acknowledged kVarInt62OneByte + 0x02, // Timestamp Range Count kVarInt62OneByte + 0x03, @@ -6489,8 +6489,8 @@ kVarInt62OneByte + 0x01, // Timestamp range 2 (one packet). - // Gap - kVarInt62OneByte + 0x05, + // Delta Largest Acknowledged + kVarInt62OneByte + 0x0b, // Timestamp Range Count kVarInt62OneByte + 0x01, // Timestamp Delta @@ -6498,8 +6498,8 @@ 0x00, // Timestamp range 3 (two packets). - // Gap - kVarInt62OneByte + 0x08, + // Delta Largest Acknowledged + kVarInt62OneByte + 0x15, // Timestamp Range Count kVarInt62OneByte + 0x02, // Timestamp Delta @@ -6583,7 +6583,7 @@ kVarInt62OneByte + 0x02, // Timestamp range 1 (three packets). - // Gap + // Delta Largest Acknowledged kVarInt62OneByte + 0x00, // Timestamp Range Count kVarInt62OneByte + 0x03, @@ -6599,8 +6599,8 @@ kVarInt62OneByte + 0x01, // Timestamp range 2 (one packet). - // Gap - kVarInt62OneByte + 0x05, + // Delta Largest Acknowledged + kVarInt62OneByte + 0x09, // Timestamp Range Count kVarInt62OneByte + 0x01, // Timestamp Delta @@ -6682,7 +6682,7 @@ kVarInt62OneByte + 0x02, // Timestamp range 1 (three packets). - // Gap + // Delta Largest Acknowledged kVarInt62OneByte + 0x02, // Timestamp Range Count kVarInt62OneByte + 0x04, @@ -6699,8 +6699,8 @@ kVarInt62OneByte + 0x01, // Timestamp range 2 (one packet). - // Gap - kVarInt62OneByte + 0x04, + // Delta Largest Acknowledged + kVarInt62OneByte + 0x0b, // Timestamp Range Count kVarInt62OneByte + 0x02, // Timestamp Delta