diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index d27b260753..a9d424f4bb 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -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("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(kMessageSize)), - kSendOptions); - - bool delivered_packet = false; - std::vector data_packet_sizes; - do { - delivered_packet = false; - std::vector 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 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("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(kSmallMessageSize)), diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index e179a8e4ae..cf62c159bf 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -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) { diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc index 8c0d227a36..9a246174b1 100644 --- a/net/dcsctp/tx/retransmission_queue.cc +++ b/net/dcsctp/tx/retransmission_queue.cc @@ -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> 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; diff --git a/net/dcsctp/tx/retransmission_queue.h b/net/dcsctp/tx/retransmission_queue.h index a0fbb33c47..15720647bf 100644 --- a/net/dcsctp/tx/retransmission_queue.h +++ b/net/dcsctp/tx/retransmission_queue.h @@ -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_; diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc index eb1e04a5bb..4c802e4531 100644 --- a/net/dcsctp/tx/retransmission_queue_test.cc +++ b/net/dcsctp/tx/retransmission_queue_test.cc @@ -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(chunk_size), "BE")); - }) - .WillRepeatedly([](Timestamp, size_t) { return absl::nullopt; }); - - EXPECT_TRUE(queue.can_send_data()); - std::vector> 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(chunk_size), "BE")); - }) - .WillRepeatedly([](Timestamp, size_t) { return absl::nullopt; }); - - EXPECT_TRUE(queue.can_send_data()); - std::vector> 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)