lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-08 09:42:06 +03:00
Code Activity

Fix Datagram Write in case there is no space in the QUIC packet

Browse Source 
Summary: When there is not enough space in the QUIC packet to write a datagram frame, we should not dequeue the datagram

Reviewed By: mjoras

Differential Revision: D33109603

fbshipit-source-id: 937a5a0ffe55c7f88e39faf224e7ad06ca599708
This commit is contained in:
Luca Niccolini
2021-12-16 11:43:58 -08:00
committed by Facebook GitHub Bot
parent cae8a1b705
commit 95ff9198dd
4 changed files with 34 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
quic/api/QuicPacketScheduler.cpp
View File

@@ -630,9 +630,18 @@ bool DatagramFrameScheduler::writeDatagramFrames(
for (size_t i = 0; i <= conn_.datagramState.writeBuffer.size(); ++i) { for (size_t i = 0; i <= conn_.datagramState.writeBuffer.size(); ++i) {
auto& payload = conn_.datagramState.writeBuffer.front(); auto& payload = conn_.datagramState.writeBuffer.front();
auto len = payload.chainLength(); auto len = payload.chainLength();
uint64_t spaceLeft = builder.remainingSpaceInPkt();
QuicInteger frameTypeQuicInt(static_cast<uint8_t>(FrameType::DATAGRAM_LEN));
QuicInteger datagramLenInt(len);
auto datagramFrameLength =
frameTypeQuicInt.getSize() + len + datagramLenInt.getSize();
if (folly::to<uint64_t>(datagramFrameLength) <= spaceLeft) {
auto datagramFrame = DatagramFrame(len, payload.move()); auto datagramFrame = DatagramFrame(len, payload.move());
if (writeFrame(datagramFrame, builder) > 0) { auto res = writeFrame(datagramFrame, builder);
QUIC_STATS(conn_.statsCallback, onDatagramWrite, payload.chainLength()); // Must always succeed since we have already checked that there is enough
// space to write the frame
CHECK_GT(res, 0);
QUIC_STATS(conn_.statsCallback, onDatagramWrite, len);
conn_.datagramState.writeBuffer.pop_front(); conn_.datagramState.writeBuffer.pop_front();
sent = true; sent = true;
} }

16
quic/api/test/QuicPacketSchedulerTest.cpp
View File

@@ -1965,7 +1965,9 @@ TEST_F(QuicPacketSchedulerTest, DatagramFrameSchedulerMultipleFramesPerPacket) {
})); }));
NiceMock<MockQuicStats> quicStats; NiceMock<MockQuicStats> quicStats;
conn.statsCallback = &quicStats; conn.statsCallback = &quicStats;
EXPECT_CALL(quicStats, onDatagramWrite(_)).Times(2); EXPECT_CALL(quicStats, onDatagramWrite(_))
.Times(2)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
// Call scheduler // Call scheduler
auto& frames = builder.frames_; auto& frames = builder.frames_;
scheduler.writeDatagramFrames(builder); scheduler.writeDatagramFrames(builder);
@@ -1993,10 +1995,14 @@ TEST_F(QuicPacketSchedulerTest, DatagramFrameSchedulerOneFramePerPacket) {
conn.statsCallback = &quicStats; conn.statsCallback = &quicStats;
// Call scheduler // Call scheduler
auto& frames = builder.frames_; auto& frames = builder.frames_;
EXPECT_CALL(quicStats, onDatagramWrite(_)).Times(1); EXPECT_CALL(quicStats, onDatagramWrite(_))
.Times(1)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
scheduler.writeDatagramFrames(builder); scheduler.writeDatagramFrames(builder);
ASSERT_EQ(frames.size(), 1); ASSERT_EQ(frames.size(), 1);
EXPECT_CALL(quicStats, onDatagramWrite(_)).Times(1); EXPECT_CALL(quicStats, onDatagramWrite(_))
.Times(1)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
scheduler.writeDatagramFrames(builder); scheduler.writeDatagramFrames(builder);
ASSERT_EQ(frames.size(), 2); ASSERT_EQ(frames.size(), 2);
} }
@@ -2025,7 +2031,9 @@ TEST_F(QuicPacketSchedulerTest, DatagramFrameWriteWhenRoomAvailable) {
ASSERT_EQ(frames.size(), 0); ASSERT_EQ(frames.size(), 0);
EXPECT_CALL(builder, remainingSpaceInPkt()) EXPECT_CALL(builder, remainingSpaceInPkt())
.WillRepeatedly(Return(conn.udpSendPacketLen / 2)); .WillRepeatedly(Return(conn.udpSendPacketLen / 2));
EXPECT_CALL(quicStats, onDatagramWrite(_)).Times(1); EXPECT_CALL(quicStats, onDatagramWrite(_))
.Times(1)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
scheduler.writeDatagramFrames(builder); scheduler.writeDatagramFrames(builder);
ASSERT_EQ(frames.size(), 1); ASSERT_EQ(frames.size(), 1);
} }

