From 91c8e82c428cea9d9e0a6911fbd1820212871ad8 Mon Sep 17 00:00:00 2001
From: Joseph Henry <josephjah@gmail.com>
Date: Wed, 2 May 2018 15:24:14 -0700
Subject: [PATCH] Adjusted locking order of _paths_m for path pruning. Other
 minor multipath changes

---
 node/Constants.hpp     | 11 +++++++++--
 node/Path.hpp          | 16 ++++++++++++++++
 node/Peer.cpp          | 24 +++++++++++++-----------
 service/OneService.cpp |  7 ++++---
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/node/Constants.hpp b/node/Constants.hpp
index 80083abe0..ee2ff0a6d 100644
--- a/node/Constants.hpp
+++ b/node/Constants.hpp
@@ -293,7 +293,7 @@
 /**
  * Number of samples to consider when computing path statistics
  */
-#define ZT_PATH_QUALITY_METRIC_WIN_SZ 128
+#define ZT_PATH_QUALITY_METRIC_WIN_SZ 64
 
 /**
  * How often important path metrics are sampled (in ms). These metrics are later used
@@ -311,7 +311,7 @@
  * since we will record a 0 bit/s measurement if no valid latency measurement was made within this
  * window of time.
  */
-#define ZT_PATH_LATENCY_SAMPLE_INTERVAL ZT_PING_CHECK_INVERVAL * 2
+#define ZT_PATH_LATENCY_SAMPLE_INTERVAL ZT_MULTIPATH_PEER_PING_PERIOD * 2
 
 /**
  * Interval used for rate-limiting the computation of path quality estimates. Set at 0
@@ -374,6 +374,13 @@
  */
 #define ZT_PEER_PING_PERIOD 60000
 
+/**
+ * Delay between full-fledge pings of directly connected peers.
+ * With multipath bonding enabled ping peers more often to measure
+ * packet loss and latency.
+ */
+#define ZT_MULTIPATH_PEER_PING_PERIOD 5000
+
 /**
  * Paths are considered expired if they have not sent us a real packet in this long
  */
diff --git a/node/Path.hpp b/node/Path.hpp
index 7ce6e0f13..e6bcecf02 100644
--- a/node/Path.hpp
+++ b/node/Path.hpp
@@ -432,6 +432,22 @@ public:
 		}
 	}
 
