From f7594b55c23fe377c6c5811879e89557e6cb4b57 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Fri, 19 Jan 2024 16:40:34 +0000 Subject: [PATCH 01/11] doc: updates to mgmtd conversion documentation Signed-off-by: Christian Hopps --- doc/developer/mgmtd-dev.rst | 172 +++++++++++++++++++++++++++++++----- 1 file changed, 150 insertions(+), 22 deletions(-) diff --git a/doc/developer/mgmtd-dev.rst b/doc/developer/mgmtd-dev.rst index d611f9c11b8d..e557d3a84a85 100644 --- a/doc/developer/mgmtd-dev.rst +++ b/doc/developer/mgmtd-dev.rst @@ -12,7 +12,7 @@ MGMTD Development ================= Overview -^^^^^^^^ +-------- ``mgmtd`` (Management Daemon) is a new centralized management daemon for FRR. @@ -33,9 +33,55 @@ each daemon. ``mgmtd`` currently provides the CLI interface for each daemon that has been converted to it, but in the future RESTCONF and NETCONF servers can easily be added as *front-ends* to mgmtd to support those protocols as well. +Conversion Status +^^^^^^^^^^^^^^^^^ + +Fully Converted To MGMTD +"""""""""""""""""""""""" + +- lib/distribute +- lib/filter +- lib/if_rmap +- lib/routemap +- ripd +- staticd +- zebra (* - partial) + +Converted To Northbound +""""""""""""""""""""""" +- bfdd +- lib/affinitymap +- lib/if +- pathd +- pbrd +- pimd +- ripngd + +Converted To Northbound With Issues +""""""""""""""""""""""""""""""""""" +- eigrp +- isisd + +Unconverted +""""""""""" +- babel +- bgpd +- ldpd +- lib/event +- lib/keychain +- lib/log_vty +- lib/nexthop_group +- lib/zlog_5424_cli +- nhrpd +- ospfd +- ospf6d +- pceplib +- qdb +- sharpd +- vrrpd Converting A Daemon to MGMTD -^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +---------------------------- A daemon must first be transitioned to the new :ref:`northbound` interface if that has not already been done (see :ref:`nb-retrofit` for how to do this). Once this @@ -43,7 +89,7 @@ is done a few simple steps are all that is required move the daemon over to ``mgmtd`` control. Overview of Changes -------------------- +^^^^^^^^^^^^^^^^^^^ Adding support for a *northbound* converted daemon involves very little work. It requires enabling *frontend* (CLI and YANG) and *backend* (YANG) support. @@ -51,31 +97,78 @@ requires enabling *frontend* (CLI and YANG) and *backend* (YANG) support. Front-End Interface: -1. Add YANG module file to ``mgmtd/subdir.am`` (e.g., ``yang/frr-staticd.yang.c``). -2. Add CLI handler file[s] to ``mgmtd/subdir.am`` (e.g., ``staticd/static_vty.c``). -3. [if needed] Exclude (#ifndef) non-configuration CLI handlers from CLI source - file (e.g., inside ``staticd/static_vty.c``). -4. [otherwise] Remove CLI handler file from SOURCES in daemon (e.g in :file:`staticd/subdir.am`) -5. Add YANG module description into array defined in ``mgmtd/mgmt_main.c`` (see :ref:`mgmtd-config-write`). -6. Initialize the CLI handlers inside :code:`mgmt_vty_init` in :file:`mgmtd/mgmt_vty.c`. -7. Direct ``vtysh`` to send CLI commands to ``mgmtd`` by modifying +#. Add YANG module file to ``mgmtd/subdir.am`` (e.g., ``yang/frr-staticd.yang.c``). + +#. Add CLI handler file[s] to ``mgmtd/subdir.am``. The `subdir.am` variable to + use is indicated in the next 2 steps. + + #. [if needed] Exclude (:code:`#ifndef`) non-configuration CLI handlers from + CLI source file (e.g., inside :file:`staticd/static_vty.c`) and add the + file to :code:`nodist_mgmtd_libmgmt_be_nb_la_SOURCES` in + :file:`mgmtd/subdir.am`. + + #. [otherwise] Remove CLI handler file from _SOURCES variable in the daemon + :file:`subdir.am` file (e.g in :file:`staticd/subdir.am`) and add to + :code:`mgmtd_libmgmtd_a_SOURCES` in :file:`mgmtd/subdir.am`. + +#. In order to have mgmtd try and load existing per-daemon config files, add + the daemon to the :code:`mgmt_daemons` array in :file:`lib/vty.c`. With the + official release of the mgmtd code FRR is no longer supporting per daemon log + files but it will take a while before all of the topotest is converted. + +#. In the daemon's :code:`struct frr_daemon_info` (i.e., inside it's + :code:`FRR_DAEMON_INFO()`) set the `.flags` bit `FRR_NO_SPLIT_CONFIG`. This + will keep the daemon from trying to read it's per-daemon config file as mgmtd + will now be doing this. + +#. Add the daemon's YANG module description[s] into the array + :code:`mgmt_yang_modules` defined in :file:`mgmtd/mgmt_main.c` (see + :ref:`mgmtd-config-write`). Make sure that all YANG modules that the daemon + uses are present in the mgmtd list. To find this list look in the daemon's + equivalent yang module array variable. + +#. Initialize the CLI handlers inside :code:`mgmt_vty_init` in :file:`mgmtd/mgmt_vty.c`. + +#. Direct ``vtysh`` to send CLI commands to ``mgmtd`` by modifying ``vtysh/vtysh.h``. At the top of this file each daemon has a bit ``#define``'d (e.g., ``#define VTYSH_STATICD 0x08000``) below this there are groupings, replace all the uses of the daemons bit with ``VTYSH_MGMTD`` instead so that the CLI commands get properly routed to ``mgmtd`` rather than the daemon now. + #. Remove initialization (and installation) of library CLI routines. These will + correspond with the VTYSH removals from the last step i.e.,: + + - change access_list_init() to access_list_init_new(false) and remove from + VTYSH_ACL_CONFIG (leave in VTYSH_ACL_SHOW). + - remove if_cmd_init_default() => remove from VTYSH_INTERFACE_SUBSET + - remove if_cmd_init() => remove from VTYSH_INTERFACE_SUBSET + - change route_map_init() to route_map_init_new(false) and remove from + VTYSH_ROUTE_MAP_CONFIG (leave in VTYSH_ROUTE_MAP_SHOW). + - remove vrf_cmd_init(NULL) => remove from VTYSH_INTERFACE_SUBSET + ... + Back-End Interface: -8. In ``mgmtd/mgmt_be_adapter.c`` add xpath prefix mappings to a one or both +#. In the daemon's main file initialize the BE client library. You add a global + `struct mgmt_be_client *mgmt_be_client` near the daemons `event_loop *master` + variable. Then where the daemon used to initialize it's CLI/VTY code replace + that with the client initialization by calling `mgmt_be_client_create`. + Likewise in the daemon's sigint cleanup code, operational walks should be + canceled with a call to `nb_oper_cancel_all_walks`, and then the BE client + should be destroyed with a call to `mgmt_be_client_destroy` and to be safe + NULL out the global `mgmt_be_client` variable. + +#. In ``mgmtd/mgmt_be_adapter.c`` add xpath prefix mappings to a one or both mapping arrays (``be_client_config_xpaths`` and ``be_client_oper_xpaths``) to direct ``mgmtd`` to send config and oper-state requests to your daemon. NOTE: make sure to include library supported xpaths prefixes as well (e.g., - "/frr-interface:lib"). - + "/frr-interface:lib"). A good way to figure these paths out are to look in + each of the YANG modules that the daemon uses and include each of their paths + in the array. Add YANG and CLI into MGMTD ---------------------------- +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ As an example here is the addition made to ``mgmtd/subdir.am`` for adding ``staticd`` support. @@ -108,7 +201,7 @@ An here is the addition to the modules array in ``mgmtd/mgmt_main.c``: CLI Config and Show Handlers ----------------------------- +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The daemon's CLI handlers for configuration (which having been converted to the :ref:`northbound` now simply generate YANG changes) will be linked directly into @@ -157,7 +250,7 @@ are present in a single file (e.g. a ``xxx_vty.c`` or ``xxx_cli.c`` file) then .. _mgmtd-config-write: CLI Config Write Handlers (:code:`cli_show`) --------------------------------------------- +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ To support writing out the CLI configuration file the northbound API defines a 2 callbacks (:code:`cli_show` and :code:`cli_show_end`). Pointers to these @@ -172,10 +265,10 @@ the *backend* config and oper-state callbacks (e.g., :code:`create`, So you will need to define 2 :code:`struct frr_yang_module_info` arrays. -1. The existing array remains in the same place in the daemon, but with all the +#. The existing array remains in the same place in the daemon, but with all the :code:`cli_show` handlers removed. -2. The removed :code:`cli_show` handlers should be added to a new +#. The removed :code:`cli_show` handlers should be added to a new :code:`struct frr_yang_module_info` array. This second array should be included in the same file that includes that actual function pointed to by the the :code:`cli_show` callbacks (i.e., the file is compiled into @@ -184,8 +277,44 @@ So you will need to define 2 :code:`struct frr_yang_module_info` arrays. This new :code:`struct frr_yang_module_info` array is the one to be included in mgmtd in `mgmt_yang_modules` inside ``mgmtd/mgmt_main.c``. +Back-End Client Connection +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In order for your daemon to communicate with mgmtd you need to initialize the +backend client library. You normally do this where you used to initialize your +CLI/VTY code. + +.. code-block:: c + + ... + struct event_loop *master; + + static struct mgmt_be_client *mgmt_be_client; + ... + + int main(int argc, char **argv) + { + ... + rip_init(); + rip_if_init(); + mgmt_be_client = mgmt_be_client_create("ripd", NULL, 0, master); + +Likewise the client should be cleaned up in the daemon cleanup routine. + +.. code-block:: c + + /* SIGINT handler. */ + static void sigint(void) + { + zlog_notice("Terminating on signal"); + ... + nb_oper_cancel_all_walks(); + mgmt_be_client_destroy(mgmt_be_client); + mgmt_be_client = NULL; + + Back-End XPATH mappings ------------------------ +^^^^^^^^^^^^^^^^^^^^^^^ In order for ``mgmtd`` to direct configuration to your daemon you need to add some XPATH mappings to ``mgmtd/mgmt_be_adapter.c``. These XPATHs determine which @@ -228,9 +357,8 @@ not conditionalized b/c it should always be present): [MGMTD_BE_CLIENT_ID_ZEBRA] = zebra_oper_xpaths, }; - MGMTD Internals -^^^^^^^^^^^^^^^ +--------------- This section will describe the internal functioning of ``mgmtd``, for now a couple diagrams are included to aide in source code perusal. From dabc92de9e3b02e5d1a73d6fc43ffbd51eaded91 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Wed, 24 Jan 2024 10:59:14 -0500 Subject: [PATCH 02/11] lib: convert route-map to mgmtd Signed-off-by: Christian Hopps --- lib/routemap.c | 12 +++++++++-- lib/routemap.h | 2 ++ lib/routemap_northbound.c | 42 +++++++++++++++++++++++++++++++++++++++ mgmtd/mgmt_main.c | 2 +- mgmtd/mgmt_vty.c | 6 ++++++ python/xref2vtysh.py | 8 ++++---- vtysh/vtysh.c | 8 ++++---- vtysh/vtysh.h | 11 +++++++++- 8 files changed, 79 insertions(+), 12 deletions(-) diff --git a/lib/routemap.c b/lib/routemap.c index e8a92cda0b08..6b3f81b4d41b 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -3409,7 +3409,7 @@ DEFUN_HIDDEN(show_route_map_pfx_tbl, show_route_map_pfx_tbl_cmd, } /* Initialization of route map vector. */ -void route_map_init(void) +void route_map_init_new(bool in_backend) { int i; @@ -3424,7 +3424,10 @@ void route_map_init(void) UNSET_FLAG(rmap_debug, DEBUG_ROUTEMAP); - route_map_cli_init(); + if (!in_backend) { + /* we do not want to handle config commands in the backend */ + route_map_cli_init(); + } /* Install route map top node. */ install_node(&rmap_debug_node); @@ -3444,3 +3447,8 @@ void route_map_init(void) install_element(ENABLE_NODE, &show_route_map_pfx_tbl_cmd); } + +void route_map_init(void) +{ + route_map_init_new(false); +} diff --git a/lib/routemap.h b/lib/routemap.h index 08e341221d8c..dfb84ced5bae 100644 --- a/lib/routemap.h +++ b/lib/routemap.h @@ -401,6 +401,7 @@ enum ecommunity_lb_type { /* Prototypes. */ extern void route_map_init(void); +extern void route_map_init_new(bool in_backend); /* * This should only be called on shutdown @@ -1024,6 +1025,7 @@ routemap_hook_context_insert(struct route_map_index *rmi); void routemap_hook_context_free(struct routemap_hook_context *rhc); extern const struct frr_yang_module_info frr_route_map_info; +extern const struct frr_yang_module_info frr_route_map_cli_info; /* routemap_cli.c */ extern int route_map_instance_cmp(const struct lyd_node *dnode1, diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index a7a77cc23b5e..1bba4dad47a6 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -1550,3 +1550,45 @@ const struct frr_yang_module_info frr_route_map_info = { }, } }; + +const struct frr_yang_module_info frr_route_map_cli_info = { + .name = "frr-route-map", + .ignore_cfg_cbs = true, + .nodes = { + { + .xpath = "/frr-route-map:lib/route-map/optimization-disabled", + .cbs.cli_show = route_map_optimization_disabled_show, + }, + { + .xpath = "/frr-route-map:lib/route-map/entry", + .cbs = { + .cli_cmp = route_map_instance_cmp, + .cli_show = route_map_instance_show, + .cli_show_end = route_map_instance_show_end, + } + }, + { + .xpath = "/frr-route-map:lib/route-map/entry/description", + .cbs.cli_show = route_map_description_show, + }, + { + .xpath = "/frr-route-map:lib/route-map/entry/call", + .cbs.cli_show = route_map_call_show, + }, + { + .xpath = "/frr-route-map:lib/route-map/entry/exit-policy", + .cbs.cli_show = route_map_exit_policy_show, + }, + { + .xpath = "/frr-route-map:lib/route-map/entry/match-condition", + .cbs.cli_show = route_map_condition_show, + }, + { + .xpath = "/frr-route-map:lib/route-map/entry/set-action", + .cbs.cli_show = route_map_action_show, + }, + { + .xpath = NULL, + }, + } +}; diff --git a/mgmtd/mgmt_main.c b/mgmtd/mgmt_main.c index 743091e5c47b..9340d3d10746 100644 --- a/mgmtd/mgmt_main.c +++ b/mgmtd/mgmt_main.c @@ -172,7 +172,7 @@ const struct frr_yang_module_info zebra_route_map_info = { static const struct frr_yang_module_info *const mgmt_yang_modules[] = { &frr_filter_info, &frr_interface_info, - &frr_route_map_info, + &frr_route_map_cli_info, &frr_routing_info, &frr_vrf_info, diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c index f4b24acf3ace..5aca6a8ef6d3 100644 --- a/mgmtd/mgmt_vty.c +++ b/mgmtd/mgmt_vty.c @@ -12,6 +12,7 @@ #include "json.h" #include "network.h" #include "northbound_cli.h" +#include "routemap.h" #include "mgmtd/mgmt.h" #include "mgmtd/mgmt_be_adapter.h" @@ -560,6 +561,11 @@ static struct cmd_node mgmtd_node = { void mgmt_vty_init(void) { + /* + * Library based CLI handlers + */ + route_map_cli_init(); + /* * Initialize command handling from VTYSH connection. * Call command initialization routines defined by diff --git a/python/xref2vtysh.py b/python/xref2vtysh.py index 75fff8ddd9c3..36e37e323033 100644 --- a/python/xref2vtysh.py +++ b/python/xref2vtysh.py @@ -37,14 +37,14 @@ "lib/filter_cli.c": "VTYSH_ACL", "lib/if.c": "VTYSH_INTERFACE", "lib/keychain.c": "VTYSH_KEYS", - "lib/mgmt_be_client.c": "VTYSH_STATICD|VTYSH_ZEBRA", - "lib/mgmt_fe_client.c": "VTYSH_MGMTD", + "lib/mgmt_be_client.c": "VTYSH_MGMT_BACKEND", + "lib/mgmt_fe_client.c": "VTYSH_MGMT_FRONTEND", "lib/lib_vty.c": "VTYSH_ALL", "lib/log_vty.c": "VTYSH_ALL", "lib/nexthop_group.c": "VTYSH_NH_GROUP", "lib/resolver.c": "VTYSH_NHRPD|VTYSH_BGPD", - "lib/routemap.c": "VTYSH_RMAP", - "lib/routemap_cli.c": "VTYSH_RMAP", + "lib/routemap.c": "VTYSH_RMAP_SHOW", + "lib/routemap_cli.c": "VTYSH_RMAP_CONFIG", "lib/spf_backoff.c": "VTYSH_ISISD", "lib/event.c": "VTYSH_ALL", "lib/vrf.c": "VTYSH_VRF", diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 3109f1510d97..e86eeeb2870e 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2303,7 +2303,7 @@ DEFUNSH(VTYSH_AFFMAP, no_affinity_map, vtysh_no_affinity_map_cmd, return CMD_SUCCESS; } -DEFUNSH(VTYSH_RMAP, vtysh_route_map, vtysh_route_map_cmd, +DEFUNSH(VTYSH_RMAP_CONFIG, vtysh_route_map, vtysh_route_map_cmd, "route-map RMAP_NAME (1-65535)", "Create route-map or enter route-map command mode\n" "Route map tag\n" @@ -2572,13 +2572,13 @@ DEFUNSH(VTYSH_RIPNGD, vtysh_quit_ripngd, vtysh_quit_ripngd_cmd, "quit", } #endif /* HAVE_RIPNGD */ -DEFUNSH(VTYSH_RMAP, vtysh_exit_rmap, vtysh_exit_rmap_cmd, "exit", +DEFUNSH(VTYSH_RMAP_CONFIG, vtysh_exit_rmap, vtysh_exit_rmap_cmd, "exit", "Exit current mode and down to previous mode\n") { return vtysh_exit(vty); } -DEFUNSH(VTYSH_RMAP, vtysh_quit_rmap, vtysh_quit_rmap_cmd, "quit", +DEFUNSH(VTYSH_RMAP_CONFIG, vtysh_quit_rmap, vtysh_quit_rmap_cmd, "quit", "Exit current mode and down to previous mode\n") { return vtysh_exit_rmap(self, vty, argc, argv); @@ -3455,7 +3455,7 @@ static void show_route_map_send(const char *route_map, bool json) const struct vtysh_client *client = &vtysh_client[i]; bool is_connected = true; - if (!CHECK_FLAG(client->flag, VTYSH_RMAP)) + if (!CHECK_FLAG(client->flag, VTYSH_RMAP_SHOW)) continue; for (; client; client = client->next) diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index 6bc0c5e2c5f8..9081cab763e2 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -51,7 +51,14 @@ extern struct event_loop *master; VTYSH_FABRICD | VTYSH_VRRPD | VTYSH_PATHD | VTYSH_MGMTD #define VTYSH_ACL VTYSH_BFDD|VTYSH_BABELD|VTYSH_BGPD|VTYSH_EIGRPD|VTYSH_ISISD|VTYSH_FABRICD|VTYSH_LDPD|VTYSH_NHRPD|VTYSH_OSPF6D|VTYSH_OSPFD|VTYSH_PBRD|VTYSH_PIMD|VTYSH_PIM6D|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_VRRPD|VTYSH_ZEBRA #define VTYSH_AFFMAP VTYSH_ZEBRA | VTYSH_ISISD -#define VTYSH_RMAP VTYSH_ZEBRA|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_OSPFD|VTYSH_OSPF6D|VTYSH_BGPD|VTYSH_ISISD|VTYSH_PIMD|VTYSH_EIGRPD|VTYSH_FABRICD +#define VTYSH_RMAP_CONFIG \ + VTYSH_ZEBRA | VTYSH_RIPNGD | VTYSH_OSPFD | VTYSH_OSPF6D | VTYSH_BGPD | \ + VTYSH_ISISD | VTYSH_PIMD | VTYSH_EIGRPD | VTYSH_FABRICD | \ + VTYSH_MGMTD +#define VTYSH_RMAP_SHOW \ + VTYSH_ZEBRA | VTYSH_RIPD | VTYSH_RIPNGD | VTYSH_OSPFD | VTYSH_OSPF6D | \ + VTYSH_BGPD | VTYSH_ISISD | VTYSH_PIMD | VTYSH_EIGRPD | \ + VTYSH_FABRICD #define VTYSH_INTERFACE_SUBSET \ VTYSH_ZEBRA | VTYSH_RIPD | VTYSH_RIPNGD | VTYSH_OSPFD | VTYSH_OSPF6D | \ VTYSH_ISISD | VTYSH_PIMD | VTYSH_PIM6D | VTYSH_NHRPD | \ @@ -64,6 +71,8 @@ extern struct event_loop *master; #define VTYSH_NH_GROUP VTYSH_PBRD|VTYSH_SHARPD #define VTYSH_SR VTYSH_ZEBRA|VTYSH_PATHD #define VTYSH_DPDK VTYSH_ZEBRA +#define VTYSH_MGMT_BACKEND VTYSH_RIPD | VTYSH_STATICD | VTYSH_ZEBRA +#define VTYSH_MGMT_FRONTEND VTYSH_MGMTD enum vtysh_write_integrated { WRITE_INTEGRATED_UNSPECIFIED, From 63ca751c1186483f82f0d464bc1c8c43e3a2755f Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Wed, 24 Jan 2024 11:35:19 -0500 Subject: [PATCH 03/11] lib: convert filters to mgmtd Signed-off-by: Christian Hopps --- lib/filter.c | 12 ++++++++++-- lib/filter.h | 5 +++-- lib/filter_nb.c | 32 ++++++++++++++++++++++++++++++++ mgmtd/mgmt_main.c | 2 +- mgmtd/mgmt_vty.c | 1 + python/xref2vtysh.py | 4 ++-- vtysh/vtysh.h | 12 +++++++++++- 7 files changed, 60 insertions(+), 8 deletions(-) diff --git a/lib/filter.c b/lib/filter.c index f86adab5d606..a0adff0e35dc 100644 --- a/lib/filter.c +++ b/lib/filter.c @@ -885,7 +885,7 @@ static void access_list_init_ipv6(void) install_element(ENABLE_NODE, &show_ipv6_access_list_name_cmd); } -void access_list_init(void) +void access_list_init_new(bool in_backend) { cmd_variable_handler_register(access_list_handlers); @@ -893,7 +893,15 @@ void access_list_init(void) access_list_init_ipv6(); access_list_init_mac(); - filter_cli_init(); + if (!in_backend) { + /* we do not want to handle config commands in the backend */ + filter_cli_init(); + } +} + +void access_list_init(void) +{ + access_list_init_new(false); } void access_list_reset(void) diff --git a/lib/filter.h b/lib/filter.h index e092f0771acc..bd9e22d38416 100644 --- a/lib/filter.h +++ b/lib/filter.h @@ -114,6 +114,7 @@ struct access_master { /* Prototypes for access-list. */ extern void access_list_init(void); +extern void access_list_init_new(bool in_backend); extern void access_list_reset(void); extern void access_list_add_hook(void (*func)(struct access_list *)); extern void access_list_delete_hook(void (*func)(struct access_list *)); @@ -124,13 +125,13 @@ extern enum filter_type access_list_apply(struct access_list *access, struct access_list *access_list_get(afi_t afi, const char *name); void access_list_delete(struct access_list *access); struct filter *filter_new(void); -void access_list_filter_add(struct access_list *access, - struct filter *filter); +void access_list_filter_add(struct access_list *access, struct filter *filter); void access_list_filter_delete(struct access_list *access, struct filter *filter); int64_t filter_new_seq_get(struct access_list *access); extern const struct frr_yang_module_info frr_filter_info; +extern const struct frr_yang_module_info frr_filter_cli_info; /* filter_nb.c */ diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 1c436cc6f10a..eba4e421c050 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -1785,3 +1785,35 @@ const struct frr_yang_module_info frr_filter_info = { }, } }; + +const struct frr_yang_module_info frr_filter_cli_info = { + .name = "frr-filter", + .ignore_cfg_cbs = true, + .nodes = { + { + .xpath = "/frr-filter:lib/access-list/remark", + .cbs.cli_show = access_list_remark_show, + }, + { + .xpath = "/frr-filter:lib/access-list/entry", + .cbs = { + .cli_cmp = access_list_cmp, + .cli_show = access_list_show, + } + }, + { + .xpath = "/frr-filter:lib/prefix-list/remark", + .cbs.cli_show = prefix_list_remark_show, + }, + { + .xpath = "/frr-filter:lib/prefix-list/entry", + .cbs = { + .cli_cmp = prefix_list_cmp, + .cli_show = prefix_list_show, + } + }, + { + .xpath = NULL, + }, + } +}; diff --git a/mgmtd/mgmt_main.c b/mgmtd/mgmt_main.c index 9340d3d10746..bd0a9895c294 100644 --- a/mgmtd/mgmt_main.c +++ b/mgmtd/mgmt_main.c @@ -170,7 +170,7 @@ const struct frr_yang_module_info zebra_route_map_info = { * MGMTd. */ static const struct frr_yang_module_info *const mgmt_yang_modules[] = { - &frr_filter_info, + &frr_filter_cli_info, &frr_interface_info, &frr_route_map_cli_info, &frr_routing_info, diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c index 5aca6a8ef6d3..5dd7be657af8 100644 --- a/mgmtd/mgmt_vty.c +++ b/mgmtd/mgmt_vty.c @@ -564,6 +564,7 @@ void mgmt_vty_init(void) /* * Library based CLI handlers */ + filter_cli_init(); route_map_cli_init(); /* diff --git a/python/xref2vtysh.py b/python/xref2vtysh.py index 36e37e323033..edaa2945cec0 100644 --- a/python/xref2vtysh.py +++ b/python/xref2vtysh.py @@ -33,8 +33,8 @@ daemon_flags = { "lib/agentx.c": "VTYSH_ISISD|VTYSH_RIPD|VTYSH_OSPFD|VTYSH_OSPF6D|VTYSH_BGPD|VTYSH_ZEBRA", - "lib/filter.c": "VTYSH_ACL", - "lib/filter_cli.c": "VTYSH_ACL", + "lib/filter.c": "VTYSH_ACL_SHOW", + "lib/filter_cli.c": "VTYSH_ACL_CONFIG", "lib/if.c": "VTYSH_INTERFACE", "lib/keychain.c": "VTYSH_KEYS", "lib/mgmt_be_client.c": "VTYSH_MGMT_BACKEND", diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index 9081cab763e2..7671609e3a48 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -49,7 +49,17 @@ extern struct event_loop *master; VTYSH_PIM6D | VTYSH_NHRPD | VTYSH_EIGRPD | VTYSH_BABELD | \ VTYSH_SHARPD | VTYSH_PBRD | VTYSH_STATICD | VTYSH_BFDD | \ VTYSH_FABRICD | VTYSH_VRRPD | VTYSH_PATHD | VTYSH_MGMTD -#define VTYSH_ACL VTYSH_BFDD|VTYSH_BABELD|VTYSH_BGPD|VTYSH_EIGRPD|VTYSH_ISISD|VTYSH_FABRICD|VTYSH_LDPD|VTYSH_NHRPD|VTYSH_OSPF6D|VTYSH_OSPFD|VTYSH_PBRD|VTYSH_PIMD|VTYSH_PIM6D|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_VRRPD|VTYSH_ZEBRA +#define VTYSH_ACL_CONFIG \ + VTYSH_BFDD | VTYSH_BABELD | VTYSH_BGPD | VTYSH_EIGRPD | VTYSH_ISISD | \ + VTYSH_FABRICD | VTYSH_LDPD | VTYSH_NHRPD | VTYSH_OSPF6D | \ + VTYSH_OSPFD | VTYSH_PBRD | VTYSH_PIMD | VTYSH_PIM6D | \ + VTYSH_RIPNGD | VTYSH_VRRPD | VTYSH_ZEBRA | VTYSH_MGMTD +#define VTYSH_ACL_SHOW \ + VTYSH_BFDD | VTYSH_BABELD | VTYSH_BGPD | VTYSH_EIGRPD | VTYSH_ISISD | \ + VTYSH_FABRICD | VTYSH_LDPD | VTYSH_NHRPD | VTYSH_OSPF6D | \ + VTYSH_OSPFD | VTYSH_PBRD | VTYSH_PIMD | VTYSH_PIM6D | \ + VTYSH_RIPD | VTYSH_RIPNGD | VTYSH_VRRPD | VTYSH_ZEBRA + #define VTYSH_AFFMAP VTYSH_ZEBRA | VTYSH_ISISD #define VTYSH_RMAP_CONFIG \ VTYSH_ZEBRA | VTYSH_RIPNGD | VTYSH_OSPFD | VTYSH_OSPF6D | VTYSH_BGPD | \ From e39b60bac1e8f99ad9053e7d7d7cb73c2f1cc414 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 26 Jan 2024 16:54:36 +0200 Subject: [PATCH 04/11] lib: add yang function for counting data nodes Signed-off-by: Igor Ryzhov --- lib/yang.c | 24 ++++++++++++++++++++++++ lib/yang.h | 15 +++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/yang.c b/lib/yang.c index 7d35fb0d3dde..ed855c849846 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -508,6 +508,30 @@ void yang_dnode_iterate(yang_dnode_iter_cb cb, void *arg, ly_set_free(set, NULL); } +uint32_t yang_dnode_count(const struct lyd_node *dnode, const char *xpath_fmt, + ...) +{ + va_list ap; + char xpath[XPATH_MAXLEN]; + struct ly_set *set; + uint32_t count; + + va_start(ap, xpath_fmt); + vsnprintf(xpath, sizeof(xpath), xpath_fmt, ap); + va_end(ap); + + if (lyd_find_xpath(dnode, xpath, &set)) { + assert(0); + return 0; + } + + count = set->count; + + ly_set_free(set, NULL); + + return count; +} + bool yang_dnode_is_default(const struct lyd_node *dnode, const char *xpath) { const struct lysc_node *snode; diff --git a/lib/yang.h b/lib/yang.h index dbb7f7163bd2..1235125f26bd 100644 --- a/lib/yang.h +++ b/lib/yang.h @@ -420,6 +420,21 @@ void yang_dnode_iterate(yang_dnode_iter_cb cb, void *arg, const struct lyd_node *dnode, const char *xpath_fmt, ...) PRINTFRR(4, 5); +/* + * Count the number of data nodes that satisfy an XPath query. + * + * dnode + * Base libyang data node to operate on. + * + * xpath_fmt + * XPath expression (absolute or relative). + * + * ... + * any parameters for xpath_fmt. + */ +uint32_t yang_dnode_count(const struct lyd_node *dnode, const char *xpath_fmt, + ...) PRINTFRR(2, 3); + /* * Check if the libyang data node contains a default value. Non-presence * containers are assumed to always contain a default value. From 1bf395232de224bec841c01a1b6c67bce5b525bc Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 26 Jan 2024 16:57:41 +0200 Subject: [PATCH 05/11] lib: fix removing access/prefix lists CLI for access/prefix list removal was using `nb_cli_apply_changes` multiple times in the same command. It's fine for regular daemons but not for mgmtd. Refactor the code to apply changes only once. Signed-off-by: Igor Ryzhov --- lib/filter_cli.c | 257 +++++++++++++++-------------------------------- 1 file changed, 80 insertions(+), 177 deletions(-) diff --git a/lib/filter_cli.c b/lib/filter_cli.c index 529b46b6ad79..28790f69e7e9 100644 --- a/lib/filter_cli.c +++ b/lib/filter_cli.c @@ -69,53 +69,60 @@ static int64_t acl_get_seq(struct vty *vty, const char *xpath, bool is_remove) return seq; } -static int acl_remove_if_empty(struct vty *vty, const char *iptype, - const char *name) +/** + * Remove main data structure filter list if there are no more entries or + * remark. This fixes compatibility with old CLI and tests. + */ +static int filter_remove_check_empty(struct vty *vty, const char *ftype, + const char *iptype, const char *name, + uint32_t del_seq, bool del_remark) { + const struct lyd_node *remark_dnode = NULL; + const struct lyd_node *entry_dnode = NULL; char xpath[XPATH_MAXLEN]; + uint32_t count; + + /* Count existing entries */ + count = yang_dnode_count(vty->candidate_config->dnode, + "/frr-filter:lib/%s-list[type='%s'][name='%s']/entry", + ftype, iptype, name); + + /* Check entry-to-delete actually exists */ + if (del_seq) { + snprintf(xpath, sizeof(xpath), + "/frr-filter:lib/%s-list[type='%s'][name='%s']/entry[sequence='%u']", + ftype, iptype, name, del_seq); + entry_dnode = yang_dnode_get(vty->candidate_config->dnode, + xpath); + + /* If exists, delete and don't count it, we need only remaining entries */ + if (entry_dnode) { + nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); + count--; + } + } + /* Delete the remark, or check whether it exists if we're keeping it */ snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='%s'][name='%s']/remark", + "/frr-filter:lib/%s-list[type='%s'][name='%s']/remark", ftype, iptype, name); - /* List is not empty if there is a remark, check that: */ - if (yang_dnode_exists(vty->candidate_config->dnode, xpath)) - return CMD_SUCCESS; - - /* Check if we have any entries: */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='%s'][name='%s']", iptype, - name); - /* - * NOTE: if the list is empty it will return the first sequence - * number: 5. - */ - if (acl_get_seq(vty, xpath, true) != 5) - return CMD_SUCCESS; + if (del_remark) + nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); + else + remark_dnode = yang_dnode_get(vty->candidate_config->dnode, + xpath); + + /* If there are no entries left and no remark, delete the whole list */ + if (count == 0 && !remark_dnode) { + snprintf(xpath, sizeof(xpath), + "/frr-filter:lib/%s-list[type='%s'][name='%s']", ftype, + iptype, name); + nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); + } - /* Nobody is using this list, lets remove it. */ - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); } -static int acl_remove(struct vty *vty, const char *iptype, const char *name, - int64_t sseq) -{ - char xpath[XPATH_MAXLEN]; - int rv; - - snprintfrr( - xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='%s'][name='%s']/entry[sequence='%" PRId64 "']", - iptype, name, sseq); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return acl_remove_if_empty(vty, iptype, name); - - return rv; -} - /* * Cisco (legacy) access lists. */ @@ -213,7 +220,8 @@ DEFPY_YANG( /* If the user provided sequence number, then just go for it. */ if (seq_str != NULL) - return acl_remove(vty, "ipv4", name, seq); + return filter_remove_check_empty(vty, "access", "ipv4", name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ ada.ada_type = "ipv4"; @@ -237,7 +245,8 @@ DEFPY_YANG( else return CMD_WARNING_CONFIG_FAILED; - return acl_remove(vty, "ipv4", name, sseq); + return filter_remove_check_empty(vty, "access", "ipv4", name, sseq, + false); } DEFPY_YANG( @@ -384,7 +393,8 @@ DEFPY_YANG( /* If the user provided sequence number, then just go for it. */ if (seq_str != NULL) - return acl_remove(vty, "ipv4", name, seq); + return filter_remove_check_empty(vty, "access", "ipv4", name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ ada.ada_type = "ipv4"; @@ -429,7 +439,8 @@ DEFPY_YANG( else return CMD_WARNING_CONFIG_FAILED; - return acl_remove(vty, "ipv4", name, sseq); + return filter_remove_check_empty(vty, "access", "ipv4", name, sseq, + false); } /* @@ -525,7 +536,8 @@ DEFPY_YANG( /* If the user provided sequence number, then just go for it. */ if (seq_str != NULL) - return acl_remove(vty, "ipv4", name, seq); + return filter_remove_check_empty(vty, "access", "ipv4", name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ ada.ada_type = "ipv4"; @@ -549,7 +561,8 @@ DEFPY_YANG( else return CMD_WARNING_CONFIG_FAILED; - return acl_remove(vty, "ipv4", name, sseq); + return filter_remove_check_empty(vty, "access", "ipv4", name, sseq, + false); } DEFPY_YANG( @@ -600,19 +613,7 @@ DEFPY_YANG( ACCESS_LIST_ZEBRA_STR ACCESS_LIST_REMARK_STR) { - char xpath[XPATH_MAXLEN]; - int rv; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv4'][name='%s']/remark", - name); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return acl_remove_if_empty(vty, "ipv4", name); - - return rv; + return filter_remove_check_empty(vty, "access", "ipv4", name, 0, true); } ALIAS( @@ -716,7 +717,8 @@ DEFPY_YANG( /* If the user provided sequence number, then just go for it. */ if (seq_str != NULL) - return acl_remove(vty, "ipv6", name, seq); + return filter_remove_check_empty(vty, "access", "ipv6", name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ ada.ada_type = "ipv6"; @@ -740,7 +742,8 @@ DEFPY_YANG( else return CMD_WARNING_CONFIG_FAILED; - return acl_remove(vty, "ipv6", name, sseq); + return filter_remove_check_empty(vty, "access", "ipv6", name, sseq, + false); } DEFPY_YANG( @@ -794,19 +797,7 @@ DEFPY_YANG( ACCESS_LIST_ZEBRA_STR ACCESS_LIST_REMARK_STR) { - char xpath[XPATH_MAXLEN]; - int rv; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv6'][name='%s']/remark", - name); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return acl_remove_if_empty(vty, "ipv6", name); - - return rv; + return filter_remove_check_empty(vty, "access", "ipv6", name, 0, true); } ALIAS( @@ -902,7 +893,8 @@ DEFPY_YANG( /* If the user provided sequence number, then just go for it. */ if (seq_str != NULL) - return acl_remove(vty, "mac", name, seq); + return filter_remove_check_empty(vty, "access", "mac", name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ ada.ada_type = "mac"; @@ -922,7 +914,8 @@ DEFPY_YANG( else return CMD_WARNING_CONFIG_FAILED; - return acl_remove(vty, "mac", name, sseq); + return filter_remove_check_empty(vty, "access", "mac", name, sseq, + false); } DEFPY_YANG( @@ -976,19 +969,7 @@ DEFPY_YANG( ACCESS_LIST_ZEBRA_STR ACCESS_LIST_REMARK_STR) { - char xpath[XPATH_MAXLEN]; - int rv; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='mac'][name='%s']/remark", - name); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return acl_remove_if_empty(vty, "mac", name); - - return rv; + return filter_remove_check_empty(vty, "access", "mac", name, 0, true); } ALIAS( @@ -1149,62 +1130,17 @@ void access_list_remark_show(struct vty *vty, const struct lyd_node *dnode, * Prefix lists. */ -/** - * Remove main data structure prefix list if there are no more entries or - * remark. This fixes compatibility with old CLI and tests. - */ -static int plist_remove_if_empty(struct vty *vty, const char *iptype, - const char *name) -{ - char xpath[XPATH_MAXLEN]; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='%s'][name='%s']/remark", - iptype, name); - /* List is not empty if there is a remark, check that: */ - if (yang_dnode_exists(vty->candidate_config->dnode, xpath)) - return CMD_SUCCESS; - - /* Check if we have any entries: */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='%s'][name='%s']", iptype, - name); - /* - * NOTE: if the list is empty it will return the first sequence - * number: 5. - */ - if (acl_get_seq(vty, xpath, true) != 5) - return CMD_SUCCESS; - - /* Nobody is using this list, lets remove it. */ - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - return nb_cli_apply_changes(vty, NULL); -} - static int plist_remove(struct vty *vty, const char *iptype, const char *name, - const char *seq, const char *action, + uint32_t seq, const char *action, union prefixconstptr prefix, int ge, int le) { int64_t sseq; struct plist_dup_args pda = {}; - char xpath[XPATH_MAXLEN]; - char xpath_entry[XPATH_MAXLEN + 32]; - int rv; /* If the user provided sequence number, then just go for it. */ - if (seq != NULL) { - snprintf( - xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='%s'][name='%s']/entry[sequence='%s']", - iptype, name, seq); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return plist_remove_if_empty(vty, iptype, name); - - return rv; - } + if (seq != 0) + return filter_remove_check_empty(vty, "prefix", iptype, name, + seq, false); /* Otherwise, to keep compatibility, we need to figure it out. */ pda.pda_type = iptype; @@ -1224,17 +1160,8 @@ static int plist_remove(struct vty *vty, const char *iptype, const char *name, else return CMD_WARNING_CONFIG_FAILED; - snprintfrr( - xpath_entry, sizeof(xpath_entry), - "/frr-filter:lib/prefix-list[type='%s'][name='%s']/entry[sequence='%" PRId64 "']", - iptype, name, sseq); - nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return plist_remove_if_empty(vty, iptype, name); - - return rv; + return filter_remove_check_empty(vty, "prefix", iptype, name, sseq, + false); } DEFPY_YANG( @@ -1347,7 +1274,7 @@ DEFPY_YANG( "Maximum prefix length to be matched\n" "Maximum prefix length\n") { - return plist_remove(vty, "ipv4", name, seq_str, action, + return plist_remove(vty, "ipv4", name, seq, action, prefix_str ? prefix : NULL, ge, le); } @@ -1360,7 +1287,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, 0, 0); + return plist_remove(vty, "ipv4", name, seq, NULL, NULL, 0, 0); } DEFPY_YANG( @@ -1414,19 +1341,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_REMARK_STR) { - char xpath[XPATH_MAXLEN]; - int rv; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='ipv4'][name='%s']/remark", - name); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return plist_remove_if_empty(vty, "ipv4", name); - - return rv; + return filter_remove_check_empty(vty, "prefix", "ipv4", name, 0, true); } ALIAS( @@ -1549,7 +1464,7 @@ DEFPY_YANG( "Minimum prefix length to be matched\n" "Minimum prefix length\n") { - return plist_remove(vty, "ipv6", name, seq_str, action, + return plist_remove(vty, "ipv6", name, seq, action, prefix_str ? prefix : NULL, ge, le); } @@ -1562,7 +1477,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, 0, 0); + return plist_remove(vty, "ipv6", name, seq, NULL, NULL, 0, 0); } DEFPY_YANG( @@ -1616,19 +1531,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_REMARK_STR) { - char xpath[XPATH_MAXLEN]; - int rv; - - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='ipv6'][name='%s']/remark", - name); - nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL); - - rv = nb_cli_apply_changes(vty, NULL); - if (rv == CMD_SUCCESS) - return plist_remove_if_empty(vty, "ipv6", name); - - return rv; + return filter_remove_check_empty(vty, "prefix", "ipv6", name, 0, true); } ALIAS( From 42b815f4881c3a0bf8c7b3ddfd2685f13aec6959 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 24 Jan 2024 23:03:24 +0200 Subject: [PATCH 06/11] mgmtd: fix sending the same value to multiple clients The value should not be cleared after sending it to the first client, otherwise other clients receive an empty string. Signed-off-by: Igor Ryzhov --- mgmtd/mgmt_txn.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index c19b39c6966d..f04dec07bfe8 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -925,7 +925,6 @@ static int mgmt_txn_create_config_batches(struct mgmt_txn_req *txn_req, MGMTD__YANG_DATA_VALUE__VALUE_ENCODED_STR_VAL; batch->value[batch->num_cfg_data].encoded_str_val = value; - value = NULL; MGMTD_TXN_DBG(" -- %s, batch item:%d", adapter->name, (int)batch->num_cfg_data); From 7e2746933758b1586196a95e7e061c37c0fed6f2 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 25 Jan 2024 13:48:52 +0200 Subject: [PATCH 07/11] mgmtd: fix log type Signed-off-by: Igor Ryzhov --- mgmtd/mgmt_be_adapter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c index c8f52a101c87..4f5ad6e0b082 100644 --- a/mgmtd/mgmt_be_adapter.c +++ b/mgmtd/mgmt_be_adapter.c @@ -318,7 +318,7 @@ static int mgmt_be_send_subscr_reply(struct mgmt_be_client_adapter *adapter, be_msg.message_case = MGMTD__BE_MESSAGE__MESSAGE_SUBSCR_REPLY; be_msg.subscr_reply = &reply; - MGMTD_FE_CLIENT_DBG("Sending SUBSCR_REPLY client: %s sucess: %u", + MGMTD_BE_CLIENT_DBG("Sending SUBSCR_REPLY client: %s sucess: %u", adapter->name, success); return mgmt_be_adapter_send_msg(adapter, &be_msg); From 771c8f3b2add1172ea656b51d089db2c6d4042d1 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 25 Jan 2024 13:51:58 +0200 Subject: [PATCH 08/11] mgmtd: cleanup txn without scheduling If the transaction is not cleaned up immediately, it can be still referenced by some threds. If it's a commit thread and it's executed before the actual cleanup, mgmtd crashes because of the missing commit_cfg_req. Signed-off-by: Igor Ryzhov --- mgmtd/mgmt_txn.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index f04dec07bfe8..9ab476716acb 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -32,7 +32,6 @@ enum mgmt_txn_event { MGMTD_TXN_PROC_GETTREE, MGMTD_TXN_COMMITCFG_TIMEOUT, MGMTD_TXN_GETTREE_TIMEOUT, - MGMTD_TXN_CLEANUP }; PREDECL_LIST(mgmt_txn_reqs); @@ -281,6 +280,8 @@ static struct mgmt_master *mgmt_txn_mm; static void mgmt_txn_register_event(struct mgmt_txn_ctx *txn, enum mgmt_txn_event event); +static void mgmt_txn_cleanup_txn(struct mgmt_txn_ctx **txn); + static struct mgmt_txn_be_cfg_batch * mgmt_txn_cfg_batch_alloc(struct mgmt_txn_ctx *txn, enum mgmt_be_client_id id, struct mgmt_be_client_adapter *be_adapter) @@ -409,7 +410,6 @@ static struct mgmt_txn_req *mgmt_txn_req_alloc(struct mgmt_txn_ctx *txn, break; case MGMTD_TXN_COMMITCFG_TIMEOUT: case MGMTD_TXN_GETTREE_TIMEOUT: - case MGMTD_TXN_CLEANUP: break; } @@ -518,7 +518,6 @@ static void mgmt_txn_req_free(struct mgmt_txn_req **txn_req) break; case MGMTD_TXN_COMMITCFG_TIMEOUT: case MGMTD_TXN_GETTREE_TIMEOUT: - case MGMTD_TXN_CLEANUP: break; } @@ -781,7 +780,7 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn, * we need to cleanup by itself. */ if (!txn->session_id) - mgmt_txn_register_event(txn, MGMTD_TXN_CLEANUP); + mgmt_txn_cleanup_txn(&txn); return 0; } @@ -1455,11 +1454,6 @@ static void mgmt_txn_process_commit_cfg(struct event *thread) case MGMTD_COMMIT_PHASE_MAX: break; } - - MGMTD_TXN_DBG("txn-id:%" PRIu64 " session-id: %" PRIu64 - " phase updated to '%s'", - txn->txn_id, txn->session_id, - mgmt_txn_commit_phase_str(txn)); } static void mgmt_init_get_data_reply(struct mgmt_get_data_reply *get_reply) @@ -1539,7 +1533,6 @@ static void mgmt_txn_send_getcfg_reply_data(struct mgmt_txn_req *txn_req, case MGMTD_TXN_PROC_GETTREE: case MGMTD_TXN_GETTREE_TIMEOUT: case MGMTD_TXN_COMMITCFG_TIMEOUT: - case MGMTD_TXN_CLEANUP: MGMTD_TXN_ERR("Invalid Txn-Req-Event %u", txn_req->req_event); break; } @@ -1898,16 +1891,6 @@ static void mgmt_txn_cleanup_all_txns(void) mgmt_txn_cleanup_txn(&txn); } -static void mgmt_txn_cleanup(struct event *thread) -{ - struct mgmt_txn_ctx *txn; - - txn = (struct mgmt_txn_ctx *)EVENT_ARG(thread); - assert(txn); - - mgmt_txn_cleanup_txn(&txn); -} - static void mgmt_txn_register_event(struct mgmt_txn_ctx *txn, enum mgmt_txn_event event) { @@ -1939,11 +1922,6 @@ static void mgmt_txn_register_event(struct mgmt_txn_ctx *txn, MGMTD_TXN_GET_TREE_MAX_DELAY_SEC, &txn->get_tree_timeout); break; - case MGMTD_TXN_CLEANUP: - tv.tv_usec = MGMTD_TXN_CLEANUP_DELAY_USEC; - event_add_timer_tv(mgmt_txn_tm, mgmt_txn_cleanup, txn, &tv, - &txn->clnup); - break; case MGMTD_TXN_PROC_GETTREE: assert(!"code bug do not register this event"); break; @@ -2528,7 +2506,6 @@ int mgmt_txn_notify_error(struct mgmt_be_client_adapter *adapter, case MGMTD_TXN_PROC_GETCFG: case MGMTD_TXN_COMMITCFG_TIMEOUT: case MGMTD_TXN_GETTREE_TIMEOUT: - case MGMTD_TXN_CLEANUP: default: assert(!"non-native req event in native erorr path"); return -1; From b92ad5046e9dd7c4b335906a0bf3b38a3ff55454 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 25 Jan 2024 13:53:53 +0200 Subject: [PATCH 09/11] mgmtd: fix memleak Signed-off-by: Igor Ryzhov --- mgmtd/mgmt_txn.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index 9ab476716acb..45bc3a293004 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -2138,6 +2138,7 @@ int mgmt_txn_notify_be_adapter_conn(struct mgmt_be_client_adapter *adapter, if (!txn) { MGMTD_TXN_ERR("Failed to create CONFIG Transaction for downloading CONFIGs for client '%s'", adapter->name); + nb_config_diff_del_changes(adapter_cfgs); return -1; } From d79ca934eb4c8084324ae8a570fc68d2b861b47d Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 25 Jan 2024 13:54:45 +0200 Subject: [PATCH 10/11] mgmtd: fix commit request overwrite There are places, where we can receive an existing commit transaction. If we don't check that the request already exists, it gets overwritten and we start having problems with transaction refcounters. Forbid having multiple configuration sessions simultaneously. Signed-off-by: Igor Ryzhov --- mgmtd/mgmt_be_adapter.c | 21 +++++---------------- mgmtd/mgmt_fe_adapter.c | 5 ++++- mgmtd/mgmt_txn.c | 19 ++++++------------- mgmtd/mgmt_txn.h | 6 +++--- 4 files changed, 18 insertions(+), 33 deletions(-) diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c index 4f5ad6e0b082..8e719457f6a3 100644 --- a/mgmtd/mgmt_be_adapter.c +++ b/mgmtd/mgmt_be_adapter.c @@ -607,28 +607,17 @@ static void mgmt_be_adapter_conn_init(struct event *thread) adapter = (struct mgmt_be_client_adapter *)EVENT_ARG(thread); assert(adapter && adapter->conn->fd >= 0); - /* - * Check first if the current session can run a CONFIG - * transaction or not. Reschedule if a CONFIG transaction - * from another session is already in progress. - */ - if (mgmt_config_txn_in_progress() != MGMTD_SESSION_ID_NONE) { - zlog_err("XXX txn in progress, retry init"); - mgmt_be_adapter_sched_init_event(adapter); - return; - } - /* * Notify TXN module to create a CONFIG transaction and * download the CONFIGs identified for this new client. * If the TXN module fails to initiate the CONFIG transaction - * disconnect from the client forcing a reconnect later. - * That should also take care of destroying the adapter. + * retry a bit later. It only fails if there's an existing config + * transaction in progress. */ if (mgmt_txn_notify_be_adapter_conn(adapter, true) != 0) { - zlog_err("XXX notify be adapter conn fail"); - msg_conn_disconnect(adapter->conn, false); - adapter = NULL; + zlog_err("XXX txn in progress, retry init"); + mgmt_be_adapter_sched_init_event(adapter); + return; } } diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index d91987d8884f..ff85f56e3e46 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -724,7 +724,7 @@ mgmt_fe_session_handle_setcfg_req_msg(struct mgmt_fe_session_ctx *session, if (session->cfg_txn_id == MGMTD_TXN_ID_NONE) { /* as we have the lock no-one else should have a config txn */ - assert(mgmt_config_txn_in_progress() == MGMTD_SESSION_ID_NONE); + assert(!mgmt_config_txn_in_progress()); /* Start a CONFIG Transaction (if not started already) */ session->cfg_txn_id = mgmt_create_txn(session->session_id, @@ -890,6 +890,9 @@ static int mgmt_fe_session_handle_commit_config_req_msg( } if (session->cfg_txn_id == MGMTD_TXN_ID_NONE) { + /* as we have the lock no-one else should have a config txn */ + assert(!mgmt_config_txn_in_progress()); + /* * Start a CONFIG Transaction (if not started already) */ diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index 45bc3a293004..786fa0d0a3e1 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -1723,15 +1723,9 @@ static struct mgmt_txn_ctx *mgmt_txn_create_new(uint64_t session_id, { struct mgmt_txn_ctx *txn = NULL; - /* - * For 'CONFIG' transaction check if one is already created - * or not. TODO: figure out what code counts on this and fix it. - */ - if (type == MGMTD_TXN_TYPE_CONFIG && mgmt_txn_mm->cfg_txn) { - if (mgmt_config_txn_in_progress() == session_id) - txn = mgmt_txn_mm->cfg_txn; - goto mgmt_create_txn_done; - } + /* Do not allow multiple config transactions */ + if (type == MGMTD_TXN_TYPE_CONFIG && mgmt_config_txn_in_progress()) + return NULL; txn = mgmt_fe_find_txn_by_session_id(mgmt_txn_mm, session_id, type); if (!txn) { @@ -1761,7 +1755,6 @@ static struct mgmt_txn_ctx *mgmt_txn_create_new(uint64_t session_id, MGMTD_TXN_LOCK(txn); } -mgmt_create_txn_done: return txn; } @@ -1949,12 +1942,12 @@ void mgmt_txn_destroy(void) mgmt_txn_hash_destroy(); } -uint64_t mgmt_config_txn_in_progress(void) +bool mgmt_config_txn_in_progress(void) { if (mgmt_txn_mm && mgmt_txn_mm->cfg_txn) - return mgmt_txn_mm->cfg_txn->session_id; + return true; - return MGMTD_SESSION_ID_NONE; + return false; } uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type) diff --git a/mgmtd/mgmt_txn.h b/mgmtd/mgmt_txn.h index 3f27f2f07b78..02b2baa95fd9 100644 --- a/mgmtd/mgmt_txn.h +++ b/mgmtd/mgmt_txn.h @@ -71,12 +71,12 @@ extern int mgmt_txn_init(struct mgmt_master *cm, struct event_loop *tm); extern void mgmt_txn_destroy(void); /* - * Check if transaction is in progress. + * Check if configuration transaction is in progress. * * Returns: - * session ID if in-progress, MGMTD_SESSION_ID_NONE otherwise. + * true if in-progress, false otherwise. */ -extern uint64_t mgmt_config_txn_in_progress(void); +extern bool mgmt_config_txn_in_progress(void); /** * Get the session ID associated with the given ``txn-id``. From 83abe9c3cb9acd610dcdf8341374428df5ea9093 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Fri, 19 Jan 2024 16:40:12 +0000 Subject: [PATCH 11/11] ripd: ripd convert to mgmtd Signed-off-by: Christian Hopps --- lib/vty.c | 3 + mgmtd/mgmt_be_adapter.c | 25 ++++++++ mgmtd/mgmt_be_adapter.h | 3 + mgmtd/mgmt_main.c | 8 ++- mgmtd/mgmt_vty.c | 6 +- mgmtd/subdir.am | 26 +++++++- python/xref2vtysh.py | 2 +- ripd/rip_cli.c | 139 +++++++++++++++++++++++++++++++++++++++- ripd/rip_interface.c | 1 - ripd/rip_main.c | 15 ++++- ripd/rip_nb.c | 38 ----------- ripd/rip_nb.h | 1 + ripd/rip_routemap.c | 2 +- ripd/ripd.c | 50 ++------------- ripd/subdir.am | 1 - staticd/static_nb.h | 1 + staticd/static_vty.c | 4 +- vtysh/vtysh.c | 6 +- 18 files changed, 232 insertions(+), 99 deletions(-) diff --git a/lib/vty.c b/lib/vty.c index 3fc7c380830e..f2076552c7cb 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -124,6 +124,9 @@ bool vty_log_commands; static bool vty_log_commands_perm; char const *const mgmt_daemons[] = { +#ifdef HAVE_RIPD + "ripd", +#endif #ifdef HAVE_STATICD "staticd", #endif diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c index 8e719457f6a3..a7f3d4390e96 100644 --- a/mgmtd/mgmt_be_adapter.c +++ b/mgmtd/mgmt_be_adapter.c @@ -36,6 +36,9 @@ const char *mgmt_be_client_names[MGMTD_BE_CLIENT_ID_MAX + 1] = { [MGMTD_BE_CLIENT_ID_ZEBRA] = "zebra", +#ifdef HAVE_STATICD + [MGMTD_BE_CLIENT_ID_RIPD] = "ripd", +#endif #ifdef HAVE_STATICD [MGMTD_BE_CLIENT_ID_STATICD] = "staticd", #endif @@ -58,6 +61,22 @@ struct mgmt_be_xpath_map { * Each client gets their own map, but also union all the strings into the * above map as well. */ + +#if HAVE_RIPD +static const char *const ripd_config_xpaths[] = { + "/frr-filter:lib", + "/frr-interface:lib/interface", + "/frr-ripd:ripd", + "/frr-route-map:lib", + "/frr-vrf:lib", + NULL, +}; +static const char *const ripd_oper_xpaths[] = { + "/frr-ripd:ripd", + NULL, +}; +#endif + #if HAVE_STATICD static const char *const staticd_config_xpaths[] = { "/frr-vrf:lib", @@ -68,6 +87,9 @@ static const char *const staticd_config_xpaths[] = { #endif static const char *const *be_client_config_xpaths[MGMTD_BE_CLIENT_ID_MAX] = { +#ifdef HAVE_RIPD + [MGMTD_BE_CLIENT_ID_RIPD] = ripd_config_xpaths, +#endif #ifdef HAVE_STATICD [MGMTD_BE_CLIENT_ID_STATICD] = staticd_config_xpaths, #endif @@ -81,6 +103,9 @@ static const char *const zebra_oper_xpaths[] = { }; static const char *const *be_client_oper_xpaths[MGMTD_BE_CLIENT_ID_MAX] = { +#ifdef HAVE_RIPD + [MGMTD_BE_CLIENT_ID_RIPD] = ripd_oper_xpaths, +#endif [MGMTD_BE_CLIENT_ID_ZEBRA] = zebra_oper_xpaths, }; diff --git a/mgmtd/mgmt_be_adapter.h b/mgmtd/mgmt_be_adapter.h index 3407d4c6a73a..35c8ec4c7add 100644 --- a/mgmtd/mgmt_be_adapter.h +++ b/mgmtd/mgmt_be_adapter.h @@ -27,6 +27,9 @@ * #ifdef HAVE_COMPONENT */ enum mgmt_be_client_id { +#ifdef HAVE_RIPD + MGMTD_BE_CLIENT_ID_RIPD, +#endif #ifdef HAVE_STATICD MGMTD_BE_CLIENT_ID_STATICD, #endif diff --git a/mgmtd/mgmt_main.c b/mgmtd/mgmt_main.c index bd0a9895c294..6532e9b1bf72 100644 --- a/mgmtd/mgmt_main.c +++ b/mgmtd/mgmt_main.c @@ -14,6 +14,7 @@ #include "frr_pthread.h" #include "mgmtd/mgmt.h" #include "mgmtd/mgmt_ds.h" +#include "ripd/rip_nb.h" #include "routing_nb.h" @@ -138,7 +139,7 @@ static struct frr_signal_t mgmt_signals[] = { }; #ifdef HAVE_STATICD -extern const struct frr_yang_module_info frr_staticd_info; +extern const struct frr_yang_module_info frr_staticd_cli_info; #endif @@ -184,8 +185,11 @@ static const struct frr_yang_module_info *const mgmt_yang_modules[] = { &affinity_map_info, &zebra_route_map_info, +#ifdef HAVE_RIPD + &frr_ripd_cli_info, +#endif #ifdef HAVE_STATICD - &frr_staticd_info, + &frr_staticd_cli_info, #endif }; diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c index 5dd7be657af8..c88fd2f25b24 100644 --- a/mgmtd/mgmt_vty.c +++ b/mgmtd/mgmt_vty.c @@ -21,6 +21,8 @@ #include "mgmtd/mgmt_history.h" #include "mgmtd/mgmt_vty_clippy.c" +#include "ripd/ripd.h" +#include "staticd/static_vty.h" extern struct frr_daemon_info *mgmt_daemon_info; @@ -573,8 +575,10 @@ void mgmt_vty_init(void) * backend components that are moved to new MGMTD infra * here one by one. */ +#if HAVE_RIPD + rip_cli_init(); +#endif #if HAVE_STATICD - extern void static_vty_init(void); static_vty_init(); #endif diff --git a/mgmtd/subdir.am b/mgmtd/subdir.am index 89a6596f49f2..244710d733f6 100644 --- a/mgmtd/subdir.am +++ b/mgmtd/subdir.am @@ -15,6 +15,8 @@ clippy_scan += \ # end lib_LTLIBRARIES += mgmtd/libmgmt_be_nb.la +mgmtd_libmgmt_be_nb_la_SOURCES = \ + # end nodist_mgmtd_libmgmt_be_nb_la_SOURCES = \ # end mgmtd_libmgmt_be_nb_la_CFLAGS = $(AM_CFLAGS) -DINCLUDE_MGMTD_CMDDEFS_ONLY @@ -54,10 +56,30 @@ mgmtd_mgmtd_CFLAGS = $(AM_CFLAGS) -I ./ mgmtd_mgmtd_LDADD = mgmtd/libmgmtd.a lib/libfrr.la $(LIBCAP) $(LIBM) $(LIBYANG_LIBS) $(UST_LIBS) mgmtd_mgmtd_LDADD += mgmtd/libmgmt_be_nb.la + +if STATICD +nodist_mgmtd_mgmtd_SOURCES += yang/frr-bfdd.yang.c +else +if RIPD +nodist_mgmtd_mgmtd_SOURCES += yang/frr-bfdd.yang.c +endif +endif + +if RIPD +nodist_mgmtd_mgmtd_SOURCES += \ + yang/frr-ripd.yang.c \ + # end +mgmtd_libmgmt_be_nb_la_SOURCES += \ + ripd/rip_cli.c \ + # end +endif + if STATICD nodist_mgmtd_mgmtd_SOURCES += \ yang/frr-staticd.yang.c \ - yang/frr-bfdd.yang.c \ # end -nodist_mgmtd_libmgmt_be_nb_la_SOURCES += staticd/static_vty.c +nodist_mgmtd_libmgmt_be_nb_la_SOURCES += \ + staticd/static_vty.c \ + # end endif + diff --git a/python/xref2vtysh.py b/python/xref2vtysh.py index edaa2945cec0..c52ae5b9a6fd 100644 --- a/python/xref2vtysh.py +++ b/python/xref2vtysh.py @@ -209,7 +209,7 @@ def _get_daemons(self): if v6_cmd: return {"VTYSH_RIPNGD"} else: - return {"VTYSH_RIPD"} + return {"VTYSH_MGMTD"} return {} diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c index fbe647c5cafa..29db1b232d1f 100644 --- a/ripd/rip_cli.c +++ b/ripd/rip_cli.c @@ -8,6 +8,7 @@ #include #include "if.h" +#include "if_rmap.h" #include "vrf.h" #include "log.h" #include "prefix.h" @@ -1257,8 +1258,19 @@ DEFPY_YANG(no_rip_distribute_list_prefix, return nb_cli_apply_changes(vty, NULL); } +/* RIP node structure. */ +static struct cmd_node rip_node = { + .name = "rip", + .node = RIP_NODE, + .parent_node = CONFIG_NODE, + .prompt = "%s(config-router)# ", + // .config_write = config_write_rip, +}; + void rip_cli_init(void) { + install_node(&rip_node); + install_element(CONFIG_NODE, &router_rip_cmd); install_element(CONFIG_NODE, &no_router_rip_cmd); @@ -1289,6 +1301,7 @@ void rip_cli_init(void) install_element(RIP_NODE, &no_rip_version_cmd); install_element(RIP_NODE, &rip_bfd_default_profile_cmd); install_element(RIP_NODE, &no_rip_bfd_default_profile_cmd); + install_default(RIP_NODE); install_element(INTERFACE_NODE, &ip_rip_split_horizon_cmd); install_element(INTERFACE_NODE, &ip_rip_v2_broadcast_cmd); @@ -1308,4 +1321,128 @@ void rip_cli_init(void) install_element(INTERFACE_NODE, &no_ip_rip_bfd_profile_cmd); install_element(ENABLE_NODE, &clear_ip_rip_cmd); -} + + if_rmap_init(RIP_NODE); +} +/* clang-format off */ +const struct frr_yang_module_info frr_ripd_cli_info = { + .name = "frr-ripd", + .ignore_cfg_cbs = true, + .nodes = { + { + .xpath = "/frr-ripd:ripd/instance", + .cbs.cli_show = cli_show_router_rip, + }, + { + .xpath = "/frr-ripd:ripd/instance/allow-ecmp", + .cbs.cli_show = cli_show_rip_allow_ecmp, + }, + { + .xpath = "/frr-ripd:ripd/instance/default-information-originate", + .cbs.cli_show = cli_show_rip_default_information_originate, + }, + { + .xpath = "/frr-ripd:ripd/instance/default-metric", + .cbs.cli_show = cli_show_rip_default_metric, + }, + { + .xpath = "/frr-ripd:ripd/instance/distance/default", + .cbs.cli_show = cli_show_rip_distance, + }, + { + .xpath = "/frr-ripd:ripd/instance/distance/source", + .cbs.cli_show = cli_show_rip_distance_source, + }, + { + .xpath = "/frr-ripd:ripd/instance/explicit-neighbor", + .cbs.cli_show = cli_show_rip_neighbor, + }, + { + .xpath = "/frr-ripd:ripd/instance/network", + .cbs.cli_show = cli_show_rip_network_prefix, + }, + { + .xpath = "/frr-ripd:ripd/instance/interface", + .cbs.cli_show = cli_show_rip_network_interface, + }, + { + .xpath = "/frr-ripd:ripd/instance/offset-list", + .cbs.cli_show = cli_show_rip_offset_list, + }, + { + .xpath = "/frr-ripd:ripd/instance/passive-default", + .cbs.cli_show = cli_show_rip_passive_default, + }, + { + .xpath = "/frr-ripd:ripd/instance/passive-interface", + .cbs.cli_show = cli_show_rip_passive_interface, + }, + { + .xpath = "/frr-ripd:ripd/instance/non-passive-interface", + .cbs.cli_show = cli_show_rip_non_passive_interface, + }, + { + .xpath = "/frr-ripd:ripd/instance/redistribute", + .cbs.cli_show = cli_show_rip_redistribute, + }, + { + .xpath = "/frr-ripd:ripd/instance/if-route-maps/if-route-map", + .cbs.cli_show = cli_show_if_route_map, + }, + { + .xpath = "/frr-ripd:ripd/instance/static-route", + .cbs.cli_show = cli_show_rip_route, + }, + { + .xpath = "/frr-ripd:ripd/instance/timers", + .cbs.cli_show = cli_show_rip_timers, + }, + { + .xpath = "/frr-ripd:ripd/instance/version", + .cbs.cli_show = cli_show_rip_version, + }, + { + .xpath = "/frr-ripd:ripd/instance/default-bfd-profile", + .cbs.cli_show = cli_show_ripd_instance_default_bfd_profile, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/split-horizon", + .cbs.cli_show = cli_show_ip_rip_split_horizon, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/v2-broadcast", + .cbs.cli_show = cli_show_ip_rip_v2_broadcast, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/version-receive", + .cbs.cli_show = cli_show_ip_rip_receive_version, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/version-send", + .cbs.cli_show = cli_show_ip_rip_send_version, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-scheme", + .cbs.cli_show = cli_show_ip_rip_authentication_scheme, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-password", + .cbs.cli_show = cli_show_ip_rip_authentication_string, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-key-chain", + .cbs.cli_show = cli_show_ip_rip_authentication_key_chain, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/bfd-monitoring/enable", + .cbs.cli_show = cli_show_ip_rip_bfd_enable, + }, + { + .xpath = "/frr-interface:lib/interface/frr-ripd:rip/bfd-monitoring/profile", + .cbs.cli_show = cli_show_ip_rip_bfd_profile, + }, + { + .xpath = NULL, + }, + } +}; diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c index 65afce8cb724..486d7b05c208 100644 --- a/ripd/rip_interface.c +++ b/ripd/rip_interface.c @@ -1109,7 +1109,6 @@ void rip_if_init(void) hook_register_prio(if_del, 0, rip_interface_delete_hook); /* Install interface node. */ - if_cmd_init_default(); hook_register_prio(if_real, 0, rip_ifp_create); hook_register_prio(if_up, 0, rip_ifp_up); hook_register_prio(if_down, 0, rip_ifp_down); diff --git a/ripd/rip_main.c b/ripd/rip_main.c index cb23098a7e26..c86caabaf1c1 100644 --- a/ripd/rip_main.c +++ b/ripd/rip_main.c @@ -22,6 +22,7 @@ #include "libfrr.h" #include "routemap.h" #include "bfd.h" +#include "mgmt_be_client.h" #include "ripd/ripd.h" #include "ripd/rip_bfd.h" @@ -53,6 +54,8 @@ struct zebra_privs_t ripd_privs = { /* Master of threads. */ struct event_loop *master; +struct mgmt_be_client *mgmt_be_client; + static struct frr_daemon_info ripd_di; /* SIGHUP handler. */ @@ -73,6 +76,11 @@ static void sigint(void) bfd_protocol_integration_set_shutdown(true); + + nb_oper_cancel_all_walks(); + mgmt_be_client_destroy(mgmt_be_client); + mgmt_be_client = NULL; + RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { if (!vrf->info) continue; @@ -133,6 +141,9 @@ FRR_DAEMON_INFO(ripd, RIP, .vty_port = RIP_VTY_PORT, .privs = &ripd_privs, .yang_modules = ripd_yang_modules, .n_yang_modules = array_size(ripd_yang_modules), + + /* mgmtd will load the per-daemon config file now */ + .flags = FRR_NO_SPLIT_CONFIG, ); #define DEPRECATED_OPTIONS "" @@ -179,7 +190,9 @@ int main(int argc, char **argv) /* RIP related initialization. */ rip_init(); rip_if_init(); - rip_cli_init(); + + mgmt_be_client = mgmt_be_client_create("ripd", NULL, 0, master); + rip_zclient_init(master); rip_bfd_init(master); diff --git a/ripd/rip_nb.c b/ripd/rip_nb.c index 7167be124a74..d5df5916ada6 100644 --- a/ripd/rip_nb.c +++ b/ripd/rip_nb.c @@ -20,7 +20,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance", .cbs = { - .cli_show = cli_show_router_rip, .create = ripd_instance_create, .destroy = ripd_instance_destroy, .get_keys = ripd_instance_get_keys, @@ -31,35 +30,30 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/allow-ecmp", .cbs = { - .cli_show = cli_show_rip_allow_ecmp, .modify = ripd_instance_allow_ecmp_modify, }, }, { .xpath = "/frr-ripd:ripd/instance/default-information-originate", .cbs = { - .cli_show = cli_show_rip_default_information_originate, .modify = ripd_instance_default_information_originate_modify, }, }, { .xpath = "/frr-ripd:ripd/instance/default-metric", .cbs = { - .cli_show = cli_show_rip_default_metric, .modify = ripd_instance_default_metric_modify, }, }, { .xpath = "/frr-ripd:ripd/instance/distance/default", .cbs = { - .cli_show = cli_show_rip_distance, .modify = ripd_instance_distance_default_modify, }, }, { .xpath = "/frr-ripd:ripd/instance/distance/source", .cbs = { - .cli_show = cli_show_rip_distance_source, .create = ripd_instance_distance_source_create, .destroy = ripd_instance_distance_source_destroy, }, @@ -80,7 +74,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/explicit-neighbor", .cbs = { - .cli_show = cli_show_rip_neighbor, .create = ripd_instance_explicit_neighbor_create, .destroy = ripd_instance_explicit_neighbor_destroy, }, @@ -88,7 +81,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/network", .cbs = { - .cli_show = cli_show_rip_network_prefix, .create = ripd_instance_network_create, .destroy = ripd_instance_network_destroy, }, @@ -96,7 +88,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/interface", .cbs = { - .cli_show = cli_show_rip_network_interface, .create = ripd_instance_interface_create, .destroy = ripd_instance_interface_destroy, }, @@ -104,7 +95,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/offset-list", .cbs = { - .cli_show = cli_show_rip_offset_list, .create = ripd_instance_offset_list_create, .destroy = ripd_instance_offset_list_destroy, }, @@ -124,14 +114,12 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/passive-default", .cbs = { - .cli_show = cli_show_rip_passive_default, .modify = ripd_instance_passive_default_modify, }, }, { .xpath = "/frr-ripd:ripd/instance/passive-interface", .cbs = { - .cli_show = cli_show_rip_passive_interface, .create = ripd_instance_passive_interface_create, .destroy = ripd_instance_passive_interface_destroy, }, @@ -139,7 +127,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/non-passive-interface", .cbs = { - .cli_show = cli_show_rip_non_passive_interface, .create = ripd_instance_non_passive_interface_create, .destroy = ripd_instance_non_passive_interface_destroy, }, @@ -187,7 +174,6 @@ const struct frr_yang_module_info frr_ripd_info = { .xpath = "/frr-ripd:ripd/instance/redistribute", .cbs = { .apply_finish = ripd_instance_redistribute_apply_finish, - .cli_show = cli_show_rip_redistribute, .create = ripd_instance_redistribute_create, .destroy = ripd_instance_redistribute_destroy, }, @@ -211,7 +197,6 @@ const struct frr_yang_module_info frr_ripd_info = { .cbs = { .create = ripd_instance_if_route_maps_if_route_map_create, .destroy = ripd_instance_if_route_maps_if_route_map_destroy, - .cli_show = cli_show_if_route_map, } }, { @@ -231,7 +216,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-ripd:ripd/instance/static-route", .cbs = { - .cli_show = cli_show_rip_route, .create = ripd_instance_static_route_create, .destroy = ripd_instance_static_route_destroy, }, @@ -240,7 +224,6 @@ const struct frr_yang_module_info frr_ripd_info = { .xpath = "/frr-ripd:ripd/instance/timers", .cbs = { .apply_finish = ripd_instance_timers_apply_finish, - .cli_show = cli_show_rip_timers, }, }, { @@ -261,12 +244,6 @@ const struct frr_yang_module_info frr_ripd_info = { .modify = ripd_instance_timers_update_interval_modify, }, }, - { - .xpath = "/frr-ripd:ripd/instance/version", - .cbs = { - .cli_show = cli_show_rip_version, - }, - }, { .xpath = "/frr-ripd:ripd/instance/version/receive", .cbs = { @@ -284,43 +261,32 @@ const struct frr_yang_module_info frr_ripd_info = { .cbs = { .modify = ripd_instance_default_bfd_profile_modify, .destroy = ripd_instance_default_bfd_profile_destroy, - .cli_show = cli_show_ripd_instance_default_bfd_profile, }, }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/split-horizon", .cbs = { - .cli_show = cli_show_ip_rip_split_horizon, .modify = lib_interface_rip_split_horizon_modify, }, }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/v2-broadcast", .cbs = { - .cli_show = cli_show_ip_rip_v2_broadcast, .modify = lib_interface_rip_v2_broadcast_modify, }, }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/version-receive", .cbs = { - .cli_show = cli_show_ip_rip_receive_version, .modify = lib_interface_rip_version_receive_modify, }, }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/version-send", .cbs = { - .cli_show = cli_show_ip_rip_send_version, .modify = lib_interface_rip_version_send_modify, }, }, - { - .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-scheme", - .cbs = { - .cli_show = cli_show_ip_rip_authentication_scheme, - }, - }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-scheme/mode", .cbs = { @@ -337,7 +303,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-password", .cbs = { - .cli_show = cli_show_ip_rip_authentication_string, .destroy = lib_interface_rip_authentication_password_destroy, .modify = lib_interface_rip_authentication_password_modify, }, @@ -345,7 +310,6 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/authentication-key-chain", .cbs = { - .cli_show = cli_show_ip_rip_authentication_key_chain, .destroy = lib_interface_rip_authentication_key_chain_destroy, .modify = lib_interface_rip_authentication_key_chain_modify, }, @@ -360,14 +324,12 @@ const struct frr_yang_module_info frr_ripd_info = { { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/bfd-monitoring/enable", .cbs = { - .cli_show = cli_show_ip_rip_bfd_enable, .modify = lib_interface_rip_bfd_enable_modify, }, }, { .xpath = "/frr-interface:lib/interface/frr-ripd:rip/bfd-monitoring/profile", .cbs = { - .cli_show = cli_show_ip_rip_bfd_profile, .modify = lib_interface_rip_bfd_profile_modify, .destroy = lib_interface_rip_bfd_profile_destroy, }, diff --git a/ripd/rip_nb.h b/ripd/rip_nb.h index 811fee55ec25..7521e0fba92c 100644 --- a/ripd/rip_nb.h +++ b/ripd/rip_nb.h @@ -10,6 +10,7 @@ #include "northbound.h" extern const struct frr_yang_module_info frr_ripd_info; +extern const struct frr_yang_module_info frr_ripd_cli_info; /* Mandatory callbacks. */ int ripd_instance_create(struct nb_cb_create_args *args); diff --git a/ripd/rip_routemap.c b/ripd/rip_routemap.c index 2ae8857e3f12..be172774490a 100644 --- a/ripd/rip_routemap.c +++ b/ripd/rip_routemap.c @@ -531,7 +531,7 @@ static const struct route_map_rule_cmd route_set_tag_cmd = { /* Route-map init */ void rip_route_map_init(void) { - route_map_init(); + route_map_init_new(true); route_map_match_interface_hook(generic_match_add); route_map_no_match_interface_hook(generic_match_delete); diff --git a/ripd/ripd.c b/ripd/ripd.c index a744e081d19c..b8a140c9ca5a 100644 --- a/ripd/ripd.c +++ b/ripd/ripd.c @@ -34,6 +34,7 @@ #include "privs.h" #include "lib_errors.h" #include "northbound_cli.h" +#include "mgmt_be_client.h" #include "network.h" #include "lib/printfrr.h" #include "frrdistance.h" @@ -3253,42 +3254,6 @@ DEFUN (show_ip_rip_status, return CMD_SUCCESS; } -/* RIP configuration write function. */ -static int config_write_rip(struct vty *vty) -{ - struct rip *rip; - int write = 0; - - RB_FOREACH(rip, rip_instance_head, &rip_instances) { - char xpath[XPATH_MAXLEN]; - struct lyd_node *dnode; - - snprintf(xpath, sizeof(xpath), - "/frr-ripd:ripd/instance[vrf='%s']", rip->vrf_name); - - dnode = yang_dnode_get(running_config->dnode, xpath); - assert(dnode); - - nb_cli_show_dnode_cmds(vty, dnode, false); - - vty_out(vty, "exit\n"); - - write = 1; - } - - return write; -} - -static int config_write_rip(struct vty *vty); -/* RIP node structure. */ -static struct cmd_node rip_node = { - .name = "rip", - .node = RIP_NODE, - .parent_node = CONFIG_NODE, - .prompt = "%s(config-router)# ", - .config_write = config_write_rip, -}; - /* Distribute-list update functions. */ static void rip_distribute_update(struct distribute_ctx *ctx, struct distribute *dist) @@ -3650,8 +3615,6 @@ static int rip_vrf_disable(struct vrf *vrf) void rip_vrf_init(void) { vrf_init(rip_vrf_new, rip_vrf_enable, rip_vrf_disable, rip_vrf_delete); - - vrf_cmd_init(NULL); } void rip_vrf_terminate(void) @@ -3662,20 +3625,17 @@ void rip_vrf_terminate(void) /* Allocate new rip structure and set default value. */ void rip_init(void) { - /* Install top nodes. */ - install_node(&rip_node); - /* Install rip commands. */ install_element(VIEW_NODE, &show_ip_rip_cmd); install_element(VIEW_NODE, &show_ip_rip_status_cmd); - install_default(RIP_NODE); - /* Debug related init. */ rip_debug_init(); + /* Enable mgmt be debug */ + mgmt_be_client_lib_vty_init(); /* Access list install. */ - access_list_init(); + access_list_init_new(true); access_list_add_hook(rip_distribute_update_all_wrapper); access_list_delete_hook(rip_distribute_update_all_wrapper); @@ -3689,6 +3649,4 @@ void rip_init(void) route_map_add_hook(rip_routemap_update); route_map_delete_hook(rip_routemap_update); - - if_rmap_init(RIP_NODE); } diff --git a/ripd/subdir.am b/ripd/subdir.am index c793a6d68538..aed8d249fea2 100644 --- a/ripd/subdir.am +++ b/ripd/subdir.am @@ -14,7 +14,6 @@ endif ripd_ripd_SOURCES = \ ripd/rip_bfd.c \ - ripd/rip_cli.c \ ripd/rip_debug.c \ ripd/rip_errors.c \ ripd/rip_interface.c \ diff --git a/staticd/static_nb.h b/staticd/static_nb.h index 9f80653b762d..f929997a78e7 100644 --- a/staticd/static_nb.h +++ b/staticd/static_nb.h @@ -11,6 +11,7 @@ extern "C" { #endif extern const struct frr_yang_module_info frr_staticd_info; +extern const struct frr_yang_module_info frr_staticd_cli_info; /* Mandatory callbacks. */ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_create( diff --git a/staticd/static_vty.c b/staticd/static_vty.c index a641d1a09f28..95f79564af9e 100644 --- a/staticd/static_vty.c +++ b/staticd/static_vty.c @@ -1578,7 +1578,7 @@ static int static_path_list_cli_cmp(const struct lyd_node *dnode1, return (int)distance1 - (int)distance2; } -const struct frr_yang_module_info frr_staticd_info = { +const struct frr_yang_module_info frr_staticd_cli_info = { .name = "frr-staticd", .ignore_cfg_cbs = true, .nodes = { @@ -1714,5 +1714,7 @@ void static_vty_init(void) install_element(CONFIG_NODE, &ipv6_route_cmd); install_element(VRF_NODE, &ipv6_route_vrf_cmd); +#ifndef INCLUDE_MGMTD_CMDDEFS_ONLY mgmt_be_client_lib_vty_init(); +#endif } diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index e86eeeb2870e..6d2600cd9682 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2006,7 +2006,7 @@ DEFUNSH(VTYSH_KEYS, key, key_cmd, "key (0-2147483647)", } #ifdef HAVE_RIPD -DEFUNSH(VTYSH_RIPD, router_rip, router_rip_cmd, "router rip [vrf NAME]", +DEFUNSH(VTYSH_MGMTD, router_rip, router_rip_cmd, "router rip [vrf NAME]", ROUTER_STR "RIP\n" VRF_CMD_HELP_STR) { vty->node = RIP_NODE; @@ -2545,13 +2545,13 @@ DEFUNSH(VTYSH_ZEBRA, exit_srv6_encap, exit_srv6_encap_cmd, "exit", } #ifdef HAVE_RIPD -DEFUNSH(VTYSH_RIPD, vtysh_exit_ripd, vtysh_exit_ripd_cmd, "exit", +DEFUNSH(VTYSH_MGMTD, vtysh_exit_ripd, vtysh_exit_ripd_cmd, "exit", "Exit current mode and down to previous mode\n") { return vtysh_exit(vty); } -DEFUNSH(VTYSH_RIPD, vtysh_quit_ripd, vtysh_quit_ripd_cmd, "quit", +DEFUNSH(VTYSH_MGMTD, vtysh_quit_ripd, vtysh_quit_ripd_cmd, "quit", "Exit current mode and down to previous mode\n") { return vtysh_exit_ripd(self, vty, argc, argv);