oweals/tinc.git
9 years agoProactively restart the SPTPS tunnel if we get receive errors.
Etienne Dechamps [Sun, 17 May 2015 17:50:11 +0000 (18:50 +0100)]
Proactively restart the SPTPS tunnel if we get receive errors.

There are a number of ways a SPTPS tunnel can get into a corrupt state.
For example, during key regeneration, the KEX and SIG messages from
other nodes might arrive out of order, which confuses the hell out of
the SPTPS code. Another possible scenario is not noticing another node
crashed and restarted because there was no point in time where the node
was seen completely disconnected from *all* nodes; this could result in
using the wrong (old) key. There are probably other scenarios which have
not even been considered yet. Distributed systems are hard.

When SPTPS got confused by a packet, it used to crash the entire
process; fortunately that was fixed by commit
2e7f68ad2b51648b89c4b5c61aeb4cec67c2fbbb. However, the error handling
(or lack thereof) leaves a lot to be desired. Currently, when SPTPS
encounters an error when receiving a packet, it just shrugs it off and
continues as if nothing happened. The problem is, sometimes getting
receive errors mean the tunnel is completely stuck and will not recover
on its own. In that case, the node will become unreachable - possibly
indefinitely.

The goal of this commit is to improve SPTPS error handling by taking
proactive action when an incoming packet triggers a failure, which is
often an indicator that the tunnel is stuck in some way. When that
happens, we simply restart SPTPS entirely, which should make the tunnel
recover quickly.

To prevent "storms" where two buggy nodes flood each other with invalid
packets and therefore spend all their time negotiating new tunnels, we
limit the frequency at which tunnel restarts happen to ten seconds.

It is likely this commit will solve the "Invalid KEX record length
during key regeneration" issue that has been seen in the wild. It is
difficult to be sure though because we do not have a full understanding
of all the possible conditions that can trigger this problem.

9 years agoDon't send local_address in ADD_EDGE messages if it's AF_UNSPEC.
Guus Sliepen [Sun, 17 May 2015 16:44:09 +0000 (18:44 +0200)]
Don't send local_address in ADD_EDGE messages if it's AF_UNSPEC.

9 years agoLet sockaddr2hostname() handle AF_UNSPEC addresses.
Sven-Haegar Koch [Sun, 17 May 2015 03:29:21 +0000 (05:29 +0200)]
Let sockaddr2hostname() handle AF_UNSPEC addresses.

9 years agoPrevent SPTPS key regeneration packets from entering an UDP relay path.
Etienne Dechamps [Sun, 17 May 2015 16:09:56 +0000 (17:09 +0100)]
Prevent SPTPS key regeneration packets from entering an UDP relay path.

Commit 10c1f60c643607d9dafd79271c3475cddf81e903 introduced a mechanism
by which a packet received by REQ_KEY could continue its journey over
UDP. This was based on the assumption that REQ_KEY messages would never
be used for handshake packets (which should never be sent over UDP,
because SPTPS currently doesn't handle lost handshake packets very
well).

Unfortunately, there is one case where handshake packets are sent using
REQ_KEY: when regenerating the SPTPS key for a pre-established channel.
With the current code, such packets risk getting relayed over UDP.

When processing a REQ_KEY message, it is impossible for the receiving
end to distinguish between a data SPTPS packet and a handshake packet,
because this information is stored in the type field which is encrypted
with the end-to-end key.

This commit fixes the issue by making tinc use ANS_KEY for all SPTPS
handshake messages. This works because ANS_KEY messages are never
forwarded using the SPTPS relay mechanisms, therefore they are
guaranteed to stick to TCP.

9 years agoLet sockaddr2str() handle AF_UNSPEC addresses.
Guus Sliepen [Sat, 16 May 2015 00:01:54 +0000 (02:01 +0200)]
Let sockaddr2str() handle AF_UNSPEC addresses.

9 years agoTry all addresses for the hostname in an invitation URL.
Guus Sliepen [Fri, 15 May 2015 21:35:46 +0000 (23:35 +0200)]
Try all addresses for the hostname in an invitation URL.

9 years agoBe more liberal accepting ADD_EDGE messages with conflicting local address information.
Guus Sliepen [Fri, 15 May 2015 21:08:53 +0000 (23:08 +0200)]
Be more liberal accepting ADD_EDGE messages with conflicting local address information.

If the ADD_EDGE is for one of the edges we own, and if it is not the
same as we actually have, send a correcting ADD_EDGE back. Otherwise, if
the ADD_EDGE contains new information, update our idea of the local
address for that edge.

If the ADD_EDGE does not contain local address information, then we
never make a correction nor log a warning.

9 years agoUse AF_UNSPEC instead of AF_UNKNOWN for unspecified local address in add_edge_h().
Guus Sliepen [Fri, 15 May 2015 21:01:06 +0000 (23:01 +0200)]
Use AF_UNSPEC instead of AF_UNKNOWN for unspecified local address in add_edge_h().

AF_UNKNOWN is reserved for valid addresses that the local node cannot
parse, but remote nodes possibly can.

9 years agoFix receiving UDP packets from tinc 1.0.x nodes.
Guus Sliepen [Thu, 14 May 2015 22:21:48 +0000 (00:21 +0200)]
Fix receiving UDP packets from tinc 1.0.x nodes.

In try_mac(), the wrong offsets were used into the packet buffer,
causing the digest verification to always fail.

9 years agoFix invitations.
Guus Sliepen [Wed, 13 May 2015 12:28:28 +0000 (14:28 +0200)]
Fix invitations.

These were broken due to a change in behaviour of sptps_receive_data()
introduced in commit d237efd325cd7bdd73f5eb111c769470238dce6e.

9 years agoIntroduce raw TCP SPTPS packet transport.
Etienne Dechamps [Sun, 10 May 2015 18:00:03 +0000 (19:00 +0100)]
Introduce raw TCP SPTPS packet transport.

Currently, SPTPS packets are transported over TCP metaconnections using
extended REQ_KEY requests, in order for the packets to pass through
tinc-1.0 nodes unaltered. Unfortunately, this method presents two
significant downsides:

 - An already encrypted SPTPS packet is decrypted and then encrypted
   again every time it passes through a node, since it is transported
   over the SPTPS channels of the metaconnections. This
   double-encryption is unnecessary and wastes CPU cycles.

 - More importantly, the only way to transport binary data over
   standard metaconnection messages such as REQ_KEY is to encode it
   in base64, which has a 33% encoding overhead. This wastes 25% of the
   network bandwidth.

This commit introduces a new protocol message, SPTPS_PACKET, which can
be used to transport SPTPS packets over a TCP metaconnection in an
efficient way. The new message is appropriately protected through a
minor protocol version increment, and extended REQ_KEY messages are
still used with nodes that do not support the new message, as well as
for the intial handshake packets, for which efficiency is not a concern.

The way SPTPS_PACKET works is very similar to how the traditional PACKET
message works: after the SPTPS_PACKET message, the raw binary packet is
sent directly over the metaconnection. There is one important
difference, however: in the case of SPTPS_PACKET, the packet is sent
directly over the TCP stream completely bypassing the SPTPS channel of
the metaconnection itself for maximum efficiency. This is secure because
the SPTPS packet that is being sent is already encrypted with an
end-to-end key.

