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

dcsctp: Handle re-received stream reset requests

Browse Source 
When re-receiving a stream reset request with the same request
sequence number, reply with the same result as previous time. In
case the original request was deferred, and "in progress" was
replied, it's important to not indicate that it was performed
successfully as that will make the sender believe it has completed
before it did.

Bug: webrtc:14277
Change-Id: I5c7082dc285180d62061d7ebebe05566e5c4195c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274080
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38012}
This commit is contained in:
Victor Boivie 2022-09-04 19:37:32 +00:00 committed by WebRTC LUCI CQ
parent a006ba152f
commit 5625a86f5a
3 changed files with 29 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
net/dcsctp/socket/stream_reset_handler.cc
View File

@ -134,11 +134,16 @@ bool StreamResetHandler::ValidateReqSeqNbr(
ReconfigRequestSN req_seq_nbr,
std::vector<ReconfigurationResponseParameter>& responses) {
if (req_seq_nbr == last_processed_req_seq_nbr_) {
// This has already been performed previously.
// https://www.rfc-editor.org/rfc/rfc6525.html#section-5.2.1 "If the
// received RE-CONFIG chunk contains at least one request and based on the
// analysis of the Re-configuration Request Sequence Numbers this is the
// last received RE-CONFIG chunk (i.e., a retransmission), the same
// RE-CONFIG chunk MUST to be sent back in response, as it was earlier."
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "req=" << *req_seq_nbr
<< " already processed";
<< " already processed, returning result="
<< ToString(last_processed_req_result_);
responses.push_back(ReconfigurationResponseParameter(
req_seq_nbr, ResponseResult::kSuccessNothingToDo));
req_seq_nbr, last_processed_req_result_));
return false;
}
@ -170,20 +175,18 @@ void StreamResetHandler::HandleResetOutgoing(
}
if (ValidateReqSeqNbr(req->request_sequence_number(), responses)) {
ResponseResult result;
RTC_DLOG(LS_VERBOSE) << log_prefix_
<< "Reset outgoing streams with req_seq_nbr="
<< *req->request_sequence_number();
last_processed_req_seq_nbr_ = req->request_sequence_number();
result = reassembly_queue_->ResetStreams(
last_processed_req_result_ = reassembly_queue_->ResetStreams(
*req, data_tracker_->last_cumulative_acked_tsn());
if (result == ResponseResult::kSuccessPerformed) {
if (last_processed_req_result_ == ResponseResult::kSuccessPerformed) {
ctx_->callbacks().OnIncomingStreamsReset(req->stream_ids());
}
responses.push_back(ReconfigurationResponseParameter(
req->request_sequence_number(), result));
req->request_sequence_number(), last_processed_req_result_));
}
}

7
net/dcsctp/socket/stream_reset_handler.h
View File

@ -88,8 +88,9 @@ class StreamResetHandler {
last_processed_req_seq_nbr_(
handover_state ? ReconfigRequestSN(
handover_state->rx.last_completed_reset_req_sn)
: ReconfigRequestSN(*ctx_->peer_initial_tsn() - 1)) {
}
: ReconfigRequestSN(*ctx_->peer_initial_tsn() - 1)),
last_processed_req_result_(
ReconfigurationResponseParameter::Result::kSuccessNothingToDo) {}
// Initiates reset of the provided streams. While there can only be one
// ongoing stream reset request at any time, this method can be called at any
@ -224,6 +225,8 @@ class StreamResetHandler {
// For incoming requests - last processed request sequence number.
ReconfigRequestSN last_processed_req_seq_nbr_;
// The result from last processed incoming request
ReconfigurationResponseParameter::Result last_processed_req_result_;
};
} // namespace dcsctp

42
net/dcsctp/socket/stream_reset_handler_test.cc
View File

@ -586,36 +586,20 @@ TEST_F(StreamResetHandlerTest, SendIncomingResetJustReturnsNothingPerformed) {
EXPECT_THAT(responses[0].result(), ResponseResult::kSuccessNothingToDo);
}
TEST_F(StreamResetHandlerTest, SendSameRequestTwiceReturnsNothingToDo) {
reasm_->Add(kPeerInitialTsn, gen_.Ordered({1, 2, 3, 4}, "BE"));
reasm_->Add(AddTo(kPeerInitialTsn, 1), gen_.Ordered({1, 2, 3, 4}, "BE"));
TEST_F(StreamResetHandlerTest, SendSameRequestTwiceIsIdempotent) {
// Simulate that receiving the same chunk twice (due to network issues,
// or retransmissions, causing a RECONFIG to be re-received) is idempotent.
for (int i = 0; i < 2; ++i) {
Parameters::Builder builder;
builder.Add(OutgoingSSNResetRequestParameter(
kPeerInitialReqSn, ReconfigRequestSN(3), AddTo(kPeerInitialTsn, 1),
{StreamID(1)}));
data_tracker_->Observe(kPeerInitialTsn);
data_tracker_->Observe(AddTo(kPeerInitialTsn, 1));
EXPECT_THAT(reasm_->FlushMessages(),
UnorderedElementsAre(
SctpMessageIs(StreamID(1), PPID(53), kShortPayload),
SctpMessageIs(StreamID(1), PPID(53), kShortPayload)));
Parameters::Builder builder1;
builder1.Add(OutgoingSSNResetRequestParameter(
kPeerInitialReqSn, ReconfigRequestSN(3), AddTo(kPeerInitialTsn, 1),
{StreamID(1)}));
std::vector<ReconfigurationResponseParameter> responses1 =
HandleAndCatchResponse(ReConfigChunk(builder1.Build()));
EXPECT_THAT(responses1, SizeIs(1));
EXPECT_EQ(responses1[0].result(), ResponseResult::kSuccessPerformed);
Parameters::Builder builder2;
builder2.Add(OutgoingSSNResetRequestParameter(
kPeerInitialReqSn, ReconfigRequestSN(3), AddTo(kPeerInitialTsn, 1),
{StreamID(1)}));
std::vector<ReconfigurationResponseParameter> responses2 =
HandleAndCatchResponse(ReConfigChunk(builder2.Build()));
EXPECT_THAT(responses2, SizeIs(1));
EXPECT_EQ(responses2[0].result(), ResponseResult::kSuccessNothingToDo);
std::vector<ReconfigurationResponseParameter> responses1 =
HandleAndCatchResponse(ReConfigChunk(builder.Build()));
EXPECT_THAT(responses1, SizeIs(1));
EXPECT_EQ(responses1[0].result(), ResponseResult::kInProgress);
}
}
TEST_F(StreamResetHandlerTest,