Skip to content

fix(installer): preserve local VPC dispatch with ingress sidecars #789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
scotwells wants to merge 1 commit into from
Closed

fix(installer): preserve local VPC dispatch with ingress sidecars #789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions internal/installer/sidecarreturn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand All @@ -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
Expand Down
55 changes: 55 additions & 0 deletions internal/installer/sidecarreturn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package installer

import (
"context"
"errors"
"net"
"net/netip"
"testing"
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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])
Expand All @@ -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
Expand Down
Loading