9 years agoOnly read one record at a time in sptps_receive_data().
Etienne Dechamps [Sun, 10 May 2015 18:28:11 +0000 (19:28 +0100)]
Only read one record at a time in sptps_receive_data().

sptps_receive_data() always consumes the entire buffer passed to it,
which is somewhat inflexible. This commit improves the interface so that
sptps_receive_data() consumes at most one record. The goal is to allow
non-SPTPS stuff to be interleaved with SPTPS records in a single TCP
stream.

9 years agoRename REQ_SPTPS to SPTPS_PACKET.
Etienne Dechamps [Sun, 10 May 2015 17:05:19 +0000 (18:05 +0100)]
Rename REQ_SPTPS to SPTPS_PACKET.

REQ_SPTPS implies the message has an ANS_ counterpart (like REQ_KEY,
ANS_KEY), but it doesn't. Therefore dropping the REQ_ seems more
appropriate, and we add a _PACKET suffix to reduce the likelihood of
naming conflicts.

9 years agoTry to use UDP to relay SPTPS packets received over TCP.
Etienne Dechamps [Sat, 9 May 2015 17:09:23 +0000 (18:09 +0100)]
Try to use UDP to relay SPTPS packets received over TCP.

Currently, when tinc receives a SPTPS packet over TCP via the REQ_KEY
encapsulation mechanism, it forwards it like any other TCP request. This
is inefficient, because even though we received the packet over TCP,
we might have an UDP link with the next hop, which means the packet
could be sent over UDP.

This commit removes that limitation by making sure SPTPS data packets
received through REQ_KEY requests are not forwarded as-is but passed
to send_sptps_data() instead, thereby using the same code path as if
the packet was received over UDP.

9 years agoExpose the raw SPTPS send interface from net_packet.
Etienne Dechamps [Sat, 9 May 2015 16:54:34 +0000 (17:54 +0100)]
Expose the raw SPTPS send interface from net_packet.

net_packet doesn't actually use send_sptps_data(); it only uses
send_sptps_data_priv(). In addition, the only user of send_sptps_data()
is protocol_key. Therefore it makes sense to expose
send_sptps_data_priv() directly, and move send_sptps_data() (which is
basically just boilerplate) as a local function in protocol_key.

9 years agoUse the correct originator node when relaying SPTPS UDP packets.
Etienne Dechamps [Sun, 10 May 2015 17:46:47 +0000 (18:46 +0100)]
Use the correct originator node when relaying SPTPS UDP packets.

Currently, when relaying SPTPS UDP packets, the code uses the direct
sender as the originator, instead of preserving the original source ID.

This wouldn't cause any issues in most cases because the originator and
the sender are the same in simple one-hop relay chains, but this will
break as soon as there is more than one relay.

9 years agoWhen relaying, send probes to the destination, not the source.
Etienne Dechamps [Sun, 10 May 2015 17:37:30 +0000 (18:37 +0100)]
When relaying, send probes to the destination, not the source.

This seems to be a typo from c23e50385d9de538af676706596f6508b2ceb01a.
Achievement unlocked: got a one-line commit wrong.

9 years agoAdd support for out-of-tree ("VPATH") builds.
Etienne Dechamps [Sat, 12 Jul 2014 15:01:41 +0000 (16:01 +0100)]
Add support for out-of-tree ("VPATH") builds.

This fixes some issues with the build system when building out of tree.

With this commit, it is now possible to do the following:

    $ cd /tmp/build
    $ /path/to/tinc/configure
    $ make

9 years agoRemove explicit distribution rules for m4 scripts.
Etienne Dechamps [Sat, 12 Jul 2014 15:21:32 +0000 (16:21 +0100)]
Remove explicit distribution rules for m4 scripts.

It turns out Automake is smart enough to include these files in the
distribution by itself.

9 years agoReally remove "release-" from the git-derived version string.
Guus Sliepen [Sat, 9 May 2015 13:41:37 +0000 (15:41 +0200)]
Really remove "release-" from the git-derived version string.

9 years agoUse git describe to populate autoconf's VERSION.
Etienne Dechamps [Sun, 29 Jun 2014 17:26:55 +0000 (18:26 +0100)]
Use git describe to populate autoconf's VERSION.

This uses the output of "git describe" directly in configure.ac to
determine the version number to use, instead of hardcoding it.

With this change, current version information is completely removed
from the codebase itself, and is always fetched on-the-fly from git as
the single source of truth.

In order to ensure make dist always uses the current version number in
the contents of the packaged configure script as well as the package
name, a dependency is added to the dist target such that autoconf is
always run before dist to regenerate the version number. If this wasn't
the case, make dist would use the version number from when autoconf was
originally run, not the version number that make dist is running from.
That said, errors from that rule are ignored so that people can still
run make dist without a working autoconf.

In addition, the NEWS check is dropped, as it would then become annoying
because it would force make dist users to always have a line for the
current commit in the NEWS file.

9 years agoFix typo in tincctl help.
Pierre Emeriaud [Fri, 8 May 2015 22:03:51 +0000 (00:03 +0200)]
Fix typo in tincctl help.

9 years agoDon't include build-time generated version_git.h in the tarball.
Guus Sliepen [Tue, 5 May 2015 21:05:22 +0000 (23:05 +0200)]
Don't include build-time generated version_git.h in the tarball.

9 years agoRemove "release-" from displayed git version.
Guus Sliepen [Tue, 5 May 2015 21:03:41 +0000 (23:03 +0200)]
Remove "release-" from displayed git version.

Also make sure that version_git.h is only written to if the "git
describe" command succeeds.

9 years agoUse git description as the tinc version.
Etienne Dechamps [Sun, 29 Jun 2014 14:22:10 +0000 (15:22 +0100)]
Use git description as the tinc version.

Instead of using the hardcoded version number in configure.ac, this
makes tinc use the live version reported by "git describe",
queried on-the-fly during the build process and regenerated for every
build.

This makes tinc version output more useful, as tinc will now display the
number of commits since the last tag as well as the commit the binary is
built from, following the format described in git-describe(1).

Here's an example of tincd --version output:

  tinc version release-1.1pre10-48-gc149315 (built Jun 29 2014 15:21:10, protocol 17.3)

When building directly from a release tag, this will look like the following:

  tinc version release-1.1pre10 (built Jun 29 2014 15:21:10, protocol 17.3)

(Note that the format is slightly different - because of the way the
tags are named, it says "release-1.1pre10" instead of just "1.1pre10")

If git describe fails (for example when building from a release
tarball), the build automatically falls back to the autoconf-provided
VERSION macro (i.e. the old behavior).

9 years agoFix typo 0fda572c88d02b0b200ef81d72cc4da594fa0e38 that prevented some errors from...
Guus Sliepen [Fri, 24 Apr 2015 21:51:29 +0000 (23:51 +0200)]
Fix typo 0fda572c88d02b0b200ef81d72cc4da594fa0e38 that prevented some errors from being logged.

