Browse Source

The remove paths on send fail thing in Peer.cpp was not well thought out, and there is no point in mallocing the TCP write buffer.

Adam Ierymenko 11 years ago
parent
commit
0e1fc06a6f
3 changed files with 21 additions and 50 deletions
  1. 15 22
      node/Peer.cpp
  2. 3 22
      node/TcpSocket.cpp
  3. 3 6
      node/TcpSocket.hpp

+ 15 - 22
node/Peer.cpp

@@ -121,30 +121,23 @@ bool Peer::send(const RuntimeEnvironment *_r,const void *data,unsigned int len,u
 {
 	Mutex::Lock _l(_lock);
 
-	for(;;) {
-		std::vector<Path>::iterator p(_paths.begin());
-		if (p == _paths.end())
-			return false;
-
-		uint64_t bestPathLastReceived = p->lastReceived();
-		std::vector<Path>::iterator bestPath = p;
-		while (++p != _paths.end()) {
-			uint64_t lr = p->lastReceived();
-			if (lr > bestPathLastReceived) {
-				bestPathLastReceived = lr;
-				bestPath = p;
-			}
+	std::vector<Path>::iterator p(_paths.begin());
+	if (p == _paths.end())
+		return false;
+
+	uint64_t bestPathLastReceived = p->lastReceived();
+	std::vector<Path>::iterator bestPath = p;
+	while (++p != _paths.end()) {
+		uint64_t lr = p->lastReceived();
+		if (lr > bestPathLastReceived) {
+			bestPathLastReceived = lr;
+			bestPath = p;
 		}
+	}
 
-		if (_r->sm->send(bestPath->address(),bestPath->tcp(),bestPath->type() == Path::PATH_TYPE_TCP_OUT,data,len)) {
-			bestPath->sent(now);
-			return true;
-		} else {
-			if (bestPath->fixed())
-				return false;
-			_paths.erase(bestPath);
-			// ... try again and pick a different path
-		}
+	if (_r->sm->send(bestPath->address(),bestPath->tcp(),bestPath->type() == Path::PATH_TYPE_TCP_OUT,data,len)) {
+		bestPath->sent(now);
+		return true;
 	}
 
 	return false;

+ 3 - 22
node/TcpSocket.cpp

@@ -48,8 +48,6 @@
 #include <signal.h>
 #endif
 
-#define ZT_TCP_MAX_SENDQ_LENGTH (ZT_SOCKET_MAX_MESSAGE_LEN * 8)
-
 namespace ZeroTier {
 
 TcpSocket::~TcpSocket()
@@ -59,8 +57,6 @@ TcpSocket::~TcpSocket()
 #else
 	::close(_sock);
 #endif
-	if (_outbuf)
-		::free(_outbuf);
 	//printf("!!! TCP SOCKET DESTROYED @%.16llx to %s\r\n",(unsigned long long)this,_remote.toString().c_str());
 }
 
@@ -72,26 +68,11 @@ bool TcpSocket::send(const InetAddress &to,const void *msg,unsigned int msglen)
 		return true; // sanity check
 
 	Mutex::Lock _l(_writeLock);
+
 	bool writeInProgress = ((_outptr != 0)||(_connecting));
 
-	// Ensure that _outbuf is large enough
-	unsigned int newptr = _outptr + 5 + msglen;
-	if (newptr > _outbufsize) {
-		unsigned int newbufsize = _outbufsize;
-		while (newbufsize < newptr)
-			newbufsize += ZT_SOCKET_MAX_MESSAGE_LEN;
-		if (newbufsize > ZT_TCP_MAX_SENDQ_LENGTH)
-			return false; // cannot send, outbuf full
-		unsigned char *newbuf = (unsigned char *)::malloc(newbufsize);
-		if (!newbuf)
-			return false; // cannot send, no memory
-		_outbufsize = newbufsize;
-		if (_outbuf) {
-			memcpy(newbuf,_outbuf,_outptr);
-			::free(_outbuf);
-		}
-		_outbuf = newbuf;
-	}
+	if ((_outptr + 5 + msglen) > (unsigned int)sizeof(_outbuf))
+		return false;
 
 	_outbuf[_outptr++] = 0x17; // look like TLS data
 	_outbuf[_outptr++] = 0x03;

+ 3 - 6
node/TcpSocket.hpp

@@ -78,10 +78,8 @@ protected:
 		Socket(t,s),
 		_lastActivity(Utils::now()),
 		_sm(sm),
-		_outbuf((unsigned char *)0),
-		_outptr(0),
-		_outbufsize(0),
 		_inptr(0),
+		_outptr(0),
 		_connecting(c),
 		_remote(r)
 	{
@@ -93,12 +91,11 @@ protected:
 
 private:
 	unsigned char _inbuf[ZT_SOCKET_MAX_MESSAGE_LEN];
+	unsigned char _outbuf[ZT_SOCKET_MAX_MESSAGE_LEN * 4];
 	uint64_t _lastActivity; // updated whenever data is received, checked directly by SocketManager for stale TCP cleanup
 	SocketManager *_sm;
-	unsigned char *_outbuf;
-	unsigned int _outptr;
-	unsigned int _outbufsize;
 	unsigned int _inptr;
+	unsigned int _outptr;
 	bool _connecting; // manipulated directly by SocketManager, true if connect() is in progress
 	InetAddress _remote;
 	Mutex _writeLock;