kevin/meshtastic_firmware
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

emdashes begone (#10847)

Browse Source
This commit is contained in:
Tom
2026-07-01 19:01:27 -05:00
committed by GitHub
co-authored by GitHub
parent dee94e0758
commit 3becaf2d95
276 changed files with 1795 additions and 1793 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/nexthop-routing-reliability.md
+46 -46
View File
@@ -1,13 +1,13 @@
# NextHop direct-message reliability on dense meshes — findings & plan
# NextHop direct-message reliability on dense meshes - findings & plan
**Status:** Implemented — mitigations and tests in `PR3-tmm-nexthop`
**Status:** Implemented - mitigations and tests in `PR3-tmm-nexthop`
**Date:** 2026-06-13
**Area:** `src/mesh` router stack (`NextHopRouter`, `ReliableRouter`, `FloodingRouter`, `Router`, `NodeDB`, `PacketHistory`)
**Constraint:** No over-the-air / wire-format changes — `next_hop` and `relay_node` stay 1 byte, no `PacketHeader` changes, no breaking protobuf changes. All new state is RAM-only.
**Constraint:** No over-the-air / wire-format changes - `next_hop` and `relay_node` stay 1 byte, no `PacketHeader` changes, no breaking protobuf changes. All new state is RAM-only.
This document captures the analysis and the proposed mitigations so the work can be
continued on this branch by anyone. It is intentionally code-grounded (file:line
references throughout) and standalone — you should not need the original investigation
references throughout) and standalone - you should not need the original investigation
context to pick it up.
---
@@ -17,16 +17,16 @@ context to pick it up.
NextHop routing for direct messages (DMs) is unreliable on dense meshes. The headline
cause is the **birthday problem**: `next_hop` and `relay_node` are each a single byte
(the last byte of a 32-bit node number), so on a mesh of N nodes the probability that
two share the same byte hits ~50% at **~19 nodes** and is near-certain by 50–100. But
two share the same byte hits ~50% at **~19 nodes** and is near-certain by 50-100. But
there are **other, equally important issues**: that single byte is trusted blindly at
five different code sites, learned routes **never decay**, routes are learned from the
**reverse (ACK) path** (asymmetric-link hazard), and collision-driven spurious
rebroadcasts **amplify congestion** exactly when the mesh is busy.
Because we can't widen the on-wire field, the fix is **interpretation-side** ("don't
trust a byte that doesn't map to a unique reachable neighbor — flood instead") plus
trust a byte that doesn't map to a unique reachable neighbor - flood instead") plus
**recovery-side** ("decay stale/failing routes so they get re-discovered"). Four
mitigations, M1–M4, all RAM-only. The net behavioral change: on dense/mobile meshes a
mitigations, M1-M4, all RAM-only. The net behavioral change: on dense/mobile meshes a
DM that today silently misroutes or black-holes instead falls back to managed flooding
(which still delivers) and re-learns a fresh route quickly. Sparse-mesh happy paths are
unchanged.
@@ -51,25 +51,25 @@ never collides with the `0`-valued sentinels `NO_NEXT_HOP_PREFERENCE` / `NO_RELA
route stored per destination, `meshtastic_NodeInfoLite::next_hop`, is also a single byte
(`src/mesh/generated/meshtastic/deviceonly.pb.h:83`).
**Sending a DM** — `NextHopRouter::send` (`src/mesh/NextHopRouter.cpp:23`):
**Sending a DM** - `NextHopRouter::send` (`src/mesh/NextHopRouter.cpp:23`):
1. `p->relay_node = getLastByteOfNodeNum(getNodeNum())` (mark ourselves as relayer).
2. `p->next_hop = getNextHop(p->to, p->relay_node)` (`src/mesh/NextHopRouter.cpp:192`):
look up `nodeDB->getMeshNode(to)->next_hop`; return it unless it equals the relayer
byte; otherwise `NO_NEXT_HOP_PREFERENCE` (→ flood).
**Relaying** — `NextHopRouter::perhapsRebroadcast` (`src/mesh/NextHopRouter.cpp:133`):
**Relaying** - `NextHopRouter::perhapsRebroadcast` (`src/mesh/NextHopRouter.cpp:133`):
rebroadcast iff `next_hop == NO_NEXT_HOP_PREFERENCE` (flood) **or**
`next_hop == getLastByteOfNodeNum(getNodeNum())` (we are the addressed next hop)
(`:147`). Each node only ever compares against **its own** byte.
**Learning** — `NextHopRouter::sniffReceived` (`src/mesh/NextHopRouter.cpp:89`): on an
**Learning** - `NextHopRouter::sniffReceived` (`src/mesh/NextHopRouter.cpp:89`): on an
ACK/reply (`request_id`/`reply_id` set), if the relayer of the ACK was also a relayer of
the original packet (validated via `PacketHistory::checkRelayers`), set
`origTx->next_hop = p->relay_node` (`:114`). I.e. the **forward** next-hop is learned
from the **reverse** path's relayer.
**Retransmission / fallback** — `NextHopRouter::doRetransmissions`
**Retransmission / fallback** - `NextHopRouter::doRetransmissions`
(`src/mesh/NextHopRouter.cpp:284`). Budgets: `NUM_RELIABLE_RETX=3` (originator: initial
- 2 retries), `NUM_INTERMEDIATE_RETX=2` (relayer: 1 retry). On the **last** retry
@@ -78,7 +78,7 @@ from the **reverse** path's relayer.
comes from `iface->getRetransmissionMsec`, whose contention window **grows with channel
utilization** (`src/mesh/RadioInterface.cpp` `getTxDelayMsec`/`getTxDelayMsecWeighted`).
**Dedup / relayer history** — `PacketHistory` (`src/mesh/PacketHistory.cpp`): a bounded
**Dedup / relayer history** - `PacketHistory` (`src/mesh/PacketHistory.cpp`): a bounded
ring (`PACKETHISTORY_MAX = max(MAX_NUM_NODES*2, 100)`, 20 B/record) keyed by
`(sender,id)`, tracking up to `NUM_RELAYERS=6` relayer **bytes** per packet in
`relayed_by[]`. `wasRelayer` (`:490`) and `checkRelayers` (`:517`) match bytes against
@@ -95,7 +95,7 @@ that array.
| 1 | Rebroadcast self-check | `NextHopRouter.cpp:147` | A remote "impostor" node sharing the intended next-hop's byte also rebroadcasts → wasted airtime / congestion. |
| 2 | Route learning | `NextHopRouter.cpp:111-114` | Stores an ambiguous byte as the route; later resolves to the wrong physical node. |
| 3 | Relayer validation | `PacketHistory.cpp:490-538` | `wasRelayer(byte)` returns true for the wrong node → mis-validated ACK / mis-learn. |
| 4 | Favorite-router hop preservation | `Router.cpp:120-145` | **First** NodeDB node whose last byte matches wins — non-deterministic; can preserve hops for the wrong relay (hop leak). |
| 4 | Favorite-router hop preservation | `Router.cpp:120-145` | **First** NodeDB node whose last byte matches wins - non-deterministic; can preserve hops for the wrong relay (hop leak). |
| 5 | Send-path lookup | `NextHopRouter.cpp:192-207` | Emits a byte that may address the wrong node; no check it still maps to a reachable neighbor. |
Collision math (uniform last byte over 255 buckets): P(collision) ≈ 50% at ~19 nodes,
@@ -106,13 +106,13 @@ Collision math (uniform last byte over 255 buckets): P(collision) ≈ 50% at ~19
The learned `next_hop` byte is cleared only on the **current DM's** last retry
(`NextHopRouter.cpp:313-321`). A route learned hours ago that has since gone dead is
still trusted on the **next** DM's first attempt — which on a congested mesh is also the
still trusted on the **next** DM's first attempt - which on a congested mesh is also the
slowest attempt. Result: silent black-hole at a dead hop until the retransmission budget
drains, then a late flood. Intermediate nodes hold stale routes indefinitely.
### 3. Reverse-path (asymmetric-link) learning
`origTx->next_hop` is learned from the ACK's relayer (`NextHopRouter.cpp:110-114`) — the
`origTx->next_hop` is learned from the ACK's relayer (`NextHopRouter.cpp:110-114`) - the
**reverse** direction. RF links are frequently asymmetric, so the best reverse relay can
be a poor forward relay. Worse, the next reverse ACK immediately re-learns the same bad
hop, so the route **flaps** back to the bad value even after a failure reset.
@@ -124,7 +124,7 @@ grows with channel utilization, so retransmit intervals **lengthen** exactly whe
mesh is busy. The 3-try reliable budget can then expire before delivery. On dense
meshes, efficiency _is_ reliability.
### Note: pubkey-derived node numbers (develop / 2.8) — does not change the plan
### Note: pubkey-derived node numbers (develop / 2.8) - does not change the plan
develop derives the node number from the public key:
`my_node_num = crc32Buffer(public_key)` (`src/mesh/NodeDB.cpp:481`), re-derived on key
@@ -140,9 +140,9 @@ plan rather than changing it:
- **Resolver gets cleaner inputs.** Stable node numbers keep a learned byte bound to one
identity (good for M3 freshness). `createNewIdentity()` retires the old entry by marking
it **ignored** and clearing its pubkey (`src/mesh/NodeDB.cpp:3123-3125`), which M1's
candidate gate already skips — so key rotation can't pollute resolution.
candidate gate already skips - so key rotation can't pollute resolution.
- **No wire-free disambiguation unlocked.** A receiver still gets only 1 byte and cannot
recover which full node number a colliding value meant — so "detect ambiguity → flood"
recover which full node number a colliding value meant - so "detect ambiguity → flood"
remains the correct strategy.
---
@@ -151,10 +151,10 @@ plan rather than changing it:
Key insight for all of M1/M2: **a 1-byte ID only needs to be unique among a node's
direct neighbors / plausible relays, not the whole mesh.** That candidate set is small
(typically 5–15), so a byte usually resolves unambiguously there; when it doesn't, fall
(typically 5-15), so a byte usually resolves unambiguously there; when it doesn't, fall
back to the _safe_ behavior (flood / decrement / don't-learn).
### M1 — Ambiguity-aware last-byte resolution (new NodeDB primitive)
### M1 - Ambiguity-aware last-byte resolution (new NodeDB primitive)
New types + methods in `src/mesh/NodeDB.h` (near line 255) / `src/mesh/NodeDB.cpp`
(near `getMeshNode`, ~2936):
@@ -181,13 +181,13 @@ bool resolveUniqueLastByte(uint8_t lastByte, bool requireDirectNeighbor, NodeNum
**and** `sinceLastSeen(node) < NEXTHOP_NEIGHBOR_FRESH_SECS`.
- `requireDirectNeighbor == false` (lenient, for learn / hop-preserve): accept if direct
neighbor **or** `nodeInfoLiteIsFavorite` **or** role ∈ {ROUTER, ROUTER_LATE, CLIENT_BASE}.
- **No tie-break.** A collision must return `Ambiguous` — picking "best SNR" would
- **No tie-break.** A collision must return `Ambiguous` - picking "best SNR" would
resurrect the silent-misroute bug. (Deliberate non-goal; document in code.)
New constant in `src/mesh/MeshTypes.h` (near line 44):
`#define NEXTHOP_NEIGHBOR_FRESH_SECS (60 * 60 * 2)` (mirrors `NUM_ONLINE_SECS`).
### M2 — Only route on bytes that resolve to a unique, reachable neighbor
### M2 - Only route on bytes that resolve to a unique, reachable neighbor
In `getNextHop` (`src/mesh/NextHopRouter.cpp:192-207`), after the existing split-horizon
check (`node->next_hop != relay_node`), require the stored byte to resolve to a **unique,
@@ -219,7 +219,7 @@ Apply M1's safe fallback at the other sites:
**Left unchanged, by design (document why in code):**
- **Site 1** rebroadcast self-check (`NextHopRouter.cpp:147`) and self-identity checks
(`ReliableRouter.cpp:127`): a node matches its **own** byte — no DB resolution helps. A
(`ReliableRouter.cpp:127`): a node matches its **own** byte - no DB resolution helps. A
remote impostor sharing the intended next-hop's byte will still rebroadcast. M1/M2
shrink the blast radius by reducing how often an ambiguous byte is ever stored or
originated; a true fix needs a wider field (out of scope). **This is the one residual
@@ -228,7 +228,7 @@ Apply M1's safe fallback at the other sites:
byte-domain (both sides are on-wire bytes); the consumer (learning) is now hardened.
Add a one-line comment; do not change.
### M3 — Route freshness / failure memory (RAM table on NextHopRouter)
### M3 - Route freshness / failure memory (RAM table on NextHopRouter)
A bounded, LRU-evicted table keyed by destination, mirroring `PacketHistory`'s
reuse-oldest discipline (not an unbounded map) to cap RAM.
@@ -253,20 +253,20 @@ Policy:
| Constant | Value | Rationale |
| ------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `ROUTE_TTL_MSEC` | 30 min | Survives a normal conversation; re-discovers a moved node within a telemetry interval. |
| `ROUTE_FAILURE_THRESHOLD` | 3 | 1–2 consecutive failures are transient LoRa collisions; 3 to the same hop = dead. Accumulates **across** DMs (independent of the per-DM 3-try budget). |
| `ROUTE_FAILURE_THRESHOLD` | 3 | 1-2 consecutive failures are transient LoRa collisions; 3 to the same hop = dead. Accumulates **across** DMs (independent of the per-DM 3-try budget). |
`isRouteStale(h, now)` = `(now - h.learnedAtMsec) >= ROUTE_TTL_MSEC || h.consecutiveFailures >= ROUTE_FAILURE_THRESHOLD`.
All age math uses **unsigned subtraction** (rollover-safe, matching
`PacketHistory.cpp:364`); treat `learnedAtMsec == 0` as "set now".
Wiring (as built — `src/mesh/NextHopRouter.cpp`, `src/mesh/ReliableRouter.cpp`):
Wiring (as built - `src/mesh/NextHopRouter.cpp`, `src/mesh/ReliableRouter.cpp`):
- `getNextHop`: if a health record matches the stored byte and `isRouteStale`, clear
`node->next_hop` (NodeDB) **and** `clearRouteHealth`, return `nullopt` (flood). No
record yet (cold path, first DM after boot) → trust NodeDB, but the M2 strict-neighbor
gate still applies.
- `sniffReceived` learn: gate the write through `resolveUniqueLastByte` (M2), then
`noteRouteLearned(p->from, p->relay_node, millis())` — resets `consecutiveFailures`
`noteRouteLearned(p->from, p->relay_node, millis())` - resets `consecutiveFailures`
**only if the hop changed** (anti-flap for asymmetric re-learn); otherwise just refreshes
`learnedAtMsec`. (No success signal is taken on the intermediate reverse-pass: an ACK
merely passing through us is not proof that _we_ delivered, and resetting failures there
@@ -287,16 +287,16 @@ the current DM (reset NodeDB `next_hop` + flood, and bump the cross-DM failure c
only place that erases a health record is the `getNextHop` decay path; the retransmission
path leaves it intact so the counter survives a reverse-path re-learn.
### M4 — Earlier flood for unverified routes (gated, off by default)
### M4 - Earlier flood for unverified routes (gated, off by default)
Compile-gated so healthy sparse meshes are untouched. **Default is off** — the define
Compile-gated so healthy sparse meshes are untouched. **Default is off** - the define
lives in `NextHopRouter.h` and must be flipped to measure:
`#define NEXTHOP_EARLY_FLOOD_ON_UNVERIFIED 1`.
In `doRetransmissions`, the directed-retry `else` branch: if the route is **not verified**
(`!findRouteHealth(to) || consecutiveFailures > 0 || isRouteStale`), reset `next_hop` and
flood on this attempt instead of spending another directed try. A **verified** route
(record present, `consecutiveFailures == 0`, within TTL — i.e. recently ACKed) takes the
(record present, `consecutiveFailures == 0`, within TTL - i.e. recently ACKed) takes the
unchanged directed-retry path, so the sparse-mesh happy path is untouched. Trade-off:
airtime ↔ latency; the gate ensures we never pay the flood cost on a proven route, only on
one we already distrust. Off by default precisely so it can be A/B-measured on the
@@ -337,13 +337,13 @@ simulator before broad enable.
`getNextHop` already early-returns for broadcast.
- **Perf:** M2 adds one O(N) resolver scan per directed send/relay (early-exit on the 2nd
match), cheaper than the crypto already on that path; site-4 is a wash. If ever hot, a
future 256-entry last-byte index is the optimization (not now — RAM).
future 256-entry last-byte index is the optimization (not now - RAM).
---
## Verification (all tiers)
### 1. Native unit tests — new `test/test_nexthop_routing/test_main.cpp`
### 1. Native unit tests - new `test/test_nexthop_routing/test_main.cpp`
`pio test -e native -f test_nexthop_routing`; on macOS `./bin/test-native-docker.sh -f test_nexthop_routing`.
Design the RouteHealth helpers to take `now` as a parameter so the 30-min TTL logic is
@@ -366,7 +366,7 @@ testable without a clock mock.
### 2. portduino SimRadio simulator
`pio run -e native && ./bin/test-simulator.sh`. Best vehicle for the **intermediate-node**
path the 2-device bench can't reach. Line topology A — B — C: establish A→C (B learns a
path the 2-device bench can't reach. Line topology A - B - C: establish A→C (B learns a
directed route), stop B relaying that dest, confirm A re-discovers via flood within
`ROUTE_FAILURE_THRESHOLD` and that B's `noteRouteFailure`/`clearRouteHealth` fires (visible
via the `LOG_INFO "Route to … stale"` / "Resetting next hop" lines). Use this to A/B M4
@@ -374,15 +374,15 @@ via the `LOG_INFO "Route to … stale"` / "Resetting next hop" lines). Use this
### 3. Hardware via meshtastic MCP (auto-detect; 3+ devices for a real hop)
- `mcp-server/tests/mesh/test_nexthop_multihop_recovery.py` — **the multi-hop validator
for this work** (added on this branch). Self-discovers an A — relay — C line, asserts a
- `mcp-server/tests/mesh/test_nexthop_multihop_recovery.py` - **the multi-hop validator
for this work** (added on this branch). Self-discovers an A - relay - C line, asserts a
directed DM is delivered across the relay (next_hop + M1/M2/M3 engaged), and asserts
delivery recovers after the relay is power-cycled (M3). Skips unless the bench is a true
multi-hop line (≥3 roles via `--hub-profile`, endpoints out of direct RF range).
- `mcp-server/tests/mesh/test_direct_with_ack.py` — happy-path regression: a fresh/unique
- `mcp-server/tests/mesh/test_direct_with_ack.py` - happy-path regression: a fresh/unique
route still delivers a want_ack DM on the first/second try (M4's gate must keep this
green).
- `mcp-server/tests/mesh/test_peer_offline_recovery.py` — 2-device recovery validator: peer
- `mcp-server/tests/mesh/test_peer_offline_recovery.py` - 2-device recovery validator: peer
off mid-conversation then back. Must stay green and ideally recover in fewer attempts.
### 4. Build / format sanity
@@ -406,13 +406,13 @@ production; sanity-check RAM headroom on the smallest nRF52 build for the ~384 B
**Pending (environment-blocked, not yet run):**
- **Multi-hop A–B–C recovery sim** — the `simulator/` broker hub is **not git-tracked**
- **Multi-hop A-B-C recovery sim** - the `simulator/` broker hub is **not git-tracked**
(only stale local `.pyc`), and two `meshtasticd -s` instances can't hear each other
without it. The intermediate-node failure-count path and the M4 A/B therefore have unit
coverage of their logic but no end-to-end multi-node run yet.
- **Hardware / multi-hop tier** — a committable bench test now exists:
- **Hardware / multi-hop tier** - a committable bench test now exists:
`mcp-server/tests/mesh/test_nexthop_multihop_recovery.py`. It self-discovers a real
multi-hop pair (A — relay — C), asserts a directed DM is delivered across the relay, and
multi-hop pair (A - relay - C), asserts a directed DM is delivered across the relay, and
asserts delivery recovers after the relay is power-cycled (the M3 path). It
`pytest.skip`s cleanly unless the bench is a true line with endpoints out of direct RF
range (≥3 roles via `--hub-profile`), so it's safe to commit and only asserts when the
@@ -424,9 +424,9 @@ production; sanity-check RAM headroom on the smallest nRF52 build for the ~384 B
## Risks & limitations
- **Site-1 impostor rebroadcast** is unfixable without a wider field — documented; M1/M2
- **Site-1 impostor rebroadcast** is unfixable without a wider field - documented; M1/M2
only shrink its frequency.
- **Dense meshes flood DMs more often** — intended (a flooded DM arrives; a mis-unicast one
- **Dense meshes flood DMs more often** - intended (a flooded DM arrives; a mis-unicast one
black-holes). Call out in the PR so reviewers expect a slightly higher DM flood rate on
very dense meshes.
- **M4 airtime** if the gate is too loose → default conservative + compile-gated +
@@ -441,14 +441,14 @@ production; sanity-check RAM headroom on the smallest nRF52 build for the ~384 B
Each step is independently testable; land them as separate commits.
1. **M1 resolver + unit tests** — `NodeDB` only; no behavior change until wired. Lands the
1. **M1 resolver + unit tests** - `NodeDB` only; no behavior change until wired. Lands the
`resolveLastByte`/`resolveUniqueLastByte` primitive and its full unit-test matrix.
2. **M2 + wiring + tests** — `getNextHop` strict gate, learning gate, favorite-router
2. **M2 + wiring + tests** - `getNextHop` strict gate, learning gate, favorite-router
preservation rewrite. Adds the `getNextHop` and site-4 tests.
3. **M3 health table + decay + tests** — RAM `RouteHealth` table, decay-on-read, failure/
3. **M3 health table + decay + tests** - RAM `RouteHealth` table, decay-on-read, failure/
success accounting, reconciliation with the existing last-retry reset. Adds the
route-health unit tests and the simulator recovery check.
4. **M4 gated tuning** — early-flood-on-unverified behind the compile flag; simulator A/B
4. **M4 gated tuning** - early-flood-on-unverified behind the compile flag; simulator A/B
and hardware regression.
Reference plan (with the same content) was developed at