Adding packet rtt sampling to instrumentationObserver (#178)

Summary:
Due to high number of RTT samples I refactored the OutstandingPacket to
split the packet data and packet metrics. We know have access to metrics
without the need of saving the packet data.

Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/178

Reviewed By: mjoras

Differential Revision: D23711641

Pulled By: bschlinker

fbshipit-source-id: 53791f1f6f6e184f37afca991a873af05909fbd2

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<OutstandingPacket::LastAckedPacketInfo>
+        lastAckedPacketInfo;
   };
 
   struct ObserverLossEvent {
@@ -52,6 +55,26 @@ class InstrumentationObserver {
     const TimePoint lossTime;
     std::vector<LostPacket> 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<OutstandingPacket::LastAckedPacketInfo>
+        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.

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;
     }
 

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;

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(

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());

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(_, _))

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;
 }
 

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)
+      .setSentTime(outstandingPacket.metadata.time)
-      .setEncodedSize(outstandingPacket.encodedSize)
+      .setEncodedSize(outstandingPacket.metadata.encodedSize)
       .setLastAckedPacketInfo(std::move(outstandingPacket.lastAckedPacketInfo))
-      .setTotalBytesSentThen(outstandingPacket.totalBytesSent)
+      .setTotalBytesSentThen(outstandingPacket.metadata.totalBytesSent)
       .setAppLimited(outstandingPacket.isAppLimited)
       .build();
 }

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.

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;
   }
 }
 

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_

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

quic/congestion_control/QuicCubic.cpp

@@ -92,12 +92,12 @@ void Cubic::onPersistentCongestion() {
 
 void Cubic::onPacketSent(const OutstandingPacket& packet) {
   if (std::numeric_limits<uint64_t>::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) {

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.lastAckedPacketInfo.emplace(
-  packet1.time = ackTime + 500us;
+      packet.metadata.time, ackTime, ackTime, 1000, 0);
+  packet1.metadata.time = ackTime + 500us;
   CongestionController::AckEvent ackEvent1;
   ackEvent1.ackedBytes = 1000;
   ackEvent1.ackTime = ackTime + 2000us;

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);
 }
 

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

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
+  OutstandingPacket createPacket(
-  createPacket(PacketNum packetNum, uint32_t size, uint64_t totalSent) {
+      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(

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);

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());

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());
 }
 

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());
 

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<int> 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<int64_t>(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<int> indices =

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
+OutstandingPacket createPacket(
-createPacket(PacketNum packetNum, uint32_t size, TimePoint sendTime) {
+    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();

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 ||
+  if (d6d.state == D6DMachineState::DISABLED || packet.metadata.isD6DProbe ||
-      packet.encodedSize <= d6d.basePMTU) {
+      packet.metadata.encodedSize <= d6d.basePMTU) {
     return;
   }
 
@@ -152,7 +152,7 @@ void detectPMTUBlackhole(
   if (d6d.thresholdCounter &&
       d6d.thresholdCounter->update(
           std::chrono::duration_cast<std::chrono::microseconds>(
-              packet.time.time_since_epoch())
+              packet.metadata.time.time_since_epoch())
               .count())) {
     LOG(ERROR)
         << "PMTU blackhole detected on packet loss, reducing PMTU to base";

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<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(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<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(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<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(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<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
   EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))

quic/loss/QuicLossFunctions.h

@@ -219,11 +219,11 @@ folly::Optional<CongestionController::LossEvent> 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<CongestionController::LossEvent> 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<CongestionController::LossEvent> 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<CongestionController::LossEvent> 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<CongestionController::LossEvent> 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);

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) {

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;

quic/state/AckHandlers.cpp

@@ -107,8 +107,9 @@ void processAckFrame(
         break;
       }
       VLOG(10) << __func__ << " acked packetNum=" << currentPacketNum
-               << " space=" << currentPacketNumberSpace
+               << " space=" << currentPacketNumberSpace << " handshake="
-               << " handshake=" << (int)rPacketIt->isHandshake << " " << conn;
+               << (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 =
+      auto ackReceiveTimeOrNow = ackReceiveTime > rPacketIt->metadata.time
-          ackReceiveTime > rPacketIt->time ? ackReceiveTime : Clock::now();
+          ? ackReceiveTime
+          : Clock::now();
       auto rttSample = std::chrono::duration_cast<std::chrono::microseconds>(
-          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)
+              .setSentTime(rPacketIt->metadata.time)
-              .setEncodedSize(rPacketIt->encodedSize)
+              .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--;

quic/state/OutstandingPacket.h

@@ -12,11 +12,8 @@
 #include <quic/state/PacketEvent.h>
 
 namespace quic {
-// Data structure to represent outstanding retransmittable packets
+
-struct OutstandingPacket {
+struct OutstandingPacketMetadata {
-  // Structure representing the frames that are outstanding including the header
-  // that was sent.
-  RegularQuicWritePacket packet;
   // 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)),
+        metadata(OutstandingPacketMetadata(
-        encodedSize(encodedSizeIn),
+            std::move(timeIn),
-        isHandshake(isHandshakeIn),
+            encodedSizeIn,
-        isD6DProbe(false),
+            isHandshakeIn,
-        totalBytesSent(totalBytesSentIn) {}
+            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)),
+        metadata(OutstandingPacketMetadata(
-        encodedSize(encodedSizeIn),
+            std::move(timeIn),
-        isHandshake(isHandshakeIn),
+            encodedSizeIn,
-        isD6DProbe(isD6DProbeIn),
+            isHandshakeIn,
-        totalBytesSent(totalBytesSentIn) {}
+            isD6DProbeIn,
+            totalBytesSentIn,
+            inflightBytesIn)) {}
 };
 } // namespace quic

quic/state/StateData.h

@@ -233,7 +233,7 @@ struct CongestionController {
 
     void addLostPacket(const OutstandingPacket& packet) {
       if (std::numeric_limits<uint64_t>::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 =
+      largestLostSentTime = std::max(
-          std::max(packet.time, largestLostSentTime.value_or(packet.time));
+          packet.metadata.time,
-      smallestLostSentTime =
+          largestLostSentTime.value_or(packet.metadata.time));
-          std::min(packet.time, smallestLostSentTime.value_or(packet.time));
+      smallestLostSentTime = std::min(
+          packet.metadata.time,
+          smallestLostSentTime.value_or(packet.metadata.time));
     }
   };
 

quic/state/test/AckHandlersTest.cpp

@@ -11,6 +11,8 @@
 
 #include <quic/common/test/TestUtils.h>
 
+#include <quic/QuicConstants.h>
+#include <quic/api/test/Mocks.h>
 #include <quic/fizz/server/handshake/FizzServerQuicHandshakeContext.h>
 #include <quic/logging/test/Mocks.h>
 #include <quic/server/state/ServerStateMachine.h>
@@ -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(
+  conn.outstandings.packets.emplace_back(OutstandingPacket(
-      OutstandingPacket(std::move(packetAfterRto), Clock::now(), 0, false, 0));
+      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(
+  conn.outstandings.packets.emplace_back(OutstandingPacket(
-      OutstandingPacket(std::move(regularPacket), Clock::now(), 1, false, 1));
+      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<MockCongestionController>();
+  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<TimePoint> 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<ackPacketData> 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<std::chrono::microseconds>(
+        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,

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) {

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);