+	/**
+	 * @param buf Buffer to store resultant string
+	 * @return Description of path, in ASCII string format
+	 */
+	inline char *toString(char *buf) {
+		sprintf(buf,"%6s, q=%8.3f, %5.3f Mb/s, j=%8.2f, ml=%8.2f, meanAge=%8.2f, addr=%45s",
+			getName(),
+			lastComputedQuality(),
+			(float)meanThroughput() / (float)1000000,
+			jitter(),
+			meanLatency(),
+			meanAge(),
+			getAddressString());
+		return buf;
+	}
+
 	/**
 	 * Record whether a packet is considered invalid by MAC/compression/cipher checks. This
 	 * could be an indication of a bit error. This function will keep a running counter of
diff --git a/node/Peer.cpp b/node/Peer.cpp
index bbc6d6d25..c46ed751c 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -100,14 +100,17 @@ void Peer::received(
 		path->trustedPacketReceived(now);
 	}
 
-	if (RR->node->getMultipathMode() != ZT_MULTIPATH_NONE) {
-		if ((now - _lastPathPrune) > ZT_CLOSED_PATH_PRUNING_INTERVAL) {
-			_lastPathPrune = now;
-			prunePaths();
-		}
-		for(unsigned int i=0;i<ZT_MAX_PEER_NETWORK_PATHS;++i) {
-			if (_paths[i].p) {
-				_paths[i].p->measureLink(now);
+	{
+		Mutex::Lock _l(_paths_m);
+		if (RR->node->getMultipathMode() != ZT_MULTIPATH_NONE) {
+			if ((now - _lastPathPrune) > ZT_CLOSED_PATH_PRUNING_INTERVAL) {
+				_lastPathPrune = now;
+				prunePaths();
+			}
+			for(unsigned int i=0;i<ZT_MAX_PEER_NETWORK_PATHS;++i) {
+				if (_paths[i].p) {
+					_paths[i].p->measureLink(now);
+				}
 			}
 		}
 	}
@@ -386,9 +389,9 @@ SharedPtr<Path> Peer::getAppropriatePath(int64_t now, bool includeExpired)
 		if (bestPath == ZT_MAX_PEER_NETWORK_PATHS || (numAlivePaths == 0 && numStalePaths == 0)) {
 			return SharedPtr<Path>();
 		} if (numAlivePaths == 1) {
-			return _paths[bestPath].p;
+			//return _paths[bestPath].p;
 		} if (numStalePaths == 1) {
-			return _paths[bestPath].p;
+			//return _paths[bestPath].p;
 		}
 
 		// Relative quality
@@ -725,7 +728,6 @@ unsigned int Peer::doPingAndKeepalive(void *tPtr,int64_t now)
 
 unsigned int Peer::prunePaths()
 {
-	Mutex::Lock _l(_paths_m);
 	unsigned int pruned = 0;
 	for(unsigned int i=0;i<ZT_MAX_PEER_NETWORK_PATHS;++i) {
 		if (_paths[i].p) {
diff --git a/service/OneService.cpp b/service/OneService.cpp
index 27930a52b..563699e66 100644
--- a/service/OneService.cpp
+++ b/service/OneService.cpp
@@ -162,6 +162,7 @@ namespace ZeroTier { typedef BSDEthernetTap EthernetTap; }
 
 // How often to check for local interface addresses
 #define ZT_LOCAL_INTERFACE_CHECK_INTERVAL 60000
+#define ZT_MULTIPATH_LOCAL_INTERFACE_CHECK_INTERVAL 5000
 
 // Maximum write buffer size for outgoing TCP connections (sanity limit)
 #define ZT_TCP_MAX_WRITEQ_SIZE 33554432
@@ -856,6 +857,7 @@ public:
 				}
 
 				// Refresh bindings
+				// Do this more frequently when multipath bonding is enabled
 				int interfaceRefreshPeriod = _multipathMode ? ZT_MULTIPATH_BINDER_REFRESH_PERIOD : ZT_BINDER_REFRESH_PERIOD;
 				if (((now - lastBindRefresh) >= interfaceRefreshPeriod)||(restarted)) {
 					lastBindRefresh = now;
@@ -889,9 +891,7 @@ public:
 					uint64_t pktBuf[ZT_LINK_TEST_DATAGRAM_SZ / sizeof(uint64_t)];
 					Utils::getSecureRandom(pktBuf, ZT_LINK_TEST_DATAGRAM_SZ);
 					ZT_PeerList *pl = _node->peers();
-					// get bindings (specifically just the sockets)
 					std::vector<PhySocket*> sockets = _binder.getBoundSockets();
-					// interfaces
 					for (int i=0; i<ZT_BINDER_MAX_BINDINGS; i++) {
 						for(size_t j=0;j<pl->peerCount;++j) {
 							for (int k=0; k<(ZT_MAX_PEER_NETWORK_PATHS/4); k++) {
@@ -936,7 +936,8 @@ public:
 				}
 
 				// Sync information about physical network interfaces
-				if ((now - lastLocalInterfaceAddressCheck) >= ZT_LOCAL_INTERFACE_CHECK_INTERVAL) {
+				int interfaceAddressCheckInterval = _multipathMode ? ZT_MULTIPATH_LOCAL_INTERFACE_CHECK_INTERVAL : ZT_LOCAL_INTERFACE_CHECK_INTERVAL;
+				if ((now - lastLocalInterfaceAddressCheck) >= interfaceAddressCheckInterval) {
 					lastLocalInterfaceAddressCheck = now;
 
 					_node->clearLocalInterfaceAddresses();