From 3e1f2d01f7790dc0458a7ff70c405c0c5f5e458c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 3 May 2022 14:16:34 -0700 Subject: [PATCH] ipn/ipnlocal: move Ping method from IPN bus to LocalBackend (HTTP) Change-Id: I61759f1dae8d9d446353db54c8b1e13bfffb3287 Signed-off-by: Brad Fitzpatrick --- client/tailscale/localclient.go | 18 +++++++ cmd/tailscale/cli/ping.go | 91 ++++++++++++++------------------- ipn/backend.go | 22 +++----- ipn/fake_test.go | 7 --- ipn/ipnlocal/local.go | 19 ++++--- ipn/localapi/localapi.go | 32 ++++++++++++ ipn/message.go | 16 ------ 7 files changed, 107 insertions(+), 98 deletions(-) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index e694a0646..33bfe0ad3 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -28,6 +28,7 @@ import ( "time" "go4.org/mem" + "inet.af/netaddr" "tailscale.com/client/tailscale/apitype" "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" @@ -662,6 +663,23 @@ func (lc *LocalClient) ExpandSNIName(ctx context.Context, name string) (fqdn str return "", false } +// Ping sends a ping of the provided type to the provided IP and waits +// for its response. +func (lc *LocalClient) Ping(ctx context.Context, ip netaddr.IP, pingtype tailcfg.PingType) (*ipnstate.PingResult, error) { + v := url.Values{} + v.Set("ip", ip.String()) + v.Set("type", string(pingtype)) + body, err := lc.send(ctx, "POST", "/localapi/v0/ping?"+v.Encode(), 200, nil) + if err != nil { + return nil, fmt.Errorf("error %w: %s", err, body) + } + pr := new(ipnstate.PingResult) + if err := json.Unmarshal(body, pr); err != nil { + return nil, err + } + return pr, nil +} + // tailscaledConnectHint gives a little thing about why tailscaled (or // platform equivalent) is not answering localapi connections. // diff --git a/cmd/tailscale/cli/ping.go b/cmd/tailscale/cli/ping.go index fe0803678..d35f9c0c0 100644 --- a/cmd/tailscale/cli/ping.go +++ b/cmd/tailscale/cli/ping.go @@ -16,7 +16,7 @@ import ( "time" "github.com/peterbourgon/ff/v3/ffcli" - "tailscale.com/ipn" + "inet.af/netaddr" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" ) @@ -87,24 +87,10 @@ func runPing(ctx context.Context, args []string) error { os.Exit(1) } - c, bc, ctx, cancel := connect(ctx) - defer cancel() - if len(args) != 1 || args[0] == "" { return errors.New("usage: ping ") } var ip string - prc := make(chan *ipnstate.PingResult, 1) - bc.SetNotifyCallback(func(n ipn.Notify) { - if n.ErrMessage != nil { - fatalf("Notify.ErrMessage: %v", *n.ErrMessage) - } - if pr := n.PingResult; pr != nil && pr.IP == ip { - prc <- pr - } - }) - pumpErr := make(chan error, 1) - go func() { pumpErr <- pump(ctx, bc, c) }() hostOrIP := args[0] ip, self, err := tailscaleIPFromArg(ctx, hostOrIP) @@ -124,48 +110,47 @@ func runPing(ctx context.Context, args []string) error { anyPong := false for { n++ - bc.Ping(ip, pingType()) - timer := time.NewTimer(pingArgs.timeout) - select { - case <-timer.C: - printf("timeout waiting for ping reply\n") - case err := <-pumpErr: - return err - case pr := <-prc: - timer.Stop() - if pr.Err != "" { - if pr.IsLocalIP { - outln(pr.Err) - return nil - } - return errors.New(pr.Err) - } - latency := time.Duration(pr.LatencySeconds * float64(time.Second)).Round(time.Millisecond) - via := pr.Endpoint - if pr.DERPRegionID != 0 { - via = fmt.Sprintf("DERP(%s)", pr.DERPRegionCode) + ctx, cancel := context.WithTimeout(ctx, pingArgs.timeout) + pr, err := localClient.Ping(ctx, netaddr.MustParseIP(ip), pingType()) + cancel() + if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + printf("ping %q timed out\n", ip) + continue } - if via == "" { - // TODO(bradfitz): populate the rest of ipnstate.PingResult for TSMP queries? - // For now just say which protocol it used. - via = string(pingType()) - } - anyPong = true - extra := "" - if pr.PeerAPIPort != 0 { - extra = fmt.Sprintf(", %d", pr.PeerAPIPort) - } - printf("pong from %s (%s%s) via %v in %v\n", pr.NodeName, pr.NodeIP, extra, via, latency) - if pingArgs.tsmp || pingArgs.icmp { - return nil - } - if pr.Endpoint != "" && pingArgs.untilDirect { + return err + } + if pr.Err != "" { + if pr.IsLocalIP { + outln(pr.Err) return nil } - time.Sleep(time.Second) - case <-ctx.Done(): - return ctx.Err() + return errors.New(pr.Err) + } + latency := time.Duration(pr.LatencySeconds * float64(time.Second)).Round(time.Millisecond) + via := pr.Endpoint + if pr.DERPRegionID != 0 { + via = fmt.Sprintf("DERP(%s)", pr.DERPRegionCode) + } + if via == "" { + // TODO(bradfitz): populate the rest of ipnstate.PingResult for TSMP queries? + // For now just say which protocol it used. + via = string(pingType()) } + anyPong = true + extra := "" + if pr.PeerAPIPort != 0 { + extra = fmt.Sprintf(", %d", pr.PeerAPIPort) + } + printf("pong from %s (%s%s) via %v in %v\n", pr.NodeName, pr.NodeIP, extra, via, latency) + if pingArgs.tsmp || pingArgs.icmp { + return nil + } + if pr.Endpoint != "" && pingArgs.untilDirect { + return nil + } + time.Sleep(time.Second) + if n == pingArgs.num { if !anyPong { return errors.New("no reply") diff --git a/ipn/backend.go b/ipn/backend.go index fd2598c9f..54aa7229d 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -65,14 +65,13 @@ type Notify struct { // For State InUseOtherUser, ErrMessage is not critical and just contains the details. ErrMessage *string - LoginFinished *empty.Message // non-nil when/if the login process succeeded - State *State // if non-nil, the new or current IPN state - Prefs *Prefs // if non-nil, the new or current preferences - NetMap *netmap.NetworkMap // if non-nil, the new or current netmap - Engine *EngineStatus // if non-nil, the new or urrent wireguard stats - BrowseToURL *string // if non-nil, UI should open a browser right now - BackendLogID *string // if non-nil, the public logtail ID used by backend - PingResult *ipnstate.PingResult // if non-nil, a ping response arrived + LoginFinished *empty.Message // non-nil when/if the login process succeeded + State *State // if non-nil, the new or current IPN state + Prefs *Prefs // if non-nil, the new or current preferences + NetMap *netmap.NetworkMap // if non-nil, the new or current netmap + Engine *EngineStatus // if non-nil, the new or urrent wireguard stats + BrowseToURL *string // if non-nil, UI should open a browser right now + BackendLogID *string // if non-nil, the public logtail ID used by backend // FilesWaiting if non-nil means that files are buffered in // the Tailscale daemon and ready for local transfer to the @@ -122,9 +121,6 @@ func (n Notify) String() string { if n.BackendLogID != nil { sb.WriteString("BackendLogID ") } - if n.PingResult != nil { - fmt.Fprintf(&sb, "ping=%v ", *n.PingResult) - } if n.FilesWaiting != nil { sb.WriteString("FilesWaiting ") } @@ -245,8 +241,4 @@ type Backend interface { // counts. Connection events are emitted automatically without // polling. RequestEngineStatus() - // Ping attempts to start connecting to the given IP and sends a Notify - // with its PingResult. If the host is down, there might never - // be a PingResult sent. The cmd/tailscale CLI client adds a timeout. - Ping(ip string, pingType tailcfg.PingType) } diff --git a/ipn/fake_test.go b/ipn/fake_test.go index 0373e6604..05a8cb46c 100644 --- a/ipn/fake_test.go +++ b/ipn/fake_test.go @@ -5,7 +5,6 @@ package ipn import ( - "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" "tailscale.com/types/netmap" ) @@ -99,9 +98,3 @@ func (b *FakeBackend) RequestEngineStatus() { b.notify(Notify{Engine: &EngineStatus{}}) } } - -func (b *FakeBackend) Ping(ip string, pingType tailcfg.PingType) { - if b.notify != nil { - b.notify(Notify{PingResult: &ipnstate.PingResult{}}) - } -} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 38c646a7b..741e4ce90 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1698,15 +1698,20 @@ func (b *LocalBackend) StartLoginInteractive() { } } -func (b *LocalBackend) Ping(ipStr string, pingType tailcfg.PingType) { - ip, err := netaddr.ParseIP(ipStr) - if err != nil { - b.logf("ignoring Ping request to invalid IP %q", ipStr) - return - } +func (b *LocalBackend) Ping(ctx context.Context, ip netaddr.IP, pingType tailcfg.PingType) (*ipnstate.PingResult, error) { + ch := make(chan *ipnstate.PingResult, 1) b.e.Ping(ip, pingType, func(pr *ipnstate.PingResult) { - b.send(ipn.Notify{PingResult: pr}) + select { + case ch <- pr: + default: + } }) + select { + case pr := <-ch: + return pr, nil + case <-ctx.Done(): + return nil, ctx.Err() + } } // parseWgStatusLocked returns an EngineStatus based on s. diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index d3390748d..699758d3e 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -111,6 +111,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.serveLogout(w, r) case "/localapi/v0/prefs": h.servePrefs(w, r) + case "/localapi/v0/ping": + h.servePing(w, r) case "/localapi/v0/check-prefs": h.serveCheckPrefs(w, r) case "/localapi/v0/check-ip-forwarding": @@ -625,6 +627,36 @@ func (h *Handler) serveSetExpirySooner(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "done\n") } +func (h *Handler) servePing(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if r.Method != "POST" { + http.Error(w, "want POST", 400) + return + } + ipStr := r.FormValue("ip") + if ipStr == "" { + http.Error(w, "missing 'ip' parameter", 400) + return + } + ip, err := netaddr.ParseIP(ipStr) + if err != nil { + http.Error(w, "invalid IP", 400) + return + } + pingTypeStr := r.FormValue("type") + if ipStr == "" { + http.Error(w, "missing 'type' parameter", 400) + return + } + res, err := h.b.Ping(ctx, ip, tailcfg.PingType(pingTypeStr)) + if err != nil { + writeErrorJSON(w, err) + return + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(res) +} + func (h *Handler) serveDial(w http.ResponseWriter, r *http.Request) { if r.Method != "POST" { http.Error(w, "POST required", http.StatusMethodNotAllowed) diff --git a/ipn/message.go b/ipn/message.go index e35a2c0af..6149a38b8 100644 --- a/ipn/message.go +++ b/ipn/message.go @@ -51,11 +51,6 @@ type SetPrefsArgs struct { New *Prefs } -type PingArgs struct { - IP string - Type tailcfg.PingType -} - // Command is a command message that is JSON encoded and sent by a // frontend to a backend. type Command struct { @@ -78,7 +73,6 @@ type Command struct { SetPrefs *SetPrefsArgs RequestEngineStatus *NoArgs RequestStatus *NoArgs - Ping *PingArgs } type BackendServer struct { @@ -170,9 +164,6 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error { if c := cmd.RequestEngineStatus; c != nil { bs.b.RequestEngineStatus() return nil - } else if c := cmd.Ping; c != nil { - bs.b.Ping(c.IP, tailcfg.PingType(c.Type)) - return nil } if IsReadonlyContext(ctx) { @@ -311,13 +302,6 @@ func (bc *BackendClient) RequestStatus() { bc.send(Command{AllowVersionSkew: true, RequestStatus: &NoArgs{}}) } -func (bc *BackendClient) Ping(ip string, pingType tailcfg.PingType) { - bc.send(Command{Ping: &PingArgs{ - IP: ip, - Type: pingType, - }}) -} - // MaxMessageSize is the maximum message size, in bytes. const MaxMessageSize = 10 << 20