ospfd, ospf6d: do not install routes for directly attached networks

[rzalamena]

There's no point in installing routes for directly attached networks since connected routes already exist for this purpose.

[riw777]

This seems like an okay change, but this will break tests. It probably needs to be transitioned in?

[Jafaral]

@greptile, you can review the files under the directories ospfd and ospf6d only?

[greptile-apps]

Greptile Summary

This PR suppresses the installation of directly-attached (connected) OSPF routes into the kernel RIB, on the basis that the kernel already has connected routes for those prefixes. The change touches both OSPFv2 (ospfd) and OSPFv3 (ospf6d) and is accompanied by a large set of topotest reference-file updates that remove connected prefixes from expected OSPF RIB output.

Key changes:

• Adds a connected boolean to ospf_route and ospf6_route.
• In OSPFv2: marks routes as connected in ospf_intra_add_transit (all-parents check for INADDR_ANY) and ospf_intra_add_stub (root-vertex check). Prevents zebra installs via a guard in ospf_zebra_add and skips old-table deletes in ospf_route_delete_uniq.
• In OSPFv3: marks SPF vertices with hops == 0 as connected, propagates the flag through intra-prefix LSA processing, and guards the ADD path of ospf6_zebra_route_update.
• Two correctness issues found:
  1. ospfd/ospf_route.c: when a stub route created from the root vertex (connected) is later overwritten by a lower-cost remote path, the connected flag is never cleared. The route stays suppressed even though the best path is now via a remote next-hop.
  2. ospf6d/ospf6_zebra.c: the connected guard only covers the initial type == ADD case. The secondary-path fallback logic can flip type to ADD and replace request with a new route after the guard, potentially installing a connected route in the RIB.

Confidence Score: 3/5

• Mergeable with caution — the approach is sound but two correctness issues could suppress valid routes or install connected routes in specific topologies.
• The core idea and most of the implementation are correct and well-tested by the large topotest reference updates. However, two logic gaps remain: (1) ospf_intra_add_stub can leave connected=true on a route that has been overwritten by a better remote path, causing that route to be silently dropped from the RIB; (2) ospf6_zebra_route_update's secondary-path promotion can turn a REM into an ADD without rechecking the connected flag, potentially installing a connected route. Both are edge-case scenarios but represent real correctness gaps introduced by this PR.
• ospfd/ospf_route.c (stub overwrite path, lines 643-656) and ospf6d/ospf6_zebra.c (secondary-path promotion, lines 430-438)

Important Files Changed

Filename	Overview
ospfd/ospf_route.c	Core OSPFv2 route management: adds connected detection in ospf_intra_add_transit (checking all parents) and ospf_intra_add_stub (root-only check). Minor issue: the stub overwrite path does not propagate/clear the connected flag when a better remote path replaces a locally-connected route.
ospfd/ospf_zebra.c	Adds early-return guard for connected routes in ospf_zebra_add; straightforward and correct.
ospfd/ospf_route.h	Adds the connected boolean field to ospf_route; clean and minimal.
ospf6d/ospf6_spf.c	Sets route->connected = true for SPF vertices with hops == 0 (directly attached networks); logically correct.
ospf6d/ospf6_intra.c	Propagates ls_entry->connected to intra-prefix LSA routes in ospf6_intra_prefix_lsa_add; correctly inherits the connected flag from the SPF linkstate entry.
ospf6d/ospf6_zebra.c	Guards ospf6_zebra_route_update ADD path against connected routes. However, the secondary-path promotion code (REM→ADD fallback) can bypass the guard and install a connected route if the promoted secondary path is also connected.
ospf6d/ospf6_route.c	Copies connected flag in ospf6_route_copy; necessary and correct.
ospf6d/ospf6_route.h	Adds connected boolean field to ospf6_route; clean addition.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SPF Run] --> B[ospf_intra_add_transit / ospf6_spf_install]
    B --> C{Root vertex?\nv->parents empty\nor v->hops == 0}
    C -- Yes --> D[mark route->connected = true]
    C -- No --> E{Any parent nexthop\n== INADDR_ANY?}
    E -- Yes --> D
    E -- No --> F[connected = false]
    D --> G[ospf_route_install / ospf6_route_table_update]
    F --> G

    G --> H[ospf_zebra_add / ospf6_zebra_route_update ADD]
    H --> I{route->connected?}
    I -- Yes --> J[Return early\ndo not send to Zebra]
    I -- No --> K[zclient_route_send ZEBRA_ROUTE_ADD]

    G --> L[ospf_route_delete_uniq]
    L --> M{or->connected?}
    M -- Yes --> N[Skip ospf_zebra_delete]
    M -- No --> O{Prefix in new table?}
    O -- No --> P[ospf_zebra_delete]
    O -- Yes --> N

    G --> Q[ospf6_zebra_route_update REM]
    Q --> R{type==REM\nbest path\n& has next?}
    R -- Yes --> S[Promote next as ADD\nrequest = request->next]
    S --> T{⚠️ request->connected\nnot re-checked}
    T --> U[zclient_route_send ZEBRA_ROUTE_ADD\npossibly for connected route]
    R -- No --> V[zclient_route_send ZEBRA_ROUTE_DELETE]

Loading

Comments Outside Diff (1)

