From be393ade4acf0fcab2bdcc730a09281895af5e6b Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 25 May 2023 15:46:21 +0300 Subject: [PATCH] bgpd: Refactor subgroup_announce_table() to reuse an existing helpers Reuse subgroup_process_announce_selected(). It does the same as we do here duplicating the logic. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 8 ++--- bgpd/bgp_route.h | 3 +- bgpd/bgp_updgrp_adv.c | 77 ++++++++++++++----------------------------- 3 files changed, 29 insertions(+), 59 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 85f08bbed5..7b7a7ad207 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2894,20 +2894,16 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, */ void subgroup_process_announce_selected(struct update_subgroup *subgrp, struct bgp_path_info *selected, - struct bgp_dest *dest, - uint32_t addpath_tx_id) + struct bgp_dest *dest, afi_t afi, + safi_t safi, uint32_t addpath_tx_id) { const struct prefix *p; struct peer *onlypeer; struct attr attr; - afi_t afi; - safi_t safi; struct bgp *bgp; bool advertise; p = bgp_dest_get_prefix(dest); - afi = SUBGRP_AFI(subgrp); - safi = SUBGRP_SAFI(subgrp); bgp = SUBGRP_INST(subgrp); onlypeer = ((SUBGRP_PCOUNT(subgrp) == 1) ? (SUBGRP_PFIRST(subgrp))->peer : NULL); diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index fbdd5fae7d..9bd9e48e22 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -813,7 +813,8 @@ extern void bgp_notify_conditional_adv_scanner(struct update_subgroup *subgrp); extern void subgroup_process_announce_selected(struct update_subgroup *subgrp, struct bgp_path_info *selected, - struct bgp_dest *dest, + struct bgp_dest *dest, afi_t afi, + safi_t safi, uint32_t addpath_tx_id); extern bool subgroup_announce_check(struct bgp_dest *dest, diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index e4bc0176d1..33617811cf 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -114,8 +114,9 @@ static void subgrp_withdraw_stale_addpath(struct updwalk_context *ctx, } if (!pi) { - subgroup_process_announce_selected( - subgrp, NULL, ctx->dest, adj->addpath_tx_id); + subgroup_process_announce_selected(subgrp, NULL, + ctx->dest, afi, safi, + adj->addpath_tx_id); } } } @@ -161,7 +162,8 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) continue; subgroup_process_announce_selected( - subgrp, pi, ctx->dest, + subgrp, pi, ctx->dest, afi, + safi, bgp_addpath_id_for_peer( peer, afi, safi, &pi->tx_addpath)); @@ -173,7 +175,8 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) */ if (ctx->pi) subgroup_process_announce_selected( - subgrp, ctx->pi, ctx->dest, + subgrp, ctx->pi, ctx->dest, afi, + safi, bgp_addpath_id_for_peer( peer, afi, safi, &ctx->pi->tx_addpath)); @@ -182,7 +185,8 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) else { if (ctx->pi) { subgroup_process_announce_selected( - subgrp, ctx->pi, ctx->dest, + subgrp, ctx->pi, ctx->dest, afi, + safi, bgp_addpath_id_for_peer( peer, afi, safi, &ctx->pi->tx_addpath)); @@ -196,7 +200,8 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg) if (adj->subgroup == subgrp) { subgroup_process_announce_selected( subgrp, NULL, - ctx->dest, + ctx->dest, afi, + safi, adj->addpath_tx_id); } } @@ -653,19 +658,15 @@ void subgroup_announce_table(struct update_subgroup *subgrp, { struct bgp_dest *dest; struct bgp_path_info *ri; - struct attr attr; struct peer *peer; afi_t afi; safi_t safi; safi_t safi_rib; bool addpath_capable; - struct bgp *bgp; - bool advertise; peer = SUBGRP_PEER(subgrp); afi = SUBGRP_AFI(subgrp); safi = SUBGRP_SAFI(subgrp); - bgp = SUBGRP_INST(subgrp); addpath_capable = bgp_addpath_encode_tx(peer, afi, safi); if (safi == SAFI_LABELED_UNICAST) @@ -685,55 +686,27 @@ void subgroup_announce_table(struct update_subgroup *subgrp, SET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); for (dest = bgp_table_top(table); dest; dest = bgp_route_next(dest)) { - const struct prefix *dest_p = bgp_dest_get_prefix(dest); - - /* Check if the route can be advertised */ - advertise = bgp_check_advertise(bgp, dest); - for (ri = bgp_dest_get_bgp_path_info(dest); ri; ri = ri->next) { if (!bgp_check_selected(ri, peer, addpath_capable, afi, safi_rib)) continue; - if (subgroup_announce_check(dest, ri, subgrp, dest_p, - &attr, NULL)) { - /* Check if route can be advertised */ - if (advertise) { - if (!bgp_check_withdrawal(bgp, dest)) { - struct attr *adv_attr = - bgp_attr_intern(&attr); + /* If default originate is enabled for + * the peer, do not send explicit + * withdraw. This will prevent deletion + * of default route advertised through + * default originate + */ + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_DEFAULT_ORIGINATE) && + is_default_prefix(bgp_dest_get_prefix(dest))) + break; - bgp_adj_out_set_subgroup( - dest, subgrp, adv_attr, - ri); - } else - bgp_adj_out_unset_subgroup( - dest, subgrp, 1, - bgp_addpath_id_for_peer( - peer, afi, - safi_rib, - &ri->tx_addpath)); - } - } else { - /* If default originate is enabled for - * the peer, do not send explicit - * withdraw. This will prevent deletion - * of default route advertised through - * default originate - */ - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_DEFAULT_ORIGINATE) && - is_default_prefix( - bgp_dest_get_prefix(dest))) - break; - - bgp_adj_out_unset_subgroup( - dest, subgrp, 1, - bgp_addpath_id_for_peer( - peer, afi, safi_rib, - &ri->tx_addpath)); - } + subgroup_process_announce_selected( + subgrp, ri, dest, afi, safi_rib, + bgp_addpath_id_for_peer(peer, afi, safi_rib, + &ri->tx_addpath)); } } UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); -- 2.39.5