9 years agoDon't log an error message when receiving a TERMREQ.
Guus Sliepen [Fri, 24 Apr 2015 21:43:58 +0000 (23:43 +0200)]
Don't log an error message when receiving a TERMREQ.

9 years agoFix a possible segmentation fault during key upgrades.
Guus Sliepen [Fri, 24 Apr 2015 21:43:19 +0000 (23:43 +0200)]
Fix a possible segmentation fault during key upgrades.

read_rsa_public_key() was bailing out early if the given node already has an Ed25519 key, and
returned true even though c->rsa was NULL. The early bailout code isn't necessary anymore, so just
remove it.

9 years agoAllow one-sided upgrades to Ed25519.
Guus Sliepen [Fri, 24 Apr 2015 21:40:20 +0000 (23:40 +0200)]
Allow one-sided upgrades to Ed25519.

This deals with the case where one node knows the Ed25519 key of another node, but not the other
way around. This was blocked by an overly paranoid check in id_h(). The upgrade_h() function already
handled this case, and the node that already knows the other's Ed25519 key checks that it has not
been changed, otherwise the connection will be aborted.

9 years agoMerge remote-tracking branch 'dechamps/wintapver' into 1.1
Guus Sliepen [Sun, 12 Apr 2015 13:43:05 +0000 (15:43 +0200)]
Merge remote-tracking branch 'dechamps/wintapver' into 1.1

9 years agoAlways call res_init() before getaddrinfo().
Guus Sliepen [Sun, 12 Apr 2015 13:42:48 +0000 (15:42 +0200)]
Always call res_init() before getaddrinfo().

Unfortunately, glibc assumes that /etc/resolv.conf is a static file that
never changes. Even on servers, /etc/resolv.conf might be a dynamically
generated file, and we never know when it changes. So just call
res_init() every time, so glibc uses up-to-date nameserver information.

Conflicts:
src/have.h
src/net.c
src/net_setup.c

9 years agoMerge remote-tracking branch 'dechamps/windevice' into 1.1
Guus Sliepen [Sun, 12 Apr 2015 13:36:50 +0000 (15:36 +0200)]
Merge remote-tracking branch 'dechamps/windevice' into 1.1

9 years agoMerge remote-tracking branch 'dechamps/winmtu' into 1.1
Guus Sliepen [Sun, 12 Apr 2015 13:35:50 +0000 (15:35 +0200)]
Merge remote-tracking branch 'dechamps/winmtu' into 1.1

9 years agoMerge remote-tracking branch 'dechamps/fsckwin' into 1.1
Guus Sliepen [Sun, 12 Apr 2015 13:35:37 +0000 (15:35 +0200)]
Merge remote-tracking branch 'dechamps/fsckwin' into 1.1

9 years agoMerge remote-tracking branch 'dechamps/staticfix' into 1.1
Guus Sliepen [Sun, 12 Apr 2015 13:34:50 +0000 (15:34 +0200)]
Merge remote-tracking branch 'dechamps/staticfix' into 1.1

9 years agoWarn about performance if using TAP-Windows >=9.21.
Etienne Dechamps [Sun, 15 Mar 2015 18:30:39 +0000 (18:30 +0000)]
Warn about performance if using TAP-Windows >=9.21.

Testing has revealed that the newer series of Windows TAP drivers (i.e.
9.0.0.21 and later, also known as NDIS6, tap-windows6) suffer from
serious performance issues in the write path. Write operations seems to
take a very long time to complete, resulting in massive packet loss even
for throughputs as low as 10 Mbit/s.

I've made some attempts to alleviate the problem using parellelism. By
using custom code that allows up to 256 write operations at the same
time the results are much better, but it's still about 2 times worse
than the traditional 9.0.0.9 driver.

We need to investigate more and file a bug against tap-windows6, but in
the mean time, let's inform the user that he might not want to use the
latest drivers.

9 years agoLog TAP-Windows driver version on startup.
Etienne Dechamps [Sun, 15 Mar 2015 18:18:04 +0000 (18:18 +0000)]
Log TAP-Windows driver version on startup.

This is generally useful. We've seen issues that are specific to some
version of these drivers (especially the newer 9.0.0.21 version), so
it's relevant to log it, especially since that means it will be
copy-pasted by people posting their logs asking for help.

9 years agoIncrease the ReplayWindow default from 16 to 32.
Etienne Dechamps [Sun, 15 Mar 2015 18:01:03 +0000 (18:01 +0000)]
Increase the ReplayWindow default from 16 to 32.

As a rule, it seems reasonable to make sure that tinc operates correctly
on at least 1G links, since these are pretty common. However, I have
observed replay window issues when operating at speeds of 600 Mbit/s and
above, especially when the receiving end is a Windows system (not sure
why). This commit increases the default so that this won't occur on
fresh setups.

9 years agoSet the default for UDPRcvBuf and UDPSndBuf to 1M.
Etienne Dechamps [Sun, 15 Mar 2015 17:50:53 +0000 (17:50 +0000)]
Set the default for UDPRcvBuf and UDPSndBuf to 1M.

It may not be obvious, but due to the way tinc operates (single-threaded
control loop with no intermediate packet buffer), UDP send and receive
buffers can have a massive impact on performance. It is therefore of
paramount importance that the buffers be large enough to prevent packet
drops that could occur while tinc is processing a packet.

Leaving that value to the OS default could be reasonable if we weren't
relying on it so much. Instead, this makes performance somewhat
unpredictable.

In practice, the worst case scenario occurs on Windows, where Microsoft
had the brillant idea of making the buffers 8K in size by default, no
matter what the link speed is. Considering that 8K flies past in a
matter of microseconds on >1G links, this is extremely inappropriate. On
these systems, changing the buffer size to 1M results in *obscene*
raw throughput improvements; I have observed a 10X jump from 40 Mbit/s
to 400 Mbit/s on my system.

In this commit, we stop trusting the OS to get this right and we use a
fixed 1M value instead, which should be enough for <=1G links.

9 years agoFix Windows device asynchronous write behavior.
Etienne Dechamps [Sat, 14 Mar 2015 18:19:22 +0000 (18:19 +0000)]
Fix Windows device asynchronous write behavior.

Write operations to the Windows device do not necessarily complete
immediately; in fact, with the latest TAP-Win32 drivers, this never
seems to be the case.

write_packet() does not handle that case correctly, because the
OVERLAPPED structure and the packet data go out of scope before the
write operation completes, resulting in race conditions.

This commit fixes the issue by making sure these data structures are
kept in global scope, and by dropping any packets that may arrive while
the previous write operation is still pending.

9 years agoWhen disabling the Windows device, wait for pending reads to complete.
Etienne Dechamps [Sat, 14 Mar 2015 17:27:14 +0000 (17:27 +0000)]
When disabling the Windows device, wait for pending reads to complete.

On Windows, when disabling the device, tinc uses the CancelIo() to
cancel the pending read operation, and then proceeds to delete the event
handle immediately.

