SCTP: Treat message size zero as "responder selects"

This also refactors some of the code in peerconnection for
handling SCTP transports to be internal to the webrtc::SctpTransport
class, rather than being in peerconnection.

Bug: webrtc:10358, webrtc:10629
Change-Id: I15ecf95c199f56b08909e5a9311d446a412ed162
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/137041
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27960}
This commit is contained in:
Harald Alvestrand 2019-05-16 11:49:17 +02:00 committed by Commit Bot
parent 6e70f18fbd
commit 8d3d6cf908
7 changed files with 114 additions and 33 deletions

View File

@ -295,6 +295,7 @@ if (rtc_include_tests) {
"../call:rtp_interfaces",
"../call:rtp_receiver",
"../logging:rtc_event_log_api",
"../media:rtc_data",
"../media:rtc_media_base",
"../media:rtc_media_tests_utils",
"../modules/rtp_rtcp:rtp_rtcp_format",

View File

@ -2584,8 +2584,15 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
data_answer->as_sctp()->set_protocol(offer_data_description->protocol());
// Respond with our max message size or the remote max messsage size,
// whichever is smaller.
data_answer->as_sctp()->set_max_message_size(std::min(
offer_data_description->max_message_size(), kSctpSendBufferSize));
// 0 is treated specially - it means "I can accept any size". Since
// we do not implement infinite size messages, reply with
// kSctpSendBufferSize.
if (offer_data_description->max_message_size() == 0) {
data_answer->as_sctp()->set_max_message_size(kSctpSendBufferSize);
} else {
data_answer->as_sctp()->set_max_message_size(std::min(
offer_data_description->max_message_size(), kSctpSendBufferSize));
}
if (!CreateMediaContentAnswer(
offer_data_description, media_description_options, session_options,
sdes_policy, GetCryptos(current_content), RtpHeaderExtensions(),

View File

@ -17,6 +17,7 @@
#include "absl/memory/memory.h"
#include "media/base/codec.h"
#include "media/base/test_utils.h"
#include "media/sctp/sctp_transport_internal.h"
#include "p2p/base/p2p_constants.h"
#include "p2p/base/transport_description.h"
#include "p2p/base/transport_info.h"
@ -1427,6 +1428,64 @@ TEST_F(MediaSessionDescriptionFactoryTest,
}
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestCreateDataAnswerToOfferWithDefinedMessageSize) {
// Need to enable DTLS offer/answer generation (disabled by default in this
// test).
f1_.set_secure(SEC_ENABLED);
f2_.set_secure(SEC_ENABLED);
tdf1_.set_secure(SEC_ENABLED);
tdf2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, nullptr);
ASSERT_TRUE(offer.get() != nullptr);
ContentInfo* dc_offer = offer->GetContentByName("data");
ASSERT_TRUE(dc_offer != nullptr);
SctpDataContentDescription* dcd_offer =
dc_offer->media_description()->as_sctp();
ASSERT_TRUE(dcd_offer);
dcd_offer->set_max_message_size(1234);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswer(offer.get(), opts, nullptr);
const ContentInfo* dc_answer = answer->GetContentByName("data");
ASSERT_TRUE(dc_answer != nullptr);
const SctpDataContentDescription* dcd_answer =
dc_answer->media_description()->as_sctp();
EXPECT_FALSE(dc_answer->rejected);
EXPECT_EQ(1234, dcd_answer->max_message_size());
}
TEST_F(MediaSessionDescriptionFactoryTest,
TestCreateDataAnswerToOfferWithZeroMessageSize) {
// Need to enable DTLS offer/answer generation (disabled by default in this
// test).
f1_.set_secure(SEC_ENABLED);
f2_.set_secure(SEC_ENABLED);
tdf1_.set_secure(SEC_ENABLED);
tdf2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, nullptr);
ASSERT_TRUE(offer.get() != nullptr);
ContentInfo* dc_offer = offer->GetContentByName("data");
ASSERT_TRUE(dc_offer != nullptr);
SctpDataContentDescription* dcd_offer =
dc_offer->media_description()->as_sctp();
ASSERT_TRUE(dcd_offer);
dcd_offer->set_max_message_size(0);
std::unique_ptr<SessionDescription> answer =
f2_.CreateAnswer(offer.get(), opts, nullptr);
const ContentInfo* dc_answer = answer->GetContentByName("data");
ASSERT_TRUE(dc_answer != nullptr);
const SctpDataContentDescription* dcd_answer =
dc_answer->media_description()->as_sctp();
EXPECT_FALSE(dc_answer->rejected);
EXPECT_EQ(cricket::kSctpSendBufferSize, dcd_answer->max_message_size());
}
// Verifies that the order of the media contents in the offer is preserved in
// the answer.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAnswerContentOrder) {

View File

