Commit Graph

17 Commits (b89c11336514d541ad25de564dcf0561c0b24e58)

Author SHA1 Message Date
Marwan Sulaiman f8f0b981ac portlist: remove async functionality
This PR removes all async functionality from the portlist package
which may be a breaking change for non-tailscale importers. The only
importer within this codebase (LocalBackend) is already using the synchronous
API so no further action needed.

Fixes #8171

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
1 year ago
Marwan Sulaiman 5dd0b02133 portlist: add a synchronous Poll method
This is a follow up on PR #8172 that adds a synchronous Poll method
which allows for the Poller to be used as a zero value without needing
the constructor. The local backend is also changed to use the new API.
A follow up PR will remove the async functionality from the portlist package.

Updates #8171

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
1 year ago
Marwan Sulaiman e32e5c0d0c portlist: add Poller.IncludeLocalhost option
This PR parameterizes receiving loopback updates from the portlist package.
Callers can now include services bound to localhost if they want.
Note that this option is off by default still.

Fixes #8171

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
2 years ago
Will Norris 71029cea2d all: update copyright and license headers
This updates all source files to use a new standard header for copyright
and license declaration.  Notably, copyright no longer includes a date,
and we now use the standard SPDX-License-Identifier header.

This commit was done almost entirely mechanically with perl, and then
some minimal manual fixes.

Updates #6865