This assumes that CancelIo() blocks until the pending read request is
completely torn down and no references to it remain. While MSDN is not
completely clear on that subject, it does suggest that this is not the
case:

  http://msdn.microsoft.com/en-us/library/windows/desktop/aa363791.aspx
  If the function succeeds [...] the cancel operation for all pending
  I/O operations issued by the calling thread for the specified file
  handle was successfully requested.

This implies that cancellation was merely "requested", and that there
are no guarantees as to the state of the operation when CancelIo()
returns. Therefore, care must be taken not to close event handles
prematurely.

While I'm no aware of this potential race condition causing any problems
in practice, I don't want to take any chances.

9 years agoMake sure packet header structures are correctly packed on Windows.
Etienne Dechamps [Sun, 15 Mar 2015 10:00:56 +0000 (10:00 +0000)]
Make sure packet header structures are correctly packed on Windows.

Modern versions of GCC handle structure packing differently when
compiling for Windows, as reported in the following GCC bug report:

  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

In practice, this affects tinc because it uses packed structs as a
convenient way to populate packet headers. "struct ip" is especially
affected - on Linux, sizeof(struct ip) returns 20 as expected, while on
Windows, it returns 24 because of the broken alignment.

This in turn completely breaks code that has to populate an IP header.
Specifically, this breaks route_ipv4_unreachable() which is responsible,
among other things, for the generation of ICMP Fragmentation Needed
messages. On Windows, these messages are corrupted beyond hope because
of this alignment issue. For TCP connections that are established
before tinc obtains a fix on the MTU (and thus are not MSS clamped),
this can result in massive disruption.

This commit fixes the issue by forcing GCC to use standard alignment
for all packed structures in the tinc codebase instead of the MSVC
alignment.

9 years agoFix HAVE_DECL_RES_INIT conditionals.
Etienne Dechamps [Sat, 14 Mar 2015 16:17:32 +0000 (16:17 +0000)]
Fix HAVE_DECL_RES_INIT conditionals.

HAVE_DECL_RES_INIT is generated using AC_CHECK_DECLS. tinc checks this
symbol using #ifdef, which is wrong because (according to autoconf docs)
the symbol is always defined, it's just set to zero if the check failed.

This broke the Windows build starting from
0b310bf406dbe58afe37fa31156b9ea47599d7be, because it introduced this
conditional in code that's not excluded from the Windows build.

9 years agoFix invalid getuid() call on Windows.
Etienne Dechamps [Sat, 14 Mar 2015 16:07:54 +0000 (16:07 +0000)]
Fix invalid getuid() call on Windows.

This is breaking the Windows build. Regression was introduced in
268e3ffca7b45cfc736e1bc9bec7a113c6c45701.

9 years agoDon't send UDP probes past static relays.
Etienne Dechamps [Sat, 14 Mar 2015 14:04:50 +0000 (14:04 +0000)]
Don't send UDP probes past static relays.

Ironically, commit 0f8e2cc78cafe47a087d3fc9b480551b841aeb30 introduced
a regression on its own, since it accidently removed a return statement
that prevented try_tx_sptps() from sending UDP/MTU probes to nodes that
are past static relays.

9 years agoThrottle the rate of MTU_INFO messages.
Etienne Dechamps [Sun, 8 Mar 2015 20:17:27 +0000 (20:17 +0000)]
Throttle the rate of MTU_INFO messages.

This makes sure MTU_INFO messages are only sent at the maximum rate of
5 per second (by default). As usual with these "probe" mechanisms, the
rate of these messages cannot be higher than the rate of data packets
themselves, since they are sent from the RX path.

9 years agoThrottle the rate of UDP_INFO messages.
Etienne Dechamps [Sun, 8 Mar 2015 19:54:44 +0000 (19:54 +0000)]
Throttle the rate of UDP_INFO messages.

This makes sure UDP_INFO messages are only sent at the maximum rate of
5 per second (by default). As usual with these "probe" mechanisms, the
rate of these messages cannot be higher than the rate of data packets
themselves, since they are sent from the RX path.

9 years agoAdd MTU_INFO protocol message.
Etienne Dechamps [Sun, 8 Mar 2015 18:54:50 +0000 (18:54 +0000)]
Add MTU_INFO protocol message.

In this commit, nodes use MTU_INFO messages to provide MTU information.

The issue this code is meant to address is the non-trivial problem of
finding the proper MTU when UDP SPTPS relays are involved. Currently,
tinc has no idea what the MTU looks like beyond the first relay, and
will arbitrarily use the first relay's MTU as the limit. This will fail
miserably if the MTU decreases after the first relay, forcing relays to
fall back to TCP. More generally, one should keep in mind that relay
paths can be arbitrarily complex, resulting in packets taking "epic
journeys" through the graph, switching back and forth between UDP (with
variable MTUs) and TCP multiple times along the path.

A solution that was considered consists in sending standard MTU probes
through the relays. This is inefficient (if there are 3 nodes on one
side of relay and 3 nodes on the other side, we end up with 3*3=9 MTU
discoveries taking place at the same time, while technically only
3+3=6 are needed) and would involve eyebrow-raising behaviors such as
probes being sent over TCP.

This commit implements an alternative solution, which consists in
the packet receiver sending MTU_INFO messages to the packet sender.
The message contains an MTU value which is set to maximum when the
message is originally sent. The message gets altered as it travels
through the metagraph, such that when the message arrives to the
destination, the MTU value contained in the message can be used to
send packets while making sure no relays will be forced to fall back to
TCP to deliver them.

The operating principles behind such a protocol message are similar to
how the UDP_INFO message works, but there is a key difference that
prevents us from simply reusing the same message: the UDP_INFO message
only cares about relay-to-relay links (i.e. it is sent between static
relays and the information it contains only makes sense between two
adjacent static relays), while the MTU_INFO cares about the end-to-end
MTU, including the entire relay path. Therefore, UDP_INFO messages stop
when they encounter static relays, while MTU_INFO messages don't stop
until they get to the original packet sender.

Note that, technically, the MTU that is obtained through this mechanism
can be slightly pessimistic, because it can be lowered by an
intermediate node that is not being used as a relay. Since nodes have no
way of knowing whether they'll be used as dynamic relays or not (and
have no say in the matter), this is not a trivial problem. That said,
this is highly unlikely to result in noticeable issues in realistic
scenarios.

9 years agoAdd UDP_INFO protocol message.
Etienne Dechamps [Sat, 3 Jan 2015 17:46:33 +0000 (17:46 +0000)]
Add UDP_INFO protocol message.

In this commit, nodes use UDP_INFO messages to provide UDP address
information. The basic principle is that the node that receives packets
sends UDP_INFO messages to the node that's sending the packets. The
message originally contains no address information, and is (hopefully)
updated with relevant address information as it gets relayed through the
metagraph - specifically, each intermediate node will update the message
with its best guess as to what the address is while forwarding it.

When a node receives an UDP_INFO message, and it doesn't have a
confirmed UDP tunnel with the originator node, it will update its
records with the new address for that node, so that it always has the
best possible guess as to how to reach that node. This applies to the
destination node of course, but also to any intermediate nodes, because
there's no reason they should pass on the free intel, and because it
results in nice behavior in the presence of relay chains (multiple nodes
in a path all trying to reach the same destination).

