diff --git a/quic/api/QuicSocket.h b/quic/api/QuicSocket.h index 47e3d5c3e..d00b4e146 100644 --- a/quic/api/QuicSocket.h +++ b/quic/api/QuicSocket.h @@ -132,6 +132,10 @@ class QuicSocket { uint64_t pacingBurstSize{0}; std::chrono::microseconds pacingInterval{0us}; uint32_t packetsRetransmitted{0}; + uint32_t totalPacketsSent{0}; + uint32_t totalPacketsMarkedLost{0}; + uint32_t totalPacketsMarkedLostByPto{0}; + uint32_t totalPacketsMarkedLostByReorderingThreshold{0}; uint32_t packetsSpuriouslyLost{0}; uint32_t timeoutBasedLoss{0}; std::chrono::microseconds pto{0us}; diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 3d203c65e..2fa88781b 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -585,6 +585,13 @@ QuicSocket::TransportInfo QuicTransportBase::getTransportInfo() const { transportInfo.pacingBurstSize = burstSize; transportInfo.pacingInterval = pacingInterval; transportInfo.packetsRetransmitted = conn_->lossState.rtxCount; + transportInfo.totalPacketsSent = conn_->lossState.totalPacketsSent; + transportInfo.totalPacketsMarkedLost = + conn_->lossState.totalPacketsMarkedLost; + transportInfo.totalPacketsMarkedLostByPto = + conn_->lossState.totalPacketsMarkedLostByPto; + transportInfo.totalPacketsMarkedLostByReorderingThreshold = + conn_->lossState.totalPacketsMarkedLostByReorderingThreshold; transportInfo.timeoutBasedLoss = conn_->lossState.timeoutBasedRtxCount; transportInfo.totalBytesRetransmitted = conn_->lossState.totalBytesRetransmitted; diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 3585e42cf..5d775c224 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -660,6 +660,7 @@ void updateConnection( conn.pendingEvents.setLossDetectionAlarm = retransmittable; } conn.lossState.totalBytesSent += encodedSize; + conn.lossState.totalPacketsSent++; if (!retransmittable && !isPing) { DCHECK(!packetEvent); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 869baca61..571667500 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -2129,6 +2129,15 @@ TEST_F(QuicTransportFunctionsTest, TotalBytesSentUpdate) { EXPECT_EQ(5555, conn->lossState.totalBytesSent); } +TEST_F(QuicTransportFunctionsTest, TotalPacketsSentUpdate) { + const auto startTotalPacketsSent = 1234; + auto conn = createConn(); + conn->lossState.totalPacketsSent = startTotalPacketsSent; + auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); + updateConnection(*conn, folly::none, packet.packet, TimePoint{}, 4321); + EXPECT_EQ(startTotalPacketsSent + 1, conn->lossState.totalPacketsSent); +} + TEST_F(QuicTransportFunctionsTest, TimeoutBasedRetxCountUpdate) { auto conn = createConn(); auto stream = conn->streamManager->createNextBidirectionalStream().value(); diff --git a/quic/loss/QuicLossFunctions.h b/quic/loss/QuicLossFunctions.h index d46c81b3b..8569d48ac 100644 --- a/quic/loss/QuicLossFunctions.h +++ b/quic/loss/QuicLossFunctions.h @@ -283,10 +283,17 @@ folly::Optional detectLossPackets( << " handshake=" << pkt.metadata.isHandshake << " " << conn; // Rather than erasing here, instead mark the packet as lost so we can // determine if this was spurious later. + conn.lossState.totalPacketsMarkedLost++; + if (lostByTimeout) { + conn.lossState.totalPacketsMarkedLostByPto++; + } + if (lostByReorder) { + conn.lossState.totalPacketsMarkedLostByReorderingThreshold++; + } conn.outstandings.declaredLostCount++; iter->declaredLost = true; iter++; - } + } // while (iter != conn.outstandings.packets.end()) { // if there are observers, enqueue a function to call it if (observerLossEvent.hasPackets()) { diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 1e6caa89d..d53c06782 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -2084,6 +2084,163 @@ TEST_F(QuicLossFunctionsTest, TestNoInstrumentationObserverCallback) { EXPECT_EQ(0, size(conn->pendingCallbacks)); } +TEST_F(QuicLossFunctionsTest, TotalPacketsMarkedLostByReordering) { + auto conn = createConn(); + auto noopLossVisitor = [](auto&, auto&, bool) {}; + + // send 7 packets + PacketNum largestSent = 0; + for (int i = 0; i < 7; ++i) { + largestSent = + sendPacket(*conn, TimePoint(i * 10ms), folly::none, PacketType::OneRtt); + } + + // Some packets are already acked + conn->outstandings.packets.erase( + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 2, + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 5); + + // setting a very low reordering threshold to force loss by reorder + conn->lossState.reorderingThreshold = 1; + // setting time out parameters higher than the time at which detectLossPackets + // is called to make sure there are no losses by timeout + conn->lossState.srtt = 400ms; + conn->lossState.lrtt = 350ms; + conn->transportSettings.timeReorderingThreshDividend = 1.0; + conn->transportSettings.timeReorderingThreshDivisor = 1.0; + TimePoint checkTime = TimePoint(200ms); + + detectLossPackets( + *conn, + largestSent + 1, + noopLossVisitor, + checkTime, + PacketNumberSpace::AppData); + + // Sent 7 packets, out of 0, 1, 2, 3, 4, 5, 6 -- we deleted (acked) 2,3,4 + // 0, 1, and 5 should be marked lost due to reordering, none due to timeout + // 6 is outstanding / on the wire still (no determination made) + EXPECT_EQ(3, conn->lossState.totalPacketsMarkedLost); + EXPECT_EQ(0, conn->lossState.totalPacketsMarkedLostByPto); + EXPECT_EQ(3, conn->lossState.totalPacketsMarkedLostByReorderingThreshold); +} + +TEST_F(QuicLossFunctionsTest, TotalPacketsMarkedLostByPto) { + auto conn = createConn(); + auto noopLossVisitor = [](auto&, auto&, bool) {}; + + // send 7 packets + PacketNum largestSent = 0; + for (int i = 0; i < 7; ++i) { + largestSent = + sendPacket(*conn, TimePoint(i * 10ms), folly::none, PacketType::OneRtt); + } + + // setting a very high reordering threshold to force loss by timeout only + conn->lossState.reorderingThreshold = 100; + // setting time out parameters lower than the time at which detectLossPackets + // is called to make sure all packets timeout + conn->lossState.srtt = 400ms; + conn->lossState.lrtt = 350ms; + conn->transportSettings.timeReorderingThreshDividend = 1.0; + conn->transportSettings.timeReorderingThreshDivisor = 1.0; + TimePoint checkTime = TimePoint(500ms); + + detectLossPackets( + *conn, + largestSent + 1, + noopLossVisitor, + checkTime, + PacketNumberSpace::AppData); + + // All 7 packets should be marked as lost by PTO + EXPECT_EQ(7, conn->lossState.totalPacketsMarkedLost); + EXPECT_EQ(7, conn->lossState.totalPacketsMarkedLostByPto); + EXPECT_EQ(0, conn->lossState.totalPacketsMarkedLostByReorderingThreshold); +} + +TEST_F(QuicLossFunctionsTest, TotalPacketsMarkedLostByPtoPartial) { + auto conn = createConn(); + auto noopLossVisitor = [](auto&, auto&, bool) {}; + + // send 7 packets + PacketNum largestSent = 0; + for (int i = 0; i < 7; ++i) { + largestSent = + sendPacket(*conn, TimePoint(i * 10ms), folly::none, PacketType::OneRtt); + } + + // Some packets are already acked + conn->outstandings.packets.erase( + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 2, + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 5); + + // setting a very high reordering threshold to force loss by timeout only + conn->lossState.reorderingThreshold = 100; + // setting time out parameters lower than the time at which detectLossPackets + // is called to make sure all packets timeout + conn->lossState.srtt = 400ms; + conn->lossState.lrtt = 350ms; + conn->transportSettings.timeReorderingThreshDividend = 1.0; + conn->transportSettings.timeReorderingThreshDivisor = 1.0; + TimePoint checkTime = TimePoint(500ms); + + detectLossPackets( + *conn, + largestSent + 1, + noopLossVisitor, + checkTime, + PacketNumberSpace::AppData); + + // Sent 7 packets, out of 0, 1, 2, 3, 4, 5, 6 -- we deleted (acked) 2,3,4 + // 0, 1, 5, and 6 should be marked lost due to timeout, none due to reordering + EXPECT_EQ(4, conn->lossState.totalPacketsMarkedLost); + EXPECT_EQ(4, conn->lossState.totalPacketsMarkedLostByPto); + EXPECT_EQ(0, conn->lossState.totalPacketsMarkedLostByReorderingThreshold); +} + +TEST_F(QuicLossFunctionsTest, TotalPacketsMarkedLostByPtoAndReordering) { + auto conn = createConn(); + auto noopLossVisitor = [](auto&, auto&, bool) {}; + + // send 7 packets + PacketNum largestSent = 0; + for (int i = 0; i < 7; ++i) { + largestSent = + sendPacket(*conn, TimePoint(i * 10ms), folly::none, PacketType::OneRtt); + } + + // Some packets are already acked + conn->outstandings.packets.erase( + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 2, + getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + 5); + + // setting a low reorder threshold + conn->lossState.reorderingThreshold = 1; + + // setting time out parameters lower than the time at which detectLossPackets + // is called to make sure all packets timeout + conn->lossState.srtt = 400ms; + conn->lossState.lrtt = 350ms; + conn->transportSettings.timeReorderingThreshDividend = 1.0; + conn->transportSettings.timeReorderingThreshDivisor = 1.0; + TimePoint checkTime = TimePoint(500ms); + + detectLossPackets( + *conn, + largestSent + 1, + noopLossVisitor, + checkTime, + PacketNumberSpace::AppData); + + // Sent 7 packets, out of 0, 1, 2, 3, 4, 5, 6 -- we deleted (acked) 2,3,4 + // 0, 1, and 5 should be marked lost due to reordering AND timeout + // 6 should be marked as lost due to timeout only + EXPECT_EQ(4, conn->lossState.totalPacketsMarkedLost); + EXPECT_EQ(4, conn->lossState.totalPacketsMarkedLostByPto); + EXPECT_EQ(3, conn->lossState.totalPacketsMarkedLostByReorderingThreshold); +} + INSTANTIATE_TEST_CASE_P( QuicLossFunctionsTests, QuicLossFunctionsTest, diff --git a/quic/state/StateData.h b/quic/state/StateData.h index e8d4144e6..1b57be228 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -452,6 +452,17 @@ struct LossState { // The total number of bytes acked on this connection when the last time a // packet is acked. uint64_t totalBytesAckedAtLastAck{0}; + // Total number of packets sent on this connection, including retransmissions. + uint32_t totalPacketsSent{0}; + // Total number of packets which were declared lost, including losses that + // we later detected were spurious (see spuriousLossCount below). + uint32_t totalPacketsMarkedLost{0}; + // Total number of packets which were declared lost due to PTO; a packet can + // marked as lost by multiple detection mechanisms. + uint32_t totalPacketsMarkedLostByPto{0}; + // Total number of packets which were declared lost based on the reordering + // threshold; a packet can marked as lost by multiple detection mechanisms. + uint32_t totalPacketsMarkedLostByReorderingThreshold{0}; // Inflight bytes uint64_t inflightBytes{0}; // Reordering threshold used