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

Put opportunistic ACKing behind TransportSetting

Browse Source 
Summary:
This has been a default behavior for us for a long time. Basically, we will always include ACK frames in a burst if there's new packets to ACK.

This is overall probably fine and is flexible to peer behavior such that the peer can get away with basically never sending anything ACK-eliciting.

However it is not without its issues. In particular, for DSR it means that we write an excessive amount of pure ACK packets since they cannot be coalesced with a DSR burst.

Make this behavior optional, such that we only include ACK frames in non-probing schedulers when we are only writing stream data.

Reviewed By: kvtsoy

Differential Revision: D39479149

fbshipit-source-id: 6e92d45311a3fb23750c93483b94e97dd3fdce26
This commit is contained in:
Matt Joras
2023-06-15 13:15:08 -07:00
committed by Facebook GitHub Bot
parent 74dc1b25e4
commit 3c5a34ee11
4 changed files with 49 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
quic/api/test/QuicTransportTest.cpp
View File

@@ -2025,6 +2025,43 @@ TEST_F(QuicTransportTest, WritePendingAckIfHavingData) {
EXPECT_EQ(0, ackState.numNonRxPacketsRecvd);
}
TEST_F(QuicTransportTest, NoWritePendingAckIfHavingData) {
auto& conn = transport_->getConnectionState();
conn.transportSettings.opportunisticAcking = false;
auto streamId = transport_->createBidirectionalStream().value();
auto buf = buildRandomInputData(20);
PacketNum start = 10;
PacketNum end = 15;
addAckStatesWithCurrentTimestamps(conn.ackStates.appDataAckState, start, end);
conn.ackStates.appDataAckState.needsToSendAckImmediately = false;
conn.ackStates.appDataAckState.numNonRxPacketsRecvd = 3;
EXPECT_CALL(*socket_, write(_, _)).WillOnce(Invoke(bufLength));
// We should write acks if there is data pending
transport_->writeChain(streamId, buf->clone(), true);
loopForWrites();
EXPECT_EQ(conn.outstandings.packets.size(), 1);
auto& packet =
getFirstOutstandingPacket(conn, PacketNumberSpace::AppData)->packet;
EXPECT_GE(packet.frames.size(), 1);
bool ackFound = false;
for (auto& frame : packet.frames) {
auto ackFrame = frame.asWriteAckFrame();
if (!ackFrame) {
continue;
}
ackFound = true;
}
EXPECT_FALSE(ackFound);
EXPECT_EQ(conn.ackStates.appDataAckState.largestAckScheduled, folly::none);
auto pnSpace = packet.header.getPacketNumberSpace();
auto ackState = getAckState(conn, pnSpace);
EXPECT_EQ(ackState.largestAckScheduled, folly::none);
EXPECT_FALSE(ackState.needsToSendAckImmediately);
EXPECT_EQ(3, ackState.numNonRxPacketsRecvd);
}
TEST_F(QuicTransportTest, RstStream) {
auto streamId = transport_->createBidirectionalStream().value();
EXPECT_CALL(*socket_, write(_, _)).WillOnce(Invoke(bufLength));