Skip to content

Commit

Permalink
Fix label allocation for multicast
Browse files Browse the repository at this point in the history
During BFD scale tests a crash was observed when attempting to
free an MPLS label, It turns out the label for a local interface
was allocated a label in the multicast range.
The root cause - during the allocation of label blocks per
control node the last label in the block wasn't actually
reserved. This label was allocated to locally on the agent for a VMI.
When attempting to free the label the check for the multicast range
suceeds, at this point it attempts to look up the vrf name from the
label data (changes made in bug#1724114), the label data is not set
in this case and causes the crash.

Fix: Correct the label block allocation for multicast to reserve all
labels in the range.

Change-Id: I47feb2a23b721a616b879d5e542fb2f80348d4fa
Closes-Bug: 1798483
  • Loading branch information
psdsouza committed Oct 25, 2018
1 parent cb45665 commit 7df5a97
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/vnsw/agent/controller/controller_init.cc
Expand Up @@ -85,7 +85,9 @@ VNController::~VNController() {
void VNController::FillMcastLabelRange(uint32_t *start_idx,
uint32_t *end_idx,
uint8_t idx) const {
// Multicast labels required by both control nodes
uint32_t max_mc_labels = 2 * (agent_->vrouter_max_vrfs());
// Multicast label count per control node
uint32_t mc_label_count = 0;
uint32_t vrouter_max_labels = agent_->vrouter_max_labels();

Expand Down Expand Up @@ -125,9 +127,9 @@ void VNController::SetAgentMcastLabelRange(uint8_t idx) {
FillMcastLabelRange(&start, &end, idx);
str << start << "-" << end;

agent_->mpls_table()->ReserveMulticastLabel(start, end + 1, idx);
agent_->mpls_table()->ReserveMulticastLabel(start, end, idx);
fabric_multicast_label_range_[idx].start = start;
fabric_multicast_label_range_[idx].end = (end + 1);
fabric_multicast_label_range_[idx].end = end;
fabric_multicast_label_range_[idx].fabric_multicast_label_range_str =
str.str();
}
Expand Down
9 changes: 6 additions & 3 deletions src/vnsw/agent/oper/mpls.cc
Expand Up @@ -282,9 +282,12 @@ bool MplsTable::OnChange(DBEntry *entry, const DBRequest *req) {
}

bool MplsTable::Delete(DBEntry *entry, const DBRequest *req) {
MplsLabelData *data = static_cast<MplsLabelData *>(req->data.get());
MplsLabel *mpls = static_cast<MplsLabel *>(entry);
if (IsFabricMulticastLabel(mpls->label())) {
MplsLabelData *data = static_cast<MplsLabelData *>(req->data.get());
// For multicast labels we not expect to be here
// via MplsTable::OnZeroRefcount where data is not set.
assert(data);
mpls->fmg_nh_list().erase(data->vrf_name());
if (mpls->fmg_nh_list().empty() == false) {
if (mpls->ChangeNH(mpls->fmg_nh_list().begin()->second)) {
Expand Down Expand Up @@ -347,15 +350,15 @@ bool MplsTable::IsFabricMulticastLabel(uint32_t label) const {
void MplsTable::ReserveLabel(uint32_t start, uint32_t end) {
// We want to allocate labels from an offset
// Pre-allocate entries
for (uint32_t i = start; i < end; i++) {
for (uint32_t i = start; i <= end; i++) {
agent()->resource_manager()->ReserveIndex(Resource::MPLS_INDEX, i);
}
}

void MplsTable::FreeReserveLabel(uint32_t start, uint32_t end) {
// We want to allocate labels from an offset
// Pre-allocate entries
for (uint32_t i = start; i < end; i++) {
for (uint32_t i = start; i <= end; i++) {
agent()->resource_manager()->ReleaseIndex(Resource::MPLS_INDEX, i);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/operdb_init.cc
Expand Up @@ -174,7 +174,7 @@ void OperDB::CreateDBTables(DB *db) {
assert(mpls_table);
agent_->set_mpls_table(mpls_table);
mpls_table->set_agent(agent_);
mpls_table->ReserveLabel(0, MplsTable::kStartLabel);
mpls_table->ReserveLabel(0, MplsTable::kStartLabel - 1);

AclTable *acl_table;
acl_table = static_cast<AclTable *>(db->CreateTable("db.acl.0"));
Expand Down Expand Up @@ -341,7 +341,7 @@ OperDB::~OperDB() {
}

void OperDB::Shutdown() {
agent_->mpls_table()->FreeReserveLabel(0, MplsTable::kStartLabel);
agent_->mpls_table()->FreeReserveLabel(0, MplsTable::kStartLabel - 1);
instance_manager_->Terminate();
if (nexthop_manager_.get()) {
nexthop_manager_->Terminate();
Expand Down

0 comments on commit 7df5a97

Please sign in to comment.