SCTP part 8 (#1770)

Author: ibc

worker/include/RTC/SCTP/TODO_SCTP.md

@@ -16,8 +16,6 @@
 
 - In `Association::FillBuffer()` we should not pass `this->sctpOptions.maxOutboundStreams/maxInboundStreams` but the current values (they may have been modified via "reconfig").
 
-- Probably remove those `MS_DEBUG_TAG(sctp, "xxxx timer has expired")` and make it be `MS_DEBUG_DEV()` instead.
-
 - Test Chrome with I-DATA (message interleaving):
 
   ```
@@ -27,8 +25,3 @@
   ```
 
 - Look for "TODO: SCTP" and `MS_SCTP_STACK` everywhere.
-
-## Related to dcsctp
-
-- Investigate `DcSctpSocket::HandleTimeout()` which is only called from `media/sctp/dcsctp_transport.cc`.
-  - Update: This is the entry point when a timer expires. It's the same as our `OnTimer()`.

worker/include/RTC/SCTP/association/Association.hpp

@@ -374,64 +374,64 @@ namespace RTC
 
 			bool ValidateReceivedPacket(const Packet* receivedPacket);
 
-			bool ProcessReceivedChunk(const Packet* receivedPacket, const Chunk* receivedChunk);
+			bool HandleReceivedChunk(const Packet* receivedPacket, const Chunk* receivedChunk);
 
-			void ProcessReceivedInitChunk(const Packet* receivedPacket, const InitChunk* receivedInitChunk);
+			void HandleReceivedInitChunk(const Packet* receivedPacket, const InitChunk* receivedInitChunk);
 
-			void ProcessReceivedInitAckChunk(
+			void HandleReceivedInitAckChunk(
 			  const Packet* receivedPacket, const InitAckChunk* receivedInitAckChunk);
 
-			void ProcessReceivedCookieEchoChunk(
+			void HandleReceivedCookieEchoChunk(
 			  const Packet* receivedPacket, const CookieEchoChunk* receivedCookieEchoChunk);
 
-			bool ProcessReceivedCookieEchoChunkWithTcb(const Packet* receivedPacket, const StateCookie* cookie);
+			bool HandleReceivedCookieEchoChunkWithTcb(const Packet* receivedPacket, const StateCookie* cookie);
 
-			void ProcessReceivedCookieAckChunk(
+			void HandleReceivedCookieAckChunk(
 			  const Packet* receivedPacket, const CookieAckChunk* receivedCookieAckChunk);
 
-			void ProcessReceivedShutdownChunk(
+			void HandleReceivedShutdownChunk(
 			  const Packet* receivedPacket, const ShutdownChunk* receivedShutdownChunk);
 
-			void ProcessReceivedShutdownAckChunk(
+			void HandleReceivedShutdownAckChunk(
 			  const Packet* receivedPacket, const ShutdownAckChunk* receivedShutdownAckChunk);
 
-			void ProcessReceivedShutdownCompleteChunk(
+			void HandleReceivedShutdownCompleteChunk(
 			  const Packet* receivedPacket, const ShutdownCompleteChunk* receivedShutdownCompleteChunk);
 
-			void ProcessReceivedOperationErrorChunk(
+			void HandleReceivedOperationErrorChunk(
 			  const Packet* receivedPacket, const OperationErrorChunk* receivedOperationErrorChunk);
 
-			void ProcessReceivedAbortAssociationChunk(
+			void HandleReceivedAbortAssociationChunk(
 			  const Packet* receivedPacket, const AbortAssociationChunk* receivedAbortAssociationChunk);
 
-			void ProcessReceivedHeartbeatRequestChunk(
+			void HandleReceivedHeartbeatRequestChunk(
 			  const Packet* receivedPacket, const HeartbeatRequestChunk* receivedHeartbeatRequestChunk);
 
-			void ProcessReceivedHeartbeatAckChunk(
+			void HandleReceivedHeartbeatAckChunk(
 			  const Packet* receivedPacket, const HeartbeatAckChunk* receivedHeartbeatAckChunk);
 
-			void ProcessReceivedReConfigChunk(
+			void HandleReceivedReConfigChunk(
 			  const Packet* receivedPacket, const ReConfigChunk* receivedReConfigChunk);
 
-			void ProcessReceivedForwardTsnChunk(
+			void HandleReceivedForwardTsnChunk(
 			  const Packet* receivedPacket, const ForwardTsnChunk* receivedForwardTsnChunk);
 
-			void ProcessReceivedIForwardTsnChunk(
+			void HandleReceivedIForwardTsnChunk(
 			  const Packet* receivedPacket, const IForwardTsnChunk* receivedIForwardTsnChunk);
 
-			void ProcessReceivedAnyForwardTsnChunk(
+			void HandleReceivedAnyForwardTsnChunk(
 			  const Packet* receivedPacket, const AnyForwardTsnChunk* receivedAnyForwardTsnChunk);
 
-			void ProcessReceivedDataChunk(const Packet* receivedPacket, const DataChunk* receivedDataChunk);
+			void HandleReceivedDataChunk(const Packet* receivedPacket, const DataChunk* receivedDataChunk);
 
-			void ProcessReceivedIDataChunk(const Packet* receivedPacket, const IDataChunk* receivedIDataChunk);
+			void HandleReceivedIDataChunk(const Packet* receivedPacket, const IDataChunk* receivedIDataChunk);
 
-			void ProcessReceivedAnyDataChunk(
+			void HandleReceivedAnyDataChunk(
 			  const Packet* receivedPacket, const AnyDataChunk* receivedAnyDataChunk);
 
-			void ProcessReceivedSackChunk(const Packet* receivedPacket, const SackChunk* receivedSackChunk);
+			void HandleReceivedSackChunk(const Packet* receivedPacket, const SackChunk* receivedSackChunk);
 
-			bool ProcessReceivedUnknownChunk(
+			bool HandleReceivedUnknownChunk(
 			  const Packet* receivedPacket, const UnknownChunk* receivedUnknownChunk);
 
 			void OnT1InitTimer(uint64_t& baseTimeoutMs, bool& stop);

worker/include/RTC/SCTP/association/HeartbeatHandler.hpp

@@ -42,13 +42,13 @@ namespace RTC
 			/**
 			 * Called on received HEARTBEAT_REQUEST Chunk.
 			 */
-			void ProcessReceivedHeartbeatRequestChunk(
+			void HandleReceivedHeartbeatRequestChunk(
 			  const HeartbeatRequestChunk* receivedHeartbeatRequestChunk);
 
 			/**
 			 * Called on received HEARTBEAT_ACK Chunk.
 			 */
-			void ProcessReceivedHeartbeatAckChunk(const HeartbeatAckChunk* receivedHeartbeatAckChunk);
+			void HandleReceivedHeartbeatAckChunk(const HeartbeatAckChunk* receivedHeartbeatAckChunk);
 
 		private:
 			void OnIntervalTimer(uint64_t& baseTimeoutMs, bool& stop);

worker/include/RTC/SCTP/association/StreamResetHandler.hpp

@@ -4,7 +4,10 @@
 #include "common.hpp"
 #include "RTC/SCTP/association/TCBContext.hpp"
 #include "RTC/SCTP/common/UnwrappedSequenceNumber.hpp"
+#include "RTC/SCTP/packet/Packet.hpp"
 #include "RTC/SCTP/packet/chunks/ReConfigChunk.hpp"
+#include "RTC/SCTP/packet/parameters/IncomingSsnResetRequestParameter.hpp"
+#include "RTC/SCTP/packet/parameters/OutgoingSsnResetRequestParameter.hpp"
 #include "RTC/SCTP/packet/parameters/ReconfigurationResponseParameter.hpp"
 #include "RTC/SCTP/public/AssociationListener.hpp"
 #include "handles/BackoffTimerHandle.hpp"
@@ -53,7 +56,7 @@ namespace RTC
 			{
 				VALID,
 				RETRANSMISSION,
-				BADSEQUENCE_NUMBER,
+				BAD_SEQUENCE_NUMBER,
 			};
 
 			/**
@@ -156,6 +159,9 @@ namespace RTC
 				bool isDeferred{ false };
 			};
 
+		private:
+			using UnwrappedReConfigRequestSn = UnwrappedSequenceNumber<uint32_t>;
+
 		public:
 			StreamResetHandler(
 			  AssociationListener& associationListener, TCBContext* tcbContext
@@ -177,23 +183,66 @@ namespace RTC
 			 */
 			void ResetStreams(std::span<const uint16_t> outgoingStreamIds);
 
+			/**
+			 * Called when handling and incoming RE-CONFIG chunk. Processes a stream
+			 * reconfiguration chunk and may send a RE-CONFIG back to the peer with
+			 * either 1 or 2 responses.
+			 */
+			void HandleReceivedReConfigChunk(const ReConfigChunk* receivedReConfigChunk);
+
+		private:
+			/**
+			 * Called to validate a received RE-CONFIG chunk.
+			 */
+			bool ValidateReceivedReConfigChunk(const ReConfigChunk* receivedReConfigChunk);
+
 			/**
 			 * Creates a Reset Streams request that must be sent if returned. Will
 			 * start the reconfig timer. Will return `nullptr` if there is no need
 			 * to create a request (no streams to reset) or if there already is an
 			 * ongoing stream reset request that hasn't completed yet.
 			 */
-			// TODO: SCTP: Do we really want to return a Chunk? Maybe we should pass
-			// a Packet to use BuilChunkInPlace().
-			ReConfigChunk* MakeStreamResetRequest();
+			ReConfigChunk* CreateStreamResetRequest();
 
 			/**
-			 * Called when handling and incoming RE-CONFIG chunk.
+			 * Creates the actual RE-CONFIG chunk. A request (which set
+			 * `currentRequest`) must have been created prior.
 			 */
-			void ProcessReceivedReConfigChunk(const ReConfigChunk* receivedReConfigChunk);
+			ReConfigChunk* CreateReconfigChunk();
 
-		private:
-			void OnReconfigTimer(uint64_t& baseTimeoutMs, bool& stop);
+			/**
+			 * Called to validate the `reqSeqNbr`, that it's the next in sequence.
+			 */
+			ReqSeqNbrValidationResult ValidateReqSeqNbr(UnwrappedReConfigRequestSn reqSeqNbr);
+
+			/**
+			 * Called when this Association receives an outgoing stream reset request.
+			 * It might either be performed straight away, or have to be deferred, and
+			 * the result of that will be put in `responses`.
+			 */
+			void HandleReceivedOutgoingSsnResetRequestParameter(
+			  const OutgoingSsnResetRequestParameter* receivedOutgoingSsnResetRequestParameter,
+			  ReConfigChunk* reConfigChunk);
+
+			/**
+			 * Called when this Association receives an incoming stream reset request.
+			 * This isn't really supported, but a successful response is put in
+			 * `responses`.
+			 */
+			void HandleReceivedIncomingSsnResetRequestParameter(
+			  const IncomingSsnResetRequestParameter* receivedIncomingSsnResetRequestParameter,
+			  ReConfigChunk* reConfigChunk);
+
+			/**
+			 * Called when receiving a response to an outgoing stream reset request.
+			 * It will either commit the stream resetting, if the operation was
+			 * successful, or will schedule a retry if it was deferred. And if it
+			 * failed, the operation will be rolled back.
+			 */
+			void HandleReceivedReconfigurationResponseParameter(
+			  const ReconfigurationResponseParameter* receivedReconfigurationResponseParameter);
+
+			void OnReConfigTimer(uint64_t& baseTimeoutMs, bool& stop);
 
 			/* Pure virtual methods inherited from BackoffTimerHandle::Listener. */
 		public:
@@ -206,14 +255,14 @@ namespace RTC
 			// DataTracker* dataTracker{ nullptr };,
 			// ReassemblyQueue* reassemblyQueue{ nullptr };,
 			// RetransmissionQueue* retransmissionQueue{ nullptr };
-			UnwrappedSequenceNumber<uint32_t>::Unwrapper incomingReconfigRequestSnUnwrapper;
-			const std::unique_ptr<BackoffTimerHandle> reconfigTimer;
+			UnwrappedReConfigRequestSn::Unwrapper incomingReConfigRequestSnUnwrapper;
+			const std::unique_ptr<BackoffTimerHandle> reConfigTimer;
 			// The next sequence number for outgoing stream requests.
 			uint32_t nextOutgoingReqSeqNbr{ 0 };
 			// The current stream request operation.
 			std::optional<CurrentRequest> currentRequest;
 			// For incoming requests. Last processed request sequence number.
-			UnwrappedSequenceNumber<uint32_t> lastProcessedReqSeqNbr;
+			UnwrappedReConfigRequestSn lastProcessedReqSeqNbr;
 			// The result from last processed incoming request.
 			ReconfigurationResponseParameter::Result lastProcessedReqResult;
 		};

worker/include/RTC/SCTP/packet/UserData.hpp

@@ -105,6 +105,7 @@ namespace RTC
 			 */
 			std::vector<uint8_t> ReleasePayload() &&
 			{
+				// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
 				return std::move(this->payload);
 			}
 

worker/include/RTC/SCTP/packet/parameters/IncomingSsnResetRequestParameter.hpp

@@ -4,6 +4,7 @@
 #include "common.hpp"
 #include "Utils.hpp"
 #include "RTC/SCTP/packet/Parameter.hpp"
+#include <vector>
 
 namespace RTC
 {
@@ -88,17 +89,9 @@ namespace RTC
 
 			void SetReconfigurationRequestSequenceNumber(uint32_t value);
 
-			uint16_t GetNumberOfStreams() const
-			{
-				return GetVariableLengthValueLength() / 2;
-			}
-
-			uint16_t GetStreamAt(uint16_t idx) const
-			{
-				return Utils::Byte::Get2Bytes(GetVariableLengthValuePointer(), (idx * 2));
-			}
+			std::vector<uint16_t> GetStreamIds() const;
 
-			void AddStream(uint16_t stream);
+			void AddStreamId(uint16_t streamId);
 
 		protected:
 			IncomingSsnResetRequestParameter* SoftClone(const uint8_t* buffer) const final;
@@ -111,6 +104,17 @@ namespace RTC
 			{
 				return IncomingSsnResetRequestParameter::IncomingSsnResetRequestParameterHeaderLength;
 			}
+
+		private:
+			uint16_t GetNumberOfStreams() const
+			{
+				return GetVariableLengthValueLength() / 2;
+			}
+
+			uint16_t GetStreamAt(uint16_t idx) const
+			{
+				return Utils::Byte::Get2Bytes(GetVariableLengthValuePointer(), (idx * 2));
+			}
 		};
 	} // namespace SCTP
 } // namespace RTC

worker/include/RTC/SCTP/packet/parameters/OutgoingSsnResetRequestParameter.hpp

@@ -4,6 +4,7 @@
 #include "common.hpp"
 #include "Utils.hpp"
 #include "RTC/SCTP/packet/Parameter.hpp"
+#include <vector>
 
 namespace RTC
 {
@@ -106,17 +107,9 @@ namespace RTC
 
 			void SetSenderLastAssignedTsn(uint32_t value);
 
-			uint16_t GetNumberOfStreams() const
-			{
-				return GetVariableLengthValueLength() / 2;
-			}
-
-			uint16_t GetStreamAt(uint16_t idx) const
-			{
-				return Utils::Byte::Get2Bytes(GetVariableLengthValuePointer(), (idx * 2));
-			}
+			std::vector<uint16_t> GetStreamIds() const;
 
-			void AddStream(uint16_t stream);
+			void AddStreamId(uint16_t streamId);
 
 		protected:
 			OutgoingSsnResetRequestParameter* SoftClone(const uint8_t* buffer) const final;
@@ -129,6 +122,17 @@ namespace RTC
 			{
 				return OutgoingSsnResetRequestParameter::OutgoingSsnResetRequestParameterHeaderLength;
 			}
+
+		private:
+			uint16_t GetNumberOfStreams() const
+			{
+				return GetVariableLengthValueLength() / 2;
+			}
+
+			uint16_t GetStreamAt(uint16_t idx) const
+			{
+				return Utils::Byte::Get2Bytes(GetVariableLengthValuePointer(), (idx * 2));
+			}
 		};
 	} // namespace SCTP
 } // namespace RTC

worker/include/RTC/SCTP/packet/parameters/ZeroChecksumAcceptableParameter.hpp

@@ -42,7 +42,7 @@ namespace RTC
 			enum class AlternateErrorDetectionMethod : uint32_t
 			{
 				NONE           = 0x0000,
-				SCTP_OVER_DTLS = 0x0001
+				SCTP_OVER_DTLS = 0x0001,
 			};
 
 		public:
@@ -96,7 +96,18 @@ namespace RTC
 
 			AlternateErrorDetectionMethod GetAlternateErrorDetectionMethod() const
 			{
-				return static_cast<AlternateErrorDetectionMethod>(Utils::Byte::Get4Bytes(GetBuffer(), 4));
+				const auto method = Utils::Byte::Get4Bytes(GetBuffer(), 4);
+
+				if (
+				  method == static_cast<uint32_t>(AlternateErrorDetectionMethod::NONE) ||
+				  method == static_cast<uint32_t>(AlternateErrorDetectionMethod::SCTP_OVER_DTLS))
+				{
+					return static_cast<AlternateErrorDetectionMethod>(method);
+				}
+				else
+				{
+					return AlternateErrorDetectionMethod::NONE;
+				}
 			}
 
 			void SetAlternateErrorDetectionMethod(AlternateErrorDetectionMethod alternateErrorDetectionMethod);

worker/include/RTC/SCTP/public/Message.hpp

@@ -71,6 +71,7 @@ namespace RTC
 			 */
 			std::vector<uint8_t> ReleasePayload() &&
 			{
+				// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
 				return std::move(this->payload);
 			}