Zebra fixup nhg handling from kernel by donaldsharp · Pull Request #20732 · FRRouting/frr

donaldsharp · 2026-02-09T15:07:10Z

Zebra was not properly handling received nhg's from the kernel. Make it right

The decoding of the netlink message into a dplane ctx is storing the nhg id not the nhe_id. Let's actually retrieve the right value. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

FRR is currently receiving routes from the kernel that have nexthop groups but is ignoring those nexthop groups and is creating new ones that do not actually match what is in the kernel. Modify the code such that we track the kernel `found` nexthop groups and we allow routes received from the kernel that use those nhg's actually have the right values. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Create a test that shows when receiving nhg's from the kernel and routes using those nhg's that they are handled correctly in zebra. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

greptile-apps · 2026-02-09T15:14:56Z

Greptile Overview

Greptile Summary

This PR improves zebra’s handling of nexthop-group (NHG) IDs received from the kernel by (1) reading the NHG ID from the dplane context when decoding netlink route updates, and (2) adding a new NHG flag (NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL) that is set when routes/NHGs originate from kernel-provided IDs. A new topotest (zebra_kernel_nhg) installs kernel nexthops/NHGs and verifies that show ip route nexthop-group summary json reports the expected receivedNexthopGroupId values.

Main concern is that nexthop_active_update() now treats the new “received from external” flag the same as PROTO_OWNED(), routing kernel-originated NHGs through the protocol-owned active-update path, which may bypass zebra’s normal ACTIVE nexthop refresh logic. The new topotest also asserts kernel NHG deduplication behavior for two identical traditional routes, which may be kernel-dependent and can cause spurious failures.

Confidence Score: 3/5

This PR is directionally correct but has a likely logic error in NHG active-update handling and a potentially kernel-dependent test assertion.
Core changes are small and targeted (correct NHG ID accessor; new flag and test). However, treating kernel-received NHGs as proto-owned in nexthop_active_update() can lead to incorrect ACTIVE nexthop refresh behavior, and the new test’s equality assertion on kernel-assigned NHG IDs may not be stable across environments.
zebra/zebra_nhg.c; tests/topotests/zebra_kernel_nhg/test_zebra_kernel_nhg.py

Important Files Changed

Filename	Overview
tests/topotests/zebra_kernel_nhg/r1/frr.conf	Adds minimal router config for new topotest; no functional concerns.
tests/topotests/zebra_kernel_nhg/test_zebra_kernel_nhg.py	Adds new topotest installing kernel NHGs/routes and checking `receivedNexthopGroupId`; includes an assertion that two traditional routes must share the same NHG ID which may be kernel-dependent/flaky.
zebra/rt_netlink.c	Fixes route-change parsing to read NHG ID via `dplane_ctx_get_nhg_id()` instead of `get_nhe_id()`; change is narrow but should be validated against dplane ctx semantics.
zebra/zebra_nhg.c	Marks new NHGs as received-from-external and changes nexthop active update to treat that flag like `PROTO_OWNED()`, routing updates through proto-owned logic; likely incorrect for kernel-originated NHGs (see comment at nexthop_active_update).
zebra/zebra_nhg.h	Adds `NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL` flag bit (1<<10) to NHG flags; bit position appears free.
zebra/zebra_rib.c	When adding a route by `re->nhe_id`, sets `NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL` on the temporary NHE before copying; looks consistent with intent to preserve kernel-received NHG IDs.

Sequence Diagram

sequenceDiagram
    participant Kernel
    participant Netlink as zebra/rt_netlink.c
    participant DPlane as zebra_dplane_ctx
    participant RIB as zebra_rib.c
    participant NHG as zebra_nhg.c
    participant VTY as zebra_vty.c

    Kernel->>Netlink: "RTM_NEWROUTE (nhid / nexthops)"
    Netlink->>DPlane: "Populate ctx (zd_nhg_id, nhe.id, ng)"
    Netlink->>Netlink: "nhe_id = dplane_ctx_get_nhg_id(ctx)"
    Netlink->>RIB: "rib_add_multipath(..., re->nhe_id = nhe_id)"

    alt "Route uses explicit NHG id"
        RIB->>RIB: "Mark NHE RECEIVED_FROM_EXTERNAL"
        RIB->>NHG: "zebra_nhe_copy(&nhe, 0)"
    else "Route uses traditional via"
        RIB->>NHG: "zebra_nhe_copy(&nhe, 0)"
    end

    Kernel->>Netlink: "RTM_NEWNEXTHOP (NHG object)"
    Netlink->>NHG: "nhg_ctx_process_new(ctx)"
    NHG->>NHG: "SET_FLAG(RECEIVED_FROM_EXTERNAL)"

    alt "PROTO_OWNED or RECEIVED_FROM_EXTERNAL"
        NHG->>NHG: "proto_nhg_nexthop_active_update()"
    else "zebra-owned"
        NHG->>NHG: "refresh ACTIVE nexthops"
    end

    VTY->>NHG: "Query route/NHG summary"
    NHG-->>VTY: "Include receivedNexthopGroupId"

