Skip to content

Commit

Permalink
fix: start packet capture immediately on speedtest (#13128)
Browse files Browse the repository at this point in the history
I initially made this change when hacking wgengine to also capture wireguard packets going into the magicsock, so that we could capture the initial wireguard handshake. 

I don't think we should ship that additional capture logic, but... it seems generally useful to capture packets from the get go on speedtest, so that you can see disco and pings before the TCP speedtest session starts.
  • Loading branch information
spikecurtis committed May 2, 2024
1 parent 93d8812 commit 3de737f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
30 changes: 15 additions & 15 deletions cli/speedtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,22 @@ func (r *RootCmd) speedtest() *serpent.Command {
if r.disableDirect {
_, _ = fmt.Fprintln(inv.Stderr, "Direct connections disabled.")
}
opts := &workspacesdk.DialAgentOptions{
Logger: logger,
}
if pcapFile != "" {
s := capture.New()
opts.CaptureHook = s.LogPacket
f, err := os.OpenFile(pcapFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
if err != nil {
return err
}
defer f.Close()
unregister := s.RegisterOutput(f)
defer unregister()
}
conn, err := workspacesdk.New(client).
DialAgent(ctx, workspaceAgent.ID, &workspacesdk.DialAgentOptions{
Logger: logger,
})
DialAgent(ctx, workspaceAgent.ID, opts)
if err != nil {
return err
}
Expand Down Expand Up @@ -102,18 +114,6 @@ func (r *RootCmd) speedtest() *serpent.Command {
conn.AwaitReachable(ctx)
}

if pcapFile != "" {
s := capture.New()
conn.InstallCaptureHook(s.LogPacket)
f, err := os.OpenFile(pcapFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
if err != nil {
return err
}
defer f.Close()
unregister := s.RegisterOutput(f)
defer unregister()
}

var tsDir tsspeedtest.Direction
switch direction {
case "up":
Expand Down
5 changes: 5 additions & 0 deletions codersdk/workspacesdk/workspacesdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"golang.org/x/xerrors"
"nhooyr.io/websocket"
"tailscale.com/tailcfg"
"tailscale.com/wgengine/capture"

"cdr.dev/slog"
"github.com/coder/coder/v2/codersdk"
Expand Down Expand Up @@ -176,6 +177,9 @@ type DialAgentOptions struct {
// BlockEndpoints forced a direct connection through DERP. The Client may
// have DisableDirect set which will override this value.
BlockEndpoints bool
// CaptureHook is a callback that captures Disco packets and packets sent
// into the tailnet tunnel.
CaptureHook capture.Callback
}

func (c *Client) DialAgent(dialCtx context.Context, agentID uuid.UUID, options *DialAgentOptions) (agentConn *AgentConn, err error) {
Expand Down Expand Up @@ -203,6 +207,7 @@ func (c *Client) DialAgent(dialCtx context.Context, agentID uuid.UUID, options *
DERPForceWebSockets: connInfo.DERPForceWebSockets,
Logger: options.Logger,
BlockEndpoints: c.client.DisableDirectConnections || options.BlockEndpoints,
CaptureHook: options.CaptureHook,
})
if err != nil {
return nil, xerrors.Errorf("create tailnet: %w", err)
Expand Down
4 changes: 4 additions & 0 deletions tailnet/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ type Options struct {
BlockEndpoints bool
Logger slog.Logger
ListenPort uint16
// CaptureHook is a callback that captures Disco packets and packets sent
// into the tailnet tunnel.
CaptureHook capture.Callback
}

// NodeID creates a Tailscale NodeID from the last 8 bytes of a UUID. It ensures
Expand Down Expand Up @@ -158,6 +161,7 @@ func NewConn(options *Options) (conn *Conn, err error) {
wireguardEngine.Close()
}
}()
wireguardEngine.InstallCaptureHook(options.CaptureHook)
dialer.UseNetstackForIP = func(ip netip.Addr) bool {
_, ok := wireguardEngine.PeerForIP(ip)
return ok
Expand Down

0 comments on commit 3de737f

Please sign in to comment.