From 35bee36549470edfd794d5ce7a370de0f8b5d526 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 23 Oct 2022 22:07:25 -0700 Subject: [PATCH] portlist: use win32 calls instead of running netstat process [windows] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out using win32 instead of shelling out to child processes is a bit faster: name old time/op new time/op delta GetListIncremental-4 278ms ± 2% 0ms ± 7% -99.93% (p=0.000 n=8+10) name old alloc/op new alloc/op delta GetListIncremental-4 238kB ± 0% 9kB ± 0% -96.12% (p=0.000 n=10+8) name old allocs/op new allocs/op delta GetListIncremental-4 1.19k ± 0% 0.02k ± 0% -98.49% (p=0.000 n=10+10) Fixes #3876 (sadly) Change-Id: I1195ac5de21a8a8b3cdace5871d263e81aa27e91 Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/depaware.txt | 2 +- portlist/netstat_exec.go | 12 +--- portlist/portlist_windows.go | 117 ++++++++++++++++++++++++++++++----- 3 files changed, 102 insertions(+), 29 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 6dfd22671..88ec65e4d 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -226,7 +226,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/neterror from tailscale.com/net/dns/resolver+ tailscale.com/net/netknob from tailscale.com/net/netns+ tailscale.com/net/netns from tailscale.com/derp/derphttp+ - 💣 tailscale.com/net/netstat from tailscale.com/ipn/ipnserver + 💣 tailscale.com/net/netstat from tailscale.com/ipn/ipnserver+ tailscale.com/net/netutil from tailscale.com/ipn/ipnlocal+ tailscale.com/net/packet from tailscale.com/net/tstun+ tailscale.com/net/ping from tailscale.com/net/netcheck diff --git a/portlist/netstat_exec.go b/portlist/netstat_exec.go index 45014cc41..d308d7a93 100644 --- a/portlist/netstat_exec.go +++ b/portlist/netstat_exec.go @@ -15,22 +15,12 @@ import ( "strings" ) -var osHideWindow func(*exec.Cmd) // non-nil on Windows; see portlist_windows.go - -// hideWindow returns c. On Windows it first sets SysProcAttr.HideWindow. -func hideWindow(c *exec.Cmd) *exec.Cmd { - if osHideWindow != nil { - osHideWindow(c) - } - return c -} - func appendListeningPortsNetstat(base []Port, arg string) ([]Port, error) { exe, err := exec.LookPath("netstat") if err != nil { return nil, fmt.Errorf("netstat: lookup: %v", err) } - output, err := hideWindow(exec.Command(exe, arg)).Output() + output, err := exec.Command(exe, arg).Output() if err != nil { xe, ok := err.(*exec.ExitError) stderr := "" diff --git a/portlist/portlist_windows.go b/portlist/portlist_windows.go index 8218eddc5..0881ff6ab 100644 --- a/portlist/portlist_windows.go +++ b/portlist/portlist_windows.go @@ -5,39 +5,122 @@ package portlist import ( - "os/exec" + "path/filepath" + "strings" "syscall" "time" "golang.org/x/sys/windows" + "tailscale.com/net/netstat" ) // Forking on Windows is insanely expensive, so don't do it too often. const pollInterval = 5 * time.Second -func appendListeningPorts(base []Port) ([]Port, error) { - // TODO(bradfitz): stop shelling out to netstat and use the - // net/netstat package instead. When doing so, be sure to filter - // out all of 127.0.0.0/8 and not just 127.0.0.1. - return appendListeningPortsNetstat(base, "-na") +func init() { + newOSImpl = newWindowsImpl +} + +type famPort struct { + proto string + port uint16 + pid uintptr +} + +type windowsImpl struct { + known map[famPort]*portMeta // inode string => metadata +} + +type portMeta struct { + port Port + keep bool } -func addProcesses(pl []Port) ([]Port, error) { - // OpenCurrentProcessToken instead of GetCurrentProcessToken, - // as GetCurrentProcessToken only works on Windows 8+. - tok, err := windows.OpenCurrentProcessToken() +func newWindowsImpl() osImpl { + return &windowsImpl{ + known: map[famPort]*portMeta{}, + } +} + +func (*windowsImpl) Close() error { return nil } + +func (im *windowsImpl) AppendListeningPorts(base []Port) ([]Port, error) { + // TODO(bradfitz): netstat.Get makes a bunch of garbage. Add an Append-style + // API to that package instead/additionally. + tab, err := netstat.Get() if err != nil { return nil, err } - defer tok.Close() - if !tok.IsElevated() { - return appendListeningPortsNetstat(nil, "-na") + + for _, pm := range im.known { + pm.keep = false } - return appendListeningPortsNetstat(nil, "-nab") + + ret := base + for _, e := range tab.Entries { + if e.State != "LISTEN" { + continue + } + if !e.Local.Addr().IsUnspecified() { + continue + } + fp := famPort{ + proto: "tcp", // TODO(bradfitz): UDP too; add to netstat + port: e.Local.Port(), + pid: uintptr(e.Pid), + } + pm, ok := im.known[fp] + if ok { + pm.keep = true + continue + } + pm = &portMeta{ + keep: true, + port: Port{ + Proto: "tcp", + Port: e.Local.Port(), + Process: procNameOfPid(e.Pid), + }, + } + im.known[fp] = pm + } + + for k, m := range im.known { + if !m.keep { + delete(im.known, k) + continue + } + ret = append(ret, m.port) + } + return sortAndDedup(ret), nil } -func init() { - osHideWindow = func(c *exec.Cmd) { - c.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} +func procNameOfPid(pid int) string { + const da = windows.PROCESS_QUERY_LIMITED_INFORMATION + h, err := syscall.OpenProcess(da, false, uint32(pid)) + if err != nil { + return "" + } + defer syscall.CloseHandle(h) + + var buf [512]uint16 + var size = uint32(len(buf)) + if err := windows.QueryFullProcessImageName(windows.Handle(h), 0, &buf[0], &size); err != nil { + return "" + } + name := filepath.Base(windows.UTF16ToString(buf[:])) + if name == "." { + return "" } + name = strings.TrimSuffix(name, ".exe") + name = strings.TrimSuffix(name, ".EXE") + return name +} + +func appendListeningPorts([]Port) ([]Port, error) { + panic("unused on windows; needed to compile for now") +} + +func addProcesses([]Port) ([]Port, error) { + panic("unused on windows; needed to compile for now") }