Apply patch from Scott Lamb adding an output buffer for the TCP sockets.

This helps coalescing multiple send_meta() commands into one TCP packet.
Also limit the size of the output buffer before dropping PACKETs.

doc/tinc.conf.5.in

@@ -120,13 +120,6 @@ will by default listen on all of them for incoming connections.
 It is possible to bind only to a single interface with this variable.
 .Pp
 This option may not work on all platforms.
-.It Va BlockingTCP Li = yes | no Po no Pc Bq experimental
-This options selects whether TCP connections, when established, should use blocking writes.
-When turned off, tinc will never block when a TCP connection becomes congested, but will have to terminate that connection instead.
-If turned on, tinc will not terminate connections but will block, thereby unable to process data to/from other connections.
-Turn this option on if you also use
-.Va TCPOnly
-and tinc terminates connections frequently.
 .It Va ConnectTo Li = Ar name
 Specifies which other tinc daemon to connect to on startup.
 Multiple
@@ -206,11 +199,13 @@ while no routing table is managed.
 .It Va Name Li = Ar name Bq required
 This is the name which identifies this tinc daemon.
 It must be unique for the virtual private network this daemon will connect to.
-.It Va PingTimeout Li = Ar seconds Pq 60
+.It Va PingInterval Li = Ar seconds Pq 60
 The number of seconds of inactivity that
 .Nm tinc
 will wait before sending a probe to the other end.
-If that other end doesn't answer within that same amount of time,
+.It Va PingTimeout Li = Ar seconds Pq 5
+The number of seconds to wait for a response to pings or to allow meta
+connections to block. If the other end doesn't respond within this time,
 the connection is terminated,
 and the others will be notified of this.
 .It Va PriorityInheritance Li = yes | no Po no Pc Bq experimental

doc/tinc.texi

@@ -841,15 +841,6 @@ variable.
 
 This option may not work on all platforms.
 
-@cindex BlockingTCP
-@item BlockingTCP = <yes|no> (no) [experimental]
-This options selects whether TCP connections, when established, should use blocking writes.
-When turned off, tinc will never block when a TCP connection becomes congested,
-but will have to terminate that connection instead.
-If turned on, tinc will not terminate connections but will block,
-thereby unable to process data to/from other connections.
-Turn this option on if you also use TCPOnly and tinc terminates connections frequently.
-
 @cindex ConnectTo
 @item ConnectTo = <@var{name}>
 Specifies which other tinc daemon to connect to on startup.
@@ -933,12 +924,16 @@ This only has effect when Mode is set to "switch".
 @item Name = <@var{name}> [required]
 This is a symbolic name for this connection.  It can be anything
 
-@cindex PingTimeout
+@cindex PingInterval
-@item PingTimeout = <@var{seconds}> (60)
+@item PingInterval = <@var{seconds}> (60)
 The number of seconds of inactivity that tinc will wait before sending a
-probe to the other end.  If that other end doesn't answer within that
+probe to the other end.
-same amount of seconds, the connection is terminated, and the others
+
-will be notified of this.
+@cindex PingTimeout
+@item PingTimeout = <@var{seconds}> (5)
+The number of seconds to wait for a response to pings or to allow meta
+connections to block. If the other end doesn't respond within this time,
+the connection is terminated, and the others will be notified of this.
 
 @cindex PriorityInheritance
 @item PriorityInheritance = <yes|no> (no) [experimental]

src/conf.c

@@ -33,7 +33,8 @@
 
 avl_tree_t *config_tree;
 
-int pingtimeout = 0;			/* seconds before timeout */
+int pinginterval = 0;			/* seconds between pings */
+int pingtimeout = 0;			/* seconds to wait for response */
 char *confbase = NULL;			/* directory in which all config files are */
 char *netname = NULL;			/* name of the vpn network */
 

src/conf.h