1. ospf6d/ospf6_zebra.c, line 430-438 (link)

   Connected route may be installed via the secondary-path fallback

   The connected-route guard at the top of the function only covers the initial type == ADD case:

   if (type == ADD && request->connected)
       return;

   However, the "best-path removal → secondary path promotion" block below can flip type to ADD and replace request with a new route after the guard has already been passed:

   if (type == REM && ospf6_route_is_best(request) && request->next &&
       ospf6_route_is_same(request, request->next) &&
       ospf6_route_num_nexthops(request->next) > 0) {
       type = ADD;
       request = request->next; // ← may be a connected route
   }

   If the secondary path (request->next) is also a directly-connected route (connected == true), the function continues past this point and ends up calling zclient_route_send(ZEBRA_ROUTE_ADD, ...) for it — exactly the scenario this PR is trying to prevent.

   This can occur when the same directly-connected prefix has multiple ECMP paths (e.g., a dual-homed link). The fix is to re-apply the connected check after the swap:

   if (type == REM && ospf6_route_is_best(request) && request->next &&
       ospf6_route_is_same(request, request->next) &&
       ospf6_route_num_nexthops(request->next) > 0) {
       type = ADD;
       request = request->next;
       /* Re-check: do not install if the promoted secondary is also connected. */
       if (request->connected)
           return;
   }

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: ospf6d/ospf6_zebra.c
   Line: 430-438
   
   Comment:
   **Connected route may be installed via the secondary-path fallback**
   
   The connected-route guard at the top of the function only covers the initial `type == ADD` case:
   
   ```c
   if (type == ADD && request->connected)
       return;
   ```
   
   However, the "best-path removal → secondary path promotion" block below can flip `type` to `ADD` and replace `request` with a new route **after** the guard has already been passed:
   
   ```c
   if (type == REM && ospf6_route_is_best(request) && request->next &&
       ospf6_route_is_same(request, request->next) &&
       ospf6_route_num_nexthops(request->next) > 0) {
       type = ADD;
       request = request->next; // ← may be a connected route
   }
   ```
   
   If the secondary path (`request->next`) is also a directly-connected route (`connected == true`), the function continues past this point and ends up calling `zclient_route_send(ZEBRA_ROUTE_ADD, ...)` for it — exactly the scenario this PR is trying to prevent.
   
   This can occur when the same directly-connected prefix has multiple ECMP paths (e.g., a dual-homed link). The fix is to re-apply the connected check after the swap:
   
   ```c
   if (type == REM && ospf6_route_is_best(request) && request->next &&
       ospf6_route_is_same(request, request->next) &&
       ospf6_route_num_nexthops(request->next) > 0) {
       type = ADD;
       request = request->next;
       /* Re-check: do not install if the promoted secondary is also connected. */
       if (request->connected)
           return;
   }
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ospfd/ospf_route.c
Line: 643-656

Comment:
**`connected` flag not cleared when better remote-path overwrites local-connected route**

In `ospf_intra_add_stub`, if the SPF first creates a route for a stub prefix via the root vertex (marking it `connected = true`), and a later vertex with a lower total cost for the same prefix then overwrites it (taking the `cost < cur_or->cost` branch), `cur_or->connected` is never cleared.

The overwrite path updates `cost`, `paths`, and `origin` from the non-root vertex `v`, but leaves `connected = true`. When `ospf_route_install` later calls `ospf_zebra_add`, the route is silently dropped because of the `or->connected` guard — even though the best path is now via a remote next-hop and should be installed in the RIB.

Concretely: if the local router has a high-metric stub (say cost 100) and a remote router also advertises the same prefix as a stub with a lower total cost (e.g., 7), the remote path overwrites the local one but the route is never pushed to zebra.

The fix is to update `cur_or->connected` in the overwrite branch:

```c
if (cost < cur_or->cost) {
    cur_or->cost = cost;
    cur_or->connected = list_isempty(v->parents); /* update connected flag */

    list_delete_all_node(cur_or->paths);
    ospf_route_copy_nexthops_from_vertex(area, cur_or, v);
    cur_or->u.std.origin = (struct lsa_header *)lsa;
    return;
}
```

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

---

This is a comment left during a code review.
Path: ospf6d/ospf6_zebra.c
Line: 430-438

Comment:
**Connected route may be installed via the secondary-path fallback**

The connected-route guard at the top of the function only covers the initial `type == ADD` case:

```c
if (type == ADD && request->connected)
    return;
```

However, the "best-path removal → secondary path promotion" block below can flip `type` to `ADD` and replace `request` with a new route **after** the guard has already been passed:

```c
if (type == REM && ospf6_route_is_best(request) && request->next &&
    ospf6_route_is_same(request, request->next) &&
    ospf6_route_num_nexthops(request->next) > 0) {
    type = ADD;
    request = request->next; // ← may be a connected route
}
```

If the secondary path (`request->next`) is also a directly-connected route (`connected == true`), the function continues past this point and ends up calling `zclient_route_send(ZEBRA_ROUTE_ADD, ...)` for it — exactly the scenario this PR is trying to prevent.

This can occur when the same directly-connected prefix has multiple ECMP paths (e.g., a dual-homed link). The fix is to re-apply the connected check after the swap:

```c
if (type == REM && ospf6_route_is_best(request) && request->next &&
    ospf6_route_is_same(request, request->next) &&
    ospf6_route_num_nexthops(request->next) > 0) {
    type = ADD;
    request = request->next;
    /* Re-check: do not install if the promoted secondary is also connected. */
    if (request->connected)
        return;
}
```

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

_{Last reviewed commit: "tests: update tests ..."}

[rzalamena]

@greptileai

[riw777]

looks good