From 8e650ed585f49bf933f9afd3535d5863c4d45bc9 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Fri, 30 Aug 2024 12:18:20 -0700 Subject: [PATCH] Rename PacketEvent -> ClonedPacketIdentifier [take 2] Summary: This is my second attempt at D61871891. This time, I ran `xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py`, which generated quic/somerge_defs.bzl Reviewed By: kvtsoy Differential Revision: D61975459 fbshipit-source-id: bec62acb2b400f4a102574e8c882927f41b9330e --- quic/api/QuicPacketScheduler.cpp | 6 +- quic/api/QuicPacketScheduler.h | 18 ++-- quic/api/QuicTransportFunctions.cpp | 19 ++-- quic/api/QuicTransportFunctions.h | 2 +- quic/api/test/QuicPacketSchedulerTest.cpp | 54 ++++++----- quic/api/test/QuicTransportFunctionsTest.cpp | 50 +++++----- quic/api/test/QuicTransportTest.cpp | 4 +- quic/codec/QuicPacketRebuilder.cpp | 34 +++---- quic/codec/QuicPacketRebuilder.h | 12 ++- quic/codec/test/QuicPacketRebuilderTest.cpp | 4 +- quic/congestion_control/BUCK | 4 +- .../congestion_control/CongestionController.h | 2 +- quic/congestion_control/PacketProcessor.h | 2 +- quic/congestion_control/test/Bbr2Test.cpp | 2 +- quic/loss/QuicLossFunctions.cpp | 25 +++-- quic/loss/QuicLossFunctions.h | 18 ++-- quic/loss/test/QuicLossFunctionsTest.cpp | 92 ++++++++++--------- quic/somerge_defs.bzl | 6 +- quic/state/AckHandlers.cpp | 20 ++-- quic/state/BUCK | 10 +- quic/state/CMakeLists.txt | 2 +- ...etEvent.cpp => ClonedPacketIdentifier.cpp} | 16 ++-- quic/state/ClonedPacketIdentifier.h | 52 +++++++++++ quic/state/OutstandingPacket.h | 8 +- quic/state/PacketEvent.h | 47 ---------- quic/state/StateData.h | 11 ++- quic/state/test/AckHandlersTest.cpp | 23 +++-- quic/state/test/BUCK | 6 +- .../state/test/ClonedPacketIdentifierTest.cpp | 49 ++++++++++ quic/state/test/PacketEventTest.cpp | 36 -------- 30 files changed, 352 insertions(+), 282 deletions(-) rename quic/state/{PacketEvent.cpp => ClonedPacketIdentifier.cpp} (65%) create mode 100644 quic/state/ClonedPacketIdentifier.h delete mode 100644 quic/state/PacketEvent.h create mode 100644 quic/state/test/ClonedPacketIdentifierTest.cpp delete mode 100644 quic/state/test/PacketEventTest.cpp diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 3de0d8715..fd4098fb4 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -924,9 +924,9 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket( } // If the packet is already a clone that has been processed, we don't clone // it again. - if (outstandingPacket.associatedEvent && - conn_.outstandings.packetEvents.count( - *outstandingPacket.associatedEvent) == 0) { + if (outstandingPacket.maybeClonedPacketIdentifier && + conn_.outstandings.clonedPacketIdentifiers.count( + *outstandingPacket.maybeClonedPacketIdentifier) == 0) { continue; } // I think this only fail if udpSendPacketLen somehow shrinks in the middle diff --git a/quic/api/QuicPacketScheduler.h b/quic/api/QuicPacketScheduler.h index 4e4bd649e..b18314ad5 100644 --- a/quic/api/QuicPacketScheduler.h +++ b/quic/api/QuicPacketScheduler.h @@ -23,15 +23,15 @@ namespace quic { struct SchedulingResult { - Optional packetEvent; + Optional clonedPacketIdentifier; Optional packet; size_t shortHeaderPadding; explicit SchedulingResult( - Optional packetEventIn, + Optional clonedPacketIdentifierIn, Optional packetIn, size_t shortHeaderPaddingIn = 0) - : packetEvent(std::move(packetEventIn)), + : clonedPacketIdentifier(std::move(clonedPacketIdentifierIn)), packet(std::move(packetIn)), shortHeaderPadding(shortHeaderPaddingIn) {} }; @@ -46,10 +46,11 @@ class QuicPacketScheduler { /** * Schedules frames and writes them to the builder and returns - * a pair of PacketEvent and the Packet that was built. + * a pair of ClonedPacketIdentifier and the Packet that was built. * - * Returns an optional PacketEvent which indicates if the built out packet is - * a clone and the associated PacketEvent for both origin and clone. + * Returns an optional ClonedPacketIdentifier which indicates if the built out + * packet is a clone and the associated ClonedPacketIdentifier for both origin + * and clone. */ virtual SchedulingResult scheduleFramesForPacket( PacketBuilderInterface&& builder, @@ -358,8 +359,9 @@ class CloningScheduler : public QuicPacketScheduler { bool hasData() const override; /** - * Returns a optional PacketEvent which indicates if the built out packet is a - * clone and the associated PacketEvent for both origin and clone. + * Returns a optional ClonedPacketIdentifier which indicates if the built out + * packet is a clone and the associated ClonedPacketIdentifier for both origin + * and clone. */ SchedulingResult scheduleFramesForPacket( PacketBuilderInterface&& builder, diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 203302ecb..d39e33c9a 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -620,7 +620,7 @@ bool handleStreamBufMetaWritten( void updateConnection( QuicConnectionStateBase& conn, - Optional packetEvent, + Optional clonedPacketIdentifier, RegularQuicWritePacket packet, TimePoint sentTime, uint32_t encodedSize, @@ -728,7 +728,7 @@ void updateConnection( if (resetIter != conn.pendingEvents.resets.end()) { conn.pendingEvents.resets.erase(resetIter); } else { - DCHECK(packetEvent.has_value()) + DCHECK(clonedPacketIdentifier.has_value()) << " reset missing from pendingEvents for non-clone packet"; } break; @@ -786,7 +786,7 @@ void updateConnection( const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame(); retransmittable = true; // We don't want this triggered for cloned frames. - if (!packetEvent.has_value()) { + if (!clonedPacketIdentifier.has_value()) { updateSimpleFrameOnPacketSent(conn, simpleFrame); } break; @@ -852,7 +852,7 @@ void updateConnection( } if (!retransmittable && !isPing) { - DCHECK(!packetEvent); + DCHECK(!clonedPacketIdentifier); return; } conn.lossState.totalAckElicitingPacketsSent++; @@ -905,9 +905,10 @@ void updateConnection( conn.lossState.totalBytesSentAtLastAck, conn.lossState.totalBytesAckedAtLastAck); } - if (packetEvent) { - DCHECK(conn.outstandings.packetEvents.count(*packetEvent)); - pkt.associatedEvent = std::move(packetEvent); + if (clonedPacketIdentifier) { + DCHECK(conn.outstandings.clonedPacketIdentifiers.count( + *clonedPacketIdentifier)); + pkt.maybeClonedPacketIdentifier = std::move(clonedPacketIdentifier); conn.lossState.totalBytesCloned += encodedSize; } pkt.isDSRPacket = isDSRPacket; @@ -935,7 +936,7 @@ void updateConnection( conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize); } conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time; - if (pkt.associatedEvent) { + if (pkt.maybeClonedPacketIdentifier) { ++conn.outstandings.clonedPacketCount[packetNumberSpace]; ++conn.lossState.timeoutBasedRtxCount; } else { @@ -1599,7 +1600,7 @@ WriteQuicDataResult writeConnectionDataToSocket( auto& result = ret.result; updateConnection( connection, - std::move(result->packetEvent), + std::move(result->clonedPacketIdentifier), std::move(result->packet->packet), sentTime, folly::to(ret.encodedSize), diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h index 41634fb99..3b403e7ef 100644 --- a/quic/api/QuicTransportFunctions.h +++ b/quic/api/QuicTransportFunctions.h @@ -202,7 +202,7 @@ bool handleStreamBufMetaWritten( */ void updateConnection( QuicConnectionStateBase& conn, - Optional packetEvent, + Optional clonedPacketIdentifier, RegularQuicWritePacket packet, TimePoint time, uint32_t encodedSize, diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index 6d4d8dc75..97b24fe64 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -490,7 +490,7 @@ TEST_F(QuicPacketSchedulerTest, NoCloningForDSR) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_FALSE(result.packetEvent.hasValue()); + EXPECT_FALSE(result.clonedPacketIdentifier.hasValue()); EXPECT_FALSE(result.packet.hasValue()); } @@ -518,8 +518,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); - EXPECT_EQ(packetNum, result.packetEvent->packetNumber); + EXPECT_TRUE( + result.clonedPacketIdentifier.has_value() && result.packet.has_value()); + EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber); } TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { @@ -568,8 +569,9 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { auto result = cloningScheduler.scheduleFramesForPacket( std::move(regularBuilder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.hasValue() && result.packet.hasValue()); - EXPECT_EQ(packetNum, result.packetEvent->packetNumber); + EXPECT_TRUE( + result.clonedPacketIdentifier.hasValue() && result.packet.hasValue()); + EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber); // written packet should not have any frame in the builder auto& writtenPacket = *result.packet; auto shortHeader = writtenPacket.packet.header.asShort(); @@ -601,14 +603,15 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) { FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); // Add two outstanding packets, but then mark the second one processed by - // adding a PacketEvent that's missing from the outstandings.packetEvents set + // adding a ClonedPacketIdentifier that's missing from the + // outstandings.clonedPacketIdentifiers set PacketNum expected = addOutstandingPacket(conn); // There needs to have retransmittable frame for the rebuilder to work conn.outstandings.packets.back().packet.frames.push_back( MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); addOutstandingPacket(conn); - conn.outstandings.packets.back().associatedEvent = - PacketEvent(PacketNumberSpace::AppData, 1); + conn.outstandings.packets.back().maybeClonedPacketIdentifier = + ClonedPacketIdentifier(PacketNumberSpace::AppData, 1); // There needs to have retransmittable frame for the rebuilder to work conn.outstandings.packets.back().packet.frames.push_back( MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); @@ -623,8 +626,9 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) { conn.ackStates.initialAckState->largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); - EXPECT_EQ(expected, result.packetEvent->packetNumber); + EXPECT_TRUE( + result.clonedPacketIdentifier.has_value() && result.packet.has_value()); + EXPECT_EQ(expected, result.clonedPacketIdentifier->packetNumber); } TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeData) { @@ -702,7 +706,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeDataAndAcks) { // Clone the packet. auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value()); + EXPECT_TRUE(result.clonedPacketIdentifier.has_value()); EXPECT_TRUE(result.packet.has_value()); // Cloned packet has to have crypto data and no acks. @@ -773,8 +777,9 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); - EXPECT_EQ(expected, result.packetEvent->packetNumber); + EXPECT_TRUE( + result.clonedPacketIdentifier.has_value() && result.packet.has_value()); + EXPECT_EQ(expected, result.clonedPacketIdentifier->packetNumber); } TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { @@ -813,7 +818,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_EQ(none, result.packetEvent); + EXPECT_EQ(none, result.clonedPacketIdentifier); EXPECT_EQ(result.packet->packet.header.getHeaderForm(), HeaderForm::Short); ShortHeader& shortHeader = *result.packet->packet.header.asShort(); EXPECT_EQ(ProtectionType::KeyPhaseOne, shortHeader.getProtectionType()); @@ -839,7 +844,7 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { auto stream = conn.streamManager->createNextBidirectionalStream().value(); FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "GiantsShoulder", 0); - PacketEvent expectedPacketEvent( + ClonedPacketIdentifier expectedClonedPacketIdentifier( PacketNumberSpace::AppData, addOutstandingPacket(conn)); ASSERT_EQ(1, conn.outstandings.packets.size()); conn.outstandings.packets.back().packet.frames.push_back(MaxDataFrame(1000)); @@ -863,7 +868,8 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto packetResult = cloningScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - EXPECT_EQ(expectedPacketEvent, *packetResult.packetEvent); + EXPECT_EQ( + expectedClonedPacketIdentifier, *packetResult.clonedPacketIdentifier); int32_t verifyConnWindowUpdate = 1, verifyStreamWindowUpdate = 1; for (const auto& frame : packetResult.packet->packet.frames) { switch (frame.type()) { @@ -944,8 +950,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilder) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); - EXPECT_EQ(packetNum, result.packetEvent->packetNumber); + EXPECT_TRUE( + result.clonedPacketIdentifier.has_value() && result.packet.has_value()); + EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber); // Something was written into the buffer: EXPECT_TRUE(bufAccessor.ownsBuffer()); @@ -1023,8 +1030,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) { auto cloneResult = cloningScheduler.scheduleFramesForPacket( std::move(internalBuilder), conn.udpSendPacketLen); EXPECT_TRUE( - cloneResult.packetEvent.has_value() && cloneResult.packet.has_value()); - EXPECT_EQ(packetNum, cloneResult.packetEvent->packetNumber); + cloneResult.clonedPacketIdentifier.has_value() && + cloneResult.packet.has_value()); + EXPECT_EQ(packetNum, cloneResult.clonedPacketIdentifier->packetNumber); // Something was written into the buffer: EXPECT_TRUE(bufAccessor.ownsBuffer()); @@ -1089,7 +1097,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) { auto cloneResult = cloningScheduler.scheduleFramesForPacket( std::move(throwawayBuilder), kDefaultUDPSendPacketLen); EXPECT_FALSE(cloneResult.packet.hasValue()); - EXPECT_FALSE(cloneResult.packetEvent.hasValue()); + EXPECT_FALSE(cloneResult.clonedPacketIdentifier.hasValue()); } class AckSchedulingTest : public TestWithParam {}; @@ -1855,7 +1863,7 @@ TEST_F( conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_FALSE(result.packetEvent.has_value()); + EXPECT_FALSE(result.clonedPacketIdentifier.has_value()); // Nothing was written into the buffer: EXPECT_TRUE(bufAccessor.ownsBuffer()); @@ -1895,7 +1903,7 @@ TEST_F( conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto result = cloningScheduler.scheduleFramesForPacket( std::move(builder), kDefaultUDPSendPacketLen); - EXPECT_FALSE(result.packetEvent.has_value()); + EXPECT_FALSE(result.clonedPacketIdentifier.has_value()); // Nothing was written into the buffer: EXPECT_TRUE(bufAccessor.ownsBuffer()); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index d661a21ed..39c988fd0 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -1590,14 +1590,14 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) { MaxDataFrame maxDataFrame(maxDataAmt); conn->pendingEvents.connWindowUpdate = true; writePacket.frames.push_back(std::move(maxDataFrame)); - PacketEvent event(PacketNumberSpace::AppData, 1); - conn->outstandings.packetEvents.insert(event); + ClonedPacketIdentifier clonedPacketIdentifier(PacketNumberSpace::AppData, 1); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); auto futureMoment = thisMoment + 50ms; MockClock::mockNow = [=]() { return futureMoment; }; EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(1); updateConnection( *conn, - event, + clonedPacketIdentifier, std::move(writePacket), MockClock::now(), 1500, @@ -1633,9 +1633,9 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) { getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) ->metadata.encodedBodySize); EXPECT_EQ( - event, + clonedPacketIdentifier, *getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->associatedEvent); + ->maybeClonedPacketIdentifier); EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm); } @@ -3745,11 +3745,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterAppData) { MaxDataFrame(conn->flowControlState.advertisedMaxOffset); conn->pendingEvents.connWindowUpdate = true; packet.packet.frames.emplace_back(connWindowUpdate); - PacketEvent packetEvent(PacketNumberSpace::AppData, 100); - conn->outstandings.packetEvents.insert(packetEvent); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, 100); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), 123, @@ -3772,11 +3773,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterHandshake) { MaxDataFrame(conn->flowControlState.advertisedMaxOffset); conn->pendingEvents.connWindowUpdate = true; packet.packet.frames.emplace_back(connWindowUpdate); - PacketEvent packetEvent(PacketNumberSpace::AppData, 100); - conn->outstandings.packetEvents.insert(packetEvent); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, 100); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), 123, @@ -3799,11 +3801,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterInitial) { MaxDataFrame(conn->flowControlState.advertisedMaxOffset); conn->pendingEvents.connWindowUpdate = true; packet.packet.frames.emplace_back(connWindowUpdate); - PacketEvent packetEvent(PacketNumberSpace::AppData, 100); - conn->outstandings.packetEvents.insert(packetEvent); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, 100); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), 123, @@ -3839,18 +3842,18 @@ TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) { TEST_F(QuicTransportFunctionsTest, ClonedBlocked) { auto conn = createConn(); - PacketEvent packetEvent( + ClonedPacketIdentifier clonedPacketIdentifier( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto stream = conn->streamManager->createNextBidirectionalStream().value(); StreamDataBlockedFrame blockedFrame(stream->id, 1000); packet.packet.frames.emplace_back(blockedFrame); - conn->outstandings.packetEvents.insert(packetEvent); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); // This shall not crash updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), getEncodedSize(packet), @@ -3923,7 +3926,7 @@ TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) { TEST_F(QuicTransportFunctionsTest, ClonedRst) { auto conn = createConn(); - PacketEvent packetEvent( + ClonedPacketIdentifier clonedPacketIdentifier( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); auto stream = conn->streamManager->createNextBidirectionalStream().value(); @@ -3931,11 +3934,11 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) { RstStreamFrame rstStreamFrame( stream->id, GenericApplicationErrorCode::UNKNOWN, 0); packet.packet.frames.emplace_back(std::move(rstStreamFrame)); - conn->outstandings.packetEvents.insert(packetEvent); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); // This shall not crash updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), getEncodedSize(packet), @@ -3986,11 +3989,12 @@ TEST_F(QuicTransportFunctionsTest, TimeoutBasedRetxCountUpdate) { RstStreamFrame rstStreamFrame( stream->id, GenericApplicationErrorCode::UNKNOWN, 0); packet.packet.frames.push_back(rstStreamFrame); - PacketEvent packetEvent(PacketNumberSpace::AppData, 100); - conn->outstandings.packetEvents.insert(packetEvent); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, 100); + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); updateConnection( *conn, - packetEvent, + clonedPacketIdentifier, packet.packet, TimePoint(), 0, diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index d781e43a9..6f70a7c26 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -2497,7 +2497,9 @@ TEST_F(QuicTransportTest, CloneAfterRecvReset) { size_t cloneCounter = std::count_if( conn.outstandings.packets.begin(), conn.outstandings.packets.end(), - [](const auto& packet) { return packet.associatedEvent.hasValue(); }); + [](const auto& packet) { + return packet.maybeClonedPacketIdentifier.hasValue(); + }); EXPECT_LE(1, cloneCounter); } diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 826472c85..c18b5426d 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -23,27 +23,29 @@ uint64_t PacketRebuilder::getHeaderBytes() const { return builder_.getHeaderBytes(); } -PacketEvent PacketRebuilder::cloneOutstandingPacket( +ClonedPacketIdentifier PacketRebuilder::cloneOutstandingPacket( OutstandingPacketWrapper& packet) { - // Either the packet has never been cloned before, or it's associatedEvent is - // still in the outstandings.packetEvents set. + // Either the packet has never been cloned before, or it's + // maybeClonedPacketIdentifier is still in the + // outstandings.clonedPacketIdentifiers set. DCHECK( - !packet.associatedEvent || - conn_.outstandings.packetEvents.count(*packet.associatedEvent)); - if (!packet.associatedEvent) { + !packet.maybeClonedPacketIdentifier || + conn_.outstandings.clonedPacketIdentifiers.count( + *packet.maybeClonedPacketIdentifier)); + if (!packet.maybeClonedPacketIdentifier) { auto packetNum = packet.packet.header.getPacketSequenceNum(); auto packetNumberSpace = packet.packet.header.getPacketNumberSpace(); - PacketEvent event(packetNumberSpace, packetNum); - DCHECK(!conn_.outstandings.packetEvents.count(event)); - packet.associatedEvent = event; - conn_.outstandings.packetEvents.insert(event); + ClonedPacketIdentifier event(packetNumberSpace, packetNum); + DCHECK(!conn_.outstandings.clonedPacketIdentifiers.count(event)); + packet.maybeClonedPacketIdentifier = event; + conn_.outstandings.clonedPacketIdentifiers.insert(event); ++conn_.outstandings .clonedPacketCount[packet.packet.header.getPacketNumberSpace()]; } - return *packet.associatedEvent; + return *packet.maybeClonedPacketIdentifier; } -Optional PacketRebuilder::rebuildFromPacket( +Optional PacketRebuilder::rebuildFromPacket( OutstandingPacketWrapper& packet) { // TODO: if PMTU changes between the transmission of the original packet and // now, then we cannot clone everything in the packet. @@ -58,10 +60,10 @@ Optional PacketRebuilder::rebuildFromPacket( // First check if there's an ACK in this packet. We do this because we need // to know before we rebuild a stream frame whether there is an ACK in this // packet. If there is an ACK, we have to always encode the stream frame's - // length. This forces the associatedEvent code to reconsider the packet for - // ACK processing. We should always be able to write an ACK since the min - // ACK frame size is 4, while 1500 MTU stream frame lengths are going to be - // 2 bytes maximum. + // length. This forces the maybeClonedPacketIdentifier code to reconsider the + // packet for ACK processing. We should always be able to write an ACK since + // the min ACK frame size is 4, while 1500 MTU stream frame lengths are going + // to be 2 bytes maximum. bool hasAckFrame = false; for (const auto& frame : packet.packet.frames) { if (frame.asWriteAckFrame()) { diff --git a/quic/codec/QuicPacketRebuilder.h b/quic/codec/QuicPacketRebuilder.h index 603c014e7..8ff2960dd 100644 --- a/quic/codec/QuicPacketRebuilder.h +++ b/quic/codec/QuicPacketRebuilder.h @@ -27,7 +27,8 @@ class PacketRebuilder { PacketBuilderInterface& regularBuilder, QuicConnectionStateBase& conn); - Optional rebuildFromPacket(OutstandingPacketWrapper& packet); + Optional rebuildFromPacket( + OutstandingPacketWrapper& packet); // TODO: Same as passing cipherOverhead into the CloningScheduler, this really // is a sad way to solve the writableBytes problem. @@ -36,11 +37,12 @@ class PacketRebuilder { private: /** * A helper function that takes a OutstandingPacketWrapper that's not - * processed, and return its associatedEvent. If this packet has never been - * cloned, then create the associatedEvent and add it into - * outstandings.packetEvents first. + * processed, and return its maybeClonedPacketIdentifier. If this packet has + * never been cloned, then create the maybeClonedPacketIdentifier and add it + * into outstandings.clonedPacketIdentifiers first. */ - PacketEvent cloneOutstandingPacket(OutstandingPacketWrapper& packet); + ClonedPacketIdentifier cloneOutstandingPacket( + OutstandingPacketWrapper& packet); bool retransmittable(const QuicStreamState& stream) const { return stream.sendState == StreamSendState::Open; diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index c93dbefa8..ebcba1101 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -518,7 +518,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) { regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); rebuilder.rebuildFromPacket(outstandingPacket); - EXPECT_TRUE(outstandingPacket.associatedEvent.has_value()); + EXPECT_TRUE(outstandingPacket.maybeClonedPacketIdentifier.has_value()); EXPECT_EQ(1, conn.outstandings.numClonedPackets()); } @@ -542,7 +542,7 @@ TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) { regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); EXPECT_EQ(none, rebuilder.rebuildFromPacket(outstandingPacket)); - EXPECT_FALSE(outstandingPacket.associatedEvent.has_value()); + EXPECT_FALSE(outstandingPacket.maybeClonedPacketIdentifier.has_value()); EXPECT_EQ(0, conn.outstandings.numClonedPackets()); } diff --git a/quic/congestion_control/BUCK b/quic/congestion_control/BUCK index 0ec9e04eb..187a67e7c 100644 --- a/quic/congestion_control/BUCK +++ b/quic/congestion_control/BUCK @@ -23,8 +23,8 @@ mvfst_cpp_library( exported_deps = [ ":bandwidth", "//quic:constants", + "//quic/state:cloned_packet_identifier", "//quic/state:outstanding_packet", - "//quic/state:packet_event", ], ) @@ -313,8 +313,8 @@ mvfst_cpp_library( exported_deps = [ ":congestion_controller", "//folly/io:socket_option_map", + "//quic/state:cloned_packet_identifier", "//quic/state:outstanding_packet", - "//quic/state:packet_event", ], ) diff --git a/quic/congestion_control/CongestionController.h b/quic/congestion_control/CongestionController.h index cc7d8cb62..5724a0d29 100644 --- a/quic/congestion_control/CongestionController.h +++ b/quic/congestion_control/CongestionController.h @@ -9,8 +9,8 @@ #include #include +#include #include -#include #include namespace quic { diff --git a/quic/congestion_control/PacketProcessor.h b/quic/congestion_control/PacketProcessor.h index df02c8595..3547dfc90 100644 --- a/quic/congestion_control/PacketProcessor.h +++ b/quic/congestion_control/PacketProcessor.h @@ -9,8 +9,8 @@ #include #include +#include #include -#include namespace quic { diff --git a/quic/congestion_control/test/Bbr2Test.cpp b/quic/congestion_control/test/Bbr2Test.cpp index 690098b0d..b93d70a33 100644 --- a/quic/congestion_control/test/Bbr2Test.cpp +++ b/quic/congestion_control/test/Bbr2Test.cpp @@ -148,7 +148,7 @@ TEST_F(Bbr2Test, GracefullyHandleMissingFields) { auto packet = makeTestingWritePacket(0, 0, 0, testStart_); packet.lastAckedPacketInfo.clear(); - packet.associatedEvent.clear(); + packet.maybeClonedPacketIdentifier.clear(); EXPECT_NO_THROW(bbr2.onPacketSent(packet)); EXPECT_NO_THROW(bbr2.onPacketAckOrLoss(nullptr, nullptr)); diff --git a/quic/loss/QuicLossFunctions.cpp b/quic/loss/QuicLossFunctions.cpp index 7fa48c449..7947d35ac 100644 --- a/quic/loss/QuicLossFunctions.cpp +++ b/quic/loss/QuicLossFunctions.cpp @@ -356,18 +356,22 @@ bool processOutstandingsForLoss( CHECK_GT(conn.outstandings.dsrCount, 0); --conn.outstandings.dsrCount; } - if (pkt.associatedEvent) { + if (pkt.maybeClonedPacketIdentifier) { CHECK(conn.outstandings.clonedPacketCount[pnSpace]); --conn.outstandings.clonedPacketCount[pnSpace]; } - // Invoke LossVisitor if the packet doesn't have a associated PacketEvent; - // or if the PacketEvent is present in conn.outstandings.packetEvents. - bool processed = pkt.associatedEvent && - !conn.outstandings.packetEvents.count(*pkt.associatedEvent); + // Invoke LossVisitor if the packet doesn't have a associated + // ClonedPacketIdentifier; or if the ClonedPacketIdentifier is present in + // conn.outstandings.clonedPacketIdentifiers. + bool processed = pkt.maybeClonedPacketIdentifier && + !conn.outstandings.clonedPacketIdentifiers.count( + *pkt.maybeClonedPacketIdentifier); lossVisitor(conn, pkt.packet, processed); - // Remove the PacketEvent from the outstandings.packetEvents set - if (pkt.associatedEvent) { - conn.outstandings.packetEvents.erase(*pkt.associatedEvent); + // Remove the ClonedPacketIdentifier from the + // outstandings.clonedPacketIdentifiers set + if (pkt.maybeClonedPacketIdentifier) { + conn.outstandings.clonedPacketIdentifiers.erase( + *pkt.maybeClonedPacketIdentifier); } if (!processed) { CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]); @@ -508,8 +512,9 @@ Optional detectLossPackets( if (earliest->metadata.scheduledForDestruction) { earliest++; } - if (!earliest->associatedEvent || - conn.outstandings.packetEvents.count(*earliest->associatedEvent)) { + if (!earliest->maybeClonedPacketIdentifier || + conn.outstandings.clonedPacketIdentifiers.count( + *earliest->maybeClonedPacketIdentifier)) { break; } } diff --git a/quic/loss/QuicLossFunctions.h b/quic/loss/QuicLossFunctions.h index afda744ca..88e353c0a 100644 --- a/quic/loss/QuicLossFunctions.h +++ b/quic/loss/QuicLossFunctions.h @@ -129,7 +129,7 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) { * cwnd. So we must set the loss timer so that we can write this data with the * slack packet space for the clones. */ - if (!hasDataToWrite && conn.outstandings.packetEvents.empty() && + if (!hasDataToWrite && conn.outstandings.clonedPacketIdentifiers.empty() && totalPacketsOutstanding == conn.outstandings.numClonedPackets()) { VLOG(10) << __func__ << " unset alarm pure ack or processed packets only" << " outstanding=" << totalPacketsOutstanding @@ -172,7 +172,8 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) { << " haDataToWrite=" << hasDataToWrite << " outstanding=" << totalPacketsOutstanding << " outstanding clone=" << conn.outstandings.numClonedPackets() - << " packetEvents=" << conn.outstandings.packetEvents.size() + << " clonedPacketIdentifiers=" + << conn.outstandings.clonedPacketIdentifiers.size() << " initialPackets=" << conn.outstandings.packetCount[PacketNumberSpace::Initial] << " handshakePackets=" @@ -287,12 +288,15 @@ void markZeroRttPacketsLost( iter->packet.header.getProtectionType() == ProtectionType::ZeroRtt; if (isZeroRttPacket) { auto& pkt = *iter; - bool processed = pkt.associatedEvent && - !conn.outstandings.packetEvents.count(*pkt.associatedEvent); + bool processed = pkt.maybeClonedPacketIdentifier && + !conn.outstandings.clonedPacketIdentifiers.count( + *pkt.maybeClonedPacketIdentifier); lossVisitor(conn, pkt.packet, processed); - // Remove the PacketEvent from the outstandings.packetEvents set - if (pkt.associatedEvent) { - conn.outstandings.packetEvents.erase(*pkt.associatedEvent); + // Remove the ClonedPacketIdentifier from the + // outstandings.clonedPacketIdentifiers set + if (pkt.maybeClonedPacketIdentifier) { + conn.outstandings.clonedPacketIdentifiers.erase( + *pkt.maybeClonedPacketIdentifier); CHECK(conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]); --conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]; } diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 51bfb6dc6..435f16127 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -95,7 +95,7 @@ class QuicLossFunctionsTest : public TestWithParam { PacketNum sendPacket( QuicConnectionStateBase& conn, TimePoint time, - Optional associatedEvent, + Optional maybeClonedPacketIdentifier, PacketType packetType, Optional forcedSize = none, bool isDsr = false); @@ -189,7 +189,7 @@ auto testingLossMarkFunc(std::vector& lostPackets) { PacketNum QuicLossFunctionsTest::sendPacket( QuicConnectionStateBase& conn, TimePoint time, - Optional associatedEvent, + Optional maybeClonedPacketIdentifier, PacketType packetType, Optional forcedSize, bool isDsr) { @@ -265,28 +265,29 @@ PacketNum QuicLossFunctionsTest::sendPacket( LossState(), 0, OutstandingPacketMetadata::DetailsPerStream()); - outstandingPacket.associatedEvent = associatedEvent; + outstandingPacket.maybeClonedPacketIdentifier = maybeClonedPacketIdentifier; conn.lossState.lastRetransmittablePacketSentTime = time; if (conn.congestionController) { conn.congestionController->onPacketSent(outstandingPacket); } - if (associatedEvent) { + if (maybeClonedPacketIdentifier) { conn.outstandings.clonedPacketCount[packetNumberSpace]++; // Simulates what the real writer does. auto it = std::find_if( conn.outstandings.packets.begin(), conn.outstandings.packets.end(), - [&associatedEvent](const auto& packet) { + [&maybeClonedPacketIdentifier](const auto& packet) { auto packetNum = packet.packet.header.getPacketSequenceNum(); auto packetNumSpace = packet.packet.header.getPacketNumberSpace(); - return packetNum == associatedEvent->packetNumber && - packetNumSpace == associatedEvent->packetNumberSpace; + return packetNum == maybeClonedPacketIdentifier->packetNumber && + packetNumSpace == maybeClonedPacketIdentifier->packetNumberSpace; }); if (it != conn.outstandings.packets.end()) { - if (!it->associatedEvent) { - conn.outstandings.packetEvents.emplace(*associatedEvent); + if (!it->maybeClonedPacketIdentifier) { + conn.outstandings.clonedPacketIdentifiers.emplace( + *maybeClonedPacketIdentifier); conn.outstandings.clonedPacketCount[packetNumberSpace]++; - it->associatedEvent = *associatedEvent; + it->maybeClonedPacketIdentifier = *maybeClonedPacketIdentifier; } } } else { @@ -318,18 +319,18 @@ RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) { TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) { auto conn = createConn(); EXPECT_CALL(*quicStats_, onPTO()).Times(0); - PacketEvent packetEvent1( + ClonedPacketIdentifier clonedPacketIdentifier1( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); - sendPacket(*conn, Clock::now(), packetEvent1, PacketType::OneRtt); - PacketEvent packetEvent2( + sendPacket(*conn, Clock::now(), clonedPacketIdentifier1, PacketType::OneRtt); + ClonedPacketIdentifier clonedPacketIdentifier2( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); - sendPacket(*conn, Clock::now(), packetEvent2, PacketType::OneRtt); - PacketEvent packetEvent3( + sendPacket(*conn, Clock::now(), clonedPacketIdentifier2, PacketType::OneRtt); + ClonedPacketIdentifier clonedPacketIdentifier3( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); - sendPacket(*conn, Clock::now(), packetEvent3, PacketType::OneRtt); + sendPacket(*conn, Clock::now(), clonedPacketIdentifier3, PacketType::OneRtt); EXPECT_CALL(timeout, cancelLossTimeout()).Times(1); setLossDetectionAlarm(*conn, timeout); EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm); @@ -450,11 +451,13 @@ TEST_F(QuicLossFunctionsTest, TestOnPTOSkipProcessed) { conn->congestionController = std::move(mockCongestionController); EXPECT_CALL(*rawCongestionController, onPacketSent(_)) .WillRepeatedly(Return()); - // By adding an associatedEvent that doesn't exist in the - // outstandings.packetEvents, they are all processed and will skip lossVisitor + // By adding an maybeClonedPacketIdentifier that doesn't exist in the + // outstandings.clonedPacketIdentifiers, they are all processed and will skip + // lossVisitor for (auto i = 0; i < 10; i++) { - PacketEvent packetEvent(PacketNumberSpace::AppData, i); - sendPacket(*conn, TimePoint(), packetEvent, PacketType::OneRtt); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, i); + sendPacket(*conn, TimePoint(), clonedPacketIdentifier, PacketType::OneRtt); } EXPECT_EQ(10, conn->outstandings.packets.size()); std::vector lostPackets; @@ -1428,8 +1431,9 @@ TEST_F(QuicLossFunctionsTest, SkipLossVisitor) { PacketNum lastSent; for (size_t i = 0; i < 5; i++) { lastSent = conn->ackStates.appDataAckState.nextPacketNum; - PacketEvent packetEvent(PacketNumberSpace::AppData, lastSent); - sendPacket(*conn, Clock::now(), packetEvent, PacketType::OneRtt); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, lastSent); + sendPacket(*conn, Clock::now(), clonedPacketIdentifier, PacketType::OneRtt); } auto& ackState = getAckState(*conn, PacketNumberSpace::AppData); @@ -1462,14 +1466,17 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) { PacketNum lastSent; lastSent = sendPacket(*conn, Clock::now(), none, PacketType::OneRtt); EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::AppData]); - PacketEvent event(PacketNumberSpace::AppData, lastSent); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::AppData, lastSent); for (size_t i = 0; i < 6; i++) { - lastSent = sendPacket(*conn, Clock::now(), event, PacketType::OneRtt); + lastSent = sendPacket( + *conn, Clock::now(), clonedPacketIdentifier, PacketType::OneRtt); } EXPECT_EQ(7, conn->outstandings.packets.size()); EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::AppData]); - // Add the PacketEvent to the outstandings.packetEvents set - conn->outstandings.packetEvents.insert(event); + // Add the ClonedPacketIdentifier to the outstandings.clonedPacketIdentifiers + // set + conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); // Ack the last sent packet. Despite three losses, lossVisitor only visit one // packet @@ -1493,10 +1500,10 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) { TEST_F(QuicLossFunctionsTest, DetectPacketLossClonedPacketsCounter) { auto conn = createConn(); - PacketEvent packetEvent1( + ClonedPacketIdentifier clonedPacketIdentifier1( PacketNumberSpace::AppData, conn->ackStates.appDataAckState.nextPacketNum); - sendPacket(*conn, Clock::now(), packetEvent1, PacketType::OneRtt); + sendPacket(*conn, Clock::now(), clonedPacketIdentifier1, PacketType::OneRtt); sendPacket(*conn, Clock::now(), none, PacketType::OneRtt); sendPacket(*conn, Clock::now(), none, PacketType::OneRtt); sendPacket(*conn, Clock::now(), none, PacketType::OneRtt); @@ -1520,7 +1527,7 @@ TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) { MockAsyncUDPSocket socket(qEvb); auto conn = createConn(); ASSERT_TRUE(conn->outstandings.packets.empty()); - ASSERT_TRUE(conn->outstandings.packetEvents.empty()); + ASSERT_TRUE(conn->outstandings.clonedPacketIdentifiers.empty()); auto stream1Id = conn->streamManager->createNextBidirectionalStream().value()->id; auto buf = folly::IOBuf::copyBuffer("I wrestled by the sea."); @@ -1543,7 +1550,7 @@ TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) { EXPECT_FALSE(conn->streamManager->pendingWindowUpdate(stream2->id)); EXPECT_FALSE(conn->pendingEvents.connWindowUpdate); ASSERT_EQ(1, conn->outstandings.packets.size()); - ASSERT_TRUE(conn->outstandings.packetEvents.empty()); + ASSERT_TRUE(conn->outstandings.clonedPacketIdentifiers.empty()); uint32_t streamDataCounter = 0, streamWindowUpdateCounter = 0, connWindowUpdateCounter = 0; auto strippedPacket = stripPaddingFrames( @@ -1638,8 +1645,9 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejected) { conn->congestionController = std::move(mockCongestionController); EXPECT_CALL(*rawCongestionController, onPacketSent(_)) .WillRepeatedly(Return()); - // By adding an associatedEvent that doesn't exist in the - // outstandings.packetEvents, they are all processed and will skip lossVisitor + // By adding an maybeClonedPacketIdentifier that doesn't exist in the + // outstandings.clonedPacketIdentifiers, they are all processed and will skip + // lossVisitor for (auto i = 0; i < 2; i++) { sendPacket(*conn, TimePoint(), none, PacketType::OneRtt); sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt); @@ -1673,27 +1681,29 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) { conn->congestionController = std::move(mockCongestionController); EXPECT_CALL(*rawCongestionController, onPacketSent(_)) .WillRepeatedly(Return()); - // By adding an associatedEvent that doesn't exist in the - // outstandings.packetEvents, they are all processed and will skip lossVisitor + // By adding an maybeClonedPacketIdentifier that doesn't exist in the + // outstandings.clonedPacketIdentifiers, they are all processed and will skip + // lossVisitor std::set zeroRttPackets; - Optional lastPacketEvent; + Optional lastClonedPacketIdentifier; for (auto i = 0; i < 2; i++) { - auto packetNum = - sendPacket(*conn, TimePoint(), lastPacketEvent, PacketType::ZeroRtt); - lastPacketEvent = PacketEvent(PacketNumberSpace::AppData, packetNum); + auto packetNum = sendPacket( + *conn, TimePoint(), lastClonedPacketIdentifier, PacketType::ZeroRtt); + lastClonedPacketIdentifier = + ClonedPacketIdentifier(PacketNumberSpace::AppData, packetNum); zeroRttPackets.emplace(packetNum); } zeroRttPackets.emplace( sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt)); for (auto zeroRttPacketNum : zeroRttPackets) { - PacketEvent zeroRttPacketEvent( + ClonedPacketIdentifier zeroRttPacketEvent( PacketNumberSpace::AppData, zeroRttPacketNum); sendPacket(*conn, TimePoint(), zeroRttPacketEvent, PacketType::OneRtt); } EXPECT_EQ(6, conn->outstandings.packets.size()); ASSERT_EQ(conn->outstandings.numClonedPackets(), 6); - ASSERT_EQ(conn->outstandings.packetEvents.size(), 2); + ASSERT_EQ(conn->outstandings.clonedPacketIdentifiers.size(), 2); ASSERT_EQ(2, conn->outstandings.packetCount[PacketNumberSpace::AppData]); std::vector lostPackets; @@ -1702,7 +1712,7 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) { markZeroRttPacketsLost(*conn, [&lostPackets](auto&, auto&, bool processed) { lostPackets.emplace_back(processed); }); - ASSERT_EQ(conn->outstandings.packetEvents.size(), 0); + ASSERT_EQ(conn->outstandings.clonedPacketIdentifiers.size(), 0); EXPECT_EQ(3, conn->outstandings.packets.size()); EXPECT_EQ(lostPackets.size(), 3); ASSERT_EQ(conn->outstandings.numClonedPackets(), 3); diff --git a/quic/somerge_defs.bzl b/quic/somerge_defs.bzl index e85430504..107618843 100644 --- a/quic/somerge_defs.bzl +++ b/quic/somerge_defs.bzl @@ -2,7 +2,7 @@ Generated by xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py -@generated SignedSource<<390b7a822f95d1254eb82e48a7d607e2>> +@generated SignedSource<<1eb8d098bfda4c6700775f29fd0044b1>> """ # Entry Points: @@ -172,6 +172,8 @@ QUIC_NATIVE_LIBRARY_MERGE_MAP = [ "fbsource//xplat/quic/state:ack_handlerAndroidAndroid", "fbsource//xplat/quic/state:ack_statesAndroid", "fbsource//xplat/quic/state:ack_statesAndroidAndroid", + "fbsource//xplat/quic/state:cloned_packet_identifierAndroid", + "fbsource//xplat/quic/state:cloned_packet_identifierAndroidAndroid", "fbsource//xplat/quic/state:datagram_handlerAndroid", "fbsource//xplat/quic/state:datagram_handlerAndroidAndroid", "fbsource//xplat/quic/state:loss_stateAndroid", @@ -180,8 +182,6 @@ QUIC_NATIVE_LIBRARY_MERGE_MAP = [ "fbsource//xplat/quic/state:outstanding_packetAndroidAndroid", "fbsource//xplat/quic/state:pacing_functionsAndroid", "fbsource//xplat/quic/state:pacing_functionsAndroidAndroid", - "fbsource//xplat/quic/state:packet_eventAndroid", - "fbsource//xplat/quic/state:packet_eventAndroidAndroid", "fbsource//xplat/quic/state:quic_connection_statsAndroid", "fbsource//xplat/quic/state:quic_connection_statsAndroidAndroid", "fbsource//xplat/quic/state:quic_priority_queueAndroid", diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index dc5a727f4..0d8ca0239 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -240,14 +240,15 @@ AckEvent processAckFrame( rPacketIt++; continue; } - bool needsProcess = !rPacketIt->associatedEvent || - conn.outstandings.packetEvents.count(*rPacketIt->associatedEvent); + bool needsProcess = !rPacketIt->maybeClonedPacketIdentifier || + conn.outstandings.clonedPacketIdentifiers.count( + *rPacketIt->maybeClonedPacketIdentifier); if (needsProcess) { CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]); --conn.outstandings.packetCount[currentPacketNumberSpace]; } ack.ackedBytes += rPacketIt->metadata.encodedSize; - if (rPacketIt->associatedEvent) { + if (rPacketIt->maybeClonedPacketIdentifier) { CHECK(conn.outstandings.clonedPacketCount[currentPacketNumberSpace]); --conn.outstandings.clonedPacketCount[currentPacketNumberSpace]; } @@ -307,9 +308,11 @@ AckEvent processAckFrame( } // if (rttSample != rttSample.zero()) } // if (!ack.implicit && currentPacketNum == frame.largestAcked) - // Remove this PacketEvent from the outstandings.packetEvents set - if (rPacketIt->associatedEvent) { - conn.outstandings.packetEvents.erase(*rPacketIt->associatedEvent); + // Remove this ClonedPacketIdentifier from the + // outstandings.clonedPacketIdentifiers set + if (rPacketIt->maybeClonedPacketIdentifier) { + conn.outstandings.clonedPacketIdentifiers.erase( + *rPacketIt->maybeClonedPacketIdentifier); } if (!ack.largestNewlyAckedPacket || *ack.largestNewlyAckedPacket < currentPacketNum) { @@ -361,8 +364,9 @@ AckEvent processAckFrame( } // Invoke AckVisitor for WriteAckFrames all the time. Invoke it for other - // frame types only if the packet doesn't have an associated PacketEvent; - // or the PacketEvent is in conn.outstandings.packetEvents + // frame types only if the packet doesn't have an associated + // ClonedPacketIdentifier; or the ClonedPacketIdentifier is in + // conn.outstandings.clonedPacketIdentifiers ack.ackedPackets.reserve(packetsWithHandlerContext.size()); for (auto packetWithHandlerContextItr = packetsWithHandlerContext.rbegin(); packetWithHandlerContextItr != packetsWithHandlerContext.rend(); diff --git a/quic/state/BUCK b/quic/state/BUCK index 8382de7ee..22a18e727 100644 --- a/quic/state/BUCK +++ b/quic/state/BUCK @@ -8,8 +8,8 @@ mvfst_cpp_library( "OutstandingPacket.h", ], exported_deps = [ + ":cloned_packet_identifier", ":loss_state", - ":packet_event", "//folly/io:socket_option_map", "//quic/codec:types", ], @@ -52,12 +52,12 @@ mvfst_cpp_library( ) mvfst_cpp_library( - name = "packet_event", + name = "cloned_packet_identifier", srcs = [ - "PacketEvent.cpp", + "ClonedPacketIdentifier.cpp", ], headers = [ - "PacketEvent.h", + "ClonedPacketIdentifier.h", ], deps = [ "//folly/hash:hash", @@ -112,9 +112,9 @@ mvfst_cpp_library( exported_deps = [ ":ack_event", ":ack_states", + ":cloned_packet_identifier", ":loss_state", ":outstanding_packet", - ":packet_event", ":quic_connection_stats", ":quic_priority_queue", ":retransmission_policy", diff --git a/quic/state/CMakeLists.txt b/quic/state/CMakeLists.txt index e3b0d5d9d..0916546fe 100644 --- a/quic/state/CMakeLists.txt +++ b/quic/state/CMakeLists.txt @@ -9,7 +9,7 @@ add_library( QuicStreamManager.cpp QuicStreamUtilities.cpp StateData.cpp - PacketEvent.cpp + ClonedPacketIdentifier.cpp PendingPathRateLimiter.cpp QuicPriorityQueue.cpp ) diff --git a/quic/state/PacketEvent.cpp b/quic/state/ClonedPacketIdentifier.cpp similarity index 65% rename from quic/state/PacketEvent.cpp rename to quic/state/ClonedPacketIdentifier.cpp index 904cbd3fd..785e496fe 100644 --- a/quic/state/PacketEvent.cpp +++ b/quic/state/ClonedPacketIdentifier.cpp @@ -5,19 +5,21 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include #include #include namespace quic { -PacketEvent::PacketEvent( +ClonedPacketIdentifier::ClonedPacketIdentifier( PacketNumberSpace packetNumberSpaceIn, PacketNum packetNumberIn) : packetNumber(packetNumberIn), packetNumberSpace(packetNumberSpaceIn) {} -bool operator==(const PacketEvent& lhs, const PacketEvent& rhs) { +bool operator==( + const ClonedPacketIdentifier& lhs, + const ClonedPacketIdentifier& rhs) { return static_cast>( lhs.packetNumberSpace) == static_cast>( @@ -25,11 +27,11 @@ bool operator==(const PacketEvent& lhs, const PacketEvent& rhs) { lhs.packetNumber == rhs.packetNumber; } -size_t PacketEventHash::operator()( - const PacketEvent& packetEvent) const noexcept { +size_t ClonedPacketIdentifierHash::operator()( + const ClonedPacketIdentifier& clonedPacketIdentifier) const noexcept { return folly::hash::hash_combine( static_cast>( - packetEvent.packetNumberSpace), - packetEvent.packetNumber); + clonedPacketIdentifier.packetNumberSpace), + clonedPacketIdentifier.packetNumber); } } // namespace quic diff --git a/quic/state/ClonedPacketIdentifier.h b/quic/state/ClonedPacketIdentifier.h new file mode 100644 index 000000000..2bae625fb --- /dev/null +++ b/quic/state/ClonedPacketIdentifier.h @@ -0,0 +1,52 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include + +namespace quic { + +/** + * There are cases that we may clone an outstanding packet and resend it as is. + * When that happens, we assign a ClonedPacketIdentifier to both the original + * and cloned packet if no ClonedPacketIdentifier is already associated with the + * original packet. If the original packet already has a ClonedPacketIdentifier, + * we copy that value into the cloned packet. A connection maintains a set of + * ClonedPacketIdentifiers. When a packet with a ClonedPacketIdentifier is acked + * or lost, we search the set. If the ClonedPacketIdentifier is present in the + * set, we process the ack or loss event (e.g. update RTT, notify + * CongestionController, and detect loss with this packet) as well as frames in + * the packet. Then we remove the ClonedPacketIdentifier from the set. If the + * ClonedPacketIdentifier is absent in the set, we consider all frames contained + * in the packet are already processed. We will still handle the ack or loss + * event and update the connection. But no frame will be processed. + * + * TODO: Current PacketNum is an alias to uint64_t. We should just make + * PacketNum be a type with both the space and the number, then + * ClonedPacketIdentifier will just be an alias to this type. + */ +struct ClonedPacketIdentifier { + PacketNum packetNumber : 62; + PacketNumberSpace packetNumberSpace : 2; + + ClonedPacketIdentifier() = delete; + ClonedPacketIdentifier( + PacketNumberSpace packetNumberSpaceIn, + PacketNum packetNumberIn); +}; + +// To work with F14 Set: +bool operator==( + const ClonedPacketIdentifier& lhs, + const ClonedPacketIdentifier& rhs); + +struct ClonedPacketIdentifierHash { + size_t operator()( + const ClonedPacketIdentifier& clonedPacketIdentifier) const noexcept; +}; +} // namespace quic diff --git a/quic/state/OutstandingPacket.h b/quic/state/OutstandingPacket.h index 2f66ab269..cedd8d748 100644 --- a/quic/state/OutstandingPacket.h +++ b/quic/state/OutstandingPacket.h @@ -9,8 +9,8 @@ #include #include +#include #include -#include #include namespace quic { @@ -174,9 +174,9 @@ struct OutstandingPacket { }; Optional lastAckedPacketInfo; - // PacketEvent associated with this OutstandingPacketWrapper. This will be a - // none if the packet isn't a clone and hasn't been cloned. - Optional associatedEvent; + // ClonedPacketIdentifier associated with this OutstandingPacketWrapper. This + // will be a none if the packet isn't a clone and hasn't been cloned. + Optional maybeClonedPacketIdentifier; OptionalIntegral nonDsrPacketSequenceNumber; diff --git a/quic/state/PacketEvent.h b/quic/state/PacketEvent.h deleted file mode 100644 index d5422447a..000000000 --- a/quic/state/PacketEvent.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace quic { - -/** - * There are cases that we may clone an outstanding packet and resend it as is. - * When that happens, we assign a PacketEvent to both the original and cloned - * packet if no PacketEvent is already associated with the original packet. If - * the original packet already has a PacketEvent, we copy that value into the - * cloned packet. - * A connection maintains a set of PacketEvents. When a packet with a - * PacketEvent is acked or lost, we search the set. If the PacketEvent is - * present in the set, we process the ack or loss event (e.g. update RTT, notify - * CongestionController, and detect loss with this packet) as well as frames in - * the packet. Then we remove the PacketEvent from the set. If the PacketEvent - * is absent in the set, we consider all frames contained in the packet are - * already processed. We will still handle the ack or loss event and update the - * connection. But no frame will be processed. - * - * TODO: Current PacketNum is an alias to uint64_t. We should just make - * PacketNum be a type with both the space and the number, then PacketEvent will - * just be an alias to this type. - */ -struct PacketEvent { - PacketNum packetNumber : 62; - PacketNumberSpace packetNumberSpace : 2; - - PacketEvent() = delete; - PacketEvent(PacketNumberSpace packetNumberSpaceIn, PacketNum packetNumberIn); -}; - -// To work with F14 Set: -bool operator==(const PacketEvent& lhs, const PacketEvent& rhs); - -struct PacketEventHash { - size_t operator()(const PacketEvent& packetEvent) const noexcept; -}; -} // namespace quic diff --git a/quic/state/StateData.h b/quic/state/StateData.h index 720b93344..c60ffe16c 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -22,9 +22,9 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -49,9 +49,10 @@ struct OutstandingsInfo { std::deque packets; // All PacketEvents of this connection. If a OutstandingPacketWrapper doesn't - // have an associatedEvent or if it's not in this set, there is no need to - // process its frames upon ack or loss. - folly::F14FastSet packetEvents; + // have an maybeClonedPacketIdentifier or if it's not in this set, there is no + // need to process its frames upon ack or loss. + folly::F14FastSet + clonedPacketIdentifiers; // Number of outstanding packets not including cloned EnumArray packetCount{}; @@ -86,7 +87,7 @@ struct OutstandingsInfo { void reset() { packets.clear(); - packetEvents.clear(); + clonedPacketIdentifiers.clear(); packetCount = {}; clonedPacketCount = {}; declaredLostCount = 0; diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index 59da225f6..9688afcbb 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -1879,9 +1879,9 @@ TEST_P(AckHandlersTest, SkipAckVisitor) { LossState(), 0, OutstandingPacketMetadata::DetailsPerStream()); - // Give this outstandingPacket an associatedEvent that's not in - // outstandings.packetEvents - outstandingPacket.associatedEvent.emplace(GetParam().pnSpace, 0); + // Give this outstandingPacket an maybeClonedPacketIdentifier that's not in + // outstandings.clonedPacketIdentifiers + outstandingPacket.maybeClonedPacketIdentifier.emplace(GetParam().pnSpace, 0); conn.outstandings.packets.push_back(std::move(outstandingPacket)); conn.outstandings.clonedPacketCount[GetParam().pnSpace]++; @@ -2000,7 +2000,8 @@ TEST_P(AckHandlersTest, NoDoubleProcess) { LossState(), 0, OutstandingPacketMetadata::DetailsPerStream()); - outstandingPacket1.associatedEvent.emplace(GetParam().pnSpace, packetNum1); + outstandingPacket1.maybeClonedPacketIdentifier.emplace( + GetParam().pnSpace, packetNum1); OutstandingPacketWrapper outstandingPacket2( std::move(regularPacket2), @@ -2012,14 +2013,16 @@ TEST_P(AckHandlersTest, NoDoubleProcess) { LossState(), 0, OutstandingPacketMetadata::DetailsPerStream()); - // The seconds packet has the same PacketEvent - outstandingPacket2.associatedEvent.emplace(GetParam().pnSpace, packetNum1); + // The seconds packet has the same ClonedPacketIdentifier + outstandingPacket2.maybeClonedPacketIdentifier.emplace( + GetParam().pnSpace, packetNum1); conn.outstandings.packetCount[GetParam().pnSpace]++; conn.outstandings.packets.push_back(std::move(outstandingPacket1)); conn.outstandings.packets.push_back(std::move(outstandingPacket2)); conn.outstandings.clonedPacketCount[GetParam().pnSpace] += 2; - conn.outstandings.packetEvents.emplace(GetParam().pnSpace, packetNum1); + conn.outstandings.clonedPacketIdentifiers.emplace( + GetParam().pnSpace, packetNum1); // A counting ack visitor uint16_t ackVisitorCounter = 0; @@ -2077,7 +2080,8 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) { LossState(), 0, OutstandingPacketMetadata::DetailsPerStream()); - outstandingPacket1.associatedEvent.emplace(GetParam().pnSpace, packetNum1); + outstandingPacket1.maybeClonedPacketIdentifier.emplace( + GetParam().pnSpace, packetNum1); conn.ackStates.appDataAckState.nextPacketNum++; auto packetNum2 = conn.ackStates.appDataAckState.nextPacketNum; @@ -2101,7 +2105,8 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) { conn.outstandings.packets.push_back(std::move(outstandingPacket1)); conn.outstandings.packets.push_back(std::move(outstandingPacket2)); conn.outstandings.clonedPacketCount[GetParam().pnSpace] = 1; - conn.outstandings.packetEvents.emplace(GetParam().pnSpace, packetNum1); + conn.outstandings.clonedPacketIdentifiers.emplace( + GetParam().pnSpace, packetNum1); ReadAckFrame ackFrame; ackFrame.largestAcked = packetNum2; diff --git a/quic/state/test/BUCK b/quic/state/test/BUCK index 6c5ef2ea2..172661104 100644 --- a/quic/state/test/BUCK +++ b/quic/state/test/BUCK @@ -134,13 +134,13 @@ cpp_unittest( ) cpp_unittest( - name = "PacketEventTest", + name = "ClonedPacketIdentifierTest", srcs = [ - "PacketEventTest.cpp", + "ClonedPacketIdentifierTest.cpp", ], deps = [ "//folly/portability:gtest", - "//quic/state:packet_event", + "//quic/state:cloned_packet_identifier", ], ) diff --git a/quic/state/test/ClonedPacketIdentifierTest.cpp b/quic/state/test/ClonedPacketIdentifierTest.cpp new file mode 100644 index 000000000..438aac07b --- /dev/null +++ b/quic/state/test/ClonedPacketIdentifierTest.cpp @@ -0,0 +1,49 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include + +namespace quic { +namespace test { +TEST(ClonedPacketIdentifierTest, EqTest) { + ClonedPacketIdentifier initialClonedPacketIdentifier( + PacketNumberSpace::Initial, 0); + ClonedPacketIdentifier initialClonedPacketIdentifier0( + PacketNumberSpace::Initial, 0); + EXPECT_TRUE(initialClonedPacketIdentifier == initialClonedPacketIdentifier0); + + ClonedPacketIdentifier initialClonedPacketIdentifier1( + PacketNumberSpace::Initial, 1); + EXPECT_FALSE( + initialClonedPacketIdentifier0 == initialClonedPacketIdentifier1); + + ClonedPacketIdentifier handshakeClonedPacketIdentifier( + PacketNumberSpace::Handshake, 0); + EXPECT_FALSE( + handshakeClonedPacketIdentifier == initialClonedPacketIdentifier); +} + +TEST(ClonedPacketIdentifierTest, HashTest) { + ClonedPacketIdentifierHash hashObj; + ClonedPacketIdentifier initialClonedPacketIdentifier0( + PacketNumberSpace::Initial, 0); + ClonedPacketIdentifier initialClonedPacketIdentifier1( + PacketNumberSpace::Initial, 1); + EXPECT_NE( + hashObj(initialClonedPacketIdentifier0), + hashObj(initialClonedPacketIdentifier1)); + + ClonedPacketIdentifier handshakeClonedPacketIdentifier0( + PacketNumberSpace::Handshake, 0); + EXPECT_NE( + hashObj(initialClonedPacketIdentifier0), + hashObj(handshakeClonedPacketIdentifier0)); +} +} // namespace test +} // namespace quic diff --git a/quic/state/test/PacketEventTest.cpp b/quic/state/test/PacketEventTest.cpp deleted file mode 100644 index 4c543d3bb..000000000 --- a/quic/state/test/PacketEventTest.cpp +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include - -#include - -namespace quic { -namespace test { -TEST(PacketEventTest, EqTest) { - PacketEvent initialEvent(PacketNumberSpace::Initial, 0); - PacketEvent initialEvent0(PacketNumberSpace::Initial, 0); - EXPECT_TRUE(initialEvent == initialEvent0); - - PacketEvent initialEvent1(PacketNumberSpace::Initial, 1); - EXPECT_FALSE(initialEvent0 == initialEvent1); - - PacketEvent handshakeEvent(PacketNumberSpace::Handshake, 0); - EXPECT_FALSE(handshakeEvent == initialEvent); -} - -TEST(PacketEventTest, HashTest) { - PacketEventHash hashObj; - PacketEvent initialEvent0(PacketNumberSpace::Initial, 0); - PacketEvent initialEvent1(PacketNumberSpace::Initial, 1); - EXPECT_NE(hashObj(initialEvent0), hashObj(initialEvent1)); - - PacketEvent handshakeEvent0(PacketNumberSpace::Handshake, 0); - EXPECT_NE(hashObj(initialEvent0), hashObj(handshakeEvent0)); -} -} // namespace test -} // namespace quic