(red-diff) Cleanup enableSegmentRouting flag and corresponding logic

Summary: As titled. There are a bunch of stale code inside Open/R with the segment routing support use cases, which has no longer active in any of the production flavor. Deprecate this for code simplicity. More cleanup is coming to remove the footprint of segment routing use cases inside SpfSolver Reviewed By: shih-hao-tseng Differential Revision: D64424576 fbshipit-source-id: 3c883b88848f44d95f09acfdd69fbf1f846763b4
facebook · Oct 17, 2024 · 16d0fc5 · 16d0fc5
1 parent 9476109
commit 16d0fc5
Showing 8 changed files with 24 additions and 102 deletions.
diff --git a/openr/config/Config.h b/openr/config/Config.h
@@ -111,11 +111,6 @@ class Config {
     return config_.enable_v4().value_or(false);
   }
 
-  bool
-  isSegmentRoutingEnabled() const {
-    return config_.enable_segment_routing().value_or(false);
-  }
-
   bool
   isNetlinkFibHandlerEnabled() const {
     return config_.enable_netlink_fib_handler().value_or(false);

diff --git a/openr/config/tests/ConfigTest.cpp b/openr/config/tests/ConfigTest.cpp
@@ -461,8 +461,6 @@ TEST(ConfigTest, GeneralGetter) {
 
     // enable_v4
     EXPECT_TRUE(config.isV4Enabled());
-    // enable_segment_routing
-    EXPECT_FALSE(config.isSegmentRoutingEnabled());
     // enable_best_route_selection
     EXPECT_FALSE(config.isBestRouteSelectionEnabled());
     // enable_v4_over_v6_nexthop

diff --git a/openr/decision/SpfSolver.cpp b/openr/decision/SpfSolver.cpp
@@ -37,21 +37,6 @@ DecisionRouteDb::calculateUpdate(DecisionRouteDb&& newDb) const {
       delta.unicastRoutesToDelete.emplace_back(prefix);
     }
   }
-
-  // mplsRoutesToUpdate
-  for (auto& [label, entry] : newDb.mplsRoutes) {
-    const auto& search = mplsRoutes.find(label);
-    if (search == mplsRoutes.end() || search->second != entry) {
-      delta.addMplsRouteToUpdate(std::move(entry));
-    }
-  }
-
-  // mplsRoutesToDelete
-  for (auto const& [label, _] : mplsRoutes) {
-    if (!newDb.mplsRoutes.count(label)) {
-      delta.mplsRoutesToDelete.emplace_back(label);
-    }
-  }
   return delta;
 }
 
