Add BFD configuration for BGP neighbors in SONiC config generator (re-implement #1626)

[berendt]

Summary

Implement automatic BFD (Bidirectional Forwarding Detection) configuration for BGP neighbors in the SONiC config generator, so BGP sessions detect link failures quickly. This re-implements the (still open, never merged) PR #1626 from scratch against the current main — the SONiC config generator has changed substantially since August 2025 and the old patch no longer fits.

Original intent (PR #1626)

The PR added an _add_bfd_configurations() helper to osism/tasks/conductor/sonic/config_generator.py that, after the BGP neighbors are built, iterates over every BGP_NEIGHBOR entry and emits a matching BFD_PEER_SINGLE_HOP entry:

• transmit/receive interval 300 ms, detect multiplier 3, echo mode enabled
• BFD key "<vrf>|<interface>|null", local_addr set to the second segment of the BGP neighbor key

It was a single-file change with no tests.

Why a re-implementation (not a rebase/merge)

Since the PR was opened, the generator gained a static config-table ownership model (OWNED_TABLE_KEYS / TOP_LEVEL_SCAFFOLD_KEYS / INHERITED_TABLE_KEYS / IMAGE_CONSUMED_TABLE_KEYS in config_generator.py) that is enforced by tests in tests/unit/tasks/conductor/sonic/test_config_generator_ownership.py. The old patch would fail these guards. The integration requirements below did not exist when the PR was written.

Hard integration requirements (current main)

1. Classify the new table. A static guard (TestStaticTableReferenceGuard::test_every_referenced_table_is_classified) parses the generator source and fails if any config["X"] table is not classified. BFD_PEER_SINGLE_HOP must be added to ON_DEMAND_OWNED_TABLE_KEYS (it is only emitted when BGP neighbors exist, so it is on-demand, not scaffolded). Adding it there also makes it dropped-and-rebuilt on every regen, which is the desired behavior (no stale BFD peers survive a neighbor removal).
2. Use statically-analyzable mutations only. test_config_mutations_are_statically_analyzable requires writes via supported forms (config["X"] = ..., config.setdefault, config.pop, update, |=). The PR's subscript writes are fine; just keep to these forms.
3. Call ordering. _add_bfd_configurations(...) must run after _add_bgp_configurations(...) in generate_sonic_config() (it reads config["BGP_NEIGHBOR"]). Note generate_sonic_config now has signature (device, hwsku, device_as_mapping=None, config_version=None).
4. Validator. BFD_PEER_SINGLE_HOP has no model in the generated YANG schemas (_generated/_schemas.py), so validate_config() will emit a warning ("No YANG schema for table …", validation skipped) rather than an error. Decide whether to also add a schema/leafref model so the table is validated strictly, or to accept the warning. At minimum, confirm the warning is acceptable.

Correctness concerns to fix vs. the old patch

These are bugs/assumptions in PR #1626 that the re-implementation must address, not copy:

1. Heterogeneous neighbor keys. BGP_NEIGHBOR keys are not all interface-based. They are built as f"{vrf}|{connected_ipv4}", f"{vrf}|{port_name}", f"{vrf}|{pc_name}", and f"{vrf}|{peer_ip}" (see config_generator.py around L1258/L1263/L1335/L1340/L1480). The PR blindly treats the second segment as an interface name and assigns it to local_addr — wrong for IP-keyed and port-channel neighbors. Handle each key shape correctly.
2. local_addr semantics. local_addr should be the device's local address/interface, not the peer key segment. Derive it the same way the BGP neighbor block already does (reuse interface_ips / transfer_ips / netbox_interfaces; see L1290–L1308) instead of echoing the neighbor key. This means the helper's extra parameters (interface_ips, netbox_interfaces, transfer_ips, etc.) should actually be used — in the PR they are passed but unused.
3. BFD key format / single-hop applicability. Verify the correct BFD_PEER_SINGLE_HOP key structure for this deployment's neighbor model (interface-based / unnumbered vs. IP-based). The |null local-addr placeholder needs validating against what SONiC actually expects.
4. Enable BFD on the BGP session too. A BFD_PEER_SINGLE_HOP entry alone may not activate BFD for the session — evaluate whether the corresponding BGP_NEIGHBOR entry also needs bfd: "true".
5. Hardcoded timers. 300 ms / multiplier 3 / echo on are hardcoded in the PR. Consider sourcing these from NetBox config context or a generator constant so they are tunable, and decide whether echo mode is safe to enable by default for all neighbors.

Acceptance criteria

• _add_bfd_configurations() (or equivalent) generates BFD_PEER_SINGLE_HOP entries for BGP neighbors, called after _add_bgp_configurations().
• BFD_PEER_SINGLE_HOP classified in ON_DEMAND_OWNED_TABLE_KEYS; all ownership/static-guard tests pass.
• local_addr and BFD keys derived correctly for IP-, interface-, and port-channel-keyed neighbors (no blind key-segment reuse).
• BFD timers/echo are either justified as fixed defaults or made configurable; decision documented.
• Unit tests covering: entry generated per neighbor, correct key/local_addr per neighbor-key shape, table dropped-and-rebuilt on regen, no entries when there are no BGP neighbors.
• validate_config() behavior on the new table is confirmed (warning accepted, or schema added).
• CI green.

References

• Original (open, unmerged) PR: Add BFD configuration for all BGP neighbor interfaces #1626 — "Add BFD configuration for all BGP neighbor interfaces"
• Ownership model: osism/tasks/conductor/sonic/config_generator.py (generate_sonic_config docstring, taxonomy constants) and tests/unit/tasks/conductor/sonic/test_config_generator_ownership.py