Adding test for unordered, fragmented SCTP message delivery.

webrtc/media/sctp/sctpdataengine.cc

Index: webrtc/media/sctp/sctpdataengine.cc
diff --git a/webrtc/media/sctp/sctpdataengine.cc b/webrtc/media/sctp/sctpdataengine.cc
index fa70ab16affdeb2401dbcab10e8927f94cc538c0..2cd72a537a5da37fc3443f616ad94fe4e29aa9f2 100644
--- a/webrtc/media/sctp/sctpdataengine.cc
+++ b/webrtc/media/sctp/sctpdataengine.cc
@@ -29,12 +29,12 @@
 #include "webrtc/media/base/streamparams.h"
 
 namespace cricket {
-// The biggest SCTP packet.  Starting from a 'safe' wire MTU value of 1280,
+// The biggest SCTP packet. Starting from a 'safe' wire MTU value of 1280,
 // take off 80 bytes for DTLS/TURN/TCP/IP overhead.
-static const size_t kSctpMtu = 1200;
+static constexpr size_t kSctpMtu = 1200;
 
 // The size of the SCTP association send buffer.  256kB, the usrsctp default.
-static const int kSendBufferSize = 262144;
+static constexpr int kSendBufferSize = 262144;
 
 struct SctpInboundPacket {
   rtc::CopyOnWriteBuffer buffer;
@@ -473,18 +473,6 @@ bool SctpDataMediaChannel::OpenSctpSocket() {
     return false;
   }
 
-  // Disable MTU discovery
-  sctp_paddrparams params = {{0}};
-  params.spp_assoc_id = 0;
-  params.spp_flags = SPP_PMTUD_DISABLE;
-  params.spp_pathmtu = kSctpMtu;
-  if (usrsctp_setsockopt(sock_, IPPROTO_SCTP, SCTP_PEER_ADDR_PARAMS, &params,
-      sizeof(params))) {
-    LOG_ERRNO(LS_ERROR) << debug_name_
-                        << "Failed to set SCTP_PEER_ADDR_PARAMS.";
-    return false;
-  }
-
   // Subscribe to SCTP event notifications.
   int event_types[] = {SCTP_ASSOC_CHANGE,
                        SCTP_PEER_ADDR_CHANGE,
@@ -562,6 +550,18 @@ bool SctpDataMediaChannel::Connect() {
     CloseSctpSocket();
     return false;
   }
+  // Disable MTU discovery.
+  // We can only do this after usrsctp_connect or it has no effect.

[honghaiz3, 2016/08/11 16:54:28]

Does it mean that previously MTU-discovery was not actually disabled but with
this change, it will be disabled?

[Taylor Brandstetter, 2016/08/11 17:22:06]

Correct. But I'm thinking it may have been a mistake to disable it in the first
place (see previous comment). We can discuss that on the new CL:
https://codereview.webrtc.org/2237073002/

+  sctp_paddrparams params = {{0}};
+  memcpy(reinterpret_cast<sockaddr*>(&params.spp_address),
+         reinterpret_cast<sockaddr*>(&remote_sconn), sizeof(sockaddr));
+  params.spp_flags = SPP_PMTUD_DISABLE;
+  params.spp_pathmtu = kSctpMtu;
+  if (usrsctp_setsockopt(sock_, IPPROTO_SCTP, SCTP_PEER_ADDR_PARAMS, &params,
+                         sizeof(params))) {
+    LOG_ERRNO(LS_ERROR) << debug_name_
+                        << "Failed to set SCTP_PEER_ADDR_PARAMS.";
+  }
   return true;
 }
 
@@ -1002,17 +1002,11 @@ bool SctpDataMediaChannel::SetRecvCodecs(const std::vector<DataCodec>& codecs) {
 
 void SctpDataMediaChannel::OnPacketFromSctpToNetwork(
     rtc::CopyOnWriteBuffer* buffer) {
-  // 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)) {
+  if (buffer->size() > (kSctpMtu)) {
     LOG(LS_ERROR) << debug_name_ << "->OnPacketFromSctpToNetwork(...): "
                   << "SCTP seems to have made a packet that is bigger "
                   << "than its official MTU: " << buffer->size()
-                  << " vs max of " << kSctpMtu
-                  << " even after adding " << kSctpOverhead
-                  << " extra SCTP overhead";
+                  << " vs max of " << kSctpMtu;
   }
   MediaChannel::SendPacket(buffer, rtc::PacketOptions());
 }