From 228e7a5c8f0e517dcede50f886965a44fca39853 Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Thu, 19 Jan 2006 17:13:18 +0000 Subject: [PATCH] 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 | 13 +++------ doc/tinc.texi | 23 ++++++--------- src/conf.c | 3 +- src/conf.h | 1 + src/connection.c | 5 ++-- src/connection.h | 8 +++++- src/meta.c | 68 ++++++++++++++++++++++++++++++++++----------- src/net.c | 64 ++++++++++++++++++++++++++++++------------ src/net.h | 3 +- src/net_setup.c | 18 ++++++++---- src/net_socket.c | 9 ++---- src/protocol.c | 2 +- src/protocol_misc.c | 7 ++++- 13 files changed, 147 insertions(+), 77 deletions(-) diff --git a/doc/tinc.conf.5.in b/doc/tinc.conf.5.in index 0f3cc2fe..a071269a 100644 --- 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 e02b6a24..0456b75b 100644 --- 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 = (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 = (no) [experimental] diff --git a/src/conf.c b/src/conf.c index fa681b81..d567cd2f 100644 --- 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 4d4047a6..f1934d06 100644 --- 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 5985cbf0..bb9e336d 100644 --- 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 962d32b2..f84c1953 100644 --- 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 8e9c8d39..7d4ae2e5 100644 --- 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 77e2c0dd..a21850af 100644 --- 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 60e4dea7..64f92a2a 100644 --- 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 cb702cc3..3847b346 100644 --- 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 ab2d79d2..fb776f89 100644 --- 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 b4e14b0d..de7f8b87 100644 --- 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 ea71d6bf..b2f6ddc5 100644 --- 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; -- 2.20.1