If, on the other hand, the node does have a confirmed UDP tunnel, it
will ignore the address information contained in the message.

In all cases, if the node that receives the message is not the
destination node specified in the message, it will forward the message
but not before overriding the address information with the one from its
own records. If the node has a confirmed UDP tunnel, that means the
message is updated with the address of the confirmed tunnel; if not,
the message simply reflects the records of the intermediate node, which
just happen to be the contents of the UDP_INFO message it just got, so
it's simply forwarded with no modification.

This is similar to the way ANS_KEY messages are currently
overloaded to provide UDP address information, with two differences:

 - UDP_INFO messages are sent way more often than ANS_KEY messages,
   thereby keeping the address information fresh. Previously, if the UDP
   situation were to change after the ANS_KEY message was sent, the
   sender would virtually never get the updated information.

 - Once a node puts address information in an ANS_KEY message, it is
   never changed again as the message travels through the metagraph; in
   contrast, UDP_INFO messages behave the opposite way, as they get
   rewritten every time they travel through a node with a confirmed UDP
   tunnel. The latter behavior seems more appropriate because UDP tunnel
   information becomes more relevant as it moves closer to the
   destination node. The ANS_KEY behavior is not satisfactory in some
   cases such as multi-layered graphs where the first hop is located
   before a NAT.

Ultimately, the rationale behind this whole process is to improve UDP
hole punching capabilities when port translation is in effect, and more
generally, to make tinc more reliable in (very) hostile network
conditions (such as multi-layered NAT).

9 years ago--syslog and --logfile are mutually exclusive.
Guus Sliepen [Sat, 14 Mar 2015 12:02:29 +0000 (12:02 +0000)]
--syslog and --logfile are mutually exclusive.

9 years agoFix the case where we detach and use --logfile.
Guus Sliepen [Sat, 14 Mar 2015 12:02:06 +0000 (12:02 +0000)]
Fix the case where we detach and use --logfile.

9 years agoMerge remote-tracking branch 'seehuhn/1.1' into 1.1
Guus Sliepen [Sat, 14 Mar 2015 11:45:55 +0000 (11:45 +0000)]
Merge remote-tracking branch 'seehuhn/1.1' into 1.1

9 years agoMerge remote-tracking branch 'dechamps/sptpsabort' into 1.1
Guus Sliepen [Sat, 14 Mar 2015 11:44:38 +0000 (11:44 +0000)]
Merge remote-tracking branch 'dechamps/sptpsabort' into 1.1

9 years agoAdd a new --syslog option for tincd.
Jochen Voss [Fri, 13 Mar 2015 11:05:22 +0000 (11:05 +0000)]
Add a new --syslog option for tincd.

This commit adds a new command line option for tincd which allows to
use tincd in non-detached mode with log messages still going to
syslog.  The motivation for this change is to ease use of tincd
in Docker containers.

9 years agoDon't abort() willy-nilly in SPTPS code.
Etienne Dechamps [Sun, 8 Mar 2015 17:32:39 +0000 (17:32 +0000)]
Don't abort() willy-nilly in SPTPS code.

If receive_handshake() or the receive_record() user callback returns an
error, sptps_receive_data_datagram() crashes the entire process. This is
heavy-handed, makes tinc very brittle to certain failures (i.e.
unexpected packets), and is inconsistent with the rest of SPTPS code.

9 years agoFix UDP/MTU discovery in intermediate SPTPS UDP relays.
Etienne Dechamps [Sun, 8 Mar 2015 14:32:01 +0000 (14:32 +0000)]
Fix UDP/MTU discovery in intermediate SPTPS UDP relays.

Refactoring commit 81578484dc74fd92f1b01f71f882016f120ab1de seems to
have introduced a regression as it moved discovery code away from
send_sptps_data_priv() and within send_packet(). The issue is,
send_packet() is not called when the node is simply relaying an UDP
SPTPS packet: indeed, send_sptps_data_priv() is called directly from
handle_incoming_vpn_data() in that case.

As a result, try_tx_sptps() is not called in the relaying case, which in
practice means that a relay doesn't initiate UDP/MTU discovery with the
next relay (unless some other activity compels it to do so). This can
result in packets getting sent over TCP instead of UDP from the relay.

9 years agoFix dynamic UDP SPTPS relaying.
Etienne Dechamps [Sun, 8 Mar 2015 14:20:15 +0000 (14:20 +0000)]
Fix dynamic UDP SPTPS relaying.

Refactoring commit 0e653260478005eb7c824a9a1a3df04f39938cd6 broke UDP
SPTPS relaying by accidently removing try_tx_sptps() logic related to
establishing connectivity to so-called "dynamic" relays (i.e. relays
that are not specified by IndirectData configuration statements, but
are used on-the-fly to circumvent loss of direct UDP connectivity).

Specifically, the TX path was not trying to establish a tunnel to
dynamic relays (nexthop) anymore. This meant that MTU was not being
discovered with dynamic relays, which basically meant that all packets
being sent to dynamic relays went over TCP, thereby defeating the whole
purpose of SPTPS UDP relaying.

Note that this bug could easily go unnoticed if a tunnel was established
with the dynamic tunnel for some other reason (i.e. exchanging actual
data packets with the relay node).

9 years agoFix compile errors introduced in cfe9285adf391ab66faeb5def811fe08e47a221a
xentec [Tue, 17 Feb 2015 03:02:35 +0000 (04:02 +0100)]
Fix compile errors introduced in cfe9285adf391ab66faeb5def811fe08e47a221a

Compiling with `--disable-legacy-protocol` resulted in failure caused by the missing exclusion of some symbols in net_packet.c.

9 years agoSuppress warnings about parsing Ed25519 keys when they are not present.
Guus Sliepen [Mon, 16 Feb 2015 07:42:30 +0000 (08:42 +0100)]
Suppress warnings about parsing Ed25519 keys when they are not present.

9 years agoDocument that --force should precede commands.
Guus Sliepen [Mon, 16 Feb 2015 07:26:49 +0000 (08:26 +0100)]
Document that --force should precede commands.

9 years agoFixed variables.test testsuite after 'Make "tinc add" idempotent.' change.
Sven-Haegar Koch [Tue, 10 Feb 2015 00:17:12 +0000 (01:17 +0100)]
Fixed variables.test testsuite after 'Make "tinc add" idempotent.' change.

9 years agoMake "tinc add" idempotent.
Guus Sliepen [Mon, 9 Feb 2015 14:23:59 +0000 (15:23 +0100)]
Make "tinc add" idempotent.

When calling "tinc add" multiple times with the same variable and value,
make sure only one unique line is added to the configuration file.

9 years agoAlways call res_init() before getaddrinfo().
Guus Sliepen [Mon, 9 Feb 2015 14:16:36 +0000 (15:16 +0100)]
Always call res_init() before getaddrinfo().

Unfortunately, glibc assumes that /etc/resolv.conf is a static file that
never changes. Even on servers, /etc/resolv.conf might be a dynamically
generated file, and we never know when it changes. So just call
res_init() every time, so glibc uses up-to-date nameserver information.

