Skip to content

Commit

Permalink
(better-eng) Remove SegmentRoutingNodeLabel from OpenrConfig.thrift
Browse files Browse the repository at this point in the history 
Summary: Dead code cleanup

Reviewed By: wilsonwinhi

Differential Revision: D56690431

fbshipit-source-id: 0e45e0a1da5b700acb0e42da57a9e2e398cec31c
  • Loading branch information
@facebook-github-bot
Xiang Xu authored and facebook-github-bot committed Apr 29, 2024
1 parent b99afd3 commit 5719c31
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 264 deletions.
10 changes: 0 additions & 10 deletions openr/config/Config.h
View file
Expand Up @@ -37,9 +37,6 @@ class AreaConfiguration {
compileRegexSet(*area.redistribute_interface_regexes());

// parse optional fields
if (auto nodeLabel = area.area_sr_node_label()) {
srNodeLabel_ = *nodeLabel;
}
if (auto policyName = area.import_policy_name()) {
importPolicyName_ = *policyName;
}
Expand All @@ -50,11 +47,6 @@ class AreaConfiguration {
return areaId_;
}

std::optional<openr::thrift::SegmentRoutingNodeLabel>
getNodeSegmentLabelConfig() const {
return srNodeLabel_;
}

bool
shouldDiscoverOnIface(std::string const& iface) const {
return !interfaceExcludeRegexSet_->Match(iface, nullptr) &&
Expand All @@ -78,8 +70,6 @@ class AreaConfiguration {

private:
const std::string areaId_;
std::optional<openr::thrift::SegmentRoutingNodeLabel> srNodeLabel_{
std::nullopt};

std::optional<std::string> importPolicyName_{std::nullopt};

Expand Down
47 changes: 0 additions & 47 deletions openr/if/OpenrConfig.thrift
View file
Expand Up @@ -416,46 +416,6 @@ struct OriginatedPrefix {
11: optional i64 minNexthop;
}

struct LabelRange {
1: i32 start_label;
2: i32 end_label;
}

enum SegmentRoutingNodeLabelType {
/**
* Current way of allocation. Needs range parameter.
*/
AUTO = 0,
/**
* Node Segment IDs are allocated statically.
* The uniqueness of node label will be determined
* by the application which generates static
* node label.
*/
STATIC = 1,
}

@cpp.MinimizePadding
struct SegmentRoutingNodeLabel {
/**
* The way node segment label should be allocated.
*/
1: SegmentRoutingNodeLabelType sr_node_label_type;

/**
* Statically allocated node label for this node.
* Applicable if SegmentRoutingNodeLabelType is
* SegmentRoutingNodeLabelType::STATIC
*/
2: optional i32 node_segment_label;

/**
* Label range for node segment label to allocate from if
* sr_node_label_type is AUTO.
*/
3: optional LabelRange node_segment_label_range;
}

/**
* The area config specifies the area name, interfaces to perform discovery
* on, neighbor names to peer with, and interface addresses to redistribute
Expand Down Expand Up @@ -530,13 +490,6 @@ struct AreaConfig {
* Area import policy, applied when a route enters this area
*/
7: optional string import_policy_name;

/**
* The way node segment label should be allocated
* in this area for segment routing. Node segment
* should be unique per device per area.
*/
8: optional SegmentRoutingNodeLabel area_sr_node_label;
}

@cpp.MinimizePadding
Expand Down
37 changes: 0 additions & 37 deletions openr/link-monitor/LinkMonitor.cpp
View file
Expand Up @@ -268,31 +268,6 @@ LinkMonitor::LinkMonitor(
}
}

if (enableSegmentRouting_) {
// create range allocator to get unique node labels
for (auto const& [areaId, areaCfg] : areas_) {
auto const& srNodeLabelCfg = areaCfg.getNodeSegmentLabelConfig();
if (not srNodeLabelCfg.has_value()) {
XLOG(INFO) << fmt::format(
"Area {} does not have segment rotuing node label config", areaId);
continue;
}

CHECK(
*srNodeLabelCfg->sr_node_label_type() ==
thrift::SegmentRoutingNodeLabelType::STATIC)
<< "Unknown segment routing node label allocation type";
// Use statically configured node segment label as node label
auto nodeLbl = getStaticNodeSegmentLabel(areaCfg);
state_.nodeLabelMap()->insert_or_assign(areaId, nodeLbl);
XLOG(INFO) << fmt::format(
"Allocating static node segment label {} inside area {} for {}",
nodeLbl,
areaId,
nodeId_);
}
}

// start initial dump timer
adjHoldTimer_->scheduleTimeout(initialAdjHoldTime);

Expand Down Expand Up @@ -1961,18 +1936,6 @@ LinkMonitor::anyAreaShouldRedistributeIface(std::string const& iface) const {
return anyMatch;
}

int32_t
LinkMonitor::getStaticNodeSegmentLabel(
AreaConfiguration const& areaConfig) const {
CHECK(areaConfig.getNodeSegmentLabelConfig().has_value());
if (areaConfig.getNodeSegmentLabelConfig()
->node_segment_label()
.has_value()) {
return *areaConfig.getNodeSegmentLabelConfig()->node_segment_label();
}
return 0;
}

void
LinkMonitor::scheduleAdvertiseAdjAllArea() {
for (const auto& [area, _] : areas_) {
Expand Down
13 changes: 2 additions & 11 deletions openr/link-monitor/LinkMonitor.h
View file
Expand Up @@ -314,15 +314,6 @@ class LinkMonitor final : public OpenrEventBase {
const std::string& peerName,
const thrift::PeerSpec& peerSpec);

/*
* [Segment Routing]
*
* util functions to support label allocation in dynamic/static way
*/

// Get static area node segment label
int32_t getStaticNodeSegmentLabel(AreaConfiguration const& areaConfig) const;

//
// immutable state/invariants
//
Expand All @@ -336,9 +327,9 @@ class LinkMonitor final : public OpenrEventBase {
const bool enableLinkStatusMeasurement_{false};
// enable v4
bool enableV4_{false};
// enable segment routing
// [TO_BE_DEPRECATED] enable segment routing
bool enableSegmentRouting_{false};
// prefix forwarding type and algorithm
// [TO_BE_DEPRECATED] prefix forwarding type and algorithm
thrift::PrefixForwardingType prefixForwardingType_;
thrift::PrefixForwardingAlgorithm prefixForwardingAlgorithm_;
// Use spark measured RTT to neighbor as link metric
Expand Down
163 changes: 4 additions & 159 deletions openr/link-monitor/tests/LinkMonitorTest.cpp
View file
Expand Up @@ -38,9 +38,6 @@ using ::testing::InSequence;
// interface iface_3_1
namespace {

using Area2SRNodeLabel =
std::unordered_map<std::string, thrift::SegmentRoutingNodeLabel>;

const auto nb2_v4_addr = "192.168.0.2";
const auto nb3_v4_addr = "192.168.0.3";
const auto nb2_v6_addr = "fe80::2";
Expand Down Expand Up @@ -309,7 +306,7 @@ class LinkMonitorTestFixture : public testing::Test {

virtual std::vector<thrift::AreaConfig>
createAreaConfigs() {
return populateAreaConfigs({}, {});
return populateAreaConfigs({});
}

/*
Expand All @@ -320,18 +317,8 @@ class LinkMonitorTestFixture : public testing::Test {
* 2) validation method for unit-test usage;
* 3) etc.
*/
thrift::SegmentRoutingNodeLabel
populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType nodeLabelType, int32_t nodeLabel) {
thrift::SegmentRoutingNodeLabel srNodeLabel;
srNodeLabel.sr_node_label_type() = nodeLabelType;
srNodeLabel.node_segment_label() = nodeLabel;
return srNodeLabel;
}

std::vector<thrift::AreaConfig>
populateAreaConfigs(
std::unordered_set<std::string> areas, Area2SRNodeLabel areaMap) {
populateAreaConfigs(std::unordered_set<std::string> areas) {
// Use kTestingAreaName by default
if (areas.empty()) {
areas.insert(kTestingAreaName);
Expand All @@ -347,28 +334,6 @@ class LinkMonitorTestFixture : public testing::Test {
cfg.include_interface_regexes() = {kTestVethNamePrefix + ".*", "iface.*"};
cfg.redistribute_interface_regexes() = {"loopback"};

auto it = areaMap.find(areaId);
if (it == areaMap.end()) {
cfg.area_sr_node_label() = populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType::STATIC, nodeLabelBase++);
} else {
thrift::SegmentRoutingNodeLabel srNodeLabel;
openr::thrift::LabelRange labelRange;
srNodeLabel.sr_node_label_type() = *it->second.sr_node_label_type();

if (it->second.node_segment_label_range().has_value()) {
labelRange.start_label() =
*it->second.node_segment_label_range()->start_label();
labelRange.end_label() =
*it->second.node_segment_label_range()->end_label();
srNodeLabel.node_segment_label_range() = labelRange;
}

if (it->second.node_segment_label().has_value()) {
srNodeLabel.node_segment_label() = *it->second.node_segment_label();
}
cfg.area_sr_node_label() = srNodeLabel;
}
areaConfig.emplace_back(std::move(cfg));
}
return areaConfig;
Expand Down Expand Up @@ -2165,131 +2130,11 @@ TEST_F(LinkMonitorTestFixture, verifyAddrEventSubscription) {
}
}

class StaticNodeLabelTestFixture : public LinkMonitorTestFixture {
public:
std::vector<thrift::AreaConfig>
createAreaConfigs() override {
Area2SRNodeLabel areaMap;
areaMap.emplace(
kTestingAreaName,
populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType::STATIC, 101));
areaMap.emplace(
kTestingSpineAreaName,
populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType::STATIC, 201));

// create list of thrift::AreaConfig
return populateAreaConfigs(
{kTestingAreaName, kTestingSpineAreaName}, areaMap);
}

protected:
const openr::AreaId kTestingSpineAreaName{"test_spine_area_name"};
};

/**
* Test allocation of static node segment label in mesh area and spine area
*/
TEST_F(StaticNodeLabelTestFixture, StaticNodeLabelAlloc) {
// spin up kNumNodesToTest - 1 new link monitors. 1 is spun up in setup()
size_t kNumNodesToTest = 10;
std::vector<std::unique_ptr<LinkMonitor>> linkMonitors;
std::vector<std::unique_ptr<std::thread>> linkMonitorThreads;

for (size_t i = 0; i < kNumNodesToTest - 1; i++) {
Area2SRNodeLabel mp;
mp.emplace(
kTestingAreaName,
populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType::STATIC, 102 + i));
mp.emplace(
kTestingSpineAreaName,
populateSegmentRoutingNodeLabel(
thrift::SegmentRoutingNodeLabelType::STATIC, 202 + i));

// create thrift::AreaConfig
auto areaCfgs =
populateAreaConfigs({kTestingAreaName, kTestingSpineAreaName}, mp);

// create thrift::OpenrConfig
auto tConfig = getBasicOpenrConfig("node-1", areaCfgs, true, true);

// override LM config
tConfig.node_name() = fmt::format("lm{}", i + 1);
tConfig.persistent_config_store_path() = kConfigStorePath;
tConfig.link_monitor_config()->linkflap_initial_backoff_ms() = 1;
tConfig.link_monitor_config()->linkflap_max_backoff_ms() = 8;
tConfig.link_monitor_config()->use_rtt_metric() = false;
auto currConfig = std::make_shared<Config>(tConfig);

// create lm instances
auto lm = std::make_unique<LinkMonitor>(
currConfig,
nlSock.get(),
configStore.get(),
interfaceUpdatesQueue,
prefixUpdatesQueue,
peerUpdatesQueue,
logSampleQueue,
kvRequestQueue,
neighborUpdatesQueue.getReader(),
nlSock->getReader());
linkMonitors.emplace_back(std::move(lm));

auto lmThread =
std::make_unique<std::thread>([&]() { linkMonitors.back()->run(); });
linkMonitorThreads.emplace_back(std::move(lmThread));
linkMonitors.back()->waitUntilRunning();
}

// label to node mapping to make sure unique label is assigned
std::unordered_map<int32_t, std::string> label2NodeMap1;
std::unordered_map<int32_t, std::string> label2NodeMap2;
auto unknownAreaPublications = 0;

// recv kv store publications until we have valid labels from each node
while (label2NodeMap1.size() < kNumNodesToTest or
label2NodeMap2.size() < kNumNodesToTest) {
auto pub = kvStoreWrapper->recvPublication();
for (auto const& [key, val] : *pub.keyVals()) {
// skip non-adj key update or ttl update
if (key.find("adj:") != 0 or (not val.value())) {
continue;
}

auto adjDb = readThriftObjStr<thrift::AdjacencyDatabase>(
val.value().value(), serializer);
if (*pub.area() == static_cast<std::string>(kTestingAreaName)) {
// kTestingAreaName node segment label
label2NodeMap1[*adjDb.nodeLabel()] = *adjDb.thisNodeName();
} else if (
pub.area() == static_cast<std::string>(kTestingSpineAreaName)) {
// kTestingSpineAreaName node segment label
label2NodeMap2[*adjDb.nodeLabel()] = *adjDb.thisNodeName();
} else {
unknownAreaPublications++;
}
}
}
EXPECT_EQ(unknownAreaPublications, 0);

// cleanup
nlSock->closeQueue();
neighborUpdatesQueue.close();
initializationEventQueue.close();
kvStoreWrapper->closeQueue();
for (size_t i = 0; i < kNumNodesToTest - 1; i++) {
linkMonitors[i]->stop();
linkMonitorThreads[i]->join();
}
}

class TwoAreaTestFixture : public LinkMonitorTestFixture {
public:
std::vector<thrift::AreaConfig>
createAreaConfigs() override {
return populateAreaConfigs({area1_, area2_}, {});
return populateAreaConfigs({area1_, area2_});
}

protected:
Expand Down Expand Up @@ -2669,7 +2514,7 @@ class MultiAreaTestFixture : public LinkMonitorTestFixture {
public:
std::vector<thrift::AreaConfig>
createAreaConfigs() override {
return populateAreaConfigs({kTestingAreaName, podArea_, planeArea_}, {});
return populateAreaConfigs({kTestingAreaName, podArea_, planeArea_});
}

protected:
Expand Down

0 comments on commit 5719c31

Please sign in to comment.