Skip to content

Commit

Permalink
(red-diff) Remove the segment routing related functionality from spfS…
Browse files Browse the repository at this point in the history 
…olver

Summary:
As titled.

Red-diff cleanup before OSS fix.

Reviewed By: shih-hao-tseng

Differential Revision: D64379600

fbshipit-source-id: 5004fdcd33f480a7b75674ead4aaebd4b1193969
  • Loading branch information
@facebook-github-bot
Xiang Xu authored and facebook-github-bot committed Oct 15, 2024
1 parent 8639752 commit 315b7e4
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 893 deletions.
1 change: 0 additions & 1 deletion openr/decision/Decision.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ Decision::Decision(
spfSolver_ = std::make_unique<SpfSolver>(
config->getNodeName(),
config->isV4Enabled(),
config->isSegmentRoutingEnabled(),
config->isBestRouteSelectionEnabled(),
config->isV4OverV6NexthopEnabled());

Expand Down
106 changes: 0 additions & 106 deletions openr/decision/SpfSolver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,34 +74,24 @@ DecisionRouteDb::update(DecisionRouteUpdate const& update) {
SpfSolver::SpfSolver(
const std::string& myNodeName,
bool enableV4,
bool enableNodeSegmentLabel,
bool enableBestRouteSelection,
bool v4OverV6Nexthop)
: myNodeName_(myNodeName),
enableV4_(enableV4),
enableNodeSegmentLabel_(enableNodeSegmentLabel),
enableBestRouteSelection_(enableBestRouteSelection),
v4OverV6Nexthop_(v4OverV6Nexthop) {
// Initialize stat keys
fb303::fbData->addStatExportType("decision.adj_db_update", fb303::COUNT);
fb303::fbData->addStatExportType(
"decision.incompatible_forwarding_type", fb303::COUNT);
fb303::fbData->addStatExportType("decision.no_route_to_label", fb303::COUNT);
fb303::fbData->addStatExportType("decision.no_route_to_prefix", fb303::COUNT);
fb303::fbData->addStatExportType("decision.path_build_ms", fb303::AVG);
fb303::fbData->addStatExportType("decision.prefix_db_update", fb303::COUNT);
fb303::fbData->addStatExportType("decision.route_build_ms", fb303::AVG);
fb303::fbData->addStatExportType("decision.route_build_runs", fb303::COUNT);
fb303::fbData->addStatExportType(
"decision.get_route_for_prefix", fb303::COUNT);
fb303::fbData->addStatExportType("decision.skipped_mpls_route", fb303::COUNT);
fb303::fbData->addStatExportType(
"decision.duplicate_node_label", fb303::COUNT);
fb303::fbData->addStatExportType("decision.spf_ms", fb303::AVG);
fb303::fbData->addStatExportType("decision.spf_runs", fb303::COUNT);
fb303::fbData->addStatExportType("decision.errors", fb303::COUNT);
fb303::fbData->addStatExportType(
"decision.incorrect_redistribution_route", fb303::COUNT);
}

SpfSolver::~SpfSolver() = default;
Expand Down Expand Up @@ -348,102 +338,6 @@ SpfSolver::buildRouteDb(
routeDb.addUnicastRoute(RibUnicastEntry(ribUnicastEntry));
}

//
// Create MPLS routes for all nodeLabel
//
if (enableNodeSegmentLabel_) {
std::unordered_map<int32_t, std::pair<std::string, RibMplsEntry>>
labelToNode;
for (const auto& [area, linkState] : areaLinkStates) {
for (const auto& [_, adjDb] : linkState.getAdjacencyDatabases()) {
const auto topLabel = *adjDb.nodeLabel();
const auto& nodeName = *adjDb.thisNodeName();
// Top label is not set => Non-SR mode
if (topLabel == 0) {
XLOG(INFO) << "Ignoring node label " << topLabel << " of node "
<< nodeName << " in area " << area;
fb303::fbData->addStatValue(
"decision.skipped_mpls_route", 1, fb303::COUNT);
continue;
}
// If mpls label is not valid then ignore it
if (not isMplsLabelValid(topLabel)) {
XLOG(ERR) << "Ignoring invalid node label " << topLabel << " of node "
<< nodeName << " in area " << area;
fb303::fbData->addStatValue(
"decision.skipped_mpls_route", 1, fb303::COUNT);
continue;
}

// There can be a temporary collision in node label allocation.
// Usually happens when two segmented networks allocating labels from
// the same range join together. In case of such conflict we respect
// the node label of bigger node-ID
auto iter = labelToNode.find(topLabel);
if (iter != labelToNode.end()) {
XLOG(INFO) << "Found duplicate label " << topLabel << "from "
<< iter->second.first << " " << nodeName << " in area "
<< area;
fb303::fbData->addStatValue(
"decision.duplicate_node_label", 1, fb303::COUNT);
if (iter->second.first < nodeName) {
continue;
}
}

// Install POP_AND_LOOKUP for next layer
if (*adjDb.thisNodeName() == myNodeName) {
thrift::NextHopThrift nh;
nh.address() = toBinaryAddress(folly::IPAddressV6("::"));
nh.area() = area;
nh.mplsAction() =
createMplsAction(thrift::MplsActionCode::POP_AND_LOOKUP);
labelToNode.erase(topLabel);
labelToNode.emplace(
topLabel,
std::make_pair(myNodeName, RibMplsEntry(topLabel, {nh})));
continue;
}

// Get best nexthop towards the node
auto metricNhs = getNextHopsWithMetric(
myNodeName, {{adjDb.thisNodeName().value(), area}}, linkState);
if (metricNhs.second.empty()) {
XLOG(WARNING) << "No route to nodeLabel " << std::to_string(topLabel)
<< " of node " << nodeName;
fb303::fbData->addStatValue(
"decision.no_route_to_label", 1, fb303::COUNT);
continue;
}

// Create nexthops with appropriate MplsAction (PHP and SWAP). Note
// that all nexthops are valid for routing without loops. Fib is
// responsible for installing these routes by making sure it programs
// least cost nexthops first and of same action type (based on HW
// limitations)
labelToNode.erase(topLabel);
labelToNode.emplace(
topLabel,
std::make_pair(
adjDb.thisNodeName().value(),
RibMplsEntry(
topLabel,
getNextHopsThrift(
myNodeName,
{{adjDb.thisNodeName().value(), area}},
false /* isV4 */,
metricNhs,
topLabel,
area,
linkState))));
}
}

for (auto& [_, nodeToEntry] : labelToNode) {
routeDb.addMplsRoute(std::move(nodeToEntry.second));
}
}

auto deltaTime = std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - startTime);
XLOG(INFO) << "Decision::buildRouteDb took " << deltaTime.count() << "ms.";
Expand Down
1 change: 0 additions & 1 deletion openr/decision/SpfSolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ class SpfSolver {
SpfSolver(
const std::string& myNodeName,
bool enableV4,
bool enableNodeSegmentLabel,
bool enableBestRouteSelection = false,
bool v4OverV6Nexthop = false);
~SpfSolver();
Expand Down
49 changes: 5 additions & 44 deletions openr/decision/tests/DecisionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,14 +544,10 @@ TEST_F(DecisionTestFixture, BasicOperations) {
sendKvPublication(publication);
auto routeDbDelta = recvRouteUpdates();
EXPECT_EQ(1, routeDbDelta.unicastRoutesToUpdate.size());
// self mpls route and node 2 mpls route label route
EXPECT_EQ(2, routeDbDelta.mplsRoutesToUpdate.size());
EXPECT_EQ(0, routeDbDelta.mplsRoutesToDelete.size());
EXPECT_EQ(0, routeDbDelta.unicastRoutesToDelete.size());

auto routeDb = dumpRouteDb({"1"})["1"];
std::sort(routeDb.unicastRoutes()->begin(), routeDb.unicastRoutes()->end());
std::sort(routeDb.mplsRoutes()->begin(), routeDb.mplsRoutes()->end());

auto routeDelta = findDeltaRoutes(routeDb, routeDbBefore);
EXPECT_TRUE(checkEqualRoutesDelta(routeDbDelta, routeDelta));
Expand Down Expand Up @@ -583,8 +579,6 @@ TEST_F(DecisionTestFixture, BasicOperations) {
std::sort(
routeDbBefore.unicastRoutes()->begin(),
routeDbBefore.unicastRoutes()->end());
std::sort(
routeDbBefore.mplsRoutes()->begin(), routeDbBefore.mplsRoutes()->end());
sendKvPublication(publication);
// validate routers

Expand All @@ -595,13 +589,10 @@ TEST_F(DecisionTestFixture, BasicOperations) {
EXPECT_EQ(
routeDbDelta.unicastRoutesToUpdate.begin()->second.prefix,
toIPNetwork(addr3));
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.size());
EXPECT_EQ(0, routeDbDelta.mplsRoutesToDelete.size());
EXPECT_EQ(0, routeDbDelta.unicastRoutesToDelete.size());

routeDb = dumpRouteDb({"1"})["1"];
std::sort(routeDb.unicastRoutes()->begin(), routeDb.unicastRoutes()->end());
std::sort(routeDb.mplsRoutes()->begin(), routeDb.mplsRoutes()->end());
routeDelta = findDeltaRoutes(routeDb, routeDbBefore);
EXPECT_TRUE(checkEqualRoutesDelta(routeDbDelta, routeDelta));
fillRouteMap("1", routeMap, routeDb);
Expand Down Expand Up @@ -651,16 +642,12 @@ TEST_F(DecisionTestFixture, BasicOperations) {
std::sort(
routeDbBefore.unicastRoutes()->begin(),
routeDbBefore.unicastRoutes()->end());
std::sort(
routeDbBefore.mplsRoutes()->begin(), routeDbBefore.mplsRoutes()->end());

sendKvPublication(publication);
routeDbDelta = recvRouteUpdates();
EXPECT_EQ(1, routeDbDelta.unicastRoutesToDelete.size());
EXPECT_EQ(1, routeDbDelta.mplsRoutesToDelete.size());
routeDb = dumpRouteDb({"1"})["1"];
std::sort(routeDb.unicastRoutes()->begin(), routeDb.unicastRoutes()->end());
std::sort(routeDb.mplsRoutes()->begin(), routeDb.mplsRoutes()->end());

routeDelta = findDeltaRoutes(routeDb, routeDbBefore);
EXPECT_TRUE(checkEqualRoutesDelta(routeDbDelta, routeDelta));
Expand All @@ -680,8 +667,6 @@ TEST_F(DecisionTestFixture, BasicOperations) {
std::sort(
routeDbBefore.unicastRoutes()->begin(),
routeDbBefore.unicastRoutes()->end());
std::sort(
routeDbBefore.mplsRoutes()->begin(), routeDbBefore.mplsRoutes()->end());
sendKvPublication(publication);
// validate routers

Expand All @@ -692,12 +677,9 @@ TEST_F(DecisionTestFixture, BasicOperations) {
EXPECT_EQ(
routeDbDelta.unicastRoutesToUpdate.begin()->second.prefix,
toIPNetwork(addr3));
EXPECT_EQ(0, routeDbDelta.mplsRoutesToDelete.size());
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.size());

routeDb = dumpRouteDb({"1"})["1"];
std::sort(routeDb.unicastRoutes()->begin(), routeDb.unicastRoutes()->end());
std::sort(routeDb.mplsRoutes()->begin(), routeDb.mplsRoutes()->end());
routeDelta = findDeltaRoutes(routeDb, routeDbBefore);
EXPECT_TRUE(checkEqualRoutesDelta(routeDbDelta, routeDelta));
}
Expand Down Expand Up @@ -828,7 +810,6 @@ TEST_F(DecisionTestFixture, InitialRouteBuildBlockedForAdjacencyDb) {
// Receive & verify all the expected updates
auto routeDbDelta = recvRouteUpdates();
EXPECT_EQ(1, routeDbDelta.unicastRoutesToUpdate.size());
EXPECT_EQ(0, routeDbDelta.mplsRoutesToDelete.size());
EXPECT_EQ(0, routeDbDelta.unicastRoutesToDelete.size());
}

Expand Down Expand Up @@ -2572,7 +2553,7 @@ TEST_F(DecisionTestFixture, DecisionSubReliability) {
//
// This test aims to verify counter reporting from Decision module
//
TEST_F(DecisionTestFixture, Counters) {
TEST_F(DecisionTestFixture, CountersTest) {
// Verifiy some initial/default counters
{
decision->updateGlobalCounters();
Expand All @@ -2596,23 +2577,12 @@ TEST_F(DecisionTestFixture, Counters) {
thrift::PrefixForwardingType::IP,
thrift::PrefixForwardingAlgorithm::SP_ECMP,
std::nullopt /* missing metric vector */);
auto bgpPrefixEntry3 = createPrefixEntry( // Conflicting forwarding type
toIpPrefix("10.3.0.0/16"),
thrift::PrefixType::BGP,
"data=10.3.0.0/16",
thrift::PrefixForwardingType::SR_MPLS,
thrift::PrefixForwardingAlgorithm::SP_ECMP);
thrift::KeyVals pubKvs = {
{"adj:1", createAdjValue(serializer, "1", 1, {adj12, adj13}, false, 1)},
{"adj:2", createAdjValue(serializer, "2", 1, {adj21, adj23}, false, 2)},
{"adj:3",
createAdjValue(serializer, "3", 1, {adj31}, false, 3 << 20)}, // invalid
// mpls
// label
{"adj:4",
createAdjValue(serializer, "4", 1, {}, false, 4)} // Disconnected
// node
};
{"adj:3", createAdjValue(serializer, "3", 1, {adj31}, false, 3)},
// Disconnected node
{"adj:4", createAdjValue(serializer, "4", 1, {}, false, 4)}};
pubKvs.emplace(createPrefixKeyValue("1", 1, addr1));
pubKvs.emplace(createPrefixKeyValue("1", 1, addr1V4));

Expand All @@ -2621,7 +2591,6 @@ TEST_F(DecisionTestFixture, Counters) {

pubKvs.emplace(createPrefixKeyValue("3", 1, addr3));
pubKvs.emplace(createPrefixKeyValue("3", 1, bgpPrefixEntry1));
pubKvs.emplace(createPrefixKeyValue("3", 1, bgpPrefixEntry3));

pubKvs.emplace(createPrefixKeyValue("4", 1, addr4));
pubKvs.emplace(createPrefixKeyValue("4", 1, bgpPrefixEntry2));
Expand All @@ -2643,9 +2612,7 @@ TEST_F(DecisionTestFixture, Counters) {
EXPECT_EQ(counters.at("decision.num_complete_adjacencies"), 2);
EXPECT_EQ(counters.at("decision.num_nodes"), 4);
EXPECT_EQ(counters.at("decision.num_prefixes"), 8);
EXPECT_EQ(counters.at("decision.no_route_to_prefix.count.60"), 1);
EXPECT_EQ(counters.at("decision.skipped_mpls_route.count.60"), 1);
EXPECT_EQ(counters.at("decision.no_route_to_label.count.60"), 1);
EXPECT_EQ(counters.at("decision.no_route_to_prefix.count.60"), 2);

// fully disconnect node 2
auto publication1 = createThriftPublication(
Expand Down Expand Up @@ -2910,9 +2877,6 @@ TEST_F(InitialRibBuildTestFixture, PrefixWithVipRoutes) {

// Static config originated route and static VIP route.
EXPECT_EQ(2, routeDbDelta.unicastRoutesToUpdate.size());
// Node label routes for the node itself (1).
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.size());
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.count(1));

// Send adj publication.
// Updated adjacency for peer "2" is received,
Expand All @@ -2929,9 +2893,6 @@ TEST_F(InitialRibBuildTestFixture, PrefixWithVipRoutes) {
EXPECT_EQ(
routeDbDelta.unicastRoutesToUpdate.begin()->second.prefix,
toIPNetwork(addr1));
// Node label route for node 2.
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.size());
EXPECT_EQ(1, routeDbDelta.mplsRoutesToUpdate.count(2));

evb.stop();
});
Expand Down
Loading

0 comments on commit 315b7e4

Please sign in to comment.