wgengine: drop userspaceEngine.peerSequence
Another baby step toward removing slices of peers from the engine. getStatus iterated peerSequence (a key snapshot built in Reconfig from cfg.Peers) and then asked wgdev for each peer's stats; peers that weren't active in wgdev silently fell out. Iterate active wgdev peers directly via RemoveMatchingPeers(returnFalse) instead. Updates #12542 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> Change-Id: I3abd348abc30db706db29b3a785179259e48abda
This commit is contained in:
committed by
Brad Fitzpatrick
parent
badd0c4f93
commit
e9ae398199
+64
-18
@@ -1819,12 +1819,9 @@ func testPingPeerLearnedViaDelta(t *testing.T, pt tailcfg.PingType) {
|
||||
|
||||
// Wait for the delta to land in s1's nodeBackend.
|
||||
if err := waitFor(t, ctx, s1, func(nm *netmap.NetworkMap) bool {
|
||||
for _, p := range nm.Peers {
|
||||
if p.Key() == s2Key {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return slices.ContainsFunc(nm.Peers, func(p tailcfg.NodeView) bool {
|
||||
return p.Key() == s2Key
|
||||
})
|
||||
}); err != nil {
|
||||
t.Fatalf("waitFor s2 in s1 netmap: %v", err)
|
||||
}
|
||||
@@ -1915,12 +1912,9 @@ func TestPingSubnetRouteOfDeltaPeer(t *testing.T) {
|
||||
|
||||
// Wait for the delta to land in s1's nodeBackend.
|
||||
if err := waitFor(t, ctx, s1, func(nm *netmap.NetworkMap) bool {
|
||||
for _, p := range nm.Peers {
|
||||
if p.Key() == s2Key {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return slices.ContainsFunc(nm.Peers, func(p tailcfg.NodeView) bool {
|
||||
return p.Key() == s2Key
|
||||
})
|
||||
}); err != nil {
|
||||
t.Fatalf("waitFor s2 in s1 netmap: %v", err)
|
||||
}
|
||||
@@ -1978,6 +1972,61 @@ func TestPingSelfReturnsIsLocalIP(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestStatusReportsPeerInEngine verifies that a peer with an active
|
||||
// wireguard session is reported as InEngine=true in the local
|
||||
// [ipnstate.Status]. This exercises [wgengine.Engine.UpdateStatus] ->
|
||||
// userspaceEngine.getStatus -> the active-wgdev-peer iteration ->
|
||||
// [ipnstate.StatusBuilder.AddPeer] with InEngine=true. It's the only
|
||||
// signal in the tree that exercises getStatus's peer-list path; the
|
||||
// wgengine and ipnlocal unit tests don't assert on it.
|
||||
func TestStatusReportsPeerInEngine(t *testing.T) {
|
||||
tstest.ResourceCheck(t)
|
||||
ctx, cancel := context.WithTimeout(t.Context(), 120*time.Second)
|
||||
defer cancel()
|
||||
|
||||
controlURL, _ := startControl(t)
|
||||
s1, _, _ := startServer(t, ctx, controlURL, "s1")
|
||||
_, s2ip, s2Key := startServer(t, ctx, controlURL, "s2")
|
||||
|
||||
lc1, err := s1.LocalClient()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if err := waitFor(t, ctx, s1, func(nm *netmap.NetworkMap) bool {
|
||||
return slices.ContainsFunc(nm.Peers, func(p tailcfg.NodeView) bool {
|
||||
return p.Key() == s2Key
|
||||
})
|
||||
}); err != nil {
|
||||
t.Fatalf("waitFor s2 in s1 netmap: %v", err)
|
||||
}
|
||||
|
||||
// Ping via ICMP so a real packet flows through wireguard-go and
|
||||
// instantiates s2 in s1's wgdev peer map. PingDisco wouldn't
|
||||
// suffice; it goes directly to magicsock and bypasses wgdev.
|
||||
pingCtx, cancelPing := pingTimeout(ctx)
|
||||
defer cancelPing()
|
||||
pr, err := lc1.Ping(pingCtx, s2ip, tailcfg.PingICMP)
|
||||
if err != nil {
|
||||
t.Fatalf("Ping: %v", err)
|
||||
}
|
||||
if pr.Err != "" {
|
||||
t.Fatalf("Ping s1->s2 failed: %s", pr.Err)
|
||||
}
|
||||
|
||||
status, err := lc1.Status(ctx)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
peer, ok := status.Peer[s2Key]
|
||||
if !ok {
|
||||
t.Fatalf("status.Peer missing s2 (%v); peers=%v", s2Key, status.Peers())
|
||||
}
|
||||
if !peer.InEngine {
|
||||
t.Errorf("peer.InEngine = false, want true (peer=%+v)", peer)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCapturePcap(t *testing.T) {
|
||||
const timeLimit = 120
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeLimit*time.Second)
|
||||
@@ -3238,12 +3287,9 @@ func TestDialUDPInjectedReadRecordsFlowState(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second)
|
||||
defer cancel()
|
||||
if err := waitFor(t, ctx, lt.s2, func(nm *netmap.NetworkMap) bool {
|
||||
for _, p := range nm.Peers {
|
||||
if p.Key() == s1Key && p.IsJailed() {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return slices.ContainsFunc(nm.Peers, func(p tailcfg.NodeView) bool {
|
||||
return p.Key() == s1Key && p.IsJailed()
|
||||
})
|
||||
}); err != nil {
|
||||
t.Fatalf("waiting for s1 to appear jailed in s2's netmap: %v", err)
|
||||
}
|
||||
|
||||
+15
-10
@@ -142,7 +142,6 @@ type userspaceEngine struct {
|
||||
netMap *netmap.NetworkMap // or nil
|
||||
closing bool // Close was called (even if we're still closing)
|
||||
statusCallback StatusCallback
|
||||
peerSequence views.Slice[key.NodePublic]
|
||||
endpoints []tailcfg.Endpoint
|
||||
pendOpen map[flowtrackTuple]*pendingOpenFlow // see pendopen.go
|
||||
|
||||
@@ -829,15 +828,11 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
|
||||
e.tundev.SetWGConfig(cfg)
|
||||
|
||||
peerSet := make(set.Set[key.NodePublic], len(cfg.Peers))
|
||||
|
||||
e.mu.Lock()
|
||||
seq := make([]key.NodePublic, 0, len(cfg.Peers))
|
||||
for _, p := range cfg.Peers {
|
||||
seq = append(seq, p.PublicKey)
|
||||
peerSet.Add(p.PublicKey)
|
||||
}
|
||||
e.peerSequence = views.SliceOf(seq)
|
||||
|
||||
e.mu.Lock()
|
||||
nm := e.netMap
|
||||
e.mu.Unlock()
|
||||
|
||||
@@ -1155,7 +1150,6 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
|
||||
|
||||
e.mu.Lock()
|
||||
closing := e.closing
|
||||
peerKeys := e.peerSequence
|
||||
localAddrs := slices.Clone(e.endpoints)
|
||||
e.mu.Unlock()
|
||||
|
||||
@@ -1163,9 +1157,20 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
|
||||
return nil, ErrEngineClosing
|
||||
}
|
||||
|
||||
peers := make([]ipnstate.PeerStatusLite, 0, peerKeys.Len())
|
||||
for _, key := range peerKeys.All() {
|
||||
if status, ok := e.getPeerStatusLite(key); ok {
|
||||
// Snapshot the set of active wgdev peers. wireguard-go has no
|
||||
// read-only iterator over its peer map; RemoveMatchingPeers with
|
||||
// a callback that always returns false is the cheap equivalent
|
||||
// (the callback can't itself call LookupActivePeer, though, as
|
||||
// RemoveMatchingPeers holds the wireguard device's mutex).
|
||||
var peerKeys []key.NodePublic
|
||||
e.wgdev.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool {
|
||||
peerKeys = append(peerKeys, key.NodePublicFromRaw32(mem.B(pk[:])))
|
||||
return false
|
||||
})
|
||||
|
||||
peers := make([]ipnstate.PeerStatusLite, 0, len(peerKeys))
|
||||
for _, k := range peerKeys {
|
||||
if status, ok := e.getPeerStatusLite(k); ok {
|
||||
peers = append(peers, status)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user