From b257b1fe249434b9ecc6299fc213823b92323aaa Mon Sep 17 00:00:00 2001 From: Matt Joras Date: Wed, 17 May 2023 11:59:53 -0700 Subject: [PATCH] Unify and move receive timestamp config Summary: We shouldn't have config in the codec types. Instead solve the plumbing problem more explicitly, and only define the config in one place. Reviewed By: jbeshay Differential Revision: D45881730 fbshipit-source-id: fab6c967a38172f16e57a8978b10460fd196902e --- quic/api/QuicPacketScheduler.cpp | 13 +-- quic/client/QuicClientTransport.cpp | 4 +- quic/codec/Decode.cpp | 2 +- quic/codec/QuicPacketRebuilder.cpp | 13 +-- quic/codec/QuicWriteCodec.cpp | 21 ++-- quic/codec/QuicWriteCodec.h | 3 +- quic/codec/Types.h | 6 - quic/codec/test/QuicWriteCodecTest.cpp | 143 ++++++++++++++--------- quic/server/state/ServerStateMachine.cpp | 8 +- quic/state/StateData.h | 4 - quic/state/TransportSettings.h | 4 +- 11 files changed, 121 insertions(+), 100 deletions(-) diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index e9fa485c3..d152a5d5b 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -574,8 +574,7 @@ folly::Optional AckScheduler::writeNextAcks( ackDelay, /* ackDelay */ static_cast(ackDelayExponentToUse), /* ackDelayExponent */ conn_.connectionTime, /* connect timestamp */ - folly::none, /* recvTimestampsConfig */ - folly::none /* maxAckReceiveTimestampsToSend */}; + }; folly::Optional ackWriteResult; @@ -594,12 +593,12 @@ folly::Optional AckScheduler::writeNextAcks( if (!isAckReceiveTimestampsSupported || !peerRequestedTimestampsCount) { ackWriteResult = writeAckFrame(meta, builder, FrameType::ACK); } else { - meta.recvTimestampsConfig = - conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer - .value(); - meta.maxAckReceiveTimestampsToSend = peerRequestedTimestampsCount; ackWriteResult = writeAckFrameWithReceivedTimestamps( - meta, builder, FrameType::ACK_RECEIVE_TIMESTAMPS); + meta, + builder, + conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer + .value(), + peerRequestedTimestampsCount); } if (!ackWriteResult) { return folly::none; diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index 506be22d7..bee88e454 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -1689,14 +1689,14 @@ void QuicClientTransport::setSupportedExtensionTransportParameters() { static_cast( TransportParameterId::max_receive_timestamps_per_ack), ts.maybeAckReceiveTimestampsConfigSentToPeer.value() - .max_receive_timestamps_per_ack); + .maxReceiveTimestampsPerAck); customTransportParameters_.push_back(maxReceiveTimestampsPerAck.encode()); CustomIntegralTransportParameter receiveTimestampsExponent( static_cast( TransportParameterId::receive_timestamps_exponent), ts.maybeAckReceiveTimestampsConfigSentToPeer.value() - .receive_timestamps_exponent); + .receiveTimestampsExponent); customTransportParameters_.push_back(receiveTimestampsExponent.encode()); } diff --git a/quic/codec/Decode.cpp b/quic/codec/Decode.cpp index 429b38373..0f941ccce 100644 --- a/quic/codec/Decode.cpp +++ b/quic/codec/Decode.cpp @@ -307,7 +307,7 @@ ReadAckFrame decodeAckFrameWithReceivedTimestamps( uint8_t receiveTimestampsExponentToUse = (params.maybeAckReceiveTimestampsConfig) ? params.maybeAckReceiveTimestampsConfig.value() - .receive_timestamps_exponent + .receiveTimestampsExponent : kDefaultReceiveTimestampsExponent; for (uint64_t i = 0; i < receiveTimeStampsLen->first; i++) { auto delta = decodeQuicInteger(cursor); diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 39a97aaf8..bcb2804a2 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -235,8 +235,7 @@ folly::Optional PacketRebuilder::rebuildFromPacket( ackDelay, /* ackDelay */ static_cast(ackDelayExponent), /* ackDelayExponent */ conn_.connectionTime, /* connect timestamp */ - folly::none, /* recvTimestampsConfig */ - folly::none /* maxAckReceiveTimestampsToSend */}; + }; folly::Optional ackWriteResult; @@ -254,12 +253,12 @@ folly::Optional PacketRebuilder::rebuildFromPacket( if (!isAckReceiveTimestampsSupported || !peerRequestedTimestampsCount) { writeAckFrame(meta, builder_, FrameType::ACK); } else { - meta.recvTimestampsConfig = - conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer - .value(); - meta.maxAckReceiveTimestampsToSend = peerRequestedTimestampsCount; writeAckFrameWithReceivedTimestamps( - meta, builder_, FrameType::ACK_RECEIVE_TIMESTAMPS); + meta, + builder_, + conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer + .value(), + peerRequestedTimestampsCount); } } // We shouldn't clone if: diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index ca3350f89..c6e7d5fb0 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -286,7 +286,8 @@ static size_t fillPacketReceiveTimestamps( WriteAckFrame& ackFrame, uint64_t largestAckedPacketNum, uint64_t spaceLeft, - uint64_t receiveTimestampsExponent) { + uint64_t receiveTimestampsExponent, + uint64_t maxRecvTimestampsToSend) { if (ackFrameMetaData.ackState.recvdPacketInfos.size() == 0) { return 0; } @@ -294,9 +295,6 @@ static size_t fillPacketReceiveTimestamps( // Insert all received packet timestamps into an interval set, to identify // continguous ranges - DCHECK(ackFrameMetaData.maxAckReceiveTimestampsToSend.has_value()); - auto maxRecvTimestampsToSend = - ackFrameMetaData.maxAckReceiveTimestampsToSend.value(); uint64_t pktsAdded = 0; IntervalSet receivedPktNumsIntervalSet; for (auto& recvdPkt : recvdPacketInfos) { @@ -491,10 +489,11 @@ folly::Optional writeAckFrame( folly::Optional writeAckFrameWithReceivedTimestamps( const quic::WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, - FrameType frameType) { + const AckReceiveTimestampsConfig& recvTimestampsConfig, + uint64_t maxRecvTimestampsToSend) { auto beginningSpace = builder.remainingSpaceInPkt(); - auto maybeAckFrame = - writeAckFrameToPacketBuilder(ackFrameMetaData, builder, frameType); + auto maybeAckFrame = writeAckFrameToPacketBuilder( + ackFrameMetaData, builder, FrameType::ACK_RECEIVE_TIMESTAMPS); if (!maybeAckFrame.has_value()) { return folly::none; } @@ -527,16 +526,14 @@ folly::Optional writeAckFrameWithReceivedTimestamps( if (spaceLeft > 0) { auto largestAckedPacket = ackState.acks.back().end; uint8_t receiveTimestampsExponentToUse = - ackFrameMetaData.recvTimestampsConfig.has_value() - ? ackFrameMetaData.recvTimestampsConfig.value() - .receive_timestamps_exponent - : kDefaultReceiveTimestampsExponent; + recvTimestampsConfig.receiveTimestampsExponent; countTimestampRanges = fillPacketReceiveTimestamps( ackFrameMetaData, ackFrame, largestAckedPacket, spaceLeft, - receiveTimestampsExponentToUse); + receiveTimestampsExponentToUse, + maxRecvTimestampsToSend); if (countTimestampRanges > 0) { QuicInteger timeStampRangeCountInt( ackFrame.recvdPacketsTimestampRanges.size()); diff --git a/quic/codec/QuicWriteCodec.h b/quic/codec/QuicWriteCodec.h index 00066d28e..c90b88827 100644 --- a/quic/codec/QuicWriteCodec.h +++ b/quic/codec/QuicWriteCodec.h @@ -126,7 +126,8 @@ size_t computeSizeUsedByRecvdTimestamps(quic::WriteAckFrame& writeAckFrame); folly::Optional writeAckFrameWithReceivedTimestamps( const WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, - FrameType frameType = FrameType::ACK_RECEIVE_TIMESTAMPS); + const AckReceiveTimestampsConfig& recvTimestampsConfig, + uint64_t maxRecvTimestampsToSend); folly::Optional writeAckFrameToPacketBuilder( const WriteAckFrameMetaData& ackFrameMetaData, diff --git a/quic/codec/Types.h b/quic/codec/Types.h index 7e527bb61..1fa4599c0 100644 --- a/quic/codec/Types.h +++ b/quic/codec/Types.h @@ -21,7 +21,6 @@ #include #include #include -#include /** * This details the types of objects that can be serialized or deserialized @@ -263,11 +262,6 @@ struct WriteAckFrameMetaData { // Receive timestamps basis TimePoint connTime; - - folly::Optional recvTimestampsConfig = - folly::none; - - folly::Optional maxAckReceiveTimestampsToSend = folly::none; }; struct WriteAckFrameResult { diff --git a/quic/codec/test/QuicWriteCodecTest.cpp b/quic/codec/test/QuicWriteCodecTest.cpp index 50fa42036..ec7008649 100644 --- a/quic/codec/test/QuicWriteCodecTest.cpp +++ b/quic/codec/test/QuicWriteCodecTest.cpp @@ -40,8 +40,7 @@ QuicFrame parseQuicFrame( folly::none; if (isAckReceiveTimestampsSupported) { receiveTimeStampsConfig.assign( - {.max_receive_timestamps_per_ack = 5, - .receive_timestamps_exponent = 3}); + {.maxReceiveTimestampsPerAck = 5, .receiveTimestampsExponent = 3}); } return quic::parseFrame( @@ -149,7 +148,7 @@ void setupCommonExpects(MockQuicPacketBuilder& pktBuilder) { using PacketsReceivedTimestampsDeque = CircularDeque; const auto kDefaultTimestampsDelta = 10us; const AckReceiveTimestampsConfig defaultAckReceiveTimestmpsConfig = { - .receive_timestamps_exponent = kDefaultReceiveTimestampsExponent}; + .receiveTimestampsExponent = kDefaultReceiveTimestampsExponent}; PacketsReceivedTimestampsDeque populateReceiveTimestamps( const AckBlocks& ackBlocks, TimePoint connTime, @@ -244,7 +243,8 @@ void assertsOnDecodedReceiveTimestamps( const WriteAckFrame& writeAckFrame, const ReadAckFrame& readAckFrame, uint64_t expectedTimestampRangesCount, - uint64_t expectedTimestampsCount) { + uint64_t expectedTimestampsCount, + uint64_t receiveTimestampsExponent) { EXPECT_TRUE(readAckFrame.maybeLatestRecvdPacketNum.has_value()); EXPECT_TRUE(readAckFrame.maybeLatestRecvdPacketTime.has_value()); EXPECT_EQ( @@ -282,8 +282,8 @@ void assertsOnDecodedReceiveTimestamps( readAckFrame.recvdPacketsTimestampRanges[i].deltas[j], writeAckFrame.recvdPacketsTimestampRanges[i].deltas[j] * pow(2, - ackFrameMetaData.recvTimestampsConfig.value() - .receive_timestamps_exponent)); + + receiveTimestampsExponent)); } } } @@ -903,13 +903,16 @@ TEST_P(QuicWriteCodecTest, AckFrameGapExceedsRepresentation) { .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig}; + }; EXPECT_THROW( ((frameType == FrameType::ACK) ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType)), + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + 0)), QuicTransportException); } @@ -961,12 +964,14 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -998,7 +1003,8 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { decodedAckFrame, 1 /* timestamp ranges count */, - kMaxReceivedPktsTimestampsStored /* timestamps count */); + kMaxReceivedPktsTimestampsStored /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1023,12 +1029,15 @@ TEST_P(QuicWriteCodecTest, AckFrameNotEnoughForAnything) { .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig}; + }; auto result = (frameType == FrameType::ACK) ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kDefaultAckDelayExponent); EXPECT_FALSE(result.has_value()); EXPECT_EQ(pktBuilder.remainingSpaceInPkt(), 4); } @@ -1047,8 +1056,7 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) { .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; // 1 type byte, // 2 bytes for largest acked, 1 bytes for ack delay => 3 bytes // 1 byte for ack block count @@ -1058,7 +1066,10 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) { auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); EXPECT_EQ(11 + addlBytesConsumed, ackFrameWriteResult.bytesWritten); @@ -1101,7 +1112,8 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) { decodedAckFrame, 1 /* timestamp ranges count */, - kMaxReceivedPktsTimestampsStored /* timestamps count */); + kMaxReceivedPktsTimestampsStored /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1119,13 +1131,15 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWillSaveAckDelay) { .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildTestPacket(); auto regularPacket = builtOut.first; WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); @@ -1166,8 +1180,7 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; // 1 type byte, // 2 bytes for largest acked, 1 bytes for ack delay => 3 bytes // 1 byte for ack block count @@ -1177,7 +1190,10 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); auto builtOut = std::move(pktBuilder).buildTestPacket(); @@ -1216,7 +1232,8 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { decodedAckFrame, 0 /* timestamp ranges count */, - 0 /* timestamps count */); + 0 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1236,13 +1253,15 @@ TEST_P(QuicWriteCodecTest, WriteWithDifferentAckDelayExponent) { .ackDelay = 1240us, .ackDelayExponent = static_cast(ackDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; (frameType == FrameType::ACK) ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildTestPacket(); auto wireBuf = std::move(builtOut.second); BufQueue queue; @@ -1272,13 +1291,15 @@ TEST_P(QuicWriteCodecTest, WriteExponentInLongHeaderPacket) { .ackDelay = 1240us, .ackDelayExponent = static_cast(ackDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; (frameType == FrameType::ACK) ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildLongHeaderPacket(); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1317,13 +1338,15 @@ TEST_P(QuicWriteCodecTest, OnlyAckLargestPacket) { .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1364,7 +1387,8 @@ TEST_P(QuicWriteCodecTest, OnlyAckLargestPacket) { decodedAckFrame, 1 /* timestamp ranges count */, - 1 /* timestamps count */); + 1 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1404,13 +1428,15 @@ TEST_P(QuicWriteCodecTest, WriteSomeAckBlocks) { .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1446,7 +1472,8 @@ TEST_P(QuicWriteCodecTest, WriteSomeAckBlocks) { decodedAckFrame, 0 /* timestamp ranges count */, - 0 /* timestamps count */); + 0 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1469,13 +1496,15 @@ TEST_P(QuicWriteCodecTest, NoSpaceForAckBlockSection) { .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); EXPECT_FALSE(ackFrameWriteResult.has_value()); } @@ -1505,13 +1534,15 @@ TEST_P(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) { .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = kMaxReceivedPktsTimestampsStored}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1546,7 +1577,8 @@ TEST_P(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) { decodedAckFrame, 0 /* timestamp ranges count */, - 0 /* timestamps count */); + 0 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1571,13 +1603,12 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWithMultipleTimestampRanges) { .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = 50}; + }; auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, pktBuilder, defaultAckReceiveTimestmpsConfig, 50); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1621,7 +1652,8 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWithMultipleTimestampRanges) { decodedAckFrame, 3 /* timestamp ranges count */, - 50 /* timestamps count */); + 50 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } @@ -1648,8 +1680,7 @@ TEST_P( .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, - .recvTimestampsConfig = defaultAckReceiveTimestmpsConfig, - .maxAckReceiveTimestampsToSend = 100}; + }; if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { pktBuilder.remaining_ = 80; @@ -1657,7 +1688,10 @@ TEST_P( auto ackFrameWriteResult = (frameType == FrameType::ACK) ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, frameType); + ackFrameMetaData, + pktBuilder, + defaultAckReceiveTimestmpsConfig, + 100); auto addlBytesConsumed = computeBytesForAckReceivedTimestamps( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1703,7 +1737,8 @@ TEST_P( decodedAckFrame, 3 /* timestamp ranges count */, - 57 /* timestamps count */); + 57 /* timestamps count */, + defaultAckReceiveTimestmpsConfig.receiveTimestampsExponent); } } diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 79dd24ce2..bb45a3de2 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -1587,15 +1587,15 @@ std::vector setSupportedExtensionTransportParameters( CustomIntegralTransportParameter maxReceiveTimestampsPerAck( static_cast( TransportParameterId::max_receive_timestamps_per_ack), - ts.maybeAckReceiveTimestampsConfigSentToPeer.value() - .max_receive_timestamps_per_ack); + ts.maybeAckReceiveTimestampsConfigSentToPeer + ->maxReceiveTimestampsPerAck); customTransportParams.push_back(maxReceiveTimestampsPerAck.encode()); CustomIntegralTransportParameter receiveTimestampsExponent( static_cast( TransportParameterId::receive_timestamps_exponent), - ts.maybeAckReceiveTimestampsConfigSentToPeer.value() - .receive_timestamps_exponent); + ts.maybeAckReceiveTimestampsConfigSentToPeer + ->receiveTimestampsExponent); customTransportParams.push_back(receiveTimestampsExponent.encode()); } diff --git a/quic/state/StateData.h b/quic/state/StateData.h index ab6f10abb..299c93be4 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -712,10 +712,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction { // GSO supported on conn. folly::Optional gsoSupported; - struct AckReceiveTimestampsConfig { - uint64_t maxReceiveTimestampsPerAck; - uint64_t receiveTimestampsExponent; - }; folly::Optional maybePeerAckReceiveTimestampsConfig; diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index 7df042d52..3ae3417d7 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -69,8 +69,8 @@ struct DatagramConfig { }; struct AckReceiveTimestampsConfig { - uint64_t max_receive_timestamps_per_ack{kMaxReceivedPktsTimestampsStored}; - uint64_t receive_timestamps_exponent{kDefaultReceiveTimestampsExponent}; + uint64_t maxReceiveTimestampsPerAck{kMaxReceivedPktsTimestampsStored}; + uint64_t receiveTimestampsExponent{kDefaultReceiveTimestampsExponent}; }; // JSON-serialized transport knobs