Skip to content

Commit

Permalink
bgpd : backpressure - Handle BGP-Zebra Install evt Creation
Browse files Browse the repository at this point in the history 
BGP is now keeping a list of dests with the dest having a pointer
to the bgp_path_info that it will be working on.

1) When bgp receives a prefix, process it, add the bgp_dest of the
prefix into the new Fifo list if not present, update the flags (Ex:
earlier if the prefix was advertised and now it is a withdrawn),
increment the ref_count and DO NOT advertise the install/withdraw
to zebra yet.

2) Schedule an event to wake up to invoke the new function which will
walk the list one by one and installs/withdraws the routes into zebra.
  a) if BUFFER_EMPTY, process the next item on the list
  b) if BUFFER_PENDING, bail out and the callback in
  zclient_flush_data() will invoke the same function when BUFFER_EMPTY

Changes
 - rename old bgp_zebra_announce to bgp_zebra_announce_actual
 - rename old bgp_zebra_withdrw to bgp_zebra_withdraw_actual
 - Handle new fifo list cleanup in bgp_exit()
 - New funcs: bgp_handle_route_announcements_to_zebra() and
   bgp_zebra_route_install()
 - Define a callback function to invoke
   bgp_handle_route_announcements_to_zebra() when BUFFER_EMPTY in
   zclient_flush_data()

The current change deals with bgp installing routes via
bgp_process_main_one()

Ticket: #3390099

Signed-off-by: Donald Sharp <[email protected]>
Signed-off-by: Rajasekar Raja <[email protected]>
  • Loading branch information
@donaldsharp @raja-rajasekar
donaldsharp authored and raja-rajasekar committed Mar 15, 2024
1 parent 917a5e0 commit 4cc141b
Show file tree
Hide file tree
Showing 8 changed files with 204 additions and 52 deletions.
9 changes: 9 additions & 0 deletions bgpd/bgp_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ static __attribute__((__noreturn__)) void bgp_exit(int status)
{
struct bgp *bgp, *bgp_default, *bgp_evpn;
struct listnode *node, *nnode;
struct bgp_dest *dest;

/* it only makes sense for this to be called on a clean exit */
assert(status == 0);
Expand All @@ -193,6 +194,14 @@ static __attribute__((__noreturn__)) void bgp_exit(int status)
bgp_default = bgp_get_default();
bgp_evpn = bgp_get_evpn();

/* Cleanup the list before table gets cleaned up in bgp_delete */
while (zebra_announce_count(&bm->zebra_announce_head)) {
dest = zebra_announce_pop(&bm->zebra_announce_head);
bgp_path_info_unlock(dest->za_bgp_pi);
bgp_dest_unlock_node(dest);
}
zebra_announce_fini(&bm->zebra_announce_head);

/* reverse bgp_master_init */
for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
if (bgp_default == bgp || bgp_evpn == bgp)
Expand Down
16 changes: 9 additions & 7 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3399,8 +3399,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
|| new_select->sub_type
== BGP_ROUTE_IMPORTED))

bgp_zebra_announce(dest, old_select,
bgp);
bgp_zebra_route_install(dest, old_select,
bgp, true);
}
}

Expand Down Expand Up @@ -3517,17 +3517,19 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
*/
if (old_select &&
is_route_parent_evpn(old_select))
bgp_zebra_withdraw(dest, old_select, bgp);
bgp_zebra_route_install(dest, old_select, bgp,
false);

bgp_zebra_announce(dest, new_select, bgp);
bgp_zebra_route_install(dest, new_select, bgp, true);
} else {
/* Withdraw the route from the kernel. */
if (old_select && old_select->type == ZEBRA_ROUTE_BGP
&& (old_select->sub_type == BGP_ROUTE_NORMAL
|| old_select->sub_type == BGP_ROUTE_AGGREGATE
|| old_select->sub_type == BGP_ROUTE_IMPORTED))

bgp_zebra_withdraw(dest, old_select, bgp);
bgp_zebra_route_install(dest, old_select, bgp,
false);
}
}

