zebra: fix wrong comparision for nexthop

[mwinter-osr]

By accident, i found the "same" field in log of the nexthop was actually opposite. Regardless of whether it is active or not, "same" field should be true.

Before:

ZEBRA: [M8H8E-HFGSQ] zebra_nhg_nexthop_compare: 3.3.3.3/32 Comparing via 192.168.0.1, enp2s0(1) ACTIVE: 1 to old: via 192.168.0.1, enp2s0(3) ACTIVE: 1 nexthop same: 0

After:

ZEBRA: [M8H8E-HFGSQ] zebra_nhg_nexthop_compare: 3.3.3.3/32 Comparing via 192.168.0.1, enp2s0(1) ACTIVE: 1 to old: via 192.168.0.1, enp2s0(3) ACTIVE: 1 nexthop same: 1

And this commit should affect the scenarios where nexthop_same_no_ifindex is introduced.

Original PR21503 by anlancs

[greptile-apps]

Greptile Summary

Fixes a boolean inversion bug in nexthop_same_no_ifindex: nexthop_cmp_internal is a comparator that returns 0 for equality (like memcmp/strcmp), but the original code returned its result directly as a bool, causing non-zero (not-equal) to become true and 0 (equal) to become false — the exact opposite of the function's contract. The fix (== 0) is correct and matches the pattern used by every other nexthop_same_* helper.

• P0 — Missing topotest: This bug inverts NHG comparison logic used in zebra_nhg_nexthop_compare. A topotest (e.g., in tests/topotests/zebra_nhg_check/) must accompany this fix to verify correctness and guard against regression.

Confidence Score: 4/5

• Fix is correct but a topotest is required before merging per mandatory FRR policy.
• The one-line fix is unambiguously correct — the inversion is obvious and the == 0 pattern matches every other nexthop_same_* sibling. Score is 4 rather than 5 solely because the mandatory-topotest rule is violated: a behavioral change in NHG comparison logic needs a regression test in tests/topotests/zebra_nhg_check/.
• lib/nexthop.c — fix is correct, but a companion topotest is missing.

Important Files Changed

Filename	Overview
lib/nexthop.c	Fixes inverted boolean return in nexthop_same_no_ifindex: nexthop_cmp_internal is a comparator returning 0 for equal, but the bare return was casting non-zero (not equal) to true and 0 (equal) to false — the exact opposite of the function's contract. Fix is correct and consistent with all other nexthop_same_* helpers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["nexthop_same_no_ifindex(nh1, nh2)"] --> B{nh1 && !nh2\nor !nh1 && nh2?}
    B -- yes --> C["return false"]
    B -- no --> D{nh1 == nh2?}
    D -- yes --> E["return true"]
    D -- no --> F["nexthop_cmp_internal(nh1, nh2, true, false)"]
    F --> G["returns int: 0 if equal, ±N if different"]

    G --> H_OLD["[BEFORE] return int directly as bool\n0 → false (same→wrong!)\n±N → true (different→wrong!)"]
    G --> H_NEW["[AFTER] return == 0\n0 → true (same→correct ✓)\n±N → false (different→correct ✓)"]

    style H_OLD fill:#ffcccc,stroke:#cc0000
    style H_NEW fill:#ccffcc,stroke:#006600

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/nexthop.c
Line: 464

Comment:
**Missing topotest for behavioral fix**

This corrects a long-standing inversion in `zebra_nhg_nexthop_compare` — nexthops that are the same were reported as different, and vice versa. That is a substantive behavioral change in NHG install/update logic and requires a topotest to verify the fix and prevent regression. The existing `tests/topotests/zebra_nhg_check/` suite is the natural home for a case that exercises a route with two equal nexthops on different interface indices and verifies the NHG is not spuriously re-installed.

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

_{Reviews (1): Last reviewed commit: "zebra: fix wrong comparision for nexthop" | Re-trigger Greptile}

[greptile-apps]

Missing topotest for behavioral fix

This corrects a long-standing inversion in zebra_nhg_nexthop_compare — nexthops that are the same were reported as different, and vice versa. That is a substantive behavioral change in NHG install/update logic and requires a topotest to verify the fix and prevent regression. The existing tests/topotests/zebra_nhg_check/ suite is the natural home for a case that exercises a route with two equal nexthops on different interface indices and verifies the NHG is not spuriously re-installed.

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/nexthop.c
Line: 464

Comment:
**Missing topotest for behavioral fix**

This corrects a long-standing inversion in `zebra_nhg_nexthop_compare` — nexthops that are the same were reported as different, and vice versa. That is a substantive behavioral change in NHG install/update logic and requires a topotest to verify the fix and prevent regression. The existing `tests/topotests/zebra_nhg_check/` suite is the natural home for a case that exercises a route with two equal nexthops on different interface indices and verifies the NHG is not spuriously re-installed.

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