T24905463 - [quic][ping] Implement ping in Quic

Summary: Implement ping functionality in ping

Reviewed By: yangchi

Differential Revision: D17885286

fbshipit-source-id: 4c328d14a023057d6889818250c0129c06e60874

quic/api/QuicTransportBase.cpp

@@ -34,6 +34,7 @@ QuicTransportBase::QuicTransportBase(
       pathValidationTimeout_(this),
       idleTimeout_(this),
       drainTimeout_(this),
+      pingTimeout_(this),
       readLooper_(new FunctionLooper(
           evb,
           [this](bool /* ignored */) { invokeReadDataAndCallbacks(); },
@@ -316,6 +317,10 @@ void QuicTransportBase::closeImpl(
   if (idleTimeout_.isScheduled()) {
     idleTimeout_.cancelTimeout();
   }
+  if (pingTimeout_.isScheduled()) {
+    pingTimeout_.cancelTimeout();
+  }
+
   VLOG(10) << "Stopping read looper due to immediate close " << *this;
   readLooper_->stop();
   peekLooper_->stop();
@@ -1397,6 +1402,22 @@ folly::
   }
 }
 
+void QuicTransportBase::handlePingCallback() {
+  if (!conn_->pendingEvents.cancelPingTimeout) {
+    return; // nothing to cancel
+  }
+  if (!pingTimeout_.isScheduled()) {
+    // set cancelpingTimeOut to false, delayed acks
+    conn_->pendingEvents.cancelPingTimeout = false;
+    return; // nothing to do, as timeout has already fired
+  }
+  pingTimeout_.cancelTimeout();
+  if (pingCallback_ != nullptr) {
+    runOnEvbAsync([](auto self) { self->pingCallback_->pingAcknowledged(); });
+  }
+  conn_->pendingEvents.cancelPingTimeout = false;
+}
+
 void QuicTransportBase::processCallbacksAfterNetworkData() {
   if (UNLIKELY(closeState_ != CloseState::OPEN)) {
     return;
@@ -1414,6 +1435,10 @@ void QuicTransportBase::processCallbacksAfterNetworkData() {
     }
   }
   conn_->streamManager->clearNewPeerStreams();
+
+  // Handle pingCallbacks
+  handlePingCallback();
+
   // TODO: we're currently assuming that canceling write callbacks will not
   // cause reset of random streams. Maybe get rid of that assumption later.
   for (auto pendingResetIt = conn_->pendingEvents.resets.begin();
@@ -2006,8 +2031,20 @@ void QuicTransportBase::checkForClosedStream() {
 }
 
 void QuicTransportBase::sendPing(
-    PingCallback* /*callback*/,
-    std::chrono::milliseconds /*pingTimeout*/) {}
+    PingCallback* callback,
+    std::chrono::milliseconds pingTimeout) {
+  /* Step 0: Connection should not be closed */
+  if (closeState_ == CloseState::CLOSED) {
+    return;
+  }
+
+  // Step 1: Send a simple ping frame
+  quic::sendSimpleFrame(*conn_, PingFrame());
+  updateWriteLooper(true);
+
+  // Step 2: Schedule the timeout on event base
+  schedulePingTimeout(callback, pingTimeout);
+}
 
 void QuicTransportBase::lossTimeoutExpired() noexcept {
   CHECK_NE(closeState_, CloseState::CLOSED);
@@ -2045,6 +2082,14 @@ void QuicTransportBase::ackTimeoutExpired() noexcept {
   pacedWriteDataToSocket(false);
 }
 
+void QuicTransportBase::pingTimeoutExpired() noexcept {
+  // If timeout expired just call the  call back Provided
+  if (pingCallback_ == nullptr) {
+    return;
+  }
+  runOnEvbAsync([](auto self) { self->pingCallback_->pingTimeout(); });
+}
+
 void QuicTransportBase::pathValidationTimeoutExpired() noexcept {
   CHECK(conn_->outstandingPathValidation);
 
@@ -2110,6 +2155,19 @@ void QuicTransportBase::scheduleAckTimeout() {
   }
 }
 
+void QuicTransportBase::schedulePingTimeout(
+    PingCallback* pingCb,
+    std::chrono::milliseconds timeout) {
+  // if a ping timeout is already scheduled, nothing to do, return
+  if (pingTimeout_.isScheduled()) {
+    return;
+  }
+
+  pingCallback_ = pingCb;
+  auto& wheelTimer = getEventBase()->timer();
+  wheelTimer.scheduleTimeout(&pingTimeout_, timeout);
+}
+
 void QuicTransportBase::schedulePathValidationTimeout() {
   if (closeState_ == CloseState::CLOSED) {
     return;

quic/api/QuicTransportBase.h

@@ -367,6 +367,26 @@ class QuicTransportBase : public QuicSocket {
     QuicTransportBase* transport_;
   };
 
+  class PingTimeout : public folly::HHWheelTimer::Callback {
+   public:
+    ~PingTimeout() override = default;
+
+    explicit PingTimeout(QuicTransportBase* transport)
+        : transport_(transport) {}
+
+    void timeoutExpired() noexcept override {
+      transport_->pingTimeoutExpired();
+    }
+
+    void callbackCanceled() noexcept override {
+      // ignore, as this happens only when event  base dies
+      return;
+    }
+
+   private:
+    QuicTransportBase* transport_;
+  };
+
   class PathValidationTimeout : public folly::HHWheelTimer::Callback {
    public:
     ~PathValidationTimeout() override = default;
@@ -460,6 +480,7 @@ class QuicTransportBase : public QuicSocket {
   void updateReadLooper();
   void updatePeekLooper();
   void updateWriteLooper(bool thisIteration);
+  void handlePingCallback();
 
   void runOnEvbAsync(
       folly::Function<void(std::shared_ptr<QuicTransportBase>)> func);
@@ -521,10 +542,14 @@ class QuicTransportBase : public QuicSocket {
   void pathValidationTimeoutExpired() noexcept;
   void idleTimeoutExpired(bool drain) noexcept;
   void drainTimeoutExpired() noexcept;
+  void pingTimeoutExpired() noexcept;
 
   void setIdleTimer();
   void scheduleAckTimeout();
   void schedulePathValidationTimeout();
+  void schedulePingTimeout(
+      PingCallback* callback,
+      std::chrono::milliseconds pingTimeout);
 
   std::atomic<folly::EventBase*> evb_;
   std::unique_ptr<folly::AsyncUDPSocket> socket_;
@@ -570,6 +595,7 @@ class QuicTransportBase : public QuicSocket {
       deliveryCallbacks_;
   std::unordered_map<StreamId, DataExpiredCallbackData> dataExpiredCallbacks_;
   std::unordered_map<StreamId, DataRejectedCallbackData> dataRejectedCallbacks_;
+  PingCallback* pingCallback_;
 
   WriteCallback* connWriteCallback_{nullptr};
   std::map<StreamId, WriteCallback*> pendingWriteCallbacks_;
@@ -581,6 +607,7 @@ class QuicTransportBase : public QuicSocket {
   PathValidationTimeout pathValidationTimeout_;
   IdleTimeout idleTimeout_;
   DrainTimeout drainTimeout_;
+  PingTimeout pingTimeout_;
   FunctionLooper::Ptr readLooper_;
   FunctionLooper::Ptr peekLooper_;
   FunctionLooper::Ptr writeLooper_;

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -369,7 +369,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
   // Write those framses with a regular builder
   writeFrame(connCloseFrame, regularBuilder);
   writeFrame(QuicSimpleFrame(maxStreamFrame), regularBuilder);
-  writeFrame(pingFrame, regularBuilder);
+  writeFrame(QuicSimpleFrame(pingFrame), regularBuilder);
   writeAckFrame(ackMeta, regularBuilder);
 
   auto result = cloningScheduler.scheduleFramesForPacket(
@@ -395,7 +395,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
     /* the next four frames should not be written */
     present |= frame.asConnectionCloseFrame() ? true : false;
     present |= frame.asQuicSimpleFrame() ? true : false;
-    present |= frame.asPingFrame() ? true : false;
+    present |= frame.asQuicSimpleFrame() ? true : false;
     present |= frame.asWriteAckFrame() ? true : false;
     ASSERT_FALSE(present);
   }

quic/api/test/QuicTransportBaseTest.cpp

@@ -11,6 +11,7 @@
 #include <folly/portability/GMock.h>
 #include <folly/portability/GTest.h>
 
+#include <quic/api/QuicSocket.h>
 #include <quic/api/QuicTransportBase.h>
 #include <quic/codec/DefaultConnectionIdAlgo.h>
 #include <quic/common/test/TestUtils.h>
@@ -227,7 +228,7 @@ class TestQuicTransport
     transportClosed = true;
   }
 
-  void invokeAckTimeout() {
+  void AckTimeout() {
     ackTimeoutExpired();
   }
 
@@ -239,6 +240,31 @@ class TestQuicTransport
     idleTimeout_.timeoutExpired();
   }
 
+  void invokeAckTimeout() {
+    ackTimeout_.timeoutExpired();
+  }
+
+  void invokeSendPing(
+      quic::QuicSocket::PingCallback* cb,
+      std::chrono::milliseconds interval) {
+    sendPing(cb, interval);
+  }
+
+  void invokeCancelPingTimeout() {
+    pingTimeout_.cancelTimeout();
+  }
+
+  void invokeHandlePingCallback() {
+    handlePingCallback();
+  }
+
+  bool isPingTimeoutScheduled() {
+    if (pingTimeout_.isScheduled()) {
+      return true;
+    }
+    return false;
+  }
+
   auto& writeLooper() {
     return writeLooper_;
   }
@@ -2428,5 +2454,29 @@ TEST_F(QuicTransportImplTest, CloseFromCancelDeliveryCallbacksForStream) {
   transport->cancelDeliveryCallbacksForStream(stream1);
 }
 
+TEST_F(QuicTransportImplTest, SuccessfulPing) {
+  auto conn = transport->transportConn;
+  std::chrono::milliseconds interval(10);
+  transport->invokeSendPing(nullptr, interval);
+  EXPECT_EQ(transport->isPingTimeoutScheduled(), true);
+  EXPECT_EQ(conn->pendingEvents.cancelPingTimeout, false);
+  conn->pendingEvents.cancelPingTimeout = true;
+  transport->invokeHandlePingCallback();
+  EXPECT_EQ(transport->isPingTimeoutScheduled(), false);
+  EXPECT_EQ(conn->pendingEvents.cancelPingTimeout, false);
+}
+
+TEST_F(QuicTransportImplTest, FailedPing) {
+  auto conn = transport->transportConn;
+  std::chrono::milliseconds interval(10);
+  transport->invokeSendPing(nullptr, interval);
+  EXPECT_EQ(transport->isPingTimeoutScheduled(), true);
+  EXPECT_EQ(conn->pendingEvents.cancelPingTimeout, false);
+  conn->pendingEvents.cancelPingTimeout = true;
+  transport->invokeCancelPingTimeout();
+  transport->invokeHandlePingCallback();
+  EXPECT_EQ(conn->pendingEvents.cancelPingTimeout, false);
+}
+
 } // namespace test
 } // namespace quic

quic/client/QuicClientTransport.cpp

@@ -20,6 +20,7 @@
 #include <quic/loss/QuicLossFunctions.h>
 #include <quic/state/AckHandlers.h>
 #include <quic/state/QuicPacingFunctions.h>
+#include <quic/state/SimpleFrameFunctions.h>
 
 namespace fsp = folly::portability::sockets;
 
@@ -351,6 +352,12 @@ void QuicClientTransport::processPacketData(
                       *cryptoStream, frame.offset, frame.len);
                   break;
                 }
+                case QuicWriteFrame::Type::QuicSimpleFrame_E: {
+                  const quic::QuicSimpleFrame simpleFrame =
+                      *packetFrame.asQuicSimpleFrame();
+                  updateSimpleFrameOnAck(*conn_, simpleFrame);
+                  break;
+                }
                 default:
                   // ignore other frames.
                   break;
@@ -502,10 +509,6 @@ void QuicClientTransport::processPacketData(
             *conn_, simpleFrame, packetNum, false);
         break;
       }
-      case QuicFrame::Type::PingFrame_E: {
-        pktHasRetransmittableData = true;
-        break;
-      }
       default:
         break;
     }

quic/client/test/QuicClientTransportTest.cpp

@@ -4478,7 +4478,7 @@ TEST_F(QuicClientTransportAfterStartTest, PingIsRetransmittable) {
       client->getConn().udpSendPacketLen,
       std::move(header),
       0 /* largestAcked */);
-  writeFrame(pingFrame, builder);
+  writeFrame(QuicSimpleFrame(pingFrame), builder);
   auto packet = packetToBuf(std::move(builder).buildPacket());
   deliverData(packet->coalesce());
   EXPECT_TRUE(client->getConn().pendingEvents.scheduleAckTimeout);

quic/codec/Decode.cpp

@@ -60,7 +60,7 @@ PaddingFrame decodePaddingFrame(folly::io::Cursor&) {
   return PaddingFrame();
 }
 
-PingFrame decodePingFrame(folly::io::Cursor& /* cursor */) {
+PingFrame decodePingFrame(folly::io::Cursor&) {
   return PingFrame();
 }
 

quic/codec/QuicWriteCodec.cpp

@@ -318,6 +318,17 @@ size_t writeSimpleFrame(
   uint64_t spaceLeft = builder.remainingSpaceInPkt();
 
   switch (frame.type()) {
+    case QuicSimpleFrame::Type::PingFrame_E: {
+      const PingFrame& pingFrame = *frame.asPingFrame();
+      QuicInteger intFrameType(static_cast<uint8_t>(FrameType::PING));
+      if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) {
+        builder.write(intFrameType);
+        builder.appendFrame(QuicSimpleFrame(pingFrame));
+        return intFrameType.getSize();
+      }
+      // no space left in packet
+      return size_t(0);
+    }
     case QuicSimpleFrame::Type::StopSendingFrame_E: {
       const StopSendingFrame& stopSendingFrame = *frame.asStopSendingFrame();
       QuicInteger intFrameType(static_cast<uint8_t>(FrameType::STOP_SENDING));
@@ -495,17 +506,6 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
       }
       return size_t(0);
     }
-    case QuicWriteFrame::Type::PingFrame_E: {
-      PingFrame& pingFrame = *frame.asPingFrame();
-      QuicInteger intFrameType(static_cast<uint8_t>(FrameType::PING));
-      if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) {
-        builder.write(intFrameType);
-        builder.appendFrame(std::move(pingFrame));
-        return intFrameType.getSize();
-      }
-      // no space left in packet
-      return size_t(0);
-    }
     case QuicWriteFrame::Type::RstStreamFrame_E: {
       RstStreamFrame& rstStreamFrame = *frame.asRstStreamFrame();
       QuicInteger intFrameType(static_cast<uint8_t>(FrameType::RST_STREAM));

quic/codec/Types.h

@@ -564,15 +564,16 @@ struct StatelessReset {
       : token(std::move(tokenIn)) {}
 };
 
-#define QUIC_SIMPLE_FRAME(F, ...)        \
-  F(StopSendingFrame, __VA_ARGS__)       \
-  F(MinStreamDataFrame, __VA_ARGS__)     \
-  F(ExpiredStreamDataFrame, __VA_ARGS__) \
-  F(PathChallengeFrame, __VA_ARGS__)     \
-  F(PathResponseFrame, __VA_ARGS__)      \
-  F(NewConnectionIdFrame, __VA_ARGS__)   \
-  F(MaxStreamsFrame, __VA_ARGS__)        \
-  F(RetireConnectionIdFrame, __VA_ARGS__)
+#define QUIC_SIMPLE_FRAME(F, ...)         \
+  F(StopSendingFrame, __VA_ARGS__)        \
+  F(MinStreamDataFrame, __VA_ARGS__)      \
+  F(ExpiredStreamDataFrame, __VA_ARGS__)  \
+  F(PathChallengeFrame, __VA_ARGS__)      \
+  F(PathResponseFrame, __VA_ARGS__)       \
+  F(NewConnectionIdFrame, __VA_ARGS__)    \
+  F(MaxStreamsFrame, __VA_ARGS__)         \
+  F(RetireConnectionIdFrame, __VA_ARGS__) \
+  F(PingFrame, __VA_ARGS__)
 
 DECLARE_VARIANT_TYPE(QuicSimpleFrame, QUIC_SIMPLE_FRAME)
 
@@ -583,7 +584,6 @@ DECLARE_VARIANT_TYPE(QuicSimpleFrame, QUIC_SIMPLE_FRAME)
   F(ApplicationCloseFrame, __VA_ARGS__)  \
   F(MaxDataFrame, __VA_ARGS__)           \
   F(MaxStreamDataFrame, __VA_ARGS__)     \
-  F(PingFrame, __VA_ARGS__)              \
   F(DataBlockedFrame, __VA_ARGS__)       \
   F(StreamDataBlockedFrame, __VA_ARGS__) \
   F(StreamsBlockedFrame, __VA_ARGS__)    \
@@ -603,7 +603,6 @@ DECLARE_VARIANT_TYPE(QuicFrame, QUIC_FRAME)
   F(ApplicationCloseFrame, __VA_ARGS__)  \
   F(MaxDataFrame, __VA_ARGS__)           \
   F(MaxStreamDataFrame, __VA_ARGS__)     \
-  F(PingFrame, __VA_ARGS__)              \
   F(DataBlockedFrame, __VA_ARGS__)       \
   F(StreamDataBlockedFrame, __VA_ARGS__) \
   F(StreamsBlockedFrame, __VA_ARGS__)    \
@@ -837,7 +836,8 @@ struct VersionNegotiationPacket {
 struct RegularPacket {
   PacketHeader header;
 
-  explicit RegularPacket(PacketHeader&& headerIn) : header(std::move(headerIn)) {}
+  explicit RegularPacket(PacketHeader&& headerIn)
+      : header(std::move(headerIn)) {}
 };
 
 /**

quic/codec/test/QuicPacketRebuilderTest.cpp

@@ -62,7 +62,6 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
       "The sun is in the sky.",
       FrameType::ACK);
   MaxStreamsFrame maxStreamsFrame(4321, true);
-  PingFrame pingFrame;
   IntervalSet<PacketNum> ackBlocks;
   ackBlocks.insert(10, 100);
   ackBlocks.insert(200, 1000);
@@ -78,10 +77,11 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
   uint64_t cryptoOffset = 0;
   auto cryptoBuf = folly::IOBuf::copyBuffer("NewSessionTicket");
 
+  PingFrame pingFrame{};
   // Write them with a regular builder
   writeFrame(connCloseFrame, regularBuilder1);
   writeFrame(QuicSimpleFrame(maxStreamsFrame), regularBuilder1);
-  writeFrame(pingFrame, regularBuilder1);
+  writeFrame(QuicSimpleFrame(pingFrame), regularBuilder1);
   writeAckFrame(ackMeta, regularBuilder1);
   writeStreamFrameHeader(
       regularBuilder1,
@@ -135,14 +135,22 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
       }
       case QuicWriteFrame::Type::QuicSimpleFrame_E: {
         const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame();
-        const MaxStreamsFrame* maxStreamFrame = simpleFrame.asMaxStreamsFrame();
-        EXPECT_NE(maxStreamFrame, nullptr);
-        EXPECT_EQ(4321, maxStreamFrame->maxStreams);
-        break;
-      }
-      case QuicWriteFrame::Type::PingFrame_E: {
-        const PingFrame& ping = *frame.asPingFrame();
-        EXPECT_EQ(PingFrame(), ping);
+        switch (simpleFrame.type()) {
+          case QuicSimpleFrame::Type::MaxStreamsFrame_E: {
+            const MaxStreamsFrame* maxStreamFrame =
+                simpleFrame.asMaxStreamsFrame();
+            EXPECT_NE(maxStreamFrame, nullptr);
+            EXPECT_EQ(4321, maxStreamFrame->maxStreams);
+            break;
+          }
+          case QuicSimpleFrame::Type::PingFrame_E: {
+            const PingFrame* simplePingFrame = simpleFrame.asPingFrame();
+            EXPECT_NE(simplePingFrame, nullptr);
+            break;
+          }
+          default:
+            EXPECT_TRUE(false); /* fail if this happens */
+        }
         break;
       }
       case QuicWriteFrame::Type::WriteAckFrame_E: {
@@ -356,7 +364,6 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) {
       "The sun is in the sky.",
       FrameType::ACK);
   StreamsBlockedFrame maxStreamIdFrame(0x1024, true);
-  PingFrame pingFrame;
   IntervalSet<PacketNum> ackBlocks;
   ackBlocks.insert(10, 100);
   ackBlocks.insert(200, 1000);
@@ -367,11 +374,11 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) {
   auto streamId = stream->id;
   auto buf =
       folly::IOBuf::copyBuffer("You can't deny you are looking for the sunset");
-
+  PingFrame pingFrame;
   // Write them with a regular builder
   writeFrame(connCloseFrame, regularBuilder1);
   writeFrame(maxStreamIdFrame, regularBuilder1);
-  writeFrame(pingFrame, regularBuilder1);
+  writeFrame(QuicSimpleFrame(pingFrame), regularBuilder1);
   writeAckFrame(ackMeta, regularBuilder1);
   writeStreamFrameHeader(
       regularBuilder1,
@@ -407,7 +414,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) {
   RegularQuicPacketBuilder regularBuilder(
       kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */);
   PingFrame pingFrame;
-  writeFrame(pingFrame, regularBuilder);
+  writeFrame(QuicSimpleFrame(pingFrame), regularBuilder);
   auto packet = std::move(regularBuilder).buildPacket();
   auto outstandingPacket = makeDummyOutstandingPacket(packet.packet, 1000);
   QuicServerConnectionState conn;

quic/codec/test/QuicWriteCodecTest.cpp

@@ -1115,17 +1115,19 @@ TEST_F(QuicWriteCodecTest, DecodeAppCloseLarge) {
 TEST_F(QuicWriteCodecTest, WritePing) {
   MockQuicPacketBuilder pktBuilder;
   setupCommonExpects(pktBuilder);
-  auto pingBytesWritten = writeFrame(PingFrame(), pktBuilder);
+  auto pingBytesWritten = writeFrame(QuicSimpleFrame(PingFrame()), pktBuilder);
 
   auto builtOut = std::move(pktBuilder).buildPacket();
   auto regularPacket = builtOut.first;
   EXPECT_EQ(1, pingBytesWritten);
-  EXPECT_NE(regularPacket.frames[0].asPingFrame(), nullptr);
+  auto simpleFrame = regularPacket.frames[0].asQuicSimpleFrame();
+  EXPECT_NE(simpleFrame->asPingFrame(), nullptr);
 
   auto wireBuf = std::move(builtOut.second);
   folly::io::Cursor cursor(wireBuf.get());
   QuicFrame decodedFrame = parseQuicFrame(cursor);
-  EXPECT_NE(decodedFrame.asPingFrame(), nullptr);
+  auto decodedSimpleFrame = decodedFrame.asQuicSimpleFrame();
+  EXPECT_NE(decodedSimpleFrame->asPingFrame(), nullptr);
 
   // At last, verify there is nothing left in the wire format bytes:
   EXPECT_TRUE(cursor.isAtEnd());
@@ -1135,7 +1137,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceForPing) {
   MockQuicPacketBuilder pktBuilder;
   pktBuilder.remaining_ = 0;
   setupCommonExpects(pktBuilder);
-  EXPECT_EQ(0, writeFrame(PingFrame(), pktBuilder));
+  EXPECT_EQ(0, writeFrame(QuicSimpleFrame(PingFrame()), pktBuilder));
 }
 
 TEST_F(QuicWriteCodecTest, WritePadding) {

quic/logging/BaseQLogger.cpp

@@ -5,6 +5,10 @@ void addQuicSimpleFrameToEvent(
     quic::QLogPacketEvent* event,
     const quic::QuicSimpleFrame& simpleFrame) {
   switch (simpleFrame.type()) {
+    case quic::QuicSimpleFrame::Type::PingFrame_E: {
+      event->frames.push_back(std::make_unique<quic::PingFrameLog>());
+      break;
+    }
     case quic::QuicSimpleFrame::Type::StopSendingFrame_E: {
       const quic::StopSendingFrame& frame = *simpleFrame.asStopSendingFrame();
       event->frames.push_back(std::make_unique<quic::StopSendingFrameLog>(
@@ -123,10 +127,6 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
             frame.streamId, frame.maximumData));
         break;
       }
-      case QuicFrame::Type::PingFrame_E: {
-        event->frames.push_back(std::make_unique<PingFrameLog>());
-        break;
-      }
       case QuicFrame::Type::DataBlockedFrame_E: {
         const auto& frame = *quicFrame.asDataBlockedFrame();
         event->frames.push_back(
@@ -244,9 +244,6 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
             frame.streamLimit, frame.isForBidirectional));
         break;
       }
-      case QuicWriteFrame::Type::PingFrame_E:
-        event->frames.push_back(std::make_unique<PingFrameLog>());
-        break;
       case QuicWriteFrame::Type::DataBlockedFrame_E: {
         const DataBlockedFrame& frame = *quicFrame.asDataBlockedFrame();
         event->frames.push_back(

quic/server/state/ServerStateMachine.cpp

@@ -16,6 +16,7 @@
 #include <quic/state/QuicPacingFunctions.h>
 #include <quic/state/QuicStreamFunctions.h>
 #include <quic/state/QuicTransportStatsCallback.h>
+#include <quic/state/SimpleFrameFunctions.h>
 
 namespace quic {
 using namespace std::chrono_literals;
@@ -800,6 +801,12 @@ void onServerReadDataFromOpen(
                     commonAckVisitorForAckFrame(ackState, frame);
                     break;
                   }
+                  case QuicWriteFrame::Type::QuicSimpleFrame_E: {
+                    const QuicSimpleFrame& frame =
+                        *packetFrame.asQuicSimpleFrame();
+                    updateSimpleFrameOnAck(conn, frame);
+                    break;
+                  }
                   default: {
                     break;
                   }
@@ -964,11 +971,6 @@ void onServerReadDataFromOpen(
               conn, simpleFrame, packetNum, readData.peer != conn.peerAddress);
           break;
         }
-        case QuicFrame::Type::PingFrame_E: {
-          pktHasRetransmittableData = true;
-          isNonProbingPacket = true;
-          break;
-        }
         default: {
           break;
         }

quic/server/test/QuicServerTransportTest.cpp

@@ -2779,7 +2779,7 @@ TEST_F(QuicServerTransportTest, PingIsRetransmittable) {
       server->getConn().udpSendPacketLen,
       std::move(header),
       0 /* largestAcked */);
-  writeFrame(pingFrame, builder);
+  writeFrame(QuicSimpleFrame(pingFrame), builder);
   auto packet = std::move(builder).buildPacket();
   deliverData(packetToBuf(packet));
   EXPECT_TRUE(server->getConn().pendingEvents.scheduleAckTimeout);

quic/state/SimpleFrameFunctions.cpp

@@ -17,15 +17,25 @@ void sendSimpleFrame(QuicConnectionStateBase& conn, QuicSimpleFrame frame) {
 }
 
 void updateSimpleFrameOnAck(
-    QuicConnectionStateBase& /*conn*/,
-    const QuicSimpleFrame& /*frame*/) {
+    QuicConnectionStateBase& conn,
+    const QuicSimpleFrame& frame) {
   // TODO implement.
+  switch (frame.type()) {
+    case QuicSimpleFrame::Type::PingFrame_E: {
+      conn.pendingEvents.cancelPingTimeout = true;
+      break;
+    }
+    default:
+      break;
+  }
 }
 
 folly::Optional<QuicSimpleFrame> updateSimpleFrameOnPacketClone(
     QuicConnectionStateBase& conn,
     const QuicSimpleFrame& frame) {
   switch (frame.type()) {
+    case QuicSimpleFrame::Type::PingFrame_E:
+      return QuicSimpleFrame(frame);
     case QuicSimpleFrame::Type::StopSendingFrame_E:
       if (!conn.streamManager->streamExists(
               frame.asStopSendingFrame()->streamId)) {
@@ -87,6 +97,9 @@ void updateSimpleFrameOnPacketLoss(
     QuicConnectionStateBase& conn,
     const QuicSimpleFrame& frame) {
   switch (frame.type()) {
+    case QuicSimpleFrame::Type::PingFrame_E: {
+      break;
+    }
     case QuicSimpleFrame::Type::StopSendingFrame_E: {
       const StopSendingFrame& stopSendingFrame = *frame.asStopSendingFrame();
       if (conn.streamManager->streamExists(stopSendingFrame.streamId)) {
@@ -138,6 +151,9 @@ bool updateSimpleFrameOnPacketReceived(
     PacketNum packetNum,
     bool fromChangedPeerAddress) {
   switch (frame.type()) {
+    case QuicSimpleFrame::Type::PingFrame_E: {
+      return true;
+    }
     case QuicSimpleFrame::Type::StopSendingFrame_E: {
       const StopSendingFrame& stopSending = *frame.asStopSendingFrame();
       auto stream = conn.streamManager->getStream(stopSending.streamId);

quic/state/StateData.h

@@ -598,6 +598,8 @@ struct QuicConnectionStateBase {
 
     // Number of probing packets to send after PTO
     uint8_t numProbePackets{0};
+
+    bool cancelPingTimeout{false};
   };
 
   PendingEvents pendingEvents;