- Notifications
You must be signed in to change notification settings - Fork2.2k
Description
7328059 for#2751 added a "clientSet" type and let the DERP server accept multiple connections for a public key at a time.910462a locked that experiment in further, after it worked well for several years.
But the PeerPresent/PeerGone "watcher" message predate that and were never updated.
Usingderphttp.Client.RunWatchConnectionLoop, theadd andremove callbacks aren't pair:
The derper server sends an "PeerPresent" frame for all new connections, even if they're connected multiple times. This lets the client track the latest IP address of a node (e.g. as it roams from wifi to LTE and maybe didn't nicely terminate its old TCP connection, so it temporarily has two connections)
But OTOH, the "PeerGone" messages are only sent on the 1->0 transition. So when that example wifi or LTE connection goes away, we don't get notified about it and don't know the remote ip:port of the surviving connection.
I guess the allow-multiple-connections-at-once code never updated the watcher code.
I did#13565 but immediately hated it, as it'd hide subsequent updates.
We probably need to change PeerGone messages to notify about all of them. Rather than add a new connection ID to PeerPresent, maybe we just use the remote IP:port as the connection ID, as that's already sent in PeerPresent.