Deprecate/block packets with a missing/invalid hop_start value (pre-hop firmware) (related to issue #7369)

[DivineOmega]

This pull request provides options to deal with packets received by nodes with pre-hop firmware (or more specifically packets with a missing/invalid hop_start value). These packets cause issue with node list reporting ? hops, nodes showing as if they are direct connections, traceroute showing missing information, and other frustrations. Anecdotally, nodes with firmware this old tend to also be unmaintained and/or configured poorly (such as with an inappropriate role).

From discussions in my local mesh and on GitHub (#7369) there appears to be demand for this.

---

This PR contains 3 policies for how to deal with these packets, as defined in src/mesh/NodeDB.h. These are as follows.

• ALLOW - The existing/previous behaviour.
• PREVENT_FORWARDING - Packets are identified, but not forwarded. Packets from these nodes are also not considered when determining a implicit acknowledgement (e.g. for channel messages), and thus are ignored for the purposes of auto-re-transmission/retry logic. This policy effectively implements the changes discussed in [Feature Request]: Deprecate packets with missing/invalid HopStart #7369. This does mean that these nodes will still appear in the node list for close nodes, and still show with ? hops.
• DROP_PACKET - Packets are identified and then immediately dropped before being processed. This means that no packets from these nodes are relayed, and they do not appear in node lists, even for nodes that could otherwise be directly connected. This is the current setting in this PR and the one I would prefer to see implemented.

---

How we identify packets as coming from pre-hop firmware nodes:

Classification lives in classifyHopStart() and mirrors getHopsAway():

• INVALID if hop_start < hop_limit
• MISSING_OR_UNKNOWN if hop_start == 0 and the packet is not decoded with a bitfield
• VALID otherwise

Bitfield detection only works for decoded packets:

  if (p.hop_start == 0) {
      if (p.which_payload_variant == meshtastic_MeshPacket_decoded_tag && p.decoded.has_bitfield)
          return HopStartStatus::VALID;
      return HopStartStatus::MISSING_OR_UNKNOWN;
  }

So “pre‑hop” is inferred per packet when:

• hop_start == 0, and
• we can’t verify the decoded bitfield (encrypted or no bitfield).

---

Testing

I happen to live locally to a prominently position pre-hop firmware node, so have been able to test this on several devices - Heltec V3, V4, and WisMesh Tag. The functionality seems to work currently in all modes with no noticeable regressions outside of the scope of the intended behaviour.

The easiest method to test the DROP_PACKET behaviour is to be near an old pre-hop firmware node, setup a node with the firmware in this PR, reset the node DB if not already done, and then ensure the pre-hop firmware node does not appear in the new firmware node's node list.

I did have to add additional checks for whether packets come from locally connected devices (see new calls to isFromUs), as my original implementation (when in DROP_PACKET) mode prevented config pages from loading while using the mobile app. I didn't initial realise that app to node packets went through the same router. This was changed and re-tested and seems to work without issue now.

---

Functionality everything seems to work as intended, but I'm new to contributing to the Meshtastic firmware (only contributed to the Android app before meshtastic/Meshtastic-Android#4002) and have only been working on microcontroller programming for a few months, so it is quite possible I've make some obvious mistake/oversight here, either code-wise or my understanding of the mesh networking.

🤝 Attestations

• I have tested that my proposed changes behave as described.
• I have tested that my proposed changes do not cause any obvious regressions on the following devices:
  • Heltec V3
  • LilyGo T-Deck
  • LilyGo T-Beam
  • RAK WisBlock 4631
  • Seeed Studio T-1000E tracker card
  • Other (please specify below)
    • Heltec V4
    • WisMesh tag

[github-actions]

@DivineOmega, Welcome to Meshtastic!

Thanks for opening your first pull request. We really appreciate it.

We discuss work as a team in discord, please join us in the #firmware channel.
There's a big backlog of patches at the moment. If you have time,
please help us with some code review and testing of other PRs!

Welcome to the team 😄

[thebentern]

@DivineOmega love the idea, but what do we think about augmenting the existing rebroadcast mode protobuf. Since this is a temporary patch and the main goal for the mesh is really stopping the propogation of these packets, I would prefer to just add options to the existing enum.

[DivineOmega]

@DivineOmega love the idea, but what do we think about augmenting the existing rebroadcast mode protobuf. Since this is a temporary patch and the main goal for the mesh is really stopping the propogation of these packets, I would prefer to just add options to the existing enum.

Thanks for taking a look.

I could add additional rebroadcast modes for these. To be clear, are you saying the new PREVENT_FORWARDING and DROP_PACKET policies should be additional rebroadcast modes? Something like REBROADCAST_ALL_PREHOP_PREVENT_FORWARDING and REBROADCAST_ALL_PREHOP_DROP_PACKET.

If so, this would not block any of these packets by default, and would only affect users that specifically changed from the default REBROADCAST ALL. Or am I misunderstanding?

---

The current setting and behaviour of this MR is to drop these packets so I think it should be stopping their propagation.

[NomDeTom]

I would prefer to just add options to the existing enum.

Here: https://github.com/meshtastic/protobufs/blob/bc63a57f9e5dba8a7c90ee0bd4a9840862d61f6d/meshtastic/config.proto#L125

Like a new option called to Modern_Only and making the all into a legacy_support mode? I think all of the other options should remove support at some point, but I can see a soft switchover being preferable.

One alpha release or so should be long enough ;)

Edit: or should that be broken up, so it becomes a series of flags for filtering, e.g. known PSK channel, known nodes, bridged traffic, etc.

[DivineOmega]

Okay. I've added a new MODERN_ONLY broadcast mode (6) which does the same as the DROP_PACKET policy mentioned previously. I have removed the policy enum, and reworked the logic to rely on the new broadcast mode instead.

Before the protobufs are updated I'm just using meshtastic --port /dev/ttyACM1 --set device.rebroadcast_mode 6 to set the config on the nodes.

[thebentern]

If the goal is to curb poor behaviors from old firmwares, there may be some merit to doing this implicitly on CORE_PORTNUMS_ONLY with the next big rev like 2.8.

[DivineOmega]

I've managed to simplify the implementation significantly due to the switch to drop packet only behaviour using a new broadcast mode.

[DivineOmega]

Actually thinking about this more, making this locked behind a new broadcast mode causes some problems. For example, my main home node is currently set to LOCAL_ONLY broadcast mode. With how I've now changed this, I wouldn't be able to get the advantages of this new functionality and also keep LOCAL_ONLY broadcasting.

Would it be easier to just make this the new default behaviour across all broadcast modes? Perhaps at a specific (alpha) firmware version, or perhaps even a specific future date time?

[DivineOmega]

How about we do this as a build flag (rather than broadcast mode), so we can easily decide if/when to enable it on a firmware level? It would also allow technical users to compile their own version of the firmware with this enabled, before it is officially enabled.

Something like: -DMESHTASTIC_PREHOP_DROP=1

[DivineOmega]

Okay, I've redone this to use a build flag instead. People can compile with -DMESHTASTIC_PREHOP_DROP=1 to enable this new behaviour, but by default, for now, it's the original behaviour.

Need to do some local testing of this but assuming it works, we should be good, if everyone is happy with this approach.

[DivineOmega]

WisMesh Tag testing complete. Appears to work fine.

For anyone else wishing to test this, the PlatformIO command to build and upload the firmware is as follows. Change rak_wismeshtag to your desired hardware.

PLATFORMIO_BUILD_FLAGS="-DMESHTASTIC_PREHOP_DROP=1" pio run -e rak_wismeshtag -t upload

[DivineOmega]

Tested on Heltec v4.

[DivineOmega]

I'm happy this works on all devices I've tested, Heltec v3, v4, and WisMesh Tag.

Do you want me to make any further changes or do any additional device testing?

If not, it would be great to get this merged as is. We could then maybe do another PR at a later point to enable this build flag and behaviour as the default.