From 63fbd4caffa4791c08f14fefabe945c0512f3186 Mon Sep 17 00:00:00 2001 From: Bruno Donassolo Date: Tue, 13 Apr 2021 20:36:23 +0200 Subject: [PATCH] Adjust add_route to accept route between netzones --- include/simgrid/kernel/routing/StarZone.hpp | 19 +++- src/kernel/routing/StarZone.cpp | 67 ++++++++---- src/kernel/routing/StarZone_test.cpp | 107 +++++++++++++++----- 3 files changed, 145 insertions(+), 48 deletions(-) diff --git a/include/simgrid/kernel/routing/StarZone.hpp b/include/simgrid/kernel/routing/StarZone.hpp index 2f27bf2624..5afe23f885 100644 --- a/include/simgrid/kernel/routing/StarZone.hpp +++ b/include/simgrid/kernel/routing/StarZone.hpp @@ -75,12 +75,25 @@ public: void do_seal() override; private: + class StarRoute { + public: + std::vector links_up; //!< list of links UP for route (can be empty) + std::vector links_down; //!< list of links DOWN for route (can be empty) + std::vector loopback; //!< loopback links, cannot be empty if configured + bool links_up_set = false; //!< bool to indicate that links_up was configured (empty or not) + bool links_down_set = false; //!< same for links_down + NetPoint* gateway = nullptr; + bool has_loopback() const { return loopback.size() > 0; } + bool has_links_up() const { return links_up_set; } + bool has_links_down() const { return links_down_set; } + }; /** @brief Auxiliary method to add links to a route */ void add_links_to_route(const std::vector& links, RouteCreationArgs* route, double* latency, std::unordered_set& added_links); - std::unordered_map> links_up_; - std::unordered_map> loopback_; - std::unordered_map> links_down_; + /** @brief Auxiliary methods to check params received in add_route method */ + void check_add_route_param(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst, + const std::vector& link_list, bool symmetrical); + std::unordered_map routes_; }; } // namespace routing } // namespace kernel diff --git a/src/kernel/routing/StarZone.cpp b/src/kernel/routing/StarZone.cpp index 57a2c8c719..2310814cae 100644 --- a/src/kernel/routing/StarZone.cpp +++ b/src/kernel/routing/StarZone.cpp @@ -35,27 +35,28 @@ void StarZone::get_local_route(NetPoint* src, NetPoint* dst, RouteCreationArgs* XBT_VERB("StarZone getLocalRoute from '%s'[%u] to '%s'[%u]", src->get_cname(), src->id(), dst->get_cname(), dst->id()); - xbt_assert(((src == dst) and (loopback_.find(src->id()) != loopback_.end())) or - (links_up_.find(src->id()) != links_up_.end()), + xbt_assert(((src == dst) and (routes_[src->id()].has_loopback())) or (routes_[src->id()].has_links_up()), "StarZone routing (%s - %s): no link UP from source node. Did you use add_route() to set it?", src->get_cname(), dst->get_cname()); - xbt_assert(((src == dst) and (loopback_.find(dst->id()) != loopback_.end())) or - (links_down_.find(dst->id()) != links_down_.end()), + xbt_assert(((src == dst) and (routes_[dst->id()].has_loopback())) or (routes_[dst->id()].has_links_down()), "StarZone routing (%s - %s): no link DOWN to destination node. Did you use add_route() to set it?", src->get_cname(), dst->get_cname()); std::unordered_set added_links; /* loopback */ - if ((src == dst) and (loopback_.find(src->id()) != loopback_.end())) { - add_links_to_route(loopback_[src->id()], route, latency, added_links); + if ((src == dst) and (routes_[src->id()].has_loopback())) { + add_links_to_route(routes_[src->id()].loopback, route, latency, added_links); return; } /* going UP */ - add_links_to_route(links_up_[src->id()], route, latency, added_links); + add_links_to_route(routes_[src->id()].links_up, route, latency, added_links); /* going DOWN */ - add_links_to_route(links_down_[dst->id()], route, latency, added_links); + add_links_to_route(routes_[dst->id()].links_down, route, latency, added_links); + /* gateways */ + route->gw_src = routes_[src->id()].gateway; + route->gw_dst = routes_[dst->id()].gateway; } void StarZone::get_graph(const s_xbt_graph_t* graph, std::map>* nodes, @@ -67,7 +68,7 @@ void StarZone::get_graph(const s_xbt_graph_t* graph, std::mapget_cname(), nodes); xbt_node_t previous = src_node; - for (auto* link : links_up_[src->id()]) { + for (auto* link : routes_[src->id()].links_up) { xbt_node_t current = new_xbt_graph_node(graph, link->get_cname(), nodes); new_xbt_graph_edge(graph, previous, current, edges); previous = current; @@ -75,7 +76,7 @@ void StarZone::get_graph(const s_xbt_graph_t* graph, std::mapid()]) { + for (auto* link : routes_[src->id()].links_down) { xbt_node_t current = new_xbt_graph_node(graph, link->get_cname(), nodes); new_xbt_graph_edge(graph, previous, current, edges); previous = current; @@ -84,8 +85,8 @@ void StarZone::get_graph(const s_xbt_graph_t* graph, std::map& link_list, bool symmetrical) +void StarZone::check_add_route_param(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst, + const std::vector& link_list, bool symmetrical) { const char* src_name = src ? src->get_cname() : "nullptr"; const char* dst_name = dst ? dst->get_cname() : "nullptr"; @@ -99,34 +100,58 @@ void StarZone::add_route(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoin "(not the contrary).", src_name, dst_name); + if (src and src->is_netzone()) { + xbt_assert(gw_src, "add_route(): source %s is a netzone but gw_src isn't configured", src->get_cname()); + xbt_assert(not gw_src->is_netzone(), "add_route(): src(%s) is a netzone, gw_src(%s) cannot be a netzone", + src->get_cname(), gw_src->get_cname()); + } + + if (dst and dst->is_netzone()) { + xbt_assert(gw_dst, "add_route(): destination %s is a netzone but gw_dst isn't configured", dst->get_cname()); + xbt_assert(not gw_dst->is_netzone(), "add_route(): dst(%s) is a netzone, gw_dst(%s) cannot be a netzone", + dst->get_cname(), gw_dst->get_cname()); + } +} + +void StarZone::add_route(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst, + const std::vector& link_list, bool symmetrical) +{ + check_add_route_param(src, dst, gw_src, gw_dst, link_list, symmetrical); + + s4u::NetZone::on_route_creation(symmetrical, gw_src, gw_dst, gw_src, gw_dst, link_list); + /* loopback */ if (src == dst) { - loopback_[src->id()] = link_list; + routes_[src->id()].loopback = link_list; } else { /* src to everyone */ if (src) { - links_up_[src->id()] = link_list; + routes_[src->id()].links_up = link_list; + routes_[src->id()].gateway = gw_src; + routes_[src->id()].links_up_set = true; if (symmetrical) { - links_down_[src->id()] = link_list; /* reverse it for down/symmetrical links */ - std::reverse(links_down_[src->id()].begin(), links_down_[src->id()].end()); + routes_[src->id()].links_down.assign(link_list.rbegin(), link_list.rend()); + routes_[src->id()].links_down_set = true; } } /* dst to everyone */ if (dst) { - links_down_[dst->id()] = link_list; + routes_[dst->id()].links_down = link_list; + routes_[dst->id()].gateway = gw_dst; + routes_[dst->id()].links_down_set = true; } } - s4u::NetZone::on_route_creation(symmetrical, gw_src, gw_dst, gw_src, gw_dst, link_list); } void StarZone::do_seal() { /* add default empty links if nothing was configured by user */ for (auto const& node : get_vertices()) { - if ((links_up_.find(node->id()) == links_up_.end()) and (links_down_.find(node->id()) == links_down_.end())) { - links_down_[node->id()] = {}; - links_up_[node->id()] = {}; + if (routes_.find(node->id()) == routes_.end()) { + routes_[node->id()] = {}; + routes_[node->id()].links_down_set = true; + routes_[node->id()].links_up_set = true; } } } diff --git a/src/kernel/routing/StarZone_test.cpp b/src/kernel/routing/StarZone_test.cpp index ef8fb3d823..4a4e878b1c 100644 --- a/src/kernel/routing/StarZone_test.cpp +++ b/src/kernel/routing/StarZone_test.cpp @@ -26,7 +26,7 @@ TEST_CASE("kernel::routing::StarZone: Creating Zone", "[creation]") // One day we may be able to test contracts and asserts with catch2 // https://github.com/catchorg/Catch2/issues/853 -TEST_CASE("kernel::routing::StarZone: Adding routes: assert", "[.][assert]") +TEST_CASE("kernel::routing::StarZone: Adding routes (hosts): assert", "[.][assert]") { int argc = 1; const char* argv[] = {"test"}; @@ -35,11 +35,37 @@ TEST_CASE("kernel::routing::StarZone: Adding routes: assert", "[.][assert]") auto* netpoint1 = new simgrid::kernel::routing::NetPoint("netpoint1", simgrid::kernel::routing::NetPoint::Type::Host); auto* netpoint2 = new simgrid::kernel::routing::NetPoint("netpoint2", simgrid::kernel::routing::NetPoint::Type::Host); - SECTION("Source and destination hosts: nullptr") { zone->add_route(nullptr, nullptr, nullptr, nullptr, {}, false); } + SECTION("src and dst: nullptr") { zone->add_route(nullptr, nullptr, nullptr, nullptr, {}, false); } - SECTION("Source nullptr and symmetrical true") { zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, true); } + SECTION("src: nullptr and symmetrical: true") { zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, true); } - SECTION("Source and destination set") { zone->add_route(netpoint1, netpoint2, nullptr, nullptr, {}, false); } + SECTION("src and dst: not nullptr") { zone->add_route(netpoint1, netpoint2, nullptr, nullptr, {}, false); } +} + +TEST_CASE("kernel::routing::StarZone: Adding routes (netzones): assert", "[.][assert]") +{ + int argc = 1; + const char* argv[] = {"test"}; + simgrid::s4u::Engine e(&argc, const_cast(argv)); + auto* zone = new simgrid::kernel::routing::StarZone("test"); + auto* netpoint1 = + new simgrid::kernel::routing::NetPoint("netpoint1", simgrid::kernel::routing::NetPoint::Type::NetZone); + auto* netpoint2 = + new simgrid::kernel::routing::NetPoint("netpoint2", simgrid::kernel::routing::NetPoint::Type::NetZone); + + SECTION("src: is a netzone and gw_src: nullptr") { zone->add_route(netpoint1, nullptr, nullptr, nullptr, {}, false); } + + SECTION("src: is a netzone and gw_src: is a netzone") + { + zone->add_route(netpoint1, nullptr, netpoint2, nullptr, {}, false); + } + + SECTION("dst: is a netzone and gw_dst: nullptr") { zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, false); } + + SECTION("dst: is a netzone and gw_dst: is a netzone") + { + zone->add_route(nullptr, netpoint2, nullptr, netpoint1, {}, false); + } } TEST_CASE("kernel::routing::StarZone: Get routes: assert", "[.][assert]") @@ -78,7 +104,7 @@ TEST_CASE("kernel::routing::StarZone: Get routes: assert", "[.][assert]") } } -TEST_CASE("kernel::routing::StarZone: Adding routes: valid", "") +TEST_CASE("kernel::routing::StarZone: Adding routes (hosts): valid", "") { int argc = 1; const char* argv[] = {"test"}; @@ -104,7 +130,21 @@ TEST_CASE("kernel::routing::StarZone: Adding routes: valid", "") SECTION("Source == destination") { zone->add_route(netpoint, netpoint, nullptr, nullptr, {}, false); } } -TEST_CASE("kernel::routing::StarZone: Get routes", "") +TEST_CASE("kernel::routing::StarZone: Adding routes (netzones): valid", "") +{ + int argc = 1; + const char* argv[] = {"test"}; + simgrid::s4u::Engine e(&argc, const_cast(argv)); + auto* zone = new simgrid::kernel::routing::StarZone("test"); + auto* netpoint = new simgrid::kernel::routing::NetPoint("netpoint1", simgrid::kernel::routing::NetPoint::Type::Host); + auto* gw = new simgrid::kernel::routing::NetPoint("gw1", simgrid::kernel::routing::NetPoint::Type::Router); + + SECTION("src: is a netzone, src_gw: is a router") { zone->add_route(netpoint, nullptr, gw, nullptr, {}, true); } + + SECTION("dst: is a netzone, dst_gw: is a router") { zone->add_route(nullptr, netpoint, nullptr, gw, {}, false); } +} + +TEST_CASE("kernel::routing::StarZone: Get routes (hosts)", "") { /* workaround to initialize things, they must be done in this particular order */ int argc = 1; @@ -131,6 +171,8 @@ TEST_CASE("kernel::routing::StarZone: Get routes", "") simgrid::kernel::routing::RouteCreationArgs route; zone->get_local_route(host1->get_netpoint(), host2->get_netpoint(), &route, &lat); REQUIRE(lat == 30); + REQUIRE(route.gw_src == nullptr); + REQUIRE(route.gw_dst == nullptr); REQUIRE(route.link_list.size() == 2); REQUIRE(route.link_list[0]->get_name() == "link1"); REQUIRE(route.link_list[1]->get_name() == "link2"); @@ -160,46 +202,63 @@ TEST_CASE("kernel::routing::StarZone: Get routes", "") REQUIRE(route.link_list[2]->get_name() == "link2"); } - SECTION("Get route: shared link(backbone)") + SECTION("Get route: loopback") { auto* backbone = zone->create_link("backbone", {1000})->set_latency(100)->get_impl(); std::vector links; links.push_back(zone->create_link("link1", {100})->set_latency(10)->get_impl()); links.push_back(backbone); - std::vector links2; - links2.push_back(zone->create_link("link2", {200})->set_latency(20)->get_impl()); - links2.push_back(backbone); - zone->add_route(host1->get_netpoint(), nullptr, nullptr, nullptr, links, true); - zone->add_route(host2->get_netpoint(), nullptr, nullptr, nullptr, links2, true); + zone->add_route(host1->get_netpoint(), host1->get_netpoint(), nullptr, nullptr, links, true); zone->seal(); double lat = 0.0; simgrid::kernel::routing::RouteCreationArgs route; - zone->get_local_route(host1->get_netpoint(), host2->get_netpoint(), &route, &lat); - REQUIRE(lat == 130); - REQUIRE(route.link_list.size() == 3); + zone->get_local_route(host1->get_netpoint(), host1->get_netpoint(), &route, &lat); + REQUIRE(lat == 110); + REQUIRE(route.link_list.size() == 2); REQUIRE(route.link_list[0]->get_name() == "link1"); REQUIRE(route.link_list[1]->get_name() == "backbone"); - REQUIRE(route.link_list[2]->get_name() == "link2"); } +} - SECTION("Get route: loopback") +TEST_CASE("kernel::routing::StarZone: Get routes (netzones)", "") +{ + /* workaround to initialize things, they must be done in this particular order */ + int argc = 1; + const char* argv[] = {"test"}; + simgrid::s4u::Engine e(&argc, const_cast(argv)); + auto* zone = new simgrid::kernel::routing::StarZone("test"); + surf_network_model_init_LegrandVelho(); + surf_cpu_model_init_Cas01(); + + auto* subzone1 = + (new simgrid::kernel::routing::NetPoint("subzone1", simgrid::kernel::routing::NetPoint::Type::NetZone)) + ->set_englobing_zone(zone); + auto* subzone2 = + (new simgrid::kernel::routing::NetPoint("subzone2", simgrid::kernel::routing::NetPoint::Type::NetZone)) + ->set_englobing_zone(zone); + auto* router1 = new simgrid::kernel::routing::NetPoint("router1", simgrid::kernel::routing::NetPoint::Type::Router); + auto* router2 = new simgrid::kernel::routing::NetPoint("router2", simgrid::kernel::routing::NetPoint::Type::Router); + + SECTION("Get route: netzone") { - auto* backbone = zone->create_link("backbone", {1000})->set_latency(100)->get_impl(); std::vector links; links.push_back(zone->create_link("link1", {100})->set_latency(10)->get_impl()); - links.push_back(backbone); - - zone->add_route(host1->get_netpoint(), host1->get_netpoint(), nullptr, nullptr, links, true); + std::vector links2; + links2.push_back(zone->create_link("link2", {200})->set_latency(20)->get_impl()); + zone->add_route(subzone1, nullptr, router1, nullptr, links, true); + zone->add_route(subzone2, nullptr, router2, nullptr, links2, true); zone->seal(); double lat = 0.0; simgrid::kernel::routing::RouteCreationArgs route; - zone->get_local_route(host1->get_netpoint(), host1->get_netpoint(), &route, &lat); - REQUIRE(lat == 110); + zone->get_local_route(subzone1, subzone2, &route, &lat); + REQUIRE(lat == 30); + REQUIRE(route.gw_src == router1); + REQUIRE(route.gw_dst == router2); REQUIRE(route.link_list.size() == 2); REQUIRE(route.link_list[0]->get_name() == "link1"); - REQUIRE(route.link_list[1]->get_name() == "backbone"); + REQUIRE(route.link_list[1]->get_name() == "link2"); } } \ No newline at end of file -- 2.20.1