Repository navigation
fix: Keep eBPF map contents through a datapath upgrade - #775
Conversation
When a new build's map spec no longer matched a pinned map, the uSID, edge and NAT loaders recovered by unpinning every map in the collection and loading fresh. Every writer's state was lost, not just the changed map's. On a compute node that is fatal for the uSID datapath. locator_table, function_table, vrf_table, ifindex_vrf_table and ifindex_egress_kind_table are written only by CNI ADD, so after the wipe every SRv6 packet to the node missed locator_table and failed open, and every VM stayed unreachable until it was recreated. That happened in staging on 2026-10-07 at 02:02 UTC when #768 grew vip_xlat_table's key from 16 to 32 bytes: both compute nodes lost every row, Envoy returned 503 UF for every VM backend, and recovery took recreating 44 VMs. Edge nodes recovered by themselves because their return loop and the ingress sidecar re-register their own rows. The new mappin package checks each pinned map against the compiled spec with MapSpec.Compatible and unpins only the ones that differ, skipping any the caller does not own (the gateway's dispatcher maps). All three loaders use it, and their warning names the maps they recreated. With this, #768's change would have recreated vip_xlat_table alone, which galactic-router refills from ServiceVIPBindings. A map whose own layout changes is still recreated empty. When that map is one only CNI ADD writes, its rows stay missing until each attachment re-attaches; rebuilding those rows at galactic-cni startup is follow-up work. The new attach test reproduces the incident: it pins an old-layout vip_xlat_table beside seeded vrf_table and locator_table rows and reloads. On the previous loader both rows are gone; with this change only vip_xlat_table is recreated. Related to #774 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If the load reports an incompatible map but no pin under the directory fails MapSpec.Compatible, retrying cannot succeed. The loaders now return the original error, naming the directory, instead of logging "recreating them" with an empty list and failing the same way again. The warning is logged only after the unpin succeeds, and a failed retry is wrapped with the maps that were recreated, so the final error says what the recovery changed. The package doc now says only a map's shape is compared, so a change to what a same-size value means needs a new map name. Related to #774 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Two reviewers read this PR and both returned merge. Conventions reviewer: "VERDICT: merge." It warned that open PR #634 also edits attach.go, attach_test.go and usid.c, so whichever lands second must rebase; this one should land first. Its nits were that the follow-up for CNI-ADD-only rows is tracked by #774 (referenced here with Related to, left open) and that the commit subjects are 53 and 54 characters, within the repo's 72 limit. Adversarial reviewer: "VERDICT: merge, once Unit Tests (root) is green." It could not run the root-only test (not root), so it gated on the Unit Tests (root) check. The author ran TestLoad_SchemaChangeRecreatesOnlyTheChangedMap as root: it passes, and fails on the previous loader. Its other nit, the incident date of 2026-10-07 in the body, is correct in UTC. The RAN: lists were not passed to the agent that posted this comment, so they are not repeated here. No fixes were applied in this phase. CI on head 01ba03d is green: Build, Lint, Unit Tests, Unit Tests (root), E2E Tests, the image and kustomize bundle publish jobs, and license/cla all passed. Auto-merge is enabled with the merge method, the only one the base ruleset allows. The ruleset requires one approving review, code owner review and approval of the last push, so the PR waits for a human approval. |
2 participants
Summary
An upgrade that changed one eBPF map's layout made each node recreate every map empty, which blacked out every VM on both staging compute nodes on 2026-10-07.
Now only the maps whose layout changed are recreated, and every other map keeps its contents.
A map whose own layout changes is still recreated empty. Rebuilding the rows only workload attach writes is a follow-up.
Test plan
Related to #774
🤖 Generated with Claude Code