dcsctp: Remove deprecated delivery checks
With https://webrtc-review.googlesource.com/c/src/+/321603, the
responsibility to not ingest duplicate received chunks was moved from
the reassembly queue to the data tracker. But in that CL, we couldn't
remove updating the internal variables in the reassembly queue, because
those were included in the handover state. Now that time has passed,
we can remove this code altogether as nothing was ever reading from
these variables - only writing to them.
Bug: webrtc:14600
Change-Id: Icf958c75f74974be6cad7cd827cf49b3ab2f5412
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329300
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41291}
diff --git a/net/dcsctp/rx/data_tracker_test.cc b/net/dcsctp/rx/data_tracker_test.cc
index f4e1c64..0e9e4fc 100644
--- a/net/dcsctp/rx/data_tracker_test.cc
+++ b/net/dcsctp/rx/data_tracker_test.cc
@@ -786,5 +786,16 @@
EXPECT_FALSE(tracker_->Observe(TSN(12)));
}
+TEST_F(DataTrackerTest, NotReadyForHandoverWhenHavingTsnGaps) {
+ tracker_->Observe(TSN(10));
+ tracker_->Observe(TSN(12));
+ EXPECT_EQ(tracker_->GetHandoverReadiness(),
+ HandoverReadinessStatus().Add(
+ HandoverUnreadinessReason::kDataTrackerTsnBlocksPending));
+
+ tracker_->Observe(TSN(11));
+ EXPECT_EQ(tracker_->GetHandoverReadiness(), HandoverReadinessStatus());
+}
+
} // namespace
} // namespace dcsctp
diff --git a/net/dcsctp/rx/reassembly_queue.cc b/net/dcsctp/rx/reassembly_queue.cc
index ae67273..4c223d0 100644
--- a/net/dcsctp/rx/reassembly_queue.cc
+++ b/net/dcsctp/rx/reassembly_queue.cc
@@ -57,8 +57,6 @@
: log_prefix_(log_prefix),
max_size_bytes_(max_size_bytes),
watermark_bytes_(max_size_bytes * kHighWatermarkLimit),
- last_assembled_tsn_watermark_(
- tsn_unwrapper_.Unwrap(TSN(*peer_initial_tsn - 1))),
last_completed_reset_req_seq_nbr_(ReconfigRequestSN(0)),
streams_(CreateStreams(
log_prefix_,
@@ -180,33 +178,9 @@
<< ", ppid=" << *message.ppid()
<< ", payload=" << message.payload().size() << " bytes";
- for (const UnwrappedTSN tsn : tsns) {
- if (tsn == last_assembled_tsn_watermark_.next_value()) {
- // Update watermark, or insert into delivered_tsns_
- last_assembled_tsn_watermark_.Increment();
- } else {
- delivered_tsns_.insert(tsn);
- }
- }
-
- // With new TSNs in delivered_tsns, gaps might be filled.
- MaybeMoveLastAssembledWatermarkFurther();
-
reassembled_messages_.emplace_back(std::move(message));
}
-void ReassemblyQueue::MaybeMoveLastAssembledWatermarkFurther() {
- // `delivered_tsns_` contain TSNS when there is a gap between ranges of
- // assembled TSNs. `last_assembled_tsn_watermark_` should not be adjacent to
- // that list, because if so, it can be moved.
- while (!delivered_tsns_.empty() &&
- *delivered_tsns_.begin() ==
- last_assembled_tsn_watermark_.next_value()) {
- last_assembled_tsn_watermark_.Increment();
- delivered_tsns_.erase(delivered_tsns_.begin());
- }
-}
-
void ReassemblyQueue::HandleForwardTsn(
TSN new_cumulative_tsn,
rtc::ArrayView<const AnyForwardTsnChunk::SkippedStream> skipped_streams) {
@@ -228,33 +202,19 @@
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "ForwardTSN to " << *tsn.Wrap()
<< " - performing.";
- last_assembled_tsn_watermark_ = std::max(last_assembled_tsn_watermark_, tsn);
- delivered_tsns_.erase(delivered_tsns_.begin(),
- delivered_tsns_.upper_bound(tsn));
- MaybeMoveLastAssembledWatermarkFurther();
queued_bytes_ -= streams_->HandleForwardTsn(tsn, skipped_streams);
RTC_DCHECK(IsConsistent());
}
bool ReassemblyQueue::IsConsistent() const {
- // `delivered_tsns_` and `last_assembled_tsn_watermark_` mustn't overlap or be
- // adjacent.
- if (!delivered_tsns_.empty() &&
- last_assembled_tsn_watermark_.next_value() >= *delivered_tsns_.begin()) {
- return false;
- }
-
// Allow queued_bytes_ to be larger than max_size_bytes, as it's not actively
- // enforced in this class. This comparison will still trigger if queued_bytes_
- // became "negative".
- return (queued_bytes_ >= 0 && queued_bytes_ <= 2 * max_size_bytes_);
+ // enforced in this class. But in case it wraps around (becomes negative, but
+ // as it's unsigned, that would wrap to very big), this would trigger.
+ return (queued_bytes_ <= 2 * max_size_bytes_);
}
HandoverReadinessStatus ReassemblyQueue::GetHandoverReadiness() const {
HandoverReadinessStatus status = streams_->GetHandoverReadiness();
- if (!delivered_tsns_.empty()) {
- status.Add(HandoverUnreadinessReason::kReassemblyQueueDeliveredTSNsGap);
- }
if (deferred_reset_streams_.has_value()) {
status.Add(HandoverUnreadinessReason::kStreamResetDeferred);
}
@@ -262,7 +222,6 @@
}
void ReassemblyQueue::AddHandoverState(DcSctpSocketHandoverState& state) {
- state.rx.last_assembled_tsn = last_assembled_tsn_watermark_.Wrap().value();
state.rx.last_completed_deferred_reset_req_sn =
last_completed_reset_req_seq_nbr_.value();
streams_->AddHandoverState(state);
@@ -272,8 +231,6 @@
// Validate that the component is in pristine state.
RTC_DCHECK(last_completed_reset_req_seq_nbr_ == ReconfigRequestSN(0));
- last_assembled_tsn_watermark_ =
- tsn_unwrapper_.Unwrap(TSN(state.rx.last_assembled_tsn));
last_completed_reset_req_seq_nbr_ =
ReconfigRequestSN(state.rx.last_completed_deferred_reset_req_sn);
streams_->RestoreFromState(state);
diff --git a/net/dcsctp/rx/reassembly_queue.h b/net/dcsctp/rx/reassembly_queue.h
index 761ec35..82a66d0 100644
--- a/net/dcsctp/rx/reassembly_queue.h
+++ b/net/dcsctp/rx/reassembly_queue.h
@@ -142,19 +142,12 @@
bool IsConsistent() const;
void AddReassembledMessage(rtc::ArrayView<const UnwrappedTSN> tsns,
DcSctpMessage message);
- void MaybeMoveLastAssembledWatermarkFurther();
const absl::string_view log_prefix_;
const size_t max_size_bytes_;
const size_t watermark_bytes_;
UnwrappedTSN::Unwrapper tsn_unwrapper_;
- // Whenever a message has been assembled, either increase
- // `last_assembled_tsn_watermark_` or - if there are gaps - add the message's
- // TSNs into delivered_tsns_ so that messages are not re-delivered on
- // duplicate chunks.
- UnwrappedTSN last_assembled_tsn_watermark_;
- std::set<UnwrappedTSN> delivered_tsns_;
// Messages that have been reassembled, and will be returned by
// `FlushMessages`.
std::vector<DcSctpMessage> reassembled_messages_;
diff --git a/net/dcsctp/rx/reassembly_queue_test.cc b/net/dcsctp/rx/reassembly_queue_test.cc
index fd8c423..81bb7af 100644
--- a/net/dcsctp/rx/reassembly_queue_test.cc
+++ b/net/dcsctp/rx/reassembly_queue_test.cc
@@ -252,33 +252,6 @@
ElementsAre(SctpMessageIs(kStreamID, kPPID, kMessage2Payload)));
}
-TEST_F(ReassemblyQueueTest, NotReadyForHandoverWhenDeliveredTsnsHaveGap) {
- ReassemblyQueue reasm("log: ", TSN(10), kBufferSize);
- reasm.Add(TSN(10), gen_.Unordered({1, 2, 3, 4}, "B"));
- EXPECT_FALSE(reasm.HasMessages());
-
- reasm.Add(TSN(12), gen_.Unordered({1, 2, 3, 4}, "BE"));
- EXPECT_TRUE(reasm.HasMessages());
- EXPECT_EQ(
- reasm.GetHandoverReadiness(),
- HandoverReadinessStatus()
- .Add(HandoverUnreadinessReason::kReassemblyQueueDeliveredTSNsGap)
- .Add(
- HandoverUnreadinessReason::kUnorderedStreamHasUnassembledChunks));
-
- EXPECT_THAT(reasm.FlushMessages(),
- ElementsAre(SctpMessageIs(kStreamID, kPPID, kShortPayload)));
- EXPECT_EQ(
- reasm.GetHandoverReadiness(),
- HandoverReadinessStatus()
- .Add(HandoverUnreadinessReason::kReassemblyQueueDeliveredTSNsGap)
- .Add(
- HandoverUnreadinessReason::kUnorderedStreamHasUnassembledChunks));
-
- reasm.HandleForwardTsn(TSN(13), std::vector<SkippedStream>());
- EXPECT_EQ(reasm.GetHandoverReadiness(), HandoverReadinessStatus());
-}
-
TEST_F(ReassemblyQueueTest, NotReadyForHandoverWhenResetStreamIsDeferred) {
ReassemblyQueue reasm("log: ", TSN(10), kBufferSize);
reasm.Add(TSN(10), gen_.Ordered({1, 2, 3, 4}, "BE", {.mid = MID(0)}));
