Traceroute response endpoints are reversed when building graph edges #66

Closed
opened 2026-10-05 06:04:37 +02:00 by adam · 6 comments
Owner

Meshview builds graph edges from completed Meshtastic traceroute responses using the packet endpoints in the wrong order.

/api/edges currently constructs the path as:

path = [tr.packet.from_node_id] + list(route.route)
path.append(tr.packet.to_node_id if tr.done else tr.gateway_node_id)

For a completed traceroute (tr.done == True), tr.packet is the response packet, whose from/to are the reverse of the forward path direction. The two endpoint-adjacent edges are therefore wrong for every completed traceroute containing at least one intermediate hop.

Affected code:

  • api.py — /api/edges, path construction (~L539–540)
  • api.py — /api/traceroute/{packet_id}, winning_forward_with_endpoints / winning_reverse_with_endpoints (~L895–913)
  • web.py — graph_traceroute, path construction (~L403–407)

Example

Actual forward path:

Alpha → Bravo → Charlie → Delta

Alpha initiated the traceroute, Delta is the target. The completed response observed over MQTT is:

packet.from = Delta
packet.to   = Alpha
route       = [Bravo, Charlie]
done        = True

Meshview builds:

Delta → Bravo → Charlie → Alpha

producing edges:

Delta   — Bravo      false
Bravo   — Charlie    correct
Charlie — Alpha      false

Expected:

Alpha   — Bravo
Bravo   — Charlie
Charlie — Delta

Why the current ordering is wrong

1. done marks response packets. In mqtt_store.py the row is created with done=not env.packet.decoded.want_response. Traceroute requests set want_response = true; the reply allocated by TraceRouteModule::allocReply() does not. So done == True means the payload came from a response packet, where from = original target and to = original initiator.

2. tr.packet is the response packet, not the request. The row stores packet_id = env.packet.id, and there is no request_id handling anywhere in the codebase. In firmware, setReplyTo() only sets p->decoded.request_id = to.id; the reply's own id comes from allocForSending() → generatePacketId(), a new distinct id. So the Traceroute row for a response joins to the response packet.

3. route[] stays in forward order inside the response. In TraceRouteModule::alterReceivedProtobuf(), relays call:

insertUnknownHops(p, r, !incoming.request_id);
appendMyIDandSNR(r, p.rx_snr, !incoming.request_id, isToUs(&p));

For a response (request_id != 0) the isTowardsDestination flag is false, so hops are appended to route_back. route[] is left as the forward path in forward order.

4. Firmware reconstructs it the opposite way from Meshview. Same function:

if (!incoming.request_id)
    printRoute(r, p.from, p.to, true);   // request
else
    printRoute(r, p.to,   p.from, false); // response

printRoute(r, origin, dest, ...) renders origin --> route[0] --> ... --> dest. For a response that is to → route[] → from.

Reproduction

from meshtastic.protobuf.mesh_pb2 import RouteDiscovery
from meshtastic.protobuf.portnums_pb2 import PortNum
from meshview import decode_payload

ALPHA, BRAVO, CHARLIE, DELTA = 0xA, 0xB, 0xC, 0xD

rd = RouteDiscovery()
rd.route.extend([BRAVO, CHARLIE])
rd.route_back.extend([CHARLIE, BRAVO])

class P:  from_node_id, to_node_id = DELTA, ALPHA   # completed response
class TR: packet, done, gateway_node_id, route = P, True, BRAVO, rd.SerializeToString()

route = decode_payload.decode_payload(PortNum.TRACEROUTE_APP, TR.route)
path = [TR.packet.from_node_id] + list(route.route)
path.append(TR.packet.to_node_id if TR.done else TR.gateway_node_id)
print([hex(n) for n in path])

Output:

current path : 0xd 0xb 0xc 0xa   edges: (D,B) (B,C) (C,A)
expected     : 0xa 0xb 0xc 0xd   edges: (A,B) (B,C) (C,D)

Observable effect

Because the false edges always attach to the two endpoints, a well-connected initiator or gateway accumulates false direct links to every remote target it ever traced.

Given real paths:

Alpha → Bravo → X1 → Remote1
Alpha → Bravo → X2 → Remote2
Alpha → Bravo → X3 → Remote3

Meshview infers Remote1 — Bravo, Remote2 — Bravo, Remote3 — Bravo, creating a false star topology around Bravo, while the correct Alpha — Bravo and Xn — Remoten edges are missing.

Suggested fix

For completed responses:

path = [tr.packet.to_node_id, *route.route, tr.packet.from_node_id]

The same orientation correction is needed in graph_traceroute and in /api/traceroute/{packet_id}'s winning-path assembly (where the reverse path needs the mirror-image correction).

