Improved internal path quality metric computation

dht/Pathfinder.c

@@ -36,6 +36,7 @@
 #include "wire/Error.h"
 #include "wire/PFChan.h"
 #include "util/CString.h"
+#include "wire/Metric.h"
 
 ///////////////////// [ Address ][ content... ]
 
@@ -127,7 +128,7 @@ static void nodeForAddress(struct PFChan_Node* nodeOut, struct Address* addr, ui
 {
     Bits_memset(nodeOut, 0, PFChan_Node_SIZE);
     nodeOut->version_be = Endian_hostToBigEndian32(addr->protocolVersion);
-    nodeOut->metric_be = Endian_hostToBigEndian32(metric | 0xffff0000);
+    nodeOut->metric_be = Endian_hostToBigEndian32(metric);
     nodeOut->path_be = Endian_hostToBigEndian64(addr->path);
     Bits_memcpy(nodeOut->publicKey, addr->key, 32);
     Bits_memcpy(nodeOut->ip6, addr->ip6.bytes, 16);
@@ -158,7 +159,9 @@ static void onBestPathChange(void* vPathfinder, struct Node_Two* node)
     } else {
         pf->bestPathChanges++;
         struct Message* msg = Message_new(0, 256, alloc);
-        Iface_CALL(sendNode, msg, &node->address, Node_getCost(node), pf);
+        Iface_CALL(sendNode, msg, &node->address,
+            (Node_getCost(node) & Metric_DHT_MASK) | Metric_DHT,
+            pf);
     }
     Allocator_free(alloc);
 }
@@ -317,7 +320,7 @@ static Iface_DEFUN peer(struct Message* msg, struct Pathfinder_pvt* pf)
     //RumorMill_addNode(pf->rumorMill, &addr);
     Router_sendGetPeers(pf->router, &addr, 0, 0, pf->alloc);
 
-    return sendNode(msg, &addr, 0xffffff00, pf);
+    return sendNode(msg, &addr, Metric_DHT_PEER, pf);
 }
 
 static Iface_DEFUN peerGone(struct Message* msg, struct Pathfinder_pvt* pf)
@@ -329,7 +332,7 @@ static Iface_DEFUN peerGone(struct Message* msg, struct Pathfinder_pvt* pf)
     NodeStore_disconnectedPeer(pf->nodeStore, addr.path);
 
     // We notify about the node but with max metric so it will be removed soon.
-    return sendNode(msg, &addr, 0xffffffff, pf);
+    return sendNode(msg, &addr, Metric_DEAD_LINK, pf);
 }
 
 static Iface_DEFUN session(struct Message* msg, struct Pathfinder_pvt* pf)
@@ -417,7 +420,7 @@ static Iface_DEFUN incomingMsg(struct Message* msg, struct Pathfinder_pvt* pf)
     DHTModuleRegistry_handleIncoming(&dht, pf->registry);
 
     struct Message* nodeMsg = Message_new(0, 256, msg->alloc);
