Clean up some YAGNI issues with implementation of GitHub issue #180, and make best path choice aware of path rank.

include/ZeroTierOne.h

@@ -987,10 +987,9 @@ void ZT1_Node_freeQueryResult(ZT1_Node *node,void *qr);
  * @param addr Local interface address
  * @param metric Local interface metric
  * @param trust How much do you trust the local network under this interface?
- * @param reliable If nonzero, this interface doesn't link to anything behind a NAT or stateful firewall
  * @return Boolean: non-zero if address was accepted and added
  */
-int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable);
+int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust);
 
 /**
  * Clear local interface addresses

node/IncomingPacket.cpp

@@ -909,7 +909,6 @@ bool IncomingPacket::_doPUSH_DIRECT_PATHS(const RuntimeEnvironment *RR,const Sha
 			// TODO: properly handle blacklisting, support other features... see Packet.hpp.
 
 			unsigned int flags = (*this)[ptr++];
-			/*int metric = (*this)[ptr++];*/ ++ptr;
 			unsigned int extLen = at<uint16_t>(ptr); ptr += 2;
 			ptr += extLen; // unused right now
 			unsigned int addrType = (*this)[ptr++];

node/InetAddress.hpp

@@ -63,17 +63,20 @@ struct InetAddress : public sockaddr_storage
 
 	/**
 	 * IP address scope
+	 *
+	 * Note that these values are in ascending order of path preference and
+	 * MUST remain that way or Path must be changed to reflect.
 	 */
 	enum IpScope
 	{
-		IP_SCOPE_NONE = 0,          // not an IP address -- also the number of classes, must be last entry
-		IP_SCOPE_LINK_LOCAL = 1,    // 169.254.x.x, IPv6 LL
-		IP_SCOPE_PRIVATE = 2,       // 10.x.x.x, etc.
-		IP_SCOPE_PSEUDOPRIVATE = 3, // 28.x.x.x, etc. -- unofficially unrouted IP blocks often "bogarted"
-		IP_SCOPE_SHARED = 4,        // 100.64.0.0/10, shared space for e.g. carrier-grade NAT
-		IP_SCOPE_GLOBAL = 5,        // globally routable IP address (all others)
-		IP_SCOPE_LOOPBACK = 6,      // 127.0.0.1
-		IP_SCOPE_MULTICAST = 7      // 224.0.0.0 and other multicast IPs
+		IP_SCOPE_NONE = 0,          // NULL or not an IP address
+		IP_SCOPE_MULTICAST = 1,     // 224.0.0.0 and other V4/V6 multicast IPs
+		IP_SCOPE_LOOPBACK = 2,      // 127.0.0.1, ::1, etc.
+		IP_SCOPE_PSEUDOPRIVATE = 3, // 28.x.x.x, etc. -- unofficially unrouted IPv4 blocks often "bogarted"
+		IP_SCOPE_GLOBAL = 4,        // globally routable IP address (all others)
+		IP_SCOPE_LINK_LOCAL = 5,    // 169.254.x.x, IPv6 LL
+		IP_SCOPE_SHARED = 6,        // 100.64.0.0/10, shared space for e.g. carrier-grade NAT
+		IP_SCOPE_PRIVATE = 7        // 10.x.x.x, 192.168.x.x, etc.
 	};
 
 	InetAddress() throw() { memset(this,0,sizeof(InetAddress)); }

node/Node.cpp

@@ -432,11 +432,11 @@ void Node::freeQueryResult(void *qr)
 		::free(qr);
 }
 
-int Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
+int Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust)
 {
 	if (Path::isAddressValidForPath(*(reinterpret_cast<const InetAddress *>(addr)))) {
 		Mutex::Lock _l(_directPaths_m);
-		_directPaths.push_back(Path(*(reinterpret_cast<const InetAddress *>(addr)),metric,(Path::Trust)trust,reliable != 0));
+		_directPaths.push_back(Path(*(reinterpret_cast<const InetAddress *>(addr)),metric,(Path::Trust)trust));
 		std::sort(_directPaths.begin(),_directPaths.end());
 		_directPaths.erase(std::unique(_directPaths.begin(),_directPaths.end()),_directPaths.end());
 		return 1;
@@ -711,10 +711,10 @@ void ZT1_Node_setNetconfMaster(ZT1_Node *node,void *networkControllerInstance)
 	} catch ( ... ) {}
 }
 
