From 0b81a7524d2056b53ea86aab8095f528d645592d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 15 Nov 2023 14:52:10 -0500 Subject: [PATCH] bgpd: MTYPE_BGP was being overused split up The MTYPE_BGP memory type was being over used as both the handler for the bgp instance itself as well as memory associated with name strings. Let's separate out the two. Signed-off-by: Donald Sharp --- bgpd/bgp_evpn.c | 6 ++--- bgpd/bgp_evpn_vty.c | 8 +++--- bgpd/bgp_memory.c | 1 + bgpd/bgp_memory.h | 1 + bgpd/bgp_mplsvpn_snmp.c | 6 ++--- bgpd/bgp_route.c | 2 +- bgpd/bgp_vty.c | 11 +++++---- bgpd/bgpd.c | 55 +++++++++++++++++++++-------------------- 8 files changed, 47 insertions(+), 43 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index ad101f171a..faafab8505 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -6097,7 +6097,7 @@ void bgp_evpn_derive_auto_rd(struct bgp *bgp, struct bgpevpn *vpn) snprintfrr(buf, sizeof(buf), "%pI4:%hu", &bgp->router_id, vpn->rd_id); (void)str2prefix_rd(buf, &vpn->prd); if (vpn->prd_pretty) - XFREE(MTYPE_BGP, vpn->prd_pretty); + XFREE(MTYPE_BGP_NAME, vpn->prd_pretty); UNSET_FLAG(vpn->flags, VNI_FLAG_RD_CFGD); } @@ -6203,7 +6203,7 @@ void bgp_evpn_free(struct bgp *bgp, struct bgpevpn *vpn) hash_release(bgp->vni_svi_hash, vpn); hash_release(bgp->vnihash, vpn); if (vpn->prd_pretty) - XFREE(MTYPE_BGP, vpn->prd_pretty); + XFREE(MTYPE_BGP_NAME, vpn->prd_pretty); QOBJ_UNREG(vpn); XFREE(MTYPE_BGP_EVPN, vpn); } @@ -7080,7 +7080,7 @@ void bgp_evpn_cleanup(struct bgp *bgp) } if (bgp->vrf_prd_pretty) - XFREE(MTYPE_BGP, bgp->vrf_prd_pretty); + XFREE(MTYPE_BGP_NAME, bgp->vrf_prd_pretty); } /* diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index eb9f34ef02..846a82ba90 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -2294,11 +2294,11 @@ static void evpn_configure_vrf_rd(struct bgp *bgp_vrf, struct prefix_rd *rd, bgp_evpn_handle_vrf_rd_change(bgp_vrf, 1); if (bgp_vrf->vrf_prd_pretty) - XFREE(MTYPE_BGP, bgp_vrf->vrf_prd_pretty); + XFREE(MTYPE_BGP_NAME, bgp_vrf->vrf_prd_pretty); /* update RD */ memcpy(&bgp_vrf->vrf_prd, rd, sizeof(struct prefix_rd)); - bgp_vrf->vrf_prd_pretty = XSTRDUP(MTYPE_BGP, rd_pretty); + bgp_vrf->vrf_prd_pretty = XSTRDUP(MTYPE_BGP_NAME, rd_pretty); SET_FLAG(bgp_vrf->vrf_flags, BGP_VRF_RD_CFGD); /* We have a new RD for VRF. @@ -2321,7 +2321,7 @@ static void evpn_unconfigure_vrf_rd(struct bgp *bgp_vrf) bgp_evpn_derive_auto_rd_for_vrf(bgp_vrf); UNSET_FLAG(bgp_vrf->vrf_flags, BGP_VRF_RD_CFGD); if (bgp_vrf->vrf_prd_pretty) - XFREE(MTYPE_BGP, bgp_vrf->vrf_prd_pretty); + XFREE(MTYPE_BGP_NAME, bgp_vrf->vrf_prd_pretty); /* We have a new RD for VRF. * Advertise all type-5 routes again with the new RD */ @@ -2343,7 +2343,7 @@ static void evpn_configure_rd(struct bgp *bgp, struct bgpevpn *vpn, /* update RD */ memcpy(&vpn->prd, rd, sizeof(struct prefix_rd)); - vpn->prd_pretty = XSTRDUP(MTYPE_BGP, rd_pretty); + vpn->prd_pretty = XSTRDUP(MTYPE_BGP_NAME, rd_pretty); SET_FLAG(vpn->flags, VNI_FLAG_RD_CFGD); if (is_vni_live(vpn)) diff --git a/bgpd/bgp_memory.c b/bgpd/bgp_memory.c index 5c3067f96d..38aa4f1c38 100644 --- a/bgpd/bgp_memory.c +++ b/bgpd/bgp_memory.c @@ -15,6 +15,7 @@ DEFINE_MGROUP(BGPD, "bgpd"); DEFINE_MTYPE(BGPD, BGP, "BGP instance"); +DEFINE_MTYPE(BGPD, BGP_NAME, "BGP Name data"); DEFINE_MTYPE(BGPD, BGP_LISTENER, "BGP listen socket details"); DEFINE_MTYPE(BGPD, BGP_PEER, "BGP peer"); DEFINE_MTYPE(BGPD, BGP_PEER_CONNECTION, "BGP peer connection"); diff --git a/bgpd/bgp_memory.h b/bgpd/bgp_memory.h index 7acb41eeb5..8c9524e2ad 100644 --- a/bgpd/bgp_memory.h +++ b/bgpd/bgp_memory.h @@ -11,6 +11,7 @@ DECLARE_MGROUP(BGPD); DECLARE_MTYPE(BGP); +DECLARE_MTYPE(BGP_NAME); DECLARE_MTYPE(BGP_LISTENER); DECLARE_MTYPE(BGP_PEER); DECLARE_MTYPE(BGP_PEER_CONNECTION); diff --git a/bgpd/bgp_mplsvpn_snmp.c b/bgpd/bgp_mplsvpn_snmp.c index 0208a6f5a5..3344e9e0a5 100644 --- a/bgpd/bgp_mplsvpn_snmp.c +++ b/bgpd/bgp_mplsvpn_snmp.c @@ -511,8 +511,8 @@ static int bgp_init_snmp_stats(struct bgp *bgp) { if (is_bgp_vrf_mplsvpn(bgp)) { if (bgp->snmp_stats == NULL) { - bgp->snmp_stats = XCALLOC( - MTYPE_BGP, sizeof(struct bgp_snmp_stats)); + bgp->snmp_stats = XCALLOC(MTYPE_BGP_NAME, + sizeof(struct bgp_snmp_stats)); /* fix up added routes */ if (bgp->snmp_stats) { bgp->snmp_stats->routes_added = @@ -523,7 +523,7 @@ static int bgp_init_snmp_stats(struct bgp *bgp) } } else { if (bgp->snmp_stats) { - XFREE(MTYPE_BGP, bgp->snmp_stats); + XFREE(MTYPE_BGP_NAME, bgp->snmp_stats); bgp->snmp_stats = NULL; } } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b4c97eb2ea..a8fb8275f9 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -6301,7 +6301,7 @@ static void bgp_static_free(struct bgp_static *bgp_static) route_map_counter_decrement(bgp_static->rmap.map); if (bgp_static->prd_pretty) - XFREE(MTYPE_BGP, bgp_static->prd_pretty); + XFREE(MTYPE_BGP_NAME, bgp_static->prd_pretty); XFREE(MTYPE_ATTR, bgp_static->eth_s_id); XFREE(MTYPE_BGP_STATIC, bgp_static); } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index c9d21b6742..e45a5fccb4 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1602,8 +1602,9 @@ DEFUN_NOSH (router_bgp, * - update asnotation if explicitly mentioned */ if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) { - XFREE(MTYPE_BGP, bgp->as_pretty); - bgp->as_pretty = XSTRDUP(MTYPE_BGP, argv[idx_asn]->arg); + XFREE(MTYPE_BGP_NAME, bgp->as_pretty); + bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, + argv[idx_asn]->arg); if (!CHECK_FLAG(bgp->config, BGP_CONFIG_ASNOTATION) && asnotation != ASNOTATION_UNDEFINED) { SET_FLAG(bgp->config, BGP_CONFIG_ASNOTATION); @@ -9392,13 +9393,13 @@ DEFPY (af_rd_vpn_export, bgp_get_default(), bgp); if (yes) { - bgp->vpn_policy[afi].tovpn_rd_pretty = - XSTRDUP(MTYPE_BGP, rd_str); + bgp->vpn_policy[afi].tovpn_rd_pretty = XSTRDUP(MTYPE_BGP_NAME, + rd_str); bgp->vpn_policy[afi].tovpn_rd = prd; SET_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_RD_SET); } else { - XFREE(MTYPE_BGP, bgp->vpn_policy[afi].tovpn_rd_pretty); + XFREE(MTYPE_BGP_NAME, bgp->vpn_policy[afi].tovpn_rd_pretty); UNSET_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_RD_SET); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 6ca0b06450..0cd54ebd8a 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -576,8 +576,8 @@ void bgp_confederation_id_set(struct bgp *bgp, as_t as, const char *as_str) already_confed = bgp_config_check(bgp, BGP_CONFIG_CONFEDERATION); bgp->confed_id = as; if (bgp->confed_id_pretty) - XFREE(MTYPE_BGP, bgp->confed_id_pretty); - bgp->confed_id_pretty = XSTRDUP(MTYPE_BGP, as_str); + XFREE(MTYPE_BGP_NAME, bgp->confed_id_pretty); + bgp->confed_id_pretty = XSTRDUP(MTYPE_BGP_NAME, as_str); bgp_config_set(bgp, BGP_CONFIG_CONFEDERATION); /* If we were doing confederation already, this is just an external @@ -630,7 +630,7 @@ void bgp_confederation_id_unset(struct bgp *bgp) struct listnode *node, *nnode; bgp->confed_id = 0; - XFREE(MTYPE_BGP, bgp->confed_id_pretty); + XFREE(MTYPE_BGP_NAME, bgp->confed_id_pretty); bgp_config_unset(bgp, BGP_CONFIG_CONFEDERATION); for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { @@ -684,7 +684,7 @@ void bgp_confederation_peers_add(struct bgp *bgp, as_t as, const char *as_str) bgp->confed_peers[bgp->confed_peers_cnt].as = as; bgp->confed_peers[bgp->confed_peers_cnt].as_pretty = - XSTRDUP(MTYPE_BGP, as_str); + XSTRDUP(MTYPE_BGP_NAME, as_str); bgp->confed_peers_cnt++; if (bgp_config_check(bgp, BGP_CONFIG_CONFEDERATION)) { @@ -722,7 +722,7 @@ void bgp_confederation_peers_remove(struct bgp *bgp, as_t as) for (i = 0; i < bgp->confed_peers_cnt; i++) if (bgp->confed_peers[i].as == as) { - XFREE(MTYPE_BGP, bgp->confed_peers[i].as_pretty); + XFREE(MTYPE_BGP_NAME, bgp->confed_peers[i].as_pretty); for (j = i + 1; j < bgp->confed_peers_cnt; j++) { bgp->confed_peers[j - 1].as = bgp->confed_peers[j].as; @@ -1265,9 +1265,9 @@ static void peer_free(struct peer *peer) bgp_addpath_set_peer_type(peer, afi, safi, BGP_ADDPATH_NONE, 0); if (peer->change_local_as_pretty) - XFREE(MTYPE_BGP, peer->change_local_as_pretty); + XFREE(MTYPE_BGP_NAME, peer->change_local_as_pretty); if (peer->as_pretty) - XFREE(MTYPE_BGP, peer->as_pretty); + XFREE(MTYPE_BGP_NAME, peer->as_pretty); bgp_peer_connection_free(&peer->connection); @@ -1862,7 +1862,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, peer->as = remote_as; /* internal and external values do not use as_pretty */ if (as_str && asn_str2asn(as_str, NULL)) - peer->as_pretty = XSTRDUP(MTYPE_BGP, as_str); + peer->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_str); peer->as_type = as_type; peer->local_id = bgp->router_id; peer->v_holdtime = bgp->default_holdtime; @@ -1987,10 +1987,10 @@ void peer_as_change(struct peer *peer, as_t as, int as_specified, peer->as = as; if (as_specified == AS_SPECIFIED && as_str) { if (peer->as_pretty) - XFREE(MTYPE_BGP, peer->as_pretty); - peer->as_pretty = XSTRDUP(MTYPE_BGP, as_str); + XFREE(MTYPE_BGP_NAME, peer->as_pretty); + peer->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_str); } else if (peer->as_type == AS_UNSPECIFIED && peer->as_pretty) - XFREE(MTYPE_BGP, peer->as_pretty); + XFREE(MTYPE_BGP_NAME, peer->as_pretty); peer->as_type = as_specified; if (bgp_config_check(peer->bgp, BGP_CONFIG_CONFEDERATION) @@ -3290,9 +3290,9 @@ static struct bgp *bgp_create(as_t *as, const char *name, bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp)); bgp->as = *as; if (as_pretty) - bgp->as_pretty = XSTRDUP(MTYPE_BGP, as_pretty); + bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_pretty); else - bgp->as_pretty = XSTRDUP(MTYPE_BGP, asn_asn2asplain(*as)); + bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, asn_asn2asplain(*as)); if (asnotation != ASNOTATION_UNDEFINED) { bgp->asnotation = asnotation; @@ -3421,14 +3421,14 @@ static struct bgp *bgp_create(as_t *as, const char *name, bgp_mplsvpn_nh_label_bind_cache_init(&bgp->mplsvpn_nh_label_bind); if (name) - bgp->name = XSTRDUP(MTYPE_BGP, name); + bgp->name = XSTRDUP(MTYPE_BGP_NAME, name); event_add_timer(bm->master, bgp_startup_timer_expire, bgp, bgp->restart_time, &bgp->t_startup); /* printable name we can use in debug messages */ if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) { - bgp->name_pretty = XSTRDUP(MTYPE_BGP, "VRF default"); + bgp->name_pretty = XSTRDUP(MTYPE_BGP_NAME, "VRF default"); } else { const char *n; int len; @@ -3440,7 +3440,7 @@ static struct bgp *bgp_create(as_t *as, const char *name, len = 4 + 1 + strlen(n) + 1; /* "view foo\0" */ - bgp->name_pretty = XCALLOC(MTYPE_BGP, len); + bgp->name_pretty = XCALLOC(MTYPE_BGP_NAME, len); snprintf(bgp->name_pretty, len, "%s %s", (bgp->inst_type == BGP_INSTANCE_TYPE_VRF) ? "VRF" @@ -4043,7 +4043,8 @@ void bgp_free(struct bgp *bgp) if (bgp->vpn_policy[afi].rtlist[dir]) ecommunity_free(&bgp->vpn_policy[afi].rtlist[dir]); if (bgp->vpn_policy[afi].tovpn_rd_pretty) - XFREE(MTYPE_BGP, bgp->vpn_policy[afi].tovpn_rd_pretty); + XFREE(MTYPE_BGP_NAME, + bgp->vpn_policy[afi].tovpn_rd_pretty); if (bgp->vpn_policy[afi].tovpn_sid_locator != NULL) srv6_locator_chunk_free( &bgp->vpn_policy[afi].tovpn_sid_locator); @@ -4060,10 +4061,10 @@ void bgp_free(struct bgp *bgp) bgp_srv6_cleanup(bgp); bgp_confederation_id_unset(bgp); - XFREE(MTYPE_BGP, bgp->as_pretty); - XFREE(MTYPE_BGP, bgp->name); - XFREE(MTYPE_BGP, bgp->name_pretty); - XFREE(MTYPE_BGP, bgp->snmp_stats); + XFREE(MTYPE_BGP_NAME, bgp->as_pretty); + XFREE(MTYPE_BGP_NAME, bgp->name); + XFREE(MTYPE_BGP_NAME, bgp->name_pretty); + XFREE(MTYPE_BGP_NAME, bgp->snmp_stats); XFREE(MTYPE_BGP, bgp); } @@ -6465,8 +6466,8 @@ int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, peer->change_local_as = as; if (as_str) { if (peer->change_local_as_pretty) - XFREE(MTYPE_BGP, peer->change_local_as_pretty); - peer->change_local_as_pretty = XSTRDUP(MTYPE_BGP, as_str); + XFREE(MTYPE_BGP_NAME, peer->change_local_as_pretty); + peer->change_local_as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_str); } (void)peer_sort(peer); @@ -6503,8 +6504,8 @@ int peer_local_as_set(struct peer *peer, as_t as, bool no_prepend, replace_as); member->change_local_as = as; if (as_str) - member->change_local_as_pretty = - XSTRDUP(MTYPE_BGP, as_str); + member->change_local_as_pretty = XSTRDUP(MTYPE_BGP_NAME, + as_str); } return 0; @@ -6530,7 +6531,7 @@ int peer_local_as_unset(struct peer *peer) peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); peer->change_local_as = 0; - XFREE(MTYPE_BGP, peer->change_local_as_pretty); + XFREE(MTYPE_BGP_NAME, peer->change_local_as_pretty); } /* Check if handling a regular peer. */ @@ -6561,7 +6562,7 @@ int peer_local_as_unset(struct peer *peer) UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); member->change_local_as = 0; - XFREE(MTYPE_BGP, member->change_local_as_pretty); + XFREE(MTYPE_BGP_NAME, member->change_local_as_pretty); /* Send notification or stop peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) { -- 2.39.5