11
quic/fizz/client/test/QuicClientTransportTest.cpp
View File

@@ -4629,7 +4629,8 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveDatagramFrameAndStore) {
conn.datagramState.maxReadBufferSize = 10; conn.datagramState.maxReadBufferSize = 10;
EXPECT_CALL(*quicStats_, onDatagramRead(_)) EXPECT_CALL(*quicStats_, onDatagramRead(_))
.Times(conn.datagramState.maxReadBufferSize); .Times(conn.datagramState.maxReadBufferSize)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead()) EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead())
.Times(conn.datagramState.maxReadBufferSize); .Times(conn.datagramState.maxReadBufferSize);
for (uint64_t i = 0; i < conn.datagramState.maxReadBufferSize * 2; i++) { for (uint64_t i = 0; i < conn.datagramState.maxReadBufferSize * 2; i++) {
@@ -4679,7 +4680,9 @@ TEST_F(
datagramPayload1.size(), IOBuf::copyBuffer(datagramPayload1)); datagramPayload1.size(), IOBuf::copyBuffer(datagramPayload1));
writeFrame(datagramFrame1, builder1); writeFrame(datagramFrame1, builder1);
auto packet1 = packetToBuf(std::move(builder1).buildPacket()); auto packet1 = packetToBuf(std::move(builder1).buildPacket());
EXPECT_CALL(*quicStats_, onDatagramRead(_)).Times(1); EXPECT_CALL(*quicStats_, onDatagramRead(_))
.Times(1)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
deliverData(packet1->coalesce()); deliverData(packet1->coalesce());
ASSERT_EQ( ASSERT_EQ(
client->getConn().datagramState.readBuffer.size(), client->getConn().datagramState.readBuffer.size(),
@@ -4698,7 +4701,9 @@ TEST_F(
writeFrame(datagramFrame2, builder2); writeFrame(datagramFrame2, builder2);
auto packet2 = packetToBuf(std::move(builder2).buildPacket()); auto packet2 = packetToBuf(std::move(builder2).buildPacket());
EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead()).Times(1); EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead()).Times(1);
EXPECT_CALL(*quicStats_, onDatagramRead(_)).Times(1); EXPECT_CALL(*quicStats_, onDatagramRead(_))
.Times(1)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
deliverData(packet2->coalesce()); deliverData(packet2->coalesce());
ASSERT_EQ( ASSERT_EQ(
client->getConn().datagramState.readBuffer.size(), client->getConn().datagramState.readBuffer.size(),

3
quic/server/test/QuicServerTransportTest.cpp
View File

@@ -2750,7 +2750,8 @@ TEST_F(QuicServerTransportTest, ReceiveDatagramFrameAndStore) {
conn.datagramState.maxReadBufferSize = 10; conn.datagramState.maxReadBufferSize = 10;
EXPECT_CALL(*quicStats_, onDatagramRead(_)) EXPECT_CALL(*quicStats_, onDatagramRead(_))
.Times(conn.datagramState.maxReadBufferSize); .Times(conn.datagramState.maxReadBufferSize)
.WillRepeatedly(Invoke([](uint64_t bytes) { EXPECT_GT(bytes, 0); }));
EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead()) EXPECT_CALL(*quicStats_, onDatagramDroppedOnRead())
.Times(conn.datagramState.maxReadBufferSize); .Times(conn.datagramState.maxReadBufferSize);
for (uint64_t i = 0; i < conn.datagramState.maxReadBufferSize * 2; i++) { for (uint64_t i = 0; i < conn.datagramState.maxReadBufferSize * 2; i++) {