bgpd: Enforce RFC 4271 Section 5.1.3 - disallow NEXT_HOP equal to peer address

[sri-mohan1]

A locally originated route must not be advertised to a peer using that
peer's own address as NEXT_HOP. After outbound route-map processing, if
the final NEXT_HOP equals the receiving peer's address, reset it to the
local address to comply with RFC 4271 Section 5.1.3 and prevent a
blackhole.

[greptile-apps]

Greptile Summary

This PR enforces RFC 4271 Section 5.1.3 in FRR's BGP outbound announcement path: after route-map processing, if a locally-originated route's NEXT_HOP equals the receiving peer's own address, the nexthop is reset to the local address and a warning is logged. The new block is correctly placed in subgroup_announce_check after the link-local reset and before the WECMP/AIGP adjustments that depend on nh_reset.

• Core logic (bgpd/bgp_route.c): New RFC 4271 guard at lines 3127–3159 covers IPv4 unicast and IPv6/ENHE paths; sets nh_reset = true so subsequent WECMP/link-bandwidth and AIGP recalculations are applied correctly to the corrected nexthop.
• Error reporting (bgpd/bgp_errors.{c,h}): New error code EC_BGP_NEXTHOP_SELF_PEER with a descriptive message and operator suggestion.
• Regression test (tests/topotests/bgp_ipv4_nexthop_peer_self/): Two-router topology verifying the nexthop reset for the violating prefix and preservation for a non-peer nexthop; R2 uses bgp disable-ebgp-connected-route-check to allow the non-connected nexthop assertion.

Confidence Score: 5/5

Safe to merge for the described scope; the guard is additive and only fires for locally-originated routes where a route-map has explicitly set the nexthop to the peer's own address.

The RFC 4271 enforcement block is correctly placed after the link-local reset and before the WECMP/AIGP adjustments, so nh_reset=true propagates correctly to those downstream paths. Both IPv4 unicast and IPv6/ENHE code paths are covered. The peer->connection and su_remote null-guards are present. The error code registration is complete and well-formed. No new defects were found in the changed paths.

The topotest topology warrants a close look: R2's bgp disable-ebgp-connected-route-check is relied upon to make the 198.51.100.0/24 (nexthop 10.0.0.1) assertion pass, and it is worth confirming that flag is sufficient to resolve the nexthop as valid in FRR before the test lands in CI.

Important Files Changed

