From d205208f57855a219e9c72b8a24976af3eb34bcd Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 30 Jan 2024 22:51:46 +0200 Subject: [PATCH 1/3] Revert "staticd: Accept full blackhole typed keywords for ip_route_cmd" This reverts commit 76b2bc97e73874d882d5cf021972cfca84656cef. This change is wrong for several reasons: - it is backwards incompatible - previously it was always possible to create blackhole/reject routes using shortened versions of the words and it suddenly became impossible if there's an interface in the system with the same name - it uses operational data for validation which is prohibited - it doesn't really solve the problem with inability to create routes using interface names like `bla` or `rej` --- staticd/static_nb_config.c | 26 +------------------------- staticd/static_vty.c | 37 ++----------------------------------- 2 files changed, 3 insertions(+), 60 deletions(-) diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index 2fee908d5d1e..78378371b0a1 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -136,8 +136,7 @@ static bool static_nexthop_create(struct nb_cb_create_args *args) switch (args->event) { case NB_EV_VALIDATE: ifname = yang_dnode_get_string(args->dnode, "interface"); - nh_type = yang_dnode_get_enum(args->dnode, "nh-type"); - if (ifname != NULL && nh_type != STATIC_BLACKHOLE) { + if (ifname != NULL) { if (strcasecmp(ifname, "Null0") == 0 || strcasecmp(ifname, "reject") == 0 || strcasecmp(ifname, "blackhole") == 0) { @@ -465,33 +464,10 @@ static int static_nexthop_bh_type_modify(struct nb_cb_modify_args *args) { struct static_nexthop *nh; enum static_nh_type nh_type; - const char *nh_ifname; - const char *nh_vrf; switch (args->event) { case NB_EV_VALIDATE: nh_type = yang_dnode_get_enum(args->dnode, "../nh-type"); - nh_ifname = yang_dnode_get_string(args->dnode, "../interface"); - nh_vrf = yang_dnode_get_string(args->dnode, "../vrf"); - if (nh_ifname && nh_vrf) { - struct vrf *vrf = vrf_lookup_by_name(nh_vrf); - - if (!vrf) { - snprintf(args->errmsg, args->errmsg_len, - "nexthop vrf %s not found", nh_vrf); - return NB_ERR_VALIDATION; - } - - struct interface *ifp = if_lookup_by_name(nh_ifname, - vrf->vrf_id); - - if (ifp && (!strmatch(nh_ifname, "blackhole") || - !strmatch(nh_ifname, "reject"))) { - snprintf(args->errmsg, args->errmsg_len, - "nexthop interface name must be (reject, blackhole)"); - return NB_ERR_VALIDATION; - } - } if (nh_type != STATIC_BLACKHOLE) { snprintf(args->errmsg, args->errmsg_len, "nexthop type is not the blackhole type"); diff --git a/staticd/static_vty.c b/staticd/static_vty.c index 05f23f54d1c9..a18028ed087c 100644 --- a/staticd/static_vty.c +++ b/staticd/static_vty.c @@ -60,8 +60,6 @@ struct static_route_args { bool bfd_multi_hop; const char *bfd_source; const char *bfd_profile; - - const char *input; }; static int static_route_nb_run(struct vty *vty, struct static_route_args *args) @@ -153,20 +151,9 @@ static int static_route_nb_run(struct vty *vty, struct static_route_args *args) else buf_gate_str = ""; - if (args->gateway == NULL && args->interface_name == NULL) { + if (args->gateway == NULL && args->interface_name == NULL) type = STATIC_BLACKHOLE; - /* If this is blackhole/reject flagged route, then - * specify interface_name with the value of what was really - * entered. - * interface_name will be validated later in NB functions - * to check if we don't create blackhole/reject routes that - * match the real interface names. - * E.g.: `ip route 10.0.0.1/32 bla` will create a blackhole - * route despite the real interface named `bla` exists. - */ - if (args->input) - args->interface_name = args->input; - } else if (args->gateway && args->interface_name) { + else if (args->gateway && args->interface_name) { if (args->afi == AFI_IP) type = STATIC_IPV4_GATEWAY_IFNAME; else @@ -553,8 +540,6 @@ DEFPY_YANG(ip_route_blackhole, "Table to configure\n" "The table number to configure\n") { - int idx_flag = 0; - struct static_route_args args = { .delete = !!no, .afi = AFI_IP, @@ -569,9 +554,6 @@ DEFPY_YANG(ip_route_blackhole, .vrf = vrf, }; - if (flag && argv_find(argv, argc, flag, &idx_flag)) - args.input = argv[idx_flag]->arg; - return static_route_nb_run(vty, &args); } @@ -600,8 +582,6 @@ DEFPY_YANG(ip_route_blackhole_vrf, "Table to configure\n" "The table number to configure\n") { - int idx_flag = 0; - struct static_route_args args = { .delete = !!no, .afi = AFI_IP, @@ -623,9 +603,6 @@ DEFPY_YANG(ip_route_blackhole_vrf, */ assert(args.prefix); - if (flag && argv_find(argv, argc, flag, &idx_flag)) - args.input = argv[idx_flag]->arg; - return static_route_nb_run(vty, &args); } @@ -916,8 +893,6 @@ DEFPY_YANG(ipv6_route_blackhole, "Table to configure\n" "The table number to configure\n") { - int idx_flag = 0; - struct static_route_args args = { .delete = !!no, .afi = AFI_IP6, @@ -932,9 +907,6 @@ DEFPY_YANG(ipv6_route_blackhole, .vrf = vrf, }; - if (flag && argv_find(argv, argc, flag, &idx_flag)) - args.input = argv[idx_flag]->arg; - return static_route_nb_run(vty, &args); } @@ -963,8 +935,6 @@ DEFPY_YANG(ipv6_route_blackhole_vrf, "Table to configure\n" "The table number to configure\n") { - int idx_flag = 0; - struct static_route_args args = { .delete = !!no, .afi = AFI_IP6, @@ -986,9 +956,6 @@ DEFPY_YANG(ipv6_route_blackhole_vrf, */ assert(args.prefix); - if (flag && argv_find(argv, argc, flag, &idx_flag)) - args.input = argv[idx_flag]->arg; - return static_route_nb_run(vty, &args); } From 5dfa36b6a7bf52041e61ea3bdcf8fd7c92fc61af Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 31 Jan 2024 14:10:09 +0200 Subject: [PATCH 2/3] staticd: fix nexthops without interface When interface is not set in "ip route" command, CLI passes "(null)" as an interface name instead of an empty string. The actual code in turn uses "nh->ifname[0] != 0" to check if the interface name was set. Fix the problem by changing the "(null)" string into an empty string when populating the nexthop structure. Signed-off-by: Igor Ryzhov --- staticd/static_nb_config.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index 78378371b0a1..d7605cb34775 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -176,6 +176,9 @@ static bool static_nexthop_create(struct nb_cb_create_args *args) nh_vrf = yang_dnode_get_string(args->dnode, "vrf"); pn = nb_running_get_entry(args->dnode, NULL, true); + if (strmatch(ifname, "(null)")) + ifname = ""; + if (!static_add_nexthop_validate(nh_vrf, nh_type, &ipaddr)) flog_warn( EC_LIB_NB_CB_CONFIG_VALIDATE, From cb781f60972aca7e0fdd18884814a9ef0403a67c Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 2 Feb 2024 00:57:59 +0200 Subject: [PATCH 3/3] staticd: fix NB dependency hack Currently, staticd configuration is tightly coupled with VRF existence. Because of that, it has to use a hack in NB infrastructure to create a VRF configuration when at least one static route is configured for this VRF. This hack is incompatible with mgmtd, because mgmtd doesn't execute configuration callbacks. Because of that, the configuration may become out of sync between mgmtd and staticd. There are two main cases: 1. Create static route in a VRF. The VRF data node will be created automatically in staticd by the NB hack, but not in mgmtd. 2. Delete VRF which has some static routes configured. The static route configuration will be deleted from staticd by the NB hack, but not from mgmtd. To fix the problem, decouple configuration of static routes from VRF configuration. Now it is possible to configure static routes even if the VRF doesn't exist yet. Once the VRF is created, staticd applies all the preconfigured routes. This change also fixes the problem with static routes being preserved in the system when staticd "control-plane-protocol" container is deleted but the VRF is still configured. Signed-off-by: Igor Ryzhov --- lib/routing_nb.h | 2 + lib/routing_nb_config.c | 5 ++ staticd/static_bfd.c | 16 ++-- staticd/static_main.c | 6 +- staticd/static_nb.h | 5 ++ staticd/static_nb_config.c | 52 +++++++++++-- staticd/static_routes.c | 152 ++++++++++--------------------------- staticd/static_routes.h | 7 +- staticd/static_vrf.c | 110 ++++++++++++++++++--------- staticd/static_vrf.h | 14 +++- staticd/static_zebra.c | 7 +- tests/lib/test_grpc.cpp | 6 +- 12 files changed, 207 insertions(+), 175 deletions(-) diff --git a/lib/routing_nb.h b/lib/routing_nb.h index c185091a4bce..cc83d8469d2d 100644 --- a/lib/routing_nb.h +++ b/lib/routing_nb.h @@ -29,6 +29,8 @@ int routing_control_plane_protocols_control_plane_protocol_destroy( * based on the control plane protocol */ DECLARE_HOOK(routing_conf_event, (struct nb_cb_create_args *args), (args)); +DECLARE_HOOK(routing_create, (struct nb_cb_create_args *args), (args)); +DECLARE_KOOH(routing_destroy, (struct nb_cb_destroy_args *args), (args)); void routing_control_plane_protocols_register_vrf_dependency(void); diff --git a/lib/routing_nb_config.c b/lib/routing_nb_config.c index 2b20e6c14b5c..d532279a22aa 100644 --- a/lib/routing_nb_config.c +++ b/lib/routing_nb_config.c @@ -14,6 +14,8 @@ DEFINE_HOOK(routing_conf_event, (struct nb_cb_create_args *args), (args)); +DEFINE_HOOK(routing_create, (struct nb_cb_create_args *args), (args)); +DEFINE_KOOH(routing_destroy, (struct nb_cb_destroy_args *args), (args)); /* * XPath: /frr-routing:routing/control-plane-protocols/control-plane-protocol @@ -49,6 +51,7 @@ int routing_control_plane_protocols_control_plane_protocol_create( assert(vrf); nb_running_set_entry(args->dnode, vrf); } + hook_call(routing_create, args); break; }; @@ -61,6 +64,8 @@ int routing_control_plane_protocols_control_plane_protocol_destroy( if (args->event != NB_EV_APPLY) return NB_OK; + hook_call(routing_destroy, args); + /* * If dependency on VRF module is registered, then VRF * pointer was stored and must be cleared. diff --git a/staticd/static_bfd.c b/staticd/static_bfd.c index b0b256615085..c35751f3972b 100644 --- a/staticd/static_bfd.c +++ b/staticd/static_bfd.c @@ -263,14 +263,13 @@ static void static_bfd_show_path_json(struct vty *vty, struct json_object *jo, static void static_bfd_show_json(struct vty *vty) { struct json_object *jo, *jo_path, *jo_afi_safi; - struct vrf *vrf; + struct static_vrf *svrf; jo = json_object_new_object(); jo_path = json_object_new_object(); json_object_object_add(jo, "path-list", jo_path); - RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { - const struct static_vrf *svrf = vrf->info; + RB_FOREACH (svrf, svrf_name_head, &svrfs) { struct route_table *rt; jo_afi_safi = json_object_new_array(); @@ -346,7 +345,7 @@ static void static_bfd_show_path(struct vty *vty, struct route_table *rt) void static_bfd_show(struct vty *vty, bool json) { - struct vrf *vrf; + struct static_vrf *svrf; if (json) { static_bfd_show_json(vty); @@ -355,21 +354,20 @@ void static_bfd_show(struct vty *vty, bool json) vty_out(vty, "Showing BFD monitored static routes:\n"); vty_out(vty, "\n Next hops:\n"); - RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { - const struct static_vrf *svrf = vrf->info; + RB_FOREACH (svrf, svrf_name_head, &svrfs) { struct route_table *rt; - vty_out(vty, " VRF %s IPv4 Unicast:\n", vrf->name); + vty_out(vty, " VRF %s IPv4 Unicast:\n", svrf->name); rt = svrf->stable[AFI_IP][SAFI_UNICAST]; if (rt) static_bfd_show_path(vty, rt); - vty_out(vty, "\n VRF %s IPv4 Multicast:\n", vrf->name); + vty_out(vty, "\n VRF %s IPv4 Multicast:\n", svrf->name); rt = svrf->stable[AFI_IP][SAFI_MULTICAST]; if (rt) static_bfd_show_path(vty, rt); - vty_out(vty, "\n VRF %s IPv6 Unicast:\n", vrf->name); + vty_out(vty, "\n VRF %s IPv6 Unicast:\n", svrf->name); rt = svrf->stable[AFI_IP6][SAFI_UNICAST]; if (rt) static_bfd_show_path(vty, rt); diff --git a/staticd/static_main.c b/staticd/static_main.c index 2fd174194841..9468a98b833e 100644 --- a/staticd/static_main.c +++ b/staticd/static_main.c @@ -168,8 +168,10 @@ int main(int argc, char **argv, char **envp) hook_register(routing_conf_event, routing_control_plane_protocols_name_validate); - - routing_control_plane_protocols_register_vrf_dependency(); + hook_register(routing_create, + routing_control_plane_protocols_staticd_create); + hook_register(routing_destroy, + routing_control_plane_protocols_staticd_destroy); /* * We set FRR_NO_SPLIT_CONFIG flag to avoid reading our config, but we diff --git a/staticd/static_nb.h b/staticd/static_nb.h index f929997a78e7..be75d9d38ce3 100644 --- a/staticd/static_nb.h +++ b/staticd/static_nb.h @@ -13,6 +13,11 @@ extern "C" { extern const struct frr_yang_module_info frr_staticd_info; extern const struct frr_yang_module_info frr_staticd_cli_info; +int routing_control_plane_protocols_staticd_create( + struct nb_cb_create_args *args); +int routing_control_plane_protocols_staticd_destroy( + struct nb_cb_destroy_args *args); + /* Mandatory callbacks. */ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_create( struct nb_cb_create_args *args); diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index d7605cb34775..7de5f0474aad 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -540,6 +540,48 @@ int routing_control_plane_protocols_name_validate( } return NB_OK; } + +/* + * XPath: + * /frr-routing:routing/control-plane-protocols/control-plane-protocol + */ +int routing_control_plane_protocols_staticd_create(struct nb_cb_create_args *args) +{ + struct static_vrf *svrf; + const char *vrf; + + vrf = yang_dnode_get_string(args->dnode, "vrf"); + svrf = static_vrf_alloc(vrf); + nb_running_set_entry(args->dnode, svrf); + + return NB_OK; +} + +int routing_control_plane_protocols_staticd_destroy( + struct nb_cb_destroy_args *args) +{ + struct static_vrf *svrf; + struct route_table *stable; + struct route_node *rn; + afi_t afi; + safi_t safi; + + svrf = nb_running_unset_entry(args->dnode); + + FOREACH_AFI_SAFI (afi, safi) { + stable = svrf->stable[afi][safi]; + if (!stable) + continue; + + for (rn = route_top(stable); rn; rn = route_next(rn)) + static_del_route(rn); + } + + static_vrf_free(svrf); + + return NB_OK; +} + /* * XPath: * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list @@ -547,8 +589,7 @@ int routing_control_plane_protocols_name_validate( int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_create( struct nb_cb_create_args *args) { - struct vrf *vrf; - struct static_vrf *s_vrf; + struct static_vrf *svrf; struct route_node *rn; const struct lyd_node *vrf_dnode; struct prefix prefix; @@ -577,15 +618,14 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_cr case NB_EV_APPLY: vrf_dnode = yang_dnode_get_parent(args->dnode, "control-plane-protocol"); - vrf = nb_running_get_entry(vrf_dnode, NULL, true); - s_vrf = vrf->info; + svrf = nb_running_get_entry(vrf_dnode, NULL, true); yang_dnode_get_prefix(&prefix, args->dnode, "prefix"); afi_safi = yang_dnode_get_string(args->dnode, "afi-safi"); yang_afi_safi_identity2value(afi_safi, &afi, &safi); - rn = static_add_route(afi, safi, &prefix, NULL, s_vrf); - if (vrf->vrf_id == VRF_UNKNOWN) + rn = static_add_route(afi, safi, &prefix, NULL, svrf); + if (!svrf->vrf || svrf->vrf->vrf_id == VRF_UNKNOWN) snprintf( args->errmsg, args->errmsg_len, "Static Route to %s not installed currently because dependent config not fully available", diff --git a/staticd/static_routes.c b/staticd/static_routes.c index 1fbbf7e99d34..db3fc32fd88e 100644 --- a/staticd/static_routes.c +++ b/staticd/static_routes.c @@ -245,21 +245,20 @@ void static_del_path(struct static_path *pn) XFREE(MTYPE_STATIC_PATH, pn); } -struct static_nexthop *static_add_nexthop(struct static_path *pn, - enum static_nh_type type, - struct ipaddr *ipaddr, - const char *ifname, - const char *nh_vrf, uint32_t color) +struct static_nexthop * +static_add_nexthop(struct static_path *pn, enum static_nh_type type, + struct ipaddr *ipaddr, const char *ifname, + const char *nh_vrfname, uint32_t color) { struct route_node *rn = pn->rn; struct static_nexthop *nh; - struct static_vrf *nh_svrf; + struct vrf *nh_vrf; struct interface *ifp; struct static_nexthop *cp; route_lock_node(rn); - nh_svrf = static_vrf_lookup_by_name(nh_vrf); + nh_vrf = vrf_lookup_by_name(nh_vrfname); /* Make new static route structure. */ nh = XCALLOC(MTYPE_STATIC_NEXTHOP, sizeof(struct static_nexthop)); @@ -274,8 +273,8 @@ struct static_nexthop *static_add_nexthop(struct static_path *pn, if (nh->type == STATIC_BLACKHOLE) nh->bh_type = STATIC_BLACKHOLE_NULL; - nh->nh_vrf_id = nh_svrf ? nh_svrf->vrf->vrf_id : VRF_UNKNOWN; - strlcpy(nh->nh_vrfname, nh_vrf, sizeof(nh->nh_vrfname)); + nh->nh_vrf_id = nh_vrf ? nh_vrf->vrf_id : VRF_UNKNOWN; + strlcpy(nh->nh_vrfname, nh_vrfname, sizeof(nh->nh_vrfname)); if (ifname) strlcpy(nh->ifname, ifname, sizeof(nh->ifname)); @@ -437,14 +436,10 @@ static void static_ifindex_update_af(struct interface *ifp, bool up, afi_t afi, struct route_node *rn; struct static_nexthop *nh; struct static_path *pn; - struct vrf *vrf; + struct static_vrf *svrf; struct static_route_info *si; - RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { - struct static_vrf *svrf; - - svrf = vrf->info; - + RB_FOREACH (svrf, svrf_name_head, &svrfs) { stable = static_vrf_static_table(afi, safi, svrf); if (!stable) continue; @@ -476,8 +471,8 @@ static void static_ifindex_update_af(struct interface *ifp, bool up, afi_t afi, * afi -> The afi to look at * safi -> the safi to look at */ -static void static_fixup_vrf(struct static_vrf *svrf, - struct route_table *stable, afi_t afi, safi_t safi) +static void static_fixup_vrf(struct vrf *vrf, struct route_table *stable, + afi_t afi, safi_t safi) { struct route_node *rn; struct static_nexthop *nh; @@ -491,13 +486,12 @@ static void static_fixup_vrf(struct static_vrf *svrf, continue; frr_each(static_path_list, &si->path_list, pn) { frr_each(static_nexthop_list, &pn->nexthop_list, nh) { - if (strcmp(svrf->vrf->name, nh->nh_vrfname) - != 0) + if (strcmp(vrf->name, nh->nh_vrfname) != 0) continue; - nh->nh_vrf_id = svrf->vrf->vrf_id; + nh->nh_vrf_id = vrf->vrf_id; nh->nh_registered = false; - if (nh->ifindex) { + if (nh->ifname[0]) { ifp = if_lookup_by_name(nh->ifname, nh->nh_vrf_id); if (ifp) @@ -521,9 +515,7 @@ static void static_fixup_vrf(struct static_vrf *svrf, * afi -> the afi in question * safi -> the safi in question */ -static void static_enable_vrf(struct static_vrf *svrf, - struct route_table *stable, afi_t afi, - safi_t safi) +static void static_enable_vrf(struct route_table *stable, afi_t afi, safi_t safi) { struct route_node *rn; struct static_nexthop *nh; @@ -537,7 +529,9 @@ static void static_enable_vrf(struct static_vrf *svrf, continue; frr_each(static_path_list, &si->path_list, pn) { frr_each(static_nexthop_list, &pn->nexthop_list, nh) { - if (nh->ifindex) { + if (nh->nh_vrf_id == VRF_UNKNOWN) + continue; + if (nh->ifname[0]) { ifp = if_lookup_by_name(nh->ifname, nh->nh_vrf_id); if (ifp) @@ -545,8 +539,7 @@ static void static_enable_vrf(struct static_vrf *svrf, else continue; } - if (nh->nh_vrf_id == VRF_UNKNOWN) - continue; + static_install_path(pn); } } @@ -560,27 +553,26 @@ static void static_enable_vrf(struct static_vrf *svrf, * * enable_svrf -> the vrf being enabled */ -void static_fixup_vrf_ids(struct static_vrf *enable_svrf) +void static_fixup_vrf_ids(struct vrf *vrf) { struct route_table *stable; - struct vrf *vrf; + struct static_vrf *svrf, *enable_svrf; afi_t afi; safi_t safi; - RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { - struct static_vrf *svrf; + enable_svrf = vrf->info; - svrf = vrf->info; + RB_FOREACH (svrf, svrf_name_head, &svrfs) { /* Install any static routes configured for this VRF. */ FOREACH_AFI_SAFI (afi, safi) { stable = svrf->stable[afi][safi]; if (!stable) continue; - static_fixup_vrf(enable_svrf, stable, afi, safi); + static_fixup_vrf(vrf, stable, afi, safi); if (enable_svrf == svrf) - static_enable_vrf(svrf, stable, afi, safi); + static_enable_vrf(stable, afi, safi); } } } @@ -595,8 +587,7 @@ void static_fixup_vrf_ids(struct static_vrf *enable_svrf) * afi -> the afi in question * safi -> the safi in question */ -static void static_cleanup_vrf(struct static_vrf *svrf, - struct route_table *stable, +static void static_cleanup_vrf(struct vrf *vrf, struct route_table *stable, afi_t afi, safi_t safi) { struct route_node *rn; @@ -610,11 +601,13 @@ static void static_cleanup_vrf(struct static_vrf *svrf, continue; frr_each(static_path_list, &si->path_list, pn) { frr_each(static_nexthop_list, &pn->nexthop_list, nh) { - if (strcmp(svrf->vrf->name, nh->nh_vrfname) - != 0) + if (strcmp(vrf->name, nh->nh_vrfname) != 0) continue; static_uninstall_path(pn); + + nh->nh_vrf_id = VRF_UNKNOWN; + nh->ifindex = IFINDEX_INTERNAL; } } } @@ -642,6 +635,9 @@ static void static_disable_vrf(struct route_table *stable, continue; frr_each(static_path_list, &si->path_list, pn) { frr_each(static_nexthop_list, &pn->nexthop_list, nh) { + if (nh->nh_vrf_id == VRF_UNKNOWN) + continue; + static_uninstall_path(pn); } } @@ -656,26 +652,23 @@ static void static_disable_vrf(struct route_table *stable, * * disable_svrf - The vrf being disabled */ -void static_cleanup_vrf_ids(struct static_vrf *disable_svrf) +void static_cleanup_vrf_ids(struct vrf *vrf) { - struct vrf *vrf; + struct route_table *stable; + struct static_vrf *svrf, *disable_svrf; afi_t afi; safi_t safi; - RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { - struct static_vrf *svrf; - - svrf = vrf->info; + disable_svrf = vrf->info; + RB_FOREACH (svrf, svrf_name_head, &svrfs) { /* Uninstall any static routes configured for this VRF. */ FOREACH_AFI_SAFI (afi, safi) { - struct route_table *stable; - stable = svrf->stable[afi][safi]; if (!stable) continue; - static_cleanup_vrf(disable_svrf, stable, afi, safi); + static_cleanup_vrf(vrf, stable, afi, safi); if (disable_svrf == svrf) static_disable_vrf(stable, afi, safi); @@ -683,71 +676,6 @@ void static_cleanup_vrf_ids(struct static_vrf *disable_svrf) } } -/* - * This function enables static routes when an interface it relies - * on in a different vrf is coming up. - * - * stable -> The stable we are looking at. - * ifp -> interface coming up - * afi -> the afi in question - * safi -> the safi in question - */ -static void static_fixup_intf_nh(struct route_table *stable, - struct interface *ifp, - afi_t afi, safi_t safi) -{ - struct route_node *rn; - struct static_nexthop *nh; - struct static_path *pn; - struct static_route_info *si; - - for (rn = route_top(stable); rn; rn = route_next(rn)) { - si = static_route_info_from_rnode(rn); - if (!si) - continue; - frr_each(static_path_list, &si->path_list, pn) { - frr_each(static_nexthop_list, &pn->nexthop_list, nh) { - if (nh->nh_vrf_id != ifp->vrf->vrf_id) - continue; - - if (nh->ifindex != ifp->ifindex) - continue; - - static_install_path(pn); - } - } - } -} - -/* - * This function enables static routes that rely on an interface in - * a different vrf when that interface comes up. - */ -void static_install_intf_nh(struct interface *ifp) -{ - struct route_table *stable; - struct vrf *vrf; - afi_t afi; - safi_t safi; - - RB_FOREACH(vrf, vrf_name_head, &vrfs_by_name) { - struct static_vrf *svrf = vrf->info; - - /* Not needed if same vrf since happens naturally */ - if (vrf->vrf_id == ifp->vrf->vrf_id) - continue; - - /* Install any static routes configured for this interface. */ - FOREACH_AFI_SAFI (afi, safi) { - stable = svrf->stable[afi][safi]; - if (!stable) - continue; - - static_fixup_intf_nh(stable, ifp, afi, safi); - } - } -} - /* called from if_{add,delete}_update, i.e. when ifindex becomes [in]valid */ void static_ifindex_update(struct interface *ifp, bool up) { diff --git a/staticd/static_routes.h b/staticd/static_routes.h index 1231ead52675..d88ed293645e 100644 --- a/staticd/static_routes.h +++ b/staticd/static_routes.h @@ -199,7 +199,8 @@ extern uint32_t zebra_ecmp_count; extern struct zebra_privs_t static_privs; -void static_fixup_vrf_ids(struct static_vrf *svrf); +extern void static_fixup_vrf_ids(struct vrf *vrf); +extern void static_cleanup_vrf_ids(struct vrf *vrf); extern struct static_nexthop * static_add_nexthop(struct static_path *pn, enum static_nh_type type, @@ -209,10 +210,6 @@ extern void static_install_nexthop(struct static_nexthop *nh); extern void static_delete_nexthop(struct static_nexthop *nh); -extern void static_cleanup_vrf_ids(struct static_vrf *disable_svrf); - -extern void static_install_intf_nh(struct interface *ifp); - extern void static_ifindex_update(struct interface *ifp, bool up); extern void static_install_path(struct static_path *pn); diff --git a/staticd/static_vrf.c b/staticd/static_vrf.c index 7a0ff01d0494..710827a9ff49 100644 --- a/staticd/static_vrf.c +++ b/staticd/static_vrf.c @@ -18,16 +18,37 @@ DEFINE_MTYPE_STATIC(STATIC, STATIC_RTABLE_INFO, "Static Route Table Info"); -static struct static_vrf *static_vrf_alloc(void) +static int svrf_name_compare(const struct static_vrf *a, + const struct static_vrf *b) +{ + return strcmp(a->name, b->name); +} + +RB_GENERATE(svrf_name_head, static_vrf, entry, svrf_name_compare); + +struct svrf_name_head svrfs = RB_INITIALIZER(&svrfs); + +static struct static_vrf *static_vrf_lookup_by_name(const char *name) +{ + struct static_vrf svrf; + + strlcpy(svrf.name, name, sizeof(svrf.name)); + return RB_FIND(svrf_name_head, &svrfs, &svrf); +} + +struct static_vrf *static_vrf_alloc(const char *name) { struct route_table *table; struct static_vrf *svrf; struct stable_info *info; + struct vrf *vrf; safi_t safi; afi_t afi; svrf = XCALLOC(MTYPE_STATIC_RTABLE_INFO, sizeof(struct static_vrf)); + strlcpy(svrf->name, name, sizeof(svrf->name)); + for (afi = AFI_IP; afi <= AFI_IP6; afi++) { for (safi = SAFI_UNICAST; safi <= SAFI_MULTICAST; safi++) { if (afi == AFI_IP6) @@ -46,16 +67,56 @@ static struct static_vrf *static_vrf_alloc(void) svrf->stable[afi][safi] = table; } } + + RB_INSERT(svrf_name_head, &svrfs, svrf); + + vrf = vrf_lookup_by_name(name); + if (vrf) { + svrf->vrf = vrf; + vrf->info = svrf; + } + return svrf; } +void static_vrf_free(struct static_vrf *svrf) +{ + struct route_table *table; + struct vrf *vrf; + safi_t safi; + afi_t afi; + void *info; + + vrf = svrf->vrf; + if (vrf) { + vrf->info = NULL; + svrf->vrf = NULL; + } + + RB_REMOVE(svrf_name_head, &svrfs, svrf); + + for (afi = AFI_IP; afi <= AFI_IP6; afi++) { + for (safi = SAFI_UNICAST; safi <= SAFI_MULTICAST; safi++) { + table = svrf->stable[afi][safi]; + info = route_table_get_info(table); + route_table_finish(table); + XFREE(MTYPE_STATIC_RTABLE_INFO, info); + svrf->stable[afi][safi] = NULL; + } + } + + XFREE(MTYPE_STATIC_RTABLE_INFO, svrf); +} + static int static_vrf_new(struct vrf *vrf) { struct static_vrf *svrf; - svrf = static_vrf_alloc(); - vrf->info = svrf; - svrf->vrf = vrf; + svrf = static_vrf_lookup_by_name(vrf->name); + if (svrf) { + vrf->info = svrf; + svrf->vrf = vrf; + } return 0; } @@ -63,37 +124,27 @@ static int static_vrf_new(struct vrf *vrf) static int static_vrf_enable(struct vrf *vrf) { static_zebra_vrf_register(vrf); - - static_fixup_vrf_ids(vrf->info); - + static_fixup_vrf_ids(vrf); return 0; } static int static_vrf_disable(struct vrf *vrf) { + static_cleanup_vrf_ids(vrf); static_zebra_vrf_unregister(vrf); return 0; } static int static_vrf_delete(struct vrf *vrf) { - struct route_table *table; struct static_vrf *svrf; - safi_t safi; - afi_t afi; - void *info; svrf = vrf->info; - for (afi = AFI_IP; afi <= AFI_IP6; afi++) { - for (safi = SAFI_UNICAST; safi <= SAFI_MULTICAST; safi++) { - table = svrf->stable[afi][safi]; - info = route_table_get_info(table); - route_table_finish(table); - XFREE(MTYPE_STATIC_RTABLE_INFO, info); - svrf->stable[afi][safi] = NULL; - } + if (svrf) { + svrf->vrf = NULL; + vrf->info = NULL; } - XFREE(MTYPE_STATIC_RTABLE_INFO, svrf); + return 0; } @@ -110,20 +161,6 @@ struct route_table *static_vrf_static_table(afi_t afi, safi_t safi, return svrf->stable[afi][safi]; } -struct static_vrf *static_vrf_lookup_by_name(const char *name) -{ - struct vrf *vrf; - - if (!name) - name = VRF_DEFAULT_NAME; - - vrf = vrf_lookup_by_name(name); - if (vrf) - return ((struct static_vrf *)vrf->info); - - return NULL; -} - void static_vrf_init(void) { vrf_init(static_vrf_new, static_vrf_enable, static_vrf_disable, @@ -134,5 +171,10 @@ void static_vrf_init(void) void static_vrf_terminate(void) { + struct static_vrf *svrf, *svrf_next; + + RB_FOREACH_SAFE (svrf, svrf_name_head, &svrfs, svrf_next) + static_vrf_free(svrf); + vrf_terminate(); } diff --git a/staticd/static_vrf.h b/staticd/static_vrf.h index 8f55775d3eec..26ee28fd816d 100644 --- a/staticd/static_vrf.h +++ b/staticd/static_vrf.h @@ -7,15 +7,27 @@ #ifndef __STATIC_VRF_H__ #define __STATIC_VRF_H__ +#include "openbsd-tree.h" + #ifdef __cplusplus extern "C" { #endif struct static_vrf { + RB_ENTRY(static_vrf) entry; + + char name[VRF_NAMSIZ + 1]; struct vrf *vrf; struct route_table *stable[AFI_MAX][SAFI_MAX]; }; +RB_HEAD(svrf_name_head, static_vrf); +RB_PROTOTYPE(svrf_name_head, static_vrf, entry, svrf_name_compare) + +extern struct svrf_name_head svrfs; + +struct static_vrf *static_vrf_alloc(const char *name); +void static_vrf_free(struct static_vrf *svrf); struct stable_info { struct static_vrf *svrf; @@ -25,8 +37,6 @@ struct stable_info { #define GET_STABLE_VRF_ID(info) info->svrf->vrf->vrf_id -struct static_vrf *static_vrf_lookup_by_name(const char *vrf_name); - void static_vrf_init(void); struct route_table *static_vrf_static_table(afi_t afi, safi_t safi, diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index 68761c0084bd..697afa515644 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -111,8 +111,6 @@ static int interface_address_delete(ZAPI_CALLBACK_ARGS) static int static_ifp_up(struct interface *ifp) { - /* Install any static reliant on this interface coming up */ - static_install_intf_nh(ifp); static_ifindex_update(ifp, true); return 0; @@ -169,7 +167,7 @@ static void zebra_connected(struct zclient *zclient) zebra_route_notify_send(ZEBRA_ROUTE_NOTIFY_REQUEST, zclient, true); zclient_send_reg_requests(zclient, VRF_DEFAULT); - static_fixup_vrf_ids(vrf_info_lookup(VRF_DEFAULT)); + static_fixup_vrf_ids(vrf_lookup_by_id(VRF_DEFAULT)); } /* API to check whether the configured nexthop address is @@ -388,6 +386,9 @@ extern void static_zebra_route_add(struct static_path *pn, bool install) struct zapi_route api; uint32_t nh_num = 0; + if (!si->svrf->vrf) + return; + p = src_pp = NULL; srcdest_rnode_prefixes(rn, &p, &src_pp); diff --git a/tests/lib/test_grpc.cpp b/tests/lib/test_grpc.cpp index 957ffdefaae7..202313603d0e 100644 --- a/tests/lib/test_grpc.cpp +++ b/tests/lib/test_grpc.cpp @@ -119,8 +119,10 @@ static void static_startup(void) hook_register(routing_conf_event, routing_control_plane_protocols_name_validate); - - routing_control_plane_protocols_register_vrf_dependency(); + hook_register(routing_create, + routing_control_plane_protocols_staticd_create); + hook_register(routing_destroy, + routing_control_plane_protocols_staticd_destroy); // Add a route vty = vty_new();