From 2d061aa464927c3c9d5126d9b11e77a91d553d33 Mon Sep 17 00:00:00 2001 From: Yang Chi Date: Fri, 19 Mar 2021 08:46:17 -0700 Subject: [PATCH] Mark DSR outstanding packets in QUIC outstanding packets queue Summary: as title Reviewed By: mjoras Differential Revision: D26986163 fbshipit-source-id: 2a90689650e6dfab1a88830188ae76fcddb894da --- quic/api/QuicTransportFunctions.cpp | 7 +- quic/api/QuicTransportFunctions.h | 3 +- quic/api/test/QuicPacketSchedulerTest.cpp | 19 +- quic/api/test/QuicTransportFunctionsTest.cpp | 255 ++++++++++++++++--- quic/dsr/WriteFunctions.cpp | 3 +- quic/dsr/test/WriteFunctionsTest.cpp | 19 +- quic/state/OutstandingPacket.h | 4 + 7 files changed, 261 insertions(+), 49 deletions(-) diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index c8851f803..b8358a20d 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -560,7 +560,8 @@ void updateConnection( folly::Optional packetEvent, RegularQuicWritePacket packet, TimePoint sentTime, - uint32_t encodedSize) { + uint32_t encodedSize, + bool isDSRPacket) { auto packetNum = packet.header.getPacketSequenceNum(); bool retransmittable = false; // AckFrame and PaddingFrame are not retx-able. bool isHandshake = false; @@ -802,6 +803,7 @@ void updateConnection( pkt.associatedEvent = std::move(packetEvent); conn.lossState.totalBytesCloned += encodedSize; } + pkt.isDSRPacket = isDSRPacket; if (conn.congestionController) { conn.congestionController->onPacketSent(pkt); @@ -1346,7 +1348,8 @@ uint64_t writeConnectionDataToSocket( std::move(result->packetEvent), std::move(result->packet->packet), Clock::now(), - folly::to(ret.encodedSize)); + folly::to(ret.encodedSize), + false /* isDSRPacket */); // if ioBufBatch.write returns false // it is because a flush() call failed diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h index 8a2e7adda..934ca644c 100644 --- a/quic/api/QuicTransportFunctions.h +++ b/quic/api/QuicTransportFunctions.h @@ -183,7 +183,8 @@ void updateConnection( folly::Optional packetEvent, RegularQuicWritePacket packet, TimePoint time, - uint32_t encodedSize); + uint32_t encodedSize, + bool isDSRPacket); /** * Returns the minimum available bytes window out of path validation rate diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index a23408f4b..2d2e29485 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -884,7 +884,12 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) { result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, bufferLength); updateConnection( - conn, folly::none, result.packet->packet, Clock::now(), bufferLength); + conn, + folly::none, + result.packet->packet, + Clock::now(), + bufferLength, + false /* isDSRPacket */); buf = bufAccessor.obtain(); ASSERT_EQ(conn.udpSendPacketLen, buf->length()); buf->clear(); @@ -954,7 +959,8 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) { folly::none, packetResult.packet->packet, Clock::now(), - encodedSize); + encodedSize, + false /* isDSRPacket */); // make packetNum too larger to be encoded into the same size: packetNum += 0xFF; @@ -1574,7 +1580,8 @@ TEST_F(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) { builder1.encodePacketHeader(); scheduler.writeStreams(builder1); auto packet1 = std::move(builder1).buildPacket().packet; - updateConnection(conn, folly::none, packet1, Clock::now(), 1000); + updateConnection( + conn, folly::none, packet1, Clock::now(), 1000, false /* isDSR */); EXPECT_EQ(1, packet1.frames.size()); auto& writeStreamFrame1 = *packet1.frames[0].asWriteStreamFrame(); EXPECT_EQ(streamId, writeStreamFrame1.streamId); @@ -1600,7 +1607,8 @@ TEST_F(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) { builder2.encodePacketHeader(); scheduler.writeStreams(builder2); auto packet2 = std::move(builder2).buildPacket().packet; - updateConnection(conn, folly::none, packet2, Clock::now(), 1000); + updateConnection( + conn, folly::none, packet2, Clock::now(), 1000, false /* isDSR */); EXPECT_EQ(1, packet2.frames.size()); auto& writeStreamFrame2 = *packet2.frames[0].asWriteStreamFrame(); EXPECT_EQ(streamId, writeStreamFrame2.streamId); @@ -1644,7 +1652,8 @@ TEST_F(QuicPacketSchedulerTest, RunOutFlowControlDuringStreamWrite) { builder1.encodePacketHeader(); scheduler.writeStreams(builder1); auto packet1 = std::move(builder1).buildPacket().packet; - updateConnection(conn, folly::none, packet1, Clock::now(), 1200); + updateConnection( + conn, folly::none, packet1, Clock::now(), 1200, false /* isDSR */); EXPECT_EQ(2, packet1.frames.size()); auto& writeStreamFrame1 = *packet1.frames[0].asWriteStreamFrame(); EXPECT_EQ(streamId1, writeStreamFrame1.streamId); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 4b258b28b..6e4f286d7 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -187,7 +187,13 @@ TEST_F(QuicTransportFunctionsTest, PingPacketGoesToOPList) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); packet.packet.frames.push_back(PingFrame()); EXPECT_EQ(0, conn->outstandings.packets.size()); - updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50); + updateConnection( + *conn, + folly::none, + packet.packet, + Clock::now(), + 50, + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.packets.size()); // But it won't set loss detection alarm EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm); @@ -231,7 +237,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnection) { .Times(1) .WillOnce(Return(true)); updateConnection( - *conn, folly::none, packet.packet, TimePoint{}, getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint{}, + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ( conn->ackStates.initialAckState.nextPacketNum, @@ -289,7 +300,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnection) { .Times(1) .WillOnce(Return(false)); updateConnection( - *conn, folly::none, packet2.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet2.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ( conn->ackStates.initialAckState.nextPacketNum, currentNextInitialPacketNum); @@ -365,7 +381,13 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionD6DNotConsumeSendPing) { packet.packet.frames.push_back(PingFrame()); auto packetNum = packet.packet.header.getPacketSequenceNum(); conn->d6d.lastProbe = D6DProbePacket(packetNum, 50); - updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50); + updateConnection( + *conn, + folly::none, + packet.packet, + Clock::now(), + 50, + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.packets.size()); EXPECT_TRUE(conn->outstandings.packets.front().metadata.isD6DProbe); EXPECT_EQ(1, conn->d6d.outstandingProbes); @@ -380,7 +402,13 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionD6DNeedsAppDataPNSpace) { packet.packet.frames.push_back(PingFrame()); auto packetNum = packet.packet.header.getPacketSequenceNum(); conn->d6d.lastProbe = D6DProbePacket(packetNum, 50); - updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50); + updateConnection( + *conn, + folly::none, + packet.packet, + Clock::now(), + 50, + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.packets.size()); EXPECT_FALSE(conn->outstandings.packets.front().metadata.isD6DProbe); EXPECT_EQ(0, conn->d6d.outstandingProbes); @@ -411,19 +439,22 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPacketSorting) { folly::none, handshakePacket.packet, TimePoint{}, - getEncodedSize(handshakePacket)); + getEncodedSize(handshakePacket), + false /* isDSRPacket */); updateConnection( *conn, folly::none, initialPacket.packet, TimePoint{}, - getEncodedSize(initialPacket)); + getEncodedSize(initialPacket), + false /* isDSRPacket */); updateConnection( *conn, folly::none, appDataPacket.packet, TimePoint{}, - getEncodedSize(appDataPacket)); + getEncodedSize(appDataPacket), + false /* isDSRPacket */); // verify qLogger added correct logs std::shared_ptr qLogger = std::dynamic_pointer_cast(conn->qLogger); @@ -469,7 +500,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionFinOnly) { writeDataToQuicStream(*stream1, nullptr, true); packet.packet.frames.push_back(WriteStreamFrame(stream1->id, 0, 0, true)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -514,7 +550,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionAllBytesExceptFin) { packet.packet.frames.push_back( WriteStreamFrame(stream1->id, 0, buf->computeChainDataLength(), false)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -558,7 +599,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionEmptyAckWriteResult) { auto currentPendingLargestAckScheduled = conn->ackStates.handshakeAckState.largestAckScheduled; updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -593,7 +639,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPureAckCounter) { ackFrame.ackBlocks.emplace_back(0, 100); packet.packet.frames.push_back(std::move(ackFrame)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); packetEncodedSize = @@ -610,7 +661,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPureAckCounter) { packet2.packet.frames.push_back(std::move(rstFrame)); updateConnection( - *conn, folly::none, packet2.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet2.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet and frame information std::shared_ptr qLogger = @@ -640,7 +696,12 @@ TEST_F(QuicTransportFunctionsTest, TestPaddingPureAckPacketIsStillPureAck) { packet.packet.frames.push_back(std::move(ackFrame)); packet.packet.frames.push_back(PaddingFrame()); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet and frames information std::shared_ptr qLogger = @@ -671,7 +732,12 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) { packet.packet.frames.push_back(WriteCryptoFrame(0, data->length())); initialStream->writeBuffer.append(data->clone()); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.initialPacketsCount); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount); EXPECT_EQ(1, conn->outstandings.packets.size()); @@ -685,7 +751,12 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) { initialStream->writeBuffer.append(data->clone()); initialStream->writeBuffer.append(data->clone()); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(2, conn->outstandings.initialPacketsCount); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount); EXPECT_EQ(2, conn->outstandings.packets.size()); @@ -710,7 +781,12 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) { packet.packet.frames.push_back(WriteCryptoFrame(0, data->length())); handshakeStream->writeBuffer.append(data->clone()); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.initialPacketsCount); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount); EXPECT_EQ(2, conn->outstandings.packets.size()); @@ -721,7 +797,12 @@ TEST_F(QuicTransportFunctionsTest, TestImplicitAck) { WriteCryptoFrame(data->length(), data->length())); handshakeStream->writeBuffer.append(data->clone()); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.initialPacketsCount); EXPECT_EQ(2, conn->outstandings.handshakePacketsCount); EXPECT_EQ(3, conn->outstandings.packets.size()); @@ -772,7 +853,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { packet.packet.frames.push_back(WriteCryptoFrame(0, 0)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.handshakePacketsCount); auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData); @@ -790,7 +876,8 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { folly::none, nonHandshake.packet, TimePoint(), - getEncodedSize(packet)); + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -840,7 +927,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionForOneRttCryptoData) { packet.packet.frames.push_back(WriteCryptoFrame(0, 0)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(0, conn->outstandings.handshakePacketsCount); EXPECT_EQ(1, conn->outstandings.packets.size()); @@ -860,7 +952,8 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionForOneRttCryptoData) { folly::none, nonHandshake.packet, TimePoint(), - getEncodedSize(packet)); + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -918,7 +1011,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithPureAck) { EXPECT_CALL(*rawController, onPacketSent(_)).Times(0); EXPECT_CALL(*rawPacer, onPacketSent()).Times(0); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_EQ(1, conn->lossState.totalPacketsSent); EXPECT_EQ(0, conn->lossState.totalAckElicitingPacketsSent); EXPECT_EQ(0, conn->outstandings.packets.size()); @@ -961,7 +1059,13 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithBytesStats) { conn->lossState.totalBytesAckedAtLastAck = 5000; conn->lossState.totalPacketsSent = 20; conn->lossState.totalAckElicitingPacketsSent = 15; - updateConnection(*conn, folly::none, packet.packet, TimePoint(), 555); + updateConnection( + *conn, + folly::none, + packet.packet, + TimePoint(), + 555, + false /* isDSRPacket */); EXPECT_EQ(21, conn->lossState.totalPacketsSent); EXPECT_EQ(16, conn->lossState.totalAckElicitingPacketsSent); @@ -1048,7 +1152,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithCloneResult) { MockClock::mockNow = [=]() { return futureMoment; }; EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(1); updateConnection( - *conn, event, std::move(writePacket), MockClock::now(), 1500); + *conn, + event, + std::move(writePacket), + MockClock::now(), + 1500, + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = std::dynamic_pointer_cast(conn->qLogger); @@ -1090,7 +1199,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionStreamWindowUpdate) { conn->streamManager->queueWindowUpdate(stream->id); packet.packet.frames.push_back(std::move(streamWindowUpdate)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -1120,7 +1234,12 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionConnWindowUpdate) { MaxDataFrame connWindowUpdate(conn->flowControlState.advertisedMaxOffset); packet.packet.frames.push_back(std::move(connWindowUpdate)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); // verify QLogger contains correct packet information std::shared_ptr qLogger = @@ -2066,7 +2185,13 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounter) { packet.packet.frames.emplace_back(connWindowUpdate); PacketEvent packetEvent(PacketNumberSpace::AppData, 100); conn->outstandings.packetEvents.insert(packetEvent); - updateConnection(*conn, packetEvent, packet.packet, TimePoint(), 123); + updateConnection( + *conn, + packetEvent, + packet.packet, + TimePoint(), + 123, + false /* isDSRPacket */); EXPECT_EQ(1, conn->outstandings.clonedPacketsCount); } @@ -2078,7 +2203,12 @@ TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) { packet.packet.frames.push_back(blockedFrame); conn->streamManager->queueBlocked(stream->id, 1000); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_FALSE(conn->streamManager->hasBlocked()); EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_EQ(0, conn->outstandings.clonedPacketsCount); @@ -2096,7 +2226,12 @@ TEST_F(QuicTransportFunctionsTest, ClonedBlocked) { conn->outstandings.packetEvents.insert(packetEvent); // This shall not crash updateConnection( - *conn, packetEvent, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + packetEvent, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_EQ(1, conn->outstandings.clonedPacketsCount); } @@ -2115,7 +2250,8 @@ TEST_F(QuicTransportFunctionsTest, TwoConnWindowUpdateWillCrash) { folly::none, packet.packet, TimePoint(), - getEncodedSize(packet)), + getEncodedSize(packet), + false /* isDSRPacket */), ".*Send more than one connection window update.*"); } @@ -2128,7 +2264,12 @@ TEST_F(QuicTransportFunctionsTest, WriteStreamFrameIsNotPureAck) { WriteStreamFrame writeStreamFrame(stream->id, 0, 5, false); packet.packet.frames.push_back(std::move(writeStreamFrame)); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_FALSE(conn->outstandings.packets.empty()); } @@ -2141,7 +2282,12 @@ TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) { packet.packet.frames.push_back(rstStreamFrame); conn->pendingEvents.resets.emplace(stream->id, rstStreamFrame); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_TRUE(conn->pendingEvents.resets.empty()); EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_EQ(0, conn->outstandings.clonedPacketsCount); @@ -2160,7 +2306,12 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) { conn->outstandings.packetEvents.insert(packetEvent); // This shall not crash updateConnection( - *conn, packetEvent, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + packetEvent, + packet.packet, + TimePoint(), + getEncodedSize(packet), + false /* isDSRPacket */); EXPECT_FALSE(conn->outstandings.packets.empty()); EXPECT_EQ(1, conn->outstandings.clonedPacketsCount); } @@ -2169,7 +2320,13 @@ TEST_F(QuicTransportFunctionsTest, TotalBytesSentUpdate) { auto conn = createConn(); conn->lossState.totalBytesSent = 1234; auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); - updateConnection(*conn, folly::none, packet.packet, TimePoint{}, 4321); + updateConnection( + *conn, + folly::none, + packet.packet, + TimePoint{}, + 4321, + false /* isDSRPacket */); EXPECT_EQ(5555, conn->lossState.totalBytesSent); } @@ -2178,7 +2335,13 @@ TEST_F(QuicTransportFunctionsTest, TotalPacketsSentUpdate) { auto conn = createConn(); conn->lossState.totalPacketsSent = startTotalPacketsSent; auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); - updateConnection(*conn, folly::none, packet.packet, TimePoint{}, 4321); + updateConnection( + *conn, + folly::none, + packet.packet, + TimePoint{}, + 4321, + false /* isDSRPacket */); EXPECT_EQ(startTotalPacketsSent + 1, conn->lossState.totalPacketsSent); } @@ -2192,7 +2355,13 @@ TEST_F(QuicTransportFunctionsTest, TimeoutBasedRetxCountUpdate) { packet.packet.frames.push_back(rstStreamFrame); PacketEvent packetEvent(PacketNumberSpace::AppData, 100); conn->outstandings.packetEvents.insert(packetEvent); - updateConnection(*conn, packetEvent, packet.packet, TimePoint(), 500); + updateConnection( + *conn, + packetEvent, + packet.packet, + TimePoint(), + 500, + false /* isDSRPacket */); EXPECT_EQ(247, conn->lossState.timeoutBasedRtxCount); } @@ -2634,7 +2803,12 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionWithBufferMeta) { packet.packet.frames.push_back(writeStreamFrame); updateConnection( - *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet)); + *conn, + folly::none, + packet.packet, + TimePoint(), + getEncodedSize(packet), + true /* dsr */); EXPECT_EQ(1000 + bufMetaStartingOffset, stream->writeBufMeta.offset); EXPECT_EQ(1000, stream->writeBufMeta.length); EXPECT_FALSE(stream->retransmissionBufMetas.empty()); @@ -2644,6 +2818,7 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionWithBufferMeta) { EXPECT_EQ(bufMetaStartingOffset, retxBufMetaIter->second.offset); EXPECT_EQ(1000, retxBufMetaIter->second.length); EXPECT_FALSE(retxBufMetaIter->second.eof); + EXPECT_TRUE(conn->outstandings.packets.back().isDSRPacket); // Manually lose this packet: stream->lossBufMetas.push_back(retxBufMetaIter->second); @@ -2660,13 +2835,15 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionWithBufferMeta) { folly::none, retxPacket.packet, TimePoint(), - getEncodedSize(retxPacket)); + getEncodedSize(retxPacket), + true /* dsr */); EXPECT_TRUE(stream->lossBufMetas.empty()); retxBufMetaIter = stream->retransmissionBufMetas.find(bufMetaStartingOffset); EXPECT_NE(retxBufMetaIter, stream->retransmissionBufMetas.end()); EXPECT_EQ(bufMetaStartingOffset, retxBufMetaIter->second.offset); EXPECT_EQ(1000, retxBufMetaIter->second.length); EXPECT_FALSE(retxBufMetaIter->second.eof); + EXPECT_TRUE(conn->outstandings.packets.back().isDSRPacket); } } // namespace test diff --git a/quic/dsr/WriteFunctions.cpp b/quic/dsr/WriteFunctions.cpp index c008ed307..58422ada4 100644 --- a/quic/dsr/WriteFunctions.cpp +++ b/quic/dsr/WriteFunctions.cpp @@ -72,7 +72,8 @@ uint64_t writePacketizationRequest( folly::none /* Packet Event */, packet.packet, Clock::now(), - packet.encodedSize); + packet.encodedSize, + true /* isDSRPacket */); if (!instructionAdded) { // TODO: should I flush? This depends on the sender I think. diff --git a/quic/dsr/test/WriteFunctionsTest.cpp b/quic/dsr/test/WriteFunctionsTest.cpp index 7d453513b..13dde2bac 100644 --- a/quic/dsr/test/WriteFunctionsTest.cpp +++ b/quic/dsr/test/WriteFunctionsTest.cpp @@ -13,6 +13,7 @@ #include #include #include +#include using namespace testing; @@ -64,6 +65,13 @@ class WriteFunctionsTest : public Test { return id; } + bool verifyAllOutstandingsAreDSR() const { + return std::all_of( + conn_.outstandings.packets.begin(), + conn_.outstandings.packets.end(), + [](const OutstandingPacket& packet) { return packet.isDSRPacket; }); + } + protected: QuicServerConnectionState conn_; DSRStreamFrameScheduler scheduler_; @@ -129,6 +137,8 @@ TEST_F(WriteFunctionsTest, WriteOne) { EXPECT_GT(stream->writeBufMeta.offset, currentBufMetaOffset); EXPECT_EQ(1, stream->retransmissionBufMetas.size()); EXPECT_EQ(1, instructionCounter_); + EXPECT_EQ(1, conn_.outstandings.packets.size()); + EXPECT_TRUE(verifyAllOutstandingsAreDSR()); } TEST_F(WriteFunctionsTest, WriteTwoInstructions) { @@ -143,6 +153,8 @@ TEST_F(WriteFunctionsTest, WriteTwoInstructions) { conn_, scheduler_, cid, packetLimit, *aead_, sender_)); EXPECT_EQ(2, stream->retransmissionBufMetas.size()); EXPECT_EQ(2, instructionCounter_); + EXPECT_EQ(2, conn_.outstandings.packets.size()); + EXPECT_TRUE(verifyAllOutstandingsAreDSR()); } TEST_F(WriteFunctionsTest, PacketLimit) { @@ -163,6 +175,8 @@ TEST_F(WriteFunctionsTest, PacketLimit) { conn_, scheduler_, cid, packetLimit, *aead_, sender_)); EXPECT_EQ(20, stream->retransmissionBufMetas.size()); EXPECT_EQ(20, instructionCounter_); + EXPECT_EQ(20, conn_.outstandings.packets.size()); + EXPECT_TRUE(verifyAllOutstandingsAreDSR()); } TEST_F(WriteFunctionsTest, WriteTwoStreams) { @@ -183,4 +197,7 @@ TEST_F(WriteFunctionsTest, WriteTwoStreams) { // 1:1 in the future. Then there will be two senders for this test case and // each of them will send out one instruction. EXPECT_EQ(2, instructionCounter_); -}} // namespace quic::test + EXPECT_EQ(2, conn_.outstandings.packets.size()); + EXPECT_TRUE(verifyAllOutstandingsAreDSR()); +} +} // namespace quic::test diff --git a/quic/state/OutstandingPacket.h b/quic/state/OutstandingPacket.h index 956ff58cf..7fd0b2f06 100644 --- a/quic/state/OutstandingPacket.h +++ b/quic/state/OutstandingPacket.h @@ -99,6 +99,10 @@ struct OutstandingPacket { // folly::none if the packet isn't a clone and hasn't been cloned. folly::Optional associatedEvent; + // Whether this is a DSR packet. A DSR packet's stream data isn't written + // by transport directly. + bool isDSRPacket{false}; + /** * Whether the packet is sent when congestion controller is in app-limited * state.