From 82ea522b2733be34b44e6e5bfbc6313d13427c4e Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Mon, 9 Aug 2021 12:26:11 +0200 Subject: [PATCH] dcsctp: Track the number of inflight DATA items This corresponds to one part of sstat_unackdata in RFC6458. The remaining part is the data in the send queue, which isn't packetized yet, so it must be estimated. But the DATA items in the retransmission queue is already determined, so it can be easily tracked and retrieved. Bug: webrtc:13052 Change-Id: I16c3b5b61eb6b3022d7104e6457d943d5df3d6b9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228240 Reviewed-by: Florent Castelli Commit-Queue: Victor Boivie Cr-Commit-Position: refs/heads/master@{#34706} --- net/dcsctp/tx/retransmission_queue.cc | 8 ++++++++ net/dcsctp/tx/retransmission_queue.h | 6 ++++++ net/dcsctp/tx/retransmission_queue_test.cc | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc index 51bb65a30c..c1334ecf0f 100644 --- a/net/dcsctp/tx/retransmission_queue.cc +++ b/net/dcsctp/tx/retransmission_queue.cc @@ -85,11 +85,13 @@ RetransmissionQueue::RetransmissionQueue( bool RetransmissionQueue::IsConsistent() const { size_t actual_outstanding_bytes = 0; + size_t actual_outstanding_items = 0; std::set actual_to_be_retransmitted; for (const auto& elem : outstanding_data_) { if (elem.second.is_outstanding()) { actual_outstanding_bytes += GetSerializedChunkSize(elem.second.data()); + ++actual_outstanding_items; } if (elem.second.should_be_retransmitted()) { @@ -98,6 +100,7 @@ bool RetransmissionQueue::IsConsistent() const { } return actual_outstanding_bytes == outstanding_bytes_ && + actual_outstanding_items == outstanding_items_ && actual_to_be_retransmitted == to_be_retransmitted_; } @@ -115,6 +118,7 @@ void RetransmissionQueue::RemoveAcked(UnwrappedTSN cumulative_tsn_ack, ack_info.acked_tsns.push_back(it->first.Wrap()); if (it->second.is_outstanding()) { outstanding_bytes_ -= GetSerializedChunkSize(it->second.data()); + --outstanding_items_; } else if (it->second.should_be_retransmitted()) { to_be_retransmitted_.erase(it->first); } @@ -143,6 +147,7 @@ void RetransmissionQueue::AckGapBlocks( iter->second.data().size(); if (iter->second.is_outstanding()) { outstanding_bytes_ -= GetSerializedChunkSize(iter->second.data()); + --outstanding_items_; } if (iter->second.should_be_retransmitted()) { to_be_retransmitted_.erase(iter->first); @@ -517,6 +522,7 @@ bool RetransmissionQueue::NackItem(UnwrappedTSN tsn, bool retransmit_now) { if (item.is_outstanding()) { outstanding_bytes_ -= GetSerializedChunkSize(item.data()); + --outstanding_items_; } switch (item.Nack(retransmit_now)) { @@ -555,6 +561,7 @@ RetransmissionQueue::GetChunksToBeRetransmitted(size_t max_size) { result.emplace_back(tsn.Wrap(), item.data().Clone()); max_size -= serialized_size; outstanding_bytes_ += serialized_size; + ++outstanding_items_; it = to_be_retransmitted_.erase(it); } else { ++it; @@ -624,6 +631,7 @@ std::vector> RetransmissionQueue::GetChunksToSend( size_t chunk_size = GetSerializedChunkSize(chunk_opt->data); max_bytes -= chunk_size; outstanding_bytes_ += chunk_size; + ++outstanding_items_; rwnd_ -= chunk_size; auto item_it = outstanding_data_ diff --git a/net/dcsctp/tx/retransmission_queue.h b/net/dcsctp/tx/retransmission_queue.h index c5a6a04db8..ceb4426606 100644 --- a/net/dcsctp/tx/retransmission_queue.h +++ b/net/dcsctp/tx/retransmission_queue.h @@ -115,6 +115,9 @@ class RetransmissionQueue { // Returns the number of bytes of packets that are in-flight. size_t outstanding_bytes() const { return outstanding_bytes_; } + // Returns the number of DATA chunks that are in-flight. + size_t outstanding_items() const { return outstanding_items_; } + // 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. @@ -381,6 +384,9 @@ class RetransmissionQueue { std::set to_be_retransmitted_; // The number of bytes that are in-flight (sent but not yet acked or nacked). size_t outstanding_bytes_ = 0; + // The number of DATA chunks that are in-flight (sent but not yet acked or + // nacked). + size_t outstanding_items_ = 0; }; } // namespace dcsctp diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc index 4aa76d66e5..a2ec3da4c2 100644 --- a/net/dcsctp/tx/retransmission_queue_test.cc +++ b/net/dcsctp/tx/retransmission_queue_test.cc @@ -455,6 +455,7 @@ TEST_F(RetransmissionQueueTest, RetransmitsWhenSendBufferIsFullT3Expiry) { queue.set_cwnd(kCwnd); EXPECT_EQ(queue.cwnd(), kCwnd); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); std::vector payload(1000); EXPECT_CALL(producer_, Produce) @@ -470,6 +471,7 @@ TEST_F(RetransmissionQueueTest, RetransmitsWhenSendBufferIsFullT3Expiry) { ElementsAre(Pair(TSN(9), State::kAcked), // Pair(TSN(10), State::kInFlight))); EXPECT_EQ(queue.outstanding_bytes(), payload.size() + DataChunk::kHeaderSize); + EXPECT_EQ(queue.outstanding_items(), 1u); // Will force chunks to be retransmitted queue.HandleT3RtxTimerExpiry(); @@ -478,6 +480,7 @@ TEST_F(RetransmissionQueueTest, RetransmitsWhenSendBufferIsFullT3Expiry) { ElementsAre(Pair(TSN(9), State::kAcked), // Pair(TSN(10), State::kToBeRetransmitted))); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); std::vector> chunks_to_rtx = queue.GetChunksToSend(now_, 1500); @@ -486,6 +489,7 @@ TEST_F(RetransmissionQueueTest, RetransmitsWhenSendBufferIsFullT3Expiry) { ElementsAre(Pair(TSN(9), State::kAcked), // Pair(TSN(10), State::kInFlight))); EXPECT_EQ(queue.outstanding_bytes(), payload.size() + DataChunk::kHeaderSize); + EXPECT_EQ(queue.outstanding_items(), 1u); } TEST_F(RetransmissionQueueTest, ProducesValidForwardTsn) { @@ -909,6 +913,7 @@ TEST_F(RetransmissionQueueTest, AccountsNackedAbandonedChunksAsNotOutstanding) { Pair(TSN(11), State::kInFlight), // Pair(TSN(12), State::kInFlight))); EXPECT_EQ(queue.outstanding_bytes(), (16 + 4) * 3u); + EXPECT_EQ(queue.outstanding_items(), 3u); // Mark the message as lost. EXPECT_CALL(producer_, Discard(IsUnordered(false), StreamID(1), MID(42))) @@ -923,16 +928,20 @@ TEST_F(RetransmissionQueueTest, AccountsNackedAbandonedChunksAsNotOutstanding) { Pair(TSN(11), State::kAbandoned), // Pair(TSN(12), State::kAbandoned))); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); // Now ACK those, one at a time. queue.HandleSack(now_, SackChunk(TSN(10), kArwnd, {}, {})); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); queue.HandleSack(now_, SackChunk(TSN(11), kArwnd, {}, {})); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); queue.HandleSack(now_, SackChunk(TSN(12), kArwnd, {}, {})); EXPECT_EQ(queue.outstanding_bytes(), 0u); + EXPECT_EQ(queue.outstanding_items(), 0u); } TEST_F(RetransmissionQueueTest, ExpireFromSendQueueWhenPartiallySent) {