From e0b45c268e26ca97e0a8b71fc04416bf5ddddb63 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Thu, 18 Aug 2022 21:12:31 +0000 Subject: [PATCH] dcsctp: Expose negotiated stream counts To allow the transport to be able to know which ranges of stream identifiers it can use, the negotiated incoming/inbound and outgoing/outbound stream counts will be exposed. They are added to Metrics, and guaranteed to be available from within the OnConnected callback. In this CL, dcSCTP will not validate that the client is sending on a stream that is within the negotiated bounds. That will be done as a follow-up CL. Bug: webrtc:14277 Change-Id: Ic764e5f93f53d55633ee547df86246022f4097cf Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272321 Reviewed-by: Harald Alvestrand Commit-Queue: Victor Boivie Cr-Commit-Position: refs/heads/main@{#37876} --- net/dcsctp/public/dcsctp_socket.h | 7 ++++ net/dcsctp/socket/capabilities.h | 4 +++ net/dcsctp/socket/dcsctp_socket.cc | 32 ++++++++++++++++--- net/dcsctp/socket/dcsctp_socket_test.cc | 24 ++++++++++++++ net/dcsctp/socket/state_cookie.cc | 4 +++ net/dcsctp/socket/state_cookie.h | 2 +- net/dcsctp/socket/state_cookie_test.cc | 18 +++++++---- .../socket/transmission_control_block.cc | 6 ++++ 8 files changed, 86 insertions(+), 11 deletions(-) diff --git a/net/dcsctp/public/dcsctp_socket.h b/net/dcsctp/public/dcsctp_socket.h index 8506397581..2df6a2c009 100644 --- a/net/dcsctp/public/dcsctp_socket.h +++ b/net/dcsctp/public/dcsctp_socket.h @@ -244,6 +244,13 @@ struct Metrics { // Indicates if RFC8260 User Message Interleaving has been negotiated by both // peers. bool uses_message_interleaving = false; + + // The number of negotiated incoming and outgoing streams, which is configured + // locally as `DcSctpOptions::announced_maximum_incoming_streams` and + // `DcSctpOptions::announced_maximum_outgoing_streams`, and which will be + // signaled by the peer during connection. + uint16_t negotiated_maximum_incoming_streams = 0; + uint16_t negotiated_maximum_outgoing_streams = 0; }; // Callbacks that the DcSctpSocket will call synchronously to the owning diff --git a/net/dcsctp/socket/capabilities.h b/net/dcsctp/socket/capabilities.h index c6d3692b2d..fa3be37d12 100644 --- a/net/dcsctp/socket/capabilities.h +++ b/net/dcsctp/socket/capabilities.h @@ -10,6 +10,7 @@ #ifndef NET_DCSCTP_SOCKET_CAPABILITIES_H_ #define NET_DCSCTP_SOCKET_CAPABILITIES_H_ +#include namespace dcsctp { // Indicates what the association supports, meaning that both parties // support it and that feature can be used. @@ -20,6 +21,9 @@ struct Capabilities { bool message_interleaving = false; // RFC6525 Stream Reconfiguration bool reconfig = false; + // Negotiated maximum incoming and outgoing stream count. + uint16_t negotiated_maximum_incoming_streams = 0; + uint16_t negotiated_maximum_outgoing_streams = 0; }; } // namespace dcsctp diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index 53838193ec..78c88c36b9 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -90,8 +90,10 @@ constexpr uint32_t kMaxVerificationTag = std::numeric_limits::max(); constexpr uint32_t kMinInitialTsn = 0; constexpr uint32_t kMaxInitialTsn = std::numeric_limits::max(); -Capabilities GetCapabilities(const DcSctpOptions& options, - const Parameters& parameters) { +Capabilities ComputeCapabilities(const DcSctpOptions& options, + uint16_t peer_nbr_outbound_streams, + uint16_t peer_nbr_inbound_streams, + const Parameters& parameters) { Capabilities capabilities; absl::optional supported_extensions = parameters.get(); @@ -114,6 +116,12 @@ Capabilities GetCapabilities(const DcSctpOptions& options, supported_extensions->supports(ReConfigChunk::kType)) { capabilities.reconfig = true; } + + capabilities.negotiated_maximum_incoming_streams = std::min( + options.announced_maximum_incoming_streams, peer_nbr_outbound_streams); + capabilities.negotiated_maximum_outgoing_streams = std::min( + options.announced_maximum_outgoing_streams, peer_nbr_inbound_streams); + return capabilities; } @@ -312,6 +320,10 @@ void DcSctpSocket::CreateTransmissionControlBlock( size_t a_rwnd, TieTag tie_tag) { metrics_.uses_message_interleaving = capabilities.message_interleaving; + metrics_.negotiated_maximum_incoming_streams = + capabilities.negotiated_maximum_incoming_streams; + metrics_.negotiated_maximum_outgoing_streams = + capabilities.negotiated_maximum_outgoing_streams; tcb_ = std::make_unique( timer_manager_, log_prefix_, options_, capabilities, callbacks_, send_queue_, my_verification_tag, my_initial_tsn, peer_verification_tag, @@ -339,6 +351,10 @@ void DcSctpSocket::RestoreFromState(const DcSctpSocketHandoverState& state) { capabilities.message_interleaving = state.capabilities.message_interleaving; capabilities.reconfig = state.capabilities.reconfig; + capabilities.negotiated_maximum_incoming_streams = + state.capabilities.negotiated_maximum_incoming_streams; + capabilities.negotiated_maximum_outgoing_streams = + state.capabilities.negotiated_maximum_outgoing_streams; send_queue_.RestoreFromState(state); @@ -578,6 +594,10 @@ absl::optional DcSctpSocket::GetMetrics() const { (send_queue_.total_buffered_amount() + packet_payload_size - 1) / packet_payload_size; metrics.peer_rwnd_bytes = tcb_->retransmission_queue().rwnd(); + metrics.negotiated_maximum_incoming_streams = + tcb_->capabilities().negotiated_maximum_incoming_streams; + metrics.negotiated_maximum_incoming_streams = + tcb_->capabilities().negotiated_maximum_incoming_streams; return metrics; } @@ -1172,7 +1192,9 @@ void DcSctpSocket::HandleInit(const CommonHeader& header, *connect_params_.verification_tag, *connect_params_.initial_tsn, *chunk->initiate_tag(), *chunk->initial_tsn()); - Capabilities capabilities = GetCapabilities(options_, chunk->parameters()); + Capabilities capabilities = + ComputeCapabilities(options_, chunk->nbr_outbound_streams(), + chunk->nbr_inbound_streams(), chunk->parameters()); SctpPacket::Builder b(chunk->initiate_tag(), options_); Parameters::Builder params_builder = @@ -1221,7 +1243,9 @@ void DcSctpSocket::HandleInitAck( "InitAck chunk doesn't contain a cookie"); return; } - Capabilities capabilities = GetCapabilities(options_, chunk->parameters()); + Capabilities capabilities = + ComputeCapabilities(options_, chunk->nbr_outbound_streams(), + chunk->nbr_inbound_streams(), chunk->parameters()); t1_init_->Stop(); metrics_.peer_implementation = DeterminePeerImplementation(cookie->data()); diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index d1e2d904e0..ab854da290 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -2668,5 +2668,29 @@ TEST(DcSctpSocketTest, LifecycleEventsForExpiredMessageWithLifetimeLimit) { EXPECT_THAT(GetReceivedMessagePpids(z), IsEmpty()); } +TEST_P(DcSctpSocketParametrizedTest, ExposesTheNumberOfNegotiatedStreams) { + DcSctpOptions options_a = { + .announced_maximum_incoming_streams = 12, + .announced_maximum_outgoing_streams = 45, + }; + SocketUnderTest a("A", options_a); + + DcSctpOptions options_z = { + .announced_maximum_incoming_streams = 23, + .announced_maximum_outgoing_streams = 34, + }; + auto z = std::make_unique("Z", options_z); + + ConnectSockets(a, *z); + z = MaybeHandoverSocket(std::move(z)); + + ASSERT_HAS_VALUE_AND_ASSIGN(Metrics metrics_a, a.socket.GetMetrics()); + EXPECT_EQ(metrics_a.negotiated_maximum_incoming_streams, 12); + EXPECT_EQ(metrics_a.negotiated_maximum_outgoing_streams, 23); + + ASSERT_HAS_VALUE_AND_ASSIGN(Metrics metrics_z, z->socket.GetMetrics()); + EXPECT_EQ(metrics_z.negotiated_maximum_incoming_streams, 23); + EXPECT_EQ(metrics_z.negotiated_maximum_outgoing_streams, 12); +} } // namespace } // namespace dcsctp diff --git a/net/dcsctp/socket/state_cookie.cc b/net/dcsctp/socket/state_cookie.cc index 7d04cbb0d7..86be77aa34 100644 --- a/net/dcsctp/socket/state_cookie.cc +++ b/net/dcsctp/socket/state_cookie.cc @@ -40,6 +40,8 @@ std::vector StateCookie::Serialize() { buffer.Store8<28>(capabilities_.partial_reliability); buffer.Store8<29>(capabilities_.message_interleaving); buffer.Store8<30>(capabilities_.reconfig); + buffer.Store16<32>(capabilities_.negotiated_maximum_incoming_streams); + buffer.Store16<34>(capabilities_.negotiated_maximum_outgoing_streams); return cookie; } @@ -70,6 +72,8 @@ absl::optional StateCookie::Deserialize( capabilities.partial_reliability = buffer.Load8<28>() != 0; capabilities.message_interleaving = buffer.Load8<29>() != 0; capabilities.reconfig = buffer.Load8<30>() != 0; + capabilities.negotiated_maximum_incoming_streams = buffer.Load16<32>(); + capabilities.negotiated_maximum_outgoing_streams = buffer.Load16<34>(); return StateCookie(verification_tag, initial_tsn, a_rwnd, tie_tag, capabilities); diff --git a/net/dcsctp/socket/state_cookie.h b/net/dcsctp/socket/state_cookie.h index df4b801397..a26dbf86f7 100644 --- a/net/dcsctp/socket/state_cookie.h +++ b/net/dcsctp/socket/state_cookie.h @@ -27,7 +27,7 @@ namespace dcsctp { // Do not trust anything in it; no pointers or anything like that. class StateCookie { public: - static constexpr size_t kCookieSize = 31; + static constexpr size_t kCookieSize = 36; StateCookie(VerificationTag initiate_tag, TSN initial_tsn, diff --git a/net/dcsctp/socket/state_cookie_test.cc b/net/dcsctp/socket/state_cookie_test.cc index 870620ba14..7d8e1339ee 100644 --- a/net/dcsctp/socket/state_cookie_test.cc +++ b/net/dcsctp/socket/state_cookie_test.cc @@ -18,9 +18,11 @@ namespace { using ::testing::SizeIs; TEST(StateCookieTest, SerializeAndDeserialize) { - Capabilities capabilities = {/*partial_reliability=*/true, - /*message_interleaving=*/false, - /*reconfig=*/true}; + Capabilities capabilities = {.partial_reliability = true, + .message_interleaving = false, + .reconfig = true, + .negotiated_maximum_incoming_streams = 123, + .negotiated_maximum_outgoing_streams = 234}; StateCookie cookie(VerificationTag(123), TSN(456), /*a_rwnd=*/789, TieTag(101112), capabilities); std::vector serialized = cookie.Serialize(); @@ -34,12 +36,16 @@ TEST(StateCookieTest, SerializeAndDeserialize) { EXPECT_TRUE(deserialized.capabilities().partial_reliability); EXPECT_FALSE(deserialized.capabilities().message_interleaving); EXPECT_TRUE(deserialized.capabilities().reconfig); + EXPECT_EQ(deserialized.capabilities().negotiated_maximum_incoming_streams, + 123); + EXPECT_EQ(deserialized.capabilities().negotiated_maximum_outgoing_streams, + 234); } TEST(StateCookieTest, ValidateMagicValue) { - Capabilities capabilities = {/*partial_reliability=*/true, - /*message_interleaving=*/false, - /*reconfig=*/true}; + Capabilities capabilities = {.partial_reliability = true, + .message_interleaving = false, + .reconfig = true}; StateCookie cookie(VerificationTag(123), TSN(456), /*a_rwnd=*/789, TieTag(101112), capabilities); std::vector serialized = cookie.Serialize(); diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index d769e26069..1dcf394813 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -274,6 +274,8 @@ std::string TransmissionControlBlock::ToString() const { if (capabilities_.reconfig) { sb << "Reconfig,"; } + sb << " max_in=" << capabilities_.negotiated_maximum_incoming_streams; + sb << " max_out=" << capabilities_.negotiated_maximum_outgoing_streams; return sb.Release(); } @@ -292,6 +294,10 @@ void TransmissionControlBlock::AddHandoverState( state.capabilities.partial_reliability = capabilities_.partial_reliability; state.capabilities.message_interleaving = capabilities_.message_interleaving; state.capabilities.reconfig = capabilities_.reconfig; + state.capabilities.negotiated_maximum_incoming_streams = + capabilities_.negotiated_maximum_incoming_streams; + state.capabilities.negotiated_maximum_outgoing_streams = + capabilities_.negotiated_maximum_outgoing_streams; state.my_verification_tag = my_verification_tag().value(); state.peer_verification_tag = peer_verification_tag().value();