ospfd: fix wrong/inactive next-hop interface for inter-area routes via virtual link

[mhchann]

Problem

In an OSPF virtual-link topology, inter-area routes whose next-hop is the far
end of the virtual link (and the networks behind it) are installed with an
extra next-hop that has the correct gateway but a wrong outgoing interface,
which zebra then marks inactive. Reproduced on the 10.6.1 release:

r2# show ip route 192.168.21.0/24
Routing entry for 192.168.21.0/24
  Known via "ospf", distance 110, metric 20, best
    10.0.2.2, via r2-eth0 inactive, weight 1   <-- bogus, backbone interface
  * 10.0.2.2, via r2-eth1, weight 1

The OSPF router routing table shows where it comes from — the backbone
router-route to the virtual-link endpoint resolves to the wrong interface:

R    3.3.3.3    [10] area: 0.0.0.10, ABR
                via 10.0.2.2, r2-eth1          <-- correct (transit area)
                [10] area: 0.0.0.0, ABR
                via 10.0.2.2, r2-eth0          <-- wrong (backbone lookup)

Topology

        area 0            area 10              area 20
 r1 ---------------- r2 ---------------- r3 ----+---- 192.168.21.0/24
 1.1.1.1  10.0.1/24   2.2.2.2  10.0.2/24  3.3.3.3     +-- 192.168.22.0/24

r2 is an ABR (backbone + transit area 10). r3 reaches the backbone only via an
area 10 virtual-link to r2. Default broadcast networks.

Root cause

struct vertex_nexthop identifies the egress interface only by lsa_pos, an
index into one area's router-LSA links — i.e. it is area-relative. The
virtual-link next-hop's lsa_pos is computed in the transit area
(ospf_vl_set_params()), but the backbone router-route to the ABR is built by
ospf_intra_add_router() → ospf_route_copy_nexthops_from_vertex() with
area == backbone, so ospf_if_lookup_by_lsa_pos() interprets the
transit-area position against the backbone router-LSA and selects whatever
interface occupies that position there.

This collides whenever the real backbone interface holds the colliding LSA
position — notably when the virtual link is configured at runtime, after the
interfaces (the usual operational order): the VLINK pseudo interface is then
created last and the backbone interface keeps position 0. The bogus path
survives de-duplication (different ifindex) and is inherited by every
inter-area route through that ABR.

Note this is not limited to setups needing abr-type standard on the far
end: in the canonical virtual-link use case — repairing a partitioned
backbone — both endpoints own backbone interfaces, are ABRs under the default
abr-type, and hit this with no special configuration.

Fix (Update)

Resolve the virtual-link egress interface once, in the transit area where
lsa_pos is valid (ospf_vl_set_params()), record its ifindex on the
next-hop (struct vertex_nexthop), and have the consumer
(ospf_route_copy_nexthops_from_vertex()) use that ifindex directly via the
new ospf_if_lookup_by_ifindex() helper. ifindex is not area-relative, so it
is correct regardless of which area's table is being populated, and it
disambiguates correctly when multiple interfaces share a subnet.

ifindex stays 0 for non virtual-link next-hops, so they keep resolving via
lsa_pos - the change is a no-op for them, and the other
ospf_if_lookup_by_lsa_pos() callers (where lsa_pos and the area always
match by construction) are left untouched.

Testing

• New topotest ospf_vlink_nexthop (3 routers; the virtual link is configured
  at runtime via vtysh, which is required to reproduce — see the module
  docstring). On 10.6.1 and master it fails before this change with
  bogus next-hop via r2-eth0 (bug present) and passes after it
  (3 passed).
• r3 uses ospf abr-type standard in the test because configuring a virtual
  link on a non-ABR is rejected, and with the default abr-type a router with
  two non-backbone areas is not an ABR until the virtual link itself attaches
  it to the backbone.
• Existing ospf_topo1 suite: 11 passed, unchanged.

[greptile-apps]

Greptile Summary

Fixes a bug where inter-area routes whose next-hop is resolved via an OSPF virtual link are installed with an extra inactive path using the wrong outgoing interface — a consequence of lsa_pos being area-relative and misinterpreted during backbone SPF.

• Adds ospf_if_lookup_by_gateway() to ospf_interface.c: a longest-prefix-match lookup over all numbered, non-virtual-link OSPF interfaces, mirroring the pattern in ospf_if_lookup_recv_if.
• In ospf_route_copy_nexthops_from_vertex(), after the existing lsa_pos-based lookup, a new guard fires only when virtual links exist and the resolved interface does not own the gateway's subnet; it then calls ospf_if_lookup_by_gateway to recover the correct egress, causing the duplicate bogus path to be de-duplicated away by ospf_path_exist.
• A new topotest (ospf_vlink_nexthop) reproduces the exact failure scenario by configuring the virtual link at runtime after physical adjacencies are established, the ordering required to trigger the colliding LSA position.

