Packet loss with LocalDiscovery

[Guus Sliepen]

On Sat, Jul 20, 2013 at 03:48:15PM +0100, Etienne Dechamps wrote:

> Good news: I have found the root cause for the bug, and came up with a fix.
> 
> Surprisingly, this is caused by a bug in hash_function(). Because of
> a small mistake in the inner loop, the function will only ever use
> the first 4 bytes of data, and will never look at the remaining
> bytes.

Ah, how stupid of me! Thanks for finding the bug. I've pulled your fix.

> To make things worse, tinc's hash table doesn't care about
> collisions, meaning, two keys with the same hash value will override
> each other. This means that if two nodes happen to use the same port
> number, they can't appear in the node UDP address cache at the same
> time.
[...]
> In summary: when a node is communicating with more than one other
> node simultaneously, the UDP address cache is constantly being
> overridden, try_harder() is called lots of time per second, and as a
> result, throughput is limited to at most one packet per second,
> which is of course completely impractical.

The hash table is intended to serve as a very simple but fast cache for
recently seen addresses. Even if there are conflicts, it would be extremely
rare to get a stream with single packets alternating between two IP addresses,
so there is no code to do chaining or overflow.

If the IP address is not in the hash table, then indeed try_harder() is called,
but that normally still finds the IP address in the list of nodes. The limit of
one packet per second is only invoked when the IP address is not associated
with any node, and when tinc has to check the MAC against known keys.

That said, I think there is still room for improvement.

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://www.tinc-vpn.org/pipermail/tinc-devel/attachments/20130720/018ccd73/attachment.sig>