greptile-apps

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-09T15:15:00Z

zebra/zebra_nhg.c

+	if (PROTO_OWNED(re->nhe) ||
+	    CHECK_FLAG(re->nhe->flags, NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL))
 		return proto_nhg_nexthop_active_update(&re->nhe->nhg);


Kernel NHGs treated as proto-owned

nexthop_active_update() routes any NHG with NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL through proto_nhg_nexthop_active_update(). That path is meant for protocol-owned NHGs (checked via PROTO_OWNED()), and it skips the normal zebra active-resolution flow that copies the NHE and runs nexthop_list_active_update()/reinstall logic. For kernel-originated NHGs, this can leave route entries with stale ACTIVE nexthop state when interfaces/recursion change. Consider handling RECEIVED_FROM_EXTERNAL separately (i.e., don’t reuse the proto-owned update path).

Prompt To Fix With AI

This is a comment left during a code review. Path: zebra/zebra_nhg.c Line: 3194:3196 Comment: **Kernel NHGs treated as proto-owned** `nexthop_active_update()` routes any NHG with `NEXTHOP_GROUP_RECEIVED_FROM_EXTERNAL` through `proto_nhg_nexthop_active_update()`. That path is meant for protocol-owned NHGs (checked via `PROTO_OWNED()`), and it skips the normal zebra active-resolution flow that copies the NHE and runs `nexthop_list_active_update()`/reinstall logic. For kernel-originated NHGs, this can leave route entries with stale ACTIVE nexthop state when interfaces/recursion change. Consider handling `RECEIVED_FROM_EXTERNAL` separately (i.e., don’t reuse the proto-owned update path). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-09T15:15:01Z

tests/topotests/zebra_kernel_nhg/test_zebra_kernel_nhg.py

+                logger.info("Traditional prefix %s missing in summary output", prefix)
+                return False
+
+        nhg_40 = route_json["10.40.0.0/24"][0].get("receivedNexthopGroupId")
+        nhg_60 = route_json["10.60.0.0/24"][0].get("receivedNexthopGroupId")
+        if nhg_40 != nhg_60:


Assumes stable NHG deduping

This test asserts that two “traditional” routes (10.40/24 and 10.60/24) that share the same gateway must have the same receivedNexthopGroupId. That’s not guaranteed by zebra itself (the ID is kernel-reported) and can vary by kernel version/config depending on whether/when it deduplicates/merges NHGs for identical nexthops. This can make the test fail even when zebra is correctly reporting kernel NHG IDs.

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/topotests/zebra_kernel_nhg/test_zebra_kernel_nhg.py Line: 122:127 Comment: **Assumes stable NHG deduping** This test asserts that two “traditional” routes (10.40/24 and 10.60/24) that share the same gateway must have the same `receivedNexthopGroupId`. That’s not guaranteed by zebra itself (the ID is kernel-reported) and can vary by kernel version/config depending on whether/when it deduplicates/merges NHGs for identical nexthops. This can make the test fail even when zebra is correctly reporting kernel NHG IDs. How can I resolve this? If you propose a fix, please make it concise.

donaldsharp · 2026-02-09T15:27:31Z

@Mergifyio backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2 stable/10.1 stable/10.0

mergify · 2026-02-09T15:27:35Z

backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2 stable/10.1 stable/10.0

✅ Backports have been created

Details

#20780 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch dev/10.6
#20781 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.5
#20782 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.4
#20783 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.3
#20784 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.2 but encountered conflicts

Cherry-pick of 4c55e4f has failed:

On branch mergify/bp/stable/10.2/pr-20732
Your branch is up to date with 'origin/stable/10.2'.

You are currently cherry-picking commit 4c55e4f0f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   zebra/rt_netlink.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

#20785 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.1 but encountered conflicts

Cherry-pick of 4c55e4f has failed:

On branch mergify/bp/stable/10.1/pr-20732
Your branch is up to date with 'origin/stable/10.1'.

You are currently cherry-picking commit 4c55e4f0f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   zebra/rt_netlink.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of ce6311a has failed:

On branch mergify/bp/stable/10.1/pr-20732
Your branch is ahead of 'origin/stable/10.1' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit ce6311a7a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   zebra/zebra_nhg.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   zebra/zebra_nhg.h
	both modified:   zebra/zebra_rib.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

#20786 Zebra fixup nhg handling from kernel (backport #20732) has been created for branch stable/10.0 but encountered conflicts

Cherry-pick of 4c55e4f has failed:

On branch mergify/bp/stable/10.0/pr-20732
Your branch is up to date with 'origin/stable/10.0'.

You are currently cherry-picking commit 4c55e4f0f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   zebra/rt_netlink.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of ce6311a has failed:

On branch mergify/bp/stable/10.0/pr-20732
Your branch is ahead of 'origin/stable/10.0' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit ce6311a7a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   zebra/zebra_nhg.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   zebra/zebra_nhg.h
	both modified:   zebra/zebra_rib.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

riw777

looks good ... I don't think the AI comments are a concern on this one

Zebra fixup nhg handling from kernel (backport #20732)

donaldsharp added 3 commits February 9, 2026 10:00

zebra: Pass correct NHG id through

4c55e4f

The decoding of the netlink message into a dplane ctx is storing the nhg id not the nhe_id. Let's actually retrieve the right value. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

tests: Simple test that shows nhg received from kernel are respected.

5d6cd84

Create a test that shows when receiving nhg's from the kernel and routes using those nhg's that they are handled correctly in zebra. Signed-off-by: Donald Sharp <sharpd@nvidia.com>

frrbot bot added tests Topotests, make check, etc zebra labels Feb 9, 2026

github-actions bot added size/L master labels Feb 9, 2026

greptile-apps bot reviewed Feb 9, 2026

View reviewed changes

github-actions bot added the backport label Feb 9, 2026

riw777 approved these changes Feb 11, 2026

View reviewed changes

riw777 merged commit f5bc4ec into FRRouting:master Feb 11, 2026
25 checks passed

donaldsharp added a commit that referenced this pull request Feb 11, 2026

Merge pull request #20780 from FRRouting/mergify/bp/dev/10.6/pr-20732

d6622ae

Zebra fixup nhg handling from kernel (backport #20732)

donaldsharp added a commit that referenced this pull request Feb 11, 2026

Merge pull request #20781 from FRRouting/mergify/bp/stable/10.5/pr-20732

1628ba0

Zebra fixup nhg handling from kernel (backport #20732)

donaldsharp added a commit that referenced this pull request Feb 11, 2026

Merge pull request #20782 from FRRouting/mergify/bp/stable/10.4/pr-20732

0fb39e6

Zebra fixup nhg handling from kernel (backport #20732)

donaldsharp added a commit that referenced this pull request Feb 11, 2026

Merge pull request #20783 from FRRouting/mergify/bp/stable/10.3/pr-20732

2803068

Zebra fixup nhg handling from kernel (backport #20732)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zebra fixup nhg handling from kernel#20732

Zebra fixup nhg handling from kernel#20732
riw777 merged 3 commits intoFRRouting:masterfrom
donaldsharp:zebra_fixup_nhg_handling_from_kernel

donaldsharp commented Feb 9, 2026

Uh oh!

greptile-apps bot commented Feb 9, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 9, 2026

Uh oh!

greptile-apps bot Feb 9, 2026

Uh oh!

donaldsharp commented Feb 9, 2026

Uh oh!

mergify bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

riw777 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

donaldsharp commented Feb 9, 2026

Uh oh!

greptile-apps bot commented Feb 9, 2026

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

donaldsharp commented Feb 9, 2026

Uh oh!

mergify bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

riw777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mergify bot commented Feb 9, 2026 •

edited

Loading