An aborted configureTunnelPeers pass withdraws every tunnel peer from the eBPF LPM trie
- Dominant language
- Go
- Stars
- 28
- Forks
- 11
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 55
Description
Found during review of #668. Pre-existing, and orthogonal to that PR: it is why "defer a reconcile pass" is not a benign outcome on the tunnel path.
## The chain
1. `site_watch_reconcile.go:2046-2048` sets `state.pendingBPFEntries = nil` **before** calling `configureTunnelPeers`.
2. `configureTunnelPeers` returns early on any error, so `pendingBPFEntries` is never repopulated. Two of the abort points are `EnsureGeneveInterface` (`tunnel_config.go:253`) and `EnsureVXLANInterface` (`:385`).
3. The caller only warns and continues: `klog.Warningf("Tunnel configuration failed (WireGuard will still be configured): %v", ...)` (`site_watch_reconcile.go:2066-2067`).
4. `reconcilePendingBPFEntries(state)` still runs at `site_watch_reconcile.go:2147`.
5. It reads the nil map and substitutes an **empty** one (`tunnel_config.go:760-762`):
```go
entries := state.pendingBPFEntries
state.pendingBPFEntries = nil
...
if entries == nil {
entries = make(map[string]ebpfpkg.TunnelEndpoint)
}
if err := tm.Reconcile(entries); err != nil {
```
6. `TunnelMap.Reconcile` is documented as *"sets the LPM trie to exactly match the desired state. Stale entries (present in the kernel but not in desired) are removed"* and deletes accordingly (`internal/net/ebpf/tunnel_map.go:288-296`).
An empty desired set therefore means **every** GENEVE/VXLAN/IPIP peer is deleted from the trie, on a pass that only logged a warning.
## Why it matters
The failure is presented as recoverable — the log says WireGuard will still be configured, and the next pass will retry. But the dataplane for every shared-tunnel peer is torn down in the meantime, so the cost of one aborted pass is a tunnel outage rather than a deferred update.
Same shape at `wireguard_config.go:137`, where an `EnsureWireGuardInterface` failure aborts before `SyncRoutes`.
## Suggested direction
Either do not clear `pendingBPFEntries` until `configureTunnelPeers` has produced a replacement, or skip `reconcilePendingBPFEntries` when the pass aborted. The distinction that matters is "desired set is genuinely empty" versus "desired set was never computed" — the current code cannot tell them apart, and treats the second as the first.
## Relationship to #668
#668 changes `Ensure*` to return an error when a link lookup fails for a reason other than the interface being absent, instead of falling through to creation. In the common case the outcome is unchanged, because the old code fell through to `LinkAdd`, got `EEXIST`, and aborted at the same line.
The one genuinely new way to reach this: the lookup fails transiently **and** the interface really is absent. The old code created it and carried on; the new code defers the pass. That is the right trade for correctness, but its cost is this wipe, which is worth fixing on its own terms.
Contributor guide
Research direction
Start at site_watch_reconcile.go:2046-2147 and trace configureTunnelPeers through tunnel_config.go:253, 385, and 760-762, then read internal/net/ebpf/tunnel_map.go:288-296. Ensure an aborted configuration does not reconcile a nil desired set as empty, while a genuinely empty desired set still reconciles normally; consider the analogous failure path in wireguard_config.go:137.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100