From: Mitch Skiba Date: Wed, 9 May 2018 23:10:02 +0000 (+0000) Subject: bgpd: Re-use TX Addpath IDs where possible X-Git-Tag: frr-7.1-dev~196^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=dcc68b5e2a598a180b370163a7022324a9b5da96;p=matthieu%2Ffrr.git bgpd: Re-use TX Addpath IDs where possible The motivation for this patch is to address a concerning behavior of tx-addpath-bestpath-per-AS. Prior to this patch, all paths' TX ID was pre-determined as the path was received from a peer. However, this meant that any time the path selected as best from an AS changed, bgpd had no choice but to withdraw the previous best path, and advertise the new best-path under a new TX ID. This could cause significant network disruption, especially for the subset of prefixes coming from only one AS that were also communicated over a bestpath-per-AS session. The patch's general approach is best illustrated by txaddpath_update_ids. After a bestpath run (required for best-per-AS to know what will and will not be sent as addpaths) ID numbers will be stripped from paths that no longer need to be sent, and held in a pool. Then, paths that will be sent as addpaths and do not already have ID numbers will allocate new ID numbers, pulling first from that pool. Finally, anything left in the pool will be returned to the allocator. In order for this to work, ID numbers had to be split by strategy. The tx-addpath-All strategy would keep every ID number "in use" constantly, preventing IDs from being transferred to different paths. Rather than create two variables for ID, this patch create a more generic array that will easily enable more addpath strategies to be implemented. The previously described ID manipulations will happen per addpath strategy, and will only be run for strategies that are enabled on at least one peer. Finally, the ID numbers are allocated from an allocator that tracks per AFI/SAFI/Addpath Strategy which IDs are in use. Though it would be very improbable, there was the possibility with the free-running counter approach for rollover to cause two paths on the same prefix to get assigned the same TX ID. As remote as the possibility is, we prefer to not leave it to chance. This ID re-use method is not perfect. In some cases you could still get withdraw-then-add behaviors where not strictly necessary. In the case of bestpath-per-AS this requires one AS to advertise a prefix for the first time, then a second AS withdraws that prefix, all within the space of an already pending MRAI timer. In those situations a withdraw-then-add is more forgivable, and fixing it would probably require a much more significant effort, as IDs would need to be moved to ADVs instead of paths. Signed-off-by Mitchell Skiba --- diff --git a/bgpd/bgp_addpath.c b/bgpd/bgp_addpath.c new file mode 100644 index 0000000000..22401f0017 --- /dev/null +++ b/bgpd/bgp_addpath.c @@ -0,0 +1,422 @@ +/* + * Addpath TX ID selection, and related utilities + * Copyright (C) 2018 Amazon.com, Inc. or its affiliates + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; see the file COPYING; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "bgp_addpath.h" +#include "bgp_route.h" + +static struct bgp_addpath_strategy_names strat_names[BGP_ADDPATH_MAX] = { + { + .config_name = "addpath-tx-all-paths", + .human_name = "All", + .human_description = "Advertise all paths via addpath", + .type_json_name = "addpathTxAllPaths", + .id_json_name = "addpathTxIdAll" + }, + { + .config_name = "addpath-tx-bestpath-per-AS", + .human_name = "Best-Per-AS", + .human_description = "Advertise bestpath per AS via addpath", + .type_json_name = "addpathTxBestpathPerAS", + .id_json_name = "addpathTxIdBestPerAS" + } +}; + +static struct bgp_addpath_strategy_names unknown_names = { + .config_name = "addpath-tx-unknown", + .human_name = "Unknown-Addpath-Strategy", + .human_description = "Unknown Addpath Strategy", + .type_json_name = "addpathTxUnknown", + .id_json_name = "addpathTxIdUnknown" +}; + +/* + * Returns a structure full of strings associated with an addpath type. Will + * never return null. + */ +struct bgp_addpath_strategy_names * +bgp_addpath_names(enum bgp_addpath_strat strat) +{ + if (strat < BGP_ADDPATH_MAX) + return &(strat_names[strat]); + else + return &unknown_names; +}; + +/* + * Returns if any peer is transmitting addpaths for a given afi/safi. + */ +int bgp_addpath_is_addpath_used(struct bgp_addpath_bgp_data *d, afi_t afi, + safi_t safi) +{ + return d->total_peercount[afi][safi] > 0; +} + +/* + * Initialize the BGP instance level data for addpath. + */ +void bgp_addpath_init_bgp_data(struct bgp_addpath_bgp_data *d) +{ + safi_t safi; + afi_t afi; + int i; + + for (afi = AFI_IP; afi < AFI_MAX; afi++) { + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + d->id_allocators[afi][safi][i] = NULL; + d->peercount[afi][safi][i] = 0; + } + d->total_peercount[afi][safi] = 0; + } + } +} + +/* + * Free up resources associated with BGP route info structures. + */ +void bgp_addpath_free_info_data(struct bgp_addpath_info_data *d, + struct bgp_addpath_node_data *nd) +{ + int i; + + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + if (d->addpath_tx_id[i] != IDALLOC_INVALID) + idalloc_free_to_pool(&nd->free_ids[i], + d->addpath_tx_id[i]); + } +} + +/* + * Return the addpath ID used to send a particular route, to a particular peer, + * in a particular AFI/SAFI. + */ +uint32_t bgp_addpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi, + struct bgp_addpath_info_data *d) +{ + if (peer->addpath_type[afi][safi] < BGP_ADDPATH_MAX) + return d->addpath_tx_id[peer->addpath_type[afi][safi]]; + else + return IDALLOC_INVALID; +} + +/* + * Returns true if the path has an assigned addpath ID for any of the addpath + * strategies. + */ +int bgp_addpath_info_has_ids(struct bgp_addpath_info_data *d) +{ + int i; + + for (i = 0; i < BGP_ADDPATH_MAX; i++) + if (d->addpath_tx_id[i] != 0) + return 1; + + return 0; +} + +/* + * Releases any ID's associated with the BGP prefix. + */ +void bgp_addpath_free_node_data(struct bgp_addpath_bgp_data *bd, + struct bgp_addpath_node_data *nd, afi_t afi, + safi_t safi) +{ + int i; + + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + idalloc_drain_pool(bd->id_allocators[afi][safi][i], + &(nd->free_ids[i])); + } +} + +/* + * Check to see if the addpath strategy requires DMED to be configured to work. + */ +int bgp_addpath_dmed_required(int strategy) +{ + return strategy == BGP_ADDPATH_BEST_PER_AS; +} + +/* + * Return true if this is a path we should advertise due to a + * configured addpath-tx knob + */ +int bgp_addpath_tx_path(enum bgp_addpath_strat strat, + struct bgp_path_info *pi) +{ + switch (strat) { + case BGP_ADDPATH_NONE: + return 0; + case BGP_ADDPATH_ALL: + return 1; + case BGP_ADDPATH_BEST_PER_AS: + if (CHECK_FLAG(pi->flags, BGP_PATH_DMED_SELECTED)) + return 1; + else + return 0; + default: + return 0; + } +} + +/* + * Purge all addpath ID's on a BGP instance associated with the addpath + * strategy, and afi/safi combination. This lets us let go of all memory held to + * track ID numbers associated with an addpath type not in use. Since + * post-bestpath ID processing is skipped for types not used, this is the only + * chance to free this data. + */ +static void bgp_addpath_flush_type(struct bgp *bgp, afi_t afi, safi_t safi, + enum bgp_addpath_strat addpath_type) +{ + struct bgp_node *rn; + struct bgp_path_info *pi; + + for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; + rn = bgp_route_next(rn)) { + idalloc_drain_pool( + bgp->tx_addpath.id_allocators[afi][safi][addpath_type], + &(rn->tx_addpath.free_ids[addpath_type])); + for (pi = rn->info; pi; pi = pi->next) { + if (pi->tx_addpath.addpath_tx_id[addpath_type] + != IDALLOC_INVALID) { + idalloc_free( + bgp->tx_addpath + .id_allocators[afi][safi] + [addpath_type], + pi->tx_addpath + .addpath_tx_id[addpath_type]); + pi->tx_addpath.addpath_tx_id[addpath_type] = + IDALLOC_INVALID; + } + } + } + + idalloc_destroy(bgp->tx_addpath.id_allocators[afi][safi][addpath_type]); + bgp->tx_addpath.id_allocators[afi][safi][addpath_type] = NULL; +} + +/* + * Allocate an Addpath ID for the given type on a path, if necessary. + */ +static void bgp_addpath_populate_path(struct id_alloc *allocator, + struct bgp_path_info *path, + enum bgp_addpath_strat addpath_type) +{ + if (bgp_addpath_tx_path(addpath_type, path)) { + path->tx_addpath.addpath_tx_id[addpath_type] = + idalloc_allocate(allocator); + } +} + +/* + * Compute addpath ID's on a BGP instance associated with the addpath strategy, + * and afi/safi combination. Since we won't waste the time computing addpath IDs + * for unused strategies, the first time a peer is configured to use a strategy, + * we have to backfill the data. + */ +static void bgp_addpath_populate_type(struct bgp *bgp, afi_t afi, safi_t safi, + enum bgp_addpath_strat addpath_type) +{ + struct bgp_node *rn; + struct bgp_path_info *bi; + char buf[200]; + struct id_alloc *allocator; + + snprintf(buf, sizeof(buf), "Addpath ID Allocator %s:%d/%d", + bgp_addpath_names(addpath_type)->config_name, (int)afi, + (int)safi); + buf[sizeof(buf) - 1] = '\0'; + zlog_info("Computing addpath IDs for addpath type %s", + bgp_addpath_names(addpath_type)->human_name); + + bgp->tx_addpath.id_allocators[afi][safi][addpath_type] = + idalloc_new(buf); + + idalloc_reserve(bgp->tx_addpath.id_allocators[afi][safi][addpath_type], + BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE); + + allocator = bgp->tx_addpath.id_allocators[afi][safi][addpath_type]; + + for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; + rn = bgp_route_next(rn)) + for (bi = rn->info; bi; bi = bi->next) + bgp_addpath_populate_path(allocator, bi, addpath_type); +} + +/* + * Handle updates to a peer or group's addpath strategy. If after adjusting + * counts a addpath strategy is in use for the first time, or no longer in use, + * the IDs for that strategy will be populated or flushed. + */ +void bgp_addpath_type_changed(struct bgp *bgp) +{ + afi_t afi; + safi_t safi; + struct listnode *node, *nnode; + struct peer *peer; + int peer_count[AFI_MAX][SAFI_MAX][BGP_ADDPATH_MAX]; + enum bgp_addpath_strat type; + + FOREACH_AFI_SAFI(afi, safi) { + for (type=0; typepeer, node, nnode, peer)) { + FOREACH_AFI_SAFI(afi, safi) { + type = peer->addpath_type[afi][safi]; + if (type != BGP_ADDPATH_NONE) { + peer_count[afi][safi][type] += 1; + } + } + } + + FOREACH_AFI_SAFI(afi, safi) { + for (type=0; typetx_addpath.peercount[afi][safi][type]; + int new = peer_count[afi][safi][type]; + + bgp->tx_addpath.peercount[afi][safi][type] = new; + + if (old == 0 && new != 0) { + bgp_addpath_populate_type(bgp, afi, safi, + type); + } else if (old != 0 && new == 0) { + bgp_addpath_flush_type(bgp, afi, safi, type); + } + } + } +} + +/* + * Change the addpath type assigned to a peer, or peer group. In addition to + * adjusting the counts, peer sessions will be reset as needed to make the + * change take effect. + */ +void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi, + enum bgp_addpath_strat addpath_type) +{ + struct bgp *bgp = peer->bgp; + enum bgp_addpath_strat old_type = peer->addpath_type[afi][safi]; + struct listnode *node, *nnode; + struct peer *tmp_peer; + struct peer_group *group; + + if (addpath_type == old_type) + return; + + if (addpath_type == BGP_ADDPATH_NONE && peer->group && + !CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* A "no" config on a group member inherits group */ + addpath_type = peer->group->conf->addpath_type[afi][safi]; + } + + peer->addpath_type[afi][safi] = addpath_type; + + bgp_addpath_type_changed(bgp); + + if (addpath_type != BGP_ADDPATH_NONE) { + if (bgp_addpath_dmed_required(addpath_type)) { + if (!bgp_flag_check(bgp, BGP_FLAG_DETERMINISTIC_MED)) { + zlog_warn( + "%s: enabling bgp deterministic-med, this is required for addpath-tx-bestpath-per-AS", + peer->host); + bgp_flag_set(bgp, BGP_FLAG_DETERMINISTIC_MED); + bgp_recalculate_all_bestpaths(bgp); + } + } + } + + zlog_info("Resetting peer %s%s due to change in addpath config\n", + CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) ? "group " : "", + peer->host); + + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + group = peer->group; + + /* group will be null as peer_group_delete calls peer_delete on + * group->conf. That peer_delete will eventuallly end up here + * if the group was configured to tx addpaths. + */ + if (group != NULL) { + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, + tmp_peer)) { + if (tmp_peer->addpath_type[afi][safi] == + old_type) { + bgp_addpath_set_peer_type(tmp_peer, + afi, + safi, + addpath_type); + } + } + } + } else { + peer_change_action(peer, afi, safi, peer_change_reset); + } + +} + +/* + * Intended to run after bestpath. This function will take TX IDs from paths + * that no longer need them, and give them to paths that do. This prevents + * best-per-as updates from needing to do a separate withdraw and update just to + * swap out which path is sent. + */ +void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, + safi_t safi) +{ + int i; + struct bgp_path_info *pi; + struct id_alloc_pool **pool_ptr; + + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + struct id_alloc *alloc = + bgp->tx_addpath.id_allocators[afi][safi][i]; + pool_ptr = &(bn->tx_addpath.free_ids[i]); + + if (bgp->tx_addpath.peercount[afi][safi][i] == 0) + continue; + + /* Free Unused IDs back to the pool.*/ + for (pi = bn->info; pi; pi = pi->next) { + if (pi->tx_addpath.addpath_tx_id[i] != IDALLOC_INVALID + && !bgp_addpath_tx_path(i, pi)) { + idalloc_free_to_pool(pool_ptr, + pi->tx_addpath.addpath_tx_id[i]); + pi->tx_addpath.addpath_tx_id[i] = + IDALLOC_INVALID; + } + } + + /* Give IDs to paths that need them (pulling from the pool) */ + for (pi = bn->info; pi; pi = pi->next) { + if (pi->tx_addpath.addpath_tx_id[i] == IDALLOC_INVALID + && bgp_addpath_tx_path(i, pi)) { + pi->tx_addpath.addpath_tx_id[i] = + idalloc_allocate_prefer_pool( + alloc, pool_ptr); + } + } + + /* Free any IDs left in the pool to the main allocator */ + idalloc_drain_pool(alloc, pool_ptr); + } +} diff --git a/bgpd/bgp_addpath.h b/bgpd/bgp_addpath.h new file mode 100644 index 0000000000..c0c182791b --- /dev/null +++ b/bgpd/bgp_addpath.h @@ -0,0 +1,72 @@ +/* + * Addpath TX ID selection, and related utilities + * Copyright (C) 2018 Amazon.com, Inc. or its affiliates + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; see the file COPYING; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef _QUAGGA_BGPD_TX_ADDPATH_H +#define _QUAGGA_BGPD_TX_ADDPATH_H + +#include +#include + +#include "bgpd/bgp_addpath_types.h" +#include "bgpd/bgp_route.h" +#include "bgpd/bgp_table.h" +#include "lib/json.h" + +#define BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE 1 + +void bgp_addpath_init_bgp_data(struct bgp_addpath_bgp_data *d); + +int bgp_addpath_is_addpath_used(struct bgp_addpath_bgp_data *d, afi_t afi, + safi_t safi); + +void bgp_addpath_free_node_data(struct bgp_addpath_bgp_data *bd, + struct bgp_addpath_node_data *nd, + afi_t afi, safi_t safi); + +void bgp_addpath_free_info_data(struct bgp_addpath_info_data *d, + struct bgp_addpath_node_data *nd); + + +int bgp_addpath_info_has_ids(struct bgp_addpath_info_data *d); + +uint32_t bgp_addpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi, + struct bgp_addpath_info_data *d); + +struct bgp_addpath_strategy_names * +bgp_addpath_names(enum bgp_addpath_strat strat); + +int bgp_addpath_dmed_required(int strategy); + +/* + * Return true if this is a path we should advertise due to a configured + * addpath-tx knob + */ +int bgp_addpath_tx_path(enum bgp_addpath_strat strat, + struct bgp_path_info *pi); +/* + * Change the type of addpath used for a peer. + */ +void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi, + enum bgp_addpath_strat addpath_type); + +void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, + safi_t safi); + +void bgp_addpath_type_changed(struct bgp *bgp); +#endif diff --git a/bgpd/bgp_addpath_types.h b/bgpd/bgp_addpath_types.h new file mode 100644 index 0000000000..b0b3302979 --- /dev/null +++ b/bgpd/bgp_addpath_types.h @@ -0,0 +1,55 @@ +/* + * Addpath TX ID selection, and related utilities + * Copyright (C) 2018 Amazon.com, Inc. or its affiliates + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; see the file COPYING; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef _QUAGGA_BGPD_TX_ADDPATH_DATA_H +#define _QUAGGA_BGPD_TX_ADDPATH_DATA_H +#include "lib/id_alloc.h" +#include + +enum bgp_addpath_strat { + BGP_ADDPATH_ALL = 0, + BGP_ADDPATH_BEST_PER_AS, + BGP_ADDPATH_MAX, + BGP_ADDPATH_NONE, +}; + +/* TX Addpath structures */ +struct bgp_addpath_bgp_data { + unsigned int peercount[AFI_MAX][SAFI_MAX][BGP_ADDPATH_MAX]; + unsigned int total_peercount[AFI_MAX][SAFI_MAX]; + struct id_alloc *id_allocators[AFI_MAX][SAFI_MAX][BGP_ADDPATH_MAX]; +}; + +struct bgp_addpath_node_data { + struct id_alloc_pool *free_ids[BGP_ADDPATH_MAX]; +}; + +struct bgp_addpath_info_data { + uint32_t addpath_tx_id[BGP_ADDPATH_MAX]; +}; + +struct bgp_addpath_strategy_names { + const char *config_name; + const char *human_name; /* path detail non-json */ + const char *human_description; /* non-json peer descriptions */ + const char *type_json_name; /* json peer listings */ + const char *id_json_name; /* path json output for tx ID# */ +}; + +#endif diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index cff050a9ef..fc3ac28723 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -47,6 +47,7 @@ #include "bgpd/bgp_aspath.h" #include "bgpd/bgp_zebra.h" #include "bgpd/bgp_nexthop.h" +#include "bgpd/bgp_addpath.h" /* * Definitions and external declarations. @@ -1059,7 +1060,7 @@ static int evpn_es_route_select_install(struct bgp *bgp, && old_select->sub_type == BGP_ROUTE_IMPORTED && !CHECK_FLAG(rn->flags, BGP_NODE_USER_CLEAR) && !CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) - && !bgp->addpath_tx_used[afi][safi]) { + && !bgp_addpath_is_addpath_used(&bgp->tx_addpath, afi, safi)) { if (bgp_zebra_has_route_changed(rn, old_select)) { ret = evpn_es_install_vtep(bgp, es, (struct prefix_evpn *)&rn->p, @@ -1142,7 +1143,7 @@ static int evpn_route_select_install(struct bgp *bgp, struct bgpevpn *vpn, && old_select->sub_type == BGP_ROUTE_IMPORTED && !CHECK_FLAG(rn->flags, BGP_NODE_USER_CLEAR) && !CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) - && !bgp->addpath_tx_used[afi][safi]) { + && !bgp_addpath_is_addpath_used(&bgp->tx_addpath, afi, safi)) { if (bgp_zebra_has_route_changed(rn, old_select)) { if (old_select->attr->sticky) SET_FLAG(flags, ZEBRA_MACIP_TYPE_STICKY); diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index cf5901df5a..3018124f45 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -1387,10 +1387,7 @@ void bgp_open_capability(struct stream *s, struct peer *peer) /* Only advertise addpath TX if a feature that * will use it is * configured */ - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_ALL_PATHS) - || CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) + if (peer->addpath_type[afi][safi] != BGP_ADDPATH_NONE) adv_addpath_tx = 1; } } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 8ae74a008c..bb8c5d8f6e 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -65,6 +65,7 @@ #include "bgpd/bgp_nht.h" #include "bgpd/bgp_updgrp.h" #include "bgpd/bgp_label.h" +#include "bgpd/bgp_addpath.h" #if ENABLE_BGP_VNC #include "bgpd/rfapi/rfapi_backend.h" @@ -247,6 +248,8 @@ static void bgp_path_info_free(struct bgp_path_info *path) bgp_unlink_nexthop(path); bgp_path_info_extra_free(&path->extra); bgp_path_info_mpath_free(&path->mpath); + bgp_addpath_free_info_data(&path->tx_addpath, + path->net ? &path->net->tx_addpath : NULL); peer_unlock(path->peer); /* bgp_path_info peer reference */ @@ -1472,7 +1475,7 @@ int subgroup_announce_check(struct bgp_node *rn, struct bgp_path_info *pi, * addpath * feature that requires us to advertise it */ if (!CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) { - if (!bgp_addpath_tx_path(peer, afi, safi, pi)) { + if (!bgp_addpath_tx_path(peer->addpath_type[afi][safi], pi)) { return 0; } } @@ -2078,6 +2081,8 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_node *rn, bgp_path_info_mpath_aggregate_update(new_select, old_select); bgp_mp_list_clear(&mp_list); + bgp_addpath_update_ids(bgp, rn, afi, safi); + result->old = old_select; result->new = new_select; @@ -2127,7 +2132,7 @@ int subgroup_process_announce_selected(struct update_subgroup *subgrp, bgp_adj_out_set_subgroup(rn, subgrp, &attr, selected); else bgp_adj_out_unset_subgroup(rn, subgrp, 1, - selected->addpath_tx_id); + addpath_tx_id); } /* If selected is NULL we must withdraw the path using addpath_tx_id */ @@ -2303,7 +2308,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_node *rn, if (old_select && old_select == new_select && !CHECK_FLAG(rn->flags, BGP_NODE_USER_CLEAR) && !CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) - && !bgp->addpath_tx_used[afi][safi]) { + && !bgp_addpath_is_addpath_used(&bgp->tx_addpath, afi, safi)) { if (bgp_zebra_has_route_changed(rn, old_select)) { #if ENABLE_BGP_VNC vnc_import_bgp_add_route(bgp, p, old_select); @@ -2776,7 +2781,6 @@ struct bgp_path_info *info_make(int type, int sub_type, unsigned short instance, new->attr = attr; new->uptime = bgp_clock(); new->net = rn; - new->addpath_tx_id = ++peer->bgp->addpath_tx_id; return new; } @@ -7486,6 +7490,18 @@ static void route_vty_out_advertised_to(struct vty *vty, struct peer *peer, } } +static void route_vty_out_tx_ids(struct vty *vty, + struct bgp_addpath_info_data *d) +{ + int i; + + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + vty_out(vty, "TX-%s %u%s", bgp_addpath_names(i)->human_name, + d->addpath_tx_id[i], + i < BGP_ADDPATH_MAX - 1 ? " " : "\n"); + } +} + void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct prefix *p, struct bgp_path_info *path, afi_t afi, safi_t safi, json_object *json_paths) @@ -7517,6 +7533,7 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct prefix *p, unsigned int first_as; bool nexthop_self = CHECK_FLAG(path->flags, BGP_PATH_ANNC_NH_SELF) ? true : false; + int i; if (json_paths) { json_path = json_object_new_object(); @@ -8228,29 +8245,53 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct prefix *p, } /* Line 8 display Addpath IDs */ - if (path->addpath_rx_id || path->addpath_tx_id) { + if (path->addpath_rx_id + || bgp_addpath_info_has_ids(&path->tx_addpath)) { if (json_paths) { json_object_int_add(json_path, "addpathRxId", path->addpath_rx_id); - json_object_int_add(json_path, "addpathTxId", - path->addpath_tx_id); + + /* Keep backwards compatibility with the old API + * by putting TX All's ID in the old field + */ + json_object_int_add( + json_path, "addpathTxId", + path->tx_addpath.addpath_tx_id + [BGP_ADDPATH_ALL]); + + /* ... but create a specific field for each + * strategy + */ + for (i = 0; i < BGP_ADDPATH_MAX; i++) { + json_object_int_add( + json_path, + bgp_addpath_names(i) + ->id_json_name, + path->tx_addpath + .addpath_tx_id[i]); + } } else { - vty_out(vty, " AddPath ID: RX %u, TX %u\n", - path->addpath_rx_id, - path->addpath_tx_id); + vty_out(vty, " AddPath ID: RX %u, ", + path->addpath_rx_id); + + route_vty_out_tx_ids(vty, &path->tx_addpath); } } /* If we used addpath to TX a non-bestpath we need to display - * "Advertised to" on a path-by-path basis */ - if (bgp->addpath_tx_used[afi][safi]) { + * "Advertised to" on a path-by-path basis + */ + if (bgp_addpath_is_addpath_used(&bgp->tx_addpath, afi, safi)) { first = 1; for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { addpath_capable = bgp_addpath_encode_tx(peer, afi, safi); has_adj = bgp_adj_out_lookup( - peer, path->net, path->addpath_tx_id); + peer, path->net, + bgp_addpath_id_for_peer( + peer, afi, safi, + &path->tx_addpath)); if ((addpath_capable && has_adj) || (!addpath_capable && has_adj @@ -8958,7 +8999,7 @@ void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp, * show what peers we advertised the bestpath to. If we are using * addpath * though then we must display Advertised to on a path-by-path basis. */ - if (!bgp->addpath_tx_used[afi][safi]) { + if (!bgp_addpath_is_addpath_used(&bgp->tx_addpath, afi, safi)) { for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { if (bgp_adj_out_lookup(peer, rn, 0)) { if (json && !json_adv_to) diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index f0edc8d49a..466d52ed64 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -24,6 +24,7 @@ #include "queue.h" #include "nexthop.h" #include "bgp_table.h" +#include "bgp_addpath_types.h" struct bgp_nexthop_cache; struct bgp_route_evpn; @@ -220,7 +221,7 @@ struct bgp_path_info { /* Addpath identifiers */ uint32_t addpath_rx_id; - uint32_t addpath_tx_id; + struct bgp_addpath_info_data tx_addpath; }; /* Structure used in BGP path selection */ diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index 231f326cef..728eeaa3a9 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -29,6 +29,7 @@ #include "bgpd/bgpd.h" #include "bgpd/bgp_table.h" +#include "bgp_addpath.h" void bgp_table_lock(struct bgp_table *rt) { @@ -76,7 +77,16 @@ static void bgp_node_destroy(route_table_delegate_t *delegate, struct route_table *table, struct route_node *node) { struct bgp_node *bgp_node; + struct bgp_table *rt; bgp_node = bgp_node_from_rnode(node); + rt = table->info; + + if (rt->bgp) { + bgp_addpath_free_node_data(&rt->bgp->tx_addpath, + &bgp_node->tx_addpath, + rt->afi, rt->safi); + } + XFREE(MTYPE_BGP_NODE, bgp_node); } diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index 60a26b76cc..c267b4fe8a 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -25,6 +25,7 @@ #include "table.h" #include "queue.h" #include "linklist.h" +#include "bgpd.h" struct bgp_table { /* table belongs to this instance */ @@ -67,6 +68,8 @@ struct bgp_node { #define BGP_NODE_USER_CLEAR (1 << 1) #define BGP_NODE_LABEL_CHANGED (1 << 2) #define BGP_NODE_REGISTERED_FOR_LABEL (1 << 3) + + struct bgp_addpath_node_data tx_addpath; }; /* diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index 393586dbec..2e2ad88314 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -152,6 +152,7 @@ static void conf_copy(struct peer *dst, struct peer *src, afi_t afi, dst->af_cap[afi][safi] = src->af_cap[afi][safi]; dst->afc_nego[afi][safi] = src->afc_nego[afi][safi]; dst->orf_plist[afi][safi] = src->orf_plist[afi][safi]; + dst->addpath_type[afi][safi] = src->addpath_type[afi][safi]; dst->local_as = src->local_as; dst->change_local_as = src->change_local_as; dst->shared_network = src->shared_network; @@ -322,6 +323,7 @@ static unsigned int updgrp_hash_key_make(void *p) key = jhash_1word(peer->sort, key); /* EBGP or IBGP */ key = jhash_1word((peer->flags & PEER_UPDGRP_FLAGS), key); key = jhash_1word((flags & PEER_UPDGRP_AF_FLAGS), key); + key = jhash_1word((uint32_t)peer->addpath_type[afi][safi], key); key = jhash_1word((peer->cap & PEER_UPDGRP_CAP_FLAGS), key); key = jhash_1word((peer->af_cap[afi][safi] & PEER_UPDGRP_AF_CAP_FLAGS), key); @@ -437,6 +439,9 @@ static bool updgrp_hash_cmp(const void *p1, const void *p2) if ((flags1 & PEER_UPDGRP_AF_FLAGS) != (flags2 & PEER_UPDGRP_AF_FLAGS)) return false; + if (pe1->addpath_type[afi][safi] != pe2->addpath_type[afi][safi]) + return 0; + if ((pe1->cap & PEER_UPDGRP_CAP_FLAGS) != (pe2->cap & PEER_UPDGRP_CAP_FLAGS)) return false; @@ -1900,22 +1905,3 @@ int bgp_addpath_encode_tx(struct peer *peer, afi_t afi, safi_t safi) && CHECK_FLAG(peer->af_cap[afi][safi], PEER_CAP_ADDPATH_AF_RX_RCV)); } - -/* - * Return true if this is a path we should advertise due to a - * configured addpath-tx knob - */ -int bgp_addpath_tx_path(struct peer *peer, afi_t afi, safi_t safi, - struct bgp_path_info *pi) -{ - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_ALL_PATHS)) - return 1; - - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS) - && CHECK_FLAG(pi->flags, BGP_PATH_DMED_SELECTED)) - return 1; - - return 0; -} diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h index b1d852d384..6b3bf9d1f7 100644 --- a/bgpd/bgp_updgrp.h +++ b/bgpd/bgp_updgrp.h @@ -64,8 +64,7 @@ | PEER_FLAG_REMOVE_PRIVATE_AS_ALL \ | PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE \ | PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE \ - | PEER_FLAG_ADDPATH_TX_ALL_PATHS \ - | PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS | PEER_FLAG_AS_OVERRIDE) + | PEER_FLAG_AS_OVERRIDE) #define PEER_UPDGRP_CAP_FLAGS (PEER_CAP_AS4_RCV) @@ -469,8 +468,6 @@ extern int update_group_clear_update_dbg(struct update_group *updgrp, extern void update_bgp_group_free(struct bgp *bgp); extern int bgp_addpath_encode_tx(struct peer *peer, afi_t afi, safi_t safi); -extern int bgp_addpath_tx_path(struct peer *peer, afi_t afi, safi_t safi, - struct bgp_path_info *pi); /* * Inline functions diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 81b782c2b0..7196bbbf12 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -49,6 +49,7 @@ #include "bgpd/bgp_mplsvpn.h" #include "bgpd/bgp_updgrp.h" #include "bgpd/bgp_advertise.h" +#include "bgpd/bgp_addpath.h" /******************** @@ -97,6 +98,40 @@ static void adj_free(struct bgp_adj_out *adj) XFREE(MTYPE_BGP_ADJ_OUT, adj); } +static void subgrp_withdraw_stale_addpath(struct updwalk_context *ctx, + struct update_subgroup *subgrp) +{ + struct bgp_adj_out *adj, *adj_next; + uint32_t id; + struct bgp_path_info *pi; + afi_t afi = SUBGRP_AFI(subgrp); + safi_t safi = SUBGRP_SAFI(subgrp); + struct peer *peer = SUBGRP_PEER(subgrp); + + /* Look through all of the paths we have advertised for this rn and send + * a withdraw for the ones that are no longer present */ + for (adj = ctx->rn->adj_out; adj; adj = adj_next) { + adj_next = adj->next; + + if (adj->subgroup == subgrp) { + for (pi = ctx->rn->info; pi; pi = pi->next) { + id = bgp_addpath_id_for_peer(peer, afi, safi, + &pi->tx_addpath); + + if (id == adj->addpath_tx_id) { + break; + } + } + + if (!pi) { + subgroup_process_announce_selected( + subgrp, NULL, ctx->rn, + adj->addpath_tx_id); + } + } + } +} + static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) { struct updwalk_context *ctx = arg; @@ -131,31 +166,7 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) if (!subgrp->t_coalesce) { /* An update-group that uses addpath */ if (addpath_capable) { - /* Look through all of the paths we have - * advertised for this rn and - * send a withdraw for the ones that are no - * longer present */ - for (adj = ctx->rn->adj_out; adj; - adj = adj_next) { - adj_next = adj->next; - - if (adj->subgroup == subgrp) { - for (pi = ctx->rn->info; pi; - pi = pi->next) { - if (pi->addpath_tx_id - == adj->addpath_tx_id) { - break; - } - } - - if (!pi) { - subgroup_process_announce_selected( - subgrp, NULL, - ctx->rn, - adj->addpath_tx_id); - } - } - } + subgrp_withdraw_stale_addpath(ctx, subgrp); for (pi = ctx->rn->info; pi; pi = pi->next) { /* Skip the bestpath for now */ @@ -164,7 +175,9 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) subgroup_process_announce_selected( subgrp, pi, ctx->rn, - pi->addpath_tx_id); + bgp_addpath_id_for_peer( + peer, afi, safi, + &pi->tx_addpath)); } /* Process the bestpath last so the "show [ip] @@ -174,7 +187,9 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) if (ctx->pi) subgroup_process_announce_selected( subgrp, ctx->pi, ctx->rn, - ctx->pi->addpath_tx_id); + bgp_addpath_id_for_peer( + peer, afi, safi, + &ctx->pi->tx_addpath)); } /* An update-group that does not use addpath */ @@ -182,7 +197,9 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) if (ctx->pi) { subgroup_process_announce_selected( subgrp, ctx->pi, ctx->rn, - ctx->pi->addpath_tx_id); + bgp_addpath_id_for_peer( + peer, afi, safi, + &ctx->pi->tx_addpath)); } else { /* Find the addpath_tx_id of the path we * had advertised and @@ -433,15 +450,27 @@ void bgp_adj_out_set_subgroup(struct bgp_node *rn, { struct bgp_adj_out *adj = NULL; struct bgp_advertise *adv; + struct peer *peer; + afi_t afi; + safi_t safi; + + peer = SUBGRP_PEER(subgrp); + afi = SUBGRP_AFI(subgrp); + safi = SUBGRP_SAFI(subgrp); if (DISABLE_BGP_ANNOUNCE) return; /* Look for adjacency information. */ - adj = adj_lookup(rn, subgrp, path->addpath_tx_id); + adj = adj_lookup( + rn, subgrp, + bgp_addpath_id_for_peer(peer, afi, safi, &path->tx_addpath)); if (!adj) { - adj = bgp_adj_out_alloc(subgrp, rn, path->addpath_tx_id); + adj = bgp_adj_out_alloc( + subgrp, rn, + bgp_addpath_id_for_peer(peer, afi, safi, + &path->tx_addpath)); if (!adj) return; } @@ -597,7 +626,9 @@ void subgroup_announce_table(struct update_subgroup *subgrp, if (CHECK_FLAG(ri->flags, BGP_PATH_SELECTED) || (addpath_capable - && bgp_addpath_tx_path(peer, afi, safi, ri))) { + && bgp_addpath_tx_path( + peer->addpath_type[afi][safi], + ri))) { if (subgroup_announce_check(rn, ri, subgrp, &rn->p, &attr)) bgp_adj_out_set_subgroup(rn, subgrp, @@ -605,7 +636,9 @@ void subgroup_announce_table(struct update_subgroup *subgrp, else bgp_adj_out_unset_subgroup( rn, subgrp, 1, - ri->addpath_tx_id); + bgp_addpath_id_for_peer( + peer, afi, safi, + &ri->tx_addpath)); } /* diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index c4fcea948f..ca6bb5ab82 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -56,6 +56,7 @@ #include "bgpd/bgp_nht.h" #include "bgpd/bgp_mplsvpn.h" #include "bgpd/bgp_label.h" +#include "bgpd/bgp_addpath.h" /******************** * PRIVATE FUNCTIONS diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index c57cd38151..4dd148bb41 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -61,6 +61,7 @@ #include "bgpd/bgp_bfd.h" #include "bgpd/bgp_io.h" #include "bgpd/bgp_evpn.h" +#include "bgpd/bgp_addpath.h" static struct peer_group *listen_range_exists(struct bgp *bgp, struct prefix *range, int exact); @@ -1888,9 +1889,8 @@ DEFUN (no_bgp_deterministic_med, for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { FOREACH_AFI_SAFI (afi, safi) - if (CHECK_FLAG( - peer->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) { + if (bgp_addpath_dmed_required( + peer->addpath_type[afi][safi])) { bestpath_per_as_used = 1; break; } @@ -6234,9 +6234,9 @@ DEFUN (neighbor_addpath_tx_all_paths, if (!peer) return CMD_WARNING_CONFIG_FAILED; - return peer_af_flag_set_vty(vty, argv[idx_peer]->arg, bgp_node_afi(vty), - bgp_node_safi(vty), - PEER_FLAG_ADDPATH_TX_ALL_PATHS); + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), + BGP_ADDPATH_ALL); + return CMD_SUCCESS; } ALIAS_HIDDEN(neighbor_addpath_tx_all_paths, @@ -6254,9 +6254,23 @@ DEFUN (no_neighbor_addpath_tx_all_paths, "Use addpath to advertise all paths to a neighbor\n") { int idx_peer = 2; - return peer_af_flag_unset_vty(vty, argv[idx_peer]->arg, - bgp_node_afi(vty), bgp_node_safi(vty), - PEER_FLAG_ADDPATH_TX_ALL_PATHS); + struct peer *peer; + + peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); + if (!peer) + return CMD_WARNING_CONFIG_FAILED; + + if (peer->addpath_type[bgp_node_afi(vty)][bgp_node_safi(vty)] + != BGP_ADDPATH_ALL) { + vty_out(vty, + "%% Peer not currently configured to transmit all paths."); + return CMD_WARNING_CONFIG_FAILED; + } + + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), + BGP_ADDPATH_NONE); + + return CMD_SUCCESS; } ALIAS_HIDDEN(no_neighbor_addpath_tx_all_paths, @@ -6279,9 +6293,10 @@ DEFUN (neighbor_addpath_tx_bestpath_per_as, if (!peer) return CMD_WARNING_CONFIG_FAILED; - return peer_af_flag_set_vty(vty, argv[idx_peer]->arg, bgp_node_afi(vty), - bgp_node_safi(vty), - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS); + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), + BGP_ADDPATH_BEST_PER_AS); + + return CMD_SUCCESS; } ALIAS_HIDDEN(neighbor_addpath_tx_bestpath_per_as, @@ -6299,9 +6314,23 @@ DEFUN (no_neighbor_addpath_tx_bestpath_per_as, "Use addpath to advertise the bestpath per each neighboring AS\n") { int idx_peer = 2; - return peer_af_flag_unset_vty(vty, argv[idx_peer]->arg, - bgp_node_afi(vty), bgp_node_safi(vty), - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS); + struct peer *peer; + + peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); + if (!peer) + return CMD_WARNING_CONFIG_FAILED; + + if (peer->addpath_type[bgp_node_afi(vty)][bgp_node_safi(vty)] + != BGP_ADDPATH_BEST_PER_AS) { + vty_out(vty, + "%% Peer not currently configured to transmit all best path per as."); + return CMD_WARNING_CONFIG_FAILED; + } + + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), + BGP_ADDPATH_NONE); + + return CMD_SUCCESS; } ALIAS_HIDDEN(no_neighbor_addpath_tx_bestpath_per_as, @@ -8656,15 +8685,11 @@ static void bgp_show_peer_afi(struct vty *vty, struct peer *p, afi_t afi, json_addr, "privateAsNumsRemovedInUpdatesToNbr"); - if (CHECK_FLAG(p->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_ALL_PATHS)) - json_object_boolean_true_add(json_addr, - "addpathTxAllPaths"); - - if (CHECK_FLAG(p->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) - json_object_boolean_true_add(json_addr, - "addpathTxBestpathPerAS"); + if (p->addpath_type[afi][safi] != BGP_ADDPATH_NONE) + json_object_boolean_true_add( + json_addr, + bgp_addpath_names(p->addpath_type[afi][safi]) + ->type_json_name); if (CHECK_FLAG(p->af_flags[afi][safi], PEER_FLAG_AS_OVERRIDE)) json_object_string_add(json_addr, @@ -8930,14 +8955,10 @@ static void bgp_show_peer_afi(struct vty *vty, struct peer *p, afi_t afi, vty_out(vty, " Private AS numbers removed in updates to this neighbor\n"); - if (CHECK_FLAG(p->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_ALL_PATHS)) - vty_out(vty, " Advertise all paths via addpath\n"); - - if (CHECK_FLAG(p->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) - vty_out(vty, - " Advertise bestpath per AS via addpath\n"); + if (p->addpath_type[afi][safi] != BGP_ADDPATH_NONE) + vty_out(vty, " %s\n", + bgp_addpath_names(p->addpath_type[afi][safi]) + ->human_description); if (CHECK_FLAG(p->af_flags[afi][safi], PEER_FLAG_AS_OVERRIDE)) vty_out(vty, diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 19384eec2f..f30d3ede89 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -85,6 +85,7 @@ #include "bgpd/bgp_flowspec.h" #include "bgpd/bgp_labelpool.h" #include "bgpd/bgp_pbr.h" +#include "bgpd/bgp_addpath.h" DEFINE_MTYPE_STATIC(BGPD, PEER_TX_SHUTDOWN_MSG, "Peer shutdown message (TX)"); DEFINE_QOBJ_TYPE(bgp_master) @@ -883,6 +884,31 @@ static bool peergroup_filter_check(struct peer *peer, afi_t afi, safi_t safi, } } +/* Return true if the addpath type is set for peer and different from + * peer-group. + */ +static int peergroup_af_addpath_check(struct peer *peer, afi_t afi, safi_t safi) +{ + enum bgp_addpath_strat type, g_type; + + type = peer->addpath_type[afi][safi]; + + if (type != BGP_ADDPATH_NONE) { + if (peer_group_active(peer)) { + g_type = peer->group->conf->addpath_type[afi][safi]; + + if (type != g_type) + return 1; + else + return 0; + } + + return 1; + } + + return 0; +} + /* Check peer's AS number and determines if this peer is IBGP or EBGP */ static inline bgp_peer_sort_t peer_calc_sort(struct peer *peer) { @@ -960,6 +986,9 @@ bgp_peer_sort_t peer_sort(struct peer *peer) static void peer_free(struct peer *peer) { + afi_t afi; + safi_t safi; + assert(peer->status == Deleted); QOBJ_UNREG(peer); @@ -1032,6 +1061,13 @@ static void peer_free(struct peer *peer) bfd_info_free(&(peer->bfd_info)); + for (afi = AFI_IP; afi < AFI_MAX; afi++) { + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { + bgp_addpath_set_peer_type(peer, afi, safi, + BGP_ADDPATH_NONE); + } + } + bgp_unlock(peer->bgp); memset(peer, 0, sizeof(struct peer)); @@ -1118,6 +1154,7 @@ struct peer *peer_new(struct bgp *bgp) SET_FLAG(peer->af_flags_invert[afi][safi], PEER_FLAG_SEND_LARGE_COMMUNITY); } + peer->addpath_type[afi][safi] = BGP_ADDPATH_NONE; } /* set nexthop-unchanged for l2vpn evpn by default */ @@ -1210,6 +1247,8 @@ void peer_xfer_config(struct peer *peer_dst, struct peer *peer_src) peer_dst->allowas_in[afi][safi] = peer_src->allowas_in[afi][safi]; peer_dst->weight[afi][safi] = peer_src->weight[afi][safi]; + peer_dst->addpath_type[afi][safi] = + peer_src->addpath_type[afi][safi]; } for (afidx = BGP_AF_START; afidx < BGP_AF_MAX; afidx++) { @@ -1808,6 +1847,11 @@ static void peer_group2peer_config_copy_af(struct peer_group *group, MTYPE_BGP_FILTER_NAME); PEER_ATTR_INHERIT(peer, group, filter[afi][safi].usmap.map); } + + if (peer->addpath_type[afi][safi] == BGP_ADDPATH_NONE) { + peer->addpath_type[afi][safi] = conf->addpath_type[afi][safi]; + bgp_addpath_type_changed(conf->bgp); + } } static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) @@ -2836,7 +2880,7 @@ static struct bgp *bgp_create(as_t *as, const char *name, #if DFLT_BGP_DETERMINISTIC_MED bgp_flag_set(bgp, BGP_FLAG_DETERMINISTIC_MED); #endif - bgp->addpath_tx_id = BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE; + bgp_addpath_init_bgp_data(&bgp->tx_addpath); bgp->as = *as; @@ -3627,15 +3671,7 @@ int peer_active_nego(struct peer *peer) return 0; } -/* peer_flag_change_type. */ -enum peer_change_type { - peer_change_none, - peer_change_reset, - peer_change_reset_in, - peer_change_reset_out, -}; - -static void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, +void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, enum peer_change_type type) { if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) @@ -3731,8 +3767,6 @@ static const struct peer_flag_action peer_af_flag_action_list[] = { {PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE, 1, peer_change_reset_out}, {PEER_FLAG_AS_OVERRIDE, 1, peer_change_reset_out}, {PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE, 1, peer_change_reset_out}, - {PEER_FLAG_ADDPATH_TX_ALL_PATHS, 1, peer_change_reset}, - {PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS, 1, peer_change_reset}, {PEER_FLAG_WEIGHT, 0, peer_change_reset_in}, {0, 0, 0}}; @@ -3951,9 +3985,7 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, { int found; int size; - int addpath_tx_used; bool invert, member_invert; - struct bgp *bgp; struct peer *member; struct listnode *node, *nnode; struct peer_flag_action action; @@ -4116,45 +4148,6 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, } } - /* Track if addpath TX is in use */ - if (flag & (PEER_FLAG_ADDPATH_TX_ALL_PATHS - | PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) { - bgp = peer->bgp; - addpath_tx_used = 0; - - if (set) { - addpath_tx_used = 1; - - if (flag & PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS) { - if (!bgp_flag_check( - bgp, BGP_FLAG_DETERMINISTIC_MED)) { - zlog_info( - "%s: enabling bgp deterministic-med, this is required" - " for addpath-tx-bestpath-per-AS", - peer->host); - bgp_flag_set( - bgp, - BGP_FLAG_DETERMINISTIC_MED); - bgp_recalculate_all_bestpaths(bgp); - } - } - } else { - for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, - member)) { - if (CHECK_FLAG(member->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_ALL_PATHS) - || CHECK_FLAG( - member->af_flags[afi][safi], - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) { - addpath_tx_used = 1; - break; - } - } - } - - bgp->addpath_tx_used[afi][safi] = addpath_tx_used; - } - return 0; } @@ -7059,15 +7052,21 @@ static void bgp_config_write_peer_af(struct vty *vty, struct bgp *bgp, } /* addpath TX knobs */ - if (peergroup_af_flag_check(peer, afi, safi, - PEER_FLAG_ADDPATH_TX_ALL_PATHS)) { - vty_out(vty, " neighbor %s addpath-tx-all-paths\n", addr); - } - - if (peergroup_af_flag_check(peer, afi, safi, - PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) { - vty_out(vty, " neighbor %s addpath-tx-bestpath-per-AS\n", - addr); + if (peergroup_af_addpath_check(peer, afi, safi)) { + switch (peer->addpath_type[afi][safi]) { + case BGP_ADDPATH_ALL: + vty_out(vty, " neighbor %s addpath-tx-all-paths\n", + addr); + break; + case BGP_ADDPATH_BEST_PER_AS: + vty_out(vty, + " neighbor %s addpath-tx-bestpath-per-AS\n", + addr); + break; + case BGP_ADDPATH_MAX: + case BGP_ADDPATH_NONE: + break; + } } /* ORF capability. */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index e14b0f39e0..5ee54e3c70 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -39,6 +39,7 @@ #include "bitfield.h" #include "vxlan.h" #include "bgp_labelpool.h" +#include "bgp_addpath_types.h" #define BGP_MAX_HOSTNAME 64 /* Linux max, is larger than most other sys */ #define BGP_PEER_MAX_HASH_SIZE 16384 @@ -462,8 +463,7 @@ struct bgp { /* Auto-shutdown new peers */ bool autoshutdown; - uint32_t addpath_tx_id; - int addpath_tx_used[AFI_MAX][SAFI_MAX]; + struct bgp_addpath_bgp_data tx_addpath; #if ENABLE_BGP_VNC struct rfapi_cfg *rfapi_cfg; @@ -938,12 +938,12 @@ struct peer { #define PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE (1 << 19) /* remove-private-as replace-as */ #define PEER_FLAG_AS_OVERRIDE (1 << 20) /* as-override */ #define PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE (1 << 21) /* remove-private-as all replace-as */ -#define PEER_FLAG_ADDPATH_TX_ALL_PATHS (1 << 22) /* addpath-tx-all-paths */ -#define PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS (1 << 23) /* addpath-tx-bestpath-per-AS */ #define PEER_FLAG_WEIGHT (1 << 24) /* weight */ #define PEER_FLAG_ALLOWAS_IN_ORIGIN (1 << 25) /* allowas-in origin */ #define PEER_FLAG_SEND_LARGE_COMMUNITY (1 << 26) /* Send large Communities */ + enum bgp_addpath_strat addpath_type[AFI_MAX][SAFI_MAX]; + /* MD5 password */ char *password; @@ -1466,6 +1466,14 @@ typedef enum { BGP_POLICY_DISTRIBUTE_LIST, } bgp_policy_type_e; +/* peer_flag_change_type. */ +enum peer_change_type { + peer_change_none, + peer_change_reset, + peer_change_reset_in, + peer_change_reset_out, +}; + extern struct bgp_master *bm; extern unsigned int multipath_num; @@ -1597,6 +1605,8 @@ extern int peer_af_flag_unset(struct peer *, afi_t, safi_t, uint32_t); extern int peer_af_flag_check(struct peer *, afi_t, safi_t, uint32_t); extern void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi, uint32_t flag); +extern void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, + enum peer_change_type type); extern int peer_ebgp_multihop_set(struct peer *, int); extern int peer_ebgp_multihop_unset(struct peer *); diff --git a/bgpd/subdir.am b/bgpd/subdir.am index 83f55939ce..a13f058f74 100644 --- a/bgpd/subdir.am +++ b/bgpd/subdir.am @@ -46,6 +46,7 @@ man8 += $(MANBUILD)/bgpd.8 endif bgpd_libbgp_a_SOURCES = \ + bgpd/bgp_addpath.c \ bgpd/bgp_advertise.c \ bgpd/bgp_aspath.c \ bgpd/bgp_attr.c \ @@ -115,6 +116,8 @@ bgpd_libbgp_a_SOURCES += \ endif noinst_HEADERS += \ + bgpd/bgp_addpath.h \ + bgpd/bgp_addpath_types.h \ bgpd/bgp_advertise.h \ bgpd/bgp_aspath.h \ bgpd/bgp_attr.h \ diff --git a/tests/.gitignore b/tests/.gitignore index 37cd245de0..cd30d4622b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -19,23 +19,24 @@ /lib/cli/test_commands_defun.c /lib/test_buffer /lib/test_checksum +/lib/test_graph /lib/test_heavy /lib/test_heavy_thread /lib/test_heavy_wq +/lib/test_idalloc /lib/test_memory /lib/test_nexthop_iter /lib/test_privs /lib/test_ringbuf -/lib/test_srcdest_table /lib/test_segv /lib/test_sig +/lib/test_srcdest_table /lib/test_stream /lib/test_table /lib/test_timer_correctness /lib/test_timer_performance /lib/test_ttable -/lib/test_zmq /lib/test_zlog -/lib/test_graph +/lib/test_zmq /ospf6d/test_lsdb /ospf6d/test_lsdb_clippy.c diff --git a/tests/bgpd/test_bgp_table.c b/tests/bgpd/test_bgp_table.c index 171b1aa22f..73243dcacf 100644 --- a/tests/bgpd/test_bgp_table.c +++ b/tests/bgpd/test_bgp_table.c @@ -28,6 +28,8 @@ #include "bgpd/bgp_table.h" #include "linklist.h" +/* Satisfy link requirements from including bgpd.h */ +struct zebra_privs_t bgpd_privs = {0}; /* * test_node_t * diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 2f464d7217..2fbc686e1e 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -254,6 +254,8 @@ TEST_STR_ATTR_HANDLER_DECL(password, password, "FRR-Peer", "FRR-Group"); TEST_ATTR_HANDLER_DECL(local_as, change_local_as, 1, 2); TEST_ATTR_HANDLER_DECL(timers_1, keepalive, 10, 20); TEST_ATTR_HANDLER_DECL(timers_2, holdtime, 30, 60); +TEST_ATTR_HANDLER_DECL(addpath_types, addpath_type[pa->afi][pa->safi], + BGP_ADDPATH_ALL, BGP_ADDPATH_BEST_PER_AS); TEST_SU_ATTR_HANDLER_DECL(update_source_su, update_source, "255.255.255.1", "255.255.255.2"); TEST_STR_ATTR_HANDLER_DECL(update_source_if, update_if, "IF-PEER", "IF-GROUP"); @@ -414,12 +416,11 @@ static struct test_peer_attr test_peer_attrs[] = { /* Address Family Attributes */ { - .cmd = "addpath-tx-all-paths", - .u.flag = PEER_FLAG_ADDPATH_TX_ALL_PATHS, - }, - { - .cmd = "addpath-tx-bestpath-per-AS", - .u.flag = PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS, + .cmd = "addpath", + .peer_cmd = "addpath-tx-all-paths", + .group_cmd = "addpath-tx-bestpath-per-AS", + .type = PEER_AT_AF_CUSTOM, + .handlers[0] = TEST_HANDLER(addpath_types), }, { .cmd = "allowas-in", diff --git a/tests/bgpd/test_peer_attr.py b/tests/bgpd/test_peer_attr.py index 7a4dd2b04a..bafd131b78 100644 --- a/tests/bgpd/test_peer_attr.py +++ b/tests/bgpd/test_peer_attr.py @@ -26,14 +26,10 @@ TestFlag.okfail('peer\\timers') TestFlag.okfail('peer\\timers connect') TestFlag.okfail('peer\\update-source') TestFlag.okfail('peer\\update-source') -TestFlag.okfail('peer\\ipv4-unicast\\addpath-tx-all-paths') -TestFlag.okfail('peer\\ipv4-multicast\\addpath-tx-all-paths') -TestFlag.okfail('peer\\ipv6-unicast\\addpath-tx-all-paths') -TestFlag.okfail('peer\\ipv6-multicast\\addpath-tx-all-paths') -TestFlag.okfail('peer\\ipv4-unicast\\addpath-tx-bestpath-per-AS') -TestFlag.okfail('peer\\ipv4-multicast\\addpath-tx-bestpath-per-AS') -TestFlag.okfail('peer\\ipv6-unicast\\addpath-tx-bestpath-per-AS') -TestFlag.okfail('peer\\ipv6-multicast\\addpath-tx-bestpath-per-AS') +TestFlag.okfail('peer\\ipv4-unicast\\addpath') +TestFlag.okfail('peer\\ipv4-multicast\\addpath') +TestFlag.okfail('peer\\ipv6-unicast\\addpath') +TestFlag.okfail('peer\\ipv6-multicast\\addpath') TestFlag.okfail('peer\\ipv4-unicast\\allowas-in') TestFlag.okfail('peer\\ipv4-multicast\\allowas-in') TestFlag.okfail('peer\\ipv6-unicast\\allowas-in')