@@ -63,12 +48,6 @@ DecisionRouteDb::update(DecisionRouteUpdate const& update) {
   for (auto const& [_, entry] : update.unicastRoutesToUpdate) {
     unicastRoutes.insert_or_assign(entry.prefix, entry);
   }
-  for (auto const& label : update.mplsRoutesToDelete) {
-    mplsRoutes.erase(label);
-  }
-  for (auto const& [_, entry] : update.mplsRoutesToUpdate) {
-    mplsRoutes.insert_or_assign(entry.label, entry);
-  }
 }
 
 SpfSolver::SpfSolver(
@@ -479,7 +458,6 @@ SpfSolver::selectBestPathsSpf(
       routeSelectionResult.allNodeAreas,
       prefix.first.isV4(), /* isV4Prefix */
       nextHopsWithMetric,
-      std::nullopt /* swapLabel */,
       area,
       linkState);
 
@@ -587,7 +565,6 @@ SpfSolver::getNextHopsThrift(
     const std::set<NodeAndArea>& dstNodeAreas,
     bool isV4,
     const BestNextHopMetrics& bestNextHopMetrics,
-    std::optional<int32_t> swapLabel,
     const std::string& area,
     const LinkState& linkState) const {
   // Use reference to avoid copy
@@ -636,23 +613,12 @@ SpfSolver::getNextHopsThrift(
       continue;
     }
 
-    // Create associated mpls action if swapLabel is provided
-    std::optional<thrift::MplsAction> mplsAction;
-    if (swapLabel.has_value()) {
-      CHECK(not mplsAction.has_value());
-      bool isNextHopAlsoDst = dstNodeAreas.count({neighborNode, area});
-      mplsAction = createMplsAction(
-          isNextHopAlsoDst ? thrift::MplsActionCode::PHP
-                           : thrift::MplsActionCode::SWAP,
-          isNextHopAlsoDst ? std::nullopt : swapLabel);
-    }
-
     nextHops.emplace(createNextHop(
         isV4 and not v4OverV6Nexthop_ ? link->getNhV4FromNode(myNodeName)
                                       : link->getNhV6FromNode(myNodeName),
         link->getIfaceFromNode(myNodeName),
         distOverLink,
-        mplsAction,
+        std::nullopt, /* mplsAction */
         link->getArea(),
         link->getOtherNodeName(myNodeName),
         0 /* ucmp weight */));

diff --git a/openr/decision/SpfSolver.h b/openr/decision/SpfSolver.h
@@ -19,7 +19,6 @@
 
 namespace openr {
 
-using StaticMplsRoutes = std::unordered_map<int32_t, RibMplsEntry>;
 using StaticUnicastRoutes =
     std::unordered_map<folly::CIDRNetwork, RibUnicastEntry>;
 using Metric = openr::LinkStateMetric;
@@ -35,13 +34,20 @@ using BestNextHopMetrics = std::pair<
  * - All selected entries `list<pair<Node, Area>>`
  */
 struct RouteSelectionResult {
-  // Representing the selected set of `<Node, Area>`.
-  // NOTE: Using `std::set` helps ensuring uniqueness and ease code for electing
-  // unique `<Node, Area>` in some-cases.
+  /*
+   * Representing the selected set of `<Node, Area>`.
+   *
+   * NOTE:
+   * It is intentional to use `std::set` to preserve the order of node + area
+   * combination. This ensures the uniqueness and ease code for electing
+   * the unique `<Node, Area>` when tie-breaking is needed.
+   */
   std::set<NodeAndArea> allNodeAreas;
 
-  // The `pair<Node, Area>` with best metrics. This should be used for
-  // redistribution across areas.
+  /*
+   * The `pair<Node, Area>` with best metrics.
+   * This will be used for redistribution across areas.
+   */
   NodeAndArea bestNodeArea;
 
   /*
@@ -69,11 +75,13 @@ class DecisionRouteDb {
  public:
   std::unordered_map<folly::CIDRNetwork /* prefix */, RibUnicastEntry>
       unicastRoutes;
-  std::unordered_map<int32_t /* label */, RibMplsEntry> mplsRoutes;
 
-  // calculate the delta between this and newDb. Note, this method is const;
-  // We are not actually updating here. We may mutate the DecisionRouteUpdate in
-  // some way before calling update with it
+  /*
+   * Function to calculate the delta between this and newDb.
+   *
+   * NOTE: this method is const. We are not actually updating here.
+   * We may mutate the `DecisionRouteUpdate` in some way before calling update.
+   */
   DecisionRouteUpdate calculateUpdate(DecisionRouteDb&& newDb) const;
 
   // update the state of this with the DecisionRouteUpdate passed
@@ -86,10 +94,6 @@ class DecisionRouteDb {
     for (const auto& [_, entry] : unicastRoutes) {
       tRouteDb.unicastRoutes()->emplace_back(entry.toThrift());
     }
-    // mpls routes
-    for (const auto& [_, entry] : mplsRoutes) {
-      tRouteDb.mplsRoutes()->emplace_back(entry.toThrift());
-    }
     return tRouteDb;
   }
 
@@ -99,13 +103,6 @@ class DecisionRouteDb {
     auto key = entry.prefix;
     CHECK(unicastRoutes.emplace(key, std::move(entry)).second);
   }
-
-  // Assertion: no route for this label may already exist in the db
-  void
-  addMplsRoute(RibMplsEntry&& entry) {
-    auto key = entry.label;
-    CHECK(mplsRoutes.emplace(key, std::move(entry)).second);
-  }
 };
 
 // The class to compute shortest-paths using Dijkstra algorithm
@@ -123,7 +120,6 @@ class SpfSolver {
   //
   // util function to update IP static route
   //
-
   void updateStaticUnicastRoutes(
       const std::unordered_map<folly::CIDRNetwork, RibUnicastEntry>&
           unicastRoutesToUpdate,
@@ -236,22 +232,19 @@ class SpfSolver {
       const std::set<NodeAndArea>& dstNodeAreas,
       const LinkState& linkState);
 
-  // This function converts best nexthop nodes to best nexthop adjacencies
-  // which can then be passed to FIB for programming. It considers and
-  // parallel link logic (tested by our UT)
-  // If swap label is provided then it will be used to associate SWAP or PHP
-  // mpls action
+  /*
+   * This function converts best nexthop nodes to best nexthop adjacencies
+   * which can then be passed to FIB for programming.
+   */
   std::unordered_set<thrift::NextHopThrift> getNextHopsThrift(
       const std::string& myNodeName,
       const std::set<NodeAndArea>& dstNodeAreas,
       bool isV4,
       const BestNextHopMetrics& bestNextHopMetrics,
-      std::optional<int32_t> swapLabel,
       const std::string& area,
       const LinkState& linkState) const;
 
-  // Collection to store static IP/MPLS routes
-  StaticMplsRoutes staticMplsRoutes_;
+  // Collection to store static IP UNICAST routes
   StaticUnicastRoutes staticUnicastRoutes_;
 
   // Cache of best route selection.
@@ -265,8 +258,6 @@ class SpfSolver {
   // nexthops to Fib module for programming. Else it will just drop them.
   const bool enableV4_{false};
 
-  const bool enableNodeSegmentLabel_{true};
-
   const bool enableBestRouteSelection_{false};
 
   // is v4 over v6 nexthop enabled. If yes then Decision will forward v4

diff --git a/openr/decision/tests/DecisionTestUtils.cpp b/openr/decision/tests/DecisionTestUtils.cpp
@@ -88,14 +88,6 @@ fillRouteMap(
       routeMap[make_pair(node, prefix)].emplace(nextHop);
     }
   }
-  for (auto const& [_, entry] : routeDb.mplsRoutes) {
-    auto topLabelStr = std::to_string(entry.label);
-    for (const auto& nextHop : entry.nexthops) {
-      VLOG(4) << "node: " << node << " label: " << topLabelStr << " -> "
-              << toString(nextHop);
-      routeMap[make_pair(node, topLabelStr)].emplace(nextHop);
-    }
-  }
 }
 
 void
@@ -112,14 +104,6 @@ fillRouteMap(
       routeMap[make_pair(node, prefix)].emplace(nextHop);
     }
   }
-  for (auto const& route : *routeDb.mplsRoutes()) {
-    auto topLabelStr = std::to_string(*route.topLabel());
-    for (const auto& nextHop : *route.nextHops()) {
-      VLOG(4) << "node: " << node << " label: " << topLabelStr << " -> "
-              << toString(nextHop);
-      routeMap[make_pair(node, topLabelStr)].emplace(nextHop);
-    }
-  }
 }
 
 } // namespace openr
diff --git a/openr/decision/tests/SpfSolverTest.cpp b/openr/decision/tests/SpfSolverTest.cpp
@@ -132,7 +132,6 @@ TEST(ShortestPathTest, UnreachableNodes) {
     auto routeDb = spfSolver.buildRouteDb(node, areaLinkStates, prefixState);
     ASSERT_TRUE(routeDb.has_value());
     EXPECT_EQ(0, routeDb->unicastRoutes.size());
-    EXPECT_EQ(0, routeDb->mplsRoutes.size()); // No label routes
   }
 }
 
