Skip to content

Commit

Permalink
bgpd: Used %pBD instead of %pRN
Browse files Browse the repository at this point in the history 
Let's use the natural data structure in bgp for the prefix display
instead of a bunch of places where we call a translator function.
The %pBD does this and actually ensures data is correct.
Also fix a few spots in bgp_zebra.c where the cast to a NULL
pointer causes the catcher functionality to not work and fix
the resulting crash that resulted.

Signed-off-by: Donald Sharp <[email protected]>
  • Loading branch information
@donaldsharp
donaldsharp committed Nov 13, 2023
1 parent c62c018 commit 12deca0
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 53 deletions.
9 changes: 4 additions & 5 deletions bgpd/bgp_label.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,8 @@ static void bgp_send_fec_register_label_msg(struct bgp_dest *dest, bool reg,
return;

if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug("%s: FEC %sregister %pRN label_index=%u label=%u",
__func__, reg ? "" : "un", bgp_dest_to_rnode(dest),
label_index, label);
zlog_debug("%s: FEC %sregister %pBD label_index=%u label=%u",
__func__, reg ? "" : "un", dest, label_index, label);
/* If the route node has a local_label assigned or the
* path node has an MPLS SR label index allowing zebra to
* derive the label, proceed with registration. */
Expand Down Expand Up @@ -199,8 +198,8 @@ int bgp_reg_for_label_callback(mpls_label_t new_label, void *labelid,
assert(dest);

if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug("%s: FEC %pRN label=%u, allocated=%d", __func__,
bgp_dest_to_rnode(dest), new_label, allocated);
zlog_debug("%s: FEC %pBD label=%u, allocated=%d", __func__,
dest, new_label, allocated);

if (!allocated) {
/*
Expand Down
63 changes: 27 additions & 36 deletions bgpd/bgp_mpath.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,12 +554,11 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
}

if (debug)
zlog_debug(
"%pRN(%s): starting mpath update, newbest %s num candidates %d old-mpath-count %d old-cum-bw %" PRIu64,
bgp_dest_to_rnode(dest), bgp->name_pretty,
new_best ? new_best->peer->host : "NONE",
mp_list ? listcount(mp_list) : 0, old_mpath_count,
old_cum_bw);
zlog_debug("%pBD(%s): starting mpath update, newbest %s num candidates %d old-mpath-count %d old-cum-bw %" PRIu64,
dest, bgp->name_pretty,
new_best ? new_best->peer->host : "NONE",
mp_list ? listcount(mp_list) : 0, old_mpath_count,
old_cum_bw);

/*
* We perform an ordered walk through both lists in parallel.
Expand Down Expand Up @@ -590,11 +589,10 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
tmp_info = mp_node ? listgetdata(mp_node) : NULL;

if (debug)
zlog_debug(
"%pRN(%s): comparing candidate %s with existing mpath %s",
bgp_dest_to_rnode(dest), bgp->name_pretty,
tmp_info ? tmp_info->peer->host : "NONE",
cur_mpath ? cur_mpath->peer->host : "NONE");
zlog_debug("%pBD(%s): comparing candidate %s with existing mpath %s",
dest, bgp->name_pretty,
tmp_info ? tmp_info->peer->host : "NONE",
cur_mpath ? cur_mpath->peer->host : "NONE");

/*
* If equal, the path was a multipath and is still a multipath.
Expand Down Expand Up @@ -622,23 +620,19 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
bgp_path_info_path_with_addpath_rx_str(
cur_mpath, path_buf,
sizeof(path_buf));
zlog_debug(
"%pRN: %s is still multipath, cur count %d",
bgp_dest_to_rnode(dest),
path_buf, mpath_count);
zlog_debug("%pBD: %s is still multipath, cur count %d",
dest, path_buf, mpath_count);
}
} else {
mpath_changed = 1;
if (debug) {
bgp_path_info_path_with_addpath_rx_str(
cur_mpath, path_buf,
sizeof(path_buf));
zlog_debug(
"%pRN: remove mpath %s nexthop %pI4, cur count %d",
bgp_dest_to_rnode(dest),
path_buf,
&cur_mpath->attr->nexthop,
mpath_count);
zlog_debug("%pBD: remove mpath %s nexthop %pI4, cur count %d",
dest, path_buf,
&cur_mpath->attr->nexthop,
mpath_count);
}
}
mp_node = mp_next_node;
Expand All @@ -663,10 +657,10 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
if (debug) {
bgp_path_info_path_with_addpath_rx_str(
cur_mpath, path_buf, sizeof(path_buf));
zlog_debug(
"%pRN: remove mpath %s nexthop %pI4, cur count %d",
bgp_dest_to_rnode(dest), path_buf,
&cur_mpath->attr->nexthop, mpath_count);
zlog_debug("%pBD: remove mpath %s nexthop %pI4, cur count %d",
dest, path_buf,
&cur_mpath->attr->nexthop,
mpath_count);
}
cur_mpath = next_mpath;
} else {
Expand Down Expand Up @@ -713,12 +707,10 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
bgp_path_info_path_with_addpath_rx_str(
new_mpath, path_buf,
sizeof(path_buf));
zlog_debug(
"%pRN: add mpath %s nexthop %pI4, cur count %d",
bgp_dest_to_rnode(dest),
path_buf,
&new_mpath->attr->nexthop,
mpath_count);
zlog_debug("%pBD: add mpath %s nexthop %pI4, cur count %d",
dest, path_buf,
&new_mpath->attr->nexthop,
mpath_count);
}
}
mp_node = mp_next_node;
Expand All @@ -737,11 +729,10 @@ void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
all_paths_lb, cum_bw);

if (debug)
zlog_debug(
"%pRN(%s): New mpath count (incl newbest) %d mpath-change %s all_paths_lb %d cum_bw %" PRIu64,
bgp_dest_to_rnode(dest), bgp->name_pretty,
mpath_count, mpath_changed ? "YES" : "NO",
all_paths_lb, cum_bw);
zlog_debug("%pBD(%s): New mpath count (incl newbest) %d mpath-change %s all_paths_lb %d cum_bw %" PRIu64,
dest, bgp->name_pretty, mpath_count,
mpath_changed ? "YES" : "NO", all_paths_lb,
cum_bw);

if (mpath_changed
|| (bgp_path_info_mpath_count(new_best) != old_mpath_count))
Expand Down
5 changes: 2 additions & 3 deletions bgpd/bgp_updgrp_adv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,8 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg)
addpath_capable = bgp_addpath_encode_tx(peer, afi, safi);

if (BGP_DEBUG(update, UPDATE_OUT))
zlog_debug("%s: afi=%s, safi=%s, p=%pRN", __func__,
afi2str(afi), safi2str(safi),
bgp_dest_to_rnode(ctx->dest));
zlog_debug("%s: afi=%s, safi=%s, p=%pBD", __func__,
afi2str(afi), safi2str(safi), ctx->dest);

UPDGRP_FOREACH_SUBGRP (updgrp, subgrp) {
/*
Expand Down
17 changes: 8 additions & 9 deletions bgpd/bgp_zebra.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2609,7 +2609,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("route %pRN : INSTALLED", (void *)dest);
zlog_debug("route %pBD : INSTALLED", dest);
/* Find the best route */
for (pi = dest->info; pi; pi = pi->next) {
/* Process aggregate route */
Expand All @@ -2622,7 +2622,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
group_announce_route(bgp, afi, safi, dest, new_select);
else {
flog_err(EC_BGP_INVALID_ROUTE,
"selected route %pRN not found", (void *)dest);
"selected route %pBD not found", dest);

bgp_dest_unlock_node(dest);
return -1;
Expand All @@ -2635,13 +2635,13 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
*/
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("route %pRN: Removed from Fib", (void *)dest);
zlog_debug("route %pBD: Removed from Fib", dest);
break;
case ZAPI_ROUTE_FAIL_INSTALL:
new_select = NULL;
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("route: %pRN Failed to Install into Fib",
(void *)dest);
zlog_debug("route: %pBD Failed to Install into Fib",
dest);
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
Expand All @@ -2654,8 +2654,8 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
break;
case ZAPI_ROUTE_BETTER_ADMIN_WON:
if (BGP_DEBUG(zebra, ZEBRA))
zlog_debug("route: %pRN removed due to better admin won",
(void *)dest);
zlog_debug("route: %pBD removed due to better admin won",
dest);
new_select = NULL;
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
Expand All @@ -2669,8 +2669,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
/* No action required */
break;
case ZAPI_ROUTE_REMOVE_FAIL:
zlog_warn("%s: Route %pRN failure to remove",
__func__, (void *)dest);
zlog_warn("%s: Route %pBD failure to remove", __func__, dest);
break;
}

Expand Down

0 comments on commit 12deca0

Please sign in to comment.