better nat mapping #549
better nat mapping #549
Conversation
ghost
assigned 
Stebalien
added
the
status/blocked
|
This PR has some pretty nasty functions and could use some multiaddr helpers. |
Stebalien
force-pushed
the
feat/better-natmapping
branch
from
b4b658e
to
5ec9533
Compare
Stebalien
force-pushed
the
feat/better-natmapping
branch
2 times, most recently
from
90576ba
to
a82f420
Compare
Stebalien
removed
the
status/blocked
1. Update to work with libp2p/go-libp2p-nat#14. 2. Avoid observed addrs when our NAT tells us about external addrs. 3. Ignore bad addrs reported by our NAT. Substitute with observed addrs. 4. Fix #428.
Stebalien
force-pushed
the
feat/better-natmapping
branch
from
4d12600
to
8c8f08e
Compare
Stebalien
force-pushed
the
feat/better-natmapping
branch
from
6a69104
to
e96aefc
Compare
These were exceeding their tolerances when run under the race detector on CI.
| // Only bother if we're listening on a | ||
| // unicast/unspecified IP. | ||
| ip := net.IP(maIP.RawValue()) | ||
| if !(ip.IsGlobalUnicast() || ip.IsUnspecified()) { |
There was a problem hiding this comment.
Just to confirm. This doesn't continue if the listen address is public (i.e. we don't need a port mapping). That's because we're presuming in that case there's no NAT device at all, and hence this func would return at line 124? What would happen if the host is listening on multiple network interfaces, of which one is public and one is private (behind a NAT)?
There was a problem hiding this comment.
So... this actually works for more than NATs. It also applies to situations where nodes have public IP addresses but are behind firewalls (I think?).
However, we could check ip.IsUnspecified() || ip.Equals(IP_ON_NAT). However, I'll need to make go-libp2p-nat expose IP_ON_NAT first. I'll mess with that in a followup patch.
| }(proto, port) | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Not relevant to the changes in this PR, but shouldn't we be triggering an "identify push" here if there were changes (maybe not exactly here, but when the mappings are done)?
There was a problem hiding this comment.
Probably (#553). However, we also need to push when:
- Our observed addresses change significantly.
- Our interface addresses change.
etc...
/quicand/wsin NAT mappings #428.(really, I'm just trying to get NAT port mapping working on my network...)