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

Back out "Send probes for all spaces."

Browse Source 
Summary: As in title. There's a bug here somewhere with empty write loops we need to find.

Reviewed By: yangchi

Differential Revision: D27279100

fbshipit-source-id: e1d26fbf8d6df1590d464a6504a8b940b46794e0
This commit is contained in:
Matt Joras
2021-03-23 16:08:36 -07:00
committed by Facebook GitHub Bot
parent bceb00346b
commit a92a24bdd5
18 changed files with 174 additions and 277 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
quic/api/QuicTransportBase.cpp
View File

@@ -458,8 +458,9 @@ void QuicTransportBase::closeImpl(
// Don't need outstanding packets. // Don't need outstanding packets.
conn_->outstandings.packets.clear(); conn_->outstandings.packets.clear();
conn_->outstandings.packetCount = {}; conn_->outstandings.initialPacketsCount = 0;
conn_->outstandings.clonedPacketCount = {}; conn_->outstandings.handshakePacketsCount = 0;
conn_->outstandings.clonedPacketsCount = 0;
// We don't need no congestion control. // We don't need no congestion control.
conn_->congestionController = nullptr; conn_->congestionController = nullptr;

44
quic/api/QuicTransportFunctions.cpp
View File

@@ -109,9 +109,7 @@ uint64_t writeQuicDataToSocketImpl(
// add a flag to the Scheduler to control the priority between them and see // add a flag to the Scheduler to control the priority between them and see
// which way is better. // which way is better.
uint64_t written = 0; uint64_t written = 0;
auto& numProbePackets = if (connection.pendingEvents.numProbePackets) {
connection.pendingEvents.numProbePackets[PacketNumberSpace::AppData];
if (numProbePackets) {
auto probeSchedulerBuilder = auto probeSchedulerBuilder =
FrameScheduler::Builder( FrameScheduler::Builder(
connection, connection,
@@ -137,12 +135,13 @@ uint64_t writeQuicDataToSocketImpl(
EncryptionLevel::AppData, EncryptionLevel::AppData,
PacketNumberSpace::AppData, PacketNumberSpace::AppData,
probeScheduler, probeScheduler,
std::min<uint64_t>(packetLimit, numProbePackets), std::min<uint64_t>(
packetLimit, connection.pendingEvents.numProbePackets),
aead, aead,
headerCipher, headerCipher,
version); version);
CHECK_GE(numProbePackets, written); CHECK_GE(connection.pendingEvents.numProbePackets, written);
numProbePackets -= written; connection.pendingEvents.numProbePackets -= written;
} }
auto schedulerBuilder = auto schedulerBuilder =
FrameScheduler::Builder( FrameScheduler::Builder(
@@ -827,13 +826,26 @@ void updateConnection(
(conn.pendingEvents.pathChallenge || conn.outstandingPathValidation)) { (conn.pendingEvents.pathChallenge || conn.outstandingPathValidation)) {
conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize); conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize);
} }
if (pkt.metadata.isHandshake) {
if (!pkt.associatedEvent) {
if (packetNumberSpace == PacketNumberSpace::Initial) {
++conn.outstandings.initialPacketsCount;
} else {
CHECK_EQ(packetNumberSpace, PacketNumberSpace::Handshake);
++conn.outstandings.handshakePacketsCount;
}
}
}
conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time; conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time;
if (pkt.associatedEvent) { if (pkt.associatedEvent) {
++conn.outstandings.clonedPacketCount[packetNumberSpace]; ++conn.outstandings.clonedPacketsCount;
++conn.lossState.timeoutBasedRtxCount; ++conn.lossState.timeoutBasedRtxCount;
} else {
++conn.outstandings.packetCount[packetNumberSpace];
} }
auto opCount = conn.outstandings.numOutstanding();
DCHECK_GE(opCount, conn.outstandings.initialPacketsCount);
DCHECK_GE(opCount, conn.outstandings.handshakePacketsCount);
DCHECK_GE(opCount, conn.outstandings.clonedPacketsCount);
} }
uint64_t congestionControlWritableBytes(const QuicConnectionStateBase& conn) { uint64_t congestionControlWritableBytes(const QuicConnectionStateBase& conn) {
@@ -924,10 +936,7 @@ uint64_t writeCryptoAndAckDataToSocket(
uint64_t written = 0; uint64_t written = 0;
auto& cryptoStream = auto& cryptoStream =
*getCryptoStream(*connection.cryptoState, encryptionLevel); *getCryptoStream(*connection.cryptoState, encryptionLevel);
auto& numProbePackets = if (connection.pendingEvents.numProbePackets &&
connection.pendingEvents
.numProbePackets[LongHeader::typeToPacketNumberSpace(packetType)];
if (numProbePackets &&
(cryptoStream.retransmissionBuffer.size() || scheduler.hasData())) { (cryptoStream.retransmissionBuffer.size() || scheduler.hasData())) {
written = writeProbingDataToSocket( written = writeProbingDataToSocket(
sock, sock,
@@ -938,13 +947,14 @@ uint64_t writeCryptoAndAckDataToSocket(
encryptionLevel, encryptionLevel,
LongHeader::typeToPacketNumberSpace(packetType), LongHeader::typeToPacketNumberSpace(packetType),
scheduler, scheduler,
std::min<uint64_t>(packetLimit, numProbePackets), std::min<uint64_t>(
packetLimit, connection.pendingEvents.numProbePackets),
cleartextCipher, cleartextCipher,
headerCipher, headerCipher,
version, version,
token); token);
CHECK_GE(numProbePackets, written); CHECK_GE(connection.pendingEvents.numProbePackets, written);
numProbePackets -= written; connection.pendingEvents.numProbePackets -= written;
} }
// Crypto data is written without aead protection. // Crypto data is written without aead protection.
written += writeConnectionDataToSocket( written += writeConnectionDataToSocket(
@@ -1467,7 +1477,7 @@ uint64_t writeD6DProbeToSocket(
} }
WriteDataReason shouldWriteData(const QuicConnectionStateBase& conn) { WriteDataReason shouldWriteData(const QuicConnectionStateBase& conn) {
if (conn.pendingEvents.anyProbePackets()) { if (conn.pendingEvents.numProbePackets) {
VLOG(10) << nodeToString(conn.nodeType) << " needs write because of PTO" VLOG(10) << nodeToString(conn.nodeType) << " needs write because of PTO"
<< conn; << conn;
return WriteDataReason::PROBES; return WriteDataReason::PROBES;

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

@@ -29,7 +29,8 @@ enum PacketBuilderType { Regular, Inplace };
namespace { namespace {
PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) { PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) {
PacketNum nextPacketNum = getNextPacketNum(conn, PacketNumberSpace::Initial); PacketNum nextPacketNum =
getNextPacketNum(conn, PacketNumberSpace::Handshake);
std::vector<uint8_t> zeroConnIdData(quic::kDefaultConnectionIdSize, 0); std::vector<uint8_t> zeroConnIdData(quic::kDefaultConnectionIdSize, 0);
ConnectionId srcConnId(zeroConnIdData); ConnectionId srcConnId(zeroConnIdData);
LongHeader header( LongHeader header(
@@ -41,8 +42,8 @@ PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) {
RegularQuicWritePacket packet(std::move(header)); RegularQuicWritePacket packet(std::move(header));
conn.outstandings.packets.emplace_back( conn.outstandings.packets.emplace_back(
packet, Clock::now(), 0, true, 0, 0, 0, LossState(), 0); packet, Clock::now(), 0, true, 0, 0, 0, LossState(), 0);
conn.outstandings.packetCount[PacketNumberSpace::Initial]++; conn.outstandings.handshakePacketsCount++;
increaseNextPacketNum(conn, PacketNumberSpace::Initial); increaseNextPacketNum(conn, PacketNumberSpace::Handshake);
return nextPacketNum; return nextPacketNum;
} }
@@ -60,7 +61,7 @@ PacketNum addHandshakeOutstandingPacket(QuicConnectionStateBase& conn) {
RegularQuicWritePacket packet(std::move(header)); RegularQuicWritePacket packet(std::move(header));
conn.outstandings.packets.emplace_back( conn.outstandings.packets.emplace_back(
packet, Clock::now(), 0, true, 0, 0, 0, LossState(), 0); packet, Clock::now(), 0, true, 0, 0, 0, LossState(), 0);
conn.outstandings.packetCount[PacketNumberSpace::Handshake]++; conn.outstandings.handshakePacketsCount++;
increaseNextPacketNum(conn, PacketNumberSpace::Handshake); increaseNextPacketNum(conn, PacketNumberSpace::Handshake);
return nextPacketNum; return nextPacketNum;
} }

8
quic/api/test/QuicTransportBaseTest.cpp
View File

@@ -1309,13 +1309,9 @@ TEST_F(QuicTransportImplTest, CloseStreamAfterReadFin) {
} }
TEST_F(QuicTransportImplTest, CloseTransportCleansupOutstandingCounters) { TEST_F(QuicTransportImplTest, CloseTransportCleansupOutstandingCounters) {
transport->transportConn->outstandings transport->transportConn->outstandings.handshakePacketsCount = 200;
.packetCount[PacketNumberSpace::Handshake] = 200;
transport->closeNow(folly::none); transport->closeNow(folly::none);
EXPECT_EQ( EXPECT_EQ(0, transport->transportConn->outstandings.handshakePacketsCount);
0,
transport->transportConn->outstandings
.packetCount[PacketNumberSpace::Handshake]);
} }
TEST_F(QuicTransportImplTest, DeliveryCallbackUnsetAll) { TEST_F(QuicTransportImplTest, DeliveryCallbackUnsetAll) {

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

@@ -628,7 +628,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPureAckCounter) {
conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client); conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client);
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
writeDataToQuicStream(*stream, nullptr, true); writeDataToQuicStream(*stream, nullptr, true);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
auto packetEncodedSize = auto packetEncodedSize =
@@ -738,8 +738,8 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(1, conn->outstandings.initialPacketsCount);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(1, conn->outstandings.packets.size()); EXPECT_EQ(1, conn->outstandings.packets.size());
EXPECT_EQ(1, initialStream->retransmissionBuffer.size()); EXPECT_EQ(1, initialStream->retransmissionBuffer.size());
@@ -757,8 +757,8 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(2, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(2, conn->outstandings.initialPacketsCount);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(2, conn->outstandings.packets.size()); EXPECT_EQ(2, conn->outstandings.packets.size());
EXPECT_EQ(3, initialStream->retransmissionBuffer.size()); EXPECT_EQ(3, initialStream->retransmissionBuffer.size());
EXPECT_TRUE(initialStream->writeBuffer.empty()); EXPECT_TRUE(initialStream->writeBuffer.empty());
@@ -770,7 +770,7 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
initialStream->retransmissionBuffer.erase(0); initialStream->retransmissionBuffer.erase(0);
initialStream->lossBuffer.emplace_back(std::move(firstBuf), 0, false); initialStream->lossBuffer.emplace_back(std::move(firstBuf), 0, false);
conn->outstandings.packets.pop_front(); conn->outstandings.packets.pop_front();
conn->outstandings.packetCount[PacketNumberSpace::Initial]--; conn->outstandings.initialPacketsCount--;
auto handshakeStream = auto handshakeStream =
getCryptoStream(*conn->cryptoState, EncryptionLevel::Handshake); getCryptoStream(*conn->cryptoState, EncryptionLevel::Handshake);
@@ -787,8 +787,8 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(1, conn->outstandings.initialPacketsCount);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(2, conn->outstandings.packets.size()); EXPECT_EQ(2, conn->outstandings.packets.size());
EXPECT_EQ(1, handshakeStream->retransmissionBuffer.size()); EXPECT_EQ(1, handshakeStream->retransmissionBuffer.size());
@@ -803,8 +803,8 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(1, conn->outstandings.initialPacketsCount);
EXPECT_EQ(2, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(2, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(3, conn->outstandings.packets.size()); EXPECT_EQ(3, conn->outstandings.packets.size());
EXPECT_EQ(2, handshakeStream->retransmissionBuffer.size()); EXPECT_EQ(2, handshakeStream->retransmissionBuffer.size());
EXPECT_TRUE(handshakeStream->writeBuffer.empty()); EXPECT_TRUE(handshakeStream->writeBuffer.empty());
@@ -820,19 +820,19 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) {
auto frame = op.packet.frames[0].asWriteCryptoFrame(); auto frame = op.packet.frames[0].asWriteCryptoFrame();
EXPECT_EQ(frame->offset, 0); EXPECT_EQ(frame->offset, 0);
conn->outstandings.packets.pop_front(); conn->outstandings.packets.pop_front();
conn->outstandings.packetCount[PacketNumberSpace::Handshake]--; conn->outstandings.handshakePacketsCount--;
implicitAckCryptoStream(*conn, EncryptionLevel::Initial); implicitAckCryptoStream(*conn, EncryptionLevel::Initial);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(0, conn->outstandings.initialPacketsCount);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(1, conn->outstandings.packets.size()); EXPECT_EQ(1, conn->outstandings.packets.size());
EXPECT_TRUE(initialStream->retransmissionBuffer.empty()); EXPECT_TRUE(initialStream->retransmissionBuffer.empty());
EXPECT_TRUE(initialStream->writeBuffer.empty()); EXPECT_TRUE(initialStream->writeBuffer.empty());
EXPECT_TRUE(initialStream->lossBuffer.empty()); EXPECT_TRUE(initialStream->lossBuffer.empty());
implicitAckCryptoStream(*conn, EncryptionLevel::Handshake); implicitAckCryptoStream(*conn, EncryptionLevel::Handshake);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(0, conn->outstandings.initialPacketsCount);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
EXPECT_TRUE(conn->outstandings.packets.empty()); EXPECT_TRUE(conn->outstandings.packets.empty());
EXPECT_TRUE(handshakeStream->retransmissionBuffer.empty()); EXPECT_TRUE(handshakeStream->retransmissionBuffer.empty());
EXPECT_TRUE(handshakeStream->writeBuffer.empty()); EXPECT_TRUE(handshakeStream->writeBuffer.empty());
@@ -844,7 +844,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) {
conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client); conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client);
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
writeDataToQuicStream(*stream, nullptr, true); writeDataToQuicStream(*stream, nullptr, true);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
auto packetEncodedSize = auto packetEncodedSize =
@@ -859,7 +859,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) {
TimePoint(), TimePoint(),
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount);
auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
packetEncodedSize = packetEncodedSize =
@@ -907,8 +907,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) {
EXPECT_EQ(gotFrame->offset, 0); EXPECT_EQ(gotFrame->offset, 0);
EXPECT_EQ(gotFrame->len, 0); EXPECT_EQ(gotFrame->len, 0);
EXPECT_TRUE(gotFrame->fin); EXPECT_TRUE(gotFrame->fin);
EXPECT_EQ( EXPECT_EQ(1, conn->outstandings.handshakePacketsCount);
1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]);
} }
} }
} }
@@ -918,7 +917,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionForOneRttCryptoData) {
conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client); conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::Client);
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
writeDataToQuicStream(*stream, nullptr, true); writeDataToQuicStream(*stream, nullptr, true);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
// Packet with CryptoFrame in AppData pn space // Packet with CryptoFrame in AppData pn space
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData, true); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData, true);
@@ -935,7 +934,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionForOneRttCryptoData) {
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(1, conn->outstandings.packets.size()); EXPECT_EQ(1, conn->outstandings.packets.size());
auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
@@ -987,7 +986,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionForOneRttCryptoData) {
} }
} }
EXPECT_EQ(0, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount);
EXPECT_EQ(2, conn->outstandings.packets.size()); EXPECT_EQ(2, conn->outstandings.packets.size());
} }
@@ -1371,9 +1370,7 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
conn->transportSettings.writeConnectionDataPacketsLimit)); conn->transportSettings.writeConnectionDataPacketsLimit));
// Normal limit // Normal limit
conn->pendingEvents.numProbePackets[PacketNumberSpace::Initial] = 0; conn->pendingEvents.numProbePackets = 0;
conn->pendingEvents.numProbePackets[PacketNumberSpace::Handshake] = 0;
conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] = 0;
conn->transportSettings.writeConnectionDataPacketsLimit = conn->transportSettings.writeConnectionDataPacketsLimit =
kDefaultWriteConnectionDataPacketLimit; kDefaultWriteConnectionDataPacketLimit;
EXPECT_CALL(*rawSocket, write(_, _)) EXPECT_CALL(*rawSocket, write(_, _))
@@ -1398,7 +1395,7 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
conn->transportSettings.writeConnectionDataPacketsLimit)); conn->transportSettings.writeConnectionDataPacketsLimit));
// Probing can be limited by packet limit too // Probing can be limited by packet limit too
conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] = conn->pendingEvents.numProbePackets =
kDefaultWriteConnectionDataPacketLimit * 2; kDefaultWriteConnectionDataPacketLimit * 2;
writeDataToQuicStream(*stream1, buf->clone(), true); writeDataToQuicStream(*stream1, buf->clone(), true);
writableBytes = 10000; writableBytes = 10000;
@@ -1873,9 +1870,7 @@ TEST_F(QuicTransportFunctionsTest, NoCryptoProbeWriteIfNoProbeCredit) {
ASSERT_EQ(1, cryptoStream->retransmissionBuffer.size()); ASSERT_EQ(1, cryptoStream->retransmissionBuffer.size());
ASSERT_TRUE(cryptoStream->writeBuffer.empty()); ASSERT_TRUE(cryptoStream->writeBuffer.empty());
conn->pendingEvents.numProbePackets[PacketNumberSpace::Initial] = 0; conn->pendingEvents.numProbePackets = 0;
conn->pendingEvents.numProbePackets[PacketNumberSpace::Handshake] = 0;
conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] = 0;
EXPECT_EQ( EXPECT_EQ(
0, 0,
writeCryptoAndAckDataToSocket( writeCryptoAndAckDataToSocket(
@@ -2180,10 +2175,9 @@ TEST_F(QuicTransportFunctionsTest, HasAppDataToWrite) {
EXPECT_EQ(WriteDataReason::STREAM, hasNonAckDataToWrite(*conn)); EXPECT_EQ(WriteDataReason::STREAM, hasNonAckDataToWrite(*conn));
} }
TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterAppData) { TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounter) {
auto conn = createConn(); auto conn = createConn();
ASSERT_EQ( ASSERT_EQ(0, conn->outstandings.clonedPacketsCount);
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
auto connWindowUpdate = auto connWindowUpdate =
MaxDataFrame(conn->flowControlState.advertisedMaxOffset); MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
@@ -2198,64 +2192,7 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterAppData) {
TimePoint(), TimePoint(),
123, 123,
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_EQ( EXPECT_EQ(1, conn->outstandings.clonedPacketsCount);
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Initial]);
EXPECT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Handshake]);
EXPECT_EQ(
1, conn->outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
}
TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterHandshake) {
auto conn = createConn();
ASSERT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Handshake]);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
auto connWindowUpdate =
MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
conn->pendingEvents.connWindowUpdate = true;
packet.packet.frames.emplace_back(connWindowUpdate);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100);
conn->outstandings.packetEvents.insert(packetEvent);
updateConnection(
*conn,
packetEvent,
packet.packet,
TimePoint(),
123,
false /* isDSRPacket */);
EXPECT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Initial]);
EXPECT_EQ(
1, conn->outstandings.clonedPacketCount[PacketNumberSpace::Handshake]);
EXPECT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
}
TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounterInitial) {
auto conn = createConn();
ASSERT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Initial]);
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Initial);
auto connWindowUpdate =
MaxDataFrame(conn->flowControlState.advertisedMaxOffset);
conn->pendingEvents.connWindowUpdate = true;
packet.packet.frames.emplace_back(connWindowUpdate);
PacketEvent packetEvent(PacketNumberSpace::AppData, 100);
conn->outstandings.packetEvents.insert(packetEvent);
updateConnection(
*conn,
packetEvent,
packet.packet,
TimePoint(),
123,
false /* isDSRPacket */);
EXPECT_EQ(
1, conn->outstandings.clonedPacketCount[PacketNumberSpace::Initial]);
EXPECT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::Handshake]);
EXPECT_EQ(
0, conn->outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
} }
TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) { TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) {
@@ -2274,7 +2211,7 @@ TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) {
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_FALSE(conn->streamManager->hasBlocked()); EXPECT_FALSE(conn->streamManager->hasBlocked());
EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_FALSE(conn->outstandings.packets.empty());
EXPECT_EQ(0, conn->outstandings.numClonedPackets()); EXPECT_EQ(0, conn->outstandings.clonedPacketsCount);
} }
TEST_F(QuicTransportFunctionsTest, ClonedBlocked) { TEST_F(QuicTransportFunctionsTest, ClonedBlocked) {
@@ -2296,8 +2233,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedBlocked) {
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_FALSE(conn->outstandings.packets.empty());
EXPECT_EQ( EXPECT_EQ(1, conn->outstandings.clonedPacketsCount);
1, conn->outstandings.clonedPacketCount[PacketNumberSpace::AppData]);
} }
TEST_F(QuicTransportFunctionsTest, TwoConnWindowUpdateWillCrash) { TEST_F(QuicTransportFunctionsTest, TwoConnWindowUpdateWillCrash) {
@@ -2354,7 +2290,7 @@ TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) {
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_TRUE(conn->pendingEvents.resets.empty()); EXPECT_TRUE(conn->pendingEvents.resets.empty());
EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_FALSE(conn->outstandings.packets.empty());
EXPECT_EQ(0, conn->outstandings.numClonedPackets()); EXPECT_EQ(0, conn->outstandings.clonedPacketsCount);
} }
TEST_F(QuicTransportFunctionsTest, ClonedRst) { TEST_F(QuicTransportFunctionsTest, ClonedRst) {
@@ -2377,7 +2313,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) {
getEncodedSize(packet), getEncodedSize(packet),
false /* isDSRPacket */); false /* isDSRPacket */);
EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_FALSE(conn->outstandings.packets.empty());
EXPECT_EQ(1, conn->outstandings.numClonedPackets()); EXPECT_EQ(1, conn->outstandings.clonedPacketsCount);
} }
TEST_F(QuicTransportFunctionsTest, TotalBytesSentUpdate) { TEST_F(QuicTransportFunctionsTest, TotalBytesSentUpdate) {
@@ -2582,7 +2518,7 @@ TEST_F(QuicTransportFunctionsTest, ProbeWriteNewFunctionalFrames) {
conn->transportSettings.writeConnectionDataPacketsLimit); conn->transportSettings.writeConnectionDataPacketsLimit);
ASSERT_EQ(1, stream->retransmissionBuffer.size()); ASSERT_EQ(1, stream->retransmissionBuffer.size());
conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] = 1; conn->pendingEvents.numProbePackets = 1;
conn->flowControlState.windowSize *= 2; conn->flowControlState.windowSize *= 2;
conn->flowControlState.timeOfLastFlowControlUpdate = Clock::now() - 20s; conn->flowControlState.timeOfLastFlowControlUpdate = Clock::now() - 20s;
maybeSendConnWindowUpdate(*conn, Clock::now()); maybeSendConnWindowUpdate(*conn, Clock::now());

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

