From f3e9db9e1793e4163ca2fd835b35af649d7a79c7 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Fri, 20 Oct 2023 12:51:36 +0200 Subject: [PATCH] dcsctp: Use InfiniteDuration for no max duration Before this change, a timer could have an optional max duration. Either that value was present, and that limited the max duration of the timer, or it was absl::nullopt, which represented "no limit". To simplify the interface, this CL makes that value "not optional" by having it always present. The previous "no limit" is now represented by DurationMs::InfiniteDuration. This is just a refactoring of internal interfaces - public interfaces are left untouched. Bug: webrtc:15593 Change-Id: I80df1d9b2f4d208411ce6cb5045db0a57865e3b4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325280 Reviewed-by: Florent Castelli Commit-Queue: Victor Boivie Cr-Commit-Position: refs/heads/main@{#41040} --- net/dcsctp/socket/transmission_control_block.cc | 7 +++++-- net/dcsctp/timer/timer.cc | 13 ++++++------- net/dcsctp/timer/timer.h | 12 +++++++----- net/dcsctp/timer/timer_test.cc | 4 ++-- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index 8bb1e8b2fb..0621b48e80 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -25,6 +25,7 @@ #include "net/dcsctp/packet/chunk/sack_chunk.h" #include "net/dcsctp/packet/sctp_packet.h" #include "net/dcsctp/public/dcsctp_options.h" +#include "net/dcsctp/public/types.h" #include "net/dcsctp/rx/data_tracker.h" #include "net/dcsctp/rx/reassembly_queue.h" #include "net/dcsctp/socket/capabilities.h" @@ -63,7 +64,9 @@ TransmissionControlBlock::TransmissionControlBlock( TimerOptions(options.rto_initial, TimerBackoffAlgorithm::kExponential, /*max_restarts=*/absl::nullopt, - options.max_timer_backoff_duration))), + options.max_timer_backoff_duration.has_value() + ? *options.max_timer_backoff_duration + : DurationMs::InfiniteDuration()))), delayed_ack_timer_(timer_manager_.CreateTimer( "delayed-ack", absl::bind_front(&TransmissionControlBlock::OnDelayedAckTimerExpiry, @@ -71,7 +74,7 @@ TransmissionControlBlock::TransmissionControlBlock( TimerOptions(options.delayed_ack_max_timeout, TimerBackoffAlgorithm::kExponential, /*max_restarts=*/0, - /*max_backoff_duration=*/absl::nullopt, + /*max_backoff_duration=*/DurationMs::InfiniteDuration(), webrtc::TaskQueueBase::DelayPrecision::kHigh))), my_verification_tag_(my_verification_tag), my_initial_tsn_(my_initial_tsn), diff --git a/net/dcsctp/timer/timer.cc b/net/dcsctp/timer/timer.cc index bde07638a5..208f26fdf9 100644 --- a/net/dcsctp/timer/timer.cc +++ b/net/dcsctp/timer/timer.cc @@ -33,19 +33,18 @@ DurationMs GetBackoffDuration(const TimerOptions& options, case TimerBackoffAlgorithm::kFixed: return base_duration; case TimerBackoffAlgorithm::kExponential: { - int32_t duration_ms = *base_duration; + DurationMs duration = base_duration; - while (expiration_count > 0 && duration_ms < *Timer::kMaxTimerDuration) { - duration_ms *= 2; + while (expiration_count > 0 && duration < Timer::kMaxTimerDuration) { + duration *= 2; --expiration_count; - if (options.max_backoff_duration.has_value() && - duration_ms > **options.max_backoff_duration) { - return *options.max_backoff_duration; + if (duration > options.max_backoff_duration) { + return options.max_backoff_duration; } } - return DurationMs(std::min(duration_ms, *Timer::kMaxTimerDuration)); + return DurationMs(std::min(duration, Timer::kMaxTimerDuration)); } } } diff --git a/net/dcsctp/timer/timer.h b/net/dcsctp/timer/timer.h index 31b496dc81..95aae570c8 100644 --- a/net/dcsctp/timer/timer.h +++ b/net/dcsctp/timer/timer.h @@ -47,12 +47,14 @@ struct TimerOptions { TimerOptions(DurationMs duration, TimerBackoffAlgorithm backoff_algorithm, absl::optional max_restarts) - : TimerOptions(duration, backoff_algorithm, max_restarts, absl::nullopt) { - } + : TimerOptions(duration, + backoff_algorithm, + max_restarts, + DurationMs::InfiniteDuration()) {} TimerOptions(DurationMs duration, TimerBackoffAlgorithm backoff_algorithm, absl::optional max_restarts, - absl::optional max_backoff_duration) + DurationMs max_backoff_duration) : TimerOptions(duration, backoff_algorithm, max_restarts, @@ -61,7 +63,7 @@ struct TimerOptions { TimerOptions(DurationMs duration, TimerBackoffAlgorithm backoff_algorithm, absl::optional max_restarts, - absl::optional max_backoff_duration, + DurationMs max_backoff_duration, webrtc::TaskQueueBase::DelayPrecision precision) : duration(duration), backoff_algorithm(backoff_algorithm), @@ -78,7 +80,7 @@ struct TimerOptions { // or absl::nullopt if there is no limit. const absl::optional max_restarts; // The maximum timeout value for exponential backoff. - const absl::optional max_backoff_duration; + const DurationMs max_backoff_duration; // The precision of the webrtc::TaskQueueBase used for scheduling. const webrtc::TaskQueueBase::DelayPrecision precision; }; diff --git a/net/dcsctp/timer/timer_test.cc b/net/dcsctp/timer/timer_test.cc index 4aebe65b48..93876160bb 100644 --- a/net/dcsctp/timer/timer_test.cc +++ b/net/dcsctp/timer/timer_test.cc @@ -442,7 +442,7 @@ TEST(TimerManagerTest, TimerManagerPassesPrecisionToCreateTimeoutMethod) { manager.CreateTimer( "test_timer", []() { return absl::optional(); }, TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential, - absl::nullopt, absl::nullopt, + absl::nullopt, DurationMs::InfiniteDuration(), webrtc::TaskQueueBase::DelayPrecision::kHigh)); EXPECT_EQ(create_timer_precison, webrtc::TaskQueueBase::DelayPrecision::kHigh); @@ -450,7 +450,7 @@ TEST(TimerManagerTest, TimerManagerPassesPrecisionToCreateTimeoutMethod) { manager.CreateTimer( "test_timer", []() { return absl::optional(); }, TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential, - absl::nullopt, absl::nullopt, + absl::nullopt, DurationMs::InfiniteDuration(), webrtc::TaskQueueBase::DelayPrecision::kLow)); EXPECT_EQ(create_timer_precison, webrtc::TaskQueueBase::DelayPrecision::kLow); }