Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer routes with lowest rtt #235

Closed
wants to merge 2 commits into from
Closed

Prefer routes with lowest rtt #235

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a65a48
Prefer routes with lowest rtt
iczero Feb 26, 2020
2b885a0
Fix bug where less specific subnet may be selected
iczero Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions src/subnet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,17 +264,39 @@ subnet_t *lookup_subnet_ipv4(const ipv4_t *address) {

// Search all subnets for a matching one

int last_rtt = INT_MAX; // current smallest rtt seen

for splay_each(subnet_t, p, &subnet_tree) {
if(!p || p->type != SUBNET_IPV4) {
continue;
}

if(!maskcmp(address, &p->net.ipv4.address, p->net.ipv4.prefixlength)) {
r = p;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice this before, but by removing this line you've subtly changed how tinc handles packets routed to unreachable nodes. Before, lookup_subnet_*() would return a non-NULL result even if the subnet found wasn't reachable. Then tinc would send ICMP_NET_UNREACH packets back. But now you'll return NULL, so then tinc will send back ICMP_NET_UNKNOWN.


if(!p->owner || p->owner->status.reachable) {
if(!p->owner) {
gsliepen marked this conversation as resolved.
Show resolved Hide resolved
// this is a broadcast subnet
r = p;
break;
}

if(p->owner->status.reachable) {
int rtt = INT_MAX - 1; // use INT_MAX - 1 as rtt for nodes not directly reachable
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A node can be directly reachable via TCP only. That still might be faster than via UDP to another node. But tinc doesn't measure TCP RTT unfortunately.


if(p->owner->udp_ping_rtt != -1) {
rtt = p->owner->udp_ping_rtt;
} else if(p->owner == myself) {
// we have this subnet, don't route it somewhere else
r = p;
break;
}

if(rtt < last_rtt) {
r = p;
last_rtt = rtt;
}
}
} else if(r) {
// no more matching subnets
break;
}
}

Expand All @@ -298,17 +320,39 @@ subnet_t *lookup_subnet_ipv6(const ipv6_t *address) {

// Search all subnets for a matching one

int last_rtt = INT_MAX; // current smallest rtt seen

for splay_each(subnet_t, p, &subnet_tree) {
if(!p || p->type != SUBNET_IPV6) {
continue;
}

if(!maskcmp(address, &p->net.ipv6.address, p->net.ipv6.prefixlength)) {
r = p;

if(!p->owner || p->owner->status.reachable) {
if(!p->owner) {
// this is a broadcast subnet
r = p;
break;
}

if(p->owner->status.reachable) {
int rtt = INT_MAX - 1; // use INT_MAX - 1 as rtt for nodes not directly reachable

if(p->owner->udp_ping_rtt != -1) {
rtt = p->owner->udp_ping_rtt;
} else if(p->owner == myself) {
// we have this subnet, don't route it somewhere else
r = p;
break;
}

if(rtt < last_rtt) {
r = p;
last_rtt = rtt;
}
}
} else if(r) {
// no more matching subnets
break;
}
}

Expand Down