cmd/tailscale, ipn/ipnstate, wgengine/magicsock: update ping output for peer relay (#16515)

Updates the output for "tailscale ping" to indicate if a peer relay was traversed, just like the output for DERP or direct connections.

Fixes tailscale/corp#30034

Signed-off-by: Dylan Bargatze <dylan@tailscale.com>

cmd/tailscale/cli/ping.go

@@ -152,7 +152,9 @@ func runPing(ctx context.Context, args []string) error {
 		}
 		latency := time.Duration(pr.LatencySeconds * float64(time.Second)).Round(time.Millisecond)
 		via := pr.Endpoint
-		if pr.DERPRegionID != 0 {
+		if pr.PeerRelay != "" {
+			via = fmt.Sprintf("peer-relay(%s)", pr.PeerRelay)
+		} else if pr.DERPRegionID != 0 {
 			via = fmt.Sprintf("DERP(%s)", pr.DERPRegionCode)
 		}
 		if via == "" {

ipn/ipnstate/ipnstate.go

@@ -701,10 +701,17 @@ type PingResult struct {
 	Err            string
 	LatencySeconds float64
 
-	// Endpoint is the ip:port if direct UDP was used.
-	// It is not currently set for TSMP pings.
+	// Endpoint is a string of the form "{ip}:{port}" if direct UDP was used. It
+	// is not currently set for TSMP.
 	Endpoint string
 
+	// PeerRelay is a string of the form "{ip}:{port}:vni:{vni}" if a peer
+	// relay was used. It is not currently set for TSMP. Note that this field
+	// is not omitted during JSON encoding if it contains a zero value. This is
+	// done for consistency with the Endpoint field; this structure is exposed
+	// externally via localAPI, so we want to maintain the existing convention.
+	PeerRelay string
+
 	// DERPRegionID is non-zero DERP region ID if DERP was used.
 	// It is not currently set for TSMP pings.
 	DERPRegionID int
@@ -739,6 +746,7 @@ func (pr *PingResult) ToPingResponse(pingType tailcfg.PingType) *tailcfg.PingRes
 		Err:            pr.Err,
 		LatencySeconds: pr.LatencySeconds,
 		Endpoint:       pr.Endpoint,
+		PeerRelay:      pr.PeerRelay,
 		DERPRegionID:   pr.DERPRegionID,
 		DERPRegionCode: pr.DERPRegionCode,
 		PeerAPIPort:    pr.PeerAPIPort,

tailcfg/tailcfg.go

@@ -1854,10 +1854,14 @@ type PingResponse struct {
 	// omitted, Err should contain information as to the cause.
 	LatencySeconds float64 `json:",omitempty"`
 
-	// Endpoint is the ip:port if direct UDP was used.
-	// It is not currently set for TSMP pings.
+	// Endpoint is a string of the form "{ip}:{port}" if direct UDP was used. It
+	// is not currently set for TSMP.
 	Endpoint string `json:",omitempty"`
 
+	// PeerRelay is a string of the form "{ip}:{port}:vni:{vni}" if a peer
+	// relay was used. It is not currently set for TSMP.
+	PeerRelay string `json:",omitempty"`
+
 	// DERPRegionID is non-zero DERP region ID if DERP was used.
 	// It is not currently set for TSMP pings.
 	DERPRegionID int `json:",omitempty"`

wgengine/magicsock/magicsock.go

@@ -1106,10 +1106,11 @@ func (c *Conn) Ping(peer tailcfg.NodeView, res *ipnstate.PingResult, size int, c
 func (c *Conn) populateCLIPingResponseLocked(res *ipnstate.PingResult, latency time.Duration, ep epAddr) {
 	res.LatencySeconds = latency.Seconds()
 	if ep.ap.Addr() != tailcfg.DerpMagicIPAddr {
-		// TODO(jwhited): if ep.vni.isSet() we are using a Tailscale client
-		//  as a UDP relay; update PingResult and its interpretation by
-		//  "tailscale ping" to make this clear.
-		res.Endpoint = ep.String()
+		if ep.vni.isSet() {
+			res.PeerRelay = ep.String()
+		} else {
+			res.Endpoint = ep.String()
+		}
 		return
 	}
 	regionID := int(ep.ap.Port())