Release packets the interface declines to send (#11087)

src/mesh/NextHopRouter.cpp

@@ -159,6 +159,9 @@ bool NextHopRouter::perhapsRebroadcast(const meshtastic_MeshPacket *p)
     }
 #endif
 
+    if (p->to == NODENUM_BROADCAST_NO_LORA)
+        return false;
+
     // Allow rebroadcast if hop_limit > 0 OR if we're exhausting hops (which sets hop_limit = 0 but still needs one relay)
     if (!isToUs(p) && !isFromUs(p) && (p->hop_limit > 0 || exhaustHops)) {
         if (p->id != 0) {
@@ -192,11 +195,10 @@ bool NextHopRouter::perhapsRebroadcast(const meshtastic_MeshPacket *p)
                     }
 #endif
 
-                    if (p->next_hop == NO_NEXT_HOP_PREFERENCE) {
+                    ErrorCode res =
-                        FloodingRouter::send(tosend);
+                        (p->next_hop == NO_NEXT_HOP_PREFERENCE) ? FloodingRouter::send(tosend) : NextHopRouter::send(tosend);
-                    } else {
+                    if (res == ERRNO_SHOULD_RELEASE)
-                        NextHopRouter::send(tosend);
+                        packetPool.release(tosend);
-                    }
 
                     return true;
                 }
@@ -401,8 +403,10 @@ int32_t NextHopRouter::doRetransmissions()
                             trafficManagementModule->clearNextHop(p.packet->to);
                         }
 #endif
-                        if (auto *copy = packetPool.allocCopy(*p.packet))
+                        if (auto *copy = packetPool.allocCopy(*p.packet)) {
-                            FloodingRouter::send(copy);
+                            if (FloodingRouter::send(copy) == ERRNO_SHOULD_RELEASE)
+                                packetPool.release(copy);
+                        }
                     } else {
 #if NEXTHOP_EARLY_FLOOD_ON_UNVERIFIED
                         // M4 (gated): if the route isn't proven healthy, don't spend a second directed
@@ -416,22 +420,30 @@ int32_t NextHopRouter::doRetransmissions()
                             meshtastic_NodeInfoLite *sentTo = nodeDB->getMeshNode(p.packet->to);
                             if (sentTo)
                                 sentTo->next_hop = NO_NEXT_HOP_PREFERENCE;
-                            if (auto *copy = packetPool.allocCopy(*p.packet))
+                            if (auto *copy = packetPool.allocCopy(*p.packet)) {
-                                FloodingRouter::send(copy);
+                                if (FloodingRouter::send(copy) == ERRNO_SHOULD_RELEASE)
+                                    packetPool.release(copy);
+                            }
                         } else {
-                            if (auto *copy = packetPool.allocCopy(*p.packet))
+                            if (auto *copy = packetPool.allocCopy(*p.packet)) {
-                                NextHopRouter::send(copy);
+                                if (NextHopRouter::send(copy) == ERRNO_SHOULD_RELEASE)
+                                    packetPool.release(copy);
+                            }
                         }
 #else
-                        if (auto *copy = packetPool.allocCopy(*p.packet))
+                        if (auto *copy = packetPool.allocCopy(*p.packet)) {
-                            NextHopRouter::send(copy);
+                            if (NextHopRouter::send(copy) == ERRNO_SHOULD_RELEASE)
+                                packetPool.release(copy);
+                        }
 #endif
                     }
                 } else {
                     // Note: we call the superclass version because we don't want to have our version of send() add a new
                     // retransmission record
-                    if (auto *copy = packetPool.allocCopy(*p.packet))
+                    if (auto *copy = packetPool.allocCopy(*p.packet)) {
-                        FloodingRouter::send(copy);
+                        if (FloodingRouter::send(copy) == ERRNO_SHOULD_RELEASE)
+                            packetPool.release(copy);
+                    }
                 }
 
                 // Queue again

src/mesh/NextHopRouter.h

@@ -205,7 +205,11 @@ class NextHopRouter : public FloodingRouter
      */
     std::optional<uint8_t> getNextHop(NodeNum to, uint8_t relay_node);
 
+#ifdef PIO_UNIT_TESTING
+  public: // expose perhapsRebroadcast to the test shim
+#else
   private:
+#endif
     /** Check if we should be rebroadcasting this packet if so, do so.
      *  @return true if we did rebroadcast */
     bool perhapsRebroadcast(const meshtastic_MeshPacket *p) override;

src/modules/RoutingModule.cpp

@@ -55,7 +55,8 @@ void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketI
     // Allow the caller to set want_ack on this ACK packet if it's important that the ACK be delivered reliably
     p->want_ack = ackWantsAck;
 
-    router->sendLocal(p); // we sometimes send directly to the local node
+    if (router->sendLocal(p) == ERRNO_SHOULD_RELEASE) // we sometimes send directly to the local node
+        packetPool.release(p);
 }
 
 uint8_t RoutingModule::getHopLimitForResponse(const meshtastic_MeshPacket &mp)

src/mqtt/MQTT.cpp

@@ -122,7 +122,8 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length)
         if (isFromUs(e.packet)) {
             auto pAck = routingModule->allocAckNak(meshtastic_Routing_Error_NONE, getFrom(e.packet), e.packet->id, ch.index);
             pAck->transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MQTT;
-            router->sendLocal(pAck);
+            if (router->sendLocal(pAck) == ERRNO_SHOULD_RELEASE)
+                packetPool.release(pAck);
         } else {
             LOG_INFO("Ignore downlink message we originally sent");
         }

test/test_nexthop_routing/test_main.cpp

