From 255b392093b73ab69bdf3a7bc8940184d1fbac1a Mon Sep 17 00:00:00 2001 From: David Schweizer Date: Mon, 2 Nov 2020 16:30:02 +0100 Subject: [PATCH 01/17] bgpd: vtysh commands for peer/group dampening profiles Additional cli commands to add dampening profiles to peers / peer groups and functions to save dampening configurations. Signed-off-by: David Schweizer --- bgpd/bgp_vty.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 5aedd6b828..eb15d8b9ae 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -9468,6 +9468,92 @@ DEFPY(no_neighbor_path_attribute_treat_as_withdraw, return CMD_SUCCESS; } +DEFPY(neighbor_damp, + neighbor_damp_cmd, + "neighbor $neighbor dampening [(1-45)$half [(1-20000)$reuse (1-20000)$suppress (1-255)$max]]", + NEIGHBOR_STR + NEIGHBOR_ADDR_STR2 + "Enable neighbor route-flap dampening\n" + "Half-life time for the penalty\n" + "Value to start reusing a route\n" + "Value to start suppressing a route\n" + "Maximum duration to suppress a stable route\n") +{ + struct peer *peer = peer_and_group_lookup_vty(vty, neighbor); + + if (!peer) + return CMD_WARNING_CONFIG_FAILED; + if (!half) + half = DEFAULT_HALF_LIFE; + if (!reuse) { + reuse = DEFAULT_REUSE; + suppress = DEFAULT_SUPPRESS; + max = half * 4; + } + if (suppress < reuse) { + vty_out(vty, "Suppress value cannot be less than reuse value\n"); + return CMD_WARNING_CONFIG_FAILED; + } + bgp_peer_damp_enable(peer, bgp_node_afi(vty), bgp_node_safi(vty), + half * 60, reuse, suppress, max * 60); + return CMD_SUCCESS; +} + +DEFPY(no_neighbor_damp, + no_neighbor_damp_cmd, + "no neighbor $neighbor dampening [HALF [REUSE SUPPRESS MAX]]", + NO_STR + NEIGHBOR_STR + NEIGHBOR_ADDR_STR2 + "Enable neighbor route-flap dampening\n" + "Half-life time for the penalty\n" + "Value to start reusing a route\n" + "Value to start suppressing a route\n" + "Maximum duration to suppress a stable route\n") +{ + struct peer *peer = peer_and_group_lookup_vty(vty, neighbor); + + if (!peer) + return CMD_WARNING_CONFIG_FAILED; + bgp_peer_damp_disable(peer, bgp_node_afi(vty), bgp_node_safi(vty)); + return CMD_SUCCESS; +} + +DEFPY (show_ip_bgp_neighbor_damp_param, + show_ip_bgp_neighbor_damp_param_cmd, + "show [ip] bgp [ [unicast]] neighbors $neighbor dampening parameters [json]$json", + SHOW_STR + IP_STR + BGP_STR + BGP_AFI_HELP_STR + "Address Family modifier\n" + NEIGHBOR_STR + NEIGHBOR_ADDR_STR2 + "Neighbor route-flap dampening information\n" + "Display detail of configured dampening parameters\n" + JSON_STR) +{ + bool use_json = false; + int idx = 0; + afi_t afi = AFI_IP; + safi_t safi = SAFI_UNICAST; + struct peer *peer; + + if (argv_find(argv, argc, "ip", &idx)) + afi = AFI_IP; + if (argv_find(argv, argc, "ipv4", &idx)) + afi = AFI_IP; + if (argv_find(argv, argc, "ipv6", &idx)) + afi = AFI_IP6; + peer = peer_and_group_lookup_vty(vty, neighbor); + if (!peer) + return CMD_WARNING; + if (json) + use_json = true; + bgp_show_peer_dampening_parameters(vty, peer, afi, safi, use_json); + return CMD_SUCCESS; +} + static int set_ecom_list(struct vty *vty, int argc, struct cmd_token **argv, struct ecommunity **list, bool is_rt6) { @@ -19002,7 +19088,15 @@ static void bgp_config_write_family(struct vty *vty, struct bgp *bgp, afi_t afi, /* BGP flag dampening. */ if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)) - bgp_config_write_damp(vty, afi, safi); + bgp_config_write_damp(vty, bgp, afi, safi); + for (ALL_LIST_ELEMENTS_RO(bgp->group, node, group)) + if (peer_af_flag_check(group->conf, afi, safi, + PEER_FLAG_CONFIG_DAMPENING)) + bgp_config_write_peer_damp(vty, group->conf, afi, safi); + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) + if (peer_af_flag_check(peer, afi, safi, + PEER_FLAG_CONFIG_DAMPENING)) + bgp_config_write_peer_damp(vty, peer, afi, safi); for (ALL_LIST_ELEMENTS(bgp->group, node, nnode, group)) bgp_config_write_peer_af(vty, bgp, group->conf, afi, safi); @@ -21381,6 +21475,23 @@ void bgp_vty_init(void) install_element(BGP_EVPN_NODE, &neighbor_soo_cmd); install_element(BGP_EVPN_NODE, &no_neighbor_soo_cmd); + /* "neighbor dampening" commands. */ + install_element(BGP_NODE, &neighbor_damp_cmd); + install_element(BGP_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV4_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV4_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV4M_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV4M_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV4L_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV4L_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV6_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV6_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV6M_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV6M_NODE, &no_neighbor_damp_cmd); + install_element(BGP_IPV6L_NODE, &neighbor_damp_cmd); + install_element(BGP_IPV6L_NODE, &no_neighbor_damp_cmd); + install_element(VIEW_NODE, &show_ip_bgp_neighbor_damp_param_cmd); + /* address-family commands. */ install_element(BGP_NODE, &address_family_ipv4_safi_cmd); install_element(BGP_NODE, &address_family_ipv6_safi_cmd); From 22473c4014c00254557b26581e1ae5a8f39b5cc4 Mon Sep 17 00:00:00 2001 From: David Schweizer Date: Mon, 2 Nov 2020 16:30:01 +0100 Subject: [PATCH 02/17] bgpd: peer / peer group dampening profiles Changes implement dampening profiles for peers and peer groups. This is achieved by introducing the possibility to have multible existing dampening configurations with their own sets of parameters and lists of associated paths. Signed-off-by: Rafael Zalamena Signed-off-by: David Schweizer --- bgpd/bgp_damp.c | 550 ++++++++++++++++++++++++++++++++++------------ bgpd/bgp_damp.h | 51 +++-- bgpd/bgp_memory.c | 1 + bgpd/bgp_memory.h | 1 + bgpd/bgp_route.c | 50 +++-- bgpd/bgpd.c | 13 ++ bgpd/bgpd.h | 8 + 7 files changed, 495 insertions(+), 179 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 6b6387b1b5..123876ed5e 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -22,19 +22,113 @@ #include "bgpd/bgp_advertise.h" #include "bgpd/bgp_vty.h" -/* Global variable to access damping configuration */ -static struct bgp_damp_config damp[AFI_MAX][SAFI_MAX]; +static void bgp_reuselist_add(struct reuselist *list, struct bgp_damp_info *info) +{ + struct reuselist_node *new_node; -/* Utility macro to add and delete BGP dampening information to no - used list. */ -#define BGP_DAMP_LIST_ADD(N, A) BGP_PATH_INFO_ADD(N, A, no_reuse_list) -#define BGP_DAMP_LIST_DEL(N, A) BGP_PATH_INFO_DEL(N, A, no_reuse_list) + assert(info); + new_node = XCALLOC(MTYPE_BGP_DAMP_REUSELIST, sizeof(*new_node)); + new_node->info = info; + SLIST_INSERT_HEAD(list, new_node, entry); +} + +static void bgp_reuselist_del(struct reuselist *list, + struct reuselist_node **node) +{ + if ((*node) == NULL) + return; + assert(list && node && *node); + SLIST_REMOVE(list, (*node), reuselist_node, entry); + XFREE(MTYPE_BGP_DAMP_REUSELIST, (*node)); + *node = NULL; +} + +static void bgp_reuselist_switch(struct reuselist *source, + struct reuselist_node *node, + struct reuselist *target) +{ + assert(source && target && node); + SLIST_REMOVE(source, node, reuselist_node, entry); + SLIST_INSERT_HEAD(target, node, entry); +} + +static void bgp_reuselist_free(struct reuselist *list) +{ + struct reuselist_node *rn; + + assert(list); + while ((rn = SLIST_FIRST(list)) != NULL) + bgp_reuselist_del(list, &rn); +} + +static struct reuselist_node *bgp_reuselist_find(struct reuselist *list, + struct bgp_damp_info *info) +{ + struct reuselist_node *rn; + + assert(list && info); + SLIST_FOREACH (rn, list, entry) { + if (rn->info == info) + return rn; + } + return NULL; +} + +static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi) +{ + struct reuselist_node *node; + + assert(bdi && bdi->config); + if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) { + node = bgp_reuselist_find(&bdi->config->no_reuse_list, bdi); + if (node) + bgp_reuselist_del(&bdi->config->no_reuse_list, &node); + } else { + node = bgp_reuselist_find(&bdi->config->reuse_list[bdi->index], + bdi); + if (node) + bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], + &node); + } + bdi->config = NULL; +} + +static void bgp_damp_info_claim(struct bgp_damp_info *bdi, + struct bgp_damp_config *bdc) +{ + assert(bdc && bdi); + if (bdi->config == NULL) { + bdi->config = bdc; + return; + } + bgp_damp_info_unclaim(bdi); + bdi->config = bdc; + bdi->afi = bdc->afi; + bdi->safi = bdc->safi; +} + +struct bgp_damp_config *get_active_bdc_from_pi(struct bgp_path_info *pi, + afi_t afi, safi_t safi) +{ + if (!pi) + return NULL; + if (CHECK_FLAG(pi->peer->af_flags[afi][safi], + PEER_FLAG_CONFIG_DAMPENING)) + return &pi->peer->damp[afi][safi]; + if (peer_group_active(pi->peer)) + if (CHECK_FLAG(pi->peer->group->conf->af_flags[afi][safi], + PEER_FLAG_CONFIG_DAMPENING)) + return &pi->peer->group->conf->damp[afi][safi]; + if (CHECK_FLAG(pi->peer->bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)) + return &pi->peer->bgp->damp[afi][safi]; + return NULL; +} /* Calculate reuse list index by penalty value. */ static int bgp_reuse_index(int penalty, struct bgp_damp_config *bdc) { unsigned int i; - int index; + unsigned int index; /* * reuse_limit can't be zero, this is for Coverity @@ -57,27 +151,45 @@ static int bgp_reuse_index(int penalty, struct bgp_damp_config *bdc) static void bgp_reuse_list_add(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc) { - int index; - - index = bdi->index = bgp_reuse_index(bdi->penalty, bdc); - - bdi->prev = NULL; - bdi->next = bdc->reuse_list[index]; - if (bdc->reuse_list[index]) - bdc->reuse_list[index]->prev = bdi; - bdc->reuse_list[index] = bdi; + bgp_damp_info_claim(bdi, bdc); + bdi->index = bgp_reuse_index(bdi->penalty, bdc); + bgp_reuselist_add(&bdc->reuse_list[bdi->index], bdi); } /* Delete BGP dampening information from reuse list. */ static void bgp_reuse_list_delete(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc) { - if (bdi->next) - bdi->next->prev = bdi->prev; - if (bdi->prev) - bdi->prev->next = bdi->next; - else - bdc->reuse_list[bdi->index] = bdi->next; + struct reuselist *list; + struct reuselist_node *rn; + + list = &bdc->reuse_list[bdi->index]; + rn = bgp_reuselist_find(list, bdi); + bgp_damp_info_unclaim(bdi); + bgp_reuselist_del(list, &rn); +} + +static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, + struct bgp_damp_config *bdc) +{ + bgp_damp_info_claim(bdi, bdc); + bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; + bgp_reuselist_add(&bdc->no_reuse_list, bdi); +} + +static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi, + struct bgp_damp_config *bdc) +{ + struct reuselist_node *rn; + + assert(bdc && bdi); + if (bdi->config == NULL) { + bgp_damp_info_unclaim(bdi); + return; + } + bdi->config = NULL; + rn = bgp_reuselist_find(&bdc->no_reuse_list, bdi); + bgp_reuselist_del(&bdc->no_reuse_list, &rn); } /* Return decayed penalty value. */ @@ -101,9 +213,10 @@ int bgp_damp_decay(time_t tdiff, int penalty, struct bgp_damp_config *bdc) static void bgp_reuse_timer(struct event *t) { struct bgp_damp_info *bdi; - struct bgp_damp_info *next; + struct reuselist plist; + struct reuselist_node *node; + struct bgp *bgp; time_t t_now, t_diff; - struct bgp_damp_config *bdc = EVENT_ARG(t); bdc->t_reuse = NULL; @@ -112,20 +225,22 @@ static void bgp_reuse_timer(struct event *t) t_now = monotime(NULL); - /* 1. save a pointer to the current zeroth queue head and zero the - list head entry. */ - bdi = bdc->reuse_list[bdc->reuse_offset]; - bdc->reuse_list[bdc->reuse_offset] = NULL; + /* 1. save a pointer to the current queue head and zero the list head + * list head entry. */ + assert(bdc->reuse_offset < bdc->reuse_list_size); + plist = bdc->reuse_list[bdc->reuse_offset]; + node = SLIST_FIRST(&plist); + SLIST_INIT(&bdc->reuse_list[bdc->reuse_offset]); /* 2. set offset = modulo reuse-list-size ( offset + 1 ), thereby rotating the circular queue of list-heads. */ bdc->reuse_offset = (bdc->reuse_offset + 1) % bdc->reuse_list_size; + assert(bdc->reuse_offset < bdc->reuse_list_size); /* 3. if ( the saved list head pointer is non-empty ) */ - for (; bdi; bdi = next) { - struct bgp *bgp = bdi->path->peer->bgp; - - next = bdi->next; + while ((node = SLIST_FIRST(&plist)) != NULL) { + bdi = node->info; + bgp = bdi->path->peer->bgp; /* Set t-diff = t-now - t-updated. */ t_diff = t_now - bdi->t_updated; @@ -154,15 +269,26 @@ static void bgp_reuse_timer(struct event *t) bdi->safi); } - if (bdi->penalty <= bdc->reuse_limit / 2.0) - bgp_damp_info_free(bdi, 1, bdc->afi, bdc->safi); - else - BGP_DAMP_LIST_ADD(bdc, bdi); - } else + if (bdi->penalty <= bdc->reuse_limit / 2.0) { + bgp_damp_info_free(&bdi, bdc, 1, bdi->afi, + bdi->safi); + bgp_reuselist_del(&plist, &node); + } else { + node->info->index = + BGP_DAMP_NO_REUSE_LIST_INDEX; + bgp_reuselist_switch(&plist, node, + &bdc->no_reuse_list); + } + } else { /* Re-insert into another list (See RFC2439 Section * 4.8.6). */ - bgp_reuse_list_add(bdi, bdc); + bdi->index = bgp_reuse_index(bdi->penalty, bdc); + bgp_reuselist_switch(&plist, node, + &bdc->reuse_list[bdi->index]); + } } + + assert(SLIST_EMPTY(&plist)); } /* A route becomes unreachable (RFC2439 Section 4.8.2). */ @@ -172,10 +298,13 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, time_t t_now; struct bgp_damp_info *bdi = NULL; unsigned int last_penalty = 0; - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc; + + bdc = get_active_bdc_from_pi(path, afi, safi); + if (!bdc) + return BGP_DAMP_USED; t_now = monotime(NULL); - /* Processing Unreachable Messages. */ if (path->extra) bdi = path->extra->damp_info; @@ -197,12 +326,13 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, bdi->flap = 1; bdi->start_time = t_now; bdi->suppress_time = 0; - bdi->index = -1; + bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; bdi->afi = afi; bdi->safi = safi; (bgp_path_info_extra_get(path))->damp_info = bdi; - BGP_DAMP_LIST_ADD(bdc, bdi); + bgp_no_reuse_list_add(bdi, bdc); } else { + bgp_damp_info_claim(bdi, bdc); last_penalty = bdi->penalty; /* 1. Set t-diff = t-now - t-updated. */ @@ -228,7 +358,7 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, /* Remove the route from a reuse list if it is on one. */ if (CHECK_FLAG(bdi->path->flags, BGP_PATH_DAMPED)) { /* If decay rate isn't equal to 0, reinsert brn. */ - if (bdi->penalty != last_penalty && bdi->index >= 0) { + if (bdi->penalty != last_penalty) { bgp_reuse_list_delete(bdi, bdc); bgp_reuse_list_add(bdi, bdc); } @@ -240,10 +370,9 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, if (bdi->penalty >= bdc->suppress_value) { bgp_path_info_set_flag(dest, path, BGP_PATH_DAMPED); bdi->suppress_time = t_now; - BGP_DAMP_LIST_DEL(bdc, bdi); + bgp_no_reuse_list_delete(bdi, bdc); bgp_reuse_list_add(bdi, bdc); } - return BGP_DAMP_USED; } @@ -253,7 +382,10 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, time_t t_now; struct bgp_damp_info *bdi; int status; - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc; + + bdc = get_active_bdc_from_pi(path, afi, safi); + assert(bdc); if (!path->extra || !((bdi = path->extra->damp_info))) return BGP_DAMP_USED; @@ -272,7 +404,7 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, && (bdi->penalty < bdc->reuse_limit)) { bgp_path_info_unset_flag(dest, path, BGP_PATH_DAMPED); bgp_reuse_list_delete(bdi, bdc); - BGP_DAMP_LIST_ADD(bdc, bdi); + bgp_no_reuse_list_add(bdi, bdc); bdi->suppress_time = 0; status = BGP_DAMP_USED; } else @@ -280,35 +412,31 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, if (bdi->penalty > bdc->reuse_limit / 2.0) bdi->t_updated = t_now; - else - bgp_damp_info_free(bdi, 0, afi, safi); + else { + bgp_damp_info_unclaim(bdi); + bgp_damp_info_free(&bdi, bdc, 0, afi, safi); + } return status; } -void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw, afi_t afi, - safi_t safi) +void bgp_damp_info_free(struct bgp_damp_info **bdi, struct bgp_damp_config *bdc, + int withdraw, afi_t afi, safi_t safi) { - struct bgp_path_info *path; - struct bgp_damp_config *bdc = &damp[afi][safi]; + assert(bdc && bdi && *bdi); - if (!bdi) + if ((*bdi)->path == NULL) { + XFREE(MTYPE_BGP_DAMP_INFO, (*bdi)); return; + } - path = bdi->path; - path->extra->damp_info = NULL; - - if (CHECK_FLAG(path->flags, BGP_PATH_DAMPED)) - bgp_reuse_list_delete(bdi, bdc); - else - BGP_DAMP_LIST_DEL(bdc, bdi); - - bgp_path_info_unset_flag(bdi->dest, path, + (*bdi)->path->extra->damp_info = NULL; + bgp_path_info_unset_flag((*bdi)->dest, (*bdi)->path, BGP_PATH_HISTORY | BGP_PATH_DAMPED); - if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { - bgp_path_info_delete(bdi->dest, path); - bgp_process(path->peer->bgp, bdi->dest, path, afi, safi); + if ((*bdi)->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { + bgp_path_info_delete((*bdi)->dest, (*bdi)->path); + bgp_process((*bdi)->path->peer->bgp, (*bdi)->dest, (*bdi)->path, afi, safi); } XFREE(MTYPE_BGP_DAMP_INFO, bdi); @@ -355,8 +483,7 @@ static void bgp_damp_parameter_set(time_t hlife, unsigned int reuse, bdc->reuse_list = XCALLOC(MTYPE_BGP_DAMP_ARRAY, - bdc->reuse_list_size * sizeof(struct bgp_reuse_node *)); - + bdc->reuse_list_size * sizeof(struct reuselist)); /* Reuse-array computations */ bdc->reuse_index = XCALLOC(MTYPE_BGP_DAMP_ARRAY, sizeof(int) * bdc->reuse_index_size); @@ -383,7 +510,7 @@ static void bgp_damp_parameter_set(time_t hlife, unsigned int reuse, int bgp_damp_enable(struct bgp *bgp, afi_t afi, safi_t safi, time_t half, unsigned int reuse, unsigned int suppress, time_t max) { - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc = &bgp->damp[afi][safi]; if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)) { if (bdc->half_life == half && bdc->reuse_limit == reuse @@ -395,6 +522,8 @@ int bgp_damp_enable(struct bgp *bgp, afi_t afi, safi_t safi, time_t half, SET_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING); bgp_damp_parameter_set(half, reuse, suppress, max, bdc); + bdc->afi = afi; + bdc->safi = safi; /* Register reuse timer. */ event_add_timer(bm->master, bgp_reuse_timer, bdc, DELTA_REUSE, @@ -403,8 +532,30 @@ int bgp_damp_enable(struct bgp *bgp, afi_t afi, safi_t safi, time_t half, return 0; } -static void bgp_damp_config_clean(struct bgp_damp_config *bdc) +/* Clean all the bgp_damp_info stored in reuse_list and no_reuse_list. */ +void bgp_damp_info_clean(struct bgp_damp_config *bdc, afi_t afi, safi_t safi) { + struct bgp_damp_info *bdi; + struct reuselist_node *rn; + struct reuselist *list; + unsigned int i; + + bdc->reuse_offset = 0; + for (i = 0; i < bdc->reuse_list_size; ++i) { + list = &bdc->reuse_list[i]; + while ((rn = SLIST_FIRST(list)) != NULL) { + bdi = rn->info; + bgp_reuselist_del(list, &rn); + bgp_damp_info_free(&bdi, bdc, 1, afi, safi); + } + } + + while ((rn = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { + bdi = rn->info; + bgp_reuselist_del(&bdc->no_reuse_list, &rn); + bgp_damp_info_free(&bdi, bdc, 1, afi, safi); + } + /* Free decay array */ XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->decay_array); bdc->decay_array_size = 0; @@ -414,40 +565,28 @@ static void bgp_damp_config_clean(struct bgp_damp_config *bdc) bdc->reuse_index_size = 0; /* Free reuse list array. */ + for (i = 0; i < bdc->reuse_list_size; ++i) + bgp_reuselist_free(&bdc->reuse_list[i]); + XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->reuse_list); bdc->reuse_list_size = 0; + + EVENT_OFF(bdc->t_reuse); } -/* Clean all the bgp_damp_info stored in reuse_list. */ -void bgp_damp_info_clean(afi_t afi, safi_t safi) -{ - unsigned int i; - struct bgp_damp_info *bdi, *next; - struct bgp_damp_config *bdc = &damp[afi][safi]; - - bdc->reuse_offset = 0; - - for (i = 0; i < bdc->reuse_list_size; i++) { - if (!bdc->reuse_list[i]) - continue; - - for (bdi = bdc->reuse_list[i]; bdi; bdi = next) { - next = bdi->next; - bgp_damp_info_free(bdi, 1, afi, safi); - } - bdc->reuse_list[i] = NULL; - } - - for (bdi = bdc->no_reuse_list; bdi; bdi = next) { - next = bdi->next; - bgp_damp_info_free(bdi, 1, afi, safi); - } - bdc->no_reuse_list = NULL; -} - +/* Disable route flap dampening for a bgp instance. + * + * Please note that this function also gets used to free memory when deleting a + * bgp instance. + */ int bgp_damp_disable(struct bgp *bgp, afi_t afi, safi_t safi) { - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc; + + bdc = &bgp->damp[afi][safi]; + if (!bdc) + return 0; + /* If it wasn't enabled, there's nothing to do. */ if (!CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)) return 0; @@ -456,54 +595,51 @@ int bgp_damp_disable(struct bgp *bgp, afi_t afi, safi_t safi) EVENT_OFF(bdc->t_reuse); /* Clean BGP dampening information. */ - bgp_damp_info_clean(afi, safi); - - /* Clear configuration */ - bgp_damp_config_clean(bdc); + bgp_damp_info_clean(bdc, afi, safi); UNSET_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING); + return 0; } -void bgp_config_write_damp(struct vty *vty, afi_t afi, safi_t safi) +void bgp_config_write_damp(struct vty *vty, struct bgp *bgp, afi_t afi, + safi_t safi) { - if (damp[afi][safi].half_life == DEFAULT_HALF_LIFE * 60 - && damp[afi][safi].reuse_limit == DEFAULT_REUSE - && damp[afi][safi].suppress_value == DEFAULT_SUPPRESS - && damp[afi][safi].max_suppress_time - == damp[afi][safi].half_life * 4) + struct bgp_damp_config *bdc; + + bdc = &bgp->damp[afi][safi]; + if (bdc->half_life == DEFAULT_HALF_LIFE * 60 && + bdc->reuse_limit == DEFAULT_REUSE && + bdc->suppress_value == DEFAULT_SUPPRESS && + bdc->max_suppress_time == bdc->half_life * 4) vty_out(vty, " bgp dampening\n"); - else if (damp[afi][safi].half_life != DEFAULT_HALF_LIFE * 60 - && damp[afi][safi].reuse_limit == DEFAULT_REUSE - && damp[afi][safi].suppress_value == DEFAULT_SUPPRESS - && damp[afi][safi].max_suppress_time - == damp[afi][safi].half_life * 4) - vty_out(vty, " bgp dampening %lld\n", - damp[afi][safi].half_life / 60LL); + else if (bdc->half_life != DEFAULT_HALF_LIFE * 60 && + bdc->reuse_limit == DEFAULT_REUSE && + bdc->suppress_value == DEFAULT_SUPPRESS && + bdc->max_suppress_time == bdc->half_life * 4) + vty_out(vty, " bgp dampening %lld\n", bdc->half_life / 60LL); else vty_out(vty, " bgp dampening %lld %d %d %lld\n", - damp[afi][safi].half_life / 60LL, - damp[afi][safi].reuse_limit, - damp[afi][safi].suppress_value, - damp[afi][safi].max_suppress_time / 60LL); + bdc->half_life / 60LL, bdc->reuse_limit, + bdc->suppress_value, bdc->max_suppress_time / 60LL); } -static const char *bgp_get_reuse_time(unsigned int penalty, char *buf, - size_t len, afi_t afi, safi_t safi, - bool use_json, json_object *json) +static const char *bgp_get_reuse_time(struct bgp_damp_config *bdc, + unsigned int penalty, char *buf, + size_t len, bool use_json, + json_object *json) { time_t reuse_time = 0; struct tm tm; int time_store = 0; - if (penalty > damp[afi][safi].reuse_limit) { - reuse_time = (int)(DELTA_T - * ((log((double)damp[afi][safi].reuse_limit - / penalty)) - / (log(damp[afi][safi].decay_array[1])))); + if (penalty > bdc->reuse_limit) { + reuse_time = (int)(DELTA_T * + ((log((double)bdc->reuse_limit / penalty)) / + (log(bdc->decay_array[1])))); - if (reuse_time > damp[afi][safi].max_suppress_time) - reuse_time = damp[afi][safi].max_suppress_time; + if (reuse_time > bdc->max_suppress_time) + reuse_time = bdc->max_suppress_time; gmtime_r(&reuse_time, &tm); } else @@ -555,14 +691,15 @@ static const char *bgp_get_reuse_time(unsigned int penalty, char *buf, return buf; } -void bgp_damp_info_vty(struct vty *vty, struct bgp_path_info *path, afi_t afi, - safi_t safi, json_object *json_path) +void bgp_damp_info_vty(struct vty *vty, struct bgp *bgp, + struct bgp_path_info *path, afi_t afi, safi_t safi, + json_object *json_path) { struct bgp_damp_info *bdi; time_t t_now, t_diff; char timebuf[BGP_UPTIME_LEN] = {}; int penalty; - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc = &bgp->damp[afi][safi]; if (!path->extra) return; @@ -590,8 +727,8 @@ void bgp_damp_info_vty(struct vty *vty, struct bgp_path_info *path, afi_t afi, if (CHECK_FLAG(path->flags, BGP_PATH_DAMPED) && !CHECK_FLAG(path->flags, BGP_PATH_HISTORY)) - bgp_get_reuse_time(penalty, timebuf, BGP_UPTIME_LEN, - afi, safi, 1, json_path); + bgp_get_reuse_time(bdc, penalty, timebuf, + BGP_UPTIME_LEN, 1, json_path); } else { vty_out(vty, " Dampinfo: penalty %d, flapped %d times in %s", @@ -602,14 +739,15 @@ void bgp_damp_info_vty(struct vty *vty, struct bgp_path_info *path, afi_t afi, if (CHECK_FLAG(path->flags, BGP_PATH_DAMPED) && !CHECK_FLAG(path->flags, BGP_PATH_HISTORY)) vty_out(vty, ", reuse in %s", - bgp_get_reuse_time(penalty, timebuf, - BGP_UPTIME_LEN, afi, safi, 0, + bgp_get_reuse_time(bdc, penalty, timebuf, + BGP_UPTIME_LEN, 0, json_path)); vty_out(vty, "\n"); } } + const char *bgp_damp_reuse_time_vty(struct vty *vty, struct bgp_path_info *path, char *timebuf, size_t len, afi_t afi, safi_t safi, bool use_json, @@ -618,7 +756,11 @@ const char *bgp_damp_reuse_time_vty(struct vty *vty, struct bgp_path_info *path, struct bgp_damp_info *bdi; time_t t_now, t_diff; int penalty; - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc; + + bdc = get_active_bdc_from_pi(path, afi, safi); + if (!bdc) + return NULL; if (!path->extra) return NULL; @@ -638,15 +780,15 @@ const char *bgp_damp_reuse_time_vty(struct vty *vty, struct bgp_path_info *path, t_diff = t_now - bdi->t_updated; penalty = bgp_damp_decay(t_diff, bdi->penalty, bdc); - return bgp_get_reuse_time(penalty, timebuf, len, afi, safi, use_json, - json); + return bgp_get_reuse_time(bdc, penalty, timebuf, len, use_json, json); } + static int bgp_print_dampening_parameters(struct bgp *bgp, struct vty *vty, afi_t afi, safi_t safi, bool use_json) { if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)) { - struct bgp_damp_config *bdc = &damp[afi][safi]; + struct bgp_damp_config *bdc = &bgp->damp[afi][safi]; if (use_json) { json_object *json = json_object_new_object(); @@ -689,7 +831,6 @@ int bgp_show_dampening_parameters(struct vty *vty, afi_t afi, safi_t safi, bool use_json = CHECK_FLAG(show_flags, BGP_SHOW_OPT_JSON); bgp = bgp_get_default(); - if (bgp == NULL) { vty_out(vty, "No BGP process is configured\n"); return CMD_WARNING; @@ -731,3 +872,130 @@ int bgp_show_dampening_parameters(struct vty *vty, afi_t afi, safi_t safi, } return CMD_SUCCESS; } + +void bgp_peer_damp_enable(struct peer *peer, afi_t afi, safi_t safi, time_t half, + unsigned int reuse, unsigned int suppress, time_t max) +{ + struct bgp_damp_config *bdc; + + if (!peer) + return; + bdc = &peer->damp[afi][safi]; + if (peer_af_flag_check(peer, afi, safi, PEER_FLAG_CONFIG_DAMPENING)) { + if (bdc->half_life == half && bdc->reuse_limit == reuse && + bdc->suppress_value == suppress && + bdc->max_suppress_time == max) + return; + bgp_peer_damp_disable(peer, afi, safi); + } + SET_FLAG(peer->af_flags[afi][safi], PEER_FLAG_CONFIG_DAMPENING); + bgp_damp_parameter_set(half, reuse, suppress, max, bdc); + bdc->afi = afi; + bdc->safi = safi; + event_add_timer(bm->master, bgp_reuse_timer, bdc, DELTA_REUSE, + &bdc->t_reuse); +} + +/* Disable route flap dampening for a peer. + * + * Please note that this function also gets used to free memory when deleting a + * peer or peer group. + */ +void bgp_peer_damp_disable(struct peer *peer, afi_t afi, safi_t safi) +{ + struct bgp_damp_config *bdc; + + if (!peer_af_flag_check(peer, afi, safi, PEER_FLAG_CONFIG_DAMPENING)) + return; + bdc = &peer->damp[afi][safi]; + if (!bdc) + return; + bgp_damp_info_clean(bdc, afi, safi); + UNSET_FLAG(peer->af_flags[afi][safi], PEER_FLAG_CONFIG_DAMPENING); +} + +void bgp_config_write_peer_damp(struct vty *vty, struct peer *peer, afi_t afi, + safi_t safi) +{ + struct bgp_damp_config *bdc; + + bdc = &peer->damp[afi][safi]; + if (bdc->half_life == DEFAULT_HALF_LIFE * 60 && + bdc->reuse_limit == DEFAULT_REUSE && + bdc->suppress_value == DEFAULT_SUPPRESS && + bdc->max_suppress_time == bdc->half_life * 4) + vty_out(vty, " neighbor %s dampening\n", peer->host); + else if (bdc->half_life != DEFAULT_HALF_LIFE * 60 && + bdc->reuse_limit == DEFAULT_REUSE && + bdc->suppress_value == DEFAULT_SUPPRESS && + bdc->max_suppress_time == bdc->half_life * 4) + vty_out(vty, " neighbor %s dampening %lld\n", peer->host, + bdc->half_life / 60LL); + else + vty_out(vty, " neighbor %s dampening %lld %d %d %lld\n", + peer->host, bdc->half_life / 60LL, bdc->reuse_limit, + bdc->suppress_value, bdc->max_suppress_time / 60LL); +} + +static void bgp_print_peer_dampening_parameters(struct vty *vty, + struct peer *peer, afi_t afi, + safi_t safi, bool use_json, + json_object *json) +{ + struct bgp_damp_config *bdc; + + if (!peer) + return; + if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_CONFIG_DAMPENING)) { + bdc = &peer->damp[afi][safi]; + if (!bdc) + return; + if (use_json) { + json_object_int_add(json, "halfLifeSecs", + bdc->half_life); + json_object_int_add(json, "reusePenalty", + bdc->reuse_limit); + json_object_int_add(json, "suppressPenalty", + bdc->suppress_value); + json_object_int_add(json, "maxSuppressTimeSecs", + bdc->max_suppress_time); + json_object_int_add(json, "maxSuppressPenalty", + bdc->ceiling); + } else { + vty_out(vty, "Half-life time: %lld min\n", + (long long)bdc->half_life / 60); + vty_out(vty, "Reuse penalty: %d\n", bdc->reuse_limit); + vty_out(vty, "Suppress penalty: %d\n", + bdc->suppress_value); + vty_out(vty, "Max suppress time: %lld min\n", + (long long)bdc->max_suppress_time / 60); + vty_out(vty, "Max suppress penalty: %u\n", bdc->ceiling); + vty_out(vty, "\n"); + } + } else if (!use_json) + vty_out(vty, "neighbor dampening not enabled for %s\n", + get_afi_safi_str(afi, safi, false)); +} + +void bgp_show_peer_dampening_parameters(struct vty *vty, struct peer *peer, + afi_t afi, safi_t safi, bool use_json) +{ + json_object *json; + + if (use_json) { + json = json_object_new_object(); + json_object_string_add(json, "addressFamily", + get_afi_safi_str(afi, safi, false)); + bgp_print_peer_dampening_parameters(vty, peer, afi, safi, true, + json); + vty_out(vty, "%s\n", + json_object_to_json_string_ext(json, + JSON_C_TO_STRING_PRETTY)); + json_object_free(json); + } else { + vty_out(vty, "\nFor address family: %s\n", + get_afi_safi_str(afi, safi, false)); + bgp_print_peer_dampening_parameters(vty, peer, afi, safi, false, + NULL); + } +} diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index 6033c34bfd..0e99d21d62 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -10,11 +10,6 @@ /* Structure maintained on a per-route basis. */ struct bgp_damp_info { - /* Doubly linked list. This information must be linked to - reuse_list or no_reuse_list. */ - struct bgp_damp_info *next; - struct bgp_damp_info *prev; - /* Figure-of-merit. */ unsigned int penalty; @@ -30,6 +25,9 @@ struct bgp_damp_info { /* Time of route start to be suppressed. */ time_t suppress_time; + /* Back reference to associated dampening configuration. */ + struct bgp_damp_config *config; + /* Back reference to bgp_path_info. */ struct bgp_path_info *path; @@ -38,6 +36,8 @@ struct bgp_damp_info { /* Current index in the reuse_list. */ int index; +#define BGP_DAMP_NO_REUSE_LIST_INDEX \ + (-1) /* index for elements on no_reuse_list */ /* Last time message type. */ uint8_t lastrecord; @@ -48,6 +48,13 @@ struct bgp_damp_info { safi_t safi; }; +struct reuselist_node { + SLIST_ENTRY(reuselist_node) entry; + struct bgp_damp_info *info; +}; + +SLIST_HEAD(reuselist, reuselist_node); + /* Specified parameter set configuration. */ struct bgp_damp_config { /* Value over which routes suppressed. */ @@ -84,12 +91,12 @@ struct bgp_damp_config { int *reuse_index; /* Reuse list array per-set based. */ - struct bgp_damp_info **reuse_list; - int reuse_offset; + struct reuselist *reuse_list; + unsigned int reuse_offset; safi_t safi; /* All dampening information which is not on reuse list. */ - struct bgp_damp_info *no_reuse_list; + struct reuselist no_reuse_list; /* Reuse timer thread per-set base. */ struct event *t_reuse; @@ -116,6 +123,8 @@ struct bgp_damp_config { #define REUSE_LIST_SIZE 256 #define REUSE_ARRAY_SIZE 1024 +extern struct bgp_damp_config *get_active_bdc_from_pi(struct bgp_path_info *pi, + afi_t afi, safi_t safi); extern int bgp_damp_enable(struct bgp *bgp, afi_t afi, safi_t safi, time_t half, unsigned int reuse, unsigned int suppress, time_t max); @@ -124,14 +133,19 @@ extern int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t safi, int attr_change); extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t saff); -extern void bgp_damp_info_free(struct bgp_damp_info *path, int withdraw, +extern void bgp_damp_info_free(struct bgp_damp_info **path, + struct bgp_damp_config *bdc, int withdraw, afi_t afi, safi_t safi); -extern void bgp_damp_info_clean(afi_t afi, safi_t safi); +extern void bgp_damp_info_clean(struct bgp_damp_config *bdc, afi_t afi, + safi_t safi); +extern void bgp_damp_config_clean(struct bgp_damp_config *bdc); extern int bgp_damp_decay(time_t tdiff, int penalty, - struct bgp_damp_config *damp); -extern void bgp_config_write_damp(struct vty *vty, afi_t afi, safi_t safi); -extern void bgp_damp_info_vty(struct vty *vty, struct bgp_path_info *path, - afi_t afi, safi_t safi, json_object *json_path); + struct bgp_damp_config *bdc); +extern void bgp_config_write_damp(struct vty *vty, struct bgp *bgp, afi_t afi, + safi_t safi); +extern void bgp_damp_info_vty(struct vty *vty, struct bgp *bgp, + struct bgp_path_info *path, afi_t afi, + safi_t safi, json_object *json_path); extern const char *bgp_damp_reuse_time_vty(struct vty *vty, struct bgp_path_info *path, char *timebuf, size_t len, afi_t afi, @@ -139,5 +153,14 @@ extern const char *bgp_damp_reuse_time_vty(struct vty *vty, json_object *json); extern int bgp_show_dampening_parameters(struct vty *vty, afi_t afi, safi_t safi, uint16_t show_flags); +extern void bgp_peer_damp_enable(struct peer *peer, afi_t afi, safi_t safi, + time_t half, unsigned int reuse, + unsigned int suppress, time_t max); +extern void bgp_peer_damp_disable(struct peer *peer, afi_t afi, safi_t safi); +extern void bgp_config_write_peer_damp(struct vty *vty, struct peer *peer, + afi_t afi, safi_t safi); +extern void bgp_show_peer_dampening_parameters(struct vty *vty, + struct peer *peer, afi_t afi, + safi_t safi, bool use_json); #endif /* _QUAGGA_BGP_DAMP_H */ diff --git a/bgpd/bgp_memory.c b/bgpd/bgp_memory.c index 0764f1ed18..53c03d8102 100644 --- a/bgpd/bgp_memory.c +++ b/bgpd/bgp_memory.c @@ -91,6 +91,7 @@ DEFINE_MTYPE(BGPD, PEER_UPDATE_SOURCE, "BGP peer update interface"); DEFINE_MTYPE(BGPD, PEER_CONF_IF, "BGP peer config interface"); DEFINE_MTYPE(BGPD, BGP_DAMP_INFO, "Dampening info"); DEFINE_MTYPE(BGPD, BGP_DAMP_ARRAY, "BGP Dampening array"); +DEFINE_MTYPE(BGPD, BGP_DAMP_REUSELIST, "BGP Dampening reuse list"); DEFINE_MTYPE(BGPD, BGP_REGEXP, "BGP regexp"); DEFINE_MTYPE(BGPD, BGP_AGGREGATE, "BGP aggregate"); DEFINE_MTYPE(BGPD, BGP_ADDR, "BGP own address"); diff --git a/bgpd/bgp_memory.h b/bgpd/bgp_memory.h index 29584cd780..865c5880db 100644 --- a/bgpd/bgp_memory.h +++ b/bgpd/bgp_memory.h @@ -87,6 +87,7 @@ DECLARE_MTYPE(PEER_UPDATE_SOURCE); DECLARE_MTYPE(PEER_CONF_IF); DECLARE_MTYPE(BGP_DAMP_INFO); DECLARE_MTYPE(BGP_DAMP_ARRAY); +DECLARE_MTYPE(BGP_DAMP_REUSELIST); DECLARE_MTYPE(BGP_REGEXP); DECLARE_MTYPE(BGP_AGGREGATE); DECLARE_MTYPE(BGP_ADDR); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5f6ef5ae07..2e7faa57c2 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -249,9 +249,6 @@ void bgp_path_info_extra_free(struct bgp_path_info_extra **extra) return; e = *extra; - if (e->damp_info) - bgp_damp_info_free(e->damp_info, 0, e->damp_info->afi, - e->damp_info->safi); e->damp_info = NULL; if (e->vrfleak && e->vrfleak->parent) { @@ -4279,14 +4276,16 @@ static void bgp_rib_withdraw(struct bgp_dest *dest, struct bgp_path_info *pi, /* apply dampening, if result is suppressed, we'll be retaining * the bgp_path_info in the RIB for historical reference. */ - if (CHECK_FLAG(peer->bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING) - && peer->sort == BGP_PEER_EBGP) - if ((bgp_damp_withdraw(pi, dest, afi, safi, 0)) - == BGP_DAMP_SUPPRESSED) { - bgp_aggregate_decrement(peer->bgp, p, pi, afi, - safi); - return; + if (peer->sort == BGP_PEER_EBGP) { + if (get_active_bdc_from_pi(pi, afi, safi)) { + if (bgp_damp_withdraw(pi, dest, afi, safi, 0) == + BGP_DAMP_SUPPRESSED) { + bgp_aggregate_decrement(peer->bgp, p, pi, afi, + safi); + return; + } } + } #ifdef ENABLE_BGP_VNC if (safi == SAFI_MPLS_VPN) { @@ -4855,10 +4854,9 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, bgp_labels_same((const mpls_label_t *)pi->extra->label, pi->extra->num_labels, label, num_labels)))) { - if (CHECK_FLAG(bgp->af_flags[afi][safi], - BGP_CONFIG_DAMPENING) - && peer->sort == BGP_PEER_EBGP - && CHECK_FLAG(pi->flags, BGP_PATH_HISTORY)) { + if (get_active_bdc_from_pi(pi, afi, safi) && + peer->sort == BGP_PEER_EBGP && + CHECK_FLAG(pi->flags, BGP_PATH_HISTORY)) { if (bgp_debug_update(peer, p, NULL, 1)) { bgp_debug_rdpfxpath2str( afi, safi, prd, p, label, @@ -4959,11 +4957,11 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, bgp_path_info_set_flag(dest, pi, BGP_PATH_ATTR_CHANGED); /* Update bgp route dampening information. */ - if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING) - && peer->sort == BGP_PEER_EBGP) { + if (get_active_bdc_from_pi(pi, afi, safi) && + peer->sort == BGP_PEER_EBGP) { /* This is implicit withdraw so we should update - dampening - information. */ + * dampening information. + */ if (!CHECK_FLAG(pi->flags, BGP_PATH_HISTORY)) bgp_damp_withdraw(pi, dest, afi, safi, 1); } @@ -5074,8 +5072,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, #endif /* Update bgp route dampening information. */ - if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING) - && peer->sort == BGP_PEER_EBGP) { + if (get_active_bdc_from_pi(pi, afi, safi) && + peer->sort == BGP_PEER_EBGP) { /* Now we do normal update dampening. */ ret = bgp_damp_update(pi, dest, afi, safi); if (ret == BGP_DAMP_SUPPRESSED) { @@ -11275,7 +11273,7 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, } if (path->extra && path->extra->damp_info) - bgp_damp_info_vty(vty, path, afi, safi, json_path); + bgp_damp_info_vty(vty, bgp, path, afi, safi, json_path); /* Remote Label */ if (path->extra && bgp_is_valid_label(&path->extra->label[0]) @@ -15828,7 +15826,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, if (pi->extra && pi->extra->damp_info) { pi_temp = pi->next; bgp_damp_info_free( - pi->extra->damp_info, + &pi->extra->damp_info, + &bgp->damp[afi][safi], 1, afi, safi); pi = pi_temp; } else @@ -15850,7 +15849,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, if (pi->extra && pi->extra->damp_info) { pi_temp = pi->next; bgp_damp_info_free( - pi->extra->damp_info, + &pi->extra->damp_info, + &bgp->damp[afi][safi], 1, afi, safi); pi = pi_temp; } else @@ -15873,7 +15873,9 @@ DEFUN (clear_ip_bgp_dampening, BGP_STR "Clear route flap dampening information\n") { - bgp_damp_info_clean(AFI_IP, SAFI_UNICAST); + VTY_DECLVAR_CONTEXT(bgp, bgp); + bgp_damp_info_clean(&bgp->damp[AFI_IP][SAFI_UNICAST], AFI_IP, + SAFI_UNICAST); return CMD_SUCCESS; } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index ff17dc9f5e..e519f26676 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2685,6 +2685,14 @@ int peer_delete(struct peer *peer) if (peer->bfd_config) bgp_peer_remove_bfd_config(peer); + /* Delete peer route flap dampening configuration. This needs to happen + * before removing the peer from peer groups. + */ + FOREACH_AFI_SAFI (afi, safi) + if (peer_af_flag_check(peer, afi, safi, + PEER_FLAG_CONFIG_DAMPENING)) + bgp_peer_damp_disable(peer, afi, safi); + /* If this peer belongs to peer group, clear up the relationship. */ if (peer->group) { @@ -3937,6 +3945,11 @@ int bgp_delete(struct bgp *bgp) EVENT_OFF(gr_info->t_route_select); } + /* Delete route flap dampening configuration */ + FOREACH_AFI_SAFI (afi, safi) { + bgp_damp_disable(bgp, afi, safi); + } + if (BGP_DEBUG(zebra, ZEBRA)) { if (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT) zlog_debug("Deleting Default VRF"); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index f577a6e5f3..ad749ba676 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -33,6 +33,7 @@ PREDECL_LIST(zebra_announce); #include "bgp_addpath_types.h" #include "bgp_nexthop.h" #include "bgp_io.h" +#include "bgp_damp.h" #include "lib/bfd.h" @@ -834,6 +835,9 @@ struct bgp { enum asnotation_mode asnotation; + /* BGP route flap dampening configuration */ + struct bgp_damp_config damp[AFI_MAX][SAFI_MAX]; + QOBJ_FIELDS; }; DECLARE_QOBJ_TYPE(bgp); @@ -1508,6 +1512,9 @@ struct peer { /* Last update packet sent time */ time_t pkt_stime[AFI_MAX][SAFI_MAX]; + /* Peer / peer group route flap dampening configuration */ + struct bgp_damp_config damp[AFI_MAX][SAFI_MAX]; + /* Peer Per AF flags */ /* * Please consult the comments for *flags_override*, *flags_invert* and @@ -1549,6 +1556,7 @@ struct peer { #define PEER_FLAG_SOO (1ULL << 28) #define PEER_FLAG_SEND_EXT_COMMUNITY_RPKI (1ULL << 29) #define PEER_FLAG_ADDPATH_RX_PATHS_LIMIT (1ULL << 30) +#define PEER_FLAG_CONFIG_DAMPENING (1U << 31) #define PEER_FLAG_ACCEPT_OWN (1ULL << 63) enum bgp_addpath_strat addpath_type[AFI_MAX][SAFI_MAX]; From b29ef1082daa3e03959c530d848881ae038dafe4 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 23 Apr 2021 13:45:44 -0400 Subject: [PATCH 03/17] bgpd: Do not output peer doppleganger dampened output When we are cycling through all peers and looking for dampening data to dump, do not consider non-configed peers( dopplegangers ). Signed-off-by: Donald Sharp --- bgpd/bgp_vty.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index eb15d8b9ae..cb94eefeba 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -19094,7 +19094,8 @@ static void bgp_config_write_family(struct vty *vty, struct bgp *bgp, afi_t afi, PEER_FLAG_CONFIG_DAMPENING)) bgp_config_write_peer_damp(vty, group->conf, afi, safi); for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) - if (peer_af_flag_check(peer, afi, safi, + if (CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE) && + peer_af_flag_check(peer, afi, safi, PEER_FLAG_CONFIG_DAMPENING)) bgp_config_write_peer_damp(vty, peer, afi, safi); From debe0f528ceb11b2bb2bc4a9fe080cecb6f23554 Mon Sep 17 00:00:00 2001 From: sudhanshukumar22 Date: Mon, 2 Nov 2020 22:36:31 -0800 Subject: [PATCH 04/17] bgpd: clear ip bgp dampening was not triggering the route calculation for the prefix Description: clear ip bgp dampening was not triggering the route calculation for the prefix, Due to this prefix are not install in RIB(Zebra) and not adv to neighbor Problem Description/Summary : clear ip bgp dampening was not triggering the route calculation for the prefix, Due to this prefix are not install in RIB(Zebra) and not adv to neighbor Fix: When clear ip bgp dampening, route are put for route-calculation as that it is install in the Zebra and adv to neighbor. Signed-off-by: sudhanshukumar22 --- bgpd/bgp_damp.c | 14 +++++++++++--- bgpd/bgp_damp.h | 4 ++-- bgpd/bgp_route.c | 17 ++++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 123876ed5e..c9aa4fff25 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -533,7 +533,8 @@ int bgp_damp_enable(struct bgp *bgp, afi_t afi, safi_t safi, time_t half, } /* Clean all the bgp_damp_info stored in reuse_list and no_reuse_list. */ -void bgp_damp_info_clean(struct bgp_damp_config *bdc, afi_t afi, safi_t safi) +void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, + afi_t afi, safi_t safi) { struct bgp_damp_info *bdi; struct reuselist_node *rn; @@ -545,6 +546,13 @@ void bgp_damp_info_clean(struct bgp_damp_config *bdc, afi_t afi, safi_t safi) list = &bdc->reuse_list[i]; while ((rn = SLIST_FIRST(list)) != NULL) { bdi = rn->info; + if (bdi->lastrecord == BGP_RECORD_UPDATE) { + bgp_aggregate_increment(bgp, &bdi->dest->p, + bdi->path, bdi->afi, + bdi->safi); + bgp_process(bgp, bdi->dest, bdi->afi, + bdi->safi); + } bgp_reuselist_del(list, &rn); bgp_damp_info_free(&bdi, bdc, 1, afi, safi); } @@ -595,7 +603,7 @@ int bgp_damp_disable(struct bgp *bgp, afi_t afi, safi_t safi) EVENT_OFF(bdc->t_reuse); /* Clean BGP dampening information. */ - bgp_damp_info_clean(bdc, afi, safi); + bgp_damp_info_clean(bgp, bdc, afi, safi); UNSET_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING); @@ -910,7 +918,7 @@ void bgp_peer_damp_disable(struct peer *peer, afi_t afi, safi_t safi) bdc = &peer->damp[afi][safi]; if (!bdc) return; - bgp_damp_info_clean(bdc, afi, safi); + bgp_damp_info_clean(peer->bgp, bdc, afi, safi); UNSET_FLAG(peer->af_flags[afi][safi], PEER_FLAG_CONFIG_DAMPENING); } diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index 0e99d21d62..ae057a01f7 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -136,8 +136,8 @@ extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, extern void bgp_damp_info_free(struct bgp_damp_info **path, struct bgp_damp_config *bdc, int withdraw, afi_t afi, safi_t safi); -extern void bgp_damp_info_clean(struct bgp_damp_config *bdc, afi_t afi, - safi_t safi); +extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, + afi_t afi, safi_t safi); extern void bgp_damp_config_clean(struct bgp_damp_config *bdc); extern int bgp_damp_decay(time_t tdiff, int penalty, struct bgp_damp_config *bdc); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 2e7faa57c2..81a99604ef 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -15848,6 +15848,21 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, while (pi) { if (pi->extra && pi->extra->damp_info) { pi_temp = pi->next; + struct bgp_damp_info *bdi = + pi->extra->damp_info; + if (bdi->lastrecord + == BGP_RECORD_UPDATE) { + bgp_aggregate_increment( + bgp, + &bdi->dest->p, + bdi->path, + bdi->afi, + bdi->safi); + bgp_process(bgp, + bdi->dest, + bdi->afi, + bdi->safi); + } bgp_damp_info_free( &pi->extra->damp_info, &bgp->damp[afi][safi], @@ -15874,7 +15889,7 @@ DEFUN (clear_ip_bgp_dampening, "Clear route flap dampening information\n") { VTY_DECLVAR_CONTEXT(bgp, bgp); - bgp_damp_info_clean(&bgp->damp[AFI_IP][SAFI_UNICAST], AFI_IP, + bgp_damp_info_clean(bgp, &bgp->damp[AFI_IP][SAFI_UNICAST], AFI_IP, SAFI_UNICAST); return CMD_SUCCESS; } From 6b3486be116e68bcb386869198fc8d13cebfd8eb Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 11 Jun 2021 18:09:05 +0300 Subject: [PATCH 05/17] bgpd: Remove useless reuselist_node assignment before while loop Seems really not necessary pointing to initial value before while loop, where it's assigned anyway. Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index c9aa4fff25..40fe5b8b59 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -229,7 +229,6 @@ static void bgp_reuse_timer(struct event *t) * list head entry. */ assert(bdc->reuse_offset < bdc->reuse_list_size); plist = bdc->reuse_list[bdc->reuse_offset]; - node = SLIST_FIRST(&plist); SLIST_INIT(&bdc->reuse_list[bdc->reuse_offset]); /* 2. set offset = modulo reuse-list-size ( offset + 1 ), thereby From a1e49ec2c9edf4e4d2fc235c6f16657208d46654 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 29 Jul 2021 00:14:31 +0300 Subject: [PATCH 06/17] bgpd: fix double free in dampening code bgp_damp_info_unclaim already calls bgp_reuselist_del. We must not call it again here. Fixes #9046. Signed-off-by: Igor Ryzhov --- bgpd/bgp_damp.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 40fe5b8b59..08859324af 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -157,16 +157,9 @@ static void bgp_reuse_list_add(struct bgp_damp_info *bdi, } /* Delete BGP dampening information from reuse list. */ -static void bgp_reuse_list_delete(struct bgp_damp_info *bdi, - struct bgp_damp_config *bdc) +static void bgp_reuse_list_delete(struct bgp_damp_info *bdi) { - struct reuselist *list; - struct reuselist_node *rn; - - list = &bdc->reuse_list[bdi->index]; - rn = bgp_reuselist_find(list, bdi); bgp_damp_info_unclaim(bdi); - bgp_reuselist_del(list, &rn); } static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, @@ -358,7 +351,7 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, if (CHECK_FLAG(bdi->path->flags, BGP_PATH_DAMPED)) { /* If decay rate isn't equal to 0, reinsert brn. */ if (bdi->penalty != last_penalty) { - bgp_reuse_list_delete(bdi, bdc); + bgp_reuse_list_delete(bdi); bgp_reuse_list_add(bdi, bdc); } return BGP_DAMP_SUPPRESSED; @@ -402,7 +395,7 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, else if (CHECK_FLAG(bdi->path->flags, BGP_PATH_DAMPED) && (bdi->penalty < bdc->reuse_limit)) { bgp_path_info_unset_flag(dest, path, BGP_PATH_DAMPED); - bgp_reuse_list_delete(bdi, bdc); + bgp_reuse_list_delete(bdi); bgp_no_reuse_list_add(bdi, bdc); bdi->suppress_time = 0; status = BGP_DAMP_USED; From 391b4fa7a657a34e6bba1ed70d8e7847bbdeb384 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 30 Apr 2024 10:52:46 +0300 Subject: [PATCH 07/17] bgpd: Drop double-pointer for bgp_damp_info_free() This causes a crash using `clear ip bgp dampening `. Signed-off-by: Donatas Abraitis Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 32 +++++++++++++++++--------------- bgpd/bgp_damp.h | 2 +- bgpd/bgp_route.c | 8 +++++--- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 08859324af..20d80eb5db 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -262,7 +262,7 @@ static void bgp_reuse_timer(struct event *t) } if (bdi->penalty <= bdc->reuse_limit / 2.0) { - bgp_damp_info_free(&bdi, bdc, 1, bdi->afi, + bgp_damp_info_free(bdi, bdc, 1, bdi->afi, bdi->safi); bgp_reuselist_del(&plist, &node); } else { @@ -406,29 +406,29 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, bdi->t_updated = t_now; else { bgp_damp_info_unclaim(bdi); - bgp_damp_info_free(&bdi, bdc, 0, afi, safi); + bgp_damp_info_free(bdi, bdc, 0, afi, safi); } return status; } -void bgp_damp_info_free(struct bgp_damp_info **bdi, struct bgp_damp_config *bdc, +void bgp_damp_info_free(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc, int withdraw, afi_t afi, safi_t safi) { - assert(bdc && bdi && *bdi); + assert(bdc && bdi); - if ((*bdi)->path == NULL) { - XFREE(MTYPE_BGP_DAMP_INFO, (*bdi)); + if (bdi->path == NULL) { + XFREE(MTYPE_BGP_DAMP_INFO, bdi); return; } - (*bdi)->path->extra->damp_info = NULL; - bgp_path_info_unset_flag((*bdi)->dest, (*bdi)->path, + bdi->path->extra->damp_info = NULL; + bgp_path_info_unset_flag(bdi->dest, bdi->path, BGP_PATH_HISTORY | BGP_PATH_DAMPED); - if ((*bdi)->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { - bgp_path_info_delete((*bdi)->dest, (*bdi)->path); - bgp_process((*bdi)->path->peer->bgp, (*bdi)->dest, (*bdi)->path, afi, safi); + if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { + bgp_path_info_delete(bdi->dest, bdi->path); + bgp_process(bdi->path->peer->bgp, bdi->dest, bdi->path, afi, safi); } XFREE(MTYPE_BGP_DAMP_INFO, bdi); @@ -539,21 +539,23 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, while ((rn = SLIST_FIRST(list)) != NULL) { bdi = rn->info; if (bdi->lastrecord == BGP_RECORD_UPDATE) { - bgp_aggregate_increment(bgp, &bdi->dest->p, + bgp_aggregate_increment(bgp, + bgp_dest_get_prefix( + bdi->dest), bdi->path, bdi->afi, bdi->safi); - bgp_process(bgp, bdi->dest, bdi->afi, + bgp_process(bgp, bdi->dest, bdi->path, bdi->afi, bdi->safi); } bgp_reuselist_del(list, &rn); - bgp_damp_info_free(&bdi, bdc, 1, afi, safi); + bgp_damp_info_free(bdi, bdc, 1, afi, safi); } } while ((rn = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { bdi = rn->info; bgp_reuselist_del(&bdc->no_reuse_list, &rn); - bgp_damp_info_free(&bdi, bdc, 1, afi, safi); + bgp_damp_info_free(bdi, bdc, 1, afi, safi); } /* Free decay array */ diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index ae057a01f7..22b2efebbc 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -133,7 +133,7 @@ extern int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t safi, int attr_change); extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t saff); -extern void bgp_damp_info_free(struct bgp_damp_info **path, +extern void bgp_damp_info_free(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc, int withdraw, afi_t afi, safi_t safi); extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 81a99604ef..ee255b59b8 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -15826,7 +15826,7 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, if (pi->extra && pi->extra->damp_info) { pi_temp = pi->next; bgp_damp_info_free( - &pi->extra->damp_info, + pi->extra->damp_info, &bgp->damp[afi][safi], 1, afi, safi); pi = pi_temp; @@ -15854,17 +15854,19 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, == BGP_RECORD_UPDATE) { bgp_aggregate_increment( bgp, - &bdi->dest->p, + bgp_dest_get_prefix( + bdi->dest), bdi->path, bdi->afi, bdi->safi); bgp_process(bgp, bdi->dest, + bdi->path, bdi->afi, bdi->safi); } bgp_damp_info_free( - &pi->extra->damp_info, + pi->extra->damp_info, &bgp->damp[afi][safi], 1, afi, safi); pi = pi_temp; From 4c500d6952ea597eb7422358e18788547eed92a6 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 29 Jul 2021 14:42:16 +0300 Subject: [PATCH 08/17] bgpd: fix missing list add in dampening One more crash in dampening code... When bgp_damp_withdraw is called, if there's already a BDI structure, bgp_damp_info_claim is called to re-assign the bdi->config in case it was changed. The problem is that bgp_damp_info_claim actually removes the BDI from the reuse list of the old config and never adds it to the reuse list of the new config. We must do this to prevent the crash because all the code assumes that BDI is always in some list. Signed-off-by: Igor Ryzhov --- bgpd/bgp_damp.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 20d80eb5db..0c75f67a89 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -324,7 +324,14 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, (bgp_path_info_extra_get(path))->damp_info = bdi; bgp_no_reuse_list_add(bdi, bdc); } else { - bgp_damp_info_claim(bdi, bdc); + if (bdi->config != bdc) { + bgp_damp_info_claim(bdi, bdc); + if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) + bgp_reuselist_add(&bdc->no_reuse_list, bdi); + else + bgp_reuselist_add(&bdc->reuse_list[bdi->index], + bdi); + } last_penalty = bdi->penalty; /* 1. Set t-diff = t-now - t-updated. */ From 1d37871588b82ae17ee4c512c293ac87904883d6 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 29 Jul 2021 01:17:50 +0300 Subject: [PATCH 09/17] bgpd: fix incorrect usage of slist in dampening Current code is a complete misuse of SLIST structure. Instead of just adding a SLIST_ENTRY to struct bgp_damp_info, it allocates a separate structure to be a node in the list. Signed-off-by: Igor Ryzhov --- bgpd/bgp_damp.c | 108 +++++++++++------------------------------------- bgpd/bgp_damp.h | 9 ++-- 2 files changed, 27 insertions(+), 90 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 0c75f67a89..34616ce9ca 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -24,72 +24,32 @@ static void bgp_reuselist_add(struct reuselist *list, struct bgp_damp_info *info) { - struct reuselist_node *new_node; - assert(info); - new_node = XCALLOC(MTYPE_BGP_DAMP_REUSELIST, sizeof(*new_node)); - new_node->info = info; - SLIST_INSERT_HEAD(list, new_node, entry); + SLIST_INSERT_HEAD(list, info, entry); } -static void bgp_reuselist_del(struct reuselist *list, - struct reuselist_node **node) +static void bgp_reuselist_del(struct reuselist *list, struct bgp_damp_info *info) { - if ((*node) == NULL) - return; - assert(list && node && *node); - SLIST_REMOVE(list, (*node), reuselist_node, entry); - XFREE(MTYPE_BGP_DAMP_REUSELIST, (*node)); - *node = NULL; + assert(info); + SLIST_REMOVE(list, info, bgp_damp_info, entry); } static void bgp_reuselist_switch(struct reuselist *source, - struct reuselist_node *node, + struct bgp_damp_info *info, struct reuselist *target) { - assert(source && target && node); - SLIST_REMOVE(source, node, reuselist_node, entry); - SLIST_INSERT_HEAD(target, node, entry); -} - -static void bgp_reuselist_free(struct reuselist *list) -{ - struct reuselist_node *rn; - - assert(list); - while ((rn = SLIST_FIRST(list)) != NULL) - bgp_reuselist_del(list, &rn); -} - -static struct reuselist_node *bgp_reuselist_find(struct reuselist *list, - struct bgp_damp_info *info) -{ - struct reuselist_node *rn; - - assert(list && info); - SLIST_FOREACH (rn, list, entry) { - if (rn->info == info) - return rn; - } - return NULL; + assert(source && target && info); + SLIST_REMOVE(source, info, bgp_damp_info, entry); + SLIST_INSERT_HEAD(target, info, entry); } static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi) { - struct reuselist_node *node; - assert(bdi && bdi->config); - if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) { - node = bgp_reuselist_find(&bdi->config->no_reuse_list, bdi); - if (node) - bgp_reuselist_del(&bdi->config->no_reuse_list, &node); - } else { - node = bgp_reuselist_find(&bdi->config->reuse_list[bdi->index], - bdi); - if (node) - bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], - &node); - } + if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) + bgp_reuselist_del(&bdi->config->no_reuse_list, bdi); + else + bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], bdi); bdi->config = NULL; } @@ -170,19 +130,9 @@ static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, bgp_reuselist_add(&bdc->no_reuse_list, bdi); } -static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi, - struct bgp_damp_config *bdc) +static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi) { - struct reuselist_node *rn; - - assert(bdc && bdi); - if (bdi->config == NULL) { - bgp_damp_info_unclaim(bdi); - return; - } - bdi->config = NULL; - rn = bgp_reuselist_find(&bdc->no_reuse_list, bdi); - bgp_reuselist_del(&bdc->no_reuse_list, &rn); + bgp_damp_info_unclaim(bdi); } /* Return decayed penalty value. */ @@ -207,7 +157,6 @@ static void bgp_reuse_timer(struct event *t) { struct bgp_damp_info *bdi; struct reuselist plist; - struct reuselist_node *node; struct bgp *bgp; time_t t_now, t_diff; struct bgp_damp_config *bdc = EVENT_ARG(t); @@ -230,8 +179,7 @@ static void bgp_reuse_timer(struct event *t) assert(bdc->reuse_offset < bdc->reuse_list_size); /* 3. if ( the saved list head pointer is non-empty ) */ - while ((node = SLIST_FIRST(&plist)) != NULL) { - bdi = node->info; + while ((bdi = SLIST_FIRST(&plist)) != NULL) { bgp = bdi->path->peer->bgp; /* Set t-diff = t-now - t-updated. */ @@ -262,20 +210,19 @@ static void bgp_reuse_timer(struct event *t) } if (bdi->penalty <= bdc->reuse_limit / 2.0) { + bgp_reuselist_del(&plist, bdi); bgp_damp_info_free(bdi, bdc, 1, bdi->afi, bdi->safi); - bgp_reuselist_del(&plist, &node); } else { - node->info->index = - BGP_DAMP_NO_REUSE_LIST_INDEX; - bgp_reuselist_switch(&plist, node, + bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; + bgp_reuselist_switch(&plist, bdi, &bdc->no_reuse_list); } } else { /* Re-insert into another list (See RFC2439 Section * 4.8.6). */ bdi->index = bgp_reuse_index(bdi->penalty, bdc); - bgp_reuselist_switch(&plist, node, + bgp_reuselist_switch(&plist, bdi, &bdc->reuse_list[bdi->index]); } } @@ -369,7 +316,7 @@ int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, if (bdi->penalty >= bdc->suppress_value) { bgp_path_info_set_flag(dest, path, BGP_PATH_DAMPED); bdi->suppress_time = t_now; - bgp_no_reuse_list_delete(bdi, bdc); + bgp_no_reuse_list_delete(bdi); bgp_reuse_list_add(bdi, bdc); } return BGP_DAMP_USED; @@ -536,15 +483,13 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, afi_t afi, safi_t safi) { struct bgp_damp_info *bdi; - struct reuselist_node *rn; struct reuselist *list; unsigned int i; bdc->reuse_offset = 0; for (i = 0; i < bdc->reuse_list_size; ++i) { list = &bdc->reuse_list[i]; - while ((rn = SLIST_FIRST(list)) != NULL) { - bdi = rn->info; + while ((bdi = SLIST_FIRST(list)) != NULL) { if (bdi->lastrecord == BGP_RECORD_UPDATE) { bgp_aggregate_increment(bgp, bgp_dest_get_prefix( @@ -554,14 +499,13 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, bgp_process(bgp, bdi->dest, bdi->path, bdi->afi, bdi->safi); } - bgp_reuselist_del(list, &rn); + bgp_reuselist_del(list, bdi); bgp_damp_info_free(bdi, bdc, 1, afi, safi); } } - while ((rn = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { - bdi = rn->info; - bgp_reuselist_del(&bdc->no_reuse_list, &rn); + while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { + bgp_reuselist_del(&bdc->no_reuse_list, bdi); bgp_damp_info_free(bdi, bdc, 1, afi, safi); } @@ -573,10 +517,6 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->reuse_index); bdc->reuse_index_size = 0; - /* Free reuse list array. */ - for (i = 0; i < bdc->reuse_list_size; ++i) - bgp_reuselist_free(&bdc->reuse_list[i]); - XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->reuse_list); bdc->reuse_list_size = 0; diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index 22b2efebbc..ce3cb87837 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -46,14 +46,11 @@ struct bgp_damp_info { afi_t afi; safi_t safi; + + SLIST_ENTRY(bgp_damp_info) entry; }; -struct reuselist_node { - SLIST_ENTRY(reuselist_node) entry; - struct bgp_damp_info *info; -}; - -SLIST_HEAD(reuselist, reuselist_node); +SLIST_HEAD(reuselist, bgp_damp_info); /* Specified parameter set configuration. */ struct bgp_damp_config { From ad97cd00a69e564ba25eff4c03bf65aadc4af78d Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 30 Apr 2024 10:56:33 +0300 Subject: [PATCH 10/17] bgpd: cleanup bgp_damp_info_free bgp_damp_config, afi and safi are never used. Signed-off-by: Igor Ryzhov Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 18 +++++++----------- bgpd/bgp_damp.h | 4 +--- bgpd/bgp_route.c | 6 ++---- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 34616ce9ca..cbeb2275a7 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -211,8 +211,7 @@ static void bgp_reuse_timer(struct event *t) if (bdi->penalty <= bdc->reuse_limit / 2.0) { bgp_reuselist_del(&plist, bdi); - bgp_damp_info_free(bdi, bdc, 1, bdi->afi, - bdi->safi); + bgp_damp_info_free(bdi, 1); } else { bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; bgp_reuselist_switch(&plist, bdi, @@ -360,16 +359,15 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, bdi->t_updated = t_now; else { bgp_damp_info_unclaim(bdi); - bgp_damp_info_free(bdi, bdc, 0, afi, safi); + bgp_damp_info_free(bdi, 0); } return status; } -void bgp_damp_info_free(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc, - int withdraw, afi_t afi, safi_t safi) +void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw) { - assert(bdc && bdi); + assert(bdi); if (bdi->path == NULL) { XFREE(MTYPE_BGP_DAMP_INFO, bdi); @@ -380,10 +378,8 @@ void bgp_damp_info_free(struct bgp_damp_info *bdi, struct bgp_damp_config *bdc, bgp_path_info_unset_flag(bdi->dest, bdi->path, BGP_PATH_HISTORY | BGP_PATH_DAMPED); - if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { + if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) bgp_path_info_delete(bdi->dest, bdi->path); - bgp_process(bdi->path->peer->bgp, bdi->dest, bdi->path, afi, safi); - } XFREE(MTYPE_BGP_DAMP_INFO, bdi); } @@ -500,13 +496,13 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, bdi->safi); } bgp_reuselist_del(list, bdi); - bgp_damp_info_free(bdi, bdc, 1, afi, safi); + bgp_damp_info_free(bdi, 1); } } while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { bgp_reuselist_del(&bdc->no_reuse_list, bdi); - bgp_damp_info_free(bdi, bdc, 1, afi, safi); + bgp_damp_info_free(bdi, 1); } /* Free decay array */ diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index ce3cb87837..2578e25954 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -130,9 +130,7 @@ extern int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t safi, int attr_change); extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t saff); -extern void bgp_damp_info_free(struct bgp_damp_info *bdi, - struct bgp_damp_config *bdc, int withdraw, - afi_t afi, safi_t safi); +extern void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw); extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, afi_t afi, safi_t safi); extern void bgp_damp_config_clean(struct bgp_damp_config *bdc); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index ee255b59b8..fcf7216ce1 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -15827,8 +15827,7 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, pi_temp = pi->next; bgp_damp_info_free( pi->extra->damp_info, - &bgp->damp[afi][safi], - 1, afi, safi); + 1); pi = pi_temp; } else pi = pi->next; @@ -15867,8 +15866,7 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, } bgp_damp_info_free( pi->extra->damp_info, - &bgp->damp[afi][safi], - 1, afi, safi); + 1); pi = pi_temp; } else pi = pi->next; From 471e373c17fa8cf1e7e4a2f08bbd977b125cccd9 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 29 Jul 2021 01:54:03 +0300 Subject: [PATCH 11/17] bgpd: fix missing damp info free when cleaning bgp path Signed-off-by: Igor Ryzhov --- bgpd/bgp_route.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index fcf7216ce1..9697245eea 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -250,6 +250,8 @@ void bgp_path_info_extra_free(struct bgp_path_info_extra **extra) e = *extra; + if (e->damp_info) + bgp_damp_info_free(e->damp_info, 0); e->damp_info = NULL; if (e->vrfleak && e->vrfleak->parent) { struct bgp_path_info *bpi = From f8e6b7ce450f494c88fdddbd8822673579b6d9b4 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 24 Apr 2024 16:42:08 +0300 Subject: [PATCH 12/17] bgpd: Use SLIST_FOREACH_SAFE when iterating over the list in bgp_reuse_timer Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index cbeb2275a7..ddf7e0db8f 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -155,7 +155,7 @@ int bgp_damp_decay(time_t tdiff, int penalty, struct bgp_damp_config *bdc) is evaluated. RFC2439 Section 4.8.7. */ static void bgp_reuse_timer(struct event *t) { - struct bgp_damp_info *bdi; + struct bgp_damp_info *bdi, *bdi_next; struct reuselist plist; struct bgp *bgp; time_t t_now, t_diff; @@ -179,7 +179,7 @@ static void bgp_reuse_timer(struct event *t) assert(bdc->reuse_offset < bdc->reuse_list_size); /* 3. if ( the saved list head pointer is non-empty ) */ - while ((bdi = SLIST_FIRST(&plist)) != NULL) { + SLIST_FOREACH_SAFE (bdi, &plist, entry, bdi_next) { bgp = bdi->path->peer->bgp; /* Set t-diff = t-now - t-updated. */ From 70ac630b399a007ae396eab42217d98638162a71 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 24 Apr 2024 16:59:25 +0300 Subject: [PATCH 13/17] bgpd: Pass the right reuse_list when handling it via bgp_reuse_timer thread This fixes the crash: ``` ==14759== Invalid read of size 8 ==14759== at 0x31032B: bgp_reuselist_del (bgp_damp.c:51) ==14759== by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69) ==14759== by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387) ==14759== by 0x311016: bgp_reuse_timer (bgp_damp.c:230) ==14759== by 0x4F227CC: thread_call (thread.c:2008) ==14759== by 0x4EDB7D7: frr_run (libfrr.c:1216) ==14759== by 0x1EF748: main (bgp_main.c:525) ==14759== Address 0x48 is not stack'd, malloc'd or (recently) free'd ==14759== ==14759== ==14759== Process terminating with default action of signal 11 (SIGSEGV) ==14759== at 0x59CC7F5: raise (raise.c:46) ==14759== by 0x4F10CEB: core_handler (sigevent.c:261) ==14759== by 0x59CC97F: ??? (in /lib/x86_64-linux-gnu/libpthread-2.27.so) ==14759== by 0x31032A: bgp_reuselist_del (bgp_damp.c:51) ==14759== by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69) ==14759== by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387) ==14759== by 0x311016: bgp_reuse_timer (bgp_damp.c:230) ==14759== by 0x4F227CC: thread_call (thread.c:2008) ==14759== by 0x4EDB7D7: frr_run (libfrr.c:1216) ==14759== by 0x1EF748: main (bgp_main.c:525) ``` Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 39 +++++++++++++++++---------------------- bgpd/bgp_damp.h | 3 ++- bgpd/bgp_route.c | 12 +++++------- 3 files changed, 24 insertions(+), 30 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index ddf7e0db8f..6b9b12dd4d 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -43,13 +43,16 @@ static void bgp_reuselist_switch(struct reuselist *source, SLIST_INSERT_HEAD(target, info, entry); } -static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi) +static void bgp_damp_info_unclaim(struct bgp_damp_info *bdi, + struct reuselist *list) { assert(bdi && bdi->config); if (bdi->index == BGP_DAMP_NO_REUSE_LIST_INDEX) bgp_reuselist_del(&bdi->config->no_reuse_list, bdi); else - bgp_reuselist_del(&bdi->config->reuse_list[bdi->index], bdi); + bgp_reuselist_del(list ? list + : &bdi->config->reuse_list[bdi->index], + bdi); bdi->config = NULL; } @@ -61,7 +64,7 @@ static void bgp_damp_info_claim(struct bgp_damp_info *bdi, bdi->config = bdc; return; } - bgp_damp_info_unclaim(bdi); + bgp_damp_info_unclaim(bdi, NULL); bdi->config = bdc; bdi->afi = bdc->afi; bdi->safi = bdc->safi; @@ -119,7 +122,7 @@ static void bgp_reuse_list_add(struct bgp_damp_info *bdi, /* Delete BGP dampening information from reuse list. */ static void bgp_reuse_list_delete(struct bgp_damp_info *bdi) { - bgp_damp_info_unclaim(bdi); + bgp_damp_info_unclaim(bdi, NULL); } static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, @@ -132,7 +135,7 @@ static void bgp_no_reuse_list_add(struct bgp_damp_info *bdi, static void bgp_no_reuse_list_delete(struct bgp_damp_info *bdi) { - bgp_damp_info_unclaim(bdi); + bgp_damp_info_unclaim(bdi, NULL); } /* Return decayed penalty value. */ @@ -210,8 +213,7 @@ static void bgp_reuse_timer(struct event *t) } if (bdi->penalty <= bdc->reuse_limit / 2.0) { - bgp_reuselist_del(&plist, bdi); - bgp_damp_info_free(bdi, 1); + bgp_damp_info_free(bdi, &plist, 1); } else { bdi->index = BGP_DAMP_NO_REUSE_LIST_INDEX; bgp_reuselist_switch(&plist, bdi, @@ -357,22 +359,18 @@ int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, if (bdi->penalty > bdc->reuse_limit / 2.0) bdi->t_updated = t_now; - else { - bgp_damp_info_unclaim(bdi); - bgp_damp_info_free(bdi, 0); - } + else + bgp_damp_info_free(bdi, NULL, 0); return status; } -void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw) +void bgp_damp_info_free(struct bgp_damp_info *bdi, struct reuselist *list, + int withdraw) { assert(bdi); - if (bdi->path == NULL) { - XFREE(MTYPE_BGP_DAMP_INFO, bdi); - return; - } + bgp_damp_info_unclaim(bdi, list); bdi->path->extra->damp_info = NULL; bgp_path_info_unset_flag(bdi->dest, bdi->path, @@ -495,15 +493,12 @@ void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, bgp_process(bgp, bdi->dest, bdi->path, bdi->afi, bdi->safi); } - bgp_reuselist_del(list, bdi); - bgp_damp_info_free(bdi, 1); + bgp_damp_info_free(bdi, list, 1); } } - while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) { - bgp_reuselist_del(&bdc->no_reuse_list, bdi); - bgp_damp_info_free(bdi, 1); - } + while ((bdi = SLIST_FIRST(&bdc->no_reuse_list)) != NULL) + bgp_damp_info_free(bdi, &bdc->no_reuse_list, 1); /* Free decay array */ XFREE(MTYPE_BGP_DAMP_ARRAY, bdc->decay_array); diff --git a/bgpd/bgp_damp.h b/bgpd/bgp_damp.h index 2578e25954..851c6f9e85 100644 --- a/bgpd/bgp_damp.h +++ b/bgpd/bgp_damp.h @@ -130,7 +130,8 @@ extern int bgp_damp_withdraw(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t safi, int attr_change); extern int bgp_damp_update(struct bgp_path_info *path, struct bgp_dest *dest, afi_t afi, safi_t saff); -extern void bgp_damp_info_free(struct bgp_damp_info *bdi, int withdraw); +extern void bgp_damp_info_free(struct bgp_damp_info *bdi, + struct reuselist *list, int withdraw); extern void bgp_damp_info_clean(struct bgp *bgp, struct bgp_damp_config *bdc, afi_t afi, safi_t safi); extern void bgp_damp_config_clean(struct bgp_damp_config *bdc); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 9697245eea..5022ba36ab 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -251,7 +251,7 @@ void bgp_path_info_extra_free(struct bgp_path_info_extra **extra) e = *extra; if (e->damp_info) - bgp_damp_info_free(e->damp_info, 0); + bgp_damp_info_free(e->damp_info, NULL, 0); e->damp_info = NULL; if (e->vrfleak && e->vrfleak->parent) { struct bgp_path_info *bpi = @@ -15827,9 +15827,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, while (pi) { if (pi->extra && pi->extra->damp_info) { pi_temp = pi->next; - bgp_damp_info_free( - pi->extra->damp_info, - 1); + bgp_damp_info_free(pi->extra->damp_info, + NULL, 1); pi = pi_temp; } else pi = pi->next; @@ -15866,9 +15865,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, bdi->afi, bdi->safi); } - bgp_damp_info_free( - pi->extra->damp_info, - 1); + bgp_damp_info_free(pi->extra->damp_info, + NULL, 1); pi = pi_temp; } else pi = pi->next; From 3921324346dae6665744fa854a082d1ce74fee32 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 24 Apr 2024 17:13:48 +0300 Subject: [PATCH 14/17] bgpd: Put dest into work queue when the path is really withdrawn by dampening Signed-off-by: Donatas Abraitis --- bgpd/bgp_damp.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 6b9b12dd4d..339bfae56d 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -370,14 +370,22 @@ void bgp_damp_info_free(struct bgp_damp_info *bdi, struct reuselist *list, { assert(bdi); + afi_t afi = bdi->afi; + safi_t safi = bdi->safi; + struct bgp_path_info *bpi = bdi->path; + struct bgp_dest *dest = bdi->dest; + struct bgp *bgp = bpi->peer->bgp; + const struct prefix *p = bgp_dest_get_prefix(bdi->dest); + bgp_damp_info_unclaim(bdi, list); - bdi->path->extra->damp_info = NULL; - bgp_path_info_unset_flag(bdi->dest, bdi->path, - BGP_PATH_HISTORY | BGP_PATH_DAMPED); - - if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) - bgp_path_info_delete(bdi->dest, bdi->path); + bpi->extra->damp_info = NULL; + bgp_path_info_unset_flag(dest, bpi, BGP_PATH_HISTORY | BGP_PATH_DAMPED); + if (bdi->lastrecord == BGP_RECORD_WITHDRAW && withdraw) { + bgp_aggregate_decrement(bgp, p, bpi, afi, SAFI_UNICAST); + bgp_path_info_delete(dest, bpi); + bgp_process(bgp, dest, bpi, afi, safi); + } XFREE(MTYPE_BGP_DAMP_INFO, bdi); } From b07a21dd1a76f752d485b38a9a99cc7081768793 Mon Sep 17 00:00:00 2001 From: David Schweizer Date: Tue, 30 Apr 2024 11:15:40 +0300 Subject: [PATCH 15/17] doc: user doc for route-flap dampening commands Changes update the user documentation to include a description of the now available commands to enable/disable route-flap dampening for peers and peer groups. Signed-off-by: David Schweizer Signed-off-by: Donatas Abraitis --- doc/user/bgp.rst | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 1de6773922..ae342e4c13 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -589,26 +589,52 @@ Route Flap Dampening .. clicmd:: bgp dampening (1-45) (1-20000) (1-50000) (1-255) - This command enables BGP route-flap dampening and specifies dampening parameters. + This command enables (with optionally specified dampening parameters) or + disables route-flap dampening for all routes of a BGP instance. + +.. clicmd:: neighbor PEER dampening [(1-45) [(1-20000) (1-20000) (1-255)]] + + This command enables (with optionally specified dampening parameters) or + disables route-flap dampening for all routes learned from a BGP peer. + +.. clicmd:: neighbor GROUP dampening [(1-45) [(1-20000) (1-20000) (1-255)]] + + This command enables (with optionally specified dampening parameters) or + disables route-flap dampening for all routes learned from peers of a peer + group. half-life - Half-life time for the penalty + Half-life time for the penalty in minutes (default value: 15). reuse-threshold - Value to start reusing a route + Value to start reusing a route (default value: 750). suppress-threshold - Value to start suppressing a route + Value to start suppressing a route (default value: 2000). max-suppress - Maximum duration to suppress a stable route + Maximum duration to suppress a stable route in minutes (default value: + 60). The route-flap damping algorithm is compatible with :rfc:`2439`. The use of - this command is not recommended nowadays. + these commands is not recommended nowadays. At the moment, route-flap dampening is not working per VRF and is working only for IPv4 unicast and multicast. + With different parameter sets configurable for BGP instances, peer groups and + peers, the active dampening profile for a route is chosen on the fly, + allowing for various changes in configuration (i.e. peer group memberships) + during runtime. The parameter sets are taking precedence in the following + order: + + 1. Peer + 2. Peer group + 3. BGP instance + + The negating commands do not allow to exclude a peer/peer group from a peer + group/BGP instances configuration. + .. seealso:: https://www.ripe.net/publications/docs/ripe-378 @@ -1335,7 +1361,7 @@ OSPFv3 into ``address-family ipv4 unicast`` as OSPFv3 supports IPv6. .. clicmd:: redistribute [metric (0-4294967295)] [route-map WORD] Redistribute routes from other protocols into BGP. - + Note - When redistributing a static route, or any better Admin Distance route, into BGP for which the same path is learned dynamically from another BGP speaker, if the redistribute path is more preferred from a BGP Best Path From 709bdcbb230a52d12822f605c8847439d05dd615 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 30 Apr 2024 12:14:23 +0300 Subject: [PATCH 16/17] tests: Check if dampening per-peer works Signed-off-by: Donatas Abraitis --- .../bgp_dampening_per_peer/__init__.py | 0 .../bgp_dampening_per_peer/r1/frr.conf | 15 ++ .../bgp_dampening_per_peer/r2/frr.conf | 17 ++ .../test_bgp_dampening_per_peer.py | 177 ++++++++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 tests/topotests/bgp_dampening_per_peer/__init__.py create mode 100644 tests/topotests/bgp_dampening_per_peer/r1/frr.conf create mode 100644 tests/topotests/bgp_dampening_per_peer/r2/frr.conf create mode 100644 tests/topotests/bgp_dampening_per_peer/test_bgp_dampening_per_peer.py diff --git a/tests/topotests/bgp_dampening_per_peer/__init__.py b/tests/topotests/bgp_dampening_per_peer/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_dampening_per_peer/r1/frr.conf b/tests/topotests/bgp_dampening_per_peer/r1/frr.conf new file mode 100644 index 0000000000..45899559cd --- /dev/null +++ b/tests/topotests/bgp_dampening_per_peer/r1/frr.conf @@ -0,0 +1,15 @@ +! +int r1-eth0 + ip address 192.168.1.1/24 +! +router bgp 65001 + no bgp ebgp-requires-policy + neighbor 192.168.1.2 remote-as external + neighbor 192.168.1.2 timers 1 3 + neighbor 192.168.1.2 timers connect 1 + neighbor 192.168.1.2 capability dynamic + ! + address-family ipv4 unicast + neighbor 192.168.1.2 dampening 1 1 1 1 + exit-address-family +! diff --git a/tests/topotests/bgp_dampening_per_peer/r2/frr.conf b/tests/topotests/bgp_dampening_per_peer/r2/frr.conf new file mode 100644 index 0000000000..d68d13d075 --- /dev/null +++ b/tests/topotests/bgp_dampening_per_peer/r2/frr.conf @@ -0,0 +1,17 @@ +! +int lo + ip address 10.10.10.10/32 +! +int r2-eth0 + ip address 192.168.1.2/24 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 1 3 + neighbor 192.168.1.1 timers connect 1 + ! + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_dampening_per_peer/test_bgp_dampening_per_peer.py b/tests/topotests/bgp_dampening_per_peer/test_bgp_dampening_per_peer.py new file mode 100644 index 0000000000..2066d848d3 --- /dev/null +++ b/tests/topotests/bgp_dampening_per_peer/test_bgp_dampening_per_peer.py @@ -0,0 +1,177 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis +# + +import os +import re +import sys +import json +import pytest +import functools + +pytestmark = [pytest.mark.bgpd] + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, get_topogen + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2")} + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_dampening_per_peer(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _converge(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.10.10.10/32 json")) + expected = { + "paths": [ + { + "valid": True, + "nexthops": [ + { + "hostname": "r2", + "accessible": True, + } + ], + } + ] + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't converge" + + #### + # Withdraw 10.10.10.10/32, and check if it's flagged as history. + #### + r2.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + no redistribute connected + """ + ) + + def _check_bgp_dampening_history(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.10.10.10/32 json")) + expected = { + "paths": [ + { + "dampeningHistoryEntry": True, + "nexthops": [ + { + "hostname": "r2", + "accessible": True, + } + ], + } + ], + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _check_bgp_dampening_history, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "10.10.10.10/32 is not flagged as history entry" + + #### + # Reannounce 10.10.10.10/32, and check if it's flagged as dampened. + #### + r2.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + redistribute connected + """ + ) + + def _check_bgp_dampening_dampened(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.10.10.10/32 json")) + expected = { + "paths": [ + { + "valid": True, + "dampeningSuppressed": True, + "nexthops": [ + { + "hostname": "r2", + "accessible": True, + } + ], + } + ], + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _check_bgp_dampening_dampened, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "10.10.10.10/32 is not flagged as dampened entry" + + #### + # Check if the route becomes non-dampened again after some time. + #### + def _check_bgp_dampening_undampened(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.10.10.10/32 json")) + expected = { + "paths": [ + { + "valid": True, + "dampeningHistoryEntry": None, + "dampeningSuppressed": None, + "nexthops": [ + { + "hostname": "r2", + "accessible": True, + } + ], + } + ], + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _check_bgp_dampening_undampened, + ) + _, result = topotest.run_and_expect(test_func, None, count=120, wait=10) + assert result is None, "10.10.10.10/32 is flagged as history/dampened" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From bf37877103d537b3b41fb7e32c6c8c465ab8a0eb Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 2 May 2024 22:48:19 +0300 Subject: [PATCH 17/17] bgpd: Reduce the nesting level for bgp_clear_damp_route() Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 60 ++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5022ba36ab..8c24ff32b3 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -15839,42 +15839,38 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name, } } else { dest = bgp_node_match(bgp->rib[afi][safi], &match); - if (dest != NULL) { - const struct prefix *dest_p = bgp_dest_get_prefix(dest); + if (!dest) + return CMD_SUCCESS; - if (!prefix_check - || dest_p->prefixlen == match.prefixlen) { - pi = bgp_dest_get_bgp_path_info(dest); - while (pi) { - if (pi->extra && pi->extra->damp_info) { - pi_temp = pi->next; - struct bgp_damp_info *bdi = - pi->extra->damp_info; - if (bdi->lastrecord - == BGP_RECORD_UPDATE) { - bgp_aggregate_increment( - bgp, - bgp_dest_get_prefix( - bdi->dest), - bdi->path, - bdi->afi, - bdi->safi); - bgp_process(bgp, - bdi->dest, - bdi->path, - bdi->afi, - bdi->safi); - } - bgp_damp_info_free(pi->extra->damp_info, - NULL, 1); - pi = pi_temp; - } else - pi = pi->next; - } + const struct prefix *dest_p = bgp_dest_get_prefix(dest); + + if (prefix_check || dest_p->prefixlen != match.prefixlen) + return CMD_SUCCESS; + + pi = bgp_dest_get_bgp_path_info(dest); + while (pi) { + if (!(pi->extra && pi->extra->damp_info)) { + pi = pi->next; + continue; } - bgp_dest_unlock_node(dest); + pi_temp = pi->next; + struct bgp_damp_info *bdi = pi->extra->damp_info; + + if (bdi->lastrecord != BGP_RECORD_UPDATE) + continue; + + bgp_aggregate_increment(bgp, + bgp_dest_get_prefix(bdi->dest), + bdi->path, bdi->afi, bdi->safi); + bgp_process(bgp, bdi->dest, bdi->path, bdi->afi, + bdi->safi); + + bgp_damp_info_free(pi->extra->damp_info, NULL, 1); + pi = pi_temp; } + + bgp_dest_unlock_node(dest); } return CMD_SUCCESS;