grpcapi+routing: #4650 GO residuals (A9-F3 gRPC egress parity + A10-F-1)

_Log.md

@@ -1,3 +1,38 @@
+## 2026-07-08 — #4650 (fable-173 LOW) GO residuals: gRPC egress-iface parity + tunnel lock-scope
+
+- **Timestamp**: 2026-07-08
+- **Action**: A9-F3 (LOW) — routed gRPC `sessionEntryV4`/`sessionEntryV6`
+  egress-interface resolution through the same `FibIfindex!=0`-guarded
+  `resolveSessionEgressIface` helper that REST (pkg/api/sessions.go) and the
+  gRPC filter path (server_sessions.go:480/525) already use. The two entry
+  paths inlined an UNCONDITIONAL `egressIfaces[{FibIfindex,FibVlanID}]` lookup,
+  so a FibIfindex==0 session with a stale `{0,vlan}` egressIfaces entry printed
+  a bogus egress iface over gRPC while REST printed the zone iface — a display
+  drift between the two APIs (and an internal gRPC inconsistency vs its own
+  filter path). Fix preserves the `zoneNames` fallback (helper returns
+  zoneIfaces[egressZone]; entry falls back to zoneNames[egressZone] when empty),
+  bit-identical to the REST entry path. Added
+  `pkg/grpcapi/session_egress_drift_4650_test.go` — a REST-oracle parity test
+  (independent guard reproduction) covering V4+V6, the stale-{0,vlan} case
+  (RED on revert: gRPC returned `ge-STALE-0`, want `ge-0-0-2`) and an unchanged
+  FibIfindex!=0 session.
+- **Action**: A10-F-1 (LOW hygiene) — assessed `routing/tunnel.go` `Apply`
+  holding `t.mu` across the full netlink+exec reconcile. DELIBERATE, kept: the
+  reconcile interleaves shared-map reads/writes (ownedNames, appliedAddrs,
+  appliedRI, wgConfigured, tunnels) with the netlink LinkDel/LinkAdd it drives,
+  every `apply*Locked` helper requires the lock, and two concurrent
+  Apply/Clear calls would race BOTH the maps AND kernel link state
+  (double-delete, delete-during-recreate). No isolable critical section to
+  narrow to. GetStatus is the counter-case that already narrows (snapshot
+  names under lock, probe netlink unlocked). Added an inline lock-discipline
+  comment documenting why the wide scope is load-bearing; no behavior change.
+- **Action**: Rust residuals A2-F1/A2-F2/A4-F5/A4-F8 deferred to a separate
+  cargo task (GO-only scope here). No operator-doc change: the egress-iface
+  resolution ordering has no dedicated operator doc; the fix is a
+  display-consistency correction documented in code + this log.
+- **File(s)**: pkg/grpcapi/server_sessions.go,
+  pkg/grpcapi/session_egress_drift_4650_test.go, pkg/routing/tunnel.go, _Log.md
+
 ## 2026-07-08 — #2852 Phase 1: lock-free SNAT port allocation
 
 - **Timestamp**: 2026-07-08

pkg/grpcapi/server_sessions.go