@@ -15,6 +15,7 @@
 #include "gps/RTC.h"
 #include "mesh/NextHopRouter.h"
 #include "mesh/NodeDB.h"
+#include "mesh/RadioInterface.h"
 #include <cstdio>
 #include <cstring>
 #include <memory>
@@ -87,6 +88,7 @@ class NextHopRouterTestShim : public NextHopRouter
     using NextHopRouter::noteRouteFailure;
     using NextHopRouter::noteRouteLearned;
     using NextHopRouter::noteRouteSuccess;
+    using NextHopRouter::perhapsRebroadcast;
     using Router::shouldDecrementHopLimit; // protected in Router
 
     void resetRouteHealthForTest()
@@ -96,6 +98,34 @@ class NextHopRouterTestShim : public NextHopRouter
     }
 };
 
+// ---------------------------------------------------------------------------
+// MockRadioInterface - mirrors RadioLibInterface::send()'s NODENUM_BROADCAST_NO_LORA branch, which
+// returns ERRNO_SHOULD_RELEASE without releasing.
+// ---------------------------------------------------------------------------
+class MockRadioInterface : public RadioInterface
+{
+  public:
+    ErrorCode send(meshtastic_MeshPacket *p) override
+    {
+        sendCount++;
+        if (declineAll || p->to == NODENUM_BROADCAST_NO_LORA)
+            return ERRNO_SHOULD_RELEASE;
+
+        packetPool.release(p);
+        return ERRNO_OK;
+    }
+
+    uint32_t getPacketTime(uint32_t totalPacketLen, bool received = false) override
+    {
+        (void)totalPacketLen;
+        (void)received;
+        return 0;
+    }
+
+    int sendCount = 0;
+    bool declineAll = false;
+};
+
 static MockNodeDB *mockNodeDB = nullptr;
 static NextHopRouterTestShim *shim = nullptr;
 
@@ -409,6 +439,65 @@ void test_hoplimit_decrement_when_resolved_not_favorite(void)
     TEST_ASSERT_TRUE(shim->shouldDecrementHopLimit(&p)); // unique but not a favorite -> decrement
 }
 
+// ===========================================================================
+// Rebroadcast of NODENUM_BROADCAST_NO_LORA
+// ===========================================================================
+
+static MockRadioInterface *installMockIface()
+{
+    MockRadioInterface *m = new MockRadioInterface();
+    shim->addInterface(std::unique_ptr<RadioInterface>(m));
+    return m;
+}
+
+// Eligible for rebroadcast: not from/to us, hops left, nonzero id, no next-hop preference.
+// Encrypted variant so Router::send() skips the encode path.
+static meshtastic_MeshPacket makeRebroadcastCandidate(NodeNum to)
+{
+    meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero;
+    p.from = 0x22222222; // not us
+    p.to = to;
+    p.id = 0x0BADF00D;
+    p.hop_start = 3;
+    p.hop_limit = 3;
+    p.next_hop = NO_NEXT_HOP_PREFERENCE;
+    p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag;
+    p.encrypted.size = 8;
+    return p;
+}
+
+// Control: proves the NO_LORA case below turns on the `to` field alone.
+void test_rebroadcast_normal_broadcast_is_relayed(void)
+{
+    MockRadioInterface *mockIface = installMockIface();
+    meshtastic_MeshPacket p = makeRebroadcastCandidate(NODENUM_BROADCAST);
+
+    TEST_ASSERT_TRUE_MESSAGE(shim->perhapsRebroadcast(&p), "ordinary broadcast must be rebroadcast");
+    TEST_ASSERT_EQUAL_MESSAGE(1, mockIface->sendCount, "exactly one packet should reach the radio");
+}
+
+void test_rebroadcast_no_lora_broadcast_is_not_relayed(void)
+{
+    MockRadioInterface *mockIface = installMockIface();
+    meshtastic_MeshPacket p = makeRebroadcastCandidate(NODENUM_BROADCAST_NO_LORA);
+
+    TEST_ASSERT_FALSE_MESSAGE(shim->perhapsRebroadcast(&p), "no-LoRa broadcast must not be rebroadcast");
+    TEST_ASSERT_EQUAL_MESSAGE(0, mockIface->sendCount, "no packet should be handed to the radio at all");
+}
+
+// Declining mock bypasses the guard so send() is reached; the release itself is only observable as
+// a sanitizer leak report, not an assertion.
+void test_rebroadcast_declined_send_releases_packet(void)
+{
+    MockRadioInterface *mockIface = installMockIface();
+    mockIface->declineAll = true;
+
+    meshtastic_MeshPacket p = makeRebroadcastCandidate(NODENUM_BROADCAST);
+
+    TEST_ASSERT_TRUE_MESSAGE(shim->perhapsRebroadcast(&p), "the rebroadcast must still be attempted");
+    TEST_ASSERT_EQUAL_MESSAGE(1, mockIface->sendCount, "the copy must have reached the mock radio");
+}
+
 // ===========================================================================
 
 void setup()
@@ -459,6 +548,11 @@ void setup()
     RUN_TEST(test_hoplimit_decrement_on_colliding_favorites);
     RUN_TEST(test_hoplimit_decrement_when_resolved_not_favorite);
 
+    printf("\n=== rebroadcast of NODENUM_BROADCAST_NO_LORA ===\n");
+    RUN_TEST(test_rebroadcast_normal_broadcast_is_relayed);
+    RUN_TEST(test_rebroadcast_no_lora_broadcast_is_not_relayed);
+    RUN_TEST(test_rebroadcast_declined_send_releases_packet);
+
     exit(UNITY_END());
 }