bgpd: fix NHT for link-local nexthops from global-address peers

[soumyar-roy]

bgpd: fix NHT for link-local nexthops from global-address peers

When a global-address peer advertises a route with a link-local
nexthop (e.g. set via route-map), bgp_find_or_add_nexthop() created
a BNC keyed with ifindex_ipv6_ll=0 because the existing conf_if
guard did not cover this case.  With ifindex 0 the BNC was registered
with zebra NHT, which resolved the ambiguous fe80::/64 prefix against
an arbitrary interface.  If that interface went down, zebra declared
the nexthop unreachable and BGP withdrew all routes using it — even
though the real peer interface was still up.

Topology:  IXIA -----(swp63s0)---- leaf12 ----(swp1s0..swp32s3)---- spine12

- IXIA peers with leaf12 using global address 2101:fee1:baad::1
- IXIA advertises n routes to leaf12
  with link-local nexthop fe80::216:1ff:fe00:1
- All interfaces (swp63s0 towards IXIA, swp1s0..swp32s3 towards
  spine12) have link-local addresses in fe80::/64

Step 1 - BGP receives route with LL nexthop:
  Route say 4001:fee1:bab1:1f00::/56, nexthop = fe80::216:1ff:fe00:1
  This LL nexthop belongs to IXIA, reachable via swp63s0.

Step 2 - BGP registers nexthop for NHT (the bug):
  bgp_find_or_add_nexthop() builds BNC key:
    prefix = fe80::216:1ff:fe00:1/128, ifindex = 0
  ifindex stays 0 because peer address (2101:fee1:baad::1) is
  global, so the conf_if guard at bgp_nht.c is skipped.
  BNC is registered with zebra NHT (not tracked locally).

Step 3 - Zebra resolves against wrong interface:
  Zebra receives NHT request for fe80::216:1ff:fe00:1 with no
  ifindex.  fe80::/64 is a connected prefix on every interface.
  Zebra picks an arbitrary match:
    Resolved: fe80::/64 via swp17s1  <-- spine interface (WRONG)
    Should be: fe80::/64 via swp63s0  <-- IXIA interface (CORRECT)
  BGP marks the nexthop VALID, routes are installed.  Traffic
  works because the actual forwarding uses peer->nexthop.ifp
  (swp63s0), not the NHT resolution interface.

Step 4 - 32 spine links are shut down (including swp17s1):
  swp17s1 goes down.  Zebra re-evaluates fe80::/64 on swp17s1,
  finds it unreachable, sends NHT update to BGP:
    fe80::216:1ff:fe00:1 -> UNREACHABLE

Step 5 - BGP withdraws all n IXIA routes:
  BGP marks BNC invalid, withdraws every route attached to it.
  Routes are deleted from zebra/kernel, causing traffic loss on
  4-5 flows (~250-611 ms).  Routes are re-added shortly after
  when zebra re-resolves fe80::/64 via another spine interface
  that is still up.

Fix: derive ifindex from peer->nexthop.ifp->ifindex so the BNC
is keyed with the correct interface and tracked locally via
interface events, bypassing zebra NHT entirely.

Also schedule bgp_nht_ifp_initial() from register_zebra_rnh() when
ifindex_ipv6_ll is set, so the BNC gets validated via interface events
even when no LL peer exists to trigger bgp_nht_interface_events().

Signed-off-by: Soumya Roy <soumyar@nvidia.com>

[greptile-apps]

Greptile Summary

This PR fixes a BGP NHT bug where a global-address peer advertising a link-local nexthop (e.g. via route-map) caused bgp_find_or_add_nexthop() to create a BNC with ifindex_ipv6_ll=0, leading zebra to resolve the ambiguous fe80::/64 against an arbitrary interface; when that interface went down, BGP falsely withdrew all routes. The fix derives ifindex from peer->nexthop.ifp for this case so the BNC is keyed correctly and tracked locally via interface events, adds proper nexthop struct lifecycle management in bgp_nht_ifp_table_handle, and schedules bgp_nht_ifp_initial from register_zebra_rnh to handle initial validation for BNCs that bypass bgp_nht_interface_events().

