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

make largestAckedByPeer and largestSent optional

Browse Source 
Summary: 0 is now a valid packet number, so we should make these optional. In cases where they are needed to construct packet builder, it should be safe to use 0 as default since it's only used for computing `twiceDistance` in PacketNumber.cpp.

Reviewed By: yangchi

Differential Revision: D21948454

fbshipit-source-id: af9fdc3e28ff85f1594296c4d436f24685a0acd6
This commit is contained in:
Xiaoting Tang
2020-06-10 10:28:08 -07:00
committed by Facebook GitHub Bot
parent dc2ef325b5
commit 03e3bb6547
15 changed files with 82 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
quic/api/QuicPacketScheduler.cpp
View File

@@ -557,14 +557,14 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket(
internalBuilder = std::make_unique<RegularQuicPacketBuilder>( internalBuilder = std::make_unique<RegularQuicPacketBuilder>(
conn_.udpSendPacketLen - cipherOverhead_, conn_.udpSendPacketLen - cipherOverhead_,
header, header,
getAckState(conn_, builderPnSpace).largestAckedByPeer); getAckState(conn_, builderPnSpace).largestAckedByPeer.value_or(0));
} else { } else {
CHECK(conn_.bufAccessor && conn_.bufAccessor->ownsBuffer()); CHECK(conn_.bufAccessor && conn_.bufAccessor->ownsBuffer());
internalBuilder = std::make_unique<InplaceQuicPacketBuilder>( internalBuilder = std::make_unique<InplaceQuicPacketBuilder>(
*conn_.bufAccessor, *conn_.bufAccessor,
conn_.udpSendPacketLen - cipherOverhead_, conn_.udpSendPacketLen - cipherOverhead_,
header, header,
getAckState(conn_, builderPnSpace).largestAckedByPeer); getAckState(conn_, builderPnSpace).largestAckedByPeer.value_or(0));
} }
// We shouldn't clone Handshake packet. // We shouldn't clone Handshake packet.
if (iter->isHandshake) { if (iter->isHandshake) {

4
quic/api/QuicSocket.h
View File

@@ -111,8 +111,8 @@ class QuicSocket {
uint64_t totalBytesRetransmitted{0}; uint64_t totalBytesRetransmitted{0};
uint32_t ptoCount{0}; uint32_t ptoCount{0};
uint32_t totalPTOCount{0}; uint32_t totalPTOCount{0};
PacketNum largestPacketAckedByPeer{0}; folly::Optional<PacketNum> largestPacketAckedByPeer;
PacketNum largestPacketSent{0}; folly::Optional<PacketNum> largestPacketSent;
}; };
/** /**

9
quic/api/QuicTransportFunctions.cpp
View File

@@ -207,7 +207,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt(
*connection.bufAccessor, *connection.bufAccessor,
connection.udpSendPacketLen, connection.udpSendPacketLen,
std::move(header), std::move(header),
getAckState(connection, pnSpace).largestAckedByPeer); getAckState(connection, pnSpace).largestAckedByPeer.value_or(0));
pktBuilder.setCipherOverhead(cipherOverhead); pktBuilder.setCipherOverhead(cipherOverhead);
CHECK(scheduler.hasData()); CHECK(scheduler.hasData());
auto result = auto result =
@@ -287,7 +287,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt(
RegularQuicPacketBuilder pktBuilder( RegularQuicPacketBuilder pktBuilder(
connection.udpSendPacketLen, connection.udpSendPacketLen,
std::move(header), std::move(header),
getAckState(connection, pnSpace).largestAckedByPeer); getAckState(connection, pnSpace).largestAckedByPeer.value_or(0));
// It's the scheduler's job to invoke encode header // It's the scheduler's job to invoke encode header
pktBuilder.setCipherOverhead(cipherOverhead); pktBuilder.setCipherOverhead(cipherOverhead);
auto result = auto result =
@@ -605,7 +605,8 @@ void updateConnection(
} }
increaseNextPacketNum(conn, packetNumberSpace); increaseNextPacketNum(conn, packetNumberSpace);
conn.lossState.largestSent = std::max(conn.lossState.largestSent, packetNum); conn.lossState.largestSent =
std::max(conn.lossState.largestSent.value_or(packetNum), packetNum);
// updateConnection may be called multiple times during write. If before or // updateConnection may be called multiple times during write. If before or
// during any updateConnection, setLossDetectionAlarm is already set, we // during any updateConnection, setLossDetectionAlarm is already set, we
// shouldn't clear it: // shouldn't clear it:
@@ -903,7 +904,7 @@ void writeCloseCommon(
RegularQuicPacketBuilder packetBuilder( RegularQuicPacketBuilder packetBuilder(
connection.udpSendPacketLen, connection.udpSendPacketLen,
std::move(header), std::move(header),
getAckState(connection, pnSpace).largestAckedByPeer); getAckState(connection, pnSpace).largestAckedByPeer.value_or(0));
packetBuilder.encodePacketHeader(); packetBuilder.encodePacketHeader();
packetBuilder.setCipherOverhead(aead.getCipherOverhead()); packetBuilder.setCipherOverhead(aead.getCipherOverhead());
size_t written = 0; size_t written = 0;

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

@@ -97,7 +97,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader), std::move(longHeader),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
FrameScheduler cryptoOnlyScheduler = FrameScheduler cryptoOnlyScheduler =
std::move( std::move(
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -129,7 +129,7 @@ TEST_F(QuicPacketSchedulerTest, PaddingInitialPureAcks) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader), std::move(longHeader),
conn.ackStates.handshakeAckState.largestAckedByPeer); conn.ackStates.handshakeAckState.largestAckedByPeer.value_or(0));
conn.ackStates.initialAckState.largestRecvdPacketTime = Clock::now(); conn.ackStates.initialAckState.largestRecvdPacketTime = Clock::now();
conn.ackStates.initialAckState.needsToSendAckImmediately = true; conn.ackStates.initialAckState.needsToSendAckImmediately = true;
conn.ackStates.initialAckState.acks.insert(10); conn.ackStates.initialAckState.acks.insert(10);
@@ -163,7 +163,7 @@ TEST_F(QuicPacketSchedulerTest, PaddingUpToWrapperSize) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader), std::move(longHeader),
conn.ackStates.handshakeAckState.largestAckedByPeer); conn.ackStates.handshakeAckState.largestAckedByPeer.value_or(0));
conn.ackStates.initialAckState.largestRecvdPacketTime = Clock::now(); conn.ackStates.initialAckState.largestRecvdPacketTime = Clock::now();
conn.ackStates.initialAckState.needsToSendAckImmediately = true; conn.ackStates.initialAckState.needsToSendAckImmediately = true;
conn.ackStates.initialAckState.acks.insert(10); conn.ackStates.initialAckState.acks.insert(10);
@@ -196,7 +196,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialPadded) {
RegularQuicPacketBuilder builder1( RegularQuicPacketBuilder builder1(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader1), std::move(longHeader1),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
FrameScheduler scheduler = FrameScheduler scheduler =
std::move( std::move(
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -228,7 +228,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) {
RegularQuicPacketBuilder builder1( RegularQuicPacketBuilder builder1(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader1), std::move(longHeader1),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
FrameScheduler scheduler = FrameScheduler scheduler =
std::move( std::move(
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -256,7 +256,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) {
RegularQuicPacketBuilder builder2( RegularQuicPacketBuilder builder2(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader2), std::move(longHeader2),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
writeDataToQuicStream( writeDataToQuicStream(
conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("shlo again")); conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("shlo again"));
auto result2 = scheduler.scheduleFramesForPacket( auto result2 = scheduler.scheduleFramesForPacket(
@@ -279,7 +279,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader), std::move(longHeader),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
FrameScheduler scheduler = FrameScheduler scheduler =
std::move( std::move(
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -310,7 +310,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(longHeader), std::move(longHeader),
conn.ackStates.handshakeAckState.largestAckedByPeer); conn.ackStates.handshakeAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
PacketBuilderWrapper builderWrapper(builder, 13); PacketBuilderWrapper builderWrapper(builder, 13);
CryptoStreamScheduler scheduler( CryptoStreamScheduler scheduler(
@@ -338,7 +338,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
25, 25,
std::move(longHeader), std::move(longHeader),
conn.ackStates.initialAckState.largestAckedByPeer); conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
FrameScheduler cryptoOnlyScheduler = FrameScheduler cryptoOnlyScheduler =
std::move( std::move(
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -375,7 +375,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerExists) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader), std::move(shortHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto originalSpace = builder.remainingSpaceInPkt(); auto originalSpace = builder.remainingSpaceInPkt();
conn.streamManager->queueWindowUpdate(stream->id); conn.streamManager->queueWindowUpdate(stream->id);
@@ -397,7 +397,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameNoSpace) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader), std::move(shortHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
PacketBuilderWrapper builderWrapper(builder, 2); PacketBuilderWrapper builderWrapper(builder, 2);
auto originalSpace = builder.remainingSpaceInPkt(); auto originalSpace = builder.remainingSpaceInPkt();
@@ -419,7 +419,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerStreamNotExists) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader), std::move(shortHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto originalSpace = builder.remainingSpaceInPkt(); auto originalSpace = builder.remainingSpaceInPkt();
conn.streamManager->queueWindowUpdate(nonExistentStream); conn.streamManager->queueWindowUpdate(nonExistentStream);
@@ -448,7 +448,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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(result.packetEvent.has_value() && result.packet.has_value());
@@ -479,7 +479,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.initialAckState.largestAckedByPeer); 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(result.packetEvent.has_value() && result.packet.has_value());
@@ -524,7 +524,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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(result.packetEvent.has_value() && result.packet.has_value());
@@ -557,7 +557,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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(folly::none, result.packetEvent); EXPECT_EQ(folly::none, result.packetEvent);
@@ -606,7 +606,7 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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(expectedPacketEvent, *packetResult.packetEvent);
@@ -687,7 +687,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilder) {
bufAccessor, bufAccessor,
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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(result.packetEvent.has_value() && result.packet.has_value());
@@ -731,7 +731,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) {
bufAccessor, bufAccessor,
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_TRUE(scheduler.hasData()); ASSERT_TRUE(scheduler.hasData());
auto result = scheduler.scheduleFramesForPacket( auto result = scheduler.scheduleFramesForPacket(
std::move(builder), conn.udpSendPacketLen); std::move(builder), conn.udpSendPacketLen);
@@ -759,7 +759,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) {
bufAccessor, bufAccessor,
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(dupHeader), std::move(dupHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto cloneResult = cloningScheduler.scheduleFramesForPacket( auto cloneResult = cloningScheduler.scheduleFramesForPacket(
std::move(internalBuilder), conn.udpSendPacketLen); std::move(internalBuilder), conn.udpSendPacketLen);
EXPECT_TRUE( EXPECT_TRUE(
@@ -798,7 +798,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto packetResult = scheduler.scheduleFramesForPacket( auto packetResult = scheduler.scheduleFramesForPacket(
std::move(builder), conn.udpSendPacketLen - cipherOverhead); std::move(builder), conn.udpSendPacketLen - cipherOverhead);
auto encodedSize = packetResult.packet->body->computeChainDataLength() + auto encodedSize = packetResult.packet->body->computeChainDataLength() +
@@ -820,7 +820,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) {
RegularQuicPacketBuilder throwawayBuilder( RegularQuicPacketBuilder throwawayBuilder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(cloneHeader), std::move(cloneHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
FrameScheduler noopScheduler("noopScheduler"); FrameScheduler noopScheduler("noopScheduler");
CloningScheduler cloningScheduler( CloningScheduler cloningScheduler(
noopScheduler, conn, "CopyCat", cipherOverhead); noopScheduler, conn, "CopyCat", cipherOverhead);
@@ -905,7 +905,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader), std::move(shortHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto stream1 = auto stream1 =
conn.streamManager->createNextBidirectionalStream().value()->id; conn.streamManager->createNextBidirectionalStream().value()->id;
@@ -944,7 +944,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto stream1 = auto stream1 =
conn.streamManager->createNextBidirectionalStream().value()->id; conn.streamManager->createNextBidirectionalStream().value()->id;
@@ -1010,7 +1010,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinStreamPerPacket) {
RegularQuicPacketBuilder builder1( RegularQuicPacketBuilder builder1(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
auto stream1 = auto stream1 =
conn.streamManager->createNextBidirectionalStream().value()->id; conn.streamManager->createNextBidirectionalStream().value()->id;
auto stream2 = auto stream2 =
@@ -1078,7 +1078,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto stream1 = auto stream1 =
conn.streamManager->createNextBidirectionalStream().value()->id; conn.streamManager->createNextBidirectionalStream().value()->id;
@@ -1160,7 +1160,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(shortHeader), std::move(shortHeader),
conn.ackStates.appDataAckState.largestAckedByPeer); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
auto stream1 = conn.streamManager->createNextBidirectionalStream().value(); auto stream1 = conn.streamManager->createNextBidirectionalStream().value();
writeDataToQuicStream(*stream1, folly::IOBuf::copyBuffer("some data"), false); writeDataToQuicStream(*stream1, folly::IOBuf::copyBuffer("some data"), false);
@@ -1242,7 +1242,7 @@ TEST_F(
bufAccessor, bufAccessor,
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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.packetEvent.has_value());
@@ -1282,7 +1282,7 @@ TEST_F(
bufAccessor, bufAccessor,
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(header), std::move(header),
conn.ackStates.appDataAckState.largestAckedByPeer); 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.packetEvent.has_value());

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

@@ -120,7 +120,7 @@ auto buildEmptyPacket(
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(*header), std::move(*header),
getAckState(conn, pnSpace).largestAckedByPeer); getAckState(conn, pnSpace).largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
DCHECK(builder.canBuildPacket()); DCHECK(builder.canBuildPacket());
return std::move(builder).buildPacket(); return std::move(builder).buildPacket();

2
quic/common/test/TestUtils.cpp
View File

@@ -119,7 +119,7 @@ RegularQuicPacketBuilder::Packet createAckPacket(
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
dstConn.udpSendPacketLen, dstConn.udpSendPacketLen,
std::move(*header), std::move(*header),
getAckState(dstConn, pnSpace).largestAckedByPeer); getAckState(dstConn, pnSpace).largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
if (aead) { if (aead) {
builder.setCipherOverhead(aead->getCipherOverhead()); builder.setCipherOverhead(aead->getCipherOverhead());

3
quic/congestion_control/test/BbrBandwidthSamplerTest.cpp
View File

@@ -144,7 +144,8 @@ TEST_F(BbrBandwidthSamplerTest, AppLimited) {
sampler.onAppLimited(); sampler.onAppLimited();
EXPECT_TRUE(sampler.isAppLimited()); EXPECT_TRUE(sampler.isAppLimited());
CongestionController::AckEvent ackEvent; CongestionController::AckEvent ackEvent;
ackEvent.largestAckedPacket = ++conn.lossState.largestSent; conn.lossState.largestSent = conn.lossState.largestSent.value_or(0);
ackEvent.largestAckedPacket = ++conn.lossState.largestSent.value();
auto packet = auto packet =
makeTestingWritePacket(*ackEvent.largestAckedPacket, 1000, 1000); makeTestingWritePacket(*ackEvent.largestAckedPacket, 1000, 1000);
ackEvent.largestAckedPacketSentTime = packet.time; ackEvent.largestAckedPacketSentTime = packet.time;

27
quic/congestion_control/test/BbrTest.cpp
View File

@@ -162,7 +162,7 @@ TEST_F(BbrTest, LeaveStartup) {
auto sendAckGrow = [&](bool growFast) { auto sendAckGrow = [&](bool growFast) {
conn.lossState.largestSent = currentLatest; conn.lossState.largestSent = currentLatest;
auto packet = makeTestingWritePacket( auto packet = makeTestingWritePacket(
conn.lossState.largestSent, 1000, 1000 + totalSent); conn.lossState.largestSent.value(), 1000, 1000 + totalSent);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
totalSent += 1000; totalSent += 1000;
EXPECT_CALL(*rawBandwidthSampler, getBandwidth()) EXPECT_CALL(*rawBandwidthSampler, getBandwidth())
@@ -231,7 +231,7 @@ TEST_F(BbrTest, ProbeRtt) {
auto sendFunc = [&]() { auto sendFunc = [&]() {
conn.lossState.largestSent = currentLatest; conn.lossState.largestSent = currentLatest;
auto packet = makeTestingWritePacket( auto packet = makeTestingWritePacket(
conn.lossState.largestSent, conn.lossState.largestSent.value(),
conn.udpSendPacketLen, conn.udpSendPacketLen,
totalSent + conn.udpSendPacketLen); totalSent + conn.udpSendPacketLen);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
@@ -396,7 +396,7 @@ TEST_F(BbrTest, AckAggregation) {
auto sendAckGrow = [&](bool growFast) { auto sendAckGrow = [&](bool growFast) {
conn.lossState.largestSent = currentLatest; conn.lossState.largestSent = currentLatest;
auto packet = makeTestingWritePacket( auto packet = makeTestingWritePacket(
conn.lossState.largestSent, 1000, 1000 + totalSent); conn.lossState.largestSent.value(), 1000, 1000 + totalSent);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
totalSent += 1000; totalSent += 1000;
EXPECT_CALL(*rawBandwidthSampler, getBandwidth()) EXPECT_CALL(*rawBandwidthSampler, getBandwidth())
@@ -437,7 +437,7 @@ TEST_F(BbrTest, AckAggregation) {
conn.lossState.largestSent = currentLatest; conn.lossState.largestSent = currentLatest;
auto packet = makeTestingWritePacket( auto packet = makeTestingWritePacket(
conn.lossState.largestSent, 1000, 1000 + totalSent); conn.lossState.largestSent.value(), 1000, 1000 + totalSent);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
totalSent += 1000; totalSent += 1000;
auto ackEvent = makeAck(currentLatest, 1000, Clock::now(), packet.time); auto ackEvent = makeAck(currentLatest, 1000, Clock::now(), packet.time);
@@ -460,7 +460,7 @@ TEST_F(BbrTest, AckAggregation) {
// Another round, this time send something larger than currentMaxAckHeight: // Another round, this time send something larger than currentMaxAckHeight:
conn.lossState.largestSent = currentLatest; conn.lossState.largestSent = currentLatest;
auto packet1 = makeTestingWritePacket( auto packet1 = makeTestingWritePacket(
conn.lossState.largestSent, conn.lossState.largestSent.value(),
currentMaxAckHeight * 2 + 100, currentMaxAckHeight * 2 + 100,
currentMaxAckHeight * 2 + 100 + totalSent); currentMaxAckHeight * 2 + 100 + totalSent);
bbr.onPacketSent(packet1); bbr.onPacketSent(packet1);
@@ -481,9 +481,10 @@ TEST_F(BbrTest, AppLimited) {
auto rawBandwidthSampler = mockBandwidthSampler.get(); auto rawBandwidthSampler = mockBandwidthSampler.get();
bbr.setBandwidthSampler(std::move(mockBandwidthSampler)); bbr.setBandwidthSampler(std::move(mockBandwidthSampler));
auto packet = makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000); auto packet = makeTestingWritePacket(
conn.lossState.largestSent.value_or(0), 1000, 1000);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
conn.lossState.largestSent++; conn.lossState.largestSent = conn.lossState.largestSent.value_or(0) + 1;
EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1); EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
bbr.setAppLimited(); bbr.setAppLimited();
EXPECT_CALL(*rawBandwidthSampler, isAppLimited()) EXPECT_CALL(*rawBandwidthSampler, isAppLimited())
@@ -504,7 +505,8 @@ TEST_F(BbrTest, AppLimitedIgnored) {
EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1); EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
bbr.setAppLimited(); bbr.setAppLimited();
auto packet = makeTestingWritePacket( auto packet = makeTestingWritePacket(
conn.lossState.largestSent++, 1000, 1000 + inflightBytes); conn.lossState.largestSent.value_or(0), 1000, 1000 + inflightBytes);
conn.lossState.largestSent = conn.lossState.largestSent.value_or(0) + 1;
inflightBytes += 1000; inflightBytes += 1000;
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
} }
@@ -526,11 +528,16 @@ TEST_F(BbrTest, ExtendMinRttExpiration) {
EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1); EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
bbr.setAppLimited(); bbr.setAppLimited();
auto packet = makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000); auto packet = makeTestingWritePacket(
conn.lossState.largestSent.value_or(0), 1000, 1000);
bbr.onPacketSent(packet); bbr.onPacketSent(packet);
EXPECT_CALL(*rawRttSampler, timestampMinRtt(_)).Times(1); EXPECT_CALL(*rawRttSampler, timestampMinRtt(_)).Times(1);
bbr.onPacketAckOrLoss( bbr.onPacketAckOrLoss(
makeAck(conn.lossState.largestSent, 1000, Clock::now(), packet.time), makeAck(
conn.lossState.largestSent.value_or(0),
1000,
Clock::now(),
packet.time),
folly::none); folly::none);
} }

4
quic/loss/QuicLossFunctions.cpp
View File

@@ -36,7 +36,7 @@ void onPTOAlarm(QuicConnectionStateBase& conn) {
QUIC_TRACE( QUIC_TRACE(
pto_alarm, pto_alarm,
conn, conn,
conn.lossState.largestSent, conn.lossState.largestSent.value_or(0),
conn.lossState.ptoCount, conn.lossState.ptoCount,
(uint64_t)conn.outstandingPackets.size()); (uint64_t)conn.outstandingPackets.size());
QUIC_STATS(conn.statsCallback, onPTO); QUIC_STATS(conn.statsCallback, onPTO);
@@ -44,7 +44,7 @@ void onPTOAlarm(QuicConnectionStateBase& conn) {
conn.lossState.totalPTOCount++; conn.lossState.totalPTOCount++;
if (conn.qLogger) { if (conn.qLogger) {
conn.qLogger->addLossAlarm( conn.qLogger->addLossAlarm(
conn.lossState.largestSent, conn.lossState.largestSent.value_or(0),
conn.lossState.ptoCount, conn.lossState.ptoCount,
(uint64_t)conn.outstandingPackets.size(), (uint64_t)conn.outstandingPackets.size(),
kPtoAlarm); kPtoAlarm);

16
quic/loss/QuicLossFunctions.h
View File

@@ -189,7 +189,7 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
template <class LossVisitor> template <class LossVisitor>
folly::Optional<CongestionController::LossEvent> detectLossPackets( folly::Optional<CongestionController::LossEvent> detectLossPackets(
QuicConnectionStateBase& conn, QuicConnectionStateBase& conn,
PacketNum largestAcked, folly::Optional<PacketNum> largestAcked,
const LossVisitor& lossVisitor, const LossVisitor& lossVisitor,
TimePoint lossTime, TimePoint lossTime,
PacketNumberSpace pnSpace) { PacketNumberSpace pnSpace) {
@@ -199,7 +199,7 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
conn.transportSettings.timeReorderingThreshDividend / conn.transportSettings.timeReorderingThreshDividend /
conn.transportSettings.timeReorderingThreshDivisor; conn.transportSettings.timeReorderingThreshDivisor;
VLOG(10) << __func__ << " outstanding=" << conn.outstandingPackets.size() VLOG(10) << __func__ << " outstanding=" << conn.outstandingPackets.size()
<< " largestAcked=" << largestAcked << " largestAcked=" << largestAcked.value_or(0)
<< " delayUntilLost=" << delayUntilLost.count() << "us" << " delayUntilLost=" << delayUntilLost.count() << "us"
<< " " << conn; << " " << conn;
CongestionController::LossEvent lossEvent(lossTime); CongestionController::LossEvent lossEvent(lossTime);
@@ -209,7 +209,7 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
while (iter != conn.outstandingPackets.end()) { while (iter != conn.outstandingPackets.end()) {
auto& pkt = *iter; auto& pkt = *iter;
auto currentPacketNum = pkt.packet.header.getPacketSequenceNum(); auto currentPacketNum = pkt.packet.header.getPacketSequenceNum();
if (currentPacketNum >= largestAcked) { if (!largestAcked.has_value() || currentPacketNum >= *largestAcked) {
break; break;
} }
auto currentPacketNumberSpace = pkt.packet.header.getPacketNumberSpace(); auto currentPacketNumberSpace = pkt.packet.header.getPacketNumberSpace();
@@ -219,7 +219,7 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
} }
bool lost = (lossTime - pkt.time) > delayUntilLost; bool lost = (lossTime - pkt.time) > delayUntilLost;
lost = lost || lost = lost ||
(largestAcked - currentPacketNum) > conn.lossState.reorderingThreshold; (*largestAcked - currentPacketNum) > conn.lossState.reorderingThreshold;
if (!lost) { if (!lost) {
// We can exit early here because if packet N doesn't meet the // We can exit early here because if packet N doesn't meet the
// threshold, then packet N + 1 will not either. // threshold, then packet N + 1 will not either.
@@ -306,13 +306,13 @@ void onHandshakeAlarm(
QUIC_TRACE( QUIC_TRACE(
handshake_alarm, handshake_alarm,
conn, conn,
conn.lossState.largestSent, conn.lossState.largestSent.value_or(0),
conn.lossState.handshakeAlarmCount, conn.lossState.handshakeAlarmCount,
(uint64_t)conn.outstandingHandshakePacketsCount, (uint64_t)conn.outstandingHandshakePacketsCount,
(uint64_t)conn.outstandingPackets.size()); (uint64_t)conn.outstandingPackets.size());
if (conn.qLogger) { if (conn.qLogger) {
conn.qLogger->addLossAlarm( conn.qLogger->addLossAlarm(
conn.lossState.largestSent, conn.lossState.largestSent.value_or(0),
conn.lossState.handshakeAlarmCount, conn.lossState.handshakeAlarmCount,
(uint64_t)conn.outstandingPackets.size(), (uint64_t)conn.outstandingPackets.size(),
kHandshakeAlarm); kHandshakeAlarm);
@@ -416,7 +416,9 @@ folly::Optional<CongestionController::LossEvent> handleAckForLoss(
// doesn't ack anything that's in OP list? // doesn't ack anything that's in OP list?
conn.lossState.ptoCount = 0; conn.lossState.ptoCount = 0;
conn.lossState.handshakeAlarmCount = 0; conn.lossState.handshakeAlarmCount = 0;
largestAcked = std::max(largestAcked, *ack.largestAckedPacket); largestAcked = std::max<PacketNum>(
largestAcked.value_or(*ack.largestAckedPacket),
*ack.largestAckedPacket);
} }
auto lossEvent = detectLossPackets( auto lossEvent = detectLossPackets(
conn, conn,

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

@@ -172,7 +172,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
conn.udpSendPacketLen, conn.udpSendPacketLen,
std::move(*header), std::move(*header),
getAckState(conn, packetNumberSpace).largestAckedByPeer); getAckState(conn, packetNumberSpace).largestAckedByPeer.value_or(0));
builder.encodePacketHeader(); builder.encodePacketHeader();
EXPECT_TRUE(builder.canBuildPacket()); EXPECT_TRUE(builder.canBuildPacket());
auto packet = std::move(builder).buildPacket(); auto packet = std::move(builder).buildPacket();
@@ -216,7 +216,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
conn.lossState.largestSent = getNextPacketNum(conn, packetNumberSpace); conn.lossState.largestSent = getNextPacketNum(conn, packetNumberSpace);
increaseNextPacketNum(conn, packetNumberSpace); increaseNextPacketNum(conn, packetNumberSpace);
conn.pendingEvents.setLossDetectionAlarm = true; conn.pendingEvents.setLossDetectionAlarm = true;
return conn.lossState.largestSent; return conn.lossState.largestSent.value();
} }
TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) { TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) {
@@ -713,9 +713,10 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckedPacket) {
sendPacket(*conn, TimePoint(), folly::none, PacketType::OneRtt); sendPacket(*conn, TimePoint(), folly::none, PacketType::OneRtt);
ReadAckFrame ackFrame; ReadAckFrame ackFrame;
ackFrame.largestAcked = conn->lossState.largestSent; ackFrame.largestAcked = conn->lossState.largestSent.value_or(0);
ackFrame.ackBlocks.emplace_back( ackFrame.ackBlocks.emplace_back(
conn->lossState.largestSent, conn->lossState.largestSent); conn->lossState.largestSent.value_or(0),
conn->lossState.largestSent.value_or(0));
bool testLossMarkFuncCalled = false; bool testLossMarkFuncCalled = false;
auto testLossMarkFunc = [&](auto& /* conn */, auto&, bool, PacketNum) { auto testLossMarkFunc = [&](auto& /* conn */, auto&, bool, PacketNum) {

6
quic/server/test/QuicServerTransportTest.cpp
View File

@@ -1720,7 +1720,8 @@ TEST_F(QuicServerTransportTest, StopSendingLoss) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
server->getConn().udpSendPacketLen, server->getConn().udpSendPacketLen,
std::move(header), std::move(header),
server->getConn().ackStates.appDataAckState.largestAckedByPeer); server->getConn().ackStates.appDataAckState.largestAckedByPeer.value_or(
0));
builder.encodePacketHeader(); builder.encodePacketHeader();
StopSendingFrame stopSendingFrame( StopSendingFrame stopSendingFrame(
streamId, GenericApplicationErrorCode::UNKNOWN); streamId, GenericApplicationErrorCode::UNKNOWN);
@@ -1751,7 +1752,8 @@ TEST_F(QuicServerTransportTest, StopSendingLossAfterStreamClosed) {
RegularQuicPacketBuilder builder( RegularQuicPacketBuilder builder(
server->getConn().udpSendPacketLen, server->getConn().udpSendPacketLen,
std::move(header), std::move(header),
server->getConn().ackStates.appDataAckState.largestAckedByPeer); server->getConn().ackStates.appDataAckState.largestAckedByPeer.value_or(
0));
builder.encodePacketHeader(); builder.encodePacketHeader();
StopSendingFrame stopSendingFrame( StopSendingFrame stopSendingFrame(
streamId, GenericApplicationErrorCode::UNKNOWN); streamId, GenericApplicationErrorCode::UNKNOWN);

3
quic/state/AckStates.h
View File

@@ -31,8 +31,7 @@ struct AckState {
// The receive time of the largest ack packet // The receive time of the largest ack packet
folly::Optional<TimePoint> largestRecvdPacketTime; folly::Optional<TimePoint> largestRecvdPacketTime;
// Latest packet number acked by peer // Latest packet number acked by peer
// TODO: 0 is a legit PacketNum now, we need to make this optional: folly::Optional<PacketNum> largestAckedByPeer;
PacketNum largestAckedByPeer{0};
// Largest received packet numbers on the connection. // Largest received packet numbers on the connection.
folly::Optional<PacketNum> largestReceivedPacketNum; folly::Optional<PacketNum> largestReceivedPacketNum;
// Largest received packet number at the time we sent our last close message. // Largest received packet number at the time we sent our last close message.

3
quic/state/StateData.h
View File

@@ -447,9 +447,8 @@ struct LossState {
// The time when last handshake packet was sent // The time when last handshake packet was sent
TimePoint lastHandshakePacketSentTime; TimePoint lastHandshakePacketSentTime;
// Latest packet number sent // Latest packet number sent
// TODO: 0 is a legit PacketNum now, we need to make this optional:
// TODO: this also needs to be 3 numbers now... // TODO: this also needs to be 3 numbers now...
PacketNum largestSent{0}; folly::Optional<PacketNum> largestSent;
// Reordering threshold used // Reordering threshold used
uint32_t reorderingThreshold{kReorderingThreshold}; uint32_t reorderingThreshold{kReorderingThreshold};
// Timer for time reordering detection or early retransmit alarm. // Timer for time reordering detection or early retransmit alarm.

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

@@ -400,7 +400,7 @@ TEST_P(AckHandlersTest, NoNewAckedPacket) {
Clock::now()); Clock::now());
EXPECT_TRUE(conn.pendingEvents.setLossDetectionAlarm); EXPECT_TRUE(conn.pendingEvents.setLossDetectionAlarm);
EXPECT_EQ(conn.lossState.ptoCount, 1); EXPECT_EQ(conn.lossState.ptoCount, 1);
EXPECT_EQ(conn.ackStates.appDataAckState.largestAckedByPeer, 0); EXPECT_TRUE(!conn.ackStates.appDataAckState.largestAckedByPeer.has_value());
} }
TEST_P(AckHandlersTest, LossByAckedRecovered) { TEST_P(AckHandlersTest, LossByAckedRecovered) {