ospf: RFC 9129 ietf-ospf YANG support (operational state + config-write)#22058
Draft
lamestllama wants to merge 45 commits into
Draft
ospf: RFC 9129 ietf-ospf YANG support (operational state + config-write)#22058lamestllama wants to merge 45 commits into
lamestllama wants to merge 45 commits into
Conversation
Member
|
Please stop closing/opening PRs for the same changes. Update your branch and force push to the same PR. |
Greptile SummaryThis PR implements broad RFC 9129
Confidence Score: 3/5Generally safe to review further but the hello/dead-interval timer omission means YANG-managed configs can silently diverge from FRR's running state after a delete operation. The timer destroy gap (hello-interval and dead-interval, both OSPFv2 and OSPFv3) means that once an operator sets these values via YANG and then removes them, FRR keeps the old values indefinitely while the YANG datastore reports RFC defaults — a silent, persistent config/oper split that is difficult to detect and cannot be recovered through YANG alone. ospfd/ospf_nb.c and ospf6d/ospf6_nb.c (callback registration for hello-interval and dead-interval), and their corresponding implementation files ospfd/ospf_nb_config.c and ospf6d/ospf6_nb_config.c where the destroy functions need to be added. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as vtysh CLI
participant NB as northbound
participant MGMTD as mgmtd
participant OSPFD as ospfd backend
participant OSPF6D as ospf6d backend
Note over CLI,OSPF6D: Config write path (e.g. area stub)
CLI->>NB: "nb_cli_enqueue_change(area-type=stub-area)"
NB->>MGMTD: candidate datastore update
MGMTD->>MGMTD: libyang VALIDATE (when/must clauses)
MGMTD->>OSPFD: NB_EV_VALIDATE callback
OSPFD-->>MGMTD: NB_OK (vlink check at VALIDATE)
MGMTD->>OSPFD: NB_EV_APPLY callback
OSPFD-->>MGMTD: ospf_area_stub_set()
Note over CLI,OSPF6D: Predicate-aware dispatch (RFC 8349 shared list)
MGMTD->>MGMTD: mgmt_be_xpath_prefix(map_path, xpath)
MGMTD->>MGMTD: strip predicates + check compatibility
MGMTD->>OSPFD: "route type=ietf-ospf:ospfv2 requests here"
MGMTD->>OSPF6D: "route type=ietf-ospf:ospfv3 requests here"
Note over CLI,OSPF6D: RPC path (clear-neighbor)
CLI->>MGMTD: mgmt rpc /ietf-ospf:clear-neighbor
MGMTD->>MGMTD: mgmt_grpc_rpc_event (or FE adapter)
MGMTD->>OSPFD: mgmt_txn_send_rpc_notify
OSPFD-->>MGMTD: RPC result
MGMTD-->>CLI: response
Note over CLI,OSPF6D: Notification path
OSPFD->>NB: nb_notification_send(nbr-state-change)
NB->>MGMTD: dispatch to subscribers
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
ospfd/ospf_nb_config.c:946-999
**Missing `destroy` callbacks for `hello-interval` and `dead-interval`**
Neither `hello-interval` nor `dead-interval` registers a destroy callback (confirmed in `ospfd/ospf_nb.c` registration tables). When an operator deletes either leaf from the YANG candidate, the northbound framework calls `nb_callback_destroy`, finds `cbs.destroy == NULL`, and silently returns `NB_OK` — leaving `params->v_hello` / `params->v_wait` / `params->is_v_wait_set` at their last-set values while the YANG running config no longer carries the explicit leaf.
The immediate consequence is a permanent split: the YANG datastore reports the RFC default (10 s / 40 s) while ospfd keeps the old operator-supplied value. Any subsequent `show run` → `mgmt commit apply` cycle based on YANG output will omit those intervals, leaving ospfd stuck with stale timers indefinitely.
Every sibling that has a non-trivial revert (`retransmit-interval`, `priority`, `cost`, etc.) registers an explicit destroy. `hello-interval` and `dead-interval` need the same treatment, resetting `params->v_hello` to `OSPF_HELLO_INTERVAL_DEFAULT`, unsetting `SET_IF_PARAM(params, v_hello)`, resetting `params->v_wait` to `OSPF_ROUTER_DEAD_INTERVAL_DEFAULT` when `is_v_wait_set` is false (the auto-derived path), clearing `is_v_wait_set`, and then calling the matching timer-update helpers.
### Issue 2 of 2
ospf6d/ospf6_nb_config.c:751-830
**Missing `destroy` callbacks for `hello-interval` and `dead-interval` (OSPFv3 mirror)**
Same issue as the OSPFv2 side: `ospf6d/ospf6_nb.c` registers only `modify` for these two leaves. Deleting them from the candidate leaves `oi->hello_interval` and `oi->dead_interval` at whatever value the last `modify` set, while the YANG running config shows the RFC defaults.
The OSPFv3 destroy should reset `oi->hello_interval` to `OSPF6_INTERFACE_HELLO_INTERVAL` and `oi->dead_interval` to `OSPF6_INTERFACE_DEAD_INTERVAL`, and call the appropriate timer-update helper, mirroring what the interface destroy already does for these fields.
Reviews (25): Last reviewed commit: "ospf: RFC 9129 nssa-translator-status-ch..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
from
5b9699b to
4b9886d
Compare
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
from
4b9886d to
1bfe80e
Compare
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
3 times, most recently
from
ed9b4d6 to
05b4fac
Compare
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
2 times, most recently
from
32dcdb3 to
c48c147
Compare
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
4 times, most recently
from
c1ac4db to
af790dd
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
2 times, most recently
from
777454f to
ada554e
Compare
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
from
e5b76ea to
2a634e7
Compare
… batching
Add a single-router topology that exercises the daemon-direct
startup config-file batching path added by
lib: batch direct-daemon config-file loads into one NB transaction
The ospfd fixture places
area 0.0.0.61 default-cost 31
area 0.0.0.61 stub no-summary
with default-cost *before* the stub-area line. With per-line commit
the first line fails RFC 9129's "default-cost is valid only on
stub/NSSA areas" when clause and the daemon never reaches the
stub-area line. Only the batched path, where the whole file commits
in one northbound transaction, lets the two leaves validate against
each other and both end up in the committed running configuration.
The ospf6d fixture carries a matching stub area without default-cost
(v3 has no default-cost surface) so the v3 startup-batching code path
is exercised too.
The test asserts both stub-area lines are present in show
running-config after the topology has started.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Replace the older "operational data only" sections in doc/user/ospfd.rst
and doc/user/ospf6d.rst with a YANG / NETCONF Support entry that lists
every config-write leaf the ietf-ospf series now supports plus the
deliberate scope gaps:
* redistribute and default-information-originate remain legacy-CLI
only (RFC 9129 / RFC 8349 leave them to a separate import / export
mechanism).
* Per-address overrides (e.g. `ip ospf cost N A.B.C.D`) have no RFC
9129 counterpart; YANG is strictly per-interface.
* FRR-specific NSSA augments (translator-role, suppress-fa,
default-information-originate) are not in the RFC 9129 area
grouping; they remain legacy-CLI-only.
* `router ospf [vrf NAME]` instance creation is still CLI-only;
YANG operations that target a non-existent instance are rejected
at VALIDATE with a clear error pointing at `router ospf`.
* ospf6d has no per-area stub default-cost knob; the leaf is not
implemented (mgmtd reports "no backend handles this path"). This
matches the existing v3 CLI surface, not a regression introduced
by this conversion.
* ospf6 interface-type rejects non-broadcast and hybrid at VALIDATE
(ospf6d only accepts broadcast, point-to-point and
point-to-multipoint).
Adds a worked example showing mgmt set-config / commit apply for the
explicit-router-id leaf so operators see the end-to-end mgmtd path.
Documents the frr-deviations-ietf-routing-ospf interface-name leafref
relaxation and the restoration of referential checks inside the
per-interface callbacks.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
The converted ietf-ospf config leaves live below the ietf-routing control-plane-protocol list. The legacy router ospf and router ospf6 commands still create daemon instances directly, so they also need to seed and destroy the corresponding RFC 9129 parent list entry in the northbound candidate. Without that parent entry, child writes can apply in the daemon while the candidate tree is missing the list entry that owns them. Register create and destroy callbacks for the OSPFv2 and OSPFv3 control-plane-protocol entries, route the CLI router/no-router lifecycle through those callbacks, and clear pending NB changes after direct daemon teardown. Keep OSPFv3 config-file batching enabled immediately before ospf6_init(), where the converted config wrappers are registered. Share the RFC 9129 control-plane-protocol XPath helpers between module info, backend registration, and CLI wrappers, and use the daemon's existing instance-name helpers instead of repeating default-instance literals. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Finishes the per-interface RFC 9129 tunable set. Adds modify+destroy
callbacks for transmit-delay on both daemons, following the same
shape as retransmit-interval:
* ospfd mutates `params->transmit_delay` on IF_DEF_PARAMS with the
SET_IF_PARAM marker; destroy restores OSPF_TRANSMIT_DELAY_DEFAULT
(1) and is also folded into the per-interface destroy cascade.
* ospf6d mutates `oi->transdelay` directly; destroy restores
OSPF6_INTERFACE_TRANSDELAY (1).
* Resolves the instance and interface from the dnode through the
same _resolve_instance / _resolve_interface helpers used for the
rest of the per-interface table, gating on NB_EV_APPLY.
* No protocol side effects: transmit-delay is a passive flood-time
scalar, so no neighbour reschedule or interface-state update is
needed.
No YANG deviation is required because RFC 9129's transmit-delay has
no inter-leaf constraint analogous to dead > hello.
Topotest: adds test_ospf_yang_area_interface_transmit_delay_config
as a focused mgmtd round-trip test for both daemons; the existing
generic negative tests (test_ospf_yang_negative_missing_instance,
test_ospf_yang_negative_missing_interface) already cover the
validation rejections through the same _resolve_* chain.
Doc: extends the converted-leaves paragraph and adds the matching
row in the mapping table, per the convention at the bottom of the
table ("When adding another RFC 9129 config node, add its row here
before or alongside the callback implementation.").
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
…spf YANG
Extends the per-interface CLI-through-YANG conversion already done for
cost / hello-interval / dead-interval / priority / mtu-ignore / passive to
the two remaining per-interface scalars that have RFC 9129 callbacks:
retransmit-interval and transmit-delay.
Commands converted on each daemon:
v2: ip ospf retransmit-interval / no ip ospf retransmit-interval +
the hidden `ospf retransmit-interval` / `no ospf retransmit-interval`
backwards-compat aliases
ip ospf transmit-delay / no ip ospf transmit-delay + the matching
hidden `ospf transmit-delay` / `no ospf transmit-delay` aliases
v3: ipv6 ospf6 retransmit-interval / no ipv6 ospf6 retransmit-interval
ipv6 ospf6 transmit-delay / no ipv6 ospf6 transmit-delay
Conversion pattern (matches the cost / hello / dead / priority / mtu-ignore
/ passive slice):
* v2 commands take an optional `[A.B.C.D]` per-address override that
has no RFC 9129 representation. Each DEFPY_YANG body checks
`if (!ifaddr_str && per_iface_xpath returns 0)` and dispatches
through nb_cli_enqueue_change / nb_cli_apply_changes; on the
negative path it falls back to the legacy direct-mutation logic,
preserving the per-address-override capability for operators that
need it.
* The YANG path also requires the interface to be in an area
(if_area on IF_DEF_PARAMS). Operators setting per-interface attrs
on an unattached interface continue to use the legacy path -- the
YANG model can't express "interface params before area assignment"
because the leaves live under areas/area[id]/interfaces.
* Per-leaf bodies factor into small `_set_apply` / `_unset_apply`
helpers shared by the main `ip ospf X` form and the hidden
backwards-compat `ospf X` alias (v2 only).
* v3 collapses the legacy DEFUN+ALIAS pair into two DEFPY_YANG forms
(one for set, one for `no`), each guarded by `ospf6_per_iface_xpath`
before falling back to direct ospf6_interface mutation.
Topotest: extends `test_ospf_per_iface_cli_routes_through_yang` to drive
`ip ospf retransmit-interval 23` + `ip ospf transmit-delay 31` (and v3
equivalents) on r1-eth1 alongside the previously-covered six leaves,
verifies each lands in `show running-config <daemon>`, then unwinds via
`no` and verifies absence. The `b3b` and `transmit-delay` focused
mgmtd-write tests continue to cover the YANG-side of the round-trip.
Doc: no change. The converted-leaves paragraph already lists
retransmit-interval and transmit-delay; the "Existing CLI commands for
those leaves set the same YANG nodes as mgmtd writes" sentence now
becomes accurate for these two.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Extends test_ospf_per_iface_cli_routes_through_yang to exercise
`ip ospf network point-to-point` and the v3 equivalent alongside
the other per-interface CLIs, and adds dedicated CLI tests for
the new YANG-routed paths:
* test_ospf_network_dmvpn_falls_back_to_legacy -- confirms the
FRR-augment fallback for `ip ospf network point-to-point dmvpn`
keeps producing the expected running-config text and that
`no ip ospf network` unwinds both the legacy and the YANG-
backed state cleanly.
* test_ospf_distance_cli_routes_through_yang -- cycles `distance
137` / `no distance` and the three-scope `distance ospf
intra-area X inter-area Y external Z` / `no distance ospf` on
ospfd and asserts the running-config cycles in and out as
expected. v3 path coverage stays in
test_ospf_yang_preference_config because legacy distance on
OSPF6_NODE additionally calls ospf6_restart_spf, which trips
a pre-existing ospf6_route_remove_all use-after-free with a
populated route table.
Coverage for the area attachment and area range CLI conversions
(`ip ospf area`, `ipv6 ospf6 area`, `area X range PREFIX [...]`)
relies on the existing
test_ospf_yang_area_interface_*_config / area_ranges_config tests,
which exercise the same YANG list-create / leaf-modify path via
`mgmt set-config`; a dedicated legacy-CLI fixture would require a
topology without `network <prefix> area X` statements and is left
for a follow-up.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Wires the per-instance maximum-ECMP-paths leaf on both ospfd and
ospf6d. RFC 9129 types `/spf-control/paths` as uint16 (range
1..65535), which trivially covers FRR's compile-time `MULTIPATH_NUM`
cap (16 by default, up to 64 with `--enable-multipath=0` etc.).
What the slice contains:
* `ospfd_ietf_ospf_spf_control_paths_modify` / `_destroy` and
matching `ospf6d_*` callbacks that update
`ospf->max_multipath` / `ospf6->max_multipath` and trigger SPF
restart, mirroring the long-standing `ospf_maxpath_set` /
`ospf6_maxpath_set` helpers.
* Registration of `OSPF{,6}D_IETF_OSPF_XPATH "/spf-control/paths"`
in the existing `ospfd_ietf_ospf_info` / `ospf6d_ietf_ospf_info`
arrays with `cfg_opt_in = true`.
* `DEFUN` -> `DEFPY_YANG` conversion of `maximum-paths N` /
`no maximum-paths` on both daemons. The grammar widens to
`(1-65535)` so the YANG range fully drives clippy; the body
enforces the FRR `MULTIPATH_NUM` cap and emits the canonical
error message before any YANG dispatch.
* Doc table row in
`doc/developer/ospf-yang-northbound-notes.rst` and the converted
leaves paragraph extended with `spf-control paths`.
* Two topotests in `ospf_topo1/test_ospf_topo1.py`:
`test_ospf_yang_spf_control_paths_config` round-trips the leaf
via `mgmt set-config` for both daemons;
`test_ospf_max_multipath_cli_routes_through_yang` confirms the
legacy `maximum-paths` / `no maximum-paths` CLI continues to
drive the YANG callback.
`show running-config` rendering is unchanged: the existing config
writer reads `max_multipath` from the FRR struct that the YANG
callback already mutates, so no `cli_show` is needed.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Wires the per-instance MPLS LDP/IGP-sync toggle for ospfd. RFC 9129
models it as a boolean leaf `/mpls/ldp/igp-sync`. ospf6d has no
LDP/IGP sync implementation, so this slice is OSPFv2-only -- the
ospf6d module-info array gets no corresponding entry and the doc
table flags the gap.
What the slice contains:
* `ospfd_ietf_ospf_mpls_ldp_igp_sync_modify` / `_destroy` in
`ospfd/ospf_nb_config.c`. The modify callback validates that the
instance is in the default VRF (FRR restricts LDP-sync to the
default VRF; the legacy CLI rejects non-default with
`ldp-sync only runs on DEFAULT VRF`). Apply for `true` registers
the opaque LDP-IGP zclient handlers and walks every interface in
the VRF to start sync; apply for `false` and destroy both call
`ospf_ldp_sync_gbl_exit(ospf, true)`, which clears the
enable/holddown flags and unwinds per-interface state.
* Registration of `OSPFD_IETF_OSPF_XPATH "/mpls/ldp/igp-sync"` in
`ospfd_ietf_ospf_info` with `cfg_opt_in = true`, placed
immediately after `/spf-control/paths`.
* `DEFPY` -> `DEFPY_YANG` conversion of `mpls ldp-sync` /
`no mpls ldp-sync` in `ospfd/ospf_ldp_sync.c`. The bodies enqueue
a YANG modify / destroy on the leaf and let the callback do the
work, so vtysh and `mgmt set-config` produce the same datastore
state.
* `ospf_per_instance_xpath` is exposed via `ospfd/ospf_vty.h` so the
new DEFPY_YANG bodies in `ospf_ldp_sync.c` can build the
instance-level xpath without duplicating snprintf logic.
* Doc table row in `doc/developer/ospf-yang-northbound-notes.rst`
naming the ospfd owner and explicitly flagging the OSPFv3 gap.
Converted-leaves paragraph extended with `OSPFv2 mpls/ldp/igp-sync`.
* Two topotests in `ospf_topo1/test_ospf_topo1.py`:
`test_ospf_yang_mpls_ldp_igp_sync_config` round-trips the leaf
via `mgmt set-config`;
`test_ospf_mpls_ldp_sync_cli_routes_through_yang` confirms the
legacy `mpls ldp-sync` / `no mpls ldp-sync` CLI continues to
drive the YANG callback.
`show running-config` rendering is unchanged: the existing
config-write reads `ospf->ldp_sync_cmd.flags` which the YANG callback
already mutates.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Wires the per-instance RFC 6987 unconditional stub-router presence
container for ospfd. RFC 9129 models it under a `choice trigger`
between `stub-router` and `always`; because YANG `choice` and `case`
nodes don't appear in the data tree (RFC 7950 sec 7.9.2), the data
path skips straight to `/stub-router/always`. ospf6d has no
stub-router implementation; this slice is OSPFv2-only and the
mapping table flags the gap.
What the slice contains:
* `ospfd_ietf_ospf_stub_router_always_create` and `_destroy` in
`ospfd/ospf_nb_config.c`. Create walks every existing area to
set `OSPF_AREA_ADMIN_STUB_ROUTED`, calls
`ospf_router_lsa_update_area` for any area that wasn't already
advertising stub, and arms `ospf->stub_router_admin_set =
OSPF_STUB_ROUTER_ADMINISTRATIVE_SET` so later-created areas
inherit the property. Destroy unwinds with the same per-area
walk, preserving any startup-timer-driven stub state already in
flight (`area->t_stub_router`) so the unset doesn't trample on
the boot-time stub timer.
* Registration of `OSPFD_IETF_OSPF_XPATH "/stub-router/always"` in
`ospfd_ietf_ospf_info` with `cfg_opt_in = true`, placed after
`/mpls/ldp/igp-sync`.
* `DEFUN` -> `DEFPY_YANG` conversion of `max-metric router-lsa
administrative` / `no max-metric router-lsa administrative` in
`ospfd/ospf_vty.c`. Bodies enqueue `NB_OP_CREATE` / `NB_OP_DESTROY`
on the presence container; the callback owns the per-area flag
flip and the LSA reorigination.
* Doc table row in `doc/developer/ospf-yang-northbound-notes.rst`
naming the ospfd owner and flagging the OSPFv3 gap. Converted-
leaves paragraph extended with `OSPFv2 stub-router unconditional`.
* Topotest in `ospf_topo1/test_ospf_topo1.py`:
`test_ospf_max_metric_router_lsa_admin_cli_routes_through_yang`
exercises the legacy CLI which routes through the YANG callback.
A direct `mgmt set-config` round-trip test is intentionally
omitted: presence containers carry no value, but FRR's mgmtd CLI
`mgmt set-config WORD VALUE` grammar requires a VALUE token, so
there's no clean entry point. The CLI test covers the same
create/destroy callback path.
`show running-config` rendering is unchanged: the existing config
writer at `ospfd/ospf_vty.c` reads `ospf->stub_router_admin_set`
which the YANG callback already mutates.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
…nversion
Wires the per-interface RFC 6860 prefix-suppression boolean for ospfd.
RFC 9129 leaf `/areas/area/interfaces/interface/prefix-suppression`,
type boolean. ospf6d has no equivalent per-interface
prefix-suppression flag, so this slice is OSPFv2-only and the mapping
table flags the gap.
What the slice contains:
* `ospfd_ietf_ospf_areas_area_interfaces_interface_prefix_`
`suppression_modify` / `_destroy` in `ospfd/ospf_nb_config.c`.
Mutates `params->prefix_suppression`, toggles the
`OSPF_IF_PARAM_CONFIGURED` bit, and -- if the value actually
changed -- reoriginates the Router-LSA on every adjacency plus
the Network-LSA on any interface where this router is DR (helper
`ospfd_ietf_ospf_prefix_suppression_lsa_update`). Mirrors the
legacy DEFPY's per-interface fan-out exactly.
* Entry added to the per-interface destroy cascade in
`ospfd_ietf_ospf_areas_area_interfaces_interface_destroy` so a
subsequent re-create of the area/interface list entry starts from
the compile-time default.
* Registration of the per-interface xpath in
`ospfd_ietf_ospf_info` with `.cfg_opt_in = true`, placed
immediately after `interface/passive` to keep the source-order
convention.
* DEFPY -> DEFPY_YANG conversion of `[no] ip ospf prefix-suppression`
`[A.B.C.D]` in `ospfd/ospf_vty.c`. The whole-interface form
(no per-address override) routes through the YANG callback; per-
address overrides stay on the legacy direct-mutation path because
RFC 9129's per-interface list cannot express them, and not-yet-
attached interfaces fall back via the `ospf_per_iface_xpath`
helper which is already gated on
`OSPF_IF_PARAM_CONFIGURED(if_area)`.
* Doc table row in `doc/developer/ospf-yang-northbound-notes.rst`
naming the ospfd owner, calling out the OSPFv3 gap, and noting
the LSA reorigination side effect. Converted-leaves paragraph
extended with `OSPFv2 prefix-suppression`.
* Two topotests in `ospf_topo1/test_ospf_topo1.py`:
`test_ospf_yang_prefix_suppression_config` round-trips the leaf
via `mgmt set-config`;
`test_ospf_prefix_suppression_cli_routes_through_yang` confirms
the legacy `ip ospf prefix-suppression` / `no ...` CLI without a
per-address override drives the YANG callback.
`show running-config` rendering is unchanged: the existing
config-write reads `params->prefix_suppression` which the YANG
callback already mutates.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds the per-instance /auto-cost/enabled and /auto-cost/reference-bandwidth leaves on both daemons and routes the matching legacy `auto-cost reference-bandwidth` CLI through ietf-ospf YANG. - /auto-cost/enabled: FRR has no off-switch for auto-cost; the modify callback rejects `false` at NB_EV_VALIDATE. A deviation pins the default to `true` so the YANG `when "../enabled = 'true'"` clause on the bandwidth leaf is always satisfied and operators never need to set this leaf themselves. - /auto-cost/reference-bandwidth: modify writes ospf->ref_bandwidth on the resolved instance and walks that instance's VRF interfaces with ospf_if_recalculate_output_cost; destroy restores OSPF_DEFAULT_REF_BANDWIDTH. The ospf6d side mirrors via ospf6_interface_recalculate_cost across the resolved ospf6's area_list / if_list. CLI conversion uses ospf_per_instance_xpath / ospf6_per_instance_xpath so the multi-instance and multi-VRF xpath is built the same way established leaves use it; both helpers are now extern (promoted with prototypes in ospf_vty.h / ospf6_top.h). Tests: - test_ospf_yang_auto_cost_reference_bandwidth_config: mgmtd round-trip for both daemons. - test_ospf_yang_auto_cost_disable_rejected: confirms the NB_EV_VALIDATE rejection. Uses the existing _mgmt_commit_attempt helper so the rejected candidate is aborted and does not leak into subsequent commits. - test_ospf_auto_cost_cli_routes_through_yang: legacy CLI lands via YANG; verified through `show mgmt get-data`. Doc: - Mapping-table rows for /auto-cost/enabled and /auto-cost/reference-bandwidth. - "auto-cost" appended to the converted-leaves paragraph. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds the OSPFv2 /mpls/te-rid/ipv4-router-id leaf and routes the legacy `mpls-te router-address A.B.C.D` CLI through ietf-ospf YANG. ospf6d has no MPLS-TE module so the leaf is OSPFv2-only. FRR's MPLS-TE state is a process-wide global (`OspfMplsTE`). The modify callback rejects non-default VRF at NB_EV_VALIDATE -- matching the constraint the legacy `mpls-te on` CLI enforces -- and delegates to a new `ospf_mpls_te_apply_router_addr` helper extracted from the DEFUN body so the YANG path and the CLI shim share identical side effects (store the value, refresh Opaque Router-Address LSAs when MPLS-TE is enabled). Destroy clears the TLV header so the running config writer skips the line cleanly. The legacy CLI never exposed `no mpls-te router-address` -- the value was cleared by `no mpls-te` (which disables MPLS-TE entirely). Adding a new `no mpls-te router-address` form here conflicts with the optional-on branch of `no mpls-te [on]` in the CLI graph and breaks both commands; the targeted clear is therefore only available through `mgmt delete-config` and is exercised by the YANG round-trip test. The legacy `ospf_mpls_te_config_write_router` unconditionally emitted `mpls-te router-address 0.0.0.0` even when no address had ever been set; gating the line on `header.type != 0` cleans up that pre-existing quirk and makes the YANG destroy path observable in running-config. ospf_te.c is added to clippy_scan so DEFPY_YANG expansion lands. Tests: - test_ospf_yang_mpls_te_router_addr_config: mgmtd round-trip (set, verify in running-config, delete, verify gone) with `mpls-te on` enabled up front and torn down in a finally block. - test_ospf_mpls_te_router_addr_cli_routes_through_yang: legacy set form lands via YANG. Doc: - Mapping-table row for /mpls/te-rid/ipv4-router-id. - "mpls/te-rid" appended to the converted-leaves paragraph. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds /graceful-restart/enabled and /graceful-restart/restart-interval on both daemons and routes the legacy `graceful-restart [grace-period N]` / `no graceful-restart` CLI through ietf-ospf YANG. The two leaves are independent in the YANG model and each callback manages its own state -- `enabled` controls `ospf->gr_info.restart_ support` plus zebra-GR enable/disable + NVM bookkeeping; `restart-interval` controls `ospf->gr_info.grace_period` and refreshes the zebra stale-route timer when GR is already on. Three helpers per daemon (`ospf_gr_restart_support_enable`, `_disable`, `_set_grace_period`) are extracted from the legacy DEFPY body so the CLI shim and the YANG callbacks share a single code path. The `enabled` modify rejects disable at NB_EV_VALIDATE when a GR prepare is in flight, matching the legacy CLI's rejection. RFC default for restart-interval is 120s, which matches FRR's compile-time `OSPF_DFLT_GRACE_INTERVAL` / `OSPF6_DFLT_GRACE_INTERVAL`, so no deviation is required. CLI shim: the unified `graceful-restart [grace-period N]` enqueues both `/enabled=true` and (when N is given) `/restart-interval=N` in one apply. The `no` form destroys both leaves so the legacy "no GR + period reset" semantics is preserved. Both daemons promote `ospf_gr_nvm_update` from static to extern so the helpers can reuse it. Tests: - test_ospf_yang_graceful_restart_config: mgmtd round-trip on both daemons -- set with non-default interval, drop interval alone (running-config collapses to bare `graceful-restart`), then disable. - test_ospf_graceful_restart_cli_routes_through_yang: legacy `graceful-restart grace-period 180` / `no graceful-restart` on both daemons. Doc: - Mapping-table rows for both leaves. - "graceful-restart enabled and restart-interval" appended to the converted-leaves paragraph. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds /graceful-restart/helper-enabled and
/graceful-restart/helper-strict-lsa-checking on both daemons and
routes the matching legacy `graceful-restart helper enable` and
`graceful-restart helper {strict-lsa-checking,lsa-check-disable}`
CLI forms through ietf-ospf YANG.
The two leaves map onto FRR's existing helper helpers:
`ospf_gr_helper_support_set` / `ospf_gr_helper_lsa_check_set` (v2)
and `ospf6_gr_helper_support_set` / `ospf6_gr_helper_lsacheck_set`
(v3). The v3 helpers are promoted from static to extern so the YANG
callbacks can call them; the v2 helpers were already extern.
Two structural mismatches with RFC 9129 are documented in the notes
mapping rows and handled at the CLI shim:
- The legacy `graceful-restart helper enable [A.B.C.D]` per-router-id
form has no RFC counterpart (the RFC has no helper enable-list).
The DEFPY_YANG shim splits: bare form enqueues
`/helper-enabled=true`; per-router-id form stays on the legacy
direct mutation path.
- v3's strict-LSA-check CLI is inverted -- `graceful-restart helper
lsa-check-disable` disables strict checking, `no ...` restores it.
The YANG leaf `helper-strict-lsa-checking` is positive, so the v3
shim flips the value (`MODIFY false` / `MODIFY true`) before
enqueueing. v2's CLI is positive and maps cleanly.
Tests:
- test_ospf_yang_graceful_restart_helper_config: mgmtd round-trip on
both daemons -- enable helper + relax strict-check, drop strict
alone (line disappears since FRR's default is strict-on), then
disable helper.
- test_ospf_graceful_restart_helper_cli_routes_through_yang:
legacy CLI on both daemons, tolerating either v2's
`no ... strict-lsa-checking` or v3's `lsa-check-disable` line in
running-config.
Doc:
- Mapping-table rows for both leaves with the per-router-id and
inverted-v3-CLI caveats called out.
- "helper-enabled and helper-strict-lsa-checking" appended to the
converted-leaves paragraph.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
…fig out of scope These four RFC 9129 nodes have no FRR-side surface to map onto: - nsr: FRR has no OSPF Non-Stop Routing. - database-control/max-lsa: FRR's max-metric router-lsa on-shutdown / on-startup is structurally different from the RFC's accept-threshold. - spf-control/ietf-spf-delay: FRR's timers throttle spf implements a different back-off algorithm. - node-tag-config: neither daemon has any node-tag CLI, struct, or LSA encoding; router-info only enables the Opaque Router Information LSA. Documenting the absence lets future readers see that each was considered and ruled out, rather than rediscovering the gap through grep. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
ospf6_route_remove_all walked the route table with `route = ospf6_route_next(route)` AFTER calling ospf6_route_remove, which drops the table-owned lock and -- on the last lock -- frees the route. The loop then dereferenced freed memory. In practice the failure surfaces when ospf6_restart_spf calls this helper while a hook_remove callback (e.g. the zebra route-table update path) re-enters ospf6_route_remove on a sibling route in the same prefix's `node->info` chain. The chain mutation underneath the in-flight remove leaves the head route detached from `node->info`, and the next iteration trips `assert(current == route)` at ospf6_route_remove (line 928). Switch to a re-read-the-head loop and explicitly drop the iterator lock that ospf6_route_head takes on the snapshot. This is the same shape that other ospf6d walkers (e.g. ospf6_abr.c:118-127) use and matches the existing convention that hook_remove may mutate the table. Triggered locally by the per-interface BFD topotest sequence in ospf_topo1 -- adding state churn ahead of test_ospf_max_multipath_cli_routes_through_yang (which routes through ospf6_restart_spf) was enough to expose the use-after-free. The fix is independent of that test suite and benefits every caller of ospf6_restart_spf (distance / max-paths / future YANG callbacks). Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds /areas/area/interfaces/interface/bfd/* on both daemons -- the RFC 9129 ietf-bfd-types client-cfg-parms surface (enabled, local-multiplier, desired-min-tx-interval, required-min-rx-interval) -- and routes the matching legacy `ip ospf bfd` / `ipv6 ospf6 bfd` CLI through ietf-ospf YANG. ospfd maps onto `params->bfd_config` (heap-allocated on enable via `ospf_interface_enable_bfd`; freed by `ospf_interface_disable_bfd`). ospf6d maps onto `oi->bfd_config` (embedded struct). The shared session-lifecycle helpers are promoted from static to extern: `ospf_interface_enable_bfd`, `ospf_interface_bfd_apply` on ospfd, and `ospf6_bfd_reg_dereg_all_nbr` on ospf6d. Two FRR-specific extensions stay on the legacy direct path because they have no RFC 9129 counterpart: - ospfd `[quick]` (quick-establishment mode) - ospf6d `[profile BFDPROF]` (FRR BFD profile selection) The DEFUN_YANG shims split: bare form enqueues YANG; parametric form stays direct. Same `if-area`-bound fallback as the other per- interface conversions -- if the interface is not in an area, no YANG path exists, so direct mutation is used. The DEFUN macros stay as DEFUN_YANG (not DEFPY_YANG) because the parametric forms have `#if HAVE_BFDD > 0` switching between DEFUN_HIDDEN and DEFUN, which the clippy scanner can't handle inside macro argument lists. ospfd's `no ip ospf bfd` is kept as plain DEFUN for the same reason (the cmd-string has #if/#else inside it) but its body routes through YANG when the area binding exists. Deviations against RFC 9129 / ietf-bfd-types: - `desired-min-tx-interval` / `required-min-rx-interval` defaults pinned to 300000us (FRR's `BFD_DEF_MIN_TX/RX = 300ms`), via `deviate replace` because the RFC base sets a default of 1000000us. - `single-interval` case marked `not-supported` -- FRR's BFD library only exposes the (tx, rx) interval form. Unit handling: - RFC uses microseconds, FRR stores milliseconds. Reject non- multiples of 1000us at NB_EV_VALIDATE; clamp to FRR's CLI grammar range (50..60000 ms = 50000..60000000 us). Build: - ospfd/ospf_vty.c: `ospf_per_iface_xpath` promoted from static to extern (declared in ospf_vty.h). - ospf6d/ospf6_interface.c: `ospf6_per_iface_xpath` promoted from static to extern (declared in ospf6_interface.h). Tests: - test_ospf_yang_interface_bfd_config: mgmtd round-trip on both daemons (enable + multiplier + tx/rx, then delete via destroy). - test_ospf_yang_interface_bfd_interval_rejection: NB_EV_VALIDATE rejection for non-multiple-of-1000us and out-of-range values, using `_mgmt_commit_attempt` for proper candidate cleanup. - test_ospf_bfd_cli_routes_through_yang: legacy `ip ospf bfd` / `ipv6 ospf6 bfd` and the param forms via legacy CLI, with try/finally cleanup. Doc: - Mapping-table rows for each of the four leaves. - Converted-leaves paragraph extended with the BFD entry. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds /areas/area/interfaces/interface/static-neighbors/neighbor on OSPFv2 only (ospf6d has no NBMA neighbour surface) with the per-leaf priority and poll-interval modify/destroy callbacks. Structural mismatch documented and contained: RFC 9129 keys the list per-(area, interface, identifier), but FRR's `struct ospf_nbr_nbma` table is per-(instance, addr). The callback walks up to the OSPF instance from the dnode and uses `identifier` only; the area / interface labels are stored in the candidate but ignored on the FRR side. FRR auto-binds the entry to whichever OI's subnet matches the neighbour address via `ospf_nbr_nbma_set`/`_if_update`. Multiple YANG entries with the same identifier collapse onto the same FRR neighbour (idempotent set, last destroy wins -- no reference counting in this slice). Cost leaf marked not-supported via deviation: FRR's NBMA neighbour struct exposes only `priority` and `v_poll`, so there is no `cost` knob to map onto. mgmtd rejects writes against `cost` cleanly. Legacy `neighbor A.B.C.D` CLI stays on the direct mutation path: it is instance-level and cannot synthesise a credible YANG area / interface key from CLI alone. Tests: - test_ospf_yang_interface_static_neighbor_config: mgmtd round-trip (set poll-interval + priority, verify in running-config, delete, verify gone) with try/finally cleanup. - test_ospf_yang_static_neighbor_cost_rejected: confirms the not-supported deviation is enforced (uses `_mgmt_commit_attempt`). Doc: - Mapping-table row spelling out the per-(area, iface) vs per-(instance) mismatch and the cost not-supported note. - Converted-leaves paragraph extended with "OSPFv2 per-interface static neighbours (poll-interval, priority)". Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Adds /areas/area/interfaces/interface/authentication/ospfv2-key-chain (OSPFv2) and ospfv3-key-chain (OSPFv3). Covers only the key-chain case of RFC 9129's authentication choice -- the explicit-key triplet (key-id + key + crypto-algorithm) and OSPFv3 IPsec SA case are deferred to follow-on slices because they map onto different FRR-side surfaces with multi-leaf coordination. OSPFv2 callback writes `params->keychain_name`, sets `auth_type` to `OSPF_AUTH_CRYPTOGRAPHIC`, and clears any explicit `auth_crypt` list -- mirrors the legacy `ip ospf authentication key-chain X` CLI exactly (see ospf_vty.c:7754+). Destroy frees the keychain string, clears the field, and restores `auth_type = OSPF_AUTH_NOTSET`. OSPFv3 callback writes `oi->at_data.keychain` and sets the `OSPF6_AUTH_TRAILER_KEYCHAIN` flag. NB_EV_VALIDATE rejects the modify if `OSPF6_AUTH_TRAILER_MANUAL_KEY` is already set -- matches the legacy CLI's mutually-exclusive lock. Destroy frees the keychain string and clears the flag set. `MTYPE_OSPF6_AUTH_KEYCHAIN` promoted from DEFINE_MTYPE-only to DECLARE_MTYPE in ospf6_interface.h so the new callback can use XSTRDUP/XFREE on it. Deferred non-key-chain authentication leaves (explicit-key triplet, IPsec SA, auth-trailer-rfc): pyang / libyang refuse the deviation paths inside the nested choice/case nodes (`ietf-ospf::ospfv2-key` etc. not found), so this slice cannot mark them not-supported. mgmtd silently accepts writes against them with no callback fire. Documented in the developer notes for the slice; future work either implements those leaves as real callbacks or sorts the schema-path- resolution gap. Tests: - test_ospf_yang_interface_authentication_keychain_config: mgmtd round-trip on both daemons. Creates a real `key chain kc-test` before the YANG writes so libyang's leafref into /key-chain:key-chains/.../name resolves; tears it down in the finally clause. The legacy CLI is more relaxed about forward references but matching YANG's leafref semantics is the principled behaviour for this slice. Doc: - Mapping-table row covering both v2 and v3 key-chain leaves. - Converted-leaves paragraph extended with the key-chain entry. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
The BFD slice (commit ba4863b) added ietf-bfd-types to each of ospfd, ospf6d and mgmtd's `frr_yang_module_info` table as a separate inline definition. Three copies of the same data, but the precedent is one: ietf-keychain has lived in lib/keychain_nb.c since the lib/ housekeeping consolidation, exported via lib/keychain.h as `ietf_key_chain_info`, and every daemon that needs it (ospfd, ospf6d, ripd, ripngd, isisd, ...) references the single symbol. Mirror that. Define `ietf_bfd_types_info` once in lib/bfd.c with `*` features (so client-base-cfg-parms enables the multiplier and tx/rx-interval leaves) and `ignore_cfg_cbs = true`. Declare it in lib/bfd.h alongside the existing BFD helpers. Drop the three inline duplicates and have ospfd, ospf6d, mgmtd reference the shared symbol. No behaviour change. ospfd, ospf6d and mgmtd continue to load ietf-bfd-types with the same feature set and the same null callback table; the symbol is just sourced from one place instead of three. The shape now matches lib/keychain.h's `ietf_key_chain_info` exactly, which keeps the lib/ <-> backends boundary uniform for future foreign-module loaders (ietf-srv6-types is the next candidate if any backend needs to publish data inheriting from it). Signed-off-by: Eric Parsonage <eric@eparsonage.com>
After completing the per-interface config-write conversions (cost, hello/dead/retransmit, priority, mtu-ignore, transmit-delay, interface-type, passive, prefix-suppression, BFD, static-neighbors, authentication key-chain), the remaining ietf-ospf config nodes were audited against FRR's actual surface on both daemons. Every leaf left in scope is either: a) blocked at the schema layer (the auth `choice` / `case` branches that pyang / libyang refuse deviation paths for), or b) has no matching FRR struct field, CLI command, or runtime helper to mutate. Document this explicitly in the "Remaining Scope" section so the boundary between "intentionally deferred" and "implementation missing" is unambiguous. New entries cover ospf/enabled and interface/enabled (no separate on/off in FRR -- the protocol runs whenever an instance is configured and an interface is bound to an area), interface/multi-areas (no multi-area-adjacency surface), interface/ttl-security (no per-interface TTL-security check), ospf/fast-reroute/lfa and the per-interface lfa subtree (FRR's only LFA surface is the instance-level `fast-reroute ti-lfa` command on ospfd, which doesn't match RFC 9129's per-interface enable shape; ospf6d has no TI-LFA at all), ospf/address-family (fixed per daemon), and the auth nested-choice branches (deviation-path limitation already discussed in the ospfv2-key-chain row). Adding empty validation-only callbacks for these would only create silent-acceptance traps -- the rule established in earlier slices is that every committed leaf must map to a real FRR mutation. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Add the two RPCs RFC 9129 defines at the ietf-ospf module root, with
matching handlers on both ospfd and ospf6d. The RPCs are registered
at the module root rather than under control-plane-protocol, so both
daemons subscribe to the same xpaths and mgmtd fans each call out
to every backend; each handler looks up the named instance and
returns NB_OK silently when the instance isn't local.
Mapping:
* `clear-neighbor` on ospfd reuses `ospf_neighbor_reset` for the
whole-instance case and walks the matching OI's neighbour table
for the per-interface case (legacy `ospf_neighbor_reset` has no
per-interface filter, so the iteration inlines the NSM_KillNbr
loop the helper would otherwise run unconditionally).
* `clear-neighbor` on ospf6d reuses `ospf6_interface_clear`,
iterating every OSPFv3-bound interface when no `interface` input
is supplied or invoking it once when an interface name is given.
* `clear-database` on both daemons maps onto `ospf_process_reset` /
`ospf6_process_reset` -- the helpers the legacy
`clear ip ospf process` / `clear ipv6 ospf6 process` commands
already invoke, so the RFC's "force every neighbour DOWN and
reoriginate self-originated LSAs" semantics line up exactly.
Per-interface clear with an interface that doesn't belong to the
instance returns NB_ERR_RESOURCE with the RFC-mandated app-tag
`ospf-interface-not-found`. RPC against an unknown
`routing-protocol-name` returns NB_OK on every backend -- that is
the legitimate "no daemon owns this name" outcome, not an error.
The xpaths register via the `rpc_xpaths` array on each daemon's
`mgmt_be_client_cbs` rather than via `frr_yang_module_info.nodes`
alone; mgmtd's RPC dispatcher consults the BE-adapter subscription
map, not the schema callback registry, so the subscribe message
that ships at client connect must list the RPC xpaths explicitly.
Audit-driven changes vs the initial draft:
* `ospf-interface-not-found` now returns NB_ERR_NOT_FOUND instead
of NB_ERR_RESOURCE. mgmtd maps NB_ERR_NOT_FOUND to
MGMTD_INVALID_PARAM and NB_ERR_RESOURCE to MGMTD_INTERNAL_ERROR;
the latter incorrectly signals a daemon-side failure rather than
a bad client reference.
* Documented two deliberate deviations from strict RFC 9129 wording:
(a) silent NB_OK on a non-local routing-protocol-name is forced by
the mgmtd fan-out model -- a non-owner cannot distinguish "name
doesn't exist on me" from "name doesn't exist anywhere"; (b)
FRR's nb_cb_rpc_args has no first-class error-app-tag field, so
the RFC-prescribed app-tag strings ride in the unstructured errmsg
buffer rather than via NETCONF / RESTCONF <error-app-tag>.
* clang-format clean on both new files.
* Investigated the per-daemon gRPC frontend (lib/northbound_grpc.cpp)
as a second invocation path: it bypasses mgmtd and calls
`nb_callback_rpc` directly via the daemon's `-M grpc:<port>`
listener. Cannot dispatch these RPCs in standalone mode because
`HandleUnaryExecute` validates input via `lyd_validate_op` with
`LYD_TYPE_RPC_YANG` before invoking the handler, and the daemon
has no `/ietf-routing:routing/...` data populated locally to
satisfy the `routing-protocol-name` leafref. Every Execute call
with a valid name is rejected at the validator with
`INVALID_ARGUMENT` / "Invalid input data". Documented as a
frontend-side gap in the RPC Support section; not specific to
these RPCs (it affects every RFC 9129 RPC whose inputs include a
leafref into the routing tree). Use the mgmtd-fronted vtysh path
until either the gRPC frontend relaxes leafref validation for
RPC inputs or backend daemons mirror the routing-protocol list
locally.
Tests:
* `test_ospf_yang_clear_neighbor_rpc` -- whole-instance reset plus
per-interface reset on both v2 and v3, verifying neighbours
renegotiate back to Full.
* `test_ospf_yang_clear_database_rpc` -- same round-trip for the
process-reset path.
* `test_ospf_yang_rpc_unknown_instance_silent` -- RPC against
`does-not-exist` returns OK and leaves r1's v2 neighbour
undisturbed.
* `test_ospf_yang_clear_neighbor_rpc_unknown_interface` -- per-
interface reset against `lo` (not in any OSPF area) surfaces the
`ospf-interface-not-found` app-tag.
After the disruptive clear-database RPC the test calls
`_force_ospf_reconvergence_to_steady_state` to restore the LSDB
baseline the downstream read-only tests
(`test_ospf_json`, `test_ospf_kernel_route`) compare against.
The libyang RPC parser expects the input wrapped in the RPC name
(`{"ietf-ospf:clear-neighbor":{"routing-protocol-name":"default"}}`);
this is documented alongside the new mapping table in
`doc/developer/ospf-yang-northbound-notes.rst`.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Emit /ietf-ospf:nbr-state-change on every NSM transition by hooking
the existing per-daemon state-change hook (ospf_nsm_change on ospfd,
ospf6_neighbor_change on ospf6d) and dispatching through
nb_notification_send.
The notification carries the RFC 9129 instance header (routing-
protocol-name), the interface link container with the
ietf-interfaces interface name, the neighbour-router-id and
neighbour-ip-addr leaves, and the RFC nbr-state-type enum.
State translation:
* OSPFv2's NSM_* constants reserve 0/1 for DependUpon/Deleted, so
NSM_Down=2..NSM_Full=9 maps to RFC values 1..8 via a small lookup
table.
* OSPFv3's OSPF6_NEIGHBOR_* values match RFC 9129's nbr-state-type
numeric values 1:1, so no translation is needed.
Each daemon registers its hook subscriber from a new
ospf{,6}d_ietf_notif_init() called once from
ospf{,6}_master_init(). Handlers log every emit through
DEBUGD(&nb_dbg_notif, ...) so operators can verify wiring with
`debug northbound notifications`.
Tests:
* test_ospf_yang_nbr_state_change_notification -- enables northbound
notification debug on r1, triggers a neighbour reset via the
existing clear-neighbor RPC, and greps both ospfd and ospf6d logs
for the OSPF-NOTIF / "northbound notification: /ietf-ospf:nbr-
state-change" markers. Restores steady-state after so downstream
LSDB-snapshot tests stay deterministic.
Doc:
* New "Notification Support" section in
doc/developer/ospf-yang-northbound-notes.rst documenting the
hook wiring, the state translation, and which RFC 9129
notifications remain to be wired in subsequent slices.
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Emit /ietf-ospf:if-state-change on every ISM transition by hooking the existing per-daemon interface state-change hook (ospf_ism_change on ospfd, ospf6_interface_change on ospf6d) and dispatching through nb_notification_send. State translation tables (numeric values differ between FRR internals and RFC 9129's `if-state-type` enum): * OSPFv2: Down/Loopback/Waiting/PointToPoint agree, but FRR orders the DR-election trio DROther=5, Backup=6, DR=7 while the RFC uses dr=5, bdr=6, dr-other=7. * OSPFv3: same RFC-vs-FRR DR ordering mismatch; FRR also reserves value 5 (gap between PointToPoint=4 and DROther=6), so the full table is needed. Tests: * test_ospf_yang_if_state_change_notification -- enables northbound notification debug on r1, toggles r1-eth1 down/up to force ISM transitions on both ospfd and ospf6d, then greps both daemon logs for the OSPF-NOTIF / "northbound notification: /ietf-ospf:if-state- change" markers. Doc: * New row in the Notification Support table documenting the hook and the state-translation rationale. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Wire two RFC 9129 graceful-restart notifications on both daemons:
* /ietf-ospf:restart-status-change emitted from ospf{,6}_gr_restart_
enter (status=planned-restart) and ospf{,6}_gr_restart_exit
(status=not-restarting + exit-reason=completed).
* /ietf-ospf:nbr-restart-helper-status-change emitted from the GR
helper accept path (status=helping, age=grace interval,
exit-reason=in-progress) and from ospf{,6}_gr_helper_exit
(status=not-helping, age=0, exit-reason translated from FRR's
helper-exit-reason enum).
Mapping notes:
* All FRR-known restart reasons (SW_RESTART, SW_UPGRADE,
SWITCH_CONTROL_PROCESSOR, UNKNOWN_RESTART) are software-initiated,
so the emit calls hardcode RFC `planned-restart` (2).
`unplanned-restart` (3) would correspond to a crash recovery path
FRR doesn't currently signal; left for future work.
* FRR's `enum ospf_helper_exit_reason` (0..4) maps name-for-name to
RFC 9129's `restart-exit-reason-type` (1..5) via a small switch.
No new state-change hook is added; we call the notification builders
directly from the existing GR enter/exit sites because graceful
restart isn't otherwise observed by FRR hook subscribers (no SNMP-
hook precedent like ospf_nsm_change).
Doc:
* New rows in the developer-notes Notification Support table
describing both notifications + the emit-site choice.
* "Out of scope for now" paragraph updated to note that triggering
GR in topotest is non-trivial; live test coverage is deferred
with a manual reproduction path documented (clear ip ospf process
on a neighbour during its grace period).
Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Two packet-side notifications on both daemons: * /ietf-ospf:if-rx-bad-packet -- emitted from ospf_read_helper (v2) and ospf6_receive (v3) when post-header verification fails. The packet-source leaf carries the source IP; packet-type is included when the header parsed far enough to extract it. * /ietf-ospf:if-config-error -- emitted at hello-interval and dead- interval mismatch checks in ospf_hello (v2) and the corresponding v3 receive path. The error leaf is set to the RFC enum identifier string (`hello-interval-mismatch`, `dead-interval-mismatch`). Both notification builders take the OI plus the packet-source / type inputs and produce the full RFC 9129 envelope (instance header, interface header, packet-source, packet-type if known, error name for the config-error variant). Other reject paths the RFC `error` enum can carry (bad-version, area-mismatch, unknown-nbma-nbr, unknown-virtual-nbr, auth-type- mismatch, auth-failure, net-mask-mismatch, option-mismatch, mtu- mismatch, duplicate-router-id) wire as incremental future work -- the emit helper is generic enough that each new reject site is a one-line `ospfd_ietf_notif_if_config_error(...)` insertion. Doc: * Notification Support table grows two rows describing the wired sites and the future-incremental-wiring note. Signed-off-by: Eric Parsonage <eric@eparsonage.com>
Emit /ietf-ospf:nssa-translator-status-change when the OSPFv2 NSSATranslatorState for an area transitions. Wired at the existing ABR check site in ospf_abr_nssa_check_status where FRR already detects the transition (`if (old_state != area->NSSATranslatorState)`). State mapping: * RFC 9129 defines three states: enabled (1), elected (2), disabled (3). * FRR tracks only DISABLED (0) and ENABLED (1). ENABLED means this router has been elected as the NSSA translator for the area, so FRR ENABLED maps to RFC `elected`. FRR has no separate "enabled but not yet elected" state, so RFC `enabled` is unreachable from FRR's side -- it's a state RFC distinguishes that FRR consolidates. Notification body carries the instance header (routing-protocol- name) plus the area-id and the translated status enum. OSPFv3 has no NSSA translator surface in FRR, so this notification is OSPFv2-only -- consistent with the existing config-write scope documented in the Remaining Scope section. Doc: * New row in the Notification Support table. * "Out of scope for now" paragraph updated: this was the last RFC 9129 notification to wire; only the two lsdb-overflow events remain unimplemented (FRR has no max-LSA threshold). Signed-off-by: Eric Parsonage <eric@eparsonage.com>
lamestllama
force-pushed
the
ospfd-yang-rfc9129-v2
branch
from
d278944 to
c704313
Compare
Author
|
@greptile[bot] review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds RFC 9129
ietf-ospfYANG support for the implemented FRR-backedOSPFv2 and OSPFv3 subset through mgmtd.
The implementation is anchored on the IETF model. FRR-specific behaviour stays
on the existing CLI surface unless there is a concrete FRR-native augment with
callbacks.
This PR provides the OSPF YANG model integration, callback wiring, mgmtd
dispatch support needed for the shared OSPFv2/OSPFv3 subtree, RPC callbacks,
notifications, user documentation and developer notes.
Generic gRPC
ExecuteandSubscribesupport through mgmtd is handled in#22158. When #22158 is present, external gRPC clients can use the same mgmtd
frontend/backend machinery for OSPF RPCs and notifications. This PR does not
depend on #22158.
What This PR Provides
Operators get broad RFC 9129 YANG configuration, RPC and operational state
coverage for the implemented FRR-backed OSPF subset.
The supported mgmtd-facing surfaces are:
vtyshthroughmgmt set-config,mgmt rpcandmgmt commit applywith mgmtd
The OSPFv2 and OSPFv3 implementations share the RFC 9129
control-plane-protocolsubtree. Requests are routed to the correct daemon bymatching the predicated protocol entry, for example the OSPFv2 or OSPFv3
typeandnamekeys.Architecture
RFC 9129 puts OSPFv2 and OSPFv3 under the shared RFC 8349
control-plane-protocollist, distinguished bytypeandnamekeys. That isthe standard shape, but it means
ospfdandospf6dshare one schema subtreeand must receive only the requests that belong to their daemon and instance.
The series adds reusable infrastructure for that shape:
control-plane-protocol[type='ietf-ospf:ospfv2'].For multi-instance OSPFv2, backend client names are made instance-aware:
non-instanced daemons keep their existing names, while instanced daemons append
the decimal instance ID, such as
ospfd-1orospfd-2. The RFC 9129 protocolnamein instance mode is the decimal instance ID.Configuration Coverage
Per-instance leaves:
explicit-router-idpreference/{all,intra-area,inter-area,internal,external}internalmaps onto intra/inter-area distances.spf-control/pathsMULTIPATH_NUM.auto-cost/{enabled,reference-bandwidth}enabled=falseis rejected at validation.mpls/ldp/igp-syncospf6dhas no LDP/IGP-sync.mpls/te-rid/ipv4-router-idospf6dhas no MPLS-TE.graceful-restart/{enabled,restart-interval,helper-enabled,helper-strict-lsa-checking}stub-router/alwaysospf6dhas no stub-router.Per-area:
areas/arealifecyclearea/area-typenormal, stub and NSSAarea/summaryarea/default-costarea/ranges/range,advertiseandcostPer-interface:
interfaces/interfacearea attachmentinterface/costinterface/{hello,dead,retransmit}-intervalinterface/priorityinterface/mtu-ignoreinterface/transmit-delayinterface/interface-typeinterface/passiveinterface/prefix-suppressioninterface/bfd/{enabled,local-multiplier,desired-min-tx-interval,required-min-rx-interval}interface/static-neighbors/neighbor,poll-intervalandpriorityinterface/authentication/ospfv2-key-chain/ospfv3-key-chainRPCs
clear-neighborclear-databaseospf{,6}_process_reset.Notifications
nbr-state-changeospf_nsm_change/ospf6_neighbor_changehooksif-state-changeospf_ism_change/ospf6_interface_changehooksrestart-status-changenbr-restart-helper-status-changeif-rx-bad-packetif-config-errornssa-translator-status-changelsdb-{approaching-,}overflowOut of Scope
Documented in
doc/developer/ospf-yang-northbound-notes.rst.Deferred because there is no matching FRR surface, the FRR surface has a
different shape, or the RFC 9129 path needs a separate future mapping:
ospf/nsrdatabase-control/max-lsaspf-control/ietf-spf-delaynode-tag-configinterface/{enabled,multi-areas,ttl-security}ospf/enabledaddress-familyfast-reroute/lfacurrent deviation paths
Related Work
Two earlier OSPF YANG attempts shaped the choices here:
It was useful as a state-callback reference, but its paths do not map onto
the current RFC 9129 surface.
frr-ospfd.yang. It was useful as a coverage map, but most callbacks werestubs.
FRR also has an experimental YANG-module translator for mapping non-native
models onto native FRR models via deviation modules and XPath translation
tables. This PR does not use it because OSPF does not yet have a complete
callback-backed native OSPF YANG model to serve as the source of truth. RFC
9129 is implemented directly as the canonical northbound surface for the OSPF
behaviour it covers.
Documentation
doc/user/ospfd.rstanddoc/user/ospf6d.rst: supported leaves,daemon-instance naming and worked
mgmt set-config/mgmt commit applyexamples.
doc/developer/ospf-yang-northbound-notes.rst: RFC 9129 design choice,predicate-aware dispatch, startup batching, validation approach,
notification wiring and future FRR-native augment boundaries.
IETF Module Sources
ietf-ospf.yang,ietf-routing.yang,ietf-bfd-types.yang,iana-routing-types.yangandiana-bfd-types.yangare pulled from theirrespective RFCs unchanged, with the IETF Trust BSD licence text.
This follows the existing handling of
ietf-interfaces.yang,ietf-key-chain.yangandietf-routing-types.yangalready inyang/ietf/.Commit Organisation
The series is organised so that reusable infrastructure lands before the OSPF
callbacks that depend on it:
Each commit is intended to be reviewable in isolation.