Merge pull request #90 from paullouisageneau/more-sctp-tuning

SCTP optimizations

src/sctptransport.cpp

@@ -67,8 +67,14 @@ void SctpTransport::Init() {
 	// Change congestion control from the default TCP Reno (RFC 2581) to H-TCP
 	usrsctp_sysctl_set_sctp_default_cc_module(SCTP_CC_HTCP);
 
+	// Enable Non-Renegable Selective Acknowledgments (NR-SACKs)
+	usrsctp_sysctl_set_sctp_nrsack_enable(1);
+
 	// Increase the initial window size to 10 MTUs (RFC 6928)
 	usrsctp_sysctl_set_sctp_initial_cwnd(10);
+
+	// Reduce SACK delay from the default 200ms to 20ms
+	usrsctp_sysctl_set_sctp_delayed_sack_time_default(20); // ms
 }
 
 void SctpTransport::Cleanup() {
@@ -273,7 +279,7 @@ void SctpTransport::incoming(message_ptr message) {
 	// There could be a race condition here where we receive the remote INIT before the local one is
 	// sent, which would result in the connection being aborted. Therefore, we need to wait for data
 	// to be sent on our side (i.e. the local INIT) before proceeding.
-	{
+	if (!mWrittenOnce) { // test the atomic boolean is not set first to prevent a lock contention
 		std::unique_lock lock(mWriteMutex);
 		mWrittenCondition.wait(lock, [&]() { return mWrittenOnce || state() != State::Connected; });
 	}
@@ -370,18 +376,20 @@ bool SctpTransport::trySendMessage(message_ptr message) {
 		ret = usrsctp_sendv(mSock, &zero, 1, nullptr, 0, &spa, sizeof(spa), SCTP_SENDV_SPA, 0);
 	}
 
-	if (ret >= 0) {
-		PLOG_VERBOSE << "SCTP sent size=" << message->size();
-		if (message->type == Message::Type::Binary || message->type == Message::Type::String)
-			mBytesSent += message->size();
-		return true;
-	} else if (errno == EWOULDBLOCK || errno == EAGAIN) {
-		PLOG_VERBOSE << "SCTP sending not possible";
-		return false;
-	} else {
+	if (ret < 0) {
+		if (errno == EWOULDBLOCK || errno == EAGAIN) {
+			PLOG_VERBOSE << "SCTP sending not possible";
+			return false;
+		}
+
 		PLOG_ERROR << "SCTP sending failed, errno=" << errno;
 		throw std::runtime_error("Sending failed, errno=" + std::to_string(errno));
 	}
+
+	PLOG_VERBOSE << "SCTP sent size=" << message->size();
+	if (message->type == Message::Type::Binary || message->type == Message::Type::String)
+		mBytesSent += message->size();
+	return true;
 }
 
 void SctpTransport::updateBufferedAmount(uint16_t streamId, long delta) {
@@ -485,6 +493,7 @@ int SctpTransport::handleWrite(byte *data, size_t len, uint8_t tos, uint8_t set_
 		std::unique_lock lock(mWriteMutex);
 		if (!outgoing(make_message(data, data + len)))
 			return -1;
+
 		mWritten = true;
 		mWrittenOnce = true;
 		mWrittenCondition.notify_all();

src/sctptransport.hpp

@@ -97,7 +97,7 @@ private:
 	std::mutex mWriteMutex;
 	std::condition_variable mWrittenCondition;
 	std::atomic<bool> mWritten = false; // written outside lock
-	bool mWrittenOnce = false;
+	std::atomic<bool> mWrittenOnce = false; // same
 
 	binary mPartialRecv, mPartialStringData, mPartialBinaryData;
 

src/tlstransport.hpp

@@ -26,7 +26,6 @@
 
 #if RTC_ENABLE_WEBSOCKET
 
-#include <mutex>
 #include <thread>
 
 namespace rtc {

test/benchmark.cpp

@@ -157,9 +157,9 @@ size_t benchmark(milliseconds duration) {
 		cout << "Received: " << receivedSize.load() / 1000 << " KB" << endl;
 	}
 
-	if (auto adc2 = std::atomic_load(&dc2); adc2->isOpen()) {
+	if (auto adc2 = std::atomic_load(&dc2)) {
 		dc1->close();
-		while (!finished)
+		while (!finished && adc2->isOpen())
 			this_thread::sleep_for(100ms);
 	}