From 067967b8c74820790fbc50f1fadd0b9ed35fa9c1 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 6 Nov 2021 16:35:33 +0100 Subject: [PATCH 1/4] ospfd6d: replace handler vector with array No point in using the vector code for this really. Signed-off-by: David Lamparter --- ospf6d/ospf6_lsa.c | 64 ++++++++++++++++++++++------------------- ospf6d/ospf6_lsa.h | 3 +- ospf6d/ospf6_neighbor.c | 9 +----- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c index 77f0f8f4e5..99d0de39cf 100644 --- a/ospf6d/ospf6_lsa.c +++ b/ospf6d/ospf6_lsa.c @@ -54,7 +54,7 @@ DEFINE_MTYPE_STATIC(OSPF6D, OSPF6_LSA, "OSPF6 LSA"); DEFINE_MTYPE_STATIC(OSPF6D, OSPF6_LSA_HEADER, "OSPF6 LSA header"); DEFINE_MTYPE_STATIC(OSPF6D, OSPF6_LSA_SUMMARY, "OSPF6 LSA summary"); -vector ospf6_lsa_handler_vector; +static struct ospf6_lsa_handler *lsa_handlers[OSPF6_LSTYPE_SIZE]; struct ospf6 *ospf6_get_by_lsdb(struct ospf6_lsa *lsa) { @@ -115,8 +115,13 @@ static struct ospf6_lsa_handler unknown_handler = { void ospf6_install_lsa_handler(struct ospf6_lsa_handler *handler) { /* type in handler is host byte order */ - int index = handler->lh_type & OSPF6_LSTYPE_FCODE_MASK; - vector_set_index(ospf6_lsa_handler_vector, index, (void *)handler); + unsigned int index = handler->lh_type & OSPF6_LSTYPE_FCODE_MASK; + + assertf(index < array_size(lsa_handlers), "index=%x", index); + assertf(lsa_handlers[index] == NULL, "old=%s, new=%s", + lsa_handlers[index]->lh_name, handler->lh_name); + + lsa_handlers[index] = handler; } struct ospf6_lsa_handler *ospf6_get_lsa_handler(uint16_t type) @@ -124,10 +129,8 @@ struct ospf6_lsa_handler *ospf6_get_lsa_handler(uint16_t type) struct ospf6_lsa_handler *handler = NULL; unsigned int index = ntohs(type) & OSPF6_LSTYPE_FCODE_MASK; - if (index >= vector_active(ospf6_lsa_handler_vector)) - handler = &unknown_handler; - else - handler = vector_slot(ospf6_lsa_handler_vector, index); + if (index < array_size(lsa_handlers)) + handler = lsa_handlers[index]; if (handler == NULL) handler = &unknown_handler; @@ -989,13 +992,11 @@ int ospf6_lsa_checksum_valid(struct ospf6_lsa_header *lsa_header) void ospf6_lsa_init(void) { - ospf6_lsa_handler_vector = vector_init(0); ospf6_install_lsa_handler(&unknown_handler); } void ospf6_lsa_terminate(void) { - vector_free(ospf6_lsa_handler_vector); } static char *ospf6_lsa_handler_name(const struct ospf6_lsa_handler *h) @@ -1020,6 +1021,22 @@ static char *ospf6_lsa_handler_name(const struct ospf6_lsa_handler *h) return buf; } +void ospf6_lsa_debug_set_all(bool val) +{ + unsigned int i; + struct ospf6_lsa_handler *handler = NULL; + + for (i = 0; i < array_size(lsa_handlers); i++) { + handler = lsa_handlers[i]; + if (handler == NULL) + continue; + if (val) + SET_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL); + else + UNSET_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL); + } +} + DEFPY (debug_ospf6_lsa_all, debug_ospf6_lsa_all_cmd, "[no$no] debug ospf6 lsa all", @@ -1029,18 +1046,7 @@ DEFPY (debug_ospf6_lsa_all, "Debug Link State Advertisements (LSAs)\n" "Display for all types of LSAs\n") { - unsigned int i; - struct ospf6_lsa_handler *handler = NULL; - - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); - if (handler == NULL) - continue; - if (!no) - SET_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL); - else - UNSET_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL); - } + ospf6_lsa_debug_set_all(!no); return CMD_SUCCESS; } @@ -1092,8 +1098,8 @@ DEFUN (debug_ospf6_lsa_type, unsigned int i; struct ospf6_lsa_handler *handler = NULL; - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); + for (i = 0; i < array_size(lsa_handlers); i++) { + handler = lsa_handlers[i]; if (handler == NULL) continue; if (strncmp(argv[idx_lsa]->arg, ospf6_lsa_handler_name(handler), @@ -1146,8 +1152,8 @@ DEFUN (no_debug_ospf6_lsa_type, unsigned int i; struct ospf6_lsa_handler *handler = NULL; - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); + for (i = 0; i < array_size(lsa_handlers); i++) { + handler = lsa_handlers[i]; if (handler == NULL) continue; if (strncmp(argv[idx_lsa]->arg, ospf6_lsa_handler_name(handler), @@ -1194,8 +1200,8 @@ int config_write_ospf6_debug_lsa(struct vty *vty) const struct ospf6_lsa_handler *handler; bool debug_all = true; - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); + for (i = 0; i < array_size(lsa_handlers); i++) { + handler = lsa_handlers[i]; if (handler == NULL) continue; if (CHECK_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL) @@ -1210,8 +1216,8 @@ int config_write_ospf6_debug_lsa(struct vty *vty) return 0; } - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); + for (i = 0; i < array_size(lsa_handlers); i++) { + handler = lsa_handlers[i]; if (handler == NULL) continue; if (CHECK_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG)) diff --git a/ospf6d/ospf6_lsa.h b/ospf6d/ospf6_lsa.h index c0d3cc149b..aa1150afca 100644 --- a/ospf6d/ospf6_lsa.h +++ b/ospf6d/ospf6_lsa.h @@ -173,8 +173,6 @@ struct ospf6_lsa_handler { #define OSPF6_LSA_IS_KNOWN(t) \ (ospf6_get_lsa_handler(t)->lh_type != OSPF6_LSTYPE_UNKNOWN ? 1 : 0) -extern vector ospf6_lsa_handler_vector; - /* Macro for LSA Origination */ /* addr is (struct prefix *) */ #define CONTINUE_IF_ADDRESS_LINKLOCAL(debug, addr) \ @@ -268,6 +266,7 @@ extern int ospf6_lsa_prohibited_duration(uint16_t type, uint32_t id, extern void ospf6_install_lsa_handler(struct ospf6_lsa_handler *handler); extern struct ospf6_lsa_handler *ospf6_get_lsa_handler(uint16_t type); +extern void ospf6_lsa_debug_set_all(bool val); extern void ospf6_lsa_init(void); extern void ospf6_lsa_terminate(void); diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c index 3d0dde8c65..8ec916dba8 100644 --- a/ospf6d/ospf6_neighbor.c +++ b/ospf6d/ospf6_neighbor.c @@ -1254,7 +1254,6 @@ DEFUN (no_debug_ospf6, OSPF6_STR) { unsigned int i; - struct ospf6_lsa_handler *handler = NULL; OSPF6_DEBUG_ABR_OFF(); OSPF6_DEBUG_ASBR_OFF(); @@ -1264,13 +1263,7 @@ DEFUN (no_debug_ospf6, OSPF6_DEBUG_FLOODING_OFF(); OSPF6_DEBUG_INTERFACE_OFF(); - for (i = 0; i < vector_active(ospf6_lsa_handler_vector); i++) { - handler = vector_slot(ospf6_lsa_handler_vector, i); - - if (handler != NULL) { - UNSET_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG); - } - } + ospf6_lsa_debug_set_all(false); for (i = 0; i < 6; i++) OSPF6_DEBUG_MESSAGE_OFF(i, From 88711d8a91333869e77b1e9abe0a39c48d5787c4 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 6 Nov 2021 16:57:52 +0100 Subject: [PATCH 2/4] lib: use hash for route-map set/match commands Why would this be in a vector to loop over with strcmp()'ing each item... that just makes no sense. Use a hash instead. Signed-off-by: David Lamparter --- lib/routemap.c | 81 ++++++++++++++++++++++++++++++++------------------ lib/routemap.h | 34 +++++++++++++++++++-- 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/lib/routemap.c b/lib/routemap.c index 6227ebf158..0ca2235d9d 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -34,6 +34,7 @@ #include "lib_errors.h" #include "table.h" #include "json.h" +#include "jhash.h" DEFINE_MTYPE_STATIC(LIB, ROUTE_MAP, "Route map"); DEFINE_MTYPE(LIB, ROUTE_MAP_NAME, "Route map name"); @@ -47,6 +48,27 @@ DEFINE_MTYPE_STATIC(LIB, ROUTE_MAP_DEP_DATA, "Route map dependency data"); DEFINE_QOBJ_TYPE(route_map_index); DEFINE_QOBJ_TYPE(route_map); +static int rmap_cmd_name_cmp(const struct route_map_rule_cmd_proxy *a, + const struct route_map_rule_cmd_proxy *b) +{ + return strcmp(a->cmd->str, b->cmd->str); +} + +static uint32_t rmap_cmd_name_hash(const struct route_map_rule_cmd_proxy *item) +{ + return jhash(item->cmd->str, strlen(item->cmd->str), 0xbfd69320); +} + +DECLARE_HASH(rmap_cmd_name, struct route_map_rule_cmd_proxy, itm, + rmap_cmd_name_cmp, rmap_cmd_name_hash); + +static struct rmap_cmd_name_head rmap_match_cmds[1] = { + INIT_HASH(rmap_match_cmds[0]), +}; +static struct rmap_cmd_name_head rmap_set_cmds[1] = { + INIT_HASH(rmap_set_cmds[0]), +}; + #define IPv4_PREFIX_LIST "ip address prefix-list" #define IPv6_PREFIX_LIST "ipv6 address prefix-list" @@ -61,12 +83,6 @@ struct route_map_pentry_dep { route_map_event_t event; }; -/* Vector for route match rules. */ -static vector route_match_vec; - -/* Vector for route set rules. */ -static vector route_set_vec; - static void route_map_pfx_tbl_update(route_map_event_t event, struct route_map_index *index, afi_t afi, const char *plist_name); @@ -1231,40 +1247,40 @@ static struct route_map_rule *route_map_rule_new(void) } /* Install rule command to the match list. */ -void route_map_install_match(const struct route_map_rule_cmd *cmd) +void _route_map_install_match(struct route_map_rule_cmd_proxy *proxy) { - vector_set(route_match_vec, (void *)cmd); + rmap_cmd_name_add(rmap_match_cmds, proxy); } /* Install rule command to the set list. */ -void route_map_install_set(const struct route_map_rule_cmd *cmd) +void _route_map_install_set(struct route_map_rule_cmd_proxy *proxy) { - vector_set(route_set_vec, (void *)cmd); + rmap_cmd_name_add(rmap_set_cmds, proxy); } /* Lookup rule command from match list. */ static const struct route_map_rule_cmd *route_map_lookup_match(const char *name) { - unsigned int i; - const struct route_map_rule_cmd *rule; + struct route_map_rule_cmd refcmd = {.str = name}; + struct route_map_rule_cmd_proxy ref = {.cmd = &refcmd}; + struct route_map_rule_cmd_proxy *res; - for (i = 0; i < vector_active(route_match_vec); i++) - if ((rule = vector_slot(route_match_vec, i)) != NULL) - if (strcmp(rule->str, name) == 0) - return rule; + res = rmap_cmd_name_find(rmap_match_cmds, &ref); + if (res) + return res->cmd; return NULL; } /* Lookup rule command from set list. */ static const struct route_map_rule_cmd *route_map_lookup_set(const char *name) { - unsigned int i; - const struct route_map_rule_cmd *rule; + struct route_map_rule_cmd refcmd = {.str = name}; + struct route_map_rule_cmd_proxy ref = {.cmd = &refcmd}; + struct route_map_rule_cmd_proxy *res; - for (i = 0; i < vector_active(route_set_vec); i++) - if ((rule = vector_slot(route_set_vec, i)) != NULL) - if (strcmp(rule->str, name) == 0) - return rule; + res = rmap_cmd_name_find(rmap_set_cmds, &ref); + if (res) + return res->cmd; return NULL; } @@ -3161,11 +3177,21 @@ void route_map_rule_tag_free(void *rule) void route_map_finish(void) { int i; + struct route_map_rule_cmd_proxy *proxy; - vector_free(route_match_vec); - route_match_vec = NULL; - vector_free(route_set_vec); - route_set_vec = NULL; + /* these 2 hash tables have INIT_HASH initializers, so the "default" + * state is "initialized & empty" => fini() followed by init() to + * return to that same state + */ + while ((proxy = rmap_cmd_name_pop(rmap_match_cmds))) + (void)proxy; + rmap_cmd_name_fini(rmap_match_cmds); + rmap_cmd_name_init(rmap_match_cmds); + + while ((proxy = rmap_cmd_name_pop(rmap_set_cmds))) + (void)proxy; + rmap_cmd_name_fini(rmap_set_cmds); + rmap_cmd_name_init(rmap_set_cmds); /* * All protocols are setting these to NULL @@ -3309,9 +3335,6 @@ void route_map_init(void) { int i; - /* Make vector for match and set. */ - route_match_vec = vector_init(1); - route_set_vec = vector_init(1); route_map_master_hash = hash_create_size(8, route_map_hash_key_make, route_map_hash_cmp, "Route Map Master Hash"); diff --git a/lib/routemap.h b/lib/routemap.h index f8fdc67d57..7e17e14fa6 100644 --- a/lib/routemap.h +++ b/lib/routemap.h @@ -21,6 +21,7 @@ #ifndef _ZEBRA_ROUTEMAP_H #define _ZEBRA_ROUTEMAP_H +#include "typesafe.h" #include "prefix.h" #include "memory.h" #include "qobj.h" @@ -422,8 +423,37 @@ extern enum rmap_compile_rets route_map_delete_set(struct route_map_index *index, const char *set_name, const char *set_arg); +/* struct route_map_rule_cmd is kept const in order to not have writable + * function pointers (which is a security benefit.) Hence, below struct is + * used as proxy for hashing these for by-name lookup. + */ + +PREDECL_HASH(rmap_cmd_name); + +struct route_map_rule_cmd_proxy { + struct rmap_cmd_name_item itm; + const struct route_map_rule_cmd *cmd; +}; + +/* ... and just automatically create a proxy struct for each call location + * to route_map_install_{match,set} to avoid unnecessarily added boilerplate + * for each route-map user + */ + +#define route_map_install_match(c) \ + do { \ + static struct route_map_rule_cmd_proxy proxy = {.cmd = c}; \ + _route_map_install_match(&proxy); \ + } while (0) + +#define route_map_install_set(c) \ + do { \ + static struct route_map_rule_cmd_proxy proxy = {.cmd = c}; \ + _route_map_install_set(&proxy); \ + } while (0) + /* Install rule command to the match list. */ -extern void route_map_install_match(const struct route_map_rule_cmd *cmd); +extern void _route_map_install_match(struct route_map_rule_cmd_proxy *proxy); /* * Install rule command to the set list. @@ -434,7 +464,7 @@ extern void route_map_install_match(const struct route_map_rule_cmd *cmd); * in the apply command). See 'set metric' command * as it is handled in ripd/ripngd and ospfd. */ -extern void route_map_install_set(const struct route_map_rule_cmd *cmd); +extern void _route_map_install_set(struct route_map_rule_cmd_proxy *proxy); /* Lookup route map by name. */ extern struct route_map *route_map_lookup_by_name(const char *name); From dd2c81b8c09ac8c27ca909cf16640815941df31e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 6 Nov 2021 21:10:41 +0100 Subject: [PATCH 3/4] lib: rework vty_check_node_for_xpath_decrement ...by having a flag in struct cmd_node rather than hardcoding it in `lib/command.c`. Signed-off-by: David Lamparter --- bgpd/bgp_vty.c | 11 +++++++++++ lib/command.c | 41 +++++++++-------------------------------- lib/command.h | 3 +++ vtysh/vtysh.c | 12 ++++++++++++ zebra/interface.c | 1 + 5 files changed, 36 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 3725f242e1..26591a5fce 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -17593,6 +17593,7 @@ static struct cmd_node bgp_ipv4_unicast_node = { .node = BGP_IPV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv4_multicast_node = { @@ -17600,6 +17601,7 @@ static struct cmd_node bgp_ipv4_multicast_node = { .node = BGP_IPV4M_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv4_labeled_unicast_node = { @@ -17607,6 +17609,7 @@ static struct cmd_node bgp_ipv4_labeled_unicast_node = { .node = BGP_IPV4L_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv6_unicast_node = { @@ -17614,6 +17617,7 @@ static struct cmd_node bgp_ipv6_unicast_node = { .node = BGP_IPV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv6_multicast_node = { @@ -17621,6 +17625,7 @@ static struct cmd_node bgp_ipv6_multicast_node = { .node = BGP_IPV6M_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv6_labeled_unicast_node = { @@ -17628,6 +17633,7 @@ static struct cmd_node bgp_ipv6_labeled_unicast_node = { .node = BGP_IPV6L_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_vpnv4_node = { @@ -17635,6 +17641,7 @@ static struct cmd_node bgp_vpnv4_node = { .node = BGP_VPNV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_vpnv6_node = { @@ -17642,6 +17649,7 @@ static struct cmd_node bgp_vpnv6_node = { .node = BGP_VPNV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af-vpnv6)# ", + .no_xpath = true, }; static struct cmd_node bgp_evpn_node = { @@ -17649,6 +17657,7 @@ static struct cmd_node bgp_evpn_node = { .node = BGP_EVPN_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-evpn)# ", + .no_xpath = true, }; static struct cmd_node bgp_evpn_vni_node = { @@ -17663,6 +17672,7 @@ static struct cmd_node bgp_flowspecv4_node = { .node = BGP_FLOWSPECV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_flowspecv6_node = { @@ -17670,6 +17680,7 @@ static struct cmd_node bgp_flowspecv6_node = { .node = BGP_FLOWSPECV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af-vpnv6)# ", + .no_xpath = true, }; static struct cmd_node bgp_srv6_node = { diff --git a/lib/command.c b/lib/command.c index ea66a17bb0..9cf93ea192 100644 --- a/lib/command.c +++ b/lib/command.c @@ -145,31 +145,6 @@ static struct cmd_node config_node = { .node_exit = vty_config_node_exit, }; -static bool vty_check_node_for_xpath_decrement(enum node_type target_node, - enum node_type node) -{ - /* bgp afi-safi (`address-family `) node - * does not increment xpath_index. - * In order to use (`router bgp`) BGP_NODE's xpath as a base, - * retain xpath_index as 1 upon exiting from - * afi-safi node. - */ - - if (target_node == BGP_NODE - && (node == BGP_IPV4_NODE || node == BGP_IPV6_NODE - || node == BGP_IPV4M_NODE || node == BGP_IPV6M_NODE - || node == BGP_VPNV4_NODE || node == BGP_VPNV6_NODE - || node == BGP_EVPN_NODE || node == BGP_IPV4L_NODE - || node == BGP_IPV6L_NODE || node == BGP_FLOWSPECV4_NODE - || node == BGP_FLOWSPECV6_NODE)) - return false; - - if (target_node == INTERFACE_NODE && node == LINK_PARAMS_NODE) - return false; - - return true; -} - /* This is called from main when a daemon is invoked with -v or --version. */ void print_version(const char *progname) { @@ -922,13 +897,15 @@ static int cmd_execute_command_real(vector vline, enum cmd_filter_type filter, * a match before calling node_exit handlers below */ for (i = 0; i < up_level; i++) { + struct cmd_node *cnode; + if (node <= CONFIG_NODE) return CMD_NO_LEVEL_UP; + cnode = vector_slot(cmdvec, node); node = node_parent(node); - if (xpath_index > 0 - && vty_check_node_for_xpath_decrement(node, vty->node)) + if (xpath_index > 0 && !cnode->no_xpath) xpath_index--; } @@ -1062,12 +1039,13 @@ int cmd_execute_command(vector vline, struct vty *vty, /* This assumes all nodes above CONFIG_NODE are childs of * CONFIG_NODE */ while (vty->node > CONFIG_NODE) { + struct cmd_node *cnode = vector_slot(cmdvec, try_node); + try_node = node_parent(try_node); vty->node = try_node; - if (vty->xpath_index > 0 - && vty_check_node_for_xpath_decrement(try_node, - onode)) + if (vty->xpath_index > 0 && !cnode->no_xpath) vty->xpath_index--; + ret = cmd_execute_command_real(vline, FILTER_RELAXED, vty, cmd, 0); if (ret == CMD_SUCCESS || ret == CMD_WARNING @@ -1386,8 +1364,7 @@ void cmd_exit(struct vty *vty) } if (cnode->parent_node) vty->node = cnode->parent_node; - if (vty->xpath_index > 0 - && vty_check_node_for_xpath_decrement(vty->node, cnode->node)) + if (vty->xpath_index > 0 && !cnode->no_xpath) vty->xpath_index--; } diff --git a/lib/command.h b/lib/command.h index e2086701ad..c888356d61 100644 --- a/lib/command.h +++ b/lib/command.h @@ -213,6 +213,9 @@ struct cmd_node { /* set as soon as any command is in cmdgraph */ bool graph_built; + + /* don't decrement vty->xpath_index on leaving this node */ + bool no_xpath; }; /* Return value of the commands. */ diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 2537ff4571..f6c86a321c 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -1279,6 +1279,7 @@ static struct cmd_node bgp_vpnv4_node = { .node = BGP_VPNV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_vpnv6_node = { @@ -1286,6 +1287,7 @@ static struct cmd_node bgp_vpnv6_node = { .node = BGP_VPNV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_flowspecv4_node = { @@ -1293,6 +1295,7 @@ static struct cmd_node bgp_flowspecv4_node = { .node = BGP_FLOWSPECV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_flowspecv6_node = { @@ -1300,6 +1303,7 @@ static struct cmd_node bgp_flowspecv6_node = { .node = BGP_FLOWSPECV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv4_node = { @@ -1307,6 +1311,7 @@ static struct cmd_node bgp_ipv4_node = { .node = BGP_IPV4_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv4m_node = { @@ -1314,6 +1319,7 @@ static struct cmd_node bgp_ipv4m_node = { .node = BGP_IPV4M_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv4l_node = { @@ -1321,6 +1327,7 @@ static struct cmd_node bgp_ipv4l_node = { .node = BGP_IPV4L_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv6_node = { @@ -1328,6 +1335,7 @@ static struct cmd_node bgp_ipv6_node = { .node = BGP_IPV6_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_ipv6m_node = { @@ -1335,6 +1343,7 @@ static struct cmd_node bgp_ipv6m_node = { .node = BGP_IPV6M_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_evpn_node = { @@ -1342,6 +1351,7 @@ static struct cmd_node bgp_evpn_node = { .node = BGP_EVPN_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; static struct cmd_node bgp_evpn_vni_node = { @@ -1356,6 +1366,7 @@ static struct cmd_node bgp_ipv6l_node = { .node = BGP_IPV6L_NODE, .parent_node = BGP_NODE, .prompt = "%s(config-router-af)# ", + .no_xpath = true, }; #ifdef ENABLE_BGP_VNC @@ -1516,6 +1527,7 @@ struct cmd_node link_params_node = { .node = LINK_PARAMS_NODE, .parent_node = INTERFACE_NODE, .prompt = "%s(config-link-params)# ", + .no_xpath = true, }; #ifdef HAVE_BGPD diff --git a/zebra/interface.c b/zebra/interface.c index 49a1e49175..c5fe5f3949 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -2905,6 +2905,7 @@ struct cmd_node link_params_node = { .node = LINK_PARAMS_NODE, .parent_node = INTERFACE_NODE, .prompt = "%s(config-link-params)# ", + .no_xpath = true, }; static void link_param_cmd_set_uint32(struct interface *ifp, uint32_t *field, From 0beb61abc21c082e18dc1f1d78e256ca57cff337 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 16 Nov 2021 13:29:44 +0100 Subject: [PATCH 4/4] vtysh: dispatch unique-id backtrace cmd properly i.e. to whoever cares, since some unique IDs (from libfrr) are valid everywhere but some others (from the daemons) only apply to specific daemons. (Default handling aborts on first error, so configuring any unique IDs that don't exist on the first daemon vtysh connects to just failed before this.) Signed-off-by: David Lamparter --- lib/log_vty.c | 23 ++++++++++----------- vtysh/vtysh.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/lib/log_vty.c b/lib/log_vty.c index 9911323553..621949ab57 100644 --- a/lib/log_vty.c +++ b/lib/log_vty.c @@ -269,14 +269,14 @@ DEFUN_HIDDEN (no_config_log_monitor, return CMD_SUCCESS; } -DEFPY (debug_uid_backtrace, - debug_uid_backtrace_cmd, - "[no] debug unique-id UID backtrace", - NO_STR - DEBUG_STR - "Options per individual log message, by unique ID\n" - "Log message unique ID (XXXXX-XXXXX)\n" - "Add backtrace to log when message is printed\n") +DEFPY_NOSH (debug_uid_backtrace, + debug_uid_backtrace_cmd, + "[no] debug unique-id UID backtrace", + NO_STR + DEBUG_STR + "Options per individual log message, by unique ID\n" + "Log message unique ID (XXXXX-XXXXX)\n" + "Add backtrace to log when message is printed\n") { struct xrefdata search, *xrd; struct xrefdata_logmsg *xrdl; @@ -285,10 +285,9 @@ DEFPY (debug_uid_backtrace, strlcpy(search.uid, uid, sizeof(search.uid)); xrd = xrefdata_uid_find(&xrefdata_uid, &search); - if (!xrd) { - vty_out(vty, "%% no log message with ID \"%s\" found\n", uid); - return CMD_WARNING; - } + if (!xrd) + return CMD_ERR_NOTHING_TODO; + if (xrd->xref->type != XREFT_LOGMSG) { vty_out(vty, "%% ID \"%s\" is not a log message\n", uid); return CMD_WARNING; diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index f6c86a321c..e695c45dd8 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -3035,6 +3035,60 @@ DEFUNSH(VTYSH_ALL, vtysh_debug_memstats, return CMD_SUCCESS; } +DEFUN(vtysh_debug_uid_backtrace, + vtysh_debug_uid_backtrace_cmd, + "[no] debug unique-id UID backtrace", + NO_STR + DEBUG_STR + "Options per individual log message, by unique ID\n" + "Log message unique ID (XXXXX-XXXXX)\n" + "Add backtrace to log when message is printed\n") +{ + unsigned int i, ok = 0; + int err = CMD_SUCCESS, ret; + const char *uid; + char line[64]; + + if (!strcmp(argv[0]->text, "no")) { + uid = argv[3]->arg; + snprintfrr(line, sizeof(line), + "no debug unique-id %s backtrace", uid); + } else { + uid = argv[2]->arg; + snprintfrr(line, sizeof(line), "debug unique-id %s backtrace", + uid); + } + + for (i = 0; i < array_size(vtysh_client); i++) + if (vtysh_client[i].fd >= 0 || vtysh_client[i].next) { + ret = vtysh_client_execute(&vtysh_client[i], line); + switch (ret) { + case CMD_SUCCESS: + ok++; + break; + case CMD_ERR_NOTHING_TODO: + /* ignore this daemon + * + * note this doesn't need to handle instances + * of the same daemon individually because + * the same daemon will have the same UIDs + */ + break; + default: + if (err == CMD_SUCCESS) + err = ret; + break; + } + } + + if (err == CMD_SUCCESS && !ok) { + vty_out(vty, "%% no running daemon recognizes unique-ID %s\n", + uid); + err = CMD_WARNING; + } + return err; +} + DEFUNSH(VTYSH_ALL, vtysh_service_password_encrypt, vtysh_service_password_encrypt_cmd, "service password-encryption", "Set up miscellaneous service\n" @@ -4461,6 +4515,8 @@ void vtysh_init_vty(void) install_element(CONFIG_NODE, &vtysh_debug_all_cmd); install_element(ENABLE_NODE, &vtysh_debug_memstats_cmd); install_element(CONFIG_NODE, &vtysh_debug_memstats_cmd); + install_element(ENABLE_NODE, &vtysh_debug_uid_backtrace_cmd); + install_element(CONFIG_NODE, &vtysh_debug_uid_backtrace_cmd); /* northbound */ install_element(ENABLE_NODE, &show_config_running_cmd);