Skip to content

feat: Cut CNI ADD over to eBPF uSID registration - #285

Closed
privateip wants to merge 0 commit into
mainfrom
pr3-cni-ebpf-cutover
Closed

feat: Cut CNI ADD over to eBPF uSID registration#285
privateip wants to merge 0 commit into
mainfrom
pr3-cni-ebpf-cutover

Conversation

@privateip

Copy link
Copy Markdown
Contributor

Summary

Fourth PR in the eBPF uSID datapath stack (on top of #283, the datapath control-plane packages).

Removes the legacy per-route netlink SRv6 ingress mechanism (srv6.RouteIngressAdd/Del, all of srv6.go) entirely and replaces it with registration against the eBPF uSID datapath's pinned maps. The eBPF/TC-BPF datapath is now the only ingress/decap path for both veth and tap attachments — there's no dual-path coexistence to maintain.

ComputeSID is rewritten onto the shared uFMT 48+16 bit layout (internal/plumbing/ebpf/uformat, #281) instead of its previous ad hoc NodeID/VRFID/Function suffix, so the BGP control plane and the eBPF dataplane can never drift on bit positions.

The CNI ADD path no longer derives the VRFID straight from the VPCAttachment identifier; it now allocates a 12-bit uFMT Argument per node from live BGPVRFInstance CRD state (allocateArgument), with a collision check (checkArgumentCollision) covering the allocate-then-create race between two concurrent ADDs. registerEBPFDatapath/unregisterEBPFDatapath write and roll back the three eBPF map entries (locator_table, function_table, vrf_table) for each attachment.

Note

This branch is built directly on #283, not on #284 (the independent BGPAdvertisement prefix-merge fix) — those two touch nearby code in bgp.go but have no real dependency on each other. Merging #284 first and rebasing this stack is the cleanest order; otherwise expect a normal merge conflict in bgp.go at merge time.

Gateway ARP/NDP neighbor priming (installGatewayNeighbor) is a separate concern and lands in its own follow-up PR stacked on this one.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./internal/cni/... ./internal/plumbing/srv6/... (one pre-existing unrelated failure: TestUpdateForwardRuleInvalidAction needs an iptables binary not present in this sandbox)
  • task lint (0 issues)

Part of the eBPF uSID datapath cutover stack (base: #283).

@ecv

ecv commented Aug 5, 2026

Copy link
Copy Markdown

Same disclosure as elsewhere in the stack — ecv reviewed this through an AI shell, apologies. Questions, not verdicts.

Two pieces of care worth naming. Checking nodeID against its real range before the uint16 narrowing, with a comment showing that 65537 wraps to a plausible 1, catches a bug that would have registered a locator this node was never assigned. And unregisterEBPFDatapath refusing to delete a slot whose VRF table id no longer matches — because a retry may have let another attachment overwrite it — reasons correctly about a subtle race.

  1. allocateArgument short-circuits on an existing BGPVRFInstance and returns its stored VRFID without checking it. The old code parsed that field as a full 16-bit value, so it ranges 0 to 65535; the new Argument range runs 1 to 4095. An existing instance holding anything outside that range flows straight into VRF.Register, which rejects it, and the ADD fails. It fails the same way on every retry, and feat: eBPF vrf_table GC sweep and control-daemon startup #287's sweep skips out-of-range VRFIDs rather than renumbering them, so nothing repairs it. Do any clusters hold BGPVRFInstance objects written by the old code? If containerlab or a dev cluster does, those attachments stop starting after this lands.

  2. allocateArgument lists every BGPVRFInstance in the namespace, then checkArgumentCollision lists them all again. One BGPRouter per node and one namespace means each list returns every attachment in the cluster, so a pod start costs two full reads against a client that does not cache. Has this run against a namespace with a few thousand attachments?

  3. The collision error is a plain fmt.Errorf, so isTransientError says no and retryK8sOps returns at once. That rolls the whole ADD back and leaves the retry to kubelet, when re-running the closure would list again and take the next free Argument. Should the collision count as transient?

  4. The kernel VRF table id still derives from (vpc, vpcAttachment), so it matches on every node. The Argument is now the first free slot per node, so it differs. One attachment now carries two numbers that disagree across nodes. Is that difference written down anywhere an operator would look?

  5. This removes the code that installs the seg6local ingress routes, but nothing removes routes already in the kernel. That matters more than dead config: usid.c returns TC_ACT_OK on a locator miss, which hands the packet to the normal routing stack, where a leftover seg6local route still decapsulates it. A missing or wrong map registration would then keep working on every node that ran the old code and fail only on fresh ones. Is there a cleanup step for those routes, or does the upgrade path assume a rebuild?

  6. The comment explains that locator and function registration moved here because the control daemon never watches BGPRouter, and calls that a departure from the design plan to revisit. Neither table has any GC either (asked on feat: eBPF vrf_table GC sweep and control-daemon startup #287). What closes both halves — a CRD watch in the daemon, or reconciliation from here?

privateip added a commit that referenced this pull request Aug 6, 2026
The eBPF uSID ingress datapath decapsulates SRv6 traffic and calls
bpf_fib_lookup() to resolve the egress path for the inner packet, then
redirects it straight to the resolved neighbor entirely in-kernel,
never touching the normal forwarding stack. bpf_fib_lookup() does not
itself trigger ARP/NDP resolution the way ordinary kernel packet
forwarding does, so without a pre-existing neighbor table entry it
fails with BPF_FIB_LKUP_RET_NO_NEIGH and the packet is dropped --
every cross-region packet to a pod that had never otherwise triggered
NDP for its own address was silently and permanently blackholed.

installGatewayNeighbor installs a permanent neighbor table entry
mapping the pod's address to its guest veth's own known MAC at CNI
ADD, so this resolution never depends on dynamic ARP/NDP. guestHWAddr
now flows from buildVethResult through publishBGPState into
configureHostGateway; nil for tap attachments, which have no separate
guest-side link in this netns to resolve a MAC from.

Stacked on #285 (eBPF uSID registration cutover), since the neighbor
entry only matters once the eBPF datapath is the one doing the fib
lookup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip added a commit that referenced this pull request Aug 6, 2026
Adds SweepEBPFVRFTable, a GC pass that reconciles the eBPF uSID
datapath's vrf_table map entries against live BGPVRFInstance CRDs
using a generation-cutoff scheme to avoid a register/sweep race. This
runs from galactic-cni's "run" container rather than
galactic-router's existing GC controller: the pinned vrf_table map
only exists inside that container, which has the /sys/fs/bpf hostPath
mount and CAP_BPF galactic-router's DaemonSet does not need for
anything else. routerNamesForNode gains a fuller sibling,
routersForNode, since the sweep needs each router's full
Spec.SRv6Locator, not just its name.

Wires installer.Run to load/attach/pin the eBPF datapath at startup
(startEBPFDatapath), serve /metrics (Prometheus), report an
"ebpf-datapath" gRPC health sub-service, and run the GC sweep on its
own ticker. Adds the --metrics-port CLI flag to galactic-cni run.

Depends only on #283 (attach/usidmap/metrics/uformat) -- no
dependency on the CNI ADD cutover in #285/#286, since this reconciles
directly against BGPRouter/BGPVRFInstance CRD state rather than
anything registerEBPFDatapath writes to resourceTracker.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip added a commit that referenced this pull request Aug 6, 2026
The eBPF uSID ingress datapath decapsulates SRv6 traffic and calls
bpf_fib_lookup() to resolve the egress path for the inner packet, then
redirects it straight to the resolved neighbor entirely in-kernel,
never touching the normal forwarding stack. bpf_fib_lookup() does not
itself trigger ARP/NDP resolution the way ordinary kernel packet
forwarding does, so without a pre-existing neighbor table entry it
fails with BPF_FIB_LKUP_RET_NO_NEIGH and the packet is dropped --
every cross-region packet to a pod that had never otherwise triggered
NDP for its own address was silently and permanently blackholed.

installGatewayNeighbor installs a permanent neighbor table entry
mapping the pod's address to its guest veth's own known MAC at CNI
ADD, so this resolution never depends on dynamic ARP/NDP. guestHWAddr
now flows from buildVethResult through publishBGPState into
configureHostGateway; nil for tap attachments, which have no separate
guest-side link in this netns to resolve a MAC from.

Stacked on #285 (eBPF uSID registration cutover), since the neighbor
entry only matters once the eBPF datapath is the one doing the fib
lookup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@ecv ecv left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conflicts

privateip added a commit that referenced this pull request Aug 6, 2026
Adds SweepEBPFVRFTable, a GC pass that reconciles the eBPF uSID
datapath's vrf_table map entries against live BGPVRFInstance CRDs
using a generation-cutoff scheme to avoid a register/sweep race. This
runs from galactic-cni's "run" container rather than
galactic-router's existing GC controller: the pinned vrf_table map
only exists inside that container, which has the /sys/fs/bpf hostPath
mount and CAP_BPF galactic-router's DaemonSet does not need for
anything else. routerNamesForNode gains a fuller sibling,
routersForNode, since the sweep needs each router's full
Spec.SRv6Locator, not just its name.

Wires installer.Run to load/attach/pin the eBPF datapath at startup
(startEBPFDatapath), serve /metrics (Prometheus), report an
"ebpf-datapath" gRPC health sub-service, and run the GC sweep on its
own ticker. Adds the --metrics-port CLI flag to galactic-cni run.

Depends only on #283 (attach/usidmap/metrics/uformat) -- no
dependency on the CNI ADD cutover in #285/#286, since this reconciles
directly against BGPRouter/BGPVRFInstance CRD state rather than
anything registerEBPFDatapath writes to resourceTracker.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip added a commit that referenced this pull request Aug 6, 2026
The eBPF uSID ingress datapath decapsulates SRv6 traffic and calls
bpf_fib_lookup() to resolve the egress path for the inner packet, then
redirects it straight to the resolved neighbor entirely in-kernel,
never touching the normal forwarding stack. bpf_fib_lookup() does not
itself trigger ARP/NDP resolution the way ordinary kernel packet
forwarding does, so without a pre-existing neighbor table entry it
fails with BPF_FIB_LKUP_RET_NO_NEIGH and the packet is dropped --
every cross-region packet to a pod that had never otherwise triggered
NDP for its own address was silently and permanently blackholed.

installGatewayNeighbor installs a permanent neighbor table entry
mapping the pod's address to its guest veth's own known MAC at CNI
ADD, so this resolution never depends on dynamic ARP/NDP. guestHWAddr
now flows from buildVethResult through publishBGPState into
configureHostGateway; nil for tap attachments, which have no separate
guest-side link in this netns to resolve a MAC from.

Stacked on #285 (eBPF uSID registration cutover), since the neighbor
entry only matters once the eBPF datapath is the one doing the fib
lookup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Base automatically changed from pr2-ebpf-datapath to main August 6, 2026 21:12
@privateip privateip closed this Aug 6, 2026
@privateip
privateip force-pushed the pr3-cni-ebpf-cutover branch from 25cf4ad to 7139fc2 Compare August 6, 2026 21:13
privateip added a commit that referenced this pull request Aug 6, 2026
…p-on-error

Three independent fixes found while validating the eBPF uSID cutover,
none of which touch the eBPF datapath itself:

- internal/plumbing/srv6/egress.go: switches SEG6 encapsulation from
  SEG6_IPTUN_MODE_ENCAP ("full" encap, always adds a Routing Header)
  to the kernel's SEG6_IPTUN_MODE_ENCAP_RED ("reduced" encap, omits it
  for a single-segment list). usid.c's ingress decap strips exactly a
  fixed 40-byte outer IPv6 header with no allowance for a Routing
  Header, so full encap corrupted every cross-region uSID packet.
  Also adds netlink.Via/family handling so an IPv4 VPC prefix
  (End.DT46) can egress through an IPv6 SID next-hop without violating
  netlink's same-family route constraint, and CheckSEG6EncapRed, a
  startup kernel-capability probe wired into cmd/galactic-router's
  tenant-mode startup (root.go).
- internal/runtime/gobgp/{monitor,paths}.go: attaches the RFC 9252 BGP
  Prefix-SID path attribute to every advertised path and prefers it
  over the EVPN NLRI's GWIPAddress field on receive, since GWIPAddress
  can't carry an IPv6 SID alongside an IPv4 EVPN Type-5 prefix.
- internal/reconcile/reconcile.go: BuildDesiredRouter now skips (logs +
  continue) a single BGPAdvertisement whose SID can't be computed
  instead of failing the whole router build.

Depends on #285's usid.go rewrite (ComputeSID onto the uFMT 48+16
layout): reconcile.go calls ComputeSID directly, and its tests assert
against uFMT-encoded values.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants