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

dcsctp: Continue reset pending streams

Browse Source 
When resetting several streams in sequence, only the first stream will
be included in the first RE_CONFIG chunk as it's created eagerly
whenever someone calls ResetStreams. The remaining ones are queued as
pending. When the first request finishes, the remaining ones should
continue to be processed, but this wasn't done prior to this commit.

This would only happen if two streams would be reset with shorter time
between them than a RTT, so that there would be an outstanding request
forcing the second reset to be enqueued.

Bug: chromium:1312009
Change-Id: Id74b375d1d1720406a3bca4ec60df5780ca7edba
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257306
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36404}
This commit is contained in:
Victor Boivie 2022-03-31 17:15:03 +02:00 committed by WebRTC LUCI CQ
parent 63542409ef
commit f9e116f46e
3 changed files with 71 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
net/dcsctp/socket/dcsctp_socket.cc
View File

@ -479,13 +479,7 @@ ResetStreamsStatus DcSctpSocket::ResetStreams(
}
tcb_->stream_reset_handler().ResetStreams(outgoing_streams);
absl::optional<ReConfigChunk> reconfig =
tcb_->stream_reset_handler().MakeStreamResetRequest();
if (reconfig.has_value()) {
SctpPacket::Builder builder = tcb_->PacketBuilder();
builder.Add(*reconfig);
packet_sender_.Send(builder);
}
MaybeSendResetStreamsRequest();
RTC_DCHECK(IsConsistent());
return ResetStreamsStatus::kPerformed;
@ -570,6 +564,16 @@ void DcSctpSocket::MaybeSendShutdownOnPacketReceived(const SctpPacket& packet) {
}
}
void DcSctpSocket::MaybeSendResetStreamsRequest() {
absl::optional<ReConfigChunk> reconfig =
tcb_->stream_reset_handler().MakeStreamResetRequest();
if (reconfig.has_value()) {
SctpPacket::Builder builder = tcb_->PacketBuilder();
builder.Add(*reconfig);
packet_sender_.Send(builder);
}
}
bool DcSctpSocket::ValidatePacket(const SctpPacket& packet) {
const CommonHeader& header = packet.common_header();
VerificationTag my_verification_tag =
@ -1463,6 +1467,10 @@ void DcSctpSocket::HandleReconfig(
absl::optional<ReConfigChunk> chunk = ReConfigChunk::Parse(descriptor.data);
if (ValidateParseSuccess(chunk) && ValidateHasTCB()) {
tcb_->stream_reset_handler().HandleReConfig(*std::move(chunk));
// Handling this response may result in outgoing stream resets finishing
// (either successfully or with failure). If there still are pending streams
// that were waiting for this request to finish, continue resetting them.
MaybeSendResetStreamsRequest();
}
}

2
net/dcsctp/socket/dcsctp_socket.h
View File

@ -155,6 +155,8 @@ class DcSctpSocket : public DcSctpSocketInterface {
void MaybeSendShutdownOrAck();
// If the socket is shutting down, responds SHUTDOWN to any incoming DATA.
void MaybeSendShutdownOnPacketReceived(const SctpPacket& packet);
// If there are streams pending to be reset, send a request to reset them.
void MaybeSendResetStreamsRequest();
// Sends a INIT chunk.
void SendInit();
// Sends a SHUTDOWN chunk.

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

@ -61,6 +61,7 @@ using ::testing::ElementsAre;
using ::testing::HasSubstr;
using ::testing::IsEmpty;
using ::testing::SizeIs;
using ::testing::UnorderedElementsAre;
constexpr SendOptions kSendOptions;
constexpr size_t kLargeMessageSize = DcSctpOptions::kMaxSafeMTUSize * 20;
@ -2262,5 +2263,58 @@ TEST(DcSctpSocketTest, ReceiveBothUnorderedAndOrderedWithSameTSN) {
std::vector<uint8_t>(10), opts))
.Build());
}
TEST(DcSctpSocketTest, CloseTwoStreamsAtTheSameTime) {
// Reported as https://crbug.com/1312009.
SocketUnderTest a("A");
SocketUnderTest z("Z");
EXPECT_CALL(z.cb, OnIncomingStreamsReset(ElementsAre(StreamID(1)))).Times(1);
EXPECT_CALL(z.cb, OnIncomingStreamsReset(ElementsAre(StreamID(2)))).Times(1);
EXPECT_CALL(a.cb, OnStreamsResetPerformed(ElementsAre(StreamID(1)))).Times(1);
EXPECT_CALL(a.cb, OnStreamsResetPerformed(ElementsAre(StreamID(2)))).Times(1);
ConnectSockets(a, z);
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), {1, 2}), kSendOptions);
a.socket.Send(DcSctpMessage(StreamID(2), PPID(53), {1, 2}), kSendOptions);
ExchangeMessages(a, z);
a.socket.ResetStreams(std::vector<StreamID>({StreamID(1)}));
a.socket.ResetStreams(std::vector<StreamID>({StreamID(2)}));
ExchangeMessages(a, z);
}
TEST(DcSctpSocketTest, CloseThreeStreamsAtTheSameTime) {
// Similar to CloseTwoStreamsAtTheSameTime, but ensuring that the two
// remaining streams are reset at the same time in the second request.
SocketUnderTest a("A");
SocketUnderTest z("Z");
EXPECT_CALL(z.cb, OnIncomingStreamsReset(ElementsAre(StreamID(1)))).Times(1);
EXPECT_CALL(z.cb, OnIncomingStreamsReset(
UnorderedElementsAre(StreamID(2), StreamID(3))))
.Times(1);
EXPECT_CALL(a.cb, OnStreamsResetPerformed(ElementsAre(StreamID(1)))).Times(1);
EXPECT_CALL(a.cb, OnStreamsResetPerformed(
UnorderedElementsAre(StreamID(2), StreamID(3))))
.Times(1);
ConnectSockets(a, z);
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), {1, 2}), kSendOptions);
a.socket.Send(DcSctpMessage(StreamID(2), PPID(53), {1, 2}), kSendOptions);
a.socket.Send(DcSctpMessage(StreamID(3), PPID(53), {1, 2}), kSendOptions);
ExchangeMessages(a, z);
a.socket.ResetStreams(std::vector<StreamID>({StreamID(1)}));
a.socket.ResetStreams(std::vector<StreamID>({StreamID(2)}));
a.socket.ResetStreams(std::vector<StreamID>({StreamID(3)}));
ExchangeMessages(a, z);
}
} // namespace
} // namespace dcsctp