bgpd: VRF-Lite fix default bgp delete

1. bgp coredump is observed when we delete default bgp instance
   when we have multi-vrf; and route-leaking is enabled between
   default, non-default vrfs.
Removing default router bgp when routes leaked between non-default vrfs.
- Routes are leaked from VRF-A to VRF-B
- VPN table is created with auto RD/RT in default instance.
- Default instance is deleted, we try to unimport the routes from all VRFs
- non-default VRF schedules a work-queue to process deleted routes.
- Meanwhile default bgp instance clears VPN tables and free the route
  entries as well, which are still referenced by non-default VRFs which
  have imported routes.
- When work queue process starts to delete imported route in VRF-A it cores
  as it accesses freed memory.

- Whenever we delete bgp in default vrf, we skip deleting routes in the vpn
  table, import and export lists.
- The default hidden bgp instance will not be listed in any of the show
  commands.
- Whenever we create new default instance, handle it with AS number change
  i.e. old hidden default bgp's AS number is updated and also changing
  local_as for all peers.

2. A default instance is created with ASN of the vrf with the import
  statement.
  This may not be the ASN desired for the default table
- First problem with current behavior.
  Define two vrfs with different ASNs and then add import between.
  starting without any bgp config (no default instance)
  A default instance is created with ASN of the vrf with the import
  statement.
  This may not be the ASN desired for the default table
- Second related problem.  Start with a default instance and a vrf in a
  different ASN. Do an import statement in the vrf for a bgp vrf instance
  not yet defined and it auto-creates that bgp/vrf instance and it inherits
  the ASN of the importing vrf
- Handle bgp instances with different ASNs and handle ASN for auto created
  BGP instance

Signed-off-by: Kantesh Mundaragi <kmundaragi@vmware.com>

1 files changed, 101 insertions, 36 deletions

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 23ab4d3d91..2b624d1652 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -3421,12 +3421,18 @@ static void bgp_vrf_string_name_delete(void *data)
 static struct bgp *bgp_create(as_t *as, const char *name,
 			      enum bgp_instance_type inst_type,
 			      const char *as_pretty,
-			      enum asnotation_mode asnotation)
+			      enum asnotation_mode asnotation,
+			      struct bgp *bgp_old, bool hidden)
 {
 	struct bgp *bgp;
 	afi_t afi;
 	safi_t safi;
 
+	if (hidden) {
+		bgp = bgp_old;
+		goto peer_init;
+	}
+
 	bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp));
 	bgp->as = *as;
 	if (as_pretty)
@@ -3480,18 +3486,24 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 		bgp->peer_self->domainname =
 			XSTRDUP(MTYPE_BGP_PEER_HOST, cmd_domainname_get());
 	bgp->peer = list_new();
+
+peer_init:
 	bgp->peer->cmp = (int (*)(void *, void *))peer_cmp;
 	bgp->peerhash = hash_create(peer_hash_key_make, peer_hash_same,
 				    "BGP Peer Hash");
 	bgp->peerhash->max_size = BGP_PEER_MAX_HASH_SIZE;
 
