From 015c62e96434e4404af8f7ff8c0a4645334d65ef Mon Sep 17 00:00:00 2001 From: Joseph Beshay Date: Thu, 13 Feb 2025 17:48:18 -0800 Subject: [PATCH] Refactor ACK writing for the different ACK types Summary: This replaces three separate functions for writing ACK, ACK_ECN, and ACK_RECEIVE_TIMESTAMPS with one function that can write all three. Previously, we have two paths: 1. Default path which writes the base ACK frame and optionally adds the ECN counts if the frame type is ACK_ECN. 2. If ACK_RECEIVE_TIMESTAMPS are supported, another path would take the ART config and write that frame with the ART fields. Path #2 does not support ECN marks. This change consolidates ACK writing into a single path which takes ART config and frame type. It decides which fields to include based upon the type and config passed. This change does not add any new frame types but it prepares for one that can carry both ECN counts and ART fields. Differential Revision: D68931147 fbshipit-source-id: 47b425b30f00b6c76574bc768d0ec249c60a0aa7 --- quic/api/QuicPacketScheduler.cpp | 3 +- quic/codec/QuicPacketRebuilder.cpp | 3 +- quic/codec/QuicWriteCodec.cpp | 222 +++++++++++++++---------- quic/codec/QuicWriteCodec.h | 16 +- quic/codec/test/QuicWriteCodecTest.cpp | 191 ++++++++++----------- 5 files changed, 227 insertions(+), 208 deletions(-) diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index c2957b04c..182cbaa52 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -621,9 +621,10 @@ Optional AckScheduler::writeNextAcks( isAckReceiveTimestampsSupported && (peerRequestedTimestampsCount > 0)) { // Use ACK_RECEIVE_TIMESTAMPS if its enabled on both endpoints AND the peer // requests at least 1 timestamp - ackWriteResult = writeAckFrameWithReceivedTimestamps( + ackWriteResult = writeAckFrame( meta, builder, + FrameType::ACK_RECEIVE_TIMESTAMPS, conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer .value(), peerRequestedTimestampsCount); diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 01b271ee1..8f11e695e 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -256,9 +256,10 @@ Optional PacketRebuilder::rebuildFromPacket( if (!isAckReceiveTimestampsSupported || !peerRequestedTimestampsCount) { writeAckFrame(meta, builder_, FrameType::ACK); } else { - writeAckFrameWithReceivedTimestamps( + writeAckFrame( meta, builder_, + FrameType::ACK_RECEIVE_TIMESTAMPS, conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer .value(), peerRequestedTimestampsCount); diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index e36557fdc..f1afbe5d5 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -274,7 +274,7 @@ size_t computeSizeUsedByRecvdTimestamps(WriteAckFrame& ackFrame) { return usedSize; } -static size_t fillPacketReceiveTimestamps( +static size_t fillFrameWithPacketReceiveTimestamps( const quic::WriteAckFrameMetaData& ackFrameMetaData, WriteAckFrame& ackFrame, uint64_t largestAckedPacketNum, @@ -369,39 +369,18 @@ static size_t fillPacketReceiveTimestamps( return ackFrame.recvdPacketsTimestampRanges.size(); } -Optional writeAckFrameToPacketBuilder( +static Optional maybeWriteAckBaseFields( const quic::WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, - FrameType frameType) { - if (ackFrameMetaData.ackState.acks.empty()) { - return none; - } + FrameType frameType, + uint64_t maxSpaceToUse) { + auto spaceLeft = maxSpaceToUse; 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 // non-overlapping. auto firstAckBlockLength = largestAckedPacket - ackState.acks.back().start; - - WriteAckFrame ackFrame; - ackFrame.frameType = frameType; - uint64_t spaceLeft = builder.remainingSpaceInPkt(); - - // Reserve space for ECN counts if enabled - QuicInteger ecnECT0Count(ackFrameMetaData.ackState.ecnECT0CountReceived); - QuicInteger ecnECT1Count(ackFrameMetaData.ackState.ecnECT1CountReceived); - QuicInteger ecnCECount(ackFrameMetaData.ackState.ecnCECountReceived); - if (frameType == FrameType::ACK_ECN) { - ackFrame.ecnECT0Count = ackFrameMetaData.ackState.ecnECT0CountReceived; - ackFrame.ecnECT1Count = ackFrameMetaData.ackState.ecnECT1CountReceived; - ackFrame.ecnCECount = ackFrameMetaData.ackState.ecnCECountReceived; - spaceLeft -= - (ecnECT0Count.getSize() + ecnECT1Count.getSize() + - ecnCECount.getSize()); - } - - ackFrame.ackBlocks.reserve(spaceLeft / 4); - // We could technically split the range if the size of the representation of // the integer is too large, but that gets super tricky and is of dubious // value. @@ -417,43 +396,25 @@ Optional writeAckFrameToPacketBuilder( // Required fields are Type, LargestAcked, AckDelay, AckBlockCount, // firstAckBlockLength QuicInteger encodedintFrameType(static_cast(frameType)); - auto headerSize = encodedintFrameType.getSize() + + uint64_t headerSize = encodedintFrameType.getSize() + largestAckedPacketInt.getSize() + ackDelayInt.getSize() + minAdditionalAckBlockCount.getSize() + firstAckBlockLengthInt.getSize(); - size_t minAdditionalAckReceiveTimestampsFieldsSize = 0; - if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { - // Compute minimum size requirements for 3 fields that must be sent - // in every ACK_RECEIVE_TIMESTAMPS frame - uint64_t countTimestampRanges = 0; - uint64_t maybeLastPktNum = 0; - std::chrono::microseconds maybeLastPktTsDelta = 0us; - if (ackState.lastRecvdPacketInfo) - maybeLastPktNum = ackState.lastRecvdPacketInfo.value().pktNum; - - maybeLastPktTsDelta = - (ackState.lastRecvdPacketInfo.value().timings.receiveTimePoint > - ackFrameMetaData.connTime - ? std::chrono::duration_cast( - ackState.lastRecvdPacketInfo.value() - .timings.receiveTimePoint - - ackFrameMetaData.connTime) - : 0us); - - minAdditionalAckReceiveTimestampsFieldsSize = - getQuicIntegerSize(countTimestampRanges).value_or(0) + - getQuicIntegerSize(maybeLastPktNum).value_or(0) + - getQuicIntegerSize(maybeLastPktTsDelta.count()).value_or(0); - } - if (spaceLeft < (headerSize + minAdditionalAckReceiveTimestampsFieldsSize)) { + if (spaceLeft < headerSize) { return none; } - spaceLeft -= (headerSize + minAdditionalAckReceiveTimestampsFieldsSize); + WriteAckFrame ackFrame; + ackFrame.frameType = frameType; + + // Reserve the number of ack blocks we could fit in the remaining space. + ackFrame.ackBlocks.reserve(spaceLeft / 4); + + // Account for the header size + spaceLeft -= headerSize; ackFrame.ackBlocks.push_back(ackState.acks.back()); auto numAdditionalAckBlocks = fillFrameWithAckBlocks(ackState.acks, ackFrame, spaceLeft); - QuicInteger numAdditionalAckBlocksInt(numAdditionalAckBlocks); builder.write(encodedintFrameType); builder.write(largestAckedPacketInt); @@ -475,45 +436,72 @@ Optional writeAckFrameToPacketBuilder( currentSeqNum = it->start; } ackFrame.ackDelay = ackFrameMetaData.ackDelay; - if (frameType == FrameType::ACK_ECN) { - builder.write(ecnECT0Count); - builder.write(ecnECT1Count); - builder.write(ecnCECount); - } + return ackFrame; } -Optional writeAckFrame( - const quic::WriteAckFrameMetaData& ackFrameMetaData, - PacketBuilderInterface& builder, - FrameType frameType) { - uint64_t beginningSpace = builder.remainingSpaceInPkt(); - auto maybeWriteAckFrame = - writeAckFrameToPacketBuilder(ackFrameMetaData, builder, frameType); - - if (maybeWriteAckFrame.has_value()) { - builder.appendFrame(std::move(maybeWriteAckFrame.value())); - return WriteAckFrameResult( - beginningSpace - builder.remainingSpaceInPkt(), - maybeWriteAckFrame.value(), - maybeWriteAckFrame.value().ackBlocks.size()); - } else { - return none; - } +static uint64_t computeEcnRequiredSpace( + const quic::WriteAckFrameMetaData& ackFrameMetaData) { + QuicInteger ecnECT0Count(ackFrameMetaData.ackState.ecnECT0CountReceived); + QuicInteger ecnECT1Count(ackFrameMetaData.ackState.ecnECT1CountReceived); + QuicInteger ecnCECount(ackFrameMetaData.ackState.ecnCECountReceived); + return ecnECT0Count.getSize() + ecnECT1Count.getSize() + ecnCECount.getSize(); } -Optional writeAckFrameWithReceivedTimestamps( +static uint64_t computeReceiveTimestampsMinimumSpace( + const quic::WriteAckFrameMetaData& ackFrameMetaData) { + // Compute minimum size requirements for 3 fields that must be sent + // in every ACK_RECEIVE_TIMESTAMPS frame + const WriteAckFrameState& ackState = ackFrameMetaData.ackState; + uint64_t countTimestampRanges = 0; + uint64_t maybeLastPktNum = 0; + std::chrono::microseconds maybeLastPktTsDelta = 0us; + if (ackState.lastRecvdPacketInfo) { + maybeLastPktNum = ackState.lastRecvdPacketInfo.value().pktNum; + + maybeLastPktTsDelta = + (ackState.lastRecvdPacketInfo.value().timings.receiveTimePoint > + ackFrameMetaData.connTime + ? std::chrono::duration_cast( + ackState.lastRecvdPacketInfo.value() + .timings.receiveTimePoint - + ackFrameMetaData.connTime) + : 0us); + } + + return getQuicIntegerSize(countTimestampRanges).value_or(0) + + getQuicIntegerSize(maybeLastPktNum).value_or(0) + + getQuicIntegerSize(maybeLastPktTsDelta.count()).value_or(0); +} + +static void writeECNFieldsToAck( const quic::WriteAckFrameMetaData& ackFrameMetaData, + WriteAckFrame& ackFrame, + PacketBuilderInterface& builder) { + ackFrame.ecnECT0Count = ackFrameMetaData.ackState.ecnECT0CountReceived; + ackFrame.ecnECT1Count = ackFrameMetaData.ackState.ecnECT1CountReceived; + ackFrame.ecnCECount = ackFrameMetaData.ackState.ecnCECountReceived; + QuicInteger ecnECT0Count(ackFrameMetaData.ackState.ecnECT0CountReceived); + QuicInteger ecnECT1Count(ackFrameMetaData.ackState.ecnECT1CountReceived); + QuicInteger ecnCECount(ackFrameMetaData.ackState.ecnCECountReceived); + builder.write(ecnECT0Count); + builder.write(ecnECT1Count); + builder.write(ecnCECount); +} + +namespace { +struct AckReceiveTimesStampsWritten { + size_t TimestampRangesWritten{0}; + size_t TimestampWritten{0}; +}; +} // namespace + +AckReceiveTimesStampsWritten writeReceiveTimestampFieldsToAck( + const quic::WriteAckFrameMetaData& ackFrameMetaData, + WriteAckFrame& ackFrame, PacketBuilderInterface& builder, const AckReceiveTimestampsConfig& recvTimestampsConfig, uint64_t maxRecvTimestampsToSend) { - auto beginningSpace = builder.remainingSpaceInPkt(); - auto maybeAckFrame = writeAckFrameToPacketBuilder( - ackFrameMetaData, builder, FrameType::ACK_RECEIVE_TIMESTAMPS); - if (!maybeAckFrame.has_value()) { - return none; - } - auto ackFrame = maybeAckFrame.value(); const WriteAckFrameState& ackState = ackFrameMetaData.ackState; uint64_t spaceLeft = builder.remainingSpaceInPkt(); uint64_t lastPktNum = 0; @@ -544,7 +532,7 @@ Optional writeAckFrameWithReceivedTimestamps( auto largestAckedPacket = ackState.acks.back().end; uint64_t receiveTimestampsExponentToUse = recvTimestampsConfig.receiveTimestampsExponent; - countTimestampRanges = fillPacketReceiveTimestamps( + countTimestampRanges = fillFrameWithPacketReceiveTimestamps( ackFrameMetaData, ackFrame, largestAckedPacket, @@ -573,12 +561,72 @@ Optional writeAckFrameWithReceivedTimestamps( builder.write(timeStampRangeCountInt); } } + return AckReceiveTimesStampsWritten{countTimestampRanges, countTimestamps}; +} + +Optional writeAckFrame( + const quic::WriteAckFrameMetaData& ackFrameMetaData, + PacketBuilderInterface& builder, + FrameType frameType, + const AckReceiveTimestampsConfig& recvTimestampsConfig, + uint64_t maxRecvTimestampsToSend) { + if (ackFrameMetaData.ackState.acks.empty()) { + return none; + } + uint64_t beginningSpace = builder.remainingSpaceInPkt(); + uint64_t spaceLeft = beginningSpace; + + // Reserve space for ECN counts if enabled + if (frameType == FrameType::ACK_ECN) { + auto ecnRequiredSpace = computeEcnRequiredSpace(ackFrameMetaData); + if (spaceLeft < ecnRequiredSpace) { + return none; + } + spaceLeft -= ecnRequiredSpace; + } + + // Reserve space for receive timestamps if enabled + if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { + auto receiveTimestampsMinimumSpace = + computeReceiveTimestampsMinimumSpace(ackFrameMetaData); + if (spaceLeft < receiveTimestampsMinimumSpace) { + return none; + } + spaceLeft -= receiveTimestampsMinimumSpace; + } + + // Start writing fields to the builder + + // 1. Write the base ack fields (ACK packet type) + auto maybeAckFrame = + maybeWriteAckBaseFields(ackFrameMetaData, builder, frameType, spaceLeft); + if (!maybeAckFrame.has_value()) { + return none; + } + auto& ackFrame = maybeAckFrame.value(); + + // 2. Write ECN fields if enabled + if (frameType == FrameType::ACK_ECN) { + writeECNFieldsToAck(ackFrameMetaData, ackFrame, builder); + } + + // 3. Write receive timestamp fields if enabled + AckReceiveTimesStampsWritten receiveTimestampsWritten; + if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { + receiveTimestampsWritten = writeReceiveTimestampFieldsToAck( + ackFrameMetaData, + ackFrame, + builder, + recvTimestampsConfig, + maxRecvTimestampsToSend); + } + // Everything written auto ackFrameWriteResult = WriteAckFrameResult( beginningSpace - builder.remainingSpaceInPkt(), ackFrame, ackFrame.ackBlocks.size(), - countTimestampRanges, - countTimestamps); + receiveTimestampsWritten.TimestampRangesWritten, + receiveTimestampsWritten.TimestampWritten); builder.appendFrame(std::move(ackFrame)); return ackFrameWriteResult; diff --git a/quic/codec/QuicWriteCodec.h b/quic/codec/QuicWriteCodec.h index 291440e78..4cc89b999 100644 --- a/quic/codec/QuicWriteCodec.h +++ b/quic/codec/QuicWriteCodec.h @@ -100,24 +100,16 @@ Optional writeCryptoFrame( Optional writeAckFrame( const WriteAckFrameMetaData& ackFrameMetaData, PacketBuilderInterface& builder, - FrameType frameType = FrameType::ACK); + FrameType frameType = FrameType::ACK, + const AckReceiveTimestampsConfig& recvTimestampsConfig = + AckReceiveTimestampsConfig(), + uint64_t maxRecvTimestampsToSend = 0); /** * Helper functions to write the fields for ACK_RECEIVE_TIMESTAMPS frame */ size_t computeSizeUsedByRecvdTimestamps(quic::WriteAckFrame& writeAckFrame); -Optional writeAckFrameWithReceivedTimestamps( - const WriteAckFrameMetaData& ackFrameMetaData, - PacketBuilderInterface& builder, - const AckReceiveTimestampsConfig& recvTimestampsConfig, - uint64_t maxRecvTimestampsToSend); - -Optional writeAckFrameToPacketBuilder( - const WriteAckFrameMetaData& ackFrameMetaData, - quic::PacketBuilderInterface& builder, - quic::FrameType frameType); - } // namespace quic // namespace quic // namespace quic diff --git a/quic/codec/test/QuicWriteCodecTest.cpp b/quic/codec/test/QuicWriteCodecTest.cpp index a170f204e..6cde2ec97 100644 --- a/quic/codec/test/QuicWriteCodecTest.cpp +++ b/quic/codec/test/QuicWriteCodecTest.cpp @@ -912,13 +912,12 @@ TEST_P(QuicWriteCodecTest, AckFrameGapExceedsRepresentation) { }; EXPECT_THROW( - ((frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - 0)), + writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + 0), QuicTransportException); } @@ -973,14 +972,12 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { .ackDelayExponent = static_cast(kDefaultAckDelayExponent), .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1040,13 +1037,12 @@ TEST_P(QuicWriteCodecTest, AckFrameNotEnoughForAnything) { .connTime = connTime, }; - auto result = (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kDefaultAckDelayExponent); + auto result = writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); EXPECT_FALSE(result.has_value()); EXPECT_EQ(pktBuilder.remainingSpaceInPkt(), 4); } @@ -1072,14 +1068,12 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) { // There is 1 gap => each represented by 2 bytes => 2 bytes // 2 byte for first ack block length, then 2 bytes for the next len => 4 bytes // total 11 bytes - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); EXPECT_EQ(11 + addlBytesConsumed, ackFrameWriteResult.bytesWritten); @@ -1143,14 +1137,12 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWillSaveAckDelay) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildTestPacket(); auto regularPacket = builtOut.first; WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); @@ -1198,14 +1190,12 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { // There is 1 gap => each represented by 2 bytes => 2 bytes // 2 byte for first ack block length, then 2 bytes for the next len => 4 bytes // total 11 bytes - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); auto builtOut = std::move(pktBuilder).buildTestPacket(); @@ -1276,13 +1266,12 @@ TEST_P(QuicWriteCodecTest, WriteWithDifferentAckDelayExponent) { .connTime = connTime, }; - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildTestPacket(); auto wireBuf = std::move(builtOut.second); BufQueue queue; @@ -1314,13 +1303,12 @@ TEST_P(QuicWriteCodecTest, WriteExponentInLongHeaderPacket) { .connTime = connTime, }; - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto builtOut = std::move(pktBuilder).buildLongHeaderPacket(); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1361,14 +1349,12 @@ TEST_P(QuicWriteCodecTest, OnlyAckLargestPacket) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1454,14 +1440,12 @@ TEST_P(QuicWriteCodecTest, WriteSomeAckBlocks) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1523,14 +1507,12 @@ TEST_P(QuicWriteCodecTest, NoSpaceForAckBlockSection) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); EXPECT_FALSE(ackFrameWriteResult.has_value()); } @@ -1563,14 +1545,12 @@ TEST_P(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - kMaxReceivedPktsTimestampsStored); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + kMaxReceivedPktsTimestampsStored); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1633,11 +1613,12 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWithMultipleTimestampRanges) { .connTime = connTime, }; - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, pktBuilder, defaultAckReceiveTimestmpsConfig, 50); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + 50); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -1714,14 +1695,12 @@ TEST_P( if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) { pktBuilder.remaining_ = 80; } - auto ackFrameWriteResult = - (frameType == FrameType::ACK || frameType == FrameType::ACK_ECN) - ? *writeAckFrame(ackFrameMetaData, pktBuilder, frameType) - : *writeAckFrameWithReceivedTimestamps( - ackFrameMetaData, - pktBuilder, - defaultAckReceiveTimestmpsConfig, - 100); + auto ackFrameWriteResult = *writeAckFrame( + ackFrameMetaData, + pktBuilder, + frameType, + defaultAckReceiveTimestmpsConfig, + 100); auto addlBytesConsumed = computeBytesForOptionalAckFields( ackFrameMetaData, ackFrameWriteResult, frameType); @@ -2476,6 +2455,4 @@ INSTANTIATE_TEST_SUITE_P( FrameType::ACK, FrameType::ACK_ECN, FrameType::ACK_RECEIVE_TIMESTAMPS)); -// Values(FrameType::ACK, FrameType::ACK_ECN, -// FrameType::ACK_RECEIVE_TIMESTAMPS)); } // namespace quic::test