Tighten up tests and code relating to peer discovery

monitor.c

@@ -565,7 +565,7 @@ int monitor_call_status(vomp_call_state *call)
   RETURN(0);
 }
 
-int monitor_announce_peer(unsigned char *sid)
+int monitor_announce_peer(const unsigned char *sid)
 {
   char msg[1024];
   int n = snprintf(msg, sizeof msg, "\nNEWPEER:%s\n",overlay_render_sid(sid));

overlay_advertise.c

@@ -179,7 +179,11 @@ int overlay_route_saw_advertisements(int i,overlay_frame *f, long long now)
   int ofs=0;
 
   /* lookup score of current sender */
-  overlay_node *sender=overlay_route_find_node(f->source,SID_SIZE,0);
+  overlay_node *sender = overlay_route_find_node(f->source, SID_SIZE, 0);
+  if (sender == NULL) {
+    WARNF("Cannot advertise %s -- overlay node not found", alloca_tohex_sid(f->source));
+    return -1;
+  }
   int sender_score=sender->best_link_score;
   if (debug&DEBUG_OVERLAYROUTEMONITOR)
     DEBUGF("score to reach %s is %d", overlay_render_sid(f->source),sender_score);

overlay_route.c

@@ -404,8 +404,7 @@ int overlay_get_nexthop(unsigned char *d,unsigned char *nexthop,int *nexthoplen,
   } else {
     /* Is not a direct neighbour.
        XXX - Very simplistic for now. */
-    overlay_node 
-      *n=overlay_route_find_node(d,SID_SIZE,0 /* don't create if missing */ );
+    overlay_node *n=overlay_route_find_node(d,SID_SIZE,0 /* don't create if missing */ );
     if (n) {
       int o;
       int best_score=0;
@@ -437,7 +436,7 @@ int overlay_get_nexthop(unsigned char *d,unsigned char *nexthop,int *nexthoplen,
   }
 }
 
-unsigned int overlay_route_hash_sid(unsigned char *sid)
+unsigned int overlay_route_hash_sid(const unsigned char *sid)
 {
   /* Calculate the bin number of an address (sid) from the sid. */
   if (!overlay_route_hash_bytes) return WHY("overlay_route_hash_bytes==0");
@@ -464,30 +463,31 @@ unsigned int overlay_route_hash_sid(unsigned char *sid)
   return bin;
 }
 
-overlay_node *overlay_route_find_node(unsigned char *sid,int prefixLen,int createP)
+overlay_node *overlay_route_find_node(const unsigned char *sid, int prefixLen, int createP)
 {
-  int bin_number=overlay_route_hash_sid(sid);
-  int free_slot=-1;
-  int slot;
-
-  if (bin_number<0) { WHY("negative bin number"); return NULL; }
-
-  for(slot=0;slot<overlay_bin_size;slot++)
-    {
-      if (!memcmp(sid,&overlay_nodes[bin_number][slot].sid[0],prefixLen))
-	{
-	  /* Found it */
-	  return &overlay_nodes[bin_number][slot];
+  int bin_number = overlay_route_hash_sid(sid);
+  if (bin_number < 0) {
+    WHY("negative bin number");
+    return NULL;
   }
-      else if (overlay_nodes[bin_number][slot].sid[0]==0) free_slot=slot;
+
+  int free_slot = -1;
+  {
+    int slot;
+    for (slot = 0; slot < overlay_bin_size; slot++)
+      if (memcmp(sid, overlay_nodes[bin_number][slot].sid, prefixLen) == 0)
+	return &overlay_nodes[bin_number][slot];
+      else if (overlay_nodes[bin_number][slot].sid[0]==0)
+	free_slot = slot;
   }
 
   /* Didn't find it */
   if (!createP) return NULL;
 
-  if (free_slot==-1)
-    {
-      /* Displace one of the others in the bin so we can go there */
+  if (free_slot == -1) {
+    /* Evict */
+    WARN("overlay_nodes[] eviction NOT IMPLEMENTED");
+    return NULL;
     int i;
     for(i=0;i<OVERLAY_MAX_OBSERVATIONS;i++)
       overlay_nodes[bin_number][free_slot].observations[i].observed_score=0;
@@ -501,11 +501,15 @@ overlay_node *overlay_route_find_node(unsigned char *sid,int prefixLen,int creat
     }
   }
 
+  bcopy(sid, overlay_nodes[bin_number][free_slot].sid, SID_SIZE);
+
   /* Ask for newly discovered node to be advertised */
-  overlay_route_please_advertise(&overlay_nodes[bin_number][slot]);
+  overlay_route_please_advertise(&overlay_nodes[bin_number][free_slot]);
   monitor_announce_peer(sid);
 
-  bcopy(sid,overlay_nodes[bin_number][free_slot].sid,SID_SIZE);
+  // This info message is used by tests; don't alter or remove it.
+  INFOF("ADD OVERLAY NODE sid=%s slot=%d", alloca_tohex_sid(sid), free_slot);
+
   return &overlay_nodes[bin_number][free_slot];
 }
 
@@ -682,8 +686,10 @@ overlay_neighbour *overlay_route_get_neighbour_structure(unsigned char *packed_s
   }  
 
   overlay_node *n=overlay_route_find_node(packed_sid,prefixLen,createP);
-  if (!n) { // WHY("Could not find node record for observed node"); 
-    return NULL; }
+  if (!n) {
+    // WHY("Could not find node record for observed node"); 
+    return NULL;
+  }
 
   /* Check if node is already a neighbour, or if not, make it one */
   if (!n->neighbour_id) if (overlay_route_make_neighbour(n)) { WHY("overlay_route_make_neighbour() failed"); return NULL; }

serval.h

@@ -1026,8 +1026,8 @@ int overlay_route_saw_selfannounce_ack(overlay_frame *f,long long now);
 int overlay_route_recalc_node_metrics(overlay_node *n,long long now);
 int overlay_route_recalc_neighbour_metrics(overlay_neighbour *n,long long now);
 int overlay_route_saw_selfannounce(overlay_frame *f,long long now);
-overlay_node *overlay_route_find_node(unsigned char *sid,int prefixLen,int createP);
-unsigned int overlay_route_hash_sid(unsigned char *sid);
+overlay_node *overlay_route_find_node(const unsigned char *sid,int prefixLen,int createP);
+unsigned int overlay_route_hash_sid(const unsigned char *sid);
 int overlay_route_init(int mb_ram);
 overlay_neighbour *overlay_route_get_neighbour_structure(unsigned char *packed_sid, 
 							 int prefixLen,int createP);
@@ -1494,7 +1494,7 @@ int monitor_setup_sockets();
 int monitor_get_fds(struct pollfd *fds,int *fdcount,int fdmax);
 int monitor_call_status(vomp_call_state *call);
 int monitor_send_audio(vomp_call_state *call,overlay_mdp_frame *audio);
-int monitor_announce_peer(unsigned char *sid);
+int monitor_announce_peer(const unsigned char *sid);
 int monitor_tell_clients(char *msg, int msglen, int mask);
 extern int monitor_socket_count;
 

tests/dnaprotocol

@@ -32,17 +32,18 @@ teardown() {
    assert_no_servald_processes
 }
 
-setup_servald_instance() {
+start_servald_instance() {
    push_instance
    set_instance "$1"
    executeOk_servald config set debug.interfaces Yes
-   executeOk_servald config set debug.packetformats Yes
-   executeOk_servald config set debug.tx Yes
-   executeOk_servald config set debug.rx Yes
+   executeOk_servald config set debug.packetformats No
+   executeOk_servald config set debug.routing No
+   executeOk_servald config set debug.tx No
+   executeOk_servald config set debug.rx No
    executeOk_servald config set interfaces "+>$2"
    executeOk_servald config set monitor.socket "org.servalproject.servald.monitor.socket.$TFWUNIQUE.$instance_name"
    executeOk_servald config set mdp.socket "org.servalproject.servald.mdp.socket.$TFWUNIQUE.$instance_name"
-   executeOk_servald config set mdp.wifi.tick_ms 50
+   executeOk_servald config set mdp.wifi.tick_ms 100
    executeOk_servald config set mdp.selfannounce.ticks_per_full_address 1
    executeOk_servald keyring add
    assert [ -e "$SERVALINSTANCE_PATH/serval.keyring" ]
@@ -52,25 +53,38 @@ setup_servald_instance() {
    assert --message='main identity known' [ -n "$sid" ]
    executeOk_servald set did $sid 5550001 "Agent $instance_name Smith"
    start_servald_server
-   executeOk_servald status
-   tfw_cat --stdout
+   log="$instance_servald_log"
    pop_instance
 }
 
-setup_servald_instances() {
+start_servald_instances() {
    DUMMYNET=$PWD/dummy
    >$DUMMYNET
-   setup_servald_instance +A $DUMMYNET
+   start_servald_instance +A $DUMMYNET
    SIDA=$sid
-   setup_servald_instance +B $DUMMYNET
+   LOGA="$log"
+   start_servald_instance +B $DUMMYNET
    SIDB=$sid
-   # Now make sure that they can see each other
-   secs=5
-   sleep $secs # Should be plenty of time
-   tfw_log "# Dummynet file after $secs seconds: "`ls -l $DUMMYNET`
+   LOGB="$log"
+   # Now wait until they see each other.
+   sleep 30 &
+   local timeout_pid=$!
+   tfw_log "Wait for instances to see each other"
+   while ! grep "^INFO:.*ADD OVERLAY NODE sid=$SIDB" "$LOGA" || ! grep "^INFO:.*ADD OVERLAY NODE sid=$SIDA" "$LOGB"; do
+      kill -0 $timeout_pid 2>/dev/null || fail "timeout"
+      tfw_log "Sleep"
+      sleep 0.1
+   done
+   tfw_log "# Dummynet file:" $(ls -l $DUMMYNET)
    set_instance +A
    executeOk_servald id peers
    assertStdoutLineCount '==' 1
+   assertStdoutGrep "$SIDB"
+   set_instance +B
+   executeOk_servald id peers
+   assertStdoutLineCount '==' 1
+   assertStdoutGrep "$SIDA"
+   set_instance +A
 }
 
 doc_MultiServer="Start two servald instances with dummy interface"
@@ -78,13 +92,13 @@ setup_MultiServer() {
    setup
 }
 test_MultiServer() {
-   setup_servald_instances
+   start_servald_instances
 }
 
 doc_LookupWildcard="Lookup by wildcard"
 setup_LookupWildcard() {
    setup
-   setup_servald_instances
+   start_servald_instances
    set_instance +A
 }
 test_LookupWildcard() {
@@ -97,7 +111,7 @@ test_LookupWildcard() {
 doc_LookupEmpty="Lookup by empty string"
 setup_LookupEmpty() {
    setup
-   setup_servald_instances
+   start_servald_instances
    set_instance +A
 }
 test_LookupEmpty() {
@@ -110,7 +124,7 @@ test_LookupEmpty() {
 doc_LookupNumber="Lookup by phone number"
 setup_LookupNumber() {
    setup
-   setup_servald_instances
+   start_servald_instances
    set_instance +A
 }
 test_LookupNumber() {
@@ -131,7 +145,7 @@ test_LookupNumber() {
 doc_NodeinfoLocal="Node info auto-resolves for local identities"
 setup_NodeinfoLocal() {
    setup
-   setup_servald_instances
+   start_servald_instances
    set_instance +A
 }
 test_NodeinfoLocal() {
@@ -146,7 +160,7 @@ test_NodeinfoLocal() {
 doc_NodeinfoRemote="Node info resolves remote identities"
 setup_NodeinfoRemote() {
    setup
-   setup_servald_instances
+   start_servald_instances
    set_instance +A
 }
 test_NodeinfoRemote() {