Expand Down Expand Up @@ -4424,7 +4426,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
if (pi && pi->attr->rmap_table_id != new_attr.rmap_table_id) {
if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
/* remove from RIB previous entry */
bgp_zebra_withdraw(dest, pi, bgp);
bgp_zebra_route_install(dest, pi, bgp, false);
}

if (peer->sort == BGP_PEER_EBGP) {
Expand Down Expand Up @@ -6050,7 +6052,7 @@ static void bgp_cleanup_table(struct bgp *bgp, struct bgp_table *table,
|| pi->sub_type == BGP_ROUTE_IMPORTED)) {

if (bgp_fibupd_safi(safi))
bgp_zebra_withdraw(dest, pi, bgp);
bgp_zebra_withdraw_actual(dest, pi, bgp);
}

dest = bgp_path_info_reap(dest, pi);
Expand Down
3 changes: 3 additions & 0 deletions bgpd/bgp_table.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ struct bgp_dest {
STAILQ_ENTRY(bgp_dest) pq;

struct zebra_announce_item zai;
struct bgp_path_info *za_bgp_pi;

uint64_t version;

Expand All @@ -93,6 +94,8 @@ struct bgp_dest {
#define BGP_NODE_LABEL_REQUESTED (1 << 7)
#define BGP_NODE_SOFT_RECONFIG (1 << 8)
#define BGP_NODE_PROCESS_CLEAR (1 << 9)
#define BGP_NODE_SCHEDULE_FOR_INSTALL (1 << 10)
#define BGP_NODE_SCHEDULE_FOR_DELETE (1 << 11)

struct bgp_addpath_node_data tx_addpath;

Expand Down
213 changes: 172 additions & 41 deletions bgpd/bgp_zebra.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1524,8 +1524,9 @@ static void bgp_debug_zebra_nh(struct zapi_route *api)
}
}

void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info,
struct bgp *bgp)
static enum zclient_send_status
bgp_zebra_announce_actual(struct bgp_dest *dest, struct bgp_path_info *info,
struct bgp *bgp)
{
struct bgp_path_info *bpi_ultimate;
struct zapi_route api = { 0 };
Expand All @@ -1541,27 +1542,10 @@ void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info,
struct bgp_table *bgp_tbl = bgp_dest_table(dest);
const struct prefix *p = bgp_dest_get_prefix(dest);

/*
* BGP is installing this route and bgp has been configured
* to suppress announcements until the route has been installed
* let's set the fact that we expect this route to be installed
*/
if (BGP_SUPPRESS_FIB_ENABLED(bgp))
SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);

/* Don't try to install if we're not connected to Zebra or Zebra doesn't
* know of this instance.
*/
if (!bgp_install_info_to_zebra(bgp))
return;

if (bgp->main_zebra_update_hold)
return;

if (bgp_tbl->safi == SAFI_FLOWSPEC) {
bgp_pbr_update_entry(bgp, p, info, bgp_tbl->afi, bgp_tbl->safi,
true);
return;
return ZCLIENT_SEND_SUCCESS;
}

/* Make Zebra API structure. */
Expand Down Expand Up @@ -1678,10 +1662,11 @@ void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info,
zlog_debug("%s: %pFX: announcing to zebra (recursion %sset)",
__func__, p, (recursion_flag ? "" : "NOT "));
}
zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE,
zclient, &api);
return zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE,
zclient, &api);
}


/* Announce all routes of a table to zebra */
void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi)
{
Expand All @@ -1707,7 +1692,7 @@ void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi)
&& (pi->sub_type == BGP_ROUTE_NORMAL
|| pi->sub_type == BGP_ROUTE_IMPORTED)))

bgp_zebra_announce(dest, pi, bgp);
bgp_zebra_route_install(dest, pi, bgp, true);
}

