bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied

[ton31337]

No description provided.

[greptile-apps]

Greptile Summary

This PR fixes a bug in bgpd where a set next-hop unchanged directive in an inbound route-map was incorrectly preventing the nexthop from being reset during outbound announcement processing.

• Root cause: bgp_rmap_nhop_changed accepted two flag sets — out_rmap_flags (attr->rmap_change_flags, the outbound working copy) and in_rmap_flags (piattr->rmap_change_flags, the stored/original path attributes). It checked BATTR_RMAP_NEXTHOP_UNCHANGED in both. If an inbound route-map set this flag (i.e., a peer had set next-hop unchanged on its import policy), the flag persisted in piattr, causing bgp_rmap_nhop_changed to return true and suppressing the nexthop reset on all subsequent outbound announcements — even when no outbound policy requested it.
• Fix: Drops the in_rmap_flags parameter entirely. The BATTR_RMAP_NEXTHOP_UNCHANGED bit is still checked in out_rmap_flags, so an outbound set next-hop unchanged route-map action continues to work correctly; only the erroneous bleed-through from inbound route-maps is eliminated.
• Scope: The function has exactly one callsite (subgroup_announce_check in bgp_route.c), so the API change is clean and complete. No other logic paths are affected.

Confidence Score: 5/5

• Safe to merge — the fix is minimal, targeted, and correctly removes an erroneous cross-contamination between inbound and outbound route-map flag state.
• The change is a two-file, ~5-line fix with a single callsite. The BATTR_RMAP_NEXTHOP_UNCHANGED flag remains correctly checked for outbound route-maps; only the inbound bleed-through is removed. No new logic is introduced, and the function signature change is exhaustively propagated. No P0 or P1 issues found.
• No files require special attention.

Important Files Changed

Filename	Overview
bgpd/bgp_attr.h	Removes the unused in_rmap_flags parameter from bgp_rmap_nhop_changed, retaining all checks on out_rmap_flags including the BATTR_RMAP_NEXTHOP_UNCHANGED flag for outbound route-maps.
bgpd/bgp_route.c	Updates the single callsite of bgp_rmap_nhop_changed in subgroup_announce_check to drop the now-removed piattr->rmap_change_flags argument.

Sequence Diagram

sequenceDiagram
    participant Peer_In as Inbound Peer
    participant BGP as BGP (route store)
    participant RM_In as Inbound Route-Map
    participant RM_Out as Outbound Route-Map
    participant Peer_Out as Outbound Peer

    Peer_In->>BGP: Receive prefix
    BGP->>RM_In: Apply inbound route-map
    Note over RM_In: "set next-hop unchanged"<br/>sets BATTR_RMAP_NEXTHOP_UNCHANGED<br/>in piattr->rmap_change_flags

    BGP->>RM_Out: Apply outbound route-map (subgroup_announce_check)
    RM_Out->>RM_Out: bgp_rmap_nhop_changed(attr->rmap_change_flags)
    Note over RM_Out: OLD: also checked piattr->rmap_change_flags<br/>→ incorrectly saw BATTR_RMAP_NEXTHOP_UNCHANGED<br/>→ skipped nexthop reset<br/><br/>NEW: only checks attr->rmap_change_flags<br/>→ inbound flag has no effect<br/>→ nexthop reset proceeds normally
    RM_Out->>Peer_Out: Announce with correct nexthop

Loading

_{Reviews (1): Last reviewed commit: "bgpd: Don't mark nexthop as changed if a..." | Re-trigger Greptile}

[enkechen-panw]

LGTM.

[riw777]

looks good ... are there any topo test changes for this? I wouldn't think so, but thought I'd ask before pushing.

[ton31337]

looks good ... are there any topo test changes for this? I wouldn't think so, but thought I'd ask before pushing.

@riw777, added.

[ton31337]

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

[mergify]

backport stable/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2

✅ Backports have been created

Details

• #21524 bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied (backport #21445) has been created for branch stable/10.6
• #21525 bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied (backport #21445) has been created for branch stable/10.5
• #21526 bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied (backport #21445) has been created for branch stable/10.4
• #21527 bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied (backport #21445) has been created for branch stable/10.3
• #21528 bgpd: Don't mark nexthop as changed if a set next-hop unchanged is applied (backport #21445) has been created for branch stable/10.2