Skip to content

Conversation

@bradfitz
Copy link
Member

bradfitz added a commit to tailscale/tailscale that referenced this pull request Nov 12, 2025
Depends on tailscale/wireguard-go#47

Updates #17858

Change-Id: I3e38484bfc3e73b29cbe9e53f28f140c2cf85ae1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
@bradfitz bradfitz requested a review from raggi November 17, 2025 19:12
@bradfitz bradfitz marked this pull request as ready for review November 20, 2025 21:32
peer.SendHandshakeInitiation(false)
}

func expiredZeroKeyMaterial(peer *Peer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should rename this "expirePeer" or similar

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I was trying to keep it closer to upstream

device/peer.go Outdated
if slices.Equal(p.state.allowedIPs, allowedIPs) {
return
}
p.device.allowedips.SetPeerPrefixes(p, allowedIPs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the clone and comment below seem sensible, but if that implies a dependence or leads to a future dependence, SetPeerPrefixes doesn't document the same behavior that this method relies on - if we need to preserve the behavior we should document it so future changes are less likely to accidentally change the semnatics.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Copy link
Member

@raggi raggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

device/device.go Outdated
//
// If the peer does not exist and a [PeerLookupFunc] is set (via
// [Device.SetPeerLookupFunc]), then that function is used to create the peer
// before returning it. Peers created via this mechanism exist only their state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only their

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added missing word "until"

Updates tailscale/tailscale#17858

Signed-off-by: Brad Fitzpatrick <brad@danga.com>
return size
}

// LookupPeer looks up a peer by its public key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonblocking API note: Rather than LookupPeer having a side-effect and LookupActivePeer skipping that side-effect, I'd have expected LookupPeer to have no side-effects and LookupOrCreatePeer (or something like that) to have the side-effect.

But if that causes too much churn for other code, I guess this is probably fine too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I'm trying to minimize API differences for now with the rest of wireguard-go & upstream.

But I'd be happy to rename these in the future during upstreaming.

@bradfitz bradfitz merged commit c6fd943 into tailscale Nov 21, 2025
13 checks passed
@shyamsoumya1991-dotcom

This comment was marked as off-topic.

bradfitz added a commit to tailscale/tailscale that referenced this pull request Nov 23, 2025
Depends on tailscale/wireguard-go#47

Updates #17858

Change-Id: I3e38484bfc3e73b29cbe9e53f28f140c2cf85ae1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit to tailscale/tailscale that referenced this pull request Nov 26, 2025
Depends on tailscale/wireguard-go#47

Updates #17858

Change-Id: I3e38484bfc3e73b29cbe9e53f28f140c2cf85ae1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants