From a5ab756f2483594a19837e0c30f6184cd966940f Mon Sep 17 00:00:00 2001
From: Chirag Shah <chirag@nvidia.com>
Date: Sat, 10 Oct 2020 15:09:11 -0700
Subject: [PATCH] bgpd: fix crash in bgp instance creation

In bgp global commands northbound local-as modify callback
check for backend db for checking existing bgp instance.

In an instance where no router bgp with old ASN cleaned up
followed by new bgp instance with new AS is created,
the nb_running_get_entry in validation phase returns stale
bgp reference, which leads to rejection of the router bgp command.

Uncovered via:
toptotest evpn_type5_test_topo1/test_evpn_type5_topo1.py
test_bgp_attributes_for_evpn_address_family_p1

Signed-off-by: Chirag Shah <chirag@nvidia.com>
---
 bgpd/bgp_nb_config.c | 70 +++++++++++++++++++++++++++++++++++++++-----
 bgpd/bgp_vty.c       |  4 +--
 2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/bgpd/bgp_nb_config.c b/bgpd/bgp_nb_config.c
index a610ce6c24..e37017bb04 100644
--- a/bgpd/bgp_nb_config.c
+++ b/bgpd/bgp_nb_config.c
@@ -44,6 +44,33 @@ FRR_CFG_DEFAULT_ULONG(BGP_KEEPALIVE,
         { .val_ulong = 60 },
 )
 
+
+static int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
+				      const char *name,
+				      enum bgp_instance_type inst_type)
+{
+	struct bgp *bgp;
+
+	if (name)
+		bgp = bgp_lookup_by_name(name);
+	else
+		bgp = bgp_get_default();
+
+	if (bgp) {
+		if (bgp->as != *as) {
+			*as = bgp->as;
+			return BGP_ERR_INSTANCE_MISMATCH;
+		}
+		if (bgp->inst_type != inst_type)
+			return BGP_ERR_INSTANCE_MISMATCH;
+		*bgp_val = bgp;
+	} else {
+		*bgp_val = NULL;
+	}
+
+	return BGP_SUCCESS;
+}
+
 int routing_control_plane_protocols_name_validate(
 	struct nb_cb_create_args *args)
 {
@@ -107,8 +134,8 @@ int bgp_router_create(struct nb_cb_create_args *args)
 		if (ret == BGP_ERR_INSTANCE_MISMATCH) {
 			snprintf(
 				args->errmsg, args->errmsg_len,
-				"BGP instance name and AS number mismatch\nBGP instance is already running; AS is %u",
-				as);
+				"BGP instance name and AS number mismatch\nBGP instance is already running; AS is %u, input-as %u",
+				bgp->as, as);
 
 			return NB_ERR_INCONSISTENCY;
 		}
@@ -140,6 +167,9 @@ int bgp_router_destroy(struct nb_cb_destroy_args *args)
 	case NB_EV_VALIDATE:
 		bgp = nb_running_get_entry(args->dnode, NULL, false);
 
+		if (!bgp)
+			return NB_OK;
+
 		if (bgp->l3vni) {
 			snprintf(args->errmsg, args->errmsg_len,
 				 "Please unconfigure l3vni %u", bgp->l3vni);
@@ -186,16 +216,42 @@ int bgp_global_local_as_modify(struct nb_cb_modify_args *args)
 {
 	struct bgp *bgp;
 	as_t as;
+	const struct lyd_node *vrf_dnode;
+	const char *vrf_name;
+	const char *name = NULL;
+	enum bgp_instance_type inst_type;
+	int ret;
+	bool is_view_inst = false;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
 		as = yang_dnode_get_uint32(args->dnode, NULL);
 
-		bgp = nb_running_get_entry_non_rec(args->dnode, NULL, false);
-		if (bgp && bgp->as != as) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "BGP instance is already running; AS is %u",
-				 bgp->as);
+		inst_type = BGP_INSTANCE_TYPE_DEFAULT;
+
+		vrf_dnode = yang_dnode_get_parent(args->dnode,
+						  "control-plane-protocol");
+		vrf_name = yang_dnode_get_string(vrf_dnode, "./vrf");
+
+		if (strmatch(vrf_name, VRF_DEFAULT_NAME)) {
+			name = NULL;
+		} else {
+			name = vrf_name;
+			inst_type = BGP_INSTANCE_TYPE_VRF;
+		}
+
+		is_view_inst = yang_dnode_get_bool(args->dnode,
+						   "../instance-type-view");
+		if (is_view_inst)
+			inst_type = BGP_INSTANCE_TYPE_VIEW;
+
+		ret = bgp_lookup_by_as_name_type(&bgp, &as, name, inst_type);
+		if (ret == BGP_ERR_INSTANCE_MISMATCH) {
+			snprintf(
+				args->errmsg, args->errmsg_len,
+				"BGP instance name and AS number mismatch\nBGP instance is already running; input-as %u",
+				as);
+
 			return NB_ERR_VALIDATION;
 		}
 
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 6b04168304..c8a3c28523 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -1222,9 +1222,6 @@ DEFUN_YANG_NOSH(router_bgp,
 			vty_out(vty, "%% Please specify ASN and VRF\n");
 			return CMD_WARNING_CONFIG_FAILED;
 		}
-		/* unset the auto created flag as the user config is now present
-		 */
-		UNSET_FLAG(bgp->vrf_flags, BGP_VRF_AUTO);
 
 		snprintf(base_xpath, sizeof(base_xpath), FRR_BGP_GLOBAL_XPATH,
 			 "frr-bgp:bgp", "bgp", VRF_DEFAULT_NAME);
@@ -1239,6 +1236,7 @@ DEFUN_YANG_NOSH(router_bgp,
 					      NB_OP_MODIFY, "true");
 		}
 
+		nb_cli_pending_commit_check(vty);
 		ret = nb_cli_apply_changes(vty, base_xpath);
 		if (ret == CMD_SUCCESS) {
 			VTY_PUSH_XPATH(BGP_NODE, base_xpath);
-- 
2.39.5