Repository navigation
fix: Retry netlink listings that another process interrupts - #1137
Conversation
Most link, address, route, neighbor and rule listings in the node binaries made a single netlink attempt. The kernel interrupts a listing when another process changes the state it walks, so any link churn on a node failed garbage collection's VRF lookups, datapath repair passes and the host gateway's tap neighbor sweep, which silently resolved nothing that pass. Under steady link churn about half of garbage collection's VRF lookups failed. nldump now also lists neighbors and rules, counted under new neighbor and rule kinds on the existing galactic_netlink_dump_* counters. A Lister from nldump.On retries over a handle the caller already holds, for callers whose handle lives in another network namespace, such as the edge agent's host attachment code and the CNI plugins' container-namespace checks. Every remaining single-attempt listing in garbage collection, datapath repair, the host gateway, host configuration, the CNI installer and plugins, the edge agent, the eBPF attach code, the egress route map, SRv6 egress and plain routes, VIP binding and loopback address detection now goes through nldump, so each discards an interrupted result and retries within the existing bound. On Linux 7.1 only link and address listings were seen interrupted under churn; route, neighbor and rule listings are retried the same way if a kernel reports one, as netlink documents they can. The new nldumptest package churns links and addresses in a test namespace and repeats a pass until the kernel has interrupted its listings. Root tests use it to require that garbage collection's VRF resolution, a full datapath repair pass and the tap neighbor sweep each complete 200 passes under churn, with at least one link listing interrupted; all three fail on the previous revision within the first few passes. Resolves #1133
|
pr-adversary: VERDICT: merge. No correctness problems found. Every non-test LinkList/AddrList/RouteList/RouteListFiltered/NeighList/RuleList in cmd and internal now goes through nldump, including handle-based callers via nldump.On(handle). pr-conventions-reviewer: VERDICT: merge. The PR meets the commit and body conventions and has no deploy-path files. Its one nit: open draft #598 edits internal/installer/installer.go and internal/plumbing/srv6/egress.go, which this PR also edits, so #598 needs a rebase whichever lands first. What ran: the adversary ran go build, go vet and unit tests for nldump, hostgw, gc, cni, installer, hostattach, srv6, vip and loaddr. It did not run the root churn tests (not root); the author ran them as root locally, passing on the fix and failing on the previous main 3 of 3. The conventions reviewer read the commits, body and changed files. No findings needed applying, so no fix commits were added. CI on head ded6132: every check passed (Build, Lint, Unit Tests, Unit Tests (root), E2E Tests, Manifests, Documentation, Service API bundle, image publishes, CLA); publish-service-api was skipped. Auto-merge is enabled with the merge method, the only one the ruleset allows. The ruleset requires one approving review from a code owner on the latest push, so the PR waits for a human approval. |
Summary
A link added or removed on a node made most netlink listings there fail, because each made one attempt, so garbage collection, datapath repair and the host gateway's tap neighbor sweep lost passes under churn.
Every link, address, route, neighbor and rule listing in the node binaries now retries an interrupted listing within the existing bound and never uses a partial result, including the CNI plugins and the edge agent.
The netlink dump counters gain
neighborandrulekinds, and nothing changes in the API, configuration or manifests.Test plan
Resolves #1133