dcsctp: Set I-SACK bit when cwnd is low

To reduce latency when delivering messages on channel with low traffic
volume and with packet loss, where retransmissions are not driven by
fast retransmit but by T3-RTX timer, set the I-SACK bit (RFC7053) when
the congestion window is low.

Note that RFC7053 doesn't have to be negotiated, as is explained in
https://www.rfc-editor.org/rfc/rfc7053.html#section-6, and if the
receiver doesn't support it, it will delay SACKs as is done today.

When T3-RTX fires, the congestion window will be set to one MTU and any
future sent message will only send one MTU's worth of data before waiting
for the receiver's SACK until more data is sent. Delayed SACK, which is
normally used, could delay the next packet from being sent unecessarily
long. Setting I-SACK when the congestion window is small will make the
receiver always send a SACK for every received packet with a DATA chunk
in it. By not setting it always, it will not affect the packet rate when
the channel is fully utilized.

This modification improves latency in the aforementioned situations
without significantly affecting bandwidth. While this change increases
SACK frequency in specific scenarios, the impact on overall network load
is expected to be minimal.

Bug: webrtc:396080535
Change-Id: If4a5aa960969f1385c9ea59baa7e2d52caf25626
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377140
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43897}

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

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<uint8_t>(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<uint8_t>(kLargeMessageSize)),
+                kSendOptions);
+
+  a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), std::vector<uint8_t>(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<uint8_t>(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

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));
       }
     }