/* Announce routes of any bgp subtype of a table to zebra */
Expand All @@ -1729,34 +1714,23 @@ void bgp_zebra_announce_table_all_subtypes(struct bgp *bgp, afi_t afi,
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next)
if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED) &&
pi->type == ZEBRA_ROUTE_BGP)
bgp_zebra_announce(dest, pi, bgp);
bgp_zebra_route_install(dest, pi, bgp, true);
}

void bgp_zebra_withdraw(struct bgp_dest *dest, struct bgp_path_info *info,
struct bgp *bgp)
enum zclient_send_status bgp_zebra_withdraw_actual(struct bgp_dest *dest,
struct bgp_path_info *info,
struct bgp *bgp)
{
struct zapi_route api;
struct peer *peer;
struct bgp_table *bgp_tbl = bgp_dest_table(dest);
const struct prefix *p = bgp_dest_get_prefix(dest);

/*
* If we are withdrawing the route, we don't need to have this
* flag set. So unset it.
*/
UNSET_FLAG(info->net->flags, BGP_NODE_FIB_INSTALL_PENDING);

/* Don't try to install if we're not connected to Zebra or Zebra doesn't
* know of this instance.
*/
if (!bgp_install_info_to_zebra(bgp))
return;

if (bgp_tbl->safi == SAFI_FLOWSPEC) {
peer = info->peer;
bgp_pbr_update_entry(peer->bgp, p, info, bgp_tbl->afi,
bgp_tbl->safi, false);
return;
return ZCLIENT_SEND_SUCCESS;
}

memset(&api, 0, sizeof(api));
Expand All @@ -1774,7 +1748,163 @@ void bgp_zebra_withdraw(struct bgp_dest *dest, struct bgp_path_info *info,
zlog_debug("Tx route delete VRF %u %pFX", bgp->vrf_id,
&api.prefix);

zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api);
return zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api);
}

/*
* Walk the new Fifo list one by one and invoke bgp_zebra_announce/withdraw
* to install/withdraw the routes to zebra.
*
* If status = ZCLIENT_SEND_SUCCESS (Buffer empt)y i.e. Zebra is free to
* receive more incoming data, then pick the next item on the list and
* continue processing.
*
* If status = ZCLIENT_SEND_BUFFERED (Buffer pending) i.e. Zebra is busy,
* break and bail out of the function because once at some point when zebra
* is free, a callback is triggered which inturn call this same function and
* continue processing items on list.
*/
#define ZEBRA_LOOP 1000
static void bgp_handle_route_announcements_to_zebra(struct event *e)
{
uint32_t count = 0;
struct bgp_dest *dest = NULL;
struct bgp_table *bgp_tbl = NULL;
enum zclient_send_status status = ZCLIENT_SEND_SUCCESS;

while (count < ZEBRA_LOOP) {
dest = zebra_announce_pop(&bm->zebra_announce_head);

if (!dest)
break;

bgp_tbl = bgp_dest_table(dest);

if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL)) {
status = bgp_zebra_announce_actual(dest, dest->za_bgp_pi,
bgp_tbl->bgp);
UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
} else if (CHECK_FLAG(dest->flags,
BGP_NODE_SCHEDULE_FOR_DELETE)) {
status = bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi,
bgp_tbl->bgp);
UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
}

bgp_path_info_unlock(dest->za_bgp_pi);
dest->za_bgp_pi = NULL;
bgp_dest_unlock_node(dest);

if (status == ZCLIENT_SEND_BUFFERED)
break;

count++;
}

if (status != ZCLIENT_SEND_BUFFERED &&
zebra_announce_count(&bm->zebra_announce_head))
event_add_event(bm->master,
bgp_handle_route_announcements_to_zebra, NULL,
0, &bm->t_bgp_zebra_route);
}

