Cleanup and modularize receive path, improve timestamp support [21/x]

Summary:
This diff renames the `RecvdPacketInfo` structure to `ReceivedPacket` and nests it within the `WriteAckFrameState` object to make its use clear and to avoid name conflicts.

--

This diff is part of a larger stack focused on the following:

- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
  - The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
  - The client currently has three receive paths, and none of them are well tested.

- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
  - `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
  - Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
  - By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.

- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.

Differential Revision: D48787902

fbshipit-source-id: 2c3e96d4888345c139e0a6d60b27f9fc9aa1879e

quic/codec/Types.h

@@ -50,14 +50,6 @@ template <class T>
 using IntervalSetVec = SmallVec<T, kNumInitialAckBlocksPerFrame>;
 using AckBlocks = IntervalSet<PacketNum, 1, IntervalSetVec>;
 
-/**
- * Info stored on receipt of a packet for use in subsequent ACK.
- */
-struct RecvdPacketInfo {
-  PacketNum pktNum;
-  TimePoint timeStamp;
-};
-
 struct PaddingFrame {
   // How many contiguous padding frames this represents.
   uint16_t numFrames{1};
@@ -222,6 +214,14 @@ struct WriteAckFrame {
 };
 
 struct WriteAckFrameState {
+  /**
+   * Info stored on receipt of a packet for use in subsequent ACK.
+   */
+  struct ReceivedPacket {
+    PacketNum pktNum;
+    TimePoint timeStamp;
+  };
+
   AckBlocks acks;
 
   // Receive timestamp and packet number for the largest received packet.
@@ -229,7 +229,7 @@ struct WriteAckFrameState {
   // Updated whenever we receive a packet with a larger packet number
   // than all previously received packets in the packet number space
   // tracked by this AckState.
-  folly::Optional<RecvdPacketInfo> largestRecvdPacketInfo;
+  folly::Optional<ReceivedPacket> largestRecvdPacketInfo;
 
   // Receive timestamp and packet number for the last received packet.
   //
@@ -237,7 +237,7 @@ struct WriteAckFrameState {
   // if the last packet was received out of order and thus had a packet
   // number less than that of a previously received packet in the packet
   // number space tracked by this AckState.
-  folly::Optional<RecvdPacketInfo> lastRecvdPacketInfo;
+  folly::Optional<ReceivedPacket> lastRecvdPacketInfo;
 
   // Packet number and timestamp of recently received packets.
   //
@@ -249,7 +249,7 @@ struct WriteAckFrameState {
   // if the packet number is greater than the packet number of the last
   // element in the deque (e.g., entries are not added for packets that
   // arrive out of order relative to previously received packets).
-  CircularDeque<RecvdPacketInfo> recvdPacketInfos;
+  CircularDeque<ReceivedPacket> recvdPacketInfos;
 };
 
 struct WriteAckFrameMetaData {

quic/codec/test/QuicWriteCodecTest.cpp

@@ -145,7 +145,9 @@ void setupCommonExpects(MockQuicPacketBuilder& pktBuilder) {
         pktBuilder.remaining_ -= quicInteger.getSize();
       }));
 }
-using PacketsReceivedTimestampsDeque = CircularDeque<RecvdPacketInfo>;
+using PacketsReceivedTimestampsDeque =
+    CircularDeque<WriteAckFrameState::ReceivedPacket>;
+
 const auto kDefaultTimestampsDelta = 10us;
 const AckReceiveTimestampsConfig defaultAckReceiveTimestmpsConfig = {
     .receiveTimestampsExponent = kDefaultReceiveTimestampsExponent};
@@ -164,7 +166,7 @@ PacketsReceivedTimestampsDeque populateReceiveTimestamps(
   for (auto it = ackBlocks.crbegin(); it != ackBlocks.crend(); it++) {
     for (auto i = it->end; i >= it->start; i--) {
       if (pktsReceivedTimestamps.size() < maxTimeStamps) {
-        RecvdPacketInfo rpi;
+        WriteAckFrameState::ReceivedPacket rpi;
         rpi.pktNum = i;
         auto diff = std::chrono::microseconds(
             lastPacketDelta -= kDefaultTimestampsDelta);
@@ -941,7 +943,7 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) {
     for (auto it = ackBlocks.crbegin(); it != ackBlocks.crend(); it++) {
       for (auto i = it->end; i >= it->start; i--) {
         if (pktsReceivedTimestamps.size() < kMaxReceivedPktsTimestampsStored) {
-          RecvdPacketInfo rpi;
+          WriteAckFrameState::ReceivedPacket rpi;
           rpi.pktNum = i;
           auto diff = std::chrono::microseconds(
               lastPacketDelta -= kDefaultTimestampsDelta);

quic/state/QuicStateFunctions.cpp

@@ -431,8 +431,8 @@ uint64_t addPacketToAckState(
         conn.transportSettings.maxReceiveTimestampsPerAckStored) {
       ackState.recvdPacketInfos.pop_front();
     }
-    ackState.recvdPacketInfos.emplace_back(
-        RecvdPacketInfo{packetNum, timings.receiveTimePoint});
+    ackState.recvdPacketInfos.emplace_back(WriteAckFrameState::ReceivedPacket{
+        packetNum, timings.receiveTimePoint});
   }
 
   if (expectedNextPacket) {

quic/state/test/AckHandlersTest.cpp

@@ -1635,7 +1635,7 @@ TEST_P(AckHandlersTest, purgeAckReceiveTimestamps) {
     // Fill up the last 25 timestamps ending at PN 40.
     for (PacketNum pktNum = 15; pktNum <= 40; ++pktNum) {
       conn.ackStates.initialAckState->recvdPacketInfos.emplace_back(
-          RecvdPacketInfo{pktNum, expectedTime});
+          WriteAckFrameState::ReceivedPacket{pktNum, expectedTime});
     }
 
     commonAckVisitorForAckFrame(*conn.ackStates.initialAckState, ackFrame);
@@ -1656,7 +1656,7 @@ TEST_P(AckHandlersTest, purgeAckReceiveTimestamps) {
     // Local ACK state has timestamps for {15, 40}
     for (PacketNum pktNum = 15; pktNum <= 40; ++pktNum) {
       conn.ackStates.initialAckState->recvdPacketInfos.emplace_back(
-          RecvdPacketInfo{pktNum, expectedTime});
+          WriteAckFrameState::ReceivedPacket{pktNum, expectedTime});
     }
     // ACK frame in the ACKed packet has ACKs for {10, 20}, {25, 35}
     ackFrame.ackBlocks.emplace_back(10, 20);
@@ -1691,7 +1691,7 @@ TEST_P(AckHandlersTest, purgeAckReceiveTimestamps) {
     // Local ACK state has timestamps for {15, 40}
     for (PacketNum pktNum = 15; pktNum <= 40; ++pktNum) {
       conn.ackStates.initialAckState->recvdPacketInfos.emplace_back(
-          RecvdPacketInfo{pktNum, expectedTime});
+          WriteAckFrameState::ReceivedPacket{pktNum, expectedTime});
     }
     // Selectively ACK some packets in the middle - {18, 20}, {25, 35}
     ackFrame.ackBlocks.emplace_back(25, 35);

quic/state/test/QuicStateFunctionsTest.cpp

@@ -223,7 +223,7 @@ TEST_P(
   }
   // Packets 1 and 5 are out of order and will not be stored.
   auto& ackState = getAckState(conn, PacketNumberSpace::AppData);
-  std::deque<RecvdPacketInfo> expectedPktsInfo = {
+  std::deque<WriteAckFrameState::ReceivedPacket> expectedPktsInfo = {
       {0, recvdTs}, {2, recvdTs}, {3, recvdTs}, {4, recvdTs}, {6, recvdTs}};
   EXPECT_EQ(expectedPktsInfo.size(), ackState.recvdPacketInfos.size());
   for (unsigned long i = 0; i < expectedPktsInfo.size(); i++) {