]> git.proxmox.com Git - mirror_frr.git/commitdiff
ospf6d: missing ECMP NHs in certain topologies
authorlynne <lynne@voltanet.io>
Wed, 23 Jun 2021 17:15:17 +0000 (13:15 -0400)
committerDonald Sharp <sharpd@nvidia.com>
Thu, 13 Apr 2023 11:55:03 +0000 (07:55 -0400)
When ospf6 is started up and SPF is run depending on which route is
selected as the parent route we could miss adding a NH.   If one
possible parent route has two equal cost paths and the second possible
parent route has only one depending on which one is selected first
determines if we have have one or two NHs.

In the network below when creating a route 2001:db8:3:4::/64 in R2.
When SPF is run there are two possible parent routes R3 and R4.

     2001:db8:1:2  +-----+   2001:db8:2:5
    +--------------+  2  +---------------+
    |          ::2 |     | ::2           |
    |              +-----+               |
    |                                    |
 ::1|                                    |
+-----+                                  |::5
|  1  |2001:db8:1:3+-----+2001:db8:3:5+-----+2001:db8:5:6+-----+
|     +------------+  3  +------------+  5  +------------+  6  |
+-----+ ::1    ::3 |     |::3     ::5 |     |::5      ::6|     |
 ::1|              +-----+            +-----+            +-----+
    |                 |::3
    |                 | 2001:db8:3:4
    |                 |
    |                 |::4
    | 2001:db8:1:4 +-----+
    +--------------+  4  |
               ::4 |     |
                   +-----+

The problem was if we first created the route to 2001:db8:3:4::/64 with R3
as the parent route all is fine.  The code was merging the NH from the parent
route and R3 has 2 NH, one pointing to R1 and one to R5.   But if route
2001:db8:3:4::/64 was first created with parent as R4, it has only one NH
pointing to R1, and then later a new vertex was created pointing to R3 the
code would only copy the nhs from the vertex not from the parent route.   The
vertex always has just one NH.   But the parent route could have more.  So
when we would bringup this setup one time we would see one NH for
2001:db8:3:4::/64 and the next time we would see two NHs.  The code has been
modified so that it behaves the same when the route is first created, or when
a vertex is created, it selects the NHs from the parent route.

Signed-off-by: Lynne Morrison <lynne@voltanet.io>
ospf6d/ospf6_spf.c
ospf6d/ospf6_spf.h

index 3cc0d5e96386ab61867c3a3c8f445c403a3a2c50..0990b14307f2254156c406ae36cf118d29d440f9 100644 (file)
@@ -310,7 +310,7 @@ static void ospf6_nexthop_calc(struct ospf6_vertex *w, struct ospf6_vertex *v,
 static int ospf6_spf_install(struct ospf6_vertex *v,
                             struct ospf6_route_table *result_table)
 {
-       struct ospf6_route *route, *parent_route;
+       struct ospf6_route *route;
        struct ospf6_vertex *prev;
 
        if (IS_OSPF6_DEBUG_SPF(PROCESS))
@@ -330,7 +330,12 @@ static int ospf6_spf_install(struct ospf6_vertex *v,
                        zlog_debug(
                                "  another path found to route %pFX lsa %s, merge",
                                &route->prefix, v->lsa->name);
-               ospf6_spf_merge_nexthops_to_route(route, v);
+
+               /* merging the parent's nexthop information to the child's
+                * if the parent is not the root of the tree.
+                */
+               if (!ospf6_merge_parents_nh_to_child(v, route, result_table))
+                       ospf6_spf_merge_nexthops_to_route(route, v);
 
                prev = (struct ospf6_vertex *)route->route_option;
                assert(prev->hops <= v->hops);
@@ -396,13 +401,7 @@ static int ospf6_spf_install(struct ospf6_vertex *v,
         * installed,
         * its parent's route's nexthops have already been installed.
         */
-       if (v->parent && v->parent->hops) {
-               parent_route =
-                       ospf6_route_lookup(&v->parent->vertex_id, result_table);
-               if (parent_route) {
-                       ospf6_route_merge_nexthops(route, parent_route);
-               }
-       }
+       ospf6_merge_parents_nh_to_child(v, route, result_table);
 
        if (v->parent)
                listnode_add_sort(v->parent->child_list, v);
@@ -1275,3 +1274,20 @@ void ospf6_ase_calculate_timer_add(struct ospf6 *ospf6)
        event_add_timer(master, ospf6_ase_calculate_timer, ospf6,
                        OSPF6_ASE_CALC_INTERVAL, &ospf6->t_ase_calc);
 }
+
+bool ospf6_merge_parents_nh_to_child(struct ospf6_vertex *v,
+                                    struct ospf6_route *route,
+                                    struct ospf6_route_table *result_table)
+{
+       struct ospf6_route *parent_route;
+
+       if (v->parent && v->parent->hops) {
+               parent_route =
+                       ospf6_route_lookup(&v->parent->vertex_id, result_table);
+               if (parent_route) {
+                       ospf6_route_merge_nexthops(route, parent_route);
+                       return true;
+               }
+       }
+       return false;
+}
index 55ca3ec4fe9ea5a37b647be05a2fb02b418b6ce2..c2fd5d9ef1003b09080560a0267c45ab3bd7afe1 100644 (file)
@@ -152,4 +152,8 @@ extern void ospf6_remove_temp_router_lsa(struct ospf6_area *area);
 extern void ospf6_ase_calculate_timer_add(struct ospf6 *ospf6);
 extern int ospf6_ase_calculate_route(struct ospf6 *ospf6, struct ospf6_lsa *lsa,
                                     struct ospf6_area *area);
+extern bool
+ospf6_merge_parents_nh_to_child(struct ospf6_vertex *v,
+                               struct ospf6_route *route,
+                               struct ospf6_route_table *result_table);
 #endif /* OSPF6_SPF_H */