proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-08-15 11:24:24 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

lib: fix if_set_value

Browse Source 
Something in there is wrong and causing test failures. Moving it back to
how it was; we'll stil assert if the message was wrong, just in a
different place now.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This commit is contained in:
Quentin Young 2020-04-08 16:29:23 -04:00
parent f7d4592509
commit 41ce53847b
1 changed files with 29 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

145
lib/zclient.c
View File

@ -49,7 +49,7 @@ enum event { ZCLIENT_SCHEDULE, ZCLIENT_READ, ZCLIENT_CONNECT };
/* Prototype for event manager. */ /* Prototype for event manager. */
static void zclient_event(enum event, struct zclient *); static void zclient_event(enum event, struct zclient *);
static int zebra_interface_if_set_value(struct stream *s, static void zebra_interface_if_set_value(struct stream *s,
struct interface *ifp); struct interface *ifp);
struct zclient_options zclient_options_default = {.receive_notify = false, struct zclient_options zclient_options_default = {.receive_notify = false,
@ -1942,137 +1942,48 @@ stream_failure:
return NULL; return NULL;
} }
static int zebra_interface_if_set_value(struct stream *s, struct interface *ifp) static void zebra_interface_if_set_value(struct stream *s,
struct interface *ifp)
{ {
int rc;
uint8_t link_params_status = 0; uint8_t link_params_status = 0;
ifindex_t old_ifindex; ifindex_t old_ifindex, new_ifindex;
/*
* XXX:
* Copy interface structure to temporary one. This way if stream
* processing fails partway through, we can safely destroy the
* temporary copy without ever affecting the actual interface, which
* would leave us in a "torn write" situation. If everything succeeds,
* we copy the temporary structure back to the real one and destroy the
* temporary structure.
*
* Obviously all fields of the ifp that could be modified need to be
* copied, so any heap blocks need to be deep copied. In both failure
* and success cases, we need to properly free any allocations that
* result from this deep copy. When modifying this code, take care.
*/
struct interface ifp_tmp = {};
memcpy(&ifp_tmp, ifp, sizeof(struct interface));
/* Not safe to use ifp_tmp in the context of an RB tree */
memset(&ifp_tmp.name_entry, 0x00, sizeof(ifp_tmp.name_entry));
memset(&ifp_tmp.index_entry, 0x00, sizeof(ifp_tmp.index_entry));
bool ifp_has_link_params = (bool)!!ifp->link_params;
if (ifp_has_link_params) {
if_link_params_get(&ifp_tmp);
memcpy(ifp_tmp.link_params, ifp->link_params,
sizeof(struct if_link_params));
}
/* Read params message */
old_ifindex = ifp_tmp.ifindex;
old_ifindex = ifp->ifindex;
/* Read interface's index. */ /* Read interface's index. */
STREAM_GETL(s, ifp_tmp.ifindex); STREAM_GETL(s, new_ifindex);
if_set_index(ifp, new_ifindex);
if (ifp_tmp.ifindex < 0) STREAM_GETC(s, ifp->status);
goto stream_failure;
STREAM_GETC(s, ifp_tmp.status);
/* Read interface's value. */ /* Read interface's value. */
STREAM_GETQ(s, ifp_tmp.flags); STREAM_GETQ(s, ifp->flags);
STREAM_GETC(s, ifp_tmp.ptm_enable); STREAM_GETC(s, ifp->ptm_enable);
STREAM_GETC(s, ifp_tmp.ptm_status); STREAM_GETC(s, ifp->ptm_status);
STREAM_GETL(s, ifp_tmp.metric); STREAM_GETL(s, ifp->metric);
STREAM_GETL(s, ifp_tmp.speed); STREAM_GETL(s, ifp->speed);
STREAM_GETL(s, ifp_tmp.mtu); STREAM_GETL(s, ifp->mtu);
STREAM_GETL(s, ifp_tmp.mtu6); STREAM_GETL(s, ifp->mtu6);
STREAM_GETL(s, ifp_tmp.bandwidth); STREAM_GETL(s, ifp->bandwidth);
STREAM_GETL(s, ifp_tmp.link_ifindex); STREAM_GETL(s, ifp->link_ifindex);
STREAM_GETL(s, ifp_tmp.ll_type); STREAM_GETL(s, ifp->ll_type);
STREAM_GETL(s, ifp_tmp.hw_addr_len); STREAM_GETL(s, ifp->hw_addr_len);
if (ifp_tmp.hw_addr_len) if (ifp->hw_addr_len)
STREAM_GET(ifp_tmp.hw_addr, s, STREAM_GET(ifp->hw_addr, s,
MIN(ifp_tmp.hw_addr_len, INTERFACE_HWADDR_MAX)); MIN(ifp->hw_addr_len, INTERFACE_HWADDR_MAX));
/* Read Traffic Engineering status */ /* Read Traffic Engineering status */
STREAM_GETC(s, link_params_status); link_params_status = stream_getc(s);
/* Then, Traffic Engineering parameters if any */ /* Then, Traffic Engineering parameters if any */
if (link_params_status) { if (link_params_status) {
/* if_link_params_get can XCALLOC if they don't exist, beware */ struct if_link_params *iflp = if_link_params_get(ifp);
struct if_link_params *iflp = if_link_params_get(&ifp_tmp); link_params_set_value(s, iflp);
if (link_params_set_value(s, iflp) != 0)
goto stream_failure;
}
/* Success - apply changes to ifp_tmp to ifp */
/*
* Note: we cannot directly memcpy() because this will overwrite the
* rb_entry, causing many hours of pain
*/
if (if_set_index(ifp, ifp_tmp.ifindex) < 0)
goto stream_failure;
ifp->status = ifp_tmp.status;
ifp->flags = ifp_tmp.flags;
ifp->ptm_enable = ifp_tmp.ptm_enable;
ifp->ptm_status = ifp_tmp.ptm_status;
ifp->metric = ifp_tmp.metric;
ifp->speed = ifp_tmp.speed;
ifp->mtu = ifp_tmp.mtu;
ifp->mtu6 = ifp_tmp.mtu6;
ifp->bandwidth = ifp_tmp.bandwidth;
ifp->link_ifindex = ifp_tmp.link_ifindex;
ifp->ll_type = ifp_tmp.ll_type;
ifp->hw_addr_len = ifp_tmp.hw_addr_len;
ifp->link_params = ifp_tmp.link_params;
/*
* If ifp had no link params, and link_params_status was nonzero, then
* if_link_params_get created them. We just copied the pointer to ifp,
* so in this case, we use move semantics, and NULL ifp_tmp.link_params
* after copying. This way we do not free ifp->link_params in our later
* free call by virtue of freeing ifp_tmp.link_params, which would make
* ifp->link_params a dangling pointer.
*
* If ifp had no link params, and link_params_status was zero, then the
* pointer is still NULL, and we MUST NOT copy; memcpy(0, _, x != 0) is
* undefined behavior. Nothing was created, so we do not have to free
* either.
*
* If it did have link params, we duplicated them for modification
* purposes, and need to copy if something was changed. This handles
* all four cases.
*/
if (!ifp_has_link_params && ifp_tmp.link_params != NULL)
ifp_tmp.link_params = NULL;
if (ifp_has_link_params && link_params_status != 0) {
assert(ifp->link_params != NULL && ifp_tmp.link_params != NULL);
memcpy(ifp->link_params, ifp_tmp.link_params,
sizeof(struct if_link_params));
} }
nexthop_group_interface_state_change(ifp, old_ifindex); nexthop_group_interface_state_change(ifp, old_ifindex);
rc = 0; return;
goto cleanup;
stream_failure: stream_failure:
rc = -1; zlog_err("Could not parse interface values; aborting");
cleanup: assert(!"Failed to parse interface values");
/* Free copied resources */
if_link_params_free(&ifp_tmp);
return rc;
} }
size_t zebra_interface_link_params_write(struct stream *s, size_t zebra_interface_link_params_write(struct stream *s,