From 8c72cc1634db39fcded8d6bd79089f8aeee27415 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Fri, 6 May 2022 12:32:01 +0200 Subject: [PATCH] dcsctp: Handle in-progress stream sequence numbers When an outgoing stream reset is sent, with sequence number A, and the receiver can't perform it immediately, it will return IN_PROGRESS with response sequence number A. This is then retried with sequence number A+1, and the peer would then possibly respond PERFORMED with response sequence number A+1. Before this CL, whenever a request was sent that didn't immediately succeed, it wouldn't increment its expected response sequence number. So in the retry above, the socket would still expect the response sequence number to stay at A, not at A+1. Bug: webrtc:13994 Change-Id: I6f36d45229a7fb312e97ad15826e0377f4efb64f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261310 Reviewed-by: Florent Castelli Commit-Queue: Victor Boivie Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#36797} --- net/dcsctp/socket/stream_reset_handler.cc | 2 +- .../socket/stream_reset_handler_test.cc | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/net/dcsctp/socket/stream_reset_handler.cc b/net/dcsctp/socket/stream_reset_handler.cc index 2d66658c95..9d86953f44 100644 --- a/net/dcsctp/socket/stream_reset_handler.cc +++ b/net/dcsctp/socket/stream_reset_handler.cc @@ -176,10 +176,10 @@ void StreamResetHandler::HandleResetOutgoing( << "Reset outgoing streams with req_seq_nbr=" << *req->request_sequence_number(); + last_processed_req_seq_nbr_ = req->request_sequence_number(); result = reassembly_queue_->ResetStreams( *req, data_tracker_->last_cumulative_acked_tsn()); if (result == ResponseResult::kSuccessPerformed) { - last_processed_req_seq_nbr_ = req->request_sequence_number(); ctx_->callbacks().OnIncomingStreamsReset(req->stream_ids()); } responses.push_back(ReconfigurationResponseParameter( diff --git a/net/dcsctp/socket/stream_reset_handler_test.cc b/net/dcsctp/socket/stream_reset_handler_test.cc index 1b7463e8d7..a9a8b36bf7 100644 --- a/net/dcsctp/socket/stream_reset_handler_test.cc +++ b/net/dcsctp/socket/stream_reset_handler_test.cc @@ -762,5 +762,37 @@ TEST_F(StreamResetHandlerTest, HandoverAfterHavingResetOneStream) { } } +TEST_F(StreamResetHandlerTest, PerformCloseAfterOneFirstFailing) { + // Inject a stream reset on the first expected TSN (which hasn't been seen). + Parameters::Builder builder; + builder.Add(OutgoingSSNResetRequestParameter( + kPeerInitialReqSn, ReconfigRequestSN(3), kPeerInitialTsn, {StreamID(1)})); + + // The socket is expected to say "in progress" as that TSN hasn't been seen. + std::vector responses = + HandleAndCatchResponse(ReConfigChunk(builder.Build())); + EXPECT_THAT(responses, SizeIs(1)); + EXPECT_EQ(responses[0].result(), ResponseResult::kInProgress); + + // Let the socket receive the TSN. + DataGeneratorOptions opts; + opts.message_id = MID(0); + reasm_->Add(kPeerInitialTsn, gen_.Ordered({1, 2, 3, 4}, "BE", opts)); + reasm_->MaybeResetStreamsDeferred(kPeerInitialTsn); + data_tracker_->Observe(kPeerInitialTsn); + + // And emulate that time has passed, and the peer retries the stream reset, + // but now with an incremented request sequence number. + Parameters::Builder builder2; + builder2.Add(OutgoingSSNResetRequestParameter( + ReconfigRequestSN(*kPeerInitialReqSn + 1), ReconfigRequestSN(3), + kPeerInitialTsn, {StreamID(1)})); + + // This is supposed to be handled well. + std::vector responses2 = + HandleAndCatchResponse(ReConfigChunk(builder2.Build())); + EXPECT_THAT(responses2, SizeIs(1)); + EXPECT_EQ(responses2[0].result(), ResponseResult::kSuccessPerformed); +} } // namespace } // namespace dcsctp