diff --git a/internal/ingresssidecar/backend.go b/internal/ingresssidecar/backend.go index 2f3fa152..bf94b4da 100644 --- a/internal/ingresssidecar/backend.go +++ b/internal/ingresssidecar/backend.go @@ -5,6 +5,7 @@ package ingresssidecar import ( + "errors" "fmt" "net" "regexp" @@ -67,7 +68,11 @@ type kernelBackend struct{} func NewKernelBackend() Backend { return kernelBackend{} } func (kernelBackend) EnsureVRF(vpc string) (uint32, error) { - if err := vrf.Add(vpc); err != nil { + if err := vrf.AddInRange(vpc, sidecarTableIDMin, sidecarTableIDMax); err != nil { + if errors.Is(err, vrf.ErrTableOutOfRange) { + return 0, fmt.Errorf("create VRF for vpc %s: %w; an older sidecar created it in this pod, "+ + "so replace the pod rather than restarting the container", vpc, err) + } return 0, fmt.Errorf("create VRF for vpc %s: %w", vpc, err) } tableID, err := vrf.TableID(vpc) diff --git a/internal/ingresssidecar/ebpfdatapath.go b/internal/ingresssidecar/ebpfdatapath.go index a07cc1e6..b567c67f 100644 --- a/internal/ingresssidecar/ebpfdatapath.go +++ b/internal/ingresssidecar/ebpfdatapath.go @@ -56,22 +56,31 @@ var ebpfPinDir = attach.PinDir // real locator the way the CNI path does. const ingressSidecarBlock = uformat.BlockIngressSidecar +// Routing table IDs this sidecar allocates its VRFs from: the range just above +// vrf.SidecarTableIDBase, one per possible uSID Argument. Disjoint from the +// host's range, because egress_route_table is keyed by table ID alone and both +// namespaces write it; see vrf.SidecarTableIDBase. +const ( + sidecarTableIDMin = vrf.SidecarTableIDBase + uint32(uformat.ArgumentMin) + sidecarTableIDMax = vrf.SidecarTableIDBase + uint32(uformat.ArgumentMax) +) + // argumentForTableID derives the uSID Argument for a VPC's VRF from its Linux -// routing table ID rather than allocating a separate value. The table ID is -// already unique per VPC on this node, which is the only property Argument -// needs here, so reusing it avoids a second allocator. +// routing table ID rather than allocating a separate value: the table ID's +// offset from vrf.SidecarTableIDBase. The table ID is already unique per VPC +// in this pod, which is the only property Argument needs here, so reusing it +// avoids a second allocator. // -// Fails if tableID does not fit Argument's 12-bit range. Table IDs are -// allocated from 1 upward, so this needs about 4095 live VPCs on one node, but -// a silently aliased Argument would be a cross-VPC datapath bug rather than a +// Fails if tableID is outside [sidecarTableIDMin, sidecarTableIDMax]. A +// silently aliased Argument would be a cross-VPC datapath bug rather than a // safe fallback. func argumentForTableID(tableID uint32) (uint16, error) { - if tableID < uint32(uformat.ArgumentMin) || tableID > uint32(uformat.ArgumentMax) { + if tableID < sidecarTableIDMin || tableID > sidecarTableIDMax { return 0, fmt.Errorf( - "ingresssidecar: VRF table id %d does not fit uSID Argument's range [%#x,%#x]", - tableID, uint16(uformat.ArgumentMin), uint16(uformat.ArgumentMax)) + "ingresssidecar: VRF table id %d is outside the sidecar's range [%d,%d]", + tableID, sidecarTableIDMin, sidecarTableIDMax) } - return uint16(tableID), nil + return uint16(tableID - vrf.SidecarTableIDBase), nil } // vrfLinkForTable returns the kernel VRF link whose routing table is tableID, @@ -93,8 +102,8 @@ func vrfLinkForTable(tableID uint32) (*netlink.Vrf, error) { // egressVethNames derives a VPC's veth pair names from tableID alone, since the // callers carry only a tableID forward. Collision-free because table IDs are -// already unique per VPC on this node, and comfortably inside IFNAMSIZ, since a -// table ID that fits Argument's 12-bit range is at most 4 decimal digits. +// already unique per VPC in this pod, and comfortably inside IFNAMSIZ, since a +// table ID in the sidecar's range is at most 5 decimal digits. func egressVethNames(tableID uint32) (inner, peer string) { return fmt.Sprintf("ivs%d", tableID), fmt.Sprintf("ivp%d", tableID) } @@ -106,19 +115,29 @@ func egressVethNames(tableID uint32) (inner, peer string) { // redoes its route lookup inside the VRF. peer stays outside the VRF, in the // pod's main namespace, and is where usid_egress actually attaches. // -// Idempotent: an existing name counts as already done, and the route is -// replaced rather than added, so it is safe on every call that ensures a -// VRF. -func ensureEgressVeth(vrfLink *netlink.Vrf, inner, peer string) (netlink.Link, error) { +// peer is created with the explicit interface index peerIndex, because its +// index keys ifindex_vrf_table, which the host writes its own indexes into; +// see ifindexvrfmap.SidecarIfindexBase. It is therefore the veth's primary +// link and inner its peer, since netlink only sets the primary's index. +// +// Idempotent: an existing name with the right index counts as already done, +// and the route is replaced rather than added, so it is safe on every call +// that ensures a VRF. An existing peer with any other index is an error, since +// registering it would put a row in the host's index range. +func ensureEgressVeth(vrfLink *netlink.Vrf, inner, peer string, peerIndex uint32) (netlink.Link, error) { peerLink, err := netlink.LinkByName(peer) + if err == nil && uint32(peerLink.Attrs().Index) != peerIndex { + return nil, fmt.Errorf("veth peer %q has index %d, want %d; an older sidecar created it in this pod, "+ + "so replace the pod rather than restarting the container", peer, peerLink.Attrs().Index, peerIndex) + } if err != nil { var notFound netlink.LinkNotFoundError if !errors.As(err, ¬Found) { return nil, fmt.Errorf("look up veth peer %q: %w", peer, err) } veth := &netlink.Veth{ - LinkAttrs: netlink.LinkAttrs{Name: inner}, - PeerName: peer, + LinkAttrs: netlink.LinkAttrs{Name: peer, Index: int(peerIndex)}, + PeerName: inner, } if err := netlink.LinkAdd(veth); err != nil { return nil, fmt.Errorf("add veth pair %q/%q: %w", inner, peer, err) @@ -279,7 +298,7 @@ func ensureEgressDatapath(vpc string, tableID uint32) error { } inner, peer := egressVethNames(tableID) - peerLink, err := ensureEgressVeth(vrfLink, inner, peer) + peerLink, err := ensureEgressVeth(vrfLink, inner, peer, ifindexvrfmap.SidecarIfindex(argument)) if err != nil { return fmt.Errorf("ensure egress veth for VRF table %d: %w", tableID, err) } @@ -313,7 +332,8 @@ func ensureEgressDatapath(vpc string, tableID uint32) error { defer func() { _ = ifindexCloser.Close() }() // Keyed by the veth peer's ifindex, not the VRF's, because usid_egress has - // to see this traffic arrive on that interface's ingress hook. + // to see this traffic arrive on that interface's ingress hook. That index + // is ifindexvrfmap.SidecarIfindex(argument), set by ensureEgressVeth. if err := ifindexTable.Register(uint32(peerLink.Attrs().Index), ingressSidecarBlock, argument); err != nil { return fmt.Errorf("register eBPF ifindex_vrf_table entry: %w", err) } diff --git a/internal/ingresssidecar/ebpfdatapath_integration_test.go b/internal/ingresssidecar/ebpfdatapath_integration_test.go index 4e402206..43d1617c 100644 --- a/internal/ingresssidecar/ebpfdatapath_integration_test.go +++ b/internal/ingresssidecar/ebpfdatapath_integration_test.go @@ -16,6 +16,7 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/attach" "go.datum.net/galactic/internal/plumbing/ebpf/ifindexvrfmap" "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" + "go.datum.net/galactic/internal/plumbing/vrf" ) func requireRoot(t *testing.T) { @@ -99,6 +100,15 @@ func assertMapState( ifindexTable *ifindexvrfmap.IfindexVRFTable, registry *usidmap.Registry, vrfLink *netlink.Vrf, peerLink netlink.Link, tableID uint32, ) error { + argument := uint16(tableID - vrf.SidecarTableIDBase) + + // The peer's index is in the range the host never registers, so a host + // interface with the same number in its own namespace cannot overwrite + // this row. + if want := ifindexvrfmap.SidecarIfindex(argument); uint32(peerLink.Attrs().Index) != want { + return fmt.Errorf("veth peer ifindex = %d, want %d", peerLink.Attrs().Index, want) + } + entry, ok, err := ifindexTable.Get(uint32(peerLink.Attrs().Index)) if err != nil { return fmt.Errorf("look up ifindex_vrf_table[peer]: %w", err) @@ -106,9 +116,9 @@ func assertMapState( if !ok { return errors.New("ifindex_vrf_table has no entry for the veth peer's ifindex") } - if entry.Block != ingressSidecarBlock || entry.Argument != uint16(tableID) { + if entry.Block != ingressSidecarBlock || entry.Argument != argument { return fmt.Errorf("ifindex_vrf_table[peer] = (block=%#x, argument=%d), want (block=%#x, argument=%d)", - entry.Block, entry.Argument, ingressSidecarBlock, tableID) + entry.Block, entry.Argument, ingressSidecarBlock, argument) } if _, ok, err := ifindexTable.Get(uint32(vrfLink.Attrs().Index)); err != nil { @@ -117,7 +127,7 @@ func assertMapState( return errors.New("ifindex_vrf_table has an entry keyed by the VRF's own ifindex, want none") } - vrfEntry, ok, err := registry.VRF.Get(ingressSidecarBlock, uint16(tableID)) + vrfEntry, ok, err := registry.VRF.Get(ingressSidecarBlock, argument) if err != nil { return fmt.Errorf("look up vrf_table entry: %w", err) } @@ -141,7 +151,7 @@ func assertTornDown( } else if ok { return errors.New("ifindex_vrf_table[peer] still present after removeEgressDatapath") } - if _, ok, err := registry.VRF.Get(ingressSidecarBlock, uint16(tableID)); err != nil { + if _, ok, err := registry.VRF.Get(ingressSidecarBlock, uint16(tableID-vrf.SidecarTableIDBase)); err != nil { return fmt.Errorf("look up vrf_table entry after removal: %w", err) } else if ok { return errors.New("vrf_table entry still present after removeEgressDatapath") @@ -166,7 +176,7 @@ func TestEnsureEgressDatapath_AttachesToVethPeerNotVRF(t *testing.T) { } t.Cleanup(func() { _ = nsObj.Close() }) - const tableID = uint32(7) + const tableID = vrf.SidecarTableIDBase + 7 const vrfName = "ivstestvrf" err = nsObj.Do(func(_ ns.NetNS) error { diff --git a/internal/ingresssidecar/ebpfdatapath_test.go b/internal/ingresssidecar/ebpfdatapath_test.go index 7788578e..de5b42e0 100644 --- a/internal/ingresssidecar/ebpfdatapath_test.go +++ b/internal/ingresssidecar/ebpfdatapath_test.go @@ -10,21 +10,26 @@ import ( "golang.org/x/sys/unix" "go.datum.net/galactic/internal/plumbing/ebpf/uformat" + "go.datum.net/galactic/internal/plumbing/vrf" ) func TestArgumentForTableID(t *testing.T) { + const base = vrf.SidecarTableIDBase cases := []struct { name string tableID uint32 want uint16 wantErr bool }{ - {name: "typical small table id", tableID: 1, want: 1}, - {name: "another typical table id", tableID: 3, want: 3}, - {name: "argument min boundary", tableID: uint32(uformat.ArgumentMin), want: uint16(uformat.ArgumentMin)}, - {name: "argument max boundary", tableID: uint32(uformat.ArgumentMax), want: uint16(uformat.ArgumentMax)}, - {name: "zero table id is out of range (Argument 0 is reserved)", tableID: 0, wantErr: true}, - {name: "table id past the 12-bit Argument range", tableID: uint32(uformat.ArgumentMax) + 1, wantErr: true}, + {name: "first sidecar table id", tableID: base + 1, want: 1}, + {name: "another sidecar table id", tableID: base + 3, want: 3}, + {name: "argument max boundary", tableID: base + uformat.ArgumentMax, want: uformat.ArgumentMax}, + {name: "base itself is out of range (Argument 0 is reserved)", tableID: base, wantErr: true}, + {name: "past the 12-bit Argument range", tableID: base + uformat.ArgumentMax + 1, wantErr: true}, + // The host allocates these, so a sidecar must never derive an Argument + // from one. + {name: "host table id 1", tableID: 1, wantErr: true}, + {name: "last host table id", tableID: vrf.HostTableIDMax, wantErr: true}, } for _, tc := range cases { @@ -67,7 +72,8 @@ func TestEgressVethNames(t *testing.T) { inner1, peer1, otherInner, otherPeer) } - for _, name := range []string{inner1, peer1, otherInner, otherPeer} { + maxInner, maxPeer := egressVethNames(sidecarTableIDMax) + for _, name := range []string{inner1, peer1, otherInner, otherPeer, maxInner, maxPeer} { if len(name) > unix.IFNAMSIZ-1 { t.Errorf("egressVethNames produced %q, %d bytes -- too long for IFNAMSIZ (%d)", name, len(name), unix.IFNAMSIZ) } diff --git a/internal/installer/sidecarreturn.go b/internal/installer/sidecarreturn.go index a0fe31de..6160a063 100644 --- a/internal/installer/sidecarreturn.go +++ b/internal/installer/sidecarreturn.go @@ -71,9 +71,9 @@ import ( const ( // sidecarReturnTableBase and sidecarReturnTableMax bound the Linux routing // table IDs this file allocates from. A dedicated range matters twice: - // tenant VRF table IDs are handed out from 1 upward, so anything low would - // eventually collide, and pruning deletes whole tables, which must never - // reach one this file did not create. + // tenant VRF table IDs are handed out up to vrf.HostTableIDMax, so anything + // at or below it would collide, and pruning deletes whole tables, which + // must never reach one this file did not create. // // Sized to hold one table per Argument, the most sidecar VRFs a node can // have. diff --git a/internal/installer/sidecarreturn_test.go b/internal/installer/sidecarreturn_test.go index a96b4f02..8f9941f7 100644 --- a/internal/installer/sidecarreturn_test.go +++ b/internal/installer/sidecarreturn_test.go @@ -160,6 +160,20 @@ func TestSidecarReturnTableID_StaysInTheReservedRange(t *testing.T) { } } +// TestSidecarReturnTableRange_SitsBetweenVRFRanges checks that the return-path +// tables, which live in the host namespace, overlap neither the host's tenant +// VRF tables nor the ingress sidecar's. +func TestSidecarReturnTableRange_SitsBetweenVRFRanges(t *testing.T) { + if sidecarReturnTableBase <= vrf.HostTableIDMax { + t.Errorf("sidecarReturnTableBase %#x overlaps host VRF tables up to %#x", + sidecarReturnTableBase, vrf.HostTableIDMax) + } + if sidecarReturnTableMax >= vrf.SidecarTableIDBase { + t.Errorf("sidecarReturnTableMax %#x reaches the sidecar's VRF tables from %#x", + sidecarReturnTableMax, vrf.SidecarTableIDBase) + } +} + // TestSidecarReturnTableID_RejectsOutOfRange guards the other direction. A // zero Argument is the reserved value uformat excludes, and anything past // ArgumentMax cannot have arrived in a SID at all -- either would compute a diff --git a/internal/plumbing/ebpf/egressroutemap/owner.go b/internal/plumbing/ebpf/egressroutemap/owner.go index 961ed05d..85bf6331 100644 --- a/internal/plumbing/ebpf/egressroutemap/owner.go +++ b/internal/plumbing/ebpf/egressroutemap/owner.go @@ -24,9 +24,10 @@ import ( // // Nothing in egress_route_table's own key or value records who wrote an entry, // and its key carries only a routing table ID, which each namespace allocates -// from 1 upward independently. vrf_table does record it: the sidecar registers -// every VRF it serves under uformat.BlockIngressSidecar, a Block no CNI -// attachment can reach, and does so before installing any route into that +// independently, from disjoint ranges (see vrf.SidecarTableIDBase), so a +// table ID belongs to one writer only. vrf_table records which: the sidecar +// registers every VRF it serves under uformat.BlockIngressSidecar, a Block no +// CNI attachment can reach, and does so before installing any route into that // table. Reading those rows back is therefore enough to tell the two // populations apart without a second copy of the ownership state. func SidecarOwnedTableIDs(vrfTable *usidmap.VRFTable) (map[uint32]struct{}, error) { diff --git a/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf.go b/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf.go index aa734ae3..4417492d 100644 --- a/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf.go +++ b/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf.go @@ -17,6 +17,26 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" ) +// SidecarIfindexBase splits the interface index space of ifindex_vrf_table +// between its two writers. The ingress sidecar creates every interface it +// registers with an explicit index of SidecarIfindexBase plus that VRF's uSID +// Argument, and the host registers only indexes below it. +// +// Each network namespace numbers its interfaces on its own, from 1 upward, +// but the sidecar registers indexes from its pod's namespace into the same +// pinned map the host CNI writes host indexes into. Without disjoint ranges, a +// sidecar interface and a host interface with the same index overwrite each +// other's row, and usid_egress routes one's traffic into the other's VRF. +// +// A host namespace allocates indexes sequentially and never gets near 2^30. +const SidecarIfindexBase = uint32(0x40000000) + +// SidecarIfindex returns the interface index the ingress sidecar gives the +// interface it registers for the VRF whose uSID Argument is argument. +func SidecarIfindex(argument uint16) uint32 { + return SidecarIfindexBase + uint32(argument) +} + // IfindexVRFEntry is one decoded ifindex_vrf_table row, kept separate from the // generated kernel layout. type IfindexVRFEntry struct { @@ -75,6 +95,9 @@ func (t *IfindexVRFTable) Register(ifindex uint32, block uint64, argument uint16 if err := uformat.ValidateArgument(argument); err != nil { return fmt.Errorf("ifindexvrfmap: ifindex_vrf_table: register ifindex=%d: %w", ifindex, err) } + if err := validateIfindexOwner(ifindex, block, argument); err != nil { + return fmt.Errorf("ifindexvrfmap: ifindex_vrf_table: register ifindex=%d: %w", ifindex, err) + } value := prog.UsidIfindexVrfValue{Block: block, Argument: argument} if err := t.table.Put(ifindex, value); err != nil { @@ -87,6 +110,23 @@ func (t *IfindexVRFTable) Register(ifindex uint32, block uint64, argument uint16 return nil } +// validateIfindexOwner rejects a registration that would cross into the other +// writer's index range; see SidecarIfindexBase. The ingress sidecar registers +// only under uformat.BlockIngressSidecar, so the Block identifies the writer. +func validateIfindexOwner(ifindex uint32, block uint64, argument uint16) error { + if block == uformat.BlockIngressSidecar { + if want := SidecarIfindex(argument); ifindex != want { + return fmt.Errorf("ingress sidecar interface for argument %#x must have index %d", argument, want) + } + return nil + } + if ifindex >= SidecarIfindexBase { + return fmt.Errorf("host interface index is in the range reserved for the ingress sidecar (>= %d)", + SidecarIfindexBase) + } + return nil +} + // Unregister removes the ifindex_vrf_table entry for ifindex if present. An // already-absent entry is not an error: DEL is idempotent per the CNI spec, and // every caller treats cleanup as best-effort. diff --git a/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf_test.go b/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf_test.go index 691710cf..82f96f5a 100644 --- a/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf_test.go +++ b/internal/plumbing/ebpf/ifindexvrfmap/ifindexvrf_test.go @@ -7,6 +7,8 @@ package ifindexvrfmap import ( "errors" "testing" + + "go.datum.net/galactic/internal/plumbing/ebpf/uformat" ) const testBlock uint64 = 0x2001_0DB8_FF01 @@ -65,6 +67,47 @@ func TestIfindexVRFTable_RegisterRejectsReservedArgumentZero(t *testing.T) { } } +// TestIfindexVRFTable_RegisterKeepsWritersApart checks that the host and the +// ingress sidecar, which number interfaces in separate namespaces but share +// this map, each stay inside their own index range. +func TestIfindexVRFTable_RegisterKeepsWritersApart(t *testing.T) { + const sidecar = uformat.BlockIngressSidecar + cases := []struct { + name string + ifindex uint32 + block uint64 + argument uint16 + wantErr bool + }{ + {name: "host index below the sidecar range", ifindex: 42, block: testBlock, argument: 0x7}, + {name: "last host index", ifindex: SidecarIfindexBase - 1, block: testBlock, argument: 0x7}, + {name: "host index in the sidecar range", ifindex: SidecarIfindex(0x7), block: testBlock, argument: 0x7, + wantErr: true}, + {name: "sidecar index matching its argument", ifindex: SidecarIfindex(0x7), block: sidecar, argument: 0x7}, + {name: "sidecar index for another argument", ifindex: SidecarIfindex(0x8), block: sidecar, argument: 0x7, + wantErr: true}, + {name: "sidecar with a host-range index", ifindex: 0x7, block: sidecar, argument: 0x7, wantErr: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + it, ft := newTestIfindexVRFTable(constClock(1)) + err := it.Register(tc.ifindex, tc.block, tc.argument) + if tc.wantErr { + if err == nil { + t.Fatalf("Register(%d, %#x, %#x) = nil, want an error", tc.ifindex, tc.block, tc.argument) + } + if ft.len() != 0 { + t.Errorf("rejected Register wrote %d entries, want 0", ft.len()) + } + return + } + if err != nil { + t.Fatalf("Register(%d, %#x, %#x): %v", tc.ifindex, tc.block, tc.argument, err) + } + }) + } +} + func TestIfindexVRFTable_RegisterRejectsBlockOverflow(t *testing.T) { it, ft := newTestIfindexVRFTable(constClock(1)) diff --git a/internal/plumbing/vrf/vrf.go b/internal/plumbing/vrf/vrf.go index 55ac57cd..7e12885a 100644 --- a/internal/plumbing/vrf/vrf.go +++ b/internal/plumbing/vrf/vrf.go @@ -11,7 +11,6 @@ package vrf import ( "errors" "fmt" - "math" "net" "sync" "time" @@ -23,8 +22,25 @@ import ( "go.datum.net/galactic/internal/plumbing/sysctl" ) -const minVRFID = uint32(1) -const maxVRFID = uint32(math.MaxUint32 - 1) +// SidecarTableIDBase splits the routing table ID space between the two +// writers of the shared pinned eBPF maps. The host's VRFs take IDs below it, +// and the ingress sidecar's take IDs above it. +// +// Each network namespace allocates table IDs on its own, but +// egress_route_table is keyed by table ID alone and is shared between the +// host and every sidecar pod on the node. Without disjoint ranges, a sidecar +// VRF and a host tenant VRF both start at table 1 and overwrite each other's +// routes. +const SidecarTableIDBase = uint32(0x10000) + +// HostTableIDMin and HostTableIDMax bound the routing table IDs Add allocates. +// HostTableIDMax stops short of SidecarTableIDBase: the host's installer keeps +// its sidecar return-path tables in the host namespace in the IDs between the +// two (internal/installer/sidecarreturn.go). +const ( + HostTableIDMin = uint32(1) + HostTableIDMax = uint32(0xEFFF) +) // UnreachableDefaultMetric is the metric of the unreachable default route Add // installs in every VRF table, for each family. It is the value the kernel's @@ -43,13 +59,19 @@ const UnreachableDefaultMetric = 4278198272 // either way. var ErrNotFound = errors.New("vrf: no VRF interface for this VPC in this network namespace") +// ErrTableOutOfRange is wrapped by AddInRange when this VPC's VRF already exists +// with a table ID outside the caller's range, for instance one created by an +// ingress sidecar version that allocated from the host's range. +var ErrTableOutOfRange = errors.New("vrf: existing VRF's table is outside the caller's range") + // vrfMu serializes VRF creation and deletion within one process. It does not by // itself protect two separate CNI invocations racing on the same node, each // being its own process, so Add and Delete also take a cross-process lock. var vrfMu sync.Mutex // Add creates the Linux VRF interface for a base62-encoded VPC, allocating the -// next available routing table ID and applying the required sysctls. +// next available routing table ID in [HostTableIDMin, HostTableIDMax] and +// applying the required sysctls. // // The VRF is shared by every attachment on this VPC on this node, so concurrent // calls, from goroutines here or from separate plugin processes attaching @@ -62,6 +84,14 @@ var vrfMu sync.Mutex // before those existed therefore gains them on its next attachment's ADD, // with no migration step. func Add(vpc string) error { + return AddInRange(vpc, HostTableIDMin, HostTableIDMax) +} + +// AddInRange is Add with the routing table ID allocated from [minID, maxID]. +// A VRF that already exists under this VPC's name with a table ID outside that +// range is an error: its table may collide with the other writer's, and +// recreating it would strand whatever is already routed through it. +func AddInRange(vpc string, minID, maxID uint32) error { vrfMu.Lock() defer vrfMu.Unlock() @@ -78,10 +108,14 @@ func Add(vpc string) error { if !ok { return fmt.Errorf("interface %q exists but is a %s, not a VRF", name, link.Type()) } + if existing.Table < minID || existing.Table > maxID { + return fmt.Errorf("VRF %q has table %d, not in [%d,%d]: %w", + name, existing.Table, minID, maxID, ErrTableOutOfRange) + } return ensureUnreachableDefaults(existing.Table) } - vrfID, err := findNextAvailableVRFID() + vrfID, err := findNextAvailableVRFID(minID, maxID) if err != nil { return err } @@ -281,7 +315,7 @@ func listVRFLinks() ([]*netlink.Vrf, error) { return ListVRFLinks() } -func findNextAvailableVRFID() (uint32, error) { +func findNextAvailableVRFID(minID, maxID uint32) (uint32, error) { vrfs, err := listVRFLinks() if err != nil { return 0, err @@ -291,14 +325,24 @@ func findNextAvailableVRFID() (uint32, error) { for _, vrf := range vrfs { used[vrf.Table] = struct{}{} } + return nextFreeTableID(used, minID, maxID) +} - for vrfID := minVRFID; vrfID <= maxVRFID; vrfID++ { - if _, ok := used[vrfID]; !ok { - return vrfID, nil +// nextFreeTableID returns the lowest table ID in [minID, maxID] that is +// neither in used nor one of the kernel's reserved tables (253 default, 254 +// main, 255 local). A VRF bound to main would route its tenant through the +// host's own table. +func nextFreeTableID(used map[uint32]struct{}, minID, maxID uint32) (uint32, error) { + for vrfID := uint64(minID); vrfID <= uint64(maxID); vrfID++ { + id := uint32(vrfID) + if id == unix.RT_TABLE_DEFAULT || id == unix.RT_TABLE_MAIN || id == unix.RT_TABLE_LOCAL { + continue + } + if _, ok := used[id]; !ok { + return id, nil } } - - return 0, errors.New("could not find any available VRF id") + return 0, fmt.Errorf("no available VRF table id in [%d,%d]", minID, maxID) } func getVRFIDForInterface(name string) (uint32, error) { diff --git a/internal/plumbing/vrf/vrf_internal_test.go b/internal/plumbing/vrf/vrf_internal_test.go new file mode 100644 index 00000000..83bf21da --- /dev/null +++ b/internal/plumbing/vrf/vrf_internal_test.go @@ -0,0 +1,79 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package vrf + +import ( + "testing" + + "golang.org/x/sys/unix" +) + +func TestNextFreeTableID(t *testing.T) { + used := func(ids ...uint32) map[uint32]struct{} { + m := make(map[uint32]struct{}, len(ids)) + for _, id := range ids { + m[id] = struct{}{} + } + return m + } + cases := []struct { + name string + used map[uint32]struct{} + min, max uint32 + want uint32 + wantErr bool + }{ + {name: "empty host range starts at its minimum", used: used(), min: HostTableIDMin, max: HostTableIDMax, want: 1}, + {name: "skips used ids", used: used(1, 2), min: HostTableIDMin, max: HostTableIDMax, want: 3}, + {name: "skips the kernel's reserved tables", used: used(252), min: 252, max: 300, want: 256}, + { + name: "allocates inside a range above the minimum", used: used(), + min: SidecarTableIDBase + 1, max: SidecarTableIDBase + 0xFFF, want: SidecarTableIDBase + 1, + }, + {name: "full range", used: used(10, 11), min: 10, max: 11, wantErr: true}, + { + name: "range of only reserved tables", used: used(), + min: unix.RT_TABLE_DEFAULT, max: unix.RT_TABLE_LOCAL, wantErr: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := nextFreeTableID(tc.used, tc.min, tc.max) + if tc.wantErr { + if err == nil { + t.Fatalf("nextFreeTableID = %d, nil; want an error", got) + } + return + } + if err != nil { + t.Fatalf("nextFreeTableID: %v", err) + } + if got != tc.want { + t.Errorf("nextFreeTableID = %d, want %d", got, tc.want) + } + }) + } +} + +// TestNextFreeTableID_HostAndSidecarNeverOverlap checks the host and the +// ingress sidecar never pick the same table ID. They allocate in separate +// network namespaces, each seeing none of the other's VRFs, but write the same +// egress_route_table keyed by table ID. +func TestNextFreeTableID_HostAndSidecarNeverOverlap(t *testing.T) { + host, err := nextFreeTableID(map[uint32]struct{}{}, HostTableIDMin, HostTableIDMax) + if err != nil { + t.Fatalf("host allocation: %v", err) + } + sidecar, err := nextFreeTableID(map[uint32]struct{}{}, SidecarTableIDBase+1, SidecarTableIDBase+0xFFF) + if err != nil { + t.Fatalf("sidecar allocation: %v", err) + } + if host == sidecar { + t.Fatalf("host and sidecar both allocated table %d from empty namespaces", host) + } + if HostTableIDMax >= SidecarTableIDBase { + t.Fatalf("HostTableIDMax %d reaches SidecarTableIDBase %d", HostTableIDMax, SidecarTableIDBase) + } +} diff --git a/internal/plumbing/vrf/vrf_test.go b/internal/plumbing/vrf/vrf_test.go index 3bc311b1..56f69786 100644 --- a/internal/plumbing/vrf/vrf_test.go +++ b/internal/plumbing/vrf/vrf_test.go @@ -510,3 +510,30 @@ func TestListVRFLinks_SurvivesLinkChurn(t *testing.T) { } } } + +// TestAddInRange_AllocatesInsideTheRange covers the ingress sidecar's use of +// AddInRange: its VRFs must take table IDs from its own range, never the +// host's. +func TestAddInRange_AllocatesInsideTheRange(t *testing.T) { + requireRoot(t) + const vpc = "vtrng" + t.Cleanup(func() { _ = vrf.Delete(vpc) }) + + minID, maxID := vrf.SidecarTableIDBase+1, vrf.SidecarTableIDBase+0xFFF + if err := vrf.AddInRange(vpc, minID, maxID); err != nil { + t.Fatalf("AddInRange: %v", err) + } + tableID, err := vrf.TableID(vpc) + if err != nil { + t.Fatalf("TableID: %v", err) + } + if tableID < minID || tableID > maxID { + t.Fatalf("table ID %d outside [%d,%d]", tableID, minID, maxID) + } + + // The same VPC from a caller with a different range is an error, not a + // silent reuse of a table that may belong to the other writer. + if err := vrf.Add(vpc); !errors.Is(err, vrf.ErrTableOutOfRange) { + t.Fatalf("Add of a VRF whose table %d is outside the host range = %v, want ErrTableOutOfRange", tableID, err) + } +}