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.

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

Better symmetric NAT avoidance. #1342

Merged
merged 2 commits into from
Jun 8, 2015
Merged

Better symmetric NAT avoidance. #1342

merged 2 commits into from
Jun 8, 2015

Conversation

jbenet
Copy link
Member

@jbenet jbenet commented Jun 8, 2015

p2p/protocol/identify: dont double count observers

If the same peer observed the same address twice, it would be double
counted as different observations. This change adds a map to make sure
we're counting each observer once.

This is easily extended to require more than two observations, but i have
not yet encountered NATs for whom this is relevant.

p2p/net/identify: clump addr observers into groups

Different mutliaddrs is not enough. Nodes may share transports. NAT port
mappings will likely only work on the base IP/TCP port pair. We go one step
further, and require different root (IP) addrs. Just in case some NATs
group by IP. In practice, this is what we want: use addresses only if hosts
that are on different parts of the network have seen this address.

If the same peer observed the same address twice, it would be
double counted as different observations. This change adds a map
to make sure we're counting each observer once.

This is easily extended to require more than two observations,
but i have not yet encountered NATs for whom this is relevant.
Different mutliaddrs is not enough. Nodes may share transports.
NAT port mappings will likely only work on the base IP/TCP port
pair. We go one step further, and require different root (IP)
addrs. Just in case some NATs group by IP. In practice, this is
what we want: use addresses only if hosts that are on different
parts of the network have seen this address.
@jbenet jbenet added the status/in-progress In progress label Jun 8, 2015
@jbenet
Copy link
Member Author

jbenet commented Jun 8, 2015

This should help a little bit with #1226

jbenet added a commit that referenced this pull request Jun 8, 2015
Better symmetric NAT avoidance.
@jbenet jbenet merged commit aad700b into master Jun 8, 2015
@jbenet jbenet removed the status/in-progress In progress label Jun 8, 2015
@jbenet jbenet deleted the observe-addr-twice branch June 8, 2015 01:14
@jbenet
Copy link
Member Author

jbenet commented Jun 8, 2015

cc @whyrusleeping for CR. maybe i should've waited

@whyrusleeping
Copy link
Member

This LGTM. Hopefully it helps the netscan issues

@wking wking mentioned this pull request Jun 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants