From 6fddf31db31a6f3d0f9c6dd7e611543f56d6fc2f Mon Sep 17 00:00:00 2001
From: Joseph Henry <josephjah@gmail.com>
Date: Tue, 12 Jun 2018 15:24:12 -0700
Subject: [PATCH] Improved rate limit logic for QoS/ACK packets. Also reduced
 how often processBackgroundPathMeasurements() is called

---
 node/Constants.hpp |  7 +------
 node/Path.hpp      |  8 +++-----
 node/Peer.cpp      |  3 ++-
 node/Peer.hpp      | 38 +++++++++++++++++++++-----------------
 4 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/node/Constants.hpp b/node/Constants.hpp
index d86775c59..d4f6faaf1 100644
--- a/node/Constants.hpp
+++ b/node/Constants.hpp
@@ -289,7 +289,7 @@
  * CUTOFF_LIMIT times per CUTOFF_TIME milliseconds per peer to prevent
  * this from being useful for DOS amplification attacks.
  */
-#define ZT_PATH_QOS_ACK_CUTOFF_LIMIT 16
+#define ZT_PATH_QOS_ACK_CUTOFF_LIMIT 128
 
 /**
  * Path choice history window size. This is used to keep track of which paths were
@@ -372,11 +372,6 @@
  */
 #define ZT_PATH_MAX_OUTSTANDING_QOS_RECORDS 128
 
-/**
- * How often we check the age of QoS records
- */
-#define ZT_PATH_QOS_RECORD_PURGE_INTERVAL 1000
-
 /**
  * Timeout for QoS records
  */
diff --git a/node/Path.hpp b/node/Path.hpp
index 80a7895af..71615d502 100644
--- a/node/Path.hpp
+++ b/node/Path.hpp
@@ -566,9 +566,9 @@ public:
 	 * @param now Current time
 	 */
 	inline void processBackgroundPathMeasurements(int64_t now, const int64_t peerId) {
-		Mutex::Lock _l(_statistics_m);
 		// Compute path stability
 		if (now - _lastPathQualityComputeTime > ZT_PATH_QUALITY_COMPUTE_INTERVAL) {
+			Mutex::Lock _l(_statistics_m);
 			_lastPathQualityComputeTime = now;
 			address().toString(_addrString);
 			_meanThroughput = _throughputSamples->mean();
@@ -593,10 +593,8 @@ public:
 			_lastComputedStability = pdv_contrib + latency_contrib + throughput_disturbance_contrib;
 			_lastComputedStability *= 1 - _packetErrorRatio;
 			_qualitySamples->push(_lastComputedStability);
-		}
-		// Prevent QoS records from sticking around for too long
-		if (now - _lastQoSRecordPurge > ZT_PATH_QOS_RECORD_PURGE_INTERVAL)
-		{
+
+			// Prevent QoS records from sticking around for too long
 			std::map<uint64_t,uint64_t>::iterator it = _outQoSRecords.begin();
 			while (it != _outQoSRecords.end()) {
 				// Time since egress of tracked packet
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 877ec2fd0..8b385ecc8 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -24,7 +24,6 @@
  * of your own application.
  */
 
-
 #include "../version.h"
 #include "Constants.hpp"
 #include "Peer.hpp"
@@ -55,6 +54,8 @@ Peer::Peer(const RuntimeEnvironment *renv,const Identity &myIdentity,const Ident
 	_lastCredentialsReceived(0),
 	_lastTrustEstablishedPacketReceived(0),
 	_lastSentFullHello(0),
+	_lastACKWindowReset(0),
+	_lastQoSWindowReset(0),
 	_vProto(0),
 	_vMajor(0),
 	_vMinor(0),
diff --git a/node/Peer.hpp b/node/Peer.hpp
index f7c0dbbd5..8807662d8 100644
--- a/node/Peer.hpp
+++ b/node/Peer.hpp
@@ -523,30 +523,34 @@ public:
 		return false;
 	}
 
-	/**
-	 * Rate limit gate for VERB_QOS_MEASUREMENT
-	 */
-	inline bool rateGateQoS(const int64_t now)
-	{
-		if ((now - _lastQoSReceive) <= ZT_PATH_QOS_ACK_CUTOFF_TIME)
-			++_QoSCutoffCount;
-		else _QoSCutoffCount = 0;
-		_lastQoSReceive = now;
-		return (_QoSCutoffCount < ZT_PATH_QOS_ACK_CUTOFF_LIMIT);
-	}
-
 	/**
 	 * Rate limit gate for VERB_ACK
 	 */
 	inline bool rateGateACK(const int64_t now)
 	{
-		if ((now - _lastACKReceive) <= ZT_PATH_QOS_ACK_CUTOFF_TIME)
+		if ((now - _lastACKWindowReset) >= ZT_PATH_QOS_ACK_CUTOFF_TIME) {
+			_lastACKWindowReset = now;
+			_ACKCutoffCount = 0;
+		} else {
 			++_ACKCutoffCount;
-		else _ACKCutoffCount = 0;
-		_lastACKReceive = now;
+		}
 		return (_ACKCutoffCount < ZT_PATH_QOS_ACK_CUTOFF_LIMIT);
 	}
 
+	/**
+	 * Rate limit gate for VERB_QOS_MEASUREMENT
+	 */
+	inline bool rateGateQoS(const int64_t now)
+	{
+		if ((now - _lastQoSWindowReset) >= ZT_PATH_QOS_ACK_CUTOFF_TIME) {
+			_lastQoSWindowReset = now;
+			_QoSCutoffCount = 0;
+		} else {
+			++_QoSCutoffCount;
+		}
+		return (_QoSCutoffCount < ZT_PATH_QOS_ACK_CUTOFF_LIMIT);
+	}
+
 	/**
 	 * Serialize a peer for storage in local cache
 	 *
@@ -644,10 +648,10 @@ private:
 	int64_t _lastComRequestSent;
 	int64_t _lastCredentialsReceived;
 	int64_t _lastTrustEstablishedPacketReceived;
-	int64_t _lastQoSReceive;
-	int64_t _lastACKReceive;
 	int64_t _lastSentFullHello;
 	int64_t _lastPathPrune;
+	int64_t _lastACKWindowReset;
+	int64_t _lastQoSWindowReset;
 
 	uint16_t _vProto;
 	uint16_t _vMajor;