dcsctp: Add burst limiter for sent packets
Some deployments, e.g. Chromium, has a limited send buffer. It's
reasonable that it's quite small, as it avoids queuing too much, which
typically results in increased latency for real-time communication. To
avoid SCTP to fill up the entire buffer at once - especially when doing
fast retransmissions - limit the amount of packets that are sent in one
go.
In a typical scenario, SCTP will not send more than three packets for
each incoming packet, which is is the case when a SACK is received which
has acknowledged two large packets, and which also adds the MTU to the
congestion window (due to in slow-start mode), which then may result in
sending three packets. So setting this value to four makes any
retransmission not use that much more of the send buffer.
This is analogous to usrsctp_sysctl_set_sctp_fr_max_burst_default in
usrsctp, which also has the default value of four (4).
Bug: webrtc:12943
Change-Id: Iff76a1668beadc8776fab10312ef9ee26f24e442
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228480
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34744}
diff --git a/net/dcsctp/public/dcsctp_options.h b/net/dcsctp/public/dcsctp_options.h
index caefcff..c00836e 100644
--- a/net/dcsctp/public/dcsctp_options.h
+++ b/net/dcsctp/public/dcsctp_options.h
@@ -132,6 +132,14 @@
// See https://tools.ietf.org/html/rfc4960#section-7.2.3.
size_t cwnd_mtus_min = 4;
+ // 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
+ // up to three packets for every SACK received, so the default limit is set
+ // just above that, and then mostly applicable for (but not limited to) fast
+ // retransmission scenarios.
+ int max_burst = 4;
+
// Maximum Data Retransmit Attempts (per DATA chunk).
int max_retransmissions = 10;
diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc
index 88138f6..4faba56 100644
--- a/net/dcsctp/socket/dcsctp_socket_test.cc
+++ b/net/dcsctp/socket/dcsctp_socket_test.cc
@@ -62,7 +62,8 @@
constexpr SendOptions kSendOptions;
constexpr size_t kLargeMessageSize = DcSctpOptions::kMaxSafeMTUSize * 20;
-static constexpr size_t kSmallMessageSize = 10;
+constexpr size_t kSmallMessageSize = 10;
+constexpr int kMaxBurstPackets = 4;
MATCHER_P(HasDataChunkWithStreamId, stream_id, "") {
absl::optional<SctpPacket> packet = SctpPacket::Parse(arg);
@@ -235,6 +236,7 @@
// To make the interval more predictable in tests.
options.heartbeat_interval_include_rtt = false;
options.enable_message_interleaving = enable_message_interleaving;
+ options.max_burst = kMaxBurstPackets;
return options;
}
@@ -1713,5 +1715,21 @@
expected_sent_packets + expected_queued_packets + 2);
}
+TEST_F(DcSctpSocketTest, DoesntSendMoreThanMaxBurstPackets) {
+ ConnectSockets();
+
+ sock_a_.Send(DcSctpMessage(StreamID(1), PPID(53),
+ std::vector<uint8_t>(kLargeMessageSize)),
+ kSendOptions);
+
+ for (int i = 0; i < kMaxBurstPackets; ++i) {
+ std::vector<uint8_t> packet = cb_a_.ConsumeSentPacket();
+ EXPECT_THAT(packet, Not(IsEmpty()));
+ sock_z_.ReceivePacket(std::move(packet)); // DATA
+ }
+
+ EXPECT_THAT(cb_a_.ConsumeSentPacket(), IsEmpty());
+}
+
} // namespace
} // namespace dcsctp
diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc
index 4fde40c..167534d 100644
--- a/net/dcsctp/socket/transmission_control_block.cc
+++ b/net/dcsctp/socket/transmission_control_block.cc
@@ -84,7 +84,7 @@
void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
TimeMs now) {
- for (int packet_idx = 0;; ++packet_idx) {
+ for (int packet_idx = 0; packet_idx < options_.max_burst; ++packet_idx) {
// Only add control chunks to the first packet that is sent, if sending
// multiple packets in one go (as allowed by the congestion window).
if (packet_idx == 0) {
