bgpd: refactor label allocation code

To prepare for fixing an issue where labels do not get released back
to the labelpool when the route is deleted some refactoring is
necessary. There are 2 parts to this.
1. restructure the code to remove the circular nature of label
allocations via the labelpool and decouple the label type decision
from the notification fo the FEC.
The code to notify the FEC association to zebra has been split out
into a separate function so that it can be called from the synchronous
path (for registration of index-based labels and de-registration of all
labels), and from the asynchronous path where we need to wait for a
callback from the labelpool code with a label allocation.
The decision about whether we are using an index-based label or an
allocated label is reflected in the state of the BGP_NODE_LABEL_REQUESTED
flag so the checks on the path_info in the labelpool callback code are
no longer required.
2. change the owned of a labelpool allocated label from the path info
structure to the bgp_dest structure. This allows labels to be released
(in a subsequent commit) when the owner (bgp_dest) goes away.

Signed-off-by: Pat Ruddy <pat@voltanet.io>
This commit is contained in:
Pat Ruddy 2020-12-17 10:41:07 +00:00
parent 189982283a
commit 992dd67ec7
4 changed files with 156 additions and 161 deletions

View File

@ -120,148 +120,29 @@ mpls_label_t bgp_adv_label(struct bgp_dest *dest, struct bgp_path_info *pi,
return dest->local_label;
}
/**
* This is passed as the callback function to bgp_labelpool.c:bgp_lp_get()
* by bgp_reg_dereg_for_label() when a label needs to be obtained from
* label pool.
* Note that it will reject the allocated label if a label index is found,
* because the label index supposes predictable labels
*/
int bgp_reg_for_label_callback(mpls_label_t new_label, void *labelid,
bool allocated)
static void bgp_send_fec_register_label_msg(struct bgp_dest *dest, bool reg,
uint32_t label_index)
{
struct bgp_path_info *pi;
struct bgp_dest *dest;
pi = labelid;
/* Is this path still valid? */
if (!bgp_path_info_unlock(pi)) {
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug(
"%s: bgp_path_info is no longer valid, ignoring",
__func__);
return -1;
}
dest = pi->net;
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug("%s: FEC %pRN label=%u, allocated=%d", __func__,
bgp_dest_to_rnode(dest), new_label, allocated);
if (!allocated) {
/*
* previously-allocated label is now invalid
*/
if (pi->attr->label_index == MPLS_INVALID_LABEL_INDEX
&& pi->attr->label != MPLS_LABEL_NONE
&& CHECK_FLAG(dest->flags, BGP_NODE_REGISTERED_FOR_LABEL)) {
bgp_unregister_for_label(dest);
label_ntop(MPLS_LABEL_IMPLICIT_NULL, 1,
&dest->local_label);
bgp_set_valid_label(&dest->local_label);
}
return 0;
}
/*
* label index is assigned, this should be handled by SR-related code,
* so retry FEC registration and then reject label allocation for
* it to be released to label pool
*/
if (pi->attr->label_index != MPLS_INVALID_LABEL_INDEX) {
flog_err(
EC_BGP_LABEL,
"%s: FEC %pRN Rejecting allocated label %u as Label Index is %u",
__func__, bgp_dest_to_rnode(dest), new_label,
pi->attr->label_index);
bgp_register_for_label(pi->net, pi);
return -1;
}
if (pi->attr->label != MPLS_INVALID_LABEL) {
if (new_label == pi->attr->label) {
/* already have same label, accept but do nothing */
return 0;
}
/* Shouldn't happen: different label allocation */
flog_err(EC_BGP_LABEL,
"%s: %pRN had label %u but got new assignment %u",
__func__, bgp_dest_to_rnode(dest), pi->attr->label,
new_label);
/* continue means use new one */
}
label_ntop(new_label, 1, &dest->local_label);
bgp_set_valid_label(&dest->local_label);
/*
* Get back to registering the FEC
*/
bgp_register_for_label(pi->net, pi);
return 0;
}
void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
bool reg)
{
bool with_label_index = false;
struct stream *s;
const struct prefix *p;
mpls_label_t *local_label;
int command;
const struct prefix *p;
uint16_t flags = 0;
size_t flags_pos = 0;
mpls_label_t *local_label = &(dest->local_label);
bool have_label_to_reg =
bgp_is_valid_label(local_label)
&& label_pton(local_label) != MPLS_LABEL_IMPLICIT_NULL;
p = bgp_dest_get_prefix(dest);
local_label = &(dest->local_label);
/* this prevents the loop when we're called by
* bgp_reg_for_label_callback()
*/
bool have_label_to_reg = bgp_is_valid_label(local_label)
&& label_pton(local_label) != MPLS_LABEL_IMPLICIT_NULL;
if (reg) {
assert(pi);
/*
* Determine if we will let zebra should derive label from
* label index instead of bgpd requesting from label pool
*/
if (CHECK_FLAG(pi->attr->flag,
ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID))
&& pi->attr->label_index != BGP_INVALID_LABEL_INDEX) {
with_label_index = true;
} else {
/*
* If no label index was provided -- assume any label
* from label pool will do. This means that label index
* always takes precedence over auto-assigned labels.
*/
if (!have_label_to_reg) {
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug(
"%s: Requesting label from LP for %pFX",
__func__, p);
/* bgp_reg_for_label_callback() will call back
* __func__ when it gets a label from the pool.
* This means we'll never register FECs without
* valid labels.
*/
bgp_lp_get(LP_TYPE_BGP_LU, pi,
bgp_reg_for_label_callback);
return;
}
}
}
/* Check socket. */
if (!zclient || zclient->sock < 0)
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_pton(local_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. */
@ -274,12 +155,13 @@ void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
stream_putw(s, PREFIX_FAMILY(p));
stream_put_prefix(s, p);
if (reg) {
if (have_label_to_reg) {
/* label index takes precedence over auto-assigned label. */
if (label_index != 0) {
flags |= ZEBRA_FEC_REGISTER_LABEL_INDEX;
stream_putl(s, label_index);
} else if (have_label_to_reg) {
flags |= ZEBRA_FEC_REGISTER_LABEL;
stream_putl(s, label_pton(local_label));
} else if (with_label_index) {
flags |= ZEBRA_FEC_REGISTER_LABEL_INDEX;
stream_putl(s, pi->attr->label_index);
}
SET_FLAG(dest->flags, BGP_NODE_REGISTERED_FOR_LABEL);
} else
@ -297,6 +179,111 @@ void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
zclient_send_message(zclient);
}
/**
* This is passed as the callback function to bgp_labelpool.c:bgp_lp_get()
* by bgp_reg_dereg_for_label() when a label needs to be obtained from
* label pool.
* Note that it will reject the allocated label if a label index is found,
* because the label index supposes predictable labels
*/
int bgp_reg_for_label_callback(mpls_label_t new_label, void *labelid,
bool allocated)
{
struct bgp_dest *dest;
dest = labelid;
/*
* if the route had been removed or the request has gone then reject
* the allocated label. The requesting code will have done what is
* required to allocate the correct label
*/
if (!CHECK_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED)) {
bgp_dest_unlock_node(dest);
return -1;
}
bgp_dest_unlock_node(dest);
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug("%s: FEC %pRN label=%u, allocated=%d", __func__,
bgp_dest_to_rnode(dest), new_label, allocated);
if (!allocated) {
/*
* previously-allocated label is now invalid, set to implicit
* null until new label arrives
*/
if (CHECK_FLAG(dest->flags, BGP_NODE_REGISTERED_FOR_LABEL)) {
UNSET_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED);
label_ntop(MPLS_LABEL_IMPLICIT_NULL, 1,
&dest->local_label);
bgp_set_valid_label(&dest->local_label);
}
}
label_ntop(new_label, 1, &dest->local_label);
bgp_set_valid_label(&dest->local_label);
/*
* Get back to registering the FEC
*/
bgp_send_fec_register_label_msg(dest, true, 0);
return 0;
}
void bgp_reg_dereg_for_label(struct bgp_dest *dest, struct bgp_path_info *pi,
bool reg)
{
bool with_label_index = false;
const struct prefix *p;
bool have_label_to_reg =
bgp_is_valid_label(&dest->local_label)
&& label_pton(&dest->local_label) != MPLS_LABEL_IMPLICIT_NULL;
p = bgp_dest_get_prefix(dest);
if (reg) {
assert(pi);
/*
* Determine if we will let zebra should derive label from
* label index instead of bgpd requesting from label pool
*/
if (CHECK_FLAG(pi->attr->flag,
ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID))
&& pi->attr->label_index != BGP_INVALID_LABEL_INDEX) {
with_label_index = true;
UNSET_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED);
} else {
/*
* If no label has been registered -- assume any label
* from label pool will do. This means that label index
* always takes precedence over auto-assigned labels.
*/
if (!have_label_to_reg) {
SET_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED);
if (BGP_DEBUG(labelpool, LABELPOOL))
zlog_debug(
"%s: Requesting label from LP for %pFX",
__func__, p);
/* bgp_reg_for_label_callback() will deal with
* fec registration when it gets a label from
* the pool. This means we'll never register
* FECs withoutvalid labels.
*/
bgp_lp_get(LP_TYPE_BGP_LU, dest,
bgp_reg_for_label_callback);
return;
}
}
} else
UNSET_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED);
bgp_send_fec_register_label_msg(
dest, reg, with_label_index ? pi->attr->label_index : 0);
}
static int bgp_nlri_get_labels(struct peer *peer, uint8_t *pnt, uint8_t plen,
mpls_label_t *label)
{

View File

@ -182,18 +182,18 @@ void bgp_lp_init(struct thread_master *master, struct labelpool *pool)
lp->callback_q->spec.max_retries = 0;
}
/* check if a label callback was for a BGP LU path, and if so, unlock it */
/* check if a label callback was for a BGP LU node, and if so, unlock it */
static void check_bgp_lu_cb_unlock(struct lp_lcb *lcb)
{
if (lcb->type == LP_TYPE_BGP_LU)
bgp_path_info_unlock(lcb->labelid);
bgp_dest_unlock_node(lcb->labelid);
}
/* check if a label callback was for a BGP LU path, and if so, lock it */
/* check if a label callback was for a BGP LU node, and if so, lock it */
static void check_bgp_lu_cb_lock(struct lp_lcb *lcb)
{
if (lcb->type == LP_TYPE_BGP_LU)
bgp_path_info_lock(lcb->labelid);
bgp_dest_lock_node(lcb->labelid);
}
void bgp_lp_finish(void)
@ -356,7 +356,7 @@ void bgp_lp_get(
q->labelid = lcb->labelid;
q->allocated = true;
/* if this is a LU request, lock path info before queueing */
/* if this is a LU request, lock node before queueing */
check_bgp_lu_cb_lock(lcb);
work_queue_add(lp->callback_q, q);
@ -384,7 +384,7 @@ void bgp_lp_get(
sizeof(struct lp_fifo));
lf->lcb = *lcb;
/* if this is a LU request, lock path info before queueing */
/* if this is a LU request, lock node before queueing */
check_bgp_lu_cb_lock(lcb);
lp_fifo_add_tail(&lp->requests, lf);
@ -461,7 +461,7 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
zlog_debug("%s: labelid %p: request no longer in effect",
__func__, labelid);
}
/* if this was a BGP_LU request, unlock path info node
/* if this was a BGP_LU request, unlock node
*/
check_bgp_lu_cb_unlock(lcb);
goto finishedrequest;
@ -475,7 +475,7 @@ void bgp_lp_event_chunk(uint8_t keep, uint32_t first, uint32_t last)
__func__, labelid,
lcb->label, lcb->label, lcb);
}
/* if this was a BGP_LU request, unlock path info node
/* if this was a BGP_LU request, unlock node
*/
check_bgp_lu_cb_unlock(lcb);
@ -667,7 +667,7 @@ DEFUN(show_bgp_labelpool_ledger, show_bgp_labelpool_ledger_cmd,
bool uj = use_json(argc, argv);
json_object *json = NULL, *json_elem = NULL;
struct lp_lcb *lcb = NULL;
struct bgp_path_info *pi;
struct bgp_dest *dest;
void *cursor = NULL;
const struct prefix *p;
int rc, count;
@ -692,9 +692,9 @@ DEFUN(show_bgp_labelpool_ledger, show_bgp_labelpool_ledger_cmd,
vty_out(vty, "---------------------------\n");
}
for (rc = skiplist_next(lp->ledger, (void **)&pi, (void **)&lcb,
for (rc = skiplist_next(lp->ledger, (void **)&dest, (void **)&lcb,
&cursor);
!rc; rc = skiplist_next(lp->ledger, (void **)&pi, (void **)&lcb,
!rc; rc = skiplist_next(lp->ledger, (void **)&dest, (void **)&lcb,
&cursor)) {
if (uj) {
json_elem = json_object_new_object();
@ -702,7 +702,7 @@ DEFUN(show_bgp_labelpool_ledger, show_bgp_labelpool_ledger_cmd,
}
switch (lcb->type) {
case LP_TYPE_BGP_LU:
if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID))
if (!CHECK_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED))
if (uj) {
json_object_string_add(
json_elem, "prefix", "INVALID");
@ -713,7 +713,7 @@ DEFUN(show_bgp_labelpool_ledger, show_bgp_labelpool_ledger_cmd,
"INVALID", lcb->label);
else {
char buf[PREFIX2STR_BUFFER];
p = bgp_dest_get_prefix(pi->net);
p = bgp_dest_get_prefix(dest);
prefix2str(p, buf, sizeof(buf));
if (uj) {
json_object_string_add(json_elem,
@ -755,7 +755,7 @@ DEFUN(show_bgp_labelpool_inuse, show_bgp_labelpool_inuse_cmd,
{
bool uj = use_json(argc, argv);
json_object *json = NULL, *json_elem = NULL;
struct bgp_path_info *pi;
struct bgp_dest *dest;
mpls_label_t label;
struct lp_lcb *lcb;
void *cursor = NULL;
@ -785,11 +785,11 @@ DEFUN(show_bgp_labelpool_inuse, show_bgp_labelpool_inuse_cmd,
vty_out(vty, "Prefix Label\n");
vty_out(vty, "---------------------------\n");
}
for (rc = skiplist_next(lp->inuse, (void **)&label, (void **)&pi,
for (rc = skiplist_next(lp->inuse, (void **)&label, (void **)&dest,
&cursor);
!rc; rc = skiplist_next(lp->ledger, (void **)&label, (void **)&pi,
&cursor)) {
if (skiplist_search(lp->ledger, pi, (void **)&lcb))
!rc; rc = skiplist_next(lp->ledger, (void **)&label,
(void **)&dest, &cursor)) {
if (skiplist_search(lp->ledger, dest, (void **)&lcb))
continue;
if (uj) {
@ -799,7 +799,7 @@ DEFUN(show_bgp_labelpool_inuse, show_bgp_labelpool_inuse_cmd,
switch (lcb->type) {
case LP_TYPE_BGP_LU:
if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID))
if (!CHECK_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED))
if (uj) {
json_object_string_add(
json_elem, "prefix", "INVALID");
@ -810,7 +810,7 @@ DEFUN(show_bgp_labelpool_inuse, show_bgp_labelpool_inuse_cmd,
label);
else {
char buf[PREFIX2STR_BUFFER];
p = bgp_dest_get_prefix(pi->net);
p = bgp_dest_get_prefix(dest);
prefix2str(p, buf, sizeof(buf));
if (uj) {
json_object_string_add(json_elem,
@ -850,7 +850,7 @@ DEFUN(show_bgp_labelpool_requests, show_bgp_labelpool_requests_cmd,
{
bool uj = use_json(argc, argv);
json_object *json = NULL, *json_elem = NULL;
struct bgp_path_info *pi;
struct bgp_dest *dest;
const struct prefix *p;
char buf[PREFIX2STR_BUFFER];
struct lp_fifo *item, *next;
@ -878,21 +878,22 @@ DEFUN(show_bgp_labelpool_requests, show_bgp_labelpool_requests_cmd,
for (item = lp_fifo_first(&lp->requests); item; item = next) {
next = lp_fifo_next_safe(&lp->requests, item);
pi = item->lcb.labelid;
dest = item->lcb.labelid;
if (uj) {
json_elem = json_object_new_object();
json_object_array_add(json, json_elem);
}
switch (item->lcb.type) {
case LP_TYPE_BGP_LU:
if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID)) {
if (!CHECK_FLAG(dest->flags,
BGP_NODE_LABEL_REQUESTED)) {
if (uj)
json_object_string_add(
json_elem, "prefix", "INVALID");
else
vty_out(vty, "INVALID\n");
} else {
p = bgp_dest_get_prefix(pi->net);
p = bgp_dest_get_prefix(dest);
prefix2str(p, buf, sizeof(buf));
if (uj)
json_object_string_add(json_elem,

View File

@ -2755,7 +2755,10 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
== BGP_ROUTE_REDISTRIBUTE) {
if (CHECK_FLAG(
dest->flags,
BGP_NODE_REGISTERED_FOR_LABEL))
BGP_NODE_REGISTERED_FOR_LABEL)
|| CHECK_FLAG(
dest->flags,
BGP_NODE_LABEL_REQUESTED))
bgp_unregister_for_label(dest);
label_ntop(MPLS_LABEL_IMPLICIT_NULL, 1,
&dest->local_label);
@ -2765,10 +2768,13 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
new_select);
}
} else if (CHECK_FLAG(dest->flags,
BGP_NODE_REGISTERED_FOR_LABEL)) {
BGP_NODE_REGISTERED_FOR_LABEL)
|| CHECK_FLAG(dest->flags,
BGP_NODE_LABEL_REQUESTED)) {
bgp_unregister_for_label(dest);
}
} else if (CHECK_FLAG(dest->flags, BGP_NODE_REGISTERED_FOR_LABEL)) {
} else if (CHECK_FLAG(dest->flags, BGP_NODE_REGISTERED_FOR_LABEL)
|| CHECK_FLAG(dest->flags, BGP_NODE_LABEL_REQUESTED)) {
bgp_unregister_for_label(dest);
}

View File

@ -104,6 +104,7 @@ struct bgp_node {
#define BGP_NODE_SELECT_DEFER (1 << 4)
#define BGP_NODE_FIB_INSTALL_PENDING (1 << 5)
#define BGP_NODE_FIB_INSTALLED (1 << 6)
#define BGP_NODE_LABEL_REQUESTED (1 << 7)
struct bgp_addpath_node_data tx_addpath;