Delete Separa

Summary: We have conclusively determined that Separa as designed is not a reliable solution for the problem it was trying to solve. In particular, it behaved poorly in cases where the backhaul connectivity was flaky. We are moving forward with OWMP/"a12s", so delete the old code. Reviewed By: ammubhave Differential Revision: D15911067 fbshipit-source-id: f43e956333a2dea3d53490dacaf2f97c95b9efe7
facebook · Jun 20, 2019 · ca9b073 · ca9b073
1 parent 6ebb032
commit ca9b073
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 757 deletions.
diff --git a/fbmeshd.cmake b/fbmeshd.cmake
@@ -82,7 +82,6 @@ add_executable(fbmeshd
     openr/fbmeshd/routing/Routing.cpp
     openr/fbmeshd/routing/SyncRoutes80211s.cpp
     openr/fbmeshd/routing/UDPRoutingPacketTransport.cpp
-    openr/fbmeshd/separa/Separa.cpp
     $<TARGET_OBJECTS:fbmeshd-cpp2-obj>
 )
 

diff --git a/openr/fbmeshd/main.cpp b/openr/fbmeshd/main.cpp
@@ -39,7 +39,6 @@
 #include <openr/fbmeshd/routing/Routing.h>
 #include <openr/fbmeshd/routing/SyncRoutes80211s.h>
 #include <openr/fbmeshd/routing/UDPRoutingPacketTransport.h>
-#include <openr/fbmeshd/separa/Separa.h>
 #include <openr/watchdog/Watchdog.h>
 
 using namespace openr::fbmeshd;
@@ -86,26 +85,27 @@ DEFINE_bool(
 DEFINE_bool(
     enable_separa,
     false,
-    "If set, Separa algorithm will be enabled to manage mesh partitions. "
-    "Implies enable_separa_broadcasts=true");
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 DEFINE_int32(
-    separa_hello_port, 6667, "The port used to send separa hello packets");
+    separa_hello_port,
+    6667,
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 DEFINE_int32(
     separa_broadcast_interval_s,
     1,
-    "how often to send separa broadcasts in seconds");
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 DEFINE_int32(
     separa_domain_lockdown_period_s,
     60,
-    "how long to lockdown domains in seconds");
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 DEFINE_double(
     separa_domain_change_threshold_factor,
     1,
-    "threshold factor for doing a separa domain change");
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 DEFINE_bool(
     enable_separa_broadcasts,
     true,
-    "If set, Separa broadcasts will be enabled");
+    "DEPRECATED; TODO: delete after Separa is disabled on all meshes");
 
 DEFINE_int32(
     decision_rep_port,
@@ -358,7 +358,6 @@ main(int argc, char* argv[]) {
         FLAGS_mesh_ifname,
         kvStoreLocalCmdUrl,
         kvStoreLocalPubUrl,
-        FLAGS_enable_separa,
         zmqContext);
   }
 
@@ -389,36 +388,6 @@ main(int argc, char* argv[]) {
                             !(FLAGS_enable_event_based_peer_selector &&
                               FLAGS_enable_userspace_mesh_peering)};
 
-  std::unique_ptr<Separa> separa;
-  if (meshSpark != nullptr && FLAGS_is_openr_enabled &&
-      (FLAGS_enable_separa_broadcasts || FLAGS_enable_separa)) {
-    separa = std::make_unique<Separa>(
-        FLAGS_separa_hello_port,
-        std::chrono::seconds{FLAGS_separa_broadcast_interval_s},
-        std::chrono::seconds{FLAGS_separa_domain_lockdown_period_s},
-        FLAGS_separa_domain_change_threshold_factor,
-        !FLAGS_enable_separa,
-        nlHandler,
-        *meshSpark,
-        prefixManagerLocalCmdUrl,
-        decisionCmdUrl,
-        kvStoreLocalCmdUrl,
-        kvStoreLocalPubUrl,
-        monitorSubmitUrl,
-        zmqContext);
-
-    static constexpr auto separaId{"Separa"};
-    monitorEventLoopWithWatchdog(separa.get(), separaId, watchdog.get());
-
-    allThreads.emplace_back(std::thread([&separa]() noexcept {
-      LOG(INFO) << "Starting the Separa thread...";
-      folly::setThreadName(separaId);
-      separa->run();
-      LOG(INFO) << "Separa thread got stopped.";
-    }));
-    separa->waitUntilRunning();
-  }
-
   std::unique_ptr<Gateway11sRootRouteProgrammer> gateway11sRootRouteProgrammer;
   static constexpr auto gateway11sRootRouteProgrammerId{
       "Gateway11sRootRouteProgrammer"};
