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