From da9371284625d2481ed496921505c8afd2dae1f0 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Wed, 28 Oct 2015 09:11:30 -0700 Subject: [PATCH] Clean up PUSH_DIRECT_PATH limits a bit more and make them a bit smarter. --- node/Constants.hpp | 10 +++++----- node/IncomingPacket.cpp | 25 +++++++++++++++---------- node/InetAddress.hpp | 8 +++++++- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/node/Constants.hpp b/node/Constants.hpp index 53cc64c7e..5a4d4a4de 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -347,11 +347,6 @@ */ #define ZT_DIRECT_PATH_PUSH_INTERVAL 120000 -/** - * Maximum number of endpoints to contact per address type (to limit pushes like GitHub issue #235) - */ -#define ZT_PUSH_DIRECT_PATHS_MAX_ENDPOINTS_PER_TYPE 5 - /** * Time horizon for push direct paths cutoff */ @@ -366,6 +361,11 @@ */ #define ZT_PUSH_DIRECT_PATHS_CUTOFF_LIMIT 5 +/** + * Maximum number of paths per IP scope (e.g. global, link-local) and family (e.g. v4/v6) + */ +#define ZT_PUSH_DIRECT_PATHS_MAX_PER_SCOPE_AND_FAMILY 1 + /** * A test pseudo-network-ID that can be joined * diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index e985b34c4..f06eb30c3 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -901,16 +901,19 @@ bool IncomingPacket::_doPUSH_DIRECT_PATHS(const RuntimeEnvironment *RR,const Sha { try { const uint64_t now = RR->node->now(); + + // First, subject this to a rate limit if (!peer->shouldRespondToDirectPathPush(now)) { TRACE("dropped PUSH_DIRECT_PATHS from %s(%s): circuit breaker tripped",source().toString().c_str(),_remoteAddress.toString().c_str()); return true; } - const Path *currentBest = peer->getBestPath(now); + // Second, limit addresses by scope and type + uint8_t countPerScope[ZT_INETADDRESS_MAX_SCOPE+1][2]; // [][0] is v4, [][1] is v6 + memset(countPerScope,0,sizeof(countPerScope)); unsigned int count = at(ZT_PACKET_IDX_PAYLOAD); unsigned int ptr = ZT_PACKET_IDX_PAYLOAD + 2; - unsigned int v4Count = 0,v6Count = 0; while (count--) { // if ptr overflows Buffer will throw // TODO: some flags are not yet implemented @@ -925,20 +928,22 @@ bool IncomingPacket::_doPUSH_DIRECT_PATHS(const RuntimeEnvironment *RR,const Sha case 4: { InetAddress a(field(ptr,4),4,at(ptr + 4)); if ( ((flags & 0x01) == 0) && (Path::isAddressValidForPath(a)) ) { - TRACE("attempting to contact %s at pushed direct path %s",peer->address().toString().c_str(),a.toString().c_str()); - if (v4Count++ < ZT_PUSH_DIRECT_PATHS_MAX_ENDPOINTS_PER_TYPE) { - if ((!currentBest)||(currentBest->address() != a)) - peer->attemptToContactAt(RR,_localAddress,a,now); + if (++countPerScope[(int)a.ipScope()][0] <= ZT_PUSH_DIRECT_PATHS_MAX_PER_SCOPE_AND_FAMILY) { + TRACE("attempting to contact %s at pushed direct path %s",peer->address().toString().c_str(),a.toString().c_str()); + peer->attemptToContactAt(RR,_localAddress,a,now); + } else { + TRACE("ignoring contact for %s at %s -- too many per scope",peer->address().toString().c_str(),a.toString().c_str()); } } } break; case 6: { InetAddress a(field(ptr,16),16,at(ptr + 16)); if ( ((flags & 0x01) == 0) && (Path::isAddressValidForPath(a)) ) { - TRACE("attempting to contact %s at pushed direct path %s",peer->address().toString().c_str(),a.toString().c_str()); - if (v6Count++ < ZT_PUSH_DIRECT_PATHS_MAX_ENDPOINTS_PER_TYPE) { - if ((!currentBest)||(currentBest->address() != a)) - peer->attemptToContactAt(RR,_localAddress,a,now); + if (++countPerScope[(int)a.ipScope()][1] <= ZT_PUSH_DIRECT_PATHS_MAX_PER_SCOPE_AND_FAMILY) { + TRACE("attempting to contact %s at pushed direct path %s",peer->address().toString().c_str(),a.toString().c_str()); + peer->attemptToContactAt(RR,_localAddress,a,now); + } else { + TRACE("ignoring contact for %s at %s -- too many per scope",peer->address().toString().c_str(),a.toString().c_str()); } } } break; diff --git a/node/InetAddress.hpp b/node/InetAddress.hpp index fcbed4b15..5e5eb06ee 100644 --- a/node/InetAddress.hpp +++ b/node/InetAddress.hpp @@ -42,6 +42,11 @@ namespace ZeroTier { +/** + * Maximum integer value of enum IpScope + */ +#define ZT_INETADDRESS_MAX_SCOPE 7 + /** * Extends sockaddr_storage with friendly C++ methods * @@ -66,7 +71,8 @@ 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. + * MUST remain that way or Path must be changed to reflect. Also be sure + * to change ZT_INETADDRESS_MAX_SCOPE if the max changes. */ enum IpScope {