Deprecate PrefixForwardingType::SR_MPLS and PrefixForwardingAlgorithm…

…::KSP2_ED_ECMP support Summary: Remove the `openr`'s support of `PrefixForwardingType::SR_MPLS` and `PrefixForwardingAlgorithm::KSP2_ED_ECMP` pair. Update the unit `decision/tests` to NOT test these `SR_MPLS` and `KSP2_ED_ECMP` capabilities. Nevertheless, we CANNOT deprecate these 2 enums from the `OpenrConfig.thrift` since `neteng::config::routing_policy` is still using it. https://www.internalfb.com/code/fbsource/[9f99519a58d621e8baa3c08e7f1ee859223a73e0]/fbcode/openr/prefix-manager/facebook/policy/PolicyUtils.cpp?lines=16-17%2C29-30 Reviewed By: xiangxu1121 Differential Revision: D61170657 fbshipit-source-id: 8ef82fe7df33ef8879e620073627cd07e9600fa6
facebook · Aug 13, 2024 · fa73312 · fa73312
1 parent a619bd5
commit fa73312
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 137 deletions.
diff --git a/openr/decision/tests/DecisionBenchmark.cpp b/openr/decision/tests/DecisionBenchmark.cpp
@@ -46,13 +46,6 @@ BENCHMARK_COUNTERS_PARAM(BM_DecisionGridAdjUpdates, counters, 1000, SP_ECMP, 1);
 BENCHMARK_COUNTERS_PARAM(
     BM_DecisionGridAdjUpdates, counters, 10000, SP_ECMP, 1);
 
-BENCHMARK_COUNTERS_PARAM(
-    BM_DecisionGridAdjUpdates, counters, 10, KSP2_ED_ECMP, 1);
-BENCHMARK_COUNTERS_PARAM(
-    BM_DecisionGridAdjUpdates, counters, 100, KSP2_ED_ECMP, 1);
-BENCHMARK_COUNTERS_PARAM(
-    BM_DecisionGridAdjUpdates, counters, 1000, KSP2_ED_ECMP, 1);
-
 /*
  * BM_DecisionGridPrefixUpdates:
  * @first param - integer: num of nodes in a grid topology
@@ -137,78 +130,6 @@ BENCHMARK_COUNTERS_PARAM2(
     1000,
     1000);
 
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10_10,
-    100,
-    KSP2_ED_ECMP,
-    10,
-    10);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_100_100,
-    100,
-    KSP2_ED_ECMP,
-    100,
-    100);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_1k_1k,
-    100,
-    KSP2_ED_ECMP,
-    1000,
-    1000);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10k_1,
-    100,
-    KSP2_ED_ECMP,
-    10000,
-    1);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10k_10,
-    100,
-    KSP2_ED_ECMP,
-    10000,
-    10);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10k_100,
-    100,
-    KSP2_ED_ECMP,
-    10000,
-    100);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10k_1k,
-    100,
-    KSP2_ED_ECMP,
-    10000,
-    1000);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_100_10k_10k,
-    100,
-    KSP2_ED_ECMP,
-    10000,
-    10000);
-BENCHMARK_COUNTERS_PARAM2(
-    BM_DecisionGridPrefixUpdates,
-    counters,
-    KSP2_ED_ECMP_1000_1k_1k,
-    1000,
-    KSP2_ED_ECMP,
-    1000,
-    1000);
 /*
  * BM_DecisionFabricInitialUpdate:
  * @first param - integer: num of pods in a fabric topology
@@ -296,10 +217,9 @@ BENCHMARK_COUNTERS_PARAM2(
  * i.e. How long does it take to incremental change routes given
  * `numOfUpdatePrefixes` per node
  *
- * total # of sws = numOfPlanes * kNumOfSswsPerPlane + numOfPods * numOfPlanes +
- *                 numOfPods * kNumOfRswsPerPod
- *                = 36 * numOfPlanes + numOfPods * numOfPlanes +
- *                 48 * kNumOfRswsPerPod
+ * total # of sws = numOfPlanes * kNumOfSswsPerPlane + numOfPods *
+ * numOfPlanes + numOfPods * kNumOfRswsPerPod = 36 * numOfPlanes + numOfPods
+ * * numOfPlanes + 48 * kNumOfRswsPerPod
  */
 // total = 85
 BENCHMARK_COUNTERS_PARAM3(

diff --git a/openr/decision/tests/RoutingBenchmarkUtils.cpp b/openr/decision/tests/RoutingBenchmarkUtils.cpp
@@ -252,10 +252,7 @@ createGridAdjacencys(const int row, const int col, const uint32_t n) {
 std::pair<
     std::unordered_map<std::string, thrift::AdjacencyDatabase>,
     std::unordered_map<std::string, thrift::PrefixDatabase>>
-createGrid(
-    const int n,
-    const int numPrefixes,
-    thrift::PrefixForwardingAlgorithm forwardingAlgorithm) {
+createGrid(const int n, const int numPrefixes) {
   LOG(INFO) << "grid: " << n << " by " << n;
   LOG(INFO) << " number of prefixes " << numPrefixes;
   std::unordered_map<std::string, thrift::AdjacencyDatabase> adjDbs;
@@ -284,11 +281,8 @@ createGrid(
                 prefix,
                 thrift::PrefixType::LOOPBACK,
                 "",
-                thrift::PrefixForwardingAlgorithm::KSP2_ED_ECMP ==
-                        forwardingAlgorithm
-                    ? thrift::PrefixForwardingType::SR_MPLS
-                    : thrift::PrefixForwardingType::IP,
-                forwardingAlgorithm));
+                thrift::PrefixForwardingType::IP,
+                thrift::PrefixForwardingAlgorithm::SP_ECMP));
         prefixDbs.emplace(key.getPrefixKeyV2(), std::move(db));
       }
     }
