diff --git a/net/dcsctp/public/dcsctp_options.h b/net/dcsctp/public/dcsctp_options.h index 115cf5e466..6e1181f7e2 100644 --- a/net/dcsctp/public/dcsctp_options.h +++ b/net/dcsctp/public/dcsctp_options.h @@ -176,6 +176,16 @@ struct DcSctpOptions { // creating small fragmented packets. size_t avoid_fragmentation_cwnd_mtus = 6; + // When the congestion window is below this number of MTUs, sent data chunks + // will have the "I" (Immediate SACK - RFC7053) bit set. That will prevent the + // receiver from delaying the SACK, which result in shorter time until the + // sender can send the next packet as its driven by SACKs. This can reduce + // latency for low utilized and lossy connections. + // + // Default value set to be same as initial congestion window. Set to zero to + // disable. + size_t immediate_sack_under_cwnd_mtus = 10; + // The number of packets that may be sent at once. This is limited to avoid // bursts that too quickly fill the send buffer. Typically in a a socket in // its "slow start" phase (when it sends as much as it can), it will send diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 299f4ec996..0684f99c72 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -68,6 +68,7 @@ using ::testing::AllOf; using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Eq; +using ::testing::Field; using ::testing::HasSubstr; using ::testing::IsEmpty; using ::testing::Not; @@ -3298,5 +3299,91 @@ TEST(DcSctpSocketTest, ConnectionCanContinueFromSecondInitAck) { EXPECT_THAT(msg->payload(), SizeIs(kLargeMessageSize)); } +TEST_P(DcSctpSocketParametrizedTest, LowCongestionWindowSetsIsackBit) { + // This test verifies the option `immediate_sack_under_cwnd_mtus`. + DcSctpOptions options = {.cwnd_mtus_initial = 4, + .immediate_sack_under_cwnd_mtus = 2}; + SocketUnderTest a("A", options); + SocketUnderTest z("Z"); + + ConnectSockets(a, z); + + EXPECT_EQ(a.socket.GetMetrics()->cwnd_bytes, + options.cwnd_mtus_initial * options.mtu); + + a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), std::vector(1)), + SendOptions()); + + // Drop the first packet, and let T3-rtx fire, which lowers cwnd. + auto packet1 = a.cb.ConsumeSentPacket(); + EXPECT_THAT(packet1, + HasChunks(ElementsAre(IsDataChunk(AllOf( + Property(&DataChunk::stream_id, StreamID(1)), + Property(&DataChunk::options, + Field(&AnyDataChunk::Options::immediate_ack, + AnyDataChunk::ImmediateAckFlag(false)))))))); + + AdvanceTime(a, z, a.options.rto_initial.ToTimeDelta()); + EXPECT_EQ(a.socket.GetMetrics()->cwnd_bytes, 1 * options.mtu); + + // Observe that the retransmission will have the I-SACK bit set. + auto packet2 = a.cb.ConsumeSentPacket(); + z.socket.ReceivePacket(packet2); + EXPECT_THAT(packet2, + HasChunks(ElementsAre(IsDataChunk(AllOf( + Property(&DataChunk::stream_id, StreamID(1)), + Property(&DataChunk::options, + Field(&AnyDataChunk::Options::immediate_ack, + AnyDataChunk::ImmediateAckFlag(true)))))))); + + // The receiver immediately SACKS. It would even without this bit set. + auto packet3 = z.cb.ConsumeSentPacket(); + a.socket.ReceivePacket(packet3); + EXPECT_THAT(packet3, HasChunks(ElementsAre(IsChunkType(SackChunk::kType)))); + + // Next sent chunk will also have the i-sack set, as cwnd is low. + a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), + std::vector(kLargeMessageSize)), + kSendOptions); + + a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), std::vector(1)), + SendOptions()); + + // Observe that the retransmission will have the I-SACK bit set. + auto packet4 = a.cb.ConsumeSentPacket(); + z.socket.ReceivePacket(packet4); + EXPECT_THAT(packet4, + HasChunks(ElementsAre(IsDataChunk(AllOf( + Property(&DataChunk::stream_id, StreamID(1)), + Property(&DataChunk::options, + Field(&AnyDataChunk::Options::immediate_ack, + AnyDataChunk::ImmediateAckFlag(true)))))))); + + // The receiver would normally delay this sack, but now it's sent directly. + auto packet5 = z.cb.ConsumeSentPacket(); + a.socket.ReceivePacket(packet5); + EXPECT_THAT(packet5, HasChunks(ElementsAre(IsChunkType(SackChunk::kType)))); + + // Transfer the rest of the message. + ExchangeMessages(a, z); + + // This will grow the cwnd, as the message was large. + EXPECT_GT(a.socket.GetMetrics()->cwnd_bytes, + options.immediate_sack_under_cwnd_mtus * options.mtu); + + // Future chunks will then not have the I-SACK bit set. + a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), std::vector(1)), + SendOptions()); + + // Drop the first packet, and let T3-rtx fire, which lowers cwnd. + auto packet6 = a.cb.ConsumeSentPacket(); + EXPECT_THAT(packet6, + HasChunks(ElementsAre(IsDataChunk(AllOf( + Property(&DataChunk::stream_id, StreamID(1)), + Property(&DataChunk::options, + Field(&AnyDataChunk::Options::immediate_ack, + AnyDataChunk::ImmediateAckFlag(false)))))))); +} + } // namespace } // namespace dcsctp diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index 38913e6257..3ee6110e2f 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -248,11 +248,13 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder, heartbeat_handler_.RestartTimer(); } + bool set_immediate_sack_bit = + cwnd() < (options_.immediate_sack_under_cwnd_mtus * options_.mtu); for (auto& [tsn, data] : chunks) { if (capabilities_.message_interleaving) { - builder.Add(IDataChunk(tsn, std::move(data), false)); + builder.Add(IDataChunk(tsn, std::move(data), set_immediate_sack_bit)); } else { - builder.Add(DataChunk(tsn, std::move(data), false)); + builder.Add(DataChunk(tsn, std::move(data), set_immediate_sack_bit)); } }