[GNUnet-SVN] [gnunet] branch master updated: implement tie-breaking in case both peers establish a connection to each other via the same path at the same time, so that only one connection survives

[gnunet]

This is an automated email from the git hooks/post-receive script.

grothoff pushed a commit to branch master
in repository gnunet.

The following commit(s) were added to refs/heads/master by this push:
     new d3b0b69e7 implement tie-breaking in case both peers establish a 
connection to each other via the same path at the same time, so that only one 
connection survives
d3b0b69e7 is described below

commit d3b0b69e7d360b4b82c5a5580a7ea21317dfee24
Author: Christian Grothoff <address@hidden>
AuthorDate: Fri Jan 27 14:07:49 2017 +0100

    implement tie-breaking in case both peers establish a connection to each 
other via the same path at the same time, so that only one connection survives
---
 src/cadet/gnunet-service-cadet-new_connection.c | 119 +++++++++++++++++++++---
 src/cadet/gnunet-service-cadet-new_connection.h |  21 ++++-
 src/cadet/gnunet-service-cadet-new_core.c       |  32 +++++--
 src/cadet/gnunet-service-cadet-new_paths.c      |   5 -
 src/cadet/gnunet-service-cadet-new_tunnels.c    |  42 +++++++--
 src/cadet/gnunet-service-cadet-new_tunnels.h    |  14 ++-
 6 files changed, 197 insertions(+), 36 deletions(-)

diff --git a/src/cadet/gnunet-service-cadet-new_connection.c 
b/src/cadet/gnunet-service-cadet-new_connection.c
index 5f1ff61e1..a098674f5 100644
--- a/src/cadet/gnunet-service-cadet-new_connection.c
+++ b/src/cadet/gnunet-service-cadet-new_connection.c
@@ -158,31 +158,21 @@ struct CadetConnection
 
 
 /**
- * Destroy a connection.
+ * Destroy a connection, part of the internal implementation.  Called
+ * only from #GCC_destroy_from_core() or #GCC_destroy_from_tunnel().
  *
  * @param cc connection to destroy
  */
-void
+static void
 GCC_destroy (struct CadetConnection *cc)
 {
-  struct GNUNET_MQ_Envelope *env = NULL;
-
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Destroying %s\n",
        GCC_2s (cc));
-  if (CADET_CONNECTION_SENDING_CREATE != cc->state)
-  {
-    struct GNUNET_CADET_ConnectionDestroyMessage *destroy_msg;
-
-    /* Need to notify next hop that we are down. */
-    env = GNUNET_MQ_msg (destroy_msg,
-                         GNUNET_MESSAGE_TYPE_CADET_CONNECTION_DESTROY);
-    destroy_msg->cid = cc->cid;
-  }
   if (NULL != cc->mq_man)
   {
     GCP_request_mq_cancel (cc->mq_man,
-                           env);
+                           NULL);
     cc->mq_man = NULL;
   }
   if (NULL != cc->task)
@@ -206,6 +196,56 @@ GCC_destroy (struct CadetConnection *cc)
 }
 
 
