zebra: tear down old L3VNI before adding new one on VNI value change

[enissim]

When a VRF's L3VNI value is changed via a single "vni " command (without first issuing "no vni "), the northbound system generates a single NB_CB_MODIFY callback for the l3vni-id leaf. The modify handler lib_vrf_zebra_l3vni_id_modify() calls zebra_vxlan_process_vrf_vni_cmd() with add=1 for the new VNI, but never tears down the old one.

This leaves the old L3VNI entry orphaned in zrouter.l3vni_table. No ZEBRA_L3VNI_DEL is sent to bgpd, so bgpd retains stale EVPN state for the old VNI. The VRF appears to have the correct VNI in running config, but the control plane is broken until FRR is fully restarted.

This path is triggered when config management systems (e.g. NVUE) apply the full frr.conf to a running daemon, causing the NB diff to see a leaf value replace ('r') rather than the explicit destroy+create that frr-reload.py would generate.

Fix by checking zvrf->l3vni before the add. If the VRF already has a different L3VNI, call zebra_vxlan_process_vrf_vni_cmd() with add=0 for the old VNI first, which removes it from the global hash table and notifies bgpd via ZEBRA_L3VNI_DEL.

[greptile-apps]

Greptile Summary

This PR fixes an orphaned L3VNI bug triggered when a VRF's L3VNI is changed in-place via a northbound NB_CB_MODIFY callback. In lib_vrf_zebra_l3vni_id_modify, before adding the new VNI the handler now checks zvrf->l3vni and, if a different value is already set, calls zebra_vxlan_process_vrf_vni_cmd() with add=0 to properly tear it down — removing it from the global hash table and sending ZEBRA_L3VNI_DEL to bgpd — before proceeding with the add.

Confidence Score: 5/5

Safe to merge — the fix correctly tears down the old L3VNI before adding the new one, matching the same pattern used by the destroy handler.

The change is minimal and targeted. Logic is sound: guard condition avoids spurious teardown when VNI hasn't changed, old VNI is removed from the global hash and bgpd is notified before the new one is added, and zvrf->l3vni is zeroed by the delete path before the add sets it to the new value. The only remaining note (assert reachability) is a pre-existing issue in the called function, not introduced by this PR.

No files require special attention; the assert(zl3vni) concern in zebra_vxlan.c is pre-existing.

Important Files Changed

Filename	Overview
zebra/zebra_nb_config.c	Adds teardown of the old L3VNI in the modify handler before adding the new one; logic is correct, but the delete path contains an assert(zl3vni) that will crash zebra if zvrf->l3vni is set but the corresponding hash entry is missing.

Sequence Diagram

sequenceDiagram
    participant NB as Northbound NB_CB_MODIFY
    participant MH as lib_vrf_zebra_l3vni_id_modify
    participant VX as zebra_vxlan_process_vrf_vni_cmd
    participant BG as bgpd

    NB->>MH: NB_EV_APPLY new_vni
    alt zvrf->l3vni set AND differs from new_vni
        MH->>VX: old_vni add=0
        VX->>VX: oper_down + delete rmacs/nexthops
        VX->>VX: zvrf->l3vni=0 zl3vni_del
        VX-->>BG: ZEBRA_L3VNI_DEL old_vni
    end
    MH->>VX: new_vni add=1
    VX->>VX: zl3vni_add + map interfaces
    VX->>VX: zvrf->l3vni=new_vni
    VX-->>BG: ZEBRA_L3VNI_ADD new_vni

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: zebra/zebra_nb_config.c
Line: 3969

Comment:
**Defensive guard before assert(zl3vni) in delete path**

`zebra_vxlan_process_vrf_vni_cmd` with `add=0` calls `assert(zl3vni)` immediately after `zl3vni_lookup(vni)`. If state is ever inconsistent — `zvrf->l3vni` is non-zero but the corresponding entry was never inserted into the global hash (e.g., a previous partial add), this assert will crash zebra. The existing destroy handler is exposed to the same risk, but this new call site makes the path more reachable. A NULL-check + `zlog_warn` before the teardown call would be more resilient than a hard assert in a modify path.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "zebra: tear down old L3VNI before adding..." | Re-trigger Greptile}

[enissim]

ci:rerun

[ton31337]

@Mergifyio backport stable/10.6 stable/10.5 stable/10.4

[mergify]

backport stable/10.6 stable/10.5 stable/10.4

✅ Backports have been created

Details

• #21856 zebra: tear down old L3VNI before adding new one on VNI value change (backport #21757) has been created for branch stable/10.6
• #21857 zebra: tear down old L3VNI before adding new one on VNI value change (backport #21757) has been created for branch stable/10.5
• #21858 zebra: tear down old L3VNI before adding new one on VNI value change (backport #21757) has been created for branch stable/10.4

[enissim]

Hi everyone,

Could you please take a look at my GitHub PR when you have a moment and share your review/comments?

I’d really appreciate your feedback.

Thanks!

[enissim]

Hi,
I'd really appreciate it if someone could take a look and review...
thanks

[riw777]

looks good