-	bgp->group = list_new();
+	if (!hidden)
+		bgp->group = list_new();
 	bgp->group->cmp = (int (*)(void *, void *))peer_group_cmp;
 
 	FOREACH_AFI_SAFI (afi, safi) {
-		bgp->route[afi][safi] = bgp_table_init(bgp, afi, safi);
-		bgp->aggregate[afi][safi] = bgp_table_init(bgp, afi, safi);
-		bgp->rib[afi][safi] = bgp_table_init(bgp, afi, safi);
+		if (!hidden) {
+			bgp->route[afi][safi] = bgp_table_init(bgp, afi, safi);
+			bgp->aggregate[afi][safi] = bgp_table_init(bgp, afi,
+								   safi);
+			bgp->rib[afi][safi] = bgp_table_init(bgp, afi, safi);
+		}
 
 		/* Enable maximum-paths */
 		bgp_maximum_paths_set(bgp, afi, safi, BGP_PEER_EBGP,
@@ -3530,7 +3542,7 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 	bgp->rmap_def_originate_eval_timer = 0;
 
 #ifdef ENABLE_BGP_VNC
-	if (inst_type != BGP_INSTANCE_TYPE_VRF) {
+	if (inst_type != BGP_INSTANCE_TYPE_VRF && !hidden) {
 		bgp->rfapi = bgp_rfapi_new(bgp);
 		assert(bgp->rfapi);
 		assert(bgp->rfapi_cfg);
@@ -3547,9 +3559,11 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 		bgp->vpn_policy[afi].import_vrf = list_new();
 		bgp->vpn_policy[afi].import_vrf->del =
 			bgp_vrf_string_name_delete;
-		bgp->vpn_policy[afi].export_vrf = list_new();
-		bgp->vpn_policy[afi].export_vrf->del =
-			bgp_vrf_string_name_delete;
+		if (!hidden) {
+			bgp->vpn_policy[afi].export_vrf = list_new();
+			bgp->vpn_policy[afi].export_vrf->del =
+				bgp_vrf_string_name_delete;
+		}
 		SET_FLAG(bgp->af_flags[afi][SAFI_MPLS_VPN],
 			 BGP_VPNVX_RETAIN_ROUTE_TARGET_ALL);
 	}
@@ -3567,7 +3581,7 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 			bgp->restart_time, &bgp->t_startup);
 
 	/* printable name we can use in debug messages */
-	if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) {
+	if (inst_type == BGP_INSTANCE_TYPE_DEFAULT && !hidden) {
 		bgp->name_pretty = XSTRDUP(MTYPE_BGP_NAME, "VRF default");
 	} else {
 		const char *n;
@@ -3595,17 +3609,20 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 	bgp->coalesce_time = BGP_DEFAULT_SUBGROUP_COALESCE_TIME;
 	bgp->default_af[AFI_IP][SAFI_UNICAST] = true;
 
-	QOBJ_REG(bgp, bgp);
+	if (!hidden)
+		QOBJ_REG(bgp, bgp);
 
 	update_bgp_group_init(bgp);
 
-	/* assign a unique rd id for auto derivation of vrf's RD */
-	bf_assign_index(bm->rd_idspace, bgp->vrf_rd_id);
+	if (!hidden) {
+		/* assign a unique rd id for auto derivation of vrf's RD */
+		bf_assign_index(bm->rd_idspace, bgp->vrf_rd_id);
 
-	bgp_evpn_init(bgp);
-	bgp_evpn_vrf_es_init(bgp);
-	bgp_pbr_init(bgp);
-	bgp_srv6_init(bgp);
+		bgp_evpn_init(bgp);
+		bgp_evpn_vrf_es_init(bgp);
+		bgp_pbr_init(bgp);
+		bgp_srv6_init(bgp);
+	}
 
 	/*initilize global GR FSM */
 	bgp_global_gr_init(bgp);
@@ -3743,10 +3760,15 @@ int bgp_handle_socket(struct bgp *bgp, struct vrf *vrf, vrf_id_t old_vrf_id,
 		return bgp_check_main_socket(create, bgp);
 }
 
-int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *name,
+int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
+			       const char *as_pretty,
+			       enum asnotation_mode asnotation, const char *name,
 			       enum bgp_instance_type inst_type)
 {
 	struct bgp *bgp;
+	struct peer *peer = NULL;
+	struct listnode *node, *nnode;
+	bool hidden = false;
 
 	/* Multiple instance check. */
 	if (name)
@@ -3755,14 +3777,28 @@ int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *name,
 		bgp = bgp_get_default();
 
 	if (bgp) {
-		*bgp_val = bgp;
+		if (IS_BGP_INSTANCE_HIDDEN(bgp) && *as != AS_UNSPECIFIED)
+			hidden = true;
+		/* Handle AS number change */
 		if (bgp->as != *as) {
-			*as = bgp->as;
-			return BGP_ERR_AS_MISMATCH;
+			if (hidden)
+				bgp_create(as, name, inst_type, as_pretty,
+					   asnotation, bgp, hidden);
+
+			/* Set all peer's local as number with this ASN */
+			for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer))
+				peer->local_as = *as;
+			UNSET_FLAG(bgp->flags, BGP_FLAG_INSTANCE_HIDDEN);
+			*bgp_val = bgp;
+			return BGP_INSTANCE_EXISTS;
 		}
 		if (bgp->inst_type != inst_type)
 			return BGP_ERR_INSTANCE_MISMATCH;
-		return BGP_SUCCESS;
+		if (hidden)
+			bgp_create(as, name, inst_type, as_pretty, asnotation,
+				   bgp, hidden);
+		*bgp_val = bgp;
+		return BGP_INSTANCE_EXISTS;
 	}
 	*bgp_val = NULL;
 
@@ -3778,11 +3814,13 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name,
 	struct vrf *vrf = NULL;
 	int ret = 0;
 
-	ret = bgp_lookup_by_as_name_type(bgp_val, as, name, inst_type);
+	ret = bgp_lookup_by_as_name_type(bgp_val, as, as_pretty, asnotation,
+					 name, inst_type);
 	if (ret || *bgp_val)
 		return ret;
 
-	bgp = bgp_create(as, name, inst_type, as_pretty, asnotation);
+	bgp = bgp_create(as, name, inst_type, as_pretty, asnotation, NULL,
+			 false);
 
 	/*
 	 * view instances will never work inside of a vrf
@@ -4022,6 +4060,15 @@ int bgp_delete(struct bgp *bgp)
 		bgp_damp_disable(bgp, afi, safi);
 	}
 
+	if (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT &&
+	    (bgp_table_top(bgp->rib[AFI_IP][SAFI_MPLS_VPN]) ||
+	     bgp_table_top(bgp->rib[AFI_IP6][SAFI_MPLS_VPN]))) {
+		if (BGP_DEBUG(zebra, ZEBRA))
+			zlog_debug(
+				"Marking the deleting default bgp instance as hidden");
+		SET_FLAG(bgp->flags, BGP_FLAG_INSTANCE_HIDDEN);
+	}
+
 	if (BGP_DEBUG(zebra, ZEBRA)) {
 		if (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)
 			zlog_debug("Deleting Default VRF");
@@ -4066,7 +4113,7 @@ int bgp_delete(struct bgp *bgp)
 		peer_delete(peer);
 	}
 
-	if (bgp->peer_self) {
+	if (bgp->peer_self && !IS_BGP_INSTANCE_HIDDEN(bgp)) {
 		peer_delete(bgp->peer_self);
 		bgp->peer_self = NULL;
 	}
@@ -4076,7 +4123,8 @@ int bgp_delete(struct bgp *bgp)
 /* TODO - Other memory may need to be freed - e.g., NHT */
 
 #ifdef ENABLE_BGP_VNC
-	rfapi_delete(bgp);
+	if (!IS_BGP_INSTANCE_HIDDEN(bgp))
+		rfapi_delete(bgp);
 #endif
 
 	/* Free memory allocated with aggregate address configuration. */
@@ -4118,7 +4166,7 @@ int bgp_delete(struct bgp *bgp)
 	}
 
 	/* Deregister from Zebra, if needed */
