Added send-thresholding and fixed text packet dumping. Also a little squelch for the over-max-MTU log spam we see in there.

BUG=https://code.google.com/p/webrtc/issues/detail?id=4468
R=pthatcher@chromium.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1266033005 .

Cr-Commit-Position: refs/heads/master@{#9788}
This commit is contained in:
Lally Singh
2015-08-26 13:15:20 -04:00
parent 3318f984cd
commit d838d27919
3 changed files with 109 additions and 8 deletions

View File

@ -109,6 +109,8 @@ typedef rtc::ScopedMessageData<rtc::Buffer> OutboundPacketMessage;
// take off 80 bytes for DTLS/TURN/TCP/IP overhead.
static const size_t kSctpMtu = 1200;
// The size of the SCTP association send buffer. 256kB, the usrsctp default.
static const int kSendBufferSize = 262144;
enum {
MSG_SCTPINBOUNDPACKET = 1, // MessageData is SctpInboundPacket
MSG_SCTPOUTBOUNDPACKET = 2, // MessageData is rtc:Buffer
@ -177,11 +179,11 @@ static bool GetDataMediaType(
}
// Log the packet in text2pcap format, if log level is at LS_VERBOSE.
static void VerboseLogPacket(void *addr, size_t length, int direction) {
static void VerboseLogPacket(void *data, size_t length, int direction) {
if (LOG_CHECK_LEVEL(LS_VERBOSE) && length > 0) {
char *dump_buf;
if ((dump_buf = usrsctp_dumppacket(
addr, length, direction)) != NULL) {
data, length, direction)) != NULL) {
LOG(LS_VERBOSE) << dump_buf;
usrsctp_freedumpbuffer(dump_buf);
}
@ -244,6 +246,10 @@ static int OnSctpInboundPacket(struct socket* sock, union sctp_sockstore addr,
// Set the initial value of the static SCTP Data Engines reference count.
int SctpDataEngine::usrsctp_engines_count = 0;
// All the channels created by this engine, used for callbacks from
// usrsctplib that only contain socket pointers. Channels are removed when
// SignalDestroyed is fired.
std::vector<SctpDataMediaChannel*> SctpDataEngine::channels_;
SctpDataEngine::SctpDataEngine() {
if (usrsctp_engines_count == 0) {
@ -258,6 +264,11 @@ SctpDataEngine::SctpDataEngine() {
// TODO(ldixon): Consider turning this on/off.
usrsctp_sysctl_set_sctp_ecn_enable(0);
int send_size = usrsctp_sysctl_get_sctp_sendspace();
if (send_size != kSendBufferSize) {
LOG(LS_ERROR) << "Got different send size than expected: " << send_size;
}
// TODO(ldixon): Consider turning this on/off.
// This is not needed right now (we don't do dynamic address changes):
// If SCTP Auto-ASCONF is enabled, the peer is informed automatically
@ -312,9 +323,48 @@ DataMediaChannel* SctpDataEngine::CreateChannel(
if (data_channel_type != DCT_SCTP) {
return NULL;
}
return new SctpDataMediaChannel(rtc::Thread::Current());
SctpDataMediaChannel *channel = new SctpDataMediaChannel(
rtc::Thread::Current());
channels_.push_back(channel);
channel->SignalDestroyed.connect(this, &SctpDataEngine::OnChannelDestroyed);
return channel;
}
// static
SctpDataMediaChannel* SctpDataEngine::GetChannelFromSocket(
struct socket* sock) {
for (auto p:channels_) {
if (p->socket() == sock) {
return p;
}
}
return 0;
}
void SctpDataEngine::OnChannelDestroyed(SctpDataMediaChannel* channel) {
auto it = std::find(channels_.begin(), channels_.end(), channel);
if (it == channels_.end()) {
LOG(LS_ERROR) << "OnChannelDestroyed: the channel wasn't registered.";
return;
}
channels_.erase(it);
}
// static
int SctpDataEngine::SendThresholdCallback(struct socket* sock,
uint32_t sb_free) {
SctpDataMediaChannel *channel = GetChannelFromSocket(sock);
if (!channel) {
LOG(LS_ERROR) << "SendThresholdCallback: Failed to get channel for socket "
<< sock;
return 0;
}
channel->SignalReadyToSend(true);
return 0;
}
SctpDataMediaChannel::SctpDataMediaChannel(rtc::Thread* thread)
: worker_thread_(thread),
local_port_(kSctpDefaultPort),
@ -327,6 +377,7 @@ SctpDataMediaChannel::SctpDataMediaChannel(rtc::Thread* thread)
SctpDataMediaChannel::~SctpDataMediaChannel() {
CloseSctpSocket();
SignalDestroyed(this);
}
sockaddr_conn SctpDataMediaChannel::GetSctpSockAddr(int port) {
@ -347,8 +398,16 @@ bool SctpDataMediaChannel::OpenSctpSocket() {
<< "->Ignoring attempt to re-create existing socket.";
return false;
}
// If kSendBufferSize isn't reflective of reality, we log an error, but we
// still have to do something reasonable here. Look up what the buffer's
// real size is and set our threshold to something reasonable.
const static int send_threshold = usrsctp_sysctl_get_sctp_sendspace() / 2;
sock_ = usrsctp_socket(AF_CONN, SOCK_STREAM, IPPROTO_SCTP,
cricket::OnSctpInboundPacket, NULL, 0, this);
cricket::OnSctpInboundPacket,
&SctpDataEngine::SendThresholdCallback,
send_threshold, this);
if (!sock_) {
LOG_ERRNO(LS_ERROR) << debug_name_ << "Failed to create SCTP socket.";
return false;
@ -393,7 +452,8 @@ bool SctpDataMediaChannel::OpenSctpSocket() {
}
// Disable MTU discovery
struct sctp_paddrparams params = {{0}};
struct sctp_paddrparams params;
memset(&params, 0, sizeof(params));
params.spp_assoc_id = 0;
params.spp_flags = SPP_PMTUD_DISABLE;
params.spp_pathmtu = kSctpMtu;
@ -904,10 +964,17 @@ bool SctpDataMediaChannel::SetRecvCodecs(const std::vector<DataCodec>& codecs) {
void SctpDataMediaChannel::OnPacketFromSctpToNetwork(
rtc::Buffer* buffer) {
if (buffer->size() > kSctpMtu) {
// usrsctp seems to interpret the MTU we give it strangely -- it seems to
// give us back packets bigger than that MTU, if only by a fixed amount.
// This is that amount that we've observed.
const int kSctpOverhead = 76;
if (buffer->size() > (kSctpOverhead + kSctpMtu)) {
LOG(LS_ERROR) << debug_name_ << "->OnPacketFromSctpToNetwork(...): "
<< "SCTP seems to have made a packet that is bigger "
"than its official MTU.";
<< "than its official MTU: " << buffer->size()
<< " vs max of " << kSctpMtu
<< " even after adding " << kSctpOverhead
<< " extra SCTP overhead";
}
MediaChannel::SendPacket(buffer);
}

View File

@ -64,6 +64,8 @@ const uint32 kMaxSctpSid = 1023;
// usrsctp.h)
const int kSctpDefaultPort = 5000;
class SctpDataMediaChannel;
// A DataEngine that interacts with usrsctp.
//
// From channel calls, data flows like this:
@ -88,7 +90,7 @@ const int kSctpDefaultPort = 5000;
// 14. SctpDataMediaChannel::SignalDataReceived(data)
// [from the same thread, methods registered/connected to
// SctpDataMediaChannel are called with the recieved data]
class SctpDataEngine : public DataEngineInterface {
class SctpDataEngine : public DataEngineInterface, public sigslot::has_slots<> {
public:
SctpDataEngine();
virtual ~SctpDataEngine();
@ -97,9 +99,15 @@ class SctpDataEngine : public DataEngineInterface {
virtual const std::vector<DataCodec>& data_codecs() { return codecs_; }
static int SendThresholdCallback(struct socket* sock, uint32_t sb_free);
private:
static std::vector<SctpDataMediaChannel*> channels_;
static int usrsctp_engines_count;
std::vector<DataCodec> codecs_;
static SctpDataMediaChannel* GetChannelFromSocket(struct socket* sock);
void OnChannelDestroyed(SctpDataMediaChannel *channel);
};
// TODO(ldixon): Make into a special type of TypedMessageData.
@ -188,6 +196,9 @@ class SctpDataMediaChannel : public DataMediaChannel,
debug_name_ = debug_name;
}
const std::string& debug_name() const { return debug_name_; }
const struct socket* socket() { return sock_; }
sigslot::signal1<SctpDataMediaChannel*> SignalDestroyed;
private:
sockaddr_conn GetSctpSockAddr(int port);

View File

@ -240,10 +240,16 @@ class SctpDataMediaChannelTest : public testing::Test,
net2_.reset(new SctpFakeNetworkInterface(rtc::Thread::Current()));
recv1_.reset(new SctpFakeDataReceiver());
recv2_.reset(new SctpFakeDataReceiver());
chan1_ready_to_send_count_ = 0;
chan2_ready_to_send_count_ = 0;
chan1_.reset(CreateChannel(net1_.get(), recv1_.get()));
chan1_->set_debug_name("chan1/connector");
chan1_->SignalReadyToSend.connect(
this, &SctpDataMediaChannelTest::OnChan1ReadyToSend);
chan2_.reset(CreateChannel(net2_.get(), recv2_.get()));
chan2_->set_debug_name("chan2/listener");
chan2_->SignalReadyToSend.connect(
this, &SctpDataMediaChannelTest::OnChan2ReadyToSend);
// Setup two connected channels ready to send and receive.
net1_->SetDestination(chan2_.get());
net2_->SetDestination(chan1_.get());
@ -330,6 +336,8 @@ class SctpDataMediaChannelTest : public testing::Test,
SctpFakeDataReceiver* receiver1() { return recv1_.get(); }
SctpFakeDataReceiver* receiver2() { return recv2_.get(); }
int channel1_ready_to_send_count() { return chan1_ready_to_send_count_; }
int channel2_ready_to_send_count() { return chan2_ready_to_send_count_; }
private:
rtc::scoped_ptr<cricket::SctpDataEngine> engine_;
rtc::scoped_ptr<SctpFakeNetworkInterface> net1_;
@ -338,6 +346,12 @@ class SctpDataMediaChannelTest : public testing::Test,
rtc::scoped_ptr<SctpFakeDataReceiver> recv2_;
rtc::scoped_ptr<cricket::SctpDataMediaChannel> chan1_;
rtc::scoped_ptr<cricket::SctpDataMediaChannel> chan2_;
int chan1_ready_to_send_count_;
int chan2_ready_to_send_count_;
void OnChan1ReadyToSend(bool send) { if (send) chan1_ready_to_send_count_++; }
void OnChan2ReadyToSend(bool send) { if (send) chan2_ready_to_send_count_++; }
};
// Verifies that SignalReadyToSend is fired.
@ -486,6 +500,15 @@ TEST_F(SctpDataMediaChannelTest, ClosesStreamsOnBothSides) {
EXPECT_TRUE_WAIT(chan_1_sig_receiver.WasStreamClosed(4), 1000);
}
TEST_F(SctpDataMediaChannelTest, EngineSignalsRightChannel) {
SetupConnectedChannels();
EXPECT_TRUE_WAIT(channel1()->socket() != NULL, 1000);
struct socket *sock = const_cast<struct socket*>(channel1()->socket());
int prior_count = channel1_ready_to_send_count();
cricket::SctpDataEngine::SendThresholdCallback(sock, 0);
EXPECT_GT(channel1_ready_to_send_count(), prior_count);
}
// Flaky on Linux and Windows. See webrtc:4453.
#if defined(WEBRTC_WIN) || defined(WEBRTC_LINUX)
#define MAYBE_ReusesAStream DISABLED_ReusesAStream