Updated the drop counter to try to fix uint32 rollover

cjdelisle · Apr 27, 2020 · d4973dd · d4973dd
1 parent 3d1fb7d
commit d4973dd
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 28 deletions.
diff --git a/net/InterfaceController.c b/net/InterfaceController.c
@@ -147,9 +147,9 @@ struct Peer
      * are monotonic and so probably what you want.
      */
     uint32_t _lastDrops;
-    uint32_t lastDrops;
     uint32_t _lastPackets;
-    uint32_t lastPackets;
+    uint64_t lastDrops;
+    uint64_t lastPackets;
 
     // traffic counters
     uint64_t bytesOut;
@@ -340,6 +340,7 @@ static void linkState(void* vic)
 
             uint32_t drops = ep->caSession->replayProtector.lostPackets;
             uint64_t newDrops = 0;
+            // We're checking uint32 rollover here
             if (drops > ep->_lastDrops) { newDrops = drops - ep->_lastDrops; }
             ep->_lastDrops = drops;
             ep->lastDrops += newDrops;
@@ -351,10 +352,10 @@ static void linkState(void* vic)
             ep->lastPackets += newPackets;
 
             struct PFChan_LinkState_Entry e = {
-                .peerLabel_be = Endian_hostToBigEndian32((uint32_t) ep->addr.path),
-                .sumOfPackets_be = Endian_hostToBigEndian32(ep->lastPackets),
-                .sumOfDrops_be = Endian_hostToBigEndian32(ep->lastDrops),
-                .sumOfKb_be = Endian_hostToBigEndian32((uint32_t) (ep->bytesIn >> 10))
+                .peerLabel = ep->addr.path,
+                .sumOfPackets = ep->lastPackets,
+                .sumOfDrops = ep->lastDrops,
+                .sumOfKb = (ep->bytesIn >> 10),
             };
             Message_push(msg, &e, PFChan_LinkState_Entry_SIZE, NULL);
         }

diff --git a/subnode/ReachabilityCollector.c b/subnode/ReachabilityCollector.c
@@ -37,13 +37,13 @@ struct PeerInfo_pvt
     uint32_t lagSamples;
     uint32_t timeOfLastLagUpdate;
 
-    uint32_t sumOfDropsLastSlot;
-    uint32_t sumOfPacketsLastSlot;
-    uint32_t sumOfKbLastSlot;
+    uint64_t sumOfDropsLastSlot;
+    uint64_t sumOfPacketsLastSlot;
+    uint64_t sumOfKbLastSlot;
 
-    uint32_t sumOfDrops;
-    uint32_t sumOfPackets;
-    uint32_t sumOfKb;
+    uint64_t sumOfDrops;
+    uint64_t sumOfPackets;
+    uint64_t sumOfKb;
 
     // This peer is waiting for response
     bool waitForResponse;
@@ -313,7 +313,9 @@ static void cycle(void* vrc)
         int sampleNum = rcp->linkStateSamples % LinkState_SLOTS;
 
         uint64_t drops = pi->sumOfDrops - pi->sumOfDropsLastSlot;
+        if (drops < pi->sumOfDrops) { drops = pi->sumOfDrops; }
         uint64_t packets = pi->sumOfPackets - pi->sumOfPacketsLastSlot;
+        if (packets < pi->sumOfPackets) { drops = pi->sumOfPackets; }
         uint64_t dropRateShl18 = packets ? (drops << 18) / packets : 0;
         pi->pub.linkState.dropSlots[sampleNum] = dropRateShl18 > 0xfffe ? 0xfffe : dropRateShl18;
         pi->sumOfDropsLastSlot = pi->sumOfDrops;
@@ -353,9 +355,9 @@ void ReachabilityCollector_lagSample(
 void ReachabilityCollector_updateBandwidthAndDrops(
     struct ReachabilityCollector* rc,
     uint64_t label,
-    uint32_t sumOfPackets,
-    uint32_t sumOfDrops,
-    uint32_t sumOfKb)
+    uint64_t sumOfPackets,
+    uint64_t sumOfDrops,
+    uint64_t sumOfKb)
 {
     struct ReachabilityCollector_pvt* rcp = Identity_check((struct ReachabilityCollector_pvt*) rc);
     struct PeerInfo_pvt* pi = piForLabel(rcp, label);

diff --git a/subnode/ReachabilityCollector.h b/subnode/ReachabilityCollector.h
@@ -60,9 +60,9 @@ void ReachabilityCollector_lagSample(
 void ReachabilityCollector_updateBandwidthAndDrops(
     struct ReachabilityCollector* rc,
     uint64_t label,
-    uint32_t sumOfPackets,
-    uint32_t sumOfDrops,
-    uint32_t sumOfKb);
+    uint64_t sumOfPackets,
+    uint64_t sumOfDrops,
+    uint64_t sumOfKb);
 
 struct ReachabilityCollector* ReachabilityCollector_new(struct Allocator* allocator,
                                                         struct MsgCore* mc,

diff --git a/subnode/SubnodePathfinder.c b/subnode/SubnodePathfinder.c
@@ -450,11 +450,10 @@ static Iface_DEFUN linkState(struct Message* msg, struct SubnodePathfinder_pvt*
         Message_pop(msg, &lse, PFChan_LinkState_Entry_SIZE, NULL);
         ReachabilityCollector_updateBandwidthAndDrops(
             pf->pub.rc,
-            Endian_bigEndianToHost32(lse.peerLabel_be),
-            Endian_bigEndianToHost32(lse.sumOfPackets_be),
-            Endian_bigEndianToHost32(lse.sumOfDrops_be),
-            Endian_bigEndianToHost32(lse.sumOfKb_be)
-        );
+            lse.peerLabel,
+            lse.sumOfPackets,
+            lse.sumOfDrops,
+            lse.sumOfKb);
     }
     return NULL;
 }

diff --git a/wire/Announce.h b/wire/Announce.h
@@ -32,6 +32,13 @@ enum Announce_Type {
     Announce_Type_LINK_STATE
 };
 
+/**
+ *                      1               2               3
+ *      0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7
+ *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *   0 |     length    |      type     |             version           |
+ *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
 struct Announce_Version
 {
     // Announce_Version_SIZE

diff --git a/wire/PFChan.h b/wire/PFChan.h
@@ -306,13 +306,14 @@ enum PFChan_Core
     PFChan_Core__TOO_HIGH = 1040,
 };
 
+// All values are in host order
 struct PFChan_LinkState_Entry {
-    uint32_t peerLabel_be;
-    uint32_t sumOfPackets_be;
-    uint32_t sumOfDrops_be;
-    uint32_t sumOfKb_be;
+    uint64_t peerLabel;
+    uint64_t sumOfPackets;
+    uint64_t sumOfDrops;
+    uint64_t sumOfKb;
 };
-#define PFChan_LinkState_Entry_SIZE 16
+#define PFChan_LinkState_Entry_SIZE 32
 Assert_compileTime(sizeof(struct PFChan_LinkState_Entry) == PFChan_LinkState_Entry_SIZE);
 
 struct PFChan_Core_SearchReq