From e1bbe1bf4594358d454bcab703aa155496fcf46f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 24 Sep 2024 15:11:31 -0700 Subject: [PATCH] derp: document the RunWatchConnectionLoop callback gotchas Updates #13566 Change-Id: I497b5adc57f8b1b97dbc3f74c0dc67140caad436 Signed-off-by: Brad Fitzpatrick --- derp/derp_client.go | 13 +++++++++++-- derp/derphttp/mesh_client.go | 4 ++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/derp/derp_client.go b/derp/derp_client.go index dcee68cad..7a646fa51 100644 --- a/derp/derp_client.go +++ b/derp/derp_client.go @@ -356,6 +356,10 @@ func (ReceivedPacket) msg() {} // PeerGoneMessage is a ReceivedMessage that indicates that the client // identified by the underlying public key is not connected to this // server. +// +// It has only historically been sent by the server when the client +// connection count decremented from 1 to 0 and not from e.g. 2 to 1. +// See https://github.com/tailscale/tailscale/issues/13566 for details. type PeerGoneMessage struct { Peer key.NodePublic Reason PeerGoneReasonType @@ -363,8 +367,13 @@ type PeerGoneMessage struct { func (PeerGoneMessage) msg() {} -// PeerPresentMessage is a ReceivedMessage that indicates that the client -// is connected to the server. (Only used by trusted mesh clients) +// PeerPresentMessage is a ReceivedMessage that indicates that the client is +// connected to the server. (Only used by trusted mesh clients) +// +// It will be sent to client watchers for every new connection from a client, +// even if the client's already connected with that public key. +// See https://github.com/tailscale/tailscale/issues/13566 for PeerPresentMessage +// and PeerGoneMessage not being 1:1. type PeerPresentMessage struct { // Key is the public key of the client. Key key.NodePublic diff --git a/derp/derphttp/mesh_client.go b/derp/derphttp/mesh_client.go index c7e48c733..66b8c166e 100644 --- a/derp/derphttp/mesh_client.go +++ b/derp/derphttp/mesh_client.go @@ -26,6 +26,10 @@ var testHookWatchLookConnectResult func(connectError error, wasSelfConnect bool) // returns. // // Otherwise, the add and remove funcs are called as clients come & go. +// Note that add is called for every new connection and remove is only +// called for the final disconnection. See https://github.com/tailscale/tailscale/issues/13566. +// This behavior will likely change. Callers should do their own accounting +// and dup suppression as needed. // // infoLogf, if non-nil, is the logger to write periodic status updates about // how many peers are on the server. Error log output is set to the c's logger,