Related issues found while investigating

These are separate and could be split out:

  1. NODENUM_BROADCAST placeholders are not filtered. Firmware's insertUnknownHops() injects 0xFFFFFFFF entries into route[] for unknown hops. /api/edges treats these as real node IDs, so they become a spurious pseudo-node in the graph.

  2. route_back is never used for edges. Only route.route feeds the graph, so asymmetric return paths contribute nothing. This may be intentional.

  3. Possible self-loop in the incomplete-traceroute branch. graph_traceroute guards the gateway append with if path[-1] != tr.gateway_node_id (some nodes add themselves to the route before uplinking). /api/edges has no such guard, so it can emit a (gw, gw) self-edge.

Environment

  • Meshview 3.0.0
  • Verified against meshtastic/firmware master (src/modules/TraceRouteModule.cpp, src/mesh/MeshModule.cpp)
Meshview builds graph edges from completed Meshtastic traceroute responses using the packet endpoints in the wrong order. `/api/edges` currently constructs the path as: ```python path = [tr.packet.from_node_id] + list(route.route) path.append(tr.packet.to_node_id if tr.done else tr.gateway_node_id) ``` For a **completed** traceroute (`tr.done == True`), tr.packet is the *response* packet, whose `from`/`to` are the reverse of the forward path direction. The two endpoint-adjacent edges are therefore wrong for every completed traceroute containing at least one intermediate hop. Affected code: - api.py — `/api/edges`, path construction (~L539–540) - api.py — `/api/traceroute/{packet_id}`, winning_forward_with_endpoints / winning_reverse_with_endpoints (~L895–913) - web.py — graph_traceroute, path construction (~L403–407) ## Example Actual forward path: ```text Alpha → Bravo → Charlie → Delta ``` Alpha initiated the traceroute, Delta is the target. The completed response observed over MQTT is: ```text packet.from = Delta packet.to = Alpha route = [Bravo, Charlie] done = True ``` Meshview builds: ```text Delta → Bravo → Charlie → Alpha ``` producing edges: ```text Delta — Bravo false Bravo — Charlie correct Charlie — Alpha false ``` Expected: ```text Alpha — Bravo Bravo — Charlie Charlie — Delta ``` ## Why the current ordering is wrong **1. done marks response packets.** In mqtt_store.py the row is created with `done=not env.packet.decoded.want_response`. Traceroute requests set `want_response = true`; the reply allocated by `TraceRouteModule::allocReply()` does not. So `done == True` means the payload came from a response packet, where `from` = original target and to = original initiator. **2. tr.packet is the response packet, not the request.** The row stores `packet_id = env.packet.id`, and there is no `request_id` handling anywhere in the codebase. In firmware, `setReplyTo()` only sets `p->decoded.request_id = to.id`; the reply's own id comes from `allocForSending()` → `generatePacketId()`, a new distinct id. So the Traceroute row for a response joins to the *response* packet. **3. `route[]` stays in forward order inside the response.** In `TraceRouteModule::alterReceivedProtobuf()`, relays call: ```cpp insertUnknownHops(p, r, !incoming.request_id); appendMyIDandSNR(r, p.rx_snr, !incoming.request_id, isToUs(&p)); ``` For a response (`request_id != 0`) the `isTowardsDestination` flag is false, so hops are appended to route_back. `route[]` is left as the forward path in forward order. **4. Firmware reconstructs it the opposite way from Meshview.** Same function: ```cpp if (!incoming.request_id) printRoute(r, p.from, p.to, true); // request else printRoute(r, p.to, p.from, false); // response ``` `printRoute(r, origin, dest, ...)` renders `origin --> route[0] --> ... --> dest`. For a response that is `to → route[] → from`. ## Reproduction ```python from meshtastic.protobuf.mesh_pb2 import RouteDiscovery from meshtastic.protobuf.portnums_pb2 import PortNum from meshview import decode_payload ALPHA, BRAVO, CHARLIE, DELTA = 0xA, 0xB, 0xC, 0xD rd = RouteDiscovery() rd.route.extend([BRAVO, CHARLIE]) rd.route_back.extend([CHARLIE, BRAVO]) class P: from_node_id, to_node_id = DELTA, ALPHA # completed response class TR: packet, done, gateway_node_id, route = P, True, BRAVO, rd.SerializeToString() route = decode_payload.decode_payload(PortNum.TRACEROUTE_APP, TR.route) path = [TR.packet.from_node_id] + list(route.route) path.append(TR.packet.to_node_id if TR.done else TR.gateway_node_id) print([hex(n) for n in path]) ``` Output: ```text current path : 0xd 0xb 0xc 0xa edges: (D,B) (B,C) (C,A) expected : 0xa 0xb 0xc 0xd edges: (A,B) (B,C) (C,D) ``` ## Observable effect Because the false edges always attach to the two endpoints, a well-connected initiator or gateway accumulates false direct links to every remote target it ever traced. Given real paths: ```text Alpha → Bravo → X1 → Remote1 Alpha → Bravo → X2 → Remote2 Alpha → Bravo → X3 → Remote3 ``` Meshview infers `Remote1 — Bravo`, `Remote2 — Bravo`, `Remote3 — Bravo`, creating a false star topology around `Bravo`, while the correct `Alpha — Bravo` and `Xn — Remoten` edges are missing. ## Suggested fix For completed responses: ```python path = [tr.packet.to_node_id, *route.route, tr.packet.from_node_id] ``` The same orientation correction is needed in graph_traceroute and in `/api/traceroute/{packet_id}`'s winning-path assembly (where the reverse path needs the mirror-image correction). ## Related issues found while investigating These are separate and could be split out: 1. **`NODENUM_BROADCAST` placeholders are not filtered.** Firmware's `insertUnknownHops()` injects `0xFFFFFFFF` entries into `route[]` for unknown hops. `/api/edges` treats these as real node IDs, so they become a spurious pseudo-node in the graph. 2. **route_back is never used for edges.** Only route.route feeds the graph, so asymmetric return paths contribute nothing. This may be intentional. 3. **Possible self-loop in the incomplete-traceroute branch.** graph_traceroute guards the gateway append with `if path[-1] != tr.gateway_node_id` (some nodes add themselves to the route before uplinking). `/api/edges` has no such guard, so it can emit a `(gw, gw)` self-edge. ## Environment - Meshview 3.0.0 - Verified against `meshtastic/firmware` `master` (`src/modules/TraceRouteModule.cpp`, `src/mesh/MeshModule.cpp`) <!-- github-sync-id:5349026649 -->
adam closed this issue 2026-10-05 06:04:38 +02:00
Author
Owner

