LinkMonitor - Remove static route advertisement feature

Summary:
Static route feature allows the well known routes to be advertised from within
Open/R. These routes are passed as CLI arguments. For all practical purposes we
never used this feature in production.

Removing this feature as we haven't been using it. The new PolicyEngine & Config
can do this in much better way if we need be.

Reviewed By: jstrizich, xiangxu1121

Differential Revision: D22541241

fbshipit-source-id: 1ee3752e12525d9b64f4ce80e7b1892bffee51ad

openr/Main.cpp

@@ -468,33 +468,6 @@ main(int argc, char** argv) {
           std::make_shared<IoProvider>(),
           config));
 
-  // Static list of prefixes to announce into the network as long as OpenR is
-  // running.
-  std::vector<openr::thrift::IpPrefix> networks;
-  try {
-    std::vector<std::string> prefixes;
-    folly::split(",", FLAGS_prefixes, prefixes, true /* ignore empty */);
-    for (auto const& prefix : prefixes) {
-      // Perform some sanity checks before announcing the list of prefixes
-      auto network = folly::IPAddress::createNetwork(prefix);
-      if (network.first.isLoopback()) {
-        LOG(FATAL) << "Default loopback addresses can't be announced "
-                   << prefix;
-      }
-      if (network.first.isLinkLocal()) {
-        LOG(FATAL) << "Link local addresses can't be announced " << prefix;
-      }
-      if (network.first.isMulticast()) {
-        LOG(FATAL) << "Multicast addresses can't be annouced " << prefix;
-      }
-      networks.emplace_back(toIpPrefix(network));
-    }
-  } catch (std::exception const& err) {
-    LOG(ERROR) << "Invalid Prefix string specified. Expeted comma separated "
-               << "list of IP/CIDR format, got '" << FLAGS_prefixes << "'";
-    return -1;
-  }
-
   // Create link monitor instance.
   auto linkMonitor = startEventBase(
       allThreads,
@@ -506,7 +479,6 @@ main(int argc, char** argv) {
           config,
           FLAGS_system_agent_port,
           kvStore,
-          networks,
           FLAGS_enable_perf_measurement,
           interfaceUpdatesQueue,
           peerUpdatesQueue,

openr/common/Flags.cpp

@@ -68,10 +68,6 @@ DEFINE_string(
 DEFINE_bool(
     dryrun, true, "Run the process in dryrun mode. No FIB programming!");
 DEFINE_string(loopback_iface, "lo", "The iface to configure with the prefix");
-DEFINE_string(
-    prefixes,
-    "",
-    "The prefix and loopback IP separated by comma for this node");
 DEFINE_string(
     seed_prefix,
     "",

openr/common/Flags.h

@@ -32,7 +32,6 @@ DECLARE_bool(assume_drained);
 DECLARE_string(node_name);
 DECLARE_bool(dryrun);
 DECLARE_string(loopback_iface);
-DECLARE_string(prefixes);
 DECLARE_string(seed_prefix);
 
 DECLARE_bool(enable_prefix_alloc);

openr/ctrl-server/tests/OpenrCtrlHandlerTest.cpp

@@ -138,7 +138,6 @@ class OpenrCtrlFixture : public ::testing::Test {
         config,
         systemThriftThread.getAddress()->getPort(),
         kvStoreWrapper->getKvStore(),
-        std::vector<thrift::IpPrefix>{},
         false /* enable perf measurement */,
         interfaceUpdatesQueue_,
         peerUpdatesQueue_,

openr/docs/Runbook.md

@@ -121,15 +121,6 @@ Network.
 DOMAIN=cluster10.dc3
 ```
 
-#### PREFIXES
-
-Static list of comma separate prefixes to announce from the current node. Can't
-be changed while running. Default value is empty
-
-```
-PREFIXES="face:cafe::1/128,face:b00c::/64"
-```
-
 #### LOOPBACK_IFACE
 
 Indicates loopback address to which auto elected prefix will be assigned if

openr/link-monitor/LinkMonitor.cpp

@@ -74,7 +74,6 @@ LinkMonitor::LinkMonitor(
     std::shared_ptr<const Config> config,
     int32_t platformThriftPort,
     KvStore* kvStore,
-    std::vector<thrift::IpPrefix> const& staticPrefixes,
     bool enablePerfMeasurement,
     messaging::ReplicateQueue<thrift::InterfaceDatabase>& intfUpdatesQueue,
     messaging::ReplicateQueue<thrift::PeerUpdateRequest>& peerUpdatesQueue,
@@ -87,7 +86,6 @@ LinkMonitor::LinkMonitor(
     std::chrono::seconds adjHoldTime)
     : nodeId_(config->getNodeName()),
       platformThriftPort_(platformThriftPort),
-      staticPrefixes_(staticPrefixes),
       enablePerfMeasurement_(enablePerfMeasurement),
       platformPubUrl_(platformPubUrl),
       enableV4_(config->isV4Enabled()),
@@ -747,17 +745,6 @@ LinkMonitor::advertiseRedistAddrs() {
   }
   std::vector<thrift::PrefixEntry> prefixes;
 
-  // Add static prefixes
-  for (auto const& prefix : staticPrefixes_) {
-    auto prefixEntry = openr::thrift::PrefixEntry();
-    prefixEntry.prefix = prefix;
-    prefixEntry.type = thrift::PrefixType::LOOPBACK;
-    prefixEntry.forwardingType = prefixForwardingType_;
-    prefixEntry.forwardingAlgorithm = prefixForwardingAlgorithm_;
-    prefixEntry.ephemeral_ref().reset();
-    prefixes.push_back(prefixEntry);
-  }
-
   // Add redistribute addresses
   for (auto& kv : interfaces_) {
     auto& interface = kv.second;

openr/link-monitor/LinkMonitor.h

@@ -84,8 +84,6 @@ class LinkMonitor final : public OpenrEventBase {
       std::shared_ptr<const Config> config,
       int32_t platformThriftPort,
       KvStore* kvstore,
-      // static list of prefixes to announce
-      std::vector<thrift::IpPrefix> const& staticPrefixes,
       // enable convergence performance measurement for Adjacencies update
       bool enablePerfMeasurement,
       // Queue for spark and kv-store
@@ -261,10 +259,8 @@ class LinkMonitor final : public OpenrEventBase {
 
   /*
    * [PrefixManager] Advertise redistribute prefixes over prefixUpdatesQueue_ to
-   * prefix manager "redistribute prefixes" includes:
-   *   1. static configured prefixes with --prefixes
-   *      TODO: move --prefixes into PrefixManager
-   *   2. addresses read from interfaces match redistribute_interface_regexes
+   * prefix manager "redistribute prefixes" includes addresses of interfaces
+   * that match redistribute_interface_regexes
    *
    * Called in
    * - adjHoldTimer_ during initial start
@@ -280,8 +276,6 @@ class LinkMonitor final : public OpenrEventBase {
   const std::string nodeId_;
   // Switch agent thrift server port
   const int32_t platformThriftPort_{0};
-  // static list of prefixes to announce
-  const std::vector<thrift::IpPrefix> staticPrefixes_;
   // enable performance measurement
   const bool enablePerfMeasurement_{false};
   // URL to receive netlink events from PlatformPublisher

openr/link-monitor/tests/LinkMonitorTest.cpp

@@ -141,9 +141,6 @@ const auto adj_3_1 = createThriftAdjacency(
     Constants::kDefaultAdjWeight /* weight */,
     "" /* otherIfName */);
 
-const auto staticPrefix1 = toIpPrefix("fc00:face:b00c::/64");
-const auto staticPrefix2 = toIpPrefix("fc00:cafe:babe::/64");
-
 thrift::SparkNeighborEvent
 createNeighborEvent(
     thrift::SparkNeighborEventType eventType,
@@ -349,7 +346,6 @@ class LinkMonitorTestFixture : public ::testing::Test {
         config,
         port, /* thrift service port */
         kvStoreWrapper->getKvStore(),
-        std::vector<thrift::IpPrefix>{staticPrefix1, staticPrefix2},
         false /* enable perf measurement */,
         interfaceUpdatesQueue,
         peerUpdatesQueue,
@@ -1686,7 +1682,6 @@ TEST_F(LinkMonitorTestFixture, NodeLabelAlloc) {
         currConfig,
         0, // platform pub port
         kvStoreWrapper->getKvStore(),
-        std::vector<thrift::IpPrefix>(),
         false /* enable perf measurement */,
         interfaceUpdatesQueue,
         peerUpdatesQueue,
@@ -1744,29 +1739,20 @@ TEST_F(LinkMonitorTestFixture, NodeLabelAlloc) {
 }
 
 /**
- * Unit-test to test advertisement of static and loopback prefixes
- * - verify initial prefix-db is set to static prefixes
+ * Unit-test to test advertisement of loopback prefixes
  * - add addresses via addrEvent and verify from KvStore prefix-db
  * - remove address via addrEvent and verify from KvStore prefix-db
  * - announce network instead of address via addrEvent and verify it doesn't
  *   change anything
  * - set link to down state and verify that it removes all associated addresses
  */
-TEST_F(LinkMonitorTestFixture, StaticLoopbackPrefixAdvertisement) {
+TEST_F(LinkMonitorTestFixture, LoopbackPrefixAdvertisement) {
   SetUp({openr::thrift::KvStore_constants::kDefaultArea()});
-  // Verify that initial DB has static prefix entries
+  // Verify that initial DB has empty prefix entries
   std::unordered_set<thrift::IpPrefix> prefixes;
-  prefixes.clear();
-  while (prefixes.size() != 2) {
-    LOG(INFO) << "Testing initial prefix database";
-    prefixes = getNextPrefixDb("prefix:node-1");
-    if (prefixes.size() != 2) {
-      LOG(INFO) << "Looking for 2 prefixes got " << prefixes.size();
-      continue;
-    }
-    EXPECT_EQ(1, prefixes.count(staticPrefix1));
-    EXPECT_EQ(1, prefixes.count(staticPrefix2));
-  }
+
+  prefixes = getNextPrefixDb("prefix:node-1");
+  EXPECT_EQ(0, prefixes.size());
 
   //
   // Send link up event
@@ -1792,15 +1778,13 @@ TEST_F(LinkMonitorTestFixture, StaticLoopbackPrefixAdvertisement) {
 
   // verify
   prefixes.clear();
-  while (prefixes.size() != 7) {
+  while (prefixes.size() != 5) {
     LOG(INFO) << "Testing address advertisements";
     prefixes = getNextPrefixDb("prefix:node-1");
-    if (prefixes.size() != 7) {
-      LOG(INFO) << "Looking for 7 prefixes got " << prefixes.size();
+    if (prefixes.size() != 5) {
+      LOG(INFO) << "Looking for 5 prefixes got " << prefixes.size();
       continue;
     }
-    EXPECT_EQ(1, prefixes.count(staticPrefix1));
-    EXPECT_EQ(1, prefixes.count(staticPrefix2));
     EXPECT_EQ(1, prefixes.count(toIpPrefix("2803:6080:4958:b403::1/128")));
     EXPECT_EQ(1, prefixes.count(toIpPrefix("2803:cafe:babe::1/128")));
     EXPECT_EQ(1, prefixes.count(toIpPrefix("10.127.240.1/32")));
@@ -1831,15 +1815,13 @@ TEST_F(LinkMonitorTestFixture, StaticLoopbackPrefixAdvertisement) {
 
   // verify
   prefixes.clear();
-  while (prefixes.size() != 3) {
+  while (prefixes.size() != 1) {
     LOG(INFO) << "Testing address withdraws";
     prefixes = getNextPrefixDb("prefix:node-1");
-    if (prefixes.size() != 3) {
-      LOG(INFO) << "Looking for 3 prefixes got " << prefixes.size();
+    if (prefixes.size() != 1) {
+      LOG(INFO) << "Looking for 1 prefixes got " << prefixes.size();
       continue;
     }
-    EXPECT_EQ(1, prefixes.count(staticPrefix1));
-    EXPECT_EQ(1, prefixes.count(staticPrefix2));
     EXPECT_EQ(1, prefixes.count(toIpPrefix("2803:6080:4958:b403::1/128")));
   }
 
@@ -1853,16 +1835,13 @@ TEST_F(LinkMonitorTestFixture, StaticLoopbackPrefixAdvertisement) {
   //
   // Verify all addresses are withdrawn on link down event
   //
-  prefixes.clear();
-  while (prefixes.size() != 2) {
+  while (prefixes.size() != 0) {
     LOG(INFO) << "Testing prefix withdraws";
     prefixes = getNextPrefixDb("prefix:node-1");
-    if (prefixes.size() != 2) {
-      LOG(INFO) << "Looking for 2 prefixes got " << prefixes.size();
+    if (prefixes.size() != 0) {
+      LOG(INFO) << "Looking for 0 prefixes got " << prefixes.size();
       continue;
     }
-    EXPECT_EQ(1, prefixes.count(staticPrefix1));
-    EXPECT_EQ(1, prefixes.count(staticPrefix2));
   }
 }
 

openr/scripts/run_openr.sh

@@ -86,7 +86,6 @@ LOOPBACK_IFACE="lo"
 MEMORY_LIMIT_MB=800
 MIN_LOG_LEVEL=0
 OVERRIDE_LOOPBACK_ADDR=false
-PREFIXES=""
 PREFIX_FWD_TYPE_MPLS=0
 PREFIX_FWD_ALGO_KSP2_ED_ECMP=0
 REDISTRIBUTE_IFACES="lo1"
@@ -221,7 +220,6 @@ ARGS="\
   --per_prefix_keys=${PER_PREFIX_KEYS} \
   --prefix_algo_type_ksp2_ed_ecmp=${PREFIX_FWD_ALGO_KSP2_ED_ECMP} \
   --prefix_fwd_type_mpls=${PREFIX_FWD_TYPE_MPLS} \
-  --prefixes=${PREFIXES} \
   --redistribute_ifaces=${REDISTRIBUTE_IFACES} \
   --seed_prefix=${SEED_PREFIX} \
   --set_leaf_node=${SET_LEAF_NODE} \

openr/tests/OpenrWrapper.cpp

@@ -168,14 +168,11 @@ OpenrWrapper<Serializer>::OpenrWrapper(
   //
   // create link monitor
   //
-  std::vector<thrift::IpPrefix> networks;
-  networks.emplace_back(toIpPrefix(folly::IPAddress::createNetwork("::/0")));
   linkMonitor_ = std::make_unique<LinkMonitor>(
       context_,
       config_,
       static_cast<int32_t>(60099), // platfrom pub port
       kvStore_.get(),
-      networks,
       false /* enable perf measurement */,
       interfaceUpdatesQueue_,
       peerUpdatesQueue_,