dcsctp: Migrate non-Timer related to rtc::TimeDelta

This does the bulk of the remaining refactoring, except timers since
they are an even bigger part - but more isolated.

Bug: webrtc:15593
Change-Id: I7afa349e2119be7592797ee6b3b198e6de4f697a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/326160
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41090}
This commit is contained in:
Victor Boivie 2023-10-26 14:22:39 +02:00 committed by WebRTC LUCI CQ
parent 0a33589db1
commit be04c98d64
9 changed files with 34 additions and 31 deletions

View file

@ -43,7 +43,7 @@ class Context {
virtual void ObserveRTT(webrtc::TimeDelta rtt_ms) = 0; virtual void ObserveRTT(webrtc::TimeDelta rtt_ms) = 0;
// Returns the current Retransmission Timeout (rto) value, in milliseconds. // Returns the current Retransmission Timeout (rto) value, in milliseconds.
virtual DurationMs current_rto() const = 0; virtual webrtc::TimeDelta current_rto() const = 0;
// Increments the transmission error counter, given a human readable reason. // Increments the transmission error counter, given a human readable reason.
virtual bool IncrementTxErrorCounter(absl::string_view reason) = 0; virtual bool IncrementTxErrorCounter(absl::string_view reason) = 0;

View file

@ -600,7 +600,7 @@ absl::optional<Metrics> DcSctpSocket::GetMetrics() const {
Metrics metrics = metrics_; Metrics metrics = metrics_;
metrics.cwnd_bytes = tcb_->cwnd(); metrics.cwnd_bytes = tcb_->cwnd();
metrics.srtt_ms = tcb_->current_srtt().value(); metrics.srtt_ms = tcb_->current_srtt().ms();
size_t packet_payload_size = size_t packet_payload_size =
options_.mtu - SctpPacket::kHeaderSize - DataChunk::kHeaderSize; options_.mtu - SctpPacket::kHeaderSize - DataChunk::kHeaderSize;
metrics.unack_data_count = metrics.unack_data_count =
@ -631,7 +631,7 @@ void DcSctpSocket::MaybeSendShutdownOnPacketReceived(const SctpPacket& packet) {
// respond to each received packet containing one or more DATA chunks with // respond to each received packet containing one or more DATA chunks with
// a SHUTDOWN chunk and restart the T2-shutdown timer."" // a SHUTDOWN chunk and restart the T2-shutdown timer.""
SendShutdown(); SendShutdown();
t2_shutdown_->set_duration(tcb_->current_rto()); t2_shutdown_->set_duration(DurationMs(tcb_->current_rto()));
t2_shutdown_->Start(); t2_shutdown_->Start();
} }
} }
@ -988,7 +988,7 @@ DurationMs DcSctpSocket::OnShutdownTimerExpiry() {
// updated last sequential TSN received from its peer." // updated last sequential TSN received from its peer."
SendShutdown(); SendShutdown();
RTC_DCHECK(IsConsistent()); RTC_DCHECK(IsConsistent());
return tcb_->current_rto(); return DurationMs(tcb_->current_rto());
} }
void DcSctpSocket::OnSentPacket(rtc::ArrayView<const uint8_t> packet, void DcSctpSocket::OnSentPacket(rtc::ArrayView<const uint8_t> packet,
@ -1731,7 +1731,7 @@ void DcSctpSocket::MaybeSendShutdownOrAck() {
// state."" // state.""
SendShutdown(); SendShutdown();
t2_shutdown_->set_duration(tcb_->current_rto()); t2_shutdown_->set_duration(DurationMs(tcb_->current_rto()));
t2_shutdown_->Start(); t2_shutdown_->Start();
SetState(State::kShutdownSent, "No more outstanding data"); SetState(State::kShutdownSent, "No more outstanding data");
} else if (state_ == State::kShutdownReceived) { } else if (state_ == State::kShutdownReceived) {
@ -1754,7 +1754,7 @@ void DcSctpSocket::SendShutdown() {
void DcSctpSocket::SendShutdownAck() { void DcSctpSocket::SendShutdownAck() {
packet_sender_.Send(tcb_->PacketBuilder().Add(ShutdownAckChunk())); packet_sender_.Send(tcb_->PacketBuilder().Add(ShutdownAckChunk()));
t2_shutdown_->set_duration(tcb_->current_rto()); t2_shutdown_->set_duration(DurationMs(tcb_->current_rto()));
t2_shutdown_->Start(); t2_shutdown_->Start();
} }

View file

@ -91,13 +91,14 @@ HeartbeatHandler::HeartbeatHandler(absl::string_view log_prefix,
: log_prefix_(log_prefix), : log_prefix_(log_prefix),
ctx_(context), ctx_(context),
timer_manager_(timer_manager), timer_manager_(timer_manager),
interval_duration_(options.heartbeat_interval), interval_duration_(options.heartbeat_interval.ToTimeDelta()),
interval_duration_should_include_rtt_( interval_duration_should_include_rtt_(
options.heartbeat_interval_include_rtt), options.heartbeat_interval_include_rtt),
interval_timer_(timer_manager_->CreateTimer( interval_timer_(timer_manager_->CreateTimer(
"heartbeat-interval", "heartbeat-interval",
absl::bind_front(&HeartbeatHandler::OnIntervalTimerExpiry, this), absl::bind_front(&HeartbeatHandler::OnIntervalTimerExpiry, this),
TimerOptions(interval_duration_, TimerBackoffAlgorithm::kFixed))), TimerOptions(DurationMs(interval_duration_),
TimerBackoffAlgorithm::kFixed))),
timeout_timer_(timer_manager_->CreateTimer( timeout_timer_(timer_manager_->CreateTimer(
"heartbeat-timeout", "heartbeat-timeout",
absl::bind_front(&HeartbeatHandler::OnTimeoutTimerExpiry, this), absl::bind_front(&HeartbeatHandler::OnTimeoutTimerExpiry, this),
@ -109,7 +110,7 @@ HeartbeatHandler::HeartbeatHandler(absl::string_view log_prefix,
} }
void HeartbeatHandler::RestartTimer() { void HeartbeatHandler::RestartTimer() {
if (interval_duration_ == DurationMs(0)) { if (interval_duration_.IsZero()) {
// Heartbeating has been disabled. // Heartbeating has been disabled.
return; return;
} }
@ -117,9 +118,10 @@ void HeartbeatHandler::RestartTimer() {
if (interval_duration_should_include_rtt_) { if (interval_duration_should_include_rtt_) {
// The RTT should be used, but it's not easy accessible. The RTO will // The RTT should be used, but it's not easy accessible. The RTO will
// suffice. // suffice.
interval_timer_->set_duration(interval_duration_ + ctx_->current_rto()); interval_timer_->set_duration(
DurationMs(interval_duration_ + ctx_->current_rto()));
} else { } else {
interval_timer_->set_duration(interval_duration_); interval_timer_->set_duration(DurationMs(interval_duration_));
} }
interval_timer_->Start(); interval_timer_->Start();
@ -167,7 +169,7 @@ void HeartbeatHandler::HandleHeartbeatAck(HeartbeatAckChunk chunk) {
DurationMs HeartbeatHandler::OnIntervalTimerExpiry() { DurationMs HeartbeatHandler::OnIntervalTimerExpiry() {
if (ctx_->is_connection_established()) { if (ctx_->is_connection_established()) {
HeartbeatInfo info(ctx_->callbacks().TimeMillis()); HeartbeatInfo info(ctx_->callbacks().TimeMillis());
timeout_timer_->set_duration(ctx_->current_rto()); timeout_timer_->set_duration(DurationMs(ctx_->current_rto()));
timeout_timer_->Start(); timeout_timer_->Start();
RTC_DLOG(LS_INFO) << log_prefix_ << "Sending HEARTBEAT with timeout " RTC_DLOG(LS_INFO) << log_prefix_ << "Sending HEARTBEAT with timeout "
<< *timeout_timer_->duration(); << *timeout_timer_->duration();

View file

@ -57,7 +57,7 @@ class HeartbeatHandler {
Context* ctx_; Context* ctx_;
TimerManager* timer_manager_; TimerManager* timer_manager_;
// The time for a connection to be idle before a heartbeat is sent. // The time for a connection to be idle before a heartbeat is sent.
const DurationMs interval_duration_; const webrtc::TimeDelta interval_duration_;
// Adding RTT to the duration will add some jitter, which is good in // Adding RTT to the duration will add some jitter, which is good in
// production, but less good in unit tests, which is why it can be disabled. // production, but less good in unit tests, which is why it can be disabled.
const bool interval_duration_should_include_rtt_; const bool interval_duration_should_include_rtt_;

View file

@ -52,8 +52,8 @@ class HeartbeatHandlerTestBase : public testing::Test {
}), }),
handler_("log: ", options_, &context_, &timer_manager_) {} handler_("log: ", options_, &context_, &timer_manager_) {}
void AdvanceTime(DurationMs duration) { void AdvanceTime(webrtc::TimeDelta duration) {
callbacks_.AdvanceTime(duration); callbacks_.AdvanceTime(DurationMs(duration));
for (;;) { for (;;) {
absl::optional<TimeoutID> timeout_id = callbacks_.GetNextExpiredTimeout(); absl::optional<TimeoutID> timeout_id = callbacks_.GetNextExpiredTimeout();
if (!timeout_id.has_value()) { if (!timeout_id.has_value()) {
@ -81,7 +81,7 @@ class DisabledHeartbeatHandlerTest : public HeartbeatHandlerTestBase {
}; };
TEST_F(HeartbeatHandlerTest, HasRunningHeartbeatIntervalTimer) { TEST_F(HeartbeatHandlerTest, HasRunningHeartbeatIntervalTimer) {
AdvanceTime(options_.heartbeat_interval); AdvanceTime(options_.heartbeat_interval.ToTimeDelta());
// Validate that a heartbeat request was sent. // Validate that a heartbeat request was sent.
std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket(); std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket();
@ -120,7 +120,7 @@ TEST_F(HeartbeatHandlerTest, RepliesToHeartbeatRequests) {
} }
TEST_F(HeartbeatHandlerTest, SendsHeartbeatRequestsOnIdleChannel) { TEST_F(HeartbeatHandlerTest, SendsHeartbeatRequestsOnIdleChannel) {
AdvanceTime(options_.heartbeat_interval); AdvanceTime(options_.heartbeat_interval.ToTimeDelta());
// Grab the request, and make a response. // Grab the request, and make a response.
std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket(); std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket();
@ -144,7 +144,7 @@ TEST_F(HeartbeatHandlerTest, SendsHeartbeatRequestsOnIdleChannel) {
} }
TEST_F(HeartbeatHandlerTest, DoesntObserveInvalidHeartbeats) { TEST_F(HeartbeatHandlerTest, DoesntObserveInvalidHeartbeats) {
AdvanceTime(options_.heartbeat_interval); AdvanceTime(options_.heartbeat_interval.ToTimeDelta());
// Grab the request, and make a response. // Grab the request, and make a response.
std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket(); std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket();
@ -168,9 +168,9 @@ TEST_F(HeartbeatHandlerTest, DoesntObserveInvalidHeartbeats) {
} }
TEST_F(HeartbeatHandlerTest, IncreasesErrorIfNotAckedInTime) { TEST_F(HeartbeatHandlerTest, IncreasesErrorIfNotAckedInTime) {
DurationMs rto(105); TimeDelta rto = TimeDelta::Millis(105);
EXPECT_CALL(context_, current_rto).WillOnce(Return(rto)); EXPECT_CALL(context_, current_rto).WillOnce(Return(rto));
AdvanceTime(options_.heartbeat_interval); AdvanceTime(options_.heartbeat_interval.ToTimeDelta());
// Validate that a request was sent. // Validate that a request was sent.
EXPECT_THAT(callbacks_.ConsumeSentPacket(), Not(IsEmpty())); EXPECT_THAT(callbacks_.ConsumeSentPacket(), Not(IsEmpty()));
@ -180,7 +180,7 @@ TEST_F(HeartbeatHandlerTest, IncreasesErrorIfNotAckedInTime) {
} }
TEST_F(DisabledHeartbeatHandlerTest, IsReallyDisabled) { TEST_F(DisabledHeartbeatHandlerTest, IsReallyDisabled) {
AdvanceTime(options_.heartbeat_interval); AdvanceTime(options_.heartbeat_interval.ToTimeDelta());
// Validate that a request was NOT sent. // Validate that a request was NOT sent.
EXPECT_THAT(callbacks_.ConsumeSentPacket(), IsEmpty()); EXPECT_THAT(callbacks_.ConsumeSentPacket(), IsEmpty());

View file

@ -40,7 +40,8 @@ class MockContext : public Context {
ON_CALL(*this, peer_initial_tsn) ON_CALL(*this, peer_initial_tsn)
.WillByDefault(testing::Return(PeerInitialTsn())); .WillByDefault(testing::Return(PeerInitialTsn()));
ON_CALL(*this, callbacks).WillByDefault(testing::ReturnRef(callbacks_)); ON_CALL(*this, callbacks).WillByDefault(testing::ReturnRef(callbacks_));
ON_CALL(*this, current_rto).WillByDefault(testing::Return(DurationMs(123))); ON_CALL(*this, current_rto)
.WillByDefault(testing::Return(webrtc::TimeDelta::Millis(123)));
ON_CALL(*this, Send).WillByDefault([this](SctpPacket::Builder& builder) { ON_CALL(*this, Send).WillByDefault([this](SctpPacket::Builder& builder) {
callbacks_.SendPacketWithStatus(builder.Build()); callbacks_.SendPacketWithStatus(builder.Build());
}); });
@ -52,7 +53,7 @@ class MockContext : public Context {
MOCK_METHOD(DcSctpSocketCallbacks&, callbacks, (), (const, override)); MOCK_METHOD(DcSctpSocketCallbacks&, callbacks, (), (const, override));
MOCK_METHOD(void, ObserveRTT, (webrtc::TimeDelta rtt), (override)); MOCK_METHOD(void, ObserveRTT, (webrtc::TimeDelta rtt), (override));
MOCK_METHOD(DurationMs, current_rto, (), (const, override)); MOCK_METHOD(webrtc::TimeDelta, current_rto, (), (const, override));
MOCK_METHOD(bool, MOCK_METHOD(bool,
IncrementTxErrorCounter, IncrementTxErrorCounter,
(absl::string_view reason), (absl::string_view reason),

View file

@ -277,7 +277,7 @@ void StreamResetHandler::HandleResponse(const ParameterDescriptor& descriptor) {
}); });
// Force this request to be sent again, but with new req_seq_nbr. // Force this request to be sent again, but with new req_seq_nbr.
current_request_->PrepareRetransmission(); current_request_->PrepareRetransmission();
reconfig_timer_->set_duration(ctx_->current_rto()); reconfig_timer_->set_duration(DurationMs(ctx_->current_rto()));
reconfig_timer_->Start(); reconfig_timer_->Start();
break; break;
case ResponseResult::kErrorRequestAlreadyInProgress: case ResponseResult::kErrorRequestAlreadyInProgress:
@ -312,7 +312,7 @@ absl::optional<ReConfigChunk> StreamResetHandler::MakeStreamResetRequest() {
current_request_.emplace(retransmission_queue_->last_assigned_tsn(), current_request_.emplace(retransmission_queue_->last_assigned_tsn(),
retransmission_queue_->BeginResetStreams()); retransmission_queue_->BeginResetStreams());
reconfig_timer_->set_duration(ctx_->current_rto()); reconfig_timer_->set_duration(DurationMs(ctx_->current_rto()));
reconfig_timer_->Start(); reconfig_timer_->Start();
return MakeReconfigChunk(); return MakeReconfigChunk();
} }
@ -362,7 +362,7 @@ DurationMs StreamResetHandler::OnReconfigTimerExpiry() {
} }
ctx_->Send(ctx_->PacketBuilder().Add(MakeReconfigChunk())); ctx_->Send(ctx_->PacketBuilder().Add(MakeReconfigChunk()));
return ctx_->current_rto(); return DurationMs(ctx_->current_rto());
} }
HandoverReadinessStatus StreamResetHandler::GetHandoverReadiness() const { HandoverReadinessStatus StreamResetHandler::GetHandoverReadiness() const {

View file

@ -58,7 +58,7 @@ constexpr TSN kPeerInitialTsn = MockContext::PeerInitialTsn();
constexpr ReconfigRequestSN kPeerInitialReqSn = constexpr ReconfigRequestSN kPeerInitialReqSn =
ReconfigRequestSN(*kPeerInitialTsn); ReconfigRequestSN(*kPeerInitialTsn);
constexpr uint32_t kArwnd = 131072; constexpr uint32_t kArwnd = 131072;
constexpr DurationMs kRto = DurationMs(250); constexpr TimeDelta kRto = TimeDelta::Millis(250);
constexpr std::array<uint8_t, 4> kShortPayload = {1, 2, 3, 4}; constexpr std::array<uint8_t, 4> kShortPayload = {1, 2, 3, 4};
@ -131,7 +131,7 @@ class StreamResetHandlerTest : public testing::Test {
} }
void AdvanceTime(DurationMs duration) { void AdvanceTime(DurationMs duration) {
callbacks_.AdvanceTime(kRto); callbacks_.AdvanceTime(DurationMs(kRto));
for (;;) { for (;;) {
absl::optional<TimeoutID> timeout_id = callbacks_.GetNextExpiredTimeout(); absl::optional<TimeoutID> timeout_id = callbacks_.GetNextExpiredTimeout();
if (!timeout_id.has_value()) { if (!timeout_id.has_value()) {
@ -630,7 +630,7 @@ TEST_F(StreamResetHandlerTest, SendOutgoingResetRetransmitOnInProgress) {
// Let some time pass, so that the reconfig timer expires, and retries the // Let some time pass, so that the reconfig timer expires, and retries the
// same request. // same request.
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(1); EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(1);
AdvanceTime(kRto); AdvanceTime(DurationMs(kRto));
std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket(); std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket();
ASSERT_FALSE(payload.empty()); ASSERT_FALSE(payload.empty());

View file

@ -68,7 +68,7 @@ class TransmissionControlBlock : public Context {
TSN peer_initial_tsn() const override { return peer_initial_tsn_; } TSN peer_initial_tsn() const override { return peer_initial_tsn_; }
DcSctpSocketCallbacks& callbacks() const override { return callbacks_; } DcSctpSocketCallbacks& callbacks() const override { return callbacks_; }
void ObserveRTT(webrtc::TimeDelta rtt) override; void ObserveRTT(webrtc::TimeDelta rtt) override;
DurationMs current_rto() const override { return DurationMs(rto_.rto()); } webrtc::TimeDelta current_rto() const override { return rto_.rto(); }
bool IncrementTxErrorCounter(absl::string_view reason) override { bool IncrementTxErrorCounter(absl::string_view reason) override {
return tx_error_counter_.Increment(reason); return tx_error_counter_.Increment(reason);
} }
@ -91,7 +91,7 @@ class TransmissionControlBlock : public Context {
StreamResetHandler& stream_reset_handler() { return stream_reset_handler_; } StreamResetHandler& stream_reset_handler() { return stream_reset_handler_; }
HeartbeatHandler& heartbeat_handler() { return heartbeat_handler_; } HeartbeatHandler& heartbeat_handler() { return heartbeat_handler_; }
size_t cwnd() const { return retransmission_queue_.cwnd(); } size_t cwnd() const { return retransmission_queue_.cwnd(); }
DurationMs current_srtt() const { return DurationMs(rto_.srtt()); } webrtc::TimeDelta current_srtt() const { return rto_.srtt(); }
// Returns this socket's verification tag, set in all packet headers. // Returns this socket's verification tag, set in all packet headers.
VerificationTag my_verification_tag() const { return my_verification_tag_; } VerificationTag my_verification_tag() const { return my_verification_tag_; }