@ -5657,35 +5657,24 @@ RTCError PeerConnection::PushdownMediaDescription(
auto remote_sctp_description = cricket::GetFirstSctpDataContentDescription(
remote_description()->description());
if (local_sctp_description && remote_sctp_description) {
int max_message_size =
std::min(local_sctp_description->max_message_size(),
remote_sctp_description->max_message_size());
bool success = network_thread()->Invoke<bool>(
RTC_FROM_HERE,
rtc::Bind(&PeerConnection::PushdownSctpParameters_n, this, source,
local_sctp_description->port(),
remote_sctp_description->port(), max_message_size));
if (!success) {
LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
"Failed to push down SCTP parameters.");
int max_message_size;
// A remote max message size of zero means "any size supported".
// We configure the connection with our own max message size.
if (remote_sctp_description->max_message_size() == 0) {
max_message_size = local_sctp_description->max_message_size();
} else {
max_message_size =
std::min(local_sctp_description->max_message_size(),
remote_sctp_description->max_message_size());
}
sctp_transport_->Start(local_sctp_description->port(),
remote_sctp_description->port(), max_message_size);
}
}
return RTCError::OK();
}
bool PeerConnection::PushdownSctpParameters_n(cricket::ContentSource source,
int local_sctp_port,
int remote_sctp_port,
int max_message_size) {
RTC_DCHECK_RUN_ON(network_thread());
// Apply the SCTP port (which is hidden inside a DataCodec structure...)
// When we support "max-message-size", that would also be pushed down here.
return cricket_sctp_transport()->Start(local_sctp_port, remote_sctp_port,
max_message_size);
}
RTCError PeerConnection::PushdownTransportDescription(
cricket::ContentSource source,
SdpType type) {

View File

@ -877,10 +877,6 @@ class PeerConnection : public PeerConnectionInternal,
// down to all of the channels.
RTCError PushdownMediaDescription(SdpType type, cricket::ContentSource source)
RTC_RUN_ON(signaling_thread());
bool PushdownSctpParameters_n(cricket::ContentSource source,
int local_sctp_port,
int remote_sctp_port,
int max_message_size);
RTCError PushdownTransportDescription(cricket::ContentSource source,
SdpType type);

View File

@ -96,6 +96,27 @@ void SctpTransport::SetDtlsTransport(
UpdateInformation(next_state);
}
void SctpTransport::Start(int local_port,
int remote_port,
int max_message_size) {
{
rtc::CritScope scope(&lock_);
// Record max message size on calling thread.
info_ = SctpTransportInformation(info_.state(), info_.dtls_transport(),
max_message_size, info_.MaxChannels());
}
if (owner_thread_->IsCurrent()) {
if (!internal()->Start(local_port, remote_port, max_message_size)) {
RTC_LOG(LS_ERROR) << "Failed to push down SCTP parameters, closing.";
UpdateInformation(SctpTransportState::kClosed);
}
} else {
owner_thread_->Invoke<void>(
RTC_FROM_HERE, rtc::Bind(&SctpTransport::Start, this, local_port,
remote_port, max_message_size));
}
}
void SctpTransport::UpdateInformation(SctpTransportState state) {
RTC_DCHECK_RUN_ON(owner_thread_);
bool must_send_update;
@ -103,12 +124,11 @@ void SctpTransport::UpdateInformation(SctpTransportState state) {
{
rtc::CritScope scope(&lock_);
must_send_update = (state != info_.state());
// TODO(https://bugs.webrtc.org/10358): Update max message size and
// max channels from internal SCTP transport when available.
// TODO(https://bugs.webrtc.org/10358): Update max channels from internal
// SCTP transport when available.
if (internal_sctp_transport_) {
info_ = SctpTransportInformation(
state, dtls_transport_, internal_sctp_transport_->max_message_size(),
info_.MaxChannels());
state, dtls_transport_, info_.MaxMessageSize(), info_.MaxChannels());
} else {
info_ = SctpTransportInformation(
state, dtls_transport_, info_.MaxMessageSize(), info_.MaxChannels());

View File

@ -36,9 +36,16 @@ class SctpTransport : public SctpTransportInterface,
void RegisterObserver(SctpTransportObserverInterface* observer) override;
void UnregisterObserver() override;
// Internal functions
void Clear();
void SetDtlsTransport(rtc::scoped_refptr<DtlsTransport>);
// Initialize the cricket::SctpTransport. This can be called from
// the signaling thread.
void Start(int local_port, int remote_port, int max_message_size);
// TODO(https://bugs.webrtc.org/10629): Move functions that need
// internal() to be functions on the webrtc::SctpTransport interface,
// and make the internal() function private.
cricket::SctpTransportInternal* internal() {
rtc::CritScope scope(&lock_);
return internal_sctp_transport_.get();
@ -58,7 +65,9 @@ class SctpTransport : public SctpTransportInterface,
void OnInternalClosingProcedureStartedRemotely(int sid);
void OnInternalClosingProcedureComplete(int sid);
const rtc::Thread* owner_thread_;
// Note - owner_thread never changes, but can't be const if we do
// Invoke() on it.
rtc::Thread* owner_thread_;
rtc::CriticalSection lock_;
// Variables accessible off-thread, guarded by lock_
SctpTransportInformation info_ RTC_GUARDED_BY(lock_);