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

Don't special case stream frames for opportunistic ACKing

Browse Source 
Summary: This was added to not bundle ACKs with stream frames. Don't special case those and also disabling opportunistic ACKing for all write reasons.

Reviewed By: jbeshay

Differential Revision: D66514392

fbshipit-source-id: f2657a5c06ea8ae37b8c8eacd04c5a3b8ac75390
This commit is contained in:
Matt Joras
2024-11-26 19:54:04 -08:00
committed by Facebook GitHub Bot
parent 2322ee8c8b
commit 058f5db28c
2 changed files with 40 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
quic/api/QuicTransportFunctions.cpp
View File

@@ -186,11 +186,9 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
.pingFrames() .pingFrames()
.datagramFrames() .datagramFrames()
.immediateAckFrames(); .immediateAckFrames();
// Only add ACK frames if we need to send an ACK, or if the write reason isn't // Only add ACK frames if we need to send an ACK.
// just streams.
if (connection.transportSettings.opportunisticAcking || if (connection.transportSettings.opportunisticAcking ||
toWriteAppDataAcks(connection) || toWriteAppDataAcks(connection)) {
(hasNonAckDataToWrite(connection) != WriteDataReason::STREAM)) {
schedulerBuilder.ackFrames(); schedulerBuilder.ackFrames();
} }
if (!exceptCryptoStream) { if (!exceptCryptoStream) {

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

@@ -2084,6 +2084,44 @@ TEST_F(QuicTransportTest, NoWritePendingAckIfHavingData) {
EXPECT_EQ(3, ackState.numNonRxPacketsRecvd); EXPECT_EQ(3, ackState.numNonRxPacketsRecvd);
} }
TEST_F(QuicTransportTest, NoWritePendingAckIfHavingDataNonStream) {
auto& conn = transport_->getConnectionState();
conn.transportSettings.opportunisticAcking = false;
auto streamId = transport_->createBidirectionalStream().value();
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(testing::WithArgs<1, 2>(Invoke(getTotalIovecLen)));
// We should write acks if there is data pending
conn.streamManager->queueWindowUpdate(streamId);
transport_->read(streamId, 0);
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, none);
auto pnSpace = packet.header.getPacketNumberSpace();
auto ackState = getAckState(conn, pnSpace);
EXPECT_EQ(ackState.largestAckScheduled, none);
EXPECT_FALSE(ackState.needsToSendAckImmediately);
EXPECT_EQ(3, ackState.numNonRxPacketsRecvd);
}
TEST_F(QuicTransportTest, RstStream) { TEST_F(QuicTransportTest, RstStream) {
auto streamId = transport_->createBidirectionalStream().value(); auto streamId = transport_->createBidirectionalStream().value();
EXPECT_CALL(*socket_, write(_, _, _)) EXPECT_CALL(*socket_, write(_, _, _))