From d4674fe3018e00d2879e5bfbe23920f0a91055b0 Mon Sep 17 00:00:00 2001 From: Aleksandr Stepanov Date: Mon, 17 Feb 2025 15:53:10 +0300 Subject: [PATCH] Fix based on review comments --- common/controlplaneconfig.h | 8 ++------ common/define.h | 2 +- controlplane/controlplane.cpp | 6 ++++-- controlplane/controlplane.h | 4 ++-- controlplane/route.h | 2 +- dataplane/globalbase.cpp | 32 +++++++++++++------------------- dataplane/globalbase.h | 10 ++++++---- dataplane/updater.h | 26 +++++++++++++++++++++----- librib/libyabird.cpp | 8 ++++---- 9 files changed, 54 insertions(+), 44 deletions(-) diff --git a/common/controlplaneconfig.h b/common/controlplaneconfig.h index 4d5a4f86..e3018266 100644 --- a/common/controlplaneconfig.h +++ b/common/controlplaneconfig.h @@ -107,11 +107,7 @@ class interface_t class config_t { public: - config_t() : - vrf(YANET_RIB_VRF_DEFAULT), - tunnel_enabled(false) - { - } + config_t() = default; /** @todo: tag:CP_MODULES void load(const nlohmann::json& json); @@ -123,7 +119,7 @@ class config_t public: tRouteId routeId; std::set to_kernel_prefixes; - std::string vrf{"default"}; + std::string vrf{YANET_RIB_VRF_DEFAULT}; bool tunnel_enabled{}; std::set ignore_tables; common::ipv4_address_t ipv4_source_address; diff --git a/common/define.h b/common/define.h index f0d2bd8a..59ec93a6 100644 --- a/common/define.h +++ b/common/define.h @@ -119,7 +119,7 @@ extern LogPriority logPriority; #define YANET_RIB_PRIORITY_DEFAULT ((uint32_t)10000) #define YANET_RIB_PRIORITY_ROUTE_TUNNEL_FALLBACK ((uint32_t)11000) #define YANET_RIB_PRIORITY_ROUTE_REPEAT ((uint32_t)12000) -#define YANET_RIB_VRF_DEFAULT "default" +constexpr auto YANET_RIB_VRF_DEFAULT = "default"; #define YANET_RIB_VRF_MAX_NUMBER 64 #define YANET_DEFAULT_BGP_AS ((uint32_t)13238) diff --git a/controlplane/controlplane.cpp b/controlplane/controlplane.cpp index 62d337b8..ff5a6517 100644 --- a/controlplane/controlplane.cpp +++ b/controlplane/controlplane.cpp @@ -1020,7 +1020,7 @@ VrfIdStorage& cControlPlane::getVrfIdsStorage() return vrfIds; } -std::optional VrfIdStorage::Get(const std::string& vrfName) +std::optional VrfIdStorage::Get(const std::string& vrfName) const { if (vrfName.empty() || vrfName == YANET_RIB_VRF_DEFAULT) { @@ -1071,7 +1071,9 @@ tVrfId VrfIdStorage::GetOrCreateOrException(const std::string& vrfName, const st std::optional vrfId = GetOrCreate(vrfName); if (!vrfId.has_value()) { - throw error_result_t(eResult::invalidVrfId, message + vrfName); + std::ostringstream oss; + oss << message << ": " << vrfName; + throw error_result_t(eResult::invalidVrfId, oss.str()); } return *vrfId; } diff --git a/controlplane/controlplane.h b/controlplane/controlplane.h index 2d7ebf4c..a368dcc0 100644 --- a/controlplane/controlplane.h +++ b/controlplane/controlplane.h @@ -29,12 +29,12 @@ class VrfIdStorage { public: - std::optional Get(const std::string& vrfName); + std::optional Get(const std::string& vrfName) const; std::optional GetOrCreate(const std::string& vrfName); tVrfId GetOrCreateOrException(const std::string& vrfName, const std::string& message); private: - std::shared_mutex mutex; + mutable std::shared_mutex mutex; std::unordered_map> vrf_ids; }; diff --git a/controlplane/route.h b/controlplane/route.h index 7945d8cd..619da422 100644 --- a/controlplane/route.h +++ b/controlplane/route.h @@ -176,7 +176,7 @@ class generation_t } } - bool is_ignored_table(const std::string& table_name) const + [[nodiscard]] bool is_ignored_table(const std::string& table_name) const { for (const auto& [name, module] : routes) { diff --git a/dataplane/globalbase.cpp b/dataplane/globalbase.cpp index 64f87f14..28efcac3 100644 --- a/dataplane/globalbase.cpp +++ b/dataplane/globalbase.cpp @@ -233,9 +233,9 @@ eResult generation::init() } { - updater.vrf_route_lpm4 = std::make_unique>("vrf_route.v4.lpm", - &dataPlane->memory_manager, - socketId); + updater.vrf_route_lpm4 = std::make_unique("vrf_route.v4.lpm", + &dataPlane->memory_manager, + socketId); result = updater.vrf_route_lpm4->init(); if (result != eResult::success) { @@ -246,9 +246,9 @@ eResult generation::init() } { - updater.vrf_route_lpm6 = std::make_unique, vrf_lpm6>>("vrf_route.v6.lpm", - &dataPlane->memory_manager, - socketId); + updater.vrf_route_lpm6 = std::make_unique("vrf_route.v6.lpm", + &dataPlane->memory_manager, + socketId); result = updater.vrf_route_lpm6->init(); if (result != eResult::success) { @@ -259,9 +259,9 @@ eResult generation::init() } { - updater.vrf_route_tunnel_lpm4 = std::make_unique>("vrf_route.tunnel.v4.lpm", - &dataPlane->memory_manager, - socketId); + updater.vrf_route_tunnel_lpm4 = std::make_unique("vrf_route.tunnel.v4.lpm", + &dataPlane->memory_manager, + socketId); result = updater.vrf_route_tunnel_lpm4->init(); if (result != eResult::success) { @@ -272,9 +272,9 @@ eResult generation::init() } { - updater.vrf_route_tunnel_lpm6 = std::make_unique, vrf_lpm6>>("vrf_route.tunnel.v6.lpm", - &dataPlane->memory_manager, - socketId); + updater.vrf_route_tunnel_lpm6 = std::make_unique("vrf_route.tunnel.v6.lpm", + &dataPlane->memory_manager, + socketId); result = updater.vrf_route_tunnel_lpm6->init(); if (result != eResult::success) { @@ -828,13 +828,7 @@ static bool checkFlow(const common::globalBase::tFlow& flow) eResult generation::updateLogicalPort(const common::idp::updateGlobalBase::updateLogicalPort::request& request) { - const auto& logicalPortId = std::get<0>(request); - const auto& portId = std::get<1>(request); - const auto& vlanId = std::get<2>(request); - const auto& vrfId = std::get<3>(request); - const auto& etherAddress = std::get<4>(request); - const auto& promiscuousMode = std::get<5>(request); - const auto& flow = std::get<6>(request); + const auto& [logicalPortId, portId, vlanId, vrfId, etherAddress, promiscuousMode, flow] = request; if (logicalPortId >= CONFIG_YADECAP_LOGICALPORTS_SIZE) { diff --git a/dataplane/globalbase.h b/dataplane/globalbase.h index 5ea908bc..59ec499b 100644 --- a/dataplane/globalbase.h +++ b/dataplane/globalbase.h @@ -136,6 +136,8 @@ class generation public: using vrf_lpm4 = lpm4_24bit_8bit_atomic; using vrf_lpm6 = lpm6_8x16bit_atomic; + using updater_vrf_lpm4 = updater_vrf_lpm; + using updater_vrf_lpm6 = updater_vrf_lpm, vrf_lpm6>; generation(cDataPlane* dataPlane, const tSocketId& socketId); ~generation() = default; @@ -218,10 +220,10 @@ class generation std::unique_ptr route_tunnel_lpm4; std::unique_ptr route_tunnel_lpm6; - std::unique_ptr> vrf_route_lpm4; - std::unique_ptr, vrf_lpm6>> vrf_route_lpm6; - std::unique_ptr> vrf_route_tunnel_lpm4; - std::unique_ptr, vrf_lpm6>> vrf_route_tunnel_lpm6; + std::unique_ptr vrf_route_lpm4; + std::unique_ptr vrf_route_lpm6; + std::unique_ptr vrf_route_tunnel_lpm4; + std::unique_ptr vrf_route_tunnel_lpm6; } updater; /// variables above are not needed for cWorker::mainThread() diff --git a/dataplane/updater.h b/dataplane/updater.h index 281769c5..d7ff79db 100644 --- a/dataplane/updater.h +++ b/dataplane/updater.h @@ -5,6 +5,7 @@ #include "common.h" #include "common/idp.h" +#include "common/static_vector.h" #include "dynamic_table.h" #include "hashtable.h" #include "lpm.h" @@ -673,10 +674,19 @@ class updater_array template class updater_vrf_lpm { -public: - using stats_t = typename InnerLpmType::stats_t; using UpdaterType = updater_lpm; + [[nodiscard]] memory_manager::unique_ptr Allocate(const std::string& name_vrf) + { + return memory_manager_->create_unique(name_.c_str(), + socket_id_, + sizeof(UpdaterType), + name_vrf.c_str(), + memory_manager_, + socket_id_); + } + +public: updater_vrf_lpm(const char* name, dataplane::memory_manager* memory_manager, tSocketId socket_id) : @@ -688,6 +698,10 @@ class updater_vrf_lpm eResult init() { + for (size_t index = 0; index < updaters_.capacity(); index++) + { + updaters_.emplace_back(memory_manager::unique_ptr{nullptr, memory_manager_->deleter()}); + } return eResult::success; } @@ -702,8 +716,10 @@ class updater_vrf_lpm } if (updaters_[vrf] == nullptr) { - std::string name = std::string(name_) + ".vrf" + std::to_string(vrf); - updaters_[vrf] = std::make_unique(name.c_str(), memory_manager_, socket_id_); + std::ostringstream oss; + oss << name_ << ".vrf" << vrf; + std::string name = oss.str(); + updaters_[vrf] = Allocate(name); if (updaters_[vrf] == nullptr) { return eResult::errorAllocatingMemory; @@ -781,7 +797,7 @@ class updater_vrf_lpm std::string name_; dataplane::memory_manager* memory_manager_; tSocketId socket_id_; - std::array, YANET_RIB_VRF_MAX_NUMBER> updaters_; + utils::StaticVector, YANET_RIB_VRF_MAX_NUMBER> updaters_; }; } diff --git a/librib/libyabird.cpp b/librib/libyabird.cpp index 04767c88..97839c0a 100644 --- a/librib/libyabird.cpp +++ b/librib/libyabird.cpp @@ -93,7 +93,7 @@ void libyabird_t::set_state(const char* peer, int state) common::icp::rib_update::clear request = {"bgp", std::nullopt}; std::get<1>(request) = {peer_address, - {"default", ///< @todo: vrf + {YANET_RIB_VRF_DEFAULT, YANET_RIB_PRIORITY_DEFAULT}}; { @@ -163,7 +163,7 @@ void libyabird_t::update(yanet_data_t* data) { common::icp::rib_update::eor request = {"bgp", - "default", + YANET_RIB_VRF_DEFAULT, YANET_RIB_PRIORITY_DEFAULT, peer_address, table_name}; @@ -234,7 +234,7 @@ void libyabird_t::update(yanet_data_t* data) std::holds_alternative(rib_request.back()))) { common::icp::rib_update::insert request = {"bgp", - "default", ///< @todo: vrf + YANET_RIB_VRF_DEFAULT, YANET_RIB_PRIORITY_DEFAULT, {}}; @@ -293,7 +293,7 @@ void libyabird_t::update(yanet_data_t* data) std::holds_alternative(rib_request.back()))) { common::icp::rib_update::remove request = {"bgp", - "default", ///< @todo: vrf + YANET_RIB_VRF_DEFAULT, YANET_RIB_PRIORITY_DEFAULT, {}}; rib_request.emplace_back(request);