diff --git a/CubeNet/cubevs/dns_reaper.go b/CubeNet/cubevs/dns_reaper.go index bad5dbe90..22e3f9b6e 100644 --- a/CubeNet/cubevs/dns_reaper.go +++ b/CubeNet/cubevs/dns_reaper.go @@ -23,7 +23,13 @@ func reapDNSState() { reapDNSQueryTrack(now) } -// reapDNSLearnedPolicies scans allow_out_v3 and removes expired DNS-learned entries. +// reapDNSLearnedPolicies removes expired DNS-learned allow_out_v3 entries. +// +// The fast path iterates ifindex_to_mvmmeta so Ready-pool TAPs (metadata +// deleted, allow_out flushed) are skipped without walking every HashOfMaps +// outer key. If metadata cannot be loaded, fall back to iterating allow_out_v3 +// outers so a transient pin/ENOENT failure cannot stall DNS TTL expiry for +// every subsequent tick. func reapDNSLearnedPolicies(now uint64) { allowOut, err := loadPinnedMap(MapNameAllowOutV3) if err != nil { @@ -35,13 +41,46 @@ func reapDNSLearnedPolicies(now uint64) { } defer allowOut.Close() + meta, err := loadPinnedMap(MapNameIfindexToMVMMetadata) + if err != nil { + enqueueEvent(Event{ + Error: err, + Message: "failed to load ifindex_to_mvmmeta map; falling back to allow_out_v3 outer scan", + }) + reapDNSLearnedPoliciesFromAllowOutOuter(allowOut, now) + return + } + defer meta.Close() + var ( - ifindex uint32 - innerMapID uint32 + ifindex uint32 + mvmMeta mvmMetadata + ) + iter := meta.Iterate() + for iter.Next(&ifindex, &mvmMeta) { + if err := reapDNSLearnedPoliciesForIfindex(allowOut, ifindex, now); err != nil { + enqueueEvent(Event{ + Error: err, + Message: fmt.Sprintf("failed to reap DNS-learned policies, ifindex: %d", ifindex), + }) + } + } + if err := iter.Err(); err != nil { + enqueueEvent(Event{ + Error: err, + Message: "failed to iterate ifindex_to_mvmmeta map", + }) + } +} + +func reapDNSLearnedPoliciesFromAllowOutOuter(allowOut *ebpf.Map, now uint64) { + var ( + ifindex uint32 + value uint32 ) iter := allowOut.Iterate() - for iter.Next(&ifindex, &innerMapID) { - if err := reapDNSLearnedPoliciesForInnerMap(innerMapID, now); err != nil { + for iter.Next(&ifindex, &value) { + if err := reapDNSLearnedPoliciesForIfindex(allowOut, ifindex, now); err != nil { enqueueEvent(Event{ Error: err, Message: fmt.Sprintf("failed to reap DNS-learned policies, ifindex: %d", ifindex), @@ -51,20 +90,25 @@ func reapDNSLearnedPolicies(now uint64) { if err := iter.Err(); err != nil { enqueueEvent(Event{ Error: err, - Message: "failed to iterate allow_out_v3 map", + Message: "failed to iterate allow_out_v3 outer map", }) - return } } -// reapDNSLearnedPoliciesForInnerMap deletes expired DNS-learned entries from one allow_out_v3 inner map. -func reapDNSLearnedPoliciesForInnerMap(innerMapID uint32, now uint64) error { - inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) +func reapDNSLearnedPoliciesForIfindex(allowOut *ebpf.Map, ifindex uint32, now uint64) error { + inner, err := acquireInnerMap(allowOut, ifindex, MapNameAllowOutV3, nil) if err != nil { - return fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) + if errors.Is(err, ebpf.ErrKeyNotExist) { + return nil + } + return fmt.Errorf("failed to open allow_out_v3 inner map: %w", err) } - defer inner.Close() + return reapDNSLearnedPoliciesForInner(inner, now) +} +// reapDNSLearnedPoliciesForInner deletes expired DNS-learned entries from one +// allow_out_v3 inner map. +func reapDNSLearnedPoliciesForInner(inner *ebpf.Map, now uint64) error { var ( key lpmKeyV3 value netPolicyValueV3 @@ -84,6 +128,17 @@ func reapDNSLearnedPoliciesForInnerMap(innerMapID uint32, now uint64) error { return nil } +// reapDNSLearnedPoliciesForInnerMap is kept for tests/callers that still pass a +// map ID. Prefer reapDNSLearnedPoliciesForInner with a cached FD. +func reapDNSLearnedPoliciesForInnerMap(innerMapID uint32, now uint64) error { + inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) + if err != nil { + return fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) + } + defer inner.Close() + return reapDNSLearnedPoliciesForInner(inner, now) +} + // reapDNSQueryTrack deletes expired pending DNS queries that never got a response. func reapDNSQueryTrack(now uint64) { queryTrack, err := loadPinnedMap(MapNameDNSQueryTrack) diff --git a/CubeNet/cubevs/dnspolicy.go b/CubeNet/cubevs/dnspolicy.go index 52d1bad50..836339690 100644 --- a/CubeNet/cubevs/dnspolicy.go +++ b/CubeNet/cubevs/dnspolicy.go @@ -123,15 +123,13 @@ func populateDNSAllowInnerMap(inner *ebpf.Map, rules []dnsAllowRule) error { } func flushDNSAllowForIfindex(outerMap *ebpf.Map, ifindex uint32) error { - inner, err := lookupInnerMap(outerMap, ifindex) + inner, err := lookupInnerMap(outerMap, ifindex, MapNameDNSAllowV2) if errors.Is(err, ebpf.ErrKeyNotExist) { return nil } if err != nil { return err } - defer inner.Close() - return flushDNSAllowInnerMap(inner) } @@ -177,18 +175,7 @@ func mergePortsIntoDNSValue(v *dnsAllowValue, src []l7PortEntry) { } func flushDNSAllowInnerMap(inner *ebpf.Map) error { - var oldKey dnsAllowKey - var oldValue dnsAllowValue - iter := inner.Iterate() - for iter.Next(&oldKey, &oldValue) { - if err := inner.Delete(&oldKey); err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { - return fmt.Errorf("dns allow delete failed: %w", err) - } - } - if err := iter.Err(); err != nil { - return fmt.Errorf("dns allow iterate failed: %w", err) - } - return nil + return flushInnerEntries[dnsAllowKey, dnsAllowValue](inner) } // cleanupDNSAllow clears the sandbox DNS allow inner map while keeping it preallocated. @@ -199,15 +186,13 @@ func cleanupDNSAllow(ifindex uint32) error { } defer dnsAllow.Close() - inner, err := lookupInnerMap(dnsAllow, ifindex) + inner, err := lookupInnerMap(dnsAllow, ifindex, MapNameDNSAllowV2) if err != nil { if errors.Is(err, ebpf.ErrKeyNotExist) { return nil } return err } - defer inner.Close() - return flushDNSAllowInnerMap(inner) } @@ -226,16 +211,11 @@ func applyDNSAllow(ifindex uint32, rules []dnsAllowRule, replace bool) error { if len(rules) == 0 { return flushDNSAllowForIfindex(dnsAllow, ifindex) } - if err := ensureDNSAllowInnerMap(dnsAllow, ifindex); err != nil { - return err - } - inner, err := lookupInnerMap(dnsAllow, ifindex) + inner, err := acquireInnerMap(dnsAllow, ifindex, MapNameDNSAllowV2, newInnerDNSAllowMap) if err != nil { return err } - defer inner.Close() - if replace { if err := flushDNSAllowInnerMap(inner); err != nil { return err diff --git a/CubeNet/cubevs/inner_cache.go b/CubeNet/cubevs/inner_cache.go new file mode 100644 index 000000000..9c6531a57 --- /dev/null +++ b/CubeNet/cubevs/inner_cache.go @@ -0,0 +1,111 @@ +package cubevs + +import ( + "errors" + "fmt" + "sync" + + "github.com/cilium/ebpf" +) + +// policyInnerCache keeps open FDs for HashOfMaps inners keyed by outer map name +// and TAP ifindex. Userspace BPF_MAP_LOOKUP_ELEM on HashOfMaps is extremely +// expensive on large hosts; create/apply/reaper paths must reuse cached inners +// instead of looking up the outer map on every call. +// +// FD budget: each live TAP may pin up to one FD per net-policy outer +// (allow_out_v2, deny_out, dns_allow) — about 3 × active-TAP-count on Cubelet, +// plus any Active ifindexes warmed by the DNS reaper. Entries are released only +// on TAP destroy / startup stale-outer GC; there is no size-bounded eviction. +// There is also no BPF-reload generation counter today (no reload path); if +// pinned outers are ever recreated, call clearPolicyInnerCacheForTest-style +// invalidation (or add a generation) before reuse. +type policyInnerKey struct { + mapName string + ifindex uint32 +} + +var policyInnerMaps sync.Map // policyInnerKey -> *ebpf.Map + +// acquireInnerMap returns the inner map for ifindex, creating it when missing +// and newInner is non-nil. The returned map is owned by the process-wide cache; +// callers must not Close it. A nil newInner means "must already exist". +// +// Cubelet completes stale-outer GC synchronously before starting background TAP +// work or serving requests, and its TAP lifecycle prevents concurrent ownership +// of one ifindex. This cache therefore does not add another per-key lock. +func acquireInnerMap(outerMap *ebpf.Map, ifindex uint32, mapName string, + newInner func() (*ebpf.Map, error), +) (*ebpf.Map, error) { + key := policyInnerKey{mapName: mapName, ifindex: ifindex} + if cached, ok := policyInnerMaps.Load(key); ok { + return cached.(*ebpf.Map), nil + } + + var inner *ebpf.Map + err := outerMap.Lookup(&ifindex, &inner) + if err == nil { + actual, loaded := policyInnerMaps.LoadOrStore(key, inner) + if loaded { + _ = inner.Close() + return actual.(*ebpf.Map), nil + } + return inner, nil + } + if !errors.Is(err, ebpf.ErrKeyNotExist) { + return nil, fmt.Errorf("map.Lookup failed: %w, name: %s", err, mapName) + } + if newInner == nil { + return nil, fmt.Errorf("map.Lookup failed: %w, name: %s", ebpf.ErrKeyNotExist, mapName) + } + + created, err := newInner() + if err != nil { + return nil, err + } + if err := outerMap.Put(&ifindex, created); err != nil { + _ = created.Close() + return nil, fmt.Errorf("map.Put failed: %w, name: %s", err, mapName) + } + actual, loaded := policyInnerMaps.LoadOrStore(key, created) + if loaded { + _ = created.Close() + return actual.(*ebpf.Map), nil + } + return created, nil +} + +// releaseCachedInner closes and drops a cached inner FD. +func releaseCachedInner(mapName string, ifindex uint32) { + key := policyInnerKey{mapName: mapName, ifindex: ifindex} + if cached, ok := policyInnerMaps.LoadAndDelete(key); ok { + _ = cached.(*ebpf.Map).Close() + } +} + +// deleteCachedInnerAndOuter removes the outer key and its cached userspace FD. +func deleteCachedInnerAndOuter(outer *ebpf.Map, mapName string, ifindex uint32) error { + key := policyInnerKey{mapName: mapName, ifindex: ifindex} + var toClose *ebpf.Map + if cached, ok := policyInnerMaps.LoadAndDelete(key); ok { + toClose = cached.(*ebpf.Map) + } + err := outer.Delete(&ifindex) + if toClose != nil { + _ = toClose.Close() + } + if err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { + return err + } + return nil +} + +func clearPolicyInnerCacheForTest() { + policyInnerMaps.Range(func(k, v any) bool { + policyInnerMaps.Delete(k) + if m, ok := v.(*ebpf.Map); ok && m != nil { + _ = m.Close() + } + return true + }) +} diff --git a/CubeNet/cubevs/migration.go b/CubeNet/cubevs/migration.go index 1baa5500a..e6eea6f56 100644 --- a/CubeNet/cubevs/migration.go +++ b/CubeNet/cubevs/migration.go @@ -219,11 +219,10 @@ func migrateAllowOutInnerMap(current *ebpf.Map, ifindex uint32, sourceName strin if err := ensureAllowOutV3InnerMap(current, ifindex); err != nil { return err } - destination, err := lookupInnerMap(current, ifindex) + destination, err := lookupInnerMap(current, ifindex, MapNameAllowOutV3) if err != nil { return err } - defer destination.Close() switch info.ValueSize { case legacyAllowOutValueSize: @@ -310,11 +309,10 @@ func migrateDNSAllowInnerMap(current *ebpf.Map, ifindex uint32, sourceName strin if err := ensureDNSAllowInnerMap(current, ifindex); err != nil { return err } - destination, err := lookupInnerMap(current, ifindex) + destination, err := lookupInnerMap(current, ifindex, MapNameDNSAllowV2) if err != nil { return err } - defer destination.Close() if info.ValueSize == legacySize { var key dnsAllowKey diff --git a/CubeNet/cubevs/migration_test.go b/CubeNet/cubevs/migration_test.go index 493b7be10..cdfbd4264 100644 --- a/CubeNet/cubevs/migration_test.go +++ b/CubeNet/cubevs/migration_test.go @@ -127,11 +127,10 @@ func TestMigrateDNSAllowInnerMapFromLegacy(t *testing.T) { t.Fatalf("migrateDNSAllowInnerMap: %v", err) } - dest, err := lookupInnerMap(current, ifindex) + dest, err := lookupInnerMap(current, ifindex, MapNameDNSAllowV2) if err != nil { t.Fatalf("lookupInnerMap: %v", err) } - defer dest.Close() for _, e := range entries { key, want, err := makeDNSAllowRule(e.domain, e.flags) @@ -175,11 +174,10 @@ func TestMigrateDNSAllowInnerMapFromCurrent(t *testing.T) { t.Fatalf("migrateDNSAllowInnerMap: %v", err) } - dest, err := lookupInnerMap(current, ifindex) + dest, err := lookupInnerMap(current, ifindex, MapNameDNSAllowV2) if err != nil { t.Fatalf("lookupInnerMap: %v", err) } - defer dest.Close() var got dnsAllowValue if err := dest.Lookup(&key, &got); err != nil { @@ -311,11 +309,10 @@ func TestMigrateDNSAllowMapOuterWithBpffs(t *testing.T) { } // The new outer must now hold the migrated rules (NameLen+Flags, PortCount=0). - dest, err := lookupInnerMap(newOuter, ifindex) + dest, err := lookupInnerMap(newOuter, ifindex, MapNameDNSAllowV2) if err != nil { t.Fatalf("lookupInnerMap: %v", err) } - defer dest.Close() for _, e := range entries { key, want, err := makeDNSAllowRule(e.domain, e.flags) if err != nil { @@ -581,11 +578,10 @@ func TestMigrateAllowOutMapOuterWithBpffs(t *testing.T) { t.Fatalf("migrateAllowOutMap: %v", err) } - dest, err := lookupInnerMap(newOuter, ifindex) + dest, err := lookupInnerMap(newOuter, ifindex, MapNameAllowOutV3) if err != nil { t.Fatalf("lookupInnerMap: %v", err) } - defer dest.Close() // The L7 entry expands to the default {80/http, 443/https} /48 set. for _, tc := range []struct { diff --git a/CubeNet/cubevs/netpolicy.go b/CubeNet/cubevs/netpolicy.go index fb6099635..5acf9c185 100644 --- a/CubeNet/cubevs/netpolicy.go +++ b/CubeNet/cubevs/netpolicy.go @@ -85,35 +85,15 @@ func ensureAllowOutV3InnerMap(outerMap *ebpf.Map, ifindex uint32) error { } func ensureDenyOutInnerMap(outerMap *ebpf.Map, ifindex uint32) error { - return ensureInnerMapWithFactory(outerMap, ifindex, MapNameDenyOut, newInnerLPMMap) + _, err := acquireInnerMap(outerMap, ifindex, MapNameDenyOut, newInnerLPMMap) + return err } func ensureInnerMapWithFactory(outerMap *ebpf.Map, ifindex uint32, mapName string, newInner func() (*ebpf.Map, error), ) error { - // Check if inner map already exists for this ifindex. - var innerMapID uint32 - err := outerMap.Lookup(&ifindex, &innerMapID) - if err == nil { - // Already present, nothing to do. - return nil - } - if !errors.Is(err, ebpf.ErrKeyNotExist) { - return fmt.Errorf("map.Lookup failed: %w, name: %s", err, mapName) - } - - // Create a new inner LPM trie map and insert it. - inner, err := newInner() - if err != nil { - return err - } - defer inner.Close() - - err = outerMap.Put(&ifindex, inner) - if err != nil { - return fmt.Errorf("map.Put failed: %w, name: %s", err, mapName) - } - return nil + _, err := acquireInnerMap(outerMap, ifindex, mapName, newInner) + return err } // initNetPolicy creates inner LPM trie maps for the given ifindex @@ -148,53 +128,51 @@ func initNetPolicy(ifindex uint32) error { // flushInnerMap removes all entries from the inner LPM trie map // associated with the given ifindex in the outer hash-of-maps. func flushInnerMap(outerMap *ebpf.Map, ifindex uint32) error { - return flushInnerMapWithValue(outerMap, ifindex, new(lpmKey), new(uint32)) + return flushInnerMapWithValue[lpmKey, uint32](outerMap, ifindex, MapNameDenyOut) } func flushAllowOutInnerMap(outerMap *ebpf.Map, ifindex uint32) error { - return flushInnerMapWithValue(outerMap, ifindex, new(lpmKeyV3), new(netPolicyValueV3)) + return flushInnerMapWithValue[lpmKeyV3, netPolicyValueV3](outerMap, ifindex, MapNameAllowOutV3) } -func flushInnerMapWithValue(outerMap *ebpf.Map, ifindex uint32, key, value any) error { - var innerMapID uint32 - err := outerMap.Lookup(&ifindex, &innerMapID) +func flushInnerMapWithValue[K any, V any](outerMap *ebpf.Map, ifindex uint32, mapName string) error { + inner, err := acquireInnerMap(outerMap, ifindex, mapName, nil) if err != nil { if errors.Is(err, ebpf.ErrKeyNotExist) { return nil } - return fmt.Errorf("map.Lookup failed: %w", err) - } - - inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) - if err != nil { - return fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) + return err } - defer inner.Close() + return flushInnerEntries[K, V](inner) +} +func flushInnerEntries[K any, V any](inner *ebpf.Map) error { + // cilium/ebpf iterators (especially LPM trie) can skip remaining + // entries if a key is deleted during Iterate. Collect first, then + // delete: Next overwrites the same key buffer, so append copies K. + var ( + key K + value V + keys []K + ) iter := inner.Iterate() - for iter.Next(key, value) { - if err := inner.Delete(key); err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { - return fmt.Errorf("inner map delete failed: %w", err) - } + for iter.Next(&key, &value) { + keys = append(keys, key) } if err := iter.Err(); err != nil { return fmt.Errorf("inner map iterate failed: %w", err) } + for i := range keys { + if err := inner.Delete(&keys[i]); err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { + return fmt.Errorf("inner map delete failed: %w", err) + } + } return nil } -func lookupInnerMap(outerMap *ebpf.Map, ifindex uint32) (*ebpf.Map, error) { - var innerMapID uint32 - err := outerMap.Lookup(&ifindex, &innerMapID) - if err != nil { - return nil, fmt.Errorf("map.Lookup failed: %w", err) - } - - inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) - if err != nil { - return nil, fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) - } - return inner, nil +// lookupInnerMap returns a cached inner map FD. Callers must not Close it. +func lookupInnerMap(outerMap *ebpf.Map, ifindex uint32, mapName string) (*ebpf.Map, error) { + return acquireInnerMap(outerMap, ifindex, mapName, nil) } // cleanupNetPolicy flushes all entries in the inner LPM trie maps @@ -224,6 +202,10 @@ func cleanupNetPolicy(ifindex uint32) error { // CleanupTAPDevicePolicy removes sandbox-specific CubeVS policy residue for one // TAP ifindex. It does not install reusable-pool defaults and does not touch TAP // metadata; callers compose those steps explicitly. +// +// Ready-pool reuse must keep the HashOfMaps outer keys (and reinstall default +// deny afterwards). Outer keys are deleted only when the TAP netdev itself is +// destroyed — see DeleteTAPDevicePolicyMaps and GCStaleNetPolicyMaps. func CleanupTAPDevicePolicy(ifindex uint32) error { if err := cleanupNetPolicy(ifindex); err != nil { return err @@ -234,6 +216,106 @@ func CleanupTAPDevicePolicy(ifindex uint32) error { return cleanupDNSPolicyFlags(ifindex) } +var netPolicyOuterMaps = []string{ + MapNameAllowOutV3, + MapNameDenyOut, + MapNameDNSAllowV2, +} + +// DeleteTAPDevicePolicyMaps removes HashOfMaps outer entries for a destroyed TAP. +// Call this only after the host netdev is gone; Ready-pool cleanup must not use it. +func DeleteTAPDevicePolicyMaps(ifindex uint32) error { + var errs []error + for _, name := range netPolicyOuterMaps { + outer, err := loadPinnedMap(name) + if err != nil { + // Drop any cached FD even when the pinned outer is unavailable so a + // later reuse of this ifindex cannot see a stale userspace entry. + releaseCachedInner(name, ifindex) + errs = append(errs, err) + continue + } + if err := deleteCachedInnerAndOuter(outer, name, ifindex); err != nil { + errs = append(errs, fmt.Errorf("delete %s[%d]: %w", name, ifindex, err)) + } + _ = outer.Close() + } + return errors.Join(errs...) +} + +// GCStaleNetPolicyMaps deletes HashOfMaps outer keys whose ifindex is not in +// keep. keep must include every live pool TAP (Ready, Cleaning, and Active), not +// only Active sandboxes — Ready TAPs still need deny_out defaults. +// +// stillPresent is optional. When set, each candidate is re-checked immediately +// before delete; if it returns true the key is kept. +// +// onConflict is optional. After a successful delete, stillPresent is checked +// again; if the ifindex is now live, onConflict is invoked so the caller can +// restore default-deny (create raced between the pre-delete check and Delete). +func GCStaleNetPolicyMaps(keep map[uint32]struct{}, stillPresent func(uint32) bool, onConflict func(uint32)) (int, error) { + if keep == nil { + keep = map[uint32]struct{}{} + } + deleted := 0 + conflicted := make(map[uint32]struct{}) + var errs []error + for _, name := range netPolicyOuterMaps { + n, err := gcStaleOuterKeys(name, keep, stillPresent, conflicted) + deleted += n + if err != nil { + errs = append(errs, err) + } + } + if onConflict != nil { + for ifindex := range conflicted { + onConflict(ifindex) + } + } + return deleted, errors.Join(errs...) +} + +func gcStaleOuterKeys(mapName string, keep map[uint32]struct{}, stillPresent func(uint32) bool, conflicted map[uint32]struct{}) (int, error) { + outer, err := loadPinnedMap(mapName) + if err != nil { + return 0, err + } + defer outer.Close() + + var ( + ifindex uint32 + value uint32 + stale []uint32 + ) + iter := outer.Iterate() + for iter.Next(&ifindex, &value) { + if _, ok := keep[ifindex]; !ok { + stale = append(stale, ifindex) + } + } + if err := iter.Err(); err != nil { + return 0, fmt.Errorf("iterate %s failed: %w", mapName, err) + } + + deleted := 0 + var errs []error + for _, ifindex := range stale { + if stillPresent != nil && stillPresent(ifindex) { + continue + } + if err := deleteCachedInnerAndOuter(outer, mapName, ifindex); err != nil { + errs = append(errs, fmt.Errorf("delete stale %s[%d]: %w", mapName, ifindex, err)) + continue + } + deleted++ + // Create may have raced in after the pre-delete stillPresent check. + if stillPresent != nil && stillPresent(ifindex) { + conflicted[ifindex] = struct{}{} + } + } + return deleted, errors.Join(errs...) +} + func cleanupDNSPolicyFlags(ifindex uint32) error { m, err := loadPinnedMap(MapNameIfindexToMVMMetadata) if err != nil { @@ -268,10 +350,11 @@ func InstallTAPDefaultDenyPolicy(ifindex uint32) error { } defer denyOut.Close() - if err := ensureDenyOutInnerMap(denyOut, ifindex); err != nil { + inner, err := acquireInnerMap(denyOut, ifindex, MapNameDenyOut, newInnerLPMMap) + if err != nil { return err } - if err := populateInnerMap(denyOut, ifindex, alwaysDeniedSandboxEntries); err != nil { + if err := populateDenyOutInner(inner, alwaysDeniedSandboxEntries); err != nil { return fmt.Errorf("populate default %s failed: %w", MapNameDenyOut, err) } return nil @@ -993,25 +1076,10 @@ func isValidDNSDomainName(domain string) bool { return true } -// populateInnerMap inserts pre-parsed deny_out entries into the inner LPM trie -// map for the specified ifindex. -func populateInnerMap(outerMap *ebpf.Map, ifindex uint32, entries []denyOutPolicyEntry) error { - var innerMapID uint32 - err := outerMap.Lookup(&ifindex, &innerMapID) - if err != nil { - return fmt.Errorf("map.Lookup failed: %w", err) - } - - inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) - if err != nil { - return fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) - } - defer inner.Close() - +func populateDenyOutInner(inner *ebpf.Map, entries []denyOutPolicyEntry) error { val := uint32(netPolicyValueStatic) for _, entry := range entries { - err = inner.Update(&entry.key, &val, ebpf.UpdateAny) - if err != nil { + if err := inner.Update(&entry.key, &val, ebpf.UpdateAny); err != nil { return fmt.Errorf("inner map update failed: %w, cidr: %s", err, entry.source) } } @@ -1031,18 +1099,14 @@ func populateInnerMap(outerMap *ebpf.Map, ifindex uint32, entries []denyOutPolic // silently drop the static verdict; a later DNS refresh preserves the // static zero expiry (dns_response.h same-key rule). func populateAllowOutInnerMap(outerMap *ebpf.Map, ifindex uint32, entries []allowOutPolicyEntry) error { - var innerMapID uint32 - err := outerMap.Lookup(&ifindex, &innerMapID) + inner, err := acquireInnerMap(outerMap, ifindex, MapNameAllowOutV3, nil) if err != nil { - return fmt.Errorf("map.Lookup failed: %w", err) - } - - inner, err := ebpf.NewMapFromID(ebpf.MapID(innerMapID)) - if err != nil { - return fmt.Errorf("ebpf.NewMapFromID failed: %w, id: %d", err, innerMapID) + return err } - defer inner.Close() + return populateAllowOutInner(inner, entries) +} +func populateAllowOutInner(inner *ebpf.Map, entries []allowOutPolicyEntry) error { for _, entry := range entries { if entry.flags&netPolicyFlagL7Required != 0 { ports := entry.ports @@ -1145,16 +1209,16 @@ func applyNetPolicyWithMode(ifindex uint32, opts MVMOptions, replace bool) error } defer allowOutMap.Close() - if err := ensureAllowOutV3InnerMap(allowOutMap, ifindex); err != nil { + inner, err := acquireInnerMap(allowOutMap, ifindex, MapNameAllowOutV3, newInnerAllowOutMap) + if err != nil { return err } if replace { - if err := flushAllowOutInnerMap(allowOutMap, ifindex); err != nil { + if err := flushInnerEntries[lpmKeyV3, netPolicyValueV3](inner); err != nil { return fmt.Errorf("flush %s failed: %w", MapNameAllowOutV3, err) } } - err = populateAllowOutInnerMap(allowOutMap, ifindex, plan.allowOutEntries) - if err != nil { + if err := populateAllowOutInner(inner, plan.allowOutEntries); err != nil { return fmt.Errorf("populate %s failed: %w", MapNameAllowOutV3, err) } } @@ -1173,16 +1237,16 @@ func applyNetPolicyWithMode(ifindex uint32, opts MVMOptions, replace bool) error } defer denyOutMap.Close() - if err := ensureDenyOutInnerMap(denyOutMap, ifindex); err != nil { + inner, err := acquireInnerMap(denyOutMap, ifindex, MapNameDenyOut, newInnerLPMMap) + if err != nil { return err } if replace { - if err := flushInnerMap(denyOutMap, ifindex); err != nil { + if err := flushInnerEntries[lpmKey, uint32](inner); err != nil { return fmt.Errorf("flush %s failed: %w", MapNameDenyOut, err) } } - err = populateInnerMap(denyOutMap, ifindex, denyOutEntries) - if err != nil { + if err := populateDenyOutInner(inner, denyOutEntries); err != nil { return fmt.Errorf("populate %s failed: %w", MapNameDenyOut, err) } } diff --git a/Cubelet/network/runtime/controller.go b/Cubelet/network/runtime/controller.go index e9cdcc348..5670728f0 100644 --- a/Cubelet/network/runtime/controller.go +++ b/Cubelet/network/runtime/controller.go @@ -214,7 +214,6 @@ func newProductionControllerDeps(cfg Config) (networkControllerDeps, error) { if err != nil { return networkControllerDeps{}, err } - startCubeVSSessionLogDrain() return networkControllerDeps{ store: store, allocator: allocator, @@ -387,6 +386,12 @@ func (s *NetworkController) startControllerRuntime() error { if err := s.recover(); err != nil { return err } + // Stale HashOfMaps cleanup is part of startup reconciliation. Complete it + // before starting the reaper, background pool creation, or returning the + // controller to request-serving code so it cannot race a new TAP lifecycle. + s.runStaleNetPolicyMapGC() + startCubeVSSessionLogDrain() + // Pool warmup runs in the background so first-deploy startup // (~63ms × TapInitNum) does not block NewNetworkController and trip // systemd's ExecStartPost timeout. EnsureNetwork transparently diff --git a/Cubelet/network/runtime/cubevs_adapter.go b/Cubelet/network/runtime/cubevs_adapter.go index 8b69c1385..8bbee2d92 100644 --- a/Cubelet/network/runtime/cubevs_adapter.go +++ b/Cubelet/network/runtime/cubevs_adapter.go @@ -26,6 +26,7 @@ type CubeVSAdapter interface { DeleteTAPDeviceMetadata(ifindex uint32, ip net.IP) error AttachFilter(ifindex uint32) error InstallTAPDefaultDenyPolicy(ifindex uint32) error + GCStaleNetPolicyMaps(keep map[uint32]struct{}, stillPresent func(uint32) bool, onConflict func(uint32)) (int, error) AddPortMapping(ifindex uint32, containerPort, hostPort uint16) error DelPortMapping(ifindex uint32, containerPort, hostPort uint16) error DeletePortMappingsByIfindex(ifindex uint32) error @@ -71,6 +72,10 @@ func (realCubeVSAdapter) InstallTAPDefaultDenyPolicy(ifindex uint32) error { return cubevs.InstallTAPDefaultDenyPolicy(ifindex) } +func (realCubeVSAdapter) GCStaleNetPolicyMaps(keep map[uint32]struct{}, stillPresent func(uint32) bool, onConflict func(uint32)) (int, error) { + return cubevs.GCStaleNetPolicyMaps(keep, stillPresent, onConflict) +} + func (realCubeVSAdapter) AddPortMapping(ifindex uint32, containerPort, hostPort uint16) error { return cubevs.AddPortMapping(ifindex, containerPort, hostPort) } diff --git a/Cubelet/network/runtime/cubevs_adapter_test.go b/Cubelet/network/runtime/cubevs_adapter_test.go index a00ebb95b..650ccb4c8 100644 --- a/Cubelet/network/runtime/cubevs_adapter_test.go +++ b/Cubelet/network/runtime/cubevs_adapter_test.go @@ -88,6 +88,10 @@ func (f *fakeCubeVSAdapter) InstallTAPDefaultDenyPolicy(ifindex uint32) error { return nil } +func (f *fakeCubeVSAdapter) GCStaleNetPolicyMaps(_ map[uint32]struct{}, _ func(uint32) bool, _ func(uint32)) (int, error) { + return 0, nil +} + func (f *fakeCubeVSAdapter) AddPortMapping(ifindex uint32, containerPort, hostPort uint16) error { if f.recorder != nil { f.recorder.record("cubevs_port_mapping") diff --git a/Cubelet/network/runtime/startup_recover.go b/Cubelet/network/runtime/startup_recover.go index 2091a1b9a..ac0431c9d 100644 --- a/Cubelet/network/runtime/startup_recover.go +++ b/Cubelet/network/runtime/startup_recover.go @@ -71,6 +71,101 @@ func (s *NetworkController) recover() error { return nil } +// runStaleNetPolicyMapGC deletes allow_out_v2 / deny_out / dns_allow outer keys +// for ifindexes that are not in the live TAP set / pool. Normal TAP teardown +// removes these keys, so the healthy startup path only scans the three outer +// maps and deletes nothing. Cleanup can be slow when failed/bypassed teardown +// has left thousands of stale inners; that exceptional recovery work runs +// synchronously during controller startup. +// +// startControllerRuntime invokes this after recover has registered every live +// TAP, but before the DNS reaper, background pool warmup, and request handling +// can create or use another TAP lifecycle. The stillPresent/onConflict callbacks +// remain as defensive checks for host-netdev changes outside this controller. +func (s *NetworkController) runStaleNetPolicyMapGC() { + logger := CubeLog.WithContext(context.Background()) + keep, err := s.buildStaleNetPolicyKeepSet() + if err != nil { + logger.Warnf( + "network runtime stale policy map gc: list taps failed, skip gc: %v", + err, + ) + return + } + deleted, err := s.cubevsAdapter.GCStaleNetPolicyMaps( + keep, + netPolicyIfindexStillPresent, + s.restoreDefaultDenyAfterStaleGCConflict, + ) + if err != nil { + logger.Warnf( + "network runtime stale policy map gc: keep=%d deleted=%d err=%v", + len(keep), deleted, err, + ) + return + } + logger.Infof( + "network runtime stale policy map gc: keep=%d deleted=%d", + len(keep), deleted, + ) +} + +// restoreDefaultDenyAfterStaleGCConflict reinstalls reusable-pool default deny +// when GC deleted outer keys for an ifindex that became live again (create +// raced between stillPresent and outer.Delete). +func (s *NetworkController) restoreDefaultDenyAfterStaleGCConflict(ifindex uint32) { + if err := s.cubevsAdapter.InstallTAPDefaultDenyPolicy(ifindex); err != nil { + CubeLog.WithContext(context.Background()).Warnf( + "network runtime stale policy map gc: restore default deny failed: ifindex=%d err=%v", + ifindex, err, + ) + return + } + CubeLog.WithContext(context.Background()).Warnf( + "network runtime stale policy map gc: restored default deny after create race: ifindex=%d", + ifindex, + ) +} + +// buildStaleNetPolicyKeepSet returns ifindexes that must not have their +// HashOfMaps outer policy keys deleted: every live Cube TAP plus every pool +// entry (Ready/Cleaning/Active). Listing live TAPs is required; an incomplete +// keep set would let GC treat live ifindexes as stale. +func (s *NetworkController) buildStaleNetPolicyKeepSet() (map[uint32]struct{}, error) { + keep := make(map[uint32]struct{}) + taps, err := s.tapAdapter.List() + if err != nil { + return nil, err + } + for _, tap := range taps { + if tap != nil && tap.Index > 0 { + keep[uint32(tap.Index)] = struct{}{} + } + } + if s.tapPool != nil { + for _, entry := range s.tapPool.Entries() { + if entry != nil && entry.TapIfIndex > 0 { + keep[uint32(entry.TapIfIndex)] = struct{}{} + } + } + } + return keep, nil +} + +// netPolicyIfindexStillPresent reports whether a host netdev still occupies +// ifindex. Transient netlink errors are treated as present so GC prefers a +// temporary leak over wiping deny_out for a live TAP. +func netPolicyIfindexStillPresent(ifindex uint32) bool { + _, err := netlinkLinkByIndex(int(ifindex)) + if err == nil { + return true + } + if isTapNotFound(err) { + return false + } + return true +} + func (s *NetworkController) indexRecoverableStates(records []*StateRecord) (map[string]*StateRecord, map[string]struct{}, error) { statesByTapNameOrIP := make(map[string]*StateRecord, len(records)*2) statesBySandboxID := make(map[string]*StateRecord, len(records)) @@ -675,19 +770,25 @@ func (s *NetworkController) claimRecoveredSuccessResources(state *managedState) } // cleanupConflictingTap destroys a stale host tap that collides with a freshly -// allocated IP. It must be called without holding s.mu: the netlink list and the -// destroy syscall run lock-free, while the membership checks against the +// allocated IP. It must be called without holding s.mu: the netlink lookup and +// the destroy syscall run lock-free, while the membership checks against the // in-memory collections are performed under s.mu. The IP is already exclusively // owned by the caller (handed out by the allocator) and tap names derive // uniquely from the IP, so no other goroutine can re-reference this tap between // the check and the destroy. +// +// Lookup is by deterministic tap name (RTM_GETLINK), not LinkList. A full dump +// of every host interface on every pool-miss create races with concurrent TAP +// churn and surfaces as netlink ErrDumpInterrupted under density load. func (s *NetworkController) cleanupConflictingTap(ip net.IP) error { - taps, err := s.tapAdapter.List() + tap, err := s.tapAdapter.GetByName(tapName(ip.String())) if err != nil { + if isTapNotFound(err) { + return nil + } return err } - tap, ok := taps[ip.String()] - if !ok { + if tap == nil { return nil } if err := s.checkTapConflict(tap, ip); err != nil { diff --git a/Cubelet/network/runtime/startup_recover_gc_test.go b/Cubelet/network/runtime/startup_recover_gc_test.go new file mode 100644 index 000000000..9654eb3c8 --- /dev/null +++ b/Cubelet/network/runtime/startup_recover_gc_test.go @@ -0,0 +1,117 @@ +// Copyright (c) 2024 Tencent Inc. +// SPDX-License-Identifier: Apache-2.0 +// + +package runtime + +import ( + "errors" + "net" + "testing" + + "github.com/vishvananda/netlink" +) + +type gcRecordingCubeVSAdapter struct { + fakeCubeVSAdapter + keep map[uint32]struct{} + stillPresent func(uint32) bool + onConflict func(uint32) +} + +func (f *gcRecordingCubeVSAdapter) GCStaleNetPolicyMaps(keep map[uint32]struct{}, stillPresent func(uint32) bool, onConflict func(uint32)) (int, error) { + f.keep = keep + f.stillPresent = stillPresent + f.onConflict = onConflict + return len(keep), nil +} + +func TestRunStaleNetPolicyMapGCKeepsLiveAndPoolIfindices(t *testing.T) { + adapter := &gcRecordingCubeVSAdapter{} + pool, err := NewTapPool() + if err != nil { + t.Fatal(err) + } + ctrl := &NetworkController{ + cubevsAdapter: adapter, + tapAdapter: &fakeTapDeviceAdapter{listResult: map[string]*tapDevice{ + "z192.168.0.1": {Name: "z192.168.0.1", Index: 101, IP: net.ParseIP("192.168.0.1")}, + "z192.168.0.2": {Name: "z192.168.0.2", Index: 102, IP: net.ParseIP("192.168.0.2")}, + }}, + tapPool: pool, + } + entry, err := NewReadyTapPoolEntry("z192.168.0.3", 103, net.ParseIP("192.168.0.3")) + if err != nil { + t.Fatal(err) + } + if err := ctrl.tapPool.Add(entry); err != nil { + t.Fatal(err) + } + + ctrl.runStaleNetPolicyMapGC() + + for _, want := range []uint32{101, 102, 103} { + if _, ok := adapter.keep[want]; !ok { + t.Fatalf("keep missing ifindex %d: %#v", want, adapter.keep) + } + } + if adapter.stillPresent == nil { + t.Fatal("stillPresent callback was not passed to GC") + } + if adapter.onConflict == nil { + t.Fatal("onConflict callback was not passed to GC") + } +} + +func TestRunStaleNetPolicyMapGCSkipsWhenListTapsFails(t *testing.T) { + adapter := &gcRecordingCubeVSAdapter{} + ctrl := &NetworkController{ + cubevsAdapter: adapter, + tapAdapter: &fakeTapDeviceAdapter{ + listErr: errors.New("dump interrupted"), + listResult: map[string]*tapDevice{ + "z192.168.0.1": {Name: "z192.168.0.1", Index: 101, IP: net.ParseIP("192.168.0.1")}, + }, + }, + } + + ctrl.runStaleNetPolicyMapGC() + + if adapter.keep != nil { + t.Fatalf("GC ran with incomplete keep %#v; List failure must skip GC", adapter.keep) + } +} + +func TestNetPolicyIfindexStillPresent(t *testing.T) { + orig := netlinkLinkByIndex + t.Cleanup(func() { netlinkLinkByIndex = orig }) + + netlinkLinkByIndex = func(index int) (netlink.Link, error) { + if index == 1 { + return &netlink.Tuntap{}, nil + } + if index == 2 { + return nil, netlink.LinkNotFoundError{} + } + return nil, errors.New("transient dump error") + } + + if !netPolicyIfindexStillPresent(1) { + t.Fatal("live link should be present") + } + if netPolicyIfindexStillPresent(2) { + t.Fatal("LinkNotFound should mean absent") + } + if !netPolicyIfindexStillPresent(3) { + t.Fatal("transient error should be treated as present") + } +} + +func TestRestoreDefaultDenyAfterStaleGCConflict(t *testing.T) { + adapter := &fakeCubeVSAdapter{} + ctrl := &NetworkController{cubevsAdapter: adapter} + ctrl.restoreDefaultDenyAfterStaleGCConflict(42) + if got := len(adapter.defaultDenyPolicyCalls); got != 1 || adapter.defaultDenyPolicyCalls[0] != 42 { + t.Fatalf("defaultDenyPolicyCalls=%v, want [42]", adapter.defaultDenyPolicyCalls) + } +} diff --git a/Cubelet/network/runtime/state_store.go b/Cubelet/network/runtime/state_store.go index 4f5d12c4b..c1274ccae 100644 --- a/Cubelet/network/runtime/state_store.go +++ b/Cubelet/network/runtime/state_store.go @@ -167,46 +167,12 @@ func newStateStore(dir string) (*stateStore, error) { return nil, err } s := &stateStore{dir: dir, noSync: isTmpfs(dir)} - if err := s.migrateFlatStateFiles(); err != nil { - return nil, err - } if err := s.normalizeFileModes(); err != nil { return nil, err } return s, nil } -// migrateFlatStateFiles moves pre-shard-layout state files from the state dir -// root into their hash shards. It runs once at startup and is a no-op for new -// installs; files written by a downgraded binary are picked up on the next -// startup. -func (s *stateStore) migrateFlatStateFiles() error { - entries, err := os.ReadDir(s.dir) - if err != nil { - return err - } - for _, entry := range entries { - if entry.IsDir() { - continue - } - sandboxID, kind, ok := parseStateFileName(entry.Name()) - if !ok { - continue - } - dst, err := s.path(sandboxID, kind) - if err != nil { - return err - } - if err := os.MkdirAll(filepath.Dir(dst), 0o700); err != nil { - return err - } - if err := os.Rename(filepath.Join(s.dir, entry.Name()), dst); err != nil { - return err - } - } - return nil -} - // normalizeFileModes enforces private permissions on every state file across // the shard dirs. func (s *stateStore) normalizeFileModes() error { @@ -355,10 +321,10 @@ func (s *stateStore) LoadAny(sandboxID string) (*StateRecord, error) { return nil, os.ErrNotExist } -// Scan returns every valid state file in deterministic order: root-level -// (pre-shard or downgrade-written) files first, then each shard in name order. -// Invalid names are ignored so unrelated files in the state directory do not -// break runtime startup. +// Scan returns every valid state file under hash shards in deterministic +// shard-name then filename order. Invalid names are ignored so unrelated files +// in the state directory do not break runtime startup. Flat (unsharded) layout +// was never released, so root-level "*.json" state files are not scanned. func (s *stateStore) Scan() ([]*StateRecord, error) { rootEntries, err := os.ReadDir(s.dir) if err != nil { @@ -368,11 +334,6 @@ func (s *stateStore) Scan() ([]*StateRecord, error) { return rootEntries[i].Name() < rootEntries[j].Name() }) var records []*StateRecord - flat, err := s.scanDir(s.dir, rootEntries) - if err != nil { - return nil, err - } - records = append(records, flat...) for _, entry := range rootEntries { if !entry.IsDir() || !isShardName(entry.Name()) { continue @@ -391,8 +352,8 @@ func (s *stateStore) Scan() ([]*StateRecord, error) { return records, nil } -// scanDir reads valid state files from one directory (state root or a single -// shard) in deterministic filename order. +// scanDir reads valid state files from one shard directory in deterministic +// filename order. func (s *stateStore) scanDir(dir string, entries []os.DirEntry) ([]*StateRecord, error) { sort.Slice(entries, func(i, j int) bool { return entries[i].Name() < entries[j].Name() diff --git a/Cubelet/network/runtime/state_store_test.go b/Cubelet/network/runtime/state_store_test.go index 165cc85f6..f859c8d0b 100644 --- a/Cubelet/network/runtime/state_store_test.go +++ b/Cubelet/network/runtime/state_store_test.go @@ -16,8 +16,11 @@ func TestStateStoreUsesPrivatePermissions(t *testing.T) { if err := os.Mkdir(stateDir, 0o755); err != nil { t.Fatal(err) } - oldStatePath := filepath.Join(stateDir, "old.success.json") - if err := os.WriteFile(oldStatePath, []byte("{}"), 0o644); err != nil { + shardPath := filepath.Join(stateDir, shardOf("old"), "old.success.json") + if err := os.MkdirAll(filepath.Dir(shardPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(shardPath, []byte("{}"), 0o644); err != nil { t.Fatal(err) } @@ -26,15 +29,7 @@ func TestStateStoreUsesPrivatePermissions(t *testing.T) { t.Fatal(err) } assertFileMode(t, stateDir, 0o700) - // The flat legacy-layout file is migrated into its shard on startup. - migratedPath, err := store.path("old", StateFileSuccess) - if err != nil { - t.Fatal(err) - } - assertFileMode(t, migratedPath, 0o600) - if _, err := os.Stat(oldStatePath); !os.IsNotExist(err) { - t.Fatalf("flat legacy state file should be migrated into shard, stat err=%v", err) - } + assertFileMode(t, shardPath, 0o600) state := testPersistedState("sandbox1") if err := store.WriteTmp(state); err != nil { @@ -47,44 +42,6 @@ func TestStateStoreUsesPrivatePermissions(t *testing.T) { assertFileMode(t, tmpPath, 0o600) } -func TestStateStoreMigratesFlatLayoutOnStartup(t *testing.T) { - stateDir := t.TempDir() - flat := testPersistedState("flat-sandbox") - data, err := flat.MarshalJSON() - if err != nil { - t.Fatal(err) - } - flatPath := filepath.Join(stateDir, "flat-sandbox.creating.json") - if err := os.WriteFile(flatPath, data, 0o600); err != nil { - t.Fatal(err) - } - - store, err := newStateStore(stateDir) - if err != nil { - t.Fatal(err) - } - if _, err := os.Stat(flatPath); !os.IsNotExist(err) { - t.Fatalf("flat file should be migrated, stat err=%v", err) - } - record, err := store.LoadAny("flat-sandbox") - if err != nil { - t.Fatal(err) - } - if record.Kind != StateFileCreating || record.State.SandboxID != "flat-sandbox" { - t.Fatalf("record = %#v", record) - } - records, err := store.Scan() - if err != nil { - t.Fatal(err) - } - if len(records) != 1 || records[0].State.SandboxID != "flat-sandbox" { - t.Fatalf("scan records = %#v", records) - } - if filepath.Dir(records[0].Path) != filepath.Join(stateDir, shardOf("flat-sandbox")) { - t.Fatalf("record not in shard dir: %s", records[0].Path) - } -} - func TestStateStoreLifecycleRenames(t *testing.T) { store, err := newStateStore(t.TempDir()) if err != nil { diff --git a/Cubelet/network/runtime/systemnet/cube_dev.go b/Cubelet/network/runtime/systemnet/cube_dev.go index e4b2c9180..f447e58d7 100644 --- a/Cubelet/network/runtime/systemnet/cube_dev.go +++ b/Cubelet/network/runtime/systemnet/cube_dev.go @@ -43,7 +43,7 @@ func GetOrCreateCubeDev(ip net.IP, mask, mtu int, macAddr string) (*CubeDev, err if !ok { return nil, fmt.Errorf("%s is not dummy", cubeDevName) } - addrs, err := netlink.AddrList(dummy, netlink.FAMILY_V4) + addrs, err := netlinkAddrList(dummy, netlink.FAMILY_V4) if err != nil { return nil, err } diff --git a/Cubelet/network/runtime/systemnet/device.go b/Cubelet/network/runtime/systemnet/device.go index c9e42af39..bcf8a6ed3 100644 --- a/Cubelet/network/runtime/systemnet/device.go +++ b/Cubelet/network/runtime/systemnet/device.go @@ -5,24 +5,59 @@ package systemnet import ( + "errors" "fmt" "net" + "time" "github.com/vishvananda/netlink" "golang.org/x/sys/unix" ) +// maxDumpRetries is higher than containernetworking/plugins netlinksafe (5) +// because Cubelet density creates can keep the link table mutating for longer +// than a single LinkList of hundreds of TAPs takes to complete. +const maxDumpRetries = 16 + +// dumpRetryBackoff is a short pause between interrupted dumps so concurrent +// LinkAdd/LinkDel traffic can settle before the next attempt. +const dumpRetryBackoff = 2 * time.Millisecond + var ( // Package-level function variables are test seams for host networking helpers. + // Dump-style reads go through WithDumpRetry so NLM_F_DUMP_INTR under TAP + // churn is retried; mutating calls are left unwrapped. netlinkRouteReplace = netlink.RouteReplace - netlinkRouteListFiltered = netlink.RouteListFiltered - netlinkRouteList = netlink.RouteList - netlinkLinkByName = netlink.LinkByName - netlinkLinkList = netlink.LinkList - netlinkLinkDel = netlink.LinkDel - netlinkNeighList = netlink.NeighList - netlinkAddrList = netlink.AddrList - netlinkRouteDel = netlink.RouteDel + netlinkRouteListFiltered = func(family int, filter *netlink.Route, mask uint64) ([]netlink.Route, error) { + return WithDumpRetry(func() ([]netlink.Route, error) { + return netlink.RouteListFiltered(family, filter, mask) + }) + } + netlinkRouteList = func(link netlink.Link, family int) ([]netlink.Route, error) { + return WithDumpRetry(func() ([]netlink.Route, error) { + return netlink.RouteList(link, family) + }) + } + netlinkLinkByName = func(name string) (netlink.Link, error) { + return WithDumpRetry(func() (netlink.Link, error) { + return netlink.LinkByName(name) + }) + } + netlinkLinkList = func() ([]netlink.Link, error) { + return WithDumpRetry(netlink.LinkList) + } + netlinkLinkDel = netlink.LinkDel + netlinkNeighList = func(linkIndex, family int) ([]netlink.Neigh, error) { + return WithDumpRetry(func() ([]netlink.Neigh, error) { + return netlink.NeighList(linkIndex, family) + }) + } + netlinkAddrList = func(link netlink.Link, family int) ([]netlink.Addr, error) { + return WithDumpRetry(func() ([]netlink.Addr, error) { + return netlink.AddrList(link, family) + }) + } + netlinkRouteDel = netlink.RouteDel ) // HostDevice captures the configured host network device selected by Cubelet. @@ -41,7 +76,7 @@ func GetHostDevice(ifName string) (*HostDevice, error) { if err != nil { return nil, err } - addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) + addrs, err := netlinkAddrList(link, netlink.FAMILY_V4) if err != nil { return nil, err } @@ -133,3 +168,28 @@ func isUsableGatewayNeighbor(neigh netlink.Neigh, gatewayIP net.IP) bool { return false } } + +// WithDumpRetry runs op, retrying when a netlink dump was interrupted because +// the table changed mid-read (ErrDumpInterrupted / EINTR). Other errors and +// success return immediately. Mutating netlink calls should not use this. +func WithDumpRetry[T any](op func() (T, error)) (T, error) { + var ( + zero T + last error + ) + for attempt := 0; attempt < maxDumpRetries; attempt++ { + v, err := op() + if err == nil || !isDumpInterrupted(err) { + return v, err + } + last = err + if attempt+1 < maxDumpRetries { + time.Sleep(dumpRetryBackoff) + } + } + return zero, last +} + +func isDumpInterrupted(err error) bool { + return errors.Is(err, netlink.ErrDumpInterrupted) || errors.Is(err, unix.EINTR) +} diff --git a/Cubelet/network/runtime/systemnet/device_test.go b/Cubelet/network/runtime/systemnet/device_test.go new file mode 100644 index 000000000..c9fa71867 --- /dev/null +++ b/Cubelet/network/runtime/systemnet/device_test.go @@ -0,0 +1,64 @@ +// Copyright (c) 2024 Tencent Inc. +// SPDX-License-Identifier: Apache-2.0 +// + +package systemnet + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" +) + +func TestWithDumpRetrySucceedsAfterInterrupt(t *testing.T) { + calls := 0 + got, err := WithDumpRetry(func() (int, error) { + calls++ + if calls < 3 { + return 0, netlink.ErrDumpInterrupted + } + return 42, nil + }) + require.NoError(t, err) + assert.Equal(t, 42, got) + assert.Equal(t, 3, calls) +} + +func TestWithDumpRetryExhaustsAttempts(t *testing.T) { + calls := 0 + _, err := WithDumpRetry(func() (int, error) { + calls++ + return 0, netlink.ErrDumpInterrupted + }) + require.ErrorIs(t, err, netlink.ErrDumpInterrupted) + assert.Equal(t, maxDumpRetries, calls) +} + +func TestWithDumpRetryDoesNotRetryOtherErrors(t *testing.T) { + want := errors.New("not a dump interrupt") + calls := 0 + _, err := WithDumpRetry(func() (int, error) { + calls++ + return 0, want + }) + require.ErrorIs(t, err, want) + assert.Equal(t, 1, calls) +} + +func TestWithDumpRetryTreatsEINTRAsInterrupt(t *testing.T) { + calls := 0 + got, err := WithDumpRetry(func() (string, error) { + calls++ + if calls == 1 { + return "", unix.EINTR + } + return "ok", nil + }) + require.NoError(t, err) + assert.Equal(t, "ok", got) + assert.Equal(t, 2, calls) +} diff --git a/Cubelet/network/runtime/tap_device.go b/Cubelet/network/runtime/tap_device.go index 8333dfd8f..72c793a55 100644 --- a/Cubelet/network/runtime/tap_device.go +++ b/Cubelet/network/runtime/tap_device.go @@ -19,10 +19,24 @@ import ( "golang.org/x/sys/unix" ) -var netlinkLinkByIndex = netlink.LinkByIndex -var netlinkLinkByName = netlink.LinkByName -var netlinkLinkList = netlink.LinkList +// Dump-style netlink reads retry on ErrDumpInterrupted (see systemnet.WithDumpRetry). +// Hot-path create prefers openTapFdByName to avoid these reads entirely; cleaner +// and destroy/restore paths still need them under concurrent TAP churn. +var netlinkLinkByIndex = func(index int) (netlink.Link, error) { + return systemnet.WithDumpRetry(func() (netlink.Link, error) { + return netlink.LinkByIndex(index) + }) +} +var netlinkLinkByName = func(name string) (netlink.Link, error) { + return systemnet.WithDumpRetry(func() (netlink.Link, error) { + return netlink.LinkByName(name) + }) +} +var netlinkLinkList = func() ([]netlink.Link, error) { + return systemnet.WithDumpRetry(netlink.LinkList) +} var netlinkLinkDel = netlink.LinkDel +var deleteTAPDevicePolicyMaps = cubevs.DeleteTAPDevicePolicyMaps var unixOpen = unix.Open var unixClose = unix.Close var unixIoctlIfreq = unix.IoctlIfreq @@ -317,6 +331,15 @@ func listCubeTaps() (map[string]*tapDevice, error) { return ipToTap, nil } +// isTapNotFound reports whether err means the named host link is absent. +func isTapNotFound(err error) bool { + if err == nil { + return false + } + var notFound netlink.LinkNotFoundError + return errors.As(err, ¬Found) +} + // getTapByName returns identity for one runtime-managed TAP by name. func getTapByName(name string) (*tapDevice, error) { link, err := netlinkLinkByName(name) @@ -345,17 +368,57 @@ func getTapByName(name string) (*tapDevice, error) { // destroyTap removes a TAP by ifindex. It first tries to clear TUNSETPERSIST via // /dev/net/tun because persistent TAPs may survive netlink deletion alone. +// After the netdev is confirmed gone it best-effort deletes HashOfMaps outer +// policy keys so destroyed ifindexes cannot accumulate stale allow_out_v2 / +// deny_out / dns_allow inners. Policy cleanup never changes the destroy result: +// a missing BPF map must not leave pool/cleanup stuck on a dead netdev. func destroyTap(ifIdx int) error { link, err := netlinkLinkByIndex(ifIdx) if err != nil { + if isTapNotFound(err) { + // Confirmed absent — drop orphaned outer keys. Transient lookup + // errors must not wipe policy for a TAP that may still be up. + cleanupDestroyedTapPolicyMaps(ifIdx) + } return err } + destroyed := false if tap, ok := link.(*netlink.Tuntap); ok { if err := deletePersistentTapByName(tap.Name); err == nil { - return nil + destroyed = true } } - return netlinkLinkDel(link) + if !destroyed { + if err := netlinkLinkDel(link); err != nil { + return err + } + } + cleanupDestroyedTapPolicyMaps(ifIdx) + return nil +} + +// cleanupDestroyedTapPolicyMaps deletes HashOfMaps outer keys for ifIdx only +// when that ifindex no longer has a host netdev. If the index was reused by a +// new device between LinkDel and this call, cleanup is skipped. +func cleanupDestroyedTapPolicyMaps(ifIdx int) { + _, err := netlinkLinkByIndex(ifIdx) + switch { + case err == nil: + // ifindex reused (or delete raced); do not touch the new device's policy. + return + case !isTapNotFound(err): + CubeLog.WithContext(context.Background()).Warnf( + "network runtime tap policy map cleanup skipped: ifindex=%d lookup_err=%v", + ifIdx, err, + ) + return + } + if cleanErr := deleteTAPDevicePolicyMaps(uint32(ifIdx)); cleanErr != nil { + CubeLog.WithContext(context.Background()).Warnf( + "network runtime tap policy map cleanup failed: ifindex=%d err=%v", + ifIdx, cleanErr, + ) + } } // deletePersistentTapByName opens the TAP and clears TUNSETPERSIST, making the diff --git a/Cubelet/network/runtime/tap_device_test.go b/Cubelet/network/runtime/tap_device_test.go index 3539bec1e..72384b3aa 100644 --- a/Cubelet/network/runtime/tap_device_test.go +++ b/Cubelet/network/runtime/tap_device_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + "github.com/vishvananda/netlink" "golang.org/x/sys/unix" ) @@ -79,3 +80,81 @@ func TestConfigureTapFDStopsWhenRequiredOffloadConfigurationFails(t *testing.T) t.Fatal("optional ethtool feature ran after required offload configuration failed") } } + +func TestDestroyTapSkipsPolicyCleanupOnTransientLookupError(t *testing.T) { + origLookup := netlinkLinkByIndex + origDelete := deleteTAPDevicePolicyMaps + t.Cleanup(func() { + netlinkLinkByIndex = origLookup + deleteTAPDevicePolicyMaps = origDelete + }) + + transient := errors.New("dump interrupted") + netlinkLinkByIndex = func(int) (netlink.Link, error) { return nil, transient } + deleted := false + deleteTAPDevicePolicyMaps = func(uint32) error { + deleted = true + return nil + } + + err := destroyTap(77) + if !errors.Is(err, transient) { + t.Fatalf("destroyTap error=%v, want %v", err, transient) + } + if deleted { + t.Fatal("policy maps must not be deleted on transient lookup errors") + } +} + +func TestDestroyTapCleansPolicyWhenLinkAlreadyGone(t *testing.T) { + origLookup := netlinkLinkByIndex + origDelete := deleteTAPDevicePolicyMaps + t.Cleanup(func() { + netlinkLinkByIndex = origLookup + deleteTAPDevicePolicyMaps = origDelete + }) + + notFound := netlink.LinkNotFoundError{} + netlinkLinkByIndex = func(int) (netlink.Link, error) { return nil, notFound } + var gotIfindex uint32 + deleteTAPDevicePolicyMaps = func(ifindex uint32) error { + gotIfindex = ifindex + return nil + } + + err := destroyTap(88) + if !errors.As(err, &netlink.LinkNotFoundError{}) { + t.Fatalf("destroyTap error=%v, want LinkNotFoundError", err) + } + if gotIfindex != 88 { + t.Fatalf("policy cleanup ifindex=%d, want 88", gotIfindex) + } +} + +func TestDestroyTapPolicyCleanupFailureDoesNotFailDestroy(t *testing.T) { + origLookup := netlinkLinkByIndex + origDelete := deleteTAPDevicePolicyMaps + origDel := netlinkLinkDel + t.Cleanup(func() { + netlinkLinkByIndex = origLookup + deleteTAPDevicePolicyMaps = origDelete + netlinkLinkDel = origDel + }) + + lookups := 0 + netlinkLinkByIndex = func(int) (netlink.Link, error) { + lookups++ + if lookups == 1 { + return &netlink.Device{LinkAttrs: netlink.LinkAttrs{Index: 99, Name: "z192.168.0.99"}}, nil + } + return nil, netlink.LinkNotFoundError{} + } + netlinkLinkDel = func(netlink.Link) error { return nil } + deleteTAPDevicePolicyMaps = func(uint32) error { + return errors.New("bpf map missing") + } + + if err := destroyTap(99); err != nil { + t.Fatalf("destroyTap error=%v, want nil when only policy cleanup fails", err) + } +} diff --git a/Cubelet/network/runtime/tap_pool_test.go b/Cubelet/network/runtime/tap_pool_test.go index 59dd833d7..97bd47b4c 100644 --- a/Cubelet/network/runtime/tap_pool_test.go +++ b/Cubelet/network/runtime/tap_pool_test.go @@ -246,6 +246,7 @@ type fakeTapDeviceAdapter struct { closeHook func(*os.File) restoreWithoutFD bool listResult map[string]*tapDevice + listErr error } func (f *fakeTapDeviceAdapter) Create(ip net.IP, _ string, _ int, _ int) (*tapDevice, error) { @@ -280,6 +281,9 @@ func (f *fakeTapDeviceAdapter) Close(file *os.File) { } func (f *fakeTapDeviceAdapter) List() (map[string]*tapDevice, error) { + if f.listErr != nil { + return nil, f.listErr + } return f.listResult, nil } diff --git a/examples/cube-bench/Makefile b/examples/cube-bench/Makefile index f494fadba..778d75b1b 100644 --- a/examples/cube-bench/Makefile +++ b/examples/cube-bench/Makefile @@ -2,13 +2,16 @@ BINARY := bin/cube-bench GOFLAGS ?= +SOURCES := $(wildcard *.go) .PHONY: build clean fmt help ## build: compile the benchmark binary (default) build: $(BINARY) -$(BINARY): +# Rebuild when any Go source changes. Without these prerequisites, Make treats +# an existing bin/cube-bench as always up to date. +$(BINARY): $(SOURCES) go.mod go.sum @mkdir -p bin go build $(GOFLAGS) -o $(BINARY) . @echo "Built: $(BINARY)" diff --git a/examples/cube-bench/README.md b/examples/cube-bench/README.md index e94813be6..06a2002d6 100644 --- a/examples/cube-bench/README.md +++ b/examples/cube-bench/README.md @@ -55,6 +55,7 @@ All env vars can be overridden by the corresponding flag. | `-m`, `--mode` | `create-delete` | `create-delete` or `create-only` | | `-o`, `--output` | *(none)* | Export JSON report to file | | `--host-mount` | *(none)* | Host mount list as a JSON array | +| `--network-policy`, `-np` | `none` | Network policy on create: `none` (no rules) or `rules` (create with egress rules) | | `--api-url` | *(env)* | CubeAPI base URL | | `--api-key` | *(env)* | API key | | `--theme` | `auto` | Color theme: `dark`, `light`, or `auto` | @@ -81,6 +82,9 @@ export CUBE_TEMPLATE_ID= # Benchmark host-mount create requests ./bin/cube-bench -c 10 -n 50 --host-mount '[{"hostPath":"/tmp/data","mountPath":"/mnt/data","readOnly":false}]' +# Create with egress rules (CubeVS maps + CubeEgress policy push) +./bin/cube-bench -c 10 -n 50 -w 2 --network-policy rules + # Non-interactive output (CI / pipe) ./bin/cube-bench --dry-run --no-tui -c 10 -n 50 @@ -88,6 +92,25 @@ export CUBE_TEMPLATE_ID= ./bin/cube-bench --dry-run --theme light -c 10 -n 100 ``` +### Network policies + +| Policy | Create payload | What it exercises | +|---|---|---| +| `none` (default) | `templateID` only (+ optional `host-mount`) | Create without network rules | +| `rules` | `allow_internet_access=false` + ~24 `allowOut` (CIDR + domain) + 6 L7 `rules` (2 with inject) | Create with network rules: CubeVS allow/dns map updates and CubeEgress policy PUT | + +`rules` uses a fixed built-in policy (stable fake hosts and dummy inject secrets). The bench only waits for create HTTP success; it does **not** validate dataplane allow/deny or in-guest connectivity. + +Suggested A/B comparison (same `-c/-n/-t`, warm the pool once): + +```bash +./bin/cube-bench -c 10 -n 50 -w 2 --network-policy none -o none.json +./bin/cube-bench -c 10 -n 50 -w 2 --network-policy rules -o rules.json +# Prefer Δ(rules − none) on create P50/P95 as the network-sensitive signal. +``` + +When comparing two Cube builds (for example pre/post network refactor), fix `--network-policy rules` and change only the server under test. + For `host-mount`, this CLI form is equivalent to the Python SDK pattern: ```python @@ -113,6 +136,7 @@ contract still receives `metadata` as strings: - Live TUI dashboard: progress bar, real-time QPS, rolling operation log - Final report: percentile table (P50/P95/P99), latency histogram, sparkline, and letter grade (S/A/B/C/D) +- Built-in `--network-policy rules` mode for create-with-rules latency - Dark/light/auto theme detection - JSON report export (`-o report.json`) - Dry-run mode for testing without a CubeSandbox server diff --git a/examples/cube-bench/main.go b/examples/cube-bench/main.go index 419ad33ce..2a77529be 100644 --- a/examples/cube-bench/main.go +++ b/examples/cube-bench/main.go @@ -35,6 +35,8 @@ type Config struct { APIKey string ThemeName string HostMount string // raw JSON array for config display and report export + NetworkPolicy string // none | rules + networkFP networkConfigFingerprint hostMountValue string // compacted once for request-time reuse requestBody []byte requestHeaders map[string]string @@ -48,8 +50,10 @@ type Config struct { } type createRequest struct { - TemplateID string `json:"templateID"` - Metadata map[string]string `json:"metadata,omitempty"` + TemplateID string `json:"templateID"` + AllowInternetAccess *bool `json:"allow_internet_access,omitempty"` + Network *sandboxNetworkConfig `json:"network,omitempty"` + Metadata map[string]string `json:"metadata,omitempty"` } func prepareHostMount(rawJSON string) (string, error) { @@ -72,11 +76,22 @@ func prepareHostMount(rawJSON string) (string, error) { return compact.String(), nil } -func buildCreateRequestBody(template string, hostMount string) ([]byte, error) { +func buildCreateRequestBody(template string, hostMount string, networkPolicy string) ([]byte, error) { reqBody := createRequest{TemplateID: template} if hostMount != "" { reqBody.Metadata = map[string]string{"host-mount": hostMount} } + switch networkPolicy { + case "", networkPolicyNone: + // empty-network baseline (historical cube-bench behavior) + case networkPolicyRules: + denyAll := false + net := rulesNetworkConfig() + reqBody.AllowInternetAccess = &denyAll + reqBody.Network = &net + default: + return nil, fmt.Errorf("unsupported network policy %q", networkPolicy) + } return json.Marshal(reqBody) } @@ -96,6 +111,8 @@ func parseConfig() *Config { flag.StringVar(&cfg.Output, "o", "", "Export JSON report to file") flag.StringVar(&cfg.Output, "output", "", "Export JSON report to file") flag.StringVar(&cfg.HostMount, "host-mount", "", "Host mount list as a JSON array") + flag.StringVar(&cfg.NetworkPolicy, "network-policy", networkPolicyNone, "Network policy on create: none (no rules) | rules") + flag.StringVar(&cfg.NetworkPolicy, "np", networkPolicyNone, "Short for --network-policy") flag.StringVar(&cfg.APIURL, "api-url", "", "CubeAPI base URL (overrides E2B_API_URL)") flag.StringVar(&cfg.APIKey, "api-key", "", "API key (overrides E2B_API_KEY)") flag.StringVar(&cfg.ThemeName, "theme", "auto", "Color theme: dark | light | auto") @@ -111,6 +128,14 @@ func parseConfig() *Config { cfg.NoTUI = noTUI || !term.IsTerminal(int(os.Stdout.Fd())) + policy, err := parseNetworkPolicy(cfg.NetworkPolicy) + if err != nil { + fmt.Fprintf(os.Stderr, "ERROR: %v\n", err) + os.Exit(1) + } + cfg.NetworkPolicy = policy + cfg.networkFP = networkFingerprint(policy) + cfg.DryLatencyMean = 80 cfg.DryLatencyStd = 30 if parts := strings.Split(*dryLatency, ","); len(parts) == 2 { @@ -163,7 +188,7 @@ func parseConfig() *Config { cfg.hostMountValue = hostMountValue cfg.requestHeaders = map[string]string{"Authorization": "Bearer " + cfg.APIKey} - requestBody, err := buildCreateRequestBody(cfg.Template, cfg.hostMountValue) + requestBody, err := buildCreateRequestBody(cfg.Template, cfg.hostMountValue, cfg.NetworkPolicy) if err != nil { fmt.Fprintf(os.Stderr, "ERROR: create request body build failed: %v\n", err) os.Exit(1) @@ -189,6 +214,7 @@ func renderConfig(cfg *Config) { {"Total Requests", fmt.Sprintf("%d", cfg.Total)}, {"Warmup Rounds", fmt.Sprintf("%d", cfg.Warmup)}, {"Mode", cfg.Mode}, + {"Network Policy", cfg.networkFP.summary()}, } if cfg.HostMount != "" { // Pretty-print the original host-mount JSON for readability. @@ -294,13 +320,17 @@ func exportJSON(results []IterResult, cfg *Config) { report := map[string]interface{}{ "timestamp": time.Now().UTC().Format(time.RFC3339), "config": map[string]interface{}{ - "template": cfg.Template, - "api_url": cfg.APIURL, - "concurrency": cfg.Concurrency, - "total": cfg.Total, - "warmup": cfg.Warmup, - "mode": cfg.Mode, - "host_mount": cfg.HostMount, + "template": cfg.Template, + "api_url": cfg.APIURL, + "concurrency": cfg.Concurrency, + "total": cfg.Total, + "warmup": cfg.Warmup, + "mode": cfg.Mode, + "host_mount": cfg.HostMount, + "network_policy": cfg.networkFP.Policy, + "network_allow_out": cfg.networkFP.AllowOut, + "network_rules": cfg.networkFP.Rules, + "network_inject_rules": cfg.networkFP.InjectRules, }, "summary": map[string]interface{}{ "total_time_s": cfg.elapsed, diff --git a/examples/cube-bench/main_test.go b/examples/cube-bench/main_test.go index e96522d02..1a5adb6ab 100644 --- a/examples/cube-bench/main_test.go +++ b/examples/cube-bench/main_test.go @@ -1,6 +1,9 @@ package main -import "testing" +import ( + "encoding/json" + "testing" +) func TestPrepareHostMountCompactsValidArray(t *testing.T) { got, err := prepareHostMount(`[ @@ -55,3 +58,124 @@ func TestPrepareHostMountRejectsEmptyArray(t *testing.T) { t.Fatal("prepareHostMount returned nil error, want empty array error") } } + +func TestParseNetworkPolicy(t *testing.T) { + got, err := parseNetworkPolicy("rules") + if err != nil { + t.Fatalf("parseNetworkPolicy(rules): %v", err) + } + if got != networkPolicyRules { + t.Fatalf("got %q, want %q", got, networkPolicyRules) + } + + got, err = parseNetworkPolicy("") + if err != nil { + t.Fatalf("parseNetworkPolicy(\"\"): %v", err) + } + if got != networkPolicyNone { + t.Fatalf("got %q, want %q", got, networkPolicyNone) + } + + if _, err := parseNetworkPolicy("stress"); err == nil { + t.Fatal("parseNetworkPolicy(stress) returned nil error, want rejection") + } +} + +func TestBuildCreateRequestBodyNoneOmitsNetwork(t *testing.T) { + raw, err := buildCreateRequestBody("tpl-1", "", networkPolicyNone) + if err != nil { + t.Fatalf("buildCreateRequestBody: %v", err) + } + + var body map[string]json.RawMessage + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if _, ok := body["allow_internet_access"]; ok { + t.Fatalf("none policy must omit allow_internet_access, got %s", body["allow_internet_access"]) + } + if _, ok := body["network"]; ok { + t.Fatalf("none policy must omit network, got %s", body["network"]) + } +} + +func TestBuildCreateRequestBodyRulesShape(t *testing.T) { + raw, err := buildCreateRequestBody("tpl-1", "", networkPolicyRules) + if err != nil { + t.Fatalf("buildCreateRequestBody: %v", err) + } + + var body struct { + TemplateID string `json:"templateID"` + AllowInternetAccess *bool `json:"allow_internet_access"` + Network *struct { + AllowOut []string `json:"allowOut"` + Rules []struct { + Name string `json:"name"` + Action struct { + Allow bool `json:"allow"` + Inject []struct { + Header string `json:"header"` + Secret string `json:"secret"` + } `json:"inject"` + } `json:"action"` + } `json:"rules"` + } `json:"network"` + } + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + if body.TemplateID != "tpl-1" { + t.Fatalf("templateID=%q, want tpl-1", body.TemplateID) + } + if body.AllowInternetAccess == nil || *body.AllowInternetAccess { + t.Fatalf("allow_internet_access=%v, want false", body.AllowInternetAccess) + } + if body.Network == nil { + t.Fatal("network missing") + } + + fp := networkFingerprint(networkPolicyRules) + if len(body.Network.AllowOut) != fp.AllowOut { + t.Fatalf("allowOut count=%d, want %d", len(body.Network.AllowOut), fp.AllowOut) + } + if len(body.Network.Rules) != fp.Rules { + t.Fatalf("rules count=%d, want %d", len(body.Network.Rules), fp.Rules) + } + + injectRules := 0 + for _, r := range body.Network.Rules { + if len(r.Action.Inject) > 0 { + injectRules++ + } + } + if injectRules != fp.InjectRules { + t.Fatalf("inject rules=%d, want %d", injectRules, fp.InjectRules) + } + if fp.AllowOut != 24 || fp.Rules != 6 || fp.InjectRules != 2 { + t.Fatalf("unexpected fingerprint: %+v", fp) + } +} + +func TestBuildCreateRequestBodyRulesKeepsHostMount(t *testing.T) { + hostMount := `[{"hostPath":"/tmp/data","mountPath":"/mnt/data","readOnly":false}]` + raw, err := buildCreateRequestBody("tpl-1", hostMount, networkPolicyRules) + if err != nil { + t.Fatalf("buildCreateRequestBody: %v", err) + } + + var body struct { + Metadata map[string]string `json:"metadata"` + Network *struct{} `json:"network"` + } + if err := json.Unmarshal(raw, &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if body.Metadata["host-mount"] != hostMount { + t.Fatalf("host-mount=%q, want %q", body.Metadata["host-mount"], hostMount) + } + if body.Network == nil { + t.Fatal("network missing when host-mount is set") + } +} diff --git a/examples/cube-bench/network_policy.go b/examples/cube-bench/network_policy.go new file mode 100644 index 000000000..fb7bf8980 --- /dev/null +++ b/examples/cube-bench/network_policy.go @@ -0,0 +1,223 @@ +package main + +import ( + "fmt" + "strings" +) + +const ( + networkPolicyNone = "none" + networkPolicyRules = "rules" +) + +// networkConfigFingerprint summarizes a built-in policy for reports / UI. +type networkConfigFingerprint struct { + Policy string `json:"policy"` + AllowOut int `json:"allow_out"` + Rules int `json:"rules"` + InjectRules int `json:"inject_rules"` +} + +type egressRuleInject struct { + Header string `json:"header"` + Secret string `json:"secret"` + Format string `json:"format,omitempty"` +} + +type egressRuleMatch struct { + SNI string `json:"sni,omitempty"` + Host string `json:"host,omitempty"` + Method []string `json:"method,omitempty"` + Path string `json:"path,omitempty"` + Scheme string `json:"scheme,omitempty"` +} + +type egressRuleAction struct { + Allow bool `json:"allow"` + Audit string `json:"audit,omitempty"` + Inject []egressRuleInject `json:"inject,omitempty"` +} + +type egressRule struct { + Name string `json:"name"` + Match egressRuleMatch `json:"match"` + Action egressRuleAction `json:"action"` +} + +type sandboxNetworkConfig struct { + AllowOut []string `json:"allowOut,omitempty"` + Rules []egressRule `json:"rules,omitempty"` +} + +func parseNetworkPolicy(raw string) (string, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "", networkPolicyNone: + return networkPolicyNone, nil + case networkPolicyRules: + return networkPolicyRules, nil + default: + return "", fmt.Errorf("--network-policy must be %q or %q, got %q", + networkPolicyNone, networkPolicyRules, raw) + } +} + +func rulesAllowOut() []string { + // 12 CIDRs + 12 domains (incl. 2 wildcards) — medium allowlist for create-path + // CubeVS allow_out_v2 / dns_allow map updates. Hosts are stable fakes; the + // bench does not require them to resolve or be reachable. + return []string{ + "1.1.1.1/32", + "1.0.0.1/32", + "8.8.8.8/32", + "8.8.4.4/32", + "9.9.9.9/32", + "149.112.112.112/32", + "208.67.222.222/32", + "208.67.220.220/32", + "94.140.14.14/32", + "94.140.15.15/32", + "76.76.2.0/24", + "76.76.10.0/24", + "dns.bench.cubesandbox.test", + "registry.bench.cubesandbox.test", + "cdn.bench.cubesandbox.test", + "npm.bench.cubesandbox.test", + "pypi.bench.cubesandbox.test", + "github.bench.cubesandbox.test", + "objects.bench.cubesandbox.test", + "telemetry.bench.cubesandbox.test", + "docs.bench.cubesandbox.test", + "status.bench.cubesandbox.test", + "*.assets.bench.cubesandbox.test", + "*.cdn.bench.cubesandbox.test", + } +} + +func rulesL7Rules() []egressRule { + // 6 L7 rules; 2 carry inject so CubeEgress policy PUT is non-trivial. + return []egressRule{ + { + Name: "allow_llm_chat", + Match: egressRuleMatch{ + Scheme: "https", + SNI: "api.bench.cubesandbox.test", + Host: "api.bench.cubesandbox.test", + Method: []string{"POST"}, + Path: "/v1/chat/completions", + }, + Action: egressRuleAction{ + Allow: true, + Audit: "metadata", + Inject: []egressRuleInject{{ + Header: "Authorization", + Secret: "cube-bench-dummy-llm-key", + Format: "Bearer ${SECRET}", + }}, + }, + }, + { + Name: "allow_llm_models", + Match: egressRuleMatch{ + Scheme: "https", + SNI: "api.bench.cubesandbox.test", + Host: "api.bench.cubesandbox.test", + Method: []string{"GET"}, + Path: "/v1/models", + }, + Action: egressRuleAction{ + Allow: true, + Audit: "metadata", + }, + }, + { + Name: "allow_embeddings", + Match: egressRuleMatch{ + Scheme: "https", + SNI: "embed.bench.cubesandbox.test", + Host: "embed.bench.cubesandbox.test", + Method: []string{"POST"}, + Path: "/v1/embeddings", + }, + Action: egressRuleAction{ + Allow: true, + Audit: "metadata", + Inject: []egressRuleInject{{ + Header: "Authorization", + Secret: "cube-bench-dummy-embed-key", + Format: "Bearer ${SECRET}", + }}, + }, + }, + { + Name: "allow_vector_upsert", + Match: egressRuleMatch{ + Scheme: "https", + SNI: "vector.bench.cubesandbox.test", + Host: "vector.bench.cubesandbox.test", + Method: []string{"POST", "PUT"}, + Path: "/v1/indexes/*", + }, + Action: egressRuleAction{ + Allow: true, + Audit: "metadata", + }, + }, + { + Name: "allow_webhook_callback", + Match: egressRuleMatch{ + Scheme: "https", + SNI: "hooks.bench.cubesandbox.test", + Host: "hooks.bench.cubesandbox.test", + Method: []string{"POST"}, + Path: "/callbacks/*", + }, + Action: egressRuleAction{ + Allow: true, + Audit: "none", + }, + }, + { + Name: "deny_metadata_probe", + Match: egressRuleMatch{ + Scheme: "http", + Host: "169.254.169.254", + Path: "/*", + }, + Action: egressRuleAction{ + Allow: false, + Audit: "full", + }, + }, + } +} + +func rulesNetworkConfig() sandboxNetworkConfig { + return sandboxNetworkConfig{ + AllowOut: rulesAllowOut(), + Rules: rulesL7Rules(), + } +} + +func networkFingerprint(policy string) networkConfigFingerprint { + fp := networkConfigFingerprint{Policy: policy} + if policy != networkPolicyRules { + return fp + } + cfg := rulesNetworkConfig() + fp.AllowOut = len(cfg.AllowOut) + fp.Rules = len(cfg.Rules) + for _, r := range cfg.Rules { + if len(r.Action.Inject) > 0 { + fp.InjectRules++ + } + } + return fp +} + +func (fp networkConfigFingerprint) summary() string { + if fp.Policy == networkPolicyNone || fp.Policy == "" { + return networkPolicyNone + } + return fmt.Sprintf("%s (allowOut=%d rules=%d injectRules=%d)", + fp.Policy, fp.AllowOut, fp.Rules, fp.InjectRules) +} diff --git a/examples/cube-bench/runner_test.go b/examples/cube-bench/runner_test.go index bbbb0ffc3..eba22f1a8 100644 --- a/examples/cube-bench/runner_test.go +++ b/examples/cube-bench/runner_test.go @@ -10,7 +10,7 @@ import ( ) func TestRunWarmupCompletesBeforeBenchmark(t *testing.T) { - requestBody, err := buildCreateRequestBody("tpl-warmup", "") + requestBody, err := buildCreateRequestBody("tpl-warmup", "", networkPolicyNone) if err != nil { t.Fatalf("buildCreateRequestBody returned error: %v", err) } @@ -56,7 +56,7 @@ func TestBenchOneSendsHostMountMetadata(t *testing.T) { if err != nil { t.Fatalf("prepareHostMount returned error: %v", err) } - requestBody, err := buildCreateRequestBody("tpl-test", hostMountValue) + requestBody, err := buildCreateRequestBody("tpl-test", hostMountValue, networkPolicyNone) if err != nil { t.Fatalf("buildCreateRequestBody returned error: %v", err) } @@ -126,7 +126,7 @@ func TestBenchOneSendsHostMountMetadata(t *testing.T) { } func TestBenchOneDeletePath(t *testing.T) { - requestBody, err := buildCreateRequestBody("tpl-delete", "") + requestBody, err := buildCreateRequestBody("tpl-delete", "", networkPolicyNone) if err != nil { t.Fatalf("buildCreateRequestBody returned error: %v", err) }