There are very few cases, if any, in which a user only wants one of
these levels, so combine it into a single level.
While we're at it, reduce indirection on the loggers by using an empty
function rather than a nil function pointer. It's not like we have
retpolines anyway, and we were always calling through a function with a
branch prior, so this seems like a net gain.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This commit overhauls wireguard-go's logging.
The primary, motivating change is to use a function instead
of a *log.Logger as the basic unit of logging.
Using functions provides a lot more flexibility for
people to bring their own logging system.
It also introduces logging helper methods on Device.
These reduce line noise at the call site.
They also allow for log functions to be nil;
when nil, instead of generating a log line and throwing it away,
we don't bother generating it at all.
This spares allocation and pointless work.
This is a breaking change, although the fix required
of clients is fairly straightforward.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The declaration of err in
nextByte, err := buffered.ReadByte
shadows the declaration of err in
op, err := buffered.ReadString('\n')
above. As a result, the assignments to err in
err = ipcErrorf(ipc.IpcErrorInvalid, "trailing character in UAPI get: %c", nextByte)
and in
err = device.IpcGetOperation(buffered.Writer)
do not modify the correct err variable.
Found by staticcheck.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Plenty more to go, but a start:
name old time/op new time/op delta
UAPIGet-4 6.37µs ± 2% 5.56µs ± 1% -12.70% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
UAPIGet-4 1.98kB ± 0% 1.22kB ± 0% -38.71% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
UAPIGet-4 42.0 ± 0% 35.0 ± 0% -16.67% (p=0.000 n=10+10)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Unify the handling of unexpected UAPI errors.
The comment that says "should never happen" is incorrect;
this could happen due to I/O errors. Correct it.
Change error message capitalization for consistency.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The goal of this change is to make the structure
of IpcSetOperation easier to follow.
IpcSetOperation contains a small state machine:
It starts by configuring the device,
then shifts to configuring one peer at a time.
Having the code all in one giant method obscured that structure.
Split out the parts into helper functions and encapsulate the peer state.
This makes the overall structure more apparent.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Expand IPCError to contain a wrapped error,
and add a helper to make constructing such errors easier.
Add a defer-based "log on returned error" to IpcSetOperation.
This lets us simplify all of the error return paths.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
bufio is not required.
strings.Builder is cheaper than bytes.Buffer for constructing strings.
io.Writer is more flexible than io.StringWriter,
and just as cheap (when used with io.WriteString).
Run gofmt.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Any io.Reader will do, and there are no performance concerns here.
This is technically backwards incompatible,
but it is very unlikely to break any existing code.
It is compatible with the existing uses in wireguard-{windows,android,apple}
and also will allow us to slightly simplify it if desired.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The sticky socket code stays in the device package for now,
as it reaches deeply into the peer list.
This is the first step in an effort to split some code out of
the very busy device package.
Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
