From 500a09d2dccc51da6fa1cc201e9e2c1923adc729 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 14:53:37 +0100 Subject: [PATCH 1/8] pbrd: replace `receive_notify` with request Send `ZEBRA_ROUTE_NOTIFY_REQUEST` rather than relying on the options field in zclient startup. Signed-off-by: David Lamparter --- pbrd/pbr_zebra.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index 2e2302b29da5..dd15beaff440 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -222,6 +222,8 @@ static int rule_notify_owner(ZAPI_CALLBACK_ARGS) static void zebra_connected(struct zclient *zclient) { DEBUGD(&pbr_dbg_zebra, "%s: Registering for fun and profit", __func__); + + zebra_route_notify_send(ZEBRA_ROUTE_NOTIFY_REQUEST, zclient, true); zclient_send_reg_requests(zclient, VRF_DEFAULT); } @@ -401,9 +403,7 @@ static zclient_handler *const pbr_handlers[] = { void pbr_zebra_init(void) { - struct zclient_options opt = { .receive_notify = true }; - - zclient = zclient_new(master, &opt, pbr_handlers, + zclient = zclient_new(master, &zclient_options_default, pbr_handlers, array_size(pbr_handlers)); zclient_init(zclient, ZEBRA_ROUTE_PBR, 0, &pbr_privs); From adf7b9efd324cf4d3782fa5ecda79e5dcd852985 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 14:53:37 +0100 Subject: [PATCH 2/8] sharpd: replace `receive_notify` with request Send `ZEBRA_ROUTE_NOTIFY_REQUEST` rather than relying on the options field in zclient startup. Signed-off-by: David Lamparter --- sharpd/sharp_zebra.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 40921ecdd38a..aa720bacf2a6 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -512,6 +512,7 @@ static int route_notify_owner(ZAPI_CALLBACK_ARGS) static void zebra_connected(struct zclient *zclient) { + zebra_route_notify_send(ZEBRA_ROUTE_NOTIFY_REQUEST, zclient, true); zclient_send_reg_requests(zclient, VRF_DEFAULT); /* @@ -1067,14 +1068,12 @@ static zclient_handler *const sharp_handlers[] = { void sharp_zebra_init(void) { - struct zclient_options opt = {.receive_notify = true}; - hook_register_prio(if_real, 0, sharp_ifp_create); hook_register_prio(if_up, 0, sharp_ifp_up); hook_register_prio(if_down, 0, sharp_ifp_down); hook_register_prio(if_unreal, 0, sharp_ifp_destroy); - zclient = zclient_new(master, &opt, sharp_handlers, + zclient = zclient_new(master, &zclient_options_default, sharp_handlers, array_size(sharp_handlers)); zclient_init(zclient, ZEBRA_ROUTE_SHARP, 0, &sharp_privs); From f1a15bd2a4a94879a04d7f87ffb89005cdaf2fc2 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 14:53:37 +0100 Subject: [PATCH 3/8] staticd: replace `receive_notify` with request Send `ZEBRA_ROUTE_NOTIFY_REQUEST` rather than relying on the options field in zclient startup. Signed-off-by: David Lamparter --- staticd/static_zebra.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index 1489f53b5b6e..68761c0084bd 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -166,6 +166,7 @@ static int route_notify_owner(ZAPI_CALLBACK_ARGS) 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)); @@ -531,14 +532,12 @@ static zclient_handler *const static_handlers[] = { void static_zebra_init(void) { - struct zclient_options opt = { .receive_notify = true }; - hook_register_prio(if_real, 0, static_ifp_create); hook_register_prio(if_up, 0, static_ifp_up); hook_register_prio(if_down, 0, static_ifp_down); hook_register_prio(if_unreal, 0, static_ifp_destroy); - zclient = zclient_new(master, &opt, static_handlers, + zclient = zclient_new(master, &zclient_options_default, static_handlers, array_size(static_handlers)); zclient_init(zclient, ZEBRA_ROUTE_STATIC, 0, &static_privs); From 7f7564bbb24aa28dd59a3983bb21f730162312cb Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 15:01:54 +0100 Subject: [PATCH 4/8] eigrpd: use default zclient options `.receive_notify = false` is the default. Signed-off-by: David Lamparter --- eigrpd/eigrp_zebra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/eigrpd/eigrp_zebra.c b/eigrpd/eigrp_zebra.c index a5cecb9c1615..a0eff683dbc9 100644 --- a/eigrpd/eigrp_zebra.c +++ b/eigrpd/eigrp_zebra.c @@ -98,9 +98,7 @@ static zclient_handler *const eigrp_handlers[] = { void eigrp_zebra_init(void) { - struct zclient_options opt = {.receive_notify = false}; - - zclient = zclient_new(master, &opt, eigrp_handlers, + zclient = zclient_new(master, &zclient_options_default, eigrp_handlers, array_size(eigrp_handlers)); zclient_init(zclient, ZEBRA_ROUTE_EIGRP, 0, &eigrpd_privs); From 25a1dccc56a7e544a855bfeda5751c91b53a03fb Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 15:03:35 +0100 Subject: [PATCH 5/8] lib: remove `.receive_notify` zclient option This should just be set with `ZEBRA_ROUTE_NOTIFY_REQUEST` instead. Signed-off-by: David Lamparter --- lib/zclient.c | 11 ++++------- lib/zclient.h | 4 ---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/zclient.c b/lib/zclient.c index 9f261e2f2993..47ce111b3338 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -41,8 +41,9 @@ static void zclient_event(enum zclient_event, struct zclient *); static void zebra_interface_if_set_value(struct stream *s, struct interface *ifp); -struct zclient_options zclient_options_default = {.receive_notify = false, - .synchronous = false}; +struct zclient_options zclient_options_default = { + .synchronous = false, +}; struct sockaddr_storage zclient_addr; socklen_t zclient_addr_len; @@ -69,7 +70,6 @@ struct zclient *zclient_new(struct event_loop *master, zclient->handlers = handlers; zclient->n_handlers = n_handlers; - zclient->receive_notify = opt->receive_notify; zclient->synchronous = opt->synchronous; return zclient; @@ -392,10 +392,7 @@ enum zclient_send_status zclient_send_hello(struct zclient *zclient) stream_putc(s, zclient->redist_default); stream_putw(s, zclient->instance); stream_putl(s, zclient->session_id); - if (zclient->receive_notify) - stream_putc(s, 1); - else - stream_putc(s, 0); + stream_putc(s, 0); /* receive_notify - removed */ if (zclient->synchronous) stream_putc(s, 1); else diff --git a/lib/zclient.h b/lib/zclient.h index 9b709ea64b71..8332f2a057de 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -303,9 +303,6 @@ struct zclient { /* Privileges to change socket values */ struct zebra_privs_t *privs; - /* Do we care about failure events for route install? */ - bool receive_notify; - /* Is this a synchronous client? */ bool synchronous; @@ -834,7 +831,6 @@ extern char *zclient_evpn_dump_macip_flags(uint8_t flags, char *buf, enum zebra_neigh_state { ZEBRA_NEIGH_INACTIVE = 0, ZEBRA_NEIGH_ACTIVE = 1 }; struct zclient_options { - bool receive_notify; bool synchronous; }; From a13d2933b755e159f78251d6a790eb153c6500fb Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 15:08:32 +0100 Subject: [PATCH 6/8] zebra, lib: remove notify field from hello message This is no longer used. Signed-off-by: David Lamparter --- lib/zclient.c | 1 - zebra/zapi_msg.c | 4 ---- 2 files changed, 5 deletions(-) diff --git a/lib/zclient.c b/lib/zclient.c index 47ce111b3338..39756933e7a8 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -392,7 +392,6 @@ enum zclient_send_status zclient_send_hello(struct zclient *zclient) stream_putc(s, zclient->redist_default); stream_putw(s, zclient->instance); stream_putl(s, zclient->session_id); - stream_putc(s, 0); /* receive_notify - removed */ if (zclient->synchronous) stream_putc(s, 1); else diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 390ebc8f4c93..679023fd84a3 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -2376,17 +2376,13 @@ static void zread_hello(ZAPI_HANDLER_ARGS) /* type of protocol (lib/zebra.h) */ uint8_t proto; unsigned short instance; - uint8_t notify; uint8_t synchronous; uint32_t session_id; STREAM_GETC(msg, proto); STREAM_GETW(msg, instance); STREAM_GETL(msg, session_id); - STREAM_GETC(msg, notify); STREAM_GETC(msg, synchronous); - if (notify) - client->notify_owner = true; if (synchronous) client->synchronous = true; From cc90c54b36818a13774edfed8d7ac9a6e36c9373 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 15:18:44 +0100 Subject: [PATCH 7/8] *: add `zclient_options_sync` ... and use it instead of fiddling with the `.synchronous` field. (Make it const while at it.) Signed-off-by: David Lamparter --- bgpd/bgp_zebra.c | 5 +---- isisd/isis_zebra.c | 4 +--- ldpd/lde.c | 6 +----- lib/zclient.c | 8 ++++++-- lib/zclient.h | 5 +++-- ospfd/ospf_zebra.c | 4 +--- pathd/path_zebra.c | 5 +---- pimd/pim_zlookup.c | 5 +---- 8 files changed, 15 insertions(+), 27 deletions(-) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 57d419fabef0..3b25348c4e79 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -3430,9 +3430,6 @@ static void bgp_zebra_capabilities(struct zclient_capabilities *cap) void bgp_zebra_init(struct event_loop *master, unsigned short instance) { - struct zclient_options options = zclient_options_default; - - options.synchronous = true; zclient_num_connects = 0; hook_register_prio(if_real, 0, bgp_ifp_create); @@ -3450,7 +3447,7 @@ void bgp_zebra_init(struct event_loop *master, unsigned short instance) zclient->instance = instance; /* Initialize special zclient for synchronous message exchanges. */ - zclient_sync = zclient_new(master, &options, NULL, 0); + zclient_sync = zclient_new(master, &zclient_options_sync, NULL, 0); zclient_sync->sock = -1; zclient_sync->redist_default = ZEBRA_ROUTE_BGP; zclient_sync->instance = instance; diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c index 8252c1ac25a6..18a0c49cebc8 100644 --- a/isisd/isis_zebra.c +++ b/isisd/isis_zebra.c @@ -1395,9 +1395,7 @@ void isis_zebra_init(struct event_loop *master, int instance) zclient->zebra_connected = isis_zebra_connected; /* Initialize special zclient for synchronous message exchanges. */ - struct zclient_options options = zclient_options_default; - options.synchronous = true; - zclient_sync = zclient_new(master, &options, NULL, 0); + zclient_sync = zclient_new(master, &zclient_options_sync, NULL, 0); zclient_sync->sock = -1; zclient_sync->redist_default = ZEBRA_ROUTE_ISIS; zclient_sync->instance = instance; diff --git a/ldpd/lde.c b/ldpd/lde.c index c7e915deb332..ef4aecadadec 100644 --- a/ldpd/lde.c +++ b/ldpd/lde.c @@ -2135,12 +2135,8 @@ static void zclient_sync_retry(struct event *thread) */ static void zclient_sync_init(void) { - struct zclient_options options = zclient_options_default; - - options.synchronous = true; - /* Initialize special zclient for synchronous message exchanges. */ - zclient_sync = zclient_new(master, &options, NULL, 0); + zclient_sync = zclient_new(master, &zclient_options_sync, NULL, 0); zclient_sync->sock = -1; zclient_sync->redist_default = ZEBRA_ROUTE_LDP; zclient_sync->session_id = 1; /* Distinguish from main session */ diff --git a/lib/zclient.c b/lib/zclient.c index 39756933e7a8..61533aecc6b8 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -41,10 +41,14 @@ static void zclient_event(enum zclient_event, struct zclient *); static void zebra_interface_if_set_value(struct stream *s, struct interface *ifp); -struct zclient_options zclient_options_default = { +const struct zclient_options zclient_options_default = { .synchronous = false, }; +const struct zclient_options zclient_options_sync = { + .synchronous = true, +}; + struct sockaddr_storage zclient_addr; socklen_t zclient_addr_len; @@ -53,7 +57,7 @@ static int zclient_debug; /* Allocate zclient structure. */ struct zclient *zclient_new(struct event_loop *master, - struct zclient_options *opt, + const struct zclient_options *opt, zclient_handler *const *handlers, size_t n_handlers) { struct zclient *zclient; diff --git a/lib/zclient.h b/lib/zclient.h index 8332f2a057de..c8dff18bb932 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -834,7 +834,8 @@ struct zclient_options { bool synchronous; }; -extern struct zclient_options zclient_options_default; +extern const struct zclient_options zclient_options_default; +extern const struct zclient_options zclient_options_sync; /* link layer representation for GRE like interfaces * ip_in is the underlay IP, ip_out is the tunnel dest @@ -881,7 +882,7 @@ int zclient_neigh_ip_encode(struct stream *s, uint16_t cmd, union sockunion *in, extern uint32_t zclient_get_nhg_start(uint32_t proto); extern struct zclient *zclient_new(struct event_loop *m, - struct zclient_options *opt, + const struct zclient_options *opt, zclient_handler *const *handlers, size_t n_handlers); diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c index 19d3c9a08306..bb6cc3a89c05 100644 --- a/ospfd/ospf_zebra.c +++ b/ospfd/ospf_zebra.c @@ -2189,9 +2189,7 @@ void ospf_zebra_init(struct event_loop *master, unsigned short instance) zclient->nexthop_update = ospf_zebra_import_check_update; /* Initialize special zclient for synchronous message exchanges. */ - struct zclient_options options = zclient_options_default; - options.synchronous = true; - zclient_sync = zclient_new(master, &options, NULL, 0); + zclient_sync = zclient_new(master, &zclient_options_sync, NULL, 0); zclient_sync->sock = -1; zclient_sync->redist_default = ZEBRA_ROUTE_OSPF; zclient_sync->instance = instance; diff --git a/pathd/path_zebra.c b/pathd/path_zebra.c index 826443f97906..645fa50856cd 100644 --- a/pathd/path_zebra.c +++ b/pathd/path_zebra.c @@ -320,9 +320,6 @@ static zclient_handler *const path_handlers[] = { */ void path_zebra_init(struct event_loop *master) { - struct zclient_options options = zclient_options_default; - options.synchronous = true; - /* Initialize asynchronous zclient. */ zclient = zclient_new(master, &zclient_options_default, path_handlers, array_size(path_handlers)); @@ -330,7 +327,7 @@ void path_zebra_init(struct event_loop *master) zclient->zebra_connected = path_zebra_connected; /* Initialize special zclient for synchronous message exchanges. */ - zclient_sync = zclient_new(master, &options, NULL, 0); + zclient_sync = zclient_new(master, &zclient_options_sync, NULL, 0); zclient_sync->sock = -1; zclient_sync->redist_default = ZEBRA_ROUTE_SRTE; zclient_sync->instance = 1; diff --git a/pimd/pim_zlookup.c b/pimd/pim_zlookup.c index 6a026f994767..c19119fa4788 100644 --- a/pimd/pim_zlookup.c +++ b/pimd/pim_zlookup.c @@ -122,10 +122,7 @@ void zclient_lookup_free(void) void zclient_lookup_new(void) { - struct zclient_options options = zclient_options_default; - options.synchronous = true; - - zlookup = zclient_new(router->master, &options, NULL, 0); + zlookup = zclient_new(router->master, &zclient_options_sync, NULL, 0); if (!zlookup) { flog_err(EC_LIB_ZAPI_SOCKET, "%s: zclient_new() failure", __func__); From 5a40f2b0e75bbb43c0d6e2f9cfe190dfaa61f386 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 Nov 2023 15:40:38 +0100 Subject: [PATCH 8/8] lib, bgp/vnc: add `.auxiliary` zclient option Avoids calling VRF/interface/... handlers in library code more than once. It's kinda surprising that this hasn't been blowing up already for the VNC code, luckily these handlers are (mostly?) idempotent. Signed-off-by: David Lamparter --- bgpd/rfapi/vnc_zebra.c | 2 +- lib/zclient.c | 11 ++++++++++- lib/zclient.h | 12 ++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/bgpd/rfapi/vnc_zebra.c b/bgpd/rfapi/vnc_zebra.c index c886beea9197..82c08cabde6e 100644 --- a/bgpd/rfapi/vnc_zebra.c +++ b/bgpd/rfapi/vnc_zebra.c @@ -872,7 +872,7 @@ static zclient_handler *const vnc_handlers[] = { void vnc_zebra_init(struct event_loop *master) { /* Set default values. */ - zclient_vnc = zclient_new(master, &zclient_options_default, + zclient_vnc = zclient_new(master, &zclient_options_auxiliary, vnc_handlers, array_size(vnc_handlers)); zclient_init(zclient_vnc, ZEBRA_ROUTE_VNC, 0, &bgpd_privs); } diff --git a/lib/zclient.c b/lib/zclient.c index 61533aecc6b8..1b2ff02f6137 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -43,10 +43,17 @@ static void zebra_interface_if_set_value(struct stream *s, const struct zclient_options zclient_options_default = { .synchronous = false, + .auxiliary = false, }; const struct zclient_options zclient_options_sync = { .synchronous = true, + .auxiliary = true, +}; + +const struct zclient_options zclient_options_auxiliary = { + .synchronous = false, + .auxiliary = true, }; struct sockaddr_storage zclient_addr; @@ -75,6 +82,7 @@ struct zclient *zclient_new(struct event_loop *master, zclient->n_handlers = n_handlers; zclient->synchronous = opt->synchronous; + zclient->auxiliary = opt->auxiliary; return zclient; } @@ -4444,7 +4452,8 @@ static void zclient_read(struct event *thread) zlog_debug("zclient %p command %s VRF %u", zclient, zserv_command_string(command), vrf_id); - if (command < array_size(lib_handlers) && lib_handlers[command]) + if (!zclient->auxiliary && command < array_size(lib_handlers) && + lib_handlers[command]) lib_handlers[command](command, zclient, length, vrf_id); if (command < zclient->n_handlers && zclient->handlers[command]) zclient->handlers[command](command, zclient, length, vrf_id); diff --git a/lib/zclient.h b/lib/zclient.h index c8dff18bb932..05eb6b20ee84 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -306,6 +306,11 @@ struct zclient { /* Is this a synchronous client? */ bool synchronous; + /* Auxiliary clients don't execute standard library handlers + * (which otherwise would duplicate VRF/interface add/delete/etc. + */ + bool auxiliary; + /* BFD enabled with bfd_protocol_integration_init() */ bool bfd_integration; @@ -832,10 +837,17 @@ enum zebra_neigh_state { ZEBRA_NEIGH_INACTIVE = 0, ZEBRA_NEIGH_ACTIVE = 1 }; struct zclient_options { bool synchronous; + + /* auxiliary = don't call common lib/ handlers that manage bits. + * Those should only run once, on the "main" zclient, which this is + * not. (This is also set for synchronous clients.) + */ + bool auxiliary; }; extern const struct zclient_options zclient_options_default; extern const struct zclient_options zclient_options_sync; +extern const struct zclient_options zclient_options_auxiliary; /* link layer representation for GRE like interfaces * ip_in is the underlay IP, ip_out is the tunnel dest