zebra: Allow connected routes NHG sent to dplane

[GaladrielZhao]

When "fpm use-next-hop-groups" is enabled, connected routes' NHGs are not being sent to dplane.
This causes routes that resolve their nexthop through a connected route to fail insertion in the FPM.

Fix this by restricting the NEXTHOP_GROUP_INITIAL_DELAY_INSTALL flag to KERNEL and LOCAL routes only,
allowing connected routes' NHG to be properly downloaded to the dplane.

[greptile-apps]

Greptile Summary

This PR fixes a bug where connected routes' nexthop groups (NHGs) were not being sent to the dataplane when fpm use-next-hop-groups is enabled, causing FPM insertion failures for routes that resolve their nexthop via a connected route.

The fix is achieved through two consistent, minimal changes:

• zebra/zebra_rib.c (rib_add_multipath): The NEXTHOP_GROUP_INITIAL_DELAY_INSTALL flag is now only set for ZEBRA_ROUTE_KERNEL and ZEBRA_ROUTE_LOCAL routes. Previously it was set for all RIB_SYSTEM_ROUTE types (which also included ZEBRA_ROUTE_CONNECT), causing the dplane subsystem to skip actual NHG programming for connected routes.
• zebra/zebra_nhg.c (zebra_nhg_install_kernel): ZEBRA_ROUTE_CONNECT is removed from the list of types that preserve the NEXTHOP_GROUP_INITIAL_DELAY_INSTALL flag, ensuring that if a connected-type NHG somehow carries the flag it will be cleared and a real installation triggered.

The two changes are logically paired and correct. The only minor issue is a stale comment in zebra_interface_nhg_reinstall that still mentions "kernel/connected routes" after the semantics have changed.

Confidence Score: 4/5

• Safe to merge; the logic fix is minimal, well-scoped, and consistent across both modified sites.
• The two changes are tightly coupled and mutually consistent. No new code paths are introduced; existing dplane, uninstall, and interface-event paths all behave correctly for the newly-enabled connected-route NHG flow. The only finding is a stale comment that does not affect correctness.
• The comment at zebra/zebra_nhg.c lines 4085–4088 references "kernel/connected routes" and should be updated to "kernel/LOCAL routes".

Important Files Changed

Filename	Overview
zebra/zebra_nhg.c	Removes ZEBRA_ROUTE_CONNECT from the exemption list in zebra_nhg_install_kernel, so connected routes no longer bypass the NEXTHOP_GROUP_INITIAL_DELAY_INSTALL guard. A comment in zebra_interface_nhg_reinstall still refers to "kernel/connected routes" and needs updating.
zebra/zebra_rib.c	Narrows the NEXTHOP_GROUP_INITIAL_DELAY_INSTALL flag assignment in rib_add_multipath from all RIB_SYSTEM_ROUTE types (KERNEL + CONNECT + LOCAL) to only RIB_KERNEL_ROUTE and LOCAL, allowing connected-route NHGs to be properly forwarded to the dataplane.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[rib_add_multipath called] --> B{Route type?}
    B -- "ZEBRA_ROUTE_KERNEL\nor ZEBRA_ROUTE_LOCAL" --> C[SET NEXTHOP_GROUP_INITIAL_DELAY_INSTALL]
    B -- "ZEBRA_ROUTE_CONNECT\n(after fix)" --> D[Flag NOT set]
    B -- "Other routes\ne.g. BGP/OSPF" --> D

    C --> E[zebra_nhg_install_kernel\ntype = KERNEL or LOCAL]
    D --> F[zebra_nhg_install_kernel\ntype = CONNECT or other]

    E --> G{type == LOCAL\nor KERNEL?}
    G -- "Yes" --> H[Flag preserved\nDelay optimization kept]
    G -- "No\ne.g. ZEBRA_ROUTE_MAX" --> I[Flag cleared\nNHG marked uninstalled]

    F --> J{type != LOCAL\nand != KERNEL?}
    J -- "Yes (CONNECT,\nROUTE_MAX, etc.)" --> K{Flag set?}
    K -- "No (normal)" --> L[Proceed to install check]
    K -- "Yes (edge case)" --> I
    J -- "No" --> H

    H --> M[dplane_nexthop_add\nFlag set → fake SUCCESS\nNHG marked INSTALLED\nNo dplane op sent]
    L --> N{NHG VALID\nand not INSTALLED?}
    I --> N
    N -- "Yes" --> O[dplane_nexthop_add\nReal op enqueued\nNHG marked QUEUED]
    N -- "No" --> P[Skip install]

    O --> Q[FPM/Kernel receives NHG]
    M --> R[NHG skipped in FPM]

Loading

Comments Outside Diff (1)

1. zebra/zebra_nhg.c, line 4085-4088 (link)

   Stale comment after connected route change

   The comment at this location still references "kernel/connected routes" but, after this PR, connected routes (ZEBRA_ROUTE_CONNECT) no longer have NEXTHOP_GROUP_INITIAL_DELAY_INSTALL set — only kernel (ZEBRA_ROUTE_KERNEL) and LOCAL (ZEBRA_ROUTE_LOCAL) routes retain this optimization. The comment should be updated to reflect that.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: zebra/zebra_nhg.c
Line: 4085-4088

Comment:
**Stale comment after connected route change**

The comment at this location still references "kernel/connected routes" but, after this PR, connected routes (`ZEBRA_ROUTE_CONNECT`) no longer have `NEXTHOP_GROUP_INITIAL_DELAY_INSTALL` set — only kernel (`ZEBRA_ROUTE_KERNEL`) and LOCAL (`ZEBRA_ROUTE_LOCAL`) routes retain this optimization. The comment should be updated to reflect that.

```suggestion
		/*
		 * If this nhe has 'initial delay' flag set, we should not install this
		 * in kernel in case of any interface events. Zebra created this entry
		 * while processing the kernel/LOCAL routes, just to pretend
		 * the successful kernel install of this NHG
		 */
```

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

_{Last reviewed commit: e978a11}

[donaldsharp]

This is a straight up NAK from me. This is not an appropriate approach to the problem. It completely recreates the problem that was being fixed here. I spoke w/ Eddie about a completely different approach and this was not taken

[donaldsharp]

At the very least a topotest that shows that we do not install the nhg to the kernel and at the same time it is forwarded to the fpm is needed here.

[GaladrielZhao]

Thanks @donaldsharp , really appreciate your feedback.
I've revised the logic to send RIB system route NHGs to dplane using dplane_ctx_set_skip_kernel(), based on the NEXTHOP_GROUP_INITIAL_DELAY_INSTALL flag, in order to avoid kernel programming.
And I've also added the corresponding test cases in topotest "test_fpm_topo1" to validate the behavior.

[donaldsharp]

I'm on vacation this week and will be able to look again next week

[donaldsharp]

I think this should still be there. Why are you removing it?

[GaladrielZhao]

Thanks, added back.

[donaldsharp]

let's use RSYSTEM_ROUTE

[GaladrielZhao]

Fixed.

[donaldsharp]

My alternate proposal is #21893