From b56758dae81c59f1b901b9e82e9fa0e7fd6dd118 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 25 Feb 2024 07:52:25 -0500 Subject: [PATCH 1/4] bgpd: Testing for valid pointer is done by for loop No need to test for valid pointer as that the for loop will do so as well. This reduces indentation. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 58 ++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index c61ffbd55834..6c2fa91cfa96 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2777,41 +2777,35 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, } new_select = pi1; - if (pi1->next) { - for (pi2 = pi1->next; pi2; pi2 = pi2->next) { - if (CHECK_FLAG(pi2->flags, - BGP_PATH_DMED_CHECK)) - continue; - if (BGP_PATH_HOLDDOWN(pi2)) - continue; - if (pi2->peer != bgp->peer_self && - !CHECK_FLAG(pi2->peer->sflags, - PEER_STATUS_NSF_WAIT) && - !peer_established( - pi2->peer->connection)) - continue; - - if (!aspath_cmp_left(pi1->attr->aspath, - pi2->attr->aspath) - && !aspath_cmp_left_confed( - pi1->attr->aspath, - pi2->attr->aspath)) - continue; + for (pi2 = pi1->next; pi2; pi2 = pi2->next) { + if (CHECK_FLAG(pi2->flags, BGP_PATH_DMED_CHECK)) + continue; + if (BGP_PATH_HOLDDOWN(pi2)) + continue; + if (pi2->peer != bgp->peer_self && + !CHECK_FLAG(pi2->peer->sflags, + PEER_STATUS_NSF_WAIT) && + !peer_established(pi2->peer->connection)) + continue; - if (bgp_path_info_cmp( - bgp, pi2, new_select, - &paths_eq, mpath_cfg, debug, - pfx_buf, afi, safi, - &dest->reason)) { - bgp_path_info_unset_flag( - dest, new_select, - BGP_PATH_DMED_SELECTED); - new_select = pi2; - } + if (!aspath_cmp_left(pi1->attr->aspath, + pi2->attr->aspath) && + !aspath_cmp_left_confed(pi1->attr->aspath, + pi2->attr->aspath)) + continue; - bgp_path_info_set_flag( - dest, pi2, BGP_PATH_DMED_CHECK); + if (bgp_path_info_cmp(bgp, pi2, new_select, + &paths_eq, mpath_cfg, + debug, pfx_buf, afi, safi, + &dest->reason)) { + bgp_path_info_unset_flag(dest, + new_select, + BGP_PATH_DMED_SELECTED); + new_select = pi2; } + + bgp_path_info_set_flag(dest, pi2, + BGP_PATH_DMED_CHECK); } bgp_path_info_set_flag(dest, new_select, BGP_PATH_DMED_CHECK); From 4d307c991427f40f1481ee938db5d8328f07a5f0 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 25 Feb 2024 07:59:25 -0500 Subject: [PATCH 2/4] bgpd: Both possible paths unset a flag, so reduce Both paths through the code unset a flag, so reduce the duplication. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 6c2fa91cfa96..54ab15a9f1dd 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2866,9 +2866,10 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, continue; } - if (CHECK_FLAG(bgp->flags, BGP_FLAG_DETERMINISTIC_MED) - && (!CHECK_FLAG(pi->flags, BGP_PATH_DMED_SELECTED))) { - bgp_path_info_unset_flag(dest, pi, BGP_PATH_DMED_CHECK); + bgp_path_info_unset_flag(dest, pi, BGP_PATH_DMED_CHECK); + + if (CHECK_FLAG(bgp->flags, BGP_FLAG_DETERMINISTIC_MED) && + (!CHECK_FLAG(pi->flags, BGP_PATH_DMED_SELECTED))) { if (debug) zlog_debug("%s: %pBD(%s) pi %s dmed", __func__, dest, bgp->name_pretty, @@ -2876,8 +2877,6 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, continue; } - bgp_path_info_unset_flag(dest, pi, BGP_PATH_DMED_CHECK); - reason = dest->reason; if (bgp_path_info_cmp(bgp, pi, new_select, &paths_eq, mpath_cfg, debug, pfx_buf, afi, safi, From f9c86734e52e9850ea5e7474f29d8c88c2e90836 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 25 Feb 2024 08:04:15 -0500 Subject: [PATCH 3/4] bgpd: Allow string creation to handle NULL case Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 54ab15a9f1dd..c91745030da8 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -572,6 +572,11 @@ void bgp_path_info_path_with_addpath_rx_str(struct bgp_path_info *pi, char *buf, { struct peer *peer; + if (!pi) { + snprintf(buf, buf_len, "NONE"); + return; + } + if (pi->sub_type == BGP_ROUTE_IMPORTED && bgp_get_imported_bpi_ultimate(pi)) peer = bgp_get_imported_bpi_ultimate(pi)->peer; @@ -2893,11 +2898,8 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, * qualify as multipaths */ if (debug) { - if (new_select) - bgp_path_info_path_with_addpath_rx_str( - new_select, path_buf, sizeof(path_buf)); - else - snprintf(path_buf, sizeof(path_buf), "NONE"); + bgp_path_info_path_with_addpath_rx_str(new_select, path_buf, + sizeof(path_buf)); zlog_debug( "%pBD(%s): After path selection, newbest is %s oldbest was %s", dest, bgp->name_pretty, path_buf, From 0a8dfbec4531056d1451ed2a397f9fa5af9e9284 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 25 Feb 2024 08:07:21 -0500 Subject: [PATCH 4/4] bgpd: Simplify for loop This for loop has no chance of removing entries so there is no need to do a bit of complicated code to handle the case where an entry can be removed. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index c91745030da8..45951e9ea88c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2907,9 +2907,7 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, } if (do_mpath && new_select) { - for (pi = bgp_dest_get_bgp_path_info(dest); - (pi != NULL) && (nextpi = pi->next, 1); pi = nextpi) { - + for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) { if (debug) bgp_path_info_path_with_addpath_rx_str( pi, path_buf, sizeof(path_buf));