fix the handling of bypassASroutes

Fred used to think he needs it for his work, so here is the fix. It
was wrong anyway.

Now that it works, we should make the code better (too much redundency
for now) at some point.

diff --git a/include/simgrid/s4u/As.hpp b/include/simgrid/s4u/As.hpp
index 72921f9..0a8c77a 100644 (file)
--- a/include/simgrid/s4u/As.hpp
+++ b/include/simgrid/s4u/As.hpp
@@ -27,6 +27,7 @@ namespace kernel {
   namespace routing {
     class AsImpl;
     class NetCard;
+    class AsRoute;
   }
 }
 namespace s4u {
@@ -66,7 +67,8 @@ private:
 
   bool sealed_ = false; // We cannot add more content when sealed
 
-  std::map<std::pair<kernel::routing::NetCard*, kernel::routing::NetCard*>, std::vector<surf::Link*>*> bypassRoutes_; // src x dst -> route
+  std::map<std::pair<kernel::routing::NetCard*, kernel::routing::NetCard*>, kernel::routing::AsRoute*>
+      bypassRoutes_;                                                                      // src x dst -> route
   xbt_dict_t children_ = xbt_dict_new_homogeneous(nullptr);                               // sub-ASes
 };
 

diff --git a/src/kernel/routing/AsImpl.cpp b/src/kernel/routing/AsImpl.cpp
index 30fe085..67e292f 100644 (file)
--- a/src/kernel/routing/AsImpl.cpp
+++ b/src/kernel/routing/AsImpl.cpp
@@ -159,13 +159,13 @@ namespace simgrid {
     /* Base case, no recursion is needed */
     if (dst->containingAS() == this && src->containingAS() == this) {
       if (bypassRoutes_.find({src, dst}) != bypassRoutes_.end()) {
-        std::vector<surf::Link*>* bypassedRoute = bypassRoutes_.at({src, dst});
-        for (surf::Link* link : *bypassedRoute) {
+        AsRoute* bypassedRoute = bypassRoutes_.at({src, dst});
+        for (surf::Link* link : bypassedRoute->links) {
           links->push_back(link);
           if (latency)
             *latency += link->latency();
         }
-        XBT_DEBUG("Found a bypass route with %zu links", bypassedRoute->size());
+        XBT_DEBUG("Found a bypass route with %zu links", bypassedRoute->links.size());
         return true;
       }
       return false;
@@ -173,7 +173,6 @@ namespace simgrid {
 
     /* Engage recursive search */
 
-    std::vector<surf::Link*>* bypassedRoute = nullptr;
 
     /* (1) find the path to the root routing component */
     std::vector<AsImpl*> path_src;
@@ -202,6 +201,8 @@ namespace simgrid {
 
     int max_index = std::max(max_index_src, max_index_dst);
 
+    /* (3) Search for a bypass making the path up to the ancestor useless */
+    AsRoute* bypassedRoute = nullptr;
     std::pair<kernel::routing::NetCard*, kernel::routing::NetCard*> key;
     for (int max = 0; max <= max_index; max++) {
       for (int i = 0; i < max; i++) {
@@ -233,12 +234,17 @@ namespace simgrid {
       }
     }
 
+    /* (4) If we have the bypass, use it. If not, caller will do the Right Thing. */
     if (bypassedRoute) {
-      for (surf::Link* link : *bypassedRoute) {
+      if (src != key.first)
+        getRouteRecursive(src, const_cast<NetCard*>(bypassedRoute->gw_src), links, latency);
+      for (surf::Link* link : bypassedRoute->links) {
         links->push_back(link);
         if (latency)
           *latency += link->latency();
       }
+      if (dst != key.second)
+        getRouteRecursive(const_cast<NetCard*>(bypassedRoute->gw_dst), dst, links, latency);
       return true;
     }
     return false;

diff --git a/src/kernel/routing/AsImpl.hpp b/src/kernel/routing/AsImpl.hpp
index 5abdb28..700f067 100644 (file)
--- a/src/kernel/routing/AsImpl.hpp
+++ b/src/kernel/routing/AsImpl.hpp
@@ -65,8 +65,8 @@ public:
                       /* OUT */ std::vector<surf::Link*> * links, double* latency);
 
   virtual void getGraph(xbt_graph_t graph, xbt_dict_t nodes, xbt_dict_t edges)=0;
-  static void getRouteRecursive(routing::NetCard *src, routing::NetCard *dst, /* OUT */ std::vector<surf::Link*> * links, double *latency);
-
+  static void getRouteRecursive(routing::NetCard * src, routing::NetCard * dst,
+                                /* OUT */ std::vector<surf::Link*> * links, double* latency);
 
   enum class RoutingMode {
     unset = 0,  /**< Undefined type                                   */

diff --git a/src/s4u/s4u_as.cpp b/src/s4u/s4u_as.cpp
index 0a14f8d..4700b23 100644 (file)
--- a/src/s4u/s4u_as.cpp
+++ b/src/s4u/s4u_as.cpp
@@ -95,9 +95,9 @@ namespace simgrid {
     }
 
     /* Build a copy that will be stored in the dict */
-    std::vector<surf::Link*>* newRoute = new std::vector<surf::Link*>();
+    kernel::routing::AsRoute* newRoute = new kernel::routing::AsRoute(e_route->gw_src, e_route->gw_dst);
     for (auto link : *e_route->link_list)
-      newRoute->push_back(link);
+      newRoute->links.push_back(link);
 
     /* Store it */
     bypassRoutes_.insert({{e_route->src, e_route->dst}, newRoute});

diff --git a/src/surf/surf_routing.hpp b/src/surf/surf_routing.hpp
index 3f5d1ad..d11b9ca 100644 (file)
--- a/src/surf/surf_routing.hpp
+++ b/src/surf/surf_routing.hpp
@@ -82,6 +82,14 @@ private:
   AsImpl *containingAS_;
 };
 
+class AsRoute {
+public:
+  explicit AsRoute(NetCard* gwSrc, NetCard* gwDst) : gw_src(gwSrc), gw_dst(gwDst) {}
+  const NetCard* gw_src;
+  const NetCard* gw_dst;
+  std::vector<Link*> links;
+};
+
 /** @ingroup SURF_routing_interface
  * @brief Link of length 1, alongside with its source and destination. This is mainly useful in the ns3 bindings
  */

diff --git a/teshsuite/simdag/basic-parsing-test/basic-parsing-test-bypass.tesh b/teshsuite/simdag/basic-parsing-test/basic-parsing-test-bypass.tesh
index 2ae691c..4e773be 100644 (file)
--- a/teshsuite/simdag/basic-parsing-test/basic-parsing-test-bypass.tesh
+++ b/teshsuite/simdag/basic-parsing-test/basic-parsing-test-bypass.tesh
@@ -1,7 +1,7 @@
 #! ./tesh
 
 p Testing a bypass ASroute
-! output sort
+
 $ ${bindir:=.}/basic-parsing-test ${srcdir:=.}/examples/platforms/bypassASroute.xml FULL_LINK
 > [0.000000] [xbt_cfg/INFO] Switching to the L07 model to handle parallel tasks.
 > Workstation number: 3, link number: 11
@@ -11,9 +11,11 @@ $ ${bindir:=.}/basic-parsing-test ${srcdir:=.}/examples/platforms/bypassASroute.
 >   Link my_cluster_1_link_1_DOWN: latency = 0.000050, bandwidth = 125000000.000000
 >   Route latency = 0.000100, route bandwidth = 125000000.000000
 > Route between 1 and 2
->   Route size 1
+>   Route size 3
+>   Link my_cluster_1_link_1_UP: latency = 0.000050, bandwidth = 125000000.000000
 >   Link link_tmp: latency = 0.000500, bandwidth = 1250000000.000000
->   Route latency = 0.000500, route bandwidth = 1250000000.000000
+>   Link my_cluster_2_link_2_DOWN: latency = 0.000050, bandwidth = 125000000.000000
+>   Route latency = 0.000600, route bandwidth = 125000000.000000
 > Route between 1 and 3
 >   Route size 4
 >   Link my_cluster_1_link_1_UP: latency = 0.000050, bandwidth = 125000000.000000

diff --git a/teshsuite/simdag/flatifier/flatifier.tesh b/teshsuite/simdag/flatifier/flatifier.tesh
index 53c8a79..2f27faa 100644 (file)
--- a/teshsuite/simdag/flatifier/flatifier.tesh
+++ b/teshsuite/simdag/flatifier/flatifier.tesh
@@ -577,13 +577,13 @@ $ ${bindir:=.}/flatifier$EXEEXT ${srcdir:=.}/examples/platforms/bypassASroute.xm
 >   <link_ctn id="my_cluster_1_link_1_UP"/><link_ctn id="my_cluster_1_link_1_DOWN"/>
 >   </route>
 >   <route src="1" dst="2">
->   <link_ctn id="link_tmp"/>
+>   <link_ctn id="my_cluster_1_link_1_UP"/><link_ctn id="link_tmp"/><link_ctn id="my_cluster_2_link_2_DOWN"/>
 >   </route>
 >   <route src="1" dst="3">
 >   <link_ctn id="my_cluster_1_link_1_UP"/><link_ctn id="link1"/><link_ctn id="link3"/><link_ctn id="my_cluster_3_link_3_DOWN"/>
 >   </route>
 >   <route src="1" dst="my_cluster_2_router">
->   <link_ctn id="link_tmp"/>
+>   <link_ctn id="my_cluster_1_link_1_UP"/><link_ctn id="link_tmp"/>
 >   </route>
 >   <route src="1" dst="my_cluster_1_router">
 >   <link_ctn id="my_cluster_1_link_1_UP"/>
@@ -658,7 +658,7 @@ $ ${bindir:=.}/flatifier$EXEEXT ${srcdir:=.}/examples/platforms/bypassASroute.xm
 >   <link_ctn id="my_cluster_1_link_1_DOWN"/>
 >   </route>
 >   <route src="my_cluster_1_router" dst="2">
->   <link_ctn id="link_tmp"/>
+>   <link_ctn id="link_tmp"/><link_ctn id="my_cluster_2_link_2_DOWN"/>
 >   </route>
 >   <route src="my_cluster_1_router" dst="3">
 >   <link_ctn id="link1"/><link_ctn id="link3"/><link_ctn id="my_cluster_3_link_3_DOWN"/>