@@ -229,7 +229,7 @@ TEST_F(QuicTransportTest, WriteDataWithProbing) {
auto streamId = transport_->createBidirectionalStream().value(); auto streamId = transport_->createBidirectionalStream().value();
auto buf = buildRandomInputData(kDefaultUDPSendPacketLen * 2); auto buf = buildRandomInputData(kDefaultUDPSendPacketLen * 2);
conn.pendingEvents.numProbePackets[PacketNumberSpace::AppData] = 1; conn.pendingEvents.numProbePackets = 1;
// Probing won't ask about getWritableBytes. Then regular write may ask // Probing won't ask about getWritableBytes. Then regular write may ask
// multiple times: // multiple times:
int getWritableBytesCounter = 0; int getWritableBytesCounter = 0;
@@ -255,7 +255,7 @@ TEST_F(QuicTransportTest, WriteDataWithProbing) {
transport_->writeChain(streamId, buf->clone(), true); transport_->writeChain(streamId, buf->clone(), true);
loopForWrites(); loopForWrites();
// Pending numProbePackets is cleared: // Pending numProbePackets is cleared:
EXPECT_EQ(0, conn.pendingEvents.numProbePackets[PacketNumberSpace::AppData]); EXPECT_EQ(0, conn.pendingEvents.numProbePackets);
transport_->close(folly::none); transport_->close(folly::none);
} }
@@ -1114,7 +1114,7 @@ TEST_F(QuicTransportTest, SendPathValidationWhileThereIsOutstandingOne) {
TEST_F(QuicTransportTest, ClonePathChallenge) { TEST_F(QuicTransportTest, ClonePathChallenge) {
auto& conn = transport_->getConnectionState(); auto& conn = transport_->getConnectionState();
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
conn.outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn.outstandings.handshakePacketsCount = 0;
conn.outstandings.packets.clear(); conn.outstandings.packets.clear();
for (auto& t : conn.lossState.lossTimes) { for (auto& t : conn.lossState.lossTimes) {
t.reset(); t.reset();
@@ -1148,7 +1148,7 @@ TEST_F(QuicTransportTest, ClonePathChallenge) {
TEST_F(QuicTransportTest, OnlyClonePathValidationIfOutstanding) { TEST_F(QuicTransportTest, OnlyClonePathValidationIfOutstanding) {
auto& conn = transport_->getConnectionState(); auto& conn = transport_->getConnectionState();
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
conn.outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn.outstandings.handshakePacketsCount = 0;
conn.outstandings.packets.clear(); conn.outstandings.packets.clear();
for (auto& t : conn.lossState.lossTimes) { for (auto& t : conn.lossState.lossTimes) {
t.reset(); t.reset();
@@ -1290,7 +1290,7 @@ TEST_F(QuicTransportTest, CloneAfterRecvReset) {
TEST_F(QuicTransportTest, ClonePathResponse) { TEST_F(QuicTransportTest, ClonePathResponse) {
auto& conn = transport_->getConnectionState(); auto& conn = transport_->getConnectionState();
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
conn.outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn.outstandings.handshakePacketsCount = 0;
conn.outstandings.packets.clear(); conn.outstandings.packets.clear();
for (auto& t : conn.lossState.lossTimes) { for (auto& t : conn.lossState.lossTimes) {
t.reset(); t.reset();
@@ -1373,8 +1373,8 @@ TEST_F(QuicTransportTest, SendNewConnectionIdFrame) {
TEST_F(QuicTransportTest, CloneNewConnectionIdFrame) { TEST_F(QuicTransportTest, CloneNewConnectionIdFrame) {
auto& conn = transport_->getConnectionState(); auto& conn = transport_->getConnectionState();
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
conn.outstandings.packetCount[PacketNumberSpace::Initial] = 0; conn.outstandings.initialPacketsCount = 0;
conn.outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn.outstandings.handshakePacketsCount = 0;
conn.outstandings.packets.clear(); conn.outstandings.packets.clear();
for (auto& t : conn.lossState.lossTimes) { for (auto& t : conn.lossState.lossTimes) {
t.reset(); t.reset();
@@ -1513,8 +1513,8 @@ TEST_F(QuicTransportTest, SendRetireConnectionIdFrame) {
TEST_F(QuicTransportTest, CloneRetireConnectionIdFrame) { TEST_F(QuicTransportTest, CloneRetireConnectionIdFrame) {
auto& conn = transport_->getConnectionState(); auto& conn = transport_->getConnectionState();
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
conn.outstandings.packetCount[PacketNumberSpace::Initial] = 0; conn.outstandings.initialPacketsCount = 0;
conn.outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn.outstandings.handshakePacketsCount = 0;
conn.outstandings.packets.clear(); conn.outstandings.packets.clear();
for (auto& t : conn.lossState.lossTimes) { for (auto& t : conn.lossState.lossTimes) {
t.reset(); t.reset();

19
quic/client/QuicClientTransport.cpp
View File

@@ -770,11 +770,10 @@ void QuicClientTransport::writeData() {
auto& initialCryptoStream = auto& initialCryptoStream =
*getCryptoStream(*conn_->cryptoState, EncryptionLevel::Initial); *getCryptoStream(*conn_->cryptoState, EncryptionLevel::Initial);
CryptoStreamScheduler initialScheduler(*conn_, initialCryptoStream); CryptoStreamScheduler initialScheduler(*conn_, initialCryptoStream);
auto& numProbePackets =
conn_->pendingEvents.numProbePackets[PacketNumberSpace::Initial];
if ((initialCryptoStream.retransmissionBuffer.size() && if ((initialCryptoStream.retransmissionBuffer.size() &&
conn_->outstandings.packetCount[PacketNumberSpace::Initial] && conn_->outstandings.initialPacketsCount &&
numProbePackets) || conn_->pendingEvents.numProbePackets) ||
initialScheduler.hasData() || initialScheduler.hasData() ||
(conn_->ackStates.initialAckState.needsToSendAckImmediately && (conn_->ackStates.initialAckState.needsToSendAckImmediately &&
hasAcksToSchedule(conn_->ackStates.initialAckState))) { hasAcksToSchedule(conn_->ackStates.initialAckState))) {
@@ -791,7 +790,7 @@ void QuicClientTransport::writeData() {
packetLimit, packetLimit,
clientConn_->retryToken); clientConn_->retryToken);
} }
if (!packetLimit && !conn_->pendingEvents.anyProbePackets()) { if (!packetLimit && !conn_->pendingEvents.numProbePackets) {
return; return;
} }
} }
@@ -799,11 +798,9 @@ void QuicClientTransport::writeData() {
auto& handshakeCryptoStream = auto& handshakeCryptoStream =
*getCryptoStream(*conn_->cryptoState, EncryptionLevel::Handshake); *getCryptoStream(*conn_->cryptoState, EncryptionLevel::Handshake);
CryptoStreamScheduler handshakeScheduler(*conn_, handshakeCryptoStream); CryptoStreamScheduler handshakeScheduler(*conn_, handshakeCryptoStream);
auto& numProbePackets = if ((conn_->outstandings.handshakePacketsCount &&
conn_->pendingEvents.numProbePackets[PacketNumberSpace::Handshake];
if ((conn_->outstandings.packetCount[PacketNumberSpace::Handshake] &&
handshakeCryptoStream.retransmissionBuffer.size() && handshakeCryptoStream.retransmissionBuffer.size() &&
numProbePackets) || conn_->pendingEvents.numProbePackets) ||
handshakeScheduler.hasData() || handshakeScheduler.hasData() ||
(conn_->ackStates.handshakeAckState.needsToSendAckImmediately && (conn_->ackStates.handshakeAckState.needsToSendAckImmediately &&
hasAcksToSchedule(conn_->ackStates.handshakeAckState))) { hasAcksToSchedule(conn_->ackStates.handshakeAckState))) {
@@ -819,7 +816,7 @@ void QuicClientTransport::writeData() {
version, version,
packetLimit); packetLimit);
} }
if (!packetLimit && !conn_->pendingEvents.anyProbePackets()) { if (!packetLimit && !conn_->pendingEvents.numProbePackets) {
return; return;
} }
} }
@@ -835,7 +832,7 @@ void QuicClientTransport::writeData() {
version, version,
packetLimit); packetLimit);
} }
if (!packetLimit && !conn_->pendingEvents.anyProbePackets()) { if (!packetLimit && !conn_->pendingEvents.numProbePackets) {
return; return;
} }
if (conn_->oneRttWriteCipher) { if (conn_->oneRttWriteCipher) {

3
quic/codec/QuicPacketRebuilder.cpp
View File

@@ -36,8 +36,7 @@ PacketEvent PacketRebuilder::cloneOutstandingPacket(OutstandingPacket& packet) {
DCHECK(!conn_.outstandings.packetEvents.count(event)); DCHECK(!conn_.outstandings.packetEvents.count(event));
packet.associatedEvent = event; packet.associatedEvent = event;
conn_.outstandings.packetEvents.insert(event); conn_.outstandings.packetEvents.insert(event);
++conn_.outstandings ++conn_.outstandings.clonedPacketsCount;
.clonedPacketCount[packet.packet.header.getPacketNumberSpace()];
} }
return *packet.associatedEvent; return *packet.associatedEvent;
} }

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

@@ -461,7 +461,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) {
PacketRebuilder rebuilder(regularBuilder2, conn); PacketRebuilder rebuilder(regularBuilder2, conn);
rebuilder.rebuildFromPacket(outstandingPacket); rebuilder.rebuildFromPacket(outstandingPacket);
EXPECT_TRUE(outstandingPacket.associatedEvent.has_value()); EXPECT_TRUE(outstandingPacket.associatedEvent.has_value());
EXPECT_EQ(1, conn.outstandings.numClonedPackets()); EXPECT_EQ(1, conn.outstandings.clonedPacketsCount);
} }
TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) { TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) {
@@ -485,7 +485,7 @@ TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) {
PacketRebuilder rebuilder(regularBuilder2, conn); PacketRebuilder rebuilder(regularBuilder2, conn);
EXPECT_EQ(folly::none, rebuilder.rebuildFromPacket(outstandingPacket)); EXPECT_EQ(folly::none, rebuilder.rebuildFromPacket(outstandingPacket));
EXPECT_FALSE(outstandingPacket.associatedEvent.has_value()); EXPECT_FALSE(outstandingPacket.associatedEvent.has_value());
EXPECT_EQ(0, conn.outstandings.numClonedPackets()); EXPECT_EQ(0, conn.outstandings.clonedPacketsCount);
} }
TEST_F(QuicPacketRebuilderTest, LastStreamFrameSkipLen) { TEST_F(QuicPacketRebuilderTest, LastStreamFrameSkipLen) {

4
quic/fizz/client/test/QuicClientTransportTest.cpp
View File

@@ -3884,8 +3884,8 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerResetNoOutstandingPackets) {
// Clear out all the outstanding packets to simulate quiescent state. // Clear out all the outstanding packets to simulate quiescent state.
client->getNonConstConn().receivedNewPacketBeforeWrite = false; client->getNonConstConn().receivedNewPacketBeforeWrite = false;
client->getNonConstConn().outstandings.packets.clear(); client->getNonConstConn().outstandings.packets.clear();
client->getNonConstConn().outstandings.packetCount = {}; client->getNonConstConn().outstandings.handshakePacketsCount = 0;
client->getNonConstConn().outstandings.clonedPacketCount = {}; client->getNonConstConn().outstandings.clonedPacketsCount = 0;
client->idleTimeout().cancelTimeout(); client->idleTimeout().cancelTimeout();
auto streamId = client->createBidirectionalStream().value(); auto streamId = client->createBidirectionalStream().value();
auto expected = folly::IOBuf::copyBuffer("hello"); auto expected = folly::IOBuf::copyBuffer("hello");

21
quic/loss/QuicLossFunctions.cpp
View File

@@ -7,7 +7,6 @@
*/ */
#include "quic/loss/QuicLossFunctions.h" #include "quic/loss/QuicLossFunctions.h"
#include "quic/QuicConstants.h"
#include "quic/state/QuicStreamFunctions.h" #include "quic/state/QuicStreamFunctions.h"
namespace quic { namespace quic {
@@ -54,21 +53,11 @@ void onPTOAlarm(QuicConnectionStateBase& conn) {
throw QuicInternalException("Exceeded max PTO", LocalErrorCode::NO_ERROR); throw QuicInternalException("Exceeded max PTO", LocalErrorCode::NO_ERROR);
} }
uint64_t numInitial = // If there is only one packet outstanding, no point to clone it twice in the
conn.outstandings.packetCount[PacketNumberSpace::Initial] + // same write loop.
conn.outstandings.clonedPacketCount[PacketNumberSpace::Initial]; conn.pendingEvents.numProbePackets =
uint64_t numHandshake = std::min<decltype(conn.pendingEvents.numProbePackets)>(
conn.outstandings.packetCount[PacketNumberSpace::Handshake] + conn.outstandings.numOutstanding(), kPacketToSendForPTO);
conn.outstandings.clonedPacketCount[PacketNumberSpace::Handshake];
uint64_t numAppData =
conn.outstandings.packetCount[PacketNumberSpace::AppData] +
conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData];
conn.pendingEvents.numProbePackets[PacketNumberSpace::Initial] =
std::min<uint8_t>(numInitial, kPacketToSendForPTO);
conn.pendingEvents.numProbePackets[PacketNumberSpace::Handshake] =
std::min<uint8_t>(numHandshake, kPacketToSendForPTO);
conn.pendingEvents.numProbePackets[PacketNumberSpace::AppData] =
std::min<uint8_t>(numAppData, kPacketToSendForPTO);
} }
void markPacketLoss( void markPacketLoss(

56
quic/loss/QuicLossFunctions.h
View File

@@ -139,11 +139,10 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
*/ */
if (!hasDataToWrite && conn.outstandings.packetEvents.empty() && if (!hasDataToWrite && conn.outstandings.packetEvents.empty() &&
(totalPacketsOutstanding - totalD6DProbesOutstanding) == (totalPacketsOutstanding - totalD6DProbesOutstanding) ==
conn.outstandings.numClonedPackets()) { conn.outstandings.clonedPacketsCount) {
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
<< " handshakePackets=" << " handshakePackets=" << conn.outstandings.handshakePacketsCount
<< conn.outstandings.packetCount[PacketNumberSpace::Handshake]
<< " " << conn; << " " << conn;
conn.pendingEvents.setLossDetectionAlarm = false; conn.pendingEvents.setLossDetectionAlarm = false;
timeout.cancelLossTimeout(); timeout.cancelLossTimeout();
@@ -165,11 +164,10 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
if (!conn.pendingEvents.setLossDetectionAlarm) { if (!conn.pendingEvents.setLossDetectionAlarm) {
VLOG_IF(10, !timeout.isLossTimeoutScheduled()) VLOG_IF(10, !timeout.isLossTimeoutScheduled())
<< __func__ << " alarm not scheduled" << __func__ << " alarm not scheduled"
<< " outstanding=" << totalPacketsOutstanding << " initialPackets=" << " outstanding=" << totalPacketsOutstanding
<< conn.outstandings.packetCount[PacketNumberSpace::Initial] << " initialPackets=" << conn.outstandings.initialPacketsCount
<< " handshakePackets=" << " handshakePackets=" << conn.outstandings.handshakePacketsCount
<< conn.outstandings.packetCount[PacketNumberSpace::Handshake] << " " << " " << nodeToString(conn.nodeType) << " " << conn;
<< nodeToString(conn.nodeType) << " " << conn;
return; return;
} }
timeout.cancelLossTimeout(); timeout.cancelLossTimeout();
@@ -180,13 +178,11 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
<< " method=" << conn.lossState.currentAlarmMethod << " method=" << conn.lossState.currentAlarmMethod
<< " haDataToWrite=" << hasDataToWrite << " haDataToWrite=" << hasDataToWrite
<< " outstanding=" << totalPacketsOutstanding << " outstanding=" << totalPacketsOutstanding
<< " outstanding clone=" << conn.outstandings.numClonedPackets() << " outstanding clone=" << conn.outstandings.clonedPacketsCount
<< " packetEvents=" << conn.outstandings.packetEvents.size() << " packetEvents=" << conn.outstandings.packetEvents.size()
<< " initialPackets=" << " initialPackets=" << conn.outstandings.initialPacketsCount
<< conn.outstandings.packetCount[PacketNumberSpace::Initial] << " handshakePackets=" << conn.outstandings.handshakePacketsCount
<< " handshakePackets=" << " " << nodeToString(conn.nodeType) << " " << conn;
<< conn.outstandings.packetCount[PacketNumberSpace::Handshake] << " "
<< nodeToString(conn.nodeType) << " " << conn;
timeout.scheduleLossTimeout(alarmDuration.first); timeout.scheduleLossTimeout(alarmDuration.first);
conn.pendingEvents.setLossDetectionAlarm = false; conn.pendingEvents.setLossDetectionAlarm = false;
} }
@@ -263,8 +259,8 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
observerLossEvent.addLostPacket(lostByTimeout, lostByReorder, pkt); observerLossEvent.addLostPacket(lostByTimeout, lostByReorder, pkt);
if (pkt.associatedEvent) { if (pkt.associatedEvent) {
DCHECK_GT(conn.outstandings.numClonedPackets(), 0); DCHECK_GT(conn.outstandings.clonedPacketsCount, 0);
--conn.outstandings.clonedPacketCount[pnSpace]; --conn.outstandings.clonedPacketsCount;
} }
// Invoke LossVisitor if the packet doesn't have a associated PacketEvent; // Invoke LossVisitor if the packet doesn't have a associated PacketEvent;
// or if the PacketEvent is present in conn.outstandings.packetEvents. // or if the PacketEvent is present in conn.outstandings.packetEvents.
@@ -277,12 +273,12 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
} }
if (pkt.metadata.isHandshake && !processed) { if (pkt.metadata.isHandshake && !processed) {
if (currentPacketNumberSpace == PacketNumberSpace::Initial) { if (currentPacketNumberSpace == PacketNumberSpace::Initial) {
CHECK(conn.outstandings.packetCount[PacketNumberSpace::Initial]); CHECK(conn.outstandings.initialPacketsCount);
--conn.outstandings.packetCount[PacketNumberSpace::Initial]; --conn.outstandings.initialPacketsCount;
} else { } else {
CHECK_EQ(PacketNumberSpace::Handshake, currentPacketNumberSpace); CHECK_EQ(PacketNumberSpace::Handshake, currentPacketNumberSpace);
CHECK(conn.outstandings.packetCount[PacketNumberSpace::Handshake]); CHECK(conn.outstandings.handshakePacketsCount);
--conn.outstandings.packetCount[PacketNumberSpace::Handshake]; --conn.outstandings.handshakePacketsCount;
} }
} }
VLOG(10) << __func__ << " lost packetNum=" << currentPacketNum VLOG(10) << __func__ << " lost packetNum=" << currentPacketNum
@@ -396,11 +392,9 @@ void onLossDetectionAlarm(
VLOG(10) << __func__ << " setLossDetectionAlarm=" VLOG(10) << __func__ << " setLossDetectionAlarm="
<< conn.pendingEvents.setLossDetectionAlarm << conn.pendingEvents.setLossDetectionAlarm
<< " outstanding=" << conn.outstandings.numOutstanding() << " outstanding=" << conn.outstandings.numOutstanding()
<< " initialPackets=" << " initialPackets=" << conn.outstandings.initialPacketsCount
<< conn.outstandings.packetCount[PacketNumberSpace::Initial] << " handshakePackets=" << conn.outstandings.handshakePacketsCount
<< " handshakePackets=" << " " << conn;
<< conn.outstandings.packetCount[PacketNumberSpace::Handshake] << " "
<< conn;
} }
/* /*
@@ -441,11 +435,9 @@ folly::Optional<CongestionController::LossEvent> handleAckForLoss(
<< " setLossDetectionAlarm=" << " setLossDetectionAlarm="
<< conn.pendingEvents.setLossDetectionAlarm << conn.pendingEvents.setLossDetectionAlarm
<< " outstanding=" << conn.outstandings.numOutstanding() << " outstanding=" << conn.outstandings.numOutstanding()
<< " initialPackets=" << " initialPackets=" << conn.outstandings.initialPacketsCount
<< conn.outstandings.packetCount[PacketNumberSpace::Initial] << " handshakePackets=" << conn.outstandings.handshakePacketsCount
<< " handshakePackets=" << " " << conn;
<< conn.outstandings.packetCount[PacketNumberSpace::Handshake] << " "
<< conn;
return lossEvent; return lossEvent;
} }
@@ -472,8 +464,8 @@ void markZeroRttPacketsLost(
// Remove the PacketEvent from the outstandings.packetEvents set // Remove the PacketEvent from the outstandings.packetEvents set
if (pkt.associatedEvent) { if (pkt.associatedEvent) {
conn.outstandings.packetEvents.erase(*pkt.associatedEvent); conn.outstandings.packetEvents.erase(*pkt.associatedEvent);
DCHECK_GT(conn.outstandings.numClonedPackets(), 0); DCHECK_GT(conn.outstandings.clonedPacketsCount, 0);
--conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]; --conn.outstandings.clonedPacketsCount;
} }
lossEvent.addLostPacket(pkt); lossEvent.addLostPacket(pkt);
iter = conn.outstandings.packets.erase(iter); iter = conn.outstandings.packets.erase(iter);

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

@@ -187,7 +187,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
*conn.serverConnectionId, *conn.serverConnectionId,
conn.ackStates.initialAckState.nextPacketNum, conn.ackStates.initialAckState.nextPacketNum,
*conn.version); *conn.version);
conn.outstandings.packetCount[PacketNumberSpace::Initial]++; conn.outstandings.initialPacketsCount++;
isHandshake = true; isHandshake = true;
break; break;
case PacketType::Handshake: case PacketType::Handshake:
@@ -197,7 +197,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
*conn.serverConnectionId, *conn.serverConnectionId,
conn.ackStates.handshakeAckState.nextPacketNum, conn.ackStates.handshakeAckState.nextPacketNum,
*conn.version); *conn.version);
conn.outstandings.packetCount[PacketNumberSpace::Handshake]++; conn.outstandings.handshakePacketsCount++;
isHandshake = true; isHandshake = true;
break; break;
case PacketType::ZeroRtt: case PacketType::ZeroRtt:
@@ -207,14 +207,12 @@ PacketNum QuicLossFunctionsTest::sendPacket(
*conn.serverConnectionId, *conn.serverConnectionId,
conn.ackStates.appDataAckState.nextPacketNum, conn.ackStates.appDataAckState.nextPacketNum,
*conn.version); *conn.version);
conn.outstandings.packetCount[PacketNumberSpace::AppData]++;
break; break;
case PacketType::OneRtt: case PacketType::OneRtt:
header = ShortHeader( header = ShortHeader(
ProtectionType::KeyPhaseZero, ProtectionType::KeyPhaseZero,
*conn.serverConnectionId, *conn.serverConnectionId,
conn.ackStates.appDataAckState.nextPacketNum); conn.ackStates.appDataAckState.nextPacketNum);
conn.outstandings.packetCount[PacketNumberSpace::AppData]++;
break; break;
} }
PacketNumberSpace packetNumberSpace; PacketNumberSpace packetNumberSpace;
@@ -260,7 +258,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
conn.congestionController->onPacketSent(outstandingPacket); conn.congestionController->onPacketSent(outstandingPacket);
} }
if (associatedEvent) { if (associatedEvent) {
conn.outstandings.clonedPacketCount[packetNumberSpace]++; conn.outstandings.clonedPacketsCount++;
// 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(),
@@ -274,7 +272,7 @@ PacketNum QuicLossFunctionsTest::sendPacket(
if (it != conn.outstandings.packets.end()) { if (it != conn.outstandings.packets.end()) {
if (!it->associatedEvent) { if (!it->associatedEvent) {
conn.outstandings.packetEvents.emplace(*associatedEvent); conn.outstandings.packetEvents.emplace(*associatedEvent);
conn.outstandings.clonedPacketCount[packetNumberSpace]++; conn.outstandings.clonedPacketsCount++;
it->associatedEvent = *associatedEvent; it->associatedEvent = *associatedEvent;
} }
} }
@@ -1012,7 +1010,7 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
for (int i = 0; i < 6; ++i) { for (int i = 0; i < 6; ++i) {
sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake); sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake);
} }
EXPECT_EQ(6, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(6, conn->outstandings.handshakePacketsCount);
// Assume some packets are already acked // Assume some packets are already acked
for (auto iter = for (auto iter =
getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 2; getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 2;
@@ -1020,7 +1018,7 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 5; getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 5;
iter++) { iter++) {
if (iter->metadata.isHandshake) { if (iter->metadata.isHandshake) {
conn->outstandings.packetCount[PacketNumberSpace::Handshake]--; conn->outstandings.handshakePacketsCount--;
} }
} }
auto firstHandshakeOpIter = auto firstHandshakeOpIter =
@@ -1042,7 +1040,7 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
EXPECT_EQ(lostPacket.back(), 2); EXPECT_EQ(lostPacket.back(), 2);
// Packet 6 is the only thing remaining inflight, it is a handshake pkt // Packet 6 is the only thing remaining inflight, it is a handshake pkt
EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount);
// Packet 6 should remain in packet as the delta is less than threshold // Packet 6 should remain in packet as the delta is less than threshold
auto numDeclaredLost = std::count_if( auto numDeclaredLost = std::count_if(
@@ -1408,12 +1406,7 @@ TEST_F(QuicLossFunctionsTest, PTOWithHandshakePackets) {
EXPECT_EQ(0, lostPackets.size()); EXPECT_EQ(0, lostPackets.size());
EXPECT_EQ(1, conn->lossState.ptoCount); EXPECT_EQ(1, conn->lossState.ptoCount);
EXPECT_EQ(0, conn->lossState.timeoutBasedRtxCount); EXPECT_EQ(0, conn->lossState.timeoutBasedRtxCount);
EXPECT_EQ( EXPECT_EQ(conn->pendingEvents.numProbePackets, kPacketToSendForPTO);
conn->pendingEvents.numProbePackets[PacketNumberSpace::Handshake],
kPacketToSendForPTO);
EXPECT_EQ(
conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData],
kPacketToSendForPTO);
EXPECT_EQ(0, conn->lossState.rtxCount); EXPECT_EQ(0, conn->lossState.rtxCount);
} }
@@ -1589,7 +1582,7 @@ TEST_F(QuicLossFunctionsTest, DetectPacketLossClonedPacketsCounter) {
noopLossMarker, noopLossMarker,
Clock::now(), Clock::now(),
PacketNumberSpace::AppData); PacketNumberSpace::AppData);
EXPECT_EQ(0, conn->outstandings.numClonedPackets()); EXPECT_EQ(0, conn->outstandings.clonedPacketsCount);
} }
TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) { TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) {
@@ -1765,7 +1758,7 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) {
} }
EXPECT_EQ(6, conn->outstandings.packets.size()); EXPECT_EQ(6, conn->outstandings.packets.size());
ASSERT_EQ(conn->outstandings.numClonedPackets(), 6); ASSERT_EQ(conn->outstandings.clonedPacketsCount, 6);
ASSERT_EQ(conn->outstandings.packetEvents.size(), 2); ASSERT_EQ(conn->outstandings.packetEvents.size(), 2);
std::vector<bool> lostPackets; std::vector<bool> lostPackets;
@@ -1777,7 +1770,7 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) {
ASSERT_EQ(conn->outstandings.packetEvents.size(), 0); ASSERT_EQ(conn->outstandings.packetEvents.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.clonedPacketsCount, 3);
size_t numProcessed = 0; size_t numProcessed = 0;
for (auto lostPacket : lostPackets) { for (auto lostPacket : lostPackets) {
numProcessed += lostPacket; numProcessed += lostPacket;
@@ -1835,7 +1828,7 @@ TEST_F(QuicLossFunctionsTest, OutstandingInitialCounting) {
largestSent = largestSent =
sendPacket(*conn, Clock::now(), folly::none, PacketType::Initial); sendPacket(*conn, Clock::now(), folly::none, PacketType::Initial);
} }
EXPECT_EQ(10, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(10, conn->outstandings.initialPacketsCount);
auto noopLossVisitor = auto noopLossVisitor =
[&](auto& /* conn */, auto& /* packet */, bool /* processed */ [&](auto& /* conn */, auto& /* packet */, bool /* processed */
) {}; ) {};
@@ -1846,7 +1839,7 @@ TEST_F(QuicLossFunctionsTest, OutstandingInitialCounting) {
TimePoint(100ms), TimePoint(100ms),
PacketNumberSpace::Initial); PacketNumberSpace::Initial);
// [1, 6] are removed, [7, 10] are still in OP list // [1, 6] are removed, [7, 10] are still in OP list
EXPECT_EQ(4, conn->outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(4, conn->outstandings.initialPacketsCount);
} }
TEST_F(QuicLossFunctionsTest, OutstandingHandshakeCounting) { TEST_F(QuicLossFunctionsTest, OutstandingHandshakeCounting) {
@@ -1858,7 +1851,7 @@ TEST_F(QuicLossFunctionsTest, OutstandingHandshakeCounting) {
largestSent = largestSent =
sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake); sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake);
} }
EXPECT_EQ(10, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(10, conn->outstandings.handshakePacketsCount);
auto noopLossVisitor = auto noopLossVisitor =
[&](auto& /* conn */, auto& /* packet */, bool /* processed */ [&](auto& /* conn */, auto& /* packet */, bool /* processed */
) {}; ) {};
@@ -1869,12 +1862,12 @@ TEST_F(QuicLossFunctionsTest, OutstandingHandshakeCounting) {
TimePoint(100ms), TimePoint(100ms),
PacketNumberSpace::Handshake); PacketNumberSpace::Handshake);
// [1, 6] are removed, [7, 10] are still in OP list // [1, 6] are removed, [7, 10] are still in OP list
EXPECT_EQ(4, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(4, conn->outstandings.handshakePacketsCount);
} }
TEST_P(QuicLossFunctionsTest, CappedShiftNoCrash) { TEST_P(QuicLossFunctionsTest, CappedShiftNoCrash) {
auto conn = createConn(); auto conn = createConn();
conn->outstandings.packetCount[PacketNumberSpace::Handshake] = 0; conn->outstandings.handshakePacketsCount = 0;
conn->outstandings.packets.clear(); conn->outstandings.packets.clear();
conn->lossState.ptoCount = conn->lossState.ptoCount =
std::numeric_limits<decltype(conn->lossState.ptoCount)>::max(); std::numeric_limits<decltype(conn->lossState.ptoCount)>::max();

17
quic/server/QuicServerTransport.cpp
View File

@@ -211,10 +211,9 @@ void QuicServerTransport::writeData() {
auto& initialCryptoStream = auto& initialCryptoStream =
*getCryptoStream(*conn_->cryptoState, EncryptionLevel::Initial); *getCryptoStream(*conn_->cryptoState, EncryptionLevel::Initial);
CryptoStreamScheduler initialScheduler(*conn_, initialCryptoStream); CryptoStreamScheduler initialScheduler(*conn_, initialCryptoStream);
auto& numProbePackets = if ((conn_->pendingEvents.numProbePackets &&
conn_->pendingEvents.numProbePackets[PacketNumberSpace::Initial]; initialCryptoStream.retransmissionBuffer.size() &&
if ((numProbePackets && initialCryptoStream.retransmissionBuffer.size() && conn_->outstandings.initialPacketsCount) ||
conn_->outstandings.packetCount[PacketNumberSpace::Initial]) ||
initialScheduler.hasData() || initialScheduler.hasData() ||
(conn_->ackStates.initialAckState.needsToSendAckImmediately && (conn_->ackStates.initialAckState.needsToSendAckImmediately &&
hasAcksToSchedule(conn_->ackStates.initialAckState))) { hasAcksToSchedule(conn_->ackStates.initialAckState))) {
@@ -231,7 +230,7 @@ void QuicServerTransport::writeData() {
version, version,
packetLimit); packetLimit);
} }
if (!packetLimit && !conn_->pendingEvents.anyProbePackets()) { if (!packetLimit && !conn_->pendingEvents.numProbePackets) {
return; return;
} }
} }
@@ -239,11 +238,9 @@ void QuicServerTransport::writeData() {
auto& handshakeCryptoStream = auto& handshakeCryptoStream =
*getCryptoStream(*conn_->cryptoState, EncryptionLevel::Handshake); *getCryptoStream(*conn_->cryptoState, EncryptionLevel::Handshake);
CryptoStreamScheduler handshakeScheduler(*conn_, handshakeCryptoStream); CryptoStreamScheduler handshakeScheduler(*conn_, handshakeCryptoStream);
auto& numProbePackets = if ((conn_->outstandings.handshakePacketsCount &&
conn_->pendingEvents.numProbePackets[PacketNumberSpace::Handshake];
if ((conn_->outstandings.packetCount[PacketNumberSpace::Handshake] &&
handshakeCryptoStream.retransmissionBuffer.size() && handshakeCryptoStream.retransmissionBuffer.size() &&
numProbePackets) || conn_->pendingEvents.numProbePackets) ||
handshakeScheduler.hasData() || handshakeScheduler.hasData() ||
(conn_->ackStates.handshakeAckState.needsToSendAckImmediately && (conn_->ackStates.handshakeAckState.needsToSendAckImmediately &&
hasAcksToSchedule(conn_->ackStates.handshakeAckState))) { hasAcksToSchedule(conn_->ackStates.handshakeAckState))) {
@@ -260,7 +257,7 @@ void QuicServerTransport::writeData() {
version, version,
packetLimit); packetLimit);
} }
if (!packetLimit && !conn_->pendingEvents.anyProbePackets()) { if (!packetLimit && !conn_->pendingEvents.numProbePackets) {
return; return;
} }
} }

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

@@ -1212,7 +1212,8 @@ TEST_F(QuicServerTransportTest, TestOpenAckStreamFrame) {
// Remove any packets that might have been queued. // Remove any packets that might have been queued.
server->getNonConstConn().outstandings.packets.clear(); server->getNonConstConn().outstandings.packets.clear();
server->getNonConstConn().outstandings.packetCount = {}; server->getNonConstConn().outstandings.initialPacketsCount = 0;
server->getNonConstConn().outstandings.handshakePacketsCount = 0;
server->writeChain(streamId, data->clone(), false); server->writeChain(streamId, data->clone(), false);
loopForWrites(); loopForWrites();
server->writeChain(streamId, data->clone(), false); server->writeChain(streamId, data->clone(), false);
@@ -1806,7 +1807,8 @@ TEST_F(QuicServerTransportTest, TestCloneStopSending) {
server->getNonConstConn().qLogger = qLogger; server->getNonConstConn().qLogger = qLogger;
server->getNonConstConn().streamManager->getStream(streamId); server->getNonConstConn().streamManager->getStream(streamId);
// knock every handshake outstanding packets out // knock every handshake outstanding packets out
server->getNonConstConn().outstandings.packetCount = {}; server->getNonConstConn().outstandings.initialPacketsCount = 0;
server->getNonConstConn().outstandings.handshakePacketsCount = 0;
server->getNonConstConn().outstandings.packets.clear(); server->getNonConstConn().outstandings.packets.clear();
for (auto& t : server->getNonConstConn().lossState.lossTimes) { for (auto& t : server->getNonConstConn().lossState.lossTimes) {
t.reset(); t.reset();

24
quic/state/AckHandlers.cpp
View File

@@ -232,26 +232,20 @@ void processAckFrame(
if (lastAckedPacketSentTime) { if (lastAckedPacketSentTime) {
conn.lossState.lastAckedPacketSentTime = *lastAckedPacketSentTime; conn.lossState.lastAckedPacketSentTime = *lastAckedPacketSentTime;
} }
CHECK_GE( CHECK_GE(conn.outstandings.initialPacketsCount, initialPacketAcked);
conn.outstandings.packetCount[PacketNumberSpace::Initial], conn.outstandings.initialPacketsCount -= initialPacketAcked;
initialPacketAcked); CHECK_GE(conn.outstandings.handshakePacketsCount, handshakePacketAcked);
conn.outstandings.packetCount[PacketNumberSpace::Initial] -= conn.outstandings.handshakePacketsCount -= handshakePacketAcked;
initialPacketAcked; CHECK_GE(conn.outstandings.clonedPacketsCount, clonedPacketsAcked);
CHECK_GE( conn.outstandings.clonedPacketsCount -= clonedPacketsAcked;
conn.outstandings.packetCount[PacketNumberSpace::Handshake],
handshakePacketAcked);
conn.outstandings.packetCount[PacketNumberSpace::Handshake] -=
handshakePacketAcked;
CHECK_GE(conn.outstandings.numClonedPackets(), clonedPacketsAcked);
conn.outstandings.clonedPacketCount[pnSpace] -= clonedPacketsAcked;
CHECK_GE( CHECK_GE(
conn.outstandings.packets.size(), conn.outstandings.declaredLostCount); conn.outstandings.packets.size(), conn.outstandings.declaredLostCount);
auto updatedOustandingPacketsCount = conn.outstandings.numOutstanding(); auto updatedOustandingPacketsCount = conn.outstandings.numOutstanding();
CHECK_GE( CHECK_GE(
updatedOustandingPacketsCount, updatedOustandingPacketsCount,
conn.outstandings.packetCount[PacketNumberSpace::Handshake] + conn.outstandings.handshakePacketsCount +
conn.outstandings.packetCount[PacketNumberSpace::Initial]); conn.outstandings.initialPacketsCount);
CHECK_GE(updatedOustandingPacketsCount, conn.outstandings.numClonedPackets()); CHECK_GE(updatedOustandingPacketsCount, conn.outstandings.clonedPacketsCount);
auto lossEvent = handleAckForLoss(conn, lossVisitor, ack, pnSpace); auto lossEvent = handleAckForLoss(conn, lossVisitor, ack, pnSpace);
if (conn.congestionController && if (conn.congestionController &&
(ack.largestAckedPacket.has_value() || lossEvent)) { (ack.largestAckedPacket.has_value() || lossEvent)) {

26
quic/state/StateData.h
View File

@@ -118,11 +118,16 @@ struct OutstandingsInfo {
// TODO: Enforce only AppTraffic packets to be clonable // TODO: Enforce only AppTraffic packets to be clonable
folly::F14FastSet<PacketEvent, PacketEventHash> packetEvents; folly::F14FastSet<PacketEvent, PacketEventHash> packetEvents;
// Number of outstanding packets not including cloned // Number of outstanding packets in Initial space, not including cloned
EnumArray<PacketNumberSpace, uint64_t> packetCount{}; // Initial packets.
uint64_t initialPacketsCount{0};
// Number of outstanding packets in Handshake space, not including cloned
// Handshake packets.
uint64_t handshakePacketsCount{0};
// Number of packets are clones or cloned. // Number of packets are clones or cloned.
EnumArray<PacketNumberSpace, uint64_t> clonedPacketCount{}; uint64_t clonedPacketsCount{0};
// Number of packets currently declared lost. // Number of packets currently declared lost.
uint64_t declaredLostCount{0}; uint64_t declaredLostCount{0};
@@ -131,13 +136,6 @@ struct OutstandingsInfo {
uint64_t numOutstanding() { uint64_t numOutstanding() {
return packets.size() - declaredLostCount; return packets.size() - declaredLostCount;
} }
// Total number of cloned packets.
uint64_t numClonedPackets() {
return clonedPacketCount[PacketNumberSpace::Initial] +
clonedPacketCount[PacketNumberSpace::Handshake] +
clonedPacketCount[PacketNumberSpace::AppData];
}
}; };
struct Pacer { struct Pacer {
@@ -568,13 +566,7 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
std::vector<KnobFrame> knobs; std::vector<KnobFrame> knobs;
// Number of probing packets to send after PTO // Number of probing packets to send after PTO
EnumArray<PacketNumberSpace, uint8_t> numProbePackets{}; uint8_t numProbePackets{0};
bool anyProbePackets() const {
return numProbePackets[PacketNumberSpace::Initial] +
numProbePackets[PacketNumberSpace::Handshake] +
numProbePackets[PacketNumberSpace::AppData];
}
// true: schedule timeout if not scheduled // true: schedule timeout if not scheduled
// false: cancel scheduled timeout // false: cancel scheduled timeout

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

@@ -672,11 +672,9 @@ TEST_P(AckHandlersTest, TestHandshakeCounterUpdate) {
0, 0,
LossState()); LossState());
if (GetParam() == PacketNumberSpace::Initial) { if (GetParam() == PacketNumberSpace::Initial) {
conn.outstandings.packetCount[PacketNumberSpace::Initial] += conn.outstandings.initialPacketsCount += packetNum % 2;
packetNum % 2;
} else if (GetParam() == PacketNumberSpace::Handshake) { } else if (GetParam() == PacketNumberSpace::Handshake) {
conn.outstandings.packetCount[PacketNumberSpace::Handshake] += conn.outstandings.handshakePacketsCount += packetNum % 2;
packetNum % 2;
} }
} }
@@ -701,13 +699,13 @@ TEST_P(AckHandlersTest, TestHandshakeCounterUpdate) {
EXPECT_EQ(numDeclaredLost, conn.outstandings.declaredLostCount); EXPECT_EQ(numDeclaredLost, conn.outstandings.declaredLostCount);
if (GetParam() == PacketNumberSpace::Initial) { if (GetParam() == PacketNumberSpace::Initial) {
EXPECT_EQ(numDeclaredLost, 1); EXPECT_EQ(numDeclaredLost, 1);
EXPECT_EQ(1, conn.outstandings.packetCount[PacketNumberSpace::Initial]); EXPECT_EQ(1, conn.outstandings.initialPacketsCount);
// AppData packets won't be acked by an ack in Initial space: // AppData packets won't be acked by an ack in Initial space:
// So 0, 2, 4, 6, 8 and 9 are left in OP list // So 0, 2, 4, 6, 8 and 9 are left in OP list
EXPECT_EQ(numDeclaredLost + 6, conn.outstandings.packets.size()); EXPECT_EQ(numDeclaredLost + 6, conn.outstandings.packets.size());
} else if (GetParam() == PacketNumberSpace::Handshake) { } else if (GetParam() == PacketNumberSpace::Handshake) {
EXPECT_EQ(numDeclaredLost, 1); EXPECT_EQ(numDeclaredLost, 1);
EXPECT_EQ(1, conn.outstandings.packetCount[PacketNumberSpace::Handshake]); EXPECT_EQ(1, conn.outstandings.handshakePacketsCount);
// AppData packets won't be acked by an ack in Handshake space: // AppData packets won't be acked by an ack in Handshake space:
// So 0, 2, 4, 6, 8 and 9 are left in OP list // So 0, 2, 4, 6, 8 and 9 are left in OP list
EXPECT_EQ(numDeclaredLost + 6, conn.outstandings.packets.size()); EXPECT_EQ(numDeclaredLost + 6, conn.outstandings.packets.size());
@@ -802,7 +800,7 @@ TEST_P(AckHandlersTest, SkipAckVisitor) {
// outstandings.packetEvents // outstandings.packetEvents
outstandingPacket.associatedEvent.emplace(PacketNumberSpace::AppData, 0); outstandingPacket.associatedEvent.emplace(PacketNumberSpace::AppData, 0);
conn.outstandings.packets.push_back(std::move(outstandingPacket)); conn.outstandings.packets.push_back(std::move(outstandingPacket));
conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]++; conn.outstandings.clonedPacketsCount++;
ReadAckFrame ackFrame; ReadAckFrame ackFrame;
ackFrame.largestAcked = 0; ackFrame.largestAcked = 0;
@@ -850,7 +848,7 @@ TEST_P(AckHandlersTest, NoDoubleProcess) {
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[PacketNumberSpace::AppData] += 2; conn.outstandings.clonedPacketsCount += 2;
conn.outstandings.packetEvents.insert( conn.outstandings.packetEvents.insert(
PacketEvent(PacketNumberSpace::AppData, packetNum1)); PacketEvent(PacketNumberSpace::AppData, packetNum1));
@@ -913,7 +911,7 @@ 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[PacketNumberSpace::AppData] = 1; conn.outstandings.clonedPacketsCount = 1;
conn.outstandings.packetEvents.emplace( conn.outstandings.packetEvents.emplace(
PacketNumberSpace::AppData, packetNum1); PacketNumberSpace::AppData, packetNum1);
@@ -936,7 +934,7 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) {
) { /* no-op */ }, ) { /* no-op */ },
Clock::now()); Clock::now());
EXPECT_EQ(2, ackVisitorCounter); EXPECT_EQ(2, ackVisitorCounter);
EXPECT_EQ(0, conn.outstandings.numClonedPackets()); EXPECT_EQ(0, conn.outstandings.clonedPacketsCount);
} }
TEST_P(AckHandlersTest, UpdateMaxAckDelay) { TEST_P(AckHandlersTest, UpdateMaxAckDelay) {
@@ -1009,7 +1007,7 @@ TEST_P(AckHandlersTest, AckNotOutstandingButLoss) {
0, 0,
LossState()); LossState());
conn.outstandings.packets.push_back(std::move(outstandingPacket)); conn.outstandings.packets.push_back(std::move(outstandingPacket));
conn.outstandings.clonedPacketCount[PacketNumberSpace::AppData]++; conn.outstandings.clonedPacketsCount++;
EXPECT_CALL(*mockQLogger, addPacketsLost(1, 1, 1)); EXPECT_CALL(*mockQLogger, addPacketsLost(1, 1, 1));