@@ -565,11 +534,6 @@ main(int argc, char* argv[]) {
   gatewayConnectivityMonitor.stop();
   gatewayConnectivityMonitor.waitUntilStopped();
 
-  if (separa) {
-    separa->stop();
-    separa->waitUntilStopped();
-  }
-
   if (routingEventLoop) {
     routing->resetSendPacketCallback();
     routingEventLoop->terminateLoopSoon();

diff --git a/openr/fbmeshd/mesh-spark/MeshSpark.cpp b/openr/fbmeshd/mesh-spark/MeshSpark.cpp
@@ -47,7 +47,6 @@ MeshSpark::MeshSpark(
     const std::string& ifName,
     const openr::KvStoreLocalCmdUrl& kvStoreLocalCmdUrl,
     const openr::KvStoreLocalPubUrl& kvStoreLocalPubUrl,
-    bool enableDomains,
     fbzmq::Context& zmqContext)
     : zmqLoop_{zmqLoop},
       zmqContext_{zmqContext},
@@ -66,8 +65,7 @@ MeshSpark::MeshSpark(
           &zmqLoop,
           "node1", /* nodeId is used for writing to kvstore. not used herei */
           kvStoreLocalCmdUrl,
-          kvStoreLocalPubUrl),
-      enableDomains_{enableDomains} {
+          kvStoreLocalPubUrl) {
   syncPeersTimer_ = fbzmq::ZmqTimeout::make(
       &zmqLoop_, [this]() mutable noexcept { syncPeers(); });
   syncPeersTimer_->scheduleTimeout(syncPeersInterval_, true);
@@ -246,45 +244,6 @@ MeshSpark::filterWhiteListedPeers(std::vector<folly::MacAddress>& peers) {
   peers = whiteListedPeers;
 }
 
-void
-MeshSpark::filterInDomainPeers(std::vector<folly::MacAddress>& peers) {
-  if (!enableDomains_) {
-    return;
-  }
-  std::vector<folly::MacAddress> inDomainPeers;
-  const auto myDomain = *myDomain_.rlock();
-  neighborDomainCache_.withWLock([&peers, &inDomainPeers, myDomain](
-                                     auto& neighborDomainCache_) {
-    // Cleanup expired peer info in the cache
-    std::unordered_set<folly::MacAddress> peerSet(peers.begin(), peers.end());
-    std::unordered_set<folly::MacAddress> keysInCache;
-    for (const auto& it : neighborDomainCache_) {
-      keysInCache.emplace(it.first);
-    }
-    for (const auto& key : keysInCache) {
-      if (peerSet.count(key) == 0) {
-        neighborDomainCache_.erase(key);
-      }
-    }
-
-    // filter out peers that don't belong to our domain
-    std::copy_if(
-        peers.begin(),
-        peers.end(),
-        std::back_inserter(inDomainPeers),
-        [&neighborDomainCache_, myDomain](const auto& peer) {
-          if (neighborDomainCache_.count(peer) == 0) {
-            return false;
-          }
-          const auto peerDomainAndEnabled = neighborDomainCache_.at(peer);
-          return !peerDomainAndEnabled.second ||
-              (peerDomainAndEnabled.first.hasValue() && myDomain.hasValue() &&
-               peerDomainAndEnabled.first == myDomain);
-        });
-  });
-  peers = inDomainPeers;
-}
-
 void
 MeshSpark::syncPeers() {
   VLOG(1) << folly::sformat("MeshSpark::{}()", __func__);
@@ -299,9 +258,6 @@ MeshSpark::syncPeers() {
     }
   }
 
-  // remove peers that are not in this node's domain
-  filterInDomainPeers(activePeers);
-
   // remove peers that are not white-listed
   filterWhiteListedPeers(activePeers);
 
@@ -352,43 +308,3 @@ MeshSpark::syncPeers() {
     }
   }
 }
-
-folly::Optional<folly::MacAddress>
-MeshSpark::getDomain() {
-  return *myDomain_.rlock();
-}
-
-void
-MeshSpark::setDomain(folly::Optional<folly::MacAddress> newDomain) {
-  myDomain_.withWLock([this, newDomain](auto& myDomain_) {
-    myDomain_ = newDomain;
-    zmqLoop_.runImmediatelyOrInEventLoop([this]() { syncPeers(); });
-  });
-}
-
-void
-MeshSpark::updateCache(
-    folly::MacAddress node,
-    std::pair<folly::Optional<folly::MacAddress>, bool> domain) {
-  std::pair<folly::Optional<folly::MacAddress>, bool> oldDomain;
-  neighborDomainCache_.withWLock(
-      [domain, node, &oldDomain](auto& neighborDomainCache_) {
-        auto it = neighborDomainCache_.find(node);
-        if (it != neighborDomainCache_.end()) {
-          oldDomain = it->second;
-          it->second = domain;
-        } else {
-          neighborDomainCache_[node] = domain;
-        }
-      });
-  if (!domain.second) {
-    return;
-  }
-  myDomain_.withRLock([this, oldDomain, domain](auto& myDomain_) {
-    if (myDomain_.hasValue() &&
-        (domain.first == myDomain_ || oldDomain.first == myDomain_) &&
-        domain.first != oldDomain.first) {
-      zmqLoop_.runImmediatelyOrInEventLoop([this]() { syncPeers(); });
-    }
-  });
-}
diff --git a/openr/fbmeshd/mesh-spark/MeshSpark.h b/openr/fbmeshd/mesh-spark/MeshSpark.h
@@ -34,17 +34,8 @@ class MeshSpark final {
       const std::string& ifName,
       const openr::KvStoreLocalCmdUrl& kvStoreLocalCmdUrl,
       const openr::KvStoreLocalPubUrl& kvStoreLocalPubUrl,
-      bool enableDomains,
       fbzmq::Context& zmqContext);
 
-  folly::Optional<folly::MacAddress> getDomain();
-
-  void setDomain(folly::Optional<folly::MacAddress> newDomain);
-
-  void updateCache(
-      folly::MacAddress node,
-      std::pair<folly::Optional<folly::MacAddress>, bool> domain);
-
  private:
   /**
    * bind/connect to openr sockets
@@ -84,8 +75,6 @@ class MeshSpark final {
 
   void filterWhiteListedPeers(std::vector<folly::MacAddress>& peers);
 
-  void filterInDomainPeers(std::vector<folly::MacAddress>& peers);
-
   // ZmqEventLoop pointer for scheduling async events and socket callback
   // registration
   fbzmq::ZmqEventLoop& zmqLoop_;
@@ -124,16 +113,4 @@ class MeshSpark final {
   // node name -> ipv4 address
   std::unordered_map<folly::MacAddress, folly::IPAddressV4> kvStoreIPs_;
 
-  // enable domain filtering?
-  const bool enableDomains_;
-
-  // Stores the current domain, nodes in the same domain can for OpenR peerings
-  folly::Synchronized<folly::Optional<folly::MacAddress>> myDomain_;
-
-  // Cache for checking which nodes belong to which domain
-  folly::Synchronized<std::unordered_map<
-      folly::MacAddress,
-      std::pair<folly::Optional<folly::MacAddress>, bool>>>
-      neighborDomainCache_;
-
 }; // MeshSpark