From 87d00ece355e88df75ee4ef41978a9fd6fac5c61 Mon Sep 17 00:00:00 2001 From: Brandon Schlinker Date: Thu, 5 Jan 2023 15:20:44 -0800 Subject: [PATCH] Fix dependency loop and improve namings Summary: Fixing dependency loop introduced by D37799050 (https://github.com/facebookincubator/mvfst/commit/96abc8160d99596cd3cbc3e3039256a23f14ce28) Running `autodeps` yields the following patch: ``` --- a/xplat/quic/state/TARGETS +++ b/xplat/quic/state/TARGETS @@ -43,8 +43,8 @@ exported_deps = [ "//folly:random", "//quic:constants", + "//quic/codec:codec", "//quic/codec:types", - "//quic/common:circular_deque", "//quic/common:interval_set", ], ) ``` If this patch is applied, there is a circular dependency loop between `//quic/codec:codec` and `//quic/state:ack_states` by way of `//quic/codec:types`; this loop was introduced by D37799050 (https://github.com/facebookincubator/mvfst/commit/96abc8160d99596cd3cbc3e3039256a23f14ce28). Fixed by separating out headers files and targets. In parallel, renamed structures used for writing ACK frames (which were the reason this loop occurred) to make their role clear. Differential Revision: D42281359 fbshipit-source-id: 8514c99f3fe72ff1d942d7f303e4a209838c7623 --- quic/api/QuicPacketScheduler.cpp | 4 +- quic/api/test/QuicPacketSchedulerTest.cpp | 4 +- quic/codec/PacketNumber.cpp | 6 ++ quic/codec/PacketNumber.h | 5 +- quic/codec/QuicPacketRebuilder.cpp | 4 +- quic/codec/QuicWriteCodec.cpp | 20 ++--- quic/codec/QuicWriteCodec.h | 81 ++----------------- quic/codec/Types.h | 72 ++++++++++++++++- quic/codec/test/QuicPacketRebuilderTest.cpp | 10 +-- quic/codec/test/QuicWriteCodecTest.cpp | 66 +++++++-------- quic/common/test/TestPacketBuilders.cpp | 4 +- quic/d6d/Types.h | 2 +- .../client/test/QuicClientTransportTest.cpp | 6 +- quic/server/test/QuicServerTransportTest.cpp | 4 +- quic/state/AckStates.h | 6 +- quic/state/TransportSettings.h | 1 - 16 files changed, 151 insertions(+), 144 deletions(-) diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 4e08f7bbf..6b51f4e81 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -586,7 +586,7 @@ folly::Optional AckScheduler::writeNextAcks( ackingTime - receivedTime) : 0us); - AckFrameMetaData meta = { + WriteAckFrameMetaData meta = { ackState_, /* ackState*/ ackDelay, /* ackDelay */ static_cast(ackDelayExponentToUse), /* ackDelayExponent */ @@ -594,7 +594,7 @@ folly::Optional AckScheduler::writeNextAcks( folly::none, /* recvTimestampsConfig */ folly::none /* maxAckReceiveTimestampsToSend */}; - folly::Optional ackWriteResult; + folly::Optional ackWriteResult; bool isAckReceiveTimestampsSupported = conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer && diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index 5757ec09e..ad18f5ab2 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -646,8 +646,8 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { AckBlocks ackBlocks; ackBlocks.insert(10, 100); ackBlocks.insert(200, 1000); - WriteAckState writeAckState = {.acks = ackBlocks}; - AckFrameMetaData ackMeta = { + WriteAckFrameState writeAckState = {.acks = ackBlocks}; + WriteAckFrameMetaData ackMeta = { .ackState = writeAckState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent)}; diff --git a/quic/codec/PacketNumber.cpp b/quic/codec/PacketNumber.cpp index 4ff770a64..8685196da 100644 --- a/quic/codec/PacketNumber.cpp +++ b/quic/codec/PacketNumber.cpp @@ -5,7 +5,13 @@ * LICENSE file in the root directory of this source tree. */ +#include +#include +#include +#include +#include #include +#include namespace quic { diff --git a/quic/codec/PacketNumber.h b/quic/codec/PacketNumber.h index b0f60f9c9..21b437759 100644 --- a/quic/codec/PacketNumber.h +++ b/quic/codec/PacketNumber.h @@ -7,10 +7,13 @@ #pragma once -#include +#include +#include namespace quic { +using PacketNum = uint64_t; + /** * Returns a decoded packet number by using the expectedNextPacketNum to * search for the most probable packet number that could satisfy that condition. diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 9f6b51372..5b02e68a0 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -229,7 +229,7 @@ folly::Optional PacketRebuilder::rebuildFromPacket( ackingTime - receivedTime) : 0us); - AckFrameMetaData meta = { + WriteAckFrameMetaData meta = { ackState_, /* ackState*/ ackDelay, /* ackDelay */ static_cast(ackDelayExponent), /* ackDelayExponent */ @@ -237,7 +237,7 @@ folly::Optional PacketRebuilder::rebuildFromPacket( folly::none, /* recvTimestampsConfig */ folly::none /* maxAckReceiveTimestampsToSend */}; - folly::Optional ackWriteResult; + folly::Optional ackWriteResult; uint64_t peerRequestedTimestampsCount = conn_.maybePeerAckReceiveTimestampsConfig.has_value() diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index 6304da4fb..ca2efedea 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -282,7 +282,7 @@ size_t computeSizeUsedByRecvdTimestamps(WriteAckFrame& ackFrame) { } static size_t fillPacketReceiveTimestamps( - const quic::AckFrameMetaData& ackFrameMetaData, + const quic::WriteAckFrameMetaData& ackFrameMetaData, WriteAckFrame& ackFrame, uint64_t largestAckedPacketNum, uint64_t spaceLeft, @@ -376,13 +376,13 @@ static size_t fillPacketReceiveTimestamps( } folly::Optional writeAckFrameToPacketBuilder( - const quic::AckFrameMetaData& ackFrameMetaData, + const quic::WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, FrameType frameType) { if (ackFrameMetaData.ackState.acks.empty()) { return folly::none; } - const WriteAckState& ackState = ackFrameMetaData.ackState; + const WriteAckFrameState& ackState = ackFrameMetaData.ackState; // The last block must be the largest block. auto largestAckedPacket = ackState.acks.back().end; // ackBlocks are already an interval set so each value is naturally @@ -469,8 +469,8 @@ folly::Optional writeAckFrameToPacketBuilder( return ackFrame; } -folly::Optional writeAckFrame( - const quic::AckFrameMetaData& ackFrameMetaData, +folly::Optional writeAckFrame( + const quic::WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, FrameType frameType) { uint64_t beginningSpace = builder.remainingSpaceInPkt(); @@ -479,7 +479,7 @@ folly::Optional writeAckFrame( if (maybeWriteAckFrame.has_value()) { builder.appendFrame(std::move(maybeWriteAckFrame.value())); - return AckFrameWriteResult( + return WriteAckFrameResult( beginningSpace - builder.remainingSpaceInPkt(), maybeWriteAckFrame.value(), maybeWriteAckFrame.value().ackBlocks.size()); @@ -488,8 +488,8 @@ folly::Optional writeAckFrame( } } -folly::Optional writeAckFrameWithReceivedTimestamps( - const quic::AckFrameMetaData& ackFrameMetaData, +folly::Optional writeAckFrameWithReceivedTimestamps( + const quic::WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, FrameType frameType) { auto beginningSpace = builder.remainingSpaceInPkt(); @@ -499,7 +499,7 @@ folly::Optional writeAckFrameWithReceivedTimestamps( return folly::none; } auto ackFrame = maybeAckFrame.value(); - const WriteAckState& ackState = ackFrameMetaData.ackState; + const WriteAckFrameState& ackState = ackFrameMetaData.ackState; uint64_t spaceLeft = builder.remainingSpaceInPkt(); uint64_t lastPktNum = 0; std::chrono::microseconds lastPktTsDelta = 0us; @@ -559,7 +559,7 @@ folly::Optional writeAckFrameWithReceivedTimestamps( builder.write(timeStampRangeCountInt); } } - auto ackFrameWriteResult = AckFrameWriteResult( + auto ackFrameWriteResult = WriteAckFrameResult( beginningSpace - builder.remainingSpaceInPkt(), ackFrame, ackFrame.ackBlocks.size(), diff --git a/quic/codec/QuicWriteCodec.h b/quic/codec/QuicWriteCodec.h index 125cf17a9..00066d28e 100644 --- a/quic/codec/QuicWriteCodec.h +++ b/quic/codec/QuicWriteCodec.h @@ -19,75 +19,6 @@ namespace quic { -// Ack and PacketNumber states. This is per-packet number space. -struct WriteAckState { - AckBlocks acks; - - // Receive timestamp and packet number for the largest received packet. - // - // Updated whenever we receive a packet with a larger packet number - // than all previously received packets in the packet number space - // tracked by this AckState. - folly::Optional largestRecvdPacketInfo; - // Receive timestamp and packet number for the last received packet. - // - // Will be different from the value stored in largestRecvdPacketInfo - // if the last packet was received out of order and thus had a packet - // number less than that of a previously received packet in the packet - // number space tracked by this AckState. - folly::Optional lastRecvdPacketInfo; - - // Packet number and timestamp of recently received packets. - // - // The maximum number of packets stored in pktsReceivedTimestamps is - // controlled by kMaxReceivedPktsTimestampsStored. - // - // The packet number of entries in the deque is guarenteed to increase - // monotonically because an entry is only added for a received packet - // if the packet number is greater than the packet number of the last - // element in the deque (e.g., entries are not added for packets that - // arrive out of order relative to previously received packets). - CircularDeque recvdPacketInfos; -}; - -struct AckFrameMetaData { - // ACK state. - const WriteAckState& ackState; - - // Delay in sending ack from time that packet was received. - std::chrono::microseconds ackDelay; - // The ack delay exponent to use. - uint8_t ackDelayExponent; - - // Receive timestamps basis - TimePoint connTime; - - folly::Optional recvTimestampsConfig = - folly::none; - - folly::Optional maxAckReceiveTimestampsToSend = folly::none; -}; - -struct AckFrameWriteResult { - uint64_t bytesWritten; - WriteAckFrame writeAckFrame; - // This includes the first ack block - size_t ackBlocksWritten; - size_t timestampRangesWritten; - size_t timestampsWritten; - AckFrameWriteResult( - uint64_t bytesWrittenIn, - WriteAckFrame writeAckFrameIn, - size_t ackBlocksWrittenIn, - size_t timestampRangesWrittenIn = 0, - size_t timestampsWrittenIn = 0) - : bytesWritten(bytesWrittenIn), - writeAckFrame(writeAckFrameIn), - ackBlocksWritten(ackBlocksWrittenIn), - timestampRangesWritten(timestampRangesWrittenIn), - timestampsWritten(timestampsWrittenIn) {} -}; - /** * Write a simple QuicFrame into builder * @@ -178,12 +109,12 @@ folly::Optional writeCryptoFrame( * of the packet sequence numbers. Exception will be thrown if they are not * sorted. * - * Return: A AckFrameWriteResult to indicate how many bytes and ack blocks are + * Return: A WriteAckFrameResult to indicate how many bytes and ack blocks are * written to the appender. Returns an empty optional if an ack block could not * be written. */ -folly::Optional writeAckFrame( - const AckFrameMetaData& ackFrameMetaData, +folly::Optional writeAckFrame( + const WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, FrameType frameType = FrameType::ACK); @@ -192,13 +123,13 @@ folly::Optional writeAckFrame( */ size_t computeSizeUsedByRecvdTimestamps(quic::WriteAckFrame& writeAckFrame); -folly::Optional writeAckFrameWithReceivedTimestamps( - const AckFrameMetaData& ackFrameMetaData, +folly::Optional writeAckFrameWithReceivedTimestamps( + const WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, FrameType frameType = FrameType::ACK_RECEIVE_TIMESTAMPS); folly::Optional writeAckFrameToPacketBuilder( - const quic::AckFrameMetaData& ackFrameMetaData, + const WriteAckFrameMetaData& ackFrameMetaData, quic::PacketBuilderInterface& builder, quic::FrameType frameType); diff --git a/quic/codec/Types.h b/quic/codec/Types.h index c1f1cb791..0fc112bbb 100644 --- a/quic/codec/Types.h +++ b/quic/codec/Types.h @@ -13,12 +13,15 @@ #include #include #include +#include #include #include #include +#include #include #include #include +#include /** * This details the types of objects that can be serialized or deserialized @@ -29,7 +32,6 @@ namespace quic { using StreamId = uint64_t; using StreamGroupId = uint64_t; -using PacketNum = uint64_t; enum class PacketNumberSpace : uint8_t { Initial, @@ -218,6 +220,74 @@ struct WriteAckFrame { } }; +struct WriteAckFrameState { + AckBlocks acks; + + // Receive timestamp and packet number for the largest received packet. + // + // Updated whenever we receive a packet with a larger packet number + // than all previously received packets in the packet number space + // tracked by this AckState. + folly::Optional largestRecvdPacketInfo; + // Receive timestamp and packet number for the last received packet. + // + // Will be different from the value stored in largestRecvdPacketInfo + // if the last packet was received out of order and thus had a packet + // number less than that of a previously received packet in the packet + // number space tracked by this AckState. + folly::Optional lastRecvdPacketInfo; + + // Packet number and timestamp of recently received packets. + // + // The maximum number of packets stored in pktsReceivedTimestamps is + // controlled by kMaxReceivedPktsTimestampsStored. + // + // The packet number of entries in the deque is guarenteed to increase + // monotonically because an entry is only added for a received packet + // if the packet number is greater than the packet number of the last + // element in the deque (e.g., entries are not added for packets that + // arrive out of order relative to previously received packets). + CircularDeque recvdPacketInfos; +}; + +struct WriteAckFrameMetaData { + // ACK state. + const WriteAckFrameState& ackState; + + // Delay in sending ack from time that packet was received. + std::chrono::microseconds ackDelay; + // The ack delay exponent to use. + uint8_t ackDelayExponent; + + // Receive timestamps basis + TimePoint connTime; + + folly::Optional recvTimestampsConfig = + folly::none; + + folly::Optional maxAckReceiveTimestampsToSend = folly::none; +}; + +struct WriteAckFrameResult { + uint64_t bytesWritten; + WriteAckFrame writeAckFrame; + // This includes the first ack block + size_t ackBlocksWritten; + size_t timestampRangesWritten; + size_t timestampsWritten; + WriteAckFrameResult( + uint64_t bytesWrittenIn, + WriteAckFrame writeAckFrameIn, + size_t ackBlocksWrittenIn, + size_t timestampRangesWrittenIn = 0, + size_t timestampsWrittenIn = 0) + : bytesWritten(bytesWrittenIn), + writeAckFrame(std::move(writeAckFrameIn)), + ackBlocksWritten(ackBlocksWrittenIn), + timestampRangesWritten(timestampRangesWrittenIn), + timestampsWritten(timestampsWrittenIn) {} +}; + struct RstStreamFrame { StreamId streamId; ApplicationErrorCode errorCode; diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index afe119b5f..12efd01cf 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -75,9 +75,9 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { AckBlocks ackBlocks; ackBlocks.insert(10, 100); ackBlocks.insert(200, 1000); - WriteAckState writeAckState = {.acks = ackBlocks}; - // AckFrameMetaData ackMeta(ackBlocks, 0us, kDefaultAckDelayExponent); - AckFrameMetaData ackMeta = { + WriteAckFrameState writeAckState = {.acks = ackBlocks}; + // WriteAckFrameMetaData ackMeta(ackBlocks, 0us, kDefaultAckDelayExponent); + WriteAckFrameMetaData ackMeta = { .ackState = writeAckState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent)}; @@ -407,8 +407,8 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) { AckBlocks ackBlocks; ackBlocks.insert(10, 100); ackBlocks.insert(200, 1000); - WriteAckState writeAckState = {.acks = ackBlocks}; - AckFrameMetaData ackMeta = { + WriteAckFrameState writeAckState = {.acks = ackBlocks}; + WriteAckFrameMetaData ackMeta = { .ackState = writeAckState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent)}; diff --git a/quic/codec/test/QuicWriteCodecTest.cpp b/quic/codec/test/QuicWriteCodecTest.cpp index 0d1c55062..44ef1df7e 100644 --- a/quic/codec/test/QuicWriteCodecTest.cpp +++ b/quic/codec/test/QuicWriteCodecTest.cpp @@ -175,8 +175,8 @@ PacketsReceivedTimestampsDeque populateReceiveTimestamps( } size_t computeBytesForAckReceivedTimestamps( - AckFrameMetaData ackFrameMetadata, - AckFrameWriteResult ackFrameWriteResult, + const WriteAckFrameMetaData& ackFrameMetadata, + WriteAckFrameResult ackFrameWriteResult, FrameType frameType) { if (frameType != FrameType::ACK_RECEIVE_TIMESTAMPS) { return 0; @@ -212,12 +212,12 @@ size_t computeBytesForAckReceivedTimestamps( return sizeConsumed; } -WriteAckState createTestWriteAckState( +WriteAckFrameState createTestWriteAckState( FrameType frameType, const TimePoint& connTime, AckBlocks& ackBlocks, uint64_t countTimestampsToStore = kMaxReceivedPktsTimestampsStored) { - WriteAckState ackState = {.acks = ackBlocks}; + WriteAckFrameState ackState = {.acks = ackBlocks}; ackState.acks = ackBlocks; if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { ackState.recvdPacketInfos = @@ -231,7 +231,7 @@ WriteAckState createTestWriteAckState( } void assertsOnDecodedReceiveTimestamps( - const AckFrameMetaData& ackFrameMetaData, + const WriteAckFrameMetaData& ackFrameMetaData, const WriteAckFrame& writeAckFrame, const ReadAckFrame& readAckFrame, uint64_t expectedTimestampRangesCount, @@ -887,9 +887,9 @@ TEST_P(QuicWriteCodecTest, AckFrameGapExceedsRepresentation) { AckBlocks ackBlocks = {{max - 10, max - 10}, {1, 1}}; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -919,7 +919,7 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { TimePoint connTime = Clock::now(); - WriteAckState ackState = {.acks = ackBlocks}; + WriteAckFrameState ackState = {.acks = ackBlocks}; ackState.acks = ackBlocks; if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { auto lastPacketDelta = @@ -947,7 +947,7 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { }); } - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1007,9 +1007,9 @@ TEST_P(QuicWriteCodecTest, AckFrameNotEnoughForAnything) { AckBlocks ackBlocks = {{1000, 1000}, {500, 700}, {100, 200}}; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1031,9 +1031,9 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) { AckBlocks ackBlocks = {{501, 1000}, {101, 400}}; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1103,9 +1103,9 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWillSaveAckDelay) { AckBlocks ackBlocks = {{501, 1000}, {101, 400}}; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1150,9 +1150,9 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { } ackBlocks.insert({largest, largest}); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1220,9 +1220,9 @@ TEST_P(QuicWriteCodecTest, WriteWithDifferentAckDelayExponent) { auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 1240us, .ackDelayExponent = static_cast(ackDelayExponent), @@ -1256,9 +1256,9 @@ TEST_P(QuicWriteCodecTest, WriteExponentInLongHeaderPacket) { uint8_t ackDelayExponent = 6; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 1240us, .ackDelayExponent = static_cast(ackDelayExponent), @@ -1301,9 +1301,9 @@ TEST_P(QuicWriteCodecTest, OnlyAckLargestPacket) { // generated as the first block to cover largestAcked => 2 bytes auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1388,9 +1388,9 @@ TEST_P(QuicWriteCodecTest, WriteSomeAckBlocks) { pktBuilder.remaining_ = 42; } TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1453,9 +1453,9 @@ TEST_P(QuicWriteCodecTest, NoSpaceForAckBlockSection) { AckBlocks ackBlocks = {{1000, 1000}, {701, 900}, {501, 600}}; auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1489,9 +1489,9 @@ TEST_P(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) { // 1 byte for first ack block length AckBlocks ackBlocks = {{1000, 1000}, {701, 900}, {501, 600}}; TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = 555us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1555,9 +1555,9 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWithMultipleTimestampRanges) { // total 11 bytes auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks, 50); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), @@ -1632,9 +1632,9 @@ TEST_P( // total 11 bytes auto frameType = GetParam(); TimePoint connTime = Clock::now(); - WriteAckState ackState = + WriteAckFrameState ackState = createTestWriteAckState(frameType, connTime, ackBlocks, 100); - AckFrameMetaData ackFrameMetaData = { + WriteAckFrameMetaData ackFrameMetaData = { .ackState = ackState, .ackDelay = ackDelay, .ackDelayExponent = static_cast(kDefaultAckDelayExponent), diff --git a/quic/common/test/TestPacketBuilders.cpp b/quic/common/test/TestPacketBuilders.cpp index 1b1a38749..aea1222c5 100644 --- a/quic/common/test/TestPacketBuilders.cpp +++ b/quic/common/test/TestPacketBuilders.cpp @@ -131,9 +131,9 @@ RegularQuicPacketBuilder::Packet AckPacketBuilder::build() && { builder.accountForCipherOverhead(maybeAead.value()->getCipherOverhead()); } DCHECK(builder.canBuildPacket()); - WriteAckState ackState; + WriteAckFrameState ackState; ackState.acks = *CHECK_NOTNULL(maybeAckBlocks.get_pointer()); - AckFrameMetaData ackData = { + WriteAckFrameMetaData ackData = { .ackState = ackState, .ackDelay = *CHECK_NOTNULL(maybeAckDelay.get_pointer()), .ackDelayExponent = static_cast( diff --git a/quic/d6d/Types.h b/quic/d6d/Types.h index 6aea4c6ce..238cc37f3 100644 --- a/quic/d6d/Types.h +++ b/quic/d6d/Types.h @@ -7,7 +7,7 @@ #pragma once -#include +#include #include namespace quic { diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index fe77fdccc..1790ff22c 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -4086,9 +4086,9 @@ TEST_F(QuicClientTransportVersionAndRetryTest, UnencryptedAckData) { kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); - // AckFrameMetaData ackData(acks, 0us, 0); - WriteAckState writeAckState = {.acks = acks}; - AckFrameMetaData ackData = { + // WriteAckFrameMetaData ackData(acks, 0us, 0); + WriteAckFrameState writeAckState = {.acks = acks}; + WriteAckFrameMetaData ackData = { .ackState = writeAckState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent)}; diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 9d36e0407..2b37f1e08 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -3287,8 +3287,8 @@ TEST_F(QuicUnencryptedServerTransportTest, TestUnencryptedAck) { kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); - WriteAckState writeAckState = {.acks = acks}; - AckFrameMetaData ackData = { + WriteAckFrameState writeAckState = {.acks = acks}; + WriteAckFrameMetaData ackData = { .ackState = writeAckState, .ackDelay = 0us, .ackDelayExponent = static_cast(kDefaultAckDelayExponent)}; diff --git a/quic/state/AckStates.h b/quic/state/AckStates.h index 5a20545a4..725ab8ea5 100644 --- a/quic/state/AckStates.h +++ b/quic/state/AckStates.h @@ -7,17 +7,15 @@ #pragma once +#include #include -#include #include #include -#include - namespace quic { // Ack and PacketNumber states. This is per-packet number space. -struct AckState : WriteAckState { +struct AckState : WriteAckFrameState { // Largest ack that has been written to a packet folly::Optional largestAckScheduled; // Count of outstanding packets received with only non-retransmittable data. diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index b2e1d71ae..a68d02880 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -8,7 +8,6 @@ #pragma once #include -#include #include #include #include