Confidence Score: 5/5

Safe to merge; the fix is well-reasoned, the three change sites are tightly coordinated, and no P0/P1 issues were found.

All remaining findings are P2 style suggestions. The core logic — deriving ifindex from peer->nexthop.ifp, proper nexthop struct allocation/free in the up/down path, and initial validation scheduling — is correct and addresses the described bug without regressing existing LL peer behaviour. Prior review concerns were addressed.

tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py — test only asserts NHT registration, not FIB installation

Important Files Changed

Filename	Overview
bgpd/bgp_nht.c	Three coordinated fixes: (1) derive ifindex from peer->nexthop.ifp for global-address peers advertising LL nexthops so the BNC is interface-tracked instead of zebra-NHT-tracked; (2) allocate/free a proper NEXTHOP_TYPE_IFINDEX nexthop struct in bgp_nht_ifp_table_handle so evaluate_paths can iterate it; (3) schedule bgp_nht_ifp_initial from register_zebra_rnh to validate BNCs that bypass bgp_nht_interface_events(). Logic is correct and well-commented.
tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py	New test reproduces the topology from the bug report and asserts the LL nexthop is NOT registered with zebra NHT after the fix. Session establishment and route receipt are verified, but nexthop validity and FIB installation are not checked, leaving a gap in coverage.

Sequence Diagram

sequenceDiagram
    participant R3 as r3 (global-addr peer)
    participant BGP as bgpd (r1)
    participant BNC as BNC cache
    participant Zebra as zebra NHT
    participant IFP as Interface events

    R3->>BGP: UPDATE: 2001:db8:3::1/128, nexthop=fe80:1::4 (LL, via route-map)
    BGP->>BGP: bgp_find_or_add_nexthop(pi), afi=IPv6, LL nexthop != peer addr
    Note over BGP: NEW: derive ifindex from peer->nexthop.ifp->ifindex
    BGP->>BNC: bnc_new(fe80:1::4/128, ifindex=r1-eth1)
    Note over BNC: ifindex_ipv6_ll = r1-eth1 ifindex
    BGP->>BGP: register_zebra_rnh(bnc)
    Note over BGP: ifindex_ipv6_ll > 0, skip sendmsg_zebra_rnh, NEW: schedule bgp_nht_ifp_initial
    BGP->>IFP: event_add_event(bgp_nht_ifp_initial, ifindex)
    IFP->>BGP: bgp_nht_ifp_initial fires
    BGP->>BNC: bgp_nht_ifp_table_handle(r1-eth1, up), NEW: alloc NEXTHOP_TYPE_IFINDEX, SET BGP_NEXTHOP_VALID
    BNC->>BGP: evaluate_paths, install route
    Note over Zebra: fe80:1::4 NOT registered with zebra NHT

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py
Line: 697-707

Comment:
**Test verifies registration but not nexthop validity or FIB installation**

The assertion confirms `fe80:1::4` is absent from zebra NHT (i.e., `sendmsg_zebra_rnh` was skipped), which proves the core bug fix. However it does not check that `bgp_nht_ifp_initial` subsequently fires and marks the BNC valid, nor that the route is actually installed in the FIB. A regression where the BNC is created with the correct `ifindex_ipv6_ll` but never validated (e.g., `bgp_nht_ifp_initial` event lost) would pass this assertion while silently leaving the prefix invalid and not forwarded.

Consider adding a check like:
```python
def _route_installed():
    output = json.loads(r1.vtysh_cmd("show ipv6 route 2001:db8:3::1/128 json"))
    routes = output.get("2001:db8:3::1/128", [])
    for route in routes:
        if route.get("protocol") == "bgp" and route.get("installed"):
            return None
    return "Route 2001:db8:3::1/128 not installed in FIB"

_, result = topotest.run_and_expect(_route_installed, None, count=30, wait=1)
assert result is None, "Route not installed despite valid nexthop"
```

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

_{Reviews (2): Last reviewed commit: "tests: add topotest for LL nexthop NHT f..." | Re-trigger Greptile}

[soumyar-roy]

ci:rerun

[ton31337]

LGTM