@@ -36,6 +36,7 @@ typedef struct config_t {
 
 extern avl_tree_t *config_tree;
 
+extern int pinginterval;
 extern int pingtimeout;
 extern int maxtimeout;
 extern bool bypass_security;

src/connection.c

@@ -121,8 +121,9 @@ void dump_connections(void)
 
 	for(node = connection_tree->head; node; node = node->next) {
 		c = node->data;
-		logger(LOG_DEBUG, _(" %s at %s options %lx socket %d status %04x"),
+		logger(LOG_DEBUG, _(" %s at %s options %lx socket %d status %04x outbuf %d/%d/%d"),
-			   c->name, c->hostname, c->options, c->socket, *(uint32_t *)&c->status);
+			   c->name, c->hostname, c->options, c->socket, *(uint32_t *)&c->status,
+			   c->outbufsize, c->outbufstart, c->outbuflen);
 	}
 
 	logger(LOG_DEBUG, _("End of connections."));

src/connection.h

@@ -91,7 +91,13 @@ typedef struct connection_t {
 	int tcplen;					/* length of incoming TCPpacket */
 	int allow_request;			/* defined if there's only one request possible */
 
-	time_t last_ping_time;		/* last time we saw some activity from the other end */
+	char *outbuf;				/* metadata output buffer */
+	int outbufstart;			/* index of first meaningful byte in output buffer */
+	int outbuflen;				/* number of meaningful bytes in output buffer */
+	int outbufsize;				/* number of bytes allocated to output buffer */
+
+	time_t last_ping_time;		/* last time we saw some activity from the other end or pinged them */
+	time_t last_flushed_time;	/* last time buffer was empty. Only meaningful if outbuflen > 0 */
 
 	avl_tree_t *config_tree;	/* Pointer to configuration tree belonging to him */
 } connection_t;

src/meta.c

@@ -35,9 +35,7 @@
 
 bool send_meta(connection_t *c, const char *buffer, int length)
 {
-	const char *bufp;
 	int outlen;
-	char outbuf[MAXBUFSIZE];
 	int result;
 
 	cp();
@@ -45,35 +43,73 @@ bool send_meta(connection_t *c, const char *buffer, int length)
 	ifdebug(META) logger(LOG_DEBUG, _("Sending %d bytes of metadata to %s (%s)"), length,
 			   c->name, c->hostname);
 
+	if(!c->outbuflen)
+		c->last_flushed_time = now;
+
+	/* Find room in connection's buffer */
+	if(length + c->outbuflen > c->outbufsize) {
+		c->outbufsize = length + c->outbuflen;
+		c->outbuf = xrealloc(c->outbuf, c->outbufsize);
+	}
+
+	if(length + c->outbuflen + c->outbufstart > c->outbufsize) {
+		memmove(c->outbuf, c->outbuf + c->outbufstart, c->outbuflen);
+		c->outbufstart = 0;
+	}
+
+	/* Add our data to buffer */
 	if(c->status.encryptout) {
-		result = EVP_EncryptUpdate(c->outctx, outbuf, &outlen, buffer, length);
+		result = EVP_EncryptUpdate(c->outctx, c->outbuf + c->outbufstart + c->outbuflen,
-		if(!result || outlen != length) {
+				&outlen, buffer, length);
+		if(!result || outlen < length) {
 			logger(LOG_ERR, _("Error while encrypting metadata to %s (%s): %s"),
 					c->name, c->hostname, ERR_error_string(ERR_get_error(), NULL));
 			return false;
+		} else if(outlen > length) {
+			logger(LOG_EMERG, _("Encrypted data too long! Heap corrupted!"));
+			abort();
 		}
-		bufp = outbuf;
+		c->outbuflen += outlen;
-		length = outlen;
+	} else {
-	} else
+		memcpy(c->outbuf + c->outbufstart + c->outbuflen, buffer, length);
-		bufp = buffer;
+		c->outbuflen += length;
+	}
 
-	while(length) {
+	return true;
-		result = send(c->socket, bufp, length, 0);
+}
+
+bool flush_meta(connection_t *c)
+{
+	int result;
+	
+	ifdebug(META) logger(LOG_DEBUG, _("Flushing %d bytes to %s (%s)"),
+			 c->outbuflen, c->name, c->hostname);
+
+	while(c->outbuflen) {
+		result = send(c->socket, c->outbuf + c->outbufstart, c->outbuflen, 0);
 		if(result <= 0) {
 			if(!errno || errno == EPIPE) {
 				ifdebug(CONNECTIONS) logger(LOG_NOTICE, _("Connection closed by %s (%s)"),
 						   c->name, c->hostname);
-			} else if(errno == EINTR)
+			} else if(errno == EINTR) {
 				continue;
-			else
+			} else if(errno == EWOULDBLOCK) {
-				logger(LOG_ERR, _("Sending meta data to %s (%s) failed: %s"), c->name,
+				ifdebug(CONNECTIONS) logger(LOG_DEBUG, _("Flushing %d bytes to %s (%s) would block"),
+						c->outbuflen, c->name, c->hostname);
+				return true;
+			} else {
+				logger(LOG_ERR, _("Flushing meta data to %s (%s) failed: %s"), c->name,
 					   c->hostname, strerror(errno));
+			}
+
 			return false;
 		}
-		bufp += result;
+
-		length -= result;
+		c->outbufstart += result;
+		c->outbuflen -= result;
 	}
-	
+
+	c->outbufstart = 0; /* avoid unnecessary memmoves */
 	return true;
 }
 

src/net.c

@@ -112,7 +112,7 @@ static void purge(void)
   put all file descriptors in an fd_set array
   While we're at it, purge stuff that needs to be removed.
 */
-static int build_fdset(fd_set * fs)
+static int build_fdset(fd_set *readset, fd_set *writeset)
 {
 	avl_node_t *node, *next;
 	connection_t *c;
@@ -120,7 +120,8 @@ static int build_fdset(fd_set * fs)
 
 	cp();
 
-	FD_ZERO(fs);
+	FD_ZERO(readset);
+	FD_ZERO(writeset);
 
 	for(node = connection_tree->head; node; node = next) {
 		next = node->next;
@@ -131,22 +132,24 @@ static int build_fdset(fd_set * fs)
 			if(!connection_tree->head)
 				purge();
 		} else {
-			FD_SET(c->socket, fs);
+			FD_SET(c->socket, readset);
+			if(c->outbuflen > 0)
+				FD_SET(c->socket, writeset);
 			if(c->socket > max)
 				max = c->socket;
 		}
 	}
 
 	for(i = 0; i < listen_sockets; i++) {
-		FD_SET(listen_socket[i].tcp, fs);
+		FD_SET(listen_socket[i].tcp, readset);
 		if(listen_socket[i].tcp > max)
 			max = listen_socket[i].tcp;
-		FD_SET(listen_socket[i].udp, fs);
+		FD_SET(listen_socket[i].udp, readset);
 		if(listen_socket[i].udp > max)
 			max = listen_socket[i].udp;
 	}
 
-	FD_SET(device_fd, fs);
+	FD_SET(device_fd, readset);
 	if(device_fd > max)
 		max = device_fd;
 	
@@ -208,6 +211,12 @@ void terminate_connection(connection_t *c, bool report)
 		retry_outgoing(c->outgoing);
 		c->outgoing = NULL;
 	}
+
+	free(c->outbuf);
+	c->outbuf = NULL;
+	c->outbuflen = 0;
+	c->outbufsize = 0;
+	c->outbufstart = 0;
 }
 
 /*
@@ -232,11 +241,11 @@ static void check_dead_connections(void)
 		if(c->last_ping_time + pingtimeout < now) {
 			if(c->status.active) {
 				if(c->status.pinged) {
-					ifdebug(CONNECTIONS) logger(LOG_INFO, _("%s (%s) didn't respond to PING"),
+					ifdebug(CONNECTIONS) logger(LOG_INFO, _("%s (%s) didn't respond to PING in %d seconds"),
-							   c->name, c->hostname);
+							   c->name, c->hostname, now - c->last_ping_time);
 					c->status.timeout = true;
 					terminate_connection(c, true);
-				} else {
+				} else if(c->last_ping_time + pinginterval < now) {
 					send_ping(c);
 				}
 			} else {
@@ -257,6 +266,16 @@ static void check_dead_connections(void)
 				}
 			}
 		}
+
+		if(c->outbuflen > 0 && c->last_flushed_time + pingtimeout < now) {
+			if(c->status.active) {
+				ifdebug(CONNECTIONS) logger(LOG_INFO,
+						_("%s (%s) could not flush for %d seconds (%d bytes remaining)"),
+						c->name, c->hostname, now - c->last_flushed_time, c->outbuflen);
+				c->status.timeout = true;
+				terminate_connection(c, true);
+			}
+		}
 	}
 }
 
@@ -264,7 +283,7 @@ static void check_dead_connections(void)
   check all connections to see if anything
   happened on their sockets
 */