-int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
+int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust)
 {
 	try {
-		return reinterpret_cast<ZeroTier::Node *>(node)->addLocalInterfaceAddress(addr,metric,trust,reliable);
+		return reinterpret_cast<ZeroTier::Node *>(node)->addLocalInterfaceAddress(addr,metric,trust);
 	} catch ( ... ) {
 		return 0;
 	}

node/Node.hpp

@@ -105,7 +105,7 @@ public:
 	ZT1_VirtualNetworkConfig *networkConfig(uint64_t nwid) const;
 	ZT1_VirtualNetworkList *networks() const;
 	void freeQueryResult(void *qr);
-	int addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable);
+	int addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust);
 	void clearLocalInterfaceAddresses();
 	void setNetconfMaster(void *networkControllerInstance);
 

node/Packet.hpp

@@ -872,7 +872,6 @@ public:
 		 *
 		 * Path record format:
 		 *   <[1] flags>
-		 *   <[1] metric from 0 (highest priority) to 255 (lowest priority)>
 		 *   <[2] length of extended path characteristics or 0 for none>
 		 *   <[...] extended path characteristics>
 		 *   <[1] address type>
@@ -882,9 +881,8 @@ public:
 		 * Path record flags:
 		 *   0x01 - Forget this path if it is currently known
 		 *   0x02 - Blacklist this path, do not use
-		 *   0x04 - Reliable path (no NAT keepalives, etc. are necessary)
-		 *   0x08 - Disable encryption (trust: privacy)
-		 *   0x10 - Disable encryption and authentication (trust: ultimate)
+		 *   0x04 - Disable encryption (trust: privacy)
+		 *   0x08 - Disable encryption and authentication (trust: ultimate)
 		 *
 		 * Address types and addresses are of the same format as the destination
 		 * address type and address in HELLO.
@@ -901,15 +899,10 @@ public:
 		 * is set.
 		 *
 		 * Only a subset of this functionality is currently implemented: basic
-		 * path pushing and learning. Metrics, most flags, and OK responses are
-		 * not yet implemented as of 1.0.4.
+		 * path pushing and learning. Blacklisting and trust are not fully
+		 * implemented yet (encryption is still always used).
 		 *
-		 * OK response payload:
-		 *   <[2] 16-bit number of active direct paths we already have>
-		 *   <[2] 16-bit number of paths in push that we don't already have>
-		 *   <[2] 16-bit number of new paths we are trying (or will try)>
-		 *
-		 * ERROR is presently not sent.
+		 * OK and ERROR are not generated.
 		 */
 		VERB_PUSH_DIRECT_PATHS = 16
 	};

node/Path.hpp

