Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: clean up zclient options #14867

Merged
merged 8 commits into from
Nov 25, 2023
Merged

*: clean up zclient options #14867

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
500a09d
pbrd: replace `receive_notify` with request
eqvinox Nov 23, 2023
adf7b9e
sharpd: replace `receive_notify` with request
eqvinox Nov 23, 2023
f1a15bd
staticd: replace `receive_notify` with request
eqvinox Nov 23, 2023
7f7564b
eigrpd: use default zclient options
eqvinox Nov 23, 2023
25a1dcc
lib: remove `.receive_notify` zclient option
eqvinox Nov 23, 2023
a13d293
zebra, lib: remove notify field from hello message
eqvinox Nov 23, 2023
cc90c54
*: add `zclient_options_sync`
eqvinox Nov 23, 2023
5a40f2b
lib, bgp/vnc: add `.auxiliary` zclient option
eqvinox Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions eigrpd/eigrp_zebra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
eqvinox marked this conversation as resolved.
Show resolved Hide resolved
array_size(eigrp_handlers));

zclient_init(zclient, ZEBRA_ROUTE_EIGRP, 0, &eigrpd_privs);
Expand Down
11 changes: 4 additions & 7 deletions lib/zclient.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am testing via bisect, won't this change cause and assert and crash in zebra every time? This makes me unhappy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Why would this crash anything?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this commit the byte of data sent in the send_hello function is removed. The equivalent code in zebra to read this will now crash as that it will try to read 1 byte beyond the end of the stream, as that we have code that intentionally crashes when this happens. As I stated if this commit is choosen as the bisect I will be stopped in my tracks if I attempt to run zebra with any other daemon at all to find a different problem. This is especially egregious from my perspective because I cannot work around this issue at all as that the initial connection from any daemon will cause zebra to crash

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The byte is not removed in this commit. Please flush your brain cache and look at the commit again 😆
25a1dcc#diff-f949c6f0ae3101476f0699de68a4a73935fdd945294d5f235e97fb5e8bdbc782L395-R395

.synchronous = false,
};

struct sockaddr_storage zclient_addr;
socklen_t zclient_addr_len;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions lib/zclient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
};

Expand Down
6 changes: 3 additions & 3 deletions pbrd/pbr_zebra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions sharpd/sharp_zebra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/*
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions staticd/static_zebra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down