@pablorevilla-meshtastic I've opened three issues covering bugs we've run into while running Meshview against our mesh. I'm happy to work on fixes for all of them.

Before I start, what's the preferred contribution flow? Specifically, should PRs target master, or the current release branch (3.0.8)? I noticed 3.0.7 was merged in from its own branch, so I want to make sure I'm branching from the right place

@pablorevilla-meshtastic I've opened three issues covering bugs we've run into while running Meshview against our mesh. I'm happy to work on fixes for all of them. Before I start, what's the preferred contribution flow? Specifically, should PRs target `master`, or the current release branch (`3.0.8`)? I noticed 3.0.7 was merged in from its own branch, so I want to make sure I'm branching from the right place <!-- github-comment-id:5544864455 -->
Author
Owner

I would love to get help, I am making changes to branch 3.0.8 right now... I would do your PRs from there.
Would love to see your fixes I have not been able to work on it for the last couple of months. (summer vacation)

I would love to get help, I am making changes to branch 3.0.8 right now... I would do your PRs from there. Would love to see your fixes I have not been able to work on it for the last couple of months. (summer vacation) <!-- github-comment-id:5545937080 -->
Author
Owner

I can take care of that over the next couple of days. In the meantime, feel free to leave any feedback on the issues!

BTW, I’d suggest doing the main development work in develop and tagging each release from there. It feels more natural to keep the latest code in develop and cut release branches/tags as needed

I can take care of that over the next couple of days. In the meantime, feel free to leave any feedback on the issues! BTW, I’d suggest doing the main development work in `develop` and tagging each release from there. It feels more natural to keep the latest code in `develop` and cut release branches/tags as needed <!-- github-comment-id:5545995881 -->
Author
Owner

Sounds good. I like that. I am going to be honest here. I picked up meshview because armoo did not update it anymore... I have learned a lot and it has been a great project, but there are gaps in my knowledge of github, so I am more than happy to take recomendations and ideas.

Sounds good. I like that. I am going to be honest here. I picked up meshview because armoo did not update it anymore... I have learned a lot and it has been a great project, but there are gaps in my knowledge of github, so I am more than happy to take recomendations and ideas. <!-- github-comment-id:5546095236 -->
Author
Owner

I have a few changes in 3.0.8, when you get your PR done we can then release it. I was thinking of releasing 3.0.8 in the next couple of weeks

I have a few changes in 3.0.8, when you get your PR done we can then release it. I was thinking of releasing 3.0.8 in the next couple of weeks <!-- github-comment-id:5546253792 -->
Author
Owner

fix added to version 3.0.8, thank you!

fix added to version 3.0.8, thank you! <!-- github-comment-id:5606743827 -->
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: adam/meshview#66