p2p/nat: add stun protocol #30882
p2p/nat: add stun protocol #30882
Conversation
p2p/nat/nat_stun.go
Outdated
| } | ||
|
|
||
| func NewSTUN(serverAddr string) STUN { | ||
| if serverAddr == "" { |
There was a problem hiding this comment.
| if serverAddr == "" { | |
| if serverAddr == "default" { |
Imo better if using the default is an explicit opt-in from the user
There was a problem hiding this comment.
Should also verify that serverAddr can be converted into a valid udp4 addr, and error otherwise.
And why not let serverAddr be and *UDPAddr instead?
addr, err := net.ResolveUDPAddr("udp4", serverAddr)
if err != nil{
return nil, err
}
return STUN{ serverAddr: addr}, nilThere was a problem hiding this comment.
yeah, that a good idea, I will use *UDPAddr instead and return err if addr are not valid
There was a problem hiding this comment.
@holiman
for this oneImo better if using the default is an explicit opt-in from the user
you mean use should use like this --nat stun: default
There was a problem hiding this comment.
yeah, something like that. --nat=stun:default without space, probably
1. upgrade to github.com/pion/stun/v2 2. check the server addr
| // "stun:default" uses stun protocol with default stun server | ||
| // "stun:192.168.0.1:1234" uses stun protocol with stun server address 192.168.0.1:1234 |
There was a problem hiding this comment.
Another way to do it would be to mimic pmp
// "stun" uses stun protocol with default stun server
// "stun:192.168.0.1:1234" uses stun protocol with stun server address 192.168.0.1:1234
Don't really know if it's a good idea to have a default. For now, some devops team maintain a stun on a server, but is that a promise for the future? What happens when DO recycles it and someone else is given the ip address?
There was a problem hiding this comment.
I don't like it either. We try to avoid hard-coding network endpoints in the client, and when we do, it should be a list of multiple ones. There are lists of public STUN server endpoints floating around that we could use.
There was a problem hiding this comment.
@fjl I make the stun addr to stun server list. When request to stun server, it's up to three requests. f the response is error, it will request to the next server until a success response or all stun server are requested. The code are inspired in p2p/discover/lookup.go
fixed #30881
This PR introduces STUN (Session Traversal Utilities for NAT) support to the
natpackage. The STUN protocol is useful for determining the public IP address of a NAT gateway and enables applications to work effectively in NAT environments.usage