mirror of
https://github.com/pablorevilla-meshtastic/meshview.git
synced 2026-10-31 03:01:58 +01:00
fix: correct traceroute path orientation when building graph edges
A completed traceroute is observed as a response packet, whose from/to are reversed with respect to the path being traced: `to` is the node that started the traceroute and `from` is the node that answered. Meshview assembled paths as [from, *route, to], so both endpoint-adjacent edges were wrong for every completed traceroute with at least one intermediate hop. Because the false edges always attach to the endpoints, a busy initiator or gateway accumulated false direct links to every remote target it ever traced. Confirmed against TraceRouteModule::alterReceivedProtobuf, which renders a response as printRoute(r, p.to, p.from, false), against the meshtastic python CLI, which starts the forward path at p["to"] and the return path at p["from"], and against real recorded traceroutes where a request's route[] is a prefix of the matching response's route[]. Add meshview/traceroute.py and use it from /api/edges, /api/traceroute and graph_traceroute, which had three separate copies of this logic that had all drifted. /api/traceroute also gains initiator/target so the traceroute page stops re-deriving the endpoints itself, which is where a fourth copy lived. While in /api/edges, skip the NODENUM_BROADCAST placeholders that insertUnknownHops() writes into route[] (they became a pseudo-node hub in the graph) and guard against the (gateway, gateway) self edge that graph_traceroute already guarded against.
This commit is contained in:
1 parent
992296b0c1
commit
becf4a8a42
5 files changed
+97
-37
No files matched your search
@@ -203,6 +203,10 @@ Returns network edges (connections between nodes) based on traceroutes and neigh
|
|||||||
Traceroute edges are collected over the last 12 hours. Neighbor edges are based on
|
Traceroute edges are collected over the last 12 hours. Neighbor edges are based on
|
||||||
port 71 packets.
|
port 71 packets.
|
||||||
|
|
||||||
|
A completed traceroute is observed as a response packet, whose `from`/`to` are reversed
|
||||||
|
with respect to the traced path, so edges are built as `to -> route[] -> from`. Unknown
|
||||||
|
hop placeholders (`0xFFFFFFFF`) are skipped.
|
||||||
|
|
||||||
Query Parameters
|
Query Parameters
|
||||||
- `type` (optional, string): `traceroute` or `neighbor`. If omitted, returns both.
|
- `type` (optional, string): `traceroute` or `neighbor`. If omitted, returns both.
|
||||||
- `node_id` (optional, int): Filter edges to only those touching a node.
|
- `node_id` (optional, int): Filter edges to only those touching a node.
|
||||||
@@ -324,13 +328,21 @@ Returns traceroute details and derived paths for a packet.
|
|||||||
Path Parameters
|
Path Parameters
|
||||||
- `packet_id` (required, int): Packet ID.
|
- `packet_id` (required, int): Packet ID.
|
||||||
|
|
||||||
|
A completed traceroute is observed as a response packet, whose `from`/`to` are reversed
|
||||||
|
with respect to the traced path. `initiator` and `target` resolve this: the forward path
|
||||||
|
runs `initiator -> forward_hops -> target` and the return path runs
|
||||||
|
`target -> reverse_hops -> initiator`, in both directions of travel. `forward_hops` and
|
||||||
|
`reverse_hops` contain only the intermediate hops, never the endpoints.
|
||||||
|
|
||||||
Response Example
|
Response Example
|
||||||
```json
|
```json
|
||||||
{
|
{
|
||||||
"packet": {
|
"packet": {
|
||||||
"id": 123,
|
"id": 123,
|
||||||
"from": 111,
|
"from": 222,
|
||||||
"to": 222,
|
"to": 111,
|
||||||
|
"initiator": 111,
|
||||||
|
"target": 222,
|
||||||
"channel": "main"
|
"channel": "main"
|
||||||
},
|
},
|
||||||
"traceroute_packets": [
|
"traceroute_packets": [
|
||||||
@@ -338,19 +350,20 @@ Response Example
|
|||||||
"index": 0,
|
"index": 0,
|
||||||
"gateway_node_id": 333,
|
"gateway_node_id": 333,
|
||||||
"done": true,
|
"done": true,
|
||||||
"forward_hops": [111, 444, 222],
|
"forward_hops": [444, 555],
|
||||||
"reverse_hops": [222, 444, 111]
|
"reverse_hops": [555, 444]
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"unique_forward_paths": [
|
"unique_forward_paths": [
|
||||||
{ "path": [111, 444, 222], "count": 2 }
|
{ "path": [444, 555], "count": 2 }
|
||||||
],
|
],
|
||||||
"unique_reverse_paths": [
|
"unique_reverse_paths": [
|
||||||
[222, 444, 111]
|
[555, 444]
|
||||||
],
|
],
|
||||||
"winning_paths": [
|
"winning_paths": {
|
||||||
[111, 444, 222]
|
"forward": [[111, 444, 555, 222]],
|
||||||
]
|
"reverse": [[222, 555, 444, 111]]
|
||||||
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|||||||
@@ -129,7 +129,7 @@ function uniqueForwardPaths(data, originId, targetId) {
|
|||||||
|
|
||||||
function uniqueReversePaths(data, originId, targetId) {
|
function uniqueReversePaths(data, originId, targetId) {
|
||||||
return (data?.unique_reverse_paths || [])
|
return (data?.unique_reverse_paths || [])
|
||||||
.map(path => addMissingEndpoints(path || [], targetId, null))
|
.map(path => addMissingEndpoints(path || [], targetId, originId))
|
||||||
.filter(path => path.length > 1);
|
.filter(path => path.length > 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -157,8 +157,9 @@ async function loadTraceroute() {
|
|||||||
const nodes = new Map();
|
const nodes = new Map();
|
||||||
const edges = [];
|
const edges = [];
|
||||||
|
|
||||||
const originId = data?.packet?.from != null ? String(data.packet.from) : null;
|
// A response packet carries from/to reversed, so use the endpoints the API resolved.
|
||||||
const targetId = data?.packet?.to != null ? String(data.packet.to) : null;
|
const originId = data?.packet?.initiator != null ? String(data.packet.initiator) : null;
|
||||||
|
const targetId = data?.packet?.target != null ? String(data.packet.target) : null;
|
||||||
const forwardPaths = (data?.winning_paths?.forward || []).map(path => path.map(id => String(id)));
|
const forwardPaths = (data?.winning_paths?.forward || []).map(path => path.map(id => String(id)));
|
||||||
const reversePaths = (data?.winning_paths?.reverse || []).map(path => path.map(id => String(id)));
|
const reversePaths = (data?.winning_paths?.reverse || []).map(path => path.map(id => String(id)));
|
||||||
const winningPathKeys = new Set([
|
const winningPathKeys = new Set([
|
||||||
|
|||||||
@@ -0,0 +1,38 @@
|
|||||||
|
"""Assemble node paths from Meshtastic RouteDiscovery payloads.
|
||||||
|
|
||||||
|
A traceroute response carries the endpoints reversed with respect to the path
|
||||||
|
being traced: `to` is the node that started the traceroute and `from` is the node
|
||||||
|
that answered. Only the response direction is affected, so both orientations are
|
||||||
|
handled here rather than at each call site.
|
||||||
|
"""
|
||||||
|
|
||||||
|
# Firmware inserts this into route[] as a placeholder for a hop it could not identify.
|
||||||
|
NODENUM_BROADCAST = 0xFFFFFFFF
|
||||||
|
|
||||||
|
|
||||||
|
def strip_unknown_hops(hops):
|
||||||
|
return [hop for hop in hops if hop != NODENUM_BROADCAST]
|
||||||
|
|
||||||
|
|
||||||
|
def forward_path(from_node_id, to_node_id, hops, done, gateway_node_id=None):
|
||||||
|
"""Path towards the traced node, starting at the node that began the traceroute."""
|
||||||
|
if done:
|
||||||
|
return [node for node in (to_node_id, *hops, from_node_id) if node is not None]
|
||||||
|
|
||||||
|
path = [node for node in (from_node_id, *hops) if node is not None]
|
||||||
|
# Some nodes add themselves to the route before uplinking.
|
||||||
|
if gateway_node_id is not None and (not path or path[-1] != gateway_node_id):
|
||||||
|
path.append(gateway_node_id)
|
||||||
|
return path
|
||||||
|
|
||||||
|
|
||||||
|
def return_path(from_node_id, to_node_id, hops):
|
||||||
|
"""Path back from the traced node, starting at the node that answered."""
|
||||||
|
return [node for node in (from_node_id, *hops, to_node_id) if node is not None]
|
||||||
|
|
||||||
|
|
||||||
|
def endpoints(from_node_id, to_node_id, done):
|
||||||
|
"""The (initiator, target) of the traceroute, regardless of packet direction."""
|
||||||
|
if done:
|
||||||
|
return to_node_id, from_node_id
|
||||||
|
return from_node_id, to_node_id
|
||||||
+11
-8
@@ -17,7 +17,7 @@ from jinja2 import Environment, PackageLoader, Undefined, select_autoescape
|
|||||||
from markupsafe import Markup
|
from markupsafe import Markup
|
||||||
|
|
||||||
from meshtastic.protobuf.portnums_pb2 import PortNum
|
from meshtastic.protobuf.portnums_pb2 import PortNum
|
||||||
from meshview import config, database, decode_payload, migrations, models, store
|
from meshview import config, database, decode_payload, migrations, models, store, traceroute
|
||||||
from meshview.__version__ import (
|
from meshview.__version__ import (
|
||||||
__version_string__,
|
__version_string__,
|
||||||
)
|
)
|
||||||
@@ -395,14 +395,17 @@ async def graph_traceroute(request):
|
|||||||
route = decode_payload.decode_payload(PortNum.TRACEROUTE_APP, tr.route)
|
route = decode_payload.decode_payload(PortNum.TRACEROUTE_APP, tr.route)
|
||||||
if route is None:
|
if route is None:
|
||||||
continue
|
continue
|
||||||
path = [packet.from_node_id]
|
path = traceroute.forward_path(
|
||||||
path.extend(route.route)
|
packet.from_node_id,
|
||||||
|
packet.to_node_id,
|
||||||
|
route.route,
|
||||||
|
tr.done,
|
||||||
|
tr.gateway_node_id,
|
||||||
|
)
|
||||||
|
if not path:
|
||||||
|
continue
|
||||||
if tr.done:
|
if tr.done:
|
||||||
dest = packet.to_node_id
|
dest = path[-1]
|
||||||
path.append(packet.to_node_id)
|
|
||||||
elif path[-1] != tr.gateway_node_id:
|
|
||||||
# It seems some nodes add them self to the list before uplinking
|
|
||||||
path.append(tr.gateway_node_id)
|
|
||||||
|
|
||||||
if not tr.done and tr.gateway_node_id not in node_seen_time and tr.import_time_us:
|
if not tr.done and tr.gateway_node_id not in node_seen_time and tr.import_time_us:
|
||||||
node_seen_time[path[-1]] = tr.import_time_us
|
node_seen_time[path[-1]] = tr.import_time_us
|
||||||
|
|||||||
+22
-17
@@ -10,7 +10,7 @@ from aiohttp import web
|
|||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select
|
||||||
|
|
||||||
from meshtastic.protobuf.portnums_pb2 import PortNum
|
from meshtastic.protobuf.portnums_pb2 import PortNum
|
||||||
from meshview import database, decode_payload, store
|
from meshview import database, decode_payload, store, traceroute
|
||||||
from meshview.__version__ import __version__, _git_revision_short, get_version_info
|
from meshview.__version__ import __version__, _git_revision_short, get_version_info
|
||||||
from meshview.config import CONFIG
|
from meshview.config import CONFIG
|
||||||
from meshview.models import DailySnapshot, Node
|
from meshview.models import DailySnapshot, Node
|
||||||
@@ -630,8 +630,13 @@ async def api_edges(request):
|
|||||||
if route is None or tr.packet is None:
|
if route is None or tr.packet is None:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
path = [tr.packet.from_node_id] + list(route.route)
|
path = traceroute.forward_path(
|
||||||
path.append(tr.packet.to_node_id if tr.done else tr.gateway_node_id)
|
tr.packet.from_node_id,
|
||||||
|
tr.packet.to_node_id,
|
||||||
|
traceroute.strip_unknown_hops(route.route),
|
||||||
|
tr.done,
|
||||||
|
tr.gateway_node_id,
|
||||||
|
)
|
||||||
|
|
||||||
for a, b in zip(path, path[1:], strict=False):
|
for a, b in zip(path, path[1:], strict=False):
|
||||||
if (a, b) not in edges:
|
if (a, b) not in edges:
|
||||||
@@ -989,21 +994,16 @@ async def api_traceroute(request):
|
|||||||
|
|
||||||
from_node_id = packet.from_node_id
|
from_node_id = packet.from_node_id
|
||||||
to_node_id = packet.to_node_id
|
to_node_id = packet.to_node_id
|
||||||
winning_forward_with_endpoints = []
|
# Winning paths are only built from responses, which carry the endpoints reversed.
|
||||||
for path in set(winning_forward_paths):
|
winning_forward_with_endpoints = [
|
||||||
full_path = list(path)
|
traceroute.forward_path(from_node_id, to_node_id, path, done=True)
|
||||||
if to_node_id is not None and (not full_path or full_path[-1] != to_node_id):
|
for path in set(winning_forward_paths)
|
||||||
full_path = [to_node_id, *full_path]
|
]
|
||||||
if from_node_id is not None and (not full_path or full_path[-1] != from_node_id):
|
|
||||||
full_path = [*full_path, from_node_id]
|
|
||||||
winning_forward_with_endpoints.append(full_path)
|
|
||||||
|
|
||||||
winning_reverse_with_endpoints = []
|
winning_reverse_with_endpoints = [
|
||||||
for path in set(winning_reverse_paths):
|
traceroute.return_path(from_node_id, to_node_id, path)
|
||||||
full_path = list(path)
|
for path in set(winning_reverse_paths)
|
||||||
if to_node_id is not None and (not full_path or full_path[0] != to_node_id):
|
]
|
||||||
full_path = [to_node_id, *full_path]
|
|
||||||
winning_reverse_with_endpoints.append(full_path)
|
|
||||||
|
|
||||||
winning_paths_json = {
|
winning_paths_json = {
|
||||||
"forward": winning_forward_with_endpoints,
|
"forward": winning_forward_with_endpoints,
|
||||||
@@ -1013,12 +1013,17 @@ async def api_traceroute(request):
|
|||||||
# --------------------------------------------
|
# --------------------------------------------
|
||||||
# Final API output
|
# Final API output
|
||||||
# --------------------------------------------
|
# --------------------------------------------
|
||||||
|
initiator, target = traceroute.endpoints(
|
||||||
|
from_node_id, to_node_id, any(tr["done"] for tr in tr_groups)
|
||||||
|
)
|
||||||
return web.json_response(
|
return web.json_response(
|
||||||
{
|
{
|
||||||
"packet": {
|
"packet": {
|
||||||
"id": packet.id,
|
"id": packet.id,
|
||||||
"from": packet.from_node_id,
|
"from": packet.from_node_id,
|
||||||
"to": packet.to_node_id,
|
"to": packet.to_node_id,
|
||||||
|
"initiator": initiator,
|
||||||
|
"target": target,
|
||||||
"channel": packet.channel,
|
"channel": packet.channel,
|
||||||
},
|
},
|
||||||
"traceroute_packets": tr_groups,
|
"traceroute_packets": tr_groups,
|
||||||
|
|||||||
Reference in new issue
Block a user