dcsctp: Use TimeDelta in TX path
This commit replaces the internal use of DurationMs, with millisecond
precision, to webrtc::TimeDelta, which uses microsecond precision.
This is just a refactoring. The only change to the public API is
convenience methods to convert between DurationMs and webrtc::TimeDelta.
Bug: webrtc:15593
Change-Id: Ida861bf585c716be5f898d0e7ef98da2c15268b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325402
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41062}
diff --git a/net/dcsctp/public/BUILD.gn b/net/dcsctp/public/BUILD.gn
index 6cb289b..032ffe5 100644
--- a/net/dcsctp/public/BUILD.gn
+++ b/net/dcsctp/public/BUILD.gn
@@ -11,6 +11,7 @@
rtc_source_set("types") {
deps = [
"../../../api:array_view",
+ "../../../api/units:time_delta",
"../../../rtc_base:strong_alias",
]
sources = [
diff --git a/net/dcsctp/public/types.h b/net/dcsctp/public/types.h
index 7d69875..02e2ce1 100644
--- a/net/dcsctp/public/types.h
+++ b/net/dcsctp/public/types.h
@@ -14,6 +14,7 @@
#include <cstdint>
#include <limits>
+#include "api/units/time_delta.h"
#include "rtc_base/strong_alias.h"
namespace dcsctp {
@@ -41,6 +42,10 @@
constexpr explicit DurationMs(const UnderlyingType& v)
: webrtc::StrongAlias<class DurationMsTag, int32_t>(v) {}
+ constexpr explicit DurationMs(webrtc::TimeDelta v)
+ : webrtc::StrongAlias<class DurationMsTag, int32_t>(
+ v.IsInfinite() ? InfiniteDuration() : DurationMs(v.ms())) {}
+
static constexpr DurationMs InfiniteDuration() {
return DurationMs(std::numeric_limits<int32_t>::max());
}
@@ -58,6 +63,11 @@
value_ *= factor;
return *this;
}
+ constexpr webrtc::TimeDelta ToTimeDelta() const {
+ return *this == DurationMs::InfiniteDuration()
+ ? webrtc::TimeDelta::PlusInfinity()
+ : webrtc::TimeDelta::Millis(value_);
+ }
};
constexpr inline DurationMs operator+(DurationMs lhs, DurationMs rhs) {
diff --git a/net/dcsctp/socket/context.h b/net/dcsctp/socket/context.h
index eca5b9e..8823308 100644
--- a/net/dcsctp/socket/context.h
+++ b/net/dcsctp/socket/context.h
@@ -39,8 +39,8 @@
// Returns the socket callbacks.
virtual DcSctpSocketCallbacks& callbacks() const = 0;
- // Observes a measured RTT value, in milliseconds.
- virtual void ObserveRTT(DurationMs rtt_ms) = 0;
+ // Observes a measured RTT value.
+ virtual void ObserveRTT(webrtc::TimeDelta rtt_ms) = 0;
// Returns the current Retransmission Timeout (rto) value, in milliseconds.
virtual DurationMs current_rto() const = 0;
diff --git a/net/dcsctp/socket/heartbeat_handler.cc b/net/dcsctp/socket/heartbeat_handler.cc
index 8d1d04a..3e0437b 100644
--- a/net/dcsctp/socket/heartbeat_handler.cc
+++ b/net/dcsctp/socket/heartbeat_handler.cc
@@ -155,7 +155,7 @@
TimeMs now = ctx_->callbacks().TimeMillis();
if (info->created_at() > TimeMs(0) && info->created_at() <= now) {
- ctx_->ObserveRTT(now - info->created_at());
+ ctx_->ObserveRTT((now - info->created_at()).ToTimeDelta());
}
// https://tools.ietf.org/html/rfc4960#section-8.1
diff --git a/net/dcsctp/socket/heartbeat_handler_test.cc b/net/dcsctp/socket/heartbeat_handler_test.cc
index d573192..fc7cd52 100644
--- a/net/dcsctp/socket/heartbeat_handler_test.cc
+++ b/net/dcsctp/socket/heartbeat_handler_test.cc
@@ -30,6 +30,7 @@
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::SizeIs;
+using ::webrtc::TimeDelta;
constexpr DurationMs kHeartbeatInterval = DurationMs(30'000);
@@ -136,7 +137,7 @@
// Respond a while later. This RTT will be measured by the handler
constexpr DurationMs rtt(313);
- EXPECT_CALL(context_, ObserveRTT(rtt)).Times(1);
+ EXPECT_CALL(context_, ObserveRTT(rtt.ToTimeDelta())).Times(1);
callbacks_.AdvanceTime(rtt);
handler_.HandleHeartbeatAck(std::move(ack));
diff --git a/net/dcsctp/socket/mock_context.h b/net/dcsctp/socket/mock_context.h
index 88e71d1..7c5cd4e 100644
--- a/net/dcsctp/socket/mock_context.h
+++ b/net/dcsctp/socket/mock_context.h
@@ -51,7 +51,7 @@
MOCK_METHOD(TSN, peer_initial_tsn, (), (const, override));
MOCK_METHOD(DcSctpSocketCallbacks&, callbacks, (), (const, override));
- MOCK_METHOD(void, ObserveRTT, (DurationMs rtt_ms), (override));
+ MOCK_METHOD(void, ObserveRTT, (webrtc::TimeDelta rtt), (override));
MOCK_METHOD(DurationMs, current_rto, (), (const, override));
MOCK_METHOD(bool,
IncrementTxErrorCounter,
diff --git a/net/dcsctp/socket/stream_reset_handler_test.cc b/net/dcsctp/socket/stream_reset_handler_test.cc
index 4b12970..d459450 100644
--- a/net/dcsctp/socket/stream_reset_handler_test.cc
+++ b/net/dcsctp/socket/stream_reset_handler_test.cc
@@ -48,6 +48,7 @@
using ::testing::Return;
using ::testing::SizeIs;
using ::testing::UnorderedElementsAre;
+using ::webrtc::TimeDelta;
using ResponseResult = ReconfigurationResponseParameter::Result;
using SkippedStream = AnyForwardTsnChunk::SkippedStream;
@@ -115,7 +116,7 @@
kMyInitialTsn,
kArwnd,
producer_,
- [](DurationMs rtt_ms) {},
+ [](TimeDelta rtt) {},
[]() {},
*t3_rtx_timer_,
DcSctpOptions())),
@@ -205,7 +206,7 @@
reasm_->RestoreFromState(state);
retransmission_queue_ = std::make_unique<RetransmissionQueue>(
"", &callbacks_, kMyInitialTsn, kArwnd, producer_,
- [](DurationMs rtt_ms) {}, []() {}, *t3_rtx_timer_, DcSctpOptions(),
+ [](TimeDelta rtt) {}, []() {}, *t3_rtx_timer_, DcSctpOptions(),
/*supports_partial_reliability=*/true,
/*use_message_interleaving=*/false);
retransmission_queue_->RestoreFromState(state);
diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc
index c403b06..c1d3533 100644
--- a/net/dcsctp/socket/transmission_control_block.cc
+++ b/net/dcsctp/socket/transmission_control_block.cc
@@ -37,6 +37,7 @@
#include "rtc_base/strings/string_builder.h"
namespace dcsctp {
+using ::webrtc::TimeDelta;
TransmissionControlBlock::TransmissionControlBlock(
TimerManager& timer_manager,
@@ -112,10 +113,10 @@
send_queue.EnableMessageInterleaving(capabilities.message_interleaving);
}
-void TransmissionControlBlock::ObserveRTT(DurationMs rtt) {
+void TransmissionControlBlock::ObserveRTT(TimeDelta rtt) {
DurationMs prev_rto = rto_.rto();
- rto_.ObserveRTT(rtt);
- RTC_DLOG(LS_VERBOSE) << log_prefix_ << "new rtt=" << *rtt
+ rto_.ObserveRTT(DurationMs(rtt));
+ RTC_DLOG(LS_VERBOSE) << log_prefix_ << "new rtt=" << webrtc::ToString(rtt)
<< ", srtt=" << *rto_.srtt() << ", rto=" << *rto_.rto()
<< " (" << *prev_rto << ")";
t3_rtx_->set_duration(rto_.rto());
diff --git a/net/dcsctp/socket/transmission_control_block.h b/net/dcsctp/socket/transmission_control_block.h
index 240454a..8fda890 100644
--- a/net/dcsctp/socket/transmission_control_block.h
+++ b/net/dcsctp/socket/transmission_control_block.h
@@ -67,7 +67,7 @@
TSN my_initial_tsn() const override { return my_initial_tsn_; }
TSN peer_initial_tsn() const override { return peer_initial_tsn_; }
DcSctpSocketCallbacks& callbacks() const override { return callbacks_; }
- void ObserveRTT(DurationMs rtt) override;
+ void ObserveRTT(webrtc::TimeDelta rtt) override;
DurationMs current_rto() const override { return rto_.rto(); }
bool IncrementTxErrorCounter(absl::string_view reason) override {
return tx_error_counter_.Increment(reason);
diff --git a/net/dcsctp/tx/BUILD.gn b/net/dcsctp/tx/BUILD.gn
index 5547ffa..cd42768 100644
--- a/net/dcsctp/tx/BUILD.gn
+++ b/net/dcsctp/tx/BUILD.gn
@@ -103,6 +103,7 @@
":retransmission_timeout",
":send_queue",
"../../../api:array_view",
+ "../../../api/units:time_delta",
"../../../rtc_base:checks",
"../../../rtc_base:logging",
"../../../rtc_base/containers:flat_set",
diff --git a/net/dcsctp/tx/outstanding_data.cc b/net/dcsctp/tx/outstanding_data.cc
index c2706bd..2a92f14 100644
--- a/net/dcsctp/tx/outstanding_data.cc
+++ b/net/dcsctp/tx/outstanding_data.cc
@@ -14,6 +14,7 @@
#include <utility>
#include <vector>
+#include "api/units/time_delta.h"
#include "net/dcsctp/common/math.h"
#include "net/dcsctp/common/sequence_numbers.h"
#include "net/dcsctp/public/types.h"
@@ -441,8 +442,8 @@
RTC_DCHECK(IsConsistent());
}
-absl::optional<DurationMs> OutstandingData::MeasureRTT(TimeMs now,
- UnwrappedTSN tsn) const {
+webrtc::TimeDelta OutstandingData::MeasureRTT(TimeMs now,
+ UnwrappedTSN tsn) const {
auto it = outstanding_data_.find(tsn);
if (it != outstanding_data_.end() && !it->second.has_been_retransmitted()) {
// https://tools.ietf.org/html/rfc4960#section-6.3.1
@@ -450,9 +451,9 @@
// packets that were retransmitted (and thus for which it is ambiguous
// whether the reply was for the first instance of the chunk or for a
// later instance)"
- return now - it->second.time_sent();
+ return (now - it->second.time_sent()).ToTimeDelta();
}
- return absl::nullopt;
+ return webrtc::TimeDelta::PlusInfinity();
}
std::vector<std::pair<TSN, OutstandingData::State>>
diff --git a/net/dcsctp/tx/outstanding_data.h b/net/dcsctp/tx/outstanding_data.h
index f8e9396..323f2de 100644
--- a/net/dcsctp/tx/outstanding_data.h
+++ b/net/dcsctp/tx/outstanding_data.h
@@ -147,8 +147,8 @@
// Given the current time and a TSN, it returns the measured RTT between when
// the chunk was sent and now. It takes into acccount Karn's algorithm, so if
- // the chunk has ever been retransmitted, it will return absl::nullopt.
- absl::optional<DurationMs> MeasureRTT(TimeMs now, UnwrappedTSN tsn) const;
+ // the chunk has ever been retransmitted, it will return `PlusInfinity()`.
+ webrtc::TimeDelta MeasureRTT(TimeMs now, UnwrappedTSN tsn) const;
// Returns the internal state of all queued chunks. This is only used in
// unit-tests.
diff --git a/net/dcsctp/tx/outstanding_data_test.cc b/net/dcsctp/tx/outstanding_data_test.cc
index b8c2e59..e119a56 100644
--- a/net/dcsctp/tx/outstanding_data_test.cc
+++ b/net/dcsctp/tx/outstanding_data_test.cc
@@ -37,6 +37,7 @@
using ::testing::Return;
using ::testing::StrictMock;
using ::testing::UnorderedElementsAre;
+using ::webrtc::TimeDelta;
constexpr TimeMs kNow(42);
constexpr OutgoingMessageId kMessageId = OutgoingMessageId(17);
@@ -365,12 +366,11 @@
buf_.Insert(kMessageId, gen_.Ordered({1}, "BE"), kNow + DurationMs(1));
buf_.Insert(kMessageId, gen_.Ordered({1}, "BE"), kNow + DurationMs(2));
- static constexpr DurationMs kDuration(123);
- ASSERT_HAS_VALUE_AND_ASSIGN(
- DurationMs duration,
- buf_.MeasureRTT(kNow + kDuration, unwrapper_.Unwrap(TSN(11))));
+ static constexpr TimeDelta kDuration = TimeDelta::Millis(123);
+ TimeDelta duration =
+ buf_.MeasureRTT(kNow + DurationMs(kDuration), unwrapper_.Unwrap(TSN(11)));
- EXPECT_EQ(duration, kDuration - DurationMs(1));
+ EXPECT_EQ(duration, kDuration - TimeDelta::Millis(1));
}
TEST_F(OutstandingDataTest, MustRetransmitBeforeGettingNackedAgain) {
diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc
index 2b9843f..2b166f1 100644
--- a/net/dcsctp/tx/retransmission_queue.cc
+++ b/net/dcsctp/tx/retransmission_queue.cc
@@ -44,6 +44,7 @@
namespace dcsctp {
namespace {
+using ::webrtc::TimeDelta;
// Allow sending only slightly less than an MTU, to account for headers.
constexpr float kMinBytesRequiredToSendFactor = 0.9;
@@ -55,7 +56,7 @@
TSN my_initial_tsn,
size_t a_rwnd,
SendQueue& send_queue,
- std::function<void(DurationMs rtt)> on_new_rtt,
+ std::function<void(TimeDelta rtt)> on_new_rtt,
std::function<void()> on_clear_retransmission_counter,
Timer& t3_rtx,
const DcSctpOptions& options,
@@ -345,11 +346,10 @@
// TODO(boivie): Consider occasionally sending DATA chunks with I-bit set and
// use only those packets for measurement.
- absl::optional<DurationMs> rtt =
- outstanding_data_.MeasureRTT(now, cumulative_tsn_ack);
+ TimeDelta rtt = outstanding_data_.MeasureRTT(now, cumulative_tsn_ack);
- if (rtt.has_value()) {
- on_new_rtt_(*rtt);
+ if (rtt.IsFinite()) {
+ on_new_rtt_(rtt);
}
}
diff --git a/net/dcsctp/tx/retransmission_queue.h b/net/dcsctp/tx/retransmission_queue.h
index b44db2a..d4c6edf 100644
--- a/net/dcsctp/tx/retransmission_queue.h
+++ b/net/dcsctp/tx/retransmission_queue.h
@@ -60,7 +60,7 @@
TSN my_initial_tsn,
size_t a_rwnd,
SendQueue& send_queue,
- std::function<void(DurationMs rtt)> on_new_rtt,
+ std::function<void(webrtc::TimeDelta rtt)> on_new_rtt,
std::function<void()> on_clear_retransmission_counter,
Timer& t3_rtx,
const DcSctpOptions& options,
@@ -230,7 +230,7 @@
// The size of the data chunk (DATA/I-DATA) header that is used.
const size_t data_chunk_header_size_;
// Called when a new RTT measurement has been done
- const std::function<void(DurationMs rtt)> on_new_rtt_;
+ const std::function<void(webrtc::TimeDelta rtt)> on_new_rtt_;
// Called when a SACK has been seen that cleared the retransmission counter.
const std::function<void()> on_clear_retransmission_counter_;
// The retransmission counter.
diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc
index 096bb12..d7b39e50 100644
--- a/net/dcsctp/tx/retransmission_queue_test.cc
+++ b/net/dcsctp/tx/retransmission_queue_test.cc
@@ -52,6 +52,7 @@
using ::testing::Return;
using ::testing::SizeIs;
using ::testing::UnorderedElementsAre;
+using ::webrtc::TimeDelta;
constexpr uint32_t kArwnd = 100000;
constexpr uint32_t kMaxMtu = 1191;
@@ -130,7 +131,7 @@
TimeMs now_ = TimeMs(0);
FakeTimeoutManager timeout_manager_;
TimerManager timer_manager_;
- NiceMock<MockFunction<void(DurationMs rtt_ms)>> on_rtt_;
+ NiceMock<MockFunction<void(TimeDelta rtt_ms)>> on_rtt_;
NiceMock<MockFunction<void()>> on_clear_retransmission_counter_;
NiceMock<MockSendQueue> producer_;
std::unique_ptr<Timer> timer_;
@@ -864,7 +865,7 @@
now_ = now_ + DurationMs(123);
- EXPECT_CALL(on_rtt_, Call(DurationMs(123))).Times(1);
+ EXPECT_CALL(on_rtt_, Call(TimeDelta::Millis(123))).Times(1);
queue.HandleSack(now_, SackChunk(TSN(10), kArwnd, {}, {}));
}