-static void check_network_activity(fd_set * f)
+static void check_network_activity(fd_set * readset, fd_set * writeset)
 {
 	connection_t *c;
 	avl_node_t *node;
@@ -274,18 +293,20 @@ static void check_network_activity(fd_set * f)
 
 	cp();
 
-	if(FD_ISSET(device_fd, f)) {
+	/* check input from kernel */
+	if(FD_ISSET(device_fd, readset)) {
 		if(read_packet(&packet))
 			route(myself, &packet);
 	}
 
+	/* check meta connections */
 	for(node = connection_tree->head; node; node = node->next) {
 		c = node->data;
 
 		if(c->status.remove)
 			continue;
 
-		if(FD_ISSET(c->socket, f)) {
+		if(FD_ISSET(c->socket, readset)) {
 			if(c->status.connecting) {
 				c->status.connecting = false;
 				getsockopt(c->socket, SOL_SOCKET, SO_ERROR, &result, &len);
@@ -307,13 +328,20 @@ static void check_network_activity(fd_set * f)
 				continue;
 			}
 		}
+
+		if(FD_ISSET(c->socket, writeset)) {
+			if(!flush_meta(c)) {
+				terminate_connection(c, c->status.active);
+				continue;
+			}
+		}
 	}
 
 	for(i = 0; i < listen_sockets; i++) {
-		if(FD_ISSET(listen_socket[i].udp, f))
+		if(FD_ISSET(listen_socket[i].udp, readset))
 			handle_incoming_vpn_data(listen_socket[i].udp);
 
-		if(FD_ISSET(listen_socket[i].tcp, f))
+		if(FD_ISSET(listen_socket[i].tcp, readset))
 			handle_new_meta_connection(listen_socket[i].tcp);
 	}
 }
@@ -323,7 +351,7 @@ static void check_network_activity(fd_set * f)
 */
 int main_loop(void)
 {
-	fd_set fset;
+	fd_set readset, writeset;
 	struct timeval tv;
 	int r, maxfd;
 	time_t last_ping_check, last_config_check;
@@ -344,9 +372,9 @@ int main_loop(void)
 		tv.tv_sec = 1;
 		tv.tv_usec = 0;
 
-		maxfd = build_fdset(&fset);
+		maxfd = build_fdset(&readset, &writeset);
 
-		r = select(maxfd + 1, &fset, NULL, NULL, &tv);
+		r = select(maxfd + 1, &readset, &writeset, NULL, &tv);
 
 		if(r < 0) {
 			if(errno != EINTR && errno != EAGAIN) {
@@ -360,7 +388,7 @@ int main_loop(void)
 			continue;
 		}
 
-		check_network_activity(&fset);
+		check_network_activity(&readset, &writeset);
 
 		if(do_purge) {
 			purge();

src/net.h

@@ -114,10 +114,9 @@ typedef struct outgoing_t {
 	struct addrinfo *aip;
 } outgoing_t;
 
-extern int maxtimeout;
+extern int maxoutbufsize;
 extern int seconds_till_retry;
 extern int addressfamily;
-extern bool blockingtcp;
 
 extern listen_socket_t listen_socket[MAXSOCKETS];
 extern int listen_sockets;

src/net_setup.c

@@ -286,8 +286,6 @@ bool setup_myself(void)
 	if(get_config_bool(lookup_config(myself->connection->config_tree, "TCPOnly"), &choice) && choice)
 		myself->options |= OPTION_TCPONLY;
 
-	get_config_bool(lookup_config(config_tree, "BlockingTCP"), &blockingtcp);
-
 	if(get_config_bool(lookup_config(myself->connection->config_tree, "PMTUDiscovery"), &choice) && choice)
 		myself->options |= OPTION_PMTU_DISCOVERY;
 
@@ -536,12 +534,20 @@ bool setup_network_connections(void)
 	init_events();
 	init_requests();
 
-	if(get_config_int(lookup_config(config_tree, "PingTimeout"), &pingtimeout)) {
+	if(get_config_int(lookup_config(config_tree, "PingInterval"), &pinginterval)) {
-		if(pingtimeout < 1) {
+		if(pinginterval < 1) {
-			pingtimeout = 86400;
+			pinginterval = 86400;
 		}
 	} else
-		pingtimeout = 60;
+		pinginterval = 60;
+
+	if(!get_config_int(lookup_config(config_tree, "PingTimeout"), &pingtimeout))
+		pingtimeout = 5;
+	if(pingtimeout < 1 || pingtimeout > pinginterval)
+		pingtimeout = pinginterval;
+
+	if(!get_config_int(lookup_config(config_tree, "MaxOutputBufferSize"), &maxoutbufsize))
+		maxoutbufsize = 4 * MTU;
 
 	if(!setup_myself())
 		return false;

src/net_socket.c

@@ -46,7 +46,6 @@
 int addressfamily = AF_UNSPEC;
 int maxtimeout = 900;
 int seconds_till_retry = 5;
-bool blockingtcp = false;
 
 listen_socket_t listen_socket[MAXSOCKETS];
 int listen_sockets;
@@ -58,12 +57,10 @@ static void configure_tcp(connection_t *c)
 	int option;
 
 #ifdef O_NONBLOCK
-	if(!blockingtcp) {
+	int flags = fcntl(c->socket, F_GETFL);
-		int flags = fcntl(c->socket, F_GETFL);
 
-		if(fcntl(c->socket, F_SETFL, flags | O_NONBLOCK) < 0) {
+	if(fcntl(c->socket, F_SETFL, flags | O_NONBLOCK) < 0) {
-			logger(LOG_ERR, _("fcntl for %s: %s"), c->hostname, strerror(errno));
+		logger(LOG_ERR, _("fcntl for %s: %s"), c->hostname, strerror(errno));
-		}
 	}
 #endif
 

src/protocol.c

@@ -241,7 +241,7 @@ void age_past_requests(void)
 		next = node->next;
 		p = node->data;
 
-		if(p->firstseen + pingtimeout < now)
+		if(p->firstseen + pinginterval < now)
 			avl_delete_node(past_request_tree, node), deleted++;
 		else
 			left++;

src/protocol_misc.c

@@ -31,6 +31,8 @@
 #include "protocol.h"
 #include "utils.h"
 
+int maxoutbufsize = 0;
+
 /* Status and error notification routines */
 
 bool send_status(connection_t *c, int statusno, const char *statusstring)
@@ -153,7 +155,10 @@ bool send_tcppacket(connection_t *c, vpn_packet_t *packet)
 {
 	cp();
 
-	/* Evil hack. */
+	/* If there already is a lot of data in the outbuf buffer, discard this packet. */
+
+	if(c->outbuflen > maxoutbufsize)
+		return true;
 
 	if(!send_request(c, "%d %hd", PACKET, packet->len))
 		return false;