fix: Keep sidecar and host VRFs apart in the shared eBPF maps

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)

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, &notFound) {
 			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)
 	}

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,16 +100,25 @@ 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)
 	}
 	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 {

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)
 		}

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.

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

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) {

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.