- simplify on-demand linking in overlay connects

5 files changed, 145 insertions, 194 deletions

diff --git a/src/testbed/gnunet-service-testbed.h b/src/testbed/gnunet-service-testbed.h
index b17c3d781..149a1ad77 100644
--- a/src/testbed/gnunet-service-testbed.h
+++ b/src/testbed/gnunet-service-testbed.h
@@ -318,6 +318,11 @@ struct ForwardedOverlayConnectContext
   struct GNUNET_MessageHeader *orig_msg;
 
   /**
+   * The client handle
+   */
+  struct GNUNET_SERVER_Client *client;
+
+  /**
    * The id of the operation which created this context information
    */
   uint64_t operation_id;
@@ -363,11 +368,6 @@ enum ClosureType
 struct RegisteredHostContext
 {
   /**
-   * The type of this data structure. Set this to CLOSURE_TYPE_RHC
-   */
-  enum ClosureType type;
-
-  /**
    * The host which is being registered
    */
   struct GNUNET_TESTBED_Host *reg_host;
@@ -378,26 +378,6 @@ struct RegisteredHostContext
   struct GNUNET_TESTBED_Host *host;
 
   /**
-   * The gateway to which this operation is forwarded to
-   */
-  struct Slave *gateway;
-
-  /**
-   * The gateway through which peer2's controller can be reached
-   */
-  struct Slave *gateway2;
-
-  /**
-   * Handle for sub-operations
-   */
-  struct GNUNET_TESTBED_Operation *sub_op;
-
-  /**
-   * The client which initiated the link controller operation
-   */
-  struct GNUNET_SERVER_Client *client;
-
-  /**
    * Head of the ForwardedOverlayConnectContext DLL
    */
   struct ForwardedOverlayConnectContext *focc_dll_head;
@@ -419,20 +399,9 @@ struct RegisteredHostContext
     RHC_INIT = 0,
 
     /**
-     * State where we attempt to get peer2's controller configuration
-     */
-    RHC_GET_CFG,
-
-    /**
-     * State where we attempt to link the controller of peer 1 to the controller
-     * of peer2
-     */
-    RHC_LINK,
-
-    /**
      * State where we attempt to do the overlay connection again
      */
-    RHC_OL_CONNECT
+    RHC_DONE
   } state;
 
 };
@@ -512,6 +481,18 @@ extern unsigned int GST_host_list_size;
  */
 extern char *GST_stats_dir;
 
+/**
+ * Condition to check if host id is invalid
+ */
+#define INVALID_HOST_ID(id) \
+  ( ((id) >= GST_host_list_size) || (NULL == GST_host_list[id]) )
+
+/**
+ * Condition to check if peer id is invalid
+ */
+#define INVALID_PEER_ID(id) \
+  ( ((id) >= GST_peer_list_size) || (NULL == GST_peer_list[id]) )
+
 
 /**
  * Similar to GNUNET_array_grow(); however instead of calling GNUNET_array_grow()
diff --git a/src/testbed/gnunet-service-testbed_links.c b/src/testbed/gnunet-service-testbed_links.c
index 6209bf248..1da071660 100644
--- a/src/testbed/gnunet-service-testbed_links.c
+++ b/src/testbed/gnunet-service-testbed_links.c
@@ -326,10 +326,6 @@ reghost_free_iterator (void *cls, const struct GNUNET_HashCode *key,
     GNUNET_CONTAINER_DLL_remove (rhc->focc_dll_head, rhc->focc_dll_tail, focc);
     GST_cleanup_focc (focc);
   }
-  if (NULL != rhc->sub_op)
-    GNUNET_TESTBED_operation_done (rhc->sub_op);
-  if (NULL != rhc->client)
-    GNUNET_SERVER_client_drop (rhc->client);
   GNUNET_free (value);
   return GNUNET_YES;
 }
@@ -618,39 +614,11 @@ static void
 slave_event_callback (void *cls,
                       const struct GNUNET_TESTBED_EventInformation *event)
 {
-  struct RegisteredHostContext *rhc;
   struct LCFContext *lcf;
-  struct GNUNET_TESTBED_Operation *old_op;
 
   /* We currently only get here when working on RegisteredHostContexts and
      LCFContexts */
   GNUNET_assert (GNUNET_TESTBED_ET_OPERATION_FINISHED == event->type);
