From 16bb31595793c0d6dd81ff104932d52c4411f7d8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 7 Nov 2024 11:29:51 -0500 Subject: [PATCH 1/4] bgpd: Pass in the prefix instead of looking it up again In an attempt to make the code faster let's just pass in the prefix instead of having to do a lookup a majillion times again after we already have it. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 74b227f97a..5b9b9e3db6 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4388,12 +4388,9 @@ void bgp_rib_remove(struct bgp_dest *dest, struct bgp_path_info *pi, bgp_process(peer->bgp, dest, pi, afi, safi); } -static void bgp_rib_withdraw(struct bgp_dest *dest, struct bgp_path_info *pi, - struct peer *peer, afi_t afi, safi_t safi, - struct prefix_rd *prd) +static void bgp_rib_withdraw(const struct prefix *p, struct bgp_dest *dest, struct bgp_path_info *pi, + struct peer *peer, afi_t afi, safi_t safi, struct prefix_rd *prd) { - const struct prefix *p = bgp_dest_get_prefix(dest); - /* apply dampening, if result is suppressed, we'll be retaining * the bgp_path_info in the RIB for historical reference. */ @@ -5568,7 +5565,7 @@ void bgp_withdraw(struct peer *peer, const struct prefix *p, /* Withdraw specified route from routing table. */ if (pi && !CHECK_FLAG(pi->flags, BGP_PATH_HISTORY)) { - bgp_rib_withdraw(dest, pi, peer, afi, safi, prd); + bgp_rib_withdraw(p, dest, pi, peer, afi, safi, prd); if (SAFI_UNICAST == safi && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) { From ea4823964c46f9537c89287439aec24816b27592 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 7 Nov 2024 11:31:59 -0500 Subject: [PATCH 2/4] bgpd: In bgp_withdraw attempt to avoid a if statement on every pass We have this: if ( (safi == SAFI_UNICAST) && ...) do stuff if ( (safi == SAFI_MPLS_VPN) && ... ) do stuff this leads to having to test safi multiple times if safi is SAFI_UNICAST. Let's make it a else if as that we know that the safi is going to not change. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5b9b9e3db6..6dc2e449ab 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5570,12 +5570,8 @@ void bgp_withdraw(struct peer *peer, const struct prefix *p, && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) { vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, pi); - } - if ((SAFI_MPLS_VPN == safi) - && (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) { - + } else if ((SAFI_MPLS_VPN == safi) && (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) vpn_leak_to_vrf_withdraw(pi); - } } else if (bgp_debug_update(peer, p, NULL, 1)) { bgp_debug_rdpfxpath2str(afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, addpath_id, NULL, From fc818fe6ad458d7edc6b2d0c9f12fc92945f3672 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 7 Nov 2024 11:50:23 -0500 Subject: [PATCH 3/4] bgpd: Mark debugs as unlikely in bgp_withdraw Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 6dc2e449ab..bc7904bde7 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5555,7 +5555,7 @@ void bgp_withdraw(struct peer *peer, const struct prefix *p, break; /* Logging. */ - if (bgp_debug_update(peer, p, NULL, 1)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { bgp_debug_rdpfxpath2str(afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, addpath_id, NULL, pfx_buf, sizeof(pfx_buf)); @@ -5572,7 +5572,7 @@ void bgp_withdraw(struct peer *peer, const struct prefix *p, vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp, pi); } else if ((SAFI_MPLS_VPN == safi) && (bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) vpn_leak_to_vrf_withdraw(pi); - } else if (bgp_debug_update(peer, p, NULL, 1)) { + } else if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { bgp_debug_rdpfxpath2str(afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, addpath_id, NULL, pfx_buf, sizeof(pfx_buf)); From bd03373c37a350773b74976feb98c1b2a73e4c98 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 7 Nov 2024 11:55:56 -0500 Subject: [PATCH 4/4] bgpd: Add unlikely for debugs in bgp_update() Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index bc7904bde7..fe5f4425f5 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5035,7 +5035,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, 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)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { bgp_debug_rdpfxpath2str( afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, @@ -5053,7 +5053,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, } } else /* Duplicate - odd */ { - if (bgp_debug_update(peer, p, NULL, 1)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { if (!peer->rcvd_attr_printed) { zlog_debug( "%pBP rcvd UPDATE w/ attr: %s", @@ -5089,7 +5089,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, /* Withdraw/Announce before we fully processed the withdraw */ if (CHECK_FLAG(pi->flags, BGP_PATH_REMOVED)) { - if (bgp_debug_update(peer, p, NULL, 1)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { bgp_debug_rdpfxpath2str( afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, addpath_id, evpn, @@ -5117,7 +5117,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, } /* Received Logging. */ - if (bgp_debug_update(peer, p, NULL, 1)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { bgp_debug_rdpfxpath2str(afi, safi, prd, p, label, num_labels, addpath_id ? 1 : 0, addpath_id, evpn, pfx_buf, @@ -5188,7 +5188,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, bgp_attr_get_ecommunity(pi->attr), bgp_attr_get_ecommunity(attr_new)); if (!cmp) { - if (bgp_debug_update(peer, p, NULL, 1)) + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) zlog_debug( "Change in EXT-COMM, existing %s new %s", ecommunity_str( @@ -5298,7 +5298,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, * EVPN nexthop is decremented appropriately. */ else if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID)) { - if (BGP_DEBUG(nht, NHT)) + if (unlikely(BGP_DEBUG(nht, NHT))) zlog_debug("%s unimport EVPN %pFX as pi %p is not VALID", __func__, p, pi); bgp_evpn_unimport_route(bgp, afi, safi, p, pi); @@ -5338,7 +5338,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, } // End of implicit withdraw /* Received Logging. */ - if (bgp_debug_update(peer, p, NULL, 1)) { + if (unlikely(bgp_debug_update(peer, p, NULL, 1))) { if (!peer->rcvd_attr_printed) { zlog_debug("%pBP rcvd UPDATE w/ attr: %s", peer, peer->rcvd_attr_str);