@@ -288,7 +287,6 @@ TEST(ShortestPathTest, MissingNeighborAdjacencyDb) {
   auto routeDb = spfSolver.buildRouteDb("1", areaLinkStates, prefixState);
   ASSERT_TRUE(routeDb.has_value());
   EXPECT_EQ(0, routeDb->unicastRoutes.size());
-  EXPECT_EQ(0, routeDb->mplsRoutes.size());
 }
 
 //

diff --git a/openr/link-monitor/LinkMonitor.cpp b/openr/link-monitor/LinkMonitor.cpp
@@ -116,7 +116,6 @@ LinkMonitor::LinkMonitor(
       enableLinkStatusMeasurement_(
           *config->getLinkMonitorConfig().enable_link_status_measurement()),
       enableV4_(config->isV4Enabled()),
-      enableSegmentRouting_(config->isSegmentRoutingEnabled()),
       prefixForwardingType_(*config->getConfig().prefix_forwarding_type()),
       prefixForwardingAlgorithm_(
           *config->getConfig().prefix_forwarding_algorithm()),
@@ -999,13 +998,6 @@ LinkMonitor::buildAdjacencyDatabase(const std::string& area) {
   adjDb.area() = area;
   adjDb.nodeLabel() = 0;
 
-  if (enableSegmentRouting_) {
-    auto it = state_.nodeLabelMap()->find(area);
-    if (it != state_.nodeLabelMap()->end()) {
-      adjDb.nodeLabel() = it->second;
-    }
-  }
-
   // [Hard-Drain] set node overload bit
   adjDb.isOverloaded() = *state_.isOverloaded();
 

diff --git a/openr/link-monitor/LinkMonitor.h b/openr/link-monitor/LinkMonitor.h
@@ -328,8 +328,6 @@ class LinkMonitor final : public OpenrEventBase {
   const bool enableLinkStatusMeasurement_{false};
   // enable v4
   bool enableV4_{false};
-  // [TO_BE_DEPRECATED] enable segment routing
-  bool enableSegmentRouting_{false};
   // [TO_BE_DEPRECATED] prefix forwarding type and algorithm
   thrift::PrefixForwardingType prefixForwardingType_;
   thrift::PrefixForwardingAlgorithm prefixForwardingAlgorithm_;