diff --git a/net/dcsctp/public/BUILD.gn b/net/dcsctp/public/BUILD.gn index d92207be48..23530a6b52 100644 --- a/net/dcsctp/public/BUILD.gn +++ b/net/dcsctp/public/BUILD.gn @@ -22,6 +22,7 @@ rtc_source_set("types") { "dcsctp_options.h", "types.h", ] + absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } rtc_source_set("socket") { diff --git a/net/dcsctp/public/dcsctp_options.h b/net/dcsctp/public/dcsctp_options.h index e357aa01a6..a96611589f 100644 --- a/net/dcsctp/public/dcsctp_options.h +++ b/net/dcsctp/public/dcsctp_options.h @@ -13,6 +13,7 @@ #include #include +#include "absl/types/optional.h" #include "net/dcsctp/public/types.h" namespace dcsctp { @@ -150,11 +151,13 @@ struct DcSctpOptions { // retransmission scenarios. int max_burst = 4; - // Maximum Data Retransmit Attempts (per DATA chunk). - int max_retransmissions = 10; + // Maximum Data Retransmit Attempts (per DATA chunk). Set to absl::nullopt for + // no limit. + absl::optional max_retransmissions = 10; - // Max.Init.Retransmits (https://tools.ietf.org/html/rfc4960#section-15) - int max_init_retransmits = 8; + // Max.Init.Retransmits (https://tools.ietf.org/html/rfc4960#section-15). Set + // to absl::nullopt for no limit. + absl::optional max_init_retransmits = 8; // RFC3758 Partial Reliability Extension bool enable_partial_reliability = true; diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index a39ec5ce88..2983b0f5c7 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -775,7 +775,7 @@ bool DcSctpSocket::HandleUnrecognizedChunk( absl::optional DcSctpSocket::OnInitTimerExpiry() { RTC_DLOG(LS_VERBOSE) << log_prefix() << "Timer " << t1_init_->name() << " has expired: " << t1_init_->expiration_count() - << "/" << t1_init_->options().max_restarts; + << "/" << t1_init_->options().max_restarts.value_or(-1); RTC_DCHECK(state_ == State::kCookieWait); if (t1_init_->is_running()) { @@ -796,7 +796,8 @@ absl::optional DcSctpSocket::OnCookieTimerExpiry() { // user." RTC_DLOG(LS_VERBOSE) << log_prefix() << "Timer " << t1_cookie_->name() << " has expired: " << t1_cookie_->expiration_count() - << "/" << t1_cookie_->options().max_restarts; + << "/" + << t1_cookie_->options().max_restarts.value_or(-1); RTC_DCHECK(state_ == State::kCookieEchoed); @@ -813,7 +814,8 @@ absl::optional DcSctpSocket::OnCookieTimerExpiry() { absl::optional DcSctpSocket::OnShutdownTimerExpiry() { RTC_DLOG(LS_VERBOSE) << log_prefix() << "Timer " << t2_shutdown_->name() << " has expired: " << t2_shutdown_->expiration_count() - << "/" << t2_shutdown_->options().max_restarts; + << "/" + << t2_shutdown_->options().max_restarts.value_or(-1); if (!t2_shutdown_->is_running()) { // https://tools.ietf.org/html/rfc4960#section-9.2 diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 1c8525a6f3..c0d7725ba5 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -487,7 +487,7 @@ TEST_F(DcSctpSocketTest, ResendingInitTooManyTimesAborts) { SctpPacket::Parse(cb_a_.ConsumeSentPacket())); EXPECT_EQ(init_packet.descriptors()[0].type, InitChunk::kType); - for (int i = 0; i < options_.max_init_retransmits; ++i) { + for (int i = 0; i < *options_.max_init_retransmits; ++i) { AdvanceTime(options_.t1_init_timeout * (1 << i)); RunTimers(); @@ -498,7 +498,7 @@ TEST_F(DcSctpSocketTest, ResendingInitTooManyTimesAborts) { } // Another timeout, after the max init retransmits. - AdvanceTime(options_.t1_init_timeout * (1 << options_.max_init_retransmits)); + AdvanceTime(options_.t1_init_timeout * (1 << *options_.max_init_retransmits)); EXPECT_CALL(cb_a_, OnAborted).Times(1); RunTimers(); @@ -543,7 +543,7 @@ TEST_F(DcSctpSocketTest, ResendingCookieEchoTooManyTimesAborts) { SctpPacket::Parse(cb_a_.ConsumeSentPacket())); EXPECT_EQ(init_packet.descriptors()[0].type, CookieEchoChunk::kType); - for (int i = 0; i < options_.max_init_retransmits; ++i) { + for (int i = 0; i < *options_.max_init_retransmits; ++i) { AdvanceTime(options_.t1_cookie_timeout * (1 << i)); RunTimers(); @@ -555,7 +555,7 @@ TEST_F(DcSctpSocketTest, ResendingCookieEchoTooManyTimesAborts) { // Another timeout, after the max init retransmits. AdvanceTime(options_.t1_cookie_timeout * - (1 << options_.max_init_retransmits)); + (1 << *options_.max_init_retransmits)); EXPECT_CALL(cb_a_, OnAborted).Times(1); RunTimers(); @@ -647,7 +647,7 @@ TEST_F(DcSctpSocketTest, ShutdownTimerExpiresTooManyTimeClosesConnection) { EXPECT_EQ(sock_a_.state(), SocketState::kShuttingDown); - for (int i = 0; i < options_.max_retransmissions; ++i) { + for (int i = 0; i < *options_.max_retransmissions; ++i) { AdvanceTime(DurationMs(options_.rto_initial * (1 << i))); RunTimers(); @@ -658,7 +658,7 @@ TEST_F(DcSctpSocketTest, ShutdownTimerExpiresTooManyTimeClosesConnection) { EXPECT_TRUE(cb_a_.ConsumeSentPacket().empty()); } // The last expiry, makes it abort the connection. - AdvanceTime(options_.rto_initial * (1 << options_.max_retransmissions)); + AdvanceTime(options_.rto_initial * (1 << *options_.max_retransmissions)); EXPECT_CALL(cb_a_, OnAborted).Times(1); RunTimers(); @@ -795,7 +795,7 @@ TEST_F(DcSctpSocketTest, CloseConnectionAfterTooManyLostHeartbeats) { DurationMs time_to_next_hearbeat = options_.heartbeat_interval; - for (int i = 0; i < options_.max_retransmissions; ++i) { + for (int i = 0; i < *options_.max_retransmissions; ++i) { RTC_LOG(LS_INFO) << "Letting HEARTBEAT interval timer expire - sending..."; AdvanceTime(time_to_next_hearbeat); RunTimers(); @@ -834,7 +834,7 @@ TEST_F(DcSctpSocketTest, RecoversAfterASuccessfulAck) { DurationMs time_to_next_hearbeat = options_.heartbeat_interval; - for (int i = 0; i < options_.max_retransmissions; ++i) { + for (int i = 0; i < *options_.max_retransmissions; ++i) { AdvanceTime(time_to_next_hearbeat); RunTimers(); diff --git a/net/dcsctp/timer/timer.cc b/net/dcsctp/timer/timer.cc index 8f64533d47..2ad6d74971 100644 --- a/net/dcsctp/timer/timer.cc +++ b/net/dcsctp/timer/timer.cc @@ -93,8 +93,8 @@ void Timer::Trigger(TimerGeneration generation) { if (is_running_ && generation == generation_) { ++expiration_count_; is_running_ = false; - if (options_.max_restarts < 0 || - expiration_count_ <= options_.max_restarts) { + if (!options_.max_restarts.has_value() || + expiration_count_ <= *options_.max_restarts) { // The timer should still be running after this triggers. Start a new // timer. Note that it might be very quickly restarted again, if the // `on_expired_` callback returns a new duration. diff --git a/net/dcsctp/timer/timer.h b/net/dcsctp/timer/timer.h index 2010b23a77..ed072de7d5 100644 --- a/net/dcsctp/timer/timer.h +++ b/net/dcsctp/timer/timer.h @@ -42,10 +42,10 @@ struct TimerOptions { explicit TimerOptions(DurationMs duration) : TimerOptions(duration, TimerBackoffAlgorithm::kExponential) {} TimerOptions(DurationMs duration, TimerBackoffAlgorithm backoff_algorithm) - : TimerOptions(duration, backoff_algorithm, -1) {} + : TimerOptions(duration, backoff_algorithm, absl::nullopt) {} TimerOptions(DurationMs duration, TimerBackoffAlgorithm backoff_algorithm, - int max_restarts) + absl::optional max_restarts) : duration(duration), backoff_algorithm(backoff_algorithm), max_restarts(max_restarts) {} @@ -55,8 +55,9 @@ struct TimerOptions { // If the duration should be increased (using exponential backoff) when it is // restarted. If not set, the same duration will be used. const TimerBackoffAlgorithm backoff_algorithm; - // The maximum number of times that the timer will be automatically restarted. - const int max_restarts; + // The maximum number of times that the timer will be automatically restarted, + // or absl::nullopt if there is no limit. + const absl::optional max_restarts; }; // A high-level timer (in contrast to the low-level `Timeout` class). diff --git a/net/dcsctp/tx/retransmission_error_counter.cc b/net/dcsctp/tx/retransmission_error_counter.cc index 111b6efe96..44b20ba2c2 100644 --- a/net/dcsctp/tx/retransmission_error_counter.cc +++ b/net/dcsctp/tx/retransmission_error_counter.cc @@ -15,14 +15,14 @@ namespace dcsctp { bool RetransmissionErrorCounter::Increment(absl::string_view reason) { ++counter_; - if (counter_ > limit_) { + if (limit_.has_value() && counter_ > limit_.value()) { RTC_DLOG(LS_INFO) << log_prefix_ << reason << ", too many retransmissions, counter=" << counter_; return false; } RTC_DLOG(LS_VERBOSE) << log_prefix_ << reason << ", new counter=" << counter_ - << ", max=" << limit_; + << ", max=" << limit_.value_or(-1); return true; } diff --git a/net/dcsctp/tx/retransmission_error_counter.h b/net/dcsctp/tx/retransmission_error_counter.h index bb8d1f754d..18af3d3c4f 100644 --- a/net/dcsctp/tx/retransmission_error_counter.h +++ b/net/dcsctp/tx/retransmission_error_counter.h @@ -33,7 +33,7 @@ class RetransmissionErrorCounter { // Increments the retransmission timer. If the maximum error count has been // reached, `false` will be returned. bool Increment(absl::string_view reason); - bool IsExhausted() const { return counter_ > limit_; } + bool IsExhausted() const { return limit_.has_value() && counter_ > *limit_; } // Clears the retransmission errors. void Clear(); @@ -43,7 +43,7 @@ class RetransmissionErrorCounter { private: const std::string log_prefix_; - const int limit_; + const absl::optional limit_; int counter_ = 0; }; } // namespace dcsctp diff --git a/net/dcsctp/tx/retransmission_error_counter_test.cc b/net/dcsctp/tx/retransmission_error_counter_test.cc index 61ee82926d..67bbc0bec5 100644 --- a/net/dcsctp/tx/retransmission_error_counter_test.cc +++ b/net/dcsctp/tx/retransmission_error_counter_test.cc @@ -72,5 +72,15 @@ TEST(RetransmissionErrorCounterTest, ClearingCounter) { EXPECT_TRUE(counter.IsExhausted()); } +TEST(RetransmissionErrorCounterTest, CanBeLimitless) { + DcSctpOptions options; + options.max_retransmissions = absl::nullopt; + RetransmissionErrorCounter counter("log: ", options); + for (int i = 0; i < 100; ++i) { + EXPECT_TRUE(counter.Increment("test")); + EXPECT_FALSE(counter.IsExhausted()); + } +} + } // namespace } // namespace dcsctp