Skip to content

zebra: Move allow-external-route-update to mgmt frontend side#21276

Merged
choppsv1 merged 1 commit intoFRRouting:masterfrom
donaldsharp:mgmt_external_route
Mar 20, 2026
Merged

zebra: Move allow-external-route-update to mgmt frontend side#21276
choppsv1 merged 1 commit intoFRRouting:masterfrom
donaldsharp:mgmt_external_route

Conversation

@donaldsharp
Copy link
Member

The allow-external-route-update command was being compiled into the zebra side of the nb code. Thus when configuration was being applied that uses mgmtd as a frontend and zebra as the frontend one would get there first and lock the database, preventing the other side from working. Move this command to the correct spot.

The `allow-external-route-update` command was being compiled into
the zebra side of the nb code.  Thus when configuration was being
applied that uses mgmtd as a frontend and zebra as the frontend
one would get there first and lock the database, preventing
the other side from working.  Move this command to the correct
spot.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
@donaldsharp
Copy link
Member Author

I'm moving the first commit from #21252 over to it's own commit. That particular commit fixes the mgmt_config frequent test failure we are seeing. The other PR has a bunch of new code that I would like to have more time to figure out how to get right based upon @choppsv1 and greptile's reviews.

@greptile-apps
Copy link

greptile-apps bot commented Mar 20, 2026

Greptile Summary

This PR relocates the allow-external-route-update CLI command from the zebra VTY layer (zebra_vty.c / zebra_vty_init()) to the mgmtd CLI frontend layer (zebra_cli.c / zebra_cli_init()), resolving a database lock contention when both mgmtd and zebra tried to apply the same YANG path.

  • Root cause fixed: zebra_vty_init() is invoked by zebra/main.c (zebra daemon), while zebra_cli_init() is invoked by mgmtd/mgmt_vty.c (management daemon). Having the command installed in the zebra path caused it to race with the mgmtd path and lock the YANG datastore.
  • Command consolidation: The two separate handlers — allow_external_route_update_cmd and no_allow_external_route_update_cmd — are replaced with a single idiomatic [no] allow-external-route-update DEFPY_YANG, which is the standard pattern used throughout zebra_cli.c.
  • No functional changes: The NB callbacks (zebra_allow_external_route_update_create / zebra_allow_external_route_update_destroy) and the cli_show write callback remain unchanged.
  • No dangling references: All references to the old no_allow_external_route_update_cmd symbol are gone; the test suite (test_zebra_rib.py) tests only the config string, so it remains valid.

Confidence Score: 5/5

  • This PR is safe to merge — it is a targeted relocation of a CLI command with no logic changes.
  • The change is minimal and surgical: one DEFPY_YANG definition moved between files, two commands collapsed into one using the standard [no] pattern, and the install site updated to match. The NB callbacks, YANG model, and test coverage are all untouched. No regressions are expected.
  • No files require special attention.

Important Files Changed

Filename Overview
zebra/zebra_cli.c Adds a combined [no] allow-external-route-update DEFPY_YANG command and registers it in zebra_cli_init() — the mgmtd-side initializer. Correctly consolidates two separate positive/negative command handlers into one idiomatic [no] variant.
zebra/zebra_vty.c Removes the two separate allow_external_route_update / no_allow_external_route_update DEFPY_YANG definitions and their install_element calls from zebra_vty_init(). Clean removal with no dangling references.

Sequence Diagram

sequenceDiagram
    participant Operator
    participant mgmtd
    participant zebra

    Note over mgmtd,zebra: Before fix — command registered in BOTH paths
    Operator->>mgmtd: allow-external-route-update
    mgmtd->>mgmtd: zebra_cli_init() → locks YANG DB
    Operator->>zebra: allow-external-route-update
    zebra->>zebra: zebra_vty_init() → tries to lock YANG DB ❌ deadlock

    Note over mgmtd,zebra: After fix — command registered only in mgmtd path
    Operator->>mgmtd: allow-external-route-update
    mgmtd->>mgmtd: zebra_cli_init() → locks YANG DB ✅
    mgmtd->>zebra: NB callback: zebra_allow_external_route_update_create()
    zebra->>zebra: zrouter.allow_delete = true
Loading

Last reviewed commit: "zebra: Move `allow-e..."

@choppsv1 choppsv1 merged commit eace856 into FRRouting:master Mar 20, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants