dcsctp: Restart T3-RTX on fast rtx of lowest TSN

RFC 4960, 7.2.4.  Fast Retransmit on Gap Reports

4)  Restart the T3-rtx timer only if the last SACK acknowledged the
    lowest outstanding TSN number sent to that address, or the
    endpoint is retransmitting the first outstanding DATA chunk sent
    to that address.

The second part of this sentence, "or the endpoint is retransmitting the
first outstanding DATA chunk sent to that address."

This means that on fast retransmit, and if the retransmitted chunks
weren't quickly acknowledged by the peer, the retransmission timer would
expire too quickly. With this CL, it will restart, as it should.

Bug: webrtc:12943
Change-Id: I7627253718530876acc516460562e66bfcc79533
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265620
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37193}
This commit is contained in:
Victor Boivie
2022-06-10 22:50:49 +02:00
committed by WebRTC LUCI CQ
parent 1220855430
commit c8e8e767e2
2 changed files with 94 additions and 1 deletions

View File

@ -398,6 +398,17 @@ RetransmissionQueue::GetChunksForFastRetransmit(size_t bytes_in_packet) {
outstanding_data_.GetChunksToBeFastRetransmitted(bytes_in_packet);
RTC_DCHECK(!to_be_sent.empty());
// https://tools.ietf.org/html/rfc4960#section-7.2.4
// "4) Restart the T3-rtx timer only if ... the endpoint is retransmitting
// the first outstanding DATA chunk sent to that address."
if (to_be_sent[0].first ==
outstanding_data_.last_cumulative_tsn_ack().next_value().Wrap()) {
RTC_DLOG(LS_VERBOSE)
<< log_prefix_
<< "First outstanding DATA to be retransmitted - restarting T3-RTX";
t3_rtx_.Stop();
}
// https://tools.ietf.org/html/rfc4960#section-6.3.2
// "Every time a DATA chunk is sent to any address (including a
// retransmission), if the T3-rtx timer of that address is not running,

View File

@ -29,6 +29,7 @@
#include "net/dcsctp/packet/data.h"
#include "net/dcsctp/public/dcsctp_options.h"
#include "net/dcsctp/testing/data_generator.h"
#include "net/dcsctp/testing/testing_macros.h"
#include "net/dcsctp/timer/fake_timeout.h"
#include "net/dcsctp/timer/timer.h"
#include "net/dcsctp/tx/mock_send_queue.h"
@ -70,7 +71,7 @@ class RetransmissionQueueTest : public testing::Test {
timer_(timer_manager_.CreateTimer(
"test/t3_rtx",
[]() { return absl::nullopt; },
TimerOptions(DurationMs(0)))) {}
TimerOptions(options_.rto_initial))) {}
std::function<SendQueue::DataToSend(TimeMs, size_t)> CreateChunk() {
return [this](TimeMs now, size_t max_size) {
@ -302,6 +303,87 @@ TEST_F(RetransmissionQueueTest, ResendPacketsWhenNackedThreeTimes) {
Pair(TSN(20), State::kAcked)));
}
TEST_F(RetransmissionQueueTest, RestartsT3RtxOnRetransmitFirstOutstandingTSN) {
// Verifies that if fast retransmit is retransmitting the first outstanding
// TSN, it will also restart T3-RTX.
RetransmissionQueue queue = CreateQueue();
EXPECT_CALL(producer_, Produce)
.WillOnce(CreateChunk())
.WillOnce(CreateChunk())
.WillOnce(CreateChunk())
.WillRepeatedly([](TimeMs, size_t) { return absl::nullopt; });
static constexpr TimeMs kStartTime(100000);
now_ = kStartTime;
EXPECT_THAT(GetSentPacketTSNs(queue),
testing::ElementsAre(TSN(10), TSN(11), TSN(12)));
// Ack 10, 12, after 100ms.
now_ += DurationMs(100);
queue.HandleSack(
now_, SackChunk(TSN(10), kArwnd, {SackChunk::GapAckBlock(2, 2)}, {}));
EXPECT_THAT(queue.GetChunkStatesForTesting(),
ElementsAre(Pair(TSN(10), State::kAcked), //
Pair(TSN(11), State::kNacked), //
Pair(TSN(12), State::kAcked)));
// Send 13
EXPECT_CALL(producer_, Produce)
.WillOnce(CreateChunk())
.WillRepeatedly([](TimeMs, size_t) { return absl::nullopt; });
EXPECT_THAT(GetSentPacketTSNs(queue), testing::ElementsAre(TSN(13)));
// Ack 10, 12-13, after 100ms.
now_ += DurationMs(100);
queue.HandleSack(
now_, SackChunk(TSN(10), kArwnd, {SackChunk::GapAckBlock(2, 3)}, {}));
// Send 14
EXPECT_CALL(producer_, Produce)
.WillOnce(CreateChunk())
.WillRepeatedly([](TimeMs, size_t) { return absl::nullopt; });
EXPECT_THAT(GetSentPacketTSNs(queue), testing::ElementsAre(TSN(14)));
// Ack 10, 12-14, after 100 ms.
now_ += DurationMs(100);
queue.HandleSack(
now_, SackChunk(TSN(10), kArwnd, {SackChunk::GapAckBlock(2, 4)}, {}));
EXPECT_THAT(queue.GetChunkStatesForTesting(),
ElementsAre(Pair(TSN(10), State::kAcked), //
Pair(TSN(11), State::kToBeRetransmitted), //
Pair(TSN(12), State::kAcked), //
Pair(TSN(13), State::kAcked), //
Pair(TSN(14), State::kAcked)));
// This will trigger "fast retransmit" mode and only chunks 13 and 16 will be
// resent right now. The send queue will not even be queried.
EXPECT_CALL(producer_, Produce).Times(0);
EXPECT_THAT(GetTSNsForFastRetransmit(queue), testing::ElementsAre(TSN(11)));
EXPECT_THAT(queue.GetChunkStatesForTesting(),
ElementsAre(Pair(TSN(10), State::kAcked), //
Pair(TSN(11), State::kInFlight), //
Pair(TSN(12), State::kAcked), //
Pair(TSN(13), State::kAcked), //
Pair(TSN(14), State::kAcked)));
// Verify that the timer was really restarted when fast-retransmitting. The
// timeout is `options_.rto_initial`, so advance the time just before that.
now_ += options_.rto_initial - DurationMs(1);
EXPECT_FALSE(timeout_manager_.GetNextExpiredTimeout().has_value());
// And ensure it really is running.
now_ += DurationMs(1);
ASSERT_HAS_VALUE_AND_ASSIGN(TimeoutID timeout,
timeout_manager_.GetNextExpiredTimeout());
// An expired timeout has to be handled (asserts validate this).
timer_manager_.HandleTimeout(timeout);
}
TEST_F(RetransmissionQueueTest, CanOnlyProduceTwoPacketsButWantsToSendThree) {
RetransmissionQueue queue = CreateQueue();
EXPECT_CALL(producer_, Produce)