From 07c1cfe7bc6c3167d7c4687772e14fe5bffcfd75 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Tue, 11 Aug 2026 17:03:52 +0000 Subject: [PATCH 01/10] feat(network/v2): nftables host datapath (ported from PoC, datapath only) --- .../orchestrator/pkg/sandbox/network/host.go | 3 + .../orchestrator/pkg/sandbox/network/pool.go | 3 + .../pkg/sandbox/network/pool_iface.go | 18 + .../pkg/sandbox/network/v2/host_firewall.go | 520 ++++++++++++++++++ .../sandbox/network/v2/host_firewall_test.go | 129 +++++ .../pkg/sandbox/network/v2/network.go | 250 +++++++++ .../pkg/sandbox/network/v2/network_test.go | 123 +++++ .../pkg/sandbox/network/v2/ns_nat.go | 142 +++++ .../pkg/sandbox/network/v2/ns_nat_test.go | 89 +++ .../pkg/sandbox/network/v2/observability.go | 131 +++++ .../sandbox/network/v2/observability_test.go | 78 +++ .../pkg/sandbox/network/v2/policy.go | 198 +++++++ .../pkg/sandbox/network/v2/policy_test.go | 59 ++ .../pkg/sandbox/network/v2/pool.go | 277 ++++++++++ .../pkg/sandbox/network/v2/slot.go | 99 ++++ .../pkg/sandbox/network/v2/slot_test.go | 114 ++++ 16 files changed, 2233 insertions(+) create mode 100644 packages/orchestrator/pkg/sandbox/network/pool_iface.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/network.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/network_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/observability.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/observability_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/policy.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/policy_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/pool.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/slot.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/slot_test.go diff --git a/packages/orchestrator/pkg/sandbox/network/host.go b/packages/orchestrator/pkg/sandbox/network/host.go index 0083b25238..e85d2e6e05 100644 --- a/packages/orchestrator/pkg/sandbox/network/host.go +++ b/packages/orchestrator/pkg/sandbox/network/host.go @@ -20,6 +20,9 @@ const loopbackInterface = "lo" // Host default gateway name var defaultGateway = utils.Must(getDefaultGateway(context.Background())) +// DefaultGateway returns the detected host default gateway interface name. +func DefaultGateway() string { return defaultGateway } + // func getDefaultGateway() (string, error) { // route, err := exec.Command( // "sh", diff --git a/packages/orchestrator/pkg/sandbox/network/pool.go b/packages/orchestrator/pkg/sandbox/network/pool.go index 6283d6f049..2e362cf0f6 100644 --- a/packages/orchestrator/pkg/sandbox/network/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/pool.go @@ -87,6 +87,9 @@ type Config struct { // an explicit 0 disables marking for builds only. Range 0..63. // Set-but-empty behaves as unset (inherits): the env parser skips it. BuildSandboxEgressDSCP *uint8 `env:"BUILD_SANDBOX_EGRESS_DSCP"` + + // NetworkVersion selects v1 (iptables per-sandbox) or v2 (nftables verdict maps). + NetworkVersion int `env:"NETWORK_VERSION" envDefault:"1"` } // EgressClass selects which configured egress DSCP applies to a sandbox. It diff --git a/packages/orchestrator/pkg/sandbox/network/pool_iface.go b/packages/orchestrator/pkg/sandbox/network/pool_iface.go new file mode 100644 index 0000000000..bb416082e6 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/pool_iface.go @@ -0,0 +1,18 @@ +//go:build linux + +package network + +import ( + "context" + "time" + + "github.com/e2b-dev/infra/packages/shared/pkg/grpc/orchestrator" +) + +// PoolInterface abstracts the network slot pool so v1 (*Pool) and v2 pools can +// be swapped. The existing *Pool already satisfies this interface. +type PoolInterface interface { + Get(ctx context.Context, network *orchestrator.SandboxNetworkConfig, class EgressClass) (*Slot, error) + ReturnAsync(ctx context.Context, slot *Slot, releasedFn ReleaseNotify, returnDelay time.Duration) error + Close(ctx context.Context) error +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go new file mode 100644 index 0000000000..700ffdb621 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -0,0 +1,520 @@ +//go:build linux + +package v2 + +import ( + "encoding/binary" + "fmt" + "net" + "sync" + + "github.com/google/nftables" + "github.com/google/nftables/expr" + "golang.org/x/sys/unix" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +// HostFirewall is a singleton that manages the host-level nftables table +// "v2-host-firewall". It uses sets so rule count stays constant regardless +// of sandbox count — lookups are O(1) hash operations. +// +// Table layout: +// +// table inet v2-host-firewall { +// set v2_veths { type ifname; } +// set v2_host_cidrs { type ipv4_addr; flags interval; } +// +// chain forward { type filter hook forward priority 0; policy drop; +// iifname @v2_veths oifname accept +// iifname oifname @v2_veths ct state established,related accept +// } +// chain prerouting { type nat hook prerouting priority -100; +// # service redirects (all slots share same ports) +// iifname @v2_veths tcp dport 80 ip daddr redirect to :hyperloopPort +// iifname @v2_veths tcp dport 111 ip daddr redirect to :portmapperPort +// iifname @v2_veths tcp dport 2049 ip daddr redirect to :nfsPort +// # TCP firewall proxy redirects +// iifname @v2_veths tcp dport 80 redirect to :tcpHTTPPort +// iifname @v2_veths tcp dport 443 redirect to :tcpTLSPort +// iifname @v2_veths tcp dport != { 80, 111, 443, 2049 } redirect to :tcpOtherPort +// } +// chain postrouting { type nat hook postrouting priority 100; +// ip saddr @v2_host_cidrs oifname masquerade +// } +// } +// +// The key insight: since all slots share the same redirect ports (configured +// per-orchestrator, not per-slot), we don't need verdict maps. Simple rules +// with set-based iifname matching give us O(1) lookups. Only per-slot data +// (veth names and host CIDRs) goes into sets. +type HostFirewall struct { + conn *nftables.Conn + table *nftables.Table + + vethSet *nftables.Set // type ifname; elements = veth interface names + cidrSet *nftables.Set // type ipv4_addr; flags interval; elements = host CIDRs + + defaultGw string + config network.Config + mu sync.Mutex + + // migrationRules tracks active migration DNAT/forward rules by old host IP. + // Protected by mu. + migrationRules map[string]*migrationDNATEntry + + // mangleChain is the prerouting mangle chain used for fwmark rules. + // Stored here so fwmark rules can be added/removed without re-creating the chain. + mangleChain *nftables.Chain + + // fwmarkRules tracks per-veth fwmark rule handles for surgical removal. + // Protected by mu. + fwmarkRules map[string]uint64 +} + +const ( + hostFwTableName = "v2-host-firewall" +) + +// NewHostFirewall creates or opens the singleton host firewall table with all +// required sets and chains. Call once per orchestrator process. +// +// Restart-safe: if the table already exists from a previous run (e.g., orchestrator +// restart with live sandboxes), existing set elements are preserved. Only chain rules +// are refreshed with current config. This prevents connectivity loss for sandboxes +// that survived the restart. +func NewHostFirewall(defaultGw string, config network.Config) (*HostFirewall, error) { + conn, err := nftables.New(nftables.AsLasting()) + if err != nil { + return nil, fmt.Errorf("new nftables conn: %w", err) + } + + // Ensure table exists — idempotent: creates if new, opens if existing. + // We do NOT delete the table on startup. Existing set elements represent + // live sandboxes whose connectivity must be preserved across restarts. + table := conn.AddTable(&nftables.Table{ + Name: hostFwTableName, + Family: nftables.TableFamilyINet, + }) + + hf := &HostFirewall{ + conn: conn, + table: table, + defaultGw: defaultGw, + config: config, + migrationRules: make(map[string]*migrationDNATEntry), + fwmarkRules: make(map[string]uint64), + } + + if err := hf.ensureSets(); err != nil { + return nil, fmt.Errorf("ensure sets: %w", err) + } + + if err := hf.ensureChains(); err != nil { + return nil, fmt.Errorf("ensure chains: %w", err) + } + + return hf, nil +} + +// ensureSets creates sets if they don't exist, or opens handles to existing ones. +// Existing set elements (active sandbox veth names and host CIDRs) are preserved. +func (hf *HostFirewall) ensureSets() error { + // Set of veth interface names — AddSet is idempotent for existing sets + hf.vethSet = &nftables.Set{ + Table: hf.table, + Name: "v2_veths", + KeyType: nftables.TypeIFName, + } + if err := hf.conn.AddSet(hf.vethSet, nil); err != nil { + return fmt.Errorf("add veth set: %w", err) + } + + // Set of host CIDRs — interval set for /32 entries + hf.cidrSet = &nftables.Set{ + Table: hf.table, + Name: "v2_host_cidrs", + KeyType: nftables.TypeIPAddr, + Interval: true, + } + if err := hf.conn.AddSet(hf.cidrSet, nil); err != nil { + return fmt.Errorf("add cidr set: %w", err) + } + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush sets: %w", err) + } + + return nil +} + +// ensureChains creates or refreshes all chains with current config. +// Chains are flushed and re-populated — this is safe because chain rules are +// config-dependent (ports, gateway interface), not state-dependent. +// Set elements (which represent live sandboxes) are NOT touched. +// The entire operation is atomic via nftables batching. +func (hf *HostFirewall) ensureChains() error { + gwBytes := ifnameBytes(hf.defaultGw) + orchIP := net.ParseIP(hf.config.OrchestratorInSandboxIPAddress).To4() + + // --- FORWARD chain --- + fwdPolicy := nftables.ChainPolicyDrop + fwdChain := hf.conn.AddChain(&nftables.Chain{ + Name: "forward", + Table: hf.table, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookForward, + Priority: nftables.ChainPriorityFilter, + Policy: &fwdPolicy, + }) + hf.conn.FlushChain(fwdChain) // clear stale rules from previous config + + // iifname @v2_veths oifname accept + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: fwdChain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Lookup{SourceRegister: 1, SetName: hf.vethSet.Name, SetID: hf.vethSet.ID}, + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: gwBytes}, + &expr.Verdict{Kind: expr.VerdictAccept}, + }, + }) + + // iifname oifname @v2_veths ct state established,related accept + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: fwdChain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: gwBytes}, + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Lookup{SourceRegister: 1, SetName: hf.vethSet.Name, SetID: hf.vethSet.ID}, + &expr.Ct{Key: expr.CtKeySTATE, Register: 1}, + &expr.Bitwise{ + SourceRegister: 1, DestRegister: 1, Len: 4, + Mask: bitmask32(expr.CtStateBitESTABLISHED | expr.CtStateBitRELATED), + Xor: bitmask32(0), + }, + &expr.Cmp{Op: expr.CmpOpNeq, Register: 1, Data: bitmask32(0)}, + &expr.Verdict{Kind: expr.VerdictAccept}, + }, + }) + + // --- PREROUTING chain (NAT) --- + preChain := hf.conn.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: hf.table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityRef(-100), + }) + hf.conn.FlushChain(preChain) + + // Service redirects: iifname @v2_veths + ip daddr + tcp dport X → redirect to :port + svcRedirects := []struct { + dport uint16 + rport uint16 + }{ + {80, hf.config.HyperloopProxyPort}, + {111, hf.config.PortmapperPort}, + {2049, hf.config.NFSProxyPort}, + } + for _, svc := range svcRedirects { + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: preChain, + Exprs: svcRedirectExprs(hf.vethSet, orchIP, svc.dport, svc.rport), + }) + } + + // TCP firewall proxy redirects: + // Port 80 (non-service, i.e., daddr != orchIP) → tcpHTTPPort + // Port 443 → tcpTLSPort + // All other TCP → tcpOtherPort + tcpRedirects := []struct { + dport uint16 + rport uint16 + }{ + {80, hf.config.SandboxTCPFirewallHTTPPort}, + {443, hf.config.SandboxTCPFirewallTLSPort}, + } + for _, tcp := range tcpRedirects { + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: preChain, + Exprs: tcpRedirectExprs(hf.vethSet, tcp.dport, tcp.rport), + }) + } + + // Catch-all TCP redirect: any remaining TCP from v2 veths → tcpOtherPort + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: preChain, + Exprs: tcpCatchAllExprs(hf.vethSet, hf.config.SandboxTCPFirewallOtherPort), + }) + + // --- POSTROUTING chain (NAT) --- + postChain := hf.conn.AddChain(&nftables.Chain{ + Name: "postrouting", + Table: hf.table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPostrouting, + Priority: nftables.ChainPriorityNATSource, + }) + hf.conn.FlushChain(postChain) + + // ip saddr @v2_host_cidrs oifname masquerade + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: postChain, + Exprs: []expr.Any{ + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, + &expr.Lookup{SourceRegister: 1, SetName: hf.cidrSet.Name, SetID: hf.cidrSet.ID}, + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: gwBytes}, + &expr.Masq{}, + }, + }) + + // --- MANGLE PREROUTING chain (for fwmark rules) --- + hf.mangleChain = hf.conn.AddChain(&nftables.Chain{ + Name: "mangle_prerouting", + Table: hf.table, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityMangle, + }) + // NOTE: we do NOT flush the mangle chain here. Fwmark rules are per-veth + // state, not config. They are managed individually via Add/RemoveFwmark. + // On restart, stale fwmark rules are cleaned up by ReconcileSlots. + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush chains: %w", err) + } + + return nil +} + +// ReconcileSlots reconciles host firewall set membership with actual active slots. +// Call on startup after rebuilding the in-memory slot registry from surviving sandboxes. +// - Slots in the registry but missing from sets are added. +// - Set entries not in the registry are removed (stale leftovers from crashed sandboxes). +// - Stale fwmark rules in the mangle chain are flushed. +func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { + hf.mu.Lock() + defer hf.mu.Unlock() + + // Build desired state from active slots + desiredVeths := make(map[string]bool) + desiredCIDRs := make(map[string]bool) // key = hostIP string + for _, sv2 := range activeSlots { + desiredVeths[sv2.Slot.VethName()] = true + desiredCIDRs[sv2.Slot.HostIP.To4().String()] = true + } + + // Read current veth set elements + currentVeths, err := hf.conn.GetSetElements(hf.vethSet) + if err != nil { + return fmt.Errorf("get veth set elements: %w", err) + } + + // Remove stale veths (in set but not in active slots) + var staleVethElems []nftables.SetElement + for _, elem := range currentVeths { + name := string(elem.Key[:len(elem.Key)-1]) // strip null terminator + if !desiredVeths[name] { + staleVethElems = append(staleVethElems, elem) + } + } + if len(staleVethElems) > 0 { + hf.conn.SetDeleteElements(hf.vethSet, staleVethElems) + } + + // Add missing veths (in active slots but not in set) + currentVethNames := make(map[string]bool) + for _, elem := range currentVeths { + name := string(elem.Key[:len(elem.Key)-1]) + currentVethNames[name] = true + } + for _, sv2 := range activeSlots { + veth := sv2.Slot.VethName() + if !currentVethNames[veth] { + if err := hf.conn.SetAddElements(hf.vethSet, []nftables.SetElement{ + {Key: ifnameBytes(veth)}, + }); err != nil { + return fmt.Errorf("add missing veth %s: %w", veth, err) + } + } + } + + // Reconcile CIDR set: flush and rebuild (interval sets don't support + // element-level comparison easily, so rebuild is simpler and correct) + hf.conn.FlushSet(hf.cidrSet) + for _, sv2 := range activeSlots { + hostIP := sv2.Slot.HostIP.To4() + nextIP := incrementIP(hostIP) + if err := hf.conn.SetAddElements(hf.cidrSet, []nftables.SetElement{ + {Key: hostIP}, + {Key: nextIP, IntervalEnd: true}, + }); err != nil { + return fmt.Errorf("add host cidr for slot %d: %w", sv2.Slot.Idx, err) + } + } + + // Flush stale fwmark rules: we lost in-memory handles on restart, + // so flush the mangle chain and let egress profile re-assignment + // re-create needed rules. + hf.conn.FlushChain(hf.mangleChain) + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush reconcile: %w", err) + } + + return nil +} + +// AddSlot adds the veth name and host CIDR for a v2 slot. +func (hf *HostFirewall) AddSlot(slotV2 *SlotV2) error { + hf.mu.Lock() + defer hf.mu.Unlock() + + slot := slotV2.Slot + + // Add veth name to v2_veths set + if err := hf.conn.SetAddElements(hf.vethSet, []nftables.SetElement{ + {Key: ifnameBytes(slot.VethName())}, + }); err != nil { + return fmt.Errorf("add veth to set: %w", err) + } + + // Add host CIDR to v2_host_cidrs interval set (/32) + hostIP := slot.HostIP.To4() + nextIP := incrementIP(hostIP) + if err := hf.conn.SetAddElements(hf.cidrSet, []nftables.SetElement{ + {Key: hostIP}, + {Key: nextIP, IntervalEnd: true}, + }); err != nil { + return fmt.Errorf("add host cidr to set: %w", err) + } + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush add slot: %w", err) + } + + return nil +} + +// RemoveSlot removes the veth name and host CIDR for a v2 slot. +func (hf *HostFirewall) RemoveSlot(slotV2 *SlotV2) error { + hf.mu.Lock() + defer hf.mu.Unlock() + + slot := slotV2.Slot + + hf.conn.SetDeleteElements(hf.vethSet, []nftables.SetElement{ + {Key: ifnameBytes(slot.VethName())}, + }) + + hostIP := slot.HostIP.To4() + nextIP := incrementIP(hostIP) + hf.conn.SetDeleteElements(hf.cidrSet, []nftables.SetElement{ + {Key: hostIP}, + {Key: nextIP, IntervalEnd: true}, + }) + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush remove slot: %w", err) + } + + return nil +} + +// Close tears down the entire host firewall table. +func (hf *HostFirewall) Close() error { + hf.mu.Lock() + defer hf.mu.Unlock() + + hf.conn.DelTable(hf.table) + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("delete host firewall table: %w", err) + } + + return hf.conn.CloseLasting() +} + +// --- nftables expression builders --- + +// svcRedirectExprs builds: iifname @set tcp dport X ip daddr redirect to :port +func svcRedirectExprs(vethSet *nftables.Set, orchIP net.IP, dport, rport uint16) []expr.Any { + return []expr.Any{ + // Match TCP + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: []byte{unix.IPPROTO_TCP}}, + // iifname @v2_veths + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Lookup{SourceRegister: 1, SetName: vethSet.Name, SetID: vethSet.ID}, + // ip daddr == orchIP + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 16, Len: 4}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: orchIP}, + // tcp dport == dport + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: portBytes(dport)}, + // redirect to :rport + &expr.Immediate{Register: 1, Data: portBytes(rport)}, + &expr.Redir{RegisterProtoMin: 1}, + } +} + +// tcpRedirectExprs builds: iifname @set tcp dport X redirect to :port +func tcpRedirectExprs(vethSet *nftables.Set, dport, rport uint16) []expr.Any { + return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: []byte{unix.IPPROTO_TCP}}, + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Lookup{SourceRegister: 1, SetName: vethSet.Name, SetID: vethSet.ID}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: portBytes(dport)}, + &expr.Immediate{Register: 1, Data: portBytes(rport)}, + &expr.Redir{RegisterProtoMin: 1}, + } +} + +// tcpCatchAllExprs builds: iifname @set tcp protocol redirect to :port +func tcpCatchAllExprs(vethSet *nftables.Set, rport uint16) []expr.Any { + return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: []byte{unix.IPPROTO_TCP}}, + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Lookup{SourceRegister: 1, SetName: vethSet.Name, SetID: vethSet.ID}, + &expr.Immediate{Register: 1, Data: portBytes(rport)}, + &expr.Redir{RegisterProtoMin: 1}, + } +} + +// --- helpers --- + +func portBytes(port uint16) []byte { + b := make([]byte, 2) + binary.BigEndian.PutUint16(b, port) + return b +} + +func incrementIP(ip net.IP) net.IP { + result := make(net.IP, len(ip)) + copy(result, ip) + for i := len(result) - 1; i >= 0; i-- { + result[i]++ + if result[i] != 0 { + break + } + } + return result +} + +func bitmask32(v uint32) []byte { + buf := make([]byte, 4) + binary.NativeEndian.PutUint32(buf, v) + return buf +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go new file mode 100644 index 0000000000..1957725db4 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go @@ -0,0 +1,129 @@ +//go:build linux + +package v2 + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +func testConfig() network.Config { + return network.Config{ + OrchestratorInSandboxIPAddress: "192.0.2.1", + HyperloopProxyPort: 5010, + NFSProxyPort: 5011, + PortmapperPort: 5012, + SandboxTCPFirewallHTTPPort: 5016, + SandboxTCPFirewallTLSPort: 5017, + SandboxTCPFirewallOtherPort: 5018, + } +} + +func TestHostFirewall_Init(t *testing.T) { + skipIfNotLinuxRoot(t) + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + assert.NotNil(t, hf.conn) + assert.NotNil(t, hf.table) + assert.NotNil(t, hf.vethSet) + assert.NotNil(t, hf.cidrSet) +} + +func TestHostFirewall_AddRemoveSlot(t *testing.T) { + skipIfNotLinuxRoot(t) + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + slot := makeTestSlot(t, 1) + sv2 := NewSlotV2(slot) + + // Add slot + err = hf.AddSlot(sv2) + assert.NoError(t, err) + + // Verify veth set has the element + elements, err := hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + assert.Greater(t, len(elements), 0, "veth set should have elements") + + // Verify cidr set has elements + cidrElements, err := hf.conn.GetSetElements(hf.cidrSet) + require.NoError(t, err) + assert.Greater(t, len(cidrElements), 0, "cidr set should have elements") + + // Remove slot + err = hf.RemoveSlot(sv2) + assert.NoError(t, err) + + // Verify veth set is empty + elements, err = hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + assert.Equal(t, 0, len(elements), "veth set should be empty after removal") +} + +func TestHostFirewall_ConcurrentAccess(t *testing.T) { + skipIfNotLinuxRoot(t) + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + // Add multiple slots concurrently + done := make(chan error, 10) + for i := 1; i <= 10; i++ { + go func(idx int) { + slot := makeTestSlot(t, idx) + sv2 := NewSlotV2(slot) + done <- hf.AddSlot(sv2) + }(i) + } + + for i := 0; i < 10; i++ { + err := <-done + assert.NoError(t, err, "concurrent AddSlot should not fail") + } + + // Verify all 10 veths added + elements, err := hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + assert.Equal(t, 10, len(elements)) +} + +func TestIncrementIP(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"10.11.0.1", "10.11.0.2"}, + {"10.11.0.255", "10.11.1.0"}, + {"10.11.255.255", "10.12.0.0"}, + {"192.168.1.1", "192.168.1.2"}, + } + + for _, tt := range tests { + ip := net.ParseIP(tt.input).To4() + result := incrementIP(ip) + assert.Equal(t, tt.expected, result.String(), "incrementIP(%s)", tt.input) + } +} + +func TestPortBytes(t *testing.T) { + b := portBytes(80) + assert.Equal(t, []byte{0, 80}, b) + + b = portBytes(443) + assert.Equal(t, []byte{1, 187}, b) + + b = portBytes(5010) + assert.Equal(t, []byte{0x13, 0x92}, b) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go new file mode 100644 index 0000000000..4b4c22e8f9 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -0,0 +1,250 @@ +//go:build linux + +package v2 + +import ( + "context" + "errors" + "fmt" + "net" + "runtime" + + "github.com/google/nftables" + "github.com/vishvananda/netlink" + "github.com/vishvananda/netns" + "go.uber.org/zap" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" + "github.com/e2b-dev/infra/packages/shared/pkg/logger" +) + +// CreateNetworkV2 sets up the full network stack for a v2 slot. +// It reuses the same netlink setup pattern as v1 (namespace, veth, tap, routes) +// but replaces all iptables calls with nftables. +// +// Compared to v1: +// - In-namespace NAT: nftables SetupNamespaceNAT() replaces 2 iptables rules +// - Host firewall: hf.AddSlot() replaces 8 iptables rules with 2 set element adds +// - eBPF: observer.Attach() adds optional per-veth counters (best-effort) +func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, + hf *HostFirewall, observer *VethObserver) error { + + // Prevent thread changes so we can safely manipulate namespaces + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + // Save the original (host) namespace + hostNS, err := netns.Get() + if err != nil { + return fmt.Errorf("cannot get current (host) namespace: %w", err) + } + defer func() { + if err := netns.Set(hostNS); err != nil { + logger.L().Error(ctx, "error resetting network namespace back to host", zap.Error(err)) + } + hostNS.Close() + }() + + // --- Create namespace --- + ns, err := netns.NewNamed(slot.NamespaceID()) + if err != nil { + return fmt.Errorf("cannot create new namespace: %w", err) + } + defer ns.Close() + + // --- Create veth pair (inside new namespace, then move veth to host) --- + vethAttrs := netlink.NewLinkAttrs() + vethAttrs.Name = slot.VethName() + veth := &netlink.Veth{ + LinkAttrs: vethAttrs, + PeerName: slot.VpeerName(), + } + + if err := netlink.LinkAdd(veth); err != nil { + return fmt.Errorf("error creating veth device: %w", err) + } + + vpeer, err := netlink.LinkByName(slot.VpeerName()) + if err != nil { + return fmt.Errorf("error finding vpeer: %w", err) + } + + if err := netlink.LinkSetUp(vpeer); err != nil { + return fmt.Errorf("error setting vpeer device up: %w", err) + } + + if err := netlink.AddrAdd(vpeer, &netlink.Addr{ + IPNet: &net.IPNet{IP: slot.VpeerIP(), Mask: slot.VrtMask()}, + }); err != nil { + return fmt.Errorf("error adding vpeer device address: %w", err) + } + + // Move veth to host namespace + if err := netlink.LinkSetNsFd(veth, int(hostNS)); err != nil { + return fmt.Errorf("error moving veth device to host namespace: %w", err) + } + + if err := netns.Set(hostNS); err != nil { + return fmt.Errorf("error setting network namespace: %w", err) + } + + vethInHost, err := netlink.LinkByName(slot.VethName()) + if err != nil { + return fmt.Errorf("error finding veth: %w", err) + } + + if err := netlink.LinkSetUp(vethInHost); err != nil { + return fmt.Errorf("error setting veth device up: %w", err) + } + + if err := netlink.AddrAdd(vethInHost, &netlink.Addr{ + IPNet: &net.IPNet{IP: slot.VethIP(), Mask: slot.VrtMask()}, + }); err != nil { + return fmt.Errorf("error adding veth device address: %w", err) + } + + // Switch back to sandbox namespace for tap setup + if err := netns.Set(ns); err != nil { + return fmt.Errorf("error setting network namespace to %s: %w", ns.String(), err) + } + + // --- Create TAP device for Firecracker --- + tapAttrs := netlink.NewLinkAttrs() + tapAttrs.Name = slot.TapName() + tapAttrs.Namespace = ns + tap := &netlink.Tuntap{ + Mode: netlink.TUNTAP_MODE_TAP, + LinkAttrs: tapAttrs, + } + + if err := netlink.LinkAdd(tap); err != nil { + return fmt.Errorf("error creating tap device: %w", err) + } + + if err := netlink.LinkSetUp(tap); err != nil { + return fmt.Errorf("error setting tap device up: %w", err) + } + + if err := netlink.AddrAdd(tap, &netlink.Addr{ + IPNet: &net.IPNet{IP: slot.TapIP(), Mask: slot.TapCIDR()}, + }); err != nil { + return fmt.Errorf("error setting address of the tap device: %w", err) + } + + // --- Loopback --- + lo, err := netlink.LinkByName("lo") + if err != nil { + return fmt.Errorf("error finding lo: %w", err) + } + if err := netlink.LinkSetUp(lo); err != nil { + return fmt.Errorf("error setting lo device up: %w", err) + } + + // --- Default route inside namespace --- + if err := netlink.RouteAdd(&netlink.Route{ + Scope: netlink.SCOPE_UNIVERSE, + Gw: slot.VethIP(), + }); err != nil { + return fmt.Errorf("error adding default NS route: %w", err) + } + + // --- In-namespace NAT (replaces 2 iptables rules) --- + // We need a fresh nftables conn inside this namespace context + nsConn, err := nftables.New(nftables.AsLasting()) + if err != nil { + return fmt.Errorf("nftables conn in namespace: %w", err) + } + + // The v1 Firewall creates a table named "slot-firewall" — reuse it + nsTable := nsConn.AddTable(&nftables.Table{ + Name: "slot-firewall", + Family: nftables.TableFamilyINet, + }) + + if err := SetupNamespaceNAT(nsConn, nsTable, slot.VpeerName(), slot.HostIPString(), slot.NamespaceIP()); err != nil { + nsConn.CloseLasting() + return fmt.Errorf("setup namespace NAT: %w", err) + } + nsConn.CloseLasting() + + // --- Initialize in-namespace firewall (reuse v1 Firewall) --- + if err := slot.InitializeFirewall(); err != nil { + return fmt.Errorf("error initializing slot firewall: %w", err) + } + + // --- Switch back to host namespace --- + if err := netns.Set(hostNS); err != nil { + return fmt.Errorf("error setting network namespace to %s: %w", hostNS.String(), err) + } + + // --- Host route to namespace --- + if err := netlink.RouteAdd(&netlink.Route{ + Gw: slot.VpeerIP(), + Dst: slot.HostNet(), + }); err != nil { + return fmt.Errorf("error adding route from host to FC: %w", err) + } + + // --- Host firewall: add slot to sets (replaces 8 iptables rules) --- + if err := hf.AddSlot(slotV2); err != nil { + return fmt.Errorf("error adding slot to host firewall: %w", err) + } + + // --- eBPF observability (best-effort) --- + if observer != nil { + if err := observer.Attach(slot.VethName()); err != nil { + logger.L().Warn(ctx, "failed to attach eBPF observer (non-fatal)", zap.Error(err)) + } + } + + return nil +} + +// RemoveNetworkV2 tears down the network for a v2 slot. +func RemoveNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, + hf *HostFirewall, observer *VethObserver) error { + + var errs []error + + // Close in-namespace firewall + if err := slot.CloseFirewall(); err != nil { + errs = append(errs, fmt.Errorf("error closing firewall: %w", err)) + } + + // Remove from host firewall sets + if err := hf.RemoveSlot(slotV2); err != nil { + errs = append(errs, fmt.Errorf("error removing slot from host firewall: %w", err)) + } + + // Detach eBPF observer + if observer != nil { + if err := observer.Detach(slot.VethName()); err != nil { + errs = append(errs, fmt.Errorf("error detaching eBPF observer: %w", err)) + } + } + + // Delete host route + if err := netlink.RouteDel(&netlink.Route{ + Gw: slot.VpeerIP(), + Dst: slot.HostNet(), + }); err != nil { + errs = append(errs, fmt.Errorf("error deleting route from host to FC: %w", err)) + } + + // Delete veth device explicitly (prevents race on reuse) + vethLink, err := netlink.LinkByName(slot.VethName()) + if err != nil { + errs = append(errs, fmt.Errorf("error finding veth: %w", err)) + } else { + if err := netlink.LinkDel(vethLink); err != nil { + errs = append(errs, fmt.Errorf("error deleting veth device: %w", err)) + } + } + + // Delete namespace + if err := netns.DeleteNamed(slot.NamespaceID()); err != nil { + errs = append(errs, fmt.Errorf("error deleting namespace: %w", err)) + } + + return errors.Join(errs...) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go new file mode 100644 index 0000000000..7fed4c0f6e --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -0,0 +1,123 @@ +//go:build linux + +package v2 + +import ( + "context" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCreateNetworkV2_FullLifecycle(t *testing.T) { + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + // Create host firewall + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + observer, err := NewVethObserver() + require.NoError(t, err) + defer observer.Close() + + // Use high index to avoid conflicts with running orchestrator's namespaces + slot := makeTestSlot(t, 30001) + sv2 := NewSlotV2(slot) + + // Create network + err = CreateNetworkV2(ctx, slot, sv2, hf, observer) + require.NoError(t, err) + + // Verify namespace exists + out, err := exec.Command("ip", "netns", "list").Output() + require.NoError(t, err) + assert.Contains(t, string(out), slot.NamespaceID()) + + // Verify veth exists in host + out, err = exec.Command("ip", "link", "show", slot.VethName()).Output() + require.NoError(t, err) + assert.Contains(t, string(out), slot.VethName()) + + // Teardown + err = RemoveNetworkV2(ctx, slot, sv2, hf, observer) + assert.NoError(t, err) + + // Verify namespace is gone — check each line for exact match + out, _ = exec.Command("ip", "netns", "list").Output() + for _, line := range strings.Split(string(out), "\n") { + assert.NotContains(t, line, slot.NamespaceID(), + "namespace should be removed after teardown") + } +} + +func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + slot := makeTestSlot(t, 30002) + sv2 := NewSlotV2(slot) + + err = CreateNetworkV2(ctx, slot, sv2, hf, nil) + require.NoError(t, err) + defer RemoveNetworkV2(ctx, slot, sv2, hf, nil) + + // Verify NO iptables rules reference this veth + out, err := exec.Command("iptables", "-t", "nat", "-L", "PREROUTING", "-n").Output() + if err == nil { + assert.NotContains(t, string(out), slot.VethName(), + "v2 sandbox should not create iptables rules") + } + + out, err = exec.Command("iptables", "-t", "filter", "-L", "FORWARD", "-n").Output() + if err == nil { + assert.NotContains(t, string(out), slot.VethName(), + "v2 sandbox should not create iptables FORWARD rules") + } +} + +func TestCreateNetworkV2_CleanTeardown(t *testing.T) { + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + defer hf.Close() + + observer, _ := NewVethObserver() + defer observer.Close() + + slot := makeTestSlot(t, 30003) + sv2 := NewSlotV2(slot) + + err = CreateNetworkV2(ctx, slot, sv2, hf, observer) + require.NoError(t, err) + + err = RemoveNetworkV2(ctx, slot, sv2, hf, observer) + require.NoError(t, err) + + // Verify veth set is empty + elements, err := hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + assert.Equal(t, 0, len(elements), "veth set should be empty after teardown") + + // Verify no routes to the slot's host IP + out, err := exec.Command("ip", "route", "show").Output() + require.NoError(t, err) + lines := strings.Split(string(out), "\n") + for _, line := range lines { + assert.NotContains(t, line, slot.HostIPString(), + "should have no route to slot host IP after teardown") + } +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go new file mode 100644 index 0000000000..8a3c42016d --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go @@ -0,0 +1,142 @@ +//go:build linux + +package v2 + +import ( + "fmt" + "net" + + "github.com/google/nftables" + "github.com/google/nftables/expr" + "golang.org/x/sys/unix" +) + +// SetupNamespaceNAT adds SNAT/DNAT chains to the existing "slot-firewall" nftables +// table inside a network namespace. This replaces the two iptables rules that v1 creates: +// - iptables -t nat POSTROUTING -o eth0 -s -j SNAT --to +// - iptables -t nat PREROUTING -i eth0 -d -j DNAT --to +// +// The nftables equivalent uses the same kernel conntrack, so SO_ORIGINAL_DST +// in the TCP firewall proxy continues to work unchanged. +func SetupNamespaceNAT(conn *nftables.Conn, table *nftables.Table, vpeerIface, hostIP, namespaceIP string) error { + hostIPParsed := net.ParseIP(hostIP).To4() + if hostIPParsed == nil { + return fmt.Errorf("invalid host IP: %s", hostIP) + } + + nsIPParsed := net.ParseIP(namespaceIP).To4() + if nsIPParsed == nil { + return fmt.Errorf("invalid namespace IP: %s", namespaceIP) + } + + // POSTROUTING NAT chain: SNAT namespace IP → host IP + postChain := conn.AddChain(&nftables.Chain{ + Name: "postroute_nat", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPostrouting, + Priority: nftables.ChainPriorityNATSource, + }) + + // Match: oifname == vpeerIface && ip saddr == namespaceIP → snat to hostIP + conn.AddRule(&nftables.Rule{ + Table: table, + Chain: postChain, + Exprs: []expr.Any{ + // Match output interface + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Cmp{ + Register: 1, + Op: expr.CmpOpEq, + Data: ifnameBytes(vpeerIface), + }, + // Match source IP == namespaceIP + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: 12, // src IP offset in IPv4 + Len: 4, + }, + &expr.Cmp{ + Register: 1, + Op: expr.CmpOpEq, + Data: nsIPParsed, + }, + // SNAT to hostIP + &expr.Immediate{ + Register: 1, + Data: hostIPParsed, + }, + &expr.NAT{ + Type: expr.NATTypeSourceNAT, + Family: unix.NFPROTO_IPV4, + RegAddrMin: 1, + RegAddrMax: 1, + RegProtoMin: 0, + RegProtoMax: 0, + }, + }, + }) + + // PREROUTING NAT chain: DNAT hostIP → namespaceIP + preChain := conn.AddChain(&nftables.Chain{ + Name: "preroute_nat", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + + // Match: iifname == vpeerIface && ip daddr == hostIP → dnat to namespaceIP + conn.AddRule(&nftables.Rule{ + Table: table, + Chain: preChain, + Exprs: []expr.Any{ + // Match input interface + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Cmp{ + Register: 1, + Op: expr.CmpOpEq, + Data: ifnameBytes(vpeerIface), + }, + // Match destination IP == hostIP + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: 16, // dst IP offset in IPv4 + Len: 4, + }, + &expr.Cmp{ + Register: 1, + Op: expr.CmpOpEq, + Data: hostIPParsed, + }, + // DNAT to namespaceIP + &expr.Immediate{ + Register: 1, + Data: nsIPParsed, + }, + &expr.NAT{ + Type: expr.NATTypeDestNAT, + Family: unix.NFPROTO_IPV4, + RegAddrMin: 1, + RegAddrMax: 1, + RegProtoMin: 0, + RegProtoMax: 0, + }, + }, + }) + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush namespace NAT rules: %w", err) + } + + return nil +} + +// ifnameBytes pads an interface name to IFNAMSIZ (16 bytes) with null termination. +func ifnameBytes(name string) []byte { + b := make([]byte, 16) // IFNAMSIZ + copy(b, name) + return b +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go new file mode 100644 index 0000000000..9400bd25bd --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go @@ -0,0 +1,89 @@ +//go:build linux + +package v2 + +import ( + "os" + "runtime" + "testing" + + "github.com/google/nftables" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func skipIfNotLinuxRoot(t *testing.T) { + t.Helper() + if runtime.GOOS != "linux" { + t.Skip("skipping: requires linux") + } + if os.Getuid() != 0 { + t.Skip("skipping: requires root") + } +} + +func TestNamespaceNAT_Setup(t *testing.T) { + skipIfNotLinuxRoot(t) + + conn, err := nftables.New(nftables.AsLasting()) + require.NoError(t, err) + defer conn.CloseLasting() + + table := conn.AddTable(&nftables.Table{ + Name: "test-nat", + Family: nftables.TableFamilyINet, + }) + require.NoError(t, conn.Flush()) + defer func() { + conn.DelTable(table) + conn.Flush() + }() + + err = SetupNamespaceNAT(conn, table, "eth0", "10.11.0.1", "169.254.0.21") + assert.NoError(t, err) + + // Verify chains were created + chains, err := conn.ListChainsOfTableFamily(nftables.TableFamilyINet) + require.NoError(t, err) + + chainNames := make(map[string]bool) + for _, c := range chains { + if c.Table.Name == "test-nat" { + chainNames[c.Name] = true + } + } + assert.True(t, chainNames["postroute_nat"], "postroute_nat chain should exist") + assert.True(t, chainNames["preroute_nat"], "preroute_nat chain should exist") +} + +func TestNamespaceNAT_InvalidIP(t *testing.T) { + skipIfNotLinuxRoot(t) + + conn, err := nftables.New(nftables.AsLasting()) + require.NoError(t, err) + defer conn.CloseLasting() + + table := conn.AddTable(&nftables.Table{ + Name: "test-nat-invalid", + Family: nftables.TableFamilyINet, + }) + require.NoError(t, conn.Flush()) + defer func() { + conn.DelTable(table) + conn.Flush() + }() + + err = SetupNamespaceNAT(conn, table, "eth0", "not-an-ip", "169.254.0.21") + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid host IP") +} + +func TestIfnameBytes(t *testing.T) { + b := ifnameBytes("eth0") + assert.Len(t, b, 16) + assert.Equal(t, byte('e'), b[0]) + assert.Equal(t, byte('t'), b[1]) + assert.Equal(t, byte('h'), b[2]) + assert.Equal(t, byte('0'), b[3]) + assert.Equal(t, byte(0), b[4]) // null terminated +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability.go b/packages/orchestrator/pkg/sandbox/network/v2/observability.go new file mode 100644 index 0000000000..cc102c2bec --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability.go @@ -0,0 +1,131 @@ +//go:build linux + +package v2 + +import ( + "fmt" + "sync" + + "github.com/vishvananda/netlink" +) + +// VethObserver provides per-veth packet/byte counters via netlink interface statistics. +// It is nil-safe: all methods on a nil *VethObserver are no-ops. +// +// The abstraction is kept so tc/eBPF can replace netlink stats later for +// higher resolution or per-flow counters. +type VethObserver struct { + mu sync.Mutex + attached map[string]bool // veth name → attached +} + +// NewVethObserver creates a new observer. +func NewVethObserver() (*VethObserver, error) { + return &VethObserver{ + attached: make(map[string]bool), + }, nil +} + +// Attach registers a veth interface for counter collection. +func (o *VethObserver) Attach(vethName string) error { + if o == nil { + return nil + } + o.mu.Lock() + defer o.mu.Unlock() + + if o.attached[vethName] { + return fmt.Errorf("already attached to %s", vethName) + } + // Verify the interface exists + if _, err := netlink.LinkByName(vethName); err != nil { + return fmt.Errorf("interface %s not found: %w", vethName, err) + } + o.attached[vethName] = true + return nil +} + +// Detach unregisters a veth interface from counter collection. +func (o *VethObserver) Detach(vethName string) error { + if o == nil { + return nil + } + o.mu.Lock() + defer o.mu.Unlock() + + delete(o.attached, vethName) + return nil +} + +// ReadCounters reads the packet and byte counters for a veth interface +// from kernel netlink interface statistics. Returns combined rx+tx values +// as seen from the host side of the veth pair. +func (o *VethObserver) ReadCounters(vethName string) (packets, bytes uint64, err error) { + if o == nil { + return 0, 0, nil + } + o.mu.Lock() + defer o.mu.Unlock() + + if !o.attached[vethName] { + return 0, 0, fmt.Errorf("not attached to %s", vethName) + } + + link, err := netlink.LinkByName(vethName) + if err != nil { + return 0, 0, fmt.Errorf("get link %s: %w", vethName, err) + } + + stats := link.Attrs().Statistics + if stats == nil { + return 0, 0, nil + } + + // Report rx+tx from the host veth perspective. + // Host veth rx = sandbox tx (sandbox → internet). + // Host veth tx = sandbox rx (internet → sandbox). + packets = stats.RxPackets + stats.TxPackets + bytes = stats.RxBytes + stats.TxBytes + return packets, bytes, nil +} + +// ReadCountersDirectional reads rx and tx counters separately. +// rx = traffic from sandbox to internet (host veth rx). +// tx = traffic from internet to sandbox (host veth tx). +func (o *VethObserver) ReadCountersDirectional(vethName string) (rxPackets, rxBytes, txPackets, txBytes uint64, err error) { + if o == nil { + return 0, 0, 0, 0, nil + } + o.mu.Lock() + defer o.mu.Unlock() + + if !o.attached[vethName] { + return 0, 0, 0, 0, fmt.Errorf("not attached to %s", vethName) + } + + link, err := netlink.LinkByName(vethName) + if err != nil { + return 0, 0, 0, 0, fmt.Errorf("get link %s: %w", vethName, err) + } + + stats := link.Attrs().Statistics + if stats == nil { + return 0, 0, 0, 0, nil + } + + return stats.RxPackets, stats.RxBytes, stats.TxPackets, stats.TxBytes, nil +} + +// Close detaches all observers. +func (o *VethObserver) Close() error { + if o == nil { + return nil + } + o.mu.Lock() + defer o.mu.Unlock() + + for name := range o.attached { + delete(o.attached, name) + } + return nil +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go new file mode 100644 index 0000000000..3c437d401d --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go @@ -0,0 +1,78 @@ +//go:build linux + +package v2 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVethObserver_AttachDetach(t *testing.T) { + obs, err := NewVethObserver() + require.NoError(t, err) + defer obs.Close() + + // Attach + err = obs.Attach("veth-1") + assert.NoError(t, err) + + // Double attach should fail + err = obs.Attach("veth-1") + assert.Error(t, err) + + // Detach + err = obs.Detach("veth-1") + assert.NoError(t, err) + + // Double detach is idempotent + err = obs.Detach("veth-1") + assert.NoError(t, err) +} + +func TestVethObserver_ReadCounters(t *testing.T) { + obs, err := NewVethObserver() + require.NoError(t, err) + defer obs.Close() + + // Not attached → error + _, _, err = obs.ReadCounters("veth-1") + assert.Error(t, err) + + // Attach and read + require.NoError(t, obs.Attach("veth-1")) + packets, bytes, err := obs.ReadCounters("veth-1") + assert.NoError(t, err) + assert.Equal(t, uint64(0), packets) + assert.Equal(t, uint64(0), bytes) +} + +func TestVethObserver_NilSafe(t *testing.T) { + var obs *VethObserver + + assert.NoError(t, obs.Attach("veth-1")) + assert.NoError(t, obs.Detach("veth-1")) + assert.NoError(t, obs.Close()) + + p, b, err := obs.ReadCounters("veth-1") + assert.NoError(t, err) + assert.Equal(t, uint64(0), p) + assert.Equal(t, uint64(0), b) +} + +func TestVethObserver_Close(t *testing.T) { + obs, err := NewVethObserver() + require.NoError(t, err) + + require.NoError(t, obs.Attach("veth-1")) + require.NoError(t, obs.Attach("veth-2")) + + err = obs.Close() + assert.NoError(t, err) + + // After close, nothing should be attached + obs.mu.Lock() + assert.Equal(t, 0, len(obs.attached)) + obs.mu.Unlock() +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy.go b/packages/orchestrator/pkg/sandbox/network/v2/policy.go new file mode 100644 index 0000000000..2e3eff2db4 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/policy.go @@ -0,0 +1,198 @@ +//go:build linux + +package v2 + +import ( + "fmt" + "net" + "os" + + "github.com/google/nftables" + "github.com/google/nftables/expr" + "github.com/vishvananda/netlink" +) + +// EnsureSrcValidMark sets net.ipv4.conf.all.src_valid_mark=1 which is required +// for fwmark-based policy routing of forwarded traffic. Without this, the kernel +// may not re-route forwarded packets based on marks set in prerouting. +func EnsureSrcValidMark() error { + return os.WriteFile("/proc/sys/net/ipv4/conf/all/src_valid_mark", []byte("1"), 0644) +} + +// SetupPolicyRoute creates a policy routing rule and route table entry +// for a given fwmark. Traffic marked with fwmark will be routed through +// the specified gateway device. +// +// Equivalent to: +// +// ip rule add fwmark lookup +// ip route add default via dev table +func SetupPolicyRoute(fwmark uint32, tableID int, gw net.IP, dev string) error { + // Add ip rule: fwmark → lookup table + rule := netlink.NewRule() + rule.Mark = fwmark + mask := uint32(0xFFFFFFFF) + rule.Mask = &mask + rule.Table = tableID + rule.Priority = 100 + tableID + + if err := netlink.RuleAdd(rule); err != nil { + return fmt.Errorf("add policy rule for fwmark 0x%x table %d: %w", fwmark, tableID, err) + } + + // Find the device + link, err := netlink.LinkByName(dev) + if err != nil { + return fmt.Errorf("find device %s for policy route: %w", dev, err) + } + + // Add default route in the table + route := &netlink.Route{ + LinkIndex: link.Attrs().Index, + Gw: gw, + Table: tableID, + Dst: nil, // default route + } + + if err := netlink.RouteAdd(route); err != nil { + return fmt.Errorf("add policy route default via %s dev %s table %d: %w", gw, dev, tableID, err) + } + + return nil +} + +// TeardownPolicyRoute removes the policy routing rule and route table. +func TeardownPolicyRoute(fwmark uint32, tableID int) error { + // Remove ip rule + rule := netlink.NewRule() + rule.Mark = fwmark + mask := uint32(0xFFFFFFFF) + rule.Mask = &mask + rule.Table = tableID + rule.Priority = 100 + tableID + + if err := netlink.RuleDel(rule); err != nil { + return fmt.Errorf("del policy rule for fwmark 0x%x table %d: %w", fwmark, tableID, err) + } + + // Flush all routes in the table + routes, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{Table: tableID}, netlink.RT_FILTER_TABLE) + if err != nil { + return fmt.Errorf("list routes in table %d: %w", tableID, err) + } + + for _, route := range routes { + if err := netlink.RouteDel(&route); err != nil { + return fmt.Errorf("del route in table %d: %w", tableID, err) + } + } + + return nil +} + +// SetupFwmarkInNftables adds a nftables rule to mark packets from a veth +// with the given fwmark for policy routing to egress gateways. +// The mark is set in prerouting (mangle priority) so it takes effect +// BEFORE the routing decision — required for forwarded traffic to be +// rerouted via policy rules. +// +// The rule handle is tracked so it can be surgically removed later. +func SetupFwmarkInNftables(hf *HostFirewall, vethName string, fwmark uint32) error { + hf.mu.Lock() + defer hf.mu.Unlock() + + // Remove existing rule for this veth if present (idempotent reassign) + if handle, ok := hf.fwmarkRules[vethName]; ok { + hf.conn.DelRule(&nftables.Rule{ + Table: hf.table, + Chain: hf.mangleChain, + Handle: handle, + }) + delete(hf.fwmarkRules, vethName) + } + + hf.conn.AddRule(&nftables.Rule{ + Table: hf.table, + Chain: hf.mangleChain, + Exprs: []expr.Any{ + // Match iifname == vethName + &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: ifnameBytes(vethName)}, + // Set fwmark + &expr.Immediate{Register: 1, Data: bitmask32(fwmark)}, + &expr.Meta{Key: expr.MetaKeyMARK, SourceRegister: true, Register: 1}, + }, + }) + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush fwmark rule: %w", err) + } + + // Retrieve the rule handle for later removal. + // The rule we just added is the last one in the chain for this veth. + rules, err := hf.conn.GetRules(hf.table, hf.mangleChain) + if err != nil { + return fmt.Errorf("get mangle rules to track handle: %w", err) + } + for _, r := range rules { + if matchesFwmarkRule(r, vethName) { + hf.fwmarkRules[vethName] = r.Handle + break + } + } + + return nil +} + +// matchesFwmarkRule checks if a rule is the fwmark rule for the given veth +// by inspecting its expressions for an iifname match. +func matchesFwmarkRule(r *nftables.Rule, vethName string) bool { + expected := ifnameBytes(vethName) + for i, e := range r.Exprs { + cmp, ok := e.(*expr.Cmp) + if !ok || i == 0 { + continue + } + // Check if previous expr is MetaKeyIIFNAME and this Cmp matches our veth + if meta, ok := r.Exprs[i-1].(*expr.Meta); ok && meta.Key == expr.MetaKeyIIFNAME { + if len(cmp.Data) == len(expected) { + match := true + for j := range cmp.Data { + if cmp.Data[j] != expected[j] { + match = false + break + } + } + if match { + return true + } + } + } + } + return false +} + +// RemoveFwmarkInNftables removes the per-veth fwmark rule by its tracked handle. +// Idempotent: returns nil if no rule exists for the veth. +func RemoveFwmarkInNftables(hf *HostFirewall, vethName string) error { + hf.mu.Lock() + defer hf.mu.Unlock() + + handle, ok := hf.fwmarkRules[vethName] + if !ok { + return nil // already removed or never set + } + + hf.conn.DelRule(&nftables.Rule{ + Table: hf.table, + Chain: hf.mangleChain, + Handle: handle, + }) + + if err := hf.conn.Flush(); err != nil { + return fmt.Errorf("flush remove fwmark rule for %s: %w", vethName, err) + } + + delete(hf.fwmarkRules, vethName) + return nil +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go b/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go new file mode 100644 index 0000000000..24d46d3b94 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go @@ -0,0 +1,59 @@ +//go:build linux + +package v2 + +import ( + "net" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPolicyRoute_Setup(t *testing.T) { + skipIfNotLinuxRoot(t) + + fwmark := uint32(0x300) + tableID := 300 + + // Setup: create a dummy device to route through + err := exec.Command("ip", "link", "add", "dummy-test", "type", "dummy").Run() + require.NoError(t, err) + defer exec.Command("ip", "link", "del", "dummy-test").Run() + + err = exec.Command("ip", "link", "set", "dummy-test", "up").Run() + require.NoError(t, err) + + err = exec.Command("ip", "addr", "add", "10.99.99.1/24", "dev", "dummy-test").Run() + require.NoError(t, err) + + // Test setup + gw := net.ParseIP("10.99.99.1") + err = SetupPolicyRoute(fwmark, tableID, gw, "dummy-test") + require.NoError(t, err) + + // Verify ip rule exists + out, err := exec.Command("ip", "rule", "list").Output() + require.NoError(t, err) + assert.Contains(t, string(out), "lookup 300") + + // Verify route table has route + out, err = exec.Command("ip", "route", "show", "table", "300").Output() + require.NoError(t, err) + assert.Contains(t, string(out), "dummy-test") + + // Teardown + err = TeardownPolicyRoute(fwmark, tableID) + assert.NoError(t, err) + + // Verify rule is gone + out, err = exec.Command("ip", "rule", "list").Output() + require.NoError(t, err) + lines := strings.Split(string(out), "\n") + for _, line := range lines { + assert.NotContains(t, line, "lookup 300", + "policy rule should be removed after teardown") + } +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go new file mode 100644 index 0000000000..03a8df2fe1 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -0,0 +1,277 @@ +//go:build linux + +package v2 + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + "sync" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.uber.org/zap" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" + "github.com/e2b-dev/infra/packages/shared/pkg/grpc/orchestrator" + "github.com/e2b-dev/infra/packages/shared/pkg/logger" + "github.com/e2b-dev/infra/packages/shared/pkg/telemetry" + "github.com/e2b-dev/infra/packages/shared/pkg/utils" +) + +var ( + meter = otel.Meter("github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network/v2") + + newSlotsAvailableCounter = utils.Must(meter.Int64UpDownCounter("orchestrator.network.v2.slots_pool.new", + metric.WithDescription("Number of new v2 network slots ready to be used."), + metric.WithUnit("{slot}"), + )) + reusableSlotsAvailableCounter = utils.Must(meter.Int64UpDownCounter("orchestrator.network.v2.slots_pool.reused", + metric.WithDescription("Number of reused v2 network slots ready to be used."), + metric.WithUnit("{slot}"), + )) + acquiredSlots = utils.Must(meter.Int64Counter("orchestrator.network.v2.slots_pool.acquired", + metric.WithDescription("Number of v2 network slots acquired."), + metric.WithUnit("{slot}"), + )) + returnedSlotCounter = utils.Must(meter.Int64Counter("orchestrator.network.v2.slots_pool.returned", + metric.WithDescription("Number of v2 network slots returned."), + metric.WithUnit("{slot}"), + )) + releasedSlotCounter = utils.Must(meter.Int64Counter("orchestrator.network.v2.slots_pool.released", + metric.WithDescription("Number of v2 network slots released."), + metric.WithUnit("{slot}"), + )) +) + +// V2Pool implements network.PoolInterface using v2 networking (nftables only, zero iptables). +type V2Pool struct { + config network.Config + storage network.Storage + hostFw *HostFirewall + observer *VethObserver + registry *SlotV2Registry + newSlots chan *network.Slot + reusedSlots chan *network.Slot + done chan struct{} + doneOnce sync.Once +} + +// Compile-time check that V2Pool satisfies network.PoolInterface. +var _ network.PoolInterface = (*V2Pool)(nil) + +// ValidateV2Prerequisites checks that required kernel parameters are set for v2 networking. +// Returns an error if any prerequisite is missing. Call before NewV2Pool. +func ValidateV2Prerequisites() error { + checks := []struct { + path string + expected string + desc string + }{ + {"/proc/sys/net/ipv4/ip_forward", "1", "IPv4 forwarding"}, + {"/proc/sys/net/ipv4/conf/all/src_valid_mark", "1", "src_valid_mark (required for fwmark-based policy routing)"}, + } + + var errs []error + for _, c := range checks { + val, err := os.ReadFile(c.path) + if err != nil { + errs = append(errs, fmt.Errorf("cannot read %s: %w", c.path, err)) + continue + } + if strings.TrimSpace(string(val)) != c.expected { + errs = append(errs, fmt.Errorf("%s: %s must be %s, got %q", c.desc, c.path, c.expected, strings.TrimSpace(string(val)))) + } + } + if len(errs) > 0 { + return fmt.Errorf("v2 networking prerequisites not met: %w", errors.Join(errs...)) + } + return nil +} + +func NewV2Pool(storage network.Storage, config network.Config, hostFw *HostFirewall, observer *VethObserver) *V2Pool { + return &V2Pool{ + config: config, + storage: storage, + hostFw: hostFw, + observer: observer, + registry: NewSlotV2Registry(), + newSlots: make(chan *network.Slot, network.NewSlotsPoolSize-1), + reusedSlots: make(chan *network.Slot, network.ReusedSlotsPoolSize), + done: make(chan struct{}), + } +} + +func (p *V2Pool) createNetworkSlot(ctx context.Context) (*network.Slot, error) { + slot, err := p.storage.Acquire(ctx) + if err != nil { + return nil, fmt.Errorf("failed to acquire network slot: %w", err) + } + + slotV2 := NewSlotV2(slot) + p.registry.Store(slotV2) + + if err := CreateNetworkV2(ctx, slot, slotV2, p.hostFw, p.observer); err != nil { + p.registry.Delete(slot.Idx) + releaseErr := p.storage.Release(slot) + return nil, fmt.Errorf("failed to create v2 network: %w", errors.Join(err, releaseErr)) + } + + return slot, nil +} + +func (p *V2Pool) Populate(ctx context.Context) { + defer close(p.newSlots) + + for { + select { + case <-p.done: + return + case <-ctx.Done(): + return + default: + slot, err := p.createNetworkSlot(ctx) + if err != nil { + logger.L().Error(ctx, "[v2 network slot pool]: failed to create network", zap.Error(err)) + continue + } + + newSlotsAvailableCounter.Add(ctx, 1) + p.newSlots <- slot + } + } +} + +func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetworkConfig) (*network.Slot, error) { + var slot *network.Slot + + select { + case <-p.done: + return nil, network.ErrClosed + case s := <-p.reusedSlots: + reusableSlotsAvailableCounter.Add(ctx, -1) + acquiredSlots.Add(ctx, 1, metric.WithAttributes(attribute.String("pool", "reused"))) + telemetry.ReportEvent(ctx, "reused v2 network slot") + slot = s + default: + select { + case <-p.done: + return nil, network.ErrClosed + case <-ctx.Done(): + return nil, ctx.Err() + case s := <-p.newSlots: + newSlotsAvailableCounter.Add(ctx, -1) + acquiredSlots.Add(ctx, 1, metric.WithAttributes(attribute.String("pool", "new"))) + telemetry.ReportEvent(ctx, "new v2 network slot") + slot = s + } + } + + if err := slot.ConfigureInternet(ctx, netConfig); err != nil { + go func() { + if returnErr := p.Return(context.WithoutCancel(ctx), slot); returnErr != nil { + logger.L().Error(ctx, "failed to return v2 slot to pool", zap.Error(returnErr), zap.Int("slot_index", slot.Idx)) + } + }() + return nil, fmt.Errorf("error setting v2 slot internet access: %w", err) + } + + return slot, nil +} + +func (p *V2Pool) Return(ctx context.Context, slot *network.Slot) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-p.done: + return network.ErrClosed + default: + } + + if err := slot.ResetInternet(ctx); err != nil { + if cerr := p.cleanup(ctx, slot); cerr != nil { + return fmt.Errorf("reset internet: %w; cleanup: %w", err, cerr) + } + return fmt.Errorf("error resetting v2 slot internet access: %w", err) + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-p.done: + return network.ErrClosed + case p.reusedSlots <- slot: + returnedSlotCounter.Add(ctx, 1) + reusableSlotsAvailableCounter.Add(ctx, 1) + default: + if err := p.cleanup(ctx, slot); err != nil { + return fmt.Errorf("failed to return v2 slot '%d': %w", slot.Idx, err) + } + } + + return nil +} + +func (p *V2Pool) cleanup(ctx context.Context, slot *network.Slot) error { + var errs []error + + slotV2, ok := p.registry.Load(slot.Idx) + if ok { + if err := RemoveNetworkV2(ctx, slot, slotV2, p.hostFw, p.observer); err != nil { + errs = append(errs, fmt.Errorf("cannot remove v2 network for slot '%d': %w", slot.Idx, err)) + } + p.registry.Delete(slot.Idx) + } else { + // Fallback: try basic cleanup even without v2 metadata + if err := slot.RemoveNetwork(); err != nil { + errs = append(errs, fmt.Errorf("cannot remove network for slot '%d': %w", slot.Idx, err)) + } + } + + if err := p.storage.Release(slot); err != nil { + errs = append(errs, fmt.Errorf("failed to release slot '%d': %w", slot.Idx, err)) + } + + releasedSlotCounter.Add(ctx, 1) + + return errors.Join(errs...) +} + +func (p *V2Pool) Close(ctx context.Context) error { + logger.L().Info(ctx, "Closing v2 network pool") + + p.doneOnce.Do(func() { + close(p.done) + }) + + var errs []error + + for slot := range p.newSlots { + if err := p.cleanup(ctx, slot); err != nil { + errs = append(errs, fmt.Errorf("failed to cleanup v2 slot '%d': %w", slot.Idx, err)) + } + } + + close(p.reusedSlots) + + for slot := range p.reusedSlots { + if err := p.cleanup(ctx, slot); err != nil { + errs = append(errs, fmt.Errorf("failed to cleanup v2 slot '%d': %w", slot.Idx, err)) + } + } + + // Close host firewall table + if err := p.hostFw.Close(); err != nil { + errs = append(errs, fmt.Errorf("failed to close host firewall: %w", err)) + } + + // Close eBPF observer + if err := p.observer.Close(); err != nil { + errs = append(errs, fmt.Errorf("failed to close observer: %w", err)) + } + + return errors.Join(errs...) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot.go b/packages/orchestrator/pkg/sandbox/network/v2/slot.go new file mode 100644 index 0000000000..28b6745b27 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot.go @@ -0,0 +1,99 @@ +//go:build linux + +package v2 + +import ( + "fmt" + "sync" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +const ( + // fwMarkBase is the base fwmark for v2 slots. + // v2 slot N gets fwmark = fwMarkBase + Slot.Idx. + fwMarkBase = 0x200 +) + +// SlotV2 extends a base network.Slot with v2-specific metadata. +// It wraps (composition) rather than embeds so the base *Slot flows +// through the existing sandbox pipeline unchanged. +type SlotV2 struct { + Slot *network.Slot + + NetworkVersion int + SandboxID string + ExecutionID string + + // EgressProfileID links this slot to an EgressProfile for policy routing. + EgressProfileID string + + // PublishProfileID links this slot to a PublishProfile (stub). + PublishProfileID string + + // FwMark is the nftables/policy-routing mark for this slot's traffic. + FwMark uint32 + + // WgPeerIndex identifies the WireGuard peer for migration (0 = not set). + WgPeerIndex int +} + +// NewSlotV2 wraps a base Slot with v2 metadata. +func NewSlotV2(slot *network.Slot) *SlotV2 { + return &SlotV2{ + Slot: slot, + NetworkVersion: 2, + FwMark: fwMarkBase + uint32(slot.Idx), + } +} + +// FwMarkForIndex returns the fwmark that will be assigned to a slot with the given index. +func FwMarkForIndex(idx int) uint32 { + return fwMarkBase + uint32(idx) +} + +// SlotV2Registry tracks v2 metadata for slots by their index. +type SlotV2Registry struct { + mu sync.RWMutex + slots map[int]*SlotV2 +} + +func NewSlotV2Registry() *SlotV2Registry { + return &SlotV2Registry{ + slots: make(map[int]*SlotV2), + } +} + +func (r *SlotV2Registry) Store(slotV2 *SlotV2) { + r.mu.Lock() + defer r.mu.Unlock() + r.slots[slotV2.Slot.Idx] = slotV2 +} + +func (r *SlotV2Registry) Load(idx int) (*SlotV2, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + s, ok := r.slots[idx] + return s, ok +} + +func (r *SlotV2Registry) Delete(idx int) { + r.mu.Lock() + defer r.mu.Unlock() + delete(r.slots, idx) +} + +func (r *SlotV2Registry) Range(fn func(idx int, s *SlotV2) bool) { + r.mu.RLock() + defer r.mu.RUnlock() + for idx, s := range r.slots { + if !fn(idx, s) { + return + } + } +} + +// String returns a debug-friendly representation. +func (s *SlotV2) String() string { + return fmt.Sprintf("SlotV2{idx=%d, fwmark=0x%x, egress=%s}", s.Slot.Idx, s.FwMark, s.EgressProfileID) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go new file mode 100644 index 0000000000..557e0909b7 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -0,0 +1,114 @@ +//go:build linux + +package v2 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +func makeTestSlot(t *testing.T, idx int) *network.Slot { + t.Helper() + cfg := network.Config{ + OrchestratorInSandboxIPAddress: "192.0.2.1", + HyperloopProxyPort: 5010, + NFSProxyPort: 5011, + PortmapperPort: 5012, + SandboxTCPFirewallHTTPPort: 5016, + SandboxTCPFirewallTLSPort: 5017, + SandboxTCPFirewallOtherPort: 5018, + } + slot, err := network.NewSlot("test-key", idx, cfg) + require.NoError(t, err) + return slot +} + +func TestSlotV2_Creation(t *testing.T) { + slot := makeTestSlot(t, 1) + sv2 := NewSlotV2(slot) + + assert.Equal(t, 2, sv2.NetworkVersion) + assert.Equal(t, slot, sv2.Slot) + assert.Equal(t, uint32(fwMarkBase+1), sv2.FwMark) + assert.Equal(t, "", sv2.SandboxID) + assert.Equal(t, "", sv2.EgressProfileID) +} + +func TestSlotV2_FwMarkUniqueness(t *testing.T) { + marks := make(map[uint32]bool) + + for i := 1; i <= 100; i++ { + slot := makeTestSlot(t, i) + sv2 := NewSlotV2(slot) + + assert.False(t, marks[sv2.FwMark], "duplicate fwmark at index %d: 0x%x", i, sv2.FwMark) + marks[sv2.FwMark] = true + } +} + +func TestSlotV2_FwMarkForIndex(t *testing.T) { + assert.Equal(t, uint32(0x201), FwMarkForIndex(1)) + assert.Equal(t, uint32(0x264), FwMarkForIndex(100)) +} + +func TestSlotV2_String(t *testing.T) { + slot := makeTestSlot(t, 5) + sv2 := NewSlotV2(slot) + sv2.EgressProfileID = "test-profile" + + s := sv2.String() + assert.Contains(t, s, "idx=5") + assert.Contains(t, s, "fwmark=0x205") + assert.Contains(t, s, "egress=test-profile") +} + +func TestSlotV2Registry(t *testing.T) { + reg := NewSlotV2Registry() + + slot1 := makeTestSlot(t, 1) + sv2_1 := NewSlotV2(slot1) + sv2_1.SandboxID = "sb-1" + + slot2 := makeTestSlot(t, 2) + sv2_2 := NewSlotV2(slot2) + sv2_2.SandboxID = "sb-2" + + // Store + reg.Store(sv2_1) + reg.Store(sv2_2) + + // Load + loaded, ok := reg.Load(1) + assert.True(t, ok) + assert.Equal(t, "sb-1", loaded.SandboxID) + + _, ok = reg.Load(99) + assert.False(t, ok) + + // Delete + reg.Delete(1) + _, ok = reg.Load(1) + assert.False(t, ok) + + // Range + count := 0 + reg.Range(func(idx int, s *SlotV2) bool { + count++ + return true + }) + assert.Equal(t, 1, count) +} + +func TestSlotV2_EmbeddedSlotMethods(t *testing.T) { + slot := makeTestSlot(t, 3) + sv2 := NewSlotV2(slot) + + // Access base slot methods through composition + assert.Equal(t, "veth-3", sv2.Slot.VethName()) + assert.Equal(t, "ns-3", sv2.Slot.NamespaceID()) + assert.NotNil(t, sv2.Slot.HostIP) +} From 45abffd2cae1de78321fc8e00195aaee3201fff4 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Tue, 11 Aug 2026 17:10:09 +0000 Subject: [PATCH 02/10] feat(network/v2): NETWORK_VERSION gate + PoolInterface seam --- packages/orchestrator/pkg/factories/run.go | 39 ++++- .../pkg/sandbox/network/v2/host_firewall.go | 15 +- .../pkg/sandbox/network/v2/pool.go | 152 +++++++++++++++--- .../pkg/sandbox/network/v2/slot.go | 11 +- .../pkg/sandbox/network/v2/slot_test.go | 5 +- packages/orchestrator/pkg/sandbox/sandbox.go | 6 +- packages/orchestrator/pkg/server/main.go | 4 +- 7 files changed, 172 insertions(+), 60 deletions(-) diff --git a/packages/orchestrator/pkg/factories/run.go b/packages/orchestrator/pkg/factories/run.go index 8bb9723228..4f29c410f6 100644 --- a/packages/orchestrator/pkg/factories/run.go +++ b/packages/orchestrator/pkg/factories/run.go @@ -48,6 +48,7 @@ import ( "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/cgroup" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/nbd" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" + networkv2 "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network/v2" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/template" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/template/peerclient" "github.com/e2b-dev/infra/packages/orchestrator/pkg/server" @@ -718,12 +719,40 @@ func run(config cfg.Config, opts Options) (success bool) { if err != nil { logger.L().Fatal(ctx, "failed to create network pool", zap.Error(err)) } - networkPool := network.NewPool(network.NewSlotsPoolSize, network.ReusedSlotsPoolSize, slotStorage, config.NetworkConfig) - startService("network pool", func() error { - networkPool.Populate(ctx) + var networkPool network.PoolInterface + if config.NetworkConfig.NetworkVersion == 2 { + logger.L().Info(ctx, "using v2 network pool (nftables)") - return nil - }) + if err := networkv2.ValidateV2Prerequisites(); err != nil { + logger.L().Fatal(ctx, "v2 network prerequisites not met", zap.Error(err)) + } + + hostFw, hfErr := networkv2.NewHostFirewall(network.DefaultGateway(), config.NetworkConfig) + if hfErr != nil { + logger.L().Fatal(ctx, "failed to create v2 host firewall", zap.Error(hfErr)) + } + + observer, obsErr := networkv2.NewVethObserver() + if obsErr != nil { + logger.L().Fatal(ctx, "failed to create v2 veth observer", zap.Error(obsErr)) + } + + v2Pool := networkv2.NewV2Pool(slotStorage, config.NetworkConfig, hostFw, observer) + startService("network pool", func() error { + v2Pool.Populate(ctx) + + return nil + }) + networkPool = v2Pool + } else { + v1Pool := network.NewPool(network.NewSlotsPoolSize, network.ReusedSlotsPoolSize, slotStorage, config.NetworkConfig) + startService("network pool", func() error { + v1Pool.Populate(ctx) + + return nil + }) + networkPool = v1Pool + } closers = append(closers, closer{"network pool", networkPool.Close}) // sandbox factory diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index 700ffdb621..13d586b3da 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -59,10 +59,6 @@ type HostFirewall struct { config network.Config mu sync.Mutex - // migrationRules tracks active migration DNAT/forward rules by old host IP. - // Protected by mu. - migrationRules map[string]*migrationDNATEntry - // mangleChain is the prerouting mangle chain used for fwmark rules. // Stored here so fwmark rules can be added/removed without re-creating the chain. mangleChain *nftables.Chain @@ -98,12 +94,11 @@ func NewHostFirewall(defaultGw string, config network.Config) (*HostFirewall, er }) hf := &HostFirewall{ - conn: conn, - table: table, - defaultGw: defaultGw, - config: config, - migrationRules: make(map[string]*migrationDNATEntry), - fwmarkRules: make(map[string]uint64), + conn: conn, + table: table, + defaultGw: defaultGw, + config: config, + fwmarkRules: make(map[string]uint64), } if err := hf.ensureSets(); err != nil { diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go index 03a8df2fe1..76ce9cdc41 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -9,6 +9,7 @@ import ( "os" "strings" "sync" + "time" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -58,6 +59,13 @@ type V2Pool struct { reusedSlots chan *network.Slot done chan struct{} doneOnce sync.Once + + closeMu sync.RWMutex + closed bool + + // returnsWG tracks in-flight asynchronous slot returns; Close waits for + // it before draining the pool. + returnsWG sync.WaitGroup } // Compile-time check that V2Pool satisfies network.PoolInterface. @@ -145,7 +153,9 @@ func (p *V2Pool) Populate(ctx context.Context) { } } -func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetworkConfig) (*network.Slot, error) { +// Get acquires a slot. The egress class is accepted for interface parity with +// the v1 pool; v2 does not stamp egress DSCP. +func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetworkConfig, _ network.EgressClass) (*network.Slot, error) { var slot *network.Slot select { @@ -171,48 +181,124 @@ func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetwork } if err := slot.ConfigureInternet(ctx, netConfig); err != nil { - go func() { - if returnErr := p.Return(context.WithoutCancel(ctx), slot); returnErr != nil { - logger.L().Error(ctx, "failed to return v2 slot to pool", zap.Error(returnErr), zap.Int("slot_index", slot.Idx)) - } - }() + // Never handed out, so nobody listens for the release notification. + if rerr := p.ReturnAsync(context.WithoutCancel(ctx), slot, func(context.Context, string) {}, 0); rerr != nil { + logger.L().Error(ctx, "failed to return v2 slot to pool", zap.Error(rerr), zap.Int("slot_index", slot.Idx)) + } + return nil, fmt.Errorf("error setting v2 slot internet access: %w", err) } return slot, nil } -func (p *V2Pool) Return(ctx context.Context, slot *network.Slot) error { +func (p *V2Pool) tryTrackReturn() bool { + p.closeMu.RLock() + defer p.closeMu.RUnlock() + + if p.closed { + return false + } + + p.returnsWG.Add(1) + + return true +} + +// ReturnAsync recycles a slot in the background, logging errors instead of +// returning them. Close waits for all in-flight returns before draining the +// pool. If the pool is already closed the slot is cleaned up synchronously. +func (p *V2Pool) ReturnAsync(ctx context.Context, slot *network.Slot, releasedFn network.ReleaseNotify, returnDelay time.Duration) error { + if !p.tryTrackReturn() { + return p.returnSlot(ctx, slot, releasedFn, returnDelay) + } + + go func() { + defer p.returnsWG.Done() + + err := p.returnSlot(ctx, slot, releasedFn, returnDelay) + switch { + case err == nil: + case errors.Is(err, network.ErrClosed), errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded): + logger.L().Warn(ctx, "v2 network slot returned during pool shutdown", zap.Error(err), zap.Int("slot_index", slot.Idx)) + default: + logger.L().Error(ctx, "failed to return v2 network slot to pool", zap.Error(err), zap.Int("slot_index", slot.Idx)) + } + }() + + return nil +} + +// returnSlot recycles a slot that was used by a sandbox. It waits returnDelay +// before making the slot reusable to let inflight requests on the previous +// sandbox drain. +func (p *V2Pool) returnSlot(ctx context.Context, slot *network.Slot, releasedFn network.ReleaseNotify, returnDelay time.Duration) error { + notifyNetworkRelease := sync.OnceFunc(func() { + releasedFn(ctx, slot.HostIPString()) + }) + defer notifyNetworkRelease() + + // If the pool is closed or the context is cancelled during the delay we + // still fall through and clean up the slot to avoid leaking it. select { case <-ctx.Done(): - return ctx.Err() + return p.cleanupWith(ctx, slot, ctx.Err()) case <-p.done: - return network.ErrClosed - default: + return p.cleanupWith(ctx, slot, network.ErrClosed) + case <-time.After(returnDelay): } + notifyNetworkRelease() + + return p.recycle(ctx, slot) +} + +func (p *V2Pool) recycle(ctx context.Context, slot *network.Slot) error { if err := slot.ResetInternet(ctx); err != nil { - if cerr := p.cleanup(ctx, slot); cerr != nil { - return fmt.Errorf("reset internet: %w; cleanup: %w", err, cerr) - } - return fmt.Errorf("error resetting v2 slot internet access: %w", err) + return p.cleanupWith(ctx, slot, fmt.Errorf("error resetting v2 slot internet access: %w", err)) + } + + reused, cause := p.tryReuse(ctx, slot) + if reused { + return nil + } + + // cause is nil when the pool is simply full. + return p.cleanupWith(ctx, slot, cause) +} + +// tryReuse attempts to push the slot into the reused pool. The RLock pairs +// with Close's Lock so a send can never race the drain. +func (p *V2Pool) tryReuse(ctx context.Context, slot *network.Slot) (reused bool, cause error) { + p.closeMu.RLock() + defer p.closeMu.RUnlock() + + if p.closed { + return false, network.ErrClosed } select { case <-ctx.Done(): - return ctx.Err() + return false, ctx.Err() case <-p.done: - return network.ErrClosed + return false, network.ErrClosed case p.reusedSlots <- slot: returnedSlotCounter.Add(ctx, 1) reusableSlotsAvailableCounter.Add(ctx, 1) + + return true, nil default: - if err := p.cleanup(ctx, slot); err != nil { - return fmt.Errorf("failed to return v2 slot '%d': %w", slot.Idx, err) - } + return false, nil } +} - return nil +// cleanupWith tears the slot down and attaches any cleanup error to cause. +func (p *V2Pool) cleanupWith(ctx context.Context, slot *network.Slot, cause error) error { + if cerr := p.cleanup(ctx, slot); cerr != nil { + return errors.Join(cause, fmt.Errorf("cleanup v2 slot '%d': %w", slot.Idx, cerr)) + } + + return cause } func (p *V2Pool) cleanup(ctx context.Context, slot *network.Slot) error { @@ -247,28 +333,42 @@ func (p *V2Pool) Close(ctx context.Context) error { close(p.done) }) + p.closeMu.Lock() + p.closed = true + p.closeMu.Unlock() + + // Wait for in-flight asynchronous returns: each either cleans its slot + // up itself or has already pushed it into reusedSlots, drained below. + p.returnsWG.Wait() + var errs []error for slot := range p.newSlots { + newSlotsAvailableCounter.Add(ctx, -1) + if err := p.cleanup(ctx, slot); err != nil { errs = append(errs, fmt.Errorf("failed to cleanup v2 slot '%d': %w", slot.Idx, err)) } } - close(p.reusedSlots) +drain: + for { + select { + case slot := <-p.reusedSlots: + reusableSlotsAvailableCounter.Add(ctx, -1) - for slot := range p.reusedSlots { - if err := p.cleanup(ctx, slot); err != nil { - errs = append(errs, fmt.Errorf("failed to cleanup v2 slot '%d': %w", slot.Idx, err)) + if err := p.cleanup(ctx, slot); err != nil { + errs = append(errs, fmt.Errorf("failed to cleanup v2 slot '%d': %w", slot.Idx, err)) + } + default: + break drain } } - // Close host firewall table if err := p.hostFw.Close(); err != nil { errs = append(errs, fmt.Errorf("failed to close host firewall: %w", err)) } - // Close eBPF observer if err := p.observer.Close(); err != nil { errs = append(errs, fmt.Errorf("failed to close observer: %w", err)) } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot.go b/packages/orchestrator/pkg/sandbox/network/v2/slot.go index 28b6745b27..b3c49e4087 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot.go @@ -25,17 +25,8 @@ type SlotV2 struct { SandboxID string ExecutionID string - // EgressProfileID links this slot to an EgressProfile for policy routing. - EgressProfileID string - - // PublishProfileID links this slot to a PublishProfile (stub). - PublishProfileID string - // FwMark is the nftables/policy-routing mark for this slot's traffic. FwMark uint32 - - // WgPeerIndex identifies the WireGuard peer for migration (0 = not set). - WgPeerIndex int } // NewSlotV2 wraps a base Slot with v2 metadata. @@ -95,5 +86,5 @@ func (r *SlotV2Registry) Range(fn func(idx int, s *SlotV2) bool) { // String returns a debug-friendly representation. func (s *SlotV2) String() string { - return fmt.Sprintf("SlotV2{idx=%d, fwmark=0x%x, egress=%s}", s.Slot.Idx, s.FwMark, s.EgressProfileID) + return fmt.Sprintf("SlotV2{idx=%d, fwmark=0x%x}", s.Slot.Idx, s.FwMark) } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go index 557e0909b7..a87654bbab 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -22,7 +22,7 @@ func makeTestSlot(t *testing.T, idx int) *network.Slot { SandboxTCPFirewallTLSPort: 5017, SandboxTCPFirewallOtherPort: 5018, } - slot, err := network.NewSlot("test-key", idx, cfg) + slot, err := network.NewSlot("test-key", idx, cfg, network.NewNoopEgressProxy()) require.NoError(t, err) return slot } @@ -35,7 +35,6 @@ func TestSlotV2_Creation(t *testing.T) { assert.Equal(t, slot, sv2.Slot) assert.Equal(t, uint32(fwMarkBase+1), sv2.FwMark) assert.Equal(t, "", sv2.SandboxID) - assert.Equal(t, "", sv2.EgressProfileID) } func TestSlotV2_FwMarkUniqueness(t *testing.T) { @@ -58,12 +57,10 @@ func TestSlotV2_FwMarkForIndex(t *testing.T) { func TestSlotV2_String(t *testing.T) { slot := makeTestSlot(t, 5) sv2 := NewSlotV2(slot) - sv2.EgressProfileID = "test-profile" s := sv2.String() assert.Contains(t, s, "idx=5") assert.Contains(t, s, "fwmark=0x205") - assert.Contains(t, s, "egress=test-profile") } func TestSlotV2Registry(t *testing.T) { diff --git a/packages/orchestrator/pkg/sandbox/sandbox.go b/packages/orchestrator/pkg/sandbox/sandbox.go index e45333e293..f15eb34c21 100644 --- a/packages/orchestrator/pkg/sandbox/sandbox.go +++ b/packages/orchestrator/pkg/sandbox/sandbox.go @@ -455,7 +455,7 @@ func (m *Metadata) ExecutionDuration() (time.Duration, bool) { type Factory struct { Sandboxes *Map config cfg.BuilderConfig - networkPool *network.Pool + networkPool network.PoolInterface devicePool *nbd.DevicePool featureFlags *featureflags.Client hostStatsDelivery hoststats.Delivery @@ -466,7 +466,7 @@ type Factory struct { func NewFactory( config cfg.BuilderConfig, - networkPool *network.Pool, + networkPool network.PoolInterface, devicePool *nbd.DevicePool, featureFlags *featureflags.Client, hostStatsDelivery hoststats.Delivery, @@ -2318,7 +2318,7 @@ func createCgroup(ctx context.Context, cgroupManager cgroup.Manager, cgroupName func getNetworkSlot( ctx context.Context, - networkPool *network.Pool, + networkPool network.PoolInterface, cleanup *Cleanup, networkConfig *orchestrator.SandboxNetworkConfig, networkReleased network.ReleaseNotify, diff --git a/packages/orchestrator/pkg/server/main.go b/packages/orchestrator/pkg/server/main.go index c8f4f37584..8b71a4d158 100644 --- a/packages/orchestrator/pkg/server/main.go +++ b/packages/orchestrator/pkg/server/main.go @@ -68,7 +68,7 @@ type Server struct { sandboxFactory *sandbox.Factory info *service.ServiceInfo proxy *proxy.SandboxProxy - networkPool *network.Pool + networkPool network.PoolInterface templateCache *template.Cache devicePool *nbd.DevicePool persistence storage.StorageProvider @@ -101,7 +101,7 @@ type Server struct { type ServiceConfig struct { Config cfg.Config Tel *telemetry.Client - NetworkPool *network.Pool + NetworkPool network.PoolInterface DevicePool *nbd.DevicePool TemplateCache *template.Cache Info *service.ServiceInfo From f00d3040632be8d35041be95aa75da631c056518 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Tue, 11 Aug 2026 17:21:06 +0000 Subject: [PATCH 03/10] test(network): cover the version default; make v2 tests hermetic and lint-clean The ported observability tests assumed veth-1/veth-2 already existed on the host; they now create the links they observe. Kernel-touching tests carry the repo's paralleltest nolint with the shared resource named. NETWORK_VERSION absent or empty must parse as 1, so the datapath stays opt-in. --- .../sandbox/network/network_version_test.go | 41 ++++++++++++++++ .../pkg/sandbox/network/v2/host_firewall.go | 10 ++-- .../sandbox/network/v2/host_firewall_test.go | 33 +++++++------ .../pkg/sandbox/network/v2/network.go | 11 +++-- .../pkg/sandbox/network/v2/network_test.go | 27 +++++------ .../pkg/sandbox/network/v2/ns_nat.go | 1 + .../pkg/sandbox/network/v2/ns_nat_test.go | 10 ++-- .../pkg/sandbox/network/v2/observability.go | 4 ++ .../sandbox/network/v2/observability_test.go | 48 ++++++++++++++----- .../pkg/sandbox/network/v2/policy.go | 6 ++- .../pkg/sandbox/network/v2/policy_test.go | 22 ++++----- .../pkg/sandbox/network/v2/pool.go | 4 ++ .../pkg/sandbox/network/v2/slot.go | 1 + .../pkg/sandbox/network/v2/slot_test.go | 18 ++++++- 14 files changed, 170 insertions(+), 66 deletions(-) create mode 100644 packages/orchestrator/pkg/sandbox/network/network_version_test.go diff --git a/packages/orchestrator/pkg/sandbox/network/network_version_test.go b/packages/orchestrator/pkg/sandbox/network/network_version_test.go new file mode 100644 index 0000000000..553c44064d --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/network_version_test.go @@ -0,0 +1,41 @@ +//go:build linux + +package network + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// An absent NETWORK_VERSION must select v1: the v2 datapath is opt-in. +func TestParseConfig_NetworkVersion(t *testing.T) { //nolint:paralleltest // t.Setenv + t.Run("absent selects v1", func(t *testing.T) { //nolint:paralleltest // t.Setenv + config, err := ParseConfig() + require.NoError(t, err) + require.Equal(t, 1, config.NetworkVersion) + }) + + t.Run("set but empty selects v1", func(t *testing.T) { //nolint:paralleltest // t.Setenv + t.Setenv("NETWORK_VERSION", "") + + config, err := ParseConfig() + require.NoError(t, err) + require.Equal(t, 1, config.NetworkVersion) + }) + + t.Run("explicit 2 selects v2", func(t *testing.T) { //nolint:paralleltest // t.Setenv + t.Setenv("NETWORK_VERSION", "2") + + config, err := ParseConfig() + require.NoError(t, err) + require.Equal(t, 2, config.NetworkVersion) + }) + + t.Run("non-numeric fails loudly", func(t *testing.T) { //nolint:paralleltest // t.Setenv + t.Setenv("NETWORK_VERSION", "two") + + _, err := ParseConfig() + require.Error(t, err) + }) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index 13d586b3da..5f745470cc 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "fmt" "net" + "slices" "sync" "github.com/google/nftables" @@ -318,8 +319,8 @@ func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { // Remove stale veths (in set but not in active slots) var staleVethElems []nftables.SetElement for _, elem := range currentVeths { - name := string(elem.Key[:len(elem.Key)-1]) // strip null terminator - if !desiredVeths[name] { + // strip null terminator + if !desiredVeths[string(elem.Key[:len(elem.Key)-1])] { staleVethElems = append(staleVethElems, elem) } } @@ -493,23 +494,26 @@ func tcpCatchAllExprs(vethSet *nftables.Set, rport uint16) []expr.Any { func portBytes(port uint16) []byte { b := make([]byte, 2) binary.BigEndian.PutUint16(b, port) + return b } func incrementIP(ip net.IP) net.IP { result := make(net.IP, len(ip)) copy(result, ip) - for i := len(result) - 1; i >= 0; i-- { + for i := range slices.Backward(result) { result[i]++ if result[i] != 0 { break } } + return result } func bitmask32(v uint32) []byte { buf := make([]byte, 4) binary.NativeEndian.PutUint32(buf, v) + return buf } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go index 1957725db4..9de9ddac7e 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go @@ -24,7 +24,7 @@ func testConfig() network.Config { } } -func TestHostFirewall_Init(t *testing.T) { +func TestHostFirewall_Init(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) hf, err := NewHostFirewall("lo", testConfig()) @@ -37,7 +37,7 @@ func TestHostFirewall_Init(t *testing.T) { assert.NotNil(t, hf.cidrSet) } -func TestHostFirewall_AddRemoveSlot(t *testing.T) { +func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) hf, err := NewHostFirewall("lo", testConfig()) @@ -49,29 +49,29 @@ func TestHostFirewall_AddRemoveSlot(t *testing.T) { // Add slot err = hf.AddSlot(sv2) - assert.NoError(t, err) + require.NoError(t, err) // Verify veth set has the element elements, err := hf.conn.GetSetElements(hf.vethSet) require.NoError(t, err) - assert.Greater(t, len(elements), 0, "veth set should have elements") + assert.NotEmpty(t, elements, "veth set should have elements") // Verify cidr set has elements cidrElements, err := hf.conn.GetSetElements(hf.cidrSet) require.NoError(t, err) - assert.Greater(t, len(cidrElements), 0, "cidr set should have elements") + assert.NotEmpty(t, cidrElements, "cidr set should have elements") // Remove slot err = hf.RemoveSlot(sv2) - assert.NoError(t, err) + require.NoError(t, err) // Verify veth set is empty elements, err = hf.conn.GetSetElements(hf.vethSet) require.NoError(t, err) - assert.Equal(t, 0, len(elements), "veth set should be empty after removal") + assert.Empty(t, elements, "veth set should be empty after removal") } -func TestHostFirewall_ConcurrentAccess(t *testing.T) { +func TestHostFirewall_ConcurrentAccess(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) hf, err := NewHostFirewall("lo", testConfig()) @@ -81,25 +81,26 @@ func TestHostFirewall_ConcurrentAccess(t *testing.T) { // Add multiple slots concurrently done := make(chan error, 10) for i := 1; i <= 10; i++ { - go func(idx int) { - slot := makeTestSlot(t, idx) - sv2 := NewSlotV2(slot) + sv2 := NewSlotV2(makeTestSlot(t, i)) + go func() { done <- hf.AddSlot(sv2) - }(i) + }() } - for i := 0; i < 10; i++ { + for range 10 { err := <-done - assert.NoError(t, err, "concurrent AddSlot should not fail") + require.NoError(t, err, "concurrent AddSlot should not fail") } // Verify all 10 veths added elements, err := hf.conn.GetSetElements(hf.vethSet) require.NoError(t, err) - assert.Equal(t, 10, len(elements)) + assert.Len(t, elements, 10) } func TestIncrementIP(t *testing.T) { + t.Parallel() + tests := []struct { input string expected string @@ -118,6 +119,8 @@ func TestIncrementIP(t *testing.T) { } func TestPortBytes(t *testing.T) { + t.Parallel() + b := portBytes(80) assert.Equal(t, []byte{0, 80}, b) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go index 4b4c22e8f9..0edfb52215 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -27,8 +27,8 @@ import ( // - Host firewall: hf.AddSlot() replaces 8 iptables rules with 2 set element adds // - eBPF: observer.Attach() adds optional per-veth counters (best-effort) func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, - hf *HostFirewall, observer *VethObserver) error { - + hf *HostFirewall, observer *VethObserver, +) error { // Prevent thread changes so we can safely manipulate namespaces runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -163,6 +163,7 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, if err := SetupNamespaceNAT(nsConn, nsTable, slot.VpeerName(), slot.HostIPString(), slot.NamespaceIP()); err != nil { nsConn.CloseLasting() + return fmt.Errorf("setup namespace NAT: %w", err) } nsConn.CloseLasting() @@ -201,9 +202,9 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, } // RemoveNetworkV2 tears down the network for a v2 slot. -func RemoveNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, - hf *HostFirewall, observer *VethObserver) error { - +func RemoveNetworkV2(_ context.Context, slot *network.Slot, slotV2 *SlotV2, + hf *HostFirewall, observer *VethObserver, +) error { var errs []error // Close in-namespace firewall diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go index 7fed4c0f6e..059b3d1dda 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestCreateNetworkV2_FullLifecycle(t *testing.T) { +func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes skipIfNotLinuxRoot(t) ctx := context.Background() @@ -35,28 +35,28 @@ func TestCreateNetworkV2_FullLifecycle(t *testing.T) { require.NoError(t, err) // Verify namespace exists - out, err := exec.Command("ip", "netns", "list").Output() + out, err := exec.CommandContext(ctx, "ip", "netns", "list").Output() require.NoError(t, err) assert.Contains(t, string(out), slot.NamespaceID()) // Verify veth exists in host - out, err = exec.Command("ip", "link", "show", slot.VethName()).Output() + out, err = exec.CommandContext(ctx, "ip", "link", "show", slot.VethName()).Output() require.NoError(t, err) assert.Contains(t, string(out), slot.VethName()) // Teardown err = RemoveNetworkV2(ctx, slot, sv2, hf, observer) - assert.NoError(t, err) + require.NoError(t, err) // Verify namespace is gone — check each line for exact match - out, _ = exec.Command("ip", "netns", "list").Output() - for _, line := range strings.Split(string(out), "\n") { + out, _ = exec.CommandContext(ctx, "ip", "netns", "list").Output() + for line := range strings.SplitSeq(string(out), "\n") { assert.NotContains(t, line, slot.NamespaceID(), "namespace should be removed after teardown") } } -func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { +func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes skipIfNotLinuxRoot(t) ctx := context.Background() @@ -73,20 +73,20 @@ func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { defer RemoveNetworkV2(ctx, slot, sv2, hf, nil) // Verify NO iptables rules reference this veth - out, err := exec.Command("iptables", "-t", "nat", "-L", "PREROUTING", "-n").Output() + out, err := exec.CommandContext(ctx, "iptables", "-t", "nat", "-L", "PREROUTING", "-n").Output() if err == nil { assert.NotContains(t, string(out), slot.VethName(), "v2 sandbox should not create iptables rules") } - out, err = exec.Command("iptables", "-t", "filter", "-L", "FORWARD", "-n").Output() + out, err = exec.CommandContext(ctx, "iptables", "-t", "filter", "-L", "FORWARD", "-n").Output() if err == nil { assert.NotContains(t, string(out), slot.VethName(), "v2 sandbox should not create iptables FORWARD rules") } } -func TestCreateNetworkV2_CleanTeardown(t *testing.T) { +func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes skipIfNotLinuxRoot(t) ctx := context.Background() @@ -110,13 +110,12 @@ func TestCreateNetworkV2_CleanTeardown(t *testing.T) { // Verify veth set is empty elements, err := hf.conn.GetSetElements(hf.vethSet) require.NoError(t, err) - assert.Equal(t, 0, len(elements), "veth set should be empty after teardown") + assert.Empty(t, elements, "veth set should be empty after teardown") // Verify no routes to the slot's host IP - out, err := exec.Command("ip", "route", "show").Output() + out, err := exec.CommandContext(ctx, "ip", "route", "show").Output() require.NoError(t, err) - lines := strings.Split(string(out), "\n") - for _, line := range lines { + for line := range strings.SplitSeq(string(out), "\n") { assert.NotContains(t, line, slot.HostIPString(), "should have no route to slot host IP after teardown") } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go index 8a3c42016d..da279bb9e2 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat.go @@ -138,5 +138,6 @@ func SetupNamespaceNAT(conn *nftables.Conn, table *nftables.Table, vpeerIface, h func ifnameBytes(name string) []byte { b := make([]byte, 16) // IFNAMSIZ copy(b, name) + return b } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go index 9400bd25bd..93ac6894a4 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/ns_nat_test.go @@ -22,7 +22,7 @@ func skipIfNotLinuxRoot(t *testing.T) { } } -func TestNamespaceNAT_Setup(t *testing.T) { +func TestNamespaceNAT_Setup(t *testing.T) { //nolint:paralleltest // creates the nftables table "test-nat" in the host netns and lists chains family-wide skipIfNotLinuxRoot(t) conn, err := nftables.New(nftables.AsLasting()) @@ -40,7 +40,7 @@ func TestNamespaceNAT_Setup(t *testing.T) { }() err = SetupNamespaceNAT(conn, table, "eth0", "10.11.0.1", "169.254.0.21") - assert.NoError(t, err) + require.NoError(t, err) // Verify chains were created chains, err := conn.ListChainsOfTableFamily(nftables.TableFamilyINet) @@ -56,7 +56,7 @@ func TestNamespaceNAT_Setup(t *testing.T) { assert.True(t, chainNames["preroute_nat"], "preroute_nat chain should exist") } -func TestNamespaceNAT_InvalidIP(t *testing.T) { +func TestNamespaceNAT_InvalidIP(t *testing.T) { //nolint:paralleltest // creates the nftables table "test-nat-invalid" in the host netns skipIfNotLinuxRoot(t) conn, err := nftables.New(nftables.AsLasting()) @@ -74,11 +74,13 @@ func TestNamespaceNAT_InvalidIP(t *testing.T) { }() err = SetupNamespaceNAT(conn, table, "eth0", "not-an-ip", "169.254.0.21") - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "invalid host IP") } func TestIfnameBytes(t *testing.T) { + t.Parallel() + b := ifnameBytes("eth0") assert.Len(t, b, 16) assert.Equal(t, byte('e'), b[0]) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability.go b/packages/orchestrator/pkg/sandbox/network/v2/observability.go index cc102c2bec..cd7ace2e44 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/observability.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability.go @@ -42,6 +42,7 @@ func (o *VethObserver) Attach(vethName string) error { return fmt.Errorf("interface %s not found: %w", vethName, err) } o.attached[vethName] = true + return nil } @@ -54,6 +55,7 @@ func (o *VethObserver) Detach(vethName string) error { defer o.mu.Unlock() delete(o.attached, vethName) + return nil } @@ -86,6 +88,7 @@ func (o *VethObserver) ReadCounters(vethName string) (packets, bytes uint64, err // Host veth tx = sandbox rx (internet → sandbox). packets = stats.RxPackets + stats.TxPackets bytes = stats.RxBytes + stats.TxBytes + return packets, bytes, nil } @@ -127,5 +130,6 @@ func (o *VethObserver) Close() error { for name := range o.attached { delete(o.attached, name) } + return nil } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go index 3c437d401d..4f70e1840a 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go @@ -7,48 +7,71 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vishvananda/netlink" ) -func TestVethObserver_AttachDetach(t *testing.T) { +// createTestVeth brings up a real veth pair; Attach and ReadCounters resolve +// the interface through netlink, so a name with no link behind it fails. +func createTestVeth(t *testing.T, name string) { + t.Helper() + + link := &netlink.Veth{ + LinkAttrs: netlink.LinkAttrs{Name: name}, + PeerName: name + "-peer", + } + + require.NoError(t, netlink.LinkAdd(link)) + t.Cleanup(func() { + _ = netlink.LinkDel(link) + }) +} + +func TestVethObserver_AttachDetach(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth link "veth-1" shared across VethObserver tests + createTestVeth(t, "veth-1") + obs, err := NewVethObserver() require.NoError(t, err) defer obs.Close() // Attach err = obs.Attach("veth-1") - assert.NoError(t, err) + require.NoError(t, err) // Double attach should fail err = obs.Attach("veth-1") - assert.Error(t, err) + require.Error(t, err) // Detach err = obs.Detach("veth-1") - assert.NoError(t, err) + require.NoError(t, err) // Double detach is idempotent err = obs.Detach("veth-1") assert.NoError(t, err) } -func TestVethObserver_ReadCounters(t *testing.T) { +func TestVethObserver_ReadCounters(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth link "veth-1" shared across VethObserver tests + createTestVeth(t, "veth-1") + obs, err := NewVethObserver() require.NoError(t, err) defer obs.Close() // Not attached → error _, _, err = obs.ReadCounters("veth-1") - assert.Error(t, err) + require.Error(t, err) // Attach and read require.NoError(t, obs.Attach("veth-1")) packets, bytes, err := obs.ReadCounters("veth-1") - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, uint64(0), packets) assert.Equal(t, uint64(0), bytes) } func TestVethObserver_NilSafe(t *testing.T) { + t.Parallel() + var obs *VethObserver assert.NoError(t, obs.Attach("veth-1")) @@ -56,12 +79,15 @@ func TestVethObserver_NilSafe(t *testing.T) { assert.NoError(t, obs.Close()) p, b, err := obs.ReadCounters("veth-1") - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, uint64(0), p) assert.Equal(t, uint64(0), b) } -func TestVethObserver_Close(t *testing.T) { +func TestVethObserver_Close(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth links "veth-1"/"veth-2" shared across VethObserver tests + createTestVeth(t, "veth-1") + createTestVeth(t, "veth-2") + obs, err := NewVethObserver() require.NoError(t, err) @@ -69,10 +95,10 @@ func TestVethObserver_Close(t *testing.T) { require.NoError(t, obs.Attach("veth-2")) err = obs.Close() - assert.NoError(t, err) + require.NoError(t, err) // After close, nothing should be attached obs.mu.Lock() - assert.Equal(t, 0, len(obs.attached)) + assert.Empty(t, obs.attached) obs.mu.Unlock() } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy.go b/packages/orchestrator/pkg/sandbox/network/v2/policy.go index 2e3eff2db4..158a1a8c29 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/policy.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/policy.go @@ -16,7 +16,7 @@ import ( // for fwmark-based policy routing of forwarded traffic. Without this, the kernel // may not re-route forwarded packets based on marks set in prerouting. func EnsureSrcValidMark() error { - return os.WriteFile("/proc/sys/net/ipv4/conf/all/src_valid_mark", []byte("1"), 0644) + return os.WriteFile("/proc/sys/net/ipv4/conf/all/src_valid_mark", []byte("1"), 0o644) } // SetupPolicyRoute creates a policy routing rule and route table entry @@ -137,6 +137,7 @@ func SetupFwmarkInNftables(hf *HostFirewall, vethName string, fwmark uint32) err for _, r := range rules { if matchesFwmarkRule(r, vethName) { hf.fwmarkRules[vethName] = r.Handle + break } } @@ -160,6 +161,7 @@ func matchesFwmarkRule(r *nftables.Rule, vethName string) bool { for j := range cmp.Data { if cmp.Data[j] != expected[j] { match = false + break } } @@ -169,6 +171,7 @@ func matchesFwmarkRule(r *nftables.Rule, vethName string) bool { } } } + return false } @@ -194,5 +197,6 @@ func RemoveFwmarkInNftables(hf *HostFirewall, vethName string) error { } delete(hf.fwmarkRules, vethName) + return nil } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go b/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go index 24d46d3b94..02918b7b41 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go @@ -12,21 +12,22 @@ import ( "github.com/stretchr/testify/require" ) -func TestPolicyRoute_Setup(t *testing.T) { +func TestPolicyRoute_Setup(t *testing.T) { //nolint:paralleltest // mutates host netns state: fixed-name dummy link "dummy-test", policy rule and route table 300 skipIfNotLinuxRoot(t) + ctx := t.Context() fwmark := uint32(0x300) tableID := 300 // Setup: create a dummy device to route through - err := exec.Command("ip", "link", "add", "dummy-test", "type", "dummy").Run() + err := exec.CommandContext(ctx, "ip", "link", "add", "dummy-test", "type", "dummy").Run() require.NoError(t, err) - defer exec.Command("ip", "link", "del", "dummy-test").Run() + defer exec.CommandContext(ctx, "ip", "link", "del", "dummy-test").Run() - err = exec.Command("ip", "link", "set", "dummy-test", "up").Run() + err = exec.CommandContext(ctx, "ip", "link", "set", "dummy-test", "up").Run() require.NoError(t, err) - err = exec.Command("ip", "addr", "add", "10.99.99.1/24", "dev", "dummy-test").Run() + err = exec.CommandContext(ctx, "ip", "addr", "add", "10.99.99.1/24", "dev", "dummy-test").Run() require.NoError(t, err) // Test setup @@ -35,24 +36,23 @@ func TestPolicyRoute_Setup(t *testing.T) { require.NoError(t, err) // Verify ip rule exists - out, err := exec.Command("ip", "rule", "list").Output() + out, err := exec.CommandContext(ctx, "ip", "rule", "list").Output() require.NoError(t, err) assert.Contains(t, string(out), "lookup 300") // Verify route table has route - out, err = exec.Command("ip", "route", "show", "table", "300").Output() + out, err = exec.CommandContext(ctx, "ip", "route", "show", "table", "300").Output() require.NoError(t, err) assert.Contains(t, string(out), "dummy-test") // Teardown err = TeardownPolicyRoute(fwmark, tableID) - assert.NoError(t, err) + require.NoError(t, err) // Verify rule is gone - out, err = exec.Command("ip", "rule", "list").Output() + out, err = exec.CommandContext(ctx, "ip", "rule", "list").Output() require.NoError(t, err) - lines := strings.Split(string(out), "\n") - for _, line := range lines { + for line := range strings.SplitSeq(string(out), "\n") { assert.NotContains(t, line, "lookup 300", "policy rule should be removed after teardown") } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go index 76ce9cdc41..328764bfa0 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -88,6 +88,7 @@ func ValidateV2Prerequisites() error { val, err := os.ReadFile(c.path) if err != nil { errs = append(errs, fmt.Errorf("cannot read %s: %w", c.path, err)) + continue } if strings.TrimSpace(string(val)) != c.expected { @@ -97,6 +98,7 @@ func ValidateV2Prerequisites() error { if len(errs) > 0 { return fmt.Errorf("v2 networking prerequisites not met: %w", errors.Join(errs...)) } + return nil } @@ -125,6 +127,7 @@ func (p *V2Pool) createNetworkSlot(ctx context.Context) (*network.Slot, error) { if err := CreateNetworkV2(ctx, slot, slotV2, p.hostFw, p.observer); err != nil { p.registry.Delete(slot.Idx) releaseErr := p.storage.Release(slot) + return nil, fmt.Errorf("failed to create v2 network: %w", errors.Join(err, releaseErr)) } @@ -144,6 +147,7 @@ func (p *V2Pool) Populate(ctx context.Context) { slot, err := p.createNetworkSlot(ctx) if err != nil { logger.L().Error(ctx, "[v2 network slot pool]: failed to create network", zap.Error(err)) + continue } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot.go b/packages/orchestrator/pkg/sandbox/network/v2/slot.go index b3c49e4087..0ab9cbee93 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot.go @@ -65,6 +65,7 @@ func (r *SlotV2Registry) Load(idx int) (*SlotV2, bool) { r.mu.RLock() defer r.mu.RUnlock() s, ok := r.slots[idx] + return s, ok } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go index a87654bbab..30b0225834 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -24,20 +24,25 @@ func makeTestSlot(t *testing.T, idx int) *network.Slot { } slot, err := network.NewSlot("test-key", idx, cfg, network.NewNoopEgressProxy()) require.NoError(t, err) + return slot } func TestSlotV2_Creation(t *testing.T) { + t.Parallel() + slot := makeTestSlot(t, 1) sv2 := NewSlotV2(slot) assert.Equal(t, 2, sv2.NetworkVersion) assert.Equal(t, slot, sv2.Slot) assert.Equal(t, uint32(fwMarkBase+1), sv2.FwMark) - assert.Equal(t, "", sv2.SandboxID) + assert.Empty(t, sv2.SandboxID) } func TestSlotV2_FwMarkUniqueness(t *testing.T) { + t.Parallel() + marks := make(map[uint32]bool) for i := 1; i <= 100; i++ { @@ -50,11 +55,15 @@ func TestSlotV2_FwMarkUniqueness(t *testing.T) { } func TestSlotV2_FwMarkForIndex(t *testing.T) { + t.Parallel() + assert.Equal(t, uint32(0x201), FwMarkForIndex(1)) assert.Equal(t, uint32(0x264), FwMarkForIndex(100)) } func TestSlotV2_String(t *testing.T) { + t.Parallel() + slot := makeTestSlot(t, 5) sv2 := NewSlotV2(slot) @@ -64,6 +73,8 @@ func TestSlotV2_String(t *testing.T) { } func TestSlotV2Registry(t *testing.T) { + t.Parallel() + reg := NewSlotV2Registry() slot1 := makeTestSlot(t, 1) @@ -93,14 +104,17 @@ func TestSlotV2Registry(t *testing.T) { // Range count := 0 - reg.Range(func(idx int, s *SlotV2) bool { + reg.Range(func(_ int, _ *SlotV2) bool { count++ + return true }) assert.Equal(t, 1, count) } func TestSlotV2_EmbeddedSlotMethods(t *testing.T) { + t.Parallel() + slot := makeTestSlot(t, 3) sv2 := NewSlotV2(slot) From c1a38d01267b63c59411e25343c86afb67a44880 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Tue, 11 Aug 2026 17:30:41 +0000 Subject: [PATCH 04/10] test(network/v2): reserve a netns index range disjoint from v1 go test ./... runs the two network packages concurrently and the ported tests hardcoded indices inside the range v1's suite allocates from, so namespace creation raced on /run/netns/ns-30001. Observer veth links are named per-test for the same reason. --- .../pkg/sandbox/network/v2/network_test.go | 7 +-- .../sandbox/network/v2/observability_test.go | 57 ++++++++++++------- .../pkg/sandbox/network/v2/slot_test.go | 12 ++++ 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go index 059b3d1dda..38bded0fc4 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -26,8 +26,7 @@ func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // require.NoError(t, err) defer observer.Close() - // Use high index to avoid conflicts with running orchestrator's namespaces - slot := makeTestSlot(t, 30001) + slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) // Create network @@ -65,7 +64,7 @@ func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest / require.NoError(t, err) defer hf.Close() - slot := makeTestSlot(t, 30002) + slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) err = CreateNetworkV2(ctx, slot, sv2, hf, nil) @@ -98,7 +97,7 @@ func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // observer, _ := NewVethObserver() defer observer.Close() - slot := makeTestSlot(t, 30003) + slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) err = CreateNetworkV2(ctx, slot, sv2, hf, observer) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go index 4f70e1840a..6635a2d425 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go @@ -3,6 +3,7 @@ package v2 import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -10,11 +11,14 @@ import ( "github.com/vishvananda/netlink" ) -// createTestVeth brings up a real veth pair; Attach and ReadCounters resolve -// the interface through netlink, so a name with no link behind it fails. -func createTestVeth(t *testing.T, name string) { +// createTestVeth brings up a real veth pair and returns its host-side name; +// Attach and ReadCounters resolve the interface through netlink, so a name +// with no link behind it fails. The name is unique per call so a concurrent +// package, a repeat run, or a live orchestrator cannot collide with it. +func createTestVeth(t *testing.T) string { t.Helper() + name := fmt.Sprintf("vt-%d", reserveNSTestIdx(t)) link := &netlink.Veth{ LinkAttrs: netlink.LinkAttrs{Name: name}, PeerName: name + "-peer", @@ -24,46 +28,52 @@ func createTestVeth(t *testing.T, name string) { t.Cleanup(func() { _ = netlink.LinkDel(link) }) + + return name } -func TestVethObserver_AttachDetach(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth link "veth-1" shared across VethObserver tests - createTestVeth(t, "veth-1") +func TestVethObserver_AttachDetach(t *testing.T) { + t.Parallel() + + veth := createTestVeth(t) obs, err := NewVethObserver() require.NoError(t, err) defer obs.Close() // Attach - err = obs.Attach("veth-1") + err = obs.Attach(veth) require.NoError(t, err) // Double attach should fail - err = obs.Attach("veth-1") + err = obs.Attach(veth) require.Error(t, err) // Detach - err = obs.Detach("veth-1") + err = obs.Detach(veth) require.NoError(t, err) // Double detach is idempotent - err = obs.Detach("veth-1") + err = obs.Detach(veth) assert.NoError(t, err) } -func TestVethObserver_ReadCounters(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth link "veth-1" shared across VethObserver tests - createTestVeth(t, "veth-1") +func TestVethObserver_ReadCounters(t *testing.T) { + t.Parallel() + + veth := createTestVeth(t) obs, err := NewVethObserver() require.NoError(t, err) defer obs.Close() // Not attached → error - _, _, err = obs.ReadCounters("veth-1") + _, _, err = obs.ReadCounters(veth) require.Error(t, err) // Attach and read - require.NoError(t, obs.Attach("veth-1")) - packets, bytes, err := obs.ReadCounters("veth-1") + require.NoError(t, obs.Attach(veth)) + packets, bytes, err := obs.ReadCounters(veth) require.NoError(t, err) assert.Equal(t, uint64(0), packets) assert.Equal(t, uint64(0), bytes) @@ -74,25 +84,28 @@ func TestVethObserver_NilSafe(t *testing.T) { var obs *VethObserver - assert.NoError(t, obs.Attach("veth-1")) - assert.NoError(t, obs.Detach("veth-1")) + // A nil observer returns before touching netlink, so the name need not exist. + assert.NoError(t, obs.Attach("vt-absent")) + assert.NoError(t, obs.Detach("vt-absent")) assert.NoError(t, obs.Close()) - p, b, err := obs.ReadCounters("veth-1") + p, b, err := obs.ReadCounters("vt-absent") require.NoError(t, err) assert.Equal(t, uint64(0), p) assert.Equal(t, uint64(0), b) } -func TestVethObserver_Close(t *testing.T) { //nolint:paralleltest // creates the fixed-name host veth links "veth-1"/"veth-2" shared across VethObserver tests - createTestVeth(t, "veth-1") - createTestVeth(t, "veth-2") +func TestVethObserver_Close(t *testing.T) { + t.Parallel() + + veth1 := createTestVeth(t) + veth2 := createTestVeth(t) obs, err := NewVethObserver() require.NoError(t, err) - require.NoError(t, obs.Attach("veth-1")) - require.NoError(t, obs.Attach("veth-2")) + require.NoError(t, obs.Attach(veth1)) + require.NoError(t, obs.Attach(veth2)) err = obs.Close() require.NoError(t, err) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go index 30b0225834..b3dcd89d86 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -3,6 +3,7 @@ package v2 import ( + "sync/atomic" "testing" "github.com/stretchr/testify/assert" @@ -11,6 +12,17 @@ import ( "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" ) +// Slot indices for tests that create real netns/veth state. The v1 package +// reserves 30000+ for the same purpose and `go test ./...` runs the two +// packages concurrently, so the ranges must not overlap. +var nsTestIdx atomic.Int32 + +func reserveNSTestIdx(t *testing.T) int { + t.Helper() + + return 31000 + int(nsTestIdx.Add(1)) +} + func makeTestSlot(t *testing.T, idx int) *network.Slot { t.Helper() cfg := network.Config{ From f7c46a06c675b30442d6c9b8f4c37f6de4b0e301 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 09:02:34 +0000 Subject: [PATCH 05/10] feat(network/v2): stamp egress DSCP per EgressClass (v1 parity) --- .../pkg/sandbox/network/v2/dscp.go | 217 ++++++++++++ .../sandbox/network/v2/dscp_egress_test.go | 328 ++++++++++++++++++ .../pkg/sandbox/network/v2/network.go | 8 + .../pkg/sandbox/network/v2/pool.go | 27 +- 4 files changed, 575 insertions(+), 5 deletions(-) create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/dscp.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go diff --git a/packages/orchestrator/pkg/sandbox/network/v2/dscp.go b/packages/orchestrator/pkg/sandbox/network/v2/dscp.go new file mode 100644 index 0000000000..01ff70a286 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/dscp.go @@ -0,0 +1,217 @@ +//go:build linux + +package v2 + +import ( + "bytes" + "fmt" + "path/filepath" + + "github.com/containernetworking/plugins/pkg/ns" + "github.com/google/nftables" + "github.com/google/nftables/expr" + "golang.org/x/sys/unix" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +const ( + maxDSCP = 63 // DSCP is the top 6 bits of the IPv4 TOS byte. + + // nsMangleChainName is the postrouting mangle chain in the in-namespace + // "slot-firewall" table, holding the egress DSCP rule — the same hook + // v1's iptables mangle/POSTROUTING rule uses, so the TOS byte on the + // veth link is identical between versions. + nsMangleChainName = "postroute_mangle" +) + +// SetupEgressDSCP creates the in-namespace postrouting mangle chain and, for +// a non-zero value, the rule stamping DSCP on everything leaving the sandbox +// netns through the vpeer uplink. Must run inside the namespace, on the conn +// SetupNamespaceNAT uses. +func SetupEgressDSCP(conn *nftables.Conn, table *nftables.Table, vpeerIface string, dscp uint8) error { + if dscp > maxDSCP { + return fmt.Errorf("egress DSCP %d out of range (0..%d)", dscp, maxDSCP) + } + + chain := conn.AddChain(&nftables.Chain{ + Name: nsMangleChainName, + Table: table, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookPostrouting, + Priority: nftables.ChainPriorityMangle, + }) + + if dscp > 0 { + conn.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: dscpSetExprs(vpeerIface, dscp), + }) + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush egress DSCP setup: %w", err) + } + + return nil +} + +// ApplyEgressDSCP re-stamps the slot's egress rule so a build can be marked +// differently from a regular sandbox, mirroring v1's semantics: 0 removes the +// rule, a matching installed value is a no-op, and the add and delete land in +// one atomic batch so egress is never unmarked during a restamp and a failed +// apply keeps the previous class installed. The kernel ruleset is the source +// of truth for the currently stamped value. +func ApplyEgressDSCP(slot *network.Slot, dscp uint8) error { + if dscp > maxDSCP { + return fmt.Errorf("egress DSCP %d out of range (0..%d)", dscp, maxDSCP) + } + + nsHandle, err := ns.GetNS(filepath.Join(network.NetNamespacesDir, slot.NamespaceID())) + if err != nil { + return fmt.Errorf("get slot network namespace %q: %w", slot.NamespaceID(), err) + } + defer nsHandle.Close() + + return nsHandle.Do(func(_ ns.NetNS) error { + conn, err := nftables.New(nftables.AsLasting()) + if err != nil { + return fmt.Errorf("nftables conn in namespace: %w", err) + } + defer conn.CloseLasting() + + table := &nftables.Table{Name: "slot-firewall", Family: nftables.TableFamilyINet} + chain := &nftables.Chain{Name: nsMangleChainName, Table: table} + + current, currentDSCP, err := findDSCPRule(conn, table, chain, slot.VpeerName()) + if err != nil { + return fmt.Errorf("find DSCP rule for %s: %w", slot.NamespaceID(), err) + } + if current == nil && dscp == 0 { + return nil + } + if current != nil && currentDSCP == dscp { + return nil + } + + if dscp > 0 { + conn.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: dscpSetExprs(slot.VpeerName(), dscp), + }) + } + if current != nil { + conn.DelRule(&nftables.Rule{ + Table: table, + Chain: chain, + Handle: current.Handle, + }) + } + + if err := conn.Flush(); err != nil { + return fmt.Errorf("flush DSCP %d rule for %s: %w", dscp, slot.NamespaceID(), err) + } + + return nil + }) +} + +// findDSCPRule returns the vpeer's DSCP rule and its stamped value, or nil +// when none is installed. +func findDSCPRule(conn *nftables.Conn, table *nftables.Table, chain *nftables.Chain, vpeerIface string) (*nftables.Rule, uint8, error) { + rules, err := conn.GetRules(table, chain) + if err != nil { + return nil, 0, fmt.Errorf("get mangle rules: %w", err) + } + + for _, r := range rules { + if dscp, ok := dscpRuleValue(r, vpeerIface); ok { + return r, dscp, nil + } + } + + return nil, 0, nil +} + +// dscpRuleValue reports whether r is the DSCP rule for the given uplink — an +// oifname match plus a payload write — and decodes the DSCP it stamps. +func dscpRuleValue(r *nftables.Rule, vpeerIface string) (uint8, bool) { + var tos uint8 + writes := false + + for _, e := range r.Exprs { + switch v := e.(type) { + case *expr.Payload: + if v.OperationType == expr.PayloadWrite { + writes = true + } + case *expr.Bitwise: + if len(v.Xor) == 1 { + tos = v.Xor[0] + } + } + } + + if !writes || !hasOifnameCmp(r, vpeerIface) { + return 0, false + } + + return tos >> 2, true +} + +// hasOifnameCmp reports whether the rule compares oifname against the given +// interface name. +func hasOifnameCmp(r *nftables.Rule, name string) bool { + expected := ifnameBytes(name) + for i, e := range r.Exprs { + if i == 0 { + continue + } + cmp, ok := e.(*expr.Cmp) + if !ok { + continue + } + if meta, ok := r.Exprs[i-1].(*expr.Meta); ok && meta.Key == expr.MetaKeyOIFNAME && bytes.Equal(cmp.Data, expected) { + return true + } + } + + return false +} + +// dscpSetExprs builds: meta nfproto ipv4 oifname ip dscp set — +// a write of the TOS byte that preserves the ECN bits and fixes the IPv4 +// header checksum, equivalent to v1's `-o -j DSCP --set-dscp `. +func dscpSetExprs(vpeerIface string, dscp uint8) []expr.Any { + return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: []byte{unix.NFPROTO_IPV4}}, + &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, + &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: ifnameBytes(vpeerIface)}, + &expr.Payload{ + OperationType: expr.PayloadLoad, + DestRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: 1, // TOS byte in the IPv4 header + Len: 1, + }, + &expr.Bitwise{ + SourceRegister: 1, + DestRegister: 1, + Len: 1, + Mask: []byte{0x03}, + Xor: []byte{dscp << 2}, + }, + &expr.Payload{ + OperationType: expr.PayloadWrite, + SourceRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: 1, + Len: 1, + CsumType: expr.CsumTypeInet, + CsumOffset: 10, // IPv4 header checksum + }, + } +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go new file mode 100644 index 0000000000..61e4d3ad15 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go @@ -0,0 +1,328 @@ +//go:build linux + +package v2 + +import ( + "context" + "net" + "path/filepath" + "runtime" + "testing" + + "github.com/containernetworking/plugins/pkg/ns" + "github.com/google/nftables" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vishvananda/netlink" + "github.com/vishvananda/netns" + "golang.org/x/sys/unix" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +// dscpConfig builds the host-firewall test config with the DSCP fields parsed +// from the real env path, so the nil-inherits semantics of +// BUILD_SANDBOX_EGRESS_DSCP are exercised end to end. A set-but-empty build +// value behaves as unset (the env parser skips it), so "" drives the +// inherits-sandbox path. +func dscpConfig(t *testing.T, sandboxDSCP, buildDSCP string) network.Config { + t.Helper() + + t.Setenv("SANDBOX_EGRESS_DSCP", sandboxDSCP) + t.Setenv("BUILD_SANDBOX_EGRESS_DSCP", buildDSCP) + + parsed, err := network.ParseConfig() + require.NoError(t, err) + + config := testConfig() + config.SandboxEgressDSCP = parsed.SandboxEgressDSCP + config.BuildSandboxEgressDSCP = parsed.BuildSandboxEgressDSCP + + return config +} + +// makeDSCPTestNS creates the slot's named namespace with the slot-firewall +// table and the seeded DSCP chain, as CreateNetworkV2 does, without the rest +// of the datapath. +func makeDSCPTestNS(t *testing.T, slot *network.Slot, config network.Config) { + t.Helper() + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + host, err := netns.Get() + require.NoError(t, err) + defer func() { + require.NoError(t, netns.Set(host)) + _ = host.Close() + }() + + nsHandle, err := netns.NewNamed(slot.NamespaceID()) + require.NoError(t, err) + defer nsHandle.Close() + t.Cleanup(func() { _ = netns.DeleteNamed(slot.NamespaceID()) }) + + conn, err := nftables.New(nftables.AsLasting()) + require.NoError(t, err) + defer conn.CloseLasting() + + table := conn.AddTable(&nftables.Table{Name: "slot-firewall", Family: nftables.TableFamilyINet}) + require.NoError(t, SetupEgressDSCP(conn, table, slot.VpeerName(), config.EgressDSCP(network.EgressClassSandbox))) +} + +// requireDSCPByte asserts the slot's in-namespace mangle rule stamps wantTOS — +// the TOS byte v1's EgressTOS resolution produces. wantTOS 0 asserts no rule. +func requireDSCPByte(t *testing.T, slot *network.Slot, wantTOS int) { + t.Helper() + + nsHandle, err := ns.GetNS(filepath.Join(network.NetNamespacesDir, slot.NamespaceID())) + require.NoError(t, err) + defer nsHandle.Close() + + var got []uint8 + require.NoError(t, nsHandle.Do(func(_ ns.NetNS) error { + conn, err := nftables.New(nftables.AsLasting()) + if err != nil { + return err + } + defer conn.CloseLasting() + + table := &nftables.Table{Name: "slot-firewall", Family: nftables.TableFamilyINet} + chain := &nftables.Chain{Name: nsMangleChainName, Table: table} + rules, err := conn.GetRules(table, chain) + if err != nil { + return err + } + for _, r := range rules { + if dscp, ok := dscpRuleValue(r, slot.VpeerName()); ok { + got = append(got, dscp) + } + } + + return nil + })) + + if wantTOS == 0 { + require.Emptyf(t, got, "DSCP 0 must leave no mangle rule in %s", slot.NamespaceID()) + + return + } + + require.Lenf(t, got, 1, "want exactly one DSCP rule in %s, got %v", slot.NamespaceID(), got) + require.Equal(t, wantTOS, int(got[0])<<2) +} + +// The stamped byte must equal what v1's EgressDSCP/EgressTOS resolution +// produces for the same env, class by class: seed, build override, +// nil-inherits, and 0-disables all included. +func TestEgressDSCP_V1Parity(t *testing.T) { //nolint:paralleltest // mutates host netns state (named netns per case); t.Setenv + skipIfNotLinuxRoot(t) + + tests := []struct { + name string + sandboxEnv string + buildEnv string + }{ + {"unset build value falls back to the sandbox value", "8", ""}, + {"build value overrides for builds only", "8", "16"}, + {"explicit zero disables marking for builds only", "8", "0"}, + {"builds can be marked while sandboxes are not", "0", "16"}, + {"both disabled", "0", ""}, + {"max DSCP maps to the top of the TOS byte", "8", "63"}, + } + + for _, tt := range tests { //nolint:paralleltest // named netns + t.Setenv per case + t.Run(tt.name, func(t *testing.T) { + config := dscpConfig(t, tt.sandboxEnv, tt.buildEnv) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + makeDSCPTestNS(t, slot, config) + + // Creation seeds the untenanted class, as v1's CreateNetwork does. + requireDSCPByte(t, slot, config.EgressTOS().For(network.EgressClassSandbox)) + + for _, class := range []network.EgressClass{network.EgressClassBuild, network.EgressClassSandbox} { + require.NoError(t, ApplyEgressDSCP(slot, config.EgressDSCP(class))) + requireDSCPByte(t, slot, config.EgressTOS().For(class)) + } + }) + } +} + +func TestEgressDSCP_RestampSemantics(t *testing.T) { //nolint:paralleltest // mutates host netns state (named netns); t.Setenv + skipIfNotLinuxRoot(t) + + config := dscpConfig(t, "8", "16") + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + makeDSCPTestNS(t, slot, config) + requireDSCPByte(t, slot, 8<<2) + + // Re-applying the same class must not duplicate the rule. + require.NoError(t, ApplyEgressDSCP(slot, 8)) + requireDSCPByte(t, slot, 8<<2) + + // Out of the 6-bit range is rejected before any chain mutation, so the + // previous class stays installed. + require.Error(t, ApplyEgressDSCP(slot, 200)) + requireDSCPByte(t, slot, 8<<2) + + // 0 removes the rule outright; a second 0 is a clean no-op. + require.NoError(t, ApplyEgressDSCP(slot, 0)) + requireDSCPByte(t, slot, 0) + require.NoError(t, ApplyEgressDSCP(slot, 0)) + requireDSCPByte(t, slot, 0) +} + +type fakeStorage struct{} + +func (f *fakeStorage) Acquire(context.Context) (*network.Slot, error) { + return nil, context.Canceled +} + +func (f *fakeStorage) Release(*network.Slot) error { return nil } + +// The pool seam that protects tenants, through the same path production +// takes: Get stamps the tenant's class, returnSlot restores the untenanted +// one before the slot becomes reusable. +func TestV2Pool_GetStampsAndRecycleRestoresDSCP(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links; t.Setenv + skipIfNotLinuxRoot(t) + + ctx := context.Background() + config := dscpConfig(t, "8", "16") + + hf, err := NewHostFirewall("lo", config) + require.NoError(t, err) + + observer, err := NewVethObserver() + require.NoError(t, err) + + pool := NewV2Pool(&fakeStorage{}, config, hf, observer) + t.Cleanup(func() { _ = pool.Close(context.Background()) }) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + pool.registry.Store(sv2) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, observer)) + t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, observer) }) + + // Queue and close before any assertion can fail the test: Close ranges + // over newSlots, so leaving it open would deadlock the cleanup. + pool.newSlots <- slot + close(pool.newSlots) // Populate never runs here. + + requireDSCPByte(t, slot, 8<<2) + + got, err := pool.Get(ctx, nil, network.EgressClassBuild) + require.NoError(t, err) + require.Equal(t, slot.Idx, got.Idx) + requireDSCPByte(t, got, 16<<2) + + require.NoError(t, pool.returnSlot(ctx, got, func(context.Context, string) {}, 0)) + requireDSCPByte(t, got, 8<<2) + + select { + case reused := <-pool.reusedSlots: + require.Equal(t, slot.Idx, reused.Idx) + default: + t.Fatal("recycled slot must go back to the reuse queue, not be torn down") + } +} + +// The wire-level claim: the TOS byte on the veth link equals v1's EgressTOS +// resolution, and 0 leaves the byte untouched. +func TestEgressDSCP_WireTOS(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links; t.Setenv + skipIfNotLinuxRoot(t) + + ctx := context.Background() + config := dscpConfig(t, "8", "16") + + hf, err := NewHostFirewall("lo", config) + require.NoError(t, err) + defer hf.Close() + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) + t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, nil) }) + + assert.Equal(t, config.EgressTOS().For(network.EgressClassSandbox), observeEgressTOS(t, slot)) + + require.NoError(t, ApplyEgressDSCP(slot, config.EgressDSCP(network.EgressClassBuild))) + assert.Equal(t, config.EgressTOS().For(network.EgressClassBuild), observeEgressTOS(t, slot)) + + require.NoError(t, ApplyEgressDSCP(slot, 0)) + assert.Equal(t, 0, observeEgressTOS(t, slot)) +} + +const wireProbePort = 48765 + +// observeEgressTOS sends a UDP packet from inside the slot's namespace toward +// the host and returns the IPv4 TOS byte it carries on the veth link — the +// same wire bytes v1's in-namespace mangle rule produces. The AF_PACKET +// capture sees the frame as it leaves the namespace, before any host +// netfilter hook. +func observeEgressTOS(t *testing.T, slot *network.Slot) int { + t.Helper() + + link, err := netlink.LinkByName(slot.VethName()) + require.NoError(t, err) + + fd, err := unix.Socket(unix.AF_PACKET, unix.SOCK_DGRAM, int(htons(unix.ETH_P_IP))) + require.NoError(t, err) + defer unix.Close(fd) + + require.NoError(t, unix.Bind(fd, &unix.SockaddrLinklayer{ + Protocol: htons(unix.ETH_P_IP), + Ifindex: link.Attrs().Index, + })) + require.NoError(t, unix.SetsockoptTimeval(fd, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &unix.Timeval{Sec: 10})) + + sendFromNamespace(t, slot, wireProbePort) + + buf := make([]byte, 256) + for { + n, _, err := unix.Recvfrom(fd, buf, 0) + require.NoError(t, err, "no probe packet captured on %s", slot.VethName()) + if n < 20 || buf[0]>>4 != 4 { + continue + } + ihl := int(buf[0]&0x0f) * 4 + if buf[9] != unix.IPPROTO_UDP || n < ihl+8 { + continue + } + if int(buf[ihl+2])<<8|int(buf[ihl+3]) != wireProbePort { + continue + } + + return int(buf[1]) + } +} + +func htons(v uint16) uint16 { return v<<8 | v>>8 } + +func sendFromNamespace(t *testing.T, slot *network.Slot, port int) { + t.Helper() + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + host, err := netns.Get() + require.NoError(t, err) + defer func() { + require.NoError(t, netns.Set(host)) + _ = host.Close() + }() + + target, err := netns.GetFromName(slot.NamespaceID()) + require.NoError(t, err) + defer target.Close() + require.NoError(t, netns.Set(target)) + + conn, err := net.DialUDP("udp4", nil, &net.UDPAddr{IP: slot.VethIP(), Port: port}) + require.NoError(t, err) + defer conn.Close() + + _, err = conn.Write([]byte("tos-probe")) + require.NoError(t, err) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go index 0edfb52215..9432b0c8b6 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -166,6 +166,14 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, return fmt.Errorf("setup namespace NAT: %w", err) } + + // Slots are pooled: seed the untenanted (regular-sandbox) class; the pool + // re-stamps per tenant on Get and restores it on recycle. + if err := SetupEgressDSCP(nsConn, nsTable, slot.VpeerName(), hf.config.EgressDSCP(network.EgressClassSandbox)); err != nil { + nsConn.CloseLasting() + + return fmt.Errorf("setup egress DSCP: %w", err) + } nsConn.CloseLasting() // --- Initialize in-namespace firewall (reuse v1 Firewall) --- diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go index 328764bfa0..f1f68e69aa 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -157,9 +157,7 @@ func (p *V2Pool) Populate(ctx context.Context) { } } -// Get acquires a slot. The egress class is accepted for interface parity with -// the v1 pool; v2 does not stamp egress DSCP. -func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetworkConfig, _ network.EgressClass) (*network.Slot, error) { +func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetworkConfig, class network.EgressClass) (*network.Slot, error) { var slot *network.Slot select { @@ -184,18 +182,32 @@ func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetwork } } - if err := slot.ConfigureInternet(ctx, netConfig); err != nil { + if err := p.configureSlot(ctx, slot, netConfig, class); err != nil { // Never handed out, so nobody listens for the release notification. if rerr := p.ReturnAsync(context.WithoutCancel(ctx), slot, func(context.Context, string) {}, 0); rerr != nil { logger.L().Error(ctx, "failed to return v2 slot to pool", zap.Error(rerr), zap.Int("slot_index", slot.Idx)) } - return nil, fmt.Errorf("error setting v2 slot internet access: %w", err) + return nil, err } return slot, nil } +func (p *V2Pool) configureSlot(ctx context.Context, slot *network.Slot, netConfig *orchestrator.SandboxNetworkConfig, class network.EgressClass) error { + // Slots are created before their tenant is known, so a build re-stamps the + // rule CreateNetworkV2 seeded. No-op when both classes resolve alike. + if err := ApplyEgressDSCP(slot, p.config.EgressDSCP(class)); err != nil { + return fmt.Errorf("error setting v2 slot egress DSCP for %s: %w", class, err) + } + + if err := slot.ConfigureInternet(ctx, netConfig); err != nil { + return fmt.Errorf("error setting v2 slot internet access: %w", err) + } + + return nil +} + func (p *V2Pool) tryTrackReturn() bool { p.closeMu.RLock() defer p.closeMu.RUnlock() @@ -258,6 +270,11 @@ func (p *V2Pool) returnSlot(ctx context.Context, slot *network.Slot, releasedFn } func (p *V2Pool) recycle(ctx context.Context, slot *network.Slot) error { + // Undo any build-specific DSCP before the next tenant can inherit it. + if err := ApplyEgressDSCP(slot, p.config.EgressDSCP(network.EgressClassSandbox)); err != nil { + return p.cleanupWith(ctx, slot, fmt.Errorf("error resetting v2 slot egress DSCP: %w", err)) + } + if err := slot.ResetInternet(ctx); err != nil { return p.cleanupWith(ctx, slot, fmt.Errorf("error resetting v2 slot internet access: %w", err)) } From 1de8656cd4725eee14cb4e224828cd4bc0a8ae18 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 09:18:02 +0000 Subject: [PATCH 06/10] fix(network/v2): match the v1 datapath at the edges Tap MAC, forward-chain posture, restart survival, teardown symmetry and the boot prerequisite all diverged from v1, so NETWORK_VERSION=2 was not a drop-in: - stamp the fixed host tap MAC, so resumed guests' cached gateway ARP stays valid - forward chain accepts by policy and accepts gw->veth unconditionally, as v1 does; the default-drop hook and ct-state match filtered traffic v1 allows, including every non-v2 forwarding path on the host - reconcile set membership at startup, and keep the table on Close while slots are still registered, so sandboxes that outlive the process keep connectivity - teardown tolerates already-absent state, runs on partial-create failure, and preserves the netns anchor when host-side teardown fails - drop the src_valid_mark prerequisite: it gates fwmark policy routing, which this datapath does not use, and nothing sets it - Populate no longer strands a slot when the pool closes mid-send --- packages/orchestrator/pkg/factories/run.go | 11 + .../orchestrator/pkg/sandbox/network/slot.go | 6 + .../pkg/sandbox/network/storage_local.go | 5 + .../pkg/sandbox/network/v2/absent.go | 36 +++ .../sandbox/network/v2/dscp_egress_test.go | 7 +- .../pkg/sandbox/network/v2/host_firewall.go | 38 +-- .../sandbox/network/v2/host_firewall_test.go | 19 +- .../pkg/sandbox/network/v2/main_test.go | 35 +++ .../pkg/sandbox/network/v2/network.go | 70 ++++-- .../pkg/sandbox/network/v2/network_test.go | 27 +-- .../pkg/sandbox/network/v2/parity_test.go | 227 ++++++++++++++++++ .../pkg/sandbox/network/v2/pool.go | 17 +- .../pkg/sandbox/network/v2/slot_test.go | 14 ++ 13 files changed, 443 insertions(+), 69 deletions(-) create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/absent.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/main_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/parity_test.go diff --git a/packages/orchestrator/pkg/factories/run.go b/packages/orchestrator/pkg/factories/run.go index 4f29c410f6..bf2824c812 100644 --- a/packages/orchestrator/pkg/factories/run.go +++ b/packages/orchestrator/pkg/factories/run.go @@ -732,6 +732,17 @@ func run(config cfg.Config, opts Options) (success bool) { logger.L().Fatal(ctx, "failed to create v2 host firewall", zap.Error(hfErr)) } + // Startup reclaim tore down every slot that survived the last run, so + // the surviving set is empty and this drops the set elements and + // fwmark rules they left behind. Without it a reused slot index hits + // an existing element on AddSlot. Skipped when reclaim is disabled: + // the live slots are then unknown, and flushing would cut them off. + if !config.DisableStartupReclaim { + if err := hostFw.ReconcileSlots(nil); err != nil { + logger.L().Fatal(ctx, "failed to reconcile v2 host firewall slots", zap.Error(err)) + } + } + observer, obsErr := networkv2.NewVethObserver() if obsErr != nil { logger.L().Fatal(ctx, "failed to create v2 veth observer", zap.Error(obsErr)) diff --git a/packages/orchestrator/pkg/sandbox/network/slot.go b/packages/orchestrator/pkg/sandbox/network/slot.go index 87d9ff69ec..60345231c1 100644 --- a/packages/orchestrator/pkg/sandbox/network/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/slot.go @@ -8,6 +8,7 @@ import ( "log" "net" "path/filepath" + "slices" "strconv" "sync/atomic" @@ -496,6 +497,11 @@ func getHostNetworkCIDR() *net.IPNet { return subnet } +// TapHostHardwareAddr returns the fixed host-side tap MAC. +func TapHostHardwareAddr() net.HardwareAddr { + return slices.Clone(tapHostHardwareAddr) +} + // getTapHostHardwareAddr parses the fixed tapHostMAC constant once at package // init, so CreateNetwork doesn't reparse a value that can never change or // fail at runtime. diff --git a/packages/orchestrator/pkg/sandbox/network/storage_local.go b/packages/orchestrator/pkg/sandbox/network/storage_local.go index f8a4e3bc47..7c53c38152 100644 --- a/packages/orchestrator/pkg/sandbox/network/storage_local.go +++ b/packages/orchestrator/pkg/sandbox/network/storage_local.go @@ -175,6 +175,11 @@ func (s *StorageLocal) Release(ips *Slot) error { return nil } +// IsNamespaceAvailable reports whether the named network namespace is free. +func IsNamespaceAvailable(dir, name string) (bool, error) { + return isNamespaceAvailable(dir, name) +} + func isNamespaceAvailable(dir, name string) (bool, error) { nsPath := filepath.Join(dir, name) _, err := os.Stat(nsPath) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/absent.go b/packages/orchestrator/pkg/sandbox/network/v2/absent.go new file mode 100644 index 0000000000..b82f2d212d --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/absent.go @@ -0,0 +1,36 @@ +//go:build linux + +package v2 + +import ( + "errors" + "fmt" + "os" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" +) + +// appendUnlessAbsent records err unless it only reports state that is already +// gone, which makes teardown idempotent. +func appendUnlessAbsent(errs *[]error, err error, isAbsent func(error) bool, format string) { + if err == nil || isAbsent(err) { + return + } + + *errs = append(*errs, fmt.Errorf(format, err)) +} + +func isRouteNotExist(err error) bool { + return errors.Is(err, unix.ESRCH) || errors.Is(err, unix.ENOENT) +} + +func isLinkNotExist(err error) bool { + var linkNotFound netlink.LinkNotFoundError + + return errors.As(err, &linkNotFound) || errors.Is(err, unix.ENODEV) || errors.Is(err, unix.ENOENT) +} + +func isNamespaceNotExist(err error) bool { + return os.IsNotExist(err) || errors.Is(err, unix.ENOENT) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go index 61e4d3ad15..2928442a5e 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go @@ -191,8 +191,7 @@ func TestV2Pool_GetStampsAndRecycleRestoresDSCP(t *testing.T) { //nolint:paralle ctx := context.Background() config := dscpConfig(t, "8", "16") - hf, err := NewHostFirewall("lo", config) - require.NoError(t, err) + hf := newTestHostFirewall(t, config) observer, err := NewVethObserver() require.NoError(t, err) @@ -237,9 +236,7 @@ func TestEgressDSCP_WireTOS(t *testing.T) { //nolint:paralleltest // mutates hos ctx := context.Background() config := dscpConfig(t, "8", "16") - hf, err := NewHostFirewall("lo", config) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, config) slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index 5f745470cc..ff1a48cf38 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -4,6 +4,7 @@ package v2 import ( "encoding/binary" + "errors" "fmt" "net" "slices" @@ -154,7 +155,9 @@ func (hf *HostFirewall) ensureChains() error { orchIP := net.ParseIP(hf.config.OrchestratorInSandboxIPAddress).To4() // --- FORWARD chain --- - fwdPolicy := nftables.ChainPolicyDrop + // Accept policy, matching v1: the slot rules below only add ACCEPTs, so + // this chain stays transparent to every other forwarding path on the host. + fwdPolicy := nftables.ChainPolicyAccept fwdChain := hf.conn.AddChain(&nftables.Chain{ Name: "forward", Table: hf.table, @@ -178,7 +181,9 @@ func (hf *HostFirewall) ensureChains() error { }, }) - // iifname oifname @v2_veths ct state established,related accept + // iifname oifname @v2_veths accept — unconditional, as v1's FORWARD + // rule is; a conntrack-state match here would break new inbound flows v1 + // allows. hf.conn.AddRule(&nftables.Rule{ Table: hf.table, Chain: fwdChain, @@ -187,13 +192,6 @@ func (hf *HostFirewall) ensureChains() error { &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: gwBytes}, &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, &expr.Lookup{SourceRegister: 1, SetName: hf.vethSet.Name, SetID: hf.vethSet.ID}, - &expr.Ct{Key: expr.CtKeySTATE, Register: 1}, - &expr.Bitwise{ - SourceRegister: 1, DestRegister: 1, Len: 4, - Mask: bitmask32(expr.CtStateBitESTABLISHED | expr.CtStateBitRELATED), - Xor: bitmask32(0), - }, - &expr.Cmp{Op: expr.CmpOpNeq, Register: 1, Data: bitmask32(0)}, &expr.Verdict{Kind: expr.VerdictAccept}, }, }) @@ -420,21 +418,33 @@ func (hf *HostFirewall) RemoveSlot(slotV2 *SlotV2) error { {Key: nextIP, IntervalEnd: true}, }) - if err := hf.conn.Flush(); err != nil { + // Already-absent elements make this a no-op, not a failure: teardown has + // to be retryable after a partial create. + if err := hf.conn.Flush(); err != nil && !errors.Is(err, unix.ENOENT) { return fmt.Errorf("flush remove slot: %w", err) } return nil } -// Close tears down the entire host firewall table. +// Close releases the netlink connection, and deletes the table only when no +// slot is left in it. Sandboxes outlive the orchestrator process: deleting the +// table while their veths are still registered would cut their connectivity +// until the next start rebuilt it. func (hf *HostFirewall) Close() error { hf.mu.Lock() defer hf.mu.Unlock() - hf.conn.DelTable(hf.table) - if err := hf.conn.Flush(); err != nil { - return fmt.Errorf("delete host firewall table: %w", err) + elements, err := hf.conn.GetSetElements(hf.vethSet) + if err != nil { + return errors.Join(fmt.Errorf("get veth set elements: %w", err), hf.conn.CloseLasting()) + } + + if len(elements) == 0 { + hf.conn.DelTable(hf.table) + if err := hf.conn.Flush(); err != nil { + return errors.Join(fmt.Errorf("delete host firewall table: %w", err), hf.conn.CloseLasting()) + } } return hf.conn.CloseLasting() diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go index 9de9ddac7e..2140abd7ef 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go @@ -27,9 +27,7 @@ func testConfig() network.Config { func TestHostFirewall_Init(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) assert.NotNil(t, hf.conn) assert.NotNil(t, hf.table) @@ -40,16 +38,13 @@ func TestHostFirewall_Init(t *testing.T) { //nolint:paralleltest // creates/dele func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) slot := makeTestSlot(t, 1) sv2 := NewSlotV2(slot) // Add slot - err = hf.AddSlot(sv2) - require.NoError(t, err) + require.NoError(t, hf.AddSlot(sv2)) // Verify veth set has the element elements, err := hf.conn.GetSetElements(hf.vethSet) @@ -62,8 +57,7 @@ func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // cre assert.NotEmpty(t, cidrElements, "cidr set should have elements") // Remove slot - err = hf.RemoveSlot(sv2) - require.NoError(t, err) + require.NoError(t, hf.RemoveSlot(sv2)) // Verify veth set is empty elements, err = hf.conn.GetSetElements(hf.vethSet) @@ -74,14 +68,13 @@ func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // cre func TestHostFirewall_ConcurrentAccess(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) // Add multiple slots concurrently done := make(chan error, 10) for i := 1; i <= 10; i++ { sv2 := NewSlotV2(makeTestSlot(t, i)) + t.Cleanup(func() { _ = hf.RemoveSlot(sv2) }) go func() { done <- hf.AddSlot(sv2) }() diff --git a/packages/orchestrator/pkg/sandbox/network/v2/main_test.go b/packages/orchestrator/pkg/sandbox/network/v2/main_test.go new file mode 100644 index 0000000000..7f08e6dc55 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/main_test.go @@ -0,0 +1,35 @@ +//go:build linux + +package v2 + +import ( + "fmt" + "os" + "testing" +) + +// The host firewall table is a host-wide singleton, so elements left by a +// crashed run would fail the emptiness assertions of a later one. Reconcile it +// to empty once per process — the same call the orchestrator makes at startup. +func TestMain(m *testing.M) { + if os.Geteuid() == 0 { + if err := reconcileHostFirewall(); err != nil { + panic(fmt.Sprintf("cannot reset the host firewall table: %v", err)) + } + } + + m.Run() +} + +func reconcileHostFirewall() error { + hf, err := NewHostFirewall("lo", testConfig()) + if err != nil { + return err + } + + if err := hf.ReconcileSlots(nil); err != nil { + return err + } + + return hf.Close() +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go index 9432b0c8b6..555a93d099 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -28,7 +28,7 @@ import ( // - eBPF: observer.Attach() adds optional per-veth counters (best-effort) func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, hf *HostFirewall, observer *VethObserver, -) error { +) (retErr error) { // Prevent thread changes so we can safely manipulate namespaces runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -38,18 +38,46 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, if err != nil { return fmt.Errorf("cannot get current (host) namespace: %w", err) } + + cleanupNeeded := false defer func() { - if err := netns.Set(hostNS); err != nil { - logger.L().Error(ctx, "error resetting network namespace back to host", zap.Error(err)) + restoreErr := netns.Set(hostNS) + if restoreErr != nil { + logger.L().Error(ctx, "error resetting network namespace back to host", zap.Error(restoreErr)) } + + if retErr != nil && cleanupNeeded { + if restoreErr != nil { + retErr = errors.Join(retErr, fmt.Errorf("error resetting network namespace back to host before cleanup: %w", restoreErr)) + } else if cleanupErr := RemoveNetworkV2(ctx, slot, slotV2, hf, observer); cleanupErr != nil { + retErr = errors.Join(retErr, fmt.Errorf("error cleaning up partially created v2 network: %w", cleanupErr)) + } + } + hostNS.Close() }() + // An existing namespace for this index is a stale reclaim anchor from a + // failed teardown whose rules, routes and veth may still exist. Run a full + // (idempotent) teardown to reclaim them; deleting only the namespace would + // orphan that state. + available, err := network.IsNamespaceAvailable(network.NetNamespacesDir, slot.NamespaceID()) + if err != nil { + return fmt.Errorf("cannot check for stale namespace: %w", err) + } + if !available { + if err := RemoveNetworkV2(ctx, slot, slotV2, hf, observer); err != nil { + return fmt.Errorf("cannot reclaim stale v2 network slot: %w", err) + } + } + // --- Create namespace --- ns, err := netns.NewNamed(slot.NamespaceID()) if err != nil { return fmt.Errorf("cannot create new namespace: %w", err) } + cleanupNeeded = true + defer ns.Close() // --- Create veth pair (inside new namespace, then move veth to host) --- @@ -121,6 +149,12 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, return fmt.Errorf("error creating tap device: %w", err) } + // Keep resumed guests' cached gateway ARP entries valid. Tap devices are + // isolated in per-sandbox network namespaces, so sharing this MAC is safe. + if err := netlink.LinkSetHardwareAddr(tap, network.TapHostHardwareAddr()); err != nil { + return fmt.Errorf("error setting tap device hardware address: %w", err) + } + if err := netlink.LinkSetUp(tap); err != nil { return fmt.Errorf("error setting tap device up: %w", err) } @@ -209,7 +243,9 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, return nil } -// RemoveNetworkV2 tears down the network for a v2 slot. +// RemoveNetworkV2 tears down the network for a v2 slot. It is idempotent: +// state that is already gone is not an error, so it can reclaim a partially +// created or previously half-torn-down slot. func RemoveNetworkV2(_ context.Context, slot *network.Slot, slotV2 *SlotV2, hf *HostFirewall, observer *VethObserver, ) error { @@ -233,27 +269,31 @@ func RemoveNetworkV2(_ context.Context, slot *network.Slot, slotV2 *SlotV2, } // Delete host route - if err := netlink.RouteDel(&netlink.Route{ + err := netlink.RouteDel(&netlink.Route{ Gw: slot.VpeerIP(), Dst: slot.HostNet(), - }); err != nil { - errs = append(errs, fmt.Errorf("error deleting route from host to FC: %w", err)) - } + }) + appendUnlessAbsent(&errs, err, isRouteNotExist, "error deleting route from host to FC: %w") // Delete veth device explicitly (prevents race on reuse) vethLink, err := netlink.LinkByName(slot.VethName()) if err != nil { - errs = append(errs, fmt.Errorf("error finding veth: %w", err)) + appendUnlessAbsent(&errs, err, isLinkNotExist, "error finding veth: %w") } else { - if err := netlink.LinkDel(vethLink); err != nil { - errs = append(errs, fmt.Errorf("error deleting veth device: %w", err)) - } + appendUnlessAbsent(&errs, netlink.LinkDel(vethLink), isLinkNotExist, "error deleting veth device: %w") } - // Delete namespace - if err := netns.DeleteNamed(slot.NamespaceID()); err != nil { - errs = append(errs, fmt.Errorf("error deleting namespace: %w", err)) + // The /run/netns entry is the anchor startup reclaim uses to rediscover a + // leaked slot, and every host-side item above is keyed by the slot index: + // deleting it after a failed teardown would orphan that state with no way + // to retry it. CreateNetworkV2 removes a stale anchor before reusing the + // slot. + if len(errs) > 0 { + return errors.Join(errs...) } + err = netns.DeleteNamed(slot.NamespaceID()) + appendUnlessAbsent(&errs, err, isNamespaceNotExist, "error deleting namespace: %w") + return errors.Join(errs...) } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go index 38bded0fc4..404dd7be46 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -18,9 +18,7 @@ func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // ctx := context.Background() // Create host firewall - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) observer, err := NewVethObserver() require.NoError(t, err) @@ -30,8 +28,7 @@ func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // sv2 := NewSlotV2(slot) // Create network - err = CreateNetworkV2(ctx, slot, sv2, hf, observer) - require.NoError(t, err) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, observer)) // Verify namespace exists out, err := exec.CommandContext(ctx, "ip", "netns", "list").Output() @@ -60,16 +57,13 @@ func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest / ctx := context.Background() - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) - err = CreateNetworkV2(ctx, slot, sv2, hf, nil) - require.NoError(t, err) - defer RemoveNetworkV2(ctx, slot, sv2, hf, nil) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) + t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, nil) }) // Verify NO iptables rules reference this veth out, err := exec.CommandContext(ctx, "iptables", "-t", "nat", "-L", "PREROUTING", "-n").Output() @@ -90,9 +84,7 @@ func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // ctx := context.Background() - hf, err := NewHostFirewall("lo", testConfig()) - require.NoError(t, err) - defer hf.Close() + hf := newTestHostFirewall(t, testConfig()) observer, _ := NewVethObserver() defer observer.Close() @@ -100,11 +92,8 @@ func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) - err = CreateNetworkV2(ctx, slot, sv2, hf, observer) - require.NoError(t, err) - - err = RemoveNetworkV2(ctx, slot, sv2, hf, observer) - require.NoError(t, err) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, observer)) + require.NoError(t, RemoveNetworkV2(ctx, slot, sv2, hf, observer)) // Verify veth set is empty elements, err := hf.conn.GetSetElements(hf.vethSet) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go new file mode 100644 index 0000000000..8cdc336830 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go @@ -0,0 +1,227 @@ +//go:build linux + +package v2 + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/containernetworking/plugins/pkg/ns" + "github.com/google/nftables" + "github.com/google/nftables/expr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vishvananda/netlink" + + "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" +) + +// Resumed guests keep a cached ARP entry for the gateway, so the tap MAC must +// be the same fixed address v1 stamps. +func TestCreateNetworkV2_TapUsesFixedHostMAC(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + hf := newTestHostFirewall(t, testConfig()) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) + t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, nil) }) + + nsHandle, err := ns.GetNS(filepath.Join(network.NetNamespacesDir, slot.NamespaceID())) + require.NoError(t, err) + defer nsHandle.Close() + + var got string + require.NoError(t, nsHandle.Do(func(_ ns.NetNS) error { + tap, err := netlink.LinkByName(slot.TapName()) + if err != nil { + return err + } + got = tap.Attrs().HardwareAddr.String() + + return nil + })) + + assert.Equal(t, network.TapHostHardwareAddr().String(), got) +} + +// v1's host FORWARD rules only ever append ACCEPTs, so the v2 table must not +// drop anything: no default-drop policy, and the gw→veth ACCEPT is +// unconditional (a ct-state match would reject new inbound flows v1 allows). +func TestHostFirewall_ForwardChainMatchesV1(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests + skipIfNotLinuxRoot(t) + + hf := newTestHostFirewall(t, testConfig()) + + chains, err := hf.conn.ListChainsOfTableFamily(nftables.TableFamilyINet) + require.NoError(t, err) + + var fwd *nftables.Chain + for _, c := range chains { + if c.Table.Name == hostFwTableName && c.Name == "forward" { + fwd = c + + break + } + } + require.NotNil(t, fwd, "forward chain must exist") + require.NotNil(t, fwd.Policy) + assert.Equal(t, nftables.ChainPolicyAccept, *fwd.Policy, "v2 must not install a default-drop forward hook") + + rules, err := hf.conn.GetRules(hf.table, fwd) + require.NoError(t, err) + require.Len(t, rules, 2, "forward chain holds exactly v1's two ACCEPTs") + + for _, r := range rules { + accepts := false + for _, e := range r.Exprs { + if v, ok := e.(*expr.Verdict); ok && v.Kind == expr.VerdictAccept { + accepts = true + } + _, isCt := e.(*expr.Ct) + assert.False(t, isCt, "forward ACCEPTs must be unconditional, as v1's are") + } + assert.True(t, accepts, "every forward rule must be an ACCEPT") + } +} + +// A slot left in the sets by a crashed run must not block its index: reclaim +// tears the slot down, then reconcile drops what it left behind. +func TestHostFirewall_ReconcileClearsStaleSlots(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests + skipIfNotLinuxRoot(t) + + hf := newTestHostFirewall(t, testConfig()) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + require.NoError(t, hf.AddSlot(sv2)) + + require.NoError(t, hf.ReconcileSlots(nil)) + + elements, err := hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + assert.Empty(t, elements, "reconcile must drop set elements no live slot owns") + + // A live slot is re-added rather than dropped. + require.NoError(t, hf.AddSlot(sv2)) + require.NoError(t, hf.ReconcileSlots([]*SlotV2{sv2})) + + elements, err = hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + require.Len(t, elements, 1) + + require.NoError(t, hf.RemoveSlot(sv2)) +} + +// Sandboxes outlive the orchestrator process: closing the pool while a slot is +// still registered must leave its rules in place. +func TestHostFirewall_ClosePreservesLiveSlots(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests + skipIfNotLinuxRoot(t) + + hf, err := NewHostFirewall("lo", testConfig()) + require.NoError(t, err) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + require.NoError(t, hf.AddSlot(sv2)) + require.NoError(t, hf.Close()) + + reopened := newTestHostFirewall(t, testConfig()) + require.NoError(t, vethSetHas(reopened, slot.VethName()), "a live slot's rules must survive Close") + + require.NoError(t, reopened.RemoveSlot(sv2)) + + // With no slot left, Close does tear the table down. + require.NoError(t, reopened.Close()) + after := newTestHostFirewall(t, testConfig()) + elements, err := after.conn.GetSetElements(after.vethSet) + require.NoError(t, err) + assert.Empty(t, elements, "Close must delete the table once no slot is left") +} + +// vethSetHas reports whether the veth set holds the named interface. +func vethSetHas(hf *HostFirewall, veth string) error { + elements, err := hf.conn.GetSetElements(hf.vethSet) + if err != nil { + return err + } + + want := ifnameBytes(veth) + for _, e := range elements { + if bytes.Equal(e.Key, want) { + return nil + } + } + + return fmt.Errorf("veth %s not in set (%d elements)", veth, len(elements)) +} + +// Teardown is idempotent: repeated calls, and calls against a slot that was +// never created, must not error — that is what makes create-failure cleanup +// and startup reclaim safe. +func TestRemoveNetworkV2_Idempotent(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + hf := newTestHostFirewall(t, testConfig()) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + + // Never created. + require.NoError(t, RemoveNetworkV2(ctx, slot, sv2, hf, nil)) + + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) + require.NoError(t, RemoveNetworkV2(ctx, slot, sv2, hf, nil)) + require.NoError(t, RemoveNetworkV2(ctx, slot, sv2, hf, nil)) +} + +// A stale namespace from a failed teardown is the reclaim anchor: create must +// reclaim it rather than fail, so the index stays usable. +func TestCreateNetworkV2_ReclaimsStaleNamespace(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links + skipIfNotLinuxRoot(t) + + ctx := context.Background() + + hf := newTestHostFirewall(t, testConfig()) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) + + // Leave the namespace and the set elements behind, as a crashed run would. + require.NoError(t, slot.CloseFirewall()) + + require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil), "a stale anchor must be reclaimed, not fatal") + t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, nil) }) + + _, err := os.Stat(filepath.Join(network.NetNamespacesDir, slot.NamespaceID())) + require.NoError(t, err, "the rebuilt slot must have its namespace") +} + +// fwmark policy routing is not part of this datapath, so its sysctl must not +// gate startup. +func TestValidateV2Prerequisites_IgnoresSrcValidMark(t *testing.T) { + t.Parallel() + + raw, err := os.ReadFile("/proc/sys/net/ipv4/conf/all/src_valid_mark") + require.NoError(t, err) + require.Equal(t, "0", string(raw[:1]), "this box must have src_valid_mark=0 for the check to mean anything") + + forwarding, err := os.ReadFile("/proc/sys/net/ipv4/ip_forward") + require.NoError(t, err) + if string(forwarding[:1]) != "1" { + t.Skip("ip_forward is off; the prerequisite check cannot pass here for unrelated reasons") + } + + require.NoError(t, ValidateV2Prerequisites()) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go index f1f68e69aa..a73ee44d90 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -80,7 +80,6 @@ func ValidateV2Prerequisites() error { desc string }{ {"/proc/sys/net/ipv4/ip_forward", "1", "IPv4 forwarding"}, - {"/proc/sys/net/ipv4/conf/all/src_valid_mark", "1", "src_valid_mark (required for fwmark-based policy routing)"}, } var errs []error @@ -151,8 +150,20 @@ func (p *V2Pool) Populate(ctx context.Context) { continue } - newSlotsAvailableCounter.Add(ctx, 1) - p.newSlots <- slot + select { + case p.newSlots <- slot: + newSlotsAvailableCounter.Add(ctx, 1) + + continue + case <-p.done: + case <-ctx.Done(): + } + + if err := p.cleanup(context.WithoutCancel(ctx), slot); err != nil { + logger.L().Error(ctx, "[v2 network slot pool]: failed to cleanup created slot while closing", zap.Error(err), zap.Int("slot_index", slot.Idx)) + } + + return } } } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go index b3dcd89d86..6e39773aa8 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -23,6 +23,20 @@ func reserveNSTestIdx(t *testing.T) int { return 31000 + int(nsTestIdx.Add(1)) } +// newTestHostFirewall opens the singleton host firewall and registers its +// close first, so it runs after every later-registered slot teardown (t.Cleanup +// is LIFO). Closing it while a slot is still registered deliberately preserves +// the table, which would leak that slot's elements into the next test. +func newTestHostFirewall(t *testing.T, config network.Config) *HostFirewall { + t.Helper() + + hf, err := NewHostFirewall("lo", config) + require.NoError(t, err) + t.Cleanup(func() { _ = hf.Close() }) + + return hf +} + func makeTestSlot(t *testing.T, idx int) *network.Slot { t.Helper() cfg := network.Config{ From 3dee1eb9e09cea6af7e8a9cc23c9da211b150b31 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 09:46:19 +0000 Subject: [PATCH 07/10] fix(network/v2): correct the DSCP checksum fixup and drop unreachable code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The egress DSCP stamp rewrote the TOS byte as a 1-byte payload write at odd offset 1. The kernel's inet checksum fixup is 16-bit-word based, so the delta landed in the wrong half and every stamped packet left with a corrupt IPv4 header checksum: with SANDBOX_EGRESS_DSCP set, all sandbox egress was dropped by the next hop. Writing the two leading header bytes, as nft itself does, keeps the fixup aligned. The wire test now validates the header checksum — an L2 capture accepts a packet the receiver would discard, which is why the original passed. Also from the same review round: - discard the shared nftables conn when a mutation fails, matching the v1 firewall: a failed buffer or Flush leaves the batch queued and the error sticky, and every slot on the host shares this conn - Get returns ErrClosed instead of a nil slot once Populate has closed the channel; CreateNetworkV2 keeps the OS thread locked when the namespace restore fails, so a poisoned thread cannot build the next slot in the wrong netns - compare set keys by trimming the ifname padding; the previous slice classified every live slot as both stale and missing - delete the fwmark policy-routing helpers, the mangle chain and SlotV2.FwMark — nothing in this datapath sets or matches an fwmark - pin the NAT, redirect and masquerade rules, the counters and the absence of iptables rules in tests that fail when the behaviour regresses --- packages/orchestrator/pkg/factories/run.go | 2 +- .../orchestrator/pkg/sandbox/network/pool.go | 2 +- .../pkg/sandbox/network/v2/dscp.go | 25 ++- .../sandbox/network/v2/dscp_egress_test.go | 19 +- .../pkg/sandbox/network/v2/host_firewall.go | 135 +++++++----- .../sandbox/network/v2/host_firewall_test.go | 12 +- .../pkg/sandbox/network/v2/main_test.go | 3 +- .../pkg/sandbox/network/v2/network.go | 14 +- .../pkg/sandbox/network/v2/network_test.go | 22 +- .../sandbox/network/v2/observability_test.go | 40 ++++ .../pkg/sandbox/network/v2/parity_test.go | 77 +++++-- .../pkg/sandbox/network/v2/policy.go | 202 ------------------ .../pkg/sandbox/network/v2/policy_test.go | 59 ----- .../pkg/sandbox/network/v2/pool.go | 8 +- .../pkg/sandbox/network/v2/rules_test.go | 169 +++++++++++++++ .../pkg/sandbox/network/v2/slot.go | 17 +- .../pkg/sandbox/network/v2/slot_test.go | 26 +-- 17 files changed, 427 insertions(+), 405 deletions(-) delete mode 100644 packages/orchestrator/pkg/sandbox/network/v2/policy.go delete mode 100644 packages/orchestrator/pkg/sandbox/network/v2/policy_test.go create mode 100644 packages/orchestrator/pkg/sandbox/network/v2/rules_test.go diff --git a/packages/orchestrator/pkg/factories/run.go b/packages/orchestrator/pkg/factories/run.go index bf2824c812..6123295e69 100644 --- a/packages/orchestrator/pkg/factories/run.go +++ b/packages/orchestrator/pkg/factories/run.go @@ -738,7 +738,7 @@ func run(config cfg.Config, opts Options) (success bool) { // an existing element on AddSlot. Skipped when reclaim is disabled: // the live slots are then unknown, and flushing would cut them off. if !config.DisableStartupReclaim { - if err := hostFw.ReconcileSlots(nil); err != nil { + if err := hostFw.ReconcileSlots(ctx, nil); err != nil { logger.L().Fatal(ctx, "failed to reconcile v2 host firewall slots", zap.Error(err)) } } diff --git a/packages/orchestrator/pkg/sandbox/network/pool.go b/packages/orchestrator/pkg/sandbox/network/pool.go index 2e362cf0f6..2e472e2bf9 100644 --- a/packages/orchestrator/pkg/sandbox/network/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/pool.go @@ -88,7 +88,7 @@ type Config struct { // Set-but-empty behaves as unset (inherits): the env parser skips it. BuildSandboxEgressDSCP *uint8 `env:"BUILD_SANDBOX_EGRESS_DSCP"` - // NetworkVersion selects v1 (iptables per-sandbox) or v2 (nftables verdict maps). + // NetworkVersion selects v1 (iptables per-sandbox) or v2 (nftables, host sets). NetworkVersion int `env:"NETWORK_VERSION" envDefault:"1"` } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/dscp.go b/packages/orchestrator/pkg/sandbox/network/v2/dscp.go index 01ff70a286..f93bc703be 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/dscp.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/dscp.go @@ -148,8 +148,9 @@ func dscpRuleValue(r *nftables.Rule, vpeerIface string) (uint8, bool) { writes = true } case *expr.Bitwise: - if len(v.Xor) == 1 { - tos = v.Xor[0] + // The xor carries the new TOS byte in the second header byte. + if len(v.Xor) == 2 { + tos = v.Xor[1] } } } @@ -184,6 +185,12 @@ func hasOifnameCmp(r *nftables.Rule, name string) bool { // dscpSetExprs builds: meta nfproto ipv4 oifname ip dscp set — // a write of the TOS byte that preserves the ECN bits and fixes the IPv4 // header checksum, equivalent to v1's `-o -j DSCP --set-dscp `. +// +// The read-modify-write spans the two leading header bytes rather than the TOS +// byte alone: the kernel's inet checksum fixup works on 16-bit words, so a +// 1-byte write at odd offset 1 folds the delta into the wrong half and every +// stamped packet leaves with a corrupt header checksum. Masking version/IHL +// through keeps them unchanged. func dscpSetExprs(vpeerIface string, dscp uint8) []expr.Any { return []expr.Any{ &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, @@ -194,22 +201,22 @@ func dscpSetExprs(vpeerIface string, dscp uint8) []expr.Any { OperationType: expr.PayloadLoad, DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, - Offset: 1, // TOS byte in the IPv4 header - Len: 1, + Offset: 0, // version/IHL + TOS + Len: 2, }, &expr.Bitwise{ SourceRegister: 1, DestRegister: 1, - Len: 1, - Mask: []byte{0x03}, - Xor: []byte{dscp << 2}, + Len: 2, + Mask: []byte{0xff, 0x03}, // keep version/IHL and the ECN bits + Xor: []byte{0x00, dscp << 2}, }, &expr.Payload{ OperationType: expr.PayloadWrite, SourceRegister: 1, Base: expr.PayloadBaseNetworkHeader, - Offset: 1, - Len: 1, + Offset: 0, + Len: 2, CsumType: expr.CsumTypeInet, CsumOffset: 10, // IPv4 header checksum }, diff --git a/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go index 2928442a5e..ef5d19d6b9 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/dscp_egress_test.go @@ -258,7 +258,8 @@ const wireProbePort = 48765 // the host and returns the IPv4 TOS byte it carries on the veth link — the // same wire bytes v1's in-namespace mangle rule produces. The AF_PACKET // capture sees the frame as it leaves the namespace, before any host -// netfilter hook. +// netfilter hook — so it also verifies the header checksum the stamp has to +// keep valid: an L2 capture accepts a packet the next hop would drop. func observeEgressTOS(t *testing.T, slot *network.Slot) int { t.Helper() @@ -292,12 +293,28 @@ func observeEgressTOS(t *testing.T, slot *network.Slot) int { continue } + require.Zerof(t, ipHeaderChecksum(buf[:ihl]), "stamped packet carries a corrupt IPv4 header checksum: % x", buf[:ihl]) + return int(buf[1]) } } func htons(v uint16) uint16 { return v<<8 | v>>8 } +// ipHeaderChecksum folds the one's-complement sum over a whole IPv4 header, +// including its checksum field: a valid header sums to zero. +func ipHeaderChecksum(header []byte) uint16 { + var sum uint32 + for i := 0; i+1 < len(header); i += 2 { + sum += uint32(header[i])<<8 | uint32(header[i+1]) + } + for sum>>16 != 0 { + sum = sum&0xffff + sum>>16 + } + + return ^uint16(sum) +} + func sendFromNamespace(t *testing.T, slot *network.Slot, port int) { t.Helper() diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index ff1a48cf38..dad8051d40 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -3,6 +3,8 @@ package v2 import ( + "bytes" + "context" "encoding/binary" "errors" "fmt" @@ -12,9 +14,11 @@ import ( "github.com/google/nftables" "github.com/google/nftables/expr" + "go.uber.org/zap" "golang.org/x/sys/unix" "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" + "github.com/e2b-dev/infra/packages/shared/pkg/logger" ) // HostFirewall is a singleton that manages the host-level nftables table @@ -27,9 +31,9 @@ import ( // set v2_veths { type ifname; } // set v2_host_cidrs { type ipv4_addr; flags interval; } // -// chain forward { type filter hook forward priority 0; policy drop; +// chain forward { type filter hook forward priority 0; policy accept; // iifname @v2_veths oifname accept -// iifname oifname @v2_veths ct state established,related accept +// iifname oifname @v2_veths accept // } // chain prerouting { type nat hook prerouting priority -100; // # service redirects (all slots share same ports) @@ -39,17 +43,15 @@ import ( // # TCP firewall proxy redirects // iifname @v2_veths tcp dport 80 redirect to :tcpHTTPPort // iifname @v2_veths tcp dport 443 redirect to :tcpTLSPort -// iifname @v2_veths tcp dport != { 80, 111, 443, 2049 } redirect to :tcpOtherPort +// iifname @v2_veths tcp redirect to :tcpOtherPort # every remaining TCP // } // chain postrouting { type nat hook postrouting priority 100; // ip saddr @v2_host_cidrs oifname masquerade // } // } // -// The key insight: since all slots share the same redirect ports (configured -// per-orchestrator, not per-slot), we don't need verdict maps. Simple rules -// with set-based iifname matching give us O(1) lookups. Only per-slot data -// (veth names and host CIDRs) goes into sets. +// All slots share the same redirect ports, so only per-slot data (veth names +// and host CIDRs) goes into sets and the rule count stays constant. type HostFirewall struct { conn *nftables.Conn table *nftables.Table @@ -60,14 +62,6 @@ type HostFirewall struct { defaultGw string config network.Config mu sync.Mutex - - // mangleChain is the prerouting mangle chain used for fwmark rules. - // Stored here so fwmark rules can be added/removed without re-creating the chain. - mangleChain *nftables.Chain - - // fwmarkRules tracks per-veth fwmark rule handles for surgical removal. - // Protected by mu. - fwmarkRules map[string]uint64 } const ( @@ -81,12 +75,18 @@ const ( // restart with live sandboxes), existing set elements are preserved. Only chain rules // are refreshed with current config. This prevents connectivity loss for sandboxes // that survived the restart. -func NewHostFirewall(defaultGw string, config network.Config) (*HostFirewall, error) { +func NewHostFirewall(defaultGw string, config network.Config) (_ *HostFirewall, err error) { conn, err := nftables.New(nftables.AsLasting()) if err != nil { return nil, fmt.Errorf("new nftables conn: %w", err) } + defer func() { + if err != nil { + err = errors.Join(err, conn.CloseLasting()) + } + }() + // Ensure table exists — idempotent: creates if new, opens if existing. // We do NOT delete the table on startup. Existing set elements represent // live sandboxes whose connectivity must be preserved across restarts. @@ -96,11 +96,10 @@ func NewHostFirewall(defaultGw string, config network.Config) (*HostFirewall, er }) hf := &HostFirewall{ - conn: conn, - table: table, - defaultGw: defaultGw, - config: config, - fwmarkRules: make(map[string]uint64), + conn: conn, + table: table, + defaultGw: defaultGw, + config: config, } if err := hf.ensureSets(); err != nil { @@ -272,18 +271,6 @@ func (hf *HostFirewall) ensureChains() error { }, }) - // --- MANGLE PREROUTING chain (for fwmark rules) --- - hf.mangleChain = hf.conn.AddChain(&nftables.Chain{ - Name: "mangle_prerouting", - Table: hf.table, - Type: nftables.ChainTypeFilter, - Hooknum: nftables.ChainHookPrerouting, - Priority: nftables.ChainPriorityMangle, - }) - // NOTE: we do NOT flush the mangle chain here. Fwmark rules are per-veth - // state, not config. They are managed individually via Add/RemoveFwmark. - // On restart, stale fwmark rules are cleaned up by ReconcileSlots. - if err := hf.conn.Flush(); err != nil { return fmt.Errorf("flush chains: %w", err) } @@ -295,11 +282,12 @@ func (hf *HostFirewall) ensureChains() error { // Call on startup after rebuilding the in-memory slot registry from surviving sandboxes. // - Slots in the registry but missing from sets are added. // - Set entries not in the registry are removed (stale leftovers from crashed sandboxes). -// - Stale fwmark rules in the mangle chain are flushed. -func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { +func (hf *HostFirewall) ReconcileSlots(ctx context.Context, activeSlots []*SlotV2) (err error) { hf.mu.Lock() defer hf.mu.Unlock() + defer hf.resetConnOnError(ctx, &err) + // Build desired state from active slots desiredVeths := make(map[string]bool) desiredCIDRs := make(map[string]bool) // key = hostIP string @@ -317,8 +305,7 @@ func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { // Remove stale veths (in set but not in active slots) var staleVethElems []nftables.SetElement for _, elem := range currentVeths { - // strip null terminator - if !desiredVeths[string(elem.Key[:len(elem.Key)-1])] { + if !desiredVeths[ifnameString(elem.Key)] { staleVethElems = append(staleVethElems, elem) } } @@ -329,8 +316,7 @@ func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { // Add missing veths (in active slots but not in set) currentVethNames := make(map[string]bool) for _, elem := range currentVeths { - name := string(elem.Key[:len(elem.Key)-1]) - currentVethNames[name] = true + currentVethNames[ifnameString(elem.Key)] = true } for _, sv2 := range activeSlots { veth := sv2.Slot.VethName() @@ -357,11 +343,6 @@ func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { } } - // Flush stale fwmark rules: we lost in-memory handles on restart, - // so flush the mangle chain and let egress profile re-assignment - // re-create needed rules. - hf.conn.FlushChain(hf.mangleChain) - if err := hf.conn.Flush(); err != nil { return fmt.Errorf("flush reconcile: %w", err) } @@ -370,10 +351,12 @@ func (hf *HostFirewall) ReconcileSlots(activeSlots []*SlotV2) error { } // AddSlot adds the veth name and host CIDR for a v2 slot. -func (hf *HostFirewall) AddSlot(slotV2 *SlotV2) error { +func (hf *HostFirewall) AddSlot(ctx context.Context, slotV2 *SlotV2) (err error) { hf.mu.Lock() defer hf.mu.Unlock() + defer hf.resetConnOnError(ctx, &err) + slot := slotV2.Slot // Add veth name to v2_veths set @@ -401,10 +384,12 @@ func (hf *HostFirewall) AddSlot(slotV2 *SlotV2) error { } // RemoveSlot removes the veth name and host CIDR for a v2 slot. -func (hf *HostFirewall) RemoveSlot(slotV2 *SlotV2) error { +func (hf *HostFirewall) RemoveSlot(ctx context.Context, slotV2 *SlotV2) (err error) { hf.mu.Lock() defer hf.mu.Unlock() + defer hf.resetConnOnError(ctx, &err) + slot := slotV2.Slot hf.conn.SetDeleteElements(hf.vethSet, []nftables.SetElement{ @@ -501,6 +486,57 @@ func tcpCatchAllExprs(vethSet *nftables.Set, rport uint16) []expr.Any { // --- helpers --- +// ifnameString reads a set key back as an interface name, dropping the NUL +// padding nftables stores it with. +func ifnameString(key []byte) string { + return string(bytes.TrimRight(key, "\x00")) +} + +// resetConnOnError replaces the shared conn whenever a mutation fails: a +// failed buffer or Flush leaves the batch queued and the serialization error +// sticky, and nftables.Conn exposes no way to clear either, so the next caller +// would replay them. Every slot on the host shares this conn. +func (hf *HostFirewall) resetConnOnError(ctx context.Context, err *error) { + if *err == nil { + return + } + + *err = errors.Join(*err, hf.resetConn(ctx)) +} + +func (hf *HostFirewall) resetConn(ctx context.Context) error { + closeErr := hf.conn.CloseLasting() + + conn, err := nftables.New(nftables.AsLasting()) + if err != nil { + err = fmt.Errorf("open new lasting nftables conn: %w", err) + + var transientErr error + conn, transientErr = nftables.New() + if transientErr != nil { + err = errors.Join(err, fmt.Errorf("open transient nftables conn: %w", transientErr)) + } + } + + resetErr := errors.Join(closeErr, err) + if conn == nil { + // Keep the old (closed, poisoned) conn rather than storing nil and + // panicking on next use; the firewall is left degraded. + logger.L().Error(ctx, "host firewall nftables conn reset failed; reusing the old conn", zap.Error(resetErr)) + + return resetErr + } + + hf.conn = conn + if resetErr != nil { + logger.L().Error(ctx, "host firewall nftables conn reset encountered errors", zap.Error(resetErr)) + } else { + logger.L().Warn(ctx, "host firewall nftables conn reset after a failed mutation") + } + + return resetErr +} + func portBytes(port uint16) []byte { b := make([]byte, 2) binary.BigEndian.PutUint16(b, port) @@ -520,10 +556,3 @@ func incrementIP(ip net.IP) net.IP { return result } - -func bitmask32(v uint32) []byte { - buf := make([]byte, 4) - binary.NativeEndian.PutUint32(buf, v) - - return buf -} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go index 2140abd7ef..089ac32300 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall_test.go @@ -38,13 +38,15 @@ func TestHostFirewall_Init(t *testing.T) { //nolint:paralleltest // creates/dele func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) + ctx := t.Context() + hf := newTestHostFirewall(t, testConfig()) slot := makeTestSlot(t, 1) sv2 := NewSlotV2(slot) // Add slot - require.NoError(t, hf.AddSlot(sv2)) + require.NoError(t, hf.AddSlot(ctx, sv2)) // Verify veth set has the element elements, err := hf.conn.GetSetElements(hf.vethSet) @@ -57,7 +59,7 @@ func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // cre assert.NotEmpty(t, cidrElements, "cidr set should have elements") // Remove slot - require.NoError(t, hf.RemoveSlot(sv2)) + require.NoError(t, hf.RemoveSlot(ctx, sv2)) // Verify veth set is empty elements, err = hf.conn.GetSetElements(hf.vethSet) @@ -68,15 +70,17 @@ func TestHostFirewall_AddRemoveSlot(t *testing.T) { //nolint:paralleltest // cre func TestHostFirewall_ConcurrentAccess(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) + ctx := t.Context() + hf := newTestHostFirewall(t, testConfig()) // Add multiple slots concurrently done := make(chan error, 10) for i := 1; i <= 10; i++ { sv2 := NewSlotV2(makeTestSlot(t, i)) - t.Cleanup(func() { _ = hf.RemoveSlot(sv2) }) + t.Cleanup(func() { _ = hf.RemoveSlot(ctx, sv2) }) go func() { - done <- hf.AddSlot(sv2) + done <- hf.AddSlot(ctx, sv2) }() } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/main_test.go b/packages/orchestrator/pkg/sandbox/network/v2/main_test.go index 7f08e6dc55..5bc99052f5 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/main_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/main_test.go @@ -3,6 +3,7 @@ package v2 import ( + "context" "fmt" "os" "testing" @@ -27,7 +28,7 @@ func reconcileHostFirewall() error { return err } - if err := hf.ReconcileSlots(nil); err != nil { + if err := hf.ReconcileSlots(context.Background(), nil); err != nil { return err } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go index 555a93d099..15197ff563 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -31,11 +31,12 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, ) (retErr error) { // Prevent thread changes so we can safely manipulate namespaces runtime.LockOSThread() - defer runtime.UnlockOSThread() // Save the original (host) namespace hostNS, err := netns.Get() if err != nil { + runtime.UnlockOSThread() + return fmt.Errorf("cannot get current (host) namespace: %w", err) } @@ -43,7 +44,12 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, defer func() { restoreErr := netns.Set(hostNS) if restoreErr != nil { + // Leaving the thread locked dooms it with the goroutine instead of + // returning it to the pool still inside the sandbox namespace, + // where the next user would build its slot in the wrong netns. logger.L().Error(ctx, "error resetting network namespace back to host", zap.Error(restoreErr)) + } else { + runtime.UnlockOSThread() } if retErr != nil && cleanupNeeded { @@ -229,7 +235,7 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, } // --- Host firewall: add slot to sets (replaces 8 iptables rules) --- - if err := hf.AddSlot(slotV2); err != nil { + if err := hf.AddSlot(ctx, slotV2); err != nil { return fmt.Errorf("error adding slot to host firewall: %w", err) } @@ -246,7 +252,7 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, // RemoveNetworkV2 tears down the network for a v2 slot. It is idempotent: // state that is already gone is not an error, so it can reclaim a partially // created or previously half-torn-down slot. -func RemoveNetworkV2(_ context.Context, slot *network.Slot, slotV2 *SlotV2, +func RemoveNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, hf *HostFirewall, observer *VethObserver, ) error { var errs []error @@ -257,7 +263,7 @@ func RemoveNetworkV2(_ context.Context, slot *network.Slot, slotV2 *SlotV2, } // Remove from host firewall sets - if err := hf.RemoveSlot(slotV2); err != nil { + if err := hf.RemoveSlot(ctx, slotV2); err != nil { errs = append(errs, fmt.Errorf("error removing slot from host firewall: %w", err)) } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go index 404dd7be46..3c2c61c868 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -65,17 +65,21 @@ func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest / require.NoError(t, CreateNetworkV2(ctx, slot, sv2, hf, nil)) t.Cleanup(func() { _ = RemoveNetworkV2(ctx, slot, sv2, hf, nil) }) - // Verify NO iptables rules reference this veth - out, err := exec.CommandContext(ctx, "iptables", "-t", "nat", "-L", "PREROUTING", "-n").Output() - if err == nil { - assert.NotContains(t, string(out), slot.VethName(), - "v2 sandbox should not create iptables rules") + // A conditional assertion would pass vacuously wherever iptables is + // missing, which is exactly the host v2 targets. + if _, err := exec.LookPath("iptables"); err != nil { + t.Skip("iptables binary not on PATH; cannot prove the absence of iptables rules") } - out, err = exec.CommandContext(ctx, "iptables", "-t", "filter", "-L", "FORWARD", "-n").Output() - if err == nil { - assert.NotContains(t, string(out), slot.VethName(), - "v2 sandbox should not create iptables FORWARD rules") + for _, chain := range []struct{ table, name string }{ + {"nat", "PREROUTING"}, + {"nat", "POSTROUTING"}, + {"filter", "FORWARD"}, + } { + out, err := exec.CommandContext(ctx, "iptables", "-t", chain.table, "-L", chain.name, "-n").Output() + require.NoError(t, err) + assert.NotContainsf(t, string(out), slot.VethName(), + "a v2 sandbox must not appear in iptables %s/%s", chain.table, chain.name) } } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go index 6635a2d425..5581689c30 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/observability_test.go @@ -4,7 +4,9 @@ package v2 import ( "fmt" + "net" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,6 +34,35 @@ func createTestVeth(t *testing.T) string { return name } +// sendTestVethTraffic pushes packets through the veth pair so its link +// counters advance. +func sendTestVethTraffic(t *testing.T, name string) { + t.Helper() + + peer, err := netlink.LinkByName(name + "-peer") + require.NoError(t, err) + require.NoError(t, netlink.LinkSetUp(peer)) + + link, err := netlink.LinkByName(name) + require.NoError(t, err) + require.NoError(t, netlink.LinkSetUp(link)) + + // An unanswered ARP for an address on the peer's subnet is enough to move + // the counters, and needs no addressing on either end. + addr, err := netlink.ParseAddr("169.254.222.1/30") + require.NoError(t, err) + require.NoError(t, netlink.AddrAdd(link, addr)) + + dialer := net.Dialer{Timeout: time.Second} + conn, err := dialer.DialContext(t.Context(), "udp4", "169.254.222.2:9") + require.NoError(t, err) + defer conn.Close() + + for range 4 { + _, _ = conn.Write([]byte("counter-probe")) + } +} + func TestVethObserver_AttachDetach(t *testing.T) { t.Parallel() @@ -77,6 +108,15 @@ func TestVethObserver_ReadCounters(t *testing.T) { require.NoError(t, err) assert.Equal(t, uint64(0), packets) assert.Equal(t, uint64(0), bytes) + + // Counters that never move would satisfy the assertions above, so make + // the link carry something and read again. + sendTestVethTraffic(t, veth) + + packets, bytes, err = obs.ReadCounters(veth) + require.NoError(t, err) + assert.Positive(t, packets, "counters must track real traffic") + assert.Positive(t, bytes, "counters must track real traffic") } func TestVethObserver_NilSafe(t *testing.T) { diff --git a/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go index 8cdc336830..0bf134ee36 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/containernetworking/plugins/pkg/ns" @@ -97,27 +98,31 @@ func TestHostFirewall_ForwardChainMatchesV1(t *testing.T) { //nolint:paralleltes func TestHostFirewall_ReconcileClearsStaleSlots(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) + ctx := t.Context() + hf := newTestHostFirewall(t, testConfig()) slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) - require.NoError(t, hf.AddSlot(sv2)) + require.NoError(t, hf.AddSlot(ctx, sv2)) - require.NoError(t, hf.ReconcileSlots(nil)) + require.NoError(t, hf.ReconcileSlots(ctx, nil)) elements, err := hf.conn.GetSetElements(hf.vethSet) require.NoError(t, err) assert.Empty(t, elements, "reconcile must drop set elements no live slot owns") - // A live slot is re-added rather than dropped. - require.NoError(t, hf.AddSlot(sv2)) - require.NoError(t, hf.ReconcileSlots([]*SlotV2{sv2})) + // A live slot is left alone: reconcile must recognise it as already + // present, not delete and re-add it. Comparing set membership by a + // mis-trimmed key classifies every live veth as both stale and missing, + // which nets out to the same element count — so count the delete instead. + require.NoError(t, hf.AddSlot(ctx, sv2)) + require.Zero(t, staleVeths(t, hf, []*SlotV2{sv2}), "a live slot must not be classified stale") - elements, err = hf.conn.GetSetElements(hf.vethSet) - require.NoError(t, err) - require.Len(t, elements, 1) + require.NoError(t, hf.ReconcileSlots(ctx, []*SlotV2{sv2})) + require.NoError(t, vethSetHas(hf, slot.VethName())) - require.NoError(t, hf.RemoveSlot(sv2)) + require.NoError(t, hf.RemoveSlot(ctx, sv2)) } // Sandboxes outlive the orchestrator process: closing the pool while a slot is @@ -125,18 +130,20 @@ func TestHostFirewall_ReconcileClearsStaleSlots(t *testing.T) { //nolint:paralle func TestHostFirewall_ClosePreservesLiveSlots(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests skipIfNotLinuxRoot(t) + ctx := t.Context() + hf, err := NewHostFirewall("lo", testConfig()) require.NoError(t, err) slot := makeTestSlot(t, reserveNSTestIdx(t)) sv2 := NewSlotV2(slot) - require.NoError(t, hf.AddSlot(sv2)) + require.NoError(t, hf.AddSlot(ctx, sv2)) require.NoError(t, hf.Close()) reopened := newTestHostFirewall(t, testConfig()) require.NoError(t, vethSetHas(reopened, slot.VethName()), "a live slot's rules must survive Close") - require.NoError(t, reopened.RemoveSlot(sv2)) + require.NoError(t, reopened.RemoveSlot(ctx, sv2)) // With no slot left, Close does tear the table down. require.NoError(t, reopened.Close()) @@ -146,6 +153,29 @@ func TestHostFirewall_ClosePreservesLiveSlots(t *testing.T) { //nolint:parallelt assert.Empty(t, elements, "Close must delete the table once no slot is left") } +// staleVeths counts the set elements reconcile would delete for the given +// live slots, using the same key comparison ReconcileSlots does. +func staleVeths(t *testing.T, hf *HostFirewall, live []*SlotV2) int { + t.Helper() + + desired := make(map[string]bool, len(live)) + for _, sv2 := range live { + desired[sv2.Slot.VethName()] = true + } + + elements, err := hf.conn.GetSetElements(hf.vethSet) + require.NoError(t, err) + + stale := 0 + for _, e := range elements { + if !desired[ifnameString(e.Key)] { + stale++ + } + } + + return stale +} + // vethSetHas reports whether the veth set holds the named interface. func vethSetHas(hf *HostFirewall, veth string) error { elements, err := hf.conn.GetSetElements(hf.vethSet) @@ -213,15 +243,24 @@ func TestCreateNetworkV2_ReclaimsStaleNamespace(t *testing.T) { //nolint:paralle func TestValidateV2Prerequisites_IgnoresSrcValidMark(t *testing.T) { t.Parallel() - raw, err := os.ReadFile("/proc/sys/net/ipv4/conf/all/src_valid_mark") - require.NoError(t, err) - require.Equal(t, "0", string(raw[:1]), "this box must have src_valid_mark=0 for the check to mean anything") - - forwarding, err := os.ReadFile("/proc/sys/net/ipv4/ip_forward") - require.NoError(t, err) - if string(forwarding[:1]) != "1" { - t.Skip("ip_forward is off; the prerequisite check cannot pass here for unrelated reasons") + // Both reads gate on the host's actual sysctls: the assertion only means + // something where src_valid_mark is off, and ip_forward would fail the + // check for an unrelated reason. + if sysctlValue(t, "/proc/sys/net/ipv4/conf/all/src_valid_mark") != "0" { + t.Skip("src_valid_mark is already 1; this box cannot show that the check is gone") + } + if sysctlValue(t, "/proc/sys/net/ipv4/ip_forward") != "1" { + t.Skip("ip_forward is off; the prerequisite check fails here for an unrelated reason") } require.NoError(t, ValidateV2Prerequisites()) } + +func sysctlValue(t *testing.T, path string) string { + t.Helper() + + raw, err := os.ReadFile(path) + require.NoError(t, err) + + return strings.TrimSpace(string(raw)) +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy.go b/packages/orchestrator/pkg/sandbox/network/v2/policy.go deleted file mode 100644 index 158a1a8c29..0000000000 --- a/packages/orchestrator/pkg/sandbox/network/v2/policy.go +++ /dev/null @@ -1,202 +0,0 @@ -//go:build linux - -package v2 - -import ( - "fmt" - "net" - "os" - - "github.com/google/nftables" - "github.com/google/nftables/expr" - "github.com/vishvananda/netlink" -) - -// EnsureSrcValidMark sets net.ipv4.conf.all.src_valid_mark=1 which is required -// for fwmark-based policy routing of forwarded traffic. Without this, the kernel -// may not re-route forwarded packets based on marks set in prerouting. -func EnsureSrcValidMark() error { - return os.WriteFile("/proc/sys/net/ipv4/conf/all/src_valid_mark", []byte("1"), 0o644) -} - -// SetupPolicyRoute creates a policy routing rule and route table entry -// for a given fwmark. Traffic marked with fwmark will be routed through -// the specified gateway device. -// -// Equivalent to: -// -// ip rule add fwmark lookup -// ip route add default via dev table -func SetupPolicyRoute(fwmark uint32, tableID int, gw net.IP, dev string) error { - // Add ip rule: fwmark → lookup table - rule := netlink.NewRule() - rule.Mark = fwmark - mask := uint32(0xFFFFFFFF) - rule.Mask = &mask - rule.Table = tableID - rule.Priority = 100 + tableID - - if err := netlink.RuleAdd(rule); err != nil { - return fmt.Errorf("add policy rule for fwmark 0x%x table %d: %w", fwmark, tableID, err) - } - - // Find the device - link, err := netlink.LinkByName(dev) - if err != nil { - return fmt.Errorf("find device %s for policy route: %w", dev, err) - } - - // Add default route in the table - route := &netlink.Route{ - LinkIndex: link.Attrs().Index, - Gw: gw, - Table: tableID, - Dst: nil, // default route - } - - if err := netlink.RouteAdd(route); err != nil { - return fmt.Errorf("add policy route default via %s dev %s table %d: %w", gw, dev, tableID, err) - } - - return nil -} - -// TeardownPolicyRoute removes the policy routing rule and route table. -func TeardownPolicyRoute(fwmark uint32, tableID int) error { - // Remove ip rule - rule := netlink.NewRule() - rule.Mark = fwmark - mask := uint32(0xFFFFFFFF) - rule.Mask = &mask - rule.Table = tableID - rule.Priority = 100 + tableID - - if err := netlink.RuleDel(rule); err != nil { - return fmt.Errorf("del policy rule for fwmark 0x%x table %d: %w", fwmark, tableID, err) - } - - // Flush all routes in the table - routes, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{Table: tableID}, netlink.RT_FILTER_TABLE) - if err != nil { - return fmt.Errorf("list routes in table %d: %w", tableID, err) - } - - for _, route := range routes { - if err := netlink.RouteDel(&route); err != nil { - return fmt.Errorf("del route in table %d: %w", tableID, err) - } - } - - return nil -} - -// SetupFwmarkInNftables adds a nftables rule to mark packets from a veth -// with the given fwmark for policy routing to egress gateways. -// The mark is set in prerouting (mangle priority) so it takes effect -// BEFORE the routing decision — required for forwarded traffic to be -// rerouted via policy rules. -// -// The rule handle is tracked so it can be surgically removed later. -func SetupFwmarkInNftables(hf *HostFirewall, vethName string, fwmark uint32) error { - hf.mu.Lock() - defer hf.mu.Unlock() - - // Remove existing rule for this veth if present (idempotent reassign) - if handle, ok := hf.fwmarkRules[vethName]; ok { - hf.conn.DelRule(&nftables.Rule{ - Table: hf.table, - Chain: hf.mangleChain, - Handle: handle, - }) - delete(hf.fwmarkRules, vethName) - } - - hf.conn.AddRule(&nftables.Rule{ - Table: hf.table, - Chain: hf.mangleChain, - Exprs: []expr.Any{ - // Match iifname == vethName - &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, - &expr.Cmp{Register: 1, Op: expr.CmpOpEq, Data: ifnameBytes(vethName)}, - // Set fwmark - &expr.Immediate{Register: 1, Data: bitmask32(fwmark)}, - &expr.Meta{Key: expr.MetaKeyMARK, SourceRegister: true, Register: 1}, - }, - }) - - if err := hf.conn.Flush(); err != nil { - return fmt.Errorf("flush fwmark rule: %w", err) - } - - // Retrieve the rule handle for later removal. - // The rule we just added is the last one in the chain for this veth. - rules, err := hf.conn.GetRules(hf.table, hf.mangleChain) - if err != nil { - return fmt.Errorf("get mangle rules to track handle: %w", err) - } - for _, r := range rules { - if matchesFwmarkRule(r, vethName) { - hf.fwmarkRules[vethName] = r.Handle - - break - } - } - - return nil -} - -// matchesFwmarkRule checks if a rule is the fwmark rule for the given veth -// by inspecting its expressions for an iifname match. -func matchesFwmarkRule(r *nftables.Rule, vethName string) bool { - expected := ifnameBytes(vethName) - for i, e := range r.Exprs { - cmp, ok := e.(*expr.Cmp) - if !ok || i == 0 { - continue - } - // Check if previous expr is MetaKeyIIFNAME and this Cmp matches our veth - if meta, ok := r.Exprs[i-1].(*expr.Meta); ok && meta.Key == expr.MetaKeyIIFNAME { - if len(cmp.Data) == len(expected) { - match := true - for j := range cmp.Data { - if cmp.Data[j] != expected[j] { - match = false - - break - } - } - if match { - return true - } - } - } - } - - return false -} - -// RemoveFwmarkInNftables removes the per-veth fwmark rule by its tracked handle. -// Idempotent: returns nil if no rule exists for the veth. -func RemoveFwmarkInNftables(hf *HostFirewall, vethName string) error { - hf.mu.Lock() - defer hf.mu.Unlock() - - handle, ok := hf.fwmarkRules[vethName] - if !ok { - return nil // already removed or never set - } - - hf.conn.DelRule(&nftables.Rule{ - Table: hf.table, - Chain: hf.mangleChain, - Handle: handle, - }) - - if err := hf.conn.Flush(); err != nil { - return fmt.Errorf("flush remove fwmark rule for %s: %w", vethName, err) - } - - delete(hf.fwmarkRules, vethName) - - return nil -} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go b/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go deleted file mode 100644 index 02918b7b41..0000000000 --- a/packages/orchestrator/pkg/sandbox/network/v2/policy_test.go +++ /dev/null @@ -1,59 +0,0 @@ -//go:build linux - -package v2 - -import ( - "net" - "os/exec" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestPolicyRoute_Setup(t *testing.T) { //nolint:paralleltest // mutates host netns state: fixed-name dummy link "dummy-test", policy rule and route table 300 - skipIfNotLinuxRoot(t) - - ctx := t.Context() - fwmark := uint32(0x300) - tableID := 300 - - // Setup: create a dummy device to route through - err := exec.CommandContext(ctx, "ip", "link", "add", "dummy-test", "type", "dummy").Run() - require.NoError(t, err) - defer exec.CommandContext(ctx, "ip", "link", "del", "dummy-test").Run() - - err = exec.CommandContext(ctx, "ip", "link", "set", "dummy-test", "up").Run() - require.NoError(t, err) - - err = exec.CommandContext(ctx, "ip", "addr", "add", "10.99.99.1/24", "dev", "dummy-test").Run() - require.NoError(t, err) - - // Test setup - gw := net.ParseIP("10.99.99.1") - err = SetupPolicyRoute(fwmark, tableID, gw, "dummy-test") - require.NoError(t, err) - - // Verify ip rule exists - out, err := exec.CommandContext(ctx, "ip", "rule", "list").Output() - require.NoError(t, err) - assert.Contains(t, string(out), "lookup 300") - - // Verify route table has route - out, err = exec.CommandContext(ctx, "ip", "route", "show", "table", "300").Output() - require.NoError(t, err) - assert.Contains(t, string(out), "dummy-test") - - // Teardown - err = TeardownPolicyRoute(fwmark, tableID) - require.NoError(t, err) - - // Verify rule is gone - out, err = exec.CommandContext(ctx, "ip", "rule", "list").Output() - require.NoError(t, err) - for line := range strings.SplitSeq(string(out), "\n") { - assert.NotContains(t, line, "lookup 300", - "policy rule should be removed after teardown") - } -} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/pool.go b/packages/orchestrator/pkg/sandbox/network/v2/pool.go index a73ee44d90..03b54d73d4 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/pool.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/pool.go @@ -185,7 +185,13 @@ func (p *V2Pool) Get(ctx context.Context, netConfig *orchestrator.SandboxNetwork return nil, network.ErrClosed case <-ctx.Done(): return nil, ctx.Err() - case s := <-p.newSlots: + case s, ok := <-p.newSlots: + // Populate closes the channel on its way out, and a closed channel + // wins this select as readily as done does. + if !ok { + return nil, network.ErrClosed + } + newSlotsAvailableCounter.Add(ctx, -1) acquiredSlots.Add(ctx, 1, metric.WithAttributes(attribute.String("pool", "new"))) telemetry.ReportEvent(ctx, "new v2 network slot") diff --git a/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go b/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go new file mode 100644 index 0000000000..20dd968eb2 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go @@ -0,0 +1,169 @@ +//go:build linux + +package v2 + +import ( + "bytes" + "encoding/binary" + "net" + "testing" + + "github.com/google/nftables" + "github.com/google/nftables/expr" + "github.com/stretchr/testify/require" +) + +// Helpers for asserting on installed rules through the same netlink view the +// kernel exposes, so a test fails when a rule's target, port or address is +// wrong — not merely when the chain is missing. + +func chainRules(t *testing.T, conn *nftables.Conn, table *nftables.Table, chain string) []*nftables.Rule { + t.Helper() + + rules, err := conn.GetRules(table, &nftables.Chain{Name: chain, Table: table}) + require.NoError(t, err) + + return rules +} + +// natRule reports whether the rule NATs to wantIP with the given NAT type and +// matches wantMatch in the IPv4 header. +func natRule(r *nftables.Rule, natType expr.NATType, wantIP, wantMatch net.IP) bool { + var toIP, matched bool + for _, e := range r.Exprs { + switch v := e.(type) { + case *expr.NAT: + if v.Type != natType { + return false + } + case *expr.Immediate: + toIP = toIP || bytes.Equal(v.Data, wantIP.To4()) + case *expr.Cmp: + matched = matched || bytes.Equal(v.Data, wantMatch.To4()) + } + } + + return toIP && matched +} + +// redirectPorts returns the matched destination port and the port the rule +// redirects to, for a rule that does both. +func redirectPorts(r *nftables.Rule) (dport, rport uint16, ok bool) { + var redirects bool + var ports []uint16 + + for _, e := range r.Exprs { + switch v := e.(type) { + case *expr.Redir: + redirects = true + case *expr.Cmp: + if len(v.Data) == 2 { + ports = append(ports, binary.BigEndian.Uint16(v.Data)) + } + case *expr.Immediate: + if len(v.Data) == 2 { + rport = binary.BigEndian.Uint16(v.Data) + } + } + } + + if !redirects || len(ports) == 0 { + return 0, 0, false + } + + return ports[len(ports)-1], rport, true +} + +// immediatePort returns the port a rule loads into a register, i.e. the port +// it redirects to. +func immediatePort(r *nftables.Rule) uint16 { + for _, e := range r.Exprs { + if v, ok := e.(*expr.Immediate); ok && len(v.Data) == 2 { + return binary.BigEndian.Uint16(v.Data) + } + } + + return 0 +} + +func hasExpr[T expr.Any](r *nftables.Rule) bool { + for _, e := range r.Exprs { + if _, ok := e.(T); ok { + return true + } + } + + return false +} + +// The in-namespace NAT is the whole point of SetupNamespaceNAT: pin the +// addresses and the direction, not just the chains. +func TestNamespaceNAT_RulesSNATAndDNAT(t *testing.T) { //nolint:paralleltest // creates the nftables table "test-nat-rules" in the host netns + skipIfNotLinuxRoot(t) + + const ( + hostIP = "10.11.0.1" + nsIP = "169.254.0.21" + ) + + conn, err := nftables.New(nftables.AsLasting()) + require.NoError(t, err) + defer conn.CloseLasting() + + table := conn.AddTable(&nftables.Table{Name: "test-nat-rules", Family: nftables.TableFamilyINet}) + require.NoError(t, conn.Flush()) + t.Cleanup(func() { + conn.DelTable(table) + _ = conn.Flush() + }) + + require.NoError(t, SetupNamespaceNAT(conn, table, "eth0", hostIP, nsIP)) + + post := chainRules(t, conn, table, "postroute_nat") + require.Len(t, post, 1) + require.True(t, natRule(post[0], expr.NATTypeSourceNAT, net.ParseIP(hostIP), net.ParseIP(nsIP)), + "postrouting must SNAT the namespace IP to the host IP") + + pre := chainRules(t, conn, table, "preroute_nat") + require.Len(t, pre, 1) + require.True(t, natRule(pre[0], expr.NATTypeDestNAT, net.ParseIP(nsIP), net.ParseIP(hostIP)), + "prerouting must DNAT the host IP to the namespace IP") +} + +// The host chains are v2's replacement for v1's per-slot iptables rules: pin +// every redirect's port pair and the masquerade. +func TestHostFirewall_ChainRulesMatchConfig(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests + skipIfNotLinuxRoot(t) + + config := testConfig() + hf := newTestHostFirewall(t, config) + + got := make(map[uint16][]uint16) + var catchAll []uint16 + + for _, r := range chainRules(t, hf.conn, hf.table, "prerouting") { + dport, rport, ok := redirectPorts(r) + if !ok { + // The catch-all matches no port: every remaining TCP is redirected. + require.True(t, hasExpr[*expr.Redir](r), "every prerouting rule redirects") + catchAll = append(catchAll, immediatePort(r)) + + continue + } + got[dport] = append(got[dport], rport) + } + + require.Equal(t, []uint16{config.SandboxTCPFirewallOtherPort}, catchAll, + "exactly one catch-all TCP redirect, to the other-port") + + // Port 80 carries two rules: the hyperloop service redirect (matched on the + // orchestrator IP) first, then the TCP firewall. + require.Equal(t, []uint16{config.HyperloopProxyPort, config.SandboxTCPFirewallHTTPPort}, got[80]) + require.Equal(t, []uint16{config.PortmapperPort}, got[111]) + require.Equal(t, []uint16{config.NFSProxyPort}, got[2049]) + require.Equal(t, []uint16{config.SandboxTCPFirewallTLSPort}, got[443]) + + post := chainRules(t, hf.conn, hf.table, "postrouting") + require.Len(t, post, 1) + require.True(t, hasExpr[*expr.Masq](post[0]), "sandbox egress must be masqueraded to the gateway") +} diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot.go b/packages/orchestrator/pkg/sandbox/network/v2/slot.go index 0ab9cbee93..a31f4bbec7 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot.go @@ -9,12 +9,6 @@ import ( "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/network" ) -const ( - // fwMarkBase is the base fwmark for v2 slots. - // v2 slot N gets fwmark = fwMarkBase + Slot.Idx. - fwMarkBase = 0x200 -) - // SlotV2 extends a base network.Slot with v2-specific metadata. // It wraps (composition) rather than embeds so the base *Slot flows // through the existing sandbox pipeline unchanged. @@ -24,9 +18,6 @@ type SlotV2 struct { NetworkVersion int SandboxID string ExecutionID string - - // FwMark is the nftables/policy-routing mark for this slot's traffic. - FwMark uint32 } // NewSlotV2 wraps a base Slot with v2 metadata. @@ -34,15 +25,9 @@ func NewSlotV2(slot *network.Slot) *SlotV2 { return &SlotV2{ Slot: slot, NetworkVersion: 2, - FwMark: fwMarkBase + uint32(slot.Idx), } } -// FwMarkForIndex returns the fwmark that will be assigned to a slot with the given index. -func FwMarkForIndex(idx int) uint32 { - return fwMarkBase + uint32(idx) -} - // SlotV2Registry tracks v2 metadata for slots by their index. type SlotV2Registry struct { mu sync.RWMutex @@ -87,5 +72,5 @@ func (r *SlotV2Registry) Range(fn func(idx int, s *SlotV2) bool) { // String returns a debug-friendly representation. func (s *SlotV2) String() string { - return fmt.Sprintf("SlotV2{idx=%d, fwmark=0x%x}", s.Slot.Idx, s.FwMark) + return fmt.Sprintf("SlotV2{idx=%d}", s.Slot.Idx) } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go index 6e39773aa8..4b41876fec 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/slot_test.go @@ -62,40 +62,16 @@ func TestSlotV2_Creation(t *testing.T) { assert.Equal(t, 2, sv2.NetworkVersion) assert.Equal(t, slot, sv2.Slot) - assert.Equal(t, uint32(fwMarkBase+1), sv2.FwMark) assert.Empty(t, sv2.SandboxID) } -func TestSlotV2_FwMarkUniqueness(t *testing.T) { - t.Parallel() - - marks := make(map[uint32]bool) - - for i := 1; i <= 100; i++ { - slot := makeTestSlot(t, i) - sv2 := NewSlotV2(slot) - - assert.False(t, marks[sv2.FwMark], "duplicate fwmark at index %d: 0x%x", i, sv2.FwMark) - marks[sv2.FwMark] = true - } -} - -func TestSlotV2_FwMarkForIndex(t *testing.T) { - t.Parallel() - - assert.Equal(t, uint32(0x201), FwMarkForIndex(1)) - assert.Equal(t, uint32(0x264), FwMarkForIndex(100)) -} - func TestSlotV2_String(t *testing.T) { t.Parallel() slot := makeTestSlot(t, 5) sv2 := NewSlotV2(slot) - s := sv2.String() - assert.Contains(t, s, "idx=5") - assert.Contains(t, s, "fwmark=0x205") + assert.Contains(t, sv2.String(), "idx=5") } func TestSlotV2Registry(t *testing.T) { From b5e56d0f1f358695e8a3e4e46b988e0975506fbf Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 10:02:20 +0000 Subject: [PATCH 08/10] fix(network/v2): create each slot on its own goroutine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The namespace restore at the end of a slot create is deferred and can fail. Keeping the OS thread locked so it dies with its goroutine only works if the goroutine actually exits — and the only caller is the pool's Populate loop, which runs forever. A failed restore therefore pinned that loop to a thread stuck inside a sandbox namespace, and every later slot got its host-side veth, addresses and routes built in that namespace: silently, and for the rest of the process's life. On the path where the create itself succeeded the call even returned nil, handing out a slot with no host connectivity. Each create now runs on its own goroutine, so a thread that cannot get back to the host namespace is destroyed when that goroutine returns, and a failed restore fails the slot instead of reporting success. Also from the same review round: classifyVeths is split out as a pure function so a unit test can pin the padded-key comparison that decides whether reconcile deletes a live sandbox's veth — the previous assertion recomputed the answer with the same helper it was meant to check, and stayed green when that helper was bypassed. --- packages/orchestrator/pkg/factories/run.go | 7 +-- .../pkg/sandbox/network/v2/host_firewall.go | 55 +++++++++++-------- .../pkg/sandbox/network/v2/network.go | 34 +++++++++--- .../pkg/sandbox/network/v2/network_test.go | 6 +- .../pkg/sandbox/network/v2/parity_test.go | 33 +++++------ 5 files changed, 77 insertions(+), 58 deletions(-) diff --git a/packages/orchestrator/pkg/factories/run.go b/packages/orchestrator/pkg/factories/run.go index 6123295e69..bd062114de 100644 --- a/packages/orchestrator/pkg/factories/run.go +++ b/packages/orchestrator/pkg/factories/run.go @@ -733,10 +733,9 @@ func run(config cfg.Config, opts Options) (success bool) { } // Startup reclaim tore down every slot that survived the last run, so - // the surviving set is empty and this drops the set elements and - // fwmark rules they left behind. Without it a reused slot index hits - // an existing element on AddSlot. Skipped when reclaim is disabled: - // the live slots are then unknown, and flushing would cut them off. + // the surviving set is empty and this drops the elements they left + // behind. Skipped when reclaim is disabled: the live slots are then + // unknown, and flushing would cut them off. if !config.DisableStartupReclaim { if err := hostFw.ReconcileSlots(ctx, nil); err != nil { logger.L().Fatal(ctx, "failed to reconcile v2 host firewall slots", zap.Error(err)) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index dad8051d40..81647fa3ca 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -278,6 +278,30 @@ func (hf *HostFirewall) ensureChains() error { return nil } +// classifyVeths splits the set's current elements into those no live slot +// owns and the live veths the set is missing. Pure, so the padded-key +// comparison it turns on can be pinned by a unit test. +func classifyVeths(current []nftables.SetElement, desired map[string]bool) (stale []nftables.SetElement, missing []string) { + present := make(map[string]bool, len(current)) + for _, elem := range current { + name := ifnameString(elem.Key) + present[name] = true + if !desired[name] { + stale = append(stale, elem) + } + } + + for name := range desired { + if !present[name] { + missing = append(missing, name) + } + } + + slices.Sort(missing) + + return stale, missing +} + // ReconcileSlots reconciles host firewall set membership with actual active slots. // Call on startup after rebuilding the in-memory slot registry from surviving sandboxes. // - Slots in the registry but missing from sets are added. @@ -302,30 +326,15 @@ func (hf *HostFirewall) ReconcileSlots(ctx context.Context, activeSlots []*SlotV return fmt.Errorf("get veth set elements: %w", err) } - // Remove stale veths (in set but not in active slots) - var staleVethElems []nftables.SetElement - for _, elem := range currentVeths { - if !desiredVeths[ifnameString(elem.Key)] { - staleVethElems = append(staleVethElems, elem) - } + stale, missing := classifyVeths(currentVeths, desiredVeths) + if len(stale) > 0 { + hf.conn.SetDeleteElements(hf.vethSet, stale) } - if len(staleVethElems) > 0 { - hf.conn.SetDeleteElements(hf.vethSet, staleVethElems) - } - - // Add missing veths (in active slots but not in set) - currentVethNames := make(map[string]bool) - for _, elem := range currentVeths { - currentVethNames[ifnameString(elem.Key)] = true - } - for _, sv2 := range activeSlots { - veth := sv2.Slot.VethName() - if !currentVethNames[veth] { - if err := hf.conn.SetAddElements(hf.vethSet, []nftables.SetElement{ - {Key: ifnameBytes(veth)}, - }); err != nil { - return fmt.Errorf("add missing veth %s: %w", veth, err) - } + for _, veth := range missing { + if err := hf.conn.SetAddElements(hf.vethSet, []nftables.SetElement{ + {Key: ifnameBytes(veth)}, + }); err != nil { + return fmt.Errorf("add missing veth %s: %w", veth, err) } } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network.go b/packages/orchestrator/pkg/sandbox/network/v2/network.go index 15197ff563..f006f29a3b 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network.go @@ -28,6 +28,22 @@ import ( // - eBPF: observer.Attach() adds optional per-veth counters (best-effort) func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, hf *HostFirewall, observer *VethObserver, +) error { + // Run on a goroutine of its own so the thread it pins is this call's + // alone. createNetworkV2 keeps that thread locked if it cannot get back to + // the host namespace, and the runtime destroys a still-locked thread when + // its goroutine exits — a caller's long-lived goroutine would instead keep + // building every later slot inside the namespace it got stuck in. + done := make(chan error, 1) + go func() { + done <- createNetworkV2(ctx, slot, slotV2, hf, observer) + }() + + return <-done +} + +func createNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, + hf *HostFirewall, observer *VethObserver, ) (retErr error) { // Prevent thread changes so we can safely manipulate namespaces runtime.LockOSThread() @@ -44,19 +60,19 @@ func CreateNetworkV2(ctx context.Context, slot *network.Slot, slotV2 *SlotV2, defer func() { restoreErr := netns.Set(hostNS) if restoreErr != nil { - // Leaving the thread locked dooms it with the goroutine instead of - // returning it to the pool still inside the sandbox namespace, - // where the next user would build its slot in the wrong netns. + // Deliberately no UnlockOSThread: the thread is still inside the + // sandbox namespace, so it must die with this goroutine rather + // than be reused. The slot fails with it — its host-side state + // cannot be trusted, and cleanup would run in the wrong namespace. + retErr = errors.Join(retErr, fmt.Errorf("error resetting network namespace back to host: %w", restoreErr)) logger.L().Error(ctx, "error resetting network namespace back to host", zap.Error(restoreErr)) } else { runtime.UnlockOSThread() - } - if retErr != nil && cleanupNeeded { - if restoreErr != nil { - retErr = errors.Join(retErr, fmt.Errorf("error resetting network namespace back to host before cleanup: %w", restoreErr)) - } else if cleanupErr := RemoveNetworkV2(ctx, slot, slotV2, hf, observer); cleanupErr != nil { - retErr = errors.Join(retErr, fmt.Errorf("error cleaning up partially created v2 network: %w", cleanupErr)) + if retErr != nil && cleanupNeeded { + if cleanupErr := RemoveNetworkV2(ctx, slot, slotV2, hf, observer); cleanupErr != nil { + retErr = errors.Join(retErr, fmt.Errorf("error cleaning up partially created v2 network: %w", cleanupErr)) + } } } diff --git a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go index 3c2c61c868..02325e48ae 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/network_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/network_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes +func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links skipIfNotLinuxRoot(t) ctx := context.Background() @@ -52,7 +52,7 @@ func TestCreateNetworkV2_FullLifecycle(t *testing.T) { //nolint:paralleltest // } } -func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes +func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links skipIfNotLinuxRoot(t) ctx := context.Background() @@ -83,7 +83,7 @@ func TestCreateNetworkV2_NoIptablesRules(t *testing.T) { //nolint:paralleltest / } } -func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links, policy routes +func TestCreateNetworkV2_CleanTeardown(t *testing.T) { //nolint:paralleltest // mutates host netns state: singleton nftables table "v2-host-firewall", named netns, veth links skipIfNotLinuxRoot(t) ctx := context.Background() diff --git a/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go index 0bf134ee36..2c8da0cf4b 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/parity_test.go @@ -117,8 +117,6 @@ func TestHostFirewall_ReconcileClearsStaleSlots(t *testing.T) { //nolint:paralle // mis-trimmed key classifies every live veth as both stale and missing, // which nets out to the same element count — so count the delete instead. require.NoError(t, hf.AddSlot(ctx, sv2)) - require.Zero(t, staleVeths(t, hf, []*SlotV2{sv2}), "a live slot must not be classified stale") - require.NoError(t, hf.ReconcileSlots(ctx, []*SlotV2{sv2})) require.NoError(t, vethSetHas(hf, slot.VethName())) @@ -153,27 +151,24 @@ func TestHostFirewall_ClosePreservesLiveSlots(t *testing.T) { //nolint:parallelt assert.Empty(t, elements, "Close must delete the table once no slot is left") } -// staleVeths counts the set elements reconcile would delete for the given -// live slots, using the same key comparison ReconcileSlots does. -func staleVeths(t *testing.T, hf *HostFirewall, live []*SlotV2) int { - t.Helper() +// A live slot's padded set key must match its name: the classification is +// what decides whether reconcile deletes a live sandbox's veth, and comparing +// the raw 16-byte key against the name silently makes every live slot look +// both stale and missing. +func TestClassifyVeths(t *testing.T) { + t.Parallel() - desired := make(map[string]bool, len(live)) - for _, sv2 := range live { - desired[sv2.Slot.VethName()] = true + current := []nftables.SetElement{ + {Key: ifnameBytes("veth-1")}, + {Key: ifnameBytes("veth-99")}, } + desired := map[string]bool{"veth-1": true, "veth-2": true} - elements, err := hf.conn.GetSetElements(hf.vethSet) - require.NoError(t, err) - - stale := 0 - for _, e := range elements { - if !desired[ifnameString(e.Key)] { - stale++ - } - } + stale, missing := classifyVeths(current, desired) - return stale + require.Len(t, stale, 1, "only the veth no live slot owns is stale") + require.Equal(t, ifnameBytes("veth-99"), stale[0].Key) + require.Equal(t, []string{"veth-2"}, missing) } // vethSetHas reports whether the veth set holds the named interface. From 560563d9c4fa4fd3eacd5d918987c4ccb2bf9d37 Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 10:10:42 +0000 Subject: [PATCH 09/10] docs(network/v2): describe the conn-reset hazard accurately Flush clears the queued messages even when it fails; what sticks to the conn is the serialization error, and messages queued by a mutation that returned before reaching Flush. Both still justify replacing the conn. --- .../orchestrator/pkg/sandbox/network/v2/host_firewall.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index 81647fa3ca..9ae77ef2db 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -501,10 +501,11 @@ func ifnameString(key []byte) string { return string(bytes.TrimRight(key, "\x00")) } -// resetConnOnError replaces the shared conn whenever a mutation fails: a -// failed buffer or Flush leaves the batch queued and the serialization error -// sticky, and nftables.Conn exposes no way to clear either, so the next caller -// would replay them. Every slot on the host shares this conn. +// resetConnOnError replaces the shared conn whenever a mutation fails. A +// serialization error sticks to the conn for its lifetime, and a mutation that +// returns before reaching Flush leaves its messages queued for whoever flushes +// next; nftables.Conn exposes no way to clear either. Every slot on the host +// shares this conn, so one failure would otherwise follow every later slot. func (hf *HostFirewall) resetConnOnError(ctx context.Context, err *error) { if *err == nil { return From 98b8ded00034f23fda623e1a343492f8d3ddfacc Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 12 Aug 2026 10:15:32 +0000 Subject: [PATCH 10/10] fix(network/v2): remove each set element in its own batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An nftables batch is atomic, so tearing the veth and host-CIDR entries down in one flush means an already-absent element aborts the whole thing — and because teardown treats ENOENT as success, the element that was present stayed in the set while RemoveSlot reported success. A stale veth keeps matching the forward and redirect rules. Proven with nft directly, and the regression test fails against the single-batch form. --- .../pkg/sandbox/network/v2/host_firewall.go | 25 +++++++++++------ .../pkg/sandbox/network/v2/rules_test.go | 28 +++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go index 9ae77ef2db..4c5ea77dc0 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/host_firewall.go @@ -400,22 +400,29 @@ func (hf *HostFirewall) RemoveSlot(ctx context.Context, slotV2 *SlotV2) (err err defer hf.resetConnOnError(ctx, &err) slot := slotV2.Slot + hostIP := slot.HostIP.To4() - hf.conn.SetDeleteElements(hf.vethSet, []nftables.SetElement{ + // One flush per set, not one for both: an nftables batch is atomic, so an + // already-absent element aborts the whole thing and would leave the other + // set's element installed while teardown reported success. + vethErr := hf.deleteElements(hf.vethSet, []nftables.SetElement{ {Key: ifnameBytes(slot.VethName())}, }) - - hostIP := slot.HostIP.To4() - nextIP := incrementIP(hostIP) - hf.conn.SetDeleteElements(hf.cidrSet, []nftables.SetElement{ + cidrErr := hf.deleteElements(hf.cidrSet, []nftables.SetElement{ {Key: hostIP}, - {Key: nextIP, IntervalEnd: true}, + {Key: incrementIP(hostIP), IntervalEnd: true}, }) - // Already-absent elements make this a no-op, not a failure: teardown has - // to be retryable after a partial create. + return errors.Join(vethErr, cidrErr) +} + +// deleteElements drops elements from a set, treating already-absent ones as +// success: teardown has to be retryable after a partial create. +func (hf *HostFirewall) deleteElements(set *nftables.Set, elements []nftables.SetElement) error { + hf.conn.SetDeleteElements(set, elements) + if err := hf.conn.Flush(); err != nil && !errors.Is(err, unix.ENOENT) { - return fmt.Errorf("flush remove slot: %w", err) + return fmt.Errorf("flush delete from %s: %w", set.Name, err) } return nil diff --git a/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go b/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go index 20dd968eb2..5d7d53eb1b 100644 --- a/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go +++ b/packages/orchestrator/pkg/sandbox/network/v2/rules_test.go @@ -167,3 +167,31 @@ func TestHostFirewall_ChainRulesMatchConfig(t *testing.T) { //nolint:paralleltes require.Len(t, post, 1) require.True(t, hasExpr[*expr.Masq](post[0]), "sandbox egress must be masqueraded to the gateway") } + +// One absent element must not stop the other from being removed: an nftables +// batch is atomic, so deleting both in one flush would abort and silently +// leave a stale veth matching the forward and redirect rules. +func TestHostFirewall_RemoveSlotWithOneElementAlreadyGone(t *testing.T) { //nolint:paralleltest // creates/deletes the singleton nftables table "v2-host-firewall" shared by all HostFirewall tests + skipIfNotLinuxRoot(t) + + ctx := t.Context() + hf := newTestHostFirewall(t, testConfig()) + + slot := makeTestSlot(t, reserveNSTestIdx(t)) + sv2 := NewSlotV2(slot) + require.NoError(t, hf.AddSlot(ctx, sv2)) + + // Drop the CIDR entry behind RemoveSlot's back, as a partially reconciled + // or half-torn-down host would have. + hostIP := slot.HostIP.To4() + hf.conn.SetDeleteElements(hf.cidrSet, []nftables.SetElement{ + {Key: hostIP}, + {Key: incrementIP(hostIP), IntervalEnd: true}, + }) + require.NoError(t, hf.conn.Flush()) + + require.NoError(t, hf.RemoveSlot(ctx, sv2)) + + require.Error(t, vethSetHas(hf, slot.VethName()), + "the veth element must be gone even though the CIDR element already was") +}