From d09328e5991c9f657758921264492825e7081175 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 23 Apr 2021 21:36:12 +0300 Subject: [PATCH] bgpd: fix bgp_get_vty return values There are multiple problems: - commit ef7c53e2 introduced a new return value 2 which broke things, because a lot of code treats non-zero return as an error, - there is an incorrect error returned when AS number mismatches. This commit fixes both. Signed-off-by: Igor Ryzhov --- bgpd/bgp_evpn.c | 5 +++-- bgpd/bgp_nb_config.c | 36 ++++++++++++++++++++++-------------- bgpd/bgpd.c | 9 ++------- bgpd/bgpd.h | 1 - 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 2d4fea413a..a9fceb6cde 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -5382,11 +5382,12 @@ int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id, switch (ret) { case BGP_ERR_AS_MISMATCH: flog_err(EC_BGP_EVPN_AS_MISMATCH, - "BGP is already running; AS is %u", as); + "BGP instance is already running; AS is %u", + as); return -1; case BGP_ERR_INSTANCE_MISMATCH: flog_err(EC_BGP_EVPN_INSTANCE_MISMATCH, - "BGP instance name and AS number mismatch"); + "BGP instance type mismatch"); return -1; } diff --git a/bgpd/bgp_nb_config.c b/bgpd/bgp_nb_config.c index 94ff362d1a..307fa4e9bc 100644 --- a/bgpd/bgp_nb_config.c +++ b/bgpd/bgp_nb_config.c @@ -105,16 +105,22 @@ int bgp_router_create(struct nb_cb_create_args *args) if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) is_new_bgp = (bgp_lookup(as, name) == NULL); + else + is_new_bgp = (bgp_lookup_by_name(name) == NULL); ret = bgp_get_vty(&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; AS is %u, input-as %u", - bgp->as, as); - + switch (ret) { + case BGP_ERR_AS_MISMATCH: + snprintf(args->errmsg, args->errmsg_len, + "BGP instance is already running; AS is %u", + as); + return NB_ERR_INCONSISTENCY; + case BGP_ERR_INSTANCE_MISMATCH: + snprintf(args->errmsg, args->errmsg_len, + "BGP instance type mismatch"); return NB_ERR_INCONSISTENCY; } + /* * If we just instantiated the default instance, complete * any pending VRF-VPN leaking that was configured via @@ -128,7 +134,7 @@ int bgp_router_create(struct nb_cb_create_args *args) * Leak the routes to importing bgp vrf instances, * only when new bgp vrf instance is configured. */ - if (ret != BGP_INSTANCE_EXISTS) + if (is_new_bgp) bgp_vpn_leak_export(bgp); UNSET_FLAG(bgp->vrf_flags, BGP_VRF_AUTO); @@ -244,15 +250,17 @@ int bgp_global_local_as_modify(struct nb_cb_modify_args *args) 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); - + switch (ret) { + case BGP_ERR_AS_MISMATCH: + snprintf(args->errmsg, args->errmsg_len, + "BGP instance is already running; AS is %u", + as); + return NB_ERR_VALIDATION; + case BGP_ERR_INSTANCE_MISMATCH: + snprintf(args->errmsg, args->errmsg_len, + "BGP instance type mismatch"); return NB_ERR_VALIDATION; } - break; case NB_EV_PREPARE: case NB_EV_ABORT: diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index bad62f9946..20bb5e5320 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3376,7 +3376,7 @@ int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *name, if (bgp) { if (bgp->as != *as) { *as = bgp->as; - return BGP_ERR_INSTANCE_MISMATCH; + return BGP_ERR_AS_MISMATCH; } if (bgp->inst_type != inst_type) return BGP_ERR_INSTANCE_MISMATCH; @@ -3397,13 +3397,8 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name, int ret = 0; ret = bgp_lookup_by_as_name_type(bgp_val, as, name, inst_type); - switch (ret) { - case BGP_ERR_INSTANCE_MISMATCH: + if (ret || *bgp_val) return ret; - case BGP_SUCCESS: - if (*bgp_val) - return BGP_INSTANCE_EXISTS; - } bgp = bgp_create(as, name, inst_type); if (bgp_option_check(BGP_OPT_NO_ZEBRA) && name) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index f9aa62c682..51134dc8c5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1844,7 +1844,6 @@ enum bgp_clear_type { /* BGP error codes. */ #define BGP_SUCCESS 0 #define BGP_CREATED 1 -#define BGP_INSTANCE_EXISTS 2 #define BGP_ERR_INVALID_VALUE -1 #define BGP_ERR_INVALID_FLAG -2 #define BGP_ERR_INVALID_AS -3 -- 2.39.5