From 9cbdbf1d739774e5c49488d42a2b5d535c7a8465 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 14 Feb 2016 22:31:27 +0100 Subject: [PATCH] further cleanups in routing: check if route already exists uniformly --- src/surf/surf_routing_dijkstra.cpp | 25 ++--- src/surf/surf_routing_floyd.cpp | 91 ++++++++----------- src/surf/surf_routing_full.cpp | 50 +++------- .../platforms/bogus_two_hosts_asymetric.tesh | 6 +- 4 files changed, 64 insertions(+), 108 deletions(-) diff --git a/src/surf/surf_routing_dijkstra.cpp b/src/surf/surf_routing_dijkstra.cpp index b6fae6e006..1e9a3196c4 100644 --- a/src/surf/surf_routing_dijkstra.cpp +++ b/src/surf/surf_routing_dijkstra.cpp @@ -378,10 +378,10 @@ AsDijkstra::AsDijkstra(const char*name, bool cached) void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) { - const char *src = route->src; - const char *dst = route->dst; - NetCard *src_net_elm = sg_netcard_by_name_or_null(src); - NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); + const char *srcName = route->src; + const char *dstName = route->dst; + NetCard *src = sg_netcard_by_name_or_null(srcName); + NetCard *dst = sg_netcard_by_name_or_null(dstName); parseRouteCheckParams(route); @@ -391,23 +391,26 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) if(!graphNodeMap_) graphNodeMap_ = xbt_dict_new_homogeneous(&graph_node_map_elem_free); + /* we don't check whether the route already exist, because the algorithm may find another path through some other nodes */ + + /* Add the route to the base */ sg_platf_route_cbarg_t e_route = newExtendedRoute(hierarchy_, route, 1); - newRoute(src_net_elm->id(), dst_net_elm->id(), e_route); + newRoute(src->id(), dst->id(), e_route); // Symmetrical YES if (route->symmetrical == TRUE) { if(!route->gw_dst && !route->gw_src) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst, src); + XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dstName, srcName); else - XBT_DEBUG("Load ASroute from %s@%s to %s@%s", dst, route->gw_dst->name(), src, route->gw_src->name()); + XBT_DEBUG("Load ASroute from %s@%s to %s@%s", dstName, route->gw_dst->name(), srcName, route->gw_src->name()); xbt_dynar_t nodes = xbt_graph_get_nodes(routeGraph_); - xbt_node_t node_s_v = xbt_dynar_get_as(nodes, src_net_elm->id(), xbt_node_t); - xbt_node_t node_e_v = xbt_dynar_get_as(nodes, dst_net_elm->id(), xbt_node_t); + xbt_node_t node_s_v = xbt_dynar_get_as(nodes, src->id(), xbt_node_t); + xbt_node_t node_e_v = xbt_dynar_get_as(nodes, dst->id(), xbt_node_t); xbt_edge_t edge = xbt_graph_get_edge(routeGraph_, node_e_v, node_s_v); if (edge) - THROWF(arg_error,0, "Route from %s@%s to %s@%s already exists", dst, route->gw_dst->name(), src, route->gw_src->name()); + THROWF(arg_error,0, "Route from %s@%s to %s@%s already exists", dstName, route->gw_dst->name(), srcName, route->gw_src->name()); if (route->gw_dst && route->gw_src) { NetCard *gw_tmp = route->gw_src; @@ -415,7 +418,7 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) route->gw_dst = gw_tmp; } sg_platf_route_cbarg_t link_route_back = newExtendedRoute(hierarchy_, route, 0); - newRoute(dst_net_elm->id(), src_net_elm->id(), link_route_back); + newRoute(dst->id(), src->id(), link_route_back); } xbt_dynar_free(&route->link_list); } diff --git a/src/surf/surf_routing_floyd.cpp b/src/surf/surf_routing_floyd.cpp index f89b2293ac..487460ca9c 100644 --- a/src/surf/surf_routing_floyd.cpp +++ b/src/surf/surf_routing_floyd.cpp @@ -131,68 +131,50 @@ static int floyd_pointer_resource_cmp(const void *a, const void *b) { void AsFloyd::parseRoute(sg_platf_route_cbarg_t route) { - const char *src = route->src; - const char *dst = route->dst; - /* set the size of table routing */ int table_size = (int)xbt_dynar_length(vertices_); - NetCard *src_net_elm = sg_netcard_by_name_or_null(src); - NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); + NetCard *src = sg_netcard_by_name_or_null(route->src); + NetCard *dst = sg_netcard_by_name_or_null(route->dst); parseRouteCheckParams(route); - if(!linkTable_) - { - int i,j; + if(!linkTable_) { /* Create Cost, Predecessor and Link tables */ costTable_ = xbt_new0(double, table_size * table_size); /* link cost from host to host */ predecessorTable_ = xbt_new0(int, table_size * table_size); /* predecessor host numbers */ linkTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size); /* actual link between src and dst */ /* Initialize costs and predecessors */ - for (i = 0; i < table_size; i++) - for (j = 0; j < table_size; j++) { + for (int i = 0; i < table_size; i++) + for (int j = 0; j < table_size; j++) { TO_FLOYD_COST(i, j) = DBL_MAX; TO_FLOYD_PRED(i, j) = -1; - TO_FLOYD_LINK(i, j) = NULL; /* fixed, missing in the previous version */ + TO_FLOYD_LINK(i, j) = NULL; } } - if(TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id())) - { - - char * link_name; - unsigned int cpt; - xbt_dynar_t link_route_to_test = xbt_dynar_new(sizeof(Link*), NULL); - xbt_dynar_foreach(route->link_list,cpt,link_name) - { - void *link = Link::byName(link_name); - xbt_assert(link,"Link : '%s' doesn't exists.",link_name); - xbt_dynar_push(link_route_to_test,&link); - } - xbt_assert(!xbt_dynar_compare( - TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id())->link_list, - link_route_to_test, - (int_f_cpvoid_cpvoid_t) floyd_pointer_resource_cmp), - "The route between \"%s\" and \"%s\" already exists", src,dst); - } + /* Check that the route does not already exist */ + if (route->gw_dst) // AS route (to adapt the error message, if any) + xbt_assert(nullptr == TO_FLOYD_LINK(src->id(), dst->id()), + "The route between %s@%s and %s@%s already exists (Rq: routes are symmetrical by default).", + src->name(),route->gw_src->name(),dst->name(),route->gw_dst->name()); else - { - TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id()) = - newExtendedRoute(hierarchy_, route, 1); - TO_FLOYD_PRED(src_net_elm->id(), dst_net_elm->id()) = src_net_elm->id(); - TO_FLOYD_COST(src_net_elm->id(), dst_net_elm->id()) = - ((TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id()))->link_list)->used; /* count of links, old model assume 1 */ - } + xbt_assert(nullptr == TO_FLOYD_LINK(src->id(), dst->id()), + "The route between %s and %s already exists (Rq: routes are symmetrical by default).", src->name(),dst->name()); + + TO_FLOYD_LINK(src->id(), dst->id()) = newExtendedRoute(hierarchy_, route, 1); + TO_FLOYD_PRED(src->id(), dst->id()) = src->id(); + TO_FLOYD_COST(src->id(), dst->id()) = ((TO_FLOYD_LINK(src->id(), dst->id()))->link_list)->used; + if (route->symmetrical == TRUE) { - if(TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id())) + if(TO_FLOYD_LINK(dst->id(), src->id())) { if(!route->gw_dst && !route->gw_src) - XBT_DEBUG("See Route from \"%s\" to \"%s\"", dst, src); + XBT_DEBUG("See Route from \"%s\" to \"%s\"", dst->name(), src->name()); else - XBT_DEBUG("See ASroute from \"%s(%s)\" to \"%s(%s)\"", dst, route->gw_src->name(), src, route->gw_dst->name()); + XBT_DEBUG("See ASroute from \"%s(%s)\" to \"%s(%s)\"", dst->name(), route->gw_src->name(), src->name(), route->gw_dst->name()); char * link_name; xbt_dynar_t link_route_to_test = xbt_dynar_new(sizeof(Link*), NULL); @@ -203,10 +185,10 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route) xbt_dynar_push(link_route_to_test,&link); } xbt_assert(!xbt_dynar_compare( - TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id())->link_list, + TO_FLOYD_LINK(dst->id(), src->id())->link_list, link_route_to_test, (int_f_cpvoid_cpvoid_t) floyd_pointer_resource_cmp), - "The route between \"%s\" and \"%s\" already exists", src,dst); + "The route between \"%s\" and \"%s\" already exists", src->name(),dst->name()); } else { @@ -217,23 +199,22 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route) } if(!route->gw_src && !route->gw_dst) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst, src); + XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst->name(), src->name()); else - XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst, - route->gw_src->name(), src, route->gw_dst->name()); + XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst->name(), + route->gw_src->name(), src->name(), route->gw_dst->name()); - TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id()) = + TO_FLOYD_LINK(dst->id(), src->id()) = newExtendedRoute(hierarchy_, route, 0); - TO_FLOYD_PRED(dst_net_elm->id(), src_net_elm->id()) = dst_net_elm->id(); - TO_FLOYD_COST(dst_net_elm->id(), src_net_elm->id()) = - ((TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id()))->link_list)->used; /* count of links, old model assume 1 */ + TO_FLOYD_PRED(dst->id(), src->id()) = dst->id(); + TO_FLOYD_COST(dst->id(), src->id()) = + ((TO_FLOYD_LINK(dst->id(), src->id()))->link_list)->used; /* count of links, old model assume 1 */ } } xbt_dynar_free(&route->link_list); } void AsFloyd::Seal(){ - unsigned int i, j, a, b, c; /* set the size of table routing */ size_t table_size = xbt_dynar_length(vertices_); @@ -245,8 +226,8 @@ void AsFloyd::Seal(){ linkTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size); /* actual link between src and dst */ /* Initialize costs and predecessors */ - for (i = 0; i < table_size; i++) - for (j = 0; j < table_size; j++) { + for (unsigned int i = 0; i < table_size; i++) + for (unsigned int j = 0; j < table_size; j++) { TO_FLOYD_COST(i, j) = DBL_MAX; TO_FLOYD_PRED(i, j) = -1; TO_FLOYD_LINK(i, j) = NULL; @@ -255,7 +236,7 @@ void AsFloyd::Seal(){ /* Add the loopback if needed */ if (routing_platf->loopback_ && hierarchy_ == SURF_ROUTING_BASE) { - for (i = 0; i < table_size; i++) { + for (unsigned int i = 0; i < table_size; i++) { sg_platf_route_cbarg_t e_route = TO_FLOYD_LINK(i, i); if (!e_route) { e_route = xbt_new0(s_sg_platf_route_cbarg_t, 1); @@ -270,9 +251,9 @@ void AsFloyd::Seal(){ } } /* Calculate path costs */ - for (c = 0; c < table_size; c++) { - for (a = 0; a < table_size; a++) { - for (b = 0; b < table_size; b++) { + for (unsigned int c = 0; c < table_size; c++) { + for (unsigned int a = 0; a < table_size; a++) { + for (unsigned int b = 0; b < table_size; b++) { if (TO_FLOYD_COST(a, c) < DBL_MAX && TO_FLOYD_COST(c, b) < DBL_MAX) { if (TO_FLOYD_COST(a, b) == DBL_MAX || (TO_FLOYD_COST(a, c) + TO_FLOYD_COST(c, b) < diff --git a/src/surf/surf_routing_full.cpp b/src/surf/surf_routing_full.cpp index 9cf68f16ed..427af14b3f 100644 --- a/src/surf/surf_routing_full.cpp +++ b/src/surf/surf_routing_full.cpp @@ -144,44 +144,18 @@ void AsFull::parseRoute(sg_platf_route_cbarg_t route) if (!routingTable_) routingTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size); - if (TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())) { - char *link_name; - unsigned int i; - xbt_dynar_t link_route_to_test = - xbt_dynar_new(sizeof(Link*), NULL); - xbt_dynar_foreach(route->link_list, i, link_name) { - void *link = Link::byName(link_name); - xbt_assert(link, "Link : '%s' doesn't exists.", link_name); - xbt_dynar_push(link_route_to_test, &link); - } - if (xbt_dynar_compare(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list, - link_route_to_test, full_pointer_resource_cmp)) { - surf_parse_error("A route between \"%s\" and \"%s\" already exists " - "with a different content. " - "If you are trying to define a reverse route, " - "you must set the symmetrical=no attribute to " - "your routes tags.", src, dst); - } else { - surf_parse_warn("Ignoring the identical redefinition of the route " - "between \"%s\" and \"%s\"", src, dst); - } - } else { - if (!route->gw_src && !route->gw_dst) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst); - else { - XBT_DEBUG("Load ASroute from \"%s\" to \"%s\"", src, dst); - if (!route->gw_src || route->gw_src->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The src_gateway \"%s\" does not exist!", - route->gw_src ? route->gw_src->name() : "(null)"); - if (!route->gw_dst || route->gw_dst->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The dst_gateway \"%s\" does not exist!", - route->gw_dst ? route->gw_dst->name() : "(null)"); - XBT_DEBUG("ASroute goes from \"%s\" to \"%s\"", - route->gw_src->name(), route->gw_dst->name()); - } - TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()) = newExtendedRoute(hierarchy_, route, 1); - xbt_dynar_shrink(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list, 0); - } + /* Check that the route does not already exist */ + if (route->gw_dst) // AS route (to adapt the error message, if any) + xbt_assert(nullptr == TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()), + "The route between %s@%s and %s@%s already exists (Rq: routes are symmetrical by default).", + src,route->gw_src->name(),dst,route->gw_dst->name()); + else + xbt_assert(nullptr == TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()), + "The route between %s and %s already exists (Rq: routes are symmetrical by default).", src,dst); + + /* Add the route to the base */ + TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()) = newExtendedRoute(hierarchy_, route, 1); + xbt_dynar_shrink(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list, 0); if (route->symmetrical == TRUE) { if (route->gw_dst && route->gw_src) { diff --git a/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh b/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh index 2403429d87..efcf85d071 100644 --- a/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh +++ b/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh @@ -1,11 +1,9 @@ ! expect signal SIGABRT $ ${bindir:=.}/flatifier bogus_two_hosts_asymetric.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n" > [ 0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks. -> [ 0.000000] [0:maestro@] Parse error at bogus_two_hosts_asymetric.xml:24: A route between "alice" and "bob" already exists with a different content. If you are trying to define a reverse route, you must set the symmetrical=no attribute to your routes tags. -> [ 0.000000] [0:maestro@] Exiting now +> [ 0.000000] [0:maestro@] Error while loading bogus_two_hosts_asymetric.xml: The route between alice and bob already exists (Rq: routes are symmetrical by default). ! expect signal SIGABRT $ ${bindir:=.}/flatifier bogus_two_hosts_asymetric-2.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n" > [ 0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks. -> [ 0.000000] [0:maestro@] Parse error at bogus_two_hosts_asymetric-2.xml:26: A route between "alice" and "bob" already exists with a different content. If you are trying to define a reverse route, you must set the symmetrical=no attribute to your routes tags. -> [ 0.000000] [0:maestro@] Exiting now +> [ 0.000000] [0:maestro@] Error while loading bogus_two_hosts_asymetric-2.xml: The route between alice and bob already exists (Rq: routes are symmetrical by default). -- 2.20.1