fix: Merge BGPAdvertisement prefixes across shared containers - #284
Merged
Conversation
A single BGPAdvertisement CRD is keyed by (vpc, vpcAttachment) alone, so multiple containers attaching under the same VPCAttachment on a node -- a second pod, or a second interface reusing it -- share one CRD. publishBGPStateK8s was building Spec.Prefixes from only the container currently being processed, so a second container's ADD would clobber the first still-live container's already-published prefix out of the spec. allAdvertisedPrefixes recomputes Spec.Prefixes from every subnet annotation currently present on the CRD instead, so each container's own durable annotation record keeps its prefix present regardless of ADD order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
privateip
added a commit
that referenced
this pull request
Aug 5, 2026
Removes the legacy per-route netlink SRv6 ingress mechanism (srv6.RouteIngressAdd/Del, srv6.go) entirely and replaces it with registration against the eBPF uSID datapath's pinned maps (attach, usidmap -- #283): the eBPF/TC-BPF datapath is now the only ingress/decap path for both veth and tap attachments, so there's no dual-path coexistence to maintain. ComputeSID (internal/plumbing/srv6/usid.go) 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 (vrfIDFromAttachment); 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. Stacked on #283 (eBPF datapath control-plane packages) and #284 (BGPAdvertisement prefix-merge fix, needed for this PR's publishBGPStateK8s changes to apply cleanly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip
assigned
ecv,
0xmc,
scotwells and
slindseysr and unassigned
ecv,
0xmc,
scotwells and
slindseysr
privateip
marked this pull request as ready for review
August 5, 2026 01:59
ecv
approved these changes
Aug 5, 2026
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
seems legit. we should do a pass pulling all the comments out into design docs later
privateip
added a commit
that referenced
this pull request
Aug 6, 2026
Removes the legacy per-route netlink SRv6 ingress mechanism (srv6.RouteIngressAdd/Del, srv6.go) entirely and replaces it with registration against the eBPF uSID datapath's pinned maps (attach, usidmap -- #283): the eBPF/TC-BPF datapath is now the only ingress/decap path for both veth and tap attachments, so there's no dual-path coexistence to maintain. ComputeSID (internal/plumbing/srv6/usid.go) 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 (vrfIDFromAttachment); 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. Stacked on #283 (eBPF datapath control-plane packages) and #284 (BGPAdvertisement prefix-merge fix, needed for this PR's publishBGPStateK8s changes to apply cleanly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip
added a commit
that referenced
this pull request
Aug 6, 2026
Removes the legacy per-route netlink SRv6 ingress mechanism (srv6.RouteIngressAdd/Del, srv6.go) entirely and replaces it with registration against the eBPF uSID datapath's pinned maps (attach, usidmap -- #283): the eBPF/TC-BPF datapath is now the only ingress/decap path for both veth and tap attachments, so there's no dual-path coexistence to maintain. ComputeSID (internal/plumbing/srv6/usid.go) 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 (vrfIDFromAttachment); 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. Stacked on #283 (eBPF datapath control-plane packages) and #284 (BGPAdvertisement prefix-merge fix, needed for this PR's publishBGPStateK8s changes to apply cleanly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
privateip
added a commit
that referenced
this pull request
Aug 6, 2026
Removes the legacy per-route netlink SRv6 ingress mechanism (srv6.RouteIngressAdd/Del, srv6.go) entirely and replaces it with registration against the eBPF uSID datapath's pinned maps (attach, usidmap -- #283): the eBPF/TC-BPF datapath is now the only ingress/decap path for both veth and tap attachments, so there's no dual-path coexistence to maintain. ComputeSID (internal/plumbing/srv6/usid.go) 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 (vrfIDFromAttachment); 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. Stacked on #283 (eBPF datapath control-plane packages) and #284 (BGPAdvertisement prefix-merge fix, needed for this PR's publishBGPStateK8s changes to apply cleanly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A single BGPAdvertisement CRD is keyed by
(vpc, vpcAttachment)alone, so multiple containers attaching under the same VPCAttachment on a node — a second pod, or a second interface reusing it — share one CRD.publishBGPStateK8swas buildingSpec.Prefixesfrom only the container currently being processed, so a second container's CNI ADD would silently drop the first still-live container's already-published prefix from the spec.allAdvertisedPrefixesfixes this by recomputingSpec.Prefixesfrom every subnet annotation currently present on the CRD (each container's durable per-container record) instead of just the one being processed, so every live container's prefix stays present regardless of ADD order.Independent of the eBPF uSID datapath work landing in a separate stack — no shared imports or call-site overlap beyond this file's existing
publishBGPStateK8s.Test plan
go build ./...go vet ./internal/cni/...go test ./internal/cni/...(newTestAllAdvertisedPrefixes*cases, including the multi-container regression case)task lint(0 issues)