From 3f981ee9797a3db969ece21263c07388a05eaa43 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Thu, 14 Oct 2021 22:28:43 +0200 Subject: [PATCH] dcsctp: Support zero window probing This is explained in RFC 4960, section 6.1, A. ... However, regardless of the value of rwnd (including if it is 0), the data sender can always have one DATA chunk in flight to the receiver if allowed by cwnd ... This rule allows the sender to probe for a change in rwnd that the sender missed due to the SACK's having been lost in transit from the data receiver to the data sender. Before this change, when a receiver has advertised a zero receiver window size (a_rwnd=0) and a subsequent SACK advertising a non-zero receiver window was lost, the sender was blocked from sending and since SACKs are only sent when a DATA chunk is received, it would be deadlocked. The retransmission timer would fire, but nothing would be retransmitted (as it respected the zero receiver window). With this change, when the retransmission timer fires (after RTO), it would send a single packet with DATA chunk(s) and then SACKs would eventually be received, with the non-zero receiver window and the socket would recover. Bug: chromium:1258225 Change-Id: I1ea62fb3c002150eeada28d3e703dbc09cfd038e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235280 Commit-Queue: Victor Boivie Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#35215} --- net/dcsctp/tx/BUILD.gn | 1 + net/dcsctp/tx/retransmission_queue.cc | 9 +++ net/dcsctp/tx/retransmission_queue_test.cc | 80 ++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/net/dcsctp/tx/BUILD.gn b/net/dcsctp/tx/BUILD.gn index 6bf439f955..4f6fb709bb 100644 --- a/net/dcsctp/tx/BUILD.gn +++ b/net/dcsctp/tx/BUILD.gn @@ -122,6 +122,7 @@ if (rtc_include_tests) { "../../../rtc_base:rtc_base_approved", "../../../test:test_support", "../common:handover_testing", + "../common:math", "../packet:chunk", "../packet:data", "../public:socket", diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc index d45c274108..00dfd562a0 100644 --- a/net/dcsctp/tx/retransmission_queue.cc +++ b/net/dcsctp/tx/retransmission_queue.cc @@ -852,6 +852,15 @@ void RetransmissionQueue::AbandonAllFor( size_t RetransmissionQueue::max_bytes_to_send() const { size_t left = outstanding_bytes_ >= cwnd_ ? 0 : cwnd_ - outstanding_bytes_; + + if (outstanding_bytes_ == 0) { + // https://datatracker.ietf.org/doc/html/rfc4960#section-6.1 + // ... However, regardless of the value of rwnd (including if it is 0), the + // data sender can always have one DATA chunk in flight to the receiver if + // allowed by cwnd (see rule B, below). + return left; + } + return std::min(rwnd(), left); } diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc index b34927a1bb..b2c2997730 100644 --- a/net/dcsctp/tx/retransmission_queue_test.cc +++ b/net/dcsctp/tx/retransmission_queue_test.cc @@ -19,6 +19,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" #include "net/dcsctp/common/handover_testing.h" +#include "net/dcsctp/common/math.h" #include "net/dcsctp/packet/chunk/data_chunk.h" #include "net/dcsctp/packet/chunk/forward_tsn_chunk.h" #include "net/dcsctp/packet/chunk/forward_tsn_common.h" @@ -1408,5 +1409,84 @@ TEST_F(RetransmissionQueueTest, HandoverTest) { Pair(TSN(14), State::kInFlight))); } +TEST_F(RetransmissionQueueTest, CanAlwaysSendOnePacket) { + RetransmissionQueue queue = CreateQueue(); + + // A large payload - enough to not fit two DATA in same packet. + size_t mtu = RoundDownTo4(options_.mtu); + std::vector payload(mtu - 100); + + EXPECT_CALL(producer_, Produce) + .WillOnce([this, payload](TimeMs, size_t) { + return SendQueue::DataToSend(gen_.Ordered(payload, "B")); + }) + .WillOnce([this, payload](TimeMs, size_t) { + return SendQueue::DataToSend(gen_.Ordered(payload, "")); + }) + .WillOnce([this, payload](TimeMs, size_t) { + return SendQueue::DataToSend(gen_.Ordered(payload, "")); + }) + .WillOnce([this, payload](TimeMs, size_t) { + return SendQueue::DataToSend(gen_.Ordered(payload, "")); + }) + .WillOnce([this, payload](TimeMs, size_t) { + return SendQueue::DataToSend(gen_.Ordered(payload, "E")); + }) + .WillRepeatedly([](TimeMs, size_t) { return absl::nullopt; }); + + // Produce all chunks and put them in the retransmission queue. + std::vector> chunks_to_send = + queue.GetChunksToSend(now_, 5 * mtu); + EXPECT_THAT(chunks_to_send, + ElementsAre(Pair(TSN(10), _), Pair(TSN(11), _), Pair(TSN(12), _), + Pair(TSN(13), _), Pair(TSN(14), _))); + EXPECT_THAT(queue.GetChunkStatesForTesting(), + ElementsAre(Pair(TSN(9), State::kAcked), // + Pair(TSN(10), State::kInFlight), // + Pair(TSN(11), State::kInFlight), // + Pair(TSN(12), State::kInFlight), + Pair(TSN(13), State::kInFlight), + Pair(TSN(14), State::kInFlight))); + + // Ack 12, and report an empty receiver window (the peer obviously has a + // tiny receive window). + queue.HandleSack( + now_, SackChunk(TSN(9), /*rwnd=*/0, {SackChunk::GapAckBlock(3, 3)}, {})); + + // Force TSN 10 to be retransmitted. + queue.HandleT3RtxTimerExpiry(); + + // Even if the receiver window is empty, it will allow TSN 10 to be sent. + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), ElementsAre(Pair(TSN(10), _))); + + // But not more than that, as there now is outstanding data. + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), IsEmpty()); + + // Don't ack any new data, and still have receiver window zero. + queue.HandleSack( + now_, SackChunk(TSN(9), /*rwnd=*/0, {SackChunk::GapAckBlock(3, 3)}, {})); + + // There is in-flight data, so new data should not be allowed to be send since + // the receiver window is full. + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), IsEmpty()); + + // Ack that packet (no more in-flight data), but still report an empty + // receiver window. + queue.HandleSack( + now_, SackChunk(TSN(10), /*rwnd=*/0, {SackChunk::GapAckBlock(2, 2)}, {})); + + // Then TSN 11 can be sent, as there is no in-flight data. + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), ElementsAre(Pair(TSN(11), _))); + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), IsEmpty()); + + // Ack and recover the receiver window + queue.HandleSack(now_, SackChunk(TSN(12), /*rwnd=*/5 * mtu, {}, {})); + + // That will unblock sending remaining chunks. + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), ElementsAre(Pair(TSN(13), _))); + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), ElementsAre(Pair(TSN(14), _))); + EXPECT_THAT(queue.GetChunksToSend(now_, mtu), IsEmpty()); +} + } // namespace } // namespace dcsctp