From 924183d2d368cf6fec746d7ec1ac1d6a967b384f Mon Sep 17 00:00:00 2001 From: Crystal Jin Date: Wed, 2 Oct 2024 13:45:06 -0700 Subject: [PATCH] Add retriable udp socket error counts Reviewed By: kvtsoy Differential Revision: D63718295 fbshipit-source-id: f60c01f607def4ee8073238533b4af18e79a3706 --- quic/api/IoBufQuicBatch.cpp | 1 + quic/api/IoBufQuicBatch.h | 5 +++++ quic/api/QuicTransportFunctions.cpp | 24 +++++++++++++++++++++++- quic/client/QuicClientTransport.cpp | 8 ++++++++ quic/client/QuicClientTransport.h | 4 ++++ quic/state/StateData.h | 3 +++ 6 files changed, 44 insertions(+), 1 deletion(-) diff --git a/quic/api/IoBufQuicBatch.cpp b/quic/api/IoBufQuicBatch.cpp index 2996ff992..f42c014c3 100644 --- a/quic/api/IoBufQuicBatch.cpp +++ b/quic/api/IoBufQuicBatch.cpp @@ -71,6 +71,7 @@ bool IOBufQuicBatch::flushInternal() { auto consumed = batchWriter_->write(sock_, peerAddress_); if (consumed < 0) { firstSocketErrno = errno; + lastRetryableErrno_ = errno; } written = (consumed >= 0); if (happyEyeballsState_) { diff --git a/quic/api/IoBufQuicBatch.h b/quic/api/IoBufQuicBatch.h index 41eb4f505..5cc4c856c 100644 --- a/quic/api/IoBufQuicBatch.h +++ b/quic/api/IoBufQuicBatch.h @@ -42,6 +42,10 @@ class IOBufQuicBatch { return result_; } + [[nodiscard]] int getLastRetryableErrno() const { + return lastRetryableErrno_; + } + private: void reset(); @@ -59,6 +63,7 @@ class IOBufQuicBatch { QuicTransportStatsCallback* statsCallback_{nullptr}; QuicClientConnectionState::HappyEyeballsState* happyEyeballsState_; BufQuicBatchResult result_; + int lastRetryableErrno_{}; }; } // namespace quic diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 1c6b2cc91..d9ea087eb 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -221,6 +221,17 @@ WriteQuicDataResult writeQuicDataToSocketImpl( return result; } +void updateErrnoCount( + QuicConnectionStateBase& connection, + IOBufQuicBatch& ioBufBatch) { + int lastErrno = ioBufBatch.getLastRetryableErrno(); + if (lastErrno == EAGAIN || lastErrno == EWOULDBLOCK) { + connection.eagainOrEwouldblockCount++; + } else if (lastErrno == ENOBUFS) { + connection.enobufsCount++; + } +} + DataPathResult continuousMemoryBuildScheduleEncrypt( QuicConnectionStateBase& connection, PacketHeader header, @@ -254,6 +265,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( if (!packet || packet->packet.frames.empty()) { rollbackBuf(); ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); if (connection.loopDetectorCallback) { connection.writeDebugState.noWriteReason = NoWriteReason::NO_FRAME; } @@ -263,6 +275,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( // No more space remaining. rollbackBuf(); ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); if (connection.loopDetectorCallback) { connection.writeDebugState.noWriteReason = NoWriteReason::NO_BODY; } @@ -307,6 +320,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( } // TODO: I think we should add an API that doesn't need a buffer. bool ret = ioBufBatch.write(nullptr /* no need to pass buf */, encodedSize); + updateErrnoCount(connection, ioBufBatch); return DataPathResult::makeWriteResult( ret, std::move(result), encodedSize, encodedBodySize); } @@ -333,6 +347,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( auto& packet = result.packet; if (!packet || packet->packet.frames.empty()) { ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); if (connection.loopDetectorCallback) { connection.writeDebugState.noWriteReason = NoWriteReason::NO_FRAME; } @@ -341,6 +356,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( if (packet->body.empty()) { // No more space remaining. ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); if (connection.loopDetectorCallback) { connection.writeDebugState.noWriteReason = NoWriteReason::NO_BODY; } @@ -378,6 +394,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( << " encodedBodySize=" << encodedBodySize; } bool ret = ioBufBatch.write(std::move(packetBuf), encodedSize); + updateErrnoCount(connection, ioBufBatch); return DataPathResult::makeWriteResult( ret, std::move(result), encodedSize, encodedBodySize); } @@ -1525,7 +1542,9 @@ WriteQuicDataResult writeConnectionDataToSocket( // If we have a pending write to retry. Flush that first and make sure it // succeeds before scheduling any new data. if (pendingBufferedWrite) { - if (!ioBufBatch.flush()) { + bool flushSuccess = ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); + if (!flushSuccess) { // Could not flush retried data. Return empty write result and wait for // next retry. return {0, 0, 0}; @@ -1591,6 +1610,7 @@ WriteQuicDataResult writeConnectionDataToSocket( // If we're returning because we couldn't schedule more packets, // make sure we flush the buffer in this function. ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); return {ioBufBatch.getPktSent(), 0, bytesWritten}; } // If we build a packet, we updateConnection(), even if write might have @@ -1632,11 +1652,13 @@ WriteQuicDataResult writeConnectionDataToSocket( // With SinglePacketInplaceBatchWriter we always write one packet, and so // ioBufBatch needs a flush. ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); } } // Ensure that the buffer is flushed before returning ioBufBatch.flush(); + updateErrnoCount(connection, ioBufBatch); if (connection.transportSettings.dataPathType == DataPathType::ContinuousMemory) { diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index b509ac649..6ddd6848b 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -1881,4 +1881,12 @@ uint64_t QuicClientTransport::getNumPingFramesSent() const { return conn_->numPingFramesSent; } +uint64_t QuicClientTransport::getEagainOrEwouldblockCount() const { + return conn_->eagainOrEwouldblockCount; +} + +uint64_t QuicClientTransport::getEnobufsCount() const { + return conn_->enobufsCount; +} + } // namespace quic diff --git a/quic/client/QuicClientTransport.h b/quic/client/QuicClientTransport.h index 6cb4a95d9..ae5f1500d 100644 --- a/quic/client/QuicClientTransport.h +++ b/quic/client/QuicClientTransport.h @@ -222,6 +222,10 @@ class QuicClientTransport uint64_t getNumPingFramesSent() const; + uint64_t getEagainOrEwouldblockCount() const; + + uint64_t getEnobufsCount() const; + class HappyEyeballsConnAttemptDelayTimeout : public QuicTimerCallback { public: explicit HappyEyeballsConnAttemptDelayTimeout( diff --git a/quic/state/StateData.h b/quic/state/StateData.h index 2c1f45bfa..72715eaad 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -501,6 +501,9 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction { uint64_t numPingFramesSent{0}; + uint64_t eagainOrEwouldblockCount{0}; + uint64_t enobufsCount{0}; + struct ConnectionFlowControlState { // The size of the connection flow control window. uint64_t windowSize{0};