From 1fc07607cb7f6b141245fbb8b08f0a14b425ae6b Mon Sep 17 00:00:00 2001 From: GUVWAF Date: Sat, 20 Sep 2025 13:02:43 +0200 Subject: [PATCH 1/2] Make sure next-hop is only set when they received us directly --- src/mesh/FloodingRouter.cpp | 3 +-- src/mesh/NextHopRouter.cpp | 11 +++++++---- src/mesh/PacketHistory.cpp | 32 ++++++++++++++++++++------------ src/mesh/PacketHistory.h | 10 ++++------ 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index f805055c8..0f4b57983 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -102,8 +102,7 @@ void FloodingRouter::perhapsRebroadcast(const meshtastic_MeshPacket *p) LOG_INFO("Rebroadcast received floodmsg"); // Note: we are careful to resend using the original senders node id - // We are careful not to call our hooked version of send() - because we don't want to check this again - Router::send(tosend); + send(tosend); } else { LOG_DEBUG("No rebroadcast: Role = CLIENT_MUTE or Rebroadcast Mode = NONE"); } diff --git a/src/mesh/NextHopRouter.cpp b/src/mesh/NextHopRouter.cpp index 9bb8b240c..084452eed 100644 --- a/src/mesh/NextHopRouter.cpp +++ b/src/mesh/NextHopRouter.cpp @@ -76,11 +76,14 @@ void NextHopRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtast if (origTx) { // Either relayer of ACK was also a relayer of the packet, or we were the *only* relayer and the ACK came directly // from the destination - if (wasRelayer(p->relay_node, p->decoded.request_id, p->to) || - (p->hop_start != 0 && p->hop_start == p->hop_limit && - wasSoleRelayer(ourRelayID, p->decoded.request_id, p->to))) { + bool wasAlreadyRelayer = wasRelayer(p->relay_node, p->decoded.request_id, p->to); + bool weWereSoleRelayer = false; + bool weWereRelayer = wasRelayer(ourRelayID, p->decoded.request_id, p->to, &weWereSoleRelayer); + if ((weWereRelayer && wasAlreadyRelayer) || + (p->hop_start != 0 && p->hop_start == p->hop_limit && weWereSoleRelayer)) { if (origTx->next_hop != p->relay_node) { // Not already set - LOG_INFO("Update next hop of 0x%x to 0x%x based on ACK/reply", p->from, p->relay_node); + LOG_INFO("Update next hop of 0x%x to 0x%x based on ACK/reply (was relayer %d we were sole %d)", p->from, + p->relay_node, wasAlreadyRelayer, weWereSoleRelayer); origTx->next_hop = p->relay_node; } } diff --git a/src/mesh/PacketHistory.cpp b/src/mesh/PacketHistory.cpp index 735386d79..09b1f84c9 100644 --- a/src/mesh/PacketHistory.cpp +++ b/src/mesh/PacketHistory.cpp @@ -66,7 +66,13 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd r.id = p->id; r.sender = getFrom(p); // If 0 then use our ID r.next_hop = p->next_hop; - r.relayed_by[0] = p->relay_node; + bool weWillRelay = false; + uint8_t ourRelayID = nodeDB->getLastByteOfNodeNum(nodeDB->getNodeNum()); + if (p->relay_node == ourRelayID) { // If the relay_node is us, store it + weWillRelay = true; + r.ourTxHopLimit = p->hop_limit; + r.relayed_by[0] = p->relay_node; + } r.rxTimeMsec = millis(); // if (r.rxTimeMsec == 0) // =0 every 49.7 days? 0 is special @@ -82,8 +88,6 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd bool seenRecently = (found != NULL); // If found -> the packet was seen recently if (seenRecently) { - uint8_t ourRelayID = nodeDB->getLastByteOfNodeNum(nodeDB->getNodeNum()); // Get our relay ID from our node number - if (wasFallback) { // If it was seen with a next-hop not set to us and now it's NO_NEXT_HOP_PREFERENCE, and the relayer relayed already // before, it's a fallback to flooding. If we didn't already relay and the next-hop neither, we might need to handle @@ -125,11 +129,23 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd found->sender, found->id, found->next_hop, found->relayed_by[0], found->relayed_by[1], found->relayed_by[2], millis() - found->rxTimeMsec); #endif + // Only update the relayer if it heard us directly (meaning hopLimit is decreased by 1) + uint8_t startIdx = weWillRelay ? 1 : 0; + if (!weWillRelay) { + bool weWereRelayer = wasRelayer(ourRelayID, *found); + // We were a relayer and the packet came in with a hop limit that is one less than when we sent it out + if (weWereRelayer && p->hop_limit == found->ourTxHopLimit - 1) { + r.relayed_by[0] = p->relay_node; + startIdx = 1; // Start copying existing relayers from index 1 + } + // keep the original ourTxHopLimit + r.ourTxHopLimit = found->ourTxHopLimit; + } // Add the existing relayed_by to the new record for (uint8_t i = 0; i < (NUM_RELAYERS - 1); i++) { if (found->relayed_by[i] != 0) - r.relayed_by[i + 1] = found->relayed_by[i]; + r.relayed_by[i + startIdx] = found->relayed_by[i]; } r.next_hop = found->next_hop; // keep the original next_hop (such that we check whether we were originally asked) #if VERBOSE_PACKET_HISTORY @@ -352,14 +368,6 @@ bool PacketHistory::wasRelayer(const uint8_t relayer, const PacketRecord &r, boo return found; } -// Check if a certain node was the *only* relayer of a packet in the history given an ID and sender -bool PacketHistory::wasSoleRelayer(const uint8_t relayer, const uint32_t id, const NodeNum sender) -{ - bool wasSole = false; - wasRelayer(relayer, id, sender, &wasSole); - return wasSole; -} - // Remove a relayer from the list of relayers of a packet in the history given an ID and sender void PacketHistory::removeRelayer(const uint8_t relayer, const uint32_t id, const NodeNum sender) { diff --git a/src/mesh/PacketHistory.h b/src/mesh/PacketHistory.h index 4b53c8f6a..20aac9999 100644 --- a/src/mesh/PacketHistory.h +++ b/src/mesh/PacketHistory.h @@ -2,8 +2,8 @@ #include "NodeDB.h" -#define NUM_RELAYERS \ - 3 // Number of relayer we keep track of. Use 3 to be efficient with memory alignment of PacketRecord to 16 bytes +// Number of relayers we keep track of. Use 6 to be efficient with memory alignment of PacketRecord to 20 bytes +#define NUM_RELAYERS 6 /** * This is a mixin that adds a record of past packets we have seen @@ -16,8 +16,9 @@ class PacketHistory PacketId id; uint32_t rxTimeMsec; // Unix time in msecs - the time we received it, 0 means empty uint8_t next_hop; // The next hop asked for this packet + uint8_t ourTxHopLimit; // The hop limit of the packet when we first transmitted it uint8_t relayed_by[NUM_RELAYERS]; // Array of nodes that relayed this packet - }; // 4B + 4B + 4B + 1B + 3B = 16B + }; // 4B + 4B + 4B + 1B + 1B + 6B = 20B uint32_t recentPacketsCapacity = 0; // Can be set in constructor, no need to recompile. Used to allocate memory for mx_recentPackets. @@ -59,9 +60,6 @@ class PacketHistory * @return true if node was indeed a relayer, false if not */ bool wasRelayer(const uint8_t relayer, const uint32_t id, const NodeNum sender, bool *wasSole = nullptr); - // Check if a certain node was the *only* relayer of a packet in the history given an ID and sender - bool wasSoleRelayer(const uint8_t relayer, const uint32_t id, const NodeNum sender); - // Remove a relayer from the list of relayers of a packet in the history given an ID and sender void removeRelayer(const uint8_t relayer, const uint32_t id, const NodeNum sender); From 12c3ddf457a80390107e1cf253cf58fca03fbcdd Mon Sep 17 00:00:00 2001 From: GUVWAF Date: Thu, 25 Sep 2025 19:54:29 +0200 Subject: [PATCH 2/2] Resolve comments --- src/mesh/PacketHistory.cpp | 51 +++++++++++++++++++++++++++++++++----- src/mesh/PacketHistory.h | 11 +++++++- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/mesh/PacketHistory.cpp b/src/mesh/PacketHistory.cpp index 09b1f84c9..97ed82b3d 100644 --- a/src/mesh/PacketHistory.cpp +++ b/src/mesh/PacketHistory.cpp @@ -66,11 +66,12 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd r.id = p->id; r.sender = getFrom(p); // If 0 then use our ID r.next_hop = p->next_hop; + setHighestHopLimit(r, p->hop_limit); bool weWillRelay = false; uint8_t ourRelayID = nodeDB->getLastByteOfNodeNum(nodeDB->getNodeNum()); if (p->relay_node == ourRelayID) { // If the relay_node is us, store it weWillRelay = true; - r.ourTxHopLimit = p->hop_limit; + setOurTxHopLimit(r, p->hop_limit); r.relayed_by[0] = p->relay_node; } @@ -134,18 +135,35 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd if (!weWillRelay) { bool weWereRelayer = wasRelayer(ourRelayID, *found); // We were a relayer and the packet came in with a hop limit that is one less than when we sent it out - if (weWereRelayer && p->hop_limit == found->ourTxHopLimit - 1) { + if (weWereRelayer && (p->hop_limit == getOurTxHopLimit(*found) || p->hop_limit == getOurTxHopLimit(*found) - 1)) { r.relayed_by[0] = p->relay_node; startIdx = 1; // Start copying existing relayers from index 1 } // keep the original ourTxHopLimit - r.ourTxHopLimit = found->ourTxHopLimit; + setOurTxHopLimit(r, getOurTxHopLimit(*found)); } - // Add the existing relayed_by to the new record - for (uint8_t i = 0; i < (NUM_RELAYERS - 1); i++) { - if (found->relayed_by[i] != 0) + // Preserve the highest hop_limit we've ever seen for this packet so upgrades aren't lost when a later copy has + // fewer hops remaining. + if (getHighestHopLimit(*found) > getHighestHopLimit(r)) + setHighestHopLimit(r, getHighestHopLimit(*found)); + + // Add the existing relayed_by to the new record, avoiding duplicates + for (uint8_t i = 0; i < (NUM_RELAYERS - startIdx); i++) { + if (found->relayed_by[i] == 0) + continue; + + bool exists = false; + for (uint8_t j = 0; j < NUM_RELAYERS; j++) { + if (r.relayed_by[j] == found->relayed_by[i]) { + exists = true; + break; + } + } + + if (!exists) { r.relayed_by[i + startIdx] = found->relayed_by[i]; + } } r.next_hop = found->next_hop; // keep the original next_hop (such that we check whether we were originally asked) #if VERBOSE_PACKET_HISTORY @@ -409,3 +427,24 @@ void PacketHistory::removeRelayer(const uint8_t relayer, const uint32_t id, cons found->id, found->relayed_by[0], found->relayed_by[1], found->relayed_by[2], relayer, i != j); #endif } + +// Getters and setters for hop limit fields packed in hop_limit +inline uint8_t PacketHistory::getHighestHopLimit(PacketRecord &r) +{ + return r.hop_limit & HOP_LIMIT_HIGHEST_MASK; +} + +inline void PacketHistory::setHighestHopLimit(PacketRecord &r, uint8_t hopLimit) +{ + r.hop_limit = (r.hop_limit & ~HOP_LIMIT_HIGHEST_MASK) | (hopLimit & HOP_LIMIT_HIGHEST_MASK); +} + +inline uint8_t PacketHistory::getOurTxHopLimit(PacketRecord &r) +{ + return (r.hop_limit & HOP_LIMIT_OUR_TX_MASK) >> HOP_LIMIT_OUR_TX_SHIFT; +} + +inline void PacketHistory::setOurTxHopLimit(PacketRecord &r, uint8_t hopLimit) +{ + r.hop_limit = (r.hop_limit & ~HOP_LIMIT_OUR_TX_MASK) | ((hopLimit << HOP_LIMIT_OUR_TX_SHIFT) & HOP_LIMIT_OUR_TX_MASK); +} \ No newline at end of file diff --git a/src/mesh/PacketHistory.h b/src/mesh/PacketHistory.h index 20aac9999..08a625fe1 100644 --- a/src/mesh/PacketHistory.h +++ b/src/mesh/PacketHistory.h @@ -4,6 +4,9 @@ // Number of relayers we keep track of. Use 6 to be efficient with memory alignment of PacketRecord to 20 bytes #define NUM_RELAYERS 6 +#define HOP_LIMIT_HIGHEST_MASK 0x07 // Bits 0-2 +#define HOP_LIMIT_OUR_TX_MASK 0x38 // Bits 3-5 +#define HOP_LIMIT_OUR_TX_SHIFT 3 // Bits 3-5 /** * This is a mixin that adds a record of past packets we have seen @@ -16,7 +19,8 @@ class PacketHistory PacketId id; uint32_t rxTimeMsec; // Unix time in msecs - the time we received it, 0 means empty uint8_t next_hop; // The next hop asked for this packet - uint8_t ourTxHopLimit; // The hop limit of the packet when we first transmitted it + uint8_t hop_limit; // bit 0-2: Highest hop limit observed for this packet, + // bit 3-5: our hop limit when we first transmitted it uint8_t relayed_by[NUM_RELAYERS]; // Array of nodes that relayed this packet }; // 4B + 4B + 4B + 1B + 1B + 6B = 20B @@ -39,6 +43,11 @@ class PacketHistory * @return true if node was indeed a relayer, false if not */ bool wasRelayer(const uint8_t relayer, const PacketRecord &r, bool *wasSole = nullptr); + uint8_t getHighestHopLimit(PacketRecord &r); + void setHighestHopLimit(PacketRecord &r, uint8_t hopLimit); + uint8_t getOurTxHopLimit(PacketRecord &r); + void setOurTxHopLimit(PacketRecord &r, uint8_t hopLimit); + PacketHistory(const PacketHistory &); // non construction-copyable PacketHistory &operator=(const PacketHistory &); // non copyable public: