mirror of
https://github.com/meshcore-dev/MeshCore.git
synced 2026-08-20 21:29:47 +00:00
Merge pull request #3106 from ViezeVingertjes/fix/scoped-reply-routing
Fix replies dropped when flood.max.unscoped is low
This commit is contained in:
ripplebiz
@@ -411,24 +411,31 @@ bool MyMesh::isLooped(const mesh::Packet* packet, const uint8_t max_counters[])
|
||||
}
|
||||
|
||||
void MyMesh::sendFloodReply(mesh::Packet* packet, unsigned long delay_millis, uint8_t path_hash_size) {
|
||||
if (recv_pkt_region && !recv_pkt_region->isWildcard()) { // if _request_ packet scope is known, send reply with same scope
|
||||
TransportKey scope;
|
||||
if (region_map.getTransportKeysFor(*recv_pkt_region, &scope, 1) > 0) {
|
||||
sendFloodScoped(scope, packet, delay_millis, path_hash_size);
|
||||
} else {
|
||||
TransportKey req_scope;
|
||||
bool is_wildcard = recv_pkt_region != NULL && recv_pkt_region->isWildcard();
|
||||
bool req_scope_known = recv_pkt_region != NULL && !is_wildcard
|
||||
&& region_map.getTransportKeysFor(*recv_pkt_region, &req_scope, 1) > 0;
|
||||
|
||||
switch (mesh::chooseReplyScope(req_scope_known, is_wildcard, !default_scope.isNull())) {
|
||||
case mesh::REPLY_SCOPE_REQUEST:
|
||||
sendFloodScoped(req_scope, packet, delay_millis, path_hash_size); // reply with same scope as request
|
||||
break;
|
||||
case mesh::REPLY_SCOPE_DEFAULT:
|
||||
// requester's scope is unknown: DIRECT request (no transport codes), or code matched no Region.
|
||||
// un-scoped would be dropped at hop 0 by repeaters running flood.max.unscoped=0
|
||||
sendFloodScoped(default_scope, packet, delay_millis, path_hash_size);
|
||||
break;
|
||||
case mesh::REPLY_SCOPE_NONE:
|
||||
sendFlood(packet, delay_millis, path_hash_size); // send un-scoped
|
||||
}
|
||||
} else {
|
||||
sendFlood(packet, delay_millis, path_hash_size); // send un-scoped
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
bool MyMesh::allowPacketForward(const mesh::Packet *packet) {
|
||||
if (_prefs.disable_fwd) return false;
|
||||
if (packet->isRouteFlood()) {
|
||||
if (packet->getPathHashCount() >= _prefs.flood_max) return false;
|
||||
if (packet->getRouteType() == ROUTE_TYPE_FLOOD && packet->getPathHashCount() >= _prefs.flood_max_unscoped) return false;
|
||||
if (packet->getPayloadType() == PAYLOAD_TYPE_ADVERT && packet->getPathHashCount() >= _prefs.flood_max_advert) return false;
|
||||
if (packet->isRouteFlood()
|
||||
&& mesh::isFloodHopLimitExceeded(packet, _prefs.flood_max, _prefs.flood_max_unscoped, _prefs.flood_max_advert)) {
|
||||
return false;
|
||||
}
|
||||
if (packet->isRouteFlood() && recv_pkt_region == NULL) {
|
||||
MESH_DEBUG_PRINTLN("allowPacketForward: unknown transport code, or wildcard not allowed for FLOOD packet");
|
||||
@@ -586,17 +593,29 @@ void MyMesh::onAnonDataRecv(mesh::Packet *packet, const uint8_t *secret, const m
|
||||
|
||||
if (reply_len == 0) return; // invalid request
|
||||
|
||||
if (packet->isRouteFlood()) {
|
||||
// a DIRECT login can reply via the stored out_path, as onPeerDataRecv() does for REQ
|
||||
ClientInfo* client = acl.getClient(sender.pub_key, PUB_KEY_SIZE);
|
||||
bool have_out_path = client != NULL && client->out_path_len != OUT_PATH_UNKNOWN;
|
||||
|
||||
auto route = mesh::chooseReplyRoute(packet->isRouteFlood(), reply_path_len != 0xFF, have_out_path);
|
||||
|
||||
if (route == mesh::REPLY_ROUTE_PATH_RETURN) {
|
||||
// let this sender know path TO here, so they can use sendDirect(), and ALSO encode the response
|
||||
mesh::Packet* path = createPathReturn(sender, secret, packet->path, packet->path_len,
|
||||
PAYLOAD_TYPE_RESPONSE, reply_data, reply_len);
|
||||
if (path) sendFloodReply(path, SERVER_RESPONSE_DELAY, packet->getPathHashSize());
|
||||
} else if (reply_path_len == 0xFF) {
|
||||
mesh::Packet* reply = createDatagram(PAYLOAD_TYPE_RESPONSE, sender, secret, reply_data, reply_len);
|
||||
if (reply) sendFloodReply(reply, SERVER_RESPONSE_DELAY, packet->getPathHashSize());
|
||||
return;
|
||||
}
|
||||
|
||||
mesh::Packet* reply = createDatagram(PAYLOAD_TYPE_RESPONSE, sender, secret, reply_data, reply_len);
|
||||
if (reply == NULL) return;
|
||||
|
||||
if (route == mesh::REPLY_ROUTE_DIRECT_SUPPLIED) {
|
||||
sendDirect(reply, reply_path, reply_path_len, SERVER_RESPONSE_DELAY);
|
||||
} else if (route == mesh::REPLY_ROUTE_DIRECT_OUT_PATH) {
|
||||
sendDirect(reply, client->out_path, client->out_path_len, SERVER_RESPONSE_DELAY);
|
||||
} else {
|
||||
mesh::Packet* reply = createDatagram(PAYLOAD_TYPE_RESPONSE, sender, secret, reply_data, reply_len);
|
||||
if (reply) sendDirect(reply, reply_path, reply_path_len, SERVER_RESPONSE_DELAY);
|
||||
sendFloodReply(reply, SERVER_RESPONSE_DELAY, packet->getPathHashSize());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,7 @@
|
||||
#include <helpers/StatsFormatHelper.h>
|
||||
#include <helpers/TxtDataHelpers.h>
|
||||
#include <helpers/RegionMap.h>
|
||||
#include <helpers/RoutingPolicy.h>
|
||||
#include "RateLimiter.h"
|
||||
|
||||
#ifdef WITH_BRIDGE
|
||||
|
||||
@@ -299,10 +299,9 @@ uint32_t MyMesh::getDirectRetransmitDelay(const mesh::Packet *packet) {
|
||||
|
||||
bool MyMesh::allowPacketForward(const mesh::Packet *packet) {
|
||||
if (_prefs.disable_fwd) return false;
|
||||
if (packet->isRouteFlood()) {
|
||||
if (packet->getPathHashCount() >= _prefs.flood_max) return false;
|
||||
if (packet->getRouteType() == ROUTE_TYPE_FLOOD && packet->getPathHashCount() >= _prefs.flood_max_unscoped) return false;
|
||||
if (packet->getPayloadType() == PAYLOAD_TYPE_ADVERT && packet->getPathHashCount() >= _prefs.flood_max_advert) return false;
|
||||
if (packet->isRouteFlood()
|
||||
&& mesh::isFloodHopLimitExceeded(packet, _prefs.flood_max, _prefs.flood_max_unscoped, _prefs.flood_max_advert)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
@@ -751,15 +750,23 @@ void MyMesh::sendFloodScoped(const TransportKey& scope, mesh::Packet* pkt, uint3
|
||||
}
|
||||
|
||||
void MyMesh::sendFloodReply(mesh::Packet* packet, unsigned long delay_millis, uint8_t path_hash_size) {
|
||||
if (recv_pkt_region && !recv_pkt_region->isWildcard()) { // if _request_ packet scope is known, send reply with same scope
|
||||
TransportKey scope;
|
||||
if (region_map.getTransportKeysFor(*recv_pkt_region, &scope, 1) > 0) {
|
||||
sendFloodScoped(scope, packet, delay_millis, path_hash_size);
|
||||
} else {
|
||||
TransportKey req_scope;
|
||||
bool is_wildcard = recv_pkt_region != NULL && recv_pkt_region->isWildcard();
|
||||
bool req_scope_known = recv_pkt_region != NULL && !is_wildcard
|
||||
&& region_map.getTransportKeysFor(*recv_pkt_region, &req_scope, 1) > 0;
|
||||
|
||||
switch (mesh::chooseReplyScope(req_scope_known, is_wildcard, !default_scope.isNull())) {
|
||||
case mesh::REPLY_SCOPE_REQUEST:
|
||||
sendFloodScoped(req_scope, packet, delay_millis, path_hash_size); // reply with same scope as request
|
||||
break;
|
||||
case mesh::REPLY_SCOPE_DEFAULT:
|
||||
// requester's scope is unknown: DIRECT request (no transport codes), or code matched no Region.
|
||||
// un-scoped would be dropped at hop 0 by repeaters running flood.max.unscoped=0
|
||||
sendFloodScoped(default_scope, packet, delay_millis, path_hash_size);
|
||||
break;
|
||||
case mesh::REPLY_SCOPE_NONE:
|
||||
sendFlood(packet, delay_millis, path_hash_size); // send un-scoped
|
||||
}
|
||||
} else {
|
||||
sendFlood(packet, delay_millis, path_hash_size); // send un-scoped
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
#include <helpers/StatsFormatHelper.h>
|
||||
#include <helpers/ClientACL.h>
|
||||
#include <helpers/RegionMap.h>
|
||||
#include <helpers/RoutingPolicy.h>
|
||||
#include <RTClib.h>
|
||||
#include <target.h>
|
||||
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
#pragma once
|
||||
|
||||
#include <Packet.h>
|
||||
|
||||
namespace mesh {
|
||||
|
||||
/**
|
||||
* \brief Test a flood packet against the configured hop limits.
|
||||
* \param packet inbound flood packet (caller has already checked isRouteFlood())
|
||||
* \param flood_max max hops for any flood packet
|
||||
* \param flood_max_unscoped max hops for ROUTE_TYPE_FLOOD (ie. un-scoped) packets
|
||||
* \param flood_max_advert max hops for ADVERT packets
|
||||
* \returns true if the packet has exceeded a limit, and must not be forwarded
|
||||
*/
|
||||
inline bool isFloodHopLimitExceeded(const Packet* packet, uint8_t flood_max,
|
||||
uint8_t flood_max_unscoped, uint8_t flood_max_advert) {
|
||||
uint8_t hops = packet->getPathHashCount();
|
||||
if (hops >= flood_max) return true;
|
||||
if (packet->getRouteType() == ROUTE_TYPE_FLOOD && hops >= flood_max_unscoped) return true;
|
||||
if (packet->getPayloadType() == PAYLOAD_TYPE_ADVERT && hops >= flood_max_advert) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* \brief How a server routes a reply back to the requesting client.
|
||||
*/
|
||||
enum ReplyRoute : uint8_t {
|
||||
REPLY_ROUTE_PATH_RETURN, // request arrived by flood: reply with a PATH return, flooded back
|
||||
REPLY_ROUTE_DIRECT_SUPPLIED, // reply DIRECT, along the return path supplied in the request
|
||||
REPLY_ROUTE_DIRECT_OUT_PATH, // reply DIRECT, along the out_path already stored for this client
|
||||
REPLY_ROUTE_FLOOD, // no return path known: flood the reply
|
||||
};
|
||||
|
||||
/**
|
||||
* \param inbound_is_flood the request arrived as a flood packet
|
||||
* \param have_supplied_path the request payload carried an explicit reply path
|
||||
* \param have_out_path this server already has a stored out_path for the client
|
||||
*/
|
||||
inline ReplyRoute chooseReplyRoute(bool inbound_is_flood, bool have_supplied_path, bool have_out_path) {
|
||||
if (inbound_is_flood) return REPLY_ROUTE_PATH_RETURN;
|
||||
if (have_supplied_path) return REPLY_ROUTE_DIRECT_SUPPLIED;
|
||||
if (have_out_path) return REPLY_ROUTE_DIRECT_OUT_PATH;
|
||||
return REPLY_ROUTE_FLOOD;
|
||||
}
|
||||
|
||||
/**
|
||||
* \brief Which transport scope a flooded reply should be sent with.
|
||||
*/
|
||||
enum ReplyScope : uint8_t {
|
||||
REPLY_SCOPE_REQUEST, // re-use the scope the request arrived on
|
||||
REPLY_SCOPE_DEFAULT, // fall back to this node's default region scope
|
||||
REPLY_SCOPE_NONE, // send un-scoped (ROUTE_TYPE_FLOOD)
|
||||
};
|
||||
|
||||
/**
|
||||
* \param request_scope_known request arrived scoped, and we resolved its Region's key
|
||||
* \param request_was_unscoped_flood request arrived as an un-scoped flood
|
||||
* \param default_scope_known this node has a default Region with a usable transport key
|
||||
*/
|
||||
inline ReplyScope chooseReplyScope(bool request_scope_known, bool request_was_unscoped_flood,
|
||||
bool default_scope_known) {
|
||||
if (request_scope_known) return REPLY_SCOPE_REQUEST;
|
||||
if (request_was_unscoped_flood) return REPLY_SCOPE_NONE; // requester chose un-scoped, so mirror it
|
||||
if (default_scope_known) return REPLY_SCOPE_DEFAULT; // scope unknowable: DIRECT, or unresolved Region
|
||||
return REPLY_SCOPE_NONE;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -0,0 +1,110 @@
|
||||
#include <gtest/gtest.h>
|
||||
#include "helpers/RoutingPolicy.h"
|
||||
|
||||
using namespace mesh;
|
||||
|
||||
static Packet makeFlood(uint8_t route_type, uint8_t payload_type, uint8_t hops) {
|
||||
Packet p;
|
||||
p.header = route_type | (payload_type << PH_TYPE_SHIFT);
|
||||
p.setPathHashSizeAndCount(1, hops);
|
||||
p.payload_len = 1;
|
||||
return p;
|
||||
}
|
||||
|
||||
TEST(FloodHopLimit, UnscopedFloodIsDroppedAtFirstHopWhenMaxUnscopedIsZero) {
|
||||
auto pkt = makeFlood(ROUTE_TYPE_FLOOD, PAYLOAD_TYPE_RESPONSE, 0);
|
||||
EXPECT_TRUE(isFloodHopLimitExceeded(&pkt, 64, 0, 8));
|
||||
}
|
||||
|
||||
TEST(FloodHopLimit, ScopedFloodIsForwardedWhenMaxUnscopedIsZero) {
|
||||
for (uint8_t hops = 0; hops < 4; hops++) {
|
||||
auto pkt = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_RESPONSE, hops);
|
||||
EXPECT_FALSE(isFloodHopLimitExceeded(&pkt, 64, 0, 8)) << "hops=" << (int)hops;
|
||||
}
|
||||
}
|
||||
|
||||
TEST(FloodHopLimit, UnscopedFloodSurvivesUpToMaxUnscopedHops) {
|
||||
// matches the reported workaround: raising flood.max.unscoped to the expected hop count
|
||||
auto ok = makeFlood(ROUTE_TYPE_FLOOD, PAYLOAD_TYPE_RESPONSE, 2);
|
||||
EXPECT_FALSE(isFloodHopLimitExceeded(&ok, 64, 3, 8));
|
||||
|
||||
auto too_far = makeFlood(ROUTE_TYPE_FLOOD, PAYLOAD_TYPE_RESPONSE, 3);
|
||||
EXPECT_TRUE(isFloodHopLimitExceeded(&too_far, 64, 3, 8));
|
||||
}
|
||||
|
||||
TEST(FloodHopLimit, ScopedFloodStillHonoursFloodMaxAndAdvertMax) {
|
||||
auto beyond_max = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_RESPONSE, 5);
|
||||
EXPECT_TRUE(isFloodHopLimitExceeded(&beyond_max, 5, 64, 8));
|
||||
|
||||
auto advert = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_ADVERT, 8);
|
||||
EXPECT_TRUE(isFloodHopLimitExceeded(&advert, 64, 64, 8));
|
||||
}
|
||||
|
||||
// flood.max.unscoped=0 hits adverts too, well before flood_max_advert applies: a node
|
||||
// still advertising un-scoped is invisible past its immediate neighbours
|
||||
TEST(FloodHopLimit, UnscopedAdvertIsAlsoDroppedAtHopZero) {
|
||||
auto advert = makeFlood(ROUTE_TYPE_FLOOD, PAYLOAD_TYPE_ADVERT, 0);
|
||||
EXPECT_TRUE(isFloodHopLimitExceeded(&advert, 64, 0, 8));
|
||||
|
||||
auto scoped = makeFlood(ROUTE_TYPE_TRANSPORT_FLOOD, PAYLOAD_TYPE_ADVERT, 0);
|
||||
EXPECT_FALSE(isFloodHopLimitExceeded(&scoped, 64, 0, 8));
|
||||
}
|
||||
|
||||
TEST(ReplyRoute, FloodRequestGetsAPathReturn) {
|
||||
EXPECT_EQ(REPLY_ROUTE_PATH_RETURN,
|
||||
chooseReplyRoute(true, false, false));
|
||||
EXPECT_EQ(REPLY_ROUTE_PATH_RETURN,
|
||||
chooseReplyRoute(true, false, true));
|
||||
}
|
||||
|
||||
TEST(ReplyRoute, DirectRequestWithSuppliedPathRepliesDirect) {
|
||||
EXPECT_EQ(REPLY_ROUTE_DIRECT_SUPPLIED, chooseReplyRoute(false, true, false));
|
||||
}
|
||||
|
||||
// the reported bug: a DIRECT login (app already has a path) was answered by flooding, even
|
||||
// with an out_path stored. Under flood.max.unscoped=0 that reply never arrives.
|
||||
TEST(ReplyRoute, DirectRequestWithKnownOutPathRepliesDirect) {
|
||||
EXPECT_EQ(REPLY_ROUTE_DIRECT_OUT_PATH,
|
||||
chooseReplyRoute(false, false, true));
|
||||
}
|
||||
|
||||
TEST(ReplyRoute, SuppliedPathWinsOverStoredOutPath) {
|
||||
EXPECT_EQ(REPLY_ROUTE_DIRECT_SUPPLIED, chooseReplyRoute(false, true, true));
|
||||
}
|
||||
|
||||
TEST(ReplyRoute, DirectRequestWithNoReturnPathFallsBackToFlood) {
|
||||
EXPECT_EQ(REPLY_ROUTE_FLOOD, chooseReplyRoute(false, false, false));
|
||||
}
|
||||
|
||||
TEST(ReplyScope, MirrorsTheRequestScopeWhenKnown) {
|
||||
EXPECT_EQ(REPLY_SCOPE_REQUEST, chooseReplyScope(true,
|
||||
false,
|
||||
false));
|
||||
EXPECT_EQ(REPLY_SCOPE_REQUEST, chooseReplyScope(true, false, true));
|
||||
}
|
||||
|
||||
// un-scoped is itself a known scope, so mirror it. Replying scoped would change a path that
|
||||
// works today, and repeaters not holding our default Region would drop it anyway.
|
||||
TEST(ReplyScope, RepliesUnscopedToAnUnscopedFloodEvenWhenADefaultScopeExists) {
|
||||
EXPECT_EQ(REPLY_SCOPE_NONE, chooseReplyScope(false,
|
||||
true,
|
||||
true));
|
||||
}
|
||||
|
||||
// second half of the bug: a DIRECT request carries no transport codes, so recv_pkt_region is
|
||||
// always NULL. Un-scoped is dropped under flood.max.unscoped=0, and floods the mesh otherwise.
|
||||
TEST(ReplyScope, FallsBackToDefaultScopeWhenRequestScopeUnknown) {
|
||||
EXPECT_EQ(REPLY_SCOPE_DEFAULT, chooseReplyScope(false,
|
||||
false,
|
||||
true));
|
||||
}
|
||||
|
||||
TEST(ReplyScope, SendsUnscopedOnlyWhenNoScopeIsAvailableAtAll) {
|
||||
EXPECT_EQ(REPLY_SCOPE_NONE, chooseReplyScope(false, false, false));
|
||||
EXPECT_EQ(REPLY_SCOPE_NONE, chooseReplyScope(false, true, false));
|
||||
}
|
||||
|
||||
int main(int argc, char** argv) {
|
||||
::testing::InitGoogleTest(&argc, argv);
|
||||
return RUN_ALL_TESTS();
|
||||
}
|
||||
Reference in New Issue
Block a user