dcsctp: Support unlimited max_retransmissions
The restart limit for timers can already be limitless, but the
RetransmissionErrorCounter didn't support this. With this change, the
max_retransmissions and max_init_retransmits can be absl::nullopt to
indicate that there should be infinite retries.
Bug: webrtc:13129
Change-Id: Ia6e91cccbc2e1bb77b3fdd7f37436290adc2f483
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230701
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34882}
diff --git a/net/dcsctp/public/BUILD.gn b/net/dcsctp/public/BUILD.gn
index d92207b..23530a6 100644
--- a/net/dcsctp/public/BUILD.gn
+++ b/net/dcsctp/public/BUILD.gn
@@ -22,6 +22,7 @@
"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 e357aa0..a966115 100644
--- a/net/dcsctp/public/dcsctp_options.h
+++ b/net/dcsctp/public/dcsctp_options.h
@@ -13,6 +13,7 @@
#include <stddef.h>
#include <stdint.h>
+#include "absl/types/optional.h"
#include "net/dcsctp/public/types.h"
namespace dcsctp {
@@ -150,11 +151,13 @@
// 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<int> 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<int> 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 a39ec5c..2983b0f 100644
--- a/net/dcsctp/socket/dcsctp_socket.cc
+++ b/net/dcsctp/socket/dcsctp_socket.cc
@@ -775,7 +775,7 @@
absl::optional<DurationMs> 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 @@
// 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<DurationMs> 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 1c8525a..c0d7725 100644
--- a/net/dcsctp/socket/dcsctp_socket_test.cc
+++ b/net/dcsctp/socket/dcsctp_socket_test.cc
@@ -487,7 +487,7 @@
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 @@
}
// 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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 8f64533..2ad6d74 100644
--- a/net/dcsctp/timer/timer.cc
+++ b/net/dcsctp/timer/timer.cc
@@ -93,8 +93,8 @@
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 2010b23..ed072de 100644
--- a/net/dcsctp/timer/timer.h
+++ b/net/dcsctp/timer/timer.h
@@ -42,10 +42,10 @@
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<int> max_restarts)
: duration(duration),
backoff_algorithm(backoff_algorithm),
max_restarts(max_restarts) {}
@@ -55,8 +55,9 @@
// 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<int> 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 111b6ef..44b20ba 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 bb8d1f7..18af3d3 100644
--- a/net/dcsctp/tx/retransmission_error_counter.h
+++ b/net/dcsctp/tx/retransmission_error_counter.h
@@ -33,7 +33,7 @@
// 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 @@
private:
const std::string log_prefix_;
- const int limit_;
+ const absl::optional<int> 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 61ee829..67bbc0b 100644
--- a/net/dcsctp/tx/retransmission_error_counter_test.cc
+++ b/net/dcsctp/tx/retransmission_error_counter_test.cc
@@ -72,5 +72,15 @@
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
