BFD status in SB DB is never reconciled with the actual session state unless a real transition occurs

[remeq]

Summary

ovn-controller's BFD implementation writes the SB BFD.status column only
when the local RFC 5880 state machine actually transitions
(bfd_check_detection_timeout() on detection-timeout expiry, or
pinctrl_handle_bfd_msg() on receiving a control packet). There is no code
path that compares the current SB value against local reality and corrects
a mismatch when no transition occurs.

As a result, once a BFD.status row is stale or externally modified,
nothing fixes it until the session undergoes a real flap. Since ECMP
nexthop selection consumes this value (via the NB mirror), a stale status
causes persistent incorrect routing with no self-healing.

Two confirmed scenarios (lab-verified on 24.03.6; code unchanged on main)

1. Fresh session meeting a stale row. When ovn-controller (re)creates a
   local session — e.g. after claiming a gateway port — it initializes to
   ADMIN_DOWN; on the next pass, if the existing SB status isn't
   "admin_down", it silently downgrades local state to DOWN with
   change_state = false (no write). If the path is genuinely unreachable
   no packets ever flow, bfd_check_detection_timeout() returns
   immediately for DOWN state, and a stale "up" persists forever.
   Verified: plant status=up on a down session, restart ovn-controller —
   the stale up is never corrected.
2. Externally modified row on a healthy session. Set status=down
   (e.g. via ovn-sbctl) while the session is UP and exchanging packets:
   the BFD_STATE_UP case only sets change_state on a transition away
   from UP, not on further confirmations — the wrong down persists until
   a real flap. Verified: it sat uncorrected indefinitely (60 s+, unbounded).

Root cause

bfd_monitor_run() (controller/pinctrl.c; the single write at
sbrec_bfd_set_status()) gates the write entirely on
entry->change_state, which only actual local state transitions set. The
sync branches that silently absorb a stale SB value (change_state = false) came from v4 of the original BFD series, scoped to handling
controller restarts — the general reconciliation gap appears unintended
(no review comment or changelog entry across the series discusses it).

Suggested fix

Add a reconciliation check in bfd_monitor_run(): when no transition is
pending, compare SB status against bfd_get_status(entry->state) and
issue a corrective write when they diverge — guarded so that steady state
writes nothing, SB "admin_down" (the NB/CMS-driven override) is never
overwritten, and a local ADMIN_DOWN entry never stamps over a live row.
Purely a writer-side fix; no schema or topology changes.

We have such a patch (including a sandbox testcase that fails without it
and passes with it) validated on a live 2-chassis deployment: an external
bogus down on a healthy session is corrected in ~30 ms, a bogus up on
a down session in ~1 s, and the fresh-session-stale-up case corrects on
the first monitor pass after the claim; statuses are write-free in steady
state. Happy to submit it — it applies to main and branch-24.03.

Related observation (separate issue?)

While testing under RBAC we found that BFD status writes from
ovn-controller are rejected for distributed-gateway-port sessions on
RBAC-enabled deployments: the RBAC rule authorizes by chassis_name, but
northd never populates chassis_name for DGP rows (it consults the LRP's
own Port_Binding, which is type patch and never bound). That affects
even today's transition writes and is addressed by the chassis_name
handling in PR #321 / issue #320; mentioning it here because any
reconciliation fix (this issue) is also subject to it.

---

Analysis and patch developed with assistance from Claude Code.