lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-01 01:44:22 +03:00
Code Activity

Rename PacketEvent -> ClonedPacketIdentifier [take 2]

Browse Source 
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
This commit is contained in:
Aman Sharma
2024-08-30 12:18:20 -07:00
committed by Facebook GitHub Bot
parent 0d7a0e9287
commit 8e650ed585
30 changed files with 352 additions and 282 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
quic/api/QuicPacketScheduler.cpp
View File

@ -924,9 +924,9 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket(
} }
// If the packet is already a clone that has been processed, we don't clone // If the packet is already a clone that has been processed, we don't clone
// it again. // it again.
if (outstandingPacket.associatedEvent && if (outstandingPacket.maybeClonedPacketIdentifier &&
conn_.outstandings.packetEvents.count( conn_.outstandings.clonedPacketIdentifiers.count(
*outstandingPacket.associatedEvent) == 0) { *outstandingPacket.maybeClonedPacketIdentifier) == 0) {
continue; continue;
} }
// I think this only fail if udpSendPacketLen somehow shrinks in the middle // I think this only fail if udpSendPacketLen somehow shrinks in the middle

18
quic/api/QuicPacketScheduler.h
View File

@ -23,15 +23,15 @@
namespace quic { namespace quic {
struct SchedulingResult { struct SchedulingResult {
Optional<PacketEvent> packetEvent; Optional<ClonedPacketIdentifier> clonedPacketIdentifier;
Optional<PacketBuilderInterface::Packet> packet; Optional<PacketBuilderInterface::Packet> packet;
size_t shortHeaderPadding; size_t shortHeaderPadding;
explicit SchedulingResult( explicit SchedulingResult(
Optional<PacketEvent> packetEventIn, Optional<ClonedPacketIdentifier> clonedPacketIdentifierIn,
Optional<PacketBuilderInterface::Packet> packetIn, Optional<PacketBuilderInterface::Packet> packetIn,
size_t shortHeaderPaddingIn = 0) size_t shortHeaderPaddingIn = 0)
: packetEvent(std::move(packetEventIn)), : clonedPacketIdentifier(std::move(clonedPacketIdentifierIn)),
packet(std::move(packetIn)), packet(std::move(packetIn)),
shortHeaderPadding(shortHeaderPaddingIn) {} shortHeaderPadding(shortHeaderPaddingIn) {}
}; };
@ -46,10 +46,11 @@ class QuicPacketScheduler {
/** /**
* Schedules frames and writes them to the builder and returns * 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 * Returns an optional ClonedPacketIdentifier which indicates if the built out
* a clone and the associated PacketEvent for both origin and clone. * packet is a clone and the associated ClonedPacketIdentifier for both origin
* and clone.
*/ */
virtual SchedulingResult scheduleFramesForPacket( virtual SchedulingResult scheduleFramesForPacket(
PacketBuilderInterface&& builder, PacketBuilderInterface&& builder,
@ -358,8 +359,9 @@ class CloningScheduler : public QuicPacketScheduler {
bool hasData() const override; bool hasData() const override;
/** /**
* Returns a optional PacketEvent which indicates if the built out packet is a * Returns a optional ClonedPacketIdentifier which indicates if the built out
* clone and the associated PacketEvent for both origin and clone. * packet is a clone and the associated ClonedPacketIdentifier for both origin
* and clone.
*/ */
SchedulingResult scheduleFramesForPacket( SchedulingResult scheduleFramesForPacket(
PacketBuilderInterface&& builder, PacketBuilderInterface&& builder,

19
quic/api/QuicTransportFunctions.cpp
View File

@ -620,7 +620,7 @@ bool handleStreamBufMetaWritten(
void updateConnection( void updateConnection(
QuicConnectionStateBase& conn, QuicConnectionStateBase& conn,
Optional<PacketEvent> packetEvent, Optional<ClonedPacketIdentifier> clonedPacketIdentifier,
RegularQuicWritePacket packet, RegularQuicWritePacket packet,
TimePoint sentTime, TimePoint sentTime,
uint32_t encodedSize, uint32_t encodedSize,
@ -728,7 +728,7 @@ void updateConnection(
if (resetIter != conn.pendingEvents.resets.end()) { if (resetIter != conn.pendingEvents.resets.end()) {
conn.pendingEvents.resets.erase(resetIter); conn.pendingEvents.resets.erase(resetIter);
} else { } else {
DCHECK(packetEvent.has_value()) DCHECK(clonedPacketIdentifier.has_value())
<< " reset missing from pendingEvents for non-clone packet"; << " reset missing from pendingEvents for non-clone packet";
} }
break; break;
@ -786,7 +786,7 @@ void updateConnection(
const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame(); const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame();
retransmittable = true; retransmittable = true;
// We don't want this triggered for cloned frames. // We don't want this triggered for cloned frames.
if (!packetEvent.has_value()) { if (!clonedPacketIdentifier.has_value()) {
updateSimpleFrameOnPacketSent(conn, simpleFrame); updateSimpleFrameOnPacketSent(conn, simpleFrame);
} }
break; break;
@ -852,7 +852,7 @@ void updateConnection(
} }
if (!retransmittable && !isPing) { if (!retransmittable && !isPing) {
DCHECK(!packetEvent); DCHECK(!clonedPacketIdentifier);
return; return;
} }
conn.lossState.totalAckElicitingPacketsSent++; conn.lossState.totalAckElicitingPacketsSent++;
@ -905,9 +905,10 @@ void updateConnection(
conn.lossState.totalBytesSentAtLastAck, conn.lossState.totalBytesSentAtLastAck,
conn.lossState.totalBytesAckedAtLastAck); conn.lossState.totalBytesAckedAtLastAck);
} }
if (packetEvent) { if (clonedPacketIdentifier) {
DCHECK(conn.outstandings.packetEvents.count(*packetEvent)); DCHECK(conn.outstandings.clonedPacketIdentifiers.count(
pkt.associatedEvent = std::move(packetEvent); *clonedPacketIdentifier));
pkt.maybeClonedPacketIdentifier = std::move(clonedPacketIdentifier);
conn.lossState.totalBytesCloned += encodedSize; conn.lossState.totalBytesCloned += encodedSize;
} }
pkt.isDSRPacket = isDSRPacket; pkt.isDSRPacket = isDSRPacket;
@ -935,7 +936,7 @@ void updateConnection(
conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize); conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize);
} }
conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time; conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time;
if (pkt.associatedEvent) { if (pkt.maybeClonedPacketIdentifier) {
++conn.outstandings.clonedPacketCount[packetNumberSpace]; ++conn.outstandings.clonedPacketCount[packetNumberSpace];
++conn.lossState.timeoutBasedRtxCount; ++conn.lossState.timeoutBasedRtxCount;
} else { } else {
@ -1599,7 +1600,7 @@ WriteQuicDataResult writeConnectionDataToSocket(
auto& result = ret.result; auto& result = ret.result;
updateConnection( updateConnection(
connection, connection,
std::move(result->packetEvent), std::move(result->clonedPacketIdentifier),
std::move(result->packet->packet), std::move(result->packet->packet),
sentTime, sentTime,
folly::to<uint32_t>(ret.encodedSize), folly::to<uint32_t>(ret.encodedSize),

2
quic/api/QuicTransportFunctions.h
View File

@ -202,7 +202,7 @@ bool handleStreamBufMetaWritten(
*/ */
void updateConnection( void updateConnection(
QuicConnectionStateBase& conn, QuicConnectionStateBase& conn,
Optional<PacketEvent> packetEvent, Optional<ClonedPacketIdentifier> clonedPacketIdentifier,
RegularQuicWritePacket packet, RegularQuicWritePacket packet,
TimePoint time, TimePoint time,
uint32_t encodedSize, uint32_t encodedSize,

54
quic/api/test/QuicPacketSchedulerTest.cpp
View File

@ -490,7 +490,7 @@ TEST_F(QuicPacketSchedulerTest, NoCloningForDSR) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_FALSE(result.packetEvent.hasValue()); EXPECT_FALSE(result.clonedPacketIdentifier.hasValue());
EXPECT_FALSE(result.packet.hasValue()); EXPECT_FALSE(result.packet.hasValue());
} }
@ -518,8 +518,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); EXPECT_TRUE(
EXPECT_EQ(packetNum, result.packetEvent->packetNumber); result.clonedPacketIdentifier.has_value() && result.packet.has_value());
EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber);
} }
TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
@ -568,8 +569,9 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(regularBuilder), kDefaultUDPSendPacketLen); std::move(regularBuilder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.hasValue() && result.packet.hasValue()); EXPECT_TRUE(
EXPECT_EQ(packetNum, result.packetEvent->packetNumber); result.clonedPacketIdentifier.hasValue() && result.packet.hasValue());
EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber);
// written packet should not have any frame in the builder // written packet should not have any frame in the builder
auto& writtenPacket = *result.packet; auto& writtenPacket = *result.packet;
auto shortHeader = writtenPacket.packet.header.asShort(); auto shortHeader = writtenPacket.packet.header.asShort();
@ -601,14 +603,15 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
FrameScheduler noopScheduler("frame", conn); FrameScheduler noopScheduler("frame", conn);
CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
// Add two outstanding packets, but then mark the second one processed by // 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); PacketNum expected = addOutstandingPacket(conn);
// There needs to have retransmittable frame for the rebuilder to work // There needs to have retransmittable frame for the rebuilder to work
conn.outstandings.packets.back().packet.frames.push_back( conn.outstandings.packets.back().packet.frames.push_back(
MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); MaxDataFrame(conn.flowControlState.advertisedMaxOffset));
addOutstandingPacket(conn); addOutstandingPacket(conn);
conn.outstandings.packets.back().associatedEvent = conn.outstandings.packets.back().maybeClonedPacketIdentifier =
PacketEvent(PacketNumberSpace::AppData, 1); ClonedPacketIdentifier(PacketNumberSpace::AppData, 1);
// There needs to have retransmittable frame for the rebuilder to work // There needs to have retransmittable frame for the rebuilder to work
conn.outstandings.packets.back().packet.frames.push_back( conn.outstandings.packets.back().packet.frames.push_back(
MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); MaxDataFrame(conn.flowControlState.advertisedMaxOffset));
@ -623,8 +626,9 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
conn.ackStates.initialAckState->largestAckedByPeer.value_or(0)); conn.ackStates.initialAckState->largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); EXPECT_TRUE(
EXPECT_EQ(expected, result.packetEvent->packetNumber); result.clonedPacketIdentifier.has_value() && result.packet.has_value());
EXPECT_EQ(expected, result.clonedPacketIdentifier->packetNumber);
} }
TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeData) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeData) {
@ -702,7 +706,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeDataAndAcks) {
// Clone the packet. // Clone the packet.
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.has_value()); EXPECT_TRUE(result.clonedPacketIdentifier.has_value());
EXPECT_TRUE(result.packet.has_value()); EXPECT_TRUE(result.packet.has_value());
// Cloned packet has to have crypto data and no acks. // 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)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); EXPECT_TRUE(
EXPECT_EQ(expected, result.packetEvent->packetNumber); result.clonedPacketIdentifier.has_value() && result.packet.has_value());
EXPECT_EQ(expected, result.clonedPacketIdentifier->packetNumber);
} }
TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
@ -813,7 +818,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_EQ(none, result.packetEvent); EXPECT_EQ(none, result.clonedPacketIdentifier);
EXPECT_EQ(result.packet->packet.header.getHeaderForm(), HeaderForm::Short); EXPECT_EQ(result.packet->packet.header.getHeaderForm(), HeaderForm::Short);
ShortHeader& shortHeader = *result.packet->packet.header.asShort(); ShortHeader& shortHeader = *result.packet->packet.header.asShort();
EXPECT_EQ(ProtectionType::KeyPhaseOne, shortHeader.getProtectionType()); EXPECT_EQ(ProtectionType::KeyPhaseOne, shortHeader.getProtectionType());
@ -839,7 +844,7 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
auto stream = conn.streamManager->createNextBidirectionalStream().value(); auto stream = conn.streamManager->createNextBidirectionalStream().value();
FrameScheduler noopScheduler("frame", conn); FrameScheduler noopScheduler("frame", conn);
CloningScheduler cloningScheduler(noopScheduler, conn, "GiantsShoulder", 0); CloningScheduler cloningScheduler(noopScheduler, conn, "GiantsShoulder", 0);
PacketEvent expectedPacketEvent( ClonedPacketIdentifier expectedClonedPacketIdentifier(
PacketNumberSpace::AppData, addOutstandingPacket(conn)); PacketNumberSpace::AppData, addOutstandingPacket(conn));
ASSERT_EQ(1, conn.outstandings.packets.size()); ASSERT_EQ(1, conn.outstandings.packets.size());
conn.outstandings.packets.back().packet.frames.push_back(MaxDataFrame(1000)); 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)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto packetResult = cloningScheduler.scheduleFramesForPacket( auto packetResult = cloningScheduler.scheduleFramesForPacket(
std::move(builder), conn.udpSendPacketLen); std::move(builder), conn.udpSendPacketLen);
EXPECT_EQ(expectedPacketEvent, *packetResult.packetEvent); EXPECT_EQ(
expectedClonedPacketIdentifier, *packetResult.clonedPacketIdentifier);
int32_t verifyConnWindowUpdate = 1, verifyStreamWindowUpdate = 1; int32_t verifyConnWindowUpdate = 1, verifyStreamWindowUpdate = 1;
for (const auto& frame : packetResult.packet->packet.frames) { for (const auto& frame : packetResult.packet->packet.frames) {
switch (frame.type()) { switch (frame.type()) {
@ -944,8 +950,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilder) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); EXPECT_TRUE(
EXPECT_EQ(packetNum, result.packetEvent->packetNumber); result.clonedPacketIdentifier.has_value() && result.packet.has_value());
EXPECT_EQ(packetNum, result.clonedPacketIdentifier->packetNumber);
// Something was written into the buffer: // Something was written into the buffer:
EXPECT_TRUE(bufAccessor.ownsBuffer()); EXPECT_TRUE(bufAccessor.ownsBuffer());
@ -1023,8 +1030,9 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) {
auto cloneResult = cloningScheduler.scheduleFramesForPacket( auto cloneResult = cloningScheduler.scheduleFramesForPacket(
std::move(internalBuilder), conn.udpSendPacketLen); std::move(internalBuilder), conn.udpSendPacketLen);
EXPECT_TRUE( EXPECT_TRUE(
cloneResult.packetEvent.has_value() && cloneResult.packet.has_value()); cloneResult.clonedPacketIdentifier.has_value() &&
EXPECT_EQ(packetNum, cloneResult.packetEvent->packetNumber); cloneResult.packet.has_value());
EXPECT_EQ(packetNum, cloneResult.clonedPacketIdentifier->packetNumber);
// Something was written into the buffer: // Something was written into the buffer:
EXPECT_TRUE(bufAccessor.ownsBuffer()); EXPECT_TRUE(bufAccessor.ownsBuffer());
@ -1089,7 +1097,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) {
auto cloneResult = cloningScheduler.scheduleFramesForPacket( auto cloneResult = cloningScheduler.scheduleFramesForPacket(
std::move(throwawayBuilder), kDefaultUDPSendPacketLen); std::move(throwawayBuilder), kDefaultUDPSendPacketLen);
EXPECT_FALSE(cloneResult.packet.hasValue()); EXPECT_FALSE(cloneResult.packet.hasValue());
EXPECT_FALSE(cloneResult.packetEvent.hasValue()); EXPECT_FALSE(cloneResult.clonedPacketIdentifier.hasValue());
} }
class AckSchedulingTest : public TestWithParam<PacketNumberSpace> {}; class AckSchedulingTest : public TestWithParam<PacketNumberSpace> {};
@ -1855,7 +1863,7 @@ TEST_F(
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_FALSE(result.packetEvent.has_value()); EXPECT_FALSE(result.clonedPacketIdentifier.has_value());
// Nothing was written into the buffer: // Nothing was written into the buffer:
EXPECT_TRUE(bufAccessor.ownsBuffer()); EXPECT_TRUE(bufAccessor.ownsBuffer());
@ -1895,7 +1903,7 @@ TEST_F(
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto result = cloningScheduler.scheduleFramesForPacket( auto result = cloningScheduler.scheduleFramesForPacket(
std::move(builder), kDefaultUDPSendPacketLen); std::move(builder), kDefaultUDPSendPacketLen);
EXPECT_FALSE(result.packetEvent.has_value()); EXPECT_FALSE(result.clonedPacketIdentifier.has_value());
// Nothing was written into the buffer: // Nothing was written into the buffer:
EXPECT_TRUE(bufAccessor.ownsBuffer()); EXPECT_TRUE(bufAccessor.ownsBuffer());

50
quic/api/test/QuicTransportFunctionsTest.cpp
View File

@ -1590,14 +1590,14 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) {
MaxDataFrame maxDataFrame(maxDataAmt); MaxDataFrame maxDataFrame(maxDataAmt);
conn->pendingEvents.connWindowUpdate = true; conn->pendingEvents.connWindowUpdate = true;
writePacket.frames.push_back(std::move(maxDataFrame)); writePacket.frames.push_back(std::move(maxDataFrame));
PacketEvent event(PacketNumberSpace::AppData, 1); ClonedPacketIdentifier clonedPacketIdentifier(PacketNumberSpace::AppData, 1);
conn->outstandings.packetEvents.insert(event); conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
auto futureMoment = thisMoment + 50ms; auto futureMoment = thisMoment + 50ms;
MockClock::mockNow = [=]() { return futureMoment; }; MockClock::mockNow = [=]() { return futureMoment; };
EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(1); EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(1);
updateConnection( updateConnection(
*conn, *conn,
event, clonedPacketIdentifier,
std::move(writePacket), std::move(writePacket),
MockClock::now(), MockClock::now(),
1500, 1500,
@ -1633,9 +1633,9 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) {
getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)
->metadata.encodedBodySize); ->metadata.encodedBodySize);
EXPECT_EQ( EXPECT_EQ(
event, clonedPacketIdentifier,
*getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) *getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)
->associatedEvent); ->maybeClonedPacketIdentifier);
EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm); EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm);
} }
@ -3745,11 +3745,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterAppData) {
MaxDataFrame(conn->flowControlState.advertisedMaxOffset); MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
conn->pendingEvents.connWindowUpdate = true; conn->pendingEvents.connWindowUpdate = true;
packet.packet.frames.emplace_back(connWindowUpdate); packet.packet.frames.emplace_back(connWindowUpdate);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100); ClonedPacketIdentifier clonedPacketIdentifier(
conn->outstandings.packetEvents.insert(packetEvent); PacketNumberSpace::AppData, 100);
conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
123, 123,
@ -3772,11 +3773,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterHandshake) {
MaxDataFrame(conn->flowControlState.advertisedMaxOffset); MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
conn->pendingEvents.connWindowUpdate = true; conn->pendingEvents.connWindowUpdate = true;
packet.packet.frames.emplace_back(connWindowUpdate); packet.packet.frames.emplace_back(connWindowUpdate);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100); ClonedPacketIdentifier clonedPacketIdentifier(
conn->outstandings.packetEvents.insert(packetEvent); PacketNumberSpace::AppData, 100);
conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
123, 123,
@ -3799,11 +3801,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterInitial) {
MaxDataFrame(conn->flowControlState.advertisedMaxOffset); MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
conn->pendingEvents.connWindowUpdate = true; conn->pendingEvents.connWindowUpdate = true;
packet.packet.frames.emplace_back(connWindowUpdate); packet.packet.frames.emplace_back(connWindowUpdate);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100); ClonedPacketIdentifier clonedPacketIdentifier(
conn->outstandings.packetEvents.insert(packetEvent); PacketNumberSpace::AppData, 100);
conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
123, 123,
@ -3839,18 +3842,18 @@ TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) {
TEST_F(QuicTransportFunctionsTest, ClonedBlocked) { TEST_F(QuicTransportFunctionsTest, ClonedBlocked) {
auto conn = createConn(); auto conn = createConn();
PacketEvent packetEvent( ClonedPacketIdentifier clonedPacketIdentifier(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); conn->ackStates.appDataAckState.nextPacketNum);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
StreamDataBlockedFrame blockedFrame(stream->id, 1000); StreamDataBlockedFrame blockedFrame(stream->id, 1000);
packet.packet.frames.emplace_back(blockedFrame); packet.packet.frames.emplace_back(blockedFrame);
conn->outstandings.packetEvents.insert(packetEvent); conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
// This shall not crash // This shall not crash
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
@ -3923,7 +3926,7 @@ TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) {
TEST_F(QuicTransportFunctionsTest, ClonedRst) { TEST_F(QuicTransportFunctionsTest, ClonedRst) {
auto conn = createConn(); auto conn = createConn();
PacketEvent packetEvent( ClonedPacketIdentifier clonedPacketIdentifier(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); conn->ackStates.appDataAckState.nextPacketNum);
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
@ -3931,11 +3934,11 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) {
RstStreamFrame rstStreamFrame( RstStreamFrame rstStreamFrame(
stream->id, GenericApplicationErrorCode::UNKNOWN, 0); stream->id, GenericApplicationErrorCode::UNKNOWN, 0);
packet.packet.frames.emplace_back(std::move(rstStreamFrame)); packet.packet.frames.emplace_back(std::move(rstStreamFrame));
conn->outstandings.packetEvents.insert(packetEvent); conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
// This shall not crash // This shall not crash
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
@ -3986,11 +3989,12 @@ TEST_F(QuicTransportFunctionsTest, TimeoutBasedRetxCountUpdate) {
RstStreamFrame rstStreamFrame( RstStreamFrame rstStreamFrame(
stream->id, GenericApplicationErrorCode::UNKNOWN, 0); stream->id, GenericApplicationErrorCode::UNKNOWN, 0);
packet.packet.frames.push_back(rstStreamFrame); packet.packet.frames.push_back(rstStreamFrame);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100); ClonedPacketIdentifier clonedPacketIdentifier(
conn->outstandings.packetEvents.insert(packetEvent); PacketNumberSpace::AppData, 100);
conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
updateConnection( updateConnection(
*conn, *conn,
packetEvent, clonedPacketIdentifier,
packet.packet, packet.packet,
TimePoint(), TimePoint(),
0, 0,

4
quic/api/test/QuicTransportTest.cpp
View File

@ -2497,7 +2497,9 @@ TEST_F(QuicTransportTest, CloneAfterRecvReset) {
size_t cloneCounter = std::count_if( size_t cloneCounter = std::count_if(
conn.outstandings.packets.begin(), conn.outstandings.packets.begin(),
conn.outstandings.packets.end(), conn.outstandings.packets.end(),
[](const auto& packet) { return packet.associatedEvent.hasValue(); }); [](const auto& packet) {
return packet.maybeClonedPacketIdentifier.hasValue();
});
EXPECT_LE(1, cloneCounter); EXPECT_LE(1, cloneCounter);
} }

34
quic/codec/QuicPacketRebuilder.cpp
View File

@ -23,27 +23,29 @@ uint64_t PacketRebuilder::getHeaderBytes() const {
return builder_.getHeaderBytes(); return builder_.getHeaderBytes();
} }
PacketEvent PacketRebuilder::cloneOutstandingPacket( ClonedPacketIdentifier PacketRebuilder::cloneOutstandingPacket(
OutstandingPacketWrapper& packet) { OutstandingPacketWrapper& packet) {
// Either the packet has never been cloned before, or it's associatedEvent is // Either the packet has never been cloned before, or it's
// still in the outstandings.packetEvents set. // maybeClonedPacketIdentifier is still in the
// outstandings.clonedPacketIdentifiers set.
DCHECK( DCHECK(
!packet.associatedEvent || !packet.maybeClonedPacketIdentifier ||
conn_.outstandings.packetEvents.count(*packet.associatedEvent)); conn_.outstandings.clonedPacketIdentifiers.count(
if (!packet.associatedEvent) { *packet.maybeClonedPacketIdentifier));
if (!packet.maybeClonedPacketIdentifier) {
auto packetNum = packet.packet.header.getPacketSequenceNum(); auto packetNum = packet.packet.header.getPacketSequenceNum();
auto packetNumberSpace = packet.packet.header.getPacketNumberSpace(); auto packetNumberSpace = packet.packet.header.getPacketNumberSpace();
PacketEvent event(packetNumberSpace, packetNum); ClonedPacketIdentifier event(packetNumberSpace, packetNum);
DCHECK(!conn_.outstandings.packetEvents.count(event)); DCHECK(!conn_.outstandings.clonedPacketIdentifiers.count(event));
packet.associatedEvent = event; packet.maybeClonedPacketIdentifier = event;
conn_.outstandings.packetEvents.insert(event); conn_.outstandings.clonedPacketIdentifiers.insert(event);
++conn_.outstandings ++conn_.outstandings
.clonedPacketCount[packet.packet.header.getPacketNumberSpace()]; .clonedPacketCount[packet.packet.header.getPacketNumberSpace()];
} }
return *packet.associatedEvent; return *packet.maybeClonedPacketIdentifier;
} }
Optional<PacketEvent> PacketRebuilder::rebuildFromPacket( Optional<ClonedPacketIdentifier> PacketRebuilder::rebuildFromPacket(
OutstandingPacketWrapper& packet) { OutstandingPacketWrapper& packet) {
// TODO: if PMTU changes between the transmission of the original packet and // TODO: if PMTU changes between the transmission of the original packet and
// now, then we cannot clone everything in the packet. // now, then we cannot clone everything in the packet.
@ -58,10 +60,10 @@ Optional<PacketEvent> PacketRebuilder::rebuildFromPacket(
// First check if there's an ACK in this packet. We do this because we need // 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 // 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 // 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 // length. This forces the maybeClonedPacketIdentifier code to reconsider the
// ACK processing. We should always be able to write an ACK since the min // packet for ACK processing. We should always be able to write an ACK since
// ACK frame size is 4, while 1500 MTU stream frame lengths are going to be // the min ACK frame size is 4, while 1500 MTU stream frame lengths are going
// 2 bytes maximum. // to be 2 bytes maximum.
bool hasAckFrame = false; bool hasAckFrame = false;
for (const auto& frame : packet.packet.frames) { for (const auto& frame : packet.packet.frames) {
if (frame.asWriteAckFrame()) { if (frame.asWriteAckFrame()) {

12
quic/codec/QuicPacketRebuilder.h
View File

@ -27,7 +27,8 @@ class PacketRebuilder {
PacketBuilderInterface& regularBuilder, PacketBuilderInterface& regularBuilder,
QuicConnectionStateBase& conn); QuicConnectionStateBase& conn);
Optional<PacketEvent> rebuildFromPacket(OutstandingPacketWrapper& packet); Optional<ClonedPacketIdentifier> rebuildFromPacket(
OutstandingPacketWrapper& packet);
// TODO: Same as passing cipherOverhead into the CloningScheduler, this really // TODO: Same as passing cipherOverhead into the CloningScheduler, this really
// is a sad way to solve the writableBytes problem. // is a sad way to solve the writableBytes problem.
@ -36,11 +37,12 @@ class PacketRebuilder {
private: private:
/** /**
* A helper function that takes a OutstandingPacketWrapper that's not * A helper function that takes a OutstandingPacketWrapper that's not
* processed, and return its associatedEvent. If this packet has never been * processed, and return its maybeClonedPacketIdentifier. If this packet has
* cloned, then create the associatedEvent and add it into * never been cloned, then create the maybeClonedPacketIdentifier and add it
* outstandings.packetEvents first. * into outstandings.clonedPacketIdentifiers first.
*/ */
PacketEvent cloneOutstandingPacket(OutstandingPacketWrapper& packet); ClonedPacketIdentifier cloneOutstandingPacket(
OutstandingPacketWrapper& packet);
bool retransmittable(const QuicStreamState& stream) const { bool retransmittable(const QuicStreamState& stream) const {
return stream.sendState == StreamSendState::Open; return stream.sendState == StreamSendState::Open;

4
quic/codec/test/QuicPacketRebuilderTest.cpp
View File

@ -518,7 +518,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) {
regularBuilder2.encodePacketHeader(); regularBuilder2.encodePacketHeader();
PacketRebuilder rebuilder(regularBuilder2, conn); PacketRebuilder rebuilder(regularBuilder2, conn);
rebuilder.rebuildFromPacket(outstandingPacket); rebuilder.rebuildFromPacket(outstandingPacket);
EXPECT_TRUE(outstandingPacket.associatedEvent.has_value()); EXPECT_TRUE(outstandingPacket.maybeClonedPacketIdentifier.has_value());
EXPECT_EQ(1, conn.outstandings.numClonedPackets()); EXPECT_EQ(1, conn.outstandings.numClonedPackets());
} }
@ -542,7 +542,7 @@ TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) {
regularBuilder2.encodePacketHeader(); regularBuilder2.encodePacketHeader();
PacketRebuilder rebuilder(regularBuilder2, conn); PacketRebuilder rebuilder(regularBuilder2, conn);
EXPECT_EQ(none, rebuilder.rebuildFromPacket(outstandingPacket)); EXPECT_EQ(none, rebuilder.rebuildFromPacket(outstandingPacket));
EXPECT_FALSE(outstandingPacket.associatedEvent.has_value()); EXPECT_FALSE(outstandingPacket.maybeClonedPacketIdentifier.has_value());
EXPECT_EQ(0, conn.outstandings.numClonedPackets()); EXPECT_EQ(0, conn.outstandings.numClonedPackets());
} }

4
quic/congestion_control/BUCK
View File

@ -23,8 +23,8 @@ mvfst_cpp_library(
exported_deps = [ exported_deps = [
":bandwidth", ":bandwidth",
"//quic:constants", "//quic:constants",
"//quic/state:cloned_packet_identifier",
"//quic/state:outstanding_packet", "//quic/state:outstanding_packet",
"//quic/state:packet_event",
], ],
) )
@ -313,8 +313,8 @@ mvfst_cpp_library(
exported_deps = [ exported_deps = [
":congestion_controller", ":congestion_controller",
"//folly/io:socket_option_map", "//folly/io:socket_option_map",
"//quic/state:cloned_packet_identifier",
"//quic/state:outstanding_packet", "//quic/state:outstanding_packet",
"//quic/state:packet_event",
], ],
) )

2
quic/congestion_control/CongestionController.h
View File

@ -9,8 +9,8 @@
#include <quic/QuicConstants.h> #include <quic/QuicConstants.h>
#include <quic/congestion_control/Bandwidth.h> #include <quic/congestion_control/Bandwidth.h>
#include <quic/state/ClonedPacketIdentifier.h>
#include <quic/state/OutstandingPacket.h> #include <quic/state/OutstandingPacket.h>
#include <quic/state/PacketEvent.h>
#include <sys/types.h> #include <sys/types.h>
namespace quic { namespace quic {

2
quic/congestion_control/PacketProcessor.h
View File

@ -9,8 +9,8 @@
#include <folly/io/SocketOptionMap.h> #include <folly/io/SocketOptionMap.h>
#include <quic/congestion_control/CongestionController.h> #include <quic/congestion_control/CongestionController.h>
#include <quic/state/ClonedPacketIdentifier.h>
#include <quic/state/OutstandingPacket.h> #include <quic/state/OutstandingPacket.h>
#include <quic/state/PacketEvent.h>
namespace quic { namespace quic {

2
quic/congestion_control/test/Bbr2Test.cpp
View File

@ -148,7 +148,7 @@ TEST_F(Bbr2Test, GracefullyHandleMissingFields) {
auto packet = makeTestingWritePacket(0, 0, 0, testStart_); auto packet = makeTestingWritePacket(0, 0, 0, testStart_);
packet.lastAckedPacketInfo.clear(); packet.lastAckedPacketInfo.clear();
packet.associatedEvent.clear(); packet.maybeClonedPacketIdentifier.clear();
EXPECT_NO_THROW(bbr2.onPacketSent(packet)); EXPECT_NO_THROW(bbr2.onPacketSent(packet));
EXPECT_NO_THROW(bbr2.onPacketAckOrLoss(nullptr, nullptr)); EXPECT_NO_THROW(bbr2.onPacketAckOrLoss(nullptr, nullptr));

25
quic/loss/QuicLossFunctions.cpp
View File

@ -356,18 +356,22 @@ bool processOutstandingsForLoss(
CHECK_GT(conn.outstandings.dsrCount, 0); CHECK_GT(conn.outstandings.dsrCount, 0);
--conn.outstandings.dsrCount; --conn.outstandings.dsrCount;
} }
if (pkt.associatedEvent) { if (pkt.maybeClonedPacketIdentifier) {
CHECK(conn.outstandings.clonedPacketCount[pnSpace]); CHECK(conn.outstandings.clonedPacketCount[pnSpace]);
--conn.outstandings.clonedPacketCount[pnSpace]; --conn.outstandings.clonedPacketCount[pnSpace];
} }
// Invoke LossVisitor if the packet doesn't have a associated PacketEvent; // Invoke LossVisitor if the packet doesn't have a associated
// or if the PacketEvent is present in conn.outstandings.packetEvents. // ClonedPacketIdentifier; or if the ClonedPacketIdentifier is present in
bool processed = pkt.associatedEvent && // conn.outstandings.clonedPacketIdentifiers.
!conn.outstandings.packetEvents.count(*pkt.associatedEvent); bool processed = pkt.maybeClonedPacketIdentifier &&
!conn.outstandings.clonedPacketIdentifiers.count(
*pkt.maybeClonedPacketIdentifier);
lossVisitor(conn, pkt.packet, processed); lossVisitor(conn, pkt.packet, processed);
// Remove the PacketEvent from the outstandings.packetEvents set // Remove the ClonedPacketIdentifier from the
if (pkt.associatedEvent) { // outstandings.clonedPacketIdentifiers set
conn.outstandings.packetEvents.erase(*pkt.associatedEvent); if (pkt.maybeClonedPacketIdentifier) {
conn.outstandings.clonedPacketIdentifiers.erase(
*pkt.maybeClonedPacketIdentifier);
} }
if (!processed) { if (!processed) {
CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]); CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]);
@ -508,8 +512,9 @@ Optional<CongestionController::LossEvent> detectLossPackets(
if (earliest->metadata.scheduledForDestruction) { if (earliest->metadata.scheduledForDestruction) {
earliest++; earliest++;
} }
if (!earliest->associatedEvent || if (!earliest->maybeClonedPacketIdentifier ||
conn.outstandings.packetEvents.count(*earliest->associatedEvent)) { conn.outstandings.clonedPacketIdentifiers.count(
*earliest->maybeClonedPacketIdentifier)) {
break; break;
} }
} }

18
quic/loss/QuicLossFunctions.h
View File

@ -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 * cwnd. So we must set the loss timer so that we can write this data with the
* slack packet space for the clones. * slack packet space for the clones.
*/ */
if (!hasDataToWrite && conn.outstandings.packetEvents.empty() && if (!hasDataToWrite && conn.outstandings.clonedPacketIdentifiers.empty() &&
totalPacketsOutstanding == conn.outstandings.numClonedPackets()) { totalPacketsOutstanding == conn.outstandings.numClonedPackets()) {
VLOG(10) << __func__ << " unset alarm pure ack or processed packets only" VLOG(10) << __func__ << " unset alarm pure ack or processed packets only"
<< " outstanding=" << totalPacketsOutstanding << " outstanding=" << totalPacketsOutstanding
@ -172,7 +172,8 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
<< " haDataToWrite=" << hasDataToWrite << " haDataToWrite=" << hasDataToWrite
<< " outstanding=" << totalPacketsOutstanding << " outstanding=" << totalPacketsOutstanding
<< " outstanding clone=" << conn.outstandings.numClonedPackets() << " outstanding clone=" << conn.outstandings.numClonedPackets()
<< " packetEvents=" << conn.outstandings.packetEvents.size() << " clonedPacketIdentifiers="
<< conn.outstandings.clonedPacketIdentifiers.size()
<< " initialPackets=" << " initialPackets="
<< conn.outstandings.packetCount[PacketNumberSpace::Initial] << conn.outstandings.packetCount[PacketNumberSpace::Initial]
<< " handshakePackets=" << " handshakePackets="
@ -287,12 +288,15 @@ void markZeroRttPacketsLost(
iter->packet.header.getProtectionType() == ProtectionType::ZeroRtt; iter->packet.header.getProtectionType() == ProtectionType::ZeroRtt;
if (isZeroRttPacket) { if (isZeroRttPacket) {
auto& pkt = *iter; auto& pkt = *iter;
bool processed = pkt.associatedEvent && bool processed = pkt.maybeClonedPacketIdentifier &&
!conn.outstandings.packetEvents.count(*pkt.associatedEvent); !conn.outstandings.clonedPacketIdentifiers.count(
*pkt.maybeClonedPacketIdentifier);
lossVisitor(conn, pkt.packet, processed); lossVisitor(conn, pkt.packet, processed);
// Remove the PacketEvent from the outstandings.packetEvents set // Remove the ClonedPacketIdentifier from the
if (pkt.associatedEvent) { // outstandings.clonedPacketIdentifiers set
conn.outstandings.packetEvents.erase(*pkt.associatedEvent); if (pkt.maybeClonedPacketIdentifier) {
conn.outstandings.clonedPacketIdentifiers.erase(
*pkt.maybeClonedPacketIdentifier);
CHECK(conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]); CHECK(conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
--conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]; --conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData];
} }

92
quic/loss/test/QuicLossFunctionsTest.cpp
View File

@ -95,7 +95,7 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
PacketNum sendPacket( PacketNum sendPacket(
QuicConnectionStateBase& conn, QuicConnectionStateBase& conn,
TimePoint time, TimePoint time,
Optional<PacketEvent> associatedEvent, Optional<ClonedPacketIdentifier> maybeClonedPacketIdentifier,
PacketType packetType, PacketType packetType,
Optional<uint16_t> forcedSize = none, Optional<uint16_t> forcedSize = none,
bool isDsr = false); bool isDsr = false);
@ -189,7 +189,7 @@ auto testingLossMarkFunc(std::vector<PacketNum>& lostPackets) {
PacketNum QuicLossFunctionsTest::sendPacket( PacketNum QuicLossFunctionsTest::sendPacket(
QuicConnectionStateBase& conn, QuicConnectionStateBase& conn,
TimePoint time, TimePoint time,
Optional<PacketEvent> associatedEvent, Optional<ClonedPacketIdentifier> maybeClonedPacketIdentifier,
PacketType packetType, PacketType packetType,
Optional<uint16_t> forcedSize, Optional<uint16_t> forcedSize,
bool isDsr) { bool isDsr) {
@ -265,28 +265,29 @@ PacketNum QuicLossFunctionsTest::sendPacket(
LossState(), LossState(),
0, 0,
OutstandingPacketMetadata::DetailsPerStream()); OutstandingPacketMetadata::DetailsPerStream());
outstandingPacket.associatedEvent = associatedEvent; outstandingPacket.maybeClonedPacketIdentifier = maybeClonedPacketIdentifier;
conn.lossState.lastRetransmittablePacketSentTime = time; conn.lossState.lastRetransmittablePacketSentTime = time;
if (conn.congestionController) { if (conn.congestionController) {
conn.congestionController->onPacketSent(outstandingPacket); conn.congestionController->onPacketSent(outstandingPacket);
} }
if (associatedEvent) { if (maybeClonedPacketIdentifier) {
conn.outstandings.clonedPacketCount[packetNumberSpace]++; conn.outstandings.clonedPacketCount[packetNumberSpace]++;
// Simulates what the real writer does. // Simulates what the real writer does.
auto it = std::find_if( auto it = std::find_if(
conn.outstandings.packets.begin(), conn.outstandings.packets.begin(),
conn.outstandings.packets.end(), conn.outstandings.packets.end(),
[&associatedEvent](const auto& packet) { [&maybeClonedPacketIdentifier](const auto& packet) {
auto packetNum = packet.packet.header.getPacketSequenceNum(); auto packetNum = packet.packet.header.getPacketSequenceNum();
auto packetNumSpace = packet.packet.header.getPacketNumberSpace(); auto packetNumSpace = packet.packet.header.getPacketNumberSpace();
return packetNum == associatedEvent->packetNumber && return packetNum == maybeClonedPacketIdentifier->packetNumber &&
packetNumSpace == associatedEvent->packetNumberSpace; packetNumSpace == maybeClonedPacketIdentifier->packetNumberSpace;
}); });
if (it != conn.outstandings.packets.end()) { if (it != conn.outstandings.packets.end()) {
if (!it->associatedEvent) { if (!it->maybeClonedPacketIdentifier) {
conn.outstandings.packetEvents.emplace(*associatedEvent); conn.outstandings.clonedPacketIdentifiers.emplace(
*maybeClonedPacketIdentifier);
conn.outstandings.clonedPacketCount[packetNumberSpace]++; conn.outstandings.clonedPacketCount[packetNumberSpace]++;
it->associatedEvent = *associatedEvent; it->maybeClonedPacketIdentifier = *maybeClonedPacketIdentifier;
} }
} }
} else { } else {
@ -318,18 +319,18 @@ RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) {
TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) { TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) {
auto conn = createConn(); auto conn = createConn();
EXPECT_CALL(*quicStats_, onPTO()).Times(0); EXPECT_CALL(*quicStats_, onPTO()).Times(0);
PacketEvent packetEvent1( ClonedPacketIdentifier clonedPacketIdentifier1(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); conn->ackStates.appDataAckState.nextPacketNum);
sendPacket(*conn, Clock::now(), packetEvent1, PacketType::OneRtt); sendPacket(*conn, Clock::now(), clonedPacketIdentifier1, PacketType::OneRtt);
PacketEvent packetEvent2( ClonedPacketIdentifier clonedPacketIdentifier2(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); conn->ackStates.appDataAckState.nextPacketNum);
sendPacket(*conn, Clock::now(), packetEvent2, PacketType::OneRtt); sendPacket(*conn, Clock::now(), clonedPacketIdentifier2, PacketType::OneRtt);
PacketEvent packetEvent3( ClonedPacketIdentifier clonedPacketIdentifier3(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); conn->ackStates.appDataAckState.nextPacketNum);
sendPacket(*conn, Clock::now(), packetEvent3, PacketType::OneRtt); sendPacket(*conn, Clock::now(), clonedPacketIdentifier3, PacketType::OneRtt);
EXPECT_CALL(timeout, cancelLossTimeout()).Times(1); EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
setLossDetectionAlarm(*conn, timeout); setLossDetectionAlarm(*conn, timeout);
EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm); EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
@ -450,11 +451,13 @@ TEST_F(QuicLossFunctionsTest, TestOnPTOSkipProcessed) {
conn->congestionController = std::move(mockCongestionController); conn->congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, onPacketSent(_)) EXPECT_CALL(*rawCongestionController, onPacketSent(_))
.WillRepeatedly(Return()); .WillRepeatedly(Return());
// By adding an associatedEvent that doesn't exist in the // By adding an maybeClonedPacketIdentifier that doesn't exist in the
// outstandings.packetEvents, they are all processed and will skip lossVisitor // outstandings.clonedPacketIdentifiers, they are all processed and will skip
// lossVisitor
for (auto i = 0; i < 10; i++) { for (auto i = 0; i < 10; i++) {
PacketEvent packetEvent(PacketNumberSpace::AppData, i); ClonedPacketIdentifier clonedPacketIdentifier(
sendPacket(*conn, TimePoint(), packetEvent, PacketType::OneRtt); PacketNumberSpace::AppData, i);
sendPacket(*conn, TimePoint(), clonedPacketIdentifier, PacketType::OneRtt);
} }
EXPECT_EQ(10, conn->outstandings.packets.size()); EXPECT_EQ(10, conn->outstandings.packets.size());
std::vector<PacketNum> lostPackets; std::vector<PacketNum> lostPackets;
@ -1428,8 +1431,9 @@ TEST_F(QuicLossFunctionsTest, SkipLossVisitor) {
PacketNum lastSent; PacketNum lastSent;
for (size_t i = 0; i < 5; i++) { for (size_t i = 0; i < 5; i++) {
lastSent = conn->ackStates.appDataAckState.nextPacketNum; lastSent = conn->ackStates.appDataAckState.nextPacketNum;
PacketEvent packetEvent(PacketNumberSpace::AppData, lastSent); ClonedPacketIdentifier clonedPacketIdentifier(
sendPacket(*conn, Clock::now(), packetEvent, PacketType::OneRtt); PacketNumberSpace::AppData, lastSent);
sendPacket(*conn, Clock::now(), clonedPacketIdentifier, PacketType::OneRtt);
} }
auto& ackState = getAckState(*conn, PacketNumberSpace::AppData); auto& ackState = getAckState(*conn, PacketNumberSpace::AppData);
@ -1462,14 +1466,17 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) {
PacketNum lastSent; PacketNum lastSent;
lastSent = sendPacket(*conn, Clock::now(), none, PacketType::OneRtt); lastSent = sendPacket(*conn, Clock::now(), none, PacketType::OneRtt);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::AppData]); 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++) { 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(7, conn->outstandings.packets.size());
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::AppData]); EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::AppData]);
// Add the PacketEvent to the outstandings.packetEvents set // Add the ClonedPacketIdentifier to the outstandings.clonedPacketIdentifiers
conn->outstandings.packetEvents.insert(event); // set
conn->outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier);
// Ack the last sent packet. Despite three losses, lossVisitor only visit one // Ack the last sent packet. Despite three losses, lossVisitor only visit one
// packet // packet
@ -1493,10 +1500,10 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) {
TEST_F(QuicLossFunctionsTest, DetectPacketLossClonedPacketsCounter) { TEST_F(QuicLossFunctionsTest, DetectPacketLossClonedPacketsCounter) {
auto conn = createConn(); auto conn = createConn();
PacketEvent packetEvent1( ClonedPacketIdentifier clonedPacketIdentifier1(
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
conn->ackStates.appDataAckState.nextPacketNum); 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); 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); MockAsyncUDPSocket socket(qEvb);
auto conn = createConn(); auto conn = createConn();
ASSERT_TRUE(conn->outstandings.packets.empty()); ASSERT_TRUE(conn->outstandings.packets.empty());
ASSERT_TRUE(conn->outstandings.packetEvents.empty()); ASSERT_TRUE(conn->outstandings.clonedPacketIdentifiers.empty());
auto stream1Id = auto stream1Id =
conn->streamManager->createNextBidirectionalStream().value()->id; conn->streamManager->createNextBidirectionalStream().value()->id;
auto buf = folly::IOBuf::copyBuffer("I wrestled by the sea."); 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->streamManager->pendingWindowUpdate(stream2->id));
EXPECT_FALSE(conn->pendingEvents.connWindowUpdate); EXPECT_FALSE(conn->pendingEvents.connWindowUpdate);
ASSERT_EQ(1, conn->outstandings.packets.size()); 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, uint32_t streamDataCounter = 0, streamWindowUpdateCounter = 0,
connWindowUpdateCounter = 0; connWindowUpdateCounter = 0;
auto strippedPacket = stripPaddingFrames( auto strippedPacket = stripPaddingFrames(
@ -1638,8 +1645,9 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejected) {
conn->congestionController = std::move(mockCongestionController); conn->congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, onPacketSent(_)) EXPECT_CALL(*rawCongestionController, onPacketSent(_))
.WillRepeatedly(Return()); .WillRepeatedly(Return());
// By adding an associatedEvent that doesn't exist in the // By adding an maybeClonedPacketIdentifier that doesn't exist in the
// outstandings.packetEvents, they are all processed and will skip lossVisitor // outstandings.clonedPacketIdentifiers, they are all processed and will skip
// lossVisitor
for (auto i = 0; i < 2; i++) { for (auto i = 0; i < 2; i++) {
sendPacket(*conn, TimePoint(), none, PacketType::OneRtt); sendPacket(*conn, TimePoint(), none, PacketType::OneRtt);
sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt); sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt);
@ -1673,27 +1681,29 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) {
conn->congestionController = std::move(mockCongestionController); conn->congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, onPacketSent(_)) EXPECT_CALL(*rawCongestionController, onPacketSent(_))
.WillRepeatedly(Return()); .WillRepeatedly(Return());
// By adding an associatedEvent that doesn't exist in the // By adding an maybeClonedPacketIdentifier that doesn't exist in the
// outstandings.packetEvents, they are all processed and will skip lossVisitor // outstandings.clonedPacketIdentifiers, they are all processed and will skip
// lossVisitor
std::set<PacketNum> zeroRttPackets; std::set<PacketNum> zeroRttPackets;
Optional<PacketEvent> lastPacketEvent; Optional<ClonedPacketIdentifier> lastClonedPacketIdentifier;
for (auto i = 0; i < 2; i++) { for (auto i = 0; i < 2; i++) {
auto packetNum = auto packetNum = sendPacket(
sendPacket(*conn, TimePoint(), lastPacketEvent, PacketType::ZeroRtt); *conn, TimePoint(), lastClonedPacketIdentifier, PacketType::ZeroRtt);
lastPacketEvent = PacketEvent(PacketNumberSpace::AppData, packetNum); lastClonedPacketIdentifier =
ClonedPacketIdentifier(PacketNumberSpace::AppData, packetNum);
zeroRttPackets.emplace(packetNum); zeroRttPackets.emplace(packetNum);
} }
zeroRttPackets.emplace( zeroRttPackets.emplace(
sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt)); sendPacket(*conn, TimePoint(), none, PacketType::ZeroRtt));
for (auto zeroRttPacketNum : zeroRttPackets) { for (auto zeroRttPacketNum : zeroRttPackets) {
PacketEvent zeroRttPacketEvent( ClonedPacketIdentifier zeroRttPacketEvent(
PacketNumberSpace::AppData, zeroRttPacketNum); PacketNumberSpace::AppData, zeroRttPacketNum);
sendPacket(*conn, TimePoint(), zeroRttPacketEvent, PacketType::OneRtt); sendPacket(*conn, TimePoint(), zeroRttPacketEvent, PacketType::OneRtt);
} }
EXPECT_EQ(6, conn->outstandings.packets.size()); EXPECT_EQ(6, conn->outstandings.packets.size());
ASSERT_EQ(conn->outstandings.numClonedPackets(), 6); 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]); ASSERT_EQ(2, conn->outstandings.packetCount[PacketNumberSpace::AppData]);
std::vector<bool> lostPackets; std::vector<bool> lostPackets;
@ -1702,7 +1712,7 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) {
markZeroRttPacketsLost(*conn, [&lostPackets](auto&, auto&, bool processed) { markZeroRttPacketsLost(*conn, [&lostPackets](auto&, auto&, bool processed) {
lostPackets.emplace_back(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(3, conn->outstandings.packets.size());
EXPECT_EQ(lostPackets.size(), 3); EXPECT_EQ(lostPackets.size(), 3);
ASSERT_EQ(conn->outstandings.numClonedPackets(), 3); ASSERT_EQ(conn->outstandings.numClonedPackets(), 3);

6
quic/somerge_defs.bzl
View File

@ -2,7 +2,7 @@
Generated by xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py Generated by xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py
@generated SignedSource<<390b7a822f95d1254eb82e48a7d607e2>> @generated SignedSource<<1eb8d098bfda4c6700775f29fd0044b1>>
""" """
# Entry Points: # Entry Points:
@ -172,6 +172,8 @@ QUIC_NATIVE_LIBRARY_MERGE_MAP = [
"fbsource//xplat/quic/state:ack_handlerAndroidAndroid", "fbsource//xplat/quic/state:ack_handlerAndroidAndroid",
"fbsource//xplat/quic/state:ack_statesAndroid", "fbsource//xplat/quic/state:ack_statesAndroid",
"fbsource//xplat/quic/state:ack_statesAndroidAndroid", "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_handlerAndroid",
"fbsource//xplat/quic/state:datagram_handlerAndroidAndroid", "fbsource//xplat/quic/state:datagram_handlerAndroidAndroid",
"fbsource//xplat/quic/state:loss_stateAndroid", "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:outstanding_packetAndroidAndroid",
"fbsource//xplat/quic/state:pacing_functionsAndroid", "fbsource//xplat/quic/state:pacing_functionsAndroid",
"fbsource//xplat/quic/state:pacing_functionsAndroidAndroid", "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_statsAndroid",
"fbsource//xplat/quic/state:quic_connection_statsAndroidAndroid", "fbsource//xplat/quic/state:quic_connection_statsAndroidAndroid",
"fbsource//xplat/quic/state:quic_priority_queueAndroid", "fbsource//xplat/quic/state:quic_priority_queueAndroid",

20
quic/state/AckHandlers.cpp
View File

@ -240,14 +240,15 @@ AckEvent processAckFrame(
rPacketIt++; rPacketIt++;
continue; continue;
} }
bool needsProcess = !rPacketIt->associatedEvent || bool needsProcess = !rPacketIt->maybeClonedPacketIdentifier ||
conn.outstandings.packetEvents.count(*rPacketIt->associatedEvent); conn.outstandings.clonedPacketIdentifiers.count(
*rPacketIt->maybeClonedPacketIdentifier);
if (needsProcess) { if (needsProcess) {
CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]); CHECK(conn.outstandings.packetCount[currentPacketNumberSpace]);
--conn.outstandings.packetCount[currentPacketNumberSpace]; --conn.outstandings.packetCount[currentPacketNumberSpace];
} }
ack.ackedBytes += rPacketIt->metadata.encodedSize; ack.ackedBytes += rPacketIt->metadata.encodedSize;
if (rPacketIt->associatedEvent) { if (rPacketIt->maybeClonedPacketIdentifier) {
CHECK(conn.outstandings.clonedPacketCount[currentPacketNumberSpace]); CHECK(conn.outstandings.clonedPacketCount[currentPacketNumberSpace]);
--conn.outstandings.clonedPacketCount[currentPacketNumberSpace]; --conn.outstandings.clonedPacketCount[currentPacketNumberSpace];
} }
@ -307,9 +308,11 @@ AckEvent processAckFrame(
} // if (rttSample != rttSample.zero()) } // if (rttSample != rttSample.zero())
} // if (!ack.implicit && currentPacketNum == frame.largestAcked) } // if (!ack.implicit && currentPacketNum == frame.largestAcked)
// Remove this PacketEvent from the outstandings.packetEvents set // Remove this ClonedPacketIdentifier from the
if (rPacketIt->associatedEvent) { // outstandings.clonedPacketIdentifiers set
conn.outstandings.packetEvents.erase(*rPacketIt->associatedEvent); if (rPacketIt->maybeClonedPacketIdentifier) {
conn.outstandings.clonedPacketIdentifiers.erase(
*rPacketIt->maybeClonedPacketIdentifier);
} }
if (!ack.largestNewlyAckedPacket || if (!ack.largestNewlyAckedPacket ||
*ack.largestNewlyAckedPacket < currentPacketNum) { *ack.largestNewlyAckedPacket < currentPacketNum) {
@ -361,8 +364,9 @@ AckEvent processAckFrame(
} }
// Invoke AckVisitor for WriteAckFrames all the time. Invoke it for other // Invoke AckVisitor for WriteAckFrames all the time. Invoke it for other
// frame types only if the packet doesn't have an associated PacketEvent; // frame types only if the packet doesn't have an associated
// or the PacketEvent is in conn.outstandings.packetEvents // ClonedPacketIdentifier; or the ClonedPacketIdentifier is in
// conn.outstandings.clonedPacketIdentifiers
ack.ackedPackets.reserve(packetsWithHandlerContext.size()); ack.ackedPackets.reserve(packetsWithHandlerContext.size());
for (auto packetWithHandlerContextItr = packetsWithHandlerContext.rbegin(); for (auto packetWithHandlerContextItr = packetsWithHandlerContext.rbegin();
packetWithHandlerContextItr != packetsWithHandlerContext.rend(); packetWithHandlerContextItr != packetsWithHandlerContext.rend();

10
quic/state/BUCK
View File

@ -8,8 +8,8 @@ mvfst_cpp_library(
"OutstandingPacket.h", "OutstandingPacket.h",
], ],
exported_deps = [ exported_deps = [
":cloned_packet_identifier",
":loss_state", ":loss_state",
":packet_event",
"//folly/io:socket_option_map", "//folly/io:socket_option_map",
"//quic/codec:types", "//quic/codec:types",
], ],
@ -52,12 +52,12 @@ mvfst_cpp_library(
) )
mvfst_cpp_library( mvfst_cpp_library(
name = "packet_event", name = "cloned_packet_identifier",
srcs = [ srcs = [
"PacketEvent.cpp", "ClonedPacketIdentifier.cpp",
], ],
headers = [ headers = [
"PacketEvent.h", "ClonedPacketIdentifier.h",
], ],
deps = [ deps = [
"//folly/hash:hash", "//folly/hash:hash",
@ -112,9 +112,9 @@ mvfst_cpp_library(
exported_deps = [ exported_deps = [
":ack_event", ":ack_event",
":ack_states", ":ack_states",
":cloned_packet_identifier",
":loss_state", ":loss_state",
":outstanding_packet", ":outstanding_packet",
":packet_event",
":quic_connection_stats", ":quic_connection_stats",
":quic_priority_queue", ":quic_priority_queue",
":retransmission_policy", ":retransmission_policy",

2
quic/state/CMakeLists.txt
View File

@ -9,7 +9,7 @@ add_library(
QuicStreamManager.cpp QuicStreamManager.cpp
QuicStreamUtilities.cpp QuicStreamUtilities.cpp
StateData.cpp StateData.cpp
PacketEvent.cpp ClonedPacketIdentifier.cpp
PendingPathRateLimiter.cpp PendingPathRateLimiter.cpp
QuicPriorityQueue.cpp QuicPriorityQueue.cpp
) )

16
quic/state/PacketEvent.cpp → quic/state/ClonedPacketIdentifier.cpp
View File

@ -5,19 +5,21 @@
* LICENSE file in the root directory of this source tree. * LICENSE file in the root directory of this source tree.
*/ */
#include <quic/state/PacketEvent.h> #include <quic/state/ClonedPacketIdentifier.h>
#include <folly/hash/Hash.h> #include <folly/hash/Hash.h>
#include <functional> #include <functional>
namespace quic { namespace quic {
PacketEvent::PacketEvent( ClonedPacketIdentifier::ClonedPacketIdentifier(
PacketNumberSpace packetNumberSpaceIn, PacketNumberSpace packetNumberSpaceIn,
PacketNum packetNumberIn) PacketNum packetNumberIn)
: packetNumber(packetNumberIn), packetNumberSpace(packetNumberSpaceIn) {} : packetNumber(packetNumberIn), packetNumberSpace(packetNumberSpaceIn) {}
bool operator==(const PacketEvent& lhs, const PacketEvent& rhs) { bool operator==(
const ClonedPacketIdentifier& lhs,
const ClonedPacketIdentifier& rhs) {
return static_cast<std::underlying_type_t<PacketNumberSpace>>( return static_cast<std::underlying_type_t<PacketNumberSpace>>(
lhs.packetNumberSpace) == lhs.packetNumberSpace) ==
static_cast<std::underlying_type_t<PacketNumberSpace>>( static_cast<std::underlying_type_t<PacketNumberSpace>>(
@ -25,11 +27,11 @@ bool operator==(const PacketEvent& lhs, const PacketEvent& rhs) {
lhs.packetNumber == rhs.packetNumber; lhs.packetNumber == rhs.packetNumber;
} }
size_t PacketEventHash::operator()( size_t ClonedPacketIdentifierHash::operator()(
const PacketEvent& packetEvent) const noexcept { const ClonedPacketIdentifier& clonedPacketIdentifier) const noexcept {
return folly::hash::hash_combine( return folly::hash::hash_combine(
static_cast<std::underlying_type_t<PacketNumberSpace>>( static_cast<std::underlying_type_t<PacketNumberSpace>>(
packetEvent.packetNumberSpace), clonedPacketIdentifier.packetNumberSpace),
packetEvent.packetNumber); clonedPacketIdentifier.packetNumber);
} }
} // namespace quic } // namespace quic

52
quic/state/ClonedPacketIdentifier.h Normal file
View File

@ -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 <quic/codec/Types.h>
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

8
quic/state/OutstandingPacket.h
View File

@ -9,8 +9,8 @@
#include <folly/io/SocketOptionMap.h> #include <folly/io/SocketOptionMap.h>
#include <quic/codec/Types.h> #include <quic/codec/Types.h>
#include <quic/state/ClonedPacketIdentifier.h>
#include <quic/state/LossState.h> #include <quic/state/LossState.h>
#include <quic/state/PacketEvent.h>
#include <chrono> #include <chrono>
namespace quic { namespace quic {
@ -174,9 +174,9 @@ struct OutstandingPacket {
}; };
Optional<LastAckedPacketInfo> lastAckedPacketInfo; Optional<LastAckedPacketInfo> lastAckedPacketInfo;
// PacketEvent associated with this OutstandingPacketWrapper. This will be a // ClonedPacketIdentifier associated with this OutstandingPacketWrapper. This
// none if the packet isn't a clone and hasn't been cloned. // will be a none if the packet isn't a clone and hasn't been cloned.
Optional<PacketEvent> associatedEvent; Optional<ClonedPacketIdentifier> maybeClonedPacketIdentifier;
OptionalIntegral<uint64_t> nonDsrPacketSequenceNumber; OptionalIntegral<uint64_t> nonDsrPacketSequenceNumber;

47
quic/state/PacketEvent.h
View File

@ -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 <quic/codec/Types.h>
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

11
quic/state/StateData.h
View File

@ -22,9 +22,9 @@
#include <quic/observer/SocketObserverTypes.h> #include <quic/observer/SocketObserverTypes.h>
#include <quic/state/AckEvent.h> #include <quic/state/AckEvent.h>
#include <quic/state/AckStates.h> #include <quic/state/AckStates.h>
#include <quic/state/ClonedPacketIdentifier.h>
#include <quic/state/LossState.h> #include <quic/state/LossState.h>
#include <quic/state/OutstandingPacket.h> #include <quic/state/OutstandingPacket.h>
#include <quic/state/PacketEvent.h>
#include <quic/state/PendingPathRateLimiter.h> #include <quic/state/PendingPathRateLimiter.h>
#include <quic/state/QuicConnectionStats.h> #include <quic/state/QuicConnectionStats.h>
#include <quic/state/QuicStreamGroupRetransmissionPolicy.h> #include <quic/state/QuicStreamGroupRetransmissionPolicy.h>
@ -49,9 +49,10 @@ struct OutstandingsInfo {
std::deque<OutstandingPacketWrapper> packets; std::deque<OutstandingPacketWrapper> packets;
// All PacketEvents of this connection. If a OutstandingPacketWrapper doesn't // 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 // have an maybeClonedPacketIdentifier or if it's not in this set, there is no
// process its frames upon ack or loss. // need to process its frames upon ack or loss.
folly::F14FastSet<PacketEvent, PacketEventHash> packetEvents; folly::F14FastSet<ClonedPacketIdentifier, ClonedPacketIdentifierHash>
clonedPacketIdentifiers;
// Number of outstanding packets not including cloned // Number of outstanding packets not including cloned
EnumArray<PacketNumberSpace, uint64_t> packetCount{}; EnumArray<PacketNumberSpace, uint64_t> packetCount{};
@ -86,7 +87,7 @@ struct OutstandingsInfo {
void reset() { void reset() {
packets.clear(); packets.clear();
packetEvents.clear(); clonedPacketIdentifiers.clear();
packetCount = {}; packetCount = {};
clonedPacketCount = {}; clonedPacketCount = {};
declaredLostCount = 0; declaredLostCount = 0;

23
quic/state/test/AckHandlersTest.cpp
View File

@ -1879,9 +1879,9 @@ TEST_P(AckHandlersTest, SkipAckVisitor) {
LossState(), LossState(),
0, 0,
OutstandingPacketMetadata::DetailsPerStream()); OutstandingPacketMetadata::DetailsPerStream());
// Give this outstandingPacket an associatedEvent that's not in // Give this outstandingPacket an maybeClonedPacketIdentifier that's not in
// outstandings.packetEvents // outstandings.clonedPacketIdentifiers
outstandingPacket.associatedEvent.emplace(GetParam().pnSpace, 0); outstandingPacket.maybeClonedPacketIdentifier.emplace(GetParam().pnSpace, 0);
conn.outstandings.packets.push_back(std::move(outstandingPacket)); conn.outstandings.packets.push_back(std::move(outstandingPacket));
conn.outstandings.clonedPacketCount[GetParam().pnSpace]++; conn.outstandings.clonedPacketCount[GetParam().pnSpace]++;
@ -2000,7 +2000,8 @@ TEST_P(AckHandlersTest, NoDoubleProcess) {
LossState(), LossState(),
0, 0,
OutstandingPacketMetadata::DetailsPerStream()); OutstandingPacketMetadata::DetailsPerStream());
outstandingPacket1.associatedEvent.emplace(GetParam().pnSpace, packetNum1); outstandingPacket1.maybeClonedPacketIdentifier.emplace(
GetParam().pnSpace, packetNum1);
OutstandingPacketWrapper outstandingPacket2( OutstandingPacketWrapper outstandingPacket2(
std::move(regularPacket2), std::move(regularPacket2),
@ -2012,14 +2013,16 @@ TEST_P(AckHandlersTest, NoDoubleProcess) {
LossState(), LossState(),
0, 0,
OutstandingPacketMetadata::DetailsPerStream()); OutstandingPacketMetadata::DetailsPerStream());
// The seconds packet has the same PacketEvent // The seconds packet has the same ClonedPacketIdentifier
outstandingPacket2.associatedEvent.emplace(GetParam().pnSpace, packetNum1); outstandingPacket2.maybeClonedPacketIdentifier.emplace(
GetParam().pnSpace, packetNum1);
conn.outstandings.packetCount[GetParam().pnSpace]++; conn.outstandings.packetCount[GetParam().pnSpace]++;
conn.outstandings.packets.push_back(std::move(outstandingPacket1)); conn.outstandings.packets.push_back(std::move(outstandingPacket1));
conn.outstandings.packets.push_back(std::move(outstandingPacket2)); conn.outstandings.packets.push_back(std::move(outstandingPacket2));
conn.outstandings.clonedPacketCount[GetParam().pnSpace] += 2; conn.outstandings.clonedPacketCount[GetParam().pnSpace] += 2;
conn.outstandings.packetEvents.emplace(GetParam().pnSpace, packetNum1); conn.outstandings.clonedPacketIdentifiers.emplace(
GetParam().pnSpace, packetNum1);
// A counting ack visitor // A counting ack visitor
uint16_t ackVisitorCounter = 0; uint16_t ackVisitorCounter = 0;
@ -2077,7 +2080,8 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) {
LossState(), LossState(),
0, 0,
OutstandingPacketMetadata::DetailsPerStream()); OutstandingPacketMetadata::DetailsPerStream());
outstandingPacket1.associatedEvent.emplace(GetParam().pnSpace, packetNum1); outstandingPacket1.maybeClonedPacketIdentifier.emplace(
GetParam().pnSpace, packetNum1);
conn.ackStates.appDataAckState.nextPacketNum++; conn.ackStates.appDataAckState.nextPacketNum++;
auto packetNum2 = 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(outstandingPacket1));
conn.outstandings.packets.push_back(std::move(outstandingPacket2)); conn.outstandings.packets.push_back(std::move(outstandingPacket2));
conn.outstandings.clonedPacketCount[GetParam().pnSpace] = 1; conn.outstandings.clonedPacketCount[GetParam().pnSpace] = 1;
conn.outstandings.packetEvents.emplace(GetParam().pnSpace, packetNum1); conn.outstandings.clonedPacketIdentifiers.emplace(
GetParam().pnSpace, packetNum1);
ReadAckFrame ackFrame; ReadAckFrame ackFrame;
ackFrame.largestAcked = packetNum2; ackFrame.largestAcked = packetNum2;

6
quic/state/test/BUCK
View File

@ -134,13 +134,13 @@ cpp_unittest(
) )
cpp_unittest( cpp_unittest(
name = "PacketEventTest", name = "ClonedPacketIdentifierTest",
srcs = [ srcs = [
"PacketEventTest.cpp", "ClonedPacketIdentifierTest.cpp",
], ],
deps = [ deps = [
"//folly/portability:gtest", "//folly/portability:gtest",
"//quic/state:packet_event", "//quic/state:cloned_packet_identifier",
], ],
) )

49
quic/state/test/ClonedPacketIdentifierTest.cpp Normal file
View File

@ -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 <quic/state/ClonedPacketIdentifier.h>
#include <folly/portability/GTest.h>
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

36
quic/state/test/PacketEventTest.cpp
View File

@ -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 <quic/state/PacketEvent.h>
#include <folly/portability/GTest.h>
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