-	if (IS_BGP_INST_KNOWN_TO_ZEBRA(bgp)) {
+	if (IS_BGP_INST_KNOWN_TO_ZEBRA(bgp) && !IS_BGP_INSTANCE_HIDDEN(bgp)) {
 		if (BGP_DEBUG(zebra, ZEBRA))
 			zlog_debug(
 				"%s: deregistering this bgp %s instance from zebra",
@@ -4126,17 +4174,19 @@ int bgp_delete(struct bgp *bgp)
 		bgp_zebra_instance_deregister(bgp);
 	}
 
-	/* Remove visibility via the master list - there may however still be
-	 * routes to be processed still referencing the struct bgp.
-	 */
-	listnode_delete(bm->bgp, bgp);
-
-	/* Free interfaces in this instance. */
-	bgp_if_finish(bgp);
+	if (!IS_BGP_INSTANCE_HIDDEN(bgp)) {
+		/* Remove visibility via the master list -
+		 * there may however still be routes to be processed
+		 * still referencing the struct bgp.
+		 */
+		listnode_delete(bm->bgp, bgp);
+		/* Free interfaces in this instance. */
+		bgp_if_finish(bgp);
+	}
 
 	vrf = bgp_vrf_lookup_by_instance_type(bgp);
 	bgp_handle_socket(bgp, vrf, VRF_UNKNOWN, false);
-	if (vrf)
+	if (vrf && !IS_BGP_INSTANCE_HIDDEN(bgp))
 		bgp_vrf_unlink(bgp, vrf);
 
 	/* Update EVPN VRF pointer */
@@ -4151,7 +4201,22 @@ int bgp_delete(struct bgp *bgp)
 		work_queue_free_and_null(&bgp->process_queue);
 
 	event_master_free_unused(bm->master);
-	bgp_unlock(bgp); /* initial reference */
+
+	if (!IS_BGP_INSTANCE_HIDDEN(bgp))
+		bgp_unlock(bgp); /* initial reference */
+	else {
+		for (afi = AFI_IP; afi < AFI_MAX; afi++) {
+			enum vpn_policy_direction dir;
+
+			if (bgp->vpn_policy[afi].import_vrf)
+				list_delete(&bgp->vpn_policy[afi].import_vrf);
+
+			dir = BGP_VPN_POLICY_DIR_FROMVPN;
+			if (bgp->vpn_policy[afi].rtlist[dir])
+				ecommunity_free(
+					&bgp->vpn_policy[afi].rtlist[dir]);
+		}
+	}
 
 	return 0;
 }