9 years agoAdd the "fsck" command to the CLI.
Guus Sliepen [Thu, 15 Jan 2015 21:57:56 +0000 (22:57 +0100)]
Add the "fsck" command to the CLI.

This will report possible problems in the configuration files, and in
some cases offers to fix them.

The code is far from perfect yet. It expects keys to be in their default
locations, it doesn't check for Public/PrivateKey[File] statemetns yet.
It also does not correctly handle Ed25519 public keys yet.

9 years agoImprove packet source detection.
Guus Sliepen [Mon, 12 Jan 2015 13:43:32 +0000 (14:43 +0100)]
Improve packet source detection.

When no UDP communication has been done yet, tinc establishes a guess
for the UDP address+port of each node. However, when there are multiple nodes
behind a NAT, tinc will guess the exact same address+port combination
for them, because it doesn't know about the NAT mappings yet. So when
receiving a packet, don't trust that guess unless we have confirmed UDP
communication.

This ensures try_harder() is called in such cases. However, this
function was actually very inefficient, trying to verify packets
multiple times for nodes with multiple edges. Only call try_mac() at
most once per node.

9 years agoSend gratuitous type 2 probe replies.
Guus Sliepen [Sun, 11 Jan 2015 16:44:50 +0000 (17:44 +0100)]
Send gratuitous type 2 probe replies.

If we receive any traffic from another node, we periodically send back a
gratuitous type 2 probe reply with the maximum received packet length.
On the other node, this causes the udp and perhaps mtu probe timers to
be reset, so it does not need to send a probe request. Gratuitous probe
replies from another node also count as received traffic for this
purpose, so for nodes that also have a meta-connection, UDP keepalive
packets in principle can now solely be type 2 replies. This reduces the
amount of probe traffic even more.

To work, gratuitous replies should be sent slightly more often than
udp_discovery_keepalive_interval, so probe requests won't be triggered.
This also means that the timer resolution must be smaller than the
difference between the two, and at the moment it's kind of a hack.

9 years agoSend the size of the largest recently received packets in type 2 probe replies.
Guus Sliepen [Sun, 11 Jan 2015 15:14:05 +0000 (16:14 +0100)]
Send the size of the largest recently received packets in type 2 probe replies.

9 years agoMove UDP probe reply code into its own function.
Guus Sliepen [Sun, 11 Jan 2015 15:12:57 +0000 (16:12 +0100)]
Move UDP probe reply code into its own function.

This reduces the level of indentation, and prepares for sending gratuitous type 2 probe replies.

9 years agoKeep track of the largest UDP packet size received from a node.
Guus Sliepen [Sun, 11 Jan 2015 15:10:58 +0000 (16:10 +0100)]
Keep track of the largest UDP packet size received from a node.

9 years agoMove detection of PMTU decrease to try_mtu().
Guus Sliepen [Sun, 11 Jan 2015 14:38:56 +0000 (15:38 +0100)]
Move detection of PMTU decrease to try_mtu().

When we have fixed the PMTU, n->mtuprobes == -1. When we send MTU probes
when mtuprobes == -1, decrease mtuprobes, and reset it back to -1 in
mtu_probe_h(). If mtuprobes < -1, send MTU probes every second, until
mtuprobes <= -4, in which case we will restart MTU discovery.

9 years agoSend MTU probes only once every PingInterval.
Guus Sliepen [Sun, 11 Jan 2015 13:44:27 +0000 (14:44 +0100)]
Send MTU probes only once every PingInterval.

9 years agoRemove RTT and packet loss estimation code.
Guus Sliepen [Sun, 11 Jan 2015 13:44:15 +0000 (14:44 +0100)]
Remove RTT and packet loss estimation code.

This is not working at all anymore. Just remove it, and we'll do another
attempt at RTT, bandwidth and packet loss estimation after the new
probing code stabilizes.

9 years agoOnly send small packets during UDP probes.
Guus Sliepen [Sun, 11 Jan 2015 12:53:16 +0000 (13:53 +0100)]
Only send small packets during UDP probes.

We are trying to decouple UDP probing from MTU probing, so only send
very small packets during UDP probing. This significantly reduces the
amount of traffic sent (54 to 67 bytes per probe instead of 1500 bytes).

This means the MTU probing code takes over sending PMTU sized probes,
but this commit does not take care of detecting PMTU decreases.

9 years agoImmediately send our key when a meta-connection is established.
Guus Sliepen [Sun, 11 Jan 2015 12:51:55 +0000 (13:51 +0100)]
Immediately send our key when a meta-connection is established.

This is what 1.0 does, and speeds up the UDP probing.

9 years agoAlways keep UDP mappings alive for nodes that also have a meta-connection.
Guus Sliepen [Sun, 11 Jan 2015 12:31:01 +0000 (13:31 +0100)]
Always keep UDP mappings alive for nodes that also have a meta-connection.

This is necessary for assisting with UDP hole punching. But we don't
need to know the PMTU for this, so only send UDP probes.

9 years agoFix segfault when sptps_test cannot open the key files.
Guus Sliepen [Sun, 11 Jan 2015 00:52:37 +0000 (01:52 +0100)]
Fix segfault when sptps_test cannot open the key files.

9 years agoFix typo in logging statement.
Etienne Dechamps [Tue, 30 Dec 2014 09:56:30 +0000 (09:56 +0000)]
Fix typo in logging statement.

This was introduced in cfe9285adf391ab66faeb5def811fe08e47a221a.

9 years agoDon't send probe replies if we don't have the other's key.
Guus Sliepen [Sat, 10 Jan 2015 22:58:35 +0000 (23:58 +0100)]
Don't send probe replies if we don't have the other's key.

This can happen with the legacy protocol. Don't try to send anything
back in this case, otherwise it will be sent via TCP, which is silly.

9 years agoProactively send our own key when we request another node's key.
Guus Sliepen [Sat, 10 Jan 2015 22:52:23 +0000 (23:52 +0100)]
Proactively send our own key when we request another node's key.

9 years agoFix size of type 2 probe replies.
Guus Sliepen [Sat, 10 Jan 2015 22:33:55 +0000 (23:33 +0100)]
Fix size of type 2 probe replies.

Type 2 replies should be as small as possible. The minimum payload size
for probe packets is 14 bytes, otherwise they won't be recognized as
such.

9 years agoCorrectly estimate the initial MTU for legacy packets.
Guus Sliepen [Sat, 10 Jan 2015 22:00:51 +0000 (23:00 +0100)]
Correctly estimate the initial MTU for legacy packets.

9 years agoTry to clarify the new code in net_packet.c a bit.
Guus Sliepen [Sat, 10 Jan 2015 21:28:47 +0000 (22:28 +0100)]
Try to clarify the new code in net_packet.c a bit.

Mainly by trying to reduce complex if statements, by splitting try_tx() into try_tx_legacy() and
try_tx_sptps(), since they don't share a lot of code.

9 years agoRemember whether we sent our key to another node.
Guus Sliepen [Sat, 10 Jan 2015 21:26:33 +0000 (22:26 +0100)]
Remember whether we sent our key to another node.

