diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index b644030a6..60266c357 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -235,6 +235,8 @@ constexpr uint64_t kDefaultMinBurstPackets = 5; // but the notifications can get delayed if the event loop is busy // this is subject to testing but I would suggest a value >= 200usec constexpr std::chrono::microseconds kDefaultPacingTimerTickInterval{1000}; +// Fraction of RTT that is used to limit how long a write function can loop +constexpr std::chrono::microseconds::rep kDefaultWriteLimitRttFraction = 25; // Congestion control: constexpr folly::StringPiece kCongestionControlCubicStr = "cubic"; diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 90fb8382f..78ab4d426 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -927,7 +927,22 @@ uint64_t writeConnectionDataToSocket( connection.debugState.noWriteReason = NoWriteReason::EMPTY_SCHEDULER; } } - while (scheduler.hasData() && ioBufBatch.getPktSent() < packetLimit) { + auto writeLoopBeginTime = Clock::now(); + // helper functor to check if we have been write in a loop for longer than the + // RTT fraction that we are allowed to write. Only kicks in if we have write + // one batch in batching write mode. + auto timeLimitHelper = [&]() -> bool { + auto batchSize = connection.transportSettings.batchingMode == + quic::QuicBatchingMode::BATCHING_MODE_NONE + ? connection.transportSettings.writeConnectionDataPacketsLimit + : connection.transportSettings.maxBatchSize; + return ioBufBatch.getPktSent() < batchSize || + connection.lossState.srtt == 0us || + Clock::now() - writeLoopBeginTime < connection.lossState.srtt / + connection.transportSettings.writeLimitRttFraction; + }; + while (scheduler.hasData() && ioBufBatch.getPktSent() < packetLimit && + timeLimitHelper()) { auto packetNum = getNextPacketNum(connection, pnSpace); auto header = builder(srcConnId, dstConnId, packetNum, version, token); uint32_t writableBytes = folly::to(std::min( diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index c746bb922..a24f948f1 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -1985,5 +1985,36 @@ TEST_F(QuicTransportFunctionsTest, TimeoutBasedRetxCountUpdate) { EXPECT_EQ(247, conn->lossState.timeoutBasedRtxCount); } +TEST_F(QuicTransportFunctionsTest, WriteLimitBytRttFraction) { + auto conn = createConn(); + conn->lossState.srtt = 50ms; + auto mockCongestionController = std::make_unique(); + auto rawCongestionController = mockCongestionController.get(); + conn->congestionController = std::move(mockCongestionController); + + EventBase evb; + auto socket = std::make_unique(&evb); + auto rawSocket = socket.get(); + + auto stream1 = conn->streamManager->createNextBidirectionalStream().value(); + auto buf = buildRandomInputData(2048 * 1024); + writeDataToQuicStream(*stream1, buf->clone(), true); + + EXPECT_CALL(*rawSocket, write(_, _)).WillRepeatedly(Return(1)); + EXPECT_CALL(*rawCongestionController, getWritableBytes()) + .WillRepeatedly(Return(50)); + EXPECT_GT( + 500, + writeQuicDataToSocket( + *rawSocket, + *conn, + *conn->clientConnectionId, + *conn->serverConnectionId, + *aead, + *headerCipher, + getVersion(*conn), + 500 /* packetLimit */)); +} + } // namespace test } // namespace quic diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index 64efec4ae..d02817ea9 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -90,6 +90,9 @@ struct TransportSettings { // writeConnectionDataToSocket. uint64_t writeConnectionDataPacketsLimit{ kDefaultWriteConnectionDataPacketLimit}; + // Fraction of RTT that is used to limit how long a write function can loop + std::chrono::microseconds::rep writeLimitRttFraction{ + kDefaultWriteLimitRttFraction}; // Frequency of sending flow control updates. We can send one update every // flowControlRttFrequency * RTT if the flow control changes. uint16_t flowControlRttFrequency{2};