From fe8f48962aacab4a8437c9d98dfcda225329c68e Mon Sep 17 00:00:00 2001
From: deadbeef <deadbeef@webrtc.org>
Date: Wed, 24 Aug 2016 16:26:55 -0700
Subject: [PATCH] Fix setting the MTU for SCTP.

It was being set at the wrong point in time and with the address
parameter missing, so it wasn't having any effect.

Review-Url: https://codereview.webrtc.org/2237073002
Cr-Commit-Position: refs/heads/master@{#13909}
---
 webrtc/media/sctp/sctpdataengine.cc | 40 ++++++++++++-----------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/webrtc/media/sctp/sctpdataengine.cc b/webrtc/media/sctp/sctpdataengine.cc
index 08b441afa8..2d8d48eee3 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;
@@ -470,18 +470,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,
@@ -559,6 +547,18 @@ bool SctpDataMediaChannel::Connect() {
     CloseSctpSocket();
     return false;
   }
+  // Set the MTU and disable MTU discovery.
+  // We can only do this after usrsctp_connect or it has no effect.
+  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;
 }
 
@@ -998,17 +998,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());
 }