diff --git a/docs/features/marker-traffic-insight.md b/docs/features/marker-traffic-insight.md index 42b16d177..a91091bf4 100644 --- a/docs/features/marker-traffic-insight.md +++ b/docs/features/marker-traffic-insight.md @@ -125,6 +125,28 @@ Because the listener ignores unknown keys, `dir` is **additive**: an older serve drops it and an older uBridge simply omits it — either way the system falls back to undirected rendering with no error. +### Choosing the capture node + +Since `dir` is relative to the capture node, *which* endpoint is the observer decides what +`tx`/`rx` mean. By default the server auto-picks (first started marker-capable endpoint, in +link-endpoint order). To pin it — e.g. so `dir=tx` unambiguously means "vpcs1 is sending" — +pass `capture_node_id` on marker **create**: + +```json +{ "bpf": "icmp", "direction": "tx", "capture_node_id": "" } +``` + +The value must be one of the link's two endpoints and a marker-capable type (`vpcs`, `qemu`, +`docker`, `iou`, `dynamips`, `cloud`); any other id is rejected with `409`. Omit it to keep +the auto-pick. The chosen id is echoed back as `capture_node_id` in the marker entry and in +each `MARK` signal's `node=`, so the Web UI always knows the observer regardless of who +picked it. + +`capture_node_id` is **create-only**: it is fixed once the marker exists (changing the +observer would silently flip the meaning of stored `direction`, so recreate the marker +instead). It is not accepted on project-level definitions — a definition is link-agnostic and +has no endpoints to choose from, so inherited markers always auto-pick per link. + ## API Endpoints All endpoints require a JWT bearer token (`POST /v3/access/users/authenticate`). The @@ -167,12 +189,17 @@ extra request. "name": "icmp", "bpf": "icmp", "tag": 1, + "direction": "tx", + "capture_node_id": "a37e2235-e21f-46c9-a2ab-ba0f8c5465e6", "color": "#ff5722", "highlight_duration": 800, "enabled": true } ``` +`direction` and `capture_node_id` are both optional and create-only (see +[Direction](#direction)). + **Definition create body** (`MarkerDefinitionCreate`, shared by POST and PUT): ```json @@ -224,7 +251,8 @@ extra request. | `enabled` | bool | Whether the marker is active | | `color` | string \| null | Hex color render hint, e.g. `#ff5722` | | `highlight_duration` | int \| null | UI highlight duration in ms after a match; `null` = UI default | -| `capture_node_id` | string | Server-chosen node whose uBridge hosts the marker | +| `direction` | string \| null | `tx` / `rx` filter relative to the capture node; `null` = both | +| `capture_node_id` | string | Node whose uBridge hosts the marker — caller-set on create, else auto-picked | | `inherited_from` | string | Source definition name — present on inherited markers only | ### Definition diff --git a/gns3server/api/routes/controller/links.py b/gns3server/api/routes/controller/links.py index 8d9f105fb..99ff3379c 100644 --- a/gns3server/api/routes/controller/links.py +++ b/gns3server/api/routes/controller/links.py @@ -466,6 +466,7 @@ async def create_marker( bpf=marker_data.bpf, tag=marker_data.tag, direction=marker_data.direction, + capture_node_id=marker_data.capture_node_id, color=marker_data.color, highlight_duration=marker_data.highlight_duration, ) diff --git a/gns3server/api/routes/mcp/__init__.py b/gns3server/api/routes/mcp/__init__.py index e99b9eb2e..4cf389c13 100644 --- a/gns3server/api/routes/mcp/__init__.py +++ b/gns3server/api/routes/mcp/__init__.py @@ -1016,6 +1016,7 @@ async def link_marker( tag: Annotated[int | None, Field(description="Numeric tag for packet correlation")] = None, enabled: Annotated[bool | None, Field(description="Enable or disable the marker (for update action)")] = None, direction: Annotated[str | None, Field(description="Direction filter: 'tx' for capture node sending only, 'rx' for capture node receiving only (omit for both)")] = None, + capture_node_id: Annotated[str | None, Field(description="UUID of the endpoint whose uBridge hosts the marker (the observer; tx/rx are from its perspective). Must be a link endpoint and marker-capable. Omit to auto-pick.")] = None, color: Annotated[str | None, Field(description="Hex color for UI highlight, e.g. '#ff5722'")] = None, highlight_duration: Annotated[int | None, Field(description="UI highlight duration in milliseconds")] = None, ) -> list[dict[str, Any]]: @@ -1034,7 +1035,7 @@ async def link_marker( and cannot be modified or deleted via this tool. """ params = {"project_id": project_id, "link_id": link_id, "action": action} - for opt in ("bpf", "marker_name", "name", "tag", "enabled", "direction", "color", "highlight_duration"): + for opt in ("bpf", "marker_name", "name", "tag", "enabled", "direction", "capture_node_id", "color", "highlight_duration"): val = locals().get(opt) if val is not None: params[opt] = val diff --git a/gns3server/api/routes/mcp/links.py b/gns3server/api/routes/mcp/links.py index c69d392ff..2b854b07f 100644 --- a/gns3server/api/routes/mcp/links.py +++ b/gns3server/api/routes/mcp/links.py @@ -395,7 +395,7 @@ def link_marker_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic if not bpf: return {"error": "bpf is required for create action"} body: dict[str, Any] = {"bpf": bpf} - for opt in ("name", "tag", "direction", "color", "highlight_duration"): + for opt in ("name", "tag", "direction", "capture_node_id", "color", "highlight_duration"): if params.get(opt) is not None: body[opt] = params[opt] return conn.http_call("post", base, json_data=body).json() diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index a766e107c..c75a8c10d 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -334,7 +334,7 @@ class Link: raise NotImplementedError - async def start_marker(self, name, bpf, tag=None, direction=None): + async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None): """ Attach a traffic-insight marker to this link (base — UDPLink overrides). """ diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 146f55de6..5e72831a5 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -310,6 +310,31 @@ class UDPLink(Link): "traffic insight" ) + def _node_by_id(self, node_id): + """ + Resolve a caller-chosen capture node by id, validating it is an + endpoint of this link and marker-capable. Used when the caller + (REST/MCP) explicitly pins the observer side instead of letting + ``_choose_marker_side`` auto-pick. + + :param node_id: node id (UUID or str) the caller requested + :returns: a ``self._nodes`` entry (node/adapter_number/port_number) + """ + + target = str(node_id) + for node in self._nodes: + if str(node["node"].id) != target: + continue + if node["node"].node_type not in _MARKER_CAPABLE_TYPES: + raise ControllerError( + f"Node {node_id} ({node['node'].node_type}) cannot host a " + f"marker — no uBridge bridge to attach the filter to" + ) + return node + raise ControllerNotFoundError( + f"Node {node_id} is not an endpoint of link {self._id}" + ) + async def node_updated(self, node): """ Called when a node member of the link is updated @@ -322,7 +347,7 @@ class UDPLink(Link): # explicitly deletes a marker via the REST API, and a marker is torn # down automatically only when its link is deleted. - async def start_marker(self, name, bpf, tag=None, direction=None, color=None, highlight_duration=None, inherited_from=None): + async def start_marker(self, name, bpf, tag=None, direction=None, capture_node_id=None, color=None, highlight_duration=None, inherited_from=None): """ Attach a traffic-insight marker to this link. @@ -335,6 +360,11 @@ class UDPLink(Link): :param name: stable filter name — echoed in MARK signals + pcap identity :param bpf: libpcap BPF expression :param tag: optional correlation id + :param capture_node_id: optional explicit observer node. When set the + marker is pinned to that endpoint's uBridge (and ``direction`` is + interpreted from its perspective); validated by ``_node_by_id``. + Omitted = auto-pick via ``_choose_marker_side``. Ignored for + inherited markers (project defs are link-agnostic → always auto). :param color: optional hex color for the Web UI (e.g. '#ff5722'); stored with the link and persisted in the topology, never sent to uBridge :param highlight_duration: optional UI-only hint (milliseconds) for how @@ -350,7 +380,10 @@ class UDPLink(Link): if not result.get("valid"): raise ControllerError(f"Invalid BPF expression: {result.get('error', 'unknown error')}") - marker_side = self._choose_marker_side() + if capture_node_id and not inherited_from: + marker_side = self._node_by_id(capture_node_id) + else: + marker_side = self._choose_marker_side() marker_entry = { "bpf": bpf, "tag": tag, diff --git a/gns3server/schemas/controller/links.py b/gns3server/schemas/controller/links.py index 44f8ac1aa..3ec06c5d5 100644 --- a/gns3server/schemas/controller/links.py +++ b/gns3server/schemas/controller/links.py @@ -180,6 +180,15 @@ class MarkerCreate(BaseModel): pattern=r"^(tx|rx)$", description="Direction filter: 'tx' = capture node sending only, 'rx' = capture node receiving only. Omitted or null = both directions.", ) + capture_node_id: Optional[UUID] = Field( + None, + description=( + "Which endpoint's uBridge hosts this marker (the 'observer'). " + "tx/rx in `direction` are interpreted from this node's perspective. " + "Must be one of the link's two endpoints and a marker-capable type. " + "Omitted = server auto-picks (first started marker-capable endpoint)." + ), + ) class MarkerDefinitionCreate(BaseModel): diff --git a/tests/controller/test_marker.py b/tests/controller/test_marker.py index 7dc0ebe13..6348a7a48 100644 --- a/tests/controller/test_marker.py +++ b/tests/controller/test_marker.py @@ -98,6 +98,40 @@ async def test_start_marker_stores_entry(project): assert "inherited_from" not in entry +@pytest.mark.asyncio +async def test_start_marker_pins_capture_node(project): + # Auto-pick would choose node1 (first endpoint); pin to node2 explicitly. + with _valid_bpf(): + link = await _make_link(project) + chosen = link._nodes[1]["node"].id + auto = link._nodes[0]["node"].id + assert chosen != auto # sanity: the pin must actually mean something + await link.start_marker("icmp", "icmp", capture_node_id=chosen) + + assert link.markers["icmp"]["capture_node_id"] == chosen + + +@pytest.mark.asyncio +async def test_start_marker_rejects_non_endpoint_capture_node(project): + + with _valid_bpf(): + link = await _make_link(project) + with pytest.raises(ControllerNotFoundError): + await link.start_marker("icmp", "icmp", capture_node_id="11111111-2222-3333-4444-555555555555") + + +@pytest.mark.asyncio +async def test_start_marker_capture_node_ignored_for_inherited(project): + # Definitions are link-agnostic: an inherited marker must auto-pick even + # if a capture_node_id leaks through, never trusting the caller's pin. + with _valid_bpf(): + link = await _make_link(project) + leaked = link._nodes[1]["node"].id + await link.start_marker("m", "icmp", capture_node_id=leaked, inherited_from="arp") + + assert link.markers["m"]["capture_node_id"] == link._nodes[0]["node"].id + + @pytest.mark.asyncio async def test_start_marker_rejects_duplicate(project):