@@ -1188,12 +1188,9 @@ func sessionEntryV4(key dataplane.SessionKey, val dataplane.SessionValue, now ui
 	if inIf == "" {
 		inIf = zoneNames[val.IngressZone]
 	}
-	outIf := egressIfaces[sessionEgressKey{ifindex: val.FibIfindex, vlanID: val.FibVlanID}]
+	outIf := resolveSessionEgressIface(val.FibIfindex, val.FibVlanID, val.EgressZone, zoneIfaces, egressIfaces)
 	if outIf == "" {
-		outIf = zoneIfaces[val.EgressZone]
-		if outIf == "" {
-			outIf = zoneNames[val.EgressZone]
-		}
+		outIf = zoneNames[val.EgressZone]
 	}
 	se := &pb.SessionEntry{
 		SrcAddr:          net.IP(key.SrcIP[:]).String(),
@@ -1244,12 +1241,9 @@ func sessionEntryV6(key dataplane.SessionKeyV6, val dataplane.SessionValueV6, no
 	if inIf == "" {
 		inIf = zoneNames[val.IngressZone]
 	}
-	outIf := egressIfaces[sessionEgressKey{ifindex: val.FibIfindex, vlanID: val.FibVlanID}]
+	outIf := resolveSessionEgressIface(val.FibIfindex, val.FibVlanID, val.EgressZone, zoneIfaces, egressIfaces)
 	if outIf == "" {
-		outIf = zoneIfaces[val.EgressZone]
-		if outIf == "" {
-			outIf = zoneNames[val.EgressZone]
-		}
+		outIf = zoneNames[val.EgressZone]
 	}
 	se := &pb.SessionEntry{
 		SrcAddr:          net.IP(key.SrcIP[:]).String(),

pkg/grpcapi/session_egress_drift_4650_test.go

@@ -0,0 +1,131 @@
+package grpcapi
+
+import (
+	"net"
+	"testing"
+
+	"github.com/psaab/xpf/pkg/dataplane"
+)
+
+// restEgressOracle reproduces, independently of the shared
+// resolveSessionEgressIface helper, exactly how the REST path
+// (pkg/api/sessions.go sessionEntryV4/V6) resolves a session's egress
+// interface: the FibIfindex!=0-guarded egressIfaces lookup, then the
+// egress zone's first interface, then the zone's display name. Used as a
+// parity oracle so a revert of the gRPC entry paths back to the
+// unconditional egressIfaces[{FibIfindex,FibVlanID}] lookup (the A9-F3
+// drift) makes the assertions below go RED.
+func restEgressOracle(fibIfindex uint32, fibVlanID uint16, egressZone uint16, zoneNames, zoneIfaces map[uint16]string, egressIfaces map[sessionEgressKey]string) string {
+	if fibIfindex != 0 {
+		if name, ok := egressIfaces[sessionEgressKey{ifindex: fibIfindex, vlanID: fibVlanID}]; ok && name != "" {
+			return name
+		}
+	}
+	if name := zoneIfaces[egressZone]; name != "" {
+		return name
+	}
+	return zoneNames[egressZone]
+}
+
+// TestSessionEntryEgressIfaceParity_4650 pins A9-F3: the gRPC
+// sessionEntryV4/V6 egress-interface resolution must match the
+// FibIfindex!=0-guarded REST/gRPC-filter resolution, so a session with
+// FibIfindex==0 and a stale {ifindex:0, vlanID} egressIfaces entry does
+// NOT print that bogus interface. Before the fix the gRPC entry paths
+// indexed egressIfaces unconditionally and returned the stale name while
+// REST returned the zone interface — a display drift between the two APIs.
+func TestSessionEntryEgressIfaceParity_4650(t *testing.T) {
+	const (
+		ingressZone uint16 = 1
+		egressZone  uint16 = 2
+		vlanID      uint16 = 50
+	)
+	zoneNames := map[uint16]string{ingressZone: "trust", egressZone: "untrust"}
+	zoneIfaces := map[uint16]string{ingressZone: "ge-0-0-1", egressZone: "ge-0-0-2"}
+
+	// A stale {ifindex:0, vlanID:50} entry — the exact shape that a
+	// FibIfindex==0 session used to mis-resolve to under the unconditional
+	// gRPC lookup. The guarded helper must never consult it.
+	egressIfaces := map[sessionEgressKey]string{
+		{ifindex: 0, vlanID: vlanID}: "ge-STALE-0",   // must be ignored (FibIfindex==0)
+		{ifindex: 7, vlanID: vlanID}: "ge-0-0-7-fib", // consulted only for FibIfindex==7
+	}
+
+	cases := []struct {
+		name       string
+		fibIfindex uint32
+		fibVlanID  uint16
+		want       string
+	}{
+		{
+			// FibIfindex==0 + stale {0,vlan} entry: must fall through to the
+			// egress zone's interface, NOT the stale name. RED on revert.
+			name:       "fib0_stale_entry_falls_through_to_zone_iface",
+			fibIfindex: 0,
+			fibVlanID:  vlanID,
+			want:       "ge-0-0-2",
+		},
+		{
+			// Normal FibIfindex!=0 session: unchanged, resolves the FIB entry.
+			name:       "fib_present_resolves_egress_entry",
+			fibIfindex: 7,
+			fibVlanID:  vlanID,
+			want:       "ge-0-0-7-fib",
+		},
+	}
+
+	key4 := dataplane.SessionKey{
+		SrcIP:    [4]byte{198, 51, 100, 10},
+		DstIP:    [4]byte{172, 16, 80, 8},
+		SrcPort:  hostToNetwork16(54321),
+		DstPort:  hostToNetwork16(443),
+		Protocol: 6,
+	}
+	var src6, dst6 [16]byte
+	copy(src6[:], net.ParseIP("2001:db8::10").To16())
+	copy(dst6[:], net.ParseIP("2001:db8:80::8").To16())
+	key6 := dataplane.SessionKeyV6{
+		SrcIP:    src6,
+		DstIP:    dst6,
+		SrcPort:  hostToNetwork16(54321),
+		DstPort:  hostToNetwork16(443),
+		Protocol: 6,
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			oracle := restEgressOracle(tc.fibIfindex, tc.fibVlanID, egressZone, zoneNames, zoneIfaces, egressIfaces)
+			if oracle != tc.want {
+				t.Fatalf("oracle sanity: got %q want %q", oracle, tc.want)
+			}
+
+			val4 := dataplane.SessionValue{
+				IngressZone: ingressZone,
+				EgressZone:  egressZone,
+				FibIfindex:  tc.fibIfindex,
+				FibVlanID:   tc.fibVlanID,
+			}
+			se4 := sessionEntryV4(key4, val4, 0, zoneNames, map[uint32]string{}, zoneIfaces, egressIfaces, true)
+			if se4.EgressInterface != tc.want {
+				t.Errorf("v4 EgressInterface = %q, want %q", se4.EgressInterface, tc.want)
+			}
+			if se4.EgressInterface != oracle {
+				t.Errorf("v4 EgressInterface = %q drifts from REST oracle %q", se4.EgressInterface, oracle)
+			}
+
+			val6 := dataplane.SessionValueV6{
+				IngressZone: ingressZone,
+				EgressZone:  egressZone,
+				FibIfindex:  tc.fibIfindex,
+				FibVlanID:   tc.fibVlanID,
+			}
+			se6 := sessionEntryV6(key6, val6, 0, zoneNames, map[uint32]string{}, zoneIfaces, egressIfaces, true)
+			if se6.EgressInterface != tc.want {
+				t.Errorf("v6 EgressInterface = %q, want %q", se6.EgressInterface, tc.want)
+			}
+			if se6.EgressInterface != oracle {
+				t.Errorf("v6 EgressInterface = %q drifts from REST oracle %q", se6.EgressInterface, oracle)
+			}
+		})
+	}
+}

pkg/routing/tunnel.go

@@ -275,6 +275,18 @@ func (t *tunnelManager) keepaliveProber() tunnelProber {
 // probes (legacy non-anchor branch only) are reconciled by identity
 // instead of being restarted every apply.
 func (t *tunnelManager) Apply(tunnels []*config.TunnelConfig) error {
+	// mu is held across the WHOLE netlink+exec reconcile deliberately (not
+	// just a small state-read section): the reconcile INTERLEAVES shared-map
+	// reads/writes (ownedNames, appliedAddrs, appliedRI, wgConfigured,
+	// tunnels) with the netlink LinkDel/LinkAdd it drives — per-tunnel
+	// removal/adoption decisions are made FROM the maps and the failure
+	// retries are written BACK into them, and every applyKernelTunnelLocked/
+	// applyAnchorLocked/applyWireguardTunLocked helper requires the lock.
+	// Two concurrent Apply/Clear calls would race both the maps AND the
+	// kernel link state (double-delete, delete-during-recreate), so the wide
+	// scope serializes them. There is no isolable critical section to narrow
+	// to. GetStatus (below) is the counter-case that CAN narrow — it only
+	// snapshots names under the lock, then probes netlink read-only unlocked.
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	t.ensureReconcileStateLocked()