@@ -34,10 +34,31 @@
 
 namespace ZeroTier {
 
+/**
+ * Base class for paths
+ *
+ * The base Path class is an immutable value.
+ */
 class Path
 {
 public:
-	// Must be the same values as ZT1_LocalInterfaceAddressTrust in ZeroTierOne.h
+	/**
+	 * Path trust category
+	 *
+	 * Note that this is NOT peer trust and has nothing to do with root server
+	 * designations or other trust metrics. This indicates how much we trust
+	 * this path to be secure and/or private. A trust level of normal means
+	 * encrypt and authenticate all traffic. Privacy trust means we can send
+	 * traffic in the clear. Ultimate trust means we don't even need
+	 * authentication. Generally a private path would be a hard-wired local
+	 * LAN, while an ultimate trust path would be a physically isolated private
+	 * server backplane.
+	 *
+	 * Nearly all paths will be normal trust. The other levels are for high
+	 * performance local SDN use only.
+	 *
+	 * These values MUST match ZT1_LocalInterfaceAddressTrust in ZeroTierOne.h
+	 */
 	enum Trust
 	{
 		TRUST_NORMAL = 0,
@@ -47,17 +68,15 @@ public:
 
 	Path() :
 		_addr(),
-		_metric(0),
-		_trust(TRUST_NORMAL),
-		_reliable(false)
+		_ipScope(InetAddress::IP_SCOPE_NONE),
+		_trust(TRUST_NORMAL)
 	{
 	}
 
-	Path(const InetAddress &addr,int metric,Trust trust,bool reliable) :
+	Path(const InetAddress &addr,int metric,Trust trust) :
 		_addr(addr),
-		_metric(metric),
-		_trust(trust),
-		_reliable(reliable)
+		_ipScope(addr.ipScope()),
+		_trust(trust)
 	{
 	}
 
@@ -67,9 +86,14 @@ public:
 	inline const InetAddress &address() const throw() { return _addr; }
 
 	/**
-	 * @return Metric (higher == worse) or negative if path is blacklisted
+	 * @return IP scope -- faster shortcut for address().ipScope()
 	 */
-	inline int metric() const throw() { return _metric; }
+	inline InetAddress::IpScope ipScope() const throw() { return _ipScope; }
+
+	/**
+	 * @return Preference rank, higher == better
+	 */
+	inline int preferenceRank() const throw() { return (int)_ipScope; } // IP scopes are in ascending rank order in InetAddress.hpp
 
 	/**
 	 * @return Path trust level
@@ -79,7 +103,10 @@ public:
 	/**
 	 * @return True if path is considered reliable (no NAT keepalives etc. are needed)
 	 */
-	inline bool reliable() const throw() { return _reliable; }
+	inline bool reliable() const throw()
+	{
+		return ((_ipScope != InetAddress::IP_SCOPE_GLOBAL)&&(_ipScope != InetAddress::IP_SCOPE_PSEUDOPRIVATE));
+	}
 
 	/**
 	 * @return True if address is non-NULL
@@ -127,11 +154,10 @@ public:
 		return false;
 	}
 
-protected:
+private:
 	InetAddress _addr;
-	int _metric; // negative == blacklisted
+	InetAddress::IpScope _ipScope; // memoize this since it's a computed value checked often
 	Trust _trust;
-	bool _reliable;
 };
 
 } // namespace ZeroTier

node/Peer.cpp

@@ -161,6 +161,21 @@ void Peer::received(
 		_lastMulticastFrame = now;
 }
 
+RemotePath *Peer::getBestPath(uint64_t now)
+{
+	RemotePath *bestPath = (RemotePath *)0;
+	uint64_t lrMax = 0;
+	int rank = 0;
+	for(unsigned int p=0,np=_numPaths;p<np;++p) {
+		if ( (_paths[p].active(now)) && ((_paths[p].lastReceived() >= lrMax)||(_paths[p].preferenceRank() >= rank)) ) {
+			lrMax = _paths[p].lastReceived();
+			rank = _paths[p].preferenceRank();
+			bestPath = &(_paths[p]);
+		}
+	}
+	return bestPath;
+}
+
 void Peer::attemptToContactAt(const RuntimeEnvironment *RR,const InetAddress &atAddress,uint64_t now)
 {
 	Packet outp(_id.address(),RR->identity.address(),Packet::VERB_HELLO);
@@ -200,7 +215,7 @@ void Peer::doPingAndKeepalive(const RuntimeEnvironment *RR,uint64_t now)
 			TRACE("PING %s(%s)",_id.address().toString().c_str(),bestPath->address().toString().c_str());
 			attemptToContactAt(RR,bestPath->address(),now);
 			bestPath->sent(now);
-		} else if ((now - bestPath->lastSend()) >= ZT_NAT_KEEPALIVE_DELAY) {
+		} else if (((now - bestPath->lastSend()) >= ZT_NAT_KEEPALIVE_DELAY)&&(!bestPath->reliable())) {
 			TRACE("NAT keepalive %s(%s)",_id.address().toString().c_str(),bestPath->address().toString().c_str());
 			RR->node->putPacket(bestPath->address(),"",0);
 			bestPath->sent(now);
@@ -230,31 +245,24 @@ void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_
 					case AF_INET6:
 						addressType = 6;
 						break;
-					default:
+					default: // we currently only push IP addresses
 						++p;
 						continue;
 				}
 
 				uint8_t flags = 0;
-				if (p->metric() < 0)
-					flags |= (0x01 | 0x02); // forget and blacklist
-				else {
-					if (p->reliable())
-						flags |= 0x04; // no NAT keepalives and such
-					switch(p->trust()) {
-						default:
-							break;
-						case Path::TRUST_PRIVACY:
-							flags |= 0x08; // no encryption
-							break;
-						case Path::TRUST_ULTIMATE:
-							flags |= (0x08 | 0x10); // no encryption, no authentication (redundant but go ahead and set both)
-							break;
-					}
+				switch(p->trust()) {
+					default:
+						break;
+					case Path::TRUST_PRIVACY:
+						flags |= 0x04; // no encryption
+						break;
+					case Path::TRUST_ULTIMATE:
+						flags |= (0x04 | 0x08); // no encryption, no authentication (redundant but go ahead and set both)
+						break;
 				}
 
 				outp.append(flags);
-				outp.append((uint8_t)((p->metric() >= 0) ? ((p->metric() <= 255) ? p->metric() : 255) : 0));
 				outp.append((uint16_t)0); // no extensions
 				outp.append(addressType);
 				outp.append((uint8_t)((addressType == 4) ? 6 : 18));

node/Peer.hpp

@@ -126,18 +126,7 @@ public:
 	 * @param now Current time
 	 * @return Best path or NULL if there are no active (or fixed) direct paths
 	 */
-	inline RemotePath *getBestPath(uint64_t now)
-	{
-		RemotePath *bestPath = (RemotePath *)0;
-		uint64_t lrMax = 0;
-		for(unsigned int p=0,np=_numPaths;p<np;++p) {
-			if ((_paths[p].active(now))&&(_paths[p].lastReceived() >= lrMax)) {
-				lrMax = _paths[p].lastReceived();
-				bestPath = &(_paths[p]);
-			}
-		}
-		return bestPath;
-	}
+	RemotePath *getBestPath(uint64_t now);
 
 	/**
 	 * Send via best path

node/RemotePath.hpp

@@ -56,7 +56,7 @@ public:
 		_fixed(false) {}
 
 	RemotePath(const InetAddress &addr,bool fixed) :
-		Path(addr,0,TRUST_NORMAL,false),
+		Path(addr,0,TRUST_NORMAL),
 		_lastSend(0),
 		_lastReceived(0),
 		_fixed(fixed) {}
@@ -123,7 +123,7 @@ public:
 	 */
 	inline bool send(const RuntimeEnvironment *RR,const void *data,unsigned int len,uint64_t now)
 	{
-		if (RR->node->putPacket(_addr,data,len)) {
+		if (RR->node->putPacket(address(),data,len)) {
 			sent(now);
 			RR->antiRec->logOutgoingZT(data,len);
 			return true;

service/OneService.cpp

@@ -592,7 +592,7 @@ public:
 								if (!isZT) {
 									InetAddress ip(ifa->ifa_addr);
 									ip.setPort(_port);
-									_node->addLocalInterfaceAddress(reinterpret_cast<const struct sockaddr_storage *>(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL,0);
+									_node->addLocalInterfaceAddress(reinterpret_cast<const struct sockaddr_storage *>(&ip),0,ZT1_LOCAL_INTERFACE_ADDRESS_TRUST_NORMAL);
 								}
 							}
 							ifa = ifa->ifa_next;