/*
* Callback function invoked when zclient_flush_data() receives a BUFFER_EMPTY
* i.e. zebra is free to receive more incoming data.
*/
static void bgp_zebra_buffer_write_ready(void)
{
bgp_handle_route_announcements_to_zebra(NULL);
}

/*
* BGP is now keeping a list of dests with the dest having a pointer
* to the bgp_path_info that it will be working on.
* Here is the sequence of events that should happen:
*
* Current State New State Action
* ------------- --------- ------
* ---- Install Place dest on list, save pi, mark
* as going to be installed
* ---- Withdrawal Place dest on list, save pi, mark
* as going to be deleted
*
* Install Install Leave dest on list, release old pi,
* save new pi, mark as going to be
* Installed
* Install Withdrawal Leave dest on list, release old pi,
* save new pi, mark as going to be
* withdrawan, remove install flag
*
* Withdrawal Install Special case, send withdrawal immediately
* Leave dest on list, release old pi,
* save new pi, mark as going to be
* installed. <see note about evpn
* in bgp_route.c in bgp_process_main_one>
* Withdrawal Withdrawal Leave dest on list, release old pi,
* save new pi, mark as going to be
* withdrawn.
*/
void bgp_zebra_route_install(struct bgp_dest *dest, struct bgp_path_info *info,
struct bgp *bgp, bool install)
{
/*
* BGP is installing this route and bgp has been configured
* to suppress announcements until the route has been installed
* let's set the fact that we expect this route to be installed
*/
if (install) {
if (BGP_SUPPRESS_FIB_ENABLED(bgp))
SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);

if (bgp->main_zebra_update_hold)
return;
} else {
UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
}

/*
* Don't try to install if we're not connected to Zebra or Zebra doesn't
* know of this instance.
*/
if (!bgp_install_info_to_zebra(bgp))
return;

if (!CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL) &&
!CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) {
zebra_announce_add_tail(&bm->zebra_announce_head, dest);
/*
* If neither flag is set and za_bgp_pi is not set then it is a bug
*/
assert(!dest->za_bgp_pi);
dest->za_bgp_pi = info;
bgp_path_info_lock(info);
bgp_dest_lock_node(dest);
} else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL)) {
assert(dest->za_bgp_pi);
bgp_path_info_unlock(dest->za_bgp_pi);
bgp_path_info_lock(info);
dest->za_bgp_pi = info;
} else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) {
assert(dest->za_bgp_pi);
if (install)
bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi, bgp);

bgp_path_info_unlock(dest->za_bgp_pi);
bgp_path_info_lock(info);
dest->za_bgp_pi = info;
}

if (install) {
UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
} else {
UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
}

event_add_event(bm->master, bgp_handle_route_announcements_to_zebra,
NULL, 0, &bm->t_bgp_zebra_route);
}

/* Withdraw all entries in a BGP instances RIB table from Zebra */
Expand All @@ -1795,7 +1925,7 @@ void bgp_zebra_withdraw_table_all_subtypes(struct bgp *bgp, afi_t afi, safi_t sa
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)
&& (pi->type == ZEBRA_ROUTE_BGP))
bgp_zebra_withdraw(dest, pi, bgp);
bgp_zebra_route_install(dest, pi, bgp, false);
}
}
}
Expand Down Expand Up @@ -3470,6 +3600,7 @@ void bgp_zebra_init(struct event_loop *master, unsigned short instance)
zclient = zclient_new(master, &zclient_options_default, bgp_handlers,
array_size(bgp_handlers));
zclient_init(zclient, ZEBRA_ROUTE_BGP, 0, &bgpd_privs);
zclient->zebra_buffer_write_ready = bgp_zebra_buffer_write_ready;
zclient->zebra_connected = bgp_zebra_connected;
zclient->zebra_capabilities = bgp_zebra_capabilities;
zclient->nexthop_update = bgp_nexthop_update;
Expand Down
Loading

0 comments on commit 4cc141b

Please sign in to comment.