From 7d0d37de0ceb274f60a06126ba0c6c889f83d808 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 17 Dec 2020 16:33:11 -0500 Subject: [PATCH 1/3] bgpd: Somewhat optimize string returns There is no need for a cascading series of if statements for the afi. Clean it up slightly Signed-off-by: Donald Sharp --- bgpd/bgp_vty.c | 172 ++++++++++++++++++++++++++----------------------- 1 file changed, 92 insertions(+), 80 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index b77c4b2297..cfac0bbb54 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -209,34 +209,38 @@ static enum node_type bgp_node_type(afi_t afi, safi_t safi) static const char *get_afi_safi_vty_str(afi_t afi, safi_t safi) { - if (afi == AFI_IP && safi == SAFI_UNICAST) - return "IPv4 Unicast"; - else if (afi == AFI_IP && safi == SAFI_MULTICAST) - return "IPv4 Multicast"; - else if (afi == AFI_IP && safi == SAFI_LABELED_UNICAST) - return "IPv4 Labeled Unicast"; - else if (afi == AFI_IP && safi == SAFI_MPLS_VPN) - return "IPv4 VPN"; - else if (afi == AFI_IP && safi == SAFI_ENCAP) - return "IPv4 Encap"; - else if (afi == AFI_IP && safi == SAFI_FLOWSPEC) - return "IPv4 Flowspec"; - else if (afi == AFI_IP6 && safi == SAFI_UNICAST) - return "IPv6 Unicast"; - else if (afi == AFI_IP6 && safi == SAFI_MULTICAST) - return "IPv6 Multicast"; - else if (afi == AFI_IP6 && safi == SAFI_LABELED_UNICAST) - return "IPv6 Labeled Unicast"; - else if (afi == AFI_IP6 && safi == SAFI_MPLS_VPN) - return "IPv6 VPN"; - else if (afi == AFI_IP6 && safi == SAFI_ENCAP) - return "IPv6 Encap"; - else if (afi == AFI_IP6 && safi == SAFI_FLOWSPEC) - return "IPv6 Flowspec"; - else if (afi == AFI_L2VPN && safi == SAFI_EVPN) - return "L2VPN EVPN"; - else - return "Unknown"; + if (afi == AFI_IP) { + if (safi == SAFI_UNICAST) + return "IPv4 Unicast"; + if (safi == SAFI_MULTICAST) + return "IPv4 Multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "IPv4 Labeled Unicast"; + if (safi == SAFI_MPLS_VPN) + return "IPv4 VPN"; + if (safi == SAFI_ENCAP) + return "IPv4 Encap"; + if (safi == SAFI_FLOWSPEC) + return "IPv4 Flowspec"; + } else if (afi == AFI_IP6) { + if (safi == SAFI_UNICAST) + return "IPv6 Unicast"; + if (safi == SAFI_MULTICAST) + return "IPv6 Multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "IPv6 Labeled Unicast"; + if (safi == SAFI_MPLS_VPN) + return "IPv6 VPN"; + if (safi == SAFI_ENCAP) + return "IPv6 Encap"; + if (safi == SAFI_FLOWSPEC) + return "IPv6 Flowspec"; + } else if (afi == AFI_L2VPN) { + if (safi == SAFI_EVPN) + return "L2VPN EVPN"; + } + + return "Unknown"; } /* @@ -247,34 +251,38 @@ static const char *get_afi_safi_vty_str(afi_t afi, safi_t safi) */ static const char *get_afi_safi_json_str(afi_t afi, safi_t safi) { - if (afi == AFI_IP && safi == SAFI_UNICAST) - return "ipv4Unicast"; - else if (afi == AFI_IP && safi == SAFI_MULTICAST) - return "ipv4Multicast"; - else if (afi == AFI_IP && safi == SAFI_LABELED_UNICAST) - return "ipv4LabeledUnicast"; - else if (afi == AFI_IP && safi == SAFI_MPLS_VPN) - return "ipv4Vpn"; - else if (afi == AFI_IP && safi == SAFI_ENCAP) - return "ipv4Encap"; - else if (afi == AFI_IP && safi == SAFI_FLOWSPEC) - return "ipv4Flowspec"; - else if (afi == AFI_IP6 && safi == SAFI_UNICAST) - return "ipv6Unicast"; - else if (afi == AFI_IP6 && safi == SAFI_MULTICAST) - return "ipv6Multicast"; - else if (afi == AFI_IP6 && safi == SAFI_LABELED_UNICAST) - return "ipv6LabeledUnicast"; - else if (afi == AFI_IP6 && safi == SAFI_MPLS_VPN) - return "ipv6Vpn"; - else if (afi == AFI_IP6 && safi == SAFI_ENCAP) - return "ipv6Encap"; - else if (afi == AFI_IP6 && safi == SAFI_FLOWSPEC) - return "ipv6Flowspec"; - else if (afi == AFI_L2VPN && safi == SAFI_EVPN) - return "l2VpnEvpn"; - else - return "Unknown"; + if (afi == AFI_IP) { + if (safi == SAFI_UNICAST) + return "ipv4Unicast"; + if (safi == SAFI_MULTICAST) + return "ipv4Multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "ipv4LabeledUnicast"; + if (safi == SAFI_MPLS_VPN) + return "ipv4Vpn"; + if (safi == SAFI_ENCAP) + return "ipv4Encap"; + if (safi == SAFI_FLOWSPEC) + return "ipv4Flowspec"; + } else if (afi == AFI_IP6) { + if (safi == SAFI_UNICAST) + return "ipv6Unicast"; + if (safi == SAFI_MULTICAST) + return "ipv6Multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "ipv6LabeledUnicast"; + if (safi == SAFI_MPLS_VPN) + return "ipv6Vpn"; + if (safi == SAFI_ENCAP) + return "ipv6Encap"; + if (safi == SAFI_FLOWSPEC) + return "ipv6Flowspec"; + } else if (afi == AFI_L2VPN) { + if (safi == SAFI_EVPN) + return "l2VpnEvpn"; + } + + return "Unknown"; } /* return string maps to afi-safi specific container names @@ -282,30 +290,34 @@ static const char *get_afi_safi_json_str(afi_t afi, safi_t safi) */ const char *bgp_afi_safi_get_container_str(afi_t afi, safi_t safi) { - if (afi == AFI_IP && safi == SAFI_UNICAST) - return "ipv4-unicast"; - else if (afi == AFI_IP && safi == SAFI_MULTICAST) - return "ipv4-multicast"; - else if (afi == AFI_IP && safi == SAFI_LABELED_UNICAST) - return "ipv4-labeled-unicast"; - else if (afi == AFI_IP && safi == SAFI_MPLS_VPN) - return "l3vpn-ipv4-unicast"; - else if (afi == AFI_IP && safi == SAFI_FLOWSPEC) - return "ipv4-flowspec"; - else if (afi == AFI_IP6 && safi == SAFI_UNICAST) - return "ipv6-unicast"; - else if (afi == AFI_IP6 && safi == SAFI_MULTICAST) - return "ipv6-multicast"; - else if (afi == AFI_IP6 && safi == SAFI_LABELED_UNICAST) - return "ipv6-labeled-unicast"; - else if (afi == AFI_IP6 && safi == SAFI_MPLS_VPN) - return "l3vpn-ipv6-unicast"; - else if (afi == AFI_IP6 && safi == SAFI_FLOWSPEC) - return "ipv6-flowspec"; - else if (afi == AFI_L2VPN && safi == SAFI_EVPN) - return "l2vpn-evpn"; - else - return "Unknown"; + if (afi == AFI_IP) { + if (safi == SAFI_UNICAST) + return "ipv4-unicast"; + if (safi == SAFI_MULTICAST) + return "ipv4-multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "ipv4-labeled-unicast"; + if (safi == SAFI_MPLS_VPN) + return "l3vpn-ipv4-unicast"; + if (safi == SAFI_FLOWSPEC) + return "ipv4-flowspec"; + } else if (afi == AFI_IP6) { + if (safi == SAFI_UNICAST) + return "ipv6-unicast"; + if (safi == SAFI_MULTICAST) + return "ipv6-multicast"; + if (safi == SAFI_LABELED_UNICAST) + return "ipv6-labeled-unicast"; + if (safi == SAFI_MPLS_VPN) + return "l3vpn-ipv6-unicast"; + if (safi == SAFI_FLOWSPEC) + return "ipv6-flowspec"; + } else if (afi == AFI_L2VPN) { + if (safi == SAFI_EVPN) + return "l2vpn-evpn"; + } + + return "Unknown"; } /* Utility function to get address family from current node. */ From 3742de8d6845814fa5713f359acb0971caf32295 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 17 Dec 2020 16:36:22 -0500 Subject: [PATCH 2/3] bgpd: Use the header Signed-off-by: Donald Sharp --- bgpd/bgp_conditional_adv.c | 3 +-- bgpd/bgp_damp.c | 3 +-- bgpd/bgp_fsm.c | 3 ++- bgpd/bgp_route.c | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_conditional_adv.c b/bgpd/bgp_conditional_adv.c index b5cd1b52b7..6755def8bc 100644 --- a/bgpd/bgp_conditional_adv.c +++ b/bgpd/bgp_conditional_adv.c @@ -19,8 +19,7 @@ */ #include "bgpd/bgp_conditional_adv.h" - -const char *get_afi_safi_str(afi_t afi, safi_t safi, bool for_json); +#include "bgpd/bgp_vty.h" static route_map_result_t bgp_check_rmap_prefixes_in_bgp_table(struct bgp_table *table, diff --git a/bgpd/bgp_damp.c b/bgpd/bgp_damp.c index 94a27ead0e..f46d416c3c 100644 --- a/bgpd/bgp_damp.c +++ b/bgpd/bgp_damp.c @@ -35,8 +35,7 @@ #include "bgpd/bgp_route.h" #include "bgpd/bgp_attr.h" #include "bgpd/bgp_advertise.h" - -const char *get_afi_safi_str(afi_t afi, safi_t safi, bool for_json); +#include "bgpd/bgp_vty.h" /* Global variable to access damping configuration */ static struct bgp_damp_config damp[AFI_MAX][SAFI_MAX]; diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index ce665feb4e..a4d17cac40 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -55,10 +55,11 @@ #include "bgpd/bgp_keepalives.h" #include "bgpd/bgp_io.h" #include "bgpd/bgp_zebra.h" +#include "bgpd/bgp_vty.h" DEFINE_HOOK(peer_backward_transition, (struct peer * peer), (peer)) DEFINE_HOOK(peer_status_changed, (struct peer * peer), (peer)) -extern const char *get_afi_safi_str(afi_t afi, safi_t safi, bool for_json); + /* Definition of display strings corresponding to FSM events. This should be * kept consistent with the events defined in bgpd.h */ diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 87e03d205f..22c576d90f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -96,7 +96,7 @@ /* Extern from bgp_dump.c */ extern const char *bgp_origin_str[]; extern const char *bgp_origin_long_str[]; -const char *get_afi_safi_str(afi_t afi, safi_t safi, bool for_json); + /* PMSI strings. */ #define PMSI_TNLTYPE_STR_NO_INFO "No info" #define PMSI_TNLTYPE_STR_DEFAULT PMSI_TNLTYPE_STR_NO_INFO From d6bbfefe145edbfff42732e9f4f3bdf6653cb4b8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 17 Dec 2020 16:49:20 -0500 Subject: [PATCH 3/3] bgpd: Remove awful test of strmatch + get_afi_safi_str Remove awful test of a strmatch against a call to get_afi_safi_str. These are the easy ones as that the real decision point is/was underneath this test. This is just duplicate expensive testing. Signed-off-by: Donald Sharp --- bgpd/bgp_conditional_adv.c | 4 ---- bgpd/bgp_route.c | 16 ---------------- 2 files changed, 20 deletions(-) diff --git a/bgpd/bgp_conditional_adv.c b/bgpd/bgp_conditional_adv.c index 6755def8bc..b9ea26e862 100644 --- a/bgpd/bgp_conditional_adv.c +++ b/bgpd/bgp_conditional_adv.c @@ -197,10 +197,6 @@ static int bgp_conditional_adv_timer(struct thread *t) continue; FOREACH_AFI_SAFI (afi, safi) { - if (strmatch(get_afi_safi_str(afi, safi, true), - "Unknown")) - continue; - if (!peer->afc_nego[afi][safi]) continue; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 22c576d90f..60ad8d20e9 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -11739,10 +11739,6 @@ DEFPY (show_ip_bgp_json, ? AFI_IP : AFI_IP6; FOREACH_SAFI (safi) { - if (strmatch(get_afi_safi_str(afi, safi, true), - "Unknown")) - continue; - if (!bgp_afi_safi_peer_exists(bgp, afi, safi)) continue; @@ -11773,10 +11769,6 @@ DEFPY (show_ip_bgp_json, } else { /* show bgp all: for each AFI and SAFI*/ FOREACH_AFI_SAFI (afi, safi) { - if (strmatch(get_afi_safi_str(afi, safi, true), - "Unknown")) - continue; - if (!bgp_afi_safi_peer_exists(bgp, afi, safi)) continue; @@ -13318,10 +13310,6 @@ DEFPY (show_ip_bgp_instance_neighbor_advertised_route, afi = CHECK_FLAG(show_flags, BGP_SHOW_OPT_AFI_IP) ? AFI_IP : AFI_IP6; FOREACH_SAFI (safi) { - if (strmatch(get_afi_safi_str(afi, safi, true), - "Unknown")) - continue; - if (!bgp_afi_safi_peer_exists(bgp, afi, safi)) continue; @@ -13341,10 +13329,6 @@ DEFPY (show_ip_bgp_instance_neighbor_advertised_route, } } else { FOREACH_AFI_SAFI (afi, safi) { - if (strmatch(get_afi_safi_str(afi, safi, true), - "Unknown")) - continue; - if (!bgp_afi_safi_peer_exists(bgp, afi, safi)) continue;