Merge pull request #2173 from chiragshah6/ospfv3_dev

ospf6d: fix area border router duplicate

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 =
@@ -768,7 +770,8 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 	}
 	if (OSPF6_LSA_IS_MAXAGE(lsa)) {
 		if (is_debug)
-			zlog_debug("LSA is MaxAge, ignore");
+			zlog_debug("%s: LSA %s is MaxAge, ignore",
+				   __PRETTY_FUNCTION__, lsa->name);
 		if (old)
 			ospf6_route_remove(old, table);
 		return;
@@ -845,9 +848,24 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 	    || CHECK_FLAG(abr_entry->flag, OSPF6_ROUTE_REMOVE)
 	    || !CHECK_FLAG(abr_entry->path.router_bits, OSPF6_ROUTER_BIT_B)) {
 		if (is_debug)
-			zlog_debug("ABR router entry does not exist, ignore");
+			zlog_debug("%s: ABR router entry does not exist, ignore",
-		if (old)
+				   __PRETTY_FUNCTION__);
+		if (old) {
+			if (old->type == OSPF6_DEST_TYPE_ROUTER &&
+			    oa->intra_brouter_calc) {
+				if (is_debug)
+					zlog_debug(
+						   "%s: intra_brouter_calc is on, skip brouter remove: %s (%p)",
+						   __PRETTY_FUNCTION__, buf,
+						   (void *)old);
+			} else {
+				if (is_debug)
+					zlog_debug("%s: remove old entry: %s %p ",
+						   __PRETTY_FUNCTION__, buf,
+						   (void *)old);
 				ospf6_route_remove(old, table);
+			}
+		}
 		return;
 	}
 
@@ -902,11 +920,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);
+	/* Inter abr_entry is same as brouter.
-
+	 * Avoid duplicate nexthops to brouter and its
-	path = ospf6_path_dup(&route->path);
+	 * learnt route. i.e. use merge nexthops.
-	ospf6_copy_nexthops(path->nh_list, abr_entry->nh_list);
+	 */
-	listnode_add_sort(route->paths, path);
+	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 +933,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)) {
+	for (old_route = old; old_route; old_route = old_route->next) {
-		ospf6_route_merge_nexthops(old, route);
+		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 +965,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); */

ospf6d/ospf6_area.h

@@ -50,6 +50,9 @@ struct ospf6_area {
 	/* Area type */
 	int no_summary;
 
+	/* Brouter traversal protection */
+	int intra_brouter_calc;
+
 	/* OSPF interface list */
 	struct list *if_list;
 

ospf6d/ospf6_intra.c

@@ -2047,8 +2047,10 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 	uint32_t brouter_id;
 	char brouter_name[16];
 
-	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id))
+	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id) ||
-		zlog_info("border-router calculation for area %s", oa->name);
+	    IS_OSPF6_DEBUG_ROUTE(MEMORY))
+		zlog_info("%s: border-router calculation for area %s",
+			  __PRETTY_FUNCTION__, oa->name);
 
 	hook_add = oa->ospf6->brouter_table->hook_add;
 	hook_remove = oa->ospf6->brouter_table->hook_remove;
@@ -2114,6 +2116,7 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 
 	for (brouter = ospf6_route_head(oa->ospf6->brouter_table); brouter;
 	     brouter = nbrouter) {
+
 		/*
 		 * brouter may have been "deleted" in the last loop iteration.
 		 * If this is the case there is still 1 final refcount lock
@@ -2122,6 +2125,8 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 		 * skip processing the deleted route.
 		 */
 		if (brouter->lock == 1) {
+			if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
+				ospf6_brouter_debug_print(brouter);
 			nbrouter = ospf6_route_next(brouter);
 			continue;
 		} else {
@@ -2173,8 +2178,14 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 				       brouter_id)
 			    || IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(
 				       oa->area_id))
-				zlog_info("brouter %s disappears via area %s",
+				zlog_info("%s: brouter %s disappears via area %s",
-					  brouter_name, oa->name);
+					  __PRETTY_FUNCTION__, brouter_name,
+					  oa->name);
+			/* This is used to protect nbrouter from removed from
+			 * the table. For an example, ospf6_abr_examin_summary,
+			 * removes brouters which are marked for remove.
+			 */
+			oa->intra_brouter_calc = 1;
 			ospf6_route_remove(brouter, oa->ospf6->brouter_table);
 			brouter = NULL;
 		} else if (CHECK_FLAG(brouter->flag, OSPF6_ROUTE_ADD)
@@ -2184,8 +2195,9 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 				       brouter_id)
 			    || IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(
 				       oa->area_id))
-				zlog_info("brouter %s appears via area %s",
+				zlog_info("%s: brouter %s appears via area %s",
-					  brouter_name, oa->name);
+					  __PRETTY_FUNCTION__, brouter_name,
+					  oa->name);
 
 			/* newly added */
 			if (hook_add)
@@ -2205,11 +2217,14 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
 			UNSET_FLAG(brouter->flag, OSPF6_ROUTE_ADD);
 			UNSET_FLAG(brouter->flag, OSPF6_ROUTE_CHANGE);
 		}
+		/* Reset for nbrouter */
+		oa->intra_brouter_calc = 0;
 	}
 
-	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id))
+	if (IS_OSPF6_DEBUG_BROUTER_SPECIFIC_AREA_ID(oa->area_id) ||
-		zlog_info("border-router calculation for area %s: done",
+	    IS_OSPF6_DEBUG_ROUTE(MEMORY))
-			  oa->name);
+		zlog_info("%s: border-router calculation for area %s: done",
+			  __PRETTY_FUNCTION__, oa->name);
 }
 
 struct ospf6_lsa_handler router_handler = {.lh_type = OSPF6_LSTYPE_ROUTER,

ospf6d/ospf6_route.c

@@ -925,10 +925,11 @@ struct ospf6_route *ospf6_route_next(struct ospf6_route *route)
 	struct ospf6_route *next = route->next;
 
 	if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
-		zlog_info("%s %p: route next: %p<-[%p]->%p",
+		zlog_info("%s %p: route next: %p<-[%p]->%p , route ref count %u",
 			  ospf6_route_table_name(route->table),
 			  (void *)route->table, (void *)route->prev,
-			  (void *)route, (void *)route->next);
+			  (void *)route, (void *)route->next,
+			  route->lock);
 
 	ospf6_route_unlock(route);
 	if (next)

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",
+		zlog_debug("%s: brouter %p %s del with adv router %x nh %u",
-			   __PRETTY_FUNCTION__, brouter_name,
+			   __PRETTY_FUNCTION__, (void *)route, brouter_name,
+			   route->path.origin.adv_router,
 			   listcount(route->nh_list));
 	}
 	route->flag |= OSPF6_ROUTE_REMOVE;