diff --git a/quic/api/Observer.h b/quic/api/Observer.h index 8127f3cb6..e292ece5a 100644 --- a/quic/api/Observer.h +++ b/quic/api/Observer.h @@ -29,10 +29,13 @@ class InstrumentationObserver { const quic::OutstandingPacket& pkt) : lostByTimeout(lostbytimeout), lostByReorderThreshold(lostbyreorder), - packet(pkt) {} + metadata(pkt.metadata), + lastAckedPacketInfo(pkt.lastAckedPacketInfo) {} bool lostByTimeout{false}; bool lostByReorderThreshold{false}; - const quic::OutstandingPacket packet; + const quic::OutstandingPacketMetadata metadata; + const folly::Optional + lastAckedPacketInfo; }; struct ObserverLossEvent { @@ -52,6 +55,26 @@ class InstrumentationObserver { const TimePoint lossTime; std::vector lostPackets; }; + + struct PacketRTT { + explicit PacketRTT( + TimePoint rcvTimeIn, + std::chrono::microseconds rttSampleIn, + std::chrono::microseconds ackDelayIn, + const quic::OutstandingPacket& pkt) + : rcvTime(rcvTimeIn), + rttSample(rttSampleIn), + ackDelay(ackDelayIn), + metadata(pkt.metadata), + lastAckedPacketInfo(pkt.lastAckedPacketInfo) {} + TimePoint rcvTime; + std::chrono::microseconds rttSample; + std::chrono::microseconds ackDelay; + const quic::OutstandingPacketMetadata metadata; + const folly::Optional + lastAckedPacketInfo; + }; + virtual ~InstrumentationObserver() = default; /** @@ -78,6 +101,13 @@ class InstrumentationObserver { */ virtual void packetLossDetected( const struct ObserverLossEvent& /* lossEvent */) {} + + /** + * rttSampleGenerated() is invoked when a RTT sample is made. + * + * @param packet const reference to the packet with the RTT + */ + virtual void rttSampleGenerated(const PacketRTT& /* RTT sample */) {} }; // Container for instrumentation observers. diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 889260e06..87fac036d 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -613,7 +613,8 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket( } // I think this only fail if udpSendPacketLen somehow shrinks in the middle // of a connection. - if (outstandingPacket.encodedSize > writableBytes + cipherOverhead_) { + if (outstandingPacket.metadata.encodedSize > + writableBytes + cipherOverhead_) { continue; } diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index d9cc6d62f..f6256e67b 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -681,7 +681,8 @@ void updateConnection( encodedSize, isHandshake, isD6DProbe, - conn.lossState.totalBytesSent); + conn.lossState.totalBytesSent, + conn.lossState.inflightBytes); if (isD6DProbe) { ++conn.d6d.outstandingProbes; } @@ -722,9 +723,9 @@ void updateConnection( } if (conn.pathValidationLimiter && (conn.pendingEvents.pathChallenge || conn.outstandingPathValidation)) { - conn.pathValidationLimiter->onPacketSent(pkt.encodedSize); + conn.pathValidationLimiter->onPacketSent(pkt.metadata.encodedSize); } - if (pkt.isHandshake) { + if (pkt.metadata.isHandshake) { if (!pkt.associatedEvent) { if (packetNumberSpace == PacketNumberSpace::Initial) { ++conn.outstandings.initialPacketsCount; @@ -733,9 +734,9 @@ void updateConnection( ++conn.outstandings.handshakePacketsCount; } } - conn.lossState.lastHandshakePacketSentTime = pkt.time; + conn.lossState.lastHandshakePacketSentTime = pkt.metadata.time; } - conn.lossState.lastRetransmittablePacketSentTime = pkt.time; + conn.lossState.lastRetransmittablePacketSentTime = pkt.metadata.time; if (pkt.associatedEvent) { ++conn.outstandings.clonedPacketsCount; ++conn.lossState.timeoutBasedRtxCount; diff --git a/quic/api/test/Mocks.h b/quic/api/test/Mocks.h index 8833a9331..1d33010a9 100644 --- a/quic/api/test/Mocks.h +++ b/quic/api/test/Mocks.h @@ -332,6 +332,7 @@ class MockInstrumentationObserver : public InstrumentationObserver { , packetLossDetected, void(const ObserverLossEvent&)); + GMOCK_METHOD1_(, noexcept, , rttSampleGenerated, void(const PacketRTT&)); static auto getLossPacketMatcher(bool reorderLoss, bool timeoutLoss) { return AllOf( diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index 03eb832aa..4ee78b00d 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -40,7 +40,7 @@ PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) { nextPacketNum, QuicVersion::QUIC_DRAFT); RegularQuicWritePacket packet(std::move(header)); - conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, true, 0); + conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, true, 0, 0); conn.outstandings.handshakePacketsCount++; increaseNextPacketNum(conn, PacketNumberSpace::Handshake); return nextPacketNum; @@ -58,7 +58,7 @@ PacketNum addHandshakeOutstandingPacket(QuicConnectionStateBase& conn) { nextPacketNum, QuicVersion::QUIC_DRAFT); RegularQuicWritePacket packet(std::move(header)); - conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, true, 0); + conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, true, 0, 0); conn.outstandings.handshakePacketsCount++; increaseNextPacketNum(conn, PacketNumberSpace::Handshake); return nextPacketNum; @@ -71,7 +71,7 @@ PacketNum addOutstandingPacket(QuicConnectionStateBase& conn) { conn.clientConnectionId.value_or(quic::test::getTestConnectionId()), nextPacketNum); RegularQuicWritePacket packet(std::move(header)); - conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, false, 0); + conn.outstandings.packets.emplace_back(packet, Clock::now(), 0, false, 0, 0); increaseNextPacketNum(conn, PacketNumberSpace::AppData); return nextPacketNum; } @@ -1371,7 +1371,8 @@ TEST_F( conn.outstandings.packets.back().packet.frames.push_back( MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); // Lie about the encodedSize to let the Cloner skip it: - conn.outstandings.packets.back().encodedSize = kDefaultUDPSendPacketLen * 2; + conn.outstandings.packets.back().metadata.encodedSize = + kDefaultUDPSendPacketLen * 2; EXPECT_TRUE(cloningScheduler.hasData()); ASSERT_FALSE(noopScheduler.hasData()); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 304a1fe6f..d2e47570e 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -367,7 +367,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionD6DNotConsumeSendPing) { conn->d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(packetNum, 50); updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50); EXPECT_EQ(1, conn->outstandings.packets.size()); - EXPECT_TRUE(conn->outstandings.packets.front().isD6DProbe); + EXPECT_TRUE(conn->outstandings.packets.front().metadata.isD6DProbe); EXPECT_EQ(1, conn->d6d.outstandingProbes); // sendPing should still be active since d6d probe should be "hidden" from // application @@ -959,7 +959,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithBytesStats) { EXPECT_EQ( 13579 + 555, getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) - ->totalBytesSent); + ->metadata.totalBytesSent); EXPECT_TRUE(getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) ->lastAckedPacketInfo.has_value()); EXPECT_EQ( @@ -1025,10 +1025,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) { EXPECT_EQ(frame->maximumData, maxDataAmt); EXPECT_EQ( futureMoment, - getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->time); + getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->metadata.time); EXPECT_EQ( 1500, - getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->encodedSize); + getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->metadata.encodedSize); EXPECT_EQ( event, *getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) @@ -1676,7 +1678,7 @@ TEST_F(QuicTransportFunctionsTest, TestCryptoWritingIsHandshakeInOutstanding) { conn->transportSettings.writeConnectionDataPacketsLimit)); ASSERT_EQ(1, conn->outstandings.packets.size()); EXPECT_TRUE(getFirstOutstandingPacket(*conn, PacketNumberSpace::Initial) - ->isHandshake); + ->metadata.isHandshake); } TEST_F(QuicTransportFunctionsTest, NoCryptoProbeWriteIfNoProbeCredit) { @@ -1702,7 +1704,7 @@ TEST_F(QuicTransportFunctionsTest, NoCryptoProbeWriteIfNoProbeCredit) { conn->transportSettings.writeConnectionDataPacketsLimit)); ASSERT_EQ(1, conn->outstandings.packets.size()); EXPECT_TRUE(getFirstOutstandingPacket(*conn, PacketNumberSpace::Initial) - ->isHandshake); + ->metadata.isHandshake); ASSERT_EQ(1, cryptoStream->retransmissionBuffer.size()); ASSERT_TRUE(cryptoStream->writeBuffer.empty()); @@ -2474,7 +2476,8 @@ TEST_F(QuicTransportFunctionsTest, WriteProbingWithInplaceBuilder) { ASSERT_FALSE(streamScheduler.hasPendingData()); // The first packet has be a full packet - auto firstPacketSize = conn->outstandings.packets.front().encodedSize; + auto firstPacketSize = + conn->outstandings.packets.front().metadata.encodedSize; auto outstandingPacketsCount = conn->outstandings.packets.size(); ASSERT_EQ(firstPacketSize, conn->udpSendPacketLen); EXPECT_CALL(mockSock, write(_, _)) diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index 726353ffb..896a7b86c 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -29,7 +29,7 @@ OutstandingPacket makeDummyOutstandingPacket( const RegularQuicWritePacket& writePacket, uint64_t totalBytesSentOnConnection) { OutstandingPacket packet( - writePacket, Clock::now(), 1000, false, totalBytesSentOnConnection); + writePacket, Clock::now(), 1000, false, totalBytesSentOnConnection, 0); return packet; } diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index c44a9acac..7750ccdc7 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -547,7 +547,8 @@ OutstandingPacket makeTestingWritePacket( PacketNum desiredPacketSeqNum, size_t desiredSize, uint64_t totalBytesSent, - TimePoint sentTime) { + TimePoint sentTime /* = Clock::now() */, + uint64_t inflightBytes /* = 0 */) { LongHeader longHeader( LongHeader::Types::ZeroRtt, getTestConnectionId(1), @@ -556,7 +557,7 @@ OutstandingPacket makeTestingWritePacket( QuicVersion::MVFST); RegularQuicWritePacket packet(std::move(longHeader)); return OutstandingPacket( - packet, sentTime, desiredSize, false, totalBytesSent); + packet, sentTime, desiredSize, false, totalBytesSent, inflightBytes); } CongestionController::AckEvent makeAck( @@ -731,10 +732,10 @@ bool matchError( CongestionController::AckEvent::AckPacket makeAckPacketFromOutstandingPacket( OutstandingPacket outstandingPacket) { return CongestionController::AckEvent::AckPacket::Builder() - .setSentTime(outstandingPacket.time) - .setEncodedSize(outstandingPacket.encodedSize) + .setSentTime(outstandingPacket.metadata.time) + .setEncodedSize(outstandingPacket.metadata.encodedSize) .setLastAckedPacketInfo(std::move(outstandingPacket.lastAckedPacketInfo)) - .setTotalBytesSentThen(outstandingPacket.totalBytesSent) + .setTotalBytesSentThen(outstandingPacket.metadata.totalBytesSent) .setAppLimited(outstandingPacket.isAppLimited) .build(); } diff --git a/quic/common/test/TestUtils.h b/quic/common/test/TestUtils.h index 8cd8d37dd..24886479c 100644 --- a/quic/common/test/TestUtils.h +++ b/quic/common/test/TestUtils.h @@ -217,7 +217,8 @@ OutstandingPacket makeTestingWritePacket( PacketNum desiredPacketSeqNum, size_t desiredSize, uint64_t totalBytesSent, - TimePoint sentTime = Clock::now()); + TimePoint sentTime = Clock::now(), + uint64_t inflightBytes = 0); // TODO: The way we setup packet sent, ack, loss in test cases can use some // major refactor. diff --git a/quic/congestion_control/Bbr.cpp b/quic/congestion_control/Bbr.cpp index efc776c8b..8057f19fc 100644 --- a/quic/congestion_control/Bbr.cpp +++ b/quic/congestion_control/Bbr.cpp @@ -116,9 +116,10 @@ void BbrCongestionController::onPacketSent(const OutstandingPacket& packet) { if (!conn_.lossState.inflightBytes && isAppLimited()) { exitingQuiescene_ = true; } - addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); + addAndCheckOverflow( + conn_.lossState.inflightBytes, packet.metadata.encodedSize); if (!ackAggregationStartTime_) { - ackAggregationStartTime_ = packet.time; + ackAggregationStartTime_ = packet.metadata.time; } } diff --git a/quic/congestion_control/Copa.cpp b/quic/congestion_control/Copa.cpp index 8894ef1c5..2bb044753 100644 --- a/quic/congestion_control/Copa.cpp +++ b/quic/congestion_control/Copa.cpp @@ -46,7 +46,8 @@ void Copa::onRemoveBytesFromInflight(uint64_t bytes) { } void Copa::onPacketSent(const OutstandingPacket& packet) { - addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); + addAndCheckOverflow( + conn_.lossState.inflightBytes, packet.metadata.encodedSize); VLOG(10) << __func__ << " writable=" << getWritableBytes() << " cwnd=" << cwndBytes_ diff --git a/quic/congestion_control/NewReno.cpp b/quic/congestion_control/NewReno.cpp index 3d343b588..7dfcf4746 100644 --- a/quic/congestion_control/NewReno.cpp +++ b/quic/congestion_control/NewReno.cpp @@ -37,7 +37,8 @@ void NewReno::onRemoveBytesFromInflight(uint64_t bytes) { } void NewReno::onPacketSent(const OutstandingPacket& packet) { - addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); + addAndCheckOverflow( + conn_.lossState.inflightBytes, packet.metadata.encodedSize); VLOG(10) << __func__ << " writable=" << getWritableBytes() << " cwnd=" << cwndBytes_ << " inflight=" << conn_.lossState.inflightBytes diff --git a/quic/congestion_control/QuicCubic.cpp b/quic/congestion_control/QuicCubic.cpp index 96ffe0b62..b4557012c 100644 --- a/quic/congestion_control/QuicCubic.cpp +++ b/quic/congestion_control/QuicCubic.cpp @@ -92,12 +92,12 @@ void Cubic::onPersistentCongestion() { void Cubic::onPacketSent(const OutstandingPacket& packet) { if (std::numeric_limits::max() - conn_.lossState.inflightBytes < - packet.encodedSize) { + packet.metadata.encodedSize) { throw QuicInternalException( "Cubic: inflightBytes overflow", LocalErrorCode::INFLIGHT_BYTES_OVERFLOW); } - conn_.lossState.inflightBytes += packet.encodedSize; + conn_.lossState.inflightBytes += packet.metadata.encodedSize; } void Cubic::onPacketLoss(const LossEvent& loss) { diff --git a/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp b/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp index 2b0bdea7c..7af894f3a 100644 --- a/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp +++ b/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp @@ -70,7 +70,7 @@ TEST_F(BbrBandwidthSamplerTest, RateCalculation) { lastAckedPacketAckTime, 0, 0); - packet.time = ackTime - 50us; + packet.metadata.time = ackTime - 50us; ackEvent.ackedPackets.push_back( makeAckPacketFromOutstandingPacket(std::move(packet))); } @@ -99,7 +99,7 @@ TEST_F(BbrBandwidthSamplerTest, RateCalculationWithAdjustedAckTime) { adjustedAckedPacketAckTime, 0, 0); - packet.time = ackTime - 50us; + packet.metadata.time = ackTime - 50us; ackEvent.ackedPackets.push_back( makeAckPacketFromOutstandingPacket(std::move(packet))); } @@ -126,7 +126,7 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { lastAckedPacketAckTime, 1000, 1000); - packet.time = ackTime - 50us; + packet.metadata.time = ackTime - 50us; ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet)); sampler.onPacketAcked(ackEvent, 0); auto firstBandwidthSample = sampler.getBandwidth(); @@ -135,11 +135,11 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { conn_.lossState.totalBytesAcked = 2000; auto packet2 = makeTestingWritePacket(pn, 500, 2500); packet2.lastAckedPacketInfo.emplace( - packet.time, ackTime, ackTime, 2000, 1000); + packet.metadata.time, ackTime, ackTime, 2000, 1000); auto ackTime2 = ackTime + 150us; CongestionController::AckEvent ackEvent2; ackEvent2.ackTime = ackTime2; - packet2.time = ackTime + 110us; + packet2.metadata.time = ackTime + 110us; ackEvent2.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet2)); sampler.onPacketAcked(ackEvent2, kBandwidthWindowLength / 4 + 1); auto secondBandwidthSample = sampler.getBandwidth(); @@ -149,11 +149,11 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { conn_.lossState.totalBytesAcked = 2500; auto packet3 = makeTestingWritePacket(pn, 200, 2700); packet3.lastAckedPacketInfo.emplace( - packet2.time, ackTime2, ackTime2, 2500, 2000); + packet2.metadata.time, ackTime2, ackTime2, 2500, 2000); auto ackTime3 = ackTime + 250us; CongestionController::AckEvent ackEvent3; ackEvent3.ackTime = ackTime3; - packet3.time = ackTime + 210us; + packet3.metadata.time = ackTime + 210us; ackEvent3.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet3)); sampler.onPacketAcked(ackEvent3, kBandwidthWindowLength / 2 + 1); EXPECT_EQ(firstBandwidthSample, sampler.getBandwidth()); @@ -162,11 +162,11 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { conn_.lossState.totalBytesAcked = 2700; auto packet4 = makeTestingWritePacket(pn, 100, 2800); packet4.lastAckedPacketInfo.emplace( - packet3.time, ackTime3, ackTime3, 2700, 2500); + packet3.metadata.time, ackTime3, ackTime3, 2700, 2500); auto ackTime4 = ackTime + 350us; CongestionController::AckEvent ackEvent4; ackEvent4.ackTime = ackTime4; - packet4.time = ackTime + 310us; + packet4.metadata.time = ackTime + 310us; ackEvent4.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet4)); sampler.onPacketAcked(ackEvent4, kBandwidthWindowLength + 1); // The bandwidth we got from packet1 has expired. Packet2 should have @@ -188,7 +188,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimited) { ackEvent.largestAckedPacket = ++conn.lossState.largestSent.value(); auto packet = makeTestingWritePacket(*ackEvent.largestAckedPacket, 1000, 1000); - ackEvent.largestAckedPacketSentTime = packet.time; + ackEvent.largestAckedPacketSentTime = packet.metadata.time; ackEvent.ackedPackets.push_back( makeAckPacketFromOutstandingPacket(std::move(packet))); sampler.onPacketAcked(ackEvent, 0); @@ -224,7 +224,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) { lastAckedPacketAckTime, 0, 0); - packet.time = ackTime - 50us; + packet.metadata.time = ackTime - 50us; ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet)); // AppLimited packet, but sample is larger than current best sampler.onPacketAcked(ackEvent, 0); @@ -234,8 +234,9 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) { pn++; auto packet1 = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn); packet1.isAppLimited = true; - packet1.lastAckedPacketInfo.emplace(packet.time, ackTime, ackTime, 1000, 0); - packet1.time = ackTime + 500us; + packet1.lastAckedPacketInfo.emplace( + packet.metadata.time, ackTime, ackTime, 1000, 0); + packet1.metadata.time = ackTime + 500us; CongestionController::AckEvent ackEvent1; ackEvent1.ackedBytes = 1000; ackEvent1.ackTime = ackTime + 2000us; diff --git a/quic/congestion_control/test/BbrTest.cpp b/quic/congestion_control/test/BbrTest.cpp index 108d2c224..607f35024 100644 --- a/quic/congestion_control/test/BbrTest.cpp +++ b/quic/congestion_control/test/BbrTest.cpp @@ -141,7 +141,7 @@ TEST_F(BbrTest, StartupCwnd) { // Target cwnd will be 100 * 5000 * 2.885 = 1442500, but you haven't finished // STARTUP, too bad kiddo, you only grow a little today bbr.onPacketAckOrLoss( - makeAck(0, 3000, Clock::now(), packet.time), folly::none); + makeAck(0, 3000, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(startingCwnd + 3000, bbr.getCongestionWindow()); } @@ -167,7 +167,7 @@ TEST_F(BbrTest, StartupCwndImplicit) { Bandwidth(5000ULL * 1000 * 1000, std::chrono::microseconds(1)))); // Target cwnd will be 100 * 5000 * 2.885 = 1442500, but you haven't finished // STARTUP, too bad kiddo, you only grow a little today - auto ack = makeAck(0, 3000, Clock::now(), packet.time); + auto ack = makeAck(0, 3000, Clock::now(), packet.metadata.time); ack.implicit = true; bbr.onPacketAckOrLoss(ack, folly::none); EXPECT_EQ(startingCwnd + 3000, bbr.getCongestionWindow()); @@ -197,7 +197,8 @@ TEST_F(BbrTest, LeaveStartup) { .WillRepeatedly(Return( mockedBandwidth * (growFast ? kExpectedStartupGrowth : 1.0))); bbr.onPacketAckOrLoss( - makeAck(currentLatest, 1000, Clock::now(), packet.time), folly::none); + makeAck(currentLatest, 1000, Clock::now(), packet.metadata.time), + folly::none); conn.lossState.totalBytesAcked += 1000; if (growFast) { mockedBandwidth = mockedBandwidth * kExpectedStartupGrowth; @@ -263,7 +264,8 @@ TEST_F(BbrTest, ProbeRtt) { conn.udpSendPacketLen, totalSent + conn.udpSendPacketLen); bbr.onPacketSent(packet); - inflightPackets.push_back(std::make_pair(currentLatest, packet.time)); + inflightPackets.push_back( + std::make_pair(currentLatest, packet.metadata.time)); inflightBytes += conn.udpSendPacketLen; currentLatest++; totalSent += conn.udpSendPacketLen; @@ -431,7 +433,8 @@ TEST_F(BbrTest, AckAggregation) { .WillRepeatedly(Return( mockedBandwidth * (growFast ? kExpectedStartupGrowth : 1.0))); bbr.onPacketAckOrLoss( - makeAck(currentLatest, 1000, Clock::now(), packet.time), folly::none); + makeAck(currentLatest, 1000, Clock::now(), packet.metadata.time), + folly::none); conn.lossState.totalBytesAcked += 1000; if (growFast) { mockedBandwidth = mockedBandwidth * kExpectedStartupGrowth; @@ -468,7 +471,8 @@ TEST_F(BbrTest, AckAggregation) { conn.lossState.largestSent.value(), 1000, 1000 + totalSent); bbr.onPacketSent(packet); totalSent += 1000; - auto ackEvent = makeAck(currentLatest, 1000, Clock::now(), packet.time); + auto ackEvent = + makeAck(currentLatest, 1000, Clock::now(), packet.metadata.time); ackEvent.ackTime = Clock::now(); // use a real large bandwidth to clear accumulated ack aggregation during // startup @@ -494,7 +498,10 @@ TEST_F(BbrTest, AckAggregation) { bbr.onPacketSent(packet1); totalSent += (currentMaxAckHeight * 2 + 100); auto ackEvent2 = makeAck( - currentLatest, currentMaxAckHeight * 2 + 100, Clock::now(), packet1.time); + currentLatest, + currentMaxAckHeight * 2 + 100, + Clock::now(), + packet1.metadata.time); // This will make the expected ack arrival rate very low: ackEvent2.ackTime = ackEvent.ackTime + 1us; bbr.onPacketAckOrLoss(ackEvent2, folly::none); @@ -565,7 +572,7 @@ TEST_F(BbrTest, ExtendMinRttExpiration) { conn.lossState.largestSent.value_or(0), 1000, Clock::now(), - packet.time), + packet.metadata.time), folly::none); } diff --git a/quic/congestion_control/test/CMakeLists.txt b/quic/congestion_control/test/CMakeLists.txt index 4f7c9f17b..ae239233f 100644 --- a/quic/congestion_control/test/CMakeLists.txt +++ b/quic/congestion_control/test/CMakeLists.txt @@ -9,14 +9,19 @@ endif() quic_add_test(TARGET CongestionControllerTests SOURCES + BandwidthTest.cpp + BbrBandwidthSamplerTest.cpp + BbrRttSamplerTest.cpp + BbrTest.cpp CongestionControlFunctionsTest.cpp + CopaTest.cpp CubicHystartTest.cpp CubicRecoveryTest.cpp CubicStateTest.cpp CubicSteadyTest.cpp CubicTest.cpp NewRenoTest.cpp - CopaTest.cpp + PacerTest.cpp DEPENDS Folly::folly mvfst_cc_algo diff --git a/quic/congestion_control/test/CopaTest.cpp b/quic/congestion_control/test/CopaTest.cpp index ddc4915a9..b336766a6 100644 --- a/quic/congestion_control/test/CopaTest.cpp +++ b/quic/congestion_control/test/CopaTest.cpp @@ -31,20 +31,23 @@ class CopaTest : public Test { ShortHeader(ProtectionType::KeyPhaseZero, connId, packetData.first)); totalSentBytes += 10; loss.addLostPacket(OutstandingPacket( - std::move(packet), Clock::now(), 10, false, totalSentBytes)); + std::move(packet), Clock::now(), 10, false, totalSentBytes, 0)); loss.lostBytes = packetData.second; } loss.lostPackets = lostPackets.size(); return loss; } - OutstandingPacket - createPacket(PacketNum packetNum, uint32_t size, uint64_t totalSent) { + OutstandingPacket createPacket( + PacketNum packetNum, + uint32_t size, + uint64_t totalSent, + uint64_t inflight = 0) { auto connId = getTestConnectionId(); RegularQuicWritePacket packet( ShortHeader(ProtectionType::KeyPhaseZero, connId, packetNum)); return OutstandingPacket( - std::move(packet), Clock::now(), size, false, totalSent); + std::move(packet), Clock::now(), size, false, totalSent, inflight); } CongestionController::AckEvent createAckEvent( diff --git a/quic/congestion_control/test/CubicHystartTest.cpp b/quic/congestion_control/test/CubicHystartTest.cpp index c1cff97d7..92aee3989 100644 --- a/quic/congestion_control/test/CubicHystartTest.cpp +++ b/quic/congestion_control/test/CubicHystartTest.cpp @@ -30,7 +30,7 @@ TEST_F(CubicHystartTest, SendAndAck) { auto packet = makeTestingWritePacket(0, 1000, 1000); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet.time), folly::none); + makeAck(0, 1000, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(initCwnd + 1000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Hystart, cubic.state()); @@ -47,7 +47,7 @@ TEST_F(CubicHystartTest, CwndLargerThanSSThresh) { auto packet = makeTestingWritePacket(0, 1000, 1000); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet.time), folly::none); + makeAck(0, 1000, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(initCwnd + 1000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Steady, cubic.state()); } @@ -67,7 +67,7 @@ TEST_F(CubicHystartTest, NoDelayIncrease) { auto packet = makeTestingWritePacket(0, 1000, 1000, realNow); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 1000, realNow + 2us, packet.time), folly::none); + makeAck(0, 1000, realNow + 2us, packet.metadata.time), folly::none); EXPECT_EQ(initCwnd + 1000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Hystart, cubic.state()); } @@ -92,7 +92,10 @@ TEST_F(CubicHystartTest, AckTrain) { // Packet 0 is acked: cubic.onPacketAckOrLoss( makeAck( - 0, kLowSsthreshInMss * conn.udpSendPacketLen, realNow, packet0.time), + 0, + kLowSsthreshInMss * conn.udpSendPacketLen, + realNow, + packet0.metadata.time), folly::none); // Packet 1 is acked: cubic.onPacketAckOrLoss( @@ -100,7 +103,7 @@ TEST_F(CubicHystartTest, AckTrain) { 1, kLowSsthreshInMss * conn.udpSendPacketLen, realNow + 2us, - packet1.time), + packet1.metadata.time), folly::none); EXPECT_EQ( initCwnd + kLowSsthreshInMss * conn.udpSendPacketLen * 2, @@ -123,7 +126,7 @@ TEST_F(CubicHystartTest, NoAckTrainNoDelayIncrease) { auto packet = makeTestingWritePacket(0, 1000, 1000, realNow); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 1000, realNow + kAckCountingGap + 2us, packet.time), + makeAck(0, 1000, realNow + kAckCountingGap + 2us, packet.metadata.time), folly::none); EXPECT_EQ(initCwnd + 1000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Hystart, cubic.state()); @@ -142,7 +145,8 @@ TEST_F(CubicHystartTest, DelayIncrease) { makeTestingWritePacket(packetNum, fullSize, fullSize + totalSent); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(packetNum++, fullSize, Clock::now(), packet.time), folly::none); + makeAck(packetNum++, fullSize, Clock::now(), packet.metadata.time), + folly::none); totalSent += fullSize; } @@ -170,10 +174,12 @@ TEST_F(CubicHystartTest, DelayIncrease) { auto ackTimeIncrease = 2us; ackTime += ackTimeIncrease; cubic.onPacketAckOrLoss( - makeAck(firstPacketNum, 1000, ackTime, packet0.time), folly::none); + makeAck(firstPacketNum, 1000, ackTime, packet0.metadata.time), + folly::none); ackTime += ackTimeIncrease; cubic.onPacketAckOrLoss( - makeAck(secondPacketNum, 1000, ackTime, packet1.time), folly::none); + makeAck(secondPacketNum, 1000, ackTime, packet1.metadata.time), + folly::none); auto estimatedRttEndTarget = Clock::now(); auto packet2 = makeTestingWritePacket( @@ -182,9 +188,9 @@ TEST_F(CubicHystartTest, DelayIncrease) { conn.lossState.largestSent = packetNum; cubic.onPacketSent(packet2); // This will end current RTT round and start a new one next time Ack happens: - ackTime = packet2.time + ackTimeIncrease; + ackTime = packet2.metadata.time + ackTimeIncrease; cubic.onPacketAckOrLoss( - makeAck(packetNum, 1000, ackTime, packet2.time), folly::none); + makeAck(packetNum, 1000, ackTime, packet2.metadata.time), folly::none); packetNum++; auto cwndEndRound = cubic.getWritableBytes(); @@ -199,7 +205,8 @@ TEST_F(CubicHystartTest, DelayIncrease) { cubic.onPacketSent(packet); totalSent += 1000; ackTime += ackTimeIncrease; - moreAcks.push_back(makeAck(1 + packetNum, 1000, ackTime, packet.time)); + moreAcks.push_back( + makeAck(1 + packetNum, 1000, ackTime, packet.metadata.time)); packetNum++; } for (auto& ack : moreAcks) { @@ -214,7 +221,7 @@ TEST_F(CubicHystartTest, DelayIncrease) { totalSent += 1000; ackTime += ackTimeIncrease; cubic.onPacketAckOrLoss( - makeAck(packetNum, 1000, ackTime, packetEnd.time), folly::none); + makeAck(packetNum, 1000, ackTime, packetEnd.metadata.time), folly::none); EXPECT_EQ(cwndEndRound + 1000 * kAckSampling, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Steady, cubic.state()); @@ -243,16 +250,19 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) { auto ackTime = realNow; auto ackTimeIncrease = 2us; ackTime += ackTimeIncrease; - cubic.onPacketAckOrLoss(makeAck(0, 1, ackTime, packet0.time), folly::none); + cubic.onPacketAckOrLoss( + makeAck(0, 1, ackTime, packet0.metadata.time), folly::none); ackTime += ackTimeIncrease; - cubic.onPacketAckOrLoss(makeAck(1, 1, ackTime, packet1.time), folly::none); + cubic.onPacketAckOrLoss( + makeAck(1, 1, ackTime, packet1.metadata.time), folly::none); auto packet2 = makeTestingWritePacket(2, 10, 10 + totalSent, realNow); conn.lossState.largestSent = 2; cubic.onPacketSent(packet2); totalSent += 10; ackTime += ackTimeIncrease; - cubic.onPacketAckOrLoss(makeAck(2, 1, ackTime, packet2.time), folly::none); + cubic.onPacketAckOrLoss( + makeAck(2, 1, ackTime, packet2.metadata.time), folly::none); auto cwndEndRound = cubic.getWritableBytes(); // New RTT round, give currSampledRtt a value larger than previous RTT: @@ -263,7 +273,7 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) { conn.lossState.largestSent = i + 3; auto packet = makeTestingWritePacket(i + 3, 1, 1 + totalSent, realNow); ackTime += ackTimeIncrease; - moreAcks.push_back(makeAck(i + 3, 1, ackTime, packet.time)); + moreAcks.push_back(makeAck(i + 3, 1, ackTime, packet.metadata.time)); cubic.onPacketSent(packet); totalSent += 1; } @@ -279,7 +289,7 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) { totalSent += 1; ackTime += ackTimeIncrease; cubic.onPacketAckOrLoss( - makeAck(kAckSampling, 1, ackTime, packetEnd.time), folly::none); + makeAck(kAckSampling, 1, ackTime, packetEnd.metadata.time), folly::none); auto expectedCwnd = cwndEndRound + 1 * kAckSampling; // Cwnd < kLowSsthresh, won't exit Hystart state: ASSERT_LT(expectedCwnd, kLowSsthreshInMss * conn.udpSendPacketLen); diff --git a/quic/congestion_control/test/CubicRecoveryTest.cpp b/quic/congestion_control/test/CubicRecoveryTest.cpp index ce99f7feb..ef02ac0ae 100644 --- a/quic/congestion_control/test/CubicRecoveryTest.cpp +++ b/quic/congestion_control/test/CubicRecoveryTest.cpp @@ -57,7 +57,7 @@ TEST_F(CubicRecoveryTest, LossBeforeRecovery) { cubic.onPacketSent(packet); totalSent += 1000; cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet.time), folly::none); + makeAck(0, 1000, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(CubicStates::Hystart, cubic.state()); // Send three packets, lose second immediately. @@ -83,9 +83,9 @@ TEST_F(CubicRecoveryTest, LossBeforeRecovery) { totalSent += 1000; conn.lossState.largestSent = 4; cubic.onPacketAckOrLoss( - makeAck(3, 1000, Clock::now(), packet3.time), folly::none); + makeAck(3, 1000, Clock::now(), packet3.metadata.time), folly::none); cubic.onPacketAckOrLoss( - makeAck(4, 1000, Clock::now(), packet4.time), folly::none); + makeAck(4, 1000, Clock::now(), packet4.metadata.time), folly::none); auto cwndAfterRecovery = cubic.getCongestionWindow(); EXPECT_EQ(CubicStates::Steady, cubic.state()); @@ -105,7 +105,7 @@ TEST_F(CubicRecoveryTest, LossAfterRecovery) { auto packet = makeTestingWritePacket(0, 1000, 1000); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet.time), folly::none); + makeAck(0, 1000, Clock::now(), packet.metadata.time), folly::none); // Lose one packet. auto packet1 = makeTestingWritePacket(1, 1000, 2000); cubic.onPacketSent(packet1); @@ -152,13 +152,13 @@ TEST_F(CubicRecoveryTest, AckNotLargestNotChangeCwnd) { // the the rest are acked: cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet1.time), folly::none); + makeAck(0, 1000, Clock::now(), packet1.metadata.time), folly::none); cubic.onPacketAckOrLoss( - makeAck(1, 1000, Clock::now(), packet2.time), folly::none); + makeAck(1, 1000, Clock::now(), packet2.metadata.time), folly::none); cubic.onPacketAckOrLoss( - makeAck(2, 1000, Clock::now(), packet3.time), folly::none); + makeAck(2, 1000, Clock::now(), packet3.metadata.time), folly::none); cubic.onPacketAckOrLoss( - makeAck(3, 1000, Clock::now(), packet4.time), folly::none); + makeAck(3, 1000, Clock::now(), packet4.metadata.time), folly::none); // Still in recovery: EXPECT_EQ(CubicStates::FastRecovery, cubic.state()); diff --git a/quic/congestion_control/test/CubicStateTest.cpp b/quic/congestion_control/test/CubicStateTest.cpp index 8d02f1be7..16c37271b 100644 --- a/quic/congestion_control/test/CubicStateTest.cpp +++ b/quic/congestion_control/test/CubicStateTest.cpp @@ -35,7 +35,7 @@ TEST_F(CubicStateTest, HystartAck) { auto packet = makeTestingWritePacket(0, 0, 0); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 0, Clock::now(), packet.time), folly::none); + makeAck(0, 0, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(CubicStates::Hystart, cubic.state()); } @@ -54,7 +54,7 @@ TEST_F(CubicStateTest, FastRecoveryAck) { loss.addLostPacket(packet); cubic.onPacketAckOrLoss(folly::none, std::move(loss)); cubic.onPacketAckOrLoss( - makeAck(2, 1000, Clock::now(), packet1.time), folly::none); + makeAck(2, 1000, Clock::now(), packet1.metadata.time), folly::none); EXPECT_EQ(CubicStates::FastRecovery, cubic.state()); } @@ -71,7 +71,7 @@ TEST_F(CubicStateTest, FastRecoveryAckToSteady) { auto packet1 = makeTestingWritePacket(1, 1, 2); cubic.onPacketSent(packet1); cubic.onPacketAckOrLoss( - makeAck(1, 1, Clock::now(), packet1.time), folly::none); + makeAck(1, 1, Clock::now(), packet1.metadata.time), folly::none); EXPECT_EQ(CubicStates::Steady, cubic.state()); } @@ -95,7 +95,7 @@ TEST_F(CubicStateTest, SteadyAck) { auto packet = makeTestingWritePacket(0, 0, 0); cubic.onPacketSent(packet); cubic.onPacketAckOrLoss( - makeAck(0, 0, Clock::now(), packet.time), folly::none); + makeAck(0, 0, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(CubicStates::Steady, cubic.state()); } diff --git a/quic/congestion_control/test/CubicSteadyTest.cpp b/quic/congestion_control/test/CubicSteadyTest.cpp index 93851f141..c0d8a7342 100644 --- a/quic/congestion_control/test/CubicSteadyTest.cpp +++ b/quic/congestion_control/test/CubicSteadyTest.cpp @@ -29,7 +29,7 @@ TEST_F(CubicSteadyTest, CubicReduction) { conn.lossState.largestSent = 0; cubic.onPacketSent(packet0); cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet0.time), folly::none); + makeAck(0, 1000, Clock::now(), packet0.metadata.time), folly::none); EXPECT_EQ(3000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Steady, cubic.state()); diff --git a/quic/congestion_control/test/CubicTest.cpp b/quic/congestion_control/test/CubicTest.cpp index e4d7308ef..d330a7310 100644 --- a/quic/congestion_control/test/CubicTest.cpp +++ b/quic/congestion_control/test/CubicTest.cpp @@ -36,7 +36,7 @@ TEST_F(CubicTest, AckIncreaseWritable) { // Acking 50, now inflight become 50. Cwnd is init + 50 cubic.onPacketAckOrLoss( - makeAck(0, 50, Clock::now(), packet.time), folly::none); + makeAck(0, 50, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(initCwnd, cubic.getWritableBytes()); } @@ -63,7 +63,8 @@ TEST_F(CubicTest, PersistentCongestion) { auto packet2 = makeTestingWritePacket(1, initCwnd / 2, initCwnd / 2 + 1000); cubic.onPacketSent(packet2); cubic.onPacketAckOrLoss( - makeAck(1, initCwnd / 2, Clock::now(), packet2.time), folly::none); + makeAck(1, initCwnd / 2, Clock::now(), packet2.metadata.time), + folly::none); EXPECT_EQ(CubicStates::Steady, cubic.state()); // Verify both lastMaxCwndBytes and lastReductionTime are also reset in @@ -74,7 +75,7 @@ TEST_F(CubicTest, PersistentCongestion) { auto packet3 = makeTestingWritePacket(2, 3000, initCwnd / 2 + 1000 + 3000); cubic.onPacketSent(packet3); cubic.onPacketAckOrLoss( - makeAck(2, 3000, Clock::now(), packet3.time), folly::none); + makeAck(2, 3000, Clock::now(), packet3.metadata.time), folly::none); std::vector indices = getQLogEventIndices(QLogEventType::CongestionMetricUpdate, qLogger); @@ -125,7 +126,7 @@ TEST_F(CubicTest, CwndIncreaseAfterReduction) { conn.lossState.largestSent = 0; cubic.onPacketSent(packet0); cubic.onPacketAckOrLoss( - makeAck(0, 1000, Clock::now(), packet0.time), folly::none); + makeAck(0, 1000, Clock::now(), packet0.metadata.time), folly::none); // Cwnd increased by 1000, inflight = 0: EXPECT_EQ(3000, cubic.getWritableBytes()); EXPECT_EQ(CubicStates::Steady, cubic.state()); @@ -142,7 +143,7 @@ TEST_F(CubicTest, CwndIncreaseAfterReduction) { EXPECT_EQ(0, cubic.getWritableBytes()); cubic.onPacketAckOrLoss( - makeAck(1, 1000, Clock::now(), packet1.time), folly::none); + makeAck(1, 1000, Clock::now(), packet1.metadata.time), folly::none); // Cwnd >= 3000, inflight = 2000: EXPECT_GE(cubic.getWritableBytes(), 1000); CongestionController::LossEvent loss; @@ -152,7 +153,7 @@ TEST_F(CubicTest, CwndIncreaseAfterReduction) { EXPECT_GE(cubic.getWritableBytes(), 1400); // This won't bring state machine back to Steady since endOfRecovery = 3 cubic.onPacketAckOrLoss( - makeAck(3, 1000, Clock::now(), packet3.time), folly::none); + makeAck(3, 1000, Clock::now(), packet3.metadata.time), folly::none); // Cwnd no change, inflight = 0: EXPECT_GE(cubic.getWritableBytes(), 2400); EXPECT_EQ(CubicStates::FastRecovery, cubic.state()); @@ -162,7 +163,7 @@ TEST_F(CubicTest, CwndIncreaseAfterReduction) { cubic.onPacketSent(packet4); // This will bring state machine back to steady cubic.onPacketAckOrLoss( - makeAck(4, 1000, Clock::now(), packet4.time), folly::none); + makeAck(4, 1000, Clock::now(), packet4.metadata.time), folly::none); EXPECT_GE(cubic.getWritableBytes(), 2400); EXPECT_EQ(CubicStates::Steady, cubic.state()); @@ -197,7 +198,8 @@ TEST_F(CubicTest, AppIdle) { auto packet1 = makeTestingWritePacket(1, 1000, 2000); cubic.onPacketSent(packet1); cubic.onPacketAckOrLoss( - makeAck(1, 1000, reductionTime + 1000ms, packet1.time), folly::none); + makeAck(1, 1000, reductionTime + 1000ms, packet1.metadata.time), + folly::none); EXPECT_EQ(CubicStates::Steady, cubic.state()); EXPECT_GT(cubic.getCongestionWindow(), cwnd); cwnd = cubic.getCongestionWindow(); @@ -207,7 +209,8 @@ TEST_F(CubicTest, AppIdle) { auto packet2 = makeTestingWritePacket(2, 1000, 3000); cubic.onPacketSent(packet2); cubic.onPacketAckOrLoss( - makeAck(2, 1000, reductionTime + 2000ms, packet2.time), folly::none); + makeAck(2, 1000, reductionTime + 2000ms, packet2.metadata.time), + folly::none); EXPECT_EQ(cubic.getCongestionWindow(), cwnd); // 1 seconds of quiescence @@ -216,7 +219,8 @@ TEST_F(CubicTest, AppIdle) { auto packet3 = makeTestingWritePacket(3, 1000, 4000); cubic.onPacketSent(packet3); cubic.onPacketAckOrLoss( - makeAck(3, 1000, reductionTime + 3000ms, packet3.time), folly::none); + makeAck(3, 1000, reductionTime + 3000ms, packet3.metadata.time), + folly::none); EXPECT_GT(cubic.getCongestionWindow(), cwnd); auto expectedDelta = static_cast(std::floor( @@ -257,7 +261,7 @@ TEST_F(CubicTest, PacingGain) { EXPECT_EQ(cubic.getCongestionWindow() * 2, cwndBytes); })); cubic.onPacketAckOrLoss( - makeAck(0, 1500, Clock::now(), packet.time), folly::none); + makeAck(0, 1500, Clock::now(), packet.metadata.time), folly::none); EXPECT_EQ(CubicStates::Hystart, cubic.state()); auto packet1 = makeTestingWritePacket(1, 1500, 3000); @@ -285,7 +289,7 @@ TEST_F(CubicTest, PacingGain) { EXPECT_EQ(cubic.getCongestionWindow(), cwndBytes); })); cubic.onPacketAckOrLoss( - makeAck(2, 1500, Clock::now(), packet2.time), folly::none); + makeAck(2, 1500, Clock::now(), packet2.metadata.time), folly::none); EXPECT_EQ(CubicStates::Steady, cubic.state()); std::vector indices = diff --git a/quic/congestion_control/test/NewRenoTest.cpp b/quic/congestion_control/test/NewRenoTest.cpp index 7a188d39a..189d7d64b 100644 --- a/quic/congestion_control/test/NewRenoTest.cpp +++ b/quic/congestion_control/test/NewRenoTest.cpp @@ -27,7 +27,7 @@ CongestionController::LossEvent createLossEvent( RegularQuicWritePacket packet( ShortHeader(ProtectionType::KeyPhaseZero, connId, packetData.first)); loss.addLostPacket( - OutstandingPacket(std::move(packet), Clock::now(), 10, false, 10)); + OutstandingPacket(std::move(packet), Clock::now(), 10, false, 10, 0)); loss.lostBytes = packetData.second; } loss.lostPackets = lostPackets.size(); @@ -46,16 +46,20 @@ CongestionController::AckEvent createAckEvent( ack.ackedBytes = ackedSize; ack.ackedPackets.push_back( makeAckPacketFromOutstandingPacket(OutstandingPacket( - std::move(packet), packetSentTime, ackedSize, false, ackedSize))); + std::move(packet), packetSentTime, ackedSize, false, ackedSize, 0))); return ack; } -OutstandingPacket -createPacket(PacketNum packetNum, uint32_t size, TimePoint sendTime) { +OutstandingPacket createPacket( + PacketNum packetNum, + uint32_t size, + TimePoint sendTime, + uint64_t inflight = 0) { auto connId = getTestConnectionId(); RegularQuicWritePacket packet( ShortHeader(ProtectionType::KeyPhaseZero, connId, packetNum)); - return OutstandingPacket(std::move(packet), sendTime, size, false, size); + return OutstandingPacket( + std::move(packet), sendTime, size, false, size, inflight); } TEST_F(NewRenoTest, TestLoss) { @@ -130,7 +134,8 @@ TEST_F(NewRenoTest, TestSlowStartAck) { reno.onPacketSent(packet); EXPECT_EQ(reno.getBytesInFlight(), ackedSize); reno.onPacketAckOrLoss( - createAckEvent(ackPacketNum1, ackedSize, packet.time), folly::none); + createAckEvent(ackPacketNum1, ackedSize, packet.metadata.time), + folly::none); EXPECT_TRUE(reno.inSlowStart()); auto newWritableBytes = reno.getWritableBytes(); @@ -159,7 +164,8 @@ TEST_F(NewRenoTest, TestSteadyStateAck) { ackPacketNum1, ackedSize, Clock::now() - std::chrono::milliseconds(10)); reno.onPacketSent(packet1); reno.onPacketAckOrLoss( - createAckEvent(ackPacketNum1, ackedSize, packet1.time), folly::none); + createAckEvent(ackPacketNum1, ackedSize, packet1.metadata.time), + folly::none); EXPECT_FALSE(reno.inSlowStart()); auto newWritableBytes2 = reno.getWritableBytes(); @@ -169,7 +175,8 @@ TEST_F(NewRenoTest, TestSteadyStateAck) { auto packet2 = createPacket(ackPacketNum2, ackedSize, Clock::now()); reno.onPacketSent(packet2); reno.onPacketAckOrLoss( - createAckEvent(ackPacketNum2, ackedSize, packet2.time), folly::none); + createAckEvent(ackPacketNum2, ackedSize, packet2.metadata.time), + folly::none); EXPECT_FALSE(reno.inSlowStart()); auto newWritableBytes3 = reno.getWritableBytes(); diff --git a/quic/d6d/QuicD6DStateFunctions.cpp b/quic/d6d/QuicD6DStateFunctions.cpp index f99e47a10..d35e07b88 100644 --- a/quic/d6d/QuicD6DStateFunctions.cpp +++ b/quic/d6d/QuicD6DStateFunctions.cpp @@ -142,8 +142,8 @@ void detectPMTUBlackhole( auto& d6d = conn.d6d; // If d6d is not activated, or it's a d6d probe, or that the packet size is // less than base pmtu, then the loss is not caused by pmtu blackhole - if (d6d.state == D6DMachineState::DISABLED || packet.isD6DProbe || - packet.encodedSize <= d6d.basePMTU) { + if (d6d.state == D6DMachineState::DISABLED || packet.metadata.isD6DProbe || + packet.metadata.encodedSize <= d6d.basePMTU) { return; } @@ -152,7 +152,7 @@ void detectPMTUBlackhole( if (d6d.thresholdCounter && d6d.thresholdCounter->update( std::chrono::duration_cast( - packet.time.time_since_epoch()) + packet.metadata.time.time_since_epoch()) .count())) { LOG(ERROR) << "PMTU blackhole detected on packet loss, reducing PMTU to base"; diff --git a/quic/d6d/test/QuicD6DStateFunctionsTest.cpp b/quic/d6d/test/QuicD6DStateFunctionsTest.cpp index c3c81a67c..8967975a4 100644 --- a/quic/d6d/test/QuicD6DStateFunctionsTest.cpp +++ b/quic/d6d/test/QuicD6DStateFunctionsTest.cpp @@ -144,7 +144,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) { true, d6d.currentProbeSize); d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket( - pkt.packet.header.getPacketSequenceNum(), pkt.encodedSize); + pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize); d6d.raiser = std::make_unique(); auto mockRaiser = dynamic_cast(d6d.raiser.get()); EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize)) @@ -172,7 +172,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) { true, d6d.currentProbeSize); d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket( - pkt.packet.header.getPacketSequenceNum(), pkt.encodedSize); + pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize); d6d.raiser = std::make_unique(); auto mockRaiser = dynamic_cast(d6d.raiser.get()); EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize)) @@ -200,7 +200,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) { true, d6d.currentProbeSize); d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket( - pkt.packet.header.getPacketSequenceNum(), pkt.encodedSize); + pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize); d6d.raiser = std::make_unique(); auto mockRaiser = dynamic_cast(d6d.raiser.get()); EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize)) @@ -227,7 +227,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) { true, d6d.currentProbeSize); d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket( - pkt.packet.header.getPacketSequenceNum(), pkt.encodedSize); + pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize); d6d.raiser = std::make_unique(); auto mockRaiser = dynamic_cast(d6d.raiser.get()); EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize)) diff --git a/quic/loss/QuicLossFunctions.h b/quic/loss/QuicLossFunctions.h index 95acc9ce5..52a3b7a44 100644 --- a/quic/loss/QuicLossFunctions.h +++ b/quic/loss/QuicLossFunctions.h @@ -219,11 +219,11 @@ folly::Optional detectLossPackets( break; } auto currentPacketNumberSpace = pkt.packet.header.getPacketNumberSpace(); - if (currentPacketNumberSpace != pnSpace) { + if (currentPacketNumberSpace != pnSpace || pkt.metadata.isD6DProbe) { iter++; continue; } - bool lostByTimeout = (lossTime - pkt.time) > delayUntilLost; + bool lostByTimeout = (lossTime - pkt.metadata.time) > delayUntilLost; bool lostByReorder = (*largestAcked - currentPacketNum) > conn.lossState.reorderingThreshold; @@ -233,7 +233,7 @@ folly::Optional detectLossPackets( shouldSetTimer = true; break; } - if (pkt.isD6DProbe) { + if (pkt.metadata.isD6DProbe) { // It's a D6D probe, we'll mark it as lost to avoid its stale // ack from affecting PMTU. We don't add it to loss event to // avoid affecting congestion control when there's probably no @@ -267,7 +267,7 @@ folly::Optional detectLossPackets( if (pkt.associatedEvent) { conn.outstandings.packetEvents.erase(*pkt.associatedEvent); } - if (pkt.isHandshake && !processed) { + if (pkt.metadata.isHandshake && !processed) { if (currentPacketNumberSpace == PacketNumberSpace::Initial) { CHECK(conn.outstandings.initialPacketsCount); --conn.outstandings.initialPacketsCount; @@ -278,7 +278,7 @@ folly::Optional detectLossPackets( } } VLOG(10) << __func__ << " lost packetNum=" << currentPacketNum - << " handshake=" << pkt.isHandshake << " " << conn; + << " 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.outstandings.declaredLostCount++; @@ -310,7 +310,7 @@ folly::Optional detectLossPackets( << conn.outstandings.packets.empty() << " delayUntilLost" << delayUntilLost.count() << "us" << " " << conn; - getLossTime(conn, pnSpace) = delayUntilLost + earliest->time; + getLossTime(conn, pnSpace) = delayUntilLost + earliest->metadata.time; } if (lossEvent.largestLostPacketNum.hasValue()) { DCHECK(lossEvent.largestLostSentTime && lossEvent.smallestLostSentTime); @@ -441,7 +441,7 @@ void markZeroRttPacketsLost( iter->packet.header.getProtectionType() == ProtectionType::ZeroRtt; if (isZeroRttPacket) { auto& pkt = *iter; - DCHECK(!pkt.isHandshake); + DCHECK(!pkt.metadata.isHandshake); bool processed = pkt.associatedEvent && !conn.outstandings.packetEvents.count(*pkt.associatedEvent); lossVisitor(conn, pkt.packet, processed); diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 533c429f1..aa869dfae 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -224,7 +224,13 @@ PacketNum QuicLossFunctionsTest::sendPacket( encodedSize += packet.body->computeChainDataLength(); } auto outstandingPacket = OutstandingPacket( - packet.packet, time, encodedSize, isHandshake, isD6DProbe, encodedSize); + packet.packet, + time, + encodedSize, + isHandshake, + isD6DProbe, + encodedSize, + 0); outstandingPacket.associatedEvent = associatedEvent; if (isHandshake) { conn.lossState.lastHandshakePacketSentTime = time; @@ -993,7 +999,7 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) { iter < getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 5; iter++) { - if (iter->isHandshake) { + if (iter->metadata.isHandshake) { conn->outstandings.handshakePacketsCount--; } } @@ -1045,7 +1051,7 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckForLoss) { RegularQuicWritePacket outstandingRegularPacket(std::move(longHeader)); auto now = Clock::now(); conn->outstandings.packets.emplace_back( - OutstandingPacket(outstandingRegularPacket, now, 0, false, 0)); + OutstandingPacket(outstandingRegularPacket, now, 0, false, 0, 0)); bool testLossMarkFuncCalled = false; auto testLossMarkFunc = [&](auto& /* conn */, auto&, bool) { diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index e652261e7..86a9a0499 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -917,7 +917,7 @@ void onServerReadDataFromOpen( break; } case QuicWriteFrame::Type::PingFrame_E: - if (!packet.isD6DProbe) { + if (!packet.metadata.isD6DProbe) { conn.pendingEvents.cancelPingTimeout = true; } return; diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index b538ab3af..c6a950ba0 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -107,8 +107,9 @@ void processAckFrame( break; } VLOG(10) << __func__ << " acked packetNum=" << currentPacketNum - << " space=" << currentPacketNumberSpace - << " handshake=" << (int)rPacketIt->isHandshake << " " << conn; + << " space=" << currentPacketNumberSpace << " handshake=" + << (int)((rPacketIt->metadata.isHandshake) ? 1 : 0) << " " + << conn; // If we hit a packet which has been lost we need to count the spurious // loss and ignore all other processing. // TODO also remove any stream data from the loss buffer. @@ -124,7 +125,7 @@ void processAckFrame( } bool needsProcess = !rPacketIt->associatedEvent || conn.outstandings.packetEvents.count(*rPacketIt->associatedEvent); - if (rPacketIt->isHandshake && needsProcess) { + if (rPacketIt->metadata.isHandshake && needsProcess) { if (currentPacketNumberSpace == PacketNumberSpace::Initial) { ++initialPacketAcked; } else { @@ -132,21 +133,29 @@ void processAckFrame( ++handshakePacketAcked; } } - ack.ackedBytes += rPacketIt->encodedSize; + ack.ackedBytes += rPacketIt->metadata.encodedSize; if (rPacketIt->associatedEvent) { ++clonedPacketsAcked; } // Update RTT if current packet is the largestAcked in the frame: - auto ackReceiveTimeOrNow = - ackReceiveTime > rPacketIt->time ? ackReceiveTime : Clock::now(); + auto ackReceiveTimeOrNow = ackReceiveTime > rPacketIt->metadata.time + ? ackReceiveTime + : Clock::now(); auto rttSample = std::chrono::duration_cast( - ackReceiveTimeOrNow - rPacketIt->time); + ackReceiveTimeOrNow - rPacketIt->metadata.time); if (!ack.implicit && currentPacketNum == frame.largestAcked) { + InstrumentationObserver::PacketRTT packetRTT( + ackReceiveTimeOrNow, rttSample, frame.ackDelay, *rPacketIt); + for (const auto& observer : conn.instrumentationObservers_) { + conn.pendingCallbacks.emplace_back([observer, packetRTT] { + observer->rttSampleGenerated(packetRTT); + }); + } updateRtt(conn, rttSample, frame.ackDelay); } // D6D probe acked. Only if it's for the last probe do we // trigger state change - if (rPacketIt->isD6DProbe) { + if (rPacketIt->metadata.isD6DProbe) { CHECK(conn.d6d.lastProbe); if (!rPacketIt->declaredLost && currentPacketNum == conn.d6d.lastProbe->packetNum) { @@ -168,27 +177,27 @@ void processAckFrame( if (!ack.largestAckedPacket || *ack.largestAckedPacket < currentPacketNum) { ack.largestAckedPacket = currentPacketNum; - ack.largestAckedPacketSentTime = rPacketIt->time; + ack.largestAckedPacketSentTime = rPacketIt->metadata.time; ack.largestAckedPacketAppLimited = rPacketIt->isAppLimited; } - if (!ack.implicit && ackReceiveTime > rPacketIt->time) { + if (!ack.implicit && ackReceiveTime > rPacketIt->metadata.time) { ack.mrttSample = std::min(ack.mrttSample.value_or(rttSample), rttSample); } - conn.lossState.totalBytesAcked += rPacketIt->encodedSize; + conn.lossState.totalBytesAcked += rPacketIt->metadata.encodedSize; conn.lossState.totalBytesSentAtLastAck = conn.lossState.totalBytesSent; conn.lossState.totalBytesAckedAtLastAck = conn.lossState.totalBytesAcked; if (!lastAckedPacketSentTime) { - lastAckedPacketSentTime = rPacketIt->time; + lastAckedPacketSentTime = rPacketIt->metadata.time; } conn.lossState.lastAckedTime = ackReceiveTime; conn.lossState.adjustedLastAckedTime = ackReceiveTime - frame.ackDelay; ack.ackedPackets.push_back( CongestionController::AckEvent::AckPacket::Builder() - .setSentTime(rPacketIt->time) - .setEncodedSize(rPacketIt->encodedSize) + .setSentTime(rPacketIt->metadata.time) + .setEncodedSize(rPacketIt->metadata.encodedSize) .setLastAckedPacketInfo(std::move(rPacketIt->lastAckedPacketInfo)) - .setTotalBytesSentThen(rPacketIt->totalBytesSent) + .setTotalBytesSentThen(rPacketIt->metadata.totalBytesSent) .setAppLimited(rPacketIt->isAppLimited) .build()); rPacketIt++; @@ -256,7 +265,7 @@ void clearOldOutstandingPackets( while (opItr != conn.outstandings.packets.end()) { // This case can happen when we have buffered an undecryptable ACK and // are able to decrypt it later. - if (time < opItr->time) { + if (time < opItr->metadata.time) { break; } if (opItr->packet.header.getPacketNumberSpace() != pnSpace) { @@ -268,7 +277,7 @@ void clearOldOutstandingPackets( eraseBegin = opItr; continue; } - auto timeSinceSent = time - opItr->time; + auto timeSinceSent = time - opItr->metadata.time; if (opItr->declaredLost && timeSinceSent > threshold) { opItr++; conn.outstandings.declaredLostCount--; diff --git a/quic/state/OutstandingPacket.h b/quic/state/OutstandingPacket.h index df196ec35..cf7810ac8 100644 --- a/quic/state/OutstandingPacket.h +++ b/quic/state/OutstandingPacket.h @@ -12,11 +12,8 @@ #include namespace quic { -// Data structure to represent outstanding retransmittable packets -struct OutstandingPacket { - // Structure representing the frames that are outstanding including the header - // that was sent. - RegularQuicWritePacket packet; + +struct OutstandingPacketMetadata { // Time that the packet was sent. TimePoint time; // Size of the packet sent on the wire. @@ -28,6 +25,33 @@ struct OutstandingPacket { // Total sent bytes on this connection including this packet itself when this // packet is sent. uint64_t totalBytesSent; + // Bytes in flight on this connection including this packet itself when this + // packet is sent. + uint64_t inflightBytes; + + OutstandingPacketMetadata( + TimePoint timeIn, + uint32_t encodedSizeIn, + bool isHandshakeIn, + bool isD6DProbeIn, + uint64_t totalBytesSentIn, + uint64_t inflightBytesIn) + : time(std::move(timeIn)), + encodedSize(encodedSizeIn), + isHandshake(isHandshakeIn), + isD6DProbe(isD6DProbeIn), + totalBytesSent(totalBytesSentIn), + inflightBytes(inflightBytesIn) {} +}; + +// Data structure to represent outstanding retransmittable packets +struct OutstandingPacket { + // Structure representing the frames that are outstanding including the header + // that was sent. + RegularQuicWritePacket packet; + // Structure representing a collection of metrics and important information + // about the packet. + OutstandingPacketMetadata metadata; // Information regarding the last acked packet on this connection when this // packet is sent. struct LastAckedPacketInfo { @@ -73,13 +97,16 @@ struct OutstandingPacket { TimePoint timeIn, uint32_t encodedSizeIn, bool isHandshakeIn, - uint64_t totalBytesSentIn) + uint64_t totalBytesSentIn, + uint64_t inflightBytesIn) : packet(std::move(packetIn)), - time(std::move(timeIn)), - encodedSize(encodedSizeIn), - isHandshake(isHandshakeIn), - isD6DProbe(false), - totalBytesSent(totalBytesSentIn) {} + metadata(OutstandingPacketMetadata( + std::move(timeIn), + encodedSizeIn, + isHandshakeIn, + false, + totalBytesSentIn, + inflightBytesIn)) {} OutstandingPacket( RegularQuicWritePacket packetIn, @@ -87,12 +114,15 @@ struct OutstandingPacket { uint32_t encodedSizeIn, bool isHandshakeIn, bool isD6DProbeIn, - uint64_t totalBytesSentIn) + uint64_t totalBytesSentIn, + uint64_t inflightBytesIn) : packet(std::move(packetIn)), - time(std::move(timeIn)), - encodedSize(encodedSizeIn), - isHandshake(isHandshakeIn), - isD6DProbe(isD6DProbeIn), - totalBytesSent(totalBytesSentIn) {} + metadata(OutstandingPacketMetadata( + std::move(timeIn), + encodedSizeIn, + isHandshakeIn, + isD6DProbeIn, + totalBytesSentIn, + inflightBytesIn)) {} }; } // namespace quic diff --git a/quic/state/StateData.h b/quic/state/StateData.h index e835a2007..2c7512147 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -233,7 +233,7 @@ struct CongestionController { void addLostPacket(const OutstandingPacket& packet) { if (std::numeric_limits::max() - lostBytes < - packet.encodedSize) { + packet.metadata.encodedSize) { throw QuicInternalException( "LossEvent: lostBytes overflow", LocalErrorCode::LOST_BYTES_OVERFLOW); @@ -241,12 +241,14 @@ struct CongestionController { PacketNum packetNum = packet.packet.header.getPacketSequenceNum(); largestLostPacketNum = std::max(packetNum, largestLostPacketNum.value_or(packetNum)); - lostBytes += packet.encodedSize; + lostBytes += packet.metadata.encodedSize; lostPackets++; - largestLostSentTime = - std::max(packet.time, largestLostSentTime.value_or(packet.time)); - smallestLostSentTime = - std::min(packet.time, smallestLostSentTime.value_or(packet.time)); + largestLostSentTime = std::max( + packet.metadata.time, + largestLostSentTime.value_or(packet.metadata.time)); + smallestLostSentTime = std::min( + packet.metadata.time, + smallestLostSentTime.value_or(packet.metadata.time)); } }; diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index 66fea0dd1..f21706f48 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -11,6 +11,8 @@ #include +#include +#include #include #include #include @@ -52,7 +54,7 @@ TEST_P(AckHandlersTest, TestAckMultipleSequentialBlocks) { WriteStreamFrame frame(currentStreamId++, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); conn.outstandings.packets.emplace_back(OutstandingPacket( - std::move(regularPacket), sentTime, 1, false, packetNum)); + std::move(regularPacket), sentTime, 1, false, packetNum, 0)); } ReadAckFrame ackFrame; ackFrame.largestAcked = 101; @@ -123,7 +125,7 @@ TEST_P(AckHandlersTest, TestAckMultipleSequentialBlocksLoss) { WriteStreamFrame frame(currentStreamId++, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); conn.outstandings.packets.emplace_back(OutstandingPacket( - std::move(regularPacket), sentTime, 1, false, packetNum)); + std::move(regularPacket), sentTime, 1, false, packetNum, 0)); } ReadAckFrame ackFrame; ackFrame.largestAcked = 101; @@ -258,7 +260,7 @@ TEST_P(AckHandlersTest, TestAckBlocksWithGaps) { WriteStreamFrame frame(currentStreamId++, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); conn.outstandings.packets.emplace_back(OutstandingPacket( - std::move(regularPacket), Clock::now(), 1, false, packetNum)); + std::move(regularPacket), Clock::now(), 1, false, packetNum, 0)); } ReadAckFrame ackFrame; @@ -356,7 +358,7 @@ TEST_P(AckHandlersTest, TestNonSequentialPacketNumbers) { WriteStreamFrame frame(current++, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); conn.outstandings.packets.emplace_back(OutstandingPacket( - std::move(regularPacket), Clock::now(), 1, false, packetNum)); + std::move(regularPacket), Clock::now(), 1, false, packetNum, 0)); } for (PacketNum packetNum = 20; packetNum < 40; packetNum += 3) { @@ -365,7 +367,7 @@ TEST_P(AckHandlersTest, TestNonSequentialPacketNumbers) { current += 3; regularPacket.frames.emplace_back(std::move(frame)); conn.outstandings.packets.emplace_back(OutstandingPacket( - std::move(regularPacket), Clock::now(), 1, false, packetNum)); + std::move(regularPacket), Clock::now(), 1, false, packetNum, 0)); } ReadAckFrame ackFrame; @@ -446,7 +448,7 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) { conn.ackStates.appDataAckState.acks.insert(500, 700); firstPacket.frames.emplace_back(std::move(firstAckFrame)); conn.outstandings.packets.emplace_back( - OutstandingPacket(std::move(firstPacket), Clock::now(), 0, false, 0)); + OutstandingPacket(std::move(firstPacket), Clock::now(), 0, false, 0, 0)); auto secondPacket = createNewPacket(101 /* packetNum */, GetParam()); WriteAckFrame secondAckFrame; @@ -456,7 +458,7 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) { conn.ackStates.appDataAckState.acks.insert(1002, 1090); secondPacket.frames.emplace_back(std::move(secondAckFrame)); conn.outstandings.packets.emplace_back( - OutstandingPacket(std::move(secondPacket), Clock::now(), 0, false, 0)); + OutstandingPacket(std::move(secondPacket), Clock::now(), 0, false, 0, 0)); ReadAckFrame firstReceivedAck; firstReceivedAck.largestAcked = 100; @@ -516,8 +518,8 @@ TEST_P(AckHandlersTest, NoNewAckedPacket) { conn.lossState.ptoCount = 1; PacketNum packetAfterRtoNum = 10; auto packetAfterRto = createNewPacket(packetAfterRtoNum, GetParam()); - conn.outstandings.packets.emplace_back( - OutstandingPacket(std::move(packetAfterRto), Clock::now(), 0, false, 0)); + conn.outstandings.packets.emplace_back(OutstandingPacket( + std::move(packetAfterRto), Clock::now(), 0, false, 0, 0)); ReadAckFrame ackFrame; ackFrame.largestAcked = 5; @@ -563,12 +565,12 @@ TEST_P(AckHandlersTest, AckPacketNumDoesNotExist) { PacketNum packetNum1 = 9; auto regularPacket1 = createNewPacket(packetNum1, GetParam()); conn.outstandings.packets.emplace_back( - std::move(regularPacket1), Clock::now(), 0, false, 0); + std::move(regularPacket1), Clock::now(), 0, false, 0, 0); PacketNum packetNum2 = 10; auto regularPacket2 = createNewPacket(packetNum2, GetParam()); conn.outstandings.packets.emplace_back( - std::move(regularPacket2), Clock::now(), 0, false, 0); + std::move(regularPacket2), Clock::now(), 0, false, 0, 0); // Ack a packet one higher than the packet so that we don't trigger reordering // threshold. @@ -601,7 +603,8 @@ TEST_P(AckHandlersTest, TestHandshakeCounterUpdate) { Clock::now(), 0, packetNum % 2 && GetParam() != PacketNumberSpace::AppData, - packetNum / 2); + packetNum / 2, + 0); if (GetParam() == PacketNumberSpace::Initial) { conn.outstandings.initialPacketsCount += packetNum % 2; } else if (GetParam() == PacketNumberSpace::Handshake) { @@ -683,8 +686,8 @@ TEST_P(AckHandlersTest, NoSkipAckVisitor) { // We need to at least have one frame to trigger ackVisitor WriteStreamFrame frame(0, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); - conn.outstandings.packets.emplace_back( - OutstandingPacket(std::move(regularPacket), Clock::now(), 1, false, 1)); + conn.outstandings.packets.emplace_back(OutstandingPacket( + std::move(regularPacket), Clock::now(), 1, false, 1, 0)); ReadAckFrame ackFrame; ackFrame.largestAcked = 0; ackFrame.ackBlocks.emplace_back(0, 0); @@ -726,7 +729,7 @@ TEST_P(AckHandlersTest, SkipAckVisitor) { WriteStreamFrame frame(0, 0, 0, true); regularPacket.frames.emplace_back(std::move(frame)); OutstandingPacket outstandingPacket( - std::move(regularPacket), Clock::now(), 1, false, 1); + std::move(regularPacket), Clock::now(), 1, false, 1, 0); // Give this outstandingPacket an associatedEvent that's not in // outstandings.packetEvents outstandingPacket.associatedEvent.emplace(PacketNumberSpace::AppData, 0); @@ -767,12 +770,12 @@ TEST_P(AckHandlersTest, NoDoubleProcess) { regularPacket2.frames.push_back(frame); OutstandingPacket outstandingPacket1( - std::move(regularPacket1), Clock::now(), 1, false, 1); + std::move(regularPacket1), Clock::now(), 1, false, 1, 0); outstandingPacket1.associatedEvent.emplace( PacketNumberSpace::AppData, packetNum1); OutstandingPacket outstandingPacket2( - std::move(regularPacket2), Clock::now(), 1, false, 1); + std::move(regularPacket2), Clock::now(), 1, false, 1, 0); // The seconds packet has the same PacketEvent outstandingPacket2.associatedEvent.emplace( PacketNumberSpace::AppData, packetNum1); @@ -829,7 +832,7 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) { auto regularPacket1 = createNewPacket(packetNum1, GetParam()); regularPacket1.frames.push_back(frame); OutstandingPacket outstandingPacket1( - std::move(regularPacket1), Clock::now(), 1, false, 1); + std::move(regularPacket1), Clock::now(), 1, false, 1, 0); outstandingPacket1.associatedEvent.emplace( PacketNumberSpace::AppData, packetNum1); @@ -838,7 +841,7 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) { auto regularPacket2 = createNewPacket(packetNum2, GetParam()); regularPacket2.frames.push_back(frame); OutstandingPacket outstandingPacket2( - std::move(regularPacket2), Clock::now(), 1, false, 1); + std::move(regularPacket2), Clock::now(), 1, false, 1, 0); conn.outstandings.packets.push_back(std::move(outstandingPacket1)); conn.outstandings.packets.push_back(std::move(outstandingPacket2)); @@ -877,7 +880,7 @@ TEST_P(AckHandlersTest, UpdateMaxAckDelay) { auto regularPacket = createNewPacket(packetNum, GetParam()); auto sentTime = Clock::now(); conn.outstandings.packets.emplace_back( - OutstandingPacket(std::move(regularPacket), sentTime, 1, false, 1)); + OutstandingPacket(std::move(regularPacket), sentTime, 1, false, 1, 0)); ReadAckFrame ackFrame; // ackDelay has no effect on mrtt @@ -933,7 +936,8 @@ TEST_P(AckHandlersTest, AckNotOutstandingButLoss) { Clock::now() - delayUntilLost - 20ms, 1, false, - 1); + 1, + 0); conn.outstandings.packets.push_back(std::move(outstandingPacket)); conn.outstandings.clonedPacketsCount++; @@ -976,7 +980,8 @@ TEST_P(AckHandlersTest, UpdatePendingAckStates) { sentTime, 111, false, - conn.lossState.totalBytesSent + 111)); + conn.lossState.totalBytesSent + 111, + 0)); conn.lossState.totalBytesSent += 111; ReadAckFrame ackFrame; @@ -1019,7 +1024,8 @@ TEST_P(AckHandlersTest, AckEventCreation) { largestSentTime, 1, false /* handshake */, - packetNum); + packetNum, + 0); sentPacket.isAppLimited = (packetNum % 2); conn.outstandings.packets.emplace_back(sentPacket); packetNum++; @@ -1073,7 +1079,8 @@ TEST_P(AckHandlersTest, ImplictAckEventCreation) { largestSentTime, 1, false /* handshake */, - packetNum); + packetNum, + packetNum + 1); sentPacket.isAppLimited = (packetNum % 2); conn.outstandings.packets.emplace_back(sentPacket); packetNum++; @@ -1107,6 +1114,114 @@ TEST_P(AckHandlersTest, ImplictAckEventCreation) { ackTime); } +TEST_P(AckHandlersTest, TestRTTPacketObserverCallback) { + QuicServerConnectionState conn( + FizzServerQuicHandshakeContext::Builder().build()); + auto mockCongestionController = std::make_unique(); + conn.congestionController = std::move(mockCongestionController); + + // Register 1 instrumentation observer + auto ib = MockInstrumentationObserver(); + conn.instrumentationObservers_.emplace_back(&ib); + + PacketNum packetNum = 0; + StreamId streamid = 0; + TimePoint sentTime; + std::vector packetRcvTime; + while (packetNum < 30) { + auto regularPacket = createNewPacket(packetNum, GetParam()); + WriteStreamFrame frame(streamid++, 0, 0, true); + regularPacket.frames.emplace_back(std::move(frame)); + sentTime = Clock::now() - 100ms + std::chrono::milliseconds(packetNum); + packetRcvTime.emplace_back(sentTime); + OutstandingPacket sentPacket( + std::move(regularPacket), + sentTime, + 1, + false /* handshake */, + packetNum, + packetNum + 1); + sentPacket.isAppLimited = false; + conn.outstandings.packets.emplace_back(sentPacket); + packetNum++; + } + + struct ackPacketData { + PacketNum startSeq, endSeq; + std::chrono::milliseconds ackDelay; + TimePoint ackTime; + ReadAckFrame ackFrame; + + explicit ackPacketData( + PacketNum startSeqIn, + PacketNum endSeqIn, + std::chrono::milliseconds ackDelayIn) + : startSeq(startSeqIn), + endSeq(endSeqIn), + ackDelay(ackDelayIn), + ackTime(Clock::now() + 5ms) { + ackFrame.largestAcked = endSeq; + ackFrame.ackDelay = ackDelay; + ackFrame.ackBlocks.emplace_back(startSeq, endSeq); + } + }; + + // See each emplace as the ACK Block [X, Y] with size (Y-X+1) + std::vector ackVec; + // Sequential test + ackVec.emplace_back(0, 5, 4ms); // +1 callback + ackVec.emplace_back(6, 10, 5ms); // +1 + ackVec.emplace_back(11, 15, 6ms); // +1 + // Out-of-order test + // + // Its important to check the if + // largestAcked - currentPacketNum > reorderingThreshold (currently 3) + // else it can trigger InstrumentationObserver::packetLossDetected + // and increase the number of callbacks + ackVec.emplace_back(18, 18, 0ms); // +1 + ackVec.emplace_back(16, 17, 2ms); // +1 + ackVec.emplace_back(19, 29, 12ms); // +1 = 6 callbacks + + // 0 pending callbacks + EXPECT_EQ(0, size(conn.pendingCallbacks)); + + for (const auto ackData : ackVec) { + processAckFrame( + conn, + GetParam(), + ackData.ackFrame, + [](const auto&, const auto&, const auto&) {}, + [](auto&, auto&, bool) {}, + ackData.ackTime); + } + + // see above + EXPECT_EQ(6, size(conn.pendingCallbacks)); + + for (const auto ackData : ackVec) { + auto rttSample = std::chrono::duration_cast( + ackData.ackTime - packetRcvTime[ackData.endSeq]); + EXPECT_CALL( + ib, + rttSampleGenerated(AllOf( + Field( + &InstrumentationObserver::PacketRTT::rcvTime, ackData.ackTime), + Field(&InstrumentationObserver::PacketRTT::rttSample, rttSample), + Field( + &InstrumentationObserver::PacketRTT::ackDelay, + ackData.ackDelay), + Field( + &InstrumentationObserver::PacketRTT::metadata, + Field( + &quic::OutstandingPacketMetadata::inflightBytes, + ackData.endSeq + 1))))); + } + + for (auto& callback : conn.pendingCallbacks) { + callback(); + } +} + INSTANTIATE_TEST_CASE_P( AckHandlersTests, AckHandlersTest, diff --git a/quic/state/test/QuicStateFunctionsTest.cpp b/quic/state/test/QuicStateFunctionsTest.cpp index 65b384931..0c7747ed0 100644 --- a/quic/state/test/QuicStateFunctionsTest.cpp +++ b/quic/state/test/QuicStateFunctionsTest.cpp @@ -553,43 +553,50 @@ TEST_F(QuicStateFunctionsTest, GetOutstandingPackets) { Clock::now(), 135, false, + 0, 0); conn.outstandings.packets.emplace_back( makeTestLongPacket(LongHeader::Types::Handshake), Clock::now(), 1217, false, + 0, 0); conn.outstandings.packets.emplace_back( - makeTestShortPacket(), Clock::now(), 5556, false, 0); + makeTestShortPacket(), Clock::now(), 5556, false, 0, 0); conn.outstandings.packets.emplace_back( makeTestLongPacket(LongHeader::Types::Initial), Clock::now(), 56, false, + 0, 0); conn.outstandings.packets.emplace_back( - makeTestShortPacket(), Clock::now(), 6665, false, 0); + makeTestShortPacket(), Clock::now(), 6665, false, 0, 0); EXPECT_EQ( 135, - getFirstOutstandingPacket(conn, PacketNumberSpace::Initial)->encodedSize); + getFirstOutstandingPacket(conn, PacketNumberSpace::Initial) + ->metadata.encodedSize); EXPECT_EQ( 56, - getLastOutstandingPacket(conn, PacketNumberSpace::Initial)->encodedSize); + getLastOutstandingPacket(conn, PacketNumberSpace::Initial) + ->metadata.encodedSize); EXPECT_EQ( 1217, getFirstOutstandingPacket(conn, PacketNumberSpace::Handshake) - ->encodedSize); + ->metadata.encodedSize); EXPECT_EQ( 1217, getFirstOutstandingPacket(conn, PacketNumberSpace::Handshake) - ->encodedSize); + ->metadata.encodedSize); EXPECT_EQ( 5556, - getFirstOutstandingPacket(conn, PacketNumberSpace::AppData)->encodedSize); + getFirstOutstandingPacket(conn, PacketNumberSpace::AppData) + ->metadata.encodedSize); EXPECT_EQ( 6665, - getLastOutstandingPacket(conn, PacketNumberSpace::AppData)->encodedSize); + getLastOutstandingPacket(conn, PacketNumberSpace::AppData) + ->metadata.encodedSize); } TEST_F(QuicStateFunctionsTest, UpdateLargestReceivePacketsAtLatCloseSent) { diff --git a/quic/state/test/StateDataTest.cpp b/quic/state/test/StateDataTest.cpp index 4e3940cd1..49a34a42a 100644 --- a/quic/state/test/StateDataTest.cpp +++ b/quic/state/test/StateDataTest.cpp @@ -35,7 +35,8 @@ TEST_F(StateDataTest, SingleLostPacketEvent) { getTestConnectionId(), 100, kVersion)); - OutstandingPacket outstandingPacket(packet, Clock::now(), 1234, false, 1234); + OutstandingPacket outstandingPacket( + packet, Clock::now(), 1234, false, 1234, 0); CongestionController::LossEvent loss; loss.addLostPacket(outstandingPacket); EXPECT_EQ(1234, loss.lostBytes); @@ -50,7 +51,7 @@ TEST_F(StateDataTest, MultipleLostPacketsEvent) { 100, kVersion)); OutstandingPacket outstandingPacket1( - packet1, Clock::now(), 1234, false, 1234); + packet1, Clock::now(), 1234, false, 1234, 0); RegularQuicWritePacket packet2(LongHeader( LongHeader::Types::Initial, @@ -59,7 +60,7 @@ TEST_F(StateDataTest, MultipleLostPacketsEvent) { 110, kVersion)); OutstandingPacket outstandingPacket2( - packet2, Clock::now(), 1357, false, 1357); + packet2, Clock::now(), 1357, false, 1357, 0); CongestionController::LossEvent loss; loss.addLostPacket(outstandingPacket1);