-    Iface_CALL(sendNode, nodeMsg, &addr, 0xfffffff0u, pf);
+    Iface_CALL(sendNode, nodeMsg, &addr, Metric_DHT_INCOMING, pf);
 
     if (dht.pleaseRespond) {
         // what a beautiful hack, see incomingFromDHT

net/InterfaceController.c

@@ -34,6 +34,7 @@
 #include "wire/Error.h"
 #include "wire/Message.h"
 #include "wire/Headers.h"
+#include "wire/Metric.h"
 
 /** After this number of milliseconds, a node will be regarded as unresponsive. */
 #define UNRESPONSIVE_AFTER_MILLISECONDS (20*1024)
@@ -238,9 +239,10 @@ static void sendPeer(uint32_t pathfinderId,
     node->version_be = Endian_hostToBigEndian32(peer->addr.protocolVersion);
     if (ev != PFChan_Core_PEER_GONE) {
         Assert_true(peer->addr.protocolVersion);
-        node->metric_be = Endian_hostToBigEndian32(0xfff00000 | latency);
+        node->metric_be =
+            Endian_hostToBigEndian32(Metric_IC_PEER | (latency & Metric_IC_PEER_MASK));
     } else {
-        node->metric_be = 0xffffffff;
+        node->metric_be = Endian_hostToBigEndian32(Metric_DEAD_LINK);
     }
     Er_assert(Message_epush32be(msg, pathfinderId));
     Er_assert(Message_epush32be(msg, ev));

net/SessionManager.c

@@ -23,6 +23,7 @@
 #include "wire/RouteHeader.h"
 #include "util/events/Timeout.h"
 #include "util/Checksum.h"
+#include "wire/Metric.h"
 
 /** Handle numbers 0-3 are reserved for CryptoAuth nonces. */
 #define MIN_FIRST_HANDLE 4
@@ -213,25 +214,23 @@ static struct SessionManager_Session_pvt* getSession(struct SessionManager_pvt*
     if (sess) {
         sess->pub.version = (sess->pub.version) ? sess->pub.version : version;
         sess->pub.maintainSession |= maintainSession;
-        if (metric == 0xffffffff) {
+        if (metric == Metric_DEAD_LINK) {
             // this is a broken path
             if (sess->pub.sendSwitchLabel == label) {
                 debugSession0(sm->log, sess, "broken path");
                 if (sess->pub.sendSwitchLabel == sess->pub.recvSwitchLabel) {
                     sess->pub.sendSwitchLabel = 0;
-                    sess->pub.metric = 0xffffffff;
+                    sess->pub.metric = Metric_DEAD_LINK;
                 } else {
                     sess->pub.sendSwitchLabel = sess->pub.recvSwitchLabel;
-                    sess->pub.metric = 0xfffffff0;
+                    sess->pub.metric = Metric_SM_INCOMING;
                 }
             }
-        } else {
-            if (metric <= sess->pub.metric) {
-                sess->pub.sendSwitchLabel = label;
-                sess->pub.version = (version) ? version : sess->pub.version;
-                sess->pub.metric = metric;
-                debugSession0(sm->log, sess, "discovered path");
-            }
+        } else if (metric <= sess->pub.metric && label) {
+            sess->pub.sendSwitchLabel = label;
+            sess->pub.version = (version) ? version : sess->pub.version;
+            sess->pub.metric = metric;
+            debugSession0(sm->log, sess, "discovered path");
         }
         return sess;
     }
@@ -379,7 +378,7 @@ static Iface_DEFUN incomingFromSwitchIf(struct Message* msg, struct Iface* iface
         }
 
         uint64_t label = Endian_bigEndianToHost64(switchHeader->label_be);
-        session = getSession(sm, ip6, caHeader->publicKey, 0, label, 0xfffff000, 0);
+        session = getSession(sm, ip6, caHeader->publicKey, 0, label, Metric_SM_INCOMING, 0);
         CryptoAuth_resetIfTimeout(session->pub.caSession);
         debugHandlesAndLabel(sm->log, session, label, "new session nonce[%d]", nonceOrHandle);
     }
@@ -476,10 +475,11 @@ static void unsetupSession(struct SessionManager_pvt* sm, struct SessionManager_
     }
     struct Allocator* eventAlloc = Allocator_child(sm->alloc);
     struct Message* eventMsg = Message_new(0, 512, eventAlloc);
-    struct PFChan_Node n;
+    struct PFChan_Node n = { .metric_be = Endian_hostToBigEndian32(Metric_DEAD_LINK) };
     n.path_be = Endian_hostToBigEndian64(sess->pub.sendSwitchLabel ?
                                          sess->pub.sendSwitchLabel : sess->pub.recvSwitchLabel);
     n.version_be = Endian_hostToBigEndian32(sess->pub.version);
+    n.metric_be = Endian_bigEndianToHost32(sess->pub.metric);
     Bits_memcpy(n.publicKey, sess->pub.caSession->herPublicKey, 32);
     Bits_memcpy(n.ip6, sess->pub.caSession->herIp6, 16);
     Er_assert(Message_epush(eventMsg, &n, PFChan_Node_SIZE));
@@ -679,7 +679,7 @@ static Iface_DEFUN incomingFromInsideIf(struct Message* msg, struct Iface* iface
                               header->publicKey,
                               Endian_bigEndianToHost32(header->version_be),
                               Endian_bigEndianToHost64(header->sh.label_be),
-                              0xfffffff0,
+                              Metric_SM_SEND,
                               !(header->flags & RouteHeader_flags_PATHFINDER));
         } else {
             needsLookup(sm, msg, false);
@@ -748,7 +748,7 @@ static Iface_DEFUN incomingFromEventIf(struct Message* msg, struct Iface* iface)
         // Node we don't care about.
         if (index == -1) { return NULL; }
         // Broken path to a node we don't have a session for...
-        if (node.metric_be == 0xffffffff) { return NULL; }
+        if (node.metric_be == Metric_DEAD_LINK) { return NULL; }
     }
     sess = getSession(sm,
                       node.ip6,

subnode/SubnodePathfinder.c

@@ -33,6 +33,7 @@
 #include "wire/PFChan.h"
 #include "wire/DataHeader.h"
 #include "util/CString.h"
+#include "wire/Metric.h"
 
 #include "subnode/ReachabilityAnnouncer.h"
 
@@ -188,7 +189,7 @@ static void getRouteReply(Dict* msg, struct Address* src, struct MsgCore_Promise
 
     //NodeCache_discoverNode(pf->nc, &al->elems[0]);
     struct Message* msgToCore = Message_new(0, 512, prom->alloc);
-    Iface_CALL(sendNode, msgToCore, &al->elems[0], 0xfff00033, PFChan_Pathfinder_NODE, pf);
+    Iface_CALL(sendNode, msgToCore, &al->elems[0], Metric_SNODE_SAYS, PFChan_Pathfinder_NODE, pf);
 }
 
 static Iface_DEFUN searchReq(struct Message* msg, struct SubnodePathfinder_pvt* pf)
@@ -207,7 +208,7 @@ static Iface_DEFUN searchReq(struct Message* msg, struct SubnodePathfinder_pvt*
         struct Address* myPeer = AddrSet_get(pf->myPeers, i);
         if (!Bits_memcmp(myPeer->ip6.bytes, addr, 16)) {
             Log_debug(pf->log, "Skip search for [%s] because it's a peer", printedAddr);
-            return sendNode(msg, myPeer, 0xfff00000, PFChan_Pathfinder_NODE, pf);
+            return sendNode(msg, myPeer, Metric_PF_PEER, PFChan_Pathfinder_NODE, pf);
         }
     }
 
@@ -218,7 +219,7 @@ static Iface_DEFUN searchReq(struct Message* msg, struct SubnodePathfinder_pvt*
 
     if (!Bits_memcmp(pf->pub.snh->snodeAddr.ip6.bytes, addr, 16)) {
         Log_debug(pf->log, "Skip search for [%s] because it is our snode", printedAddr);
-        return sendNode(msg, &pf->pub.snh->snodeAddr, 0xfff00000, PFChan_Pathfinder_NODE, pf);
+        return sendNode(msg, &pf->pub.snh->snodeAddr, Metric_SNODE, PFChan_Pathfinder_NODE, pf);
     }
 
     struct Query q = { .routeFrom = { 0 } };
@@ -286,11 +287,12 @@ static Iface_DEFUN peer(struct Message* msg, struct SubnodePathfinder_pvt* pf)
     //NodeCache_discoverNode(pf->nc, &addr);
 
     ReachabilityCollector_change(pf->pub.rc, &addr);
-    if ((metric & 0xffff) < 0xffff) {
-        ReachabilityCollector_lagSample(pf->pub.rc, addr.path, (metric & 0xffff));
+
+    if ((metric & ~Metric_IC_PEER_MASK) == Metric_IC_PEER) {
+        ReachabilityCollector_lagSample(pf->pub.rc, addr.path, (metric & Metric_IC_PEER_MASK));
     }
 
-    return sendNode(msg, &addr, 0xfff00000, PFChan_Pathfinder_NODE, pf);
+    return sendNode(msg, &addr, metric, PFChan_Pathfinder_NODE, pf);
 }
 
 static Iface_DEFUN peerGone(struct Message* msg, struct SubnodePathfinder_pvt* pf)
@@ -315,7 +317,7 @@ static Iface_DEFUN peerGone(struct Message* msg, struct SubnodePathfinder_pvt* p
     ReachabilityCollector_change(pf->pub.rc, &zaddr);
 
     // We notify about the node but with max metric so it will be removed soon.
-    return sendNode(msg, &addr, 0xffffffff, PFChan_Pathfinder_NODE, pf);
+    return sendNode(msg, &addr, Metric_DEAD_LINK, PFChan_Pathfinder_NODE, pf);
 }
 
 static Iface_DEFUN session(struct Message* msg, struct SubnodePathfinder_pvt* pf)
@@ -395,7 +397,7 @@ static void unsetupSessionPingReply(Dict* msg, struct Address* src, struct MsgCo
     //Log_debug(pf->log, "\n\n\n\nPING reply from [%s]!\n\n\n\n",
     //    Address_toString(src, prom->alloc)->bytes);
     struct Message* msgToCore = Message_new(0, 512, prom->alloc);
-    Iface_CALL(sendNode, msgToCore, src, 0xfffffff0, PFChan_Pathfinder_NODE, pf);
+    Iface_CALL(sendNode, msgToCore, src, Metric_PING_REPLY, PFChan_Pathfinder_NODE, pf);
 }
 
 static Iface_DEFUN unsetupSession(struct Message* msg, struct SubnodePathfinder_pvt* pf)

wire/Metric.h

@@ -0,0 +1,71 @@
+/* vim: set expandtab ts=4 sw=4: */
+/*
+ * You may redistribute this program and/or modify it under the terms of
+ * the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+#ifndef Metric_H
+#define Metric_H
+
+// This is INTERNAL to cjdns, it does not get used in the protocol at all,
+// but it is an internal protocol which is used to coordinate the different
+// modules.
+//
+// Different parts of the code become aware of the validity of different paths.
+// The InterfaceController becomes aware of peers, the DHT and the subnode
+// Pathfinder become aware of more distant nodes. And of course the SessionManager
+// becomes aware of a node by virtue of communicating with it.
+//
+// Each of these components informs the SessionManager when it becomes aware
+// of a path, if the SessionManager has a session for the node and the metric
+// for the path is lower than the metric which the SessionManager currently
+// has, it will update the path to that node.
+//
+// Because this is not inter-node protocol, you can change these numbers at any
+// time.
+
+enum Metric {
+    // This is known to be a peer by InterfaceController, OR'd with the latency
+    Metric_IC_PEER =         0xff000000,
+    Metric_IC_PEER_MASK =    0x0000ffff,
+
+    // Node is a direct peer according to the SubnodePathfinder
+    Metric_PF_PEER =         0xff100000,
+
+    // The snode says this is a path to the node
+    Metric_SNODE_SAYS =      0xff200000,
+
+    // This is our discovered path to our snode
+    Metric_SNODE =           0xff300000,
+
+    // Node is a direct peer according to the (dht) Pathfinder
+    Metric_DHT_PEER =        0xff400000,
+
+    // We sent a ping to a node to complete the session setup, it replied.
+    // This is a path which we know works, but we don't know if it's any good.
+    Metric_PING_REPLY =      0xff500000,
+    Metric_DHT_INCOMING =    0xff510000,
+
+    // Anything that comes from the DHT Pathfinder is &'d with Metric_DHT_MASK
+    // and OR'd with Metric_DHT.
+    Metric_DHT =             0xff600000,
+    Metric_DHT_MASK =        0x000fffff,
+
+    // Incoming message, CryptoAuth has not yet checked that the key is real
+    Metric_SM_INCOMING =     0xff700000,
+    // Outgoing message, some upper layer thinks this is the path
+    Metric_SM_SEND =         0xff710000,
+
+    // This will cause the SM to kill off a path
+    Metric_DEAD_LINK =       0xffffffff
+};
+
+#endif