In tinc 1.0.x, this was tracked in node->inkey, however in tinc 1.1 we have an abstraction layer for
the legacy cipher and digest, and we don't keep an explicit copy of the key around. We cannot use
cipher_active() or digest_active(), since it is possible to set both to the null algorithm. So add a bit to
node_status_t.

9 years agoUse global "now" in try_udp() and try_mtu().
Guus Sliepen [Sun, 4 Jan 2015 15:00:02 +0000 (16:00 +0100)]
Use global "now" in try_udp() and try_mtu().

9 years agoUse void pointers for opaque data blobs in the SHA512 code.
Guus Sliepen [Sun, 4 Jan 2015 13:19:23 +0000 (14:19 +0100)]
Use void pointers for opaque data blobs in the SHA512 code.

9 years agoFix indentation and some whitespace issues.
Guus Sliepen [Sun, 4 Jan 2015 13:15:35 +0000 (14:15 +0100)]
Fix indentation and some whitespace issues.

9 years agoUse a different UDP discovery interval if the tunnel is established.
Etienne Dechamps [Sat, 3 Jan 2015 10:05:57 +0000 (10:05 +0000)]
Use a different UDP discovery interval if the tunnel is established.

This introduces a new configuration option,
UDPDiscoveryKeepaliveInterval, which is used as the UDP discovery
interval once the UDP tunnel is established. The pre-existing option,
UDPDiscoveryInterval, is therefore only used before UDP connectivity
is established.

The defaults are set so that tinc sends UDP pings more aggressively
if the tunnel is not established yet. This is appropriate since the
size of probes in that scenario is very small (16 bytes).

9 years agoRecalculate and resend MTU probes if they are too large for the system.
Etienne Dechamps [Thu, 1 Jan 2015 16:59:45 +0000 (16:59 +0000)]
Recalculate and resend MTU probes if they are too large for the system.

Currently, if a MTU probe is sent and gets rejected by the system
because it is too large (i.e. send() returns EMSGSIZE), the MTU
discovery algorithm is not aware of it and still behaves as if the probe
was actually sent.

This patch makes the MTU discovery algorithm recalculate and send a new
probe when this happens, so that the probe "slot" does not go to waste.

9 years agoFine-tune the MTU discovery multiplier for the maxmtu < MTU case.
Etienne Dechamps [Wed, 31 Dec 2014 16:21:08 +0000 (16:21 +0000)]
Fine-tune the MTU discovery multiplier for the maxmtu < MTU case.

The original multiplier constant for the MTU discovery algorithm, 0.97,
assumes a somewhat pessmistic scenario where we don't get any help from
the OS - i.e. maxmtu never changes. This can happen if IP_MTU is not
usable and the OS doesn't reject overly large packets.

However, in most systems the OS will, in fact, contribute to the MTU
discovery process. In these situations, an actual MTU equal to maxmtu
is quite likely (as opposed to the maxmtu = 1518 case where that is
highly unlikely, unless the physical network supports jumbo frames).
It therefore makes sense to use a multiplier of 1 - that will make the
first probe length equal to maxmtu.

The best results are obtained if the OS supports the getsockopt(IP_MTU)
call, and its result is accurate. In that case, tinc will typically fix
the MTU after one single probe(!), like so:

    Using system-provided maximum tinc MTU for foobar (1.2.3.4 port 655): 1442
    Sending UDP probe length 1442 to foobar (1.2.3.4 port 655)
    Got type 2 UDP probe reply 1442 from foobar (1.2.3.4 port 655)
    Fixing MTU of foobar (1.2.3.4 port 655) to 1442 after 1 probes

9 years agoAdd IP_MTU-based maxmtu estimation.
Etienne Dechamps [Wed, 31 Dec 2014 16:12:11 +0000 (16:12 +0000)]
Add IP_MTU-based maxmtu estimation.

Linux provides a getsockopt() option, IP_MTU, to get the kernel's best
guess at a connection MTU. In practice, it seems to return the MTU of
the physical interface the socket is using.

This patch uses this option to initialize maxmtu to a better value when
MTU discovery starts.

Unfortunately, this is not supported on Windows. Winsock has options
such as SO_MAX_MSG_SIZE, SO_MAXDG and SO_MAXPATHDG but they seem useless
as they always return absurdly large values (typically, 65507), as
confirmed by http://support.microsoft.com/kb/822061/

9 years agoDon't send MTU probes smaller than 512 bytes.
Etienne Dechamps [Wed, 31 Dec 2014 09:26:14 +0000 (09:26 +0000)]
Don't send MTU probes smaller than 512 bytes.

If MTU discovery comes up with an MTU smaller than 512 bytes (e.g. due
to massive packet loss), it's pretty much guaranteed to be wrong. Even
if it's not, most Internet applications assume the MTU will be at least
512, so fixing the MTU to a small value is likely to cause trouble
anyway.

This also makes the discovery algorithm converge even faster, since the
interval it has to consider is smaller.

9 years agoAdjust MTU probe counts.
Etienne Dechamps [Tue, 30 Dec 2014 17:02:38 +0000 (17:02 +0000)]
Adjust MTU probe counts.

The recently introduced new MTU discovery algorithm converges much
faster than the previous one, which allows us to reduce the number
of probes required before we can confidently fix the MTU. This commit
reduces the number of initial discovery probes from 90 to 20. With the
new algorithm this is more than enough to get to the precise (byte-level
accuracy) MTU value; in cases of packet loss or weird MTU values for
which the algorithm is not optimized, we should get close to the actual
value, and then we rely on MTU increase detection (steady state probes)
to fine-tune it later if the need arises.

This patch also triggers MTU increase detection even if the MTU we have
is off by only one byte. Previously we only did that if it was off by at
least 8 bytes. Considering that (1) this should happen less often,
(2) restarting MTU discovery is cheaper than before and (3) having MTUs
that are subtly off from their intended values by just a few bytes
sounds like trouble, this sounds like a good idea.

9 years agoUse a smarter algorithm for choosing MTU discovery probe sizes.
Etienne Dechamps [Tue, 30 Dec 2014 16:34:48 +0000 (16:34 +0000)]
Use a smarter algorithm for choosing MTU discovery probe sizes.

Currently, tinc uses a naive algorithm for choosing MTU discovery probe
sizes, picking a size at random between minmtu and maxmtu.

This is of course suboptimal - since the behavior of probes is
deterministic (assuming no packet loss), it seems likely that using a
non-deterministic discovery algorithm will not yield the best results.
Furthermore, the randomness introduces a lot of variation in convergence
times.

The random solution also suffers from pathological cases - since it's
using a uniform distribution, it doesn't take into account the fact that
it's often more interesting to send small probes rather than large ones,
because getting replies is the only way we can make progress (assuming
the worst case scenario in which the OS doesn't know anything, therefore
keeping maxmtu constant). This can lead to absurd situations where the
discovery algorithm is close to the real MTU, but can't get to it
because the random number generator keeps generating numbers that are
past it.