-  rhc = event->op_cls;
-  if (CLOSURE_TYPE_RHC == rhc->type)
-  {
-    GNUNET_assert (rhc->sub_op == event->op);
-    switch (rhc->state)
-    {
-    case RHC_GET_CFG:
-      old_op = rhc->sub_op;
-      rhc->state = RHC_LINK;
-      rhc->sub_op =
-          GNUNET_TESTBED_controller_link (rhc, rhc->gateway->controller,
-                                          rhc->reg_host, rhc->host, GNUNET_NO);
-      GNUNET_TESTBED_operation_done (old_op);
-      break;
-    case RHC_LINK:
-      LOG_DEBUG ("OL: Linking controllers successfull\n");
-      GNUNET_TESTBED_operation_done (rhc->sub_op);
-      rhc->sub_op = NULL;
-      rhc->state = RHC_OL_CONNECT;
-      GST_process_next_focc (rhc);
-      break;
-    default:
-      GNUNET_assert (0);
-    }
-    return;
-  }
   lcf = event->op_cls;
   if (CLOSURE_TYPE_LCF == lcf->type)
   {    
@@ -955,6 +923,18 @@ neighbour_connect_cb (void *cls, struct GNUNET_TESTBED_Controller *c)
   cleanup_ncc (ncc);
 }
 
+struct Neighbour *
+GST_create_neighbour (struct GNUNET_TESTBED_Host *host)
+{
+  struct Neighbour *n;
+
+  n = GNUNET_malloc (sizeof (struct Neighbour));
+  n->host_id = GNUNET_TESTBED_host_get_id_ (host);
+  neighbour_list_add (n);   /* just add; connect on-demand */
+  return n;
+}
+
+
 /**
  * Message handler for GNUNET_MESSAGE_TYPE_TESTBED_LCONTROLLERS message
  *
@@ -1033,9 +1013,7 @@ GST_handle_link_controllers (void *cls, struct GNUNET_SERVER_Client *client,
       }
       LOG_DEBUG ("Received request to establish a link to host %u\n",
                  delegated_host_id);
-      n = GNUNET_malloc (sizeof (struct Neighbour));
-      n->host_id = delegated_host_id;
-      neighbour_list_add (n);   /* just add; connect on-demand */
+      n = GST_create_neighbour (GST_host_list[delegated_host_id]);
       ncc = GNUNET_malloc (sizeof (struct NeighbourConnectCtxt));
       ncc->n = n;
       ncc->op_id = op_id;
diff --git a/src/testbed/gnunet-service-testbed_links.h b/src/testbed/gnunet-service-testbed_links.h
index b6a38c09e..65741c21c 100644
--- a/src/testbed/gnunet-service-testbed_links.h
+++ b/src/testbed/gnunet-service-testbed_links.h
@@ -119,6 +119,9 @@ GST_neighbour_get_connection_cancel (struct NeighbourConnectNotification *h);
 void
 GST_neighbour_release_connection (struct Neighbour *n);
 