+
+/**
+ * Destroy a connection, called when the CORE layer is already done
+ * (i.e. has received a BROKEN message), but if we still have to
+ * communicate the destruction of the connection to the tunnel (if one
+ * exists).
+ *
+ * @param cc connection to destroy
+ */
+void
+GCC_destroy_without_core (struct CadetConnection *cc)
+{
+  if (NULL != cc->ct)
+  {
+    GCT_connection_lost (cc->ct);
+    cc->ct = NULL;
+  }
+  GCC_destroy (cc);
+}
+
+
+/**
+ * Destroy a connection, called if the tunnel association with the
+ * connection was already broken, but we still need to notify the CORE
+ * layer about the breakage.
+ *
+ * @param cc connection to destroy
+ */
+void
+GCC_destroy_without_tunnel (struct CadetConnection *cc)
+{
+  cc->ct = NULL;
+  if ( (CADET_CONNECTION_SENDING_CREATE != cc->state) &&
+       (NULL != cc->mq_man) )
+  {
+    struct GNUNET_MQ_Envelope *env;
+    struct GNUNET_CADET_ConnectionDestroyMessage *destroy_msg;
+
+    /* Need to notify next hop that we are down. */
+    env = GNUNET_MQ_msg (destroy_msg,
+                         GNUNET_MESSAGE_TYPE_CADET_CONNECTION_DESTROY);
+    destroy_msg->cid = cc->cid;
+    GCP_request_mq_cancel (cc->mq_man,
+                           env);
+    cc->mq_man = NULL;
+  }
+  GCC_destroy (cc);
+}
+
+
 /**
  * Return the tunnel associated with this connection.
  *
@@ -630,7 +670,8 @@ connection_create (struct CadetPeer *destination,
  * @param ct which tunnel uses this connection
  * @param ready_cb function to call when ready to transmit
  * @param ready_cb_cls closure for @a cb
- * @return handle to the connection
+ * @return handle to the connection, NULL if we already have
+ *         a connection that takes precedence on @a path
  */
 struct CadetConnection *
 GCC_create_inbound (struct CadetPeer *destination,
@@ -640,6 +681,54 @@ GCC_create_inbound (struct CadetPeer *destination,
                     GCC_ReadyCallback ready_cb,
                     void *ready_cb_cls)
 {
+  struct CadetConnection *cc;
+  unsigned int off;
+
+  off = GCPP_find_peer (path,
+                        destination);
+  GNUNET_assert (UINT_MAX != off);
+  cc = GCPP_get_connection (path,
+                            destination,
+                            off);
+  if (NULL != cc)
+  {
+    int cmp;
+
+    cmp = memcmp (cid,
+                  &cc->cid,
+                  sizeof (*cid));
+    if (0 == cmp)
+    {
+      /* Two peers picked the SAME random connection identifier at the
+         same time for the same path? Must be malicious.  Drop
+         connection (existing and inbound), even if it is the only
+         one. */
+      GNUNET_break_op (0);
+      GCT_connection_lost (cc->ct);
+      GCC_destroy_without_tunnel (cc);
+      return NULL;
+    }
+    if (0 < cmp)
+    {
+      /* drop existing */
+      LOG (GNUNET_ERROR_TYPE_DEBUG,
+           "Got two connections on %s, dropping my existing %s\n",
+           GCPP_2s (path),
+           GCC_2s (cc));
+      GCT_connection_lost (cc->ct);
+      GCC_destroy_without_tunnel (cc);
+    }
+    else
+    {
+      /* keep existing */
+      LOG (GNUNET_ERROR_TYPE_DEBUG,
+           "Got two connections on %s, keeping my existing %s\n",
+           GCPP_2s (path),
+           GCC_2s (cc));
+      return NULL;
+    }
+  }
+
   return connection_create (destination,
                             path,
                             ct,
diff --git a/src/cadet/gnunet-service-cadet-new_connection.h 
b/src/cadet/gnunet-service-cadet-new_connection.h
index efdf9929a..692d03e4c 100644
--- a/src/cadet/gnunet-service-cadet-new_connection.h
+++ b/src/cadet/gnunet-service-cadet-new_connection.h
@@ -47,12 +47,26 @@ typedef void
 
 
 /**
- * Destroy a connection.
+ * Destroy a connection, called when the CORE layer is already done
+ * (i.e. has received a BROKEN message), but if we still have to
+ * communicate the destruction of the connection to the tunnel (if one
+ * exists).
  *
  * @param cc connection to destroy
  */
 void
-GCC_destroy (struct CadetConnection *cc);
+GCC_destroy_without_core (struct CadetConnection *cc);
+
+
+/**
+ * Destroy a connection, called if the tunnel association with the
+ * connection was already broken, but we still need to notify the CORE
+ * layer about the breakage.
+ *
+ * @param cc connection to destroy
+ */
+void
+GCC_destroy_without_tunnel (struct CadetConnection *cc);
 
 
 /**
@@ -84,7 +98,8 @@ GCC_create (struct CadetPeer *destination,
  * @param ct which tunnel uses this connection
  * @param ready_cb function to call when ready to transmit
  * @param ready_cb_cls closure for @a cb
- * @return handle to the connection
+ * @return handle to the connection, NULL if we already have
+ *         a connection that takes precedence on @a path
  */
 struct CadetConnection *
 GCC_create_inbound (struct CadetPeer *destination,
diff --git a/src/cadet/gnunet-service-cadet-new_core.c 
b/src/cadet/gnunet-service-cadet-new_core.c
index 4a4ead05b..086337c9a 100644
--- a/src/cadet/gnunet-service-cadet-new_core.c
+++ b/src/cadet/gnunet-service-cadet-new_core.c
@@ -479,10 +479,30 @@ handle_connection_create (void *cls,
          GNUNET_sh2s (&msg->cid.connection_of_tunnel));
     path = GCPP_get_path_from_route (path_length - 1,
                                      pids);
-    GCT_add_inbound_connection (GCP_get_tunnel (origin,
-                                                GNUNET_YES),
-                                &msg->cid,
-                                path);
+    if (GNUNET_OK !=
+        GCT_add_inbound_connection (GCP_get_tunnel (origin,
+                                                    GNUNET_YES),
+                                    &msg->cid,
+                                    path))
+    {
+      /* Send back BROKEN: duplicate connection on the same path,
+         we will use the other one. */
+      struct GNUNET_MQ_Envelope *env;
+      struct GNUNET_CADET_ConnectionBrokenMessage *bm;
+
+      LOG (GNUNET_ERROR_TYPE_DEBUG,
+           "Received CADET_CONNECTION_CREATE from %s for %s, but %s already 
has a connection. Sending BROKEN\n",
+           GCP_2s (sender),
+           GNUNET_sh2s (&msg->cid.connection_of_tunnel),
+           GCPP_2s (path));
+      env = GNUNET_MQ_msg (bm,
+                           GNUNET_MESSAGE_TYPE_CADET_CONNECTION_BROKEN);
+      bm->cid = msg->cid;
+      bm->peer1 = my_full_id;
+      GCP_send_ooo (sender,
+                    env);
+      return;
+    }
     return;
   }
   /* We are merely a hop on the way, check if we can support the route */
@@ -611,7 +631,7 @@ handle_connection_broken (void *cls,
     LOG (GNUNET_ERROR_TYPE_DEBUG,
          "Received CONNECTION_BROKEN for connection %s. Destroying it.\n",
          GNUNET_sh2s (&msg->cid.connection_of_tunnel));
-    GCC_destroy (cc);
+    GCC_destroy_without_core (cc);
 
     /* FIXME: also destroy the path up to the specified link! */
     return;
@@ -661,7 +681,7 @@ handle_connection_destroy (void *cls,
          "Received CONNECTION_DESTROY for connection %s. Destroying 
connection.\n",
          GNUNET_sh2s (&msg->cid.connection_of_tunnel));
 
-    GCC_destroy (cc);
+    GCC_destroy_without_core (cc);
     return;
   }
 
diff --git a/src/cadet/gnunet-service-cadet-new_paths.c 
b/src/cadet/gnunet-service-cadet-new_paths.c
index a5d201e0b..685656ec3 100644
--- a/src/cadet/gnunet-service-cadet-new_paths.c
+++ b/src/cadet/gnunet-service-cadet-new_paths.c
@@ -24,11 +24,6 @@
  * @author Christian Grothoff
  *
  * TODO:
- * - BUG: currently only allowing one unique connection per path,
- *   but need to allow 2 in case WE are establishing one from A to B
- *   while at the same time B establishes one to A.
- *   Also, must not ASSERT if B establishes a 2nd one to us.
- *   Need to have some reasonable tie-breaking to only keep ONE.
  * - path desirability score calculations are not done
  */
 #include "platform.h"
diff --git a/src/cadet/gnunet-service-cadet-new_tunnels.c 
b/src/cadet/gnunet-service-cadet-new_tunnels.c
index e677a7436..bd46dc151 100644
--- a/src/cadet/gnunet-service-cadet-new_tunnels.c
+++ b/src/cadet/gnunet-service-cadet-new_tunnels.c
@@ -1523,6 +1523,24 @@ GCT_add_channel (struct CadetTunnel *t,
 
 
 /**
+ * We lost a connection, remove it from our list and clean up
+ * the connection object itself.
+ *
+ * @param ct binding of connection to tunnel of the connection that was lost.
+ */
+void
+GCT_connection_lost (struct CadetTConnection *ct)
+{
+  struct CadetTunnel *t = ct->t;
+
+  GNUNET_CONTAINER_DLL_remove (t->connection_head,
+                               t->connection_tail,
+                               ct);
+  GNUNET_free (ct);
+}
+
+
+/**
  * This tunnel is no longer used, destroy it.
  *
  * @param cls the idle tunnel
@@ -1541,12 +1559,12 @@ destroy_tunnel (void *cls)
   GNUNET_assert (0 == GNUNET_CONTAINER_multihashmap32_size (t->channels));
   while (NULL != (ct = t->connection_head))
   {
+    struct CadetConnection *cc;
+
     GNUNET_assert (ct->t == t);
-    GNUNET_CONTAINER_DLL_remove (t->connection_head,
-                                 t->connection_tail,
-                                 ct);
-    GCC_destroy (ct->cc);
-    GNUNET_free (ct);
+    cc = ct->cc;
+    GCT_connection_lost (ct);
+    GCC_destroy_without_tunnel (cc);
   }
   while (NULL != (tq = t->tq_head))
   {
@@ -2291,8 +2309,10 @@ GCT_create_tunnel (struct CadetPeer *destination)
  * @param t a tunnel
  * @param cid connection identifer to use for the connection
  * @param path path to use for the connection
+ * @return #GNUNET_OK on success,
+ *         #GNUNET_SYSERR on failure (duplicate connection)
  */
-void
+int
 GCT_add_inbound_connection (struct CadetTunnel *t,
                             const struct 
GNUNET_CADET_ConnectionTunnelIdentifier *cid,
                             struct CadetPeerPath *path)
@@ -2308,6 +2328,15 @@ GCT_add_inbound_connection (struct CadetTunnel *t,
                                cid,
                                &connection_ready_cb,
                                ct);
+  if (NULL == ct->cc)
+  {
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "Tunnel %s refused inbound connection %s (duplicate)\n",
+         GCT_2s (t),
+         GCC_2s (ct->cc));
+    GNUNET_free (ct);
+    return GNUNET_SYSERR;
+  }
   /* FIXME: schedule job to kill connection (and path?)  if it takes
      too long to get ready! (And track performance data on how long
      other connections took with the tunnel!)
@@ -2320,6 +2349,7 @@ GCT_add_inbound_connection (struct CadetTunnel *t,
        "Tunnel %s has new connection %s\n",
        GCT_2s (t),
        GCC_2s (ct->cc));
+  return GNUNET_OK;
 }
 
 
diff --git a/src/cadet/gnunet-service-cadet-new_tunnels.h 
b/src/cadet/gnunet-service-cadet-new_tunnels.h
index c867a9e82..463780686 100644
--- a/src/cadet/gnunet-service-cadet-new_tunnels.h
+++ b/src/cadet/gnunet-service-cadet-new_tunnels.h
@@ -113,14 +113,26 @@ GCT_destroy_tunnel_now (struct CadetTunnel *t);
  * @param t a tunnel
  * @param cid connection identifer to use for the connection
  * @param path path to use for the connection
+ * @return #GNUNET_OK on success,
+ *         #GNUNET_SYSERR on failure (duplicate connection)
  */
-void
+int
 GCT_add_inbound_connection (struct CadetTunnel *t,
                             const struct 
GNUNET_CADET_ConnectionTunnelIdentifier *cid,
                             struct CadetPeerPath *path);
 
 
 /**
+ * We lost a connection, remove it from our list and clean up
+ * the connection object itself.
+ *
+ * @param ct binding of connection to tunnel of the connection that was lost.
+ */
+void
+GCT_connection_lost (struct CadetTConnection *ct);
+
+
+/**
  * Return the peer to which this tunnel goes.
  *
  * @param t a tunnel

-- 
To stop receiving notification emails like this one, please contact
address@hidden