diff --git a/internal/installer/sidecarreturn.go b/internal/installer/sidecarreturn.go index 066223c1..a3a946b3 100644 --- a/internal/installer/sidecarreturn.go +++ b/internal/installer/sidecarreturn.go @@ -22,6 +22,8 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/attach" "go.datum.net/galactic/internal/plumbing/ebpf/uformat" "go.datum.net/galactic/internal/plumbing/ebpf/usidmap" + "go.datum.net/galactic/internal/plumbing/intf" + "go.datum.net/galactic/internal/plumbing/vrf" bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) @@ -79,6 +81,12 @@ const ( // have. sidecarReturnTableBase = 0xF000 sidecarReturnTableMax = sidecarReturnTableBase + uformat.ArgumentMax + + // sidecarReturnRouteProtocol marks routes installed into an existing + // tenant VRF table. The reserved-table range identifies the legacy case, + // but a route shared with a real tenant table needs its own ownership mark + // so pruning cannot touch any other route in that table. + sidecarReturnRouteProtocol = 242 ) // sidecarReturnTableID maps an advertised Argument to the routing table holding @@ -97,8 +105,14 @@ func sidecarReturnTableID(vrfID uint16) (uint32, error) { // toward, plus where on this node that pod can be reached. type sidecarEndpoint struct { advName string + // vpc is the base62 VPC identifier recovered from advName. When this node + // already hosts a real attachment for the VPC, its VRF table must also own + // the sidecar gateway route; registering a second return table under the + // same on-wire (Block, Argument) would overwrite the attachment datapath. + vpc string addr netip.Addr vrfID uint16 + tableID uint32 // hostIfindex is the ifindex, in the host namespace, of the peer of the // pod's primary interface, which is what bpf_redirect_peer must target to // enter that pod. hostMAC is the pod-side end's hardware address. @@ -145,10 +159,15 @@ func ensureSidecarReturnPath(ctx context.Context, k8s client.Client, namespace, // being routed here even on a node whose identity has gone away, and // pruning is the only step that is correct with an empty endpoint set. live := make(map[uint32]netip.Addr, len(endpoints)) - for _, e := range endpoints { - if table, terr := sidecarReturnTableID(e.vrfID); terr == nil { - live[table] = e.addr + var resolveErrs []error + for i := range endpoints { + table, terr := sidecarReturnTableForEndpoint(endpoints[i], vrf.TableID) + if terr != nil { + resolveErrs = append(resolveErrs, fmt.Errorf("advertisement %s: %w", endpoints[i].advName, terr)) + continue } + endpoints[i].tableID = table + live[table] = endpoints[i].addr } if perr := pruneSidecarReturnRoutes(live); perr != nil { slog.Warn("Could not prune stale sidecar return routes", "err", perr) @@ -185,8 +204,11 @@ func ensureSidecarReturnPath(ctx context.Context, k8s client.Client, namespace, return fmt.Errorf("register function_table entry for the sidecar return path: %w", err) } - var errs []error + errs := resolveErrs for _, e := range endpoints { + if e.tableID == 0 { + continue + } if err := installSidecarReturn(registry, block, e); err != nil { // Per-endpoint and never fatal to the sweep: one Envoy pod still // starting must not stop another VPC's return path from being @@ -214,9 +236,9 @@ func installSidecarReturn(registry *usidmap.Registry, block uint64, e sidecarEnd e.hostIfindex, e.addr, err) } - table, err := sidecarReturnTableID(e.vrfID) - if err != nil { - return err + table := e.tableID + if table == 0 { + return errors.New("sidecar return path: endpoint has no resolved routing table") } if err := ensureSidecarReturnRoute(table, e.addr, e.hostIfindex); err != nil { @@ -246,6 +268,7 @@ func ensureSidecarReturnRoute(table uint32, addr netip.Addr, hostIfindex int) er Dst: &net.IPNet{IP: addr.AsSlice(), Mask: net.CIDRMask(addr.BitLen(), addr.BitLen())}, LinkIndex: hostIfindex, Table: int(table), + Protocol: sidecarReturnRouteProtocol, } if err := netlink.RouteReplace(route); err != nil { return fmt.Errorf("install return route %s dev %d table %d: %w", addr, hostIfindex, table, err) @@ -312,7 +335,8 @@ func pruneSidecarReturnRoutes(live map[uint32]netip.Addr) error { // destination, or whose table holds an address live still wants, is left // alone. func staleSidecarReturnRoute(r netlink.Route, live map[uint32]netip.Addr) bool { - if r.Table < sidecarReturnTableBase || r.Table > sidecarReturnTableMax { + reservedTable := r.Table >= sidecarReturnTableBase && r.Table <= sidecarReturnTableMax + if !reservedTable && r.Protocol != sidecarReturnRouteProtocol { return false } if r.Dst == nil { @@ -356,6 +380,16 @@ func sidecarGatewayEndpoints( if !isIngressAdvertisementName(a.Name, nodeName, segment) { continue } + vpcHex, _, ok := strings.Cut(a.Name, "-") + if !ok { + continue + } + vpc, err := intf.HexToBase62(vpcHex) + if err != nil || vpc == "" { + slog.Debug("Sidecar gateway advertisement has an invalid VPC name segment", + "advertisement", a.Name, "segment", vpcHex, "err", err) + continue + } if a.Spec.VRFID == nil { continue } @@ -378,6 +412,7 @@ func sidecarGatewayEndpoints( } out = append(out, sidecarEndpoint{ advName: a.Name, + vpc: vpc, addr: pref.Addr().Unmap(), vrfID: uint16(*a.Spec.VRFID), hostIfindex: hostIfindex, @@ -388,6 +423,21 @@ func sidecarGatewayEndpoints( return out, nil } +// sidecarReturnTableForEndpoint selects the one vrf_table target an arriving +// SID can name. A real local VPC VRF wins because CNI attachments and an +// ingress-sidecar gateway for the same VPC share the same on-wire Argument. +// With no local attachment, the dedicated return table remains the fallback. +func sidecarReturnTableForEndpoint( + e sidecarEndpoint, tableID func(string) (uint32, error), +) (uint32, error) { + if table, err := tableID(e.vpc); err == nil { + return table, nil + } else if !errors.Is(err, vrf.ErrNotFound) { + return 0, fmt.Errorf("resolve local VRF table for vpc %s: %w", e.vpc, err) + } + return sidecarReturnTableID(e.vrfID) +} + // isIngressAdvertisementName reports whether name is what // crdnames.BGPAdvertisementName renders for the ingress attachment on nodeName. // Matched from the end, because a node name may contain the separator while the diff --git a/internal/installer/sidecarreturn_test.go b/internal/installer/sidecarreturn_test.go index 8f9941f7..4eb8b29f 100644 --- a/internal/installer/sidecarreturn_test.go +++ b/internal/installer/sidecarreturn_test.go @@ -6,6 +6,7 @@ package installer import ( "context" + "errors" "net" "net/netip" "testing" @@ -210,6 +211,9 @@ func TestStaleSidecarReturnRoute(t *testing.T) { } keep := netip.MustParseAddr(testSidecarAddr) live := map[uint32]netip.Addr{table: keep} + markedLive := returnRoute(testSidecarAddr+"/128", 1) + markedLive.Protocol = sidecarReturnRouteProtocol + live[1] = keep for _, tc := range []struct { name string @@ -228,6 +232,12 @@ func TestStaleSidecarReturnRoute(t *testing.T) { {"a tenant VRF table", returnRoute(testSidecarAddr+"/128", 1), false}, {"just below the range", returnRoute(testSidecarAddr+"/128", sidecarReturnTableBase-1), false}, {"just above the range", returnRoute(testSidecarAddr+"/128", sidecarReturnTableMax+1), false}, + {"the marked route we want to keep in a tenant table", markedLive, false}, + {"a marked sidecar route in a tenant table", func() netlink.Route { + r := returnRoute("fd30:e2e:1::1/128", 1) + r.Protocol = sidecarReturnRouteProtocol + return r + }(), true}, // vrf.Add's catch-all, which stops tenant traffic falling through to // the main table. Deleting it would reopen that leak. {"a tenant VRF's unreachable default", netlink.Route{ @@ -269,6 +279,9 @@ func TestSidecarGatewayEndpoints_ReadsAdvertisedArgument(t *testing.T) { byAddr := map[string]uint16{} for _, e := range got { byAddr[e.addr.String()] = e.vrfID + if e.vpc != "2" && e.vpc != "4" { + t.Errorf("endpoint %s vpc = %q, want decoded identifier 2 or 4", e.addr, e.vpc) + } } if byAddr[testSidecarAddr] != 1 { t.Errorf("endpoint %s vrfID = %d, want the advertised 1", testSidecarAddr, byAddr[testSidecarAddr]) @@ -278,6 +291,48 @@ func TestSidecarGatewayEndpoints_ReadsAdvertisedArgument(t *testing.T) { } } +func TestSidecarReturnTableForEndpointPrefersLocalVRF(t *testing.T) { + e := sidecarEndpoint{vpc: "2", vrfID: 6} + got, err := sidecarReturnTableForEndpoint(e, func(vpc string) (uint32, error) { + if vpc != "2" { + t.Fatalf("table lookup vpc = %q, want 2", vpc) + } + return 17, nil + }) + if err != nil { + t.Fatalf("sidecarReturnTableForEndpoint() error = %v", err) + } + if got != 17 { + t.Errorf("sidecarReturnTableForEndpoint() = %d, want local table 17", got) + } +} + +func TestSidecarReturnTableForEndpointFallsBackWithoutLocalVRF(t *testing.T) { + e := sidecarEndpoint{vpc: "2", vrfID: 6} + got, err := sidecarReturnTableForEndpoint(e, func(string) (uint32, error) { + return 0, vrf.ErrNotFound + }) + if err != nil { + t.Fatalf("sidecarReturnTableForEndpoint() error = %v", err) + } + want, err := sidecarReturnTableID(e.vrfID) + if err != nil { + t.Fatalf("sidecarReturnTableID() error = %v", err) + } + if got != want { + t.Errorf("sidecarReturnTableForEndpoint() = %d, want reserved table %d", got, want) + } +} + +func TestSidecarReturnTableForEndpointPropagatesLookupFailure(t *testing.T) { + want := errors.New("netlink failed") + _, err := sidecarReturnTableForEndpoint(sidecarEndpoint{vpc: "2", vrfID: 6}, + func(string) (uint32, error) { return 0, want }) + if !errors.Is(err, want) { + t.Fatalf("sidecarReturnTableForEndpoint() error = %v, want %v", err, want) + } +} + // TestSidecarGatewayEndpoints_IgnoresPodAdvertisements is the isolation this // feature needs most. A pod's own advertisement is the same shape as a // sidecar's, and internal/cnibgp has already registered a vrf_table entry