-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
device: initial DeviceOptions #12
base: master
Are you sure you want to change the base?
Conversation
All atomic access must be aligned to 64 bits, even on 32-bit platforms. Go promises that the start of allocated structs is aligned to 64 bits. So, place the atomically-accessed things first in the struct so that they benefit from that alignment. As a side bonus, it cleanly separates fields that are accessed by atomic ops, and those that should be accessed under mu. Also adds a test that will fail consistently on 32-bit platforms if the struct ever changes again to violate the rules. This is likely not needed because unaligned access crashes reliably, but this will reliably fail even if tests accidentally pass due to lucky alignment. Signed-Off-By: David Anderson <[email protected]>
Signed-off-by: Avery Pennarun <[email protected]>
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 <[email protected]>
Signed-off-by: Brad Fitzpatrick <[email protected]>
Signed-off-by: Brad Fitzpatrick <[email protected]>
Based on types and config parser from wireguard-windows. Signed-off-by: David Crawshaw <[email protected]>
Signed-off-by: Tyler Kropp <[email protected]>
Signed-off-by: Brad Fitzpatrick <[email protected]>
Signed-off-by: Brad Fitzpatrick <[email protected]>
Signed-off-by: Avery Pennarun <[email protected]>
cc @danderson |
This exposes a lot of internal machinery of WireGuard that isn't meant to be exposed. Are we sure we should be giving this to the user? |
Signed-off-by: David Crawshaw <[email protected]>
Every time a peer handshake completes, we call this function. That lets a GUI immediately notice when the connection has been established. Signed-off-by: Avery Pennarun <[email protected]>
Some of this I believe is justifiable, some less so. The main CreateEndpoint/CreateBind callbacks are very useful to several different kinds of projects, which generally fall under the umbrella of "the OS network stack does not have the features we need." We use it in Tailscale to do platform-independent STUN packet interception and TCP fallback routing. Another project I've considered doing with it is integration into a user-space network stack like DPDK (or something built on gVisor's netstack that I worked on), which would require avoiding the Go net package and OS system calls entirely. The UnexpectedIP callback is useful GUI information, it can be used to inform the user that a trusted peer is misconfigured relative to the host because it's violating the AllowedIPs. You're right about the SkipBindUpdate feature, I think it's a failed experiment of ours. I've put a TODO on it to remove and will do once I'm through my backlog of commits. The HandshakeDone is halfway to something useful. As is, it's a bit of a hack, library users don't really need to know about the state of the WireGuard handshake with a peer. But they could use some more information about the connection with a peer. We are using it right now as a link status signal. I think there's some value in this, and we should evolve this into some kind of general callback along the lines of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the understanding that we're going to remove the excessively internal parts, LGTM.
I feel like this is hitting a tension between two uses of wireguard-go:
- as a drop-in replacement for the kernel module on systems that don't have access to a kernel implementation
- as a building block for larger pieces of software (e.g. tailscale) that need to virtualize the environment around the WireGuard engine (programmatic updating, inserting shims between the engine and the kernel network stack...)
IMO (and with obvious bias, since I work on tailscale), I think it's okay to expose APIs for the latter at the Go level, at least for things that are swapping out layers around the engine, or giving read-only visibility into what the engine is doing (for debugging or UI purposes). That's what we're aiming for here, although there's obviously more work to do.
Okay. Let's hold off on merging this for a bit until we can fully study the use case. I'm very hesitant to introduce extensive internal apis without thinking about network implications. |
8a3c04a
to
28c4d04
Compare
e467e07
to
2a607d1
Compare
c597c63
to
3b3de75
Compare
6b5293b
to
6005c57
Compare
58beb0f
to
54dbe24
Compare
1ae3898
to
4e9e5da
Compare
eba36c5
to
ffb742d
Compare
89a9432
to
b9669b7
Compare
These are some of the hooks we use in wireguard-go for using custom socket objects with unusual routing properties. The general structure can be used in future commits to provider library users more ways to use their WireGuard tunnel.