Refactor and tie-up of capabilities and tags and packet evaluation points. Some optimization is possible here but it is minor and we will make it work first.

node/Membership.cpp

@@ -36,10 +36,10 @@ bool Membership::sendCredentialsIfNeeded(const RuntimeEnvironment *RR,const uint
 		unsigned int appendedCaps = 0;
 		if (cap) {
 			capsAndTags.addSize(2);
-			CState *const cs = _caps.get(cap->id());
+			std::map<uint32_t,CState>::iterator cs(_caps.find(cap->id()));
-			if ((now - cs->lastPushed) >= ZT_CREDENTIAL_PUSH_EVERY) {
+			if ((cs != _caps.end())&&((now - cs->second.lastPushed) >= ZT_CREDENTIAL_PUSH_EVERY)) {
 				cap->serialize(capsAndTags);
-				cs->lastPushed = now;
+				cs->second.lastPushed = now;
 				++appendedCaps;
 			}
 			capsAndTags.setAt<uint16_t>(0,(uint16_t)appendedCaps);
@@ -95,13 +95,13 @@ int Membership::addCredential(const RuntimeEnvironment *RR,const uint64_t now,co
 {
 	if (tag.issuedTo() != RR->identity.address())
 		return -1;
-	TState *t = _tags.get(tag.networkId());
+	TState *t = _tags.get(tag.id());
 	if ((t)&&(t->lastReceived != 0)&&(t->tag == tag))
 		return 0;
 	const int vr = tag.verify(RR);
 	if (vr == 0) {
 		if (!t)
-			t = &(_tags[tag.networkId()]);
+			t = &(_tags[tag.id()]);
 		t->lastReceived = now;
 		t->tag = tag;
 	}
@@ -112,15 +112,19 @@ int Membership::addCredential(const RuntimeEnvironment *RR,const uint64_t now,co
 {
 	if (!cap.wasIssuedTo(RR->identity.address()))
 		return -1;
-	CState *c = _caps.get(cap.networkId());
+	std::map<uint32_t,CState>::iterator c(_caps.find(cap.id()));
-	if ((c)&&(c->lastReceived != 0)&&(c->cap == cap))
+	if ((c != _caps.end())&&(c->second.lastReceived != 0)&&(c->second.cap == cap))
 		return 0;
 	const int vr = cap.verify(RR);
 	if (vr == 0) {
-		if (!c)
+		if (c == _caps.end()) {
-			c = &(_caps[cap.networkId()]);
+			CState &c2 = _caps[cap.id()];
-		c->lastReceived = now;
+			c2.lastReceived = now;
-		c->cap = cap;
+			c2.cap = cap;
+		} else {
+			c->second.lastReceived = now;
+			c->second.cap = cap;
+		}
 	}
 	return vr;
 }

node/Membership.hpp

@@ -21,8 +21,7 @@
 
 #include <stdint.h>
 
-#include <utility>
+#include <map>
-#include <algorithm>
 
 #include "Constants.hpp"
 #include "../include/ZeroTierOne.h"
@@ -43,40 +42,67 @@ namespace ZeroTier {
 class RuntimeEnvironment;
 
 /**
- * Information related to a peer's participation on a network
+ * A container for certificates of membership and other credentials for peer participation on networks
- *
- * This structure is not thread-safe and must be locked during use.
  */
 class Membership
 {
 private:
+	// Tags and related state
 	struct TState
 	{
 		TState() : lastPushed(0),lastReceived(0) {}
-		// Last time we pushed our tag to this peer (our tag with the same ID)
+		// Last time we pushed OUR tag to this peer (with this ID)
 		uint64_t lastPushed;
-		// Last time we received this tag from this peer
+		// Last time we received THEIR tag (with this ID)
 		uint64_t lastReceived;
-		// Tag from peer (remote tag)
+		// THEIR tag
 		Tag tag;
 	};
 
+	// Credentials and related state
 	struct CState
 	{
 		CState() : lastPushed(0),lastReceived(0) {}
-		// Last time we pushed our capability to this peer (our capability with this ID)
+		// Last time we pushed OUR capability to this peer (with this ID)
 		uint64_t lastPushed;
-		// Last time we received this capability from this peer
+		// Last time we received THEIR capability (with this ID)
 		uint64_t lastReceived;
-		// Capability from peer
+		// THEIR capability
 		Capability cap;
 	};
 
 public:
+	/**
+	 * A wrapper to iterate through capabilities in ascending order of capability ID
+	 */
+	class CapabilityIterator
+	{
+	public:
+		CapabilityIterator(const Membership &m) :
+			_i(m._caps.begin()),
+			_e(m._caps.end())
+		{
+		}
+
+		inline const Capability *next()
+		{
+			while (_i != _e) {
+				if (_i->second.lastReceived)
+					return &((_i++)->second.cap);
+				else ++_i;
+			}
+			return (const Capability *)0;
+		}
+
+	private:
+		std::map<uint32_t,CState>::const_iterator _i,_e;
+	};
+	friend class CapabilityIterator;
+
 	Membership() :
 		_lastPushedCom(0),
 		_com(),
-		_caps(8),
+		_caps(),
 		_tags(8)
 	{
 	}
@@ -90,7 +116,7 @@ public:
 	 * @param RR Runtime environment
 	 * @param now Current time
 	 * @param peerAddress Address of member peer
-	 * @param com Network certificate of membership (if any)
+	 * @param com My network certificate of membership (if any) (not the one here, but ours -- in NetworkConfig)
 	 * @param cap Capability to send or 0 if none
 	 * @param tags Tags that this peer might need
 	 * @param tagCount Number of tag IDs
@@ -145,8 +171,8 @@ public:
 	 */
 	inline const Capability *getCapability(const NetworkConfig &nconf,const uint32_t id) const
 	{
-		const CState *c = _caps.get(id);
+		std::map<uint32_t,CState>::const_iterator c(_caps.find(id));
-		return ((c) ? (((c->lastReceived != 0)&&(c->cap.expiration() < nconf.timestamp)) ? &(c->cap) : (const Capability *)0) : (const Capability *)0);
+		return ((c != _caps.end()) ? (((c->second.lastReceived != 0)&&(c->second.cap.expiration() < nconf.timestamp)) ? &(c->second.cap) : (const Capability *)0) : (const Capability *)0);
 	}
 
 	/**
@@ -179,18 +205,18 @@ public:
 	{
 		uint64_t lastAct = _lastPushedCom;
 
-		uint32_t *i = (uint32_t *)0;
+		for(std::map<uint32_t,CState>::iterator i(_caps.begin());i!=_caps.end();) {
-		CState *cs = (CState *)0;
+			const uint64_t la = std::max(i->second.lastPushed,i->second.lastReceived);
-		Hashtable<uint32_t,CState>::Iterator csi(_caps);
+			if ((now - la) > ZT_MEMBERSHIP_STATE_EXPIRATION_TIME) {
-		while (csi.next(i,cs)) {
+				_caps.erase(i++);
-			const uint64_t la = std::max(cs->lastPushed,cs->lastReceived);
+			} else {
-			if ((now - la) > ZT_MEMBERSHIP_STATE_EXPIRATION_TIME)
+				++i;
-				_caps.erase(*i);
+				if (la > lastAct)
-			else if (la > lastAct)
 					lastAct = la;
 			}
+		}
 
-		i = (uint32_t *)0;
+		uint32_t *i = (uint32_t *)0;
 		TState *ts = (TState *)0;
 		Hashtable<uint32_t,TState>::Iterator tsi(_tags);
 		while (tsi.next(i,ts)) {
@@ -211,8 +237,8 @@ private:
 	// COM from this peer
 	CertificateOfMembership _com;
 
-	// Capability-related state
+	// Capability-related state (we need an ordered container here, hence std::map)
-	Hashtable<uint32_t,CState> _caps;
+	std::map<uint32_t,CState> _caps;
 
 	// Tag-related state
 	Hashtable<uint32_t,TState> _tags;

node/Network.cpp

@@ -66,7 +66,8 @@ static bool _ipv6GetPayload(const uint8_t *frameData,unsigned int frameLen,unsig
 	return false; // overflow == invalid
 }
 
-static bool _doZtFilter(
+// 0 == no match, -1 == match/drop, 1 == match/accept
+static int _doZtFilter(
 	const RuntimeEnvironment *RR,
 	const uint64_t nwid,
 	const bool inbound,
@@ -99,18 +100,21 @@ static bool _doZtFilter(
 		switch(rt) {
 			// Actions -------------------------------------------------------------
 
+			// An action is performed if thisSetMatches is true, and if not
+			// (or if the action is non-terminating) we start a new set of rules.
+
 			case ZT_NETWORK_RULE_ACTION_DROP:
 				if (thisSetMatches) {
-					return false;
+					return -1; // match, drop packet
 				} else {
-					thisSetMatches = 1; // continue parsing next set of rules
+					thisSetMatches = 1; // no match, evaluate next set
 				}
 				break;
 			case ZT_NETWORK_RULE_ACTION_ACCEPT:
 				if (thisSetMatches) {
-					return true;
+					return 1; // match, accept packet
 				} else {
-					thisSetMatches = 1; // continue parsing next set of rules
+					thisSetMatches = 1; // no match, evaluate next set
 				}
 				break;
 			case ZT_NETWORK_RULE_ACTION_TEE:
@@ -126,14 +130,16 @@ static bool _doZtFilter(
 				RR->sw->send(outp,true,nwid);
 
 				if (rt == ZT_NETWORK_RULE_ACTION_REDIRECT) {
-					return false;
+					return -1; // match, drop packet (we redirected it)
 				} else {
-					thisSetMatches = 1; // TEE does not terminate parsing
+					thisSetMatches = 1; // TEE does not terminate evaluation
 				}
 			}	break;
 
 			// Rules ---------------------------------------------------------------
 
+			// thisSetMatches is the binary AND of the result of all rules in a set
+
 			case ZT_NETWORK_RULE_MATCH_SOURCE_ZEROTIER_ADDRESS:
 				thisRuleMatches = (uint8_t)(rules[rn].v.zt == ztSource.toInt());
 				break;
@@ -320,13 +326,13 @@ static bool _doZtFilter(
 			}	break;
 		}
 
-		// thisSetMatches remains true if the current rule matched... or does NOT match if not bit (0x80) is 1
+		// thisSetMatches remains true if the current rule matched (or did NOT match if NOT bit is set)
 		thisSetMatches &= (thisRuleMatches ^ ((rules[rn].t & 0x80) >> 7));
 
 		//TRACE("[%u] %u result==%u set==%u",rn,(unsigned int)rt,(unsigned int)thisRuleMatches,(unsigned int)thisSetMatches);
 	}
 
-	return false;
+	return 0;
 }
 
 const ZeroTier::MulticastGroup Network::BROADCAST(ZeroTier::MAC(0xffffffffffffULL),0);
@@ -414,56 +420,20 @@ bool Network::filterOutgoingPacket(
 	Membership &m = _memberships[ztDest];
 	const unsigned int remoteTagCount = m.getAllTags(_config,remoteTagIds,remoteTagValues,ZT_MAX_NETWORK_TAGS);
 
-	if (_doZtFilter(
+	switch(_doZtFilter(RR,_id,false,ztSource,ztDest,macSource,macDest,frameData,frameLen,etherType,vlanId,_config.rules,_config.ruleCount,_config.tags,_config.tagCount,remoteTagIds,remoteTagValues,remoteTagCount,relevantLocalTags,relevantLocalTagCount)) {
-		RR,
+		case -1:
-		_id,
+			return false;
-		false,
+		case 1:
-		ztSource,
-		ztDest,
-		macSource,
-		macDest,
-		frameData,
-		frameLen,
-		etherType,
-		vlanId,
-		_config.rules,
-		_config.ruleCount,
-		_config.tags,
-		_config.tagCount,
-		remoteTagIds,
-		remoteTagValues,
-		remoteTagCount,
-		relevantLocalTags,
-		relevantLocalTagCount
-	)) {
 			m.sendCredentialsIfNeeded(RR,RR->node->now(),ztDest,_config.com,(const Capability *)0,relevantLocalTags,relevantLocalTagCount);
 			return true;
 	}
 
 	for(unsigned int c=0;c<_config.capabilityCount;++c) {
 		relevantLocalTagCount = 0;
-		if (_doZtFilter(
+		switch (_doZtFilter(RR,_id,false,ztSource,ztDest,macSource,macDest,frameData,frameLen,etherType,vlanId,_config.capabilities[c].rules(),_config.capabilities[c].ruleCount(),_config.tags,_config.tagCount,remoteTagIds,remoteTagValues,remoteTagCount,relevantLocalTags,relevantLocalTagCount)) {
-			RR,
+			case -1:
-			_id,
+				return false;
-			false,
+			case 1:
-			ztSource,
-			ztDest,
-			macSource,
-			macDest,
-			frameData,
-			frameLen,
-			etherType,
-			vlanId,
-			_config.capabilities[c].rules(),
-			_config.capabilities[c].ruleCount(),
-			_config.tags,
-			_config.tagCount,
-			remoteTagIds,
-			remoteTagValues,
-			remoteTagCount,
-			relevantLocalTags,
-			relevantLocalTagCount
-		)) {
 				m.sendCredentialsIfNeeded(RR,RR->node->now(),ztDest,_config.com,&(_config.capabilities[c]),relevantLocalTags,relevantLocalTagCount);
 				return true;
 		}
@@ -492,32 +462,24 @@ bool Network::filterIncomingPacket(
 	Membership &m = _memberships[ztDest];
 	const unsigned int remoteTagCount = m.getAllTags(_config,remoteTagIds,remoteTagValues,ZT_MAX_NETWORK_TAGS);
 
-	if (_doZtFilter(
+	switch (_doZtFilter(RR,_id,true,sourcePeer->address(),ztDest,macSource,macDest,frameData,frameLen,etherType,vlanId,_config.rules,_config.ruleCount,_config.tags,_config.tagCount,remoteTagIds,remoteTagValues,remoteTagCount,relevantLocalTags,relevantLocalTagCount)) {
-		RR,
+		case -1:
-		_id,
+			return false;
-		true,
+		case 1:
-		sourcePeer->address(),
-		ztDest,
-		macSource,
-		macDest,
-		frameData,
-		frameLen,
-		etherType,
-		vlanId,
-		_config.rules,
-		_config.ruleCount,
-		_config.tags,
-		_config.tagCount,
-		remoteTagIds,
-		remoteTagValues,
-		remoteTagCount,
-		relevantLocalTags,
-		relevantLocalTagCount
-	)) {
 			return true;
 	}
 
-
+	Membership::CapabilityIterator mci(m);
+	const Capability *c;
+	while ((c = mci.next())) {
+		relevantLocalTagCount = 0;
+		switch(_doZtFilter(RR,_id,false,sourcePeer->address(),ztDest,macSource,macDest,frameData,frameLen,etherType,vlanId,c->rules(),c->ruleCount(),_config.tags,_config.tagCount,remoteTagIds,remoteTagValues,remoteTagCount,relevantLocalTags,relevantLocalTagCount)) {
+			case -1:
+				return false;
+			case 1:
+				return true;
+		}
+	}
 
 	return false;
 }
@@ -817,12 +779,11 @@ bool Network::_isAllowed(const SharedPtr<Peer> &peer) const
 			if (_config.isPublic()) {
 				return true;
 			} else {
-				LockingPtr<Membership> m(peer->membership(_id,false));
+				const Membership *m = _memberships.get(peer->address());
-				if (m) {
+				if (m)
 					return _config.com.agreesWith(m->com());
 			}
 		}
-		}
 	} catch ( ... ) {
 		TRACE("isAllowed() check failed for peer %s: unexpected exception: unexpected exception",peer->address().toString().c_str());
 	}
@@ -866,14 +827,15 @@ void Network::_announceMulticastGroups()
 		_announceMulticastGroupsTo(*i,allMulticastGroups);
 }
 
-void Network::_announceMulticastGroupsTo(const SharedPtr<Peer> &peer,const std::vector<MulticastGroup> &allMulticastGroups) const
+void Network::_announceMulticastGroupsTo(const SharedPtr<Peer> &peer,const std::vector<MulticastGroup> &allMulticastGroups)
 {
 	// Assumes _lock is locked
 
 	// Anyone we announce multicast groups to will need our COM to authenticate GATHER requests.
 	{
-		LockingPtr<Membership> m(peer->membership(_id,false));
+		Membership *m = _memberships.get(peer->address());
-		if (m) m->sendCredentialsIfNeeded(RR,RR->node->now(),*peer,_config);
+		if (m)
+			m->sendCredentialsIfNeeded(RR,RR->node->now(),peer->address(),_config.com,(const Capability *)0,(const Tag **)0,0);
 	}
 
 	Packet outp(peer->address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);

node/Network.hpp

@@ -362,7 +362,7 @@ private:
 	void _externalConfig(ZT_VirtualNetworkConfig *ec) const; // assumes _lock is locked
 	bool _isAllowed(const SharedPtr<Peer> &peer) const;
 	void _announceMulticastGroups();
-	void _announceMulticastGroupsTo(const SharedPtr<Peer> &peer,const std::vector<MulticastGroup> &allMulticastGroups) const;
+	void _announceMulticastGroupsTo(const SharedPtr<Peer> &peer,const std::vector<MulticastGroup> &allMulticastGroups);
 	std::vector<MulticastGroup> _allMulticastGroups() const;
 
 	const RuntimeEnvironment *RR;