Browse Source

Fix InetAddress sizing by delving into crazy C++ weeds, fix Peer compile issues.

Adam Ierymenko 5 years ago
parent
commit
c3b5c45fea
6 changed files with 66 additions and 58 deletions
  1. 22 22
      node/InetAddress.cpp
  2. 28 18
      node/InetAddress.hpp
  3. 11 14
      node/Peer.cpp
  4. 2 1
      node/Peer.hpp
  5. 1 0
      node/Tests.cpp
  6. 2 3
      node/TriviallyCopyable.hpp

+ 22 - 22
node/InetAddress.cpp

@@ -26,7 +26,7 @@ const InetAddress InetAddress::NIL;
 
 InetAddress::IpScope InetAddress::ipScope() const noexcept
 {
-	switch(ss_family) {
+	switch(_data.ss_family) {
 
 		case AF_INET: {
 			const uint32_t ip = Utils::ntoh((uint32_t)reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr);
@@ -100,11 +100,11 @@ void InetAddress::set(const void *ipBytes,unsigned int ipLen,unsigned int port)
 	if (ipLen == 4) {
 		uint32_t ipb[1];
 		memcpy(ipb,ipBytes,4);
-		ss_family = AF_INET;
+		_data.ss_family = AF_INET;
 		reinterpret_cast<struct sockaddr_in *>(this)->sin_addr.s_addr = ipb[0];
 		reinterpret_cast<struct sockaddr_in *>(this)->sin_port = Utils::hton((uint16_t)port);
 	} else if (ipLen == 16) {
-		ss_family = AF_INET6;
+		_data.ss_family = AF_INET6;
 		memcpy(reinterpret_cast<struct sockaddr_in6 *>(this)->sin6_addr.s6_addr,ipBytes,16);
 		reinterpret_cast<struct sockaddr_in6 *>(this)->sin6_port = Utils::hton((uint16_t)port);
 	}
@@ -112,7 +112,7 @@ void InetAddress::set(const void *ipBytes,unsigned int ipLen,unsigned int port)
 
 bool InetAddress::isDefaultRoute() const noexcept
 {
-	switch(ss_family) {
+	switch(_data.ss_family) {
 		case AF_INET:
 			return ( (reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr == 0) && (reinterpret_cast<const struct sockaddr_in *>(this)->sin_port == 0) );
 		case AF_INET6:
@@ -140,7 +140,7 @@ char *InetAddress::toString(char buf[ZT_INETADDRESS_STRING_SIZE_MAX]) const noex
 char *InetAddress::toIpString(char buf[ZT_INETADDRESS_STRING_SIZE_MAX]) const noexcept
 {
 	buf[0] = (char)0;
-	switch(ss_family) {
+	switch(_data.ss_family) {
 		case AF_INET: {
 #ifdef _WIN32
 			inet_ntop(AF_INET, (void*)&reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr, buf, INET_ADDRSTRLEN);
@@ -200,7 +200,7 @@ bool InetAddress::fromString(const char *ipSlashPort) noexcept
 InetAddress InetAddress::netmask() const noexcept
 {
 	InetAddress r(*this);
-	switch(r.ss_family) {
+	switch(r._data.ss_family) {
 		case AF_INET:
 			reinterpret_cast<struct sockaddr_in *>(&r)->sin_addr.s_addr = Utils::hton((uint32_t)(0xffffffffU << (32 - netmaskBits())));
 			break;
@@ -222,7 +222,7 @@ InetAddress InetAddress::netmask() const noexcept
 
 InetAddress InetAddress::broadcast() const noexcept
 {
-	if (ss_family == AF_INET) {
+	if (_data.ss_family == AF_INET) {
 		InetAddress r(*this);
 		reinterpret_cast<struct sockaddr_in *>(&r)->sin_addr.s_addr |= Utils::hton((uint32_t)(0xffffffffU >> netmaskBits()));
 		return r;
@@ -233,7 +233,7 @@ InetAddress InetAddress::broadcast() const noexcept
 InetAddress InetAddress::network() const noexcept
 {
 	InetAddress r(*this);
-	switch(r.ss_family) {
+	switch(r._data.ss_family) {
 		case AF_INET:
 			reinterpret_cast<struct sockaddr_in *>(&r)->sin_addr.s_addr &= Utils::hton((uint32_t)(0xffffffffU << (32 - netmaskBits())));
 			break;
@@ -251,8 +251,8 @@ InetAddress InetAddress::network() const noexcept
 
 bool InetAddress::isEqualPrefix(const InetAddress &addr) const noexcept
 {
-	if (addr.ss_family == ss_family) {
-		switch(ss_family) {
+	if (addr._data.ss_family == _data.ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET6: {
 				const InetAddress mask(netmask());
 				InetAddress addr_mask(addr.netmask());
@@ -273,8 +273,8 @@ bool InetAddress::isEqualPrefix(const InetAddress &addr) const noexcept
 
 bool InetAddress::containsAddress(const InetAddress &addr) const noexcept
 {
-	if (addr.ss_family == ss_family) {
-		switch(ss_family) {
+	if (addr._data.ss_family == _data.ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET: {
 				const unsigned int bits = netmaskBits();
 				if (bits == 0)
@@ -299,9 +299,9 @@ bool InetAddress::containsAddress(const InetAddress &addr) const noexcept
 
 unsigned long InetAddress::hashCode() const noexcept
 {
-	if (ss_family == AF_INET) {
+	if (_data.ss_family == AF_INET) {
 		return ((unsigned long)reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr + (unsigned long)reinterpret_cast<const struct sockaddr_in *>(this)->sin_port);
-	} else if (ss_family == AF_INET6) {
+	} else if (_data.ss_family == AF_INET6) {
 		unsigned long tmp = reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_port;
 		const uint8_t *a = reinterpret_cast<const uint8_t *>(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr);
 		for(long i=0;i<16;++i)
@@ -319,7 +319,7 @@ unsigned long InetAddress::hashCode() const noexcept
 void InetAddress::forTrace(ZT_TraceEventPathAddress &ta) const noexcept
 {
 	uint32_t tmp;
-	switch(ss_family) {
+	switch(_data.ss_family) {
 		default:
 			memset(&ta,0,sizeof(ZT_TraceEventPathAddress));
 			break;
@@ -344,7 +344,7 @@ void InetAddress::forTrace(ZT_TraceEventPathAddress &ta) const noexcept
 
 bool InetAddress::isNetwork() const noexcept
 {
-	switch(ss_family) {
+	switch(_data.ss_family) {
 		case AF_INET: {
 			unsigned int bits = netmaskBits();
 			if (bits <= 0)
@@ -377,7 +377,7 @@ bool InetAddress::isNetwork() const noexcept
 int InetAddress::marshal(uint8_t data[ZT_INETADDRESS_MARSHAL_SIZE_MAX]) const noexcept
 {
 	unsigned int port;
-	switch(ss_family) {
+	switch(_data.ss_family) {
 		case AF_INET:
 			port = Utils::ntoh((uint16_t)reinterpret_cast<const sockaddr_in *>(this)->sin_port);
 			data[0] = 4;
@@ -438,8 +438,8 @@ int InetAddress::unmarshal(const uint8_t *restrict data,const int len) noexcept
 
 bool InetAddress::operator==(const InetAddress &a) const noexcept
 {
-	if (ss_family == a.ss_family) {
-		switch(ss_family) {
+	if (_data.ss_family == a._data.ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET:
 				return (
 					(reinterpret_cast<const struct sockaddr_in *>(this)->sin_port == reinterpret_cast<const struct sockaddr_in *>(&a)->sin_port)&&
@@ -461,10 +461,10 @@ bool InetAddress::operator==(const InetAddress &a) const noexcept
 
 bool InetAddress::operator<(const InetAddress &a) const noexcept
 {
-	if (ss_family < a.ss_family)
+	if (_data.ss_family < a._data.ss_family)
 		return true;
-	else if (ss_family == a.ss_family) {
-		switch(ss_family) {
+	else if (_data.ss_family == a._data.ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET:
 				if (reinterpret_cast<const struct sockaddr_in *>(this)->sin_port < reinterpret_cast<const struct sockaddr_in *>(&a)->sin_port)
 					return true;

+ 28 - 18
node/InetAddress.hpp

@@ -29,14 +29,14 @@ namespace ZeroTier {
 #define ZT_INETADDRESS_STRING_SIZE_MAX 64
 
 /**
- * Extends sockaddr_storage with friendly C++ methods
+ * C++ class that overlaps in size with sockaddr_storage and adds convenience methods
  *
  * This is basically a "mixin" for sockaddr_storage. It adds methods and
  * operators, but does not modify the structure. This can be cast to/from
  * sockaddr_storage and used interchangeably. DO NOT change this by e.g.
  * adding non-static fields, since much code depends on this identity.
  */
-struct InetAddress : public sockaddr_storage,public TriviallyCopyable
+struct InetAddress : public TriviallyCopyable
 {
 private:
 	// Internal function to copy any sockaddr_X structure to this one even if it's smaller and unpadded.
@@ -161,6 +161,11 @@ public:
 		return *this;
 	}
 
+	/**
+	 * @return Address family (ss_family in sockaddr_storage)
+	 */
+	ZT_ALWAYS_INLINE uint8_t family() const noexcept { return _data.ss_family; }
+
 	/**
 	 * @return IP scope classification (e.g. loopback, link-local, private, global)
 	 */
@@ -182,7 +187,7 @@ public:
 	 */
 	ZT_ALWAYS_INLINE void setPort(unsigned int port) noexcept
 	{
-		switch(ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET:
 				reinterpret_cast<struct sockaddr_in *>(this)->sin_port = Utils::hton((uint16_t)port);
 				break;
@@ -218,7 +223,7 @@ public:
 	 */
 	ZT_ALWAYS_INLINE unsigned int port() const noexcept
 	{
-		switch(ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET:  return Utils::ntoh((uint16_t)(reinterpret_cast<const struct sockaddr_in *>(this)->sin_port));
 			case AF_INET6: return Utils::ntoh((uint16_t)(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_port));
 			default:       return 0;
@@ -242,7 +247,7 @@ public:
 	ZT_ALWAYS_INLINE bool netmaskBitsValid() const noexcept
 	{
 		const unsigned int n = port();
-		switch(ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET: return (n <= 32);
 			case AF_INET6: return (n <= 128);
 		}
@@ -302,19 +307,19 @@ public:
 	/**
 	 * @return True if this is an IPv4 address
 	 */
-	ZT_ALWAYS_INLINE bool isV4() const noexcept { return (ss_family == AF_INET); }
+	ZT_ALWAYS_INLINE bool isV4() const noexcept { return (family() == AF_INET); }
 
 	/**
 	 * @return True if this is an IPv6 address
 	 */
-	ZT_ALWAYS_INLINE bool isV6() const noexcept { return (ss_family == AF_INET6); }
+	ZT_ALWAYS_INLINE bool isV6() const noexcept { return (family() == AF_INET6); }
 
 	/**
 	 * @return pointer to raw address bytes or NULL if not available
 	 */
 	ZT_ALWAYS_INLINE const void *rawIpData() const noexcept
 	{
-		switch(ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET: return (const void *)&(reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr);
 			case AF_INET6: return (const void *)(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr);
 			default: return nullptr;
@@ -327,13 +332,13 @@ public:
 	ZT_ALWAYS_INLINE InetAddress ipOnly() const noexcept
 	{
 		InetAddress r;
-		switch(ss_family) {
+		switch(_data.ss_family) {
 			case AF_INET:
-				r.ss_family = AF_INET;
+				reinterpret_cast<struct sockaddr_in *>(&r)->sin_family = AF_INET;
 				reinterpret_cast<struct sockaddr_in *>(&r)->sin_addr.s_addr = reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr;
 				break;
 			case AF_INET6:
-				r.ss_family = AF_INET6;
+				reinterpret_cast<struct sockaddr_in6 *>(&r)->sin6_family = AF_INET;
 				memcpy(reinterpret_cast<struct sockaddr_in6 *>(&r)->sin6_addr.s6_addr,reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr,16);
 				break;
 		}
@@ -348,10 +353,11 @@ public:
 	 */
 	ZT_ALWAYS_INLINE bool ipsEqual(const InetAddress &a) const noexcept
 	{
-		if (ss_family == a.ss_family) {
-			if (ss_family == AF_INET)
+		const uint8_t f = _data.ss_family;
+		if (f == a._data.ss_family) {
+			if (f == AF_INET)
 				return (reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr == reinterpret_cast<const struct sockaddr_in *>(&a)->sin_addr.s_addr);
-			if (ss_family == AF_INET6)
+			if (f == AF_INET6)
 				return (memcmp(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr,reinterpret_cast<const struct sockaddr_in6 *>(&a)->sin6_addr.s6_addr,16) == 0);
 			return (memcmp(this,&a,sizeof(InetAddress)) == 0);
 		}
@@ -368,10 +374,11 @@ public:
 	 */
 	ZT_ALWAYS_INLINE bool ipsEqual2(const InetAddress &a) const noexcept
 	{
-		if (ss_family == a.ss_family) {
-			if (ss_family == AF_INET)
+		const uint8_t f = _data.ss_family;
+		if (f == a._data.ss_family) {
+			if (f == AF_INET)
 				return (reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr == reinterpret_cast<const struct sockaddr_in *>(&a)->sin_addr.s_addr);
-			if (ss_family == AF_INET6)
+			if (f == AF_INET6)
 				return (memcmp(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr,reinterpret_cast<const struct sockaddr_in6 *>(&a)->sin6_addr.s6_addr,8) == 0);
 			return (memcmp(this,&a,sizeof(InetAddress)) == 0);
 		}
@@ -400,7 +407,7 @@ public:
 	/**
 	 * @return True if address family is non-zero
 	 */
-	explicit ZT_ALWAYS_INLINE operator bool() const noexcept { return (ss_family != 0); }
+	explicit ZT_ALWAYS_INLINE operator bool() const noexcept { return (family() != 0); }
 
 	static constexpr int marshalSizeMax() noexcept { return ZT_INETADDRESS_MARSHAL_SIZE_MAX; }
 	int marshal(uint8_t data[ZT_INETADDRESS_MARSHAL_SIZE_MAX]) const noexcept;
@@ -468,6 +475,9 @@ public:
 	 * Compute a private IPv6 "6plane" unicast address from network ID and ZeroTier address
 	 */
 	static InetAddress makeIpv66plane(uint64_t nwid,uint64_t zeroTierAddress) noexcept;
+
+private:
+	sockaddr_storage _data;
 };
 
 static ZT_ALWAYS_INLINE InetAddress *asInetAddress(sockaddr_in *p) noexcept { return reinterpret_cast<InetAddress *>(p); }

+ 11 - 14
node/Peer.cpp

@@ -20,6 +20,7 @@
 #include "SelfAwareness.hpp"
 #include "InetAddress.hpp"
 #include "Protocol.hpp"
+#include "Endpoint.hpp"
 
 #include <set>
 
@@ -93,7 +94,7 @@ void Peer::received(
 			int64_t lastReceiveTimeMax = 0;
 			int lastReceiveTimeMaxAt = 0;
 			for(int i=0;i<ZT_MAX_PEER_NETWORK_PATHS;++i) {
-				if ((_paths[i]->address().ss_family == path->address().ss_family) &&
+				if ((_paths[i]->address().family() == path->address().family()) &&
 				    (_paths[i]->localSocket() == path->localSocket()) && // TODO: should be localInterface when multipath is integrated
 				    (_paths[i]->address().ipsEqual2(path->address()))) {
 					// Replace older path if everything is the same except the port number.
@@ -117,14 +118,13 @@ void Peer::received(
 			if (_paths[lastReceiveTimeMaxAt])
 				old = _paths[lastReceiveTimeMaxAt]->address();
 			_paths[lastReceiveTimeMaxAt] = path;
-			_bootstrap = path->address();
+			_bootstrap = Endpoint(path->address());
 			_prioritizePaths(now);
 			RR->t->learnedNewPath(tPtr,0x582fabdd,packetId,_id,path->address(),old);
 		} else {
 			if (RR->node->shouldUsePathForZeroTierTraffic(tPtr,_id,path->localSocket(),path->address())) {
 				RR->t->tryingNewPath(tPtr,0xb7747ddd,_id,path->address(),path->address(),packetId,(uint8_t)verb,_id.address(),_id.hash().data(),ZT_TRACE_TRYING_NEW_PATH_REASON_PACKET_RECEIVED_FROM_UNKNOWN_PATH);
-				sendHELLO(tPtr,path->localSocket(),path->address(),now);
-				path->sent(now);
+				path->sent(now,sendHELLO(tPtr,path->localSocket(),path->address(),now));
 			}
 		}
 	}
@@ -134,7 +134,7 @@ path_check_done:
 		_lastAttemptedP2PInit = now;
 
 		InetAddress addr;
-		if ((_bootstrap.type() == Endpoint::INETADDR_V4)||(_bootstrap.type() == Endpoint::INETADDR_V6)) {
+		if ((_bootstrap.type() == Endpoint::TYPE_INETADDR_V4)||(_bootstrap.type() == Endpoint::TYPE_INETADDR_V6)) {
 			RR->t->tryingNewPath(tPtr,0x0a009444,_id,_bootstrap.inetAddr(),InetAddress::NIL,0,0,0,nullptr,ZT_TRACE_TRYING_NEW_PATH_REASON_BOOTSTRAP_ADDRESS);
 			sendHELLO(tPtr,-1,_bootstrap.inetAddr(),now);
 		} if (RR->node->externalPathLookup(tPtr,_id,-1,addr)) {
@@ -207,7 +207,7 @@ path_check_done:
 	}
 }
 
-void Peer::sendHELLO(void *tPtr,const int64_t localSocket,const InetAddress &atAddress,int64_t now)
+unsigned int Peer::sendHELLO(void *tPtr,const int64_t localSocket,const InetAddress &atAddress,int64_t now)
 {
 #if 0
 	Packet outp(_id.address(),RR->identity.address(),Packet::VERB_HELLO);
@@ -253,23 +253,21 @@ void Peer::ping(void *tPtr,int64_t now,const bool pingAllAddressTypes)
 
 	if (_alivePathCount > 0) {
 		for (unsigned int i = 0; i < _alivePathCount; ++i) {
-			sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now);
-			_paths[i]->sent(now);
+			_paths[i]->sent(now,sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now));
 			if (!pingAllAddressTypes)
 				return;
 		}
 		return;
 	}
 
-	if ((_bootstrap.type() == Endpoint::INETADDR_V4)||(_bootstrap.type() == Endpoint::INETADDR_V6))
+	if ((_bootstrap.type() == Endpoint::TYPE_INETADDR_V4)||(_bootstrap.type() == Endpoint::TYPE_INETADDR_V6))
 		sendHELLO(tPtr,-1,_bootstrap.inetAddr(),now);
 
 	SharedPtr<Peer> r(RR->topology->root());
 	if ((r)&&(r.ptr() != this)) {
 		SharedPtr<Path> rp(r->path(now));
 		if (rp) {
-			sendHELLO(tPtr,rp->localSocket(),rp->address(),now);
-			rp->sent(now);
+			rp->sent(now,sendHELLO(tPtr,rp->localSocket(),rp->address(),now));
 			return;
 		}
 	}
@@ -279,9 +277,8 @@ void Peer::resetWithinScope(void *tPtr,InetAddress::IpScope scope,int inetAddres
 {
 	RWMutex::RLock l(_lock);
 	for(unsigned int i=0; i < _alivePathCount; ++i) {
-		if ((_paths[i])&&((_paths[i]->address().ss_family == inetAddressFamily)&&(_paths[i]->address().ipScope() == scope))) {
-			sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now);
-			_paths[i]->sent(now);
+		if ((_paths[i])&&((_paths[i]->address().family() == inetAddressFamily)&&(_paths[i]->address().ipScope() == scope))) {
+			_paths[i]->sent(now,sendHELLO(tPtr,_paths[i]->localSocket(),_paths[i]->address(),now));
 		}
 	}
 }

+ 2 - 1
node/Peer.hpp

@@ -121,8 +121,9 @@ public:
 	 * @param localSocket Local source socket
 	 * @param atAddress Destination address
 	 * @param now Current time
+	 * @return Number of bytes sent
 	 */
-	void sendHELLO(void *tPtr,int64_t localSocket,const InetAddress &atAddress,int64_t now);
+	unsigned int sendHELLO(void *tPtr,int64_t localSocket,const InetAddress &atAddress,int64_t now);
 
 	/**
 	 * Send a NOP message to e.g. probe a new link

+ 1 - 0
node/Tests.cpp

@@ -205,6 +205,7 @@ extern "C" const char *ZTT_general()
 			ZT_T_ASSERT(ZT_PROTO_PACKET_ENCRYPTED_SECTION_START < ZT_PROTO_MIN_PACKET_LENGTH);
 			ZT_T_ASSERT(sizeof(Protocol::Header) == ZT_PROTO_MIN_PACKET_LENGTH);
 			ZT_T_ASSERT(sizeof(Protocol::FragmentHeader) == ZT_PROTO_MIN_FRAGMENT_LENGTH);
+			ZT_T_ASSERT(sizeof(sockaddr_storage) == sizeof(InetAddress));
 			ZT_T_PRINTF("OK" ZT_EOL_S);
 		}
 

+ 2 - 3
node/TriviallyCopyable.hpp

@@ -27,9 +27,8 @@ namespace ZeroTier {
  *
  * It also includes some static methods to do this conveniently.
  */
-class TriviallyCopyable
+ZT_PACKED_STRUCT(struct TriviallyCopyable
 {
-public:
 	/**
 	 * Be absolutely sure a TriviallyCopyable object is zeroed using Utils::burn()
 	 *
@@ -165,7 +164,7 @@ public:
 		TriviallyCopyable *const tmp = &dest;
 		memcpy(tmp,&src,sizeof(T));
 	}
-};
+});
 
 } // namespace ZeroTier