Skip to content

fix vrf active value in YANG oper-state#20633

Merged
Jafaral merged 3 commits intoFRRouting:masterfrom
LabNConsulting:chopps/fix-vrf-yang-active-status
Jan 30, 2026
Merged

fix vrf active value in YANG oper-state#20633
Jafaral merged 3 commits intoFRRouting:masterfrom
LabNConsulting:chopps/fix-vrf-yang-active-status

Conversation

@choppsv1
Copy link
Contributor

  • lib: Check vrf active state as a flag not with == VRF_ACTIVE
  • tests: Update oper-state test to explicitly configure the vrf in FRR so that the VRF_CONFIGURED flag gets set and exposes the bug and verifies the fix.
  • lib: Drop vrf's YANG state cache, and just use new printf variants nb_notif_addf() nb_notif_deletef().
  • tests: Extend notify topotest to verify these changes.

Update one test to explicitly configure the vrf in FRR so that the
VRF_CONFIGURED flag gets set and exposes the bug and verifies the fix.

Signed-off-by: Christian Hopps <chopps@labn.net>
- use new path "printf" variants for nb_notif_add() nb_notif_delete() functions.

Signed-off-by: Christian Hopps <chopps@labn.net>
- also modify the mgmt_testc client. We need to improve this further
to load whatever modules are needed by the user provided paths to listen for.

Signed-off-by: Christian Hopps <chopps@labn.net>
@frrbot frrbot bot added libfrr tests Topotests, make check, etc labels Jan 30, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR fixes a critical bug where VRF active state was incorrectly reported in YANG operational state when both VRF_ACTIVE and VRF_CONFIGURED flags were set. The bug stemmed from using equality comparison (status == VRF_ACTIVE) instead of proper flag checking (CHECK_FLAG(status, VRF_ACTIVE)), causing active to be reported as false when the VRF had configuration.

Key changes:

  • Fixed lib_vrf_state_active_get_elem() in lib/vrf.c:1046 to use CHECK_FLAG instead of equality comparison
  • Removed YANG state cache (vrf->state field) and migrated to notification-based architecture using new nb_notif_addf() and nb_notif_deletef() functions
  • Added printf-style notification helpers that simplify path formatting
  • Extended test coverage to explicitly configure VRFs in FRR, which sets the VRF_CONFIGURED flag and exposes the bug
  • Test now validates that active state is correctly reported as true when both flags are set

The refactoring improves the architecture by eliminating the cached YANG state tree and using direct notifications, making the code simpler and more maintainable.

Confidence Score: 5/5

  • This PR is safe to merge - it fixes a critical bug with comprehensive test coverage and improves architecture
  • The fix is straightforward and correct, addressing a clear logical bug in flag checking. The refactoring removes unnecessary state caching without changing semantics. All changes have corresponding test coverage that validates both the fix and the new notification mechanism. The implementation follows existing patterns in the codebase.
  • No files require special attention - all changes are well-structured and properly tested

Important Files Changed

Filename Overview
lib/vrf.c Fixed critical bug in active state checking (== to CHECK_FLAG), removed YANG state cache, migrated to notification-based updates
lib/northbound_notif.c Implemented nb_notif_addf and nb_notif_deletef using varargs and dynamic string formatting
tests/topotests/mgmt_notif/test_ds_notify.py Added VRF notification test with active state validation, extended timeout, added helper function for JSON matching

Sequence Diagram

sequenceDiagram
    participant User
    participant VtyShell
    participant VRF Module
    participant Northbound
    participant YANG State
    participant Management Daemon

    Note over User,Management Daemon: VRF Creation Flow

    User->>VtyShell: Configure VRF (vrf red)
    VtyShell->>VRF Module: vrf_get(name="red")
    VRF Module->>VRF Module: Create/lookup VRF struct
    VRF Module->>VRF Module: SET_FLAG(status, VRF_CONFIGURED)
    VRF Module->>Northbound: nb_notif_addf("/frr-vrf:lib/vrf[name='red']")
    Northbound->>Northbound: darr_vsprintf(path, args)
    Northbound->>Northbound: nb_notif_add(formatted_path)
    Northbound->>Management Daemon: Queue notification

    Note over User,Management Daemon: VRF Activation Flow (kernel link up)

    User->>VRF Module: ip link add red (kernel event)
    VRF Module->>VRF Module: vrf_enable()
    VRF Module->>VRF Module: SET_FLAG(status, VRF_ACTIVE)
    VRF Module->>VRF Module: vrf_update_state()
    VRF Module->>Northbound: nb_notif_addf("/frr-vrf:lib/vrf[name='red']/state")
    Northbound->>Management Daemon: REPLACE notification

    Note over YANG State: State Query (Fixed Bug Here)
    
    Management Daemon->>VRF Module: lib_vrf_state_active_get_elem()
    VRF Module->>VRF Module: CHECK_FLAG(status, VRF_ACTIVE)
    Note right of VRF Module: OLD: status == VRF_ACTIVE (BUG!)<br/>NEW: CHECK_FLAG(status, VRF_ACTIVE)<br/>Correctly handles multiple flags
    VRF Module->>Management Daemon: return yang_data_new_bool(active)

    Note over User,Management Daemon: VRF Deletion Flow

    User->>VtyShell: Delete VRF (no vrf red)
    VtyShell->>VRF Module: vrf_delete()
    VRF Module->>VRF Module: UNSET_FLAG(status, VRF_CONFIGURED)
    VRF Module->>Northbound: nb_notif_deletef("/frr-vrf:lib/vrf[name='red']")
    Northbound->>Management Daemon: DELETE notification
Loading

@choppsv1
Copy link
Contributor Author

@Mergifyio backport stable/10.5

@mergify
Copy link

mergify bot commented Jan 30, 2026

backport stable/10.5

✅ Backports have been created

Details

@Jafaral Jafaral merged commit 7fb207a into FRRouting:master Jan 30, 2026
25 checks passed
Jafaral added a commit that referenced this pull request Jan 30, 2026
fix vrf active value in YANG oper-state (backport #20633)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants