Calculate hops correctly even when hop_start==0 (#9120)

* Calculate hops correctly even when hop_start==0.

* Use the same type (int8_t) in the loop, avoiding signed/unsigned mismatches.

* Clarify defaultIfUnknown is returned for encrypted packets.
This commit is contained in:
Eric Severance
2025-12-30 13:31:35 -08:00
committed by GitHub
parent 3a723ceae8
commit 1b2dc10e77
14 changed files with 63 additions and 40 deletions

View File

@@ -195,7 +195,7 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src)
// but opted NOT TO. Because it is not a good idea to let remote nodes 'probe' to find out which PSKs were "good" vs
// bad.
routingModule->sendAckNak(meshtastic_Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel,
routingModule->getHopLimitForResponse(mp.hop_start, mp.hop_limit));
routingModule->getHopLimitForResponse(mp));
}
}
@@ -235,7 +235,7 @@ void setReplyTo(meshtastic_MeshPacket *p, const meshtastic_MeshPacket &to)
assert(p->which_payload_variant == meshtastic_MeshPacket_decoded_tag); // Should already be set by now
p->to = getFrom(&to); // Make sure that if we are sending to the local node, we use our local node addr, not 0
p->channel = to.channel; // Use the same channel that the request came in on
p->hop_limit = routingModule->getHopLimitForResponse(to.hop_start, to.hop_limit);
p->hop_limit = routingModule->getHopLimitForResponse(to);
// No need for an ack if we are just delivering locally (it just generates an ignored ack)
p->want_ack = (to.from != 0) ? to.want_ack : false;

View File

@@ -95,11 +95,8 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp)
} else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && !nodeDB->getMeshNode(mp->from)->has_user &&
nodeInfoModule && !isPreferredRebroadcaster && !nodeDB->isFull()) {
if (airTime->isTxAllowedChannelUtil(true)) {
// Hops used by the request. If somebody in between running modified firmware modified it, ignore it
auto hopStart = mp->hop_start;
auto hopLimit = mp->hop_limit;
uint8_t hopsUsed = hopStart < hopLimit ? config.lora.hop_limit : hopStart - hopLimit;
if (hopsUsed > config.lora.hop_limit + 2) {
const int8_t hopsUsed = getHopsAway(*mp, config.lora.hop_limit);
if (hopsUsed > (int32_t)(config.lora.hop_limit + 2)) {
LOG_DEBUG("Skip send NodeInfo: %d hops away is too far away", hopsUsed);
} else {
LOG_INFO("Heard new node on ch. %d, send NodeInfo and ask for response", mp->channel);

View File

@@ -64,7 +64,7 @@ bool NextHopRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
perhapsRebroadcast(p);
}
} else {
bool isRepeated = p->hop_start > 0 && p->hop_start == p->hop_limit;
bool isRepeated = getHopsAway(*p) == 0;
// If repeated and not in Tx queue anymore, try relaying again, or if we are the destination, send the ACK again
if (isRepeated) {
if (!findInTxQueue(p->from, p->id)) {
@@ -101,8 +101,7 @@ void NextHopRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtast
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 ((weWereRelayer && wasAlreadyRelayer) || (getHopsAway(*p) == 0 && 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 (was relayer %d we were sole %d)", p->from,
p->relay_node, wasAlreadyRelayer, weWereSoleRelayer);

View File

@@ -1549,6 +1549,23 @@ uint32_t sinceReceived(const meshtastic_MeshPacket *p)
return delta;
}
int8_t getHopsAway(const meshtastic_MeshPacket &p, int8_t defaultIfUnknown)
{
// Firmware prior to 2.3.0 (585805c) lacked a hop_start field. Firmware version 2.5.0 (bf34329) introduced a
// bitfield that is always present. Use the presence of the bitfield to determine if the origin's firmware
// version is guaranteed to have hop_start populated. Note that this can only be done for decoded packets as
// the bitfield is encrypted under the channel encryption key. For encrypted packets, this returns
// defaultIfUnknown when hop_start is 0.
if (p.hop_start == 0 && !(p.which_payload_variant == meshtastic_MeshPacket_decoded_tag && p.decoded.has_bitfield))
return defaultIfUnknown; // Cannot reliably determine the number of hops.
// Guard against invalid values.
if (p.hop_start < p.hop_limit)
return defaultIfUnknown;
return p.hop_start - p.hop_limit;
}
#define NUM_ONLINE_SECS (60 * 60 * 2) // 2 hrs to consider someone offline
size_t NodeDB::getNumOnlineMeshNodes(bool localOnly)
@@ -1801,9 +1818,10 @@ void NodeDB::updateFrom(const meshtastic_MeshPacket &mp)
info->via_mqtt = mp.via_mqtt; // Store if we received this packet via MQTT
// If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway
if (mp.hop_start != 0 && mp.hop_limit <= mp.hop_start) {
const int8_t hopsAway = getHopsAway(mp);
if (hopsAway >= 0) {
info->has_hops_away = true;
info->hops_away = mp.hop_start - mp.hop_limit;
info->hops_away = hopsAway;
}
sortMeshDB();
}

View File

@@ -110,6 +110,10 @@ uint32_t sinceLastSeen(const meshtastic_NodeInfoLite *n);
/// Given a packet, return how many seconds in the past (vs now) it was received
uint32_t sinceReceived(const meshtastic_MeshPacket *p);
/// Given a packet, return the number of hops used to reach this node.
/// Returns defaultIfUnknown if the number of hops couldn't be determined.
int8_t getHopsAway(const meshtastic_MeshPacket &p, int8_t defaultIfUnknown = -1);
enum LoadFileResult {
// Successfully opened the file
LOAD_SUCCESS = 1,

View File

@@ -1,6 +1,7 @@
#include "ReliableRouter.h"
#include "Default.h"
#include "MeshTypes.h"
#include "NodeDB.h"
#include "configuration.h"
#include "memGet.h"
#include "mesh-pb-constants.h"
@@ -108,12 +109,12 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas
// If this packet should always be ACKed reliably with want_ack back to the original sender, make sure we
// do that unconditionally.
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel,
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit), true);
routingModule->getHopLimitForResponse(*p), true);
} else if (!p->decoded.request_id && !p->decoded.reply_id) {
// If it's not an ACK or a reply, send an ACK.
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel,
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
} else if ((p->hop_start > 0 && p->hop_start == p->hop_limit) || p->next_hop != NO_NEXT_HOP_PREFERENCE) {
routingModule->getHopLimitForResponse(*p));
} else if ((getHopsAway(*p) == 0) || p->next_hop != NO_NEXT_HOP_PREFERENCE) {
// If we received the packet directly from the original sender, send a 0-hop ACK since the original sender
// won't overhear any implicit ACKs. If we received the packet via NextHopRouter, also send a 0-hop ACK to
// stop the immediate relayer's retransmissions.
@@ -123,11 +124,11 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas
(nodeDB->getMeshNode(p->from) == nullptr || nodeDB->getMeshNode(p->from)->user.public_key.size == 0)) {
LOG_INFO("PKI packet from unknown node, send PKI_UNKNOWN_PUBKEY");
sendAckNak(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, getFrom(p), p->id, channels.getPrimaryIndex(),
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
routingModule->getHopLimitForResponse(*p));
} else {
// Send a 'NO_CHANNEL' error on the primary channel if want_ack packet destined for us cannot be decoded
sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(),
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
routingModule->getHopLimitForResponse(*p));
}
} else if (p->next_hop == nodeDB->getLastByteOfNodeNum(getNodeNum()) && p->hop_limit > 0) {
// No wantAck, but we need to ACK with hop limit of 0 if we were the next hop to stop their retransmissions

View File

@@ -81,8 +81,7 @@ Router::Router() : concurrency::OSThread("Router"), fromRadioQueue(MAX_RX_FROMRA
bool Router::shouldDecrementHopLimit(const meshtastic_MeshPacket *p)
{
// First hop MUST always decrement to prevent retry issues
bool isFirstHop = (p->hop_start != 0 && p->hop_start == p->hop_limit);
if (isFirstHop) {
if (getHopsAway(*p) == 0) {
return true; // Always decrement on first hop
}