@@ -558,7 +552,6 @@ updateRandomGridPrefixes(
     const std::shared_ptr<DecisionWrapper>& decisionWrapper,
     const int n,
     const int numOfUpdatePrefixes,
-    thrift::PrefixForwardingAlgorithm forwardingAlgorithm,
     folly::BenchmarkSuspender& suspender) {
   PrefixGenerator prefixGenerator;
   apache::thrift::CompactSerializer serializer;
@@ -572,12 +565,9 @@ updateRandomGridPrefixes(
       auto prefixEntries =
           generatePrefixEntries(prefixGenerator, numOfUpdatePrefixes);
       for (auto& prefixEntry : prefixEntries) {
-        prefixEntry.forwardingType() =
-            (thrift::PrefixForwardingAlgorithm::KSP2_ED_ECMP ==
-                     forwardingAlgorithm
-                 ? thrift::PrefixForwardingType::SR_MPLS
-                 : thrift::PrefixForwardingType::IP);
-        prefixEntry.forwardingAlgorithm() = forwardingAlgorithm;
+        prefixEntry.forwardingType() = thrift::PrefixForwardingType::IP;
+        prefixEntry.forwardingAlgorithm() =
+            thrift::PrefixForwardingAlgorithm::SP_ECMP;
         auto keyDbPair = createPrefixKeyAndDb(nodeName, prefixEntry);
         keyVals.emplace(
             keyDbPair.first.getPrefixKeyV2(),
@@ -601,7 +591,6 @@ generatePrefixUpdatePublication(
     const uint32_t& numOfPrefixes,
     const std::unordered_map<std::string, std::vector<std::string>>&
         listOfNodenames,
-    const thrift::PrefixForwardingAlgorithm& forwardingAlgorithm,
     thrift::Publication& initialPub) {
   PrefixGenerator prefixGenerator;
   apache::thrift::CompactSerializer serializer;
@@ -611,12 +600,9 @@ generatePrefixUpdatePublication(
       auto prefixEntries =
           generatePrefixEntries(prefixGenerator, numOfPrefixes);
       for (auto& prefixEntry : prefixEntries) {
-        prefixEntry.forwardingType() =
-            (thrift::PrefixForwardingAlgorithm::KSP2_ED_ECMP ==
-                     forwardingAlgorithm
-                 ? thrift::PrefixForwardingType::SR_MPLS
-                 : thrift::PrefixForwardingType::IP);
-        prefixEntry.forwardingAlgorithm() = forwardingAlgorithm;
+        prefixEntry.forwardingType() = thrift::PrefixForwardingType::IP;
+        prefixEntry.forwardingAlgorithm() =
+            thrift::PrefixForwardingAlgorithm::SP_ECMP;
         auto keyDbPair = createPrefixKeyAndDb(nodeName, prefixEntry);
         keyVals.emplace(
             keyDbPair.first.getPrefixKeyV2(),
@@ -645,8 +631,7 @@ BM_DecisionGridInitialUpdate(
   for (uint32_t i = 0; i < iters; i++) {
     const std::string nodeName{"1"};
     int n = std::sqrt(numOfSws);
-    auto [adjs, prefixes] =
-        createGrid(n, numberOfPrefixes, forwardingAlgorithm);
+    auto [adjs, prefixes] = createGrid(n, numberOfPrefixes);
 
     if (record) {
       auto mem = sysMetrics.getVirtualMemBytes();
@@ -684,7 +669,7 @@ BM_DecisionGridAdjUpdates(
   const std::string nodeName{"1"};
   auto decisionWrapper = std::make_shared<DecisionWrapper>(nodeName);
   int n = std::sqrt(numOfSws);
-  auto [adjs, prefixes] = createGrid(n, numberOfPrefixes, forwardingAlgorithm);
+  auto [adjs, prefixes] = createGrid(n, numberOfPrefixes);
 
   sendRecvInitialUpdate(
       decisionWrapper, nodeName, std::move(adjs), std::move(prefixes));
@@ -719,7 +704,7 @@ BM_DecisionGridPrefixUpdates(
     const std::string nodeName{"1"};
     auto decisionWrapper = std::make_shared<DecisionWrapper>(nodeName);
     int n = std::sqrt(numOfNodes);
-    auto [adjs, prefixes] = createGrid(n, numOfPrefixes, forwardingAlgorithm);
+    auto [adjs, prefixes] = createGrid(n, numOfPrefixes);
 
     sendRecvInitialUpdate(
         decisionWrapper, nodeName, std::move(adjs), std::move(prefixes));
@@ -733,11 +718,7 @@ BM_DecisionGridPrefixUpdates(
 
     // Advertise new random prefix from random node to build route
     updateRandomGridPrefixes(
-        decisionWrapper,
-        n,
-        numOfUpdatePrefixes,
-        forwardingAlgorithm,
-        suspender);
+        decisionWrapper, n, numOfUpdatePrefixes, suspender);
 
     if (record) {
       auto mem = sysMetrics.getVirtualMemBytes();
@@ -787,7 +768,7 @@ BM_DecisionFabricInitialUpdate(
         listOfNodenames);
 
     generatePrefixUpdatePublication(
-        numberOfPrefixes, listOfNodenames, forwardingAlgorithm, initialPub);
+        numberOfPrefixes, listOfNodenames, initialPub);
 
     suspender.dismiss(); // Start measuring benchmark time
     //
@@ -841,7 +822,7 @@ BM_DecisionFabricPrefixUpdates(
         listOfNodenames);
 
     generatePrefixUpdatePublication(
-        numberOfPrefixes, listOfNodenames, forwardingAlgorithm, initialPub);
+        numberOfPrefixes, listOfNodenames, initialPub);
 
     //
     // Publish initial link state info to KvStore, This should trigger the
@@ -864,8 +845,7 @@ BM_DecisionFabricPrefixUpdates(
 
     thrift::Publication pub;
     pub.area() = kTestingAreaName;
-    generatePrefixUpdatePublication(
-        numOfUpdatePrefixes, listOfNodenames, forwardingAlgorithm, pub);
+    generatePrefixUpdatePublication(numOfUpdatePrefixes, listOfNodenames, pub);
 
     suspender.dismiss(); // Start measuring benchmark time
 

diff --git a/openr/decision/tests/RoutingBenchmarkUtils.h b/openr/decision/tests/RoutingBenchmarkUtils.h
@@ -303,10 +303,7 @@ inline std::vector<thrift::Adjacency> createGridAdjacencys(
 std::pair<
     std::unordered_map<std::string, thrift::AdjacencyDatabase>,
     std::unordered_map<std::string, thrift::PrefixDatabase>>
-createGrid(
-    const int n,
-    const int numPrefixes,
-    thrift::PrefixForwardingAlgorithm forwardingAlgorithm);
+createGrid(const int n, const int numPrefixes);
 
 /**
  * Create Adjacencies for spine switches.
@@ -384,7 +381,6 @@ void updateRandomGridPrefixes(
     const std::shared_ptr<DecisionWrapper>& decisionWrapper,
     std::optional<std::pair<int, int>>& selectedNode,
     const int n,
-    thrift::PrefixForwardingAlgorithm forwardingAlgorithm,
     folly::BenchmarkSuspender& suspender);
 
 //
@@ -401,13 +397,11 @@ void generatePrefixUpdatePublication(
     const uint32_t& numOfPrefixes,
     const std::unordered_map<std::string, std::vector<std::string>>&
         listOfNodenames,
-    const thrift::PrefixForwardingAlgorithm& forwardingAlgorithm,
     thrift::Publication& initialPub);
 
 //
 // Benchmark tests for grid topology
 //
-
 void BM_DecisionGridInitialUpdate(
     folly::UserCounters& counters,
     uint32_t iters,
@@ -433,6 +427,7 @@ void BM_DecisionGridAdjUpdates(
 //
 // Benchmark test for fabric topology.
 //
+
 void BM_DecisionFabricInitialUpdate(
     folly::UserCounters& counters,
     uint32_t iters,
@@ -451,5 +446,4 @@ void BM_DecisionFabricPrefixUpdates(
     thrift::PrefixForwardingAlgorithm forwardingAlgorithm);
 
 const auto SP_ECMP = thrift::PrefixForwardingAlgorithm::SP_ECMP;
-const auto KSP2_ED_ECMP = thrift::PrefixForwardingAlgorithm::KSP2_ED_ECMP;
 } // namespace openr
diff --git a/openr/docs/Operator_Guide/Route_Representation.md b/openr/docs/Operator_Guide/Route_Representation.md
@@ -89,18 +89,12 @@ route selection and can't be modified by the policy.
 #### > `forwarding-algorithm`
 
 `non-transitive`, `mutable` Link-state algorithm for route computaion. Open/R
-supports two forwarding algorithm, `SP_ECMP` (Shortest Path ECMP) and
-`KSP2_ED_ECMP` (K-Shortest Path). The algorithm with the lowest value is chosen
-in-case of conflicting advertisements.
+supports one forwarding algorithm, `SP_ECMP` (Shortest Path ECMP).
 
 #### > `forwarding-type`
 
 `non-transitive`, `mutable` Data plane forwarding mechanism to use. Open/R
-supports two forwarding types, `IP` (Usual IP routing) and `SR_MPLS` (Source
-Routing with MPLS data plane). The type with the lowest value is chosen in-case
-of conflicting advertisements.
-
-> NOTE: `KSP2_ED_ECMP` is only compatible with `SR_MPLS` forwarding type.
+supports one forwarding type, `IP` (Usual IP routing).
 
 #### > `min-nexthops`
 

diff --git a/openr/tests/utils/Utils.cpp b/openr/tests/utils/Utils.cpp
@@ -176,8 +176,8 @@ generateDecisionRouteUpdateFromPrefixEntries(
     prefixEntry.metrics()->distance() = 1;
     prefixEntry.type() = thrift::PrefixType::DEFAULT;
     prefixEntry.forwardingAlgorithm() =
-        thrift::PrefixForwardingAlgorithm::KSP2_ED_ECMP;
-    prefixEntry.forwardingType() = thrift::PrefixForwardingType::SR_MPLS;
+        thrift::PrefixForwardingAlgorithm::SP_ECMP;
+    prefixEntry.forwardingType() = thrift::PrefixForwardingType::IP;
     prefixEntry.minNexthop() = 10;
 
     auto unicastRoute = RibUnicastEntry(