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.

diff --git a/doc/tinc.conf.5.in b/doc/tinc.conf.5.in
index 0f3cc2fe6d722544667dd3bd37acf5c820a98404..a071269a904f90a1495e73b5016b74cb61b371fc 100644 (file)
--- a/doc/tinc.conf.5.in
+++ b/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

diff --git a/doc/tinc.texi b/doc/tinc.texi
index e02b6a24bdd822615aab1e40bc168d7f164ff341..0456b75b85e4e975708eec9f0ccd99715cfadc2a 100644 (file)
--- a/doc/tinc.texi
+++ b/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
-@item PingTimeout = <@var{seconds}> (60)
+@cindex PingInterval
+@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
-same amount of seconds, the connection is terminated, and the others
-will be notified of this.
+probe to the other end.
+
+@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]

diff --git a/src/conf.c b/src/conf.c
index fa681b81f081fa4ef25c1a1c8058c0e91a226bc9..d567cd2f0cd12c6c8b9c0ba321297ed9b24470bd 100644 (file)
--- a/src/conf.c
+++ b/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 */
 

diff --git a/src/conf.h b/src/conf.h
index 4d4047a692bd63ede9a8f4a94cefc7ced2dc86cf..f1934d062dd12dcca92f866eb67e22464ba2b5b9 100644 (file)
--- a/src/conf.h
+++ b/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;

diff --git a/src/connection.c b/src/connection.c
index 5985cbf0c5ac4317c912c07088862b6c394c76cf..bb9e336d459d2f37b36687b778ff229459e2c9be 100644 (file)
--- a/src/connection.c
+++ b/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"),
-                          c->name, c->hostname, c->options, c->socket, *(uint32_t *)&c->status);
+               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->outbufsize, c->outbufstart, c->outbuflen);
        }
 
        logger(LOG_DEBUG, _("End of connections."));

diff --git a/src/connection.h b/src/connection.h
index 962d32b27b5b5f0bdd19107e4dc512207f3ba083..f84c1953dd1f764dccc8f846b15c07fcd9986b84 100644 (file)
--- a/src/connection.h
+++ b/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;

diff --git a/src/meta.c b/src/meta.c
index 8e9c8d390d69574782b0494f188b24d9c216e391..7d4ae2e50bbec23c60676a87147e553b72748613 100644 (file)
--- a/src/meta.c
+++ b/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);
-               if(!result || outlen != length) {
+               result = EVP_EncryptUpdate(c->outctx, c->outbuf + c->outbufstart + c->outbuflen,
+                               &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;
-               length = outlen;
-       } else
-               bufp = buffer;
+               c->outbuflen += outlen;
+       } else {
+               memcpy(c->outbuf + c->outbufstart + c->outbuflen, buffer, length);
+               c->outbuflen += length;
+       }
 
-       while(length) {
-               result = send(c->socket, bufp, length, 0);
+       return true;
+}
+
+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
-                               logger(LOG_ERR, _("Sending meta data to %s (%s) failed: %s"), c->name,
+                       } else if(errno == EWOULDBLOCK) {
+                               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;
 }
 

diff --git a/src/net.c b/src/net.c
index 77e2c0ddf8bf7d71df342ccb6e7ebb6db23c3362..a21850afd0c692038e244f8c37837325e6ee68a1 100644 (file)
--- a/src/net.c
+++ b/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"),
-                                                          c->name, c->hostname);
+                                       ifdebug(CONNECTIONS) logger(LOG_INFO, _("%s (%s) didn't respond to PING in %d seconds"),
+                                                          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();

diff --git a/src/net.h b/src/net.h
index 60e4dea7ba6474eb2c4da056d42556495f308347..64f92a2ae290a470efd9e005e8e39f9691462431 100644 (file)
--- a/src/net.h
+++ b/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;

diff --git a/src/net_setup.c b/src/net_setup.c
index cb702cc3d687feab79ebce756143e4794c2367a6..3847b3460e6cf29d4fb86c21f01d25bb60de6f2a 100644 (file)
--- a/src/net_setup.c
+++ b/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(pingtimeout < 1) {
-                       pingtimeout = 86400;
+       if(get_config_int(lookup_config(config_tree, "PingInterval"), &pinginterval)) {
+               if(pinginterval < 1) {
+                       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;

diff --git a/src/net_socket.c b/src/net_socket.c
index ab2d79d2af5037269e7fa22f92c4ddc0da8cf8dc..fb776f89fc649edd94bd73dceecdbd64b34152b5 100644 (file)
--- a/src/net_socket.c
+++ b/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) {
-                       logger(LOG_ERR, _("fcntl for %s: %s"), c->hostname, strerror(errno));
-               }
+       if(fcntl(c->socket, F_SETFL, flags | O_NONBLOCK) < 0) {
+               logger(LOG_ERR, _("fcntl for %s: %s"), c->hostname, strerror(errno));
        }
 #endif
 

diff --git a/src/protocol.c b/src/protocol.c
index b4e14b0da6813143d106cf7a131d511dfd3b25e0..de7f8b8760cb0bb96c40888b892d67c12f106b7e 100644 (file)
--- a/src/protocol.c
+++ b/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++;

diff --git a/src/protocol_misc.c b/src/protocol_misc.c
index ea71d6bfbc0e81217b9a9b815413d0647e5d4dee..b2f6ddc5f3f0e3b8e4ea3cd41d09ec54b1f10ec8 100644 (file)
--- a/src/protocol_misc.c
+++ b/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;