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>

diff --git a/bgpd/bgp_nb_config.c b/bgpd/bgp_nb_config.c
index a610ce6c248b847ca86297d5d90cc6fb02878ebc..e37017bb04894d390d75ef310645cfbd0db31986 100644 (file)
--- 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 6b04168304cf267cf9e57cfc73d914f1ef033f23..c8a3c28523a498cc4b8339f993612a57f22bd721 100644 (file)
--- 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);