Implement max-channels for SCTP datachannels.
This involves catching another callback from usrsctp.
It also moves the definition of "connected" a little later
in the sequence: From "ready to send data" to the reception
of the SCTP_COMM_UP event.
Bug: chromium:943976
Change-Id: Ib9e1b17d0cc356f19cdfa675159b29bf1efdcb55
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/137435
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28004}
diff --git a/media/sctp/sctp_transport.cc b/media/sctp/sctp_transport.cc
index 0858401..7c60832 100644
--- a/media/sctp/sctp_transport.cc
+++ b/media/sctp/sctp_transport.cc
@@ -1047,7 +1047,12 @@
RTC_DCHECK_RUN_ON(network_thread_);
switch (change.sac_state) {
case SCTP_COMM_UP:
- RTC_LOG(LS_VERBOSE) << "Association change SCTP_COMM_UP";
+ RTC_LOG(LS_VERBOSE) << "Association change SCTP_COMM_UP, stream # is "
+ << change.sac_outbound_streams << " outbound, "
+ << change.sac_inbound_streams << " inbound.";
+ max_outbound_streams_ = change.sac_outbound_streams;
+ max_inbound_streams_ = change.sac_inbound_streams;
+ SignalAssociationChangeCommunicationUp();
break;
case SCTP_COMM_LOST:
RTC_LOG(LS_INFO) << "Association change SCTP_COMM_LOST";
diff --git a/media/sctp/sctp_transport.h b/media/sctp/sctp_transport.h
index 554407b..297379b 100644
--- a/media/sctp/sctp_transport.h
+++ b/media/sctp/sctp_transport.h
@@ -80,6 +80,12 @@
SendDataResult* result = nullptr) override;
bool ReadyToSendData() override;
int max_message_size() const override { return max_message_size_; }
+ absl::optional<int> max_outbound_streams() const override {
+ return max_outbound_streams_;
+ }
+ absl::optional<int> max_inbound_streams() const override {
+ return max_inbound_streams_;
+ }
void set_debug_name_for_testing(const char* debug_name) override {
debug_name_ = debug_name;
}
@@ -208,6 +214,9 @@
const char* debug_name_ = "SctpTransport";
// Hides usrsctp interactions from this header file.
class UsrSctpWrapper;
+ // Number of channels negotiated. Not set before negotiation completes.
+ absl::optional<int> max_outbound_streams_;
+ absl::optional<int> max_inbound_streams_;
RTC_DISALLOW_COPY_AND_ASSIGN(SctpTransport);
};
diff --git a/media/sctp/sctp_transport_internal.h b/media/sctp/sctp_transport_internal.h
index c08c414..378453a 100644
--- a/media/sctp/sctp_transport_internal.h
+++ b/media/sctp/sctp_transport_internal.h
@@ -114,8 +114,14 @@
virtual bool ReadyToSendData() = 0;
// Returns the current max message size, set with Start().
virtual int max_message_size() const = 0;
+ // Returns the current negotiated max # of outbound streams.
+ // Will return absl::nullopt if negotiation is incomplete.
+ virtual absl::optional<int> max_outbound_streams() const = 0;
+ // Returns the current negotiated max # of inbound streams.
+ virtual absl::optional<int> max_inbound_streams() const = 0;
sigslot::signal0<> SignalReadyToSendData;
+ sigslot::signal0<> SignalAssociationChangeCommunicationUp;
// ReceiveDataParams includes SID, seq num, timestamp, etc. CopyOnWriteBuffer
// contains message payload.
sigslot::signal2<const ReceiveDataParams&, const rtc::CopyOnWriteBuffer&>
diff --git a/pc/sctp_transport.cc b/pc/sctp_transport.cc
index 1c0289a9..6c4a8be 100644
--- a/pc/sctp_transport.cc
+++ b/pc/sctp_transport.cc
@@ -10,6 +10,7 @@
#include "pc/sctp_transport.h"
+#include <algorithm>
#include <utility>
namespace webrtc {
@@ -20,8 +21,8 @@
info_(SctpTransportState::kNew),
internal_sctp_transport_(std::move(internal)) {
RTC_DCHECK(internal_sctp_transport_.get());
- internal_sctp_transport_->SignalReadyToSendData.connect(
- this, &SctpTransport::OnInternalReadyToSendData);
+ internal_sctp_transport_->SignalAssociationChangeCommunicationUp.connect(
+ this, &SctpTransport::OnAssociationChangeCommunicationUp);
// TODO(https://bugs.webrtc.org/10360): Add handlers for transport closing.
if (dtls_transport_) {
@@ -143,7 +144,21 @@
}
}
-void SctpTransport::OnInternalReadyToSendData() {
+void SctpTransport::OnAssociationChangeCommunicationUp() {
+ RTC_DCHECK_RUN_ON(owner_thread_);
+ {
+ rtc::CritScope scope(&lock_);
+ RTC_DCHECK(internal_sctp_transport_);
+ if (internal_sctp_transport_->max_outbound_streams() &&
+ internal_sctp_transport_->max_inbound_streams()) {
+ int max_channels =
+ std::min(*(internal_sctp_transport_->max_outbound_streams()),
+ *(internal_sctp_transport_->max_inbound_streams()));
+ // Record max channels.
+ info_ = SctpTransportInformation(info_.state(), info_.dtls_transport(),
+ info_.MaxMessageSize(), max_channels);
+ }
+ }
UpdateInformation(SctpTransportState::kConnected);
}
diff --git a/pc/sctp_transport.h b/pc/sctp_transport.h
index 4a1c6f5..c7727df 100644
--- a/pc/sctp_transport.h
+++ b/pc/sctp_transport.h
@@ -62,6 +62,7 @@
private:
void UpdateInformation(SctpTransportState state);
void OnInternalReadyToSendData();
+ void OnAssociationChangeCommunicationUp();
void OnInternalClosingProcedureStartedRemotely(int sid);
void OnInternalClosingProcedureComplete(int sid);
diff --git a/pc/sctp_transport_unittest.cc b/pc/sctp_transport_unittest.cc
index 3438b17..ca57d6a 100644
--- a/pc/sctp_transport_unittest.cc
+++ b/pc/sctp_transport_unittest.cc
@@ -21,6 +21,7 @@
#include "test/gtest.h"
constexpr int kDefaultTimeout = 1000; // milliseconds
+constexpr int kTestMaxSctpStreams = 1234;
using cricket::FakeDtlsTransport;
using ::testing::ElementsAre;
@@ -45,9 +46,19 @@
bool ReadyToSendData() override { return true; }
void set_debug_name_for_testing(const char* debug_name) override {}
int max_message_size() const override { return 0; }
+ absl::optional<int> max_outbound_streams() const override {
+ return max_outbound_streams_;
+ }
+ absl::optional<int> max_inbound_streams() const override {
+ return max_inbound_streams_;
+ }
// Methods exposed for testing
void SendSignalReadyToSendData() { SignalReadyToSendData(); }
+ void SendSignalAssociationChangeCommunicationUp() {
+ SignalAssociationChangeCommunicationUp();
+ }
+
void SendSignalClosingProcedureStartedRemotely() {
SignalClosingProcedureStartedRemotely(1);
}
@@ -55,13 +66,24 @@
void SendSignalClosingProcedureComplete() {
SignalClosingProcedureComplete(1);
}
+ void set_max_outbound_streams(int streams) {
+ max_outbound_streams_ = streams;
+ }
+ void set_max_inbound_streams(int streams) { max_inbound_streams_ = streams; }
+
+ private:
+ absl::optional<int> max_outbound_streams_;
+ absl::optional<int> max_inbound_streams_;
};
} // namespace
class TestSctpTransportObserver : public SctpTransportObserverInterface {
public:
+ TestSctpTransportObserver() : info_(SctpTransportState::kNew) {}
+
void OnStateChange(SctpTransportInformation info) override {
+ info_ = info;
states_.push_back(info.state());
}
@@ -75,8 +97,11 @@
const std::vector<SctpTransportState>& States() { return states_; }
+ const SctpTransportInformation LastReceivedInformation() { return info_; }
+
private:
std::vector<SctpTransportState> states_;
+ SctpTransportInformation info_;
};
class SctpTransportTest : public ::testing::Test {
@@ -102,6 +127,11 @@
void CompleteSctpHandshake() {
CricketSctpTransport()->SendSignalReadyToSendData();
+ // The computed MaxChannels shall be the minimum of the outgoing
+ // and incoming # of streams.
+ CricketSctpTransport()->set_max_outbound_streams(kTestMaxSctpStreams);
+ CricketSctpTransport()->set_max_inbound_streams(kTestMaxSctpStreams + 1);
+ CricketSctpTransport()->SendSignalAssociationChangeCommunicationUp();
}
FakeCricketSctpTransport* CricketSctpTransport() {
@@ -149,4 +179,20 @@
kDefaultTimeout);
}
+TEST_F(SctpTransportTest, MaxChannelsSignalled) {
+ CreateTransport();
+ transport()->RegisterObserver(observer());
+ AddDtlsTransport();
+ EXPECT_FALSE(transport()->Information().MaxChannels());
+ EXPECT_FALSE(observer_.LastReceivedInformation().MaxChannels());
+ CompleteSctpHandshake();
+ ASSERT_EQ_WAIT(SctpTransportState::kConnected, observer_.State(),
+ kDefaultTimeout);
+ EXPECT_TRUE(transport()->Information().MaxChannels());
+ EXPECT_EQ(kTestMaxSctpStreams, *(transport()->Information().MaxChannels()));
+ EXPECT_TRUE(observer_.LastReceivedInformation().MaxChannels());
+ EXPECT_EQ(kTestMaxSctpStreams,
+ *(observer_.LastReceivedInformation().MaxChannels()));
+}
+
} // namespace webrtc
diff --git a/pc/test/fake_sctp_transport.h b/pc/test/fake_sctp_transport.h
index b3eeee0..50e59f1 100644
--- a/pc/test/fake_sctp_transport.h
+++ b/pc/test/fake_sctp_transport.h
@@ -38,6 +38,8 @@
void set_debug_name_for_testing(const char* debug_name) override {}
int max_message_size() const { return max_message_size_; }
+ absl::optional<int> max_outbound_streams() const { return absl::nullopt; }
+ absl::optional<int> max_inbound_streams() const { return absl::nullopt; }
int local_port() const { return *local_port_; }
int remote_port() const { return *remote_port_; }
