Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Adjust add_route to accept route between netzones
authorBruno Donassolo <bruno.donassolo@inria.fr>
Tue, 13 Apr 2021 18:36:23 +0000 (20:36 +0200)
committerBruno Donassolo <bruno.donassolo@inria.fr>
Wed, 14 Apr 2021 11:56:59 +0000 (13:56 +0200)
include/simgrid/kernel/routing/StarZone.hpp
src/kernel/routing/StarZone.cpp
src/kernel/routing/StarZone_test.cpp

index 2f27bf2..5afe23f 100644 (file)
@@ -75,12 +75,25 @@ public:
   void do_seal() override;
 
 private:
+  class StarRoute {
+  public:
+    std::vector<resource::LinkImpl*> links_up;   //!< list of links UP for route (can be empty)
+    std::vector<resource::LinkImpl*> links_down; //!< list of links DOWN for route (can be empty)
+    std::vector<resource::LinkImpl*> 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<resource::LinkImpl*>& links, RouteCreationArgs* route, double* latency,
                           std::unordered_set<resource::LinkImpl*>& added_links);
-  std::unordered_map<unsigned int, std::vector<resource::LinkImpl*>> links_up_;
-  std::unordered_map<unsigned int, std::vector<resource::LinkImpl*>> loopback_;
-  std::unordered_map<unsigned int, std::vector<resource::LinkImpl*>> 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<kernel::resource::LinkImpl*>& link_list, bool symmetrical);
+  std::unordered_map<unsigned int, StarRoute> routes_;
 };
 } // namespace routing
 } // namespace kernel
index 57a2c8c..2310814 100644 (file)
@@ -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<resource::LinkImpl*> 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<std::string, xbt_node_t, std::less<>>* nodes,
@@ -67,7 +68,7 @@ void StarZone::get_graph(const s_xbt_graph_t* graph, std::map<std::string, xbt_n
     /* going up */
     xbt_node_t src_node = new_xbt_graph_node(graph, src->get_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::map<std::string, xbt_n
     new_xbt_graph_edge(graph, previous, star_node, edges);
     /* going down */
     previous = star_node;
-    for (auto* link : links_down_[src->id()]) {
+    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<std::string, xbt_n
   }
 }
 
-void StarZone::add_route(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst,
-                         const std::vector<kernel::resource::LinkImpl*>& link_list, bool symmetrical)
+void StarZone::check_add_route_param(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst,
+                                     const std::vector<kernel::resource::LinkImpl*>& 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<kernel::resource::LinkImpl*>& 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;
     }
   }
 }
index ef8fb3d..4a4e878 100644 (file)
@@ -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<char**>(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<char**>(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<simgrid::kernel::resource::LinkImpl*> links;
     links.push_back(zone->create_link("link1", {100})->set_latency(10)->get_impl());
     links.push_back(backbone);
-    std::vector<simgrid::kernel::resource::LinkImpl*> 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<char**>(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<simgrid::kernel::resource::LinkImpl*> 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<simgrid::kernel::resource::LinkImpl*> 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