dcsctp: Simplify congestion control algorithm
There was a feature in the retransmission queue to avoid fragmenting large messages when the congestion window was large. This was a feature that intended to improve data channel performance under Chrome, where communication with the network process (over MOJO) was lossy and losing messages with small fragments could result in unnecessary retransmissions. But back then, the implementation for fast retransmit wasn't implemented correctly, so the benchmarking result don't reproduce any longer. So just improve the algorithm by removing this code. This aligns it with the RFC and makes it easier to implement pluggable congestion control algorithms (that wouldn't want this feature). Bug: webrtc:42223116 Change-Id: Ifaaa82dac4b8fe2f55418158ae8b3da97199212f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359681 Reviewed-by: Florent Castelli <orphis@webrtc.org> Commit-Queue: Victor Boivie <boivie@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42797}
This commit is contained in:
parent
de972c1126
commit
adb224b3e9
@ -2141,55 +2141,6 @@ TEST_P(DcSctpSocketParametrizedTest, DoesntSendMoreThanMaxBurstPackets) {
|
||||
MaybeHandoverSocketAndSendMessage(a, std::move(z));
|
||||
}
|
||||
|
||||
TEST_P(DcSctpSocketParametrizedTest, SendsOnlyLargePackets) {
|
||||
SocketUnderTest a("A");
|
||||
auto z = std::make_unique<SocketUnderTest>("Z");
|
||||
|
||||
ConnectSockets(a, *z);
|
||||
z = MaybeHandoverSocket(std::move(z));
|
||||
|
||||
// A really large message, to ensure that the congestion window is often full.
|
||||
constexpr size_t kMessageSize = 100000;
|
||||
a.socket.Send(
|
||||
DcSctpMessage(StreamID(1), PPID(53), std::vector<uint8_t>(kMessageSize)),
|
||||
kSendOptions);
|
||||
|
||||
bool delivered_packet = false;
|
||||
std::vector<size_t> data_packet_sizes;
|
||||
do {
|
||||
delivered_packet = false;
|
||||
std::vector<uint8_t> packet_from_a = a.cb.ConsumeSentPacket();
|
||||
if (!packet_from_a.empty()) {
|
||||
data_packet_sizes.push_back(packet_from_a.size());
|
||||
delivered_packet = true;
|
||||
z->socket.ReceivePacket(std::move(packet_from_a));
|
||||
}
|
||||
std::vector<uint8_t> packet_from_z = z->cb.ConsumeSentPacket();
|
||||
if (!packet_from_z.empty()) {
|
||||
delivered_packet = true;
|
||||
a.socket.ReceivePacket(std::move(packet_from_z));
|
||||
}
|
||||
} while (delivered_packet);
|
||||
|
||||
size_t packet_payload_bytes =
|
||||
a.options.mtu - SctpPacket::kHeaderSize - DataChunk::kHeaderSize;
|
||||
// +1 accounts for padding, and rounding up.
|
||||
size_t expected_packets =
|
||||
(kMessageSize + packet_payload_bytes - 1) / packet_payload_bytes + 1;
|
||||
EXPECT_THAT(data_packet_sizes, SizeIs(expected_packets));
|
||||
|
||||
// Remove the last size - it will be the remainder. But all other sizes should
|
||||
// be large.
|
||||
data_packet_sizes.pop_back();
|
||||
|
||||
for (size_t size : data_packet_sizes) {
|
||||
// The 4 is for padding/alignment.
|
||||
EXPECT_GE(size, a.options.mtu - 4);
|
||||
}
|
||||
|
||||
MaybeHandoverSocketAndSendMessage(a, std::move(z));
|
||||
}
|
||||
|
||||
TEST(DcSctpSocketTest, SendMessagesAfterHandover) {
|
||||
SocketUnderTest a("A");
|
||||
auto z = std::make_unique<SocketUnderTest>("Z");
|
||||
@ -2884,20 +2835,6 @@ TEST(DcSctpSocketTest, ResetStreamsDeferred) {
|
||||
kSuccessPerformed))))))));
|
||||
a.socket.ReceivePacket(reconfig3);
|
||||
|
||||
EXPECT_THAT(
|
||||
data1,
|
||||
HasChunks(ElementsAre(IsDataChunk(Property(&DataChunk::ssn, SSN(0))))));
|
||||
EXPECT_THAT(
|
||||
data2,
|
||||
HasChunks(ElementsAre(IsDataChunk(Property(&DataChunk::ssn, SSN(0))))));
|
||||
EXPECT_THAT(
|
||||
data3,
|
||||
HasChunks(ElementsAre(IsDataChunk(Property(&DataChunk::ssn, SSN(1))))));
|
||||
EXPECT_THAT(reconfig, HasChunks(ElementsAre(IsReConfig(HasParameters(
|
||||
ElementsAre(IsOutgoingResetRequest(Property(
|
||||
&OutgoingSSNResetRequestParameter::stream_ids,
|
||||
ElementsAre(StreamID(1))))))))));
|
||||
|
||||
// Send a new message after the stream has been reset.
|
||||
a.socket.Send(DcSctpMessage(StreamID(1), PPID(55),
|
||||
std::vector<uint8_t>(kSmallMessageSize)),
|
||||
|
||||
@ -210,9 +210,7 @@ void TransmissionControlBlock::MaybeSendFastRetransmit() {
|
||||
|
||||
void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
|
||||
Timestamp now) {
|
||||
for (int packet_idx = 0;
|
||||
packet_idx < options_.max_burst && retransmission_queue_.can_send_data();
|
||||
++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) {
|
||||
|
||||
@ -46,9 +46,6 @@ namespace dcsctp {
|
||||
namespace {
|
||||
using ::webrtc::TimeDelta;
|
||||
using ::webrtc::Timestamp;
|
||||
|
||||
// Allow sending only slightly less than an MTU, to account for headers.
|
||||
constexpr float kMinBytesRequiredToSendFactor = 0.9;
|
||||
} // namespace
|
||||
|
||||
RetransmissionQueue::RetransmissionQueue(
|
||||
@ -65,7 +62,6 @@ RetransmissionQueue::RetransmissionQueue(
|
||||
bool use_message_interleaving)
|
||||
: callbacks_(*callbacks),
|
||||
options_(options),
|
||||
min_bytes_required_to_send_(options.mtu * kMinBytesRequiredToSendFactor),
|
||||
partial_reliability_(supports_partial_reliability),
|
||||
log_prefix_(log_prefix),
|
||||
data_chunk_header_size_(use_message_interleaving
|
||||
@ -534,11 +530,6 @@ std::vector<std::pair<TSN, Data>> RetransmissionQueue::GetChunksToSend(
|
||||
return to_be_sent;
|
||||
}
|
||||
|
||||
bool RetransmissionQueue::can_send_data() const {
|
||||
return cwnd_ < options_.avoid_fragmentation_cwnd_mtus * options_.mtu ||
|
||||
max_bytes_to_send() >= min_bytes_required_to_send_;
|
||||
}
|
||||
|
||||
bool RetransmissionQueue::ShouldSendForwardTsn(Timestamp now) {
|
||||
if (!partial_reliability_) {
|
||||
return false;
|
||||
|
||||
@ -126,9 +126,6 @@ class RetransmissionQueue {
|
||||
// Returns the number of DATA chunks that are in-flight.
|
||||
size_t unacked_items() const { return outstanding_data_.unacked_items(); }
|
||||
|
||||
// Indicates if the congestion control algorithm allows data to be sent.
|
||||
bool can_send_data() const;
|
||||
|
||||
// Given the current time `now`, it will evaluate if there are chunks that
|
||||
// have expired and that need to be discarded. It returns true if a
|
||||
// FORWARD-TSN should be sent.
|
||||
@ -217,9 +214,6 @@ class RetransmissionQueue {
|
||||
|
||||
DcSctpSocketCallbacks& callbacks_;
|
||||
const DcSctpOptions options_;
|
||||
// The minimum bytes required to be available in the congestion window to
|
||||
// allow packets to be sent - to avoid sending too small packets.
|
||||
const size_t min_bytes_required_to_send_;
|
||||
// If the peer supports RFC3758 - SCTP Partial Reliability Extension.
|
||||
const bool partial_reliability_;
|
||||
const absl::string_view log_prefix_;
|
||||
|
||||
@ -1405,66 +1405,6 @@ TEST_F(RetransmissionQueueTest, CwndRecoversWhenAcking) {
|
||||
EXPECT_EQ(queue.cwnd(), kCwnd + serialized_size);
|
||||
}
|
||||
|
||||
// Verifies that it doesn't produce tiny packets, when getting close to
|
||||
// the full congestion window.
|
||||
TEST_F(RetransmissionQueueTest, OnlySendsLargePacketsOnLargeCongestionWindow) {
|
||||
RetransmissionQueue queue = CreateQueue();
|
||||
size_t intial_cwnd = options_.avoid_fragmentation_cwnd_mtus * options_.mtu;
|
||||
queue.set_cwnd(intial_cwnd);
|
||||
EXPECT_EQ(queue.cwnd(), intial_cwnd);
|
||||
|
||||
// Fill the congestion window almost - leaving 500 bytes.
|
||||
size_t chunk_size = intial_cwnd - 500;
|
||||
EXPECT_CALL(producer_, Produce)
|
||||
.WillOnce([chunk_size, this](Timestamp, size_t) {
|
||||
return SendQueue::DataToSend(
|
||||
OutgoingMessageId(0),
|
||||
gen_.Ordered(std::vector<uint8_t>(chunk_size), "BE"));
|
||||
})
|
||||
.WillRepeatedly([](Timestamp, size_t) { return absl::nullopt; });
|
||||
|
||||
EXPECT_TRUE(queue.can_send_data());
|
||||
std::vector<std::pair<TSN, Data>> chunks_to_send =
|
||||
queue.GetChunksToSend(now_, 10000);
|
||||
EXPECT_THAT(chunks_to_send, ElementsAre(Pair(TSN(10), _)));
|
||||
|
||||
// To little space left - will not send more.
|
||||
EXPECT_FALSE(queue.can_send_data());
|
||||
|
||||
// But when the first chunk is acked, it will continue.
|
||||
queue.HandleSack(now_, SackChunk(TSN(10), kArwnd, {}, {}));
|
||||
|
||||
EXPECT_TRUE(queue.can_send_data());
|
||||
EXPECT_EQ(queue.unacked_bytes(), 0u);
|
||||
EXPECT_EQ(queue.cwnd(), intial_cwnd + kMaxMtu);
|
||||
}
|
||||
|
||||
TEST_F(RetransmissionQueueTest, AllowsSmallFragmentsOnSmallCongestionWindow) {
|
||||
RetransmissionQueue queue = CreateQueue();
|
||||
size_t intial_cwnd =
|
||||
options_.avoid_fragmentation_cwnd_mtus * options_.mtu - 1;
|
||||
queue.set_cwnd(intial_cwnd);
|
||||
EXPECT_EQ(queue.cwnd(), intial_cwnd);
|
||||
|
||||
// Fill the congestion window almost - leaving 500 bytes.
|
||||
size_t chunk_size = intial_cwnd - 500;
|
||||
EXPECT_CALL(producer_, Produce)
|
||||
.WillOnce([chunk_size, this](Timestamp, size_t) {
|
||||
return SendQueue::DataToSend(
|
||||
OutgoingMessageId(0),
|
||||
gen_.Ordered(std::vector<uint8_t>(chunk_size), "BE"));
|
||||
})
|
||||
.WillRepeatedly([](Timestamp, size_t) { return absl::nullopt; });
|
||||
|
||||
EXPECT_TRUE(queue.can_send_data());
|
||||
std::vector<std::pair<TSN, Data>> chunks_to_send =
|
||||
queue.GetChunksToSend(now_, 10000);
|
||||
EXPECT_THAT(chunks_to_send, ElementsAre(Pair(TSN(10), _)));
|
||||
|
||||
// With congestion window under limit, allow small packets to be created.
|
||||
EXPECT_TRUE(queue.can_send_data());
|
||||
}
|
||||
|
||||
TEST_F(RetransmissionQueueTest, ReadyForHandoverWhenHasNoOutstandingData) {
|
||||
RetransmissionQueue queue = CreateQueue();
|
||||
EXPECT_CALL(producer_, Produce)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user