ospf6d: fix area border router duplicate

Avoid duplicate ABR brouter entry and
its nexthops.
The route lookup results in first element of the
route/redix node. In case of Intra and inter area
brouter, the first element always intra brouter.
the first element comparison results in always addition
of new element for inter area brouter in brouter table.
Now, iterate all elements of the route node and compare
for brouter origin, if it is same simply update its
nexthops to FIB.

brouter and brouter route nexthops should be merge to avoid
duplicate nexthops for the inter area routes.

Ticket:CM-20807

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>

diff --git a/ospf6d/ospf6_abr.c b/ospf6d/ospf6_abr.c
index b895b5ad8b5ff034f0a45dd10e8504a503894792..ba02457b4fee333016115a5e7090934d892eee7c 100644 (file)
--- a/ospf6d/ospf6_abr.c
+++ b/ospf6d/ospf6_abr.c
@@ -684,7 +684,7 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 {
        struct prefix prefix, abr_prefix;
        struct ospf6_route_table *table = NULL;
-       struct ospf6_route *range, *route, *old = NULL;
+       struct ospf6_route *range, *route, *old = NULL, *old_route;
        struct ospf6_route *abr_entry;
        uint8_t type = 0;
        char options[3] = {0, 0, 0};
@@ -695,14 +695,15 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
        int is_debug = 0;
        struct ospf6_inter_prefix_lsa *prefix_lsa = NULL;
        struct ospf6_inter_router_lsa *router_lsa = NULL;
-       struct ospf6_path *path;
+       bool old_entry_updated = false;
 
        memset(&prefix, 0, sizeof(prefix));
 
        if (lsa->header->type == htons(OSPF6_LSTYPE_INTER_PREFIX)) {
                if (IS_OSPF6_DEBUG_EXAMIN(INTER_PREFIX)) {
                        is_debug++;
-                       zlog_debug("Examin %s in area %s", lsa->name, oa->name);
+                       zlog_debug("%s: Examin %s in area %s",
+                                  __PRETTY_FUNCTION__, lsa->name, oa->name);
                }
 
                prefix_lsa =
@@ -720,7 +721,8 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
        } else if (lsa->header->type == htons(OSPF6_LSTYPE_INTER_ROUTER)) {
                if (IS_OSPF6_DEBUG_EXAMIN(INTER_ROUTER)) {
                        is_debug++;
-                       zlog_debug("Examin %s in area %s", lsa->name, oa->name);
+                       zlog_debug("%s: Examin %s in area %s",
+                                  __PRETTY_FUNCTION__, lsa->name, oa->name);
                }
 
                router_lsa =
@@ -902,11 +904,11 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
        route->path.type = OSPF6_PATH_TYPE_INTER;
        route->path.cost = abr_entry->path.cost + cost;
 
-       ospf6_route_copy_nexthops(route, abr_entry);
-
-       path = ospf6_path_dup(&route->path);
-       ospf6_copy_nexthops(path->nh_list, abr_entry->nh_list);
-       listnode_add_sort(route->paths, path);
+       /* Inter abr_entry is same as brouter.
+        * Avoid duplicate nexthops to brouter and its
+        * learnt route. i.e. use merge nexthops.
+        */
+       ospf6_route_merge_nexthops(route, abr_entry);
 
        /* (7) If the routes are identical, copy the next hops over to existing
           route. ospf6's route table implementation will otherwise string both
@@ -915,11 +917,28 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
        */
        old = ospf6_route_lookup(&prefix, table);
 
-       if (old && (ospf6_route_cmp(route, old) == 0)) {
-               ospf6_route_merge_nexthops(old, route);
+       for (old_route = old; old_route; old_route = old_route->next) {
+               if (!ospf6_route_is_same(old_route, route) ||
+                       (old_route->type != route->type) ||
+                       (old_route->path.type != route->path.type))
+                       continue;
 
+               if ((ospf6_route_cmp(route, old_route) != 0)) {
+                       if (is_debug) {
+                               prefix2str(&prefix, buf, sizeof(buf));
+                               zlog_debug("%s: old %p %s cost %u new route cost %u are not same",
+                                          __PRETTY_FUNCTION__,
+                                          (void *)old_route, buf,
+                                          old_route->path.cost,
+                                          route->path.cost);
+                       }
+                       continue;
+               }
+
+               old_entry_updated = true;
+               ospf6_route_merge_nexthops(old, route);
                if (is_debug)
-                       zlog_debug("%s: Update route: %s old cost %u new cost %u nh count %u",
+                       zlog_debug("%s: Update route: %s old cost %u new cost %u nh %u",
                                   __PRETTY_FUNCTION__,
                                   buf, old->path.cost, route->path.cost,
                                   listcount(route->nh_list));
@@ -930,9 +949,12 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 
                /* Delete new route */
                ospf6_route_delete(route);
-       } else {
+               break;
+       }
+
+       if (old_entry_updated == false) {
                if (is_debug)
-                       zlog_debug("%s: Install route: %s cost %u nh count %u",
+                       zlog_debug("%s: Install route: %s cost %u nh %u",
                                   __PRETTY_FUNCTION__, buf, route->path.cost,
                                   listcount(route->nh_list));
                /* ospf6_ia_add_nw_route (table, &prefix, route); */

diff --git a/ospf6d/ospf6_top.c b/ospf6d/ospf6_top.c
index e4a48915264c7d5286cc91a4501b32d0c7859c8d..7bf099fbbfb2dd33bc0c008e41ca7a6812e12e88 100644 (file)
--- a/ospf6d/ospf6_top.c
+++ b/ospf6d/ospf6_top.c
@@ -97,7 +97,8 @@ static void ospf6_top_route_hook_remove(struct ospf6_route *route)
 
 static void ospf6_top_brouter_hook_add(struct ospf6_route *route)
 {
-       if (IS_OSPF6_DEBUG_EXAMIN(AS_EXTERNAL)) {
+       if (IS_OSPF6_DEBUG_EXAMIN(AS_EXTERNAL) ||
+           IS_OSPF6_DEBUG_BROUTER) {
                uint32_t brouter_id;
                char brouter_name[16];
 
@@ -116,15 +117,17 @@ static void ospf6_top_brouter_hook_add(struct ospf6_route *route)
 
 static void ospf6_top_brouter_hook_remove(struct ospf6_route *route)
 {
-       if (IS_OSPF6_DEBUG_EXAMIN(AS_EXTERNAL)) {
+       if (IS_OSPF6_DEBUG_EXAMIN(AS_EXTERNAL) ||
+           IS_OSPF6_DEBUG_BROUTER) {
                uint32_t brouter_id;
                char brouter_name[16];
 
                brouter_id = ADV_ROUTER_IN_PREFIX(&route->prefix);
                inet_ntop(AF_INET, &brouter_id, brouter_name,
                          sizeof(brouter_name));
-               zlog_debug("%s: brouter %s del with nh count %u",
-                          __PRETTY_FUNCTION__, brouter_name,
+               zlog_debug("%s: brouter %p %s del with adv router %x nh %u",
+                          __PRETTY_FUNCTION__, (void *)route, brouter_name,
+                          route->path.origin.adv_router,
                           listcount(route->nh_list));
        }
        route->flag |= OSPF6_ROUTE_REMOVE;