Filename	Overview
bgpd/bgp_route.c	New RFC 4271 block correctly placed between link-local reset and WECMP/AIGP adjustments; sets nh_reset=true; covers AFI_IP/SAFI_UNICAST and NEXTHOP_IS_V6 paths; also fixes a pre-existing indentation bug in the link-local block.
bgpd/bgp_errors.c	Adds EC_BGP_NEXTHOP_SELF_PEER log-ref entry with title, description, and suggestion; appended before END_FERR sentinel.
bgpd/bgp_errors.h	EC_BGP_NEXTHOP_SELF_PEER enum value added to bgp_log_refs; no issues.
tests/topotests/bgp_ipv4_nexthop_peer_self/test_bgp_ipv4_nexthop_peer_self.py	Two functional tests plus memory-leak check; first test correctly validates that the peer-address nexthop is reset to 10.0.12.1; second test checks non-peer nexthop (10.0.0.1) is preserved with bgp disable-ebgp-connected-route-check on R2 to allow nexthop reachability bypass.
tests/topotests/bgp_ipv4_nexthop_peer_self/r1/frr.conf	R1 config advertising two prefixes with outbound route-map; one nexthop set to peer address (10.0.12.2) to trigger RFC 4271 enforcement, one to a non-peer address (10.0.0.1) for regression coverage.
tests/topotests/bgp_ipv4_nexthop_peer_self/r2/frr.conf	R2 config with bgp disable-ebgp-connected-route-check to allow the non-connected nexthop 10.0.0.1 assertion in the regression test; no other issues.
tests/topotests/bgp_ipv4_nexthop_peer_self/init.py	Empty init.py required for Python package discovery; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[subgroup_announce_check] --> B{bgp_rmap_nhop_changed?\nor transparent?\nor NH_UNCHANGED?}
    B -- No --> C[Nexthop-self block\nlines 3041-3110\nnh_reset=true]
    B -- Yes --> D[Skip nexthop-self block]
    C --> E
    D --> E{p->family==AF_INET6\nor peer_cap_enhe?}
    E -- Yes --> F{mp_nexthop_global\nis link-local?}
    F -- Yes --> G[subgroup_announce_reset_nhop AF_INET6\nnh_reset=true]
    F -- No --> H
    G --> H
    E -- No --> H
    H{from==bgp->peer_self\nand peer->connection->su_remote?}
    H -- No --> K
    H -- Yes --> I{AFI_IP + SAFI_UNICAST +\nAF_INET + nexthop==peer_addr?}
    I -- Yes --> I1[flog_warn EC_BGP_NEXTHOP_SELF_PEER\nsubgroup_announce_reset_nhop AF_INET\nnh_reset=true\nRFC 4271 5.1.3]
    I -- No --> J{NEXTHOP_IS_V6 +\nAF_INET6 +\nmp_nexthop_global==peer_addr?}
    J -- Yes --> J1[flog_warn EC_BGP_NEXTHOP_SELF_PEER\nsubgroup_announce_reset_nhop AF_INET6\nnh_reset=true\nRFC 4271 5.1.3]
    J -- No --> K
    I1 --> K
    J1 --> K
    K{nh_reset &&\nWECMP link-bandwidth?}
    K -- Yes --> L[Replace cumulative\nbandwidth ext-community]
    K -- No --> M
    L --> M{AIGP attr exists?}
    M -- Yes --> N{AIGP transmit allowed\nand nh_reset or\nBATTR_RMAP_NEXTHOP_PEER_ADDRESS?}
    N -- Yes --> O[Recalculate AIGP metric]
    N -- No --> P[Unset AIGP metric]
    M -- No --> Q[Continue...]
    O --> Q
    P --> Q

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[subgroup_announce_check] --> B{bgp_rmap_nhop_changed?\nor transparent?\nor NH_UNCHANGED?}
    B -- No --> C[Nexthop-self block\nlines 3041-3110\nnh_reset=true]
    B -- Yes --> D[Skip nexthop-self block]
    C --> E
    D --> E{p->family==AF_INET6\nor peer_cap_enhe?}
    E -- Yes --> F{mp_nexthop_global\nis link-local?}
    F -- Yes --> G[subgroup_announce_reset_nhop AF_INET6\nnh_reset=true]
    F -- No --> H
    G --> H
    E -- No --> H
    H{from==bgp->peer_self\nand peer->connection->su_remote?}
    H -- No --> K
    H -- Yes --> I{AFI_IP + SAFI_UNICAST +\nAF_INET + nexthop==peer_addr?}
    I -- Yes --> I1[flog_warn EC_BGP_NEXTHOP_SELF_PEER\nsubgroup_announce_reset_nhop AF_INET\nnh_reset=true\nRFC 4271 5.1.3]
    I -- No --> J{NEXTHOP_IS_V6 +\nAF_INET6 +\nmp_nexthop_global==peer_addr?}
    J -- Yes --> J1[flog_warn EC_BGP_NEXTHOP_SELF_PEER\nsubgroup_announce_reset_nhop AF_INET6\nnh_reset=true\nRFC 4271 5.1.3]
    J -- No --> K
    I1 --> K
    J1 --> K
    K{nh_reset &&\nWECMP link-bandwidth?}
    K -- Yes --> L[Replace cumulative\nbandwidth ext-community]
    K -- No --> M
    L --> M{AIGP attr exists?}
    M -- Yes --> N{AIGP transmit allowed\nand nh_reset or\nBATTR_RMAP_NEXTHOP_PEER_ADDRESS?}
    N -- Yes --> O[Recalculate AIGP metric]
    N -- No --> P[Unset AIGP metric]
    M -- No --> Q[Continue...]
    O --> Q
    P --> Q

Loading

_{Reviews (3): Last reviewed commit: "bgpd: Enforce RFC 4271 Section 5.1.3 - d..." | Re-trigger Greptile}

[sri-mohan1]

@greptile review

[sri-mohan1]

@greptile review

[donaldsharp]

This whole thing started because of a route-map setting a nexthop equal to the peers address. Why do we want to override policy here? Someone is intentionally doing this right?

[sri-mohan1]

This whole thing started because of a route-map setting a nexthop equal to the peers address. Why do we want to override policy here? Someone is intentionally doing this right?

Thanks Mr. @donaldsharp for your comment.

RFC 4271 Section 5.1.3 mandates this behavior - it's not a policy override, it's protocol compliance:
"A route originated by a BGP speaker SHALL NOT be advertised to a peer using an address of that peer as NEXT_HOP."
we must fix it,
which prevents routing blockholes (Peer installing a route pointing to itself = traffic drop/loop)
almost always config error (Admin meant local address, not peer's address)
consistent with existing behavior (we already fix 0.0.0.0/:: next hops the same way)
logged for visibility (EC_BGP_NEXTHOP_SELF_PEER alerts operators to the misconfiguration)

This is similar to how FRR (and other BGP implementations) prevent, Advertising a route with an invalid AS-path, Sending updates to a peer in the wrong BGP state and Using reserved/well-known community values incorrectly.