admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dcsctp: Handle losing first DATA on ordered stream

Browse Source 
When a FORWARD-TSN is received as the first chunk on an ordered stream,
it will fail to set the new "next expected SSN" that is present in the
FORWARD-TSN as that stream hasn't been allocated yet. It's allocated
when the first DATA is received on that stream.

This is a non-issue for ordinary data channels as the first message on
any stream will be the "Data Channel Establishment Protocol" messages,
which are always sent reliably. But if prenegotiated channels are used,
and the very first packet received on an ordered data channel is lost
_and_ signaled to the receiver as lost _before_ the receiver has
received any other fragments on that data channel, future messages will
not be delivered on that channel.

Bug: webrtc:13799
Change-Id: Ide5c656243b3a51a2ed9d76615cfc3631cfe900c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253902
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36155}
This commit is contained in:
Victor Boivie 2022-03-08 09:36:55 +01:00 committed by WebRTC LUCI CQ
parent 4d54260ae2
commit a04b8b5043
3 changed files with 61 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
net/dcsctp/rx/traditional_reassembly_streams.cc
View File

@ -261,11 +261,11 @@ size_t TraditionalReassemblyStreams::OrderedStream::EraseTo(SSN ssn) {
int TraditionalReassemblyStreams::Add(UnwrappedTSN tsn, Data data) {
if (data.is_unordered) {
auto it = unordered_streams_.emplace(data.stream_id, this).first;
auto it = unordered_streams_.try_emplace(data.stream_id, this).first;
return it->second.Add(tsn, std::move(data));
}
auto it = ordered_streams_.emplace(data.stream_id, this).first;
auto it = ordered_streams_.try_emplace(data.stream_id, this).first;
return it->second.Add(tsn, std::move(data));
}
@ -280,10 +280,9 @@ size_t TraditionalReassemblyStreams::HandleForwardTsn(
}
for (const auto& skipped_stream : skipped_streams) {
auto it = ordered_streams_.find(skipped_stream.stream_id);
if (it != ordered_streams_.end()) {
bytes_removed += it->second.EraseTo(skipped_stream.ssn);
}
auto it =
ordered_streams_.try_emplace(skipped_stream.stream_id, this).first;
bytes_removed += it->second.EraseTo(skipped_stream.ssn);
}
return bytes_removed;

21
net/dcsctp/rx/traditional_reassembly_streams_test.cc
View File

@ -25,8 +25,10 @@
namespace dcsctp {
namespace {
using ::testing::ElementsAre;
using ::testing::MockFunction;
using ::testing::NiceMock;
using ::testing::Property;
class TraditionalReassemblyStreamsTest : public testing::Test {
protected:
@ -232,5 +234,24 @@ TEST_F(TraditionalReassemblyStreamsTest,
EXPECT_EQ(streams2.Add(tsn(4), gen_.Unordered({7})), 1);
}
TEST_F(TraditionalReassemblyStreamsTest, CanDeleteFirstOrderedMessage) {
NiceMock<MockFunction<ReassemblyStreams::OnAssembledMessage>> on_assembled;
EXPECT_CALL(on_assembled,
Call(ElementsAre(tsn(2)),
Property(&DcSctpMessage::payload, ElementsAre(2, 3, 4))));
TraditionalReassemblyStreams streams("", on_assembled.AsStdFunction());
// Not received, SID=1. TSN=1, SSN=0
gen_.Ordered({1}, "BE");
// And deleted (SID=1, TSN=1, SSN=0)
ForwardTsnChunk::SkippedStream skipped[] = {
ForwardTsnChunk::SkippedStream(StreamID(1), SSN(0))};
EXPECT_EQ(streams.HandleForwardTsn(tsn(1), skipped), 0u);
// Receive SID=1, TSN=2, SSN=1
EXPECT_EQ(streams.Add(tsn(2), gen_.Ordered({2, 3, 4}, "BE")), 0);
}
} // namespace
} // namespace dcsctp

35
net/dcsctp/socket/dcsctp_socket_test.cc
View File

@ -2179,5 +2179,40 @@ TEST(DcSctpSocketTest, BothCanDetectDcsctpImplementation) {
EXPECT_EQ(a.socket.peer_implementation(), SctpImplementation::kDcsctp);
EXPECT_EQ(z.socket.peer_implementation(), SctpImplementation::kDcsctp);
}
TEST_P(DcSctpSocketParametrizedTest, CanLoseFirstOrderedMessage) {
SocketUnderTest a("A");
auto z = std::make_unique<SocketUnderTest>("Z");
ConnectSockets(a, *z);
z = MaybeHandoverSocket(std::move(z));
SendOptions send_options;
send_options.unordered = IsUnordered(false);
send_options.max_retransmissions = 0;
std::vector<uint8_t> payload(a.options.mtu - 100);
// Send a first message (SID=1, SSN=0)
a.socket.Send(DcSctpMessage(StreamID(1), PPID(51), payload), send_options);
// First DATA is lost, and retransmission timer will delete it.
a.cb.ConsumeSentPacket();
AdvanceTime(a, *z, a.options.rto_initial);
ExchangeMessages(a, *z);
// Send a second message (SID=0, SSN=1).
a.socket.Send(DcSctpMessage(StreamID(1), PPID(52), payload), send_options);
ExchangeMessages(a, *z);
// The Z socket should receive the second message, but not the first.
absl::optional<DcSctpMessage> msg = z->cb.ConsumeReceivedMessage();
ASSERT_TRUE(msg.has_value());
EXPECT_EQ(msg->ppid(), PPID(52));
EXPECT_FALSE(z->cb.ConsumeReceivedMessage().has_value());
MaybeHandoverSocketAndSendMessage(a, std::move(z));
}
} // namespace
} // namespace dcsctp