diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index c7e651a18e..425a1f7e10 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -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()); } } } diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index 19022b77be..2b2d43987f 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -34,6 +34,7 @@ #include #include #include +#include #include "RateLimiter.h" #ifdef WITH_BRIDGE diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 0aff39cc1a..b951f4275d 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -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; } @@ -749,15 +748,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; } } diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index 5f78bee81a..a45ed9cd44 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include diff --git a/src/helpers/RoutingPolicy.h b/src/helpers/RoutingPolicy.h new file mode 100644 index 0000000000..5251a590a4 --- /dev/null +++ b/src/helpers/RoutingPolicy.h @@ -0,0 +1,68 @@ +#pragma once + +#include + +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; +} + +} diff --git a/test/test_routing_policy/test_routing_policy.cpp b/test/test_routing_policy/test_routing_policy.cpp new file mode 100644 index 0000000000..bba9ec7901 --- /dev/null +++ b/test/test_routing_policy/test_routing_policy.cpp @@ -0,0 +1,110 @@ +#include +#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(); +}