Signed-off-by: Will Norris <will@tailscale.com>
2 years ago
Brad Fitzpatrick f81351fdef portlist: fix data race
Maisem spotted the bug. The initial getList call in NewPoller wasn't
making a clone (only the Run loop's getList calls).

Fixes #6314

Change-Id: I8ab8799fcccea8e799140340d0ff88a825bb6ff0
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 575c599410 portlist: add a test that verifies changes are picked up over time
To avoid annoying firewall dialogs on macOS and Windows, only run it
on Linux by default without the flag.

Change-Id: If8486c31d4243ade54b0131f673237c6c9184c08
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 036f70b7b4 portlist: refactor, introduce OS-specific types
Add an osImpl interface that can be stateful and thus more efficient
between calls. It will later be implemented by all OSes but for now
this change only adds a Linux implementation.

Remove Port.inode. It was only used by Linux and moves into its osImpl.

Don't reopen /proc/net/* files on each run. Turns out you can just
keep then open and seek to the beginning and reread and the contents
are fresh.

    name                    old time/op    new time/op    delta
    GetListIncremental-8    7.29ms ± 2%    6.53ms ± 1%  -10.50%  (p=0.000 n=9+9)

    name                   old alloc/op   new alloc/op   delta
    GetListIncremental-8    1.30kB ±13%    0.70kB ± 5%  -46.38%  (p=0.000 n=9+10)

    name                  old allocs/op  new allocs/op  delta
    GetListIncremental-8      33.2 ±11%      18.0 ± 0%  -45.82%  (p=0.000 n=9+10)

Updates #5958

Change-Id: I4be83463cbd23c2e2fa5d0bdf38560004f53401b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 774fa72d32 portlist: add BenchmarkGetListIncremental
In contrast to BenchmarkGetList, this new BenchmarkGetListIncremental
acts like what happens in practice, remembering the previous run and
avoiding work that's already been done previously.

Currently:

    BenchmarkGetList
    BenchmarkGetList-8                           100          11011100 ns/op           68411 B/op       2211 allocs/op
    BenchmarkGetList-8                           100          11443410 ns/op           69073 B/op       2223 allocs/op
    BenchmarkGetList-8                           100          11217311 ns/op           68421 B/op       2197 allocs/op
    BenchmarkGetList-8                           100          11035559 ns/op           68801 B/op       2220 allocs/op
    BenchmarkGetList-8                           100          10921596 ns/op           69226 B/op       2225 allocs/op
    BenchmarkGetListIncremental
    BenchmarkGetListIncremental-8                168           7187217 ns/op            1192 B/op         28 allocs/op
    BenchmarkGetListIncremental-8                172           7004525 ns/op            1194 B/op         28 allocs/op
    BenchmarkGetListIncremental-8                162           7235889 ns/op            1221 B/op         29 allocs/op
    BenchmarkGetListIncremental-8                164           7035671 ns/op            1219 B/op         29 allocs/op
    BenchmarkGetListIncremental-8                174           7095448 ns/op            1114 B/op         27 allocs/op

Updates #5958

Change-Id: I1bd5a4b206df4173e2cb8e8a780429d9daa6ef1d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 46ce80758d portlist: update some internals to use append-style APIs
In prep for reducing garbage, being able to reuse memory.  So far this
doesn't actually reuse much. This is just changing signatures around.

But some improvement in any case:

    bradfitz@tsdev:~/src/tailscale.com$ ~/go/bin/benchstat before after
    name       old time/op    new time/op    delta
    GetList-8    11.8ms ± 9%     9.9ms ± 3%  -15.98%  (p=0.000 n=10+10)

    name       old alloc/op   new alloc/op   delta
    GetList-8    99.5kB ± 2%    91.9kB ± 0%   -7.62%  (p=0.000 n=9+9)

    name       old allocs/op  new allocs/op  delta
    GetList-8     3.05k ± 1%     2.93k ± 0%   -3.83%  (p=0.000 n=8+9)

More later, once parsers can reuse strings from previous parses.

Updates #5958

Change-Id: I76cd5048246dd24d11c4e263d8bb8041747fb2b0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 67597bfc9e portlist: unexport GetList
It's an internal implementation detail, and I plan to refactor it
for performance (garbage) reasons anyway, so start by hiding it.

Updates #5958

Change-Id: I2c0d1f743d3495c5f798d1d8afc364692cd9d290
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago
Brad Fitzpatrick 5c5acadb2a portlist: unexport SameInodes method
Signed-off-by: Brad Fitzpatrick <brad@danga.com>
4 years ago
Josh Bleecher Snyder e8cd7bb66f tstest: simplify goroutine leak tests
Use tb.Cleanup to simplify both the API and the implementation.

One behavior change: When the number of goroutines shrinks, don't log.
I've never found these logs to be useful, and they frequently add noise.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
4 years ago
Denton Gentry 0aed59b691 portlist: add a test for SameInodes
Exercise all cases.

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
4 years ago
Denton Gentry 07e4009e15 portlist: fully exercise lessThan in tests
All cases in lessThan are not reliably exercised by other tests.
This shows up in code coverage metrics as lines in lessThan are
alternately added and removed from coverage.

Add a test case to systematically test all conditions.

Signed-off-by: Denton Gentry <dgentry@tailscale.com>
4 years ago
Avery Pennarun 806de4ac94 portlist: fix "readdirent: no such file or directory" errors on Linux.
This could happen when a process disappeared while we were reading its
file descriptor list.

I was able to replicate the problem by running this in another
terminal:

    while :; do for i in $(seq 10); do
      /bin/true & done >&/dev/null; wait >&/dev/null;
    done

And then running the portlist tests thousands of times.

Fixes #339.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
5 years ago
Brad Fitzpatrick f8d67bb591 portlist: ignore ports bound to localhost
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
5 years ago
Brad Fitzpatrick 120273d7f6 portlist: document, clean up, fix an open fd spike, optimize a bit
I noticed portlist when looking at some profiles and hadn't looked at
the code much before. This is a first pass over it. It allocates a
fair bit. More love remains, but this does a bit:

name       old time/op    new time/op    delta
GetList-8    9.92ms ± 8%    9.64ms ±12%     ~     (p=0.247 n=10+10)

name       old alloc/op   new alloc/op   delta
GetList-8     931kB ± 0%     869kB ± 0%   -6.70%  (p=0.000 n=10+10)

name       old allocs/op  new allocs/op  delta
GetList-8     4.59k ± 0%     3.69k ± 1%  -19.71%  (p=0.000 n=10+10)

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
5 years ago