The algorithm implemented in this patch aims to improve on the naive
random algorithm. It is organized around "cycles" of 8 probes; the sizes
of the probes decrease as we go through the cycle, thus making sure the
algorithm can cover lots of ground quickly (in case we're far from
actual MTU), but also examining the local area (in case we're close to
actual MTU). Using cycles ensures that the algorithm will "go back" to
large probes to better cover the new interval and to protect against
packet loss.

For the probe size itself, various mathematical models were simulated in
an attempt to find the one that converges the fastest; it has been
determined that using an exponential based on the size of the remaining
interval was the most effective option. The exponential is adjusted with
a magic multiplier fine-tuned to make tinc jump to the "most
interesting" (i.e. 1400+) section as soon as discovery starts.

Simulations indicate that assuming no packet loss and no help from the
OS (i.e. maxmtu stays constant), this algorithm will typically converge
to the *exact* MTU value in less than 10 probes, and will get within 8
bytes in less than 5 probes, for actual MTUs between 1417 and ~1450
(which is the range the algorithm is fine-tuned for). In contrast, the
previous algorithm gives results all over the place, sometimes taking
30+ probes to get in the ballpark. Because of the issues with the
distribution, the previous algorithm sometimes never gets to the precise
MTU value within any reasonable amount of time - in contrast, the new
algorithm will always get to the precise value in less than 30 probes,
even if the actual MTU is completely outside the optimized range.

9 years agoRemove bandwidth estimation code.
Etienne Dechamps [Tue, 30 Dec 2014 10:47:56 +0000 (10:47 +0000)]
Remove bandwidth estimation code.

tinc bandwidth estimation has always been quite unreliable (at least in
my experience), but there's no chance of it working anymore since the
last changes to MTU discovery code, because packets are not sent in
batches of three anymore.

This commit removes the dead code - fortunately, nothing depends on this
estimation (it's not even shown in node info). We probably need be
smarter about this if we do want this estimation back.

9 years agoSend one MTU probe at a time.
Etienne Dechamps [Tue, 30 Dec 2014 10:16:32 +0000 (10:16 +0000)]
Send one MTU probe at a time.

Currently, tinc sends MTU probes in batches of three every second. This
commit changes that to send one packet every 333 milliseconds instead.

This change brings two benefits:

 - It makes MTU probing faster, because MTU probe lengths are calculated
   based on minmtu, and minmtu is adjusted based on the replies. When
   sending batches of three packets, all three packets are based on the
   same minmtu estimation; in contrast, by sending one packet more
   frequently, each subsequent packet can benefit from the replies that
   have been received since the last packet was sent. As a result, MTU
   discovery converges much faster (2-3 times as fast, typically).

 - It reduces network spikiness - it's more network-friendly to send
   one packet from time to time as opposed to sending bursts.

9 years agoUse -1 to identify the post-initial MTU discovery state.
Etienne Dechamps [Thu, 1 Jan 2015 16:04:08 +0000 (16:04 +0000)]
Use -1 to identify the post-initial MTU discovery state.

This is a minor cosmetic nit to emphasise the distinction between the
initial MTU discovery phase, and the post-initial phase (i.e. maxmtu
checking).

Furthermore, this is an improvement with regard to the DRY (Don't
Repeat Yourself) principle, as the maximum mtuprobes value is only
written once.

9 years agoFix MTU as soon as possible.
Etienne Dechamps [Thu, 1 Jan 2015 10:32:14 +0000 (10:32 +0000)]
Fix MTU as soon as possible.

If a probe reply is received that makes minmtu equal to maxmtu, we
have to wait until try_mtu() runs to realize that. Since try_mtu()
runs after a packet is sent, this means there is at least one packet
(possibly more, depending on timing) that won't benefit from the
fixed MTU. This also happens when maxmtu is updated from the send()
path.

This commit fixes that by making sure we check whether the MTU can be
fixed every time minmtu or maxmtu is touched.

9 years agoMove try_mtu() closer to try_tx().
Etienne Dechamps [Mon, 29 Dec 2014 17:05:19 +0000 (17:05 +0000)]
Move try_mtu() closer to try_tx().

This moves related functions together, and is a pure cut-and-paste
change. The reason it was not done in the previous commit is because it
would have made the diff harder to review.

9 years agoMove PMTU discovery code into the TX path.
Etienne Dechamps [Mon, 29 Dec 2014 16:47:49 +0000 (16:47 +0000)]
Move PMTU discovery code into the TX path.

Currently, the PMTU discovery code is run by a timeout callback,
independently of tunnel activity. This commit moves it into the TX
path, meaning that send_mtu_probe_handler() is only called if a
packet is about to be sent. Consequently, it has been renamed to
try_mtu() for consistency with try_tx(), try_udp() and try_sptps().

Running PMTU discovery code only as part of the TX path prevents
PMTU discovery from generating unreasonable amounts of traffic when
the "real" traffic is negligible. One extreme example is sending one
real packet and then going silent: in the current code this one little
packet will result in the entire PMTU discovery algorithm being run
from start to finish, resulting in absurd write traffic amplification.
With this patch, PMTU discovery stops as soon as "real" packets stop
flowing, and will be no more aggressive than the underlying traffic.

Furthermore, try_mtu() only runs if there is confirmed UDP
connectivity as per the UDP discovery mechanism. This prevents
unnecessary network chatter - previously, the PMTU discovery code
would send bursts of (potentially large) probe packets every second
even if there was nothing on the other side. With this patch, the
PMTU code only does that if something replied to the lightweight UDP
discovery pings.

These inefficiencies were made even worse when the node is not a
direct neighbour, as tinc will use PMTU discovery both on the
destination node *and* the relay. UDP discovery is more lightweight for
this purpose.

As a bonus, this code simplifies overall code somewhat - state is
easier to manage when code is run in predictable contexts as opposed
to "surprise callbacks". In addition, there is no need to call PMTU
discovery code outside of net_packet.c anymore, thereby simplifying
module boundaries.

9 years agoRemove PMTU discovery code redundant with UDP discovery.
Etienne Dechamps [Mon, 29 Dec 2014 16:11:04 +0000 (16:11 +0000)]
Remove PMTU discovery code redundant with UDP discovery.

This is a rewrite of the send_mtu_probe_handler() function to make it
focus on the actual discovery of PMTU. In particular, the PMTU
discovery code doesn't care about tunnel state anymore - it only cares
about doing the initial PMTU discovery, and once that's done, making
sure PMTU did not increase by checking it from time to time. All other
duties have already been rewritten in the UDP discovery code.

As a result, the send_mtu_probe_handler(), which previously implemented
a nightmarish state machine which was very difficult to follow and
understand, has been massively simplified. We moved from four persistent
states to only two - initial discovery and steady state.

Furthermore, a side effect is that network chatter is reduced: instead
of sending bursts of three minmtu-sized packets in the steady state,
there is only one such packet that's sent from the UDP discovery code.
However, that introduces a slight regression in the bandwidth estimation
code, which relies on three-packet bursts in order to function.
Considering that this estimation is extremely unreliable (in my
experience) and isn't relied on by anything, this seems like an
acceptable regression.