Route management bug fixes.

osdep/ManagedRoute.cpp

@@ -240,6 +240,7 @@ static std::vector<_RTE> _getRTEs(const InetAddress &target,bool contains)
 
 static void _routeCmd(const char *op,const InetAddress &target,const InetAddress &via,const char *ifscope,const char *localInterface)
 {
+	//printf("route %s %s %s %s %s\n",op,target.toString().c_str(),(via) ? via.toString().c_str() : "(null)",(ifscope) ? ifscope : "(null)",(localInterface) ? localInterface : "(null)");
 	long p = (long)fork();
 	if (p > 0) {
 		int exitcode = -1;
@@ -436,7 +437,7 @@ bool ManagedRoute::sync()
 	}
 
 	if (!_applied.count(leftt)) {
-		_applied[rightt] = false; // not ifscoped
+		_applied[leftt] = false; // not ifscoped
 		_routeCmd("add",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
 		_routeCmd("change",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
 	}

osdep/ManagedRoute.hpp

@@ -6,6 +6,9 @@
 
 #include "../node/InetAddress.hpp"
 #include "../node/Utils.hpp"
+#include "../node/SharedPtr.hpp"
+#include "../node/AtomicCounter.hpp"
+#include "../node/NonCopyable.hpp"
 
 #include <stdexcept>
 #include <vector>
@@ -16,58 +19,13 @@ namespace ZeroTier {
 /**
  * A ZT-managed route that used C++ RAII semantics to automatically clean itself up on deallocate
  */
-class ManagedRoute
+class ManagedRoute : NonCopyable
 {
-public:
-	ManagedRoute()
-	{
-		_device[0] = (char)0;
-		_systemDevice[0] = (char)0;
-	}
-
-	~ManagedRoute()
-	{
-		this->remove();
-	}
-
-	ManagedRoute(const ManagedRoute &r)
-	{
-		*this = r;
-	}
-
-	inline ManagedRoute &operator=(const ManagedRoute &r)
-	{
-		if (_applied.size() == 0) {
-			_target = r._target;
-			_via = r._via;
-			_systemVia = r._systemVia;
-			_applied = r._applied;
-			Utils::scopy(_device,sizeof(_device),r._device);
-			Utils::scopy(_systemDevice,sizeof(_systemDevice),r._systemDevice);
-		} else {
-			// Sanity check -- this is an 'assert' to detect a bug
-			fprintf(stderr,"Applied ManagedRoute isn't copyable!\n");
-			abort();
-		}
-		return *this;
-	}
+	friend class SharedPtr<ManagedRoute>;
 
-	/**
-	 * Initialize object and set route
-	 *
-	 * Note: on Windows, use the interface NET_LUID in hexadecimal as the
-	 * "device name."
-	 *
-	 * @param target Route target (e.g. 0.0.0.0/0 for default)
-	 * @param via Route next L3 hop or NULL InetAddress if local in which case it will be routed via device
-	 * @param device Name or hex LUID of ZeroTier device (e.g. zt#)
-	 * @return True if route was successfully set
-	 */
-	inline bool set(const InetAddress &target,const InetAddress &via,const char *device)
+public:
+	ManagedRoute(const InetAddress &target,const InetAddress &via,const char *device)
 	{
-		if ((!via)&&(!device[0]))
-			return false;
-		this->remove();
 		_target = target;
 		_via = via;
 		if (via.ss_family == AF_INET)
@@ -75,7 +33,12 @@ public:
 		else if (via.ss_family == AF_INET6)
 			_via.setPort(128);
 		Utils::scopy(_device,sizeof(_device),device);
-		return this->sync();
+		_systemDevice[0] = (char)0;
+	}
+
+	~ManagedRoute()
+	{
+		this->remove();
 	}
 
 	/**
@@ -108,6 +71,8 @@ private:
 	std::map<InetAddress,bool> _applied; // routes currently applied
 	char _device[128];
 	char _systemDevice[128]; // for route overrides
+
+	AtomicCounter __refCount;
 };
 
 } // namespace ZeroTier

service/OneService.cpp

@@ -537,7 +537,7 @@ public:
 		EthernetTap *tap;
 		ZT_VirtualNetworkConfig config; // memcpy() of raw config from core
 		std::vector<InetAddress> managedIps;
-		std::list<ManagedRoute> managedRoutes;
+		std::list< SharedPtr<ManagedRoute> > managedRoutes;
 		NetworkSettings settings;
 	};
 	std::map<uint64_t,NetworkState> _nets;
@@ -1128,13 +1128,13 @@ public:
 			std::vector<InetAddress> myIps(n.tap->ips());
 
 			// Nuke applied routes that are no longer in n.config.routes[] and/or are not allowed
-			for(std::list<ManagedRoute>::iterator mr(n.managedRoutes.begin());mr!=n.managedRoutes.end();) {
+			for(std::list< SharedPtr<ManagedRoute> >::iterator mr(n.managedRoutes.begin());mr!=n.managedRoutes.end();) {
 				bool haveRoute = false;
-				if ( (checkIfManagedIsAllowed(n,mr->target())) && ((mr->via().ss_family != mr->target().ss_family)||(!matchIpOnly(myIps,mr->via()))) ) {
+				if ( (checkIfManagedIsAllowed(n,(*mr)->target())) && (((*mr)->via().ss_family != (*mr)->target().ss_family)||(!matchIpOnly(myIps,(*mr)->via()))) ) {
 					for(unsigned int i=0;i<n.config.routeCount;++i) {
 						const InetAddress *const target = reinterpret_cast<const InetAddress *>(&(n.config.routes[i].target));
 						const InetAddress *const via = reinterpret_cast<const InetAddress *>(&(n.config.routes[i].via));
-						if ( (mr->target() == *target) && ( ((via->ss_family == target->ss_family)&&(mr->via() == *via)) || (tapdev == mr->device()) ) ) {
+						if ( ((*mr)->target() == *target) && ( ((via->ss_family == target->ss_family)&&((*mr)->via().ipsEqual(*via))) || (tapdev == (*mr)->device()) ) ) {
 							haveRoute = true;
 							break;
 						}
@@ -1168,10 +1168,10 @@ public:
 					continue;
 
 				// If we've already applied this route, just sync it and continue
-				for(std::list<ManagedRoute>::iterator mr(n.managedRoutes.begin());mr!=n.managedRoutes.end();++mr) {
-					if ( (mr->target() == *target) && ( ((via->ss_family == target->ss_family)&&(mr->via() == *via)) || (tapdev == mr->device()) ) ) {
+				for(std::list< SharedPtr<ManagedRoute> >::iterator mr(n.managedRoutes.begin());mr!=n.managedRoutes.end();++mr) {
+					if ( ((*mr)->target() == *target) && ( ((via->ss_family == target->ss_family)&&((*mr)->via().ipsEqual(*via))) || (tapdev == (*mr)->device()) ) ) {
 						haveRoute = true;
-						mr->sync();
+						(*mr)->sync();
 						break;
 					}
 				}
@@ -1179,8 +1179,8 @@ public:
 					continue;
 
 				// Add and apply new routes
-				n.managedRoutes.push_back(ManagedRoute());
-				if (!n.managedRoutes.back().set(*target,*via,tapdev))
+				n.managedRoutes.push_back(SharedPtr<ManagedRoute>(new ManagedRoute(*target,*via,tapdev)));
+				if (!n.managedRoutes.back()->sync())
 					n.managedRoutes.pop_back();
 			}
 		}