Maitain ingress ECN counts in the AckState for writing ACKs

Summary:
Keep track of ECN values in incoming packets and update the AckState that is used for echoing the counts to the peer in ACK_ECN frames.

Reading and writing the ACK_ECN frames is in the next change.

Reviewed By: kvtsoy

Differential Revision: D54927783

fbshipit-source-id: 44471c6224ee8578aaacc0d1a1c54370ef6d2ffe

quic/QuicConstants.h

@@ -739,5 +739,5 @@ constexpr uint8_t kDefaultReceiveTimestampsExponent = 3;
 // ECN field values
 constexpr uint8_t kEcnECT1 = 0b01;
 constexpr uint8_t kEcnECT0 = 0b10;
-constexpr uint8_t kEcnECE = 0b11;
+constexpr uint8_t kEcnCE = 0b11;
 } // namespace quic

quic/client/QuicClientTransport.cpp

@@ -375,7 +375,7 @@ void QuicClientTransport::processUdpPacketData(
   // Add the packet to the AckState associated with the packet number space.
   auto& ackState = getAckState(*conn_, pnSpace);
   uint64_t distanceFromExpectedPacketNum =
-      addPacketToAckState(*conn_, ackState, packetNum, udpPacket.timings);
+      addPacketToAckState(*conn_, ackState, packetNum, udpPacket);
   if (distanceFromExpectedPacketNum > 0) {
     QUIC_STATS(conn_->statsCallback, onOutOfOrderPacketReceived);
   }

quic/codec/Types.h

@@ -251,6 +251,10 @@ struct WriteAckFrameState {
   // element in the deque (e.g., entries are not added for packets that
   // arrive out of order relative to previously received packets).
   CircularDeque<ReceivedPacket> recvdPacketInfos;
+  // The count of ECN marks seen on received packets.
+  uint32_t ecnECT0CountReceived{0};
+  uint32_t ecnECT1CountReceived{0};
+  uint32_t ecnCECountReceived{0};
 };
 
 struct WriteAckFrameMetaData {

quic/common/test/TestUtils.cpp

@@ -503,10 +503,10 @@ void updateAckState(
     bool pkHasRetransmittableData,
     bool pkHasCryptoData,
     TimePoint receiveTimePoint) {
-  ReceivedUdpPacket::Timings packetTimings;
+  ReceivedUdpPacket packet;
-  packetTimings.receiveTimePoint = receiveTimePoint;
+  packet.timings.receiveTimePoint = receiveTimePoint;
-  uint64_t distance = addPacketToAckState(
+  uint64_t distance =
-      conn, getAckState(conn, pnSpace), packetNum, packetTimings);
+      addPacketToAckState(conn, getAckState(conn, pnSpace), packetNum, packet);
   updateAckSendStateOnRecvPacket(
       conn,
       getAckState(conn, pnSpace),

quic/server/state/ServerStateMachine.cpp

@@ -1015,8 +1015,8 @@ void onServerReadDataFromOpen(
     }
 
     auto& ackState = getAckState(conn, packetNumberSpace);
-    uint64_t distanceFromExpectedPacketNum = addPacketToAckState(
+    uint64_t distanceFromExpectedPacketNum =
-        conn, ackState, packetNum, readData.udpPacket.timings);
+        addPacketToAckState(conn, ackState, packetNum, readData.udpPacket);
     if (distanceFromExpectedPacketNum > 0) {
       QUIC_STATS(conn.statsCallback, onOutOfOrderPacketReceived);
     }

quic/state/QuicStateFunctions.cpp

@@ -400,7 +400,7 @@ uint64_t addPacketToAckState(
     QuicConnectionStateBase& conn,
     AckState& ackState,
     const PacketNum packetNum,
-    const ReceivedUdpPacket::Timings& timings) {
+    const ReceivedUdpPacket& udpPacket) {
   PacketNum expectedNextPacket = 0;
   if (ackState.largestRecvdPacketNum) {
     expectedNextPacket = *ackState.largestRecvdPacketNum + 1;
@@ -413,11 +413,11 @@ uint64_t addPacketToAckState(
     QUIC_STATS(conn.statsCallback, onDuplicatedPacketReceived);
   }
   if (ackState.largestRecvdPacketNum == packetNum) {
-    ackState.largestRecvdPacketTime = timings.receiveTimePoint;
+    ackState.largestRecvdPacketTime = udpPacket.timings.receiveTimePoint;
   }
   static_assert(Clock::is_steady, "Needs steady clock");
 
-  ackState.lastRecvdPacketInfo.assign({packetNum, timings});
+  ackState.lastRecvdPacketInfo.assign({packetNum, udpPacket.timings});
 
   if (packetNum >= expectedNextPacket) {
     if (ackState.recvdPacketInfos.size() ==
@@ -425,7 +425,22 @@ uint64_t addPacketToAckState(
       ackState.recvdPacketInfos.pop_front();
     }
     ackState.recvdPacketInfos.emplace_back(
-        WriteAckFrameState::ReceivedPacket{packetNum, timings});
+        WriteAckFrameState::ReceivedPacket{packetNum, udpPacket.timings});
+  }
+
+  auto ecnValue = udpPacket.tosValue & 0b11;
+  switch (ecnValue) {
+    case kEcnCE:
+      ackState.ecnCECountReceived++;
+      break;
+    case kEcnECT1:
+      ackState.ecnECT1CountReceived++;
+      break;
+    case kEcnECT0:
+      ackState.ecnECT0CountReceived++;
+      break;
+    default:
+      break;
   }
 
   if (expectedNextPacket) {

quic/state/QuicStateFunctions.h

@@ -69,7 +69,7 @@ uint64_t addPacketToAckState(
     QuicConnectionStateBase& conn,
     AckState& ackState,
     const PacketNum packetNum,
-    const ReceivedUdpPacket::Timings& timings);
+    const ReceivedUdpPacket& udpPacket);
 
 std::deque<OutstandingPacketWrapper>::iterator getNextOutstandingPacket(
     QuicConnectionStateBase& conn,

quic/state/test/QuicStateFunctionsTest.cpp

@@ -58,12 +58,12 @@ RegularQuicWritePacket makeTestLongPacket(LongHeader::Types type) {
 class AddPacketToAckStateTest : public TestWithParam<PacketNumberSpace> {
  public:
   /**
-   * Build a ReceivedUdpPacket::Timings structure with minimal fields set.
+   * Build a ReceivedUdpPacket structure with minimal fields set.
    */
-  static ReceivedUdpPacket::Timings buildPacketTimingsMinimal() {
+  static ReceivedUdpPacket buildPacketMinimal() {
-    ReceivedUdpPacket::Timings timings;
+    ReceivedUdpPacket packet;
-    timings.receiveTimePoint = Clock::now();
+    packet.timings.receiveTimePoint = Clock::now();
-    return timings;
+    return packet;
   }
 };
 
@@ -78,10 +78,7 @@ TEST_P(AddPacketToAckStateTest, FirstPacketNotOutOfOrder) {
    */
   PacketNum firstPacket = folly::Random::rand32(1, 100);
   EXPECT_FALSE(addPacketToAckState(
-      conn,
+      conn, getAckState(conn, GetParam()), firstPacket, buildPacketMinimal()));
-      getAckState(conn, GetParam()),
-      firstPacket,
-      buildPacketTimingsMinimal()));
 }
 
 TEST_P(AddPacketToAckStateTest, ReceiveNew) {
@@ -92,10 +89,7 @@ TEST_P(AddPacketToAckStateTest, ReceiveNew) {
       *getAckState(conn, GetParam()).largestRecvdPacketNum;
   PacketNum newReceived = currentLargestReceived + 1;
   auto distance = addPacketToAckState(
-      conn,
+      conn, getAckState(conn, GetParam()), newReceived, buildPacketMinimal());
-      getAckState(conn, GetParam()),
-      newReceived,
-      buildPacketTimingsMinimal());
   EXPECT_EQ(distance, 0);
   EXPECT_GT(
       *getAckState(conn, GetParam()).largestRecvdPacketNum,
@@ -110,10 +104,7 @@ TEST_P(AddPacketToAckStateTest, ReceiveNewWithGap) {
       *getAckState(conn, GetParam()).largestRecvdPacketNum;
   PacketNum newReceived = currentLargestReceived + 3;
   auto distance = addPacketToAckState(
-      conn,
+      conn, getAckState(conn, GetParam()), newReceived, buildPacketMinimal());
-      getAckState(conn, GetParam()),
-      newReceived,
-      buildPacketTimingsMinimal());
   EXPECT_EQ(distance, 2); // newReceived is 2 after the expected pkt num
   EXPECT_GT(
       *getAckState(conn, GetParam()).largestRecvdPacketNum,
@@ -128,10 +119,7 @@ TEST_P(AddPacketToAckStateTest, ReceiveOld) {
       *getAckState(conn, GetParam()).largestRecvdPacketNum;
   PacketNum newReceived = currentLargestReceived - 1;
   auto distance = addPacketToAckState(
-      conn,
+      conn, getAckState(conn, GetParam()), newReceived, buildPacketMinimal());
-      getAckState(conn, GetParam()),
-      newReceived,
-      buildPacketTimingsMinimal());
   EXPECT_EQ(distance, 2); // newReceived is 2 before the expected pkt num
   EXPECT_EQ(
       *getAckState(conn, GetParam()).largestRecvdPacketNum,
@@ -146,16 +134,79 @@ TEST_P(AddPacketToAckStateTest, ReceiveOldWithGap) {
       *getAckState(conn, GetParam()).largestRecvdPacketNum;
   PacketNum newReceived = currentLargestReceived - 5;
   auto distance = addPacketToAckState(
-      conn,
+      conn, getAckState(conn, GetParam()), newReceived, buildPacketMinimal());
-      getAckState(conn, GetParam()),
-      newReceived,
-      buildPacketTimingsMinimal());
   EXPECT_EQ(distance, 6); // newReceived is 6 before the expected pkt num
   EXPECT_EQ(
       *getAckState(conn, GetParam()).largestRecvdPacketNum,
       currentLargestReceived);
 }
 
+TEST_P(AddPacketToAckStateTest, ReceiveWithECN) {
+  QuicServerConnectionState conn(
+      FizzServerQuicHandshakeContext::Builder().build());
+  getAckState(conn, GetParam()).largestRecvdPacketNum = 100;
+  auto nextPacketNum = *getAckState(conn, GetParam()).largestRecvdPacketNum;
+
+  {
+    // No ECN values seen yet.
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnCECountReceived, 0);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT0CountReceived, 0);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT1CountReceived, 0);
+  }
+
+  {
+    auto packet = buildPacketMinimal();
+    packet.tosValue = kEcnECT0;
+    auto distance = addPacketToAckState(
+        conn, getAckState(conn, GetParam()), ++nextPacketNum, packet);
+    EXPECT_EQ(distance, 0);
+
+    // Seen 1 ECT0, 0 ECT1, 0 CE.
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnCECountReceived, 0);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT0CountReceived, 1);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT1CountReceived, 0);
+  }
+
+  {
+    auto packet = buildPacketMinimal();
+    packet.tosValue = kEcnECT1;
+    auto distance = addPacketToAckState(
+        conn, getAckState(conn, GetParam()), ++nextPacketNum, packet);
+    EXPECT_EQ(distance, 0);
+
+    // Seen 1 ECT0, 1 ECT1, 0 CE.
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnCECountReceived, 0);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT0CountReceived, 1);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT1CountReceived, 1);
+  }
+
+  {
+    auto packet = buildPacketMinimal();
+    packet.tosValue = kEcnCE;
+    auto distance = addPacketToAckState(
+        conn, getAckState(conn, GetParam()), ++nextPacketNum, packet);
+    EXPECT_EQ(distance, 0);
+
+    // Seen 1 ECT0, 1 ECT1, 1 CE.
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnCECountReceived, 1);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT0CountReceived, 1);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT1CountReceived, 1);
+  }
+
+  {
+    auto packet = buildPacketMinimal();
+    packet.tosValue = kEcnCE;
+    auto distance = addPacketToAckState(
+        conn, getAckState(conn, GetParam()), ++nextPacketNum, packet);
+    EXPECT_EQ(distance, 0);
+
+    // Seen 1 ECT0, 1 ECT1, 2 CE.
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnCECountReceived, 2);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT0CountReceived, 1);
+    EXPECT_EQ(getAckState(conn, GetParam()).ecnECT1CountReceived, 1);
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(
     AddPacketToAckStateTests,
     AddPacketToAckStateTest,