+struct Neighbour *
+GST_create_neighbour (struct GNUNET_TESTBED_Host *host);
+
 /**
  * Message handler for GNUNET_MESSAGE_TYPE_TESTBED_LCONTROLLERS message
  *
diff --git a/src/testbed/gnunet-service-testbed_oc.c b/src/testbed/gnunet-service-testbed_oc.c
index c53bec007..744ae3d34 100644
--- a/src/testbed/gnunet-service-testbed_oc.c
+++ b/src/testbed/gnunet-service-testbed_oc.c
@@ -364,6 +364,7 @@ static struct RemoteOverlayConnectCtx *roccq_tail;
 void
 GST_cleanup_focc (struct ForwardedOverlayConnectContext *focc)
 {
+  GNUNET_SERVER_client_drop (focc->client);  
   GNUNET_free_non_null (focc->orig_msg);
   GNUNET_free (focc);
 }
@@ -387,10 +388,10 @@ forwarded_overlay_connect_timeout (void *cls,
   rhc = fopc->cls;
   focc = rhc->focc_dll_head;
   GNUNET_CONTAINER_DLL_remove (rhc->focc_dll_head, rhc->focc_dll_tail, focc);
-  GST_cleanup_focc (focc);
   LOG_DEBUG ("Overlay linking between peers %u and %u failed\n", focc->peer1,
              focc->peer2);
-  GST_forwarded_operation_timeout (cls, tc);
+  GST_cleanup_focc (focc);
+  GST_forwarded_operation_timeout (fopc, tc);
   if (NULL != rhc->focc_dll_head)
     GST_process_next_focc (rhc);
 }
@@ -432,18 +433,24 @@ GST_process_next_focc (struct RegisteredHostContext *rhc)
 {
   struct ForwardedOperationContext *fopc;
   struct ForwardedOverlayConnectContext *focc;
+  struct Peer *peer;
+  struct Slave *slave;
 
   focc = rhc->focc_dll_head;
   GNUNET_assert (NULL != focc);
-  GNUNET_assert (RHC_OL_CONNECT == rhc->state);
+  GNUNET_assert (RHC_DONE == rhc->state);
+  GNUNET_assert (!INVALID_PEER_ID (focc->peer1));
+  peer = GST_peer_list[focc->peer1];
+  GNUNET_assert (GNUNET_YES == peer->is_remote);
+  GNUNET_assert (NULL != (slave = peer->details.remote.slave));
   fopc = GNUNET_malloc (sizeof (struct ForwardedOperationContext));
-  GNUNET_SERVER_client_keep (rhc->client);
-  fopc->client = rhc->client;
+  GNUNET_SERVER_client_keep (focc->client);
+  fopc->client = focc->client;
   fopc->operation_id = focc->operation_id;
   fopc->cls = rhc;
   fopc->type = OP_OVERLAY_CONNECT;
   fopc->opc =
-      GNUNET_TESTBED_forward_operation_msg_ (rhc->gateway->controller,
+      GNUNET_TESTBED_forward_operation_msg_ (slave->controller,
                                              focc->operation_id, focc->orig_msg,
                                              &forwarded_overlay_connect_listener,
                                              fopc);
@@ -1135,28 +1142,12 @@ overlay_connect_get_config (void *cls, const struct GNUNET_MessageHeader *msg)
  * @param emsg the error message; NULL if host registration is successful
  */
 static void
-registeredhost_registration_completion (void *cls, const char *emsg)
+host_registration_comp (void *cls, const char *emsg)
 {
   struct RegisteredHostContext *rhc = cls;
-  uint32_t peer2_host_id;
-
-  peer2_host_id = GNUNET_TESTBED_host_get_id_ (rhc->reg_host);
-  GNUNET_assert (RHC_INIT == rhc->state);
-  GNUNET_assert (NULL == rhc->sub_op);
-  if ((NULL == rhc->gateway2) || 
-      ( (peer2_host_id < GST_host_list_size) /* Check if we have the needed config */
-        && (NULL != GST_host_list[peer2_host_id]) )  )
-  {
-    rhc->state = RHC_LINK;
-    rhc->sub_op =
-        GNUNET_TESTBED_controller_link (rhc, rhc->gateway->controller,
-                                        rhc->reg_host, rhc->host, GNUNET_NO);
-    return;
-  }
-  rhc->state = RHC_GET_CFG;
-  rhc->sub_op =
-      GNUNET_TESTBED_get_slave_config (rhc, rhc->gateway2->controller,
-                                       rhc->reg_host);
+  
+  rhc->state = RHC_DONE;
+  GST_process_next_focc (rhc);
 }
 
 
@@ -1208,6 +1199,47 @@ hash_hosts (struct GNUNET_TESTBED_Host *reg_host,
 }
 
 
