From 318cac96ef7e16a17fd1ab97b67ede5c07651c52 Mon Sep 17 00:00:00 2001 From: Daniel Walton Date: Tue, 22 Aug 2017 18:14:50 +0000 Subject: [PATCH] bgpd: Memory wasting in zebra by non used MPLS FECs Signed-off-by: Daniel Walton --- bgpd/bgp_route.c | 7 +++- bgpd/bgp_vty.c | 23 +----------- bgpd/bgpd.c | 95 ++++++++++++++++++++++++++++++++++++++---------- bgpd/bgpd.h | 4 ++ 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index bb204b01f1..0c60231ded 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2045,7 +2045,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_node *rn, * to do this upon changes to best path except of the label index * changes. */ - if (safi == SAFI_UNICAST) { + if (bgp->allocate_mpls_labels[afi][safi]) { if (new_select) { if (!old_select || bgp_label_index_differs(new_select, old_select) @@ -2066,8 +2066,11 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_node *rn, } else bgp_register_for_label(rn, new_select); } - } else if (CHECK_FLAG(rn->flags, BGP_NODE_REGISTERED_FOR_LABEL)) + } else if (CHECK_FLAG(rn->flags, BGP_NODE_REGISTERED_FOR_LABEL)) { bgp_unregister_for_label(rn); + } + } else if (CHECK_FLAG(rn->flags, BGP_NODE_REGISTERED_FOR_LABEL)) { + bgp_unregister_for_label(rn); } /* If best route remains the same and this is not due to user-initiated diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2e18a6d44f..f040ed9a08 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -7018,26 +7018,6 @@ static int bgp_show_summary(struct vty *vty, struct bgp *bgp, int afi, int safi, return CMD_SUCCESS; } -/* - * Return if we have a peer configured to use this afi/safi - */ -static int bgp_show_summary_afi_safi_peer_exists(struct bgp *bgp, int afi, - int safi) -{ - struct listnode *node; - struct peer *peer; - - for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { - if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) - continue; - - if (peer->afc[afi][safi]) - return 1; - } - - return 0; -} - static void bgp_show_summary_afi_safi(struct vty *vty, struct bgp *bgp, int afi, int safi, u_char use_json, json_object *json) @@ -7056,8 +7036,7 @@ static void bgp_show_summary_afi_safi(struct vty *vty, struct bgp *bgp, int afi, if (safi_wildcard) safi = 1; /* SAFI_UNICAST */ while (safi < SAFI_MAX) { - if (bgp_show_summary_afi_safi_peer_exists(bgp, afi, - safi)) { + if (bgp_afi_safi_peer_exists(bgp, afi, safi)) { json_output = true; if (is_wildcard) { /* diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 9d7c38c871..ba2e82a42e 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1391,6 +1391,32 @@ void bgp_peer_conf_if_to_su_update(struct peer *peer) hash_get(peer->bgp->peerhash, peer, hash_alloc_intern); } +static void bgp_recalculate_afi_safi_bestpaths(struct bgp *bgp, afi_t afi, + safi_t safi) +{ + struct bgp_node *rn, *nrn; + + for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; + rn = bgp_route_next(rn)) { + if (rn->info != NULL) { + /* Special handling for 2-level routing + * tables. */ + if (safi == SAFI_MPLS_VPN + || safi == SAFI_ENCAP + || safi == SAFI_EVPN) { + for (nrn = bgp_table_top(( + struct bgp_table + *)(rn->info)); + nrn; + nrn = bgp_route_next(nrn)) + bgp_process(bgp, nrn, + afi, safi); + } else + bgp_process(bgp, rn, afi, safi); + } + } +} + /* Force a bestpath recalculation for all prefixes. This is used * when 'bgp bestpath' commands are entered. */ @@ -1398,29 +1424,10 @@ void bgp_recalculate_all_bestpaths(struct bgp *bgp) { afi_t afi; safi_t safi; - struct bgp_node *rn, *nrn; for (afi = AFI_IP; afi < AFI_MAX; afi++) { for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { - for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; - rn = bgp_route_next(rn)) { - if (rn->info != NULL) { - /* Special handling for 2-level routing - * tables. */ - if (safi == SAFI_MPLS_VPN - || safi == SAFI_ENCAP - || safi == SAFI_EVPN) { - for (nrn = bgp_table_top(( - struct bgp_table - *)(rn->info)); - nrn; - nrn = bgp_route_next(nrn)) - bgp_process(bgp, nrn, - afi, safi); - } else - bgp_process(bgp, rn, afi, safi); - } - } + bgp_recalculate_afi_safi_bestpaths(bgp, afi, safi); } } } @@ -1500,6 +1507,25 @@ struct peer *peer_create_accept(struct bgp *bgp) return peer; } +/* + * Return true if we have a peer configured to use this afi/safi + */ +int bgp_afi_safi_peer_exists(struct bgp *bgp, afi_t afi, safi_t safi) +{ + struct listnode *node; + struct peer *peer; + + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { + if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + continue; + + if (peer->afc[afi][safi]) + return 1; + } + + return 0; +} + /* Change peer's AS number. */ void peer_as_change(struct peer *peer, as_t as, int as_specified) { @@ -1714,11 +1740,14 @@ int peer_activate(struct peer *peer, afi_t afi, safi_t safi) struct peer_group *group; struct listnode *node, *nnode; struct peer *tmp_peer; + struct bgp *bgp; /* Nothing to do if we've already activated this peer */ if (peer->afc[afi][safi]) return ret; + bgp = peer->bgp; + /* This is a peer-group so activate all of the members of the * peer-group as well */ if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { @@ -1741,6 +1770,17 @@ int peer_activate(struct peer *peer, afi_t afi, safi_t safi) ret |= non_peergroup_activate_af(peer, afi, safi); } + /* If this is the first peer to be activated for this afi/labeled-unicast + * recalc bestpaths to trigger label allocation */ + if (safi == SAFI_LABELED_UNICAST && !bgp->allocate_mpls_labels[afi][SAFI_UNICAST]) { + + if (BGP_DEBUG(zebra, ZEBRA)) + zlog_info("peer(s) are now active for labeled-unicast, allocate MPLS labels"); + + bgp->allocate_mpls_labels[afi][SAFI_UNICAST] = 1; + bgp_recalculate_afi_safi_bestpaths(bgp, afi, SAFI_UNICAST); + } + return ret; } @@ -1798,6 +1838,7 @@ int peer_deactivate(struct peer *peer, afi_t afi, safi_t safi) struct peer_group *group; struct peer *tmp_peer; struct listnode *node, *nnode; + struct bgp *bgp; /* Nothing to do if we've already de-activated this peer */ if (!peer->afc[afi][safi]) @@ -1821,6 +1862,20 @@ int peer_deactivate(struct peer *peer, afi_t afi, safi_t safi) ret |= non_peergroup_deactivate_af(peer, afi, safi); } + bgp = peer->bgp; + + /* If this is the last peer to be deactivated for this afi/labeled-unicast + * recalc bestpaths to trigger label deallocation */ + if (safi == SAFI_LABELED_UNICAST && + bgp->allocate_mpls_labels[afi][SAFI_UNICAST] && + !bgp_afi_safi_peer_exists(bgp, afi, safi)) { + + if (BGP_DEBUG(zebra, ZEBRA)) + zlog_info("peer(s) are no longer active for labeled-unicast, deallocate MPLS labels"); + + bgp->allocate_mpls_labels[afi][SAFI_UNICAST] = 0; + bgp_recalculate_afi_safi_bestpaths(bgp, afi, SAFI_UNICAST); + } return ret; } diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index f6e7b2277f..5ede9ba13d 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -333,6 +333,9 @@ struct bgp { /* BGP redistribute configuration. */ struct list *redist[AFI_MAX][ZEBRA_ROUTE_MAX]; + /* Allocate MPLS labels */ + u_char allocate_mpls_labels[AFI_MAX][SAFI_MAX]; + /* timer to re-evaluate neighbor default-originate route-maps */ struct thread *t_rmap_def_originate_eval; #define RMAP_DEFAULT_ORIGINATE_EVAL_TIMER 5 @@ -1280,6 +1283,7 @@ extern int bgp_listen_limit_unset(struct bgp *); extern int bgp_update_delay_active(struct bgp *); extern int bgp_update_delay_configured(struct bgp *); +extern int bgp_afi_safi_peer_exists(struct bgp *bgp, afi_t afi, safi_t safi); extern void peer_as_change(struct peer *, as_t, int); extern int peer_remote_as(struct bgp *, union sockunion *, const char *, as_t *, int, afi_t, safi_t); -- 2.39.5