-
Notifications
You must be signed in to change notification settings - Fork 547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[muxorch] Using bulker to program routes/neighbors during switchover #3148
Merged
Merged
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
d4db44e
[muxorch] Using bulker to program routes/neighbors during switchover
Ndancejic d0d3fe9
Merge branch 'master' into bulk_switchover
Ndancejic 4fd3503
Merge branch 'master' into bulk_switchover
Ndancejic 11e5cce
fixing aclorch_ut
Ndancejic 2b3240b
skip MuxRollbackTests until create_neighbor_entries api is supported
Ndancejic f259759
[mux_rollback_ut.cpp] fixing mux_rollback_ut tests for bulk switchover
Ndancejic 7c2bcbb
Merge branch 'master' into bulk_switchover
prsunny 11f1674
Merge branch 'master' into bulk_switchover
prsunny 811f5a9
Merge branch 'master' into bulk_switchover
prsunny 934b566
Merge branch 'master' into bulk_switchover
prsunny e318676
[muxorch] Removing INFO logging from tests and ensuring neigh is set
Ndancejic 9a675bf
Merge branch 'master' into bulk_switchover
Ndancejic fc9f317
Merge branch 'master' into bulk_switchover
prsunny 411da0f
[muxorch] Refactoring bulk switchover
Ndancejic 31e0dd9
Addressing PR comments
Ndancejic c823cb2
Removing un-needed code and expanding test coverage
Ndancejic 1cfcb4a
addressing PR comments
Ndancejic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -744,20 +744,30 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu | |
bool MuxNbrHandler::enable(bool update_rt) | ||
{ | ||
NeighborEntry neigh; | ||
std::list<NeighborContext> neigh_ctx_list; | ||
std::list<MuxRouteBulkContext> route_ctx_list; | ||
|
||
auto it = neighbors_.begin(); | ||
while (it != neighbors_.end()) | ||
{ | ||
SWSS_LOG_INFO("Enabling neigh %s on %s", it->first.to_string().c_str(), alias_.c_str()); | ||
|
||
neigh = NeighborEntry(it->first, alias_); | ||
if (!gNeighOrch->enableNeighbor(neigh)) | ||
{ | ||
SWSS_LOG_INFO("Enabling neigh failed for %s", neigh.ip_address.to_string().c_str()); | ||
return false; | ||
} | ||
// Create neighbor context with bulk_op enabled | ||
neigh_ctx_list.push_back(NeighborContext(neigh, true)); | ||
it++; | ||
} | ||
|
||
if (!gNeighOrch->enableNeighbors(neigh_ctx_list)) | ||
{ | ||
return false; | ||
} | ||
|
||
it = neighbors_.begin(); | ||
while (it != neighbors_.end()) | ||
{ | ||
/* Update NH to point to learned neighbor */ | ||
neigh = NeighborEntry(it->first, alias_); | ||
it->second = gNeighOrch->getLocalNextHopId(neigh); | ||
|
||
/* Reprogram route */ | ||
|
@@ -795,22 +805,28 @@ bool MuxNbrHandler::enable(bool update_rt) | |
IpPrefix pfx = it->first.to_string(); | ||
if (update_rt) | ||
{ | ||
if (remove_route(pfx) != SAI_STATUS_SUCCESS) | ||
{ | ||
return false; | ||
} | ||
updateTunnelRoute(nh_key, false); | ||
route_ctx_list.push_back(MuxRouteBulkContext(pfx)); | ||
} | ||
|
||
updateTunnelRoute(nh_key, false); | ||
|
||
it++; | ||
} | ||
|
||
if (update_rt && !removeRoutes(route_ctx_list)) | ||
{ | ||
return false; | ||
} | ||
|
||
gRouteBulker.clear(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we clear this even if its returned false above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Took a look at this, it's not needed since we call clear from removeRoutes |
||
return true; | ||
} | ||
|
||
bool MuxNbrHandler::disable(sai_object_id_t tnh) | ||
{ | ||
NeighborEntry neigh; | ||
std::list<NeighborContext> neigh_ctx_list; | ||
std::list<MuxRouteBulkContext> route_ctx_list; | ||
|
||
auto it = neighbors_.begin(); | ||
while (it != neighbors_.end()) | ||
|
@@ -852,21 +868,25 @@ bool MuxNbrHandler::disable(sai_object_id_t tnh) | |
updateTunnelRoute(nh_key, true); | ||
|
||
IpPrefix pfx = it->first.to_string(); | ||
if (create_route(pfx, it->second) != SAI_STATUS_SUCCESS) | ||
{ | ||
return false; | ||
} | ||
route_ctx_list.push_back(MuxRouteBulkContext(pfx, it->second)); | ||
|
||
neigh = NeighborEntry(it->first, alias_); | ||
if (!gNeighOrch->disableNeighbor(neigh)) | ||
{ | ||
SWSS_LOG_INFO("Disabling neigh failed for %s", neigh.ip_address.to_string().c_str()); | ||
return false; | ||
} | ||
// Create neighbor context with bulk_op enabled | ||
neigh_ctx_list.push_back(NeighborContext(neigh, true)); | ||
|
||
it++; | ||
} | ||
|
||
if (!addRoutes(route_ctx_list)) | ||
{ | ||
return false; | ||
} | ||
|
||
if (!gNeighOrch->disableNeighbors(neigh_ctx_list)) | ||
{ | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
|
@@ -881,6 +901,141 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey) | |
return SAI_NULL_OBJECT_ID; | ||
} | ||
|
||
bool MuxNbrHandler::addRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list) | ||
{ | ||
sai_status_t status; | ||
bool ret = true; | ||
|
||
for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) | ||
{ | ||
auto& object_statuses = ctx->object_statuses; | ||
sai_route_entry_t route_entry; | ||
route_entry.switch_id = gSwitchId; | ||
route_entry.vr_id = gVirtualRouterId; | ||
copy(route_entry.destination, ctx->pfx); | ||
subnet(route_entry.destination, route_entry.destination); | ||
|
||
SWSS_LOG_INFO("Adding route entry %s, nh %" PRIx64 " to bulker", ctx->pfx.getIp().to_string().c_str(), ctx->nh); | ||
|
||
object_statuses.emplace_back(); | ||
sai_attribute_t attr; | ||
vector<sai_attribute_t> attrs; | ||
|
||
attr.id = SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION; | ||
attr.value.s32 = SAI_PACKET_ACTION_FORWARD; | ||
attrs.push_back(attr); | ||
|
||
attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID; | ||
attr.value.oid = ctx->nh; | ||
attrs.push_back(attr); | ||
|
||
status = gRouteBulker.create_entry(&object_statuses.back(), &route_entry, (uint32_t)attrs.size(), attrs.data()); | ||
} | ||
|
||
gRouteBulker.flush(); | ||
|
||
for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) | ||
{ | ||
auto& object_statuses = ctx->object_statuses; | ||
auto it_status = object_statuses.begin(); | ||
status = *it_status++; | ||
|
||
sai_route_entry_t route_entry; | ||
route_entry.switch_id = gSwitchId; | ||
route_entry.vr_id = gVirtualRouterId; | ||
copy(route_entry.destination, ctx->pfx); | ||
subnet(route_entry.destination, route_entry.destination); | ||
|
||
if (status != SAI_STATUS_SUCCESS) | ||
{ | ||
if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) { | ||
SWSS_LOG_INFO("Tunnel route to %s already exists", ctx->pfx.to_string().c_str()); | ||
continue; | ||
} | ||
SWSS_LOG_ERROR("Failed to create tunnel route %s,nh %" PRIx64 " rv:%d", | ||
ctx->pfx.getIp().to_string().c_str(), ctx->nh, status); | ||
ret = false; | ||
continue; | ||
} | ||
|
||
if (route_entry.destination.addr_family == SAI_IP_ADDR_FAMILY_IPV4) | ||
{ | ||
gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE); | ||
} | ||
else | ||
{ | ||
gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); | ||
} | ||
|
||
SWSS_LOG_NOTICE("Created tunnel route to %s ", ctx->pfx.to_string().c_str()); | ||
} | ||
|
||
gRouteBulker.clear(); | ||
return ret; | ||
} | ||
|
||
bool MuxNbrHandler::removeRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list) | ||
{ | ||
sai_status_t status; | ||
bool ret = true; | ||
|
||
for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) | ||
{ | ||
auto& object_statuses = ctx->object_statuses; | ||
sai_route_entry_t route_entry; | ||
route_entry.switch_id = gSwitchId; | ||
route_entry.vr_id = gVirtualRouterId; | ||
copy(route_entry.destination, ctx->pfx); | ||
subnet(route_entry.destination, route_entry.destination); | ||
|
||
SWSS_LOG_INFO("Removing route entry %s, nh %" PRIx64 "", ctx->pfx.getIp().to_string().c_str(), ctx->nh); | ||
|
||
object_statuses.emplace_back(); | ||
status = gRouteBulker.remove_entry(&object_statuses.back(), &route_entry); | ||
} | ||
|
||
gRouteBulker.flush(); | ||
|
||
for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) | ||
{ | ||
auto& object_statuses = ctx->object_statuses; | ||
auto it_status = object_statuses.begin(); | ||
status = *it_status++; | ||
|
||
sai_route_entry_t route_entry; | ||
route_entry.switch_id = gSwitchId; | ||
route_entry.vr_id = gVirtualRouterId; | ||
copy(route_entry.destination, ctx->pfx); | ||
subnet(route_entry.destination, route_entry.destination); | ||
|
||
if (status != SAI_STATUS_SUCCESS) | ||
{ | ||
if (status == SAI_STATUS_ITEM_NOT_FOUND) { | ||
SWSS_LOG_INFO("Tunnel route to %s already removed", ctx->pfx.to_string().c_str()); | ||
continue; | ||
} | ||
SWSS_LOG_ERROR("Failed to remove tunnel route %s, rv:%d", | ||
ctx->pfx.getIp().to_string().c_str(), status); | ||
ret = false; | ||
continue; | ||
} | ||
|
||
if (route_entry.destination.addr_family == SAI_IP_ADDR_FAMILY_IPV4) | ||
{ | ||
gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE); | ||
} | ||
else | ||
{ | ||
gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); | ||
} | ||
|
||
SWSS_LOG_NOTICE("Removed tunnel route to %s ", ctx->pfx.to_string().c_str()); | ||
} | ||
|
||
gRouteBulker.clear(); | ||
return ret; | ||
} | ||
|
||
void MuxNbrHandler::updateTunnelRoute(NextHopKey nh, bool add) | ||
{ | ||
MuxOrch* mux_orch = gDirectory.get<MuxOrch*>(); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be inside the
update_rt
check?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I'll move that back