+static struct RegisteredHostContext *
+register_p2_host (struct Slave *slave, uint32_t peer2_host_id)
+{
+  struct GNUNET_HashCode hash;
+  struct RegisteredHostContext *rhc;
+  
+  rhc = GNUNET_malloc (sizeof (struct RegisteredHostContext));
+  rhc->reg_host = GST_host_list[peer2_host_id];
+  rhc->host = GST_host_list[slave->host_id];
+  GNUNET_assert (NULL != rhc->reg_host);
+  GNUNET_assert (NULL != rhc->host);
+  rhc->state = RHC_INIT;
+  hash = hash_hosts (rhc->reg_host, rhc->host);
+  if ((GNUNET_NO ==
+       GNUNET_CONTAINER_multihashmap_contains (slave->reghost_map, &hash)) ||
+      (GNUNET_SYSERR !=
+       GNUNET_CONTAINER_multihashmap_get_multiple (slave->reghost_map,
+                                                   &hash,
+                                                   reghost_match_iterator,
+                                                   &rhc)))
+  {
+    /* create and add a new registerd host context */
+    /* add the focc to its queue */
+    GNUNET_CONTAINER_multihashmap_put (slave->reghost_map, &hash, rhc,
+                                       GNUNET_CONTAINER_MULTIHASHMAPOPTION_MULTIPLE);
+    GST_queue_host_registration (slave, host_registration_comp,
+                                 rhc, rhc->reg_host);
+  }
+  else
+  {
+    /* rhc is now set to the existing one from the hash map by
+     * reghost_match_iterator() */
+    /* if queue is empty then ignore creating focc and proceed with normal
+     * forwarding */
+    if (RHC_DONE == rhc->state)
+      return NULL;
+  }
+  return rhc;
+}
+
+
 /**
  * Forwards the overlay connect request to a slave controller.  Before
  * forwarding, any hosts which are needed to be known by the slave controller to
@@ -1225,6 +1257,8 @@ forward_overlay_connect (const struct GNUNET_TESTBED_OverlayConnectMessage *msg,
   struct Route *route_to_peer2_host;
   struct Route *route_to_peer1_host;
   struct Peer *peer;
+  struct RegisteredHostContext *rhc;
+  struct ForwardedOverlayConnectContext *focc;
   uint64_t op_id;
   uint32_t peer2_host_id;
   uint32_t p1;
@@ -1234,96 +1268,39 @@ forward_overlay_connect (const struct GNUNET_TESTBED_OverlayConnectMessage *msg,
   p2 = ntohl (msg->peer2);
   op_id = GNUNET_ntohll (msg->operation_id);
   peer2_host_id = ntohl (msg->peer2_host_id);
-  GNUNET_assert (p1 < GST_peer_list_size);
-  GNUNET_assert (NULL != (peer = GST_peer_list[p1]));
+  GNUNET_assert (! INVALID_PEER_ID (p1));
+  GNUNET_assert (! INVALID_HOST_ID (peer2_host_id));
+  peer = GST_peer_list[p1];
   GNUNET_assert (GNUNET_YES == peer->is_remote);
   LOG_DEBUG ("0x%llx: Forwarding overlay connect\n", op_id);
-  route_to_peer2_host = NULL;
-  route_to_peer1_host = NULL;
   route_to_peer2_host = GST_find_dest_route (peer2_host_id);
-  if ((NULL != route_to_peer2_host) ||
-      (peer2_host_id == GST_context->host_id))
+  route_to_peer1_host = GST_find_dest_route
+      (peer->details.remote.remote_host_id);
+  GNUNET_assert (NULL != route_to_peer1_host);
+  if ((NULL != route_to_peer2_host) && 
+      (route_to_peer1_host->dest == route_to_peer2_host->dest))
+    goto forward;
+  /* Peer2 is either with us OR peer1 and peer2 can be reached through
+     different subtrees OR peer2 is on a subtree unknown to us */
+  if (NULL != (rhc = register_p2_host (peer->details.remote.slave,
+                                       peer2_host_id)))
   {
-    /* Peer 2 either below us OR with us */
-    route_to_peer1_host =
-        GST_find_dest_route (GST_peer_list[p1]->details.
-                             remote.remote_host_id);
-    /* Because we get this message only if we know where peer 1 is */
-    GNUNET_assert (NULL != route_to_peer1_host);
-    if ((peer2_host_id == GST_context->host_id) ||
-        (route_to_peer2_host->dest != route_to_peer1_host->dest))
-    {
-      /* Peer2 is either with us OR peer1 and peer2 can be reached through
-       * different gateways */
-      struct GNUNET_HashCode hash;
-      struct RegisteredHostContext *rhc;
-      int skip_focc;
-
-      rhc = GNUNET_malloc (sizeof (struct RegisteredHostContext));
-      rhc->type = CLOSURE_TYPE_RHC;
-      if (NULL != route_to_peer2_host)
-        rhc->reg_host = GST_host_list[route_to_peer2_host->dest];
-      else
-        rhc->reg_host = GST_host_list[GST_context->host_id];
-      rhc->host = GST_host_list[route_to_peer1_host->dest];
-      GNUNET_assert (NULL != rhc->reg_host);
-      GNUNET_assert (NULL != rhc->host);
-      rhc->gateway = peer->details.remote.slave;
-      rhc->gateway2 =
-          (NULL ==
-           route_to_peer2_host) ? NULL :
-          GST_slave_list[route_to_peer2_host->dest];
-      rhc->state = RHC_INIT;
-      GNUNET_SERVER_client_keep (client);
-      rhc->client = client;
-      hash = hash_hosts (rhc->reg_host, rhc->host);
-      skip_focc = GNUNET_NO;
-      if ((GNUNET_NO ==
-           GNUNET_CONTAINER_multihashmap_contains (peer->details.
-                                                   remote.slave->reghost_map,
-                                                   &hash)) ||
-          (GNUNET_SYSERR !=
-           GNUNET_CONTAINER_multihashmap_get_multiple (peer->details.remote.
-                                                       slave->reghost_map,
-                                                       &hash,
-                                                       reghost_match_iterator,
-                                                       &rhc)))
-      {
-        /* create and add a new registerd host context */
-        /* add the focc to its queue */
-        GNUNET_CONTAINER_multihashmap_put (peer->details.remote.
-                                           slave->reghost_map, &hash, rhc,
-                                           GNUNET_CONTAINER_MULTIHASHMAPOPTION_MULTIPLE);
-        GNUNET_assert (NULL != GST_host_list[peer2_host_id]);
-        GST_queue_host_registration (peer->details.remote.slave,
-                                     registeredhost_registration_completion,
-                                     rhc, GST_host_list[peer2_host_id]);
-      }
-      else
-      {
-        /* rhc is now set to the existing one from the hash map by
-         * reghost_match_iterator() */
-        /* if queue is empty then ignore creating focc and proceed with
-         * normal forwarding */
-        if (RHC_OL_CONNECT == rhc->state)
-          skip_focc = GNUNET_YES;
-      }
-      if (GNUNET_NO == skip_focc)
-      {
-        struct ForwardedOverlayConnectContext *focc;
-
-        focc = GNUNET_malloc (sizeof (struct ForwardedOverlayConnectContext));
-        focc->peer1 = p1;
-        focc->peer2 = p2;
-        focc->peer2_host_id = peer2_host_id;
-        focc->orig_msg = GNUNET_copy_message (&msg->header);
-        focc->operation_id = op_id;
-        GNUNET_CONTAINER_DLL_insert_tail (rhc->focc_dll_head,
-                                          rhc->focc_dll_tail, focc);
-        return;
-      }
-    }
+    LOG_DEBUG ("Forwarding with FOCC for connecting peers %u and %u\n", p1, p2);
+    focc = GNUNET_malloc (sizeof (struct ForwardedOverlayConnectContext));
+    focc->peer1 = p1;
+    focc->peer2 = p2;
+    focc->peer2_host_id = peer2_host_id;
+    focc->orig_msg = GNUNET_copy_message (&msg->header);
+    focc->operation_id = op_id;
+    focc->client = client;
+    GNUNET_SERVER_client_keep (client);
+    GNUNET_CONTAINER_DLL_insert_tail (rhc->focc_dll_head, rhc->focc_dll_tail,
+                                      focc);
+    return;
   }
