Mark geo-redirected paths as suboptimal and do not report that we have a peer if all we have is one of these. Also a few other small fixes.

5 files changed, 59 insertions, 46 deletions

diff --git a/node/Cluster.cpp b/node/Cluster.cpp
index 419948e2..a819372e 100644
--- a/node/Cluster.cpp
+++ b/node/Cluster.cpp
@@ -239,7 +239,7 @@ void Cluster::handleIncomingStateMessage(const void *msg,unsigned int len)
 					case CLUSTER_MESSAGE_WANT_PEER: {
 						const Address zeroTierAddress(dmsg.field(ptr,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH); ptr += ZT_ADDRESS_LENGTH;
 						SharedPtr<Peer> peer(RR->topology->getPeerNoCache(zeroTierAddress));
-						if ( (peer) && (peer->hasActiveDirectPath(RR->node->now())) ) {
+						if ( (peer) && (peer->hasClusterOptimalPath(RR->node->now())) ) {
 							Buffer<1024> buf;
 							peer->identity().serialize(buf);
 							Mutex::Lock _l2(_members[fromMemberId].lock);
diff --git a/node/Cluster.hpp b/node/Cluster.hpp
index 4197a14b..1c4331b4 100644
--- a/node/Cluster.hpp
+++ b/node/Cluster.hpp
@@ -55,7 +55,7 @@
  * A cluster member is considered dead and will no longer have peers
  * redirected to it if we have not heard a heartbeat in this long.
  */
-#define ZT_CLUSTER_TIMEOUT 10000
+#define ZT_CLUSTER_TIMEOUT 5000
 
 /**
  * Desired period between doPeriodicTasks() in milliseconds
diff --git a/node/Path.hpp b/node/Path.hpp
index 39a18c43..00f8ed36 100644
--- a/node/Path.hpp
+++ b/node/Path.hpp
@@ -37,6 +37,16 @@
 #include "Constants.hpp"
 #include "InetAddress.hpp"
 
+/**
+ * Flag indicating that this path is suboptimal
+ *
+ * This is used in cluster mode to indicate that the peer has been directed
+ * to a better path. This path can continue to be used but shouldn't be kept
+ * or advertised to other cluster members. Not used if clustering is not
+ * built and enabled.
+ */
+#define ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL 0x0001
+
 namespace ZeroTier {
 
 class RuntimeEnvironment;
@@ -54,6 +64,7 @@ public:
 		_lastReceived(0),
 		_addr(),
 		_localAddress(),
+		_flags(0),
 		_ipScope(InetAddress::IP_SCOPE_NONE)
 	{
 	}
@@ -63,12 +74,12 @@ public:
 		_lastReceived(0),
 		_addr(addr),
 		_localAddress(localAddress),
+		_flags(0),
 		_ipScope(addr.ipScope())
 	{
 	}
 
 	inline Path &operator=(const Path &p)
-		throw()
 	{
 		if (this != &p)
 			memcpy(this,&p,sizeof(Path));
@@ -82,22 +93,14 @@ public:
 	 *
 	 * @param t Time of send
 	 */
-	inline void sent(uint64_t t)
-		throw()
-	{
-		_lastSend = t;
-	}
+	inline void sent(uint64_t t) { _lastSend = t; }
 
 	/**
 	 * Called when a packet is received from this remote path
 	 *
 	 * @param t Time of receive
 	 */
-	inline void received(uint64_t t)
-		throw()
-	{
-		_lastReceived = t;
-	}
+	inline void received(uint64_t t) { _lastReceived = t; }
 
 	/**
 	 * @param now Current time
@@ -207,26 +210,40 @@ public:
 		return false;
 	}
 
+#ifdef ZT_ENABLE_CLUSTER
+	/**
+	 * @param f New value of ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL
+	 */
+	inline void setClusterSuboptimal(bool f) { _flags = ((f) ? (_flags | ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL) : (_flags & (~ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL))); }
+
+	/**
+	 * @return True if ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL is set
+	 */
+	inline bool isClusterSuboptimal() const { return ((_flags & ZT_PATH_FLAG_CLUSTER_SUBOPTIMAL) != 0); }
+#endif
+
 	template<unsigned int C>
 	inline void serialize(Buffer<C> &b) const
 	{
-		b.append((uint8_t)1); // version
+		b.append((uint8_t)0); // version
 		b.append((uint64_t)_lastSend);
 		b.append((uint64_t)_lastReceived);
 		_addr.serialize(b);
 		_localAddress.serialize(b);
+		b.append((uint16_t)_flags);
 	}
 
 	template<unsigned int C>
 	inline unsigned int deserialize(const Buffer<C> &b,unsigned int startAt = 0)
 	{
 		unsigned int p = startAt;
-		if (b[p++] != 1)
+		if (b[p++] != 0)
 			throw std::invalid_argument("invalid serialized Path");
 		_lastSend = b.template at<uint64_t>(p); p += 8;
 		_lastReceived = b.template at<uint64_t>(p); p += 8;
 		p += _addr.deserialize(b,p);
 		p += _localAddress.deserialize(b,p);
+		_flags = b.template at<uint16_t>(p); p += 2;
 		_ipScope = _addr.ipScope();
 		return (p - startAt);
 	}
@@ -236,6 +253,7 @@ private:
 	uint64_t _lastReceived;
 	InetAddress _addr;
 	InetAddress _localAddress;
+	unsigned int _flags;
 	InetAddress::IpScope _ipScope; // memoize this since it's a computed value checked often
 };
 
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 6f987da2..0e90b17d 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -83,6 +83,7 @@ void Peer::received(
 	Packet::Verb inReVerb)
 {
 #ifdef ZT_ENABLE_CLUSTER
+	bool suboptimalPath = false;
 	if ((RR->cluster)&&(hops == 0)) {
 		// Note: findBetterEndpoint() is first since we still want to check
 		// for a better endpoint even if we don't actually send a redirect.
@@ -124,6 +125,7 @@ void Peer::received(
 				RR->antiRec->logOutgoingZT(outp.data(),outp.size());
 				RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
 			}
+			suboptimalPath = true;
 		}
 	}
 #endif
@@ -151,6 +153,9 @@ void Peer::received(
 			for(unsigned int p=0;p<np;++p) {
 				if ((_paths[p].address() == remoteAddr)&&(_paths[p].localAddress() == localAddr)) {
 					_paths[p].received(now);
+#ifdef ZT_ENABLE_CLUSTER
+					_paths[p].setClusterSuboptimal(suboptimalPath);
+#endif
 					pathIsConfirmed = true;
 					break;
 				}
@@ -174,6 +179,9 @@ void Peer::received(
 					if (slot) {
 						*slot = Path(localAddr,remoteAddr);
 						slot->received(now);
+#ifdef ZT_ENABLE_CLUSTER
+						slot->setClusterSuboptimal(suboptimalPath);
+#endif
 						_numPaths = np;
 						pathIsConfirmed = true;
 						_sortPaths(now);
diff --git a/node/Peer.hpp b/node/Peer.hpp
index 42708128..bf627caf 100644
--- a/node/Peer.hpp
+++ b/node/Peer.hpp
@@ -269,7 +269,6 @@ public:
 	 * @param l Direct latency measurment in ms
 	 */
 	inline void addDirectLatencyMeasurment(unsigned int l)
-		throw()
 	{
 		unsigned int ol = _latency;
 		if ((ol > 0)&&(ol < 10000))
@@ -282,7 +281,6 @@ public:
 	 * @return True if this peer has at least one active direct path
 	 */
 	inline bool hasActiveDirectPath(uint64_t now) const
-		throw()
 	{
 		Mutex::Lock _l(_lock);
 		for(unsigned int p=0,np=_numPaths;p<np;++p) {
@@ -292,6 +290,22 @@ public:
 		return false;
 	}
 
+#ifdef ZT_ENABLE_CLUSTER
+	/**
+	 * @param now Current time
+	 * @return True if this peer has at least one active direct path that is not cluster-suboptimal
+	 */
+	inline bool hasClusterOptimalPath(uint64_t now) const
+	{
+		Mutex::Lock _l(_lock);
+		for(unsigned int p=0,np=_numPaths;p<np;++p) {
+			if ((_paths[p].active(now))&&(!_paths[p].isClusterSuboptimal()))
+				return true;
+		}
+		return false;
+	}
+#endif
+
 	/**
 	 * Reset paths within a given scope
 	 *
@@ -330,33 +344,6 @@ public:
 	inline bool remoteVersionKnown() const throw() { return ((_vMajor > 0)||(_vMinor > 0)||(_vRevision > 0)); }
 
 	/**
-	 * Check whether this peer's version is both known and is at least what is specified
-	 *
-	 * @param major Major version to check against
-	 * @param minor Minor version
-	 * @param rev Revision
-	 * @return True if peer's version is at least supplied tuple
-	 */
-	inline bool atLeastVersion(unsigned int major,unsigned int minor,unsigned int rev)
-		throw()
-	{
-		Mutex::Lock _l(_lock);
-		if ((_vMajor > 0)||(_vMinor > 0)||(_vRevision > 0)) {
-			if (_vMajor > major)
-				return true;
-			else if (_vMajor == major) {
-				if (_vMinor > minor)
-					return true;
-				else if (_vMinor == minor) {
-					if (_vRevision >= rev)
-						return true;
-				}
-			}
-		}
-		return false;
-	}
-
-	/**
 	 * Get most recently active path addresses for IPv4 and/or IPv6
 	 *
 	 * Note that v4 and v6 are not modified if they are not found, so
@@ -467,7 +454,7 @@ public:
 		const unsigned int recSizePos = b.size();
 		b.addSize(4); // space for uint32_t field length
 
-		b.append((uint16_t)1); // version of serialized Peer data
+		b.append((uint16_t)0); // version of serialized Peer data
 
 		_id.serialize(b,false);
 
@@ -531,7 +518,7 @@ public:
 		const unsigned int recSize = b.template at<uint32_t>(p); p += 4;
 		if ((p + recSize) > b.size())
 			return SharedPtr<Peer>(); // size invalid
-		if (b.template at<uint16_t>(p) != 1)
+		if (b.template at<uint16_t>(p) != 0)
 			return SharedPtr<Peer>(); // version mismatch
 		p += 2;