Confidence Score: 4/5

The change is safe to merge; it only activates when virtual links are configured, leaves all other code paths untouched, and the new gateway-lookup function follows an established pattern already used elsewhere in the same file.

The fix touches the hot path inside ospf_route_copy_nexthops_from_vertex — called on every SPF run — and the extra prefix-match check now fires for every numbered, non-INADDR_ANY nexthop on any OSPF instance that has virtual links. The logic is sound and the fallback (if (roi) oi = roi) is conservative, but a second set of eyes on the routing-table internals is worthwhile before landing.

ospfd/ospf_route.c — specifically the new guard block and its interaction with the outer ospf_path_exist de-duplication check.

Important Files Changed

Filename	Overview
ospfd/ospf_route.c	Core fix: adds a gateway-subnet mismatch check in ospf_route_copy_nexthops_from_vertex to correct the egress interface for virtual-link nexthops whose lsa_pos was computed in a different area.
ospfd/ospf_interface.c	Adds ospf_if_lookup_by_gateway — a longest-prefix-match helper over ospf->oiflist (excluding virtual-link, loopback, and unnumbered interfaces) — mirrors the existing ospf_if_lookup_recv_if pattern correctly.
ospfd/ospf_interface.h	Adds the extern declaration for the new ospf_if_lookup_by_gateway function; no issues.
tests/topotests/ospf_vlink_nexthop/test_ospf_vlink_nexthop.py	New topotest that reproduces the bug by configuring the virtual link at runtime (after adjacencies form), then polls for 2 min to confirm no bogus inactive next-hop via the backbone interface.
tests/topotests/ospf_vlink_nexthop/r2/frr.conf	r2 ABR config (area 0 + area 10); virtual-link intentionally omitted so it is added at runtime to reproduce the lsa_pos collision.
tests/topotests/ospf_vlink_nexthop/r3/frr.conf	r3 ABR config (area 10 + area 20) with abr-type standard to enable ABR recognition before the virtual link is added; no issues.
tests/topotests/ospf_vlink_nexthop/r1/frr.conf	Minimal r1 backbone router config; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ospf_route_copy_nexthops_from_vertex] --> B{spf_dry_run?}
    B -- yes --> G
    B -- no --> C[ospf_if_lookup_by_lsa_pos\narea, nexthop->lsa_pos]
    C --> D{oi != NULL AND\nrouter != INADDR_ANY AND\nvlinks non-empty AND\noi not unnumbered?}
    D -- no --> G
    D -- yes --> E{prefix_match\noi->connected subnet\nvs gateway?}
    E -- match correct interface --> G
    E -- no match wrong interface --> F[ospf_if_lookup_by_gateway\nospf->oiflist LPM]
    F --> F2{roi found?}
    F2 -- yes --> F3[oi = roi correct egress interface]
    F2 -- no --> G
    F3 --> G{oi != NULL AND\npath not already in list?}
    G -- yes --> H[ospf_path_new add to paths]
    G -- no --> I[skip]

Loading

_{Reviews (1): Last reviewed commit: "ospfd: fix wrong/inactive nexthop interf..." | Re-trigger Greptile}

[donaldsharp]

We call ospf_if_lookup_by_lsa_pos in multiple spots, presumably we can have the same problem in those spots. Why is it not necessary for those?

[mhchann]

Good point - there's no reason to special-case one consumer.
Reworked the fix to address it at the root rather than recovering at the consumer.

The virtual-link next-hop is the only one whose lsa_pos is computed in
a different area (the transit area, in ospf_vl_set_params()) than the
area it is later resolved against (the backbone, in
ospf_route_copy_nexthops_from_vertex()). Instead of detecting that
mismatch downstream, I now resolve the egress interface once where
lsa_pos is valid — in the transit area — and record its ifindex on
the next-hop (struct vertex_nexthop). The consumer uses that ifindex
directly (ospf_if_lookup_by_ifindex()); ifindex is not area-relative,
so it is correct regardless of which area's table is being populated.

The other ospf_if_lookup_by_lsa_pos() callers are unchanged: there the
lsa_pos is always produced for the same area it is resolved against, so
they were never affected. ifindex stays 0 for non virtual-link
next-hops, so they keep using the lsa_pos path — the change is a no-op
for them.

This also removes the gateway/subnet heuristic from the previous version
and disambiguates correctly when multiple interfaces sit on the same
subnet.

Updated; PTAL.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request