+
+ forward:
+  LOG_DEBUG ("Forwarding without FOCC for connecting peers %u and %u\n", p1, p2);
   fopc = GNUNET_malloc (sizeof (struct ForwardedOperationContext));
   GNUNET_SERVER_client_keep (client);
   fopc->client = client;
@@ -1409,7 +1386,7 @@ GST_handle_overlay_connect (void *cls, struct GNUNET_SERVER_Client *client,
   msg = (const struct GNUNET_TESTBED_OverlayConnectMessage *) message;
   p1 = ntohl (msg->peer1);
   p2 = ntohl (msg->peer2);
-  if ((p1 >= GST_peer_list_size) || (NULL == GST_peer_list[p1]))
+  if INVALID_PEER_ID (p1)
   {
     GNUNET_break (0);
     GNUNET_SERVER_receive_done (client, GNUNET_SYSERR);
@@ -1420,8 +1397,15 @@ GST_handle_overlay_connect (void *cls, struct GNUNET_SERVER_Client *client,
   LOG_DEBUG
       ("Received overlay connect for peers %u and %u with op id: 0x%llx\n", p1,
        p2, operation_id);
+  peer2_host_id = ntohl (msg->peer2_host_id);
   if (GNUNET_YES == peer->is_remote)
   {
+    if INVALID_HOST_ID (peer2_host_id)
+    {
+      GNUNET_break (0);
+      GNUNET_SERVER_receive_done (client, GNUNET_SYSERR);
+      return;      
+    }
     forward_overlay_connect (msg, client);
     GNUNET_SERVER_receive_done (client, GNUNET_OK);
     return;
@@ -1429,18 +1413,21 @@ GST_handle_overlay_connect (void *cls, struct GNUNET_SERVER_Client *client,
   p2n = NULL;
   occ = GNUNET_malloc (sizeof (struct OverlayConnectContext));  
   occ->type = OCC_TYPE_LOCAL;
-  peer2_host_id = ntohl (msg->peer2_host_id);
-  if ((p2 >= GST_peer_list_size) || (NULL == GST_peer_list[p2]))
-  {
+  if INVALID_PEER_ID (p2)       /* May be peer2 is on a another controller */
+  {    
     if (NULL == (p2n = GST_get_neighbour (peer2_host_id)))
     {
-      GNUNET_break (0);
-      LOG (GNUNET_ERROR_TYPE_WARNING,
-           "0x%llx: Peer %u's host not in our neighbours list\n",
-           operation_id, p2);
-      GNUNET_SERVER_receive_done (client, GNUNET_SYSERR);
-      GNUNET_free (occ);
-      return;
+      if (INVALID_HOST_ID (peer2_host_id))
+      {
+        GNUNET_break (0);
+        LOG (GNUNET_ERROR_TYPE_WARNING,
+             "0x%llx: Peer %u's host not in our neighbours list\n",
+             operation_id, p2);
+        GNUNET_SERVER_receive_done (client, GNUNET_SYSERR);
+        GNUNET_free (occ);
+        return;
+      }
+      p2n = GST_create_neighbour (GST_host_list[peer2_host_id]);
     }
     occ->type = OCC_TYPE_REMOTE_LATERAL;
     occ->p2ctx.remote.p2n = p2n;
diff --git a/src/testbed/test_testbed_api_controllerlink.c b/src/testbed/test_testbed_api_controllerlink.c
index c5fae9257..c60b8227e 100644
--- a/src/testbed/test_testbed_api_controllerlink.c
+++ b/src/testbed/test_testbed_api_controllerlink.c
@@ -541,6 +541,8 @@ controller_cb (void *cls, const struct GNUNET_TESTBED_EventInformation *event)
     FAIL_TEST (event->details.peer_start.peer == slave3_peer);
     GNUNET_TESTBED_operation_done (op);
     result = SLAVE3_PEER_START_SUCCESS;
+    sleep (1);
+    LOG_DEBUG ("**************************************\n");
     op = GNUNET_TESTBED_overlay_connect (mc, NULL, NULL, slave